Apply suggestions from code review

Co-authored-by: mguetschow <[email protected]>
Co-authored-by: crasbe <[email protected]>

Author: 3 people

boards/rpi-pico-2-arm/include/board.h

@@ -7,7 +7,7 @@
 #pragma once
 
 /**
- * @ingroup         boards_rpi_pico_2
+ * @ingroup         boards_rpi_pico_2_arm
  * @{
  *
  * @file

cpu/riscv_common/irq_arch.c

@@ -247,7 +247,7 @@ static void __attribute__((interrupt)) trap_entry(void)
         "csrr a0, mcause                                    \n"
 
         /* Call trap handler, a0 contains mcause before, and the return value after
-        * the call */
+         * the call */
         "call handle_trap                                   \n"
 
         /* Load the sched_context_switch_request */
@@ -264,12 +264,12 @@ static void __attribute__((interrupt)) trap_entry(void)
 
         "no_sched:                                          \n"
         /* Restore the thread stack pointer and check if a new thread must be
-        * scheduled */
+         * scheduled */
         "mv sp, s0                                          \n"
 
         /* No context switch required, shortcut to restore. a0 contains the return
-        * value of sched_run, or the sched_context_switch_request if the sched_run
-        * was skipped */
+         * value of sched_run, or the sched_context_switch_request if the sched_run
+         * was skipped */
         "beqz a0, no_switch                                 \n"
 
         /* Skips the rest of the save if no active thread */

cpu/rp2350_arm/Makefile

@@ -2,6 +2,7 @@
 MODULE = cpu
 
 # add a list of subdirectories, that should also be built
-DIRS = $(RIOTCPU)/rp2350_common $(RIOTCPU)/cortexm_common
+DIRS += $(RIOTCPU)/rp2350_common
+DIRS += $(RIOTCPU)/cortexm_common
 
 include $(RIOTBASE)/Makefile.base

cpu/rp2350_common/include/periph_cpu.h

@@ -33,8 +33,8 @@ typedef uint32_t gpio_t;
 #define _u(x) ((uint32_t)(x))
 
 #ifdef RP2350_USE_RISCV
-#include "periph_cpu_common.h"
-#include "xh3irq.h"
+#  include "periph_cpu_common.h"
+#  include "xh3irq.h"
 #endif
 #include "cpu.h"
 #include "core_cm33.h" /* Trick RP2350 into believing the file exists on RISCV */
@@ -45,11 +45,11 @@ typedef uint32_t gpio_t;
 #include "uart_conf.h"
 
 #if !(defined(RP2350_USE_ARM) || defined(RP2350_USE_RISCV))
-#error "Either RP2350_USE_ARM or RP2350_USE_RISCV must be defined"
+#  error "Either RP2350_USE_ARM or RP2350_USE_RISCV must be defined"
 #endif
 
 #if (defined(RP2350_USE_ARM) && defined(RP2350_USE_RISCV))
-#error "Only one of RP2350_USE_ARM or RP2350_USE_RISCV can be defined"
+#  error "Only one of RP2350_USE_ARM or RP2350_USE_RISCV can be defined"
 #endif
 
 #ifdef __cplusplus

cpu/rp2350_common/periph/uart.c

@@ -25,7 +25,7 @@
 #include "debug.h"
 
 #ifdef RP2350_USE_RISCV
-#include "xh3irq.h"
+#  include "xh3irq.h"
 #endif
 
 #include "board.h"
