Skip to content

error: Cluster USART has no determinable size field #191

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.

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

Open
wez opened this issue Mar 13, 2018 · 9 comments
Open

error: Cluster USART has no determinable size field #191

wez opened this issue Mar 13, 2018 · 9 comments

Comments

@wez
Copy link
Contributor

wez commented Mar 13, 2018

I get this error with ATSAMD21G18A.svd

I'm not opposed to rolling up my sleeves to help fix this if someone with context can tell me roughly what needs to happen. I'm a fast learner!

I see this with 631ab3e

error: Cluster USART has no determinable `size` field
caused by: Warning! overlap while calculating Register Size within a Cluster! Cluster contents may be incorrectly aligned!
backtrace: stack backtrace:
   0:     0x55bb5fecc8d4 - backtrace::backtrace::libunwind::trace::hbf9f65f12207138f
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.5/src/backtrace/libunwind.rs:53
                         - backtrace::backtrace::trace::hd67ff4d2334f7ed6
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.5/src/backtrace/mod.rs:42
   1:     0x55bb5feca63c - backtrace::capture::Backtrace::new_unresolved::h642c5b6c9400d9d5
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.5/src/capture.rs:88
   2:     0x55bb5feca58e - backtrace::capture::Backtrace::new::h6b30f701e69e550b
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.5/src/capture.rs:63
   3:     0x55bb5fec76b6 - error_chain::make_backtrace::hc918c7599bfaea64
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/error-chain-0.11.0/src/lib.rs:616
   4:     0x55bb5fec772f - <error_chain::State as core::default::Default>::default::h548074ab69a8084d
                        at /home/wez/.cargo/registry/src/github.com-1ecc6299db9ec823/error-chain-0.11.0/src/lib.rs:710
   5:     0x55bb5fde81bd - svd2rust::errors::Error::from_kind::hd36a5afffca28cb6
                        at /home/wez/github/svd2rust/<impl_error_chain_processed macros>:53
   6:     0x55bb5fde83b9 - <svd2rust::errors::Error as core::convert::From<alloc::string::String>>::from::hd00d5407c74d1305
                        at src/errors.rs:1
   7:     0x55bb5fd7ef70 - svd2rust::generate::peripheral::cluster_size_in_bits::ha74a72683196d068
                        at src/generate/peripheral.rs:210
   8:     0x55bb5fe1a6ca - svd2rust::generate::peripheral::expand_cluster::{{closure}}::hbe5bc632c7386e96
                        at src/generate/peripheral.rs:246
   9:     0x55bb5fd875dc - <core::result::Result<T, E>>::or_else::he8ffab4c771caf21
                        at /checkout/src/libcore/result.rs:691
  10:     0x55bb5fd7f86d - svd2rust::generate::peripheral::expand_cluster::h447512a0d57fb8aa
                        at src/generate/peripheral.rs:243
  11:     0x55bb5fd7e89f - svd2rust::generate::peripheral::expand::h8fc1592519c5606e
                        at src/generate/peripheral.rs:191
  12:     0x55bb5fd7d6c8 - svd2rust::generate::peripheral::register_or_cluster_block::h682fffe6a6a9ec45
                        at src/generate/peripheral.rs:125
  13:     0x55bb5fd7c792 - svd2rust::generate::peripheral::render::h008b4049f11701b1
                        at src/generate/peripheral.rs:78
  14:     0x55bb5fde0b5f - svd2rust::generate::device::render::h25c68a216b631a00
                        at src/generate/device.rs:128
  15:     0x55bb5fe25453 - svd2rust::run::hb3070a0e4fb05ceb
                        at src/main.rs:84
  16:     0x55bb5fe25b86 - svd2rust::main::h76b2401efe189507
                        at src/main.rs:99
  17:     0x55bb5fde9031 - std::rt::lang_start::{{closure}}::hf182223c25a0e056
                        at /checkout/src/libstd/rt.rs:74
  18:     0x55bb60082ee7 - std::rt::lang_start_internal::{{closure}}::h6683f918b5e41df1
                        at libstd/rt.rs:59
                         - std::panicking::try::do_call::h9980f1b0ce760291
                        at libstd/panicking.rs:306
  19:     0x55bb6008ca8e - __rust_maybe_catch_panic
                        at libpanic_unwind/lib.rs:102
  20:     0x55bb60083683 - std::panicking::try::h2b055bf317f9a3ac
                        at libstd/panicking.rs:285
                         - std::panic::catch_unwind::h8402e9b2ba9c09e5
                        at libstd/panic.rs:361
                         - std::rt::lang_start_internal::h591dc8f1c9bda6fc
                        at libstd/rt.rs:58
  21:     0x55bb5fde9011 - std::rt::lang_start::hcbc8bede4be6939e
                        at /checkout/src/libstd/rt.rs:74
  22:     0x55bb5fe2613d - main
  23:     0x7f4a2898a1c0 - __libc_start_main
  24:     0x55bb5fd7b4a9 - _start
  25:                0x0 - <unknown>
