Skip to content

Commit 5e30d5d

Browse files
bors[bot]burrbullrcls
authored
Merge #519
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 <zgarbul.andrey@gmail.com> Co-authored-by: Ralph Loader <ralph1loader@gmail.com>
2 parents a02a5cf + d59a264 commit 5e30d5d

File tree

2 files changed

+51
-13
lines changed

2 files changed

+51
-13
lines changed

CHANGELOG.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,24 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1111

1212
- MSP430 API for atomically changing register bits, gated behind the `--nightly` flag
1313
- New SVD test for `msp430fr2355`
14-
15-
### Added
16-
1714
- Option `-o`(`--output-path`) let you specify output directory path
1815

1916
### Changed
2017

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

26+
### Fixed
27+
28+
- Padding has been corrected for SVD files containing nested array clusters.
29+
30+
This showed up on Cypress PSOC and Traveo II CPUs.
31+
2832
## [v0.18.0] - 2021-04-17
2933

3034
### Added

src/generate/peripheral.rs

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ fn register_or_cluster_block(
468468
let pad = region.offset - last_end;
469469
if pad != 0 {
470470
let name = Ident::new(&format!("_reserved{}", i), span);
471-
let pad = pad as usize;
471+
let pad = util::hex(pad as u64);
472472
rbfs.extend(quote! {
473473
#name : [u8; #pad],
474474
});
@@ -478,11 +478,20 @@ fn register_or_cluster_block(
478478
let is_region_a_union = region.is_union();
479479

480480
for reg_block_field in &region.rbfs {
481-
let comment = &format!(
482-
"0x{:02x} - {}",
483-
reg_block_field.offset,
484-
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
485-
)[..];
481+
let comment = if reg_block_field.size > 32 {
482+
format!(
483+
"0x{:02x}..0x{:02x} - {}",
484+
reg_block_field.offset,
485+
reg_block_field.offset + reg_block_field.size / 8,
486+
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
487+
)
488+
} else {
489+
format!(
490+
"0x{:02x} - {}",
491+
reg_block_field.offset,
492+
util::escape_brackets(util::respace(&reg_block_field.description).as_ref()),
493+
)
494+
};
486495

487496
if is_region_a_union {
488497
let name = &reg_block_field.field.ident;
@@ -533,7 +542,7 @@ fn register_or_cluster_block(
533542
),
534543
span,
535544
);
536-
let pad = (region.end - region.offset) as usize;
545+
let pad = util::hex((region.end - region.offset) as u64);
537546
rbfs.extend(quote! {
538547
#name: [u8; #pad],
539548
})
@@ -592,9 +601,34 @@ fn expand(
592601
Ok(ercs_expanded)
593602
}
594603

595-
/// Recursively calculate the size of a cluster. A cluster's size is the maximum
596-
/// end position of its recursive children.
604+
/// Calculate the size of a Cluster. If it is an array, then the dimensions
605+
/// tell us the size of the array. Otherwise, inspect the contents using
606+
/// [cluster_info_size_in_bits].
597607
fn cluster_size_in_bits(
608+
cluster: &Cluster,
609+
defs: &RegisterProperties,
610+
config: &Config,
611+
) -> Result<u32> {
612+
match cluster {
613+
Cluster::Single(info) => cluster_info_size_in_bits(info, defs, config),
614+
// If the contained array cluster has a mismatch between the
615+
// dimIncrement and the size of the array items, then the array
616+
// will get expanded in expand_cluster below. The overall size
617+
// then ends at the last array entry.
618+
Cluster::Array(info, dim) => {
619+
if dim.dim == 0 {
620+
return Ok(0); // Special case!
621+
}
622+
let last_offset = (dim.dim - 1) * dim.dim_increment * BITS_PER_BYTE;
623+
let last_size = cluster_info_size_in_bits(info, defs, config);
624+
Ok(last_offset + last_size?)
625+
}
626+
}
627+
}
628+
629+
/// Recursively calculate the size of a ClusterInfo. A cluster's size is the
630+
/// maximum end position of its recursive children.
631+
fn cluster_info_size_in_bits(
598632
info: &ClusterInfo,
599633
defs: &RegisterProperties,
600634
config: &Config,
@@ -632,7 +666,7 @@ fn expand_cluster(
632666

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

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

638672
match cluster {

0 commit comments

Comments
 (0)