RP2350 (ARM): Multicore Support, Interrupts, UART updates and better debugging

boards/rpi-pico-2/dist/openocd.cfg

@@ -1,7 +1,7 @@
 echo "Make sure to use the Raspberry Pi OpenOCD version!"
 source [find target/rp2350.cfg]
-set USE_CORE 0
+set USE_CORE SMP
 set RESCUE 1
-$_TARGETNAME_0 configure -rtos auto
 adapter speed 5000
 rp2350.dap.core1 cortex_m reset_config sysresetreq
+rp2350.dap.core0 cortex_m reset_config sysresetreq

boards/rpi-pico-2/include/periph_conf.h

@@ -15,6 +15,37 @@
 extern "C" {
 #endif
 
+/**
+ * @brief   Configuration details for an UART interface needed by the RPX0XX peripheral
+    @todo this is shared between both
+ */
+typedef struct {
+    UART0_Type *dev;    /**< Base address of the I/O registers of the device */
+    gpio_t rx_pin;      /**< GPIO pin to use for RX */
+    gpio_t tx_pin;      /**< GPIO pin to use for TX */
+    IRQn_Type irqn;     /**< IRQ number of the UART interface */
+} uart_conf_t;
+
+static const uart_conf_t uart_config[] = {
+    {
+        .dev = UART0,
+        .rx_pin = GPIO_PIN(0, 1),
+        .tx_pin = GPIO_PIN(0, 0),
+        .irqn = UART0_IRQ_IRQn
+    },
+    {
+        .dev = UART1,
+        .rx_pin = GPIO_PIN(0, 9),
+        .tx_pin = GPIO_PIN(0, 8),
+        .irqn = UART1_IRQ_IRQn
+    }
+};
+
+#define UART_0_ISR      (isr_uart0)
+#define UART_1_ISR      (isr_uart1)
+
+#define UART_NUMOF      ARRAY_SIZE(uart_config)
+
 #ifdef __cplusplus
 }
 #endif

cpu/rp2350/Makefile.include

@@ -14,6 +14,7 @@ CFLAGS += -Wno-error
 INCLUDES += -I$(RIOTCPU)/rp2350/include
 INCLUDES += -isystem$(RIOTBASE)/build/pkg/picosdk/src/rp2_common/cmsis/stub/CMSIS/Core/Include
 INCLUDES += -isystem$(RIOTBASE)/build/pkg/picosdk/src/rp2_common/cmsis/stub/CMSIS/Device/RP2350/Include
+INCLUDES += -isystem$(RIOTBASE)/build/pkg/picosdk/src/rp2350/hardware_regs/include/hardware
 
 # Linker flags
 LINKFLAGS += -mcpu=$(CPU_ARCH) -mthumb

cpu/rp2350/core.c

