stm32-rs icon indicating copy to clipboard operation
stm32-rs copied to clipboard

Add STM32H7Rx/Sx

Open jspngh opened this issue 1 year ago • 13 comments

I'd like to add support for these newly released devices.

However, I am unsure if they should be added to the STM32H7 family or added as a separate family. On their site, ST lists them under the "STM32H7" category, but for the rest they seem to treat them like a new family (with a separate Cube package and HAL). What do you think?

One annoying thing about these new SVD's, is that they tend to use the bit position of a field as the name.

Example
              <field>
                <name>UE</name>
                <description>USART enable ...</description>
                <bitOffset>0</bitOffset>
                <bitWidth>1</bitWidth>
                <access>read-write</access>
                <enumeratedValues>
                  <enumeratedValue>
                    <name>B_0x0</name>
                    <description>USART prescaler and outputs disabled, low-power mode</description>
                    <value>0x0</value>
                  </enumeratedValue>
                  <enumeratedValue>
                    <name>B_0x1</name>
                    <description>USART enabled</description>
                    <value>0x1</value>
                  </enumeratedValue>
                </enumeratedValues>
              </field>

I'm not sure how to solve this without manually overwriting the values with meaningful names.

jspngh avatar Mar 29 '24 11:03 jspngh

just delete all those enums with "_clear_fields"

https://github.com/stm32-rs/stm32-rs/blob/bc29551e979aa19b5d85024b4dfaa68b884248a2/devices/stm32g071.yaml#L6

burrbull avatar Mar 29 '24 12:03 burrbull

just delete all those enums with "_clear_fields"

Thanks, that seems like the best option.

Any opinion on where they belong (in STM32H7 or a separate family)?

jspngh avatar Mar 29 '24 14:03 jspngh

I think keeping them in the stm32h7 crate is probably best, we also kept the stm32l4+ in the l4 crate and I think it's a similar distinction.

It doesn't really matter what they have in common as nothing is shared between devices in a crate anyway; it's just about keeping each crate a reasonable size and having things where people expect to find them.

adamgreig avatar Mar 29 '24 14:03 adamgreig

I think keeping them in the stm32h7 crate is probably best, we also kept the stm32l4+ in the l4 crate and I think it's a similar distinction.

As far as I can see, ST uses the same cube package and hal for L4 and L4+, so it's slightly different for these chips. But I'm perfectly fine with keeping them in the stm32h7 crate.

I guess we can always split them off later if necessary?

jspngh avatar Mar 29 '24 14:03 jspngh

The "compare mmaps" step will continue to fail because of the new SVD files, I guess?

jspngh avatar Mar 29 '24 20:03 jspngh

Yep, for security reasons it can only run on the SVD files already present in the repository. Do you think this is otherwise ready for review? Have you been able to test it at all?

adamgreig avatar Mar 29 '24 20:03 adamgreig

Yep, for security reasons it can only run on the SVD files already present in the repository. Do you think this is otherwise ready for review? Have you been able to test it at all?

https://github.com/stm32-rs/stm32-rs/pull/949

burrbull avatar Mar 29 '24 20:03 burrbull

Do you think this is otherwise ready for review? Have you been able to test it at all?

It's definitely not perfect yet, but I hope it is a good enough starting point to be mergeable. I'm going to start with a blinky/hello world now, so we could wait for that before merging.

I don't expect big issues with getting something to run, but maybe those are famous last words :sweat_smile:

jspngh avatar Mar 29 '24 20:03 jspngh

A simple blinky works. I'll start working with some of the peripherals by trying to add support to stm32h7xx-hal for these devices, but that will probably take quite some time.

@adamgreig, is there anything you would like to see checked or tested in particular or are you okay with merging this now and then fixing potential issues in a follow-up PR?

jspngh avatar Mar 30 '24 09:03 jspngh

Thanks, looks good. The only thing I wanted to double check is deriving all the GPIOs from GPIOA. Typically GPIOA (and B) have a different reset value for MODER, OSPEEDR, and PUPDR than the other ports because of the JTAG/SWD pins there. IF everything derives from GPIOA, doing something like gpioc.moder.write(|w| w) would incorrectly set PC13/14/15 to alternate function. You can see devices/common_patches/gpio/f3_reset_values.yaml for an example of handling this where there's GPIOA, GPIOB, and GPIOC, and all other ports derive from GPIOC.

adamgreig avatar Mar 30 '24 17:03 adamgreig

Thanks, looks good. The only thing I wanted to double check is deriving all the GPIOs from GPIOA. Typically GPIOA (and B) have a different reset value for MODER, OSPEEDR, and PUPDR than the other ports because of the JTAG/SWD pins there. IF everything derives from GPIOA, doing something like gpioc.moder.write(|w| w) would incorrectly set PC13/14/15 to alternate function. You can see devices/common_patches/gpio/f3_reset_values.yaml for an example of handling this where there's GPIOA, GPIOB, and GPIOC, and all other ports derive from GPIOC.

You are correct, that is indeed the case (the different reset value), I didn't know it mattered. I'll fix it!

jspngh avatar Mar 30 '24 18:03 jspngh

I was looking into this and comparing with the other STM32H7 devices. I noticed this issue is present there as well: all GPIOs are derived from GPIOA, but not all reset values are the same.

I propose to fix this for the entire family, but maybe that should be a separate PR? And just to check, this means that each "different reset value" will result in a new gpiox module with distinct RegisterBlock, right?

jspngh avatar Mar 31 '24 09:03 jspngh

I created #973 to fix the issue for the current STM32H7 devices. When that is merged, I can rebase this PR and fix it in the same way for the new devices.

jspngh avatar Apr 01 '24 08:04 jspngh

Rebase, please.

burrbull avatar Jul 08 '24 15:07 burrbull

Rebase, please.

Done

jspngh avatar Jul 10 '24 05:07 jspngh