From 0d977b3b3c415b0abced414c90a14903c813b82c Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Tue, 20 Aug 2019 20:52:07 +0200 Subject: [PATCH 1/2] cpu/sam0_common/periph/uart: implement buffered write Implement interrupt based uart_write() using a tsrb for the TX buffer. To enable it, add USEMODULE += periph_uart_nonblocking to your Makefile. --- Makefile.dep | 5 + boards/common/saml1x/include/periph_conf.h | 1 + boards/same54-xpro/include/periph_conf.h | 1 + cpu/sam0_common/Makefile.dep | 3 + cpu/sam0_common/Makefile.features | 1 + cpu/sam0_common/include/periph_cpu_common.h | 8 ++ cpu/sam0_common/periph/uart.c | 118 ++++++++++++++++++-- cpu/samd21/Makefile.dep | 1 + cpu/samd5x/Makefile.dep | 1 + cpu/saml1x/Makefile.dep | 1 + cpu/saml21/Makefile.dep | 1 + 11 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 cpu/sam0_common/Makefile.dep create mode 100644 cpu/samd21/Makefile.dep create mode 100644 cpu/samd5x/Makefile.dep create mode 100644 cpu/saml1x/Makefile.dep create mode 100644 cpu/saml21/Makefile.dep diff --git a/Makefile.dep b/Makefile.dep index 5e74feb159..fdcb2e3706 100644 --- a/Makefile.dep +++ b/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 diff --git a/boards/common/saml1x/include/periph_conf.h b/boards/common/saml1x/include/periph_conf.h index b2014a639a..3baf0e6bb9 100644 --- a/boards/common/saml1x/include/periph_conf.h +++ b/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) /** @} */ diff --git a/boards/same54-xpro/include/periph_conf.h b/boards/same54-xpro/include/periph_conf.h index 80dffffbd2..b4125c6c93 100644 --- a/boards/same54-xpro/include/periph_conf.h +++ b/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) /** @} */ diff --git a/cpu/sam0_common/Makefile.dep b/cpu/sam0_common/Makefile.dep new file mode 100644 index 0000000000..d14f2bdb7b --- /dev/null +++ b/cpu/sam0_common/Makefile.dep @@ -0,0 +1,3 @@ +ifneq (,$(filter periph_uart_nonblocking,$(USEMODULE))) + USEMODULE += tsrb +endif diff --git a/cpu/sam0_common/Makefile.features b/cpu/sam0_common/Makefile.features index 63eb3f31ae..38f83ce8b7 100644 --- a/cpu/sam0_common/Makefile.features +++ b/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 diff --git a/cpu/sam0_common/include/periph_cpu_common.h b/cpu/sam0_common/include/periph_cpu_common.h index 65ed47eaff..e0180acd5f 100644 --- a/cpu/sam0_common/include/periph_cpu_common.h +++ b/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 */ diff --git a/cpu/sam0_common/periph/uart.c b/cpu/sam0_common/periph/uart.c index 29833fe694..1d3ed1bb38 100644 --- a/cpu/sam0_common/periph/uart.c +++ b/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 */ diff --git a/cpu/samd21/Makefile.dep b/cpu/samd21/Makefile.dep new file mode 100644 index 0000000000..eccd575bc9 --- /dev/null +++ b/cpu/samd21/Makefile.dep @@ -0,0 +1 @@ +include $(RIOTCPU)/sam0_common/Makefile.dep diff --git a/cpu/samd5x/Makefile.dep b/cpu/samd5x/Makefile.dep new file mode 100644 index 0000000000..eccd575bc9 --- /dev/null +++ b/cpu/samd5x/Makefile.dep @@ -0,0 +1 @@ +include $(RIOTCPU)/sam0_common/Makefile.dep diff --git a/cpu/saml1x/Makefile.dep b/cpu/saml1x/Makefile.dep new file mode 100644 index 0000000000..eccd575bc9 --- /dev/null +++ b/cpu/saml1x/Makefile.dep @@ -0,0 +1 @@ +include $(RIOTCPU)/sam0_common/Makefile.dep diff --git a/cpu/saml21/Makefile.dep b/cpu/saml21/Makefile.dep new file mode 100644 index 0000000000..eccd575bc9 --- /dev/null +++ b/cpu/saml21/Makefile.dep @@ -0,0 +1 @@ +include $(RIOTCPU)/sam0_common/Makefile.dep From 0c08abd19a11c64e2e5349e508e7c53da90add99 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 15 Nov 2019 14:43:19 +0100 Subject: [PATCH 2/2] tests/periph_uart_nonblocking: add simple test application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The application is mainly to compile-test non-blocking UART functionality, but some functional testing is also possible. With non-blocking UART the total runtime of the program is 2100735 µs on same54-xpro. With blocking UART the total runtime is 2152407 µs. --- tests/periph_uart_nonblocking/Makefile | 6 ++ tests/periph_uart_nonblocking/main.c | 69 +++++++++++++++++++ tests/periph_uart_nonblocking/tests/01-run.py | 24 +++++++ 3 files changed, 99 insertions(+) create mode 100644 tests/periph_uart_nonblocking/Makefile create mode 100644 tests/periph_uart_nonblocking/main.c create mode 100755 tests/periph_uart_nonblocking/tests/01-run.py diff --git a/tests/periph_uart_nonblocking/Makefile b/tests/periph_uart_nonblocking/Makefile new file mode 100644 index 0000000000..dd5a14dadc --- /dev/null +++ b/tests/periph_uart_nonblocking/Makefile @@ -0,0 +1,6 @@ +include ../Makefile.tests_common + +FEATURES_REQUIRED += periph_uart_nonblocking +USEMODULE += xtimer + +include $(RIOTBASE)/Makefile.include diff --git a/tests/periph_uart_nonblocking/main.c b/tests/periph_uart_nonblocking/main.c new file mode 100644 index 0000000000..62e2f6fabb --- /dev/null +++ b/tests/periph_uart_nonblocking/main.c @@ -0,0 +1,69 @@ +/* + * 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. + */ + +/** + * @ingroup tests + * @{ + * + * @file + * @brief Simple test application for non-blocking UART functionality + * + * @author Benjamin Valentin + * + * @} + */ + +#include +#include + +#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; +} diff --git a/tests/periph_uart_nonblocking/tests/01-run.py b/tests/periph_uart_nonblocking/tests/01-run.py new file mode 100755 index 0000000000..11f5abbc9a --- /dev/null +++ b/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))