@@ -0,0 +1,123 @@
+#include "cpu.h"
+#include "cpu_conf.h"
+#include "helpers.h"
+
+extern uint32_t _estack;  /* End of stack based on cortex_m.ld */
+extern uint32_t _sstack; /* Start of stack based on cortex_m.ld */
+extern uint32_t _isr_vectors;
+
+void _core1_trampoline(void) {
+    cortexm_init();
+
+    core_1_fn_t function = (core_1_fn_t) core_1_stack[0];
+    void *arg = (void *) core_1_stack[1];
+    (*function)(arg);
+}
+
+void core1_init(core_1_fn_t function, void *arg) {
+    /* First we need to get core1 online (See 5.3)
+     * for that we need to get it out of reset (See 7.4.4)
+     * this allows proc1 to power on */
+    atomic_set(&PSM->FRCE_ON, 1<<core1_psm_bit);
+    /* Check whether PSM Done is set (See Table 533 / 7.4.4)*/
+    while (~PSM->DONE & 1<<core1_psm_bit) {
+        /* Wait for the reset to complete */
+    }
+
+    /* At this point Core1 is powered on but sleeping (See 5.2)*/
+
+    /* Next we need to define the launching code by passing all relevant info
+    * via the inter-processor FIFO (See 3.1.5).
+    * While the definition of 5.3 isn't great since it relies on the SDK,
+    * if we actually look at the SDK at pico_multicore/multicore.c
+    * we can see that we simply have to pass a data struct
+    * I still can't find the exact reason for the first 3 values though?
+    */
+   const uint32_t cmd_sequence[] = {
+        0,
+        0,
+        1,
+        /**
+        * This might be a really bad way to do this but I'm a bit unsure how else
+        * to tackle it, the vector_table uses macro magic and I'm unsure how to
+        * get the address, however, the vector table should sit right at the front
+        * of the ROM so *technically* it might be fine
+        */
+        (uint32_t) &_isr_vectors,
+        /**
+        * We allocate a stack "locally" instead of in the linker script
+        * since that would require changes to the base cortexm script
+        * which sound complicated to do on a per-cpu basis
+        */
+        (uint32_t) &core_1_stack[0],
+        /** Pointer to main function for core1 */
+        (uint32_t) _core1_trampoline,
+    };
+
+    /**
+     * Set the entry point to the first stack element
+     * and arguments to the second, so we can later
+     * pop them via the core1 trampoline
+     */
+    core_1_stack[0] = (uint32_t) function;
+    core_1_stack[1] = (uint32_t) arg;
+
+    uint32_t seq = 0;
+    /** We iterate through the cmd_sequence till we covered every param
+     *(seq does not increase with each loop, thus we need to while loop this)
+     */
+    while(seq < 6) {
+        uint32_t cmd = cmd_sequence[seq];
+        /* If the cmd is 0 we need to drain the READ FIFO first*/
+        if(cmd == 0) {
+            /**
+             * The official SDK does this
+             * ```c
+             *     while (multicore_fifo_rvalid())
+             *         (void) sio_hw->fifo_rd;
+             * ```
+             * fifo_rvalid checks whether rx fifo is empty and then the value
+             * gets discarded (called multicore_fifo_drain in chapter 5.3)
+             */
+            while(SIO->FIFO_ST & 1<<SIO_FIFO_READ_VALID_BIT) {
+                (void) SIO->FIFO_RD; /* Table 39 FIFO_RD*/
+            };
+
+            /** SEV -> Set Event
+             * @see https://developer.arm.com/documentation/dui0473/m/arm-and-thumb-instructions/sev
+             * The Doc (p376) says that this is done to cover cases where core 1
+             * is waiting for FIFO space. Though, as I understand it, this shouldn't technically
+             * happen since don't dynamically re-enable core1 (yet :D)
+             */
+            __SEV();
+        }
+
+        /* This is eq. to the SDK multicore_fifo_push_blocking_inline */
+        /* Check whether queue is full */
+        while (!(SIO->FIFO_ST & 1<<SIO_FIFO_SEND_READY_BIT)) {
+            /* Wait for queue space */
+        }
+        /* Write data since we know we have space */
+        SIO->FIFO_WR = cmd;
+        /* Send event */
+        __SEV();
+
+        /* This is eq. to the SDK multicore_fifo_pop_blocking_inline*/
+        /* We check whether there are events */
+        while(!(SIO->FIFO_ST & 1<<SIO_FIFO_READ_VALID_BIT)) {
+            /* If not we simply wait,
+             * fun fact, it appears like WFE is not optional in this scenario
+             * not using wfe causes a double fault crash
+             * Also for ref, WFE -> Wait For Event
+             * https://developer.arm.com/documentation/dui0552/a/the-cortex-m3-instruction-set/miscellaneous-instructions/wfe
+             */
+            __WFE();
+        };
+
+        /* Get the event since this is our response */
+        volatile uint32_t response = SIO->FIFO_RD;
+
+        /* move to next state on correct response (echo-d value) otherwise start over */
+        seq = cmd == response ? seq + 1 : 0;
+    };
+}

cpu/rp2350/cpu.c

@@ -18,13 +18,20 @@
 
 #include "RP2350.h"
 #include "board.h"
+#include "clock_conf.h"
+#include "cpu_conf_common.h"
+#include "helpers.h"
 #include "kernel_init.h"
 #include "macros/units.h"
 #include "periph/gpio.h"
+#include "periph/uart.h"
 #include "periph/init.h"
 #include "periph_cpu.h"
 #include "stdio_base.h"
 
+#include <stdint.h>
+#include <stdio.h>
+
 #define DEBUG_WITH_OSC
 
 void gpio_reset(void) {
@@ -35,7 +42,7 @@ void gpio_reset(void) {
 void cpu_init(void) {
     /* initialize the Cortex-M core, once UART support is moved
      * to shared driver as currently this will cause unhandled interrupts */
-    /* cortexm_init(); */
+    cortexm_init();
 
     /* Reset GPIO state */
     gpio_reset();

cpu/rp2350/include/cpu_conf.h

@@ -26,6 +26,37 @@
 extern "C" {
 #endif
 
+/* Table 37 FIFO_ST, 1 if not empty*/
+#define SIO_FIFO_READ_VALID_BIT 0
+/* TABLE 37, 1 if not full */
+#define SIO_FIFO_SEND_READY_BIT 1
+#define core1_psm_bit 24
+
+/**
+ * The stack used by core 1, 16 times the thread stack size
+ */
+static volatile uint32_t core_1_stack[16*THREAD_STACKSIZE_DEFAULT];
+
+/**
+ * The function signature used for any function passed onto
+ * core 1
+ */
+typedef void *(*core_1_fn_t)(void *arg);
+
+/**
+ * @brief Init Core 1
+ * @param function The function to be loaded onto Core 1
+ * @param arg The argument to pass onto the the function (NULL if none)
+ */
+void core1_init(core_1_fn_t function, void *arg);
+
+/**
+ * @brief This is the internal trampoline setting everything up
+ *  before the entry function gets executed, it retrieves the required
+ *  data from the stack and then jumps to the designated function
+ */
+void _core1_trampoline(void);
+
 #ifdef __cplusplus
 }
 #endif

cpu/rp2350/include/periph_cpu.h

@@ -30,6 +30,12 @@
 #define HAVE_GPIO_T
 typedef uint32_t gpio_t;
 
+/* Im currently copying the original rp2040 def but this causes the other port to not be addressable (I think)*/
+#define GPIO_PIN(port, pin)     (((port) & 0) | (pin))
+
+/* This is a define used throughout the pico sdk */
+#define _u(x) ((uint32_t)(x))
+
 #include "periph/gpio.h"
 
 /** GPIO Pin ID for oscillator debugging */