@@ -42,6 +42,7 @@ void _irq_enable(uart_t uart)
 {
     UART0_Type *dev = uart_config[uart].dev;
     /* We set the UART Receive Interrupt Mask (Bit 4) [See p979 UART 12.1]*/
+<<<<<<< HEAD
     dev->UARTIMSC = UART_UARTIMSC_RXIM_BITS;
     /* Enable the IRQ */
     rp_irq_enable(uart_config[uart].irqn);
@@ -69,6 +70,15 @@ void _set_symbolrate(uart_t uart, uint32_t baud)
 
     dev->UARTIBRD = baud_ibrd;
     dev->UARTFBRD = baud_fbrd;
+=======
+    dev->UARTIMSC = UART0_UARTIMSC_RXIM_Msk;
+    /* Enable the IRQ in the NVIC */
+#ifdef RP2350_USE_RISCV
+    xh3irq_enable_irq(uart_config[uart].irqn);
+#else
+    NVIC_EnableIRQ(uart_config[uart].irqn);
+#endif
+>>>>>>> a64ab76543 (Apply suggestions from code review)
 }
 
 int uart_mode(uart_t uart, uart_data_bits_t data_bits, uart_parity_t parity,
@@ -203,7 +213,7 @@ void uart_write(uart_t uart, const uint8_t *data, size_t len)
 
 void uart_poweron(uart_t uart) {
     assert((unsigned)uart < UART_NUMOF);
-    /* Get into a save state where we know whats up */
+    /* Get into a save state where we know what's up */
     _reset_uart(uart);
     UART0_Type *dev = uart_config[uart].dev;
     /* Restore config from registers */

cpu/rp2350_common/vectors.c

@@ -14,14 +14,15 @@
  * @author          Tom Hert <[email protected]>
  */
 
+#include <stdint.h>
+
 #include "cpu_conf.h"
 #include "panic.h"
-#include <stdint.h>
 
 #ifdef RP2350_USE_RISCV
-#include "xh3irq.h"
+#  include "xh3irq.h"
 #else
-#include "vectors_cortexm.h"
+#  include "vectors_cortexm.h"
 #endif
 
 #define WEAK_DEFAULT __attribute__((weak, alias("dummy_handler")))
@@ -91,7 +92,7 @@ WEAK_DEFAULT void isr_spareirq_5(void);
  * @see 3.2 Interrupts and IRQn_Type in RP2350.h
  */
 #ifdef RP2350_USE_ARM
-/* CortexM does some macro magic, RISCV doesnt */
+/* CortexM does some macro magic, RISCV does not */
 ISR_VECTOR(1)
 #endif
 const void* vector_cpu[CPU_IRQ_NUMOF] = {

cpu/rp2350_riscv/Makefile

@@ -2,6 +2,7 @@
 MODULE = cpu
 
 # add a list of subdirectories, that should also be built
-DIRS = $(RIOTCPU)/rp2350_common $(RIOTCPU)/riscv_common
+DIRS += $(RIOTCPU)/rp2350_common
+DIRS += $(RIOTCPU)/riscv_common
 
 include $(RIOTBASE)/Makefile.base

cpu/rp2350_riscv/Makefile.features

@@ -1,9 +1,8 @@
-CPU_CORE := rv32imac
+CPU_CORE    := rv32imac
 CPU_FAM     := RP2350
 CPU_MODEL   = rp2350_hazard3
 
 FEATURES_PROVIDED += periph_xh3irq
 
 include $(RIOTCPU)/rp2350_common/Makefile.features
-
 include $(RIOTCPU)/riscv_common/Makefile.features

cpu/rp2350_riscv/picobin_block.s

@@ -4,9 +4,10 @@
  * SPDX-License-Identifier: LGPL-2.1-only
  */
 
-/* Picobin block required for the binary */
-/* This defines the minimum viable image metadata to be recognized by the RP2350 bootloader */
-/* based on RP2350 Chapter 5.9.1 */
+/* Picobin block required for the binary.
+ * This defines the minimum viable image metadata to be recognized by the RP2350 bootloader
+ * based on RP2350 Chapter 5.9.1
+ */
 
 .section .picobin_block, "a" /* "a" means "allocatable" (can be moved by the linker) */
 

features.yaml

@@ -284,9 +284,9 @@ groups:
         - name: cpu_rpx0xx
           help: The MCU is part of the Raspberry PI RPx0xx family.
         - name: cpu_rp2350_arm
-          help: The MCU is a Raspberry Pi RP2350 CortexM33 ARMM
+          help: The MCU is a Raspberry Pi RP2350 Cortex-M33 ARM,
         - name: cpu_rp2350_riscv
-          help: The MCU is a Raspberry Pi RP2350 Hazard3 RISCV
+          help: The MCU is a Raspberry Pi RP2350 Hazard3 RISC-V.
 
       - title: Silicon Laboratories EFM32 Grouping
         features: