Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling of multiple declaration/meanings of the same interrupt vector #20

Merged
merged 1 commit into from
Nov 25, 2020
Merged

Handling of multiple declaration/meanings of the same interrupt vector #20

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/atdf/chip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use crate::atdf;
use crate::chip;
use crate::ElementExt;

use std::collections::BTreeMap;

pub fn parse(el: &xmltree::Element) -> crate::Result<chip::Chip> {
let devices = el.first_child("devices")?;
if devices.children.len() != 1 {
Expand All @@ -20,7 +22,7 @@ pub fn parse(el: &xmltree::Element) -> crate::Result<chip::Chip> {
.map(|p| (p.name.clone(), p))
.collect();

let interrupts = device
let interrupts_vec = device
.first_child("interrupts")?
.children
.iter()
Expand All @@ -31,8 +33,35 @@ pub fn parse(el: &xmltree::Element) -> crate::Result<chip::Chip> {
})
.filter(|e| e.name == "interrupt")
.map(atdf::interrupt::parse)
.map(|r| r.map(|int| (int.name.clone(), int)))
.collect::<Result<_, _>>()?;
.collect::<Result<Vec<_>, _>>()?;

// Check for duplicate index, merge names if duplicate index exists
let mut interrupts = BTreeMap::<usize, chip::Interrupt>::new();
for int in interrupts_vec {
if let Some(existing_int) = interrupts.get_mut(&int.index) {
let old_name = existing_int.name.clone();
if let Some(split_idx) = int.name.find('_') {
existing_int.name.push_str(int.name.split_at(split_idx).1);
} else {
existing_int.name.push('_');
existing_int.name.push_str(&int.name);
}
Comment on lines +43 to +48
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say that I am a big fan of this ad-hoc string manipulation because it's very fragile ... But I cannot think of a different solution that does not require bigger changes, so I guess it is fine. We won't see this 'duplicate interrupt' situation too often anyway, I hope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for me. The other option would have been adding a "module-instance" field to the interrupt struct, but than serialization would not work (or needs to exclude the module-instance). This would give a much simpler string manipulation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw now that you do the serialization manually, so actually then an additional field in the interrupt struct would not change anything?

Then we could add a field module-instance as Option<std::string::String>, and in chip.rs we could append name with module-instance if it exists, and append also the other module-instance in case of duplicate index number.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change would then combine #19 with #20.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it like it is for now and only fix it if it breaks for someone ;) No point wasting too much effort on this rare issue ...

log::warn!(
"Merging interrupt {} and {} to {}",
old_name,
int.name,
existing_int.name
);
} else {
interrupts.insert(int.index, int);
}
}

// Map interrupts from <usize, chip::Interrupt> to <std::string::String, chip::Interrupt>
let interrupts = interrupts
.iter()
.map(|(_, int)| (int.name.clone(), int.clone()))
.collect();

Ok(chip::Chip {
name: device.attr("name")?.clone(),
Expand Down