From 9194440ead1837dccea6a76fa98c35b48fdf0032 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 27 May 2021 22:19:40 +0200 Subject: [PATCH 1/4] drivers/periph/uart: add periph_uart_rx_start feature --- drivers/include/periph/uart.h | 52 +++++++++++++++++++++++++++++- drivers/periph_common/Kconfig.uart | 4 +++ kconfigs/Kconfig.features | 5 +++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/drivers/include/periph/uart.h b/drivers/include/periph/uart.h index d293cab2d1..c7f49b9317 100644 --- a/drivers/include/periph/uart.h +++ b/drivers/include/periph/uart.h @@ -98,13 +98,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 @@ -243,6 +254,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 * diff --git a/drivers/periph_common/Kconfig.uart b/drivers/periph_common/Kconfig.uart index 2d87dd4818..8e5402f3fc 100644 --- a/drivers/periph_common/Kconfig.uart +++ b/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 diff --git a/kconfigs/Kconfig.features b/kconfigs/Kconfig.features index 43b800f18a..c31359934c 100644 --- a/kconfigs/Kconfig.features +++ b/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 From 02269ef8698af3290786f59c431a9d67675140ef Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 27 May 2021 22:20:15 +0200 Subject: [PATCH 2/4] cpu/sam0_common: implement periph_uart_rx_start feature --- cpu/sam0_common/Kconfig | 1 + cpu/sam0_common/Makefile.features | 1 + cpu/sam0_common/periph/uart.c | 47 ++++++++++++++++++++++++++----- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/cpu/sam0_common/Kconfig b/cpu/sam0_common/Kconfig index cb0248f79c..1d7d50c358 100644 --- a/cpu/sam0_common/Kconfig +++ b/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 diff --git a/cpu/sam0_common/Makefile.features b/cpu/sam0_common/Makefile.features index 49a6e245e9..e541dc161b 100644 --- a/cpu/sam0_common/Makefile.features +++ b/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 diff --git a/cpu/sam0_common/periph/uart.c b/cpu/sam0_common/periph/uart.c index 48ddeb9dc6..b330cdcafb 100644 --- a/cpu/sam0_common/periph/uart.c +++ b/cpu/sam0_common/periph/uart.c @@ -368,7 +368,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) @@ -389,6 +420,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) { @@ -396,17 +429,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(); } From d48f673597c827b1344fe595256f2e4879f5d55b Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 27 May 2021 22:20:53 +0200 Subject: [PATCH 3/4] drivers/dose: make use of periph_uart_rx_start feature --- drivers/dose/Makefile.dep | 1 + drivers/dose/dose.c | 60 ++++++++++++++++++++++-------- drivers/dose/include/dose_params.h | 5 +++ drivers/include/dose.h | 7 +++- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/drivers/dose/Makefile.dep b/drivers/dose/Makefile.dep index 07fc6ad171..a2b81d4233 100644 --- a/drivers/dose/Makefile.dep +++ b/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 diff --git a/drivers/dose/dose.c b/drivers/dose/dose.c index e4e882fdd1..4cc207adbf 100644 --- a/drivers/dose/dose.c +++ b/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((netdev_t *) ctx); } - 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); diff --git a/drivers/dose/include/dose_params.h b/drivers/dose/include/dose_params.h index e91c01b5b6..f493f41365 100644 --- a/drivers/dose/include/dose_params.h +++ b/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 /**@}*/ /** diff --git a/drivers/include/dose.h b/drivers/include/dose.h index 0fad35cc69..7a3479d29b 100644 --- a/drivers/include/dose.h +++ b/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; From 1cacb11a261928e4552934791b3cff0139c76fbc Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 31 May 2021 16:35:50 +0200 Subject: [PATCH 4/4] tests/periph_uart: add automated test --- tests/periph_uart/Kconfig | 1 + tests/periph_uart/Makefile | 1 + tests/periph_uart/main.c | 94 +++++++++++++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/tests/periph_uart/Kconfig b/tests/periph_uart/Kconfig index e43f5f8de8..394cc6b398 100644 --- a/tests/periph_uart/Kconfig +++ b/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 diff --git a/tests/periph_uart/Makefile b/tests/periph_uart/Makefile index c7fa83da21..20dd17f59a 100644 --- a/tests/periph_uart/Makefile +++ b/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 diff --git a/tests/periph_uart/main.c b/tests/periph_uart/main.c index 90afb22167..0a58ae7d09 100644 --- a/tests/periph_uart/main.c +++ b/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 \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 } };