-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
rp2: Add support for new RP2350 MCU. #15619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Code size report:
|
c8a882b
to
48e53eb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15619 +/- ##
=======================================
Coverage 98.57% 98.57%
=======================================
Files 164 164
Lines 21336 21336
=======================================
Hits 21031 21031
Misses 305 305 ☔ View full report in Codecov by Sentry. |
Pre-built firmware can be found here: https://micropython.org/download/RPI_PICO2/ |
e20b042
to
eed9e22
Compare
Build size, or build time? Do we need to set |
Build/firmware size. Setting |
It looks like this is due to the new always-on timer (aon timer) code, which calls It doesn't look easy to remove all that code even though it's probably never used by MicroPython. So we may have to live with it. |
Hmm so It might be possible to provide our own target_link_options(${MICROPY_TARGET} PRIVATE "LINKER:--wrap=localtime_r") struct tm* __wrap_localtime_r(time_t *time, struct tm *local_time) {
return gmtime_r(time, local_time);
} Which I think resolves to Edit: See: dpgeorge/micropython@rp2-add-rp2350...pimoroni:micropython:patch-localtime_r |
Good idea.... but I tried your patch and it did not work, it only reduced firmware size by about 1k. We need to reclaim 17k. Looking at |
Ah, yes, this might be a vote against using the aon_timer API on RP2040 after all - the POWMAN timer is not date based, but the RTC one is, so one of them takes the hit for a common API :-( ... we should probably add both forms for both though (and maybe a documentation warning) |
Here's an extended version of @Gadgetoid's patch that gets the code size all the way down (PICO_W binary size is actually slightly smaller than the current master). I have verified this compiles and boots but I have not tested it further: (Two tips for anyone who is interested: 1. Linker wrap is actually not necessary for any symbol that is linked from a library, as any object provided on the linker command line will take precedence. Great blog post on the details. 2. The --cref linker option is worth its weight in gold for figuring this stuff out. I will submit a PR to add it either to pico-sdk or rp2 port linker command lines.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an impressively small and clean set of changes for adding not just a new chip but also a new CPU arch! Kudos to the pico-sdk team and everyone who worked on this PR.
I've said it once, I'll say it again, Open Source is just the formal version of posting the wrong answer on the internet to get the right one 😆 thank you! I've just run our batteries-included builds against this branch and that extra 17k blows our flash budget 😬 |
Hey. I just wanted to chime in here to point out that one of our PGA2350 users (which uses the B chip) was not able to use PWM on the pins above 31. Checking this repo, it seems |
Afacit PWM just needs this: pimoroni@7f1fcc7 I'll cut some builds to test it! PGA builds here should include the fix: https://github.com/pimoroni/pga/actions/runs/10391381219 Edit 2: Afaict this fix works. @dpgeorge if you don't mind casting your eye over it and cherry-picking into your RP2350 feature branch, I can rebase accordingly. Thank you! |
Thanks @Gadgetoid . Looks like there's at least one hardcoded slice count left: void machine_pwm_deinit_all(void) {
for (int i = 0; i < 8; i++) {
slice_freq_set[i] = false;
pwm_set_enabled(machine_pwm_obj[i].slice, false);
}
} |
Good spot! Curse the magic numbers! Pushed a fix. Edit: Updated the link above, but for clarity the patch is now: pimoroni@7f1fcc7 Our very own Hodgy has a PGA2350 set up with all of the PWM (it looks like an electronic mangrove) and confirmed this works. |
Built > mpremote
Connected to MicroPython at /dev/ttyACM0
Use Ctrl-] or Ctrl-x to exit this shell
MicroPython v1.24.0-preview.203.gb9f6698d30.dirty on 2024-08-23; Raspberry Pi Pico2 with RP2350
Type "help()" for more information.
>>> from machine import Pin
>>> # Check user LED
>>> led = Pin(25, Pin.OUT)
>>> led.toggle()
>>> # Check RGB LED
>>> power_pin = Pin(23, Pin.OUT) # Provide power first
>>> power_pin.value(1)
>>> from neopixel import NeoPixel
>>> neo = NeoPixel(Pin(22), 1)
>>> neo[0] = (0, 0, 40)
>>> neo.write() # Blue LED |
b9f6698
to
89b17d1
Compare
Thanks for that, I've now added that commit to this PR. |
The decision is to stick with |
Looks like I have some PSRAM rebasing to do! Will sure make it easier to play with the new UART/PPP stuff 😁 (I know you’re probably still working your way down here, but don’t miss the PWM patch above! - pimoroni@7f1fcc7 ) |
Might be my board config, but I run into this when trying to build for RP2350B or
Edit: Might need to include Edit2: Looks like nrf uses
Since this table is defined in <BOARD_NAME>_pins.c (included in the CMakeLists) but not declared anywhere. Oh also:
The following heinous patch gets things building for me again: diff --git a/ports/rp2/CMakeLists.txt b/ports/rp2/CMakeLists.txt
index 5e69da4b8..9dc5dbc94 100644
--- a/ports/rp2/CMakeLists.txt
+++ b/ports/rp2/CMakeLists.txt
@@ -538,6 +538,12 @@ set_source_files_properties(
COMPILE_OPTIONS "-O2"
)
+set_source_files_properties(
+ rp2_pio.c
+ PROPERTIES
+ COMPILE_OPTIONS "-Wno-error=discarded-qualifiers"
+)
+
set_source_files_properties(
${PICO_SDK_PATH}/src/rp2_common/pico_double/double_math.c
${PICO_SDK_PATH}/src/rp2_common/pico_float/float_math.c
diff --git a/ports/rp2/rp2_pio.c b/ports/rp2/rp2_pio.c
index 8e91c1d8f..0d8854715 100644
--- a/ports/rp2/rp2_pio.c
+++ b/ports/rp2/rp2_pio.c
@@ -32,11 +32,15 @@
#include "py/mphal.h"
#include "shared/runtime/mpirq.h"
#include "modrp2.h"
+#include "machine_pin.h"
+#include "genhdr/pins.h"
#include "hardware/clocks.h"
#include "hardware/irq.h"
#include "hardware/pio.h"
+extern const machine_pin_obj_t machine_pin_obj_table[NUM_BANK0_GPIOS];
+
typedef struct _rp2_pio_obj_t {
mp_obj_base_t base;
PIO pio; |
This commit separates various build settings and include files that are specific to RP2040 and RP2350, and uses the aon_timer interface instead of rtc, to work across both MCU variants. Signed-off-by: Damien George <damien@micropython.org> Signed-off-by: Phil Howard <phil@gadgetoid.com>
Update NUM_GPIOS to match NUM_BANK0_GPIOS, and increase bit-width of variables that store pin numbers. Signed-off-by: Phil Howard <phil@gadgetoid.com>
So other code can include `machine_pin.h` and use the pin name macros such as `pin_GPIO0`. Signed-off-by: Damien George <damien@micropython.org>
Add support for 32 and 48 pin variants of RP2350. Add new `PIO.gpio_base()` method, mirroring the Pico SDK. Signed-off-by: Phil Howard <phil@gadgetoid.com> Signed-off-by: Damien George <damien@micropython.org>
NUM_GPIOS amd NUM_EXT_GPIOS are currently hardcoded in make-pins.py, which makes it difficult to support SoCs with different pin count. This commit generalises make-pins.py by passing in the pin count in via the new arguments `--num-gpios` and `--num-ext-gpios`. These default to the current values supported by Pico, namely 30/10. This can be changed with PICO_NUM_GPIOS and PICO_NUM_EXT_GPIOS in `mpconfigboard.cmake`. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Fix the gpio_irq function so that it looks at all six iobank0_hw->intr[n] registers, for up to 48 IOs. Signed-off-by: Phil Howard <phil@gadgetoid.com>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
As part of this change, the RV32I native emitter is enabled on RISCV board variants. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
In case it doesn't have the correct value. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Provide stub implementations of localtime_r() and mktime() to avoid code size increase. Reported upstream at raspberrypi/pico-sdk#1810 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Signed-off-by: Phil Howard <phil@gadgetoid.com>
Signed-off-by: Damien George <damien@micropython.org>
This is the same form-factor as the Pico but with an RP2350. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
To test construction of UART instances. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Otherwise a previous value of `timeout_char` may be left over after a soft reset. Signed-off-by: Damien George <damien@micropython.org>
71de1eb
to
51663b9
Compare
This is finally merged! Thanks to everyone here who contributed/helped/tested/reviewed. |
Summary
This PR adds support for the new RP2350 MCU, and the new Pico 2 board.
Both the Cortex-M33 and RISC-V CPUs are supported. You can build for them both (as separate firmware) with:
Download the .uf2 to the board as usual via mass storage. The bootloader will detect which architecture (ARM or RISC-V) the firmware has and boot the correct CPU(s).
See https://www.raspberrypi.com/products/rp2350/
Thanks to @peterharperuk and @Gadgetoid for doing a lot of the work in this PR.
Testing
The existing test suite (mostly) passes on the Pico 2, in both ARM and RISC-V mode. There are some issues with lightsleep mode on ARM and machine-idle on both ARM and RISC-V which need to be sorted out.
TODO
fix lightsleepwill do at a later date when there's a new version of pico-sdk with required improvementslocaltime_r
andmktime
test other TinyUSB portsno longer needed, TinyUSB already updated in lib/tinyusb: Update TinyUSB to release 0.17.0 #15902RPI_PICO2
vsRPI_PICO_2
(latter matchesRPI_PICO_W
, and officially it'sPico 2
) -> go withRPI_PICO2
to match pico-sdk headerpico2.h