From e1cfa0b2db3deaf7d083c42f026ca3549b41e400 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 25 Apr 2024 22:47:35 +0200 Subject: [PATCH 1/2] drivers/periph_uart: document acquire/release semantic The UART API has not spelled out what happens when `uart_init()` is called twice. This adds precise language that states that acquire/release semantic is to be expected from the caller. Hence, a caller needs to call `uart_poweroff()` before reconfiguring the UART with a second call `uart_init()` for the same UART interface. In practise, few apps will ever reconfigure the symbol rate. So the impact is rather low. However: This API now allows drivers to implement sharing of a serial peripheral that can provide multiple interfaces (e.g. UART, SPI, I2C, etc.). It would require some cooperation from the code that does use the UART to actually release the UART again after each transaction; something that will only work when RX data is only expected at known points in time (e.g. in response to a request, or never in case of TX only stdio). But still, this can mean the difference between a use case becoming feasible on an MCU with a low number of serial peripherals or not. --- drivers/include/periph/uart.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/include/periph/uart.h b/drivers/include/periph/uart.h index 87bbc965c1d6..b57e4ea55864 100644 --- a/drivers/include/periph/uart.h +++ b/drivers/include/periph/uart.h @@ -177,7 +177,7 @@ typedef enum { #endif /** - * @brief Initialize a given UART device + * @brief Initialize and acquire a given UART device * * The UART device will be initialized with the following configuration: * - 8 data bits @@ -188,6 +188,14 @@ typedef enum { * If no callback parameter is given (rx_cb := NULL), the UART will be * initialized in TX only mode. * + * @pre The caller is not calling `uart_init()` twice without a call of + * to @ref uart_poweroff in between. + * + * @note This may block if the UART is already acquired until it is released. + * This allows sharing the underlying peripheral to provide other + * serial interfaces (e.g. if the peripheral can also provide SPI, I2C, + * etc.) + * * @param[in] uart UART device to initialize * @param[in] baud desired symbol rate in baud * @param[in] rx_cb receive callback, executed in interrupt context once @@ -401,14 +409,17 @@ int uart_mode(uart_t uart, uart_data_bits_t data_bits, uart_parity_t parity, void uart_write(uart_t uart, const uint8_t *data, size_t len); /** - * @brief Power on the given UART device + * @brief Power on and acquire the given UART device + * + * The UART will resume with the configuration it was most recently configured + * with. * * @param[in] uart the UART device to power on */ void uart_poweron(uart_t uart); /** - * @brief Power off the given UART device + * @brief Power off and release the given UART device * * @param[in] uart the UART device to power off */ From ebec3258c31eded98479118ae9abde35ff8828a1 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 25 Apr 2024 22:54:42 +0200 Subject: [PATCH 2/2] tests/periph/uart: update to API change Release the UART before acquiring it again to not dead-lock the code if acquire/release semantics is implemented by the UART peripheral. --- tests/periph/uart/Makefile | 1 + tests/periph/uart/main.c | 24 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/periph/uart/Makefile b/tests/periph/uart/Makefile index b60500e81f0e..c8ec516c0fd3 100644 --- a/tests/periph/uart/Makefile +++ b/tests/periph/uart/Makefile @@ -6,6 +6,7 @@ FEATURES_OPTIONAL += periph_uart_collision FEATURES_OPTIONAL += periph_uart_modecfg FEATURES_OPTIONAL += periph_uart_rxstart_irq +USEMODULE += bitfield USEMODULE += shell USEMODULE += ztimer_msec diff --git a/tests/periph/uart/main.c b/tests/periph/uart/main.c index 6c5a17a6eacb..b7b36f597650 100644 --- a/tests/periph/uart/main.c +++ b/tests/periph/uart/main.c @@ -22,11 +22,12 @@ #include #include -#include "shell.h" -#include "thread.h" +#include "bitfield.h" #include "msg.h" -#include "ringbuffer.h" #include "periph/uart.h" +#include "ringbuffer.h" +#include "shell.h" +#include "thread.h" #include "ztimer.h" #ifdef MODULE_STDIO_UART @@ -86,6 +87,8 @@ static uart_stop_bits_t stop_bits_lut[] = { UART_STOP_BITS_1, UART_STOP_BITS_2 } static int stop_bits_lut_len = ARRAY_SIZE(stop_bits_lut); #endif +static BITFIELD(uarts_initialized_mask, UART_NUMOF); + static int parse_dev(char *arg) { unsigned dev = atoi(arg); @@ -180,6 +183,9 @@ static int _self_test(uart_t dev, unsigned baud) uart_collision_detect_disable(dev); #endif + uart_poweroff(UART_DEV(dev)); + + test_mode = false; return 0; failure: @@ -248,6 +254,11 @@ static int cmd_init(int argc, char **argv) } baud = strtol(argv[2], NULL, 0); + if (bf_isset(uarts_initialized_mask, dev)) { + uart_poweroff(UART_DEV(dev)); + bf_unset(uarts_initialized_mask, dev); + } + /* initialize UART */ res = uart_init(UART_DEV(dev), baud, rx_cb, (void *)(intptr_t)dev); if (res == UART_NOBAUD) { @@ -260,6 +271,8 @@ static int cmd_init(int argc, char **argv) } printf("Success: Initialized UART_DEV(%i) at BAUD %"PRIu32"\n", dev, baud); + bf_set(uarts_initialized_mask, dev); + /* also test if poweron() and poweroff() work (or at least don't break * anything) */ sleep_test(dev, UART_DEV(dev)); @@ -396,6 +409,11 @@ static int cmd_test(int argc, char **argv) puts("[START]"); + if (bf_isset(uarts_initialized_mask, dev)) { + uart_poweroff(UART_DEV(dev)); + bf_unset(uarts_initialized_mask, dev); + } + /* run self test with different baud rates */ test_mode = true; for (unsigned i = 1; i <= 12; ++i) {