Use the GPIO trait on STM32F4's pin #325
Conversation
|
Can one of the admins verify this patch? |
|
test this please |
| } | ||
|
|
||
| /// Sets output GPIO direction. | ||
| fn set_direction(&self, new_mode: GpioDirection) { |
There was a problem hiding this comment.
You've got some 4-space indenting in here. Until we decide to go through and refactor the whole project (which will make git blame useless) we'll stick to the 2-space existing standard.
There was a problem hiding this comment.
Sorry about that! I'll fix the indentation now
|
This also could be as good a time as any to try and roll over to the new |
|
The indentation should be fixed now. |
| /// Sets output GPIO direction. | ||
| fn set_direction(&self, new_mode: GpioDirection) { | ||
| // TODO: Verify that this works | ||
| // TODO: Change the Pin.function field to the new mode |
stm32f4: switch GPIO to new ioregs macro
|
retest this please |
|
LGTM. Thanks for the contributions! |
Modernize STM32F4 GPIO implementation
| reg_rw!(GPIO, u32, LCKR, set_LCKR, LCKR); | ||
| reg_rw!(GPIO, u32, AFRL, set_AFRL, AFRL); | ||
| reg_rw!(GPIO, u32, AFRH, set_AFRH, AFRH); | ||
| ioregs!(GPIO = { |
There was a problem hiding this comment.
we should do placement code and get rid of linker part too, really. Does it make sense to add multiple placement definitions for a case like this or we're looking for some svd for stm32f4 to sort it for us?
There was a problem hiding this comment.
@farcaller do you mean the #[link_name="..."] / iomem.ld part? Which alternatives are there? Maybe we should track this in an RFC issue.
There was a problem hiding this comment.
#300 added placement getters, used like that: https://github.com/farcaller/zinc/blob/lpc111x/src/hal/lpc11xx/regs.rs#L25
There was a problem hiding this comment.
ah nice, did not see this. I just started to wrap my head around the codebase. So the placement getters still need some documentation, don't they? I just started a "new" empty port of the stm32f4 (just to get to know all the traits and structs). I hope I can document all obscurities on the way.
There was a problem hiding this comment.
Yes, they aren't mentioned in the ioreg crate docs actually. If you'll revive the stm32f4 codebase it would be awesome.
There was a problem hiding this comment.
yes seems reasonable. At least NXP seems to be very permissive with their SVD files.
There was a problem hiding this comment.
The other approach I see is to generate "terrible" code from the SVDs as a starting point and do one of two things from there:
- Change the generator to patch the intermediate representation of the SVD (which I believe is free from any licensing problems). This patching is probably vendor specific to deal with that vendors particular deficiencies.
- Have the SVD transformation be a one-time thing and patch the resulting
ioregs!by hand.
There was a problem hiding this comment.
I just discovered that the SVD for the STM32F4 also has some strange quirks. I'm going with option 2 for now. I'll just take the register definitions needed to implement the HAL and fix any quirks by hand. This way one saves a lot of work, but also allows easy fixing. Option 1 on the other hand is great if the ioregs! macros change in the future.
There was a problem hiding this comment.
There are very little cases not covered by ioregs I think? I'd vouch for reasonable changes to SVD xml to make ioregs readable and store SVDs in tree if it aligns with the license.
There was a problem hiding this comment.
yes, if the license permits modifications and the generated definitions are ok, it makes sense. But as far as I understood the STM license does not permit modifications. So we have MCUs where we can use and fix SVDs and we have MCUs where we have to do it more selectively.
This changes the GPIO interface for the STM32F4 so that it uses the GPIO trait.
set_directiononly partially works as it does not update thePin's internalfunctionvariable to the new mode. Theset_directionfunction would need to take a&mut selffor this to happen.