From 95d8deb57294805bb9409912bb4de5215edddc1b Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Tue, 5 Apr 2022 20:12:56 +1000 Subject: [PATCH] modem: modem_iface_uart_async: added Adds a communications backend based on the asynchronous UART API, instead of the interrupt-driven UART API. The primary advantage of this backend is an improved robustness to dropping bytes under high interrupt or critical section loads. Under all loads system efficiency is improved by: * Reducing the time spent writing out individual bytes. * Reducing the number of UART interrupts fired. * Waking up the RX thread much less often. When utilising this backend over `nordic,nrf-uarte` on a nRF52840, the baudrate of an esp-at modem could be pushed to at least 921600 without dropping bytes, compared to a maximum of 230400 with the interrupt API. Signed-off-by: Jordan Yates --- drivers/modem/CMakeLists.txt | 1 + drivers/modem/Kconfig | 32 +++++ drivers/modem/modem_iface_uart.h | 7 + drivers/modem/modem_iface_uart_async.c | 180 +++++++++++++++++++++++++ 4 files changed, 220 insertions(+) create mode 100644 drivers/modem/modem_iface_uart_async.c diff --git a/drivers/modem/CMakeLists.txt b/drivers/modem/CMakeLists.txt index 730aad5f0be..b65e9d3ecec 100644 --- a/drivers/modem/CMakeLists.txt +++ b/drivers/modem/CMakeLists.txt @@ -11,6 +11,7 @@ zephyr_library_sources_ifdef(CONFIG_MODEM_CONTEXT ) zephyr_library_sources_ifdef(CONFIG_MODEM_IFACE_UART_INTERRUPT modem_iface_uart_interrupt.c) +zephyr_library_sources_ifdef(CONFIG_MODEM_IFACE_UART_ASYNC modem_iface_uart_async.c) zephyr_library_sources_ifdef(CONFIG_MODEM_CMD_HANDLER modem_cmd_handler.c) zephyr_library_sources_ifdef(CONFIG_MODEM_SOCKET modem_socket.c) diff --git a/drivers/modem/Kconfig b/drivers/modem/Kconfig index 453fa41f0e0..c80f19528d6 100644 --- a/drivers/modem/Kconfig +++ b/drivers/modem/Kconfig @@ -82,8 +82,40 @@ config MODEM_IFACE_UART_INTERRUPT depends on SERIAL_SUPPORT_INTERRUPT select UART_INTERRUPT_DRIVEN +config MODEM_IFACE_UART_ASYNC + bool "UART-based modem interface using async API" + depends on SERIAL_SUPPORT_ASYNC + select UART_ASYNC_API + endchoice +if MODEM_IFACE_UART_ASYNC + +config MODEM_IFACE_UART_ASYNC_RX_BUFFER_SIZE + int "Size in bytes of the RX buffers provided to UART driver" + default 64 + help + Increasing this value decreases the number of UART interrupts needed + to receive large packets. + +config MODEM_IFACE_UART_ASYNC_RX_NUM_BUFFERS + int "Number of RX buffers available to the UART driver" + default 2 + help + This value needs to be twice the number of UART modems using the + driver to avoid buffer starvation. + +config MODEM_IFACE_UART_ASYNC_RX_TIMEOUT_US + int "Timeout for flushing RX buffers after receiving no additional data" + default 278 + help + Decreasing this value can help increase data throughput when high + baudrates are used. 278us is 4 bytes at 115200 baud. Decreasing this + value too much can result in spurious interrupts. Leaving it too + high can reduce data throughput. + +endif # MODEM_IFACE_UART_ASYNC + endif # MODEM_IFACE_UART config MODEM_CMD_HANDLER diff --git a/drivers/modem/modem_iface_uart.h b/drivers/modem/modem_iface_uart.h index bdd32946f5a..a60be74d2ef 100644 --- a/drivers/modem/modem_iface_uart.h +++ b/drivers/modem/modem_iface_uart.h @@ -32,6 +32,13 @@ struct modem_iface_uart_data { /* rx semaphore */ struct k_sem rx_sem; + +#ifdef CONFIG_MODEM_IFACE_UART_ASYNC + + /* tx semaphore */ + struct k_sem tx_sem; + +#endif /* CONFIG_MODEM_IFACE_UART_ASYNC */ }; /** diff --git a/drivers/modem/modem_iface_uart_async.c b/drivers/modem/modem_iface_uart_async.c new file mode 100644 index 00000000000..35d76a24d80 --- /dev/null +++ b/drivers/modem/modem_iface_uart_async.c @@ -0,0 +1,180 @@ +/* + * Copyright (c) 2022, Commonwealth Scientific and Industrial Research + * Organisation (CSIRO) ABN 41 687 119 230. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +#include "modem_context.h" +#include "modem_iface_uart.h" + +LOG_MODULE_REGISTER(modem_iface_uart_async, CONFIG_MODEM_LOG_LEVEL); + +#define RX_BUFFER_SIZE CONFIG_MODEM_IFACE_UART_ASYNC_RX_BUFFER_SIZE +#define RX_BUFFER_NUM CONFIG_MODEM_IFACE_UART_ASYNC_RX_NUM_BUFFERS + +K_MEM_SLAB_DEFINE(uart_modem_async_rx_slab, RX_BUFFER_SIZE, RX_BUFFER_NUM, 1); + +static void iface_uart_async_callback(const struct device *dev, + struct uart_event *evt, + void *user_data) +{ + struct modem_iface *iface = user_data; + struct modem_iface_uart_data *data = iface->iface_data; + uint32_t written; + void *buf; + int rc; + + switch (evt->type) { + case UART_TX_DONE: + k_sem_give(&data->tx_sem); + break; + case UART_RX_BUF_REQUEST: + /* Allocate next RX buffer for UART driver */ + rc = k_mem_slab_alloc(&uart_modem_async_rx_slab, (void **)&buf, K_NO_WAIT); + if (rc < 0) { + /* Major problems, UART_RX_BUF_RELEASED event is not being generated, or + * CONFIG_MODEM_IFACE_UART_ASYNC_RX_NUM_BUFFERS is not large enough. + */ + LOG_ERR("RX buffer starvation"); + break; + } + /* Provide the buffer to the UART driver */ + uart_rx_buf_rsp(dev, buf, RX_BUFFER_SIZE); + break; + case UART_RX_BUF_RELEASED: + /* UART driver is done with memory, free it */ + k_mem_slab_free(&uart_modem_async_rx_slab, (void **)&evt->data.rx_buf.buf); + break; + case UART_RX_RDY: + /* Place received data on the ring buffer */ + written = ring_buf_put(&data->rx_rb, + evt->data.rx.buf + evt->data.rx.offset, + evt->data.rx.len); + if (written != evt->data.rx.len) { + LOG_WRN("Received bytes dropped from ring buf"); + } + /* Notify upper layer that new data has arrived */ + k_sem_give(&data->rx_sem); + break; + default: + break; + } +} + +static int modem_iface_uart_async_read(struct modem_iface *iface, + uint8_t *buf, size_t size, size_t *bytes_read) +{ + struct modem_iface_uart_data *data; + + if (!iface || !iface->iface_data) { + return -EINVAL; + } + + if (size == 0) { + *bytes_read = 0; + return 0; + } + + /* Pull data off the ring buffer */ + data = iface->iface_data; + *bytes_read = ring_buf_get(&data->rx_rb, buf, size); + return 0; +} + +static int modem_iface_uart_async_write(struct modem_iface *iface, + const uint8_t *buf, size_t size) +{ + struct modem_iface_uart_data *data; + int rc; + + if (!iface || !iface->iface_data) { + return -EINVAL; + } + + if (size == 0) { + return 0; + } + + /* Start the transmission */ + rc = uart_tx(iface->dev, buf, size, SYS_FOREVER_MS); + if (rc >= 0) { + /* Wait until the transmission completes */ + data = iface->iface_data; + k_sem_take(&data->tx_sem, K_FOREVER); + } + return rc; +} + +int modem_iface_uart_init_dev(struct modem_iface *iface, + const struct device *dev) +{ + struct modem_iface_uart_data *data; + void *buf; + int rc; + + if (!device_is_ready(dev)) { + return -ENODEV; + } + + /* Check if there's already a device inited to this iface. If so, + * interrupts needs to be disabled on that too before switching to avoid + * race conditions with modem_iface_uart_isr. + */ + if (iface->dev) { + LOG_WRN("Device %s already inited", iface->dev->name); + uart_rx_disable(iface->dev); + } + + iface->dev = dev; + data = iface->iface_data; + + /* Configure async UART callback */ + rc = uart_callback_set(dev, iface_uart_async_callback, iface); + if (rc < 0) { + LOG_ERR("Failed to set UART callback"); + return rc; + } + /* Enable reception permanently on the interface */ + k_mem_slab_alloc(&uart_modem_async_rx_slab, (void **)&buf, K_FOREVER); + rc = uart_rx_enable(dev, buf, RX_BUFFER_SIZE, CONFIG_MODEM_IFACE_UART_ASYNC_RX_TIMEOUT_US); + if (rc < 0) { + LOG_ERR("Failed to enable UART RX"); + } + return rc; +} + +int modem_iface_uart_init(struct modem_iface *iface, + struct modem_iface_uart_data *data, + const struct device *dev) +{ + int ret; + + if (!iface || !data) { + return -EINVAL; + } + + iface->iface_data = data; + iface->read = modem_iface_uart_async_read; + iface->write = modem_iface_uart_async_write; + + ring_buf_init(&data->rx_rb, data->rx_rb_buf_len, data->rx_rb_buf); + k_sem_init(&data->rx_sem, 0, 1); + k_sem_init(&data->tx_sem, 0, 1); + + /* get UART device */ + ret = modem_iface_uart_init_dev(iface, dev); + if (ret < 0) { + iface->iface_data = NULL; + iface->read = NULL; + iface->write = NULL; + + return ret; + } + + return 0; +}