cpu/rpx0xx: port RIOT to the Raspberry Pi RP2040 MCU by maribu · Pull Request #16609 · RIOT-OS/RIOT

maribu · 2021-07-05T08:15:14Z

Contribution description

This provides:

Support for the RP2040 MCU
- Only periph_gpio, periph_gpio_int, and periph_uart are provided in this PR. Other than that no timers, no SPI, no nothing is provided.
- Work on additional drivers can more easily be parallelized once basic support is upstream, which this PR tries to do.
Support for the Raspberry Pi Pico board (rpi-pico), the reference board for the RP2040 MCU, is provided

Acknowledgements

This code was developed as part of a software project in a combined effort of students and mentors. Thanks to @nickw96, @MaestroOnICe, and @Franz2000.

Most of the low level code and especially the boot sequence and clock configuration was developed by @fabian18.

Testing procedure

The following test applications will be useful:

examples/default
- stdio via UART should work
- Via SAUL the on-board LED should be controllable
tests/periph_gpio
- The GPIO pins should work as input (floating, with pull up, with pull down) and output and IRQs should be supported on rising, falling, and both edges

It would be useful to test all of the flashing options:

Using OpenOCD and pico-debug (currently the default option)
- Beware: First "flash" the gimmechace variant from here https://github.com/majbthrd/pico-debug/releases via the UF2 bootloader into RAM. (Needs to be re-"flashed" after each cold boot, or RAM corruption.)
Using J-Link
Using the UF2 bootloader

Issues/PRs references

~~None~~ closes #15822

cpu/rp2040/boot2_w25q080_padded_checksummed.S

benpicco

Nice, looking good!
Some first comments:

boards/rpi-pico/Makefile.features

boards/rpi-pico/include/board.h

cpu/rp2040/boot_stage2/boot2_generic_03h.S

cpu/rp2040/cpu.c

cpu/rp2040/include/periph_cpu.h

cpu/rp2040/periph/gpio.c

cpu/rp2040/periph/uart.c

cpu/rp2040/pll.c

cpu/rp2040/system_RP2040.c

maribu · 2021-07-06T07:26:52Z

@benpicco: May I do a first round of squashing?

benpicco · 2021-07-06T07:32:52Z

sure, squash away!

maribu · 2021-07-06T07:48:42Z

This is the diff from before the first squashing attempt and after the second attempt: https://github.com/RIOT-OS/RIOT/compare/86681da759ab8e2f57b3cb7fd2ba890f26148fc8..cafacf07845d3f1d0c6a3665943b75ebd55bbb21

(Sadly I incorrectly manually merged something on the first try.)

maribu · 2021-07-07T08:11:54Z

The CI is now happy. Time for another round of squashing?

benpicco

looks good!
What's the output of tests/periph_gpio's auto_test?

boards/rpi-pico/doc.txt

boards/rpi-pico/include/periph_conf.h

benpicco · 2021-07-07T22:03:57Z

cpu/rpx0xx/Makefile.include

@@ -0,0 +1,19 @@
+PICO_DEBUG      ?=
+ROM_LEN         ?= 2097152 # = 2 MiB used in the RPi Pico


since the MCU doesn't come with flash, shouldn't this always be set by the board?

It is likely that most boards just use the reference flash chip, as the 2nd stage bootloader will then just work. Not every flash ship supports XIP in the same way, requiring other flash chips to use a different 2nd stage bootloader.

The board can still overwrite this value, if it indeed used a different value. IMO having the "reference flash chip" as default is sensible.

cpu/rpx0xx/include/cpu_conf.h

cpu/rpx0xx/periph/uart.c

maribu · 2021-07-08T07:33:17Z

What's the output of tests/periph_gpio's auto_test?

$ make BOARD=rpi-pico flash test -C tests/periph_gpio
make: Entering directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'
There are unsatisfied feature requirements: periph_timer
/home/maribu/Repos/software/RIOT/tests/periph_gpio/../../Makefile.include:934: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.
make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'

