Skip to content

"overloaded" registers #16

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

Closed
japaric opened this issue Oct 20, 2016 · 15 comments
Closed

"overloaded" registers #16

japaric opened this issue Oct 20, 2016 · 15 comments
Milestone

Comments

@japaric
Copy link
Member

japaric commented Oct 20, 2016

File: STM32F30x.svd
Peripheral: TIM2

The SVD file specifies that two registers exist at offset 0x1C: CCMR2_Input and CCMR2_Output. The thing is that this register can be used differently depending on whether the TIM2 peripheral is configured in Output Compare Mode or in Input Compare Mode. By differently, I mean that the bits of this register have different meaning depending on the compare mode.

I'm not sure how to model this. We could use a union here but that's not stable. For now, I think, I'm just going to pick one register and ignore the other but emit a warning.

japaric pushed a commit that referenced this issue Oct 20, 2016
@cwoodall
Copy link

One approach would be to make both registers available but disable any constraints that would make read or write access to the register safe.

@etrombly
Copy link

etrombly commented May 11, 2017

Having a similar issue with an STM32F103. The MODE register of a GPIO configures it for input or output, the CNF register handles the configuration (open-drain, push-pull, etc) but the settings are different depending on the mode.

edit: not entirely the same issue, this could be handled by commands having aliases, the register is still the same.

@japaric japaric added this to the cmsis-svd milestone May 25, 2017
@silven
Copy link

silven commented Jun 27, 2017

I think I've run into this as well.

Trying to use the generated file from: https://github.com/nicholastmosher/lpc176x5x-rs/blob/master/src/uart.rs which has a register called "DLL", but because it overlaps with the "RBR" register at offset 0x0, it cannot be used at all.

