Merge pull request #12064 from benpicco/sam0-buffered_uart

cpu/sam0_common/periph/uart: implement non-blocking write

Makefile.dep

@@ -976,6 +976,11 @@ ifneq (,$(filter suit_%,$(USEMODULE)))
   USEMODULE += suit
 endif
 
+# Enable periph_uart when periph_uart_nonblocking is enabled
+ifneq (,$(filter periph_uart_nonblocking,$(USEMODULE)))
+  FEATURES_REQUIRED += periph_uart
+endif
+
 # Enable periph_gpio when periph_gpio_irq is enabled
 ifneq (,$(filter periph_gpio_irq,$(USEMODULE)))
   FEATURES_REQUIRED += periph_gpio

boards/common/saml1x/include/periph_conf.h

@@ -73,6 +73,7 @@ static const uart_conf_t uart_config[] = {
 
 /* interrupt function name mapping */
 #define UART_0_ISR          isr_sercom2_2
+#define UART_0_ISR_TX       isr_sercom2_0
 
 #define UART_NUMOF          ARRAY_SIZE(uart_config)
 /** @} */

boards/same54-xpro/include/periph_conf.h

@@ -88,6 +88,7 @@ static const uart_conf_t uart_config[] = {
 
 /* interrupt function name mapping */
 #define UART_0_ISR          isr_sercom2_2
+#define UART_0_ISR_TX       isr_sercom2_0
 
 #define UART_NUMOF          ARRAY_SIZE(uart_config)
 /** @} */

cpu/sam0_common/Makefile.dep

@@ -0,0 +1,3 @@
+ifneq (,$(filter periph_uart_nonblocking,$(USEMODULE)))
+  USEMODULE += tsrb
+endif

cpu/sam0_common/Makefile.features

@@ -4,6 +4,7 @@ FEATURES_PROVIDED += periph_flashpage_raw
 FEATURES_PROVIDED += periph_flashpage_rwee
 FEATURES_PROVIDED += periph_gpio periph_gpio_irq
 FEATURES_PROVIDED += periph_uart_modecfg
+FEATURES_PROVIDED += periph_uart_nonblocking
 FEATURES_PROVIDED += periph_wdt periph_wdt_cb
 
 -include $(RIOTCPU)/cortexm_common/Makefile.features

cpu/sam0_common/include/periph_cpu_common.h

@@ -196,6 +196,14 @@ typedef enum {
 /** @} */
 #endif /* ndef DOXYGEN */
 
+
+/**
+ * @brief   Size of the UART TX buffer for non-blocking mode.
+ */
+#ifndef SAM0_UART_TXBUF_SIZE
+#define SAM0_UART_TXBUF_SIZE    (64)
+#endif
+
 /**
  * @brief   UART device configuration
  */

cpu/sam0_common/periph/uart.c

@@ -31,9 +31,18 @@
 #define ENABLE_DEBUG (0)
 #include "debug.h"
 
+#if defined (CPU_SAML1X) || defined (CPU_SAMD5X)
+#define UART_HAS_TX_ISR
+#endif
+
 /**
- * @brief   Allocate memory to store the callback functions
+ * @brief   Allocate memory to store the callback functions & buffers
  */
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+#include "tsrb.h"
+static tsrb_t uart_tx_rb[UART_NUMOF];
+static uint8_t uart_tx_rb_buf[UART_NUMOF][SAM0_UART_TXBUF_SIZE];
+#endif
 static uart_isr_ctx_t uart_ctx[UART_NUMOF];
 
 /**
@@ -57,6 +66,11 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     /* must disable here first to ensure idempotency */
     dev(uart)->CTRLA.reg &= ~(SERCOM_USART_CTRLA_ENABLE);
 
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+    /* set up the TX buffer */
+    tsrb_init(&uart_tx_rb[uart], uart_tx_rb_buf[uart], SAM0_UART_TXBUF_SIZE);
+#endif
+
     /* configure pins */
     if (uart_config[uart].rx_pin != GPIO_UNDEF) {
         gpio_init(uart_config[uart].rx_pin, GPIO_IN);
@@ -99,11 +113,13 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     if ((rx_cb) && (uart_config[uart].rx_pin != GPIO_UNDEF)) {
         uart_ctx[uart].rx_cb = rx_cb;
         uart_ctx[uart].arg = arg;
-#if defined (CPU_SAML1X) || defined (CPU_SAMD5X)
+#ifdef UART_HAS_TX_ISR
+        /* enable RXNE ISR */
         NVIC_EnableIRQ(SERCOM0_2_IRQn + (sercom_id(dev(uart)) * 4));
 #else
+        /* enable UART ISR */
         NVIC_EnableIRQ(SERCOM0_IRQn + sercom_id(dev(uart)));
-#endif
+#endif /* UART_HAS_TX_ISR */
         dev(uart)->CTRLB.reg |= SERCOM_USART_CTRLB_RXEN;
         dev(uart)->INTENSET.reg |= SERCOM_USART_INTENSET_RXC;
         /* set wakeup receive from sleep if enabled */
@@ -111,6 +127,18 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
             dev(uart)->CTRLB.reg |= SERCOM_USART_CTRLB_SFDE;
         }
     }
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+#ifndef UART_HAS_TX_ISR
+    else {
+        /* enable UART ISR */
+        NVIC_EnableIRQ(SERCOM0_IRQn + sercom_id(dev(uart)));
+    }
+#else
+    /* enable TXE ISR */
+    NVIC_EnableIRQ(SERCOM0_0_IRQn + (sercom_id(dev(uart)) * 4));
+#endif
+#endif /* MODULE_PERIPH_UART_NONBLOCKING */
+
     while (dev(uart)->SYNCBUSY.bit.CTRLB) {}
 
     /* and finally enable the device */
@@ -121,11 +149,18 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
 
 void uart_write(uart_t uart, const uint8_t *data, size_t len)
 {
-    for (size_t i = 0; i < len; i++) {
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+    for (const void* end = data + len; data != end; ++data) {
+        while (tsrb_add_one(&uart_tx_rb[uart], *data) < 0) {}
+        dev(uart)->INTENSET.reg = SERCOM_USART_INTENSET_DRE;
+    }
+#else
+    for (const void* end = data + len; data != end; ++data) {
         while (!dev(uart)->INTFLAG.bit.DRE) {}
-        dev(uart)->DATA.reg = data[i];
+        dev(uart)->DATA.reg = *data;
     }
     while (!dev(uart)->INTFLAG.bit.TXC) {}
+#endif
 }
 
 void uart_poweron(uart_t uart)
@@ -181,14 +216,38 @@ int uart_mode(uart_t uart, uart_data_bits_t data_bits, uart_parity_t parity,
 }
 #endif
 
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+static inline void irq_handler_tx(unsigned uartnum)
+{
+    /* workaround for saml1x */
+    int c = tsrb_get_one(&uart_tx_rb[uartnum]);
+    if (c >= 0) {
+        dev(uartnum)->DATA.reg = c;
+    }
+
+    /* disable the interrupt if there are no more bytes to send */
+    if (tsrb_empty(&uart_tx_rb[uartnum])) {
+        dev(uartnum)->INTENCLR.reg = SERCOM_USART_INTENSET_DRE;
+    }
+}
+#endif
+
 static inline void irq_handler(unsigned uartnum)
 {
-    if (dev(uartnum)->INTFLAG.bit.RXC) {
+    uint32_t status = dev(uartnum)->INTFLAG.reg;
+
+#if !defined(UART_HAS_TX_ISR) && defined(MODULE_PERIPH_UART_NONBLOCKING)
+    if ((status & SERCOM_USART_INTFLAG_DRE) && dev(uartnum)->INTENSET.bit.DRE) {
+        irq_handler_tx(uartnum);
+    }
+#endif
+
+    if (status & SERCOM_USART_INTFLAG_RXC) {
         /* interrupt flag is cleared by reading the data register */
         uart_ctx[uartnum].rx_cb(uart_ctx[uartnum].arg,
                                 (uint8_t)(dev(uartnum)->DATA.reg));
     }
-    else if (dev(uartnum)->INTFLAG.bit.ERROR) {
+    else if (status & SERCOM_USART_INTFLAG_ERROR) {
         /* clear error flag */
         dev(uartnum)->INTFLAG.reg = SERCOM_USART_INTFLAG_ERROR;
     }
@@ -237,3 +296,48 @@ void UART_5_ISR(void)
     irq_handler(5);
 }
 #endif
+
+#ifdef MODULE_PERIPH_UART_NONBLOCKING
+
+#ifdef UART_0_ISR_TX
+void UART_0_ISR_TX(void)
+{
+    irq_handler_tx(0);
+}
+#endif
+
+#ifdef UART_1_ISR_TX
+void UART_1_ISR_TX(void)
+{
+    irq_handler_tx(1);
+}
+#endif
+
+#ifdef UART_2_ISR_TX
+void UART_2_ISR_TX(void)
+{
+    irq_handler_tx(2);
+}
+#endif
+
+#ifdef UART_3_ISR_TX
+void UART_3_ISR_TX(void)
+{
+    irq_handler_tx(3);
+}
+#endif
+
+#ifdef UART_4_ISR_TX
+void UART_4_ISR_TX(void)
+{
+    irq_handler_tx(4);
+}
+#endif
+
+#ifdef UART_5_ISR_TX
+void UART_5_ISR_TX(void)
+{
+    irq_handler_tx(5);
+}
+#endif
+#endif /* MODULE_PERIPH_UART_NONBLOCKING */

cpu/samd21/Makefile.dep

@@ -0,0 +1 @@
+include $(RIOTCPU)/sam0_common/Makefile.dep

cpu/samd5x/Makefile.dep

@@ -0,0 +1 @@
+include $(RIOTCPU)/sam0_common/Makefile.dep

cpu/saml1x/Makefile.dep

@@ -0,0 +1 @@
+include $(RIOTCPU)/sam0_common/Makefile.dep

cpu/saml21/Makefile.dep

@@ -0,0 +1 @@
+include $(RIOTCPU)/sam0_common/Makefile.dep

tests/periph_uart_nonblocking/Makefile

@@ -0,0 +1,6 @@
+include ../Makefile.tests_common
+
+FEATURES_REQUIRED += periph_uart_nonblocking
+USEMODULE += xtimer
+
+include $(RIOTBASE)/Makefile.include

tests/periph_uart_nonblocking/main.c

@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2019 Benjamin Valentin <benpicco@googlemail.com>
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     tests
+ * @{
+ *
+ * @file
+ * @brief       Simple test application for non-blocking UART functionality
+ *
+ * @author      Benjamin Valentin <benpicco@googlemail.com>
+ *
+ * @}
+ */
+
+#include <stdio.h>
+#include <xtimer.h>
+
+#define LINE_DELAY_MS   100
+
+static inline uint32_t puts_delay(const char* str)
+{
+    puts(str);
+    xtimer_usleep(LINE_DELAY_MS * 1000);
+    return LINE_DELAY_MS * 1000;
+}
+
+int main(void)
+{
+    uint32_t total_us = 0;
+    xtimer_ticks32_t counter = xtimer_now();
+
+    /* Richard Stallman and the Free Software Foundation
+       claim no copyright on this song. */
+    total_us += puts_delay("");
+    total_us += puts_delay("Join us now and share the software;");
+    total_us += puts_delay("You'll be free, hackers, you'll be free.");
+    total_us += puts_delay("Join us now and share the software;");
+    total_us += puts_delay("You'll be free, hackers, you'll be free.");
+    total_us += puts_delay("");
+    total_us += puts_delay("Hoarders can get piles of money,");
+    total_us += puts_delay("That is true, hackers, that is true.");
+    total_us += puts_delay("But they cannot help their neighbors;");
+    total_us += puts_delay("That's not good, hackers, that's not good.");
+    total_us += puts_delay("");
+    total_us += puts_delay("When we have enough free software");
+    total_us += puts_delay("At our call, hackers, at our call,");
+    total_us += puts_delay("We'll kick out those dirty licenses");
+    total_us += puts_delay("Ever more, hackers, ever more.");
+    total_us += puts_delay("");
+    total_us += puts_delay("Join us now and share the software;");
+    total_us += puts_delay("You'll be free, hackers, you'll be free.");
+    total_us += puts_delay("Join us now and share the software;");
+    total_us += puts_delay("You'll be free, hackers, you'll be free.");
+    total_us += puts_delay("");
+
+    counter.ticks32 = xtimer_now().ticks32 - counter.ticks32;
+
+    printf("== printed in %" PRIu32 "/%" PRIu32 " µs ==\n", xtimer_usec_from_ticks(counter), total_us);
+
+    puts("[SUCCESS]");
+
+    return 0;
+}

tests/periph_uart_nonblocking/tests/01-run.py

@@ -0,0 +1,24 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2019 Benjamin Valentin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+import sys
+from testrunner import run
+
+
+def testfunc(child):
+    child.expect(r'== printed in (\d+)/(\d+) µs ==')
+    time_actual = int(child.match.group(1))
+    time_expect = int(child.match.group(2))
+
+    assert time_actual / time_expect < 1.0015
+
+    child.expect_exact("[SUCCESS]")
+
+
+if __name__ == "__main__":
+    sys.exit(run(testfunc))