[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 
```c
static uart_isr_ctx_t isr_ctx[UART_NUMOF];
```
right?

> @@ -76,128 +107,185 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     PSEL_TXD = UART_PIN_TX;
 
     /* enable HW-flow control if defined */
-#if UART_HWFLOWCTRL
-    /* 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 */
-    PSEL_RTS = UART_PIN_RTS;
-    PSEL_CTS = UART_PIN_CTS;
-    NRF_UART0->CONFIG |= UART_CONFIG_HWFC_Msk;  /* enable HW flow control */
-#else
-    PSEL_RTS = 0xffffffff;            /* pin disconnected */
-    PSEL_CTS = 0xffffffff;            /* pin disconnected */
-#endif
+    if (UART_HWFLOWCTRL) {

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:
https://github.com/RIOT-OS/RIOT/pull/10621#pullrequestreview-192116315
-------------- 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