@wez
Copy link
Contributor Author

wez commented Mar 13, 2018

I added more context to the warning:

caused by: Warning! overlap while calculating Register Size within a Cluster! Cluster contents may be incorrectly aligned! Single(
    RegisterInfo {
        name: "BAUD_FRAC_MODE",
        description: "USART Baud Rate",
        address_offset: 12,
        size: Some(
            16
        ),
        access: None,
        reset_value: None,
        reset_mask: None,
        fields: Some(
            [
                Field {
                    name: "BAUD",
                    description: Some(
                        "Baud Rate Value"
                    ),
                    bit_range: BitRange {
                        offset: 0,
                        width: 13
                    },
                    access: None,
                    enumerated_values: [],
                    write_constraint: None,
                    _extensible: ()
                },
                Field {
                    name: "FP",
                    description: Some(
                        "Fractional Part"
                    ),
                    bit_range: BitRange {
                        offset: 13,
                        width: 3
                    },
                    access: None,
                    enumerated_values: [],
                    write_constraint: None,
                    _extensible: ()
                }
            ]
        ),
        write_constraint: None,
        _extensible: ()
    }
)

@wez
Copy link
Contributor Author

wez commented Mar 13, 2018

Here's the offending sequence in the svd file:

        <register>
          <name>BAUD</name>
          <description>USART Baud Rate</description>
          <addressOffset>0x0C</addressOffset>
          <size>16</size>
          <fields>
            <field>
              <name>BAUD</name>
              <description>Baud Rate Value</description>
              <bitOffset>0</bitOffset>
              <bitWidth>16</bitWidth>
            </field>
          </fields>
        </register>
        <register>
          <name>BAUD_FRAC_MODE</name>
          <description>USART Baud Rate</description>
          <alternateRegister>BAUD</alternateRegister>
          <addressOffset>0x0C</addressOffset>
          <size>16</size>
          <fields>
            <field>
              <name>BAUD</name>
              <description>Baud Rate Value</description>
              <bitOffset>0</bitOffset>
              <bitWidth>13</bitWidth>
            </field>
            <field>
              <name>FP</name>
              <description>Fractional Part</description>
              <bitOffset>13</bitOffset>
              <bitWidth>3</bitWidth>
            </field>
          </fields>
        </register>

@wez
Copy link
Contributor Author

wez commented Mar 13, 2018

May be another incarnation of #16 ?

@wez
Copy link
Contributor Author

wez commented Mar 13, 2018

I've hacked in some code to skip entries that have an alternateRegister, which seems to fit with the default disposition of #16 and probably fine (caveat: I haven't read any specs :-p)
I still had some more overlaps though, so I modified the code to skip entries that overlap:

diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index 90df735..7480e3a 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -207,11 +207,16 @@ fn cluster_size_in_bits(info: &ClusterInfo, defs: &Defaults) -> Result<u32> {
     for c in &info.children {
         size += match *c {
             Either::Left(ref reg) => {
-                let pad = reg.address_offset.checked_sub(offset)
-                .ok_or_else(||
-                    format!("Warning! overlap while calculating Register Size within a Cluster! Cluster contents may be incorrectly aligned!"))?
-                * BITS_PER_BYTE;
-
+                if reg.alternate_register.is_some() {
+                    0
+                } else {
+                let pad = match reg.address_offset.checked_sub(offset) {
+                    Some(size) => size * BITS_PER_BYTE,
+                    None => {
+                        eprintln!("Warning! overlap while calculating Register Size within a Cluster! Cluster contents may be incorrectly aligned! offset={} {:#?}", offset, reg);
+                        continue;
+                    }
+                };

                 let reg_size: u32 = expand_register(reg, defs, None)?
                     .iter()
@@ -219,6 +224,7 @@ fn cluster_size_in_bits(info: &ClusterInfo, defs: &Defaults) -> Result<u32> {
                     .sum();

                 pad + reg_size
+                }
             }
             Either::Right(ref clust) => {
                 let pad = clust.address_offset.checked_sub(offset)

@wez
Copy link
Contributor Author

wez commented Mar 13, 2018

Thinking about this a bit more, can we simplify this?

If addressOffset is the start of the range of bits for the register (relative to its container; the cluster) and we can compute the bit width of a group, then we know the end of the range of bits for each of those registers.

The size is therefore the maximum end position for all of its children.

Am I missing something there?

/// Recursively calculate the size of a cluster. A cluster's size is the maximum
/// end position of its recursive children.
fn cluster_size_in_bits(info: &ClusterInfo, defs: &Defaults) -> Result<u32> {
    let mut size = 0;

    for c in &info.children {
        let end = match *c {
            Either::Left(ref reg) => {
                let reg_size: u32 = expand_register(reg, defs, None)?
                    .iter()
                    .map(|rbf| rbf.size)
                    .sum();

                (reg.address_offset * BITS_PER_BYTE) + reg_size
            }
            Either::Right(ref clust) => {
                (clust.address_offset * BITS_PER_BYTE) + cluster_size_in_bits(clust, defs)?
            }
        };

        size = size.max(end);
    }
    Ok(size)
}

@jamesmunns
Copy link
Member

Hey @wez, contributions are welcome :) I'll try and take a look at this issue (and your solutions) this evening.

bors bot added a commit to rust-embedded/svd that referenced this issue Mar 16, 2018
50: Parse <register><alternateRegister> elements r=Emilgardis a=wez

In looking at rust-embedded/svd2rust#191 and rust-embedded/svd2rust#16 I thought that this might help, so here's the trivial patch.
@bachp
Copy link

bachp commented Apr 16, 2018

@wez @jamesmunns Did you already solve this? I also ran into this issue with a newer SVD file for the same chip from Atmel ATSAMD21G18AU.svd

I tried with the current master (fb54754) but still got this error.

@wez
Copy link
Contributor Author

wez commented Apr 17, 2018

@bachp I have WIP here: https://github.com/wez/atsamd21-rs
There are a couple of issues with the code gen that block my patches from being merged, and I hope to find time to address those in the coming week

bors bot added a commit that referenced this issue May 8, 2018
192: Emit unions for overlapping/overloaded registers r=Emilgardis a=wez

I want to get a complete working build for ATSAMD21 (rust-embedded/wg#61) so I'm taking a stab at that.

* Introduced a `FieldRegion` helper to reason about overlapping regions
* If overlaps are detected, a `union` container is emitted
* If the only item in a `RegisterBlock` is a union, that `RegisterBlock` is emitted as a union
* Otherwise: we generate a name for the union field by taking the longest common prefix of the union's alternates, or just pick an artificial name like `u1` if there was no common prefix.
* If the starting address offset of elements in a union are not all the same, we don't have a way to emit padding for them today.  We will emit a warning (and bad code) in that case (example below).  The one example of this I see in ATSAMD21 is due to missing `derivedFrom` support for registers; we're currently generating bad code for these anyway.  I have resolved in another branch that I'll turn into a PR once this one is landed.

```
WARNING: field Some(Ident("pmux1_1")) has different offset 177 than its union container 176
WARNING: field Some(Ident("pmux1_2")) has different offset 178 than its union container 176
```

Examples:

```
#[doc = "Real-Time Counter"]
pub mod rtc {
    #[doc = r" Register block"]
    #[repr(C)]
    pub union RegisterBlock {
        #[doc = "0x00 - Clock/Calendar with Alarm"]
        pub mode2: MODE2,
        #[doc = "0x00 - 16-bit Counter with Two 16-bit Compares"]
        pub mode1: MODE1,
        #[doc = "0x00 - 32-bit Counter with Single 32-bit Compare"]
        pub mode0: MODE0,
    }
```

```
    #[doc = r" Register block"]
    #[repr(C)]
    pub struct USART {
        #[doc = "0x00 - USART Control A"]
        pub ctrla: self::usart::CTRLA,
        #[doc = "0x04 - USART Control B"]
        pub ctrlb: self::usart::CTRLB,
        _reserved2: [u8; 4usize],
        #[doc = "USART Baud Rate"]
        pub baud: baud_union,
      ...
    }
    #[doc = "USART Baud Rate"]
    #[repr(C)]
    pub union baud_union {
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_usartfp_mode: self::usart::BAUD_USARTFP_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_fracfp_mode: self::usart::BAUD_FRACFP_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_frac_mode: self::usart::BAUD_FRAC_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud: self::usart::BAUD,
    }
```

Refs: #191 
#16



Co-authored-by: Wez Furlong <wez@wezfurlong.org>
Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
bors bot added a commit that referenced this issue May 8, 2018
192: Emit unions for overlapping/overloaded registers r=Emilgardis a=wez

I want to get a complete working build for ATSAMD21 (rust-embedded/wg#61) so I'm taking a stab at that.

* Introduced a `FieldRegion` helper to reason about overlapping regions
* If overlaps are detected, a `union` container is emitted
* If the only item in a `RegisterBlock` is a union, that `RegisterBlock` is emitted as a union
* Otherwise: we generate a name for the union field by either taking the shortest common prefix of the union's alternates or the shortest register name (depending on type name conflicts). If that doesn't work just pick an artificial name like `u1`.
* If the starting address offset of elements in a union are not all the same, we don't have a way to emit padding for them today.  We will emit a warning (and bad code) in that case (example below).  The one example of this I see in ATSAMD21 is due to missing `derivedFrom` support for registers; we're currently generating bad code for these anyway.  I have resolved in another branch that I'll turn into a PR once this one is landed.

```
WARNING: field Some(Ident("pmux1_1")) has different offset 177 than its union container 176
WARNING: field Some(Ident("pmux1_2")) has different offset 178 than its union container 176
```

Examples:

```
#[doc = "Real-Time Counter"]
pub mod rtc {
    #[doc = r" Register block"]
    #[repr(C)]
    pub union RegisterBlock {
        #[doc = "0x00 - Clock/Calendar with Alarm"]
        pub mode2: MODE2,
        #[doc = "0x00 - 16-bit Counter with Two 16-bit Compares"]
        pub mode1: MODE1,
        #[doc = "0x00 - 32-bit Counter with Single 32-bit Compare"]
        pub mode0: MODE0,
    }
```

```
    #[doc = r" Register block"]
    #[repr(C)]
    pub struct USART {
        #[doc = "0x00 - USART Control A"]
        pub ctrla: self::usart::CTRLA,
        #[doc = "0x04 - USART Control B"]
        pub ctrlb: self::usart::CTRLB,
        _reserved2: [u8; 4usize],
        #[doc = "USART Baud Rate"]
        pub baud: baud_union,
      ...
    }
    #[doc = "USART Baud Rate"]
    #[repr(C)]
    pub union baud_union {
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_usartfp_mode: self::usart::BAUD_USARTFP_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_fracfp_mode: self::usart::BAUD_FRACFP_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud_frac_mode: self::usart::BAUD_FRAC_MODE,
        #[doc = "0x0c - USART Baud Rate"]
        pub baud: self::usart::BAUD,
    }
```

Refs: #191 
#16



Co-authored-by: Wez Furlong <wez@wezfurlong.org>
Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
@wez
Copy link
Contributor Author

wez commented May 9, 2018

The cluster/union stuff has now been merged into master.
The samd21 SVDs have one known issue remaining with derivedFrom that causes the GPIO pin configuration and mux registers not to be generated correctly.
#193 is open for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants