Factor common GPIO code out of stm32{g0,h7}#416
Conversation
93f9496 to
2c0b6b8
Compare
2c0b6b8 to
753727b
Compare
| do_configure!(gpio, pins, packed_attributes); | ||
| } | ||
| } | ||
| unsafe { get_gpio_regs(port) }.configure(pins, packed_attributes); |
There was a problem hiding this comment.
General design question - why is getting the GPIO registers unsafe, but writing to them isn't?
This seems like the opposite of normal, e.g. getting a raw pointer is safe but dereferencing it is unsafe.
There was a problem hiding this comment.
siiiiiiiiigh
So.
The family of APIs that grew up around svd2rust decided, early on, to try to manage peripheral ownership using little "tokens" that can't be duplicated. You do Peripherals::take() at boot and it gives you a struct of tokens, and you can access peripherals through those tokens. This approach is useful in some contexts.
We don't use it, though, because it assumes that all code is in the same address space and is simply unsound in unprivileged isolated mode.
This means we're sort of on the gray areas of the svd2rust-generated APIs. In particular, they decided that any access to peripherals outside of this token scheme would be unsafe. This is an abuse of unsafe to mean something other than unsafe -- technically there are peripherals one could poke that could violate memory safety, like anything capable of remapping memory or a DMA controller, but they are exceptions and the GPIO peripheral is certainly not among them.
So, a side effect of their decision is that there's no static GPIOA item defined, to prevent "ambient access" to the GPIOA peripheral bypassing the token scheme. Instead, all they'll do is give us a raw pointer from the GPIOA::ptr() call, which is the only call we can make without holding one of their tokens. We have to indirect it to get at the actual RegisterBlock, which is unsafe-in-the-Rust-sense.
Now, in this case, why did I also make get_gpio_regs unsafe, rather than hiding the unsafe within it? I could go either way on that.
There was a problem hiding this comment.
Got it, that all makes sense. I don't feel strongly about this, so matching svd2rust's decisions (however misguided) seems reasonable.
There was a problem hiding this comment.
(This is related to the reason why accessing raw bits is unsafe on some registers and not others. Basically the default is to make any bit manipulation unsafe; someone has to go through and edit a giant YAML file to mark operations as ok to do from safe code. People have not done that for our processors. It gets really fun when they've done it for some of our processors and not others, which gets us "unnecessary unsafe" warnings when compiling for some targets. This is part of why I'm concerned that our use case is diverging too far from what the PAC authors intend.)
753727b to
f58f8ee
Compare
f58f8ee to
a27ba32
Compare
This is a step toward reducing the overall high level of code duplication between stm32 targets. It exploits some traits that the PACs started generating circa 0.14, which are unfortunately not put into a common crate, but, hey, at least they exist now. Knocks some bytes (100+) off GPIO servers by reducing macro-driven code duplication and giving the compiler an easier task. This is sort of a HAL, but unlike most HALs it's very careful to allow static monomorphization on properties you know at compile time (like your SoC model target) and only use dynamic dispatch for things you vary at runtime (like which gpio port you're poking). Scary bits are mostly hidden in the server.rs file.
a27ba32 to
2e7f3dd
Compare
This is a step toward reducing the overall high level of code
duplication between stm32 targets. It exploits some traits that the PACs
started generating circa 0.14, which are unfortunately not put into a
common crate, but, hey, at least they exist now.
Knocks some bytes (100+) off GPIO servers by reducing macro-driven code
duplication and giving the compiler an easier task.
This is sort of a HAL, but unlike most HALs it's very careful to allow
static monomorphization on properties you know at compile time (like
your SoC model target) and only use dynamic dispatch for things you vary
at runtime (like which gpio port you're poking).
Scary bits are mostly hidden in the server.rs file.