Merge pull request #16506 from benpicco/drivers/dose-rx_start

drivers/dose: make use of start condition received interrupt

cpu/sam0_common/Kconfig

@@ -21,6 +21,7 @@ config CPU_COMMON_SAM0
     select HAS_PERIPH_UART_MODECFG
     select HAS_PERIPH_UART_NONBLOCKING
     select HAS_PERIPH_UART_RECONFIGURE
+    select HAS_PERIPH_UART_RXSTART_IRQ
     select HAS_PERIPH_WDT
     select HAS_PERIPH_WDT_CB
     select HAS_PERIPH_WDT_WARNING_PERIOD

cpu/sam0_common/Makefile.features

@@ -19,6 +19,7 @@ FEATURES_PROVIDED += periph_timer_periodic # implements timer_set_periodic()
 FEATURES_PROVIDED += periph_uart_modecfg
 FEATURES_PROVIDED += periph_uart_nonblocking
 FEATURES_PROVIDED += periph_uart_reconfigure
+FEATURES_PROVIDED += periph_uart_rxstart_irq
 FEATURES_PROVIDED += periph_wdt periph_wdt_cb periph_wdt_warning_period
 
 FEATURES_CONFLICT += periph_rtc:periph_rtt

cpu/sam0_common/periph/uart.c

@@ -356,7 +356,38 @@ int uart_mode(uart_t uart, uart_data_bits_t data_bits, uart_parity_t parity,
 
     return UART_OK;
 }
-#endif
+#endif /* MODULE_PERIPH_UART_MODECFG */
+
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+void uart_rxstart_irq_configure(uart_t uart, uart_rxstart_cb_t cb, void *arg)
+{
+    /* CTRLB is enable-proteced */
+    dev(uart)->CTRLA.bit.ENABLE = 0;
+
+    /* set start of frame detection enable */
+    dev(uart)->CTRLB.reg |= SERCOM_USART_CTRLB_SFDE;
+
+    uart_ctx[uart].rxs_cb  = cb;
+    uart_ctx[uart].rxs_arg = arg;
+
+    /* enable UART again */
+    dev(uart)->CTRLA.bit.ENABLE = 1;
+}
+
+void uart_rxstart_irq_enable(uart_t uart)
+{
+    /* clear stale interrupt flag */
+    dev(uart)->INTFLAG.reg  = SERCOM_USART_INTFLAG_RXS;
+
+    /* enable interrupt */
+    dev(uart)->INTENSET.reg = SERCOM_USART_INTENSET_RXS;
+}
+
+void uart_rxstart_irq_disable(uart_t uart)
+{
+    dev(uart)->INTENCLR.reg = SERCOM_USART_INTENCLR_RXS;
+}
+#endif /* MODULE_PERIPH_UART_RXSTART_IRQ */
 
 #ifdef MODULE_PERIPH_UART_NONBLOCKING
 static inline void irq_handler_tx(unsigned uartnum)
@@ -377,6 +408,8 @@ static inline void irq_handler_tx(unsigned uartnum)
 static inline void irq_handler(unsigned uartnum)
 {
     uint32_t status = dev(uartnum)->INTFLAG.reg;
+    /* TXC is used by uart_write() */
+    dev(uartnum)->INTFLAG.reg = status & ~SERCOM_USART_INTFLAG_TXC;
 
 #if !defined(UART_HAS_TX_ISR) && defined(MODULE_PERIPH_UART_NONBLOCKING)
     if ((status & SERCOM_USART_INTFLAG_DRE) && dev(uartnum)->INTENSET.bit.DRE) {
@@ -384,17 +417,17 @@ static inline void irq_handler(unsigned uartnum)
     }
 #endif
 
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    if (status & SERCOM_USART_INTFLAG_RXS && dev(uartnum)->INTENSET.bit.RXS) {
+        uart_ctx[uartnum].rxs_cb(uart_ctx[uartnum].rxs_arg);
+    }
+#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));
     }