The SVD file used (https://github.com/raw/posborne/cmsis-svd/aa4721af946a253d18c8737b01d23e9c88a42e84/data/NXP/LPC176x5x_v0.2.svd) annotates this with "alternateRegister".

So, "+1" I guess.

@protomors
Copy link

protomors commented Jun 28, 2017

To @etrombly. I solved this problem by changing field definitions in SVD file. CNF and MODE bits for the same pin are adjacent so instead of separate 2-bit fields I declared them as one 4-bit field. And then created enum for them according to the "Table 20. Port bit configuration table" in the reference manual. So now I can select mode with single call like "output10pp" (Output, push-pull, max speed 10MHz) or "input_floating".

But there are cases where such solution is not possible. I think that in situation like example with timers it will be best to generate API for both registers (maybe just print a warning about it during generation). And then programmer can use appropriate API depending of the mode.

Just checked SVD file for stm32f103. Registers CCMR1_Input and CCMR1_Output even have attribute alternateRegister which exists exactly for such cases. So I think that in cases where registers explicitly marked as alternate variants using this attribute svd2rust can give access to both. And in other cases overlapping registers should be considered error in SVD file.

@pftbest
Copy link
Contributor

pftbest commented Jun 29, 2017

SVD files I am generating for MSP430 are also affected by this issue, but since the overlapping registers are not in the same peripheral, the svd2rust does not complain about them.

@japaric
Copy link
Member Author

japaric commented Jun 29, 2017

@pftbest

but since the overlapping registers are not in the same peripheral,

Could you elaborate on this? How can they be overlapping if the registers are in different peripherals? What would be an example of this in the SVD file you linked?

@pftbest
Copy link
Contributor

pftbest commented Jun 30, 2017

How can they be overlapping if the registers are in different peripherals?

On MSP430 the peripheral (module) is a logical group of registers, not physical group. Each register of a peripheral may be placed in some random location in memory. In practice almost all peripherals overlap with each other, but it's not a problem since they have a different combination of registers and reserved fields inside.

But there are some peripherals that do contain overlapping registers, for example
USCI_B0_SPI_Mode and USCI_B0_I2C_Mode

@idubrov
Copy link
Contributor

idubrov commented Sep 19, 2017

Adding some context here (as I keep stumbling at ccmr1_input/ccmr1_output registers on STM32 F103 chip).

First, unions are in Rust 1.19: rust-lang/rust#42068

However, using them at this point would require breaking the API as anonymous unions are not supported (i.e, instead of tim1.ccmr1_output(...) / tim1.ccmr1_input.write(...) access pattern would be like tim1.ccmr1_output.ccmr1_output.write(...) / tim1.ccmr1_output.ccmr1_input.write(...) (first ccmr1_output is the name of the group). Anonymous unions are currently discussed here.

@rnestler
Copy link
Contributor

I think this currently bites me as well with the LPC11Uxx:

<register>
	<name>RBR</name>
	<description>Receiver Buffer Register. Contains the next received character to be read. (DLAB=0)</description>
	<addressOffset>0x000</addressOffset>
	<access>read-only</access>
	<resetValue>0</resetValue>
	<resetMask>0x00000000</resetMask>
	<readAction>modify</readAction>
	<fields>
		<field>
			<name>RBR</name>
			<description>The USART Receiver Buffer Register contains the oldest received byte in the USART RX FIFO.</description>
			<bitRange>[7:0]</bitRange>
		</field>
		<field>
			<name>RESERVED</name>
			<description>Reserved</description>
			<bitRange>[31:8]</bitRange>
		</field>
	</fields>
</register>
<register>
	<name>THR</name>
	<description>Transmit Holding Register. The next character to be transmitted is written here. (DLAB=0)</description>
	<alternateRegister>RBR</alternateRegister>
	<addressOffset>0x000</addressOffset>
	<access>write-only</access>
	<resetValue>0</resetValue>
	<resetMask>0x00000000</resetMask>
	<readAction>modify</readAction>
	<fields>
		<field>
			<name>THR</name>
			<description>Writing to the USART Transmit Holding Register causes the data to be stored in the USART transmit FIFO. The byte will be sent when it is the oldest byte in the FIFO and the transmitter is available.</description>
			<bitRange>[7:0]</bitRange>
		</field>
		<field>
			<name>RESERVED</name>
			<description>Reserved</description>
			<bitRange>[31:8]</bitRange>
		</field>
	</fields>
</register>

The receive and transmit buffer register are mapped to the same address. Here it would be actually the best to just have both available.

@wez
Copy link
Contributor

wez commented Mar 14, 2018

I have some code that does something about this using unions. #192 (comment) has an example of how this looks. If you have strong opinions on naming/modeling, or if there are really bizarre situations that need to be accounted for, I'd love to hear them.

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.
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 <[email protected]>
Co-authored-by: Emil Gardström <[email protected]>
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 <[email protected]>
Co-authored-by: Emil Gardström <[email protected]>
@rnestler
Copy link
Contributor

rnestler commented Jun 4, 2018

I have some code that does something about this using unions. #192 (comment) has an example of how this looks. If you have strong opinions on naming/modeling, or if there are really bizarre situations that need to be accounted for, I'd love to hear them.

I think for the LPC11uxx use case above a union isn't really the right thing, since the read and write register are unrelated. So we could just generate a read and a write struct for each and it would be fine.
I'd be willing to implement support for that in svd2rust, so that if we have exactly one read and one write representation of a register we'll just generate them without a union.

@dbrgn
Copy link

dbrgn commented Jun 14, 2018

@japaric would the approach proposed by @rnestler be sensible? solving this issue is a requirement for properly supporting MCUs like the NXP LPC11Uxx.

@Emilgardis
Copy link
Member

Emilgardis commented Jun 14, 2018 via email

@rnestler
Copy link
Contributor

What does this mean? How are they unrelated

The Tx and Rx register for example use the same address. But writing to the Tx register doesn't influence the Rx register at all, they are just mapped to the same address.
So one can't write to the Rx register and one can't read from the Tx register.

@burrbull
Copy link
Member

burrbull commented Nov 7, 2022

Closing as already supported

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

No branches or pull requests