Skip to content

Commit

Permalink
Merge #519
Browse files Browse the repository at this point in the history
519: Be more careful computing the size of an array Cluster. r=burrbull a=rcls

`cluster_size_in_bits` was ignoring array information, leading to incorrect padding with nested clusters.
I noticed this as incorrect padding on the three SARs in PASS0 on the Cypress cyt4bb. (The .svd for this requires registration on the Cypress website.)
It also shows up as incorrect padding on the TCPWM0 in the Cypress PSOC6_04 (svd at https://github.com/cypresssemiconductorco/psoc6pdl/blob/master/devices/svd/psoc6_04.svd)

E.g., for the PSOC TCPWM0, the diffs on the generated code from this change are:
```
diff --git a/src/tcpwm0.rs b/src/tcpwm0.rs
index 678d146..7165c46 100644
--- a/src/tcpwm0.rs
+++ b/src/tcpwm0.rs
@@ -3,7 +3,7 @@
 pub struct RegisterBlock {
     #[doc = "0x00 - Group of counters"]
     pub grp0: GRP,
-    _reserved1: [u8; 32640usize],
+    _reserved1: [u8; 31744usize],
     #[doc = "0x8000 - Group of counters"]
     pub grp1: GRP,
 }
```
Eyeballing the GRP, it has 8 * 128byte entries = 1024 bytes, and the correct padding is then 32768 - 1024 = 31744 bytes.

Co-authored-by: Andrey Zgarbul <[email protected]>
Co-authored-by: Ralph Loader <[email protected]>
  • Loading branch information
3 people authored May 11, 2021
2 parents a02a5cf + d59a264 commit 5e30d5d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 13 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,24 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- MSP430 API for atomically changing register bits, gated behind the `--nightly` flag
- New SVD test for `msp430fr2355`

### Added

- Option `-o`(`--output-path`) let you specify output directory path

### Changed

- `_rererved` fields in `RegisterBlock` now hexidemical usize
- options can be set now with `svd2rust.toml` config
- option `ignore_groups` for optional disabling #506
- [breaking-change] move `const_generic` from features to options
- use `Config` to pass options over all render levels
- Use register iterator from `svd-parser`
- rm unneeded `core::convert::` prefix on `From`

### Fixed

- Padding has been corrected for SVD files containing nested array clusters.

This showed up on Cypress PSOC and Traveo II CPUs.

## [v0.18.0] - 2021-04-17

### Added
Expand Down
54 changes: 44 additions & 10 deletions src/generate/peripheral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ fn register_or_cluster_block(
let pad = region.offset - last_end;
if pad != 0 {
let name = Ident::new(&format!("_reserved{}", i), span);
let pad = pad as usize;
let pad = util::hex(pad as u64);
rbfs.extend(quote! {
#name : [u8; #pad],
});
Expand All @@ -478,11 +478,20 @@ fn register_or_cluster_block(
let is_region_a_union = region.is_union();

for reg_block_field in &region.rbfs {
let comment = &format!(
"0x{:02x} - {}",
reg_block_field.offset,
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
)[..];
let comment = if reg_block_field.size > 32 {
format!(
"0x{:02x}..0x{:02x} - {}",
reg_block_field.offset,
reg_block_field.offset + reg_block_field.size / 8,
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
)
} else {
format!(
"0x{:02x} - {}",
reg_block_field.offset,
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
)
};

if is_region_a_union {
let name = &reg_block_field.field.ident;
Expand Down Expand Up @@ -533,7 +542,7 @@ fn register_or_cluster_block(
),
span,
);
let pad = (region.end - region.offset) as usize;
let pad = util::hex((region.end - region.offset) as u64);
rbfs.extend(quote! {
#name: [u8; #pad],
})
Expand Down Expand Up @@ -592,9 +601,34 @@ fn expand(
Ok(ercs_expanded)
}

/// Recursively calculate the size of a cluster. A cluster's size is the maximum
/// end position of its recursive children.
/// Calculate the size of a Cluster. If it is an array, then the dimensions
/// tell us the size of the array. Otherwise, inspect the contents using
/// [cluster_info_size_in_bits].
fn cluster_size_in_bits(
cluster: &Cluster,
defs: &RegisterProperties,
config: &Config,
) -> Result<u32> {
match cluster {
Cluster::Single(info) => cluster_info_size_in_bits(info, defs, config),
// If the contained array cluster has a mismatch between the
// dimIncrement and the size of the array items, then the array
// will get expanded in expand_cluster below. The overall size
// then ends at the last array entry.
Cluster::Array(info, dim) => {
if dim.dim == 0 {
return Ok(0); // Special case!
}
let last_offset = (dim.dim - 1) * dim.dim_increment * BITS_PER_BYTE;
let last_size = cluster_info_size_in_bits(info, defs, config);
Ok(last_offset + last_size?)
}
}
}

/// Recursively calculate the size of a ClusterInfo. A cluster's size is the
/// maximum end position of its recursive children.
fn cluster_info_size_in_bits(
info: &ClusterInfo,
defs: &RegisterProperties,
config: &Config,
Expand Down Expand Up @@ -632,7 +666,7 @@ fn expand_cluster(

let defs = cluster.default_register_properties.derive_from(defs);

let cluster_size = cluster_size_in_bits(cluster, &defs, config)
let cluster_size = cluster_info_size_in_bits(cluster, &defs, config)
.with_context(|| format!("Cluster {} has no determinable `size` field", cluster.name))?;

match cluster {
Expand Down

0 comments on commit 5e30d5d

Please sign in to comment.