CPU/Boards: Add Raspberry Pi Pico 2 (RP2350) support#21545
CPU/Boards: Add Raspberry Pi Pico 2 (RP2350) support#21545crasbe merged 2 commits intoRIOT-OS:masterfrom
Conversation
AnnsAnns
requested review from
MrKevinWeiss,
bergzand,
jia200x,
kaspar030 and
leandrolanzieri
as code owners
github-actions
bot
added
Platform: ARM
crasbe
added
Type: new feature
|
Also Murdock is "failing" because of the example that will not be merged, please ignore that 😛 |
crasbe
left a comment
crasbe
left a comment
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.
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.
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
AnnsAnns
changed the title
|
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. |
crasbe
left a comment
crasbe
left a comment
There was a problem hiding this comment.
Again, mostly small stuff.
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) |
crasbe
removed
the
State: waiting for other PR
|
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. |
github-actions
bot
removed
Area: pkg
crasbe
removed
the
State: needs rebase
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 |
crasbe
left a comment
crasbe
left a comment
There was a problem hiding this comment.
This one block got missed. But other than that I think we went through everything enough times.
Do you still have anything left to do or is this ready?
github-actions
bot
removed
the
Area: CI
Squashing time 😛 |
|
Thank you for the countless reviews and patches over the last months 🥳 |
|
You're welcome :) |
6 participants
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