CPU/Boards: Add Raspberry Pi Pico 2 (RP2350) support #21545
Conversation
|
Also Murdock is "failing" because of the example that will not be merged, please ignore that 😛 |
There was a problem hiding this comment.
Some mostly minor style comments. Speaking of comments, a lot of comments are still in the // style, which we don't use: https://github.com/RIOT-OS/RIOT/blob/master/CODING_CONVENTIONS.md#comments
Some defines and functions are not documented yet, but I'll have to review what Doxygen complains about. Also I have not checked the generated Doxygen documentation yet.
There was a problem hiding this comment.
I think RIOT already has a library of atomic commands. Perhaps @maribu knows more about that?
There was a problem hiding this comment.
I did not know that this was something universal, the more you know
There was a problem hiding this comment.
Oh are you referring to these? https://github.com/AnnsAnns/RIOT/blob/cac44edec7c7ccdc7ea618aef27c4335490b26d1/cpu/rpx0xx/include/io_reg.h#L54
There was a problem hiding this comment.
No, I had this in mind: https://github.com/RIOT-OS/RIOT/tree/master/sys/include/atomic_utils.h
Sorry for not posting the link initially.
There was a problem hiding this comment.
I wont lie, that header is a bit overwhelming and complicated 😰 Is it something where you still need to write the functions for or is this just something that works out of the box.
There was a problem hiding this comment.
It will work out of the box and provides atomic access, assuming all (potentially) concurrent accesses go via those functions.
There was a problem hiding this comment.
Here is the doc btw: https://doc.riot-os.org/group__sys__atomic__utils.html
There was a problem hiding this comment.
If I understand this correctly this would mean Id also have to adjust the shared UART code in the followup so Id rather avoid it and simply use the same method signatures that the rp2040 uses for atomic operations
|
After investigating the feasibility of using shared components such as UART I have decided that it probably makes more sense to limit the scope of this PR. (Also mentioned here: #21545 (comment)) The idea of this PR is to add minimal support, and then later in a followup PR add support for the shared components such as the RP2040 UART Driver. Doing this in this PR would most likely heavily impact reviewability, which I'd like to avoid, esp. when moving to a shared component would require major adjustments to some parts of the code. |
There was a problem hiding this comment.
No, I had this in mind: https://github.com/RIOT-OS/RIOT/tree/master/sys/include/atomic_utils.h
Sorry for not posting the link initially.
|
If you want to, you can also squash the comments down a bit. Just make sure not to accidentally squash in stuff you don't want to keep 😅 |
Squashed everything before the PR cherry-pick (otherwise rebasing would probably become hell) |
|
You can rebase this now :) |
|
Some of the files have the wrong indentation width (two spaces instead of four). I think VS Code can automagically fix that for you. |
VSCode was really fighting back against me creating rubbish formatting and trying to gaslight me into believing that 2 spaces are actually 4 spaces in some files but I think Ive fixed all of the indentation issues now, ty for noticing |
|
I think this had some unintended side-effects: All the block comments are now incorrectly formatted 👀 |
fixed |
Squashing time 😛 |
|
Thank you for the countless reviews and patches over the last months 🥳 |
|
You're welcome :) |
Contribution description
This PR introduces the Raspberry Pi Pico 2 to Riot.
Currently the periph list is rather limited, it has GPIO and non-configurable extremely basic write only UART support. It does however, already configure the clocks using the XOSC and works well within the RIOT Ecosystem.
It supports OpenOCD, which I am currently using and (as introduced in #21269) Picotool for debugging/flashing. (And JLink though I did not test this)
One other big note is that we currently only support the Cortex M33 cores of the Pico 2, however, most of the code should reusable when adding support for the Pico 2s RISC-V cores.
There is still a lot to be done, the documentation is still "limited", however, I felt like it would make more sense to at least open the PR itself before continuing. Esp. to avoid potentially duplicated efforts from occurring.
I also took the liberty to already use #21515 for the license headers. I am not sure whether this is already the consensus.
Testing procedure
To showcase the current features I included a very minimal example, this is not intended to get merged but merely to showcase what is available as of now.
Issues/PRs references