-#ifdef SERCOM_USART_INTFLAG_ERROR
-    else if (status & SERCOM_USART_INTFLAG_ERROR) {
-        /* clear error flag */
-        dev(uartnum)->INTFLAG.reg = SERCOM_USART_INTFLAG_ERROR;
-    }
-#endif
 
     cortexm_isr_end();
 }

drivers/dose/Makefile.dep

@@ -1,5 +1,6 @@
 FEATURES_REQUIRED += periph_gpio_irq
 FEATURES_REQUIRED += periph_uart
+FEATURES_OPTIONAL += periph_uart_rxstart_irq
 
 USEMODULE += eui_provider
 USEMODULE += iolist

drivers/dose/dose.c

@@ -60,6 +60,42 @@ static uint16_t crc16_update(uint16_t crc, uint8_t octet)
     return crc;
 }
 
+static void _init_sense(dose_t *ctx, const dose_params_t *params)
+{
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    (void)params;
+    uart_rxstart_irq_configure(ctx->uart, _isr_gpio, ctx);
+#else
+    ctx->sense_pin = params->sense_pin;
+    if (gpio_is_valid(ctx->sense_pin)) {
+        gpio_init_int(ctx->sense_pin, GPIO_IN, GPIO_FALLING, _isr_gpio, ctx);
+        gpio_irq_disable(ctx->sense_pin);
+    }
+#endif
+}
+
+static inline void _enable_sense(dose_t *ctx)
+{
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    uart_rxstart_irq_enable(ctx->uart);
+#else
+    if (gpio_is_valid(ctx->sense_pin)) {
+        gpio_irq_enable(ctx->sense_pin);
+    }
+#endif
+}
+
+static inline void _disable_sense(dose_t *ctx)
+{
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    uart_rxstart_irq_disable(ctx->uart);
+#else
+    if (gpio_is_valid(ctx->sense_pin)) {
+        gpio_irq_disable(ctx->sense_pin);
+    }
+#endif
+}
+
 static dose_signal_t state_transit_blocked(dose_t *ctx, dose_signal_t signal)
 {
     uint32_t backoff;
@@ -73,10 +109,8 @@ static dose_signal_t state_transit_blocked(dose_t *ctx, dose_signal_t signal)
         netdev_trigger_event_isr(&ctx->netdev);
     }
 
-    if (gpio_is_valid(ctx->sense_pin)) {
-        /* Enable GPIO interrupt for start bit sensing */
-        gpio_irq_enable(ctx->sense_pin);
-    }
+    /* Enable interrupt for start bit sensing */
+    _enable_sense(ctx);
 
     /* The timeout will bring us back into IDLE state by a random time.
      * If we entered this state from RECV state, the random time lays
@@ -107,10 +141,10 @@ static dose_signal_t state_transit_recv(dose_t *ctx, dose_signal_t signal)
 {
     dose_signal_t rc = DOSE_SIGNAL_NONE;
 
-    if (ctx->state != DOSE_STATE_RECV && gpio_is_valid(ctx->sense_pin)) {
+    if (ctx->state != DOSE_STATE_RECV) {
         /* We freshly entered this state. Thus, no start bit sensing is required
-         * anymore. Disable GPIO IRQs during the transmission. */
-        gpio_irq_disable(ctx->sense_pin);
+         * anymore. Disable RX Start IRQs during the transmission. */
+        _disable_sense(ctx);
     }
 
     if (signal == DOSE_SIGNAL_UART) {
@@ -149,9 +183,9 @@ static dose_signal_t state_transit_send(dose_t *ctx, dose_signal_t signal)
 {
     (void) signal;
 
-    if (ctx->state != DOSE_STATE_SEND && gpio_is_valid(ctx->sense_pin)) {
-        /* Disable GPIO IRQs during the transmission. */
-        gpio_irq_disable(ctx->sense_pin);
+    if (ctx->state != DOSE_STATE_SEND) {
+        /* Disable RX Start IRQs during the transmission. */
+        _disable_sense(ctx);
     }
 
     /* Don't trace any END octets ... the timeout or the END signal
@@ -550,11 +584,7 @@ void dose_setup(dose_t *ctx, const dose_params_t *params, uint8_t index)
     ctx->uart = params->uart;
     uart_init(ctx->uart, params->baudrate, _isr_uart, (void *) ctx);
 
-    ctx->sense_pin = params->sense_pin;
-    if (gpio_is_valid(ctx->sense_pin)) {
-        gpio_init_int(ctx->sense_pin, GPIO_IN, GPIO_FALLING, _isr_gpio, (void *) ctx);
-        gpio_irq_disable(ctx->sense_pin);
-    }
+    _init_sense(ctx, params);
 
     netdev_register(&ctx->netdev, NETDEV_DOSE, index);
 

drivers/dose/include/dose_params.h

@@ -40,10 +40,15 @@ extern "C" {
 #endif
 
 #ifndef DOSE_PARAMS
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+#define DOSE_PARAMS             { .uart = DOSE_PARAM_UART,         \
+                                  .baudrate = DOSE_PARAM_BAUDRATE }
+#else
 #define DOSE_PARAMS             { .uart = DOSE_PARAM_UART,         \
                                   .baudrate = DOSE_PARAM_BAUDRATE, \
                                   .sense_pin = DOSE_PARAM_SENSE_PIN }
 #endif
+#endif
 /**@}*/
 
 /**

drivers/include/dose.h

@@ -31,7 +31,8 @@
  * you could use an IC such as the SN65HVD233.)
  *
  * Basically, UART TX and RX are connected to respective pins of the
- * transceiver. In addition, the RX pin can also be connected to the sense GPIO.
+ * transceiver. In addition, the RX pin can also be connected to the sense GPIO
+ * if the UART does not implement the `periph_uart_rxstart_irq` feature.
  * In this case, the bus allocation can be detected more precisely and
  * collisions are less likely.
  *
@@ -157,7 +158,9 @@ typedef struct {
     size_t recv_buf_ptr;                    /**< Index of the next empty octet of the recveive buffer */
     uart_t uart;                            /**< UART device to use */
     uint8_t uart_octet;                     /**< Last received octet */
+#if !defined(MODULE_PERIPH_UART_RXSTART_IRQ) || DOXYGEN
     gpio_t sense_pin;                       /**< GPIO to sense for start bits on the UART's rx line */
+#endif
     xtimer_t timeout;                       /**< Timeout timer ensuring always to get back to IDLE state */
     uint32_t timeout_base;                  /**< Base timeout in us */
 } dose_t;
@@ -167,7 +170,9 @@ typedef struct {
  */
 typedef struct {
     uart_t uart;                            /**< UART device to use */
+#if !defined(MODULE_PERIPH_UART_RXSTART_IRQ) || DOXYGEN
     gpio_t sense_pin;                       /**< GPIO to sense for start bits on the UART's rx line */
+#endif
     uint32_t baudrate;                      /**< Baudrate to UART device */
 } dose_params_t;
 

drivers/include/periph/uart.h

@@ -99,13 +99,24 @@ typedef unsigned int uart_t;
  */
 typedef void(*uart_rx_cb_t)(void *arg, uint8_t data);
 
+/**
+ * @brief   Signature for receive start condition interrupt callback
+ *
+ * @param[in] arg           context to the callback (optional)
+ */
+typedef void(*uart_rxstart_cb_t)(void *arg);
+
 /**
  * @brief   Interrupt context for a UART device
  */
 #ifndef HAVE_UART_ISR_CTX_T
 typedef struct {
     uart_rx_cb_t rx_cb;     /**< data received interrupt callback */
-    void *arg;              /**< argument to both callback routines */
+    void *arg;              /**< argument to data received callback */
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    uart_rxstart_cb_t rxs_cb;   /**< start condition received interrupt callback */
+    void *rxs_arg;          /**< argument to start condition received callback */
+#endif
 } uart_isr_ctx_t;
 #endif
 
@@ -246,6 +257,45 @@ gpio_t uart_pin_tx(uart_t uart);
 #endif /* DOXYGEN */
 #endif /* MODULE_PERIPH_UART_RECONFIGURE */
 
+#if defined(MODULE_PERIPH_UART_RXSTART_IRQ) || DOXYGEN
+
+/**
+ * @brief   Configure the function that will be called when a start condition
+ *          is detected.
+ *
+ *          This will not enable / disable the generation of the RX start
+ *          interrupt.
+ *
+ * @note    You have to add the module `periph_uart_rxstart_irq` to your project
+ *          to enable this function
+ *
+ * @param[in] uart      The device to configure
+ * @param[in] cb        The function called when a start condition is detected
+ * @param[in] arg       Optional function argument
+ */
+void uart_rxstart_irq_configure(uart_t uart, uart_rxstart_cb_t cb, void *arg);
+
+/**
+ * @brief   Enable the RX start interrupt.
+ *
+ * @note    You have to add the module `periph_uart_rxstart_irq` to your project
+ *          to enable this function
+ *
+ * @param[in] uart      The device to configure
+ */
+void uart_rxstart_irq_enable(uart_t uart);
+
+/**
+ * @brief   Disable the RX start interrupt.
+ *
+ * @note    You have to add the module `periph_uart_rxstart_irq` to your project
+ *          to enable this function
+ *
+ * @param[in] uart      The device to configure
+ */
+void uart_rxstart_irq_disable(uart_t uart);
+#endif /* MODULE_PERIPH_UART_RXSTART_IRQ */
+
 /**
  * @brief   Setup parity, data and stop bits for a given UART device
  *

drivers/periph_common/Kconfig.uart

@@ -32,6 +32,10 @@ config MODULE_PERIPH_UART_NONBLOCKING
     bool "Non-blocking support"
     depends on HAS_PERIPH_UART_NONBLOCKING
 
+config MODULE_PERIPH_UART_RXSTART_IRQ
+    bool "Enable Start Condition Interrupt"
+    depends on HAS_PERIPH_UART_RXSTART_IRQ
+
 config MODULE_PERIPH_INIT_UART_MODECFG
     bool
     depends on MODULE_PERIPH_UART_MODECFG

kconfigs/Kconfig.features

@@ -333,6 +333,11 @@ config HAS_PERIPH_UART_RECONFIGURE
     help
         Indicates that the UART pins can be re-configured as GPIOs.
 
+config HAS_PERIPH_UART_RXSTART_IRQ
+    bool
+    help
+        Indicates that the UART has an Interrupt for Start Condition detected.
+
 config HAS_PERIPH_USBDEV
     bool
     help

tests/periph_uart/Kconfig

@@ -10,4 +10,5 @@ config APPLICATION
     default y
     imply MODULE_PERIPH_UART_MODECFG
     imply MODULE_PERIPH_LPUART
+    imply MODULE_PERIPH_UART_RXSTART_IRQ
     depends on TEST_KCONFIG

tests/periph_uart/Makefile

@@ -3,6 +3,7 @@ include ../Makefile.tests_common
 FEATURES_REQUIRED += periph_uart
 FEATURES_OPTIONAL += periph_lpuart  # STM32 L0 and L4 provides lpuart support
 FEATURES_OPTIONAL += periph_uart_modecfg
+FEATURES_OPTIONAL += periph_uart_rxstart_irq
 
 USEMODULE += shell
 USEMODULE += xtimer

tests/periph_uart/main.c

@@ -50,6 +50,10 @@
 #define STDIO_UART_DEV      (UART_UNDEF)
 #endif
 
+#ifndef STX
+#define STX 0x2
+#endif
+
 typedef struct {
     char rx_mem[UART_BUFSIZE];
     ringbuffer_t rx_buf;
@@ -60,6 +64,8 @@ static uart_ctx_t ctx[UART_NUMOF];
 static kernel_pid_t printer_pid;
 static char printer_stack[THREAD_STACKSIZE_MAIN];
 
+static bool test_mode;
+
 #ifdef MODULE_PERIPH_UART_MODECFG
 static uart_data_bits_t data_bits_lut[] = { UART_DATA_BITS_5, UART_DATA_BITS_6,
                                             UART_DATA_BITS_7, UART_DATA_BITS_8 };
@@ -83,18 +89,73 @@ static int parse_dev(char *arg)
     return dev;
 }
 
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+static void rxs_cb(void *arg)
+{
+    ringbuffer_add_one(arg, STX);
+}
+#endif
+
 static void rx_cb(void *arg, uint8_t data)
 {
     uart_t dev = (uart_t)arg;
 
-    ringbuffer_add_one(&(ctx[dev].rx_buf), data);
-    if (data == '\n') {
+    ringbuffer_add_one(&ctx[dev].rx_buf, data);
+
+    if (!test_mode && data == '\n') {
         msg_t msg;
         msg.content.value = (uint32_t)dev;
         msg_send(&msg, printer_pid);
     }
 }
 
+static int _self_test(uart_t dev, unsigned baud)
+{
+    const char test_string[] = "Hello UART!";
+
+    if (uart_init(UART_DEV(dev), baud, rx_cb, (void *)dev)) {
+        printf("error configuring %u baud\n", baud);
+        return -1;
+    }
+
+    test_mode = true;
+
+    uart_write(dev, (uint8_t*)test_string, sizeof(test_string));
+    for (unsigned i = 0; i < sizeof(test_string); ++i) {
+        int c = ringbuffer_get_one(&ctx[dev].rx_buf);
+        if (c != test_string[i]) {
+            printf("mismatch at index %u: %x != %x\n", i, c, test_string[i]);
+            return -1;
+        }
+    }
+
+#ifdef MODULE_PERIPH_UART_RXSTART_IRQ
+    /* test RX Start detection if available */
+    uart_rxstart_irq_configure(dev, rxs_cb, &ctx[dev].rx_buf);
+    uart_rxstart_irq_enable(dev);
+
+    uart_write(dev, (uint8_t*)test_string, sizeof(test_string));
+    for (unsigned i = 0; i < sizeof(test_string); ++i) {
+        int c = ringbuffer_get_one(&ctx[dev].rx_buf);
+        if (c != STX) {
+            printf("expected start condition, got %x\n", c);
+            return -1;
+        }
+
+        c = ringbuffer_get_one(&ctx[dev].rx_buf);
+        if (c != test_string[i]) {
+            printf("mismatch at index %u: %x != %x, start condition reported\n",
+                   i, c, test_string[i]);
+            return -1;
+        }
+    }
+    uart_rxstart_irq_disable(dev);
+#endif
+
+    test_mode = false;
+    return 0;
+}
+
 static void *printer(void *arg)
 {
     (void)arg;
@@ -259,12 +320,41 @@ static int cmd_send(int argc, char **argv)
     return 0;
 }
 
+static int cmd_test(int argc, char **argv)
+{
+    int dev;
+
+    if (argc < 2) {
+        printf("usage: %s <dev>\n", argv[0]);
+        return 1;
+    }
+    /* parse parameters */
+    dev = parse_dev(argv[1]);
+    if (dev < 0) {
+        return 1;
+    }
+
+    puts("[START]");
+
+    /* run self test with different baud rates */
+    for (unsigned i = 1; i <= 12; ++i) {
+        if (_self_test(dev, 9600 * i)) {
+            puts("[FAILURE]");
+            return -1;
+        }
+    }
+
+    puts("[SUCCESS]");
+    return 0;
+}
+
 static const shell_command_t shell_commands[] = {
     { "init", "Initialize a UART device with a given baudrate", cmd_init },
 #ifdef MODULE_PERIPH_UART_MODECFG
     { "mode", "Setup data bits, stop bits and parity for a given UART device", cmd_mode },
 #endif
     { "send", "Send a string through given UART device", cmd_send },
+    { "test", "Run an automated test on a UART with RX and TX connected", cmd_test },
     { NULL, NULL, NULL }
 };