[riot-notifications] [RIOT-OS/RIOT] cpu/nrf5x: rework periph_uart driver to allow use of multiple UARTs with nrf52840 (#10621)

Hauke Petersen notifications at github.com
Mon Jan 14 12:56:42 CET 2019

haukepetersen requested changes on this pull request.

I did not have the time to fully review, here are just two findings I saw on a quick scan through the code just now. Will give this a proper review tomorrow (have another deadline tonight...).

>  #endif
  * @brief Allocate memory for the interrupt context
-static uart_isr_ctx_t uart_config;
+static uart_isr_ctx_t isr_ctx;

just a single ISR context for (potentially) more than 1 configured UART interface? This seems broken to me. Probably this should be 
static uart_isr_ctx_t isr_ctx[UART_NUMOF];

> @@ -76,128 +107,185 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     /* enable HW-flow control if defined */
-    /* set pin mode for RTS and CTS pins */
-    gpio_init(UART_PIN_RTS, GPIO_OUT);
-    gpio_init(UART_PIN_CTS, GPIO_IN);
-    /* configure RTS and CTS pins to use */
-    NRF_UART0->CONFIG |= UART_CONFIG_HWFC_Msk;  /* enable HW flow control */
-    PSEL_RTS = 0xffffffff;            /* pin disconnected */
-    PSEL_CTS = 0xffffffff;            /* pin disconnected */

if I see it correctly, this would disallow for using hw flow control for all non nrf52840 platforms, right? But we want to use that feature also with the nrf51 etc...

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190114/aacdb94e/attachment-0001.html>

More information about the notifications mailing list