:-/

cpu/rpx0xx/periph/uart.c

benpicco · 2021-07-08T11:35:37Z

What's the output of tests/periph_gpio's auto_test?

$ make BOARD=rpi-pico flash test -C tests/periph_gpio
make: Entering directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'
There are unsatisfied feature requirements: periph_timer
/home/maribu/Repos/software/RIOT/tests/periph_gpio/../../Makefile.include:934: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.
make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'

:-/

you could try #14553 to use SysTick for xtimer

fabian18 · 2021-07-08T13:12:02Z

What's the output of tests/periph_gpio's auto_test?

$ make BOARD=rpi-pico flash test -C tests/periph_gpio
make: Entering directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'
There are unsatisfied feature requirements: periph_timer
/home/maribu/Repos/software/RIOT/tests/periph_gpio/../../Makefile.include:934: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.
make: Leaving directory '/home/maribu/Repos/software/RIOT/tests/periph_gpio'

:-/

you could try #14553 to use SysTick for xtimer

I took this as an occasion to upload #16627.

cpu/rpx0xx/periph/uart.c

maribu · 2021-07-09T14:36:28Z

cpu/rpx0xx/periph/uart.c

+void uart_poweron(uart_t uart)
+{
+    uint32_t reset_bit_mask = (uart) ? RESETS_RESET_uart1_Msk : RESETS_RESET_uart0_Msk;
+    periph_reset(reset_bit_mask);
+    periph_reset_done(reset_bit_mask);
+}
+
+void uart_poweroff(uart_t uart)
+{
+    uart_deinit_pins(uart);
+    periph_reset((uart) ? RESETS_RESET_uart1_Msk : RESETS_RESET_uart0_Msk);
+}


Those are again wonderfully underspecified APIs. IMO, we should just drop uart_poweron(). Assume a UART is first initialized, then powered off, then power on again: Should it resume operation in the state before power off? Why should the driver need to sacrifice RAM to back up the configuration, when the user of the UART exactly knows all configuration details?

IMO, we should drop uart_poweron(). uart_init() powers on and initialized the UART, uart_poweroff() powers it off again. There is no use case for uart_poweron().

Oh, it is actually documented:

After initialization, the UART peripheral should be powered on and active. The UART can later be explicitly put to sleep and woken up by calling the uart_poweron() and uart_poweroff() functions. Once woken up using uart_poweron(), the UART should transparently continue it's previously configured operation.

Just not at the API of the functions. I still wonder why a low level driver is burdened with backing up the device state. Especially since in most case the compiler will not be able to optimize the RAM used for this out, even if it is never used.

benpicco · 2021-07-09T16:28:03Z

Please squash!

benpicco · 2021-07-10T10:48:40Z

keep squashing

maribu · 2021-07-12T06:39:44Z

cpu/rpx0xx/periph/uart.c

    dev->UARTFBRD.reg = baud_fbrd;

-    io_reg_atomic_set(&(dev->UARTLCR_H.reg), 0);
+    dev->UARTLCR_H.reg = 0;


This shouldn't be needed, actually. (This also explains why this worked with the previous no-op atomic |= 0 operation). Because:

0x0 is the reset value of the register. So after the reset which is triggered during uart_init(), it will have this value anyway

In uart_mode() the register is overwritten anyway

I'll just drop this.

maribu · 2021-07-12T07:40:56Z

I squashed again and added a fix for uart_poweron() / uart_poweroff(), the mixing pin access helper functions, as well as a corrected list of provided features .

I have to check with a logic analyzer to confirm that the uart_poweron() / uart_poweroff() is working and esoteric modes are correctly implemented.

Sadly, the UART is sending some garbage upon cold boot. (Warm boot it garbage-free.) And also a gargabe char is received upon boot (no matter if cold or warm boot). It would be nice to get rid of that, if possible.

maribu · 2021-07-13T13:11:15Z

Tested the UART using tests/periph_uart_mode with the following patch applied:

diff --git a/tests/periph_uart_mode/Makefile b/tests/periph_uart_mode/Makefile
index 04882515c0..329a1ac637 100644
--- a/tests/periph_uart_mode/Makefile
+++ b/tests/periph_uart_mode/Makefile
@@ -2,8 +2,7 @@ include ../Makefile.tests_common
 
 FEATURES_REQUIRED += periph_uart
 FEATURES_REQUIRED += periph_uart_modecfg
-
-USEMODULE += xtimer
+FEATURES_OPTIONAL += periph_timer
 
 # Set this to prevent welcome message from printing and confusing output
 CFLAGS+=-DLOG_LEVEL=LOG_NONE
diff --git a/tests/periph_uart_mode/main.c b/tests/periph_uart_mode/main.c
index d15df67ba0..b37d44796b 100644
--- a/tests/periph_uart_mode/main.c
+++ b/tests/periph_uart_mode/main.c
@@ -22,7 +22,9 @@
 #include <string.h>
 #include <stdlib.h>
 
+#include "board.h"
 #include "periph/uart.h"
+#include "periph_conf.h"
 #include "stdio_uart.h"
 #include "xtimer.h"
 
@@ -52,6 +54,25 @@
 /* Stores each mode string for printing at the end of the test */
 static char mode_strings[TOTAL_OPTIONS][MODE_STR_LEN];
 
+static void _delay(void)
+{
+    if (IS_USED(MODULE_XTIMER)) {
+        xtimer_usleep(DELAY_US);
+    }
+    else {
+        /*
+         * As fallback for freshly ported boards with no timer drivers written
+         * yet, we just use the CPU to delay execution and assume that roughly
+         * 20 CPU cycles are spend per loop iteration.
+         *
+         * Note that the volatile qualifier disables compiler optimizations for
+         * all accesses to the counter variable. Without volatile, modern
+         * compilers would detect that the loop is only wasting CPU cycles and
+         * optimize it out - but here the wasting of CPU cycles is desired.
+         */
+        for (volatile uint32_t i = 0; i < CLOCK_CORECLOCK / 20; i++) { }
+    }
+}
 
 static void _get_mode(const uart_data_bits_t data_bits,
                       const uart_parity_t parity, const uart_stop_bits_t stop_bits, char* mode_str) {
@@ -153,7 +174,7 @@ int main(void)
                 if (status == UART_OK) {
                     results[ridx] = true;
                     printf("%s\n", mode_strings[ridx]);
-                    xtimer_usleep(DELAY_US);
+                    _delay();
                 }
                 else {
                     results[ridx] = false;

5 Data Bits, No Parity, 1 Stop Bit

5 Data Bits, No Parity, 2 Stop Bit

5 Data Bits, Even Parity, 1 Stop Bit

5 Data Bits, Even Parity, 2 Stop Bit

6 Data Bits, No Parity, 1 Stop Bit

7 Data Bits, No Parity, 1 Stop Bit

8 Data Bits, No Parity, 1 Stop Bit

8 Data Bits, No Parity, 2 Stop Bit

8 Data Bits, Even Parity, 1 Stop Bit

8 Data Bits, Even Parity, 2 Stop Bit

8 Data Bits, Odd Parity, 1 Stop Bit

8 Data Bits, Odd Parity, 2 Stop Bit

maribu · 2021-07-13T14:22:26Z

In addition to all modes now working correctly (without sending one char with the previous config) the single garbage char previously send / transmitted on (cold) boot is now also gone.

maribu · 2021-07-13T15:54:29Z

Output of the tests/periph_gpio when using #16627 and the following patch applied:

diff --git a/tests/periph_gpio/main.c b/tests/periph_gpio/main.c
index 8011f45dc1..51e8469419 100644
--- a/tests/periph_gpio/main.c
+++ b/tests/periph_gpio/main.c
@@ -167,6 +167,7 @@ static int enable_int(int argc, char **argv)
     }
 
     po = atoi(argv[1]);
+    (void)po;
     pi = atoi(argv[2]);
 
     status = atoi(argv[3]);

> auto_test 0 2 0 3
2021-07-13 17:51:19,245 # auto_test 0 2 0 3
2021-07-13 17:51:19,246 # [START]
2021-07-13 17:51:19,253 # [SUCCESS]

benpicco

Looks pretty good!
Please squash.

benpicco · 2021-07-13T19:56:51Z

cpu/rpx0xx/periph/uart.c

+    /* restore configuration registers */
+    dev->UARTIBRD.reg = uartibrd;
+    dev->UARTFBRD.reg = uartfbrd;
+    dev->UARTLCR_H.reg = uartlcr_h;
+    dev->UARTCR.reg = uartcr;


I think this is the only driver that properly implements this API 😄

maribu · 2021-07-14T05:01:56Z

Looks like Murdock tripped over another bug in make info-boards-supported, as there is no periph_flashpage feature provided by the pico. Locally, I cannot reproduce the issue :-(

benpicco · 2021-07-14T10:22:56Z

Adding the missing ROM_START_ADDR define shouldn't hurt though

cpu/rpx0xx/include/cpu_conf.h

Co-authored-by: Fabian Hüßler <[email protected]>

Co-authored-by: nickw96 <[email protected]> Co-authored-by: MaestroOnICe <[email protected]> Co-authored-by: Franz2000 <[email protected]>

Co-authored-by: Fabian Hüßler <[email protected]> Co-authored-by: nickw96 <[email protected]> Co-authored-by: MaestroOnICe <[email protected]> Co-authored-by: Franz2000 <[email protected]>

maribu · 2021-07-14T14:35:21Z

All green :-)

maribu · 2021-07-14T14:36:18Z

(And I think the failing test previously might just have been the unit tests for periph_flashpage, which do not rely on the feature. So no bug in make info-boards-supported.)

maribu · 2021-07-14T14:42:39Z

Thanks for the review! And thanks for the automatic GPIO test. It was very helpful in detecting a bug that would likely have gone unnoticed for quite some time!

maribu requested a review from jia200x as a code owner July 5, 2021 08:15

github-actions bot added Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation labels Jul 5, 2021

maribu added Type: new feature The issue requests / The PR implemements a new feature for RIOT and removed Area: doc Area: Documentation labels Jul 5, 2021

maribu commented Jul 5, 2021

View reviewed changes

cpu/rp2040/boot2_w25q080_padded_checksummed.S Show resolved Hide resolved

iia mentioned this pull request Jul 5, 2021

Raspberry Pi Pico and RP2040 Support #16612

Closed

benpicco reviewed Jul 5, 2021

View reviewed changes

iia mentioned this pull request Jul 5, 2021

Add support for Raspberry Pi RP2040 MCU and the Pico board #16613

Closed

github-actions bot added the Area: doc Area: Documentation label Jul 5, 2021

maribu changed the title ~~cpu/rp2040: port RIOT to the Raspberry Pi RP2040 MCU~~ cpu/rpx0xx: port RIOT to the Raspberry Pi RP2040 MCU Jul 5, 2021

aabadie mentioned this pull request Jul 5, 2021

boards/rpi-pico: add support #15822

Open

maribu force-pushed the cpu/rp2040 branch 2 times, most recently from 0c4a2d7 to cafacf0 Compare July 6, 2021 07:47

maribu requested review from cgundogan and leandrolanzieri as code owners July 6, 2021 08:42

github-actions bot added the Area: Kconfig Area: Kconfig integration label Jul 6, 2021

maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 6, 2021

benpicco reviewed Jul 7, 2021

View reviewed changes

fabian18 reviewed Jul 8, 2021

View reviewed changes

cpu/rpx0xx/periph/uart.c Outdated Show resolved Hide resolved

cpu/rpx0xx/periph/uart.c Outdated Show resolved Hide resolved

cpu/rpx0xx/periph/uart.c Outdated Show resolved Hide resolved

maribu removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 8, 2021

fabian18 mentioned this pull request Jul 8, 2021

cpu/rpx0xx: add periph timer #16627

Merged

benpicco reviewed Jul 9, 2021

View reviewed changes

cpu/rpx0xx/periph/uart.c Outdated Show resolved Hide resolved

maribu commented Jul 9, 2021

View reviewed changes

maribu force-pushed the cpu/rp2040 branch from 46eb292 to 2615a47 Compare July 9, 2021 18:03

maribu commented Jul 12, 2021

View reviewed changes

maribu force-pushed the cpu/rp2040 branch from 13da7a3 to 8d15078 Compare July 12, 2021 06:59

maribu mentioned this pull request Jul 13, 2021

tests/periph_uart_mode: Drop dep to periph_timer #16637

Merged

benpicco approved these changes Jul 13, 2021

View reviewed changes

maribu force-pushed the cpu/rp2040 branch from be9498f to 4a28cf4 Compare July 13, 2021 20:10

maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 13, 2021

benpicco reviewed Jul 14, 2021

View reviewed changes

cpu/rpx0xx/include/cpu_conf.h Show resolved Hide resolved

maribu and others added 3 commits July 14, 2021 12:41

cpu/rpx0xx: add support for the RP2040 MCU

ea56dfc

Co-authored-by: Fabian Hüßler <[email protected]>

cpu/rpx0xx: implement periph_uart

e382148

Co-authored-by: nickw96 <[email protected]> Co-authored-by: MaestroOnICe <[email protected]> Co-authored-by: Franz2000 <[email protected]>

boards/rpi-pico: add the Raspberry Pi Pico

d778e2b

Co-authored-by: Fabian Hüßler <[email protected]> Co-authored-by: nickw96 <[email protected]> Co-authored-by: MaestroOnICe <[email protected]> Co-authored-by: Franz2000 <[email protected]>

maribu force-pushed the cpu/rp2040 branch from 4a28cf4 to d778e2b Compare July 14, 2021 10:41

benpicco merged commit 9c8b62a into RIOT-OS:master Jul 14, 2021

maribu deleted the cpu/rp2040 branch July 14, 2021 14:41

benpicco added this to the Release 2021.10 milestone Oct 20, 2021

		@@ -0,0 +1,19 @@
		PICO_DEBUG ?=
		ROM_LEN ?= 2097152 # = 2 MiB used in the RPi Pico

Conversation

maribu commented Jul 5, 2021 • edited by aabadie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Acknowledgements

Testing procedure

Issues/PRs references

Uh oh!

Uh oh!

benpicco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu commented Jul 6, 2021

Uh oh!

benpicco commented Jul 6, 2021

Uh oh!

maribu commented Jul 6, 2021

Uh oh!

maribu commented Jul 7, 2021

Uh oh!

benpicco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benpicco Jul 7, 2021

Choose a reason for hiding this comment

Uh oh!

maribu Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maribu commented Jul 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benpicco commented Jul 8, 2021

Uh oh!

fabian18 commented Jul 8, 2021

Uh oh!

Uh oh!

maribu Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

maribu Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

benpicco commented Jul 9, 2021

Uh oh!

benpicco commented Jul 10, 2021

Uh oh!

maribu Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

maribu commented Jul 12, 2021

Uh oh!

maribu commented Jul 13, 2021

5 Data Bits, No Parity, 1 Stop Bit

5 Data Bits, No Parity, 2 Stop Bit

5 Data Bits, Even Parity, 1 Stop Bit

5 Data Bits, Even Parity, 2 Stop Bit

6 Data Bits, No Parity, 1 Stop Bit

7 Data Bits, No Parity, 1 Stop Bit

8 Data Bits, No Parity, 1 Stop Bit

8 Data Bits, No Parity, 2 Stop Bit

8 Data Bits, Even Parity, 1 Stop Bit

maribu commented Jul 5, 2021 •

edited by aabadie

Loading

benpicco commented Jul 14, 2021 •

edited

Loading