From f4b2dc00287ddabb5ce0eada34b25c4c4a9f181a Mon Sep 17 00:00:00 2001 From: hendrik Date: Tue, 29 Apr 2025 10:08:37 +0200 Subject: [PATCH 1/6] implement union peripherals with clustering (TCA single mode / split mode on attiny's) --- src/atdf/peripheral.rs | 42 +- src/atdf/register.rs | 147 +++ src/chip.rs | 69 ++ src/svd/chip.rs | 6 + src/svd/peripheral.rs | 68 +- tests/snapshots/regression__atmega4809.snap | 1145 +++++++++++++++++++ 6 files changed, 1435 insertions(+), 42 deletions(-) diff --git a/src/atdf/peripheral.rs b/src/atdf/peripheral.rs index b5ff771..40df477 100644 --- a/src/atdf/peripheral.rs +++ b/src/atdf/peripheral.rs @@ -13,8 +13,6 @@ pub fn parse_list( let module_name = module.attr("name")?; for instance in module.iter_children_with_name("instance", Some("module")) { - let mut registers = vec![]; - // Find corresponding module let module = modules.first_child_by_attr(Some("module"), "name", module_name)?; @@ -22,36 +20,21 @@ pub fn parse_list( // level as the register-groups, so we parse them in here first. let value_groups = atdf::values::parse_value_groups(module)?; - for register_group in instance - .children - .iter() - .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) - { - let name = register_group.attr("name-in-module")?; - let offset = util::parse_int(register_group.attr("offset")?)?; - - let group = module.first_child_by_attr(Some("register-group"), "name", name)?; - - for register in group.iter_children_with_name("register", Some("register-group")) { - registers.push(atdf::register::parse(register, offset, &value_groups)?); - } - } - - let registers = registers - .into_iter() - .map(|r| { - ( - match r.mode { - Some(ref mode) => format!("{mode}_{}", r.name), - _ => r.name.clone(), - }, - r, - ) - }) - .collect(); + // An instance should always have one register-group + let instance_register_group = match instance.first_child("register-group") { + Ok(rg) => rg, + Err(_) => continue, + }; + let name_in_module = instance_register_group.attr("name-in-module")?; + let offset = util::parse_int(instance_register_group.attr("offset")?)?; + let register_group_headers = + atdf::register::parse_register_group_headers(module, offset, &value_groups)?; + let main_register_group = register_group_headers.get(name_in_module).cloned().unwrap(); + let registers = main_register_group.registers; peripherals.push(chip::Peripheral { name: instance.attr("name")?.clone(), + name_in_module: name_in_module.clone(), description: instance .attr("caption") .or(module.attr("caption")) @@ -59,6 +42,7 @@ pub fn parse_list( .cloned() .and_then(|d| if !d.is_empty() { Some(d) } else { None }), registers, + register_group_headers: register_group_headers.clone(), }) } } diff --git a/src/atdf/register.rs b/src/atdf/register.rs index df1a819..9423be6 100644 --- a/src/atdf/register.rs +++ b/src/atdf/register.rs @@ -84,3 +84,150 @@ pub fn parse( fields, }) } + +pub fn parse_list( + register_group_header_el: &xmltree::Element, + offset: usize, + value_groups: &atdf::values::ValueGroups, +) -> crate::Result> { + let mut registers = vec![]; + + for register in + register_group_header_el.iter_children_with_name("register", Some("register-group")) + { + registers.push(atdf::register::parse(register, offset, value_groups)?); + } + Ok(registers + .into_iter() + .map(|r| { + ( + match r.mode { + Some(ref mode) => format!("{mode}_{}", r.name), + _ => r.name.clone(), + }, + r, + ) + }) + .collect()) +} + +pub fn parse_register_group_headers( + module_el: &xmltree::Element, + offset: usize, + value_groups: &atdf::values::ValueGroups, +) -> crate::Result> { + let mut register_group_headers = BTreeMap::new(); + for register_group_header_el in module_el + .children + .iter() + .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) + { + let name = register_group_header_el.attr("name")?.clone(); + let class = register_group_header_el + .attributes + .get("class") + .and_then(|d| if !d.is_empty() { Some(d) } else { None }) + .cloned(); + let description = register_group_header_el + .attributes + .get("caption") + .and_then(|d| if !d.is_empty() { Some(d) } else { None }) + .cloned(); + + let size = register_group_header_el + .attributes + .get("size") + .and_then(|d| { + if !d.is_empty() { + util::parse_int(d).ok() + } else { + None + } + }); + + let register_group_items = parse_register_group_items(register_group_header_el)?; + let registers = parse_list(register_group_header_el, offset, value_groups)?; + + register_group_headers.insert( + name.clone(), + chip::RegisterGroupHeader { + name, + class, + description, + size, + register_group_items, + registers, + }, + ); + } + Ok(register_group_headers) +} + +pub fn parse_register_group_items( + register_group_header_el: &xmltree::Element, +) -> crate::Result> { + let mut register_group_items = BTreeMap::new(); + + for register_group_item_el in register_group_header_el + .children + .iter() + .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) + { + let name = register_group_item_el.attr("name")?.clone(); + let description = register_group_item_el + .attributes + .get("caption") + .and_then(|d| if !d.is_empty() { Some(d) } else { None }) + .cloned(); + + let name_in_module = register_group_item_el + .attributes + .get("name-in-module") + .and_then(|d| if !d.is_empty() { Some(d) } else { None }) + .cloned(); + + let size = register_group_item_el.attributes.get("size").and_then(|d| { + if !d.is_empty() { + util::parse_int(d).ok() + } else { + None + } + }); + + let offset = register_group_item_el + .attributes + .get("offset") + .and_then(|d| { + if !d.is_empty() { + util::parse_int(d).ok() + } else { + None + } + }); + + let count = register_group_item_el + .attributes + .get("count") + .and_then(|d| { + if !d.is_empty() { + util::parse_int(d).ok() + } else { + None + } + }); + + register_group_items.insert( + name.clone(), + chip::RegisterGroupItem { + name, + name_in_module, + description, + size, + offset, + count, + }, + ); + } + + Ok(register_group_items) +} diff --git a/src/chip.rs b/src/chip.rs index d782edf..22b8dcb 100644 --- a/src/chip.rs +++ b/src/chip.rs @@ -18,15 +18,57 @@ pub struct Chip { #[derive(Debug, Clone)] pub struct Peripheral { pub name: String, + pub name_in_module: String, pub description: Option, pub registers: BTreeMap, + pub register_group_headers: BTreeMap, } impl Peripheral { pub fn base_address(&self) -> Option { + if self.is_union() { + return self + .get_union_register_group_headers() + .iter() + .flat_map(|(h, _)| h.registers.values()) + .map(|r| r.address) + .min(); + } self.registers.values().map(|r| r.address).min() } + + pub fn module_register_group_header(&self) -> Option<&RegisterGroupHeader> { + self.register_group_headers.get(&self.name_in_module) + } + + pub fn is_union(&self) -> bool { + let module_register_group_header = self.module_register_group_header(); + match module_register_group_header { + Some(header) => header.is_union(), + None => false, + } + } + + pub fn get_union_register_group_headers( + &self, + ) -> Vec<(&RegisterGroupHeader, &RegisterGroupItem)> { + match self.module_register_group_header() { + Some(module_header) if module_header.is_union() => module_header + .register_group_items + .values() + .filter_map(|group_item| { + group_item.name_in_module.as_ref().and_then(|header_name| { + self.register_group_headers + .iter() + .find(|(_, header)| &header.name == header_name) + .map(|(_, header)| (header, group_item)) + }) + }) + .collect(), + _ => vec![], + } + } } #[derive(Debug, Clone)] @@ -52,6 +94,33 @@ pub enum ValueRestriction { Enumerated(BTreeMap), } +#[derive(Debug, Clone)] +pub struct RegisterGroupHeader { + pub name: String, + pub class: Option, + pub description: Option, + pub size: Option, + + pub register_group_items: BTreeMap, + pub registers: BTreeMap, +} + +impl RegisterGroupHeader { + pub fn is_union(&self) -> bool { + self.class.as_deref() == Some("union") + } +} + +#[derive(Debug, Clone)] +pub struct RegisterGroupItem { + pub name: String, + pub name_in_module: Option, + pub description: Option, + pub size: Option, + pub offset: Option, + pub count: Option, +} + #[derive(Debug, Clone)] pub struct Register { pub name: String, diff --git a/src/svd/chip.rs b/src/svd/chip.rs index 5bbd5f2..a4c1a88 100644 --- a/src/svd/chip.rs +++ b/src/svd/chip.rs @@ -45,6 +45,12 @@ pub fn generate(c: &chip::Chip) -> crate::Result { } fn has_registers(peripheral: &&chip::Peripheral) -> bool { + if peripheral.is_union() { + return peripheral + .get_union_register_group_headers() + .iter() + .any(|(header, _)| !header.registers.values().len() > 0) + } let regs = !peripheral.registers.is_empty(); if !regs { log::warn!("No registers found for peripheral {}", peripheral.name); diff --git a/src/svd/peripheral.rs b/src/svd/peripheral.rs index 241c4f5..6ffe479 100644 --- a/src/svd/peripheral.rs +++ b/src/svd/peripheral.rs @@ -1,16 +1,27 @@ +use svd_rs::MaybeArray; + use crate::chip; use crate::svd; use std::convert::TryInto; -fn create_address_blocks(p: &chip::Peripheral) -> crate::Result>> { - let mut registers: Vec<_> = p.registers.values().collect(); +fn create_address_blocks( + p: &chip::Peripheral, + base: usize, +) -> crate::Result>> { + let mut registers: Vec<_> = match p.is_union() { + true => { + let &(header, _) = p + .get_union_register_group_headers() + .first() + .expect("No union header found"); + header.registers.values().collect() + } + false => p.registers.values().collect(), + }; registers.sort_by(|a, b| a.address.cmp(&b.address)); - let base = p.base_address().expect("no base address"); - let new_address_block = |offset, size| { - let offset = (offset as usize - base) - .try_into() - .map_err(crate::Error::from)?; + let new_address_block = |offset: usize, size| { + let offset = (offset - base).try_into().map_err(crate::Error::from)?; svd_rs::AddressBlock::builder() .offset(offset) @@ -51,11 +62,42 @@ pub fn generate(p: &chip::Peripheral) -> crate::Result { .expect("Could not retrieve peripheral base address") .try_into()?; - let registers = p - .registers - .values() - .map(|r| svd::register::generate(r, base).map(svd_rs::RegisterCluster::Register)) - .collect::, _>>()?; + let registers: Vec = match p.is_union() { + true => { + println!("cargo:warning=svd generate union style {}", p.name); + p.get_union_register_group_headers() + .iter() + .map(|(header, item)| { + let cluster = svd_rs::ClusterInfo::builder() + .name(header.name.clone()) + .description(header.description.clone()) + .address_offset(item.offset.unwrap_or(0) as u32) + .children( + header + .registers + .values() + .map(|r| { + svd::register::generate(r, base) + .map(svd_rs::RegisterCluster::Register) + }) + .collect::, _>>()?, + ); + + cluster + .build(svd_rs::ValidateLevel::Strict) + .map(|cluster_info| { + svd_rs::RegisterCluster::Cluster(MaybeArray::Single(cluster_info)) + }) + .map_err(crate::Error::from) + }) + .collect::, _>>()? + } + false => p + .registers + .values() + .map(|r| svd::register::generate(r, base).map(svd_rs::RegisterCluster::Register)) + .collect::, _>>()?, + }; svd_rs::PeripheralInfo::builder() .name(p.name.clone()) @@ -64,7 +106,7 @@ pub fn generate(p: &chip::Peripheral) -> crate::Result { Some("No Description.".to_owned()) })) .base_address(u64::from(base)) - .address_block(create_address_blocks(p)?) + .address_block(create_address_blocks(p, base as usize)?) .registers(Some(registers)) .build(svd_rs::ValidateLevel::Strict) .map(svd_rs::Peripheral::Single) diff --git a/tests/snapshots/regression__atmega4809.snap b/tests/snapshots/regression__atmega4809.snap index 934101c..4f37e98 100644 --- a/tests/snapshots/regression__atmega4809.snap +++ b/tests/snapshots/regression__atmega4809.snap @@ -15774,6 +15774,1151 @@ expression: svd + + TCA0 + 16-bit Timer/Counter Type A + 0x00000A00 + + 0x0 + 0x8 + registers + + + 0x9 + 0x3 + registers + + + 0xE + 0x2 + registers + + + 0x20 + 0x2 + registers + + + 0x26 + 0x8 + registers + + + 0x36 + 0x8 + registers + + + + TCA_SINGLE + 16-bit Timer/Counter Type A - Single Mode + 0x0 + + CMP0 + Compare 0 + 0x28 + 0x10 + read-write + + + 0 + 65535 + + + + + CMP0BUF + Compare 0 Buffer + 0x38 + 0x10 + read-write + + + 0 + 65535 + + + + + CMP1 + Compare 1 + 0x2A + 0x10 + read-write + + + 0 + 65535 + + + + + CMP1BUF + Compare 1 Buffer + 0x3A + 0x10 + read-write + + + 0 + 65535 + + + + + CMP2 + Compare 2 + 0x2C + 0x10 + read-write + + + 0 + 65535 + + + + + CMP2BUF + Compare 2 Buffer + 0x3C + 0x10 + read-write + + + 0 + 65535 + + + + + CNT + Count + 0x20 + 0x10 + read-write + + + 0 + 65535 + + + + + CTRLA + Control A + 0x0 + 0x8 + read-write + + + ENABLE + Module Enable + [0:0] + read-write + + + CLKSEL + Clock Selection + [3:1] + read-write + + true + + + + DIV1 + System Clock + 0 + + + DIV2 + System Clock / 2 + 1 + + + DIV4 + System Clock / 4 + 2 + + + DIV8 + System Clock / 8 + 3 + + + DIV16 + System Clock / 16 + 4 + + + DIV64 + System Clock / 64 + 5 + + + DIV256 + System Clock / 256 + 6 + + + DIV1024 + System Clock / 1024 + 7 + + + + + + + CTRLB + Control B + 0x1 + 0x8 + read-write + + + WGMODE + Waveform generation mode + [2:0] + read-write + + true + + + + NORMAL + Normal Mode + 0 + + + FRQ + Frequency Generation Mode + 1 + + + SINGLESLOPE + Single Slope PWM + 3 + + + DSTOP + Dual Slope PWM, overflow on TOP + 5 + + + DSBOTH + Dual Slope PWM, overflow on TOP and BOTTOM + 6 + + + DSBOTTOM + Dual Slope PWM, overflow on BOTTOM + 7 + + + + + ALUPD + Auto Lock Update + [3:3] + read-write + + + CMP0EN + Compare 0 Enable + [4:4] + read-write + + + CMP1EN + Compare 1 Enable + [5:5] + read-write + + + CMP2EN + Compare 2 Enable + [6:6] + read-write + + + + + CTRLC + Control C + 0x2 + 0x8 + read-write + + + CMP0OV + Compare 0 Waveform Output Value + [0:0] + read-write + + + CMP1OV + Compare 1 Waveform Output Value + [1:1] + read-write + + + CMP2OV + Compare 2 Waveform Output Value + [2:2] + read-write + + + + + CTRLD + Control D + 0x3 + 0x8 + read-write + + + SPLITM + Split Mode Enable + [0:0] + read-write + + + + + CTRLECLR + Control E Clear + 0x4 + 0x8 + read-write + + + DIR + Direction + [0:0] + read-write + + + LUPD + Lock Update + [1:1] + read-write + + + CMD + Command + [3:2] + read-write + + true + + + + NONE + No Command + 0 + + + UPDATE + Force Update + 1 + + + RESTART + Force Restart + 2 + + + RESET + Force Hard Reset + 3 + + + + + + + CTRLESET + Control E Set + 0x5 + 0x8 + read-write + + + DIR + Direction + [0:0] + read-write + + true + + + + UP + Count up + 0 + + + DOWN + Count down + 1 + + + + + LUPD + Lock Update + [1:1] + read-write + + + CMD + Command + [3:2] + read-write + + true + + + + NONE + No Command + 0 + + + UPDATE + Force Update + 1 + + + RESTART + Force Restart + 2 + + + RESET + Force Hard Reset + 3 + + + + + + + CTRLFCLR + Control F Clear + 0x6 + 0x8 + read-write + + + PERBV + Period Buffer Valid + [0:0] + read-write + + + CMP0BV + Compare 0 Buffer Valid + [1:1] + read-write + + + CMP1BV + Compare 1 Buffer Valid + [2:2] + read-write + + + CMP2BV + Compare 2 Buffer Valid + [3:3] + read-write + + + + + CTRLFSET + Control F Set + 0x7 + 0x8 + read-write + + + PERBV + Period Buffer Valid + [0:0] + read-write + + + CMP0BV + Compare 0 Buffer Valid + [1:1] + read-write + + + CMP1BV + Compare 1 Buffer Valid + [2:2] + read-write + + + CMP2BV + Compare 2 Buffer Valid + [3:3] + read-write + + + + + DBGCTRL + Degbug Control + 0xE + 0x8 + read-write + + + DBGRUN + Debug Run + [0:0] + read-write + + + + + EVCTRL + Event Control + 0x9 + 0x8 + read-write + + + CNTEI + Count on Event Input + [0:0] + read-write + + + EVACT + Event Action + [2:1] + read-write + + true + + + + POSEDGE + Count on positive edge event + 0 + + + ANYEDGE + Count on any edge event + 1 + + + HIGHLVL + Count on prescaled clock while event line is 1. + 2 + + + UPDOWN + Count on prescaled clock. Event controls count direction. Up-count when event line is 0, down-count when event line is 1. + 3 + + + + + + + INTCTRL + Interrupt Control + 0xA + 0x8 + read-write + + + OVF + Overflow Interrupt + [0:0] + read-write + + + CMP0 + Compare 0 Interrupt + [4:4] + read-write + + + CMP1 + Compare 1 Interrupt + [5:5] + read-write + + + CMP2 + Compare 2 Interrupt + [6:6] + read-write + + + + + INTFLAGS + Interrupt Flags + 0xB + 0x8 + read-write + + + OVF + Overflow Interrupt + [0:0] + read-write + + + CMP0 + Compare 0 Interrupt + [4:4] + read-write + + + CMP1 + Compare 1 Interrupt + [5:5] + read-write + + + CMP2 + Compare 2 Interrupt + [6:6] + read-write + + + + + PER + Period + 0x26 + 0x10 + read-write + + + 0 + 65535 + + + + + PERBUF + Period Buffer + 0x36 + 0x10 + read-write + + + 0 + 65535 + + + + + TEMP + Temporary data for 16-bit Access + 0xF + 0x8 + read-write + + + 0 + 255 + + + + + + TCA_SPLIT + 16-bit Timer/Counter Type A - Split Mode + 0x0 + + CTRLA + Control A + 0x0 + 0x8 + read-write + + + ENABLE + Module Enable + [0:0] + read-write + + + CLKSEL + Clock Selection + [3:1] + read-write + + true + + + + DIV1 + System Clock + 0 + + + DIV2 + System Clock / 2 + 1 + + + DIV4 + System Clock / 4 + 2 + + + DIV8 + System Clock / 8 + 3 + + + DIV16 + System Clock / 16 + 4 + + + DIV64 + System Clock / 64 + 5 + + + DIV256 + System Clock / 256 + 6 + + + DIV1024 + System Clock / 1024 + 7 + + + + + + + CTRLB + Control B + 0x1 + 0x8 + read-write + + + LCMP0EN + Low Compare 0 Enable + [0:0] + read-write + + + LCMP1EN + Low Compare 1 Enable + [1:1] + read-write + + + LCMP2EN + Low Compare 2 Enable + [2:2] + read-write + + + HCMP0EN + High Compare 0 Enable + [4:4] + read-write + + + HCMP1EN + High Compare 1 Enable + [5:5] + read-write + + + HCMP2EN + High Compare 2 Enable + [6:6] + read-write + + + + + CTRLC + Control C + 0x2 + 0x8 + read-write + + + LCMP0OV + Low Compare 0 Output Value + [0:0] + read-write + + + LCMP1OV + Low Compare 1 Output Value + [1:1] + read-write + + + LCMP2OV + Low Compare 2 Output Value + [2:2] + read-write + + + HCMP0OV + High Compare 0 Output Value + [4:4] + read-write + + + HCMP1OV + High Compare 1 Output Value + [5:5] + read-write + + + HCMP2OV + High Compare 2 Output Value + [6:6] + read-write + + + + + CTRLD + Control D + 0x3 + 0x8 + read-write + + + SPLITM + Split Mode Enable + [0:0] + read-write + + + + + CTRLECLR + Control E Clear + 0x4 + 0x8 + read-write + + + CMD + Command + [3:2] + read-write + + true + + + + NONE + No Command + 0 + + + UPDATE + Force Update + 1 + + + RESTART + Force Restart + 2 + + + RESET + Force Hard Reset + 3 + + + + + + + CTRLESET + Control E Set + 0x5 + 0x8 + read-write + + + CMD + Command + [3:2] + read-write + + true + + + + NONE + No Command + 0 + + + UPDATE + Force Update + 1 + + + RESTART + Force Restart + 2 + + + RESET + Force Hard Reset + 3 + + + + + + + DBGCTRL + Degbug Control + 0xE + 0x8 + read-write + + + DBGRUN + Debug Run + [0:0] + read-write + + + + + HCMP0 + High Compare + 0x29 + 0x8 + read-write + + + 0 + 255 + + + + + HCMP1 + High Compare + 0x2B + 0x8 + read-write + + + 0 + 255 + + + + + HCMP2 + High Compare + 0x2D + 0x8 + read-write + + + 0 + 255 + + + + + HCNT + High Count + 0x21 + 0x8 + read-write + + + 0 + 255 + + + + + HPER + High Period + 0x27 + 0x8 + read-write + + + 0 + 255 + + + + + INTCTRL + Interrupt Control + 0xA + 0x8 + read-write + + + LUNF + Low Underflow Interrupt Enable + [0:0] + read-write + + + HUNF + High Underflow Interrupt Enable + [1:1] + read-write + + + LCMP0 + Low Compare 0 Interrupt Enable + [4:4] + read-write + + + LCMP1 + Low Compare 1 Interrupt Enable + [5:5] + read-write + + + LCMP2 + Low Compare 2 Interrupt Enable + [6:6] + read-write + + + + + INTFLAGS + Interrupt Flags + 0xB + 0x8 + read-write + + + LUNF + Low Underflow Interrupt Flag + [0:0] + read-write + + + HUNF + High Underflow Interrupt Flag + [1:1] + read-write + + + LCMP0 + Low Compare 2 Interrupt Flag + [4:4] + read-write + + + LCMP1 + Low Compare 1 Interrupt Flag + [5:5] + read-write + + + LCMP2 + Low Compare 0 Interrupt Flag + [6:6] + read-write + + + + + LCMP0 + Low Compare + 0x28 + 0x8 + read-write + + + 0 + 255 + + + + + LCMP1 + Low Compare + 0x2A + 0x8 + read-write + + + 0 + 255 + + + + + LCMP2 + Low Compare + 0x2C + 0x8 + read-write + + + 0 + 255 + + + + + LCNT + Low Count + 0x20 + 0x8 + read-write + + + 0 + 255 + + + + + LPER + Low Period + 0x26 + 0x8 + read-write + + + 0 + 255 + + + + + + TCB0 16-bit Timer Type B From f9b07a5a6d67af4d5e48ba6386cd034414641194 Mon Sep 17 00:00:00 2001 From: hendrik Date: Tue, 29 Apr 2025 14:12:15 +0200 Subject: [PATCH 2/6] run cargo fmt --- src/svd/chip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/svd/chip.rs b/src/svd/chip.rs index a4c1a88..4d5d2cc 100644 --- a/src/svd/chip.rs +++ b/src/svd/chip.rs @@ -49,7 +49,7 @@ fn has_registers(peripheral: &&chip::Peripheral) -> bool { return peripheral .get_union_register_group_headers() .iter() - .any(|(header, _)| !header.registers.values().len() > 0) + .any(|(header, _)| !header.registers.values().len() > 0); } let regs = !peripheral.registers.is_empty(); if !regs { From 035344ce5d2f1441fcaf1797598b33310aa5c6a8 Mon Sep 17 00:00:00 2001 From: hammerlink Date: Thu, 1 May 2025 07:48:32 +0200 Subject: [PATCH 3/6] Implement register group restructure + recursive implementation (#1) * WIP - improve the register group structure and update the parsing accordingly * fix the dynamic address calculation * remove unused base_address fn * rename parse_list args & run cargo fmt --- src/atdf/error.rs | 23 ++++++ src/atdf/mod.rs | 1 + src/atdf/patch.rs | 17 ++-- src/atdf/peripheral.rs | 19 +++-- src/atdf/register.rs | 157 ++++--------------------------------- src/atdf/register_group.rs | 120 ++++++++++++++++++++++++++++ src/chip.rs | 92 +++++++++------------- src/svd/chip.rs | 8 +- src/svd/peripheral.rs | 117 +++++++++++++-------------- src/svd/register.rs | 4 +- 10 files changed, 279 insertions(+), 279 deletions(-) create mode 100644 src/atdf/register_group.rs diff --git a/src/atdf/error.rs b/src/atdf/error.rs index d9abcee..c8bda30 100644 --- a/src/atdf/error.rs +++ b/src/atdf/error.rs @@ -19,3 +19,26 @@ impl UnsupportedError { UnsupportedError(what.into(), el.debug()) } } + +pub struct RecursiveRegisterGroupError { + group_name: String, +} + +impl crate::DisplayError for RecursiveRegisterGroupError { + fn format(&self, w: &mut dyn std::io::Write) -> std::io::Result<()> { + write!( + w, + "{}: Recursive register group reference detected leading to {}", + "Error".red().bold(), + self.group_name.dimmed() + ) + } +} + +impl RecursiveRegisterGroupError { + pub fn new>(group_name: S) -> Self { + let group_name = group_name.into(); + + RecursiveRegisterGroupError { group_name } + } +} diff --git a/src/atdf/mod.rs b/src/atdf/mod.rs index ddc330d..63c2ec9 100644 --- a/src/atdf/mod.rs +++ b/src/atdf/mod.rs @@ -7,6 +7,7 @@ pub mod interrupt; pub mod patch; pub mod peripheral; pub mod register; +pub mod register_group; pub mod values; pub fn parse( diff --git a/src/atdf/patch.rs b/src/atdf/patch.rs index d97270f..cb73f3b 100644 --- a/src/atdf/patch.rs +++ b/src/atdf/patch.rs @@ -44,7 +44,7 @@ pub fn signals_to_port_fields(chip: &mut chip::Chip, tree: &xmltree::Element) -> .map(|f| (f.name.clone(), f)) .collect(); - for reg in port.registers.values_mut() { + for reg in port.register_group.registers.values_mut() { if reg.name.ends_with(name) || NEW_PORT_REGS.iter().any(|r| r == ®.name) { reg.fields = fields.clone(); // Ensure that direct access to the register is unsafe @@ -57,8 +57,8 @@ pub fn signals_to_port_fields(chip: &mut chip::Chip, tree: &xmltree::Element) -> pub fn remove_unsafe_cpu_regs(chip: &mut chip::Chip, _el: &xmltree::Element) -> crate::Result<()> { if let Some(cpu) = chip.peripherals.get_mut("CPU") { - cpu.registers.remove("SREG"); - cpu.registers.remove("SP"); + cpu.register_group.registers.remove("SREG"); + cpu.register_group.registers.remove("SP"); } Ok(()) @@ -88,16 +88,21 @@ pub fn remove_register_common_prefix(chip: &mut chip::Chip) -> crate::Result<()> for peripheral in chip.peripherals.values_mut() { // There's not enough quorum in less than two elements to find a // prefix. - if peripheral.registers.len() < 2 { + if peripheral.register_group.registers.len() < 2 { continue; } - let register_names: Vec<_> = peripheral.registers.keys().map(String::as_str).collect(); + let register_names: Vec<_> = peripheral + .register_group + .registers + .keys() + .map(String::as_str) + .collect(); let common_prefix = longest_common_prefix(®ister_names).to_string(); let is_valid_prefix = common_prefix.ends_with("_") && common_prefix.chars().count() >= 2; if is_valid_prefix { - for register in peripheral.registers.values_mut() { + for register in peripheral.register_group.registers.values_mut() { if let Some(s) = register.name.strip_prefix(&common_prefix) { log::debug!( "[remove_register_common_prefix] Renaming {} to {}", diff --git a/src/atdf/peripheral.rs b/src/atdf/peripheral.rs index 40df477..52dbff4 100644 --- a/src/atdf/peripheral.rs +++ b/src/atdf/peripheral.rs @@ -26,11 +26,16 @@ pub fn parse_list( Err(_) => continue, }; let name_in_module = instance_register_group.attr("name-in-module")?; - let offset = util::parse_int(instance_register_group.attr("offset")?)?; - let register_group_headers = - atdf::register::parse_register_group_headers(module, offset, &value_groups)?; - let main_register_group = register_group_headers.get(name_in_module).cloned().unwrap(); - let registers = main_register_group.registers; + let address = util::parse_int(instance_register_group.attr("offset")?)?; + + let mut register_groups = atdf::register_group::parse_list(module, 0, &value_groups)?; + let mut main_register_group = register_groups.get(name_in_module).cloned().unwrap(); + atdf::register_group::build_register_group_hierarchy( + &mut main_register_group, + &mut register_groups, + address, + 0, + )?; peripherals.push(chip::Peripheral { name: instance.attr("name")?.clone(), @@ -41,8 +46,8 @@ pub fn parse_list( .ok() .cloned() .and_then(|d| if !d.is_empty() { Some(d) } else { None }), - registers, - register_group_headers: register_group_headers.clone(), + address, + register_group: main_register_group, }) } } diff --git a/src/atdf/register.rs b/src/atdf/register.rs index 9423be6..e334266 100644 --- a/src/atdf/register.rs +++ b/src/atdf/register.rs @@ -19,7 +19,7 @@ fn field_map_from_bitfield_children( pub fn parse( el: &xmltree::Element, - offset: usize, + address: usize, values: &atdf::values::ValueGroups, ) -> crate::Result { let name = el.attr("name")?.clone(); @@ -69,11 +69,14 @@ pub fn parse( crate::Result::Ok(()) })?; + let offset = util::parse_int(el.attr("offset")?)?; + Ok(chip::Register { name, description, mode, - address: util::parse_int(el.attr("offset")?)? + offset, + address: address + offset, + offset, size: util::parse_int(el.attr("size")?)?, access, restriction: if fields.is_empty() { @@ -86,148 +89,20 @@ pub fn parse( } pub fn parse_list( - register_group_header_el: &xmltree::Element, + register_group_el: &xmltree::Element, offset: usize, value_groups: &atdf::values::ValueGroups, ) -> crate::Result> { - let mut registers = vec![]; - - for register in - register_group_header_el.iter_children_with_name("register", Some("register-group")) - { - registers.push(atdf::register::parse(register, offset, value_groups)?); - } - Ok(registers - .into_iter() - .map(|r| { - ( - match r.mode { + register_group_el + .iter_children_with_name("register", Some("register-group")) + .map(|reg| { + atdf::register::parse(reg, offset, value_groups).map(|r| { + let key = match r.mode { Some(ref mode) => format!("{mode}_{}", r.name), - _ => r.name.clone(), - }, - r, - ) + None => r.name.clone(), + }; + (key, r) + }) }) - .collect()) -} - -pub fn parse_register_group_headers( - module_el: &xmltree::Element, - offset: usize, - value_groups: &atdf::values::ValueGroups, -) -> crate::Result> { - let mut register_group_headers = BTreeMap::new(); - for register_group_header_el in module_el - .children - .iter() - .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) - { - let name = register_group_header_el.attr("name")?.clone(); - let class = register_group_header_el - .attributes - .get("class") - .and_then(|d| if !d.is_empty() { Some(d) } else { None }) - .cloned(); - let description = register_group_header_el - .attributes - .get("caption") - .and_then(|d| if !d.is_empty() { Some(d) } else { None }) - .cloned(); - - let size = register_group_header_el - .attributes - .get("size") - .and_then(|d| { - if !d.is_empty() { - util::parse_int(d).ok() - } else { - None - } - }); - - let register_group_items = parse_register_group_items(register_group_header_el)?; - let registers = parse_list(register_group_header_el, offset, value_groups)?; - - register_group_headers.insert( - name.clone(), - chip::RegisterGroupHeader { - name, - class, - description, - size, - register_group_items, - registers, - }, - ); - } - Ok(register_group_headers) -} - -pub fn parse_register_group_items( - register_group_header_el: &xmltree::Element, -) -> crate::Result> { - let mut register_group_items = BTreeMap::new(); - - for register_group_item_el in register_group_header_el - .children - .iter() - .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) - { - let name = register_group_item_el.attr("name")?.clone(); - let description = register_group_item_el - .attributes - .get("caption") - .and_then(|d| if !d.is_empty() { Some(d) } else { None }) - .cloned(); - - let name_in_module = register_group_item_el - .attributes - .get("name-in-module") - .and_then(|d| if !d.is_empty() { Some(d) } else { None }) - .cloned(); - - let size = register_group_item_el.attributes.get("size").and_then(|d| { - if !d.is_empty() { - util::parse_int(d).ok() - } else { - None - } - }); - - let offset = register_group_item_el - .attributes - .get("offset") - .and_then(|d| { - if !d.is_empty() { - util::parse_int(d).ok() - } else { - None - } - }); - - let count = register_group_item_el - .attributes - .get("count") - .and_then(|d| { - if !d.is_empty() { - util::parse_int(d).ok() - } else { - None - } - }); - - register_group_items.insert( - name.clone(), - chip::RegisterGroupItem { - name, - name_in_module, - description, - size, - offset, - count, - }, - ); - } - - Ok(register_group_items) + .collect() } diff --git a/src/atdf/register_group.rs b/src/atdf/register_group.rs new file mode 100644 index 0000000..d78db7c --- /dev/null +++ b/src/atdf/register_group.rs @@ -0,0 +1,120 @@ +use crate::atdf; +use crate::chip; +use crate::util; +use crate::ElementExt; +use std::collections::BTreeMap; + +pub fn parse_list( + module_el: &xmltree::Element, + address: usize, + value_groups: &atdf::values::ValueGroups, +) -> crate::Result> { + let mut register_group_headers = BTreeMap::new(); + for register_group_header_el in module_el + .children + .iter() + .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) + { + let name = register_group_header_el.attr("name")?.clone(); + let description = register_group_header_el + .attributes + .get("caption") + .filter(|d| !d.is_empty()) + .cloned(); + + let references = parse_references(register_group_header_el)?; + let registers = + atdf::register::parse_list(register_group_header_el, address, value_groups)?; + + register_group_headers.insert( + name.clone(), + chip::RegisterGroup { + name, + description, + offset: 0, + registers, + references, + // subgroups will be filled in when all register-groups are parsed + subgroups: vec![], + }, + ); + } + Ok(register_group_headers) +} + +pub fn build_register_group_hierarchy( + register_group: &mut chip::RegisterGroup, + register_groups: &mut BTreeMap, + current_address: usize, + level: usize, +) -> crate::Result<()> { + // Infinite recursion guard + if level > 20 { + return Err( + atdf::error::RecursiveRegisterGroupError::new(register_group.name.clone()).into(), + ); + } + for reference in register_group.references.iter() { + if let Some(name_in_module) = &reference.name_in_module { + if let Some(subgroup) = register_groups.get(name_in_module) { + let mut subgroup = subgroup.clone(); + // The reference offset & name is used to override the subgroup + let offset = reference.offset.unwrap_or(0); + let new_address = current_address + offset; + + if offset > 0 { + // Adjust each register's address by the calculated adjustment + subgroup.registers.iter_mut().for_each(|(_, register)| { + register.address = new_address + register.offset; + }); + } + subgroup.name = reference.name.clone(); + + build_register_group_hierarchy( + &mut subgroup, + register_groups, + new_address, + level + 1, + )?; + register_group.subgroups.push(subgroup); + } + } + } + + Ok(()) +} + +pub fn parse_references( + register_group_el: &xmltree::Element, +) -> crate::Result> { + let mut register_group_references = vec![]; + + for register_group_item_el in register_group_el + .children + .iter() + .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) + { + let name = register_group_item_el.attr("name")?.clone(); + + let name_in_module = register_group_item_el + .attributes + .get("name-in-module") + .filter(|d| !d.is_empty()) + .cloned(); + + let offset = register_group_item_el + .attributes + .get("offset") + .filter(|d| !d.is_empty()) + .map(|d| util::parse_int(d)) + .transpose()?; + + register_group_references.push(chip::RegisterGroupReference { + name, + name_in_module, + offset, + }); + } + + Ok(register_group_references) +} diff --git a/src/chip.rs b/src/chip.rs index 22b8dcb..6adb2bc 100644 --- a/src/chip.rs +++ b/src/chip.rs @@ -21,54 +21,8 @@ pub struct Peripheral { pub name_in_module: String, pub description: Option, - pub registers: BTreeMap, - pub register_group_headers: BTreeMap, -} - -impl Peripheral { - pub fn base_address(&self) -> Option { - if self.is_union() { - return self - .get_union_register_group_headers() - .iter() - .flat_map(|(h, _)| h.registers.values()) - .map(|r| r.address) - .min(); - } - self.registers.values().map(|r| r.address).min() - } - - pub fn module_register_group_header(&self) -> Option<&RegisterGroupHeader> { - self.register_group_headers.get(&self.name_in_module) - } - - pub fn is_union(&self) -> bool { - let module_register_group_header = self.module_register_group_header(); - match module_register_group_header { - Some(header) => header.is_union(), - None => false, - } - } - - pub fn get_union_register_group_headers( - &self, - ) -> Vec<(&RegisterGroupHeader, &RegisterGroupItem)> { - match self.module_register_group_header() { - Some(module_header) if module_header.is_union() => module_header - .register_group_items - .values() - .filter_map(|group_item| { - group_item.name_in_module.as_ref().and_then(|header_name| { - self.register_group_headers - .iter() - .find(|(_, header)| &header.name == header_name) - .map(|(_, header)| (header, group_item)) - }) - }) - .collect(), - _ => vec![], - } - } + pub address: usize, + pub register_group: RegisterGroup, } #[derive(Debug, Clone)] @@ -95,22 +49,47 @@ pub enum ValueRestriction { } #[derive(Debug, Clone)] -pub struct RegisterGroupHeader { +pub struct RegisterGroup { pub name: String, - pub class: Option, pub description: Option, - pub size: Option, + /// Offset relative to the peripheral base address + pub offset: usize, - pub register_group_items: BTreeMap, + /// The register group references to other register groups. This is only used for filling up + /// the subgroups. + pub references: Vec, + pub subgroups: Vec, pub registers: BTreeMap, } -impl RegisterGroupHeader { - pub fn is_union(&self) -> bool { - self.class.as_deref() == Some("union") +impl RegisterGroup { + /// Recursively collects all registers from this register group and its subgroups. + /// + /// This function traverses the register group hierarchy, including all subgroups, + /// and returns a collection of all registers found. + pub fn get_all_registers(&self) -> Vec<&Register> { + let mut registers = Vec::new(); + + // Add registers from the current group + registers.extend(self.registers.values()); + + // Recursively add registers from all subgroups + for subgroup in &self.subgroups { + registers.extend(subgroup.get_all_registers()); + } + + // Return the collected registers + registers } } +#[derive(Debug, Clone)] +pub struct RegisterGroupReference { + pub name: String, + pub name_in_module: Option, + pub offset: Option, +} + #[derive(Debug, Clone)] pub struct RegisterGroupItem { pub name: String, @@ -126,7 +105,10 @@ pub struct Register { pub name: String, pub description: Option, pub mode: Option, + /// The absolute memory address where this register is located in the memory map pub address: usize, + /// The relative offset of this register within its parent register group + pub offset: usize, pub size: usize, pub access: AccessMode, pub restriction: ValueRestriction, diff --git a/src/svd/chip.rs b/src/svd/chip.rs index 4d5d2cc..e9467f5 100644 --- a/src/svd/chip.rs +++ b/src/svd/chip.rs @@ -45,13 +45,7 @@ pub fn generate(c: &chip::Chip) -> crate::Result { } fn has_registers(peripheral: &&chip::Peripheral) -> bool { - if peripheral.is_union() { - return peripheral - .get_union_register_group_headers() - .iter() - .any(|(header, _)| !header.registers.values().len() > 0); - } - let regs = !peripheral.registers.is_empty(); + let regs = !peripheral.register_group.get_all_registers().is_empty(); if !regs { log::warn!("No registers found for peripheral {}", peripheral.name); } diff --git a/src/svd/peripheral.rs b/src/svd/peripheral.rs index 6ffe479..5975b57 100644 --- a/src/svd/peripheral.rs +++ b/src/svd/peripheral.rs @@ -4,24 +4,25 @@ use crate::chip; use crate::svd; use std::convert::TryInto; -fn create_address_blocks( - p: &chip::Peripheral, - base: usize, -) -> crate::Result>> { - let mut registers: Vec<_> = match p.is_union() { - true => { - let &(header, _) = p - .get_union_register_group_headers() - .first() - .expect("No union header found"); - header.registers.values().collect() - } - false => p.registers.values().collect(), - }; +fn create_address_blocks(p: &chip::Peripheral) -> crate::Result>> { + let mut registers = p.register_group.get_all_registers().clone(); registers.sort_by(|a, b| a.address.cmp(&b.address)); - let new_address_block = |offset: usize, size| { - let offset = (offset - base).try_into().map_err(crate::Error::from)?; + let new_address_block = |offset: usize, size, reg: Option<&chip::Register>| { + // Calculate the offset relative to the peripheral's base address + // Handle the case where offset might be less than p.address + let offset_value = if offset >= p.address { + offset - p.address + } else { + let fd = if let Some(reg) = reg { + reg.name.clone() + } else { + "Unknown".to_string() + }; + println!("cargo:warning=Register address {} ({:#x}) is less than peripheral base address ({:#x})", fd, offset, p.address); + 0 // Default to 0 if we have an underflow situation + }; + let offset = offset_value.try_into().map_err(crate::Error::from)?; svd_rs::AddressBlock::builder() .offset(offset) @@ -39,13 +40,21 @@ fn create_address_blocks( if current_address == reg.address { current_size += reg.size; } else { - address_blocks.push(new_address_block(current_offset, current_size.try_into()?)?); + address_blocks.push(new_address_block( + current_offset, + current_size.try_into()?, + Some(reg), + )?); current_offset = reg.address; current_size = reg.size; } } - address_blocks.push(new_address_block(current_offset, current_size.try_into()?)?); + address_blocks.push(new_address_block( + current_offset, + current_size.try_into()?, + None, + )?); let address_blocks = if !address_blocks.is_empty() { Some(address_blocks) @@ -56,58 +65,44 @@ fn create_address_blocks( Ok(address_blocks) } -pub fn generate(p: &chip::Peripheral) -> crate::Result { - let base: u32 = p - .base_address() - .expect("Could not retrieve peripheral base address") - .try_into()?; +pub fn create_register_clusters( + register_group: &chip::RegisterGroup, +) -> crate::Result> { + let mut result: Vec = vec![]; - let registers: Vec = match p.is_union() { - true => { - println!("cargo:warning=svd generate union style {}", p.name); - p.get_union_register_group_headers() - .iter() - .map(|(header, item)| { - let cluster = svd_rs::ClusterInfo::builder() - .name(header.name.clone()) - .description(header.description.clone()) - .address_offset(item.offset.unwrap_or(0) as u32) - .children( - header - .registers - .values() - .map(|r| { - svd::register::generate(r, base) - .map(svd_rs::RegisterCluster::Register) - }) - .collect::, _>>()?, - ); + for register in register_group.registers.values() { + let register_cluster = + svd::register::generate(register).map(svd_rs::RegisterCluster::Register)?; + result.push(register_cluster); + } - cluster - .build(svd_rs::ValidateLevel::Strict) - .map(|cluster_info| { - svd_rs::RegisterCluster::Cluster(MaybeArray::Single(cluster_info)) - }) - .map_err(crate::Error::from) - }) - .collect::, _>>()? - } - false => p - .registers - .values() - .map(|r| svd::register::generate(r, base).map(svd_rs::RegisterCluster::Register)) - .collect::, _>>()?, - }; + for subgroup in register_group.subgroups.iter() { + let subgroup_clusters = create_register_clusters(subgroup)?; + let cluster = svd_rs::RegisterCluster::Cluster(MaybeArray::Single( + svd_rs::ClusterInfo::builder() + .name(subgroup.name.clone()) + .description(subgroup.description.clone()) + .address_offset(subgroup.offset as u32) + .children(subgroup_clusters) + .build(svd_rs::ValidateLevel::Strict) + .map_err(crate::Error::from)?, + )); + result.push(cluster); + } + Ok(result) +} + +pub fn generate(p: &chip::Peripheral) -> crate::Result { svd_rs::PeripheralInfo::builder() .name(p.name.clone()) .description(p.description.clone().or_else(|| { log::warn!("Description missing for peripheral {:?}", p.name); Some("No Description.".to_owned()) })) - .base_address(u64::from(base)) - .address_block(create_address_blocks(p, base as usize)?) - .registers(Some(registers)) + .base_address(u64::from(p.address as u32)) + .address_block(create_address_blocks(p)?) + .registers(Some(create_register_clusters(&p.register_group)?)) .build(svd_rs::ValidateLevel::Strict) .map(svd_rs::Peripheral::Single) .map_err(crate::Error::from) diff --git a/src/svd/register.rs b/src/svd/register.rs index cff52f2..bc98ff9 100644 --- a/src/svd/register.rs +++ b/src/svd/register.rs @@ -1,7 +1,7 @@ use crate::chip; use crate::svd::restriction::generate_access; -pub fn generate(r: &chip::Register, base: u32) -> crate::Result { +pub fn generate(r: &chip::Register) -> crate::Result { let (write_constraint, _) = crate::svd::restriction::generate(&r.restriction, r.size as u32 * 8)?; @@ -11,7 +11,7 @@ pub fn generate(r: &chip::Register, base: u32) -> crate::Result Date: Thu, 1 May 2025 23:27:20 +0200 Subject: [PATCH 4/6] Fix regression test issues (#2) * fix invalid address block regressions * re-implement baselining the start address & fix address block issue * apply PR comments * run format --- src/atdf/peripheral.rs | 43 +++++++++++- src/atdf/register_group.rs | 25 ++++--- src/chip.rs | 5 ++ src/svd/peripheral.rs | 76 +++++++++------------ tests/snapshots/regression__atmega4809.snap | 4 +- 5 files changed, 96 insertions(+), 57 deletions(-) diff --git a/src/atdf/peripheral.rs b/src/atdf/peripheral.rs index 52dbff4..1f39a46 100644 --- a/src/atdf/peripheral.rs +++ b/src/atdf/peripheral.rs @@ -3,6 +3,41 @@ use crate::chip; use crate::util; use crate::ElementExt; +fn update_register_group(register_group: &mut chip::RegisterGroup, delta: usize) { + for register in register_group.registers.values_mut() { + register.offset -= delta; + } + for subgroup in register_group.subgroups.iter_mut() { + update_register_group(subgroup, delta); + } +} + +/// Adjusts the peripheral's base address to the first register's absolute address. +/// +/// Finds the minimum address among all registers and updates register offsets +/// accordingly. This normalizes the peripheral's address space representation. +fn base_line_address(peripheral: &mut chip::Peripheral) { + let register_addresses = peripheral + .register_group + .registers + .values() + .map(|r| peripheral.address + r.offset) + .collect::>(); + + if register_addresses.is_empty() { + return; + } + let min_address = match register_addresses.iter().min() { + Some(addr) => *addr, + None => peripheral.address, + }; + if min_address > peripheral.address { + let delta = min_address - peripheral.address; + peripheral.address = min_address; + update_register_group(&mut peripheral.register_group, delta); + } +} + pub fn parse_list( el: &xmltree::Element, modules: &xmltree::Element, @@ -37,7 +72,7 @@ pub fn parse_list( 0, )?; - peripherals.push(chip::Peripheral { + let mut peripheral = chip::Peripheral { name: instance.attr("name")?.clone(), name_in_module: name_in_module.clone(), description: instance @@ -48,7 +83,11 @@ pub fn parse_list( .and_then(|d| if !d.is_empty() { Some(d) } else { None }), address, register_group: main_register_group, - }) + }; + + base_line_address(&mut peripheral); + + peripherals.push(peripheral) } } diff --git a/src/atdf/register_group.rs b/src/atdf/register_group.rs index d78db7c..4184da1 100644 --- a/src/atdf/register_group.rs +++ b/src/atdf/register_group.rs @@ -10,21 +10,25 @@ pub fn parse_list( value_groups: &atdf::values::ValueGroups, ) -> crate::Result> { let mut register_group_headers = BTreeMap::new(); - for register_group_header_el in module_el + for register_group_el in module_el .children .iter() .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) { - let name = register_group_header_el.attr("name")?.clone(); - let description = register_group_header_el + let name = register_group_el.attr("name")?.clone(); + let description = register_group_el .attributes .get("caption") .filter(|d| !d.is_empty()) .cloned(); + let class = register_group_el + .attributes + .get("class") + .filter(|d| !d.is_empty()) + .cloned(); - let references = parse_references(register_group_header_el)?; - let registers = - atdf::register::parse_list(register_group_header_el, address, value_groups)?; + let references = parse_references(register_group_el)?; + let registers = atdf::register::parse_list(register_group_el, address, value_groups)?; register_group_headers.insert( name.clone(), @@ -32,6 +36,7 @@ pub fn parse_list( name, description, offset: 0, + is_union: class == Some("union".to_string()), registers, references, // subgroups will be filled in when all register-groups are parsed @@ -89,20 +94,20 @@ pub fn parse_references( ) -> crate::Result> { let mut register_group_references = vec![]; - for register_group_item_el in register_group_el + for register_group_ref_el in register_group_el .children .iter() .filter_map(|node| node.as_element().filter(|e| e.name == "register-group")) { - let name = register_group_item_el.attr("name")?.clone(); + let name = register_group_ref_el.attr("name")?.clone(); - let name_in_module = register_group_item_el + let name_in_module = register_group_ref_el .attributes .get("name-in-module") .filter(|d| !d.is_empty()) .cloned(); - let offset = register_group_item_el + let offset = register_group_ref_el .attributes .get("offset") .filter(|d| !d.is_empty()) diff --git a/src/chip.rs b/src/chip.rs index 6adb2bc..ed8d0d4 100644 --- a/src/chip.rs +++ b/src/chip.rs @@ -54,6 +54,11 @@ pub struct RegisterGroup { pub description: Option, /// Offset relative to the peripheral base address pub offset: usize, + /// Indicates if this register group is a union. + /// + /// Currently limits nested register group functionality. + /// If removed, full nested register group support would be enabled (#4). + pub is_union: bool, /// The register group references to other register groups. This is only used for filling up /// the subgroups. diff --git a/src/svd/peripheral.rs b/src/svd/peripheral.rs index 5975b57..56d67b1 100644 --- a/src/svd/peripheral.rs +++ b/src/svd/peripheral.rs @@ -5,27 +5,21 @@ use crate::svd; use std::convert::TryInto; fn create_address_blocks(p: &chip::Peripheral) -> crate::Result>> { - let mut registers = p.register_group.get_all_registers().clone(); - registers.sort_by(|a, b| a.address.cmp(&b.address)); - - let new_address_block = |offset: usize, size, reg: Option<&chip::Register>| { - // Calculate the offset relative to the peripheral's base address - // Handle the case where offset might be less than p.address - let offset_value = if offset >= p.address { - offset - p.address + let mut registers: Vec<&chip::Register> = if p.register_group.is_union { + if let Some(first_subgroup) = p.register_group.subgroups.first() { + first_subgroup.registers.values().collect::>() } else { - let fd = if let Some(reg) = reg { - reg.name.clone() - } else { - "Unknown".to_string() - }; - println!("cargo:warning=Register address {} ({:#x}) is less than peripheral base address ({:#x})", fd, offset, p.address); - 0 // Default to 0 if we have an underflow situation - }; - let offset = offset_value.try_into().map_err(crate::Error::from)?; + log::warn!("Union peripheral {:?} has no subgroups", p.name); + Vec::new() // Return empty vector if there are no subgroups + } + } else { + p.register_group.registers.values().collect::>() + }; + registers.sort_by(|a, b| a.address.cmp(&b.address)); + let new_address_block = |offset: usize, size| { svd_rs::AddressBlock::builder() - .offset(offset) + .offset(offset as u32) .size(size) .usage(svd_rs::AddressBlockUsage::Registers) .build(svd_rs::ValidateLevel::Strict) @@ -33,28 +27,22 @@ fn create_address_blocks(p: &chip::Peripheral) -> crate::Result 0 { + address_blocks.push(new_address_block(current_offset, current_size.try_into()?)?); + } - current_offset = reg.address; + current_offset = reg.offset; current_size = reg.size; } } - address_blocks.push(new_address_block( - current_offset, - current_size.try_into()?, - None, - )?); + address_blocks.push(new_address_block(current_offset, current_size.try_into()?)?); let address_blocks = if !address_blocks.is_empty() { Some(address_blocks) @@ -76,18 +64,20 @@ pub fn create_register_clusters( result.push(register_cluster); } - for subgroup in register_group.subgroups.iter() { - let subgroup_clusters = create_register_clusters(subgroup)?; - let cluster = svd_rs::RegisterCluster::Cluster(MaybeArray::Single( - svd_rs::ClusterInfo::builder() - .name(subgroup.name.clone()) - .description(subgroup.description.clone()) - .address_offset(subgroup.offset as u32) - .children(subgroup_clusters) - .build(svd_rs::ValidateLevel::Strict) - .map_err(crate::Error::from)?, - )); - result.push(cluster); + if register_group.is_union { + for subgroup in register_group.subgroups.iter() { + let subgroup_clusters = create_register_clusters(subgroup)?; + let cluster = svd_rs::RegisterCluster::Cluster(MaybeArray::Single( + svd_rs::ClusterInfo::builder() + .name(subgroup.name.clone()) + .description(subgroup.description.clone()) + .address_offset(subgroup.offset as u32) + .children(subgroup_clusters) + .build(svd_rs::ValidateLevel::Strict) + .map_err(crate::Error::from)?, + )); + result.push(cluster); + } } Ok(result) diff --git a/tests/snapshots/regression__atmega4809.snap b/tests/snapshots/regression__atmega4809.snap index 4f37e98..6f99089 100644 --- a/tests/snapshots/regression__atmega4809.snap +++ b/tests/snapshots/regression__atmega4809.snap @@ -15810,7 +15810,7 @@ expression: svd - TCA_SINGLE + SINGLE 16-bit Timer/Counter Type A - Single Mode 0x0 @@ -16439,7 +16439,7 @@ expression: svd - TCA_SPLIT + SPLIT 16-bit Timer/Counter Type A - Split Mode 0x0 From c74b75e3bc9e5ae2941dc0844c18cc75d4bf4d73 Mon Sep 17 00:00:00 2001 From: hendrik Date: Thu, 1 May 2025 23:39:42 +0200 Subject: [PATCH 5/6] remove register group item --- src/chip.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/chip.rs b/src/chip.rs index ed8d0d4..ee88775 100644 --- a/src/chip.rs +++ b/src/chip.rs @@ -95,16 +95,6 @@ pub struct RegisterGroupReference { pub offset: Option, } -#[derive(Debug, Clone)] -pub struct RegisterGroupItem { - pub name: String, - pub name_in_module: Option, - pub description: Option, - pub size: Option, - pub offset: Option, - pub count: Option, -} - #[derive(Debug, Clone)] pub struct Register { pub name: String, From b35963a2ca445a53b73e7cf5dec0e139045a7a94 Mon Sep 17 00:00:00 2001 From: hendrik Date: Thu, 1 May 2025 23:48:10 +0200 Subject: [PATCH 6/6] fix register-group offset docs & update --- src/atdf/register_group.rs | 1 + src/chip.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/atdf/register_group.rs b/src/atdf/register_group.rs index 4184da1..5714ff5 100644 --- a/src/atdf/register_group.rs +++ b/src/atdf/register_group.rs @@ -68,6 +68,7 @@ pub fn build_register_group_hierarchy( let new_address = current_address + offset; if offset > 0 { + subgroup.offset = offset; // Adjust each register's address by the calculated adjustment subgroup.registers.iter_mut().for_each(|(_, register)| { register.address = new_address + register.offset; diff --git a/src/chip.rs b/src/chip.rs index ee88775..3111dcc 100644 --- a/src/chip.rs +++ b/src/chip.rs @@ -52,7 +52,7 @@ pub enum ValueRestriction { pub struct RegisterGroup { pub name: String, pub description: Option, - /// Offset relative to the peripheral base address + /// Offset relative to the parent address pub offset: usize, /// Indicates if this register group is a union. ///