Repository navigation
Add support for atomic xor/clear/set register operations for Raspberry Pi RP2040 microcontroller #535
Description
As per https://datasheets.raspberrypi.org/rp2040/rp2040-datasheet.pdf "2.1.2. Atomic Register Access"
This looks vaguely similar to something that was done for the msp430. Unsure how common this is for cortex M registers across the board. Would this be some type of very specific target?
Reacted by ilovefurnaces
Activity
henkkuli commentedon Sep 30, 2021
I have described one possible implementation for this in a comment elsewhere, but I don't know how to gate that for specific registers or peripherals only. I tried to look at the corresponding SVD file but haven't found any indication there that some registers should be atomic*. I also couldn't find a way to encode this info in a SVD file, at least based on a surface look at svd_parser. Does anyone here have an idea how to mark only some peripherals/registers as atomic? Maybe have another file listing all atomic peripherals/registers? Or list each atomic peripheral as a flag?
*All registers except those belonging to SIO peripheral should be atomic.
burrbull commentedon Nov 5, 2021
As SVD doesn't have such info, the only way I see is to implement atomic operation for all registers, but make them
unsafeas it done forwrite_with_zero.henkkuli commentedon Nov 5, 2021
That would be unfortunate, but at least it would work.
By the way, I have a POC available in my fork if someone wants to take a look. Would something like that fly? I can try to finish a PR, but for that I need some help with how to gate the feature for RP2040 target only.
adfernandes commentedon Oct 18, 2024
FWIW, on the
RP235Xat least here's the definitive answer on "Which peripherals, specifically, support atomic modification?"I'm trying to add this as a type-safe
traitright now...burrbull commentedon Oct 20, 2024
If those
writes are markedunsafedo we really need to restrict them?Lets just implement
xor/clear/setfor allRegs.jannic commentedon Oct 20, 2024
Just playing devils advocate: If
writes are markedunsafe, why do we bother with a PAC at all and don't just do volatile writes do raw addresses?There is some value in only providing write access to registers that actually exist. So just implementing xor/clear/set for all registers even if some of the registers don't provide that function would technically work, and wouldn't be unsound (due to
unsafe), but would slightly reduce the value the PAC provides.It might still be the right trade-off, if an exact implementation would be too much work, or would clutter the PAC implementations with lots of manual patches because the vendor SVDs don't provide the required information.
burrbull commentedon Oct 20, 2024
I did not say about ordinary writes, only about atomic:
svd2rust/src/generate/generic_atomic.rs
Line 51 in 29bcfd8
adfernandes commentedon Oct 20, 2024
Another problem with this approach is that it makes static analysis difficult if not impossible.
This is especially important since there is a huge push to use rust in safety critical applications including industrial robotics, automotive control, and avionics.
jannic commentedon Oct 20, 2024
Yes, I know. But with the same argument "it's unsafe anyway, so it doesn't matter if we implement it for some registers that don't actually implement it" one could also ask "why do we need a PAC at all, we can do unsafe register writes without it?".
Part of the answer is that PACs do not allow access to every arbitrary address, but only to registers that actually exist. This doesn't prevent you from writing to arbitrary addresses (you can still use
ptr::volatile_write), but it makes it less likely that you accidentally confuse addresses and write to some register that doesn't actually exist on your MCU.Therefore, I'm not sure which one of these two choices is better:
Sure, the second approach requires users to write a few more lines of code. But at the same time, it becomes obvious that they are doing something not directly supported by the PAC, so the caller needs to be careful whether it's actually allowed.
Of course, from the user's point of view, a third choice is better than both of the two above:
But that may be a lot of work for PAC authors, even if svd2rust provided support for it.
adfernandes commentedon Oct 21, 2024
I am in the process right now of trying to figure out how registers in the PAC can be annotated for atomic access, since that is unfortunately not something supported in the SVD, nor is it supported bt the
svdtoolspatcher.I do not think that the HAL is the correct place to house the atomic access API because it is not a hardware abstraction, it is a physical property of how the peripheral interacts with the bus… You can't get more PAC than that!
The atomic registers are real registers at a real address on a real bus, so I think there is a strong argument to treat them as first class citizens, no different than readable, writable, or read/writable.
burrbull commentedon Oct 21, 2024
#856 (comment)
romancardenas commentedon Oct 21, 2024
Something like this would also be great for RISC-V Zaamo targets, which only support AMO* instructions. We are currently dealing with this special situation with
portable-atomic.burrbull commentedon Oct 21, 2024
As #856 is merged you could try to add a section in
Settingsdescribing ranges.