Apply suggestions from code review

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

Author: AnnsAnns

cpu/rp2350_common/include/periph_cpu.h

@@ -87,14 +87,15 @@ extern "C" {
 void rp2350_init(void);
 
 /**
- * @brief   Calculate the address of the GPIO pad register for a given pin
- * @param   pin     The GPIO pin number
- * @return  The address of the GPIO pad register for the given pin
+ * @brief       Calculate the address of the GPIO pad register for a given pin
+ * @param[in]   pin     The GPIO pin number
+ * @return      The address of the GPIO pad register for the given pin
  */
-static inline uint32_t* calculate_gpio_pad_register_addr(gpio_t pin) {
+static inline uint32_t* calculate_gpio_pad_register_addr(gpio_t pin)
+{
     /* Each pin has a 4 byte register, so we can calculate the address
-    * by adding 4 bytes for each pin, starting at the base address of PADS_BANK0
-    * and adding 4 bytes to skip VOLTAGE_SELECT */
+     * by adding 4 bytes for each pin, starting at the base address of PADS_BANK0
+     * and adding 4 bytes to skip VOLTAGE_SELECT */
     return (uint32_t*) (PADS_BANK0_BASE + (4 * (pin + 1)));
 }
 
@@ -121,8 +122,6 @@ static inline uint32_t* calculate_gpio_io_ctrl_register_addr(gpio_t pin) {
     return (uint32_t*) (calculate_gpio_io_status_register_addr(pin) + 4);
 }
 
-
-
 #ifdef __cplusplus
 }
 #endif

cpu/rp2350_common/periph/uart.c

@@ -28,7 +28,8 @@
 #  include "xh3irq.h"
 #endif
 
-#include "board.h"
+#define ENABLE_DEBUG 0
+#include "debug.h"
 
 static uart_isr_ctx_t ctx[UART_NUMOF];
 
@@ -42,7 +43,6 @@ 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);
@@ -70,19 +70,11 @@ 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,
-              uart_stop_bits_t stop_bits) {
+              uart_stop_bits_t stop_bits)
+{
     assert((unsigned)uart < UART_NUMOF);
     UART0_Type *dev = uart_config[uart].dev;
 
@@ -121,7 +113,8 @@ int uart_mode(uart_t uart, uart_data_bits_t data_bits, uart_parity_t parity,
     return UART_OK;
 }
 
-static void _reset_uart(uart_t uart) {
+static void _reset_uart(uart_t uart)
+{
     switch (uart) {
     case 0:
         /* We reset UART0 here, so we can be sure it is in a known state */
@@ -136,7 +129,8 @@ static void _reset_uart(uart_t uart) {
     }
 }
 
-void uart_init_pins(uart_t uart) {
+void uart_init_pins(uart_t uart)
+{
     assert((unsigned)uart < UART_NUMOF);
 
     /* We reset UART0 here, so we can be sure it is in a known state */
@@ -211,9 +205,10 @@ void uart_write(uart_t uart, const uint8_t *data, size_t len)
     }
 }
 
-void uart_poweron(uart_t uart) {
+void uart_poweron(uart_t uart)
+{
     assert((unsigned)uart < UART_NUMOF);
-    /* Get into a save state where we know what's up */
+    /* Get into a safe state where we know what's up */
     _reset_uart(uart);
     UART0_Type *dev = uart_config[uart].dev;
     /* Restore config from registers */
@@ -228,7 +223,8 @@ void uart_poweron(uart_t uart) {
     uart_init_pins(uart);
 }
 
-void uart_deinit_pins(uart_t uart) {
+void uart_deinit_pins(uart_t uart)
+{
     assert((unsigned)uart < UART_NUMOF);
     /* @TODO */
     /* gpio_reset_all_config(uart_config[uart].tx_pin); */
@@ -238,7 +234,8 @@ void uart_deinit_pins(uart_t uart) {
     }
 }
 
-void uart_poweroff(uart_t uart) {
+void uart_poweroff(uart_t uart)
+{
     assert((unsigned)uart < UART_NUMOF);
     UART0_Type *dev = uart_config[uart].dev;
     /* backup configuration registers */
@@ -251,7 +248,8 @@ void uart_poweroff(uart_t uart) {
     _reset_uart(uart);
 }
 
-void isr_handler(uint8_t num) {
+void isr_handler(uint8_t num)
+{
     UART0_Type *dev = uart_config[num].dev;
 
     uint32_t status = dev->UARTMIS;
@@ -269,12 +267,14 @@ void isr_handler(uint8_t num) {
 }
 
 /** Overwrites the WEAK_DEFAULT isr_uart0 */
-void isr_uart0(void) {
+void isr_uart0(void)
+{
     isr_handler(0);
     rp_end_isr();
 }
 
-void isr_uart1(void) {
+void isr_uart1(void)
+{
     isr_handler(1);
     rp_end_isr();
 }

cpu/rp2350_common/vectors.c

@@ -92,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 does not */
+/* Cortex-M does some macro magic, RISC-V does not */
 ISR_VECTOR(1)
 #endif
 const void* vector_cpu[CPU_IRQ_NUMOF] = {

cpu/rp2350_riscv/cpu.c

@@ -15,15 +15,16 @@
  * @}
  */
 
-#include "board.h"
+#include <sys/unistd.h>
+
 #include "cpu.h"
+#include "board.h"
 #include "clock_conf.h"
 #include "kernel_init.h"
-#include "periph/init.h"
-#include "periph/uart.h"
 #include "periph_conf.h"
 
-#include <sys/unistd.h>
+#include "periph/init.h"
+#include "periph/uart.h"
 
 /**
  * @brief Initialize the CPU, set IRQ priorities, clocks, peripheral

cpu/rp2350_riscv/include/core_cm33.h

@@ -10,9 +10,9 @@
 extern "C" {
 #endif
 
-#define     __IM     volatile const      /*! Defines 'read only' structure member permissions */
-#define     __OM     volatile            /*! Defines 'write only' structure member permissions */
-#define     __IOM    volatile            /*! Defines 'read / write' structure member permissions */
+#define     __IM     volatile const  /**< Defines 'read only' structure member permissions */
+#define     __OM     volatile        /**< Defines 'write only' structure member permissions */
+#define     __IOM    volatile        /**< Defines 'read / write' structure member permissions */
 
 #ifdef __cplusplus
 }