From 90b444cc12ee14f37e5c5d0f43decff310b464f8 Mon Sep 17 00:00:00 2001 From: Hake Huang Date: Wed, 2 Oct 2024 12:26:15 +0000 Subject: [PATCH 1/2] driver: uart: uart_mcux_lpuart fix issues in async api 1. optimized the logic for buffer usage in async api 2. skip timeout flush when the remaining counts is 0, as this will trigger dma_callback to process. 3. remove scatter mode, as we are not using this mode 4. trigger after dma_reload. Signed-off-by: Hake Huang --- drivers/serial/uart_mcux_lpuart.c | 62 ++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/drivers/serial/uart_mcux_lpuart.c b/drivers/serial/uart_mcux_lpuart.c index 233457af2c65cc..a4c775c5b4e49e 100644 --- a/drivers/serial/uart_mcux_lpuart.c +++ b/drivers/serial/uart_mcux_lpuart.c @@ -515,10 +515,11 @@ static void mcux_lpuart_async_rx_flush(const struct device *dev) const size_t rx_rcv_len = data->async.rx_dma_params.buf_len - status.pending_length; - if (rx_rcv_len > data->async.rx_dma_params.counter) { + if (rx_rcv_len > data->async.rx_dma_params.counter && status.pending_length) { data->async.rx_dma_params.counter = rx_rcv_len; async_evt_rx_rdy(dev); } + LPUART_ClearStatusFlags(config->base, kLPUART_RxOverrunFlag); } else { LOG_ERR("Error getting DMA status"); } @@ -584,7 +585,7 @@ static void prepare_rx_dma_block_config(const struct device *dev) head_block_config->dest_address = (uint32_t)rx_dma_params->buf; head_block_config->source_address = LPUART_GetDataRegisterAddress(lpuart); head_block_config->block_size = rx_dma_params->buf_len; - head_block_config->dest_scatter_en = true; + head_block_config->dest_scatter_en = false; } static int configure_and_start_rx_dma( @@ -615,20 +616,38 @@ static int uart_mcux_lpuart_dma_replace_rx_buffer(const struct device *dev) struct mcux_lpuart_data *data = (struct mcux_lpuart_data *)dev->data; const struct mcux_lpuart_config *config = dev->config; LPUART_Type *lpuart = config->base; + struct mcux_lpuart_rx_dma_params *rx_dma_params = &data->async.rx_dma_params; LOG_DBG("Replacing RX buffer, new length: %d", data->async.next_rx_buffer_len); + /* There must be a buffer to replace this one with */ assert(data->async.next_rx_buffer != NULL); assert(data->async.next_rx_buffer_len != 0U); - const int success = dma_reload(config->rx_dma_config.dma_dev, - config->rx_dma_config.dma_channel, - LPUART_GetDataRegisterAddress(lpuart), - (uint32_t)data->async.next_rx_buffer, - data->async.next_rx_buffer_len); + rx_dma_params->buf = data->async.next_rx_buffer; + rx_dma_params->buf_len = data->async.next_rx_buffer_len; + rx_dma_params->offset = 0; + rx_dma_params->counter = 0; + data->async.next_rx_buffer = NULL; + data->async.next_rx_buffer_len = 0U; + + const int success = + dma_reload(config->rx_dma_config.dma_dev, config->rx_dma_config.dma_channel, + LPUART_GetDataRegisterAddress(lpuart), (uint32_t)rx_dma_params->buf, + rx_dma_params->buf_len); if (success != 0) { LOG_ERR("Error %d reloading DMA with next RX buffer", success); } + /* Request next buffer */ + async_evt_rx_buf_request(dev); + + int ret = dma_start(config->rx_dma_config.dma_dev, config->rx_dma_config.dma_channel); + + if (ret < 0) { + LOG_ERR("Failed to start DMA(Rx) Ch %d(%d)", config->rx_dma_config.dma_channel, + ret); + } + return success; } @@ -673,16 +692,9 @@ static void dma_callback(const struct device *dma_dev, void *callback_arg, uint3 async_evt_rx_rdy(dev); async_evt_rx_buf_release(dev); - rx_dma_params->buf = data->async.next_rx_buffer; - rx_dma_params->buf_len = data->async.next_rx_buffer_len; - data->async.next_rx_buffer = NULL; - data->async.next_rx_buffer_len = 0U; - - /* A new buffer was available (and already loaded into the DMA engine) */ - if (rx_dma_params->buf != NULL && - rx_dma_params->buf_len > 0) { + if (data->async.next_rx_buffer != NULL && data->async.next_rx_buffer_len > 0) { /* Request the next buffer */ - async_evt_rx_buf_request(dev); + uart_mcux_lpuart_dma_replace_rx_buffer(dev); } else { /* Buffer full without valid next buffer, disable RX DMA */ LOG_INF("Disabled RX DMA, no valid next buffer "); @@ -832,6 +844,8 @@ static int mcux_lpuart_rx_enable(const struct device *dev, uint8_t *buf, const s rx_dma_params->timeout_us = timeout_us; rx_dma_params->buf = buf; rx_dma_params->buf_len = len; + data->async.next_rx_buffer = NULL; + data->async.next_rx_buffer_len = 0U; LPUART_EnableInterrupts(config->base, kLPUART_IdleLineInterruptEnable); prepare_rx_dma_block_config(dev); @@ -841,10 +855,9 @@ static int mcux_lpuart_rx_enable(const struct device *dev, uint8_t *buf, const s async_evt_rx_buf_request(dev); /* Clear these status flags as they can prevent the UART device from receiving data */ - LPUART_ClearStatusFlags(config->base, kLPUART_RxOverrunFlag | - kLPUART_ParityErrorFlag | - kLPUART_FramingErrorFlag | - kLPUART_NoiseErrorFlag); + LPUART_ClearStatusFlags(config->base, kLPUART_RxOverrunFlag | kLPUART_ParityErrorFlag | + kLPUART_FramingErrorFlag | + kLPUART_NoiseErrorFlag); LPUART_EnableRx(lpuart, true); irq_unlock(key); return ret; @@ -853,13 +866,14 @@ static int mcux_lpuart_rx_enable(const struct device *dev, uint8_t *buf, const s static int mcux_lpuart_rx_buf_rsp(const struct device *dev, uint8_t *buf, size_t len) { struct mcux_lpuart_data *data = dev->data; + unsigned int key; + key = irq_lock(); assert(data->async.next_rx_buffer == NULL); assert(data->async.next_rx_buffer_len == 0); data->async.next_rx_buffer = buf; data->async.next_rx_buffer_len = len; - uart_mcux_lpuart_dma_replace_rx_buffer(dev); - + irq_unlock(key); return 0; } @@ -921,6 +935,10 @@ static inline void mcux_lpuart_async_isr(struct mcux_lpuart_data *data, data->async.rx_dma_params.timeout_us); LPUART_ClearStatusFlags(config->base, kLPUART_IdleLineFlag); } + + if (status & kLPUART_RxOverrunFlag) { + LPUART_ClearStatusFlags(config->base, kLPUART_RxOverrunFlag); + } } #endif From 0e89fb71f5dca48808acac8d783769be2bc4eea6 Mon Sep 17 00:00:00 2001 From: Hake Huang Date: Wed, 2 Oct 2024 12:28:53 +0000 Subject: [PATCH 2/2] tests: uart_async_api: update test for dma usage 1. ensure the two dma buffers all aligned with 32 bits 2. clean the rx_data_idx at test begin Signed-off-by: Hake Huang --- .../uart/uart_async_api/src/test_uart_async.c | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/drivers/uart/uart_async_api/src/test_uart_async.c b/tests/drivers/uart/uart_async_api/src/test_uart_async.c index 15b4b6fba41a9a..660bd843323fcd 100644 --- a/tests/drivers/uart/uart_async_api/src/test_uart_async.c +++ b/tests/drivers/uart/uart_async_api/src/test_uart_async.c @@ -324,10 +324,15 @@ ZTEST_USER(uart_async_multi_rx, test_multiple_rx_enable) } #if NOCACHE_MEM -static __aligned(32) uint8_t chained_read_buf[2][8] __used __NOCACHE; +/* To ensure 32-bit alignment of the buffer array, + * the two arrays are defined instead using an array of arrays + */ +static __aligned(32) uint8_t chained_read_buf_0[8] __used __NOCACHE; +static __aligned(32) uint8_t chained_read_buf_1[8] __used __NOCACHE; static __aligned(32) uint8_t chained_cpy_buf[10] __used __NOCACHE; #else -ZTEST_BMEM uint8_t chained_read_buf[2][8]; +ZTEST_BMEM uint8_t chained_read_buf_0[8]; +ZTEST_BMEM uint8_t chained_read_buf_1[8]; ZTEST_BMEM uint8_t chained_cpy_buf[10]; #endif /* NOCACHE_MEM */ ZTEST_BMEM volatile uint8_t rx_data_idx; @@ -335,6 +340,8 @@ ZTEST_BMEM uint8_t rx_buf_idx; ZTEST_BMEM uint8_t *read_ptr; +static uint8_t *chained_read_buf[2] = {chained_read_buf_0, chained_read_buf_1}; + static void test_chained_read_callback(const struct device *dev, struct uart_event *evt, void *user_data) { @@ -352,9 +359,8 @@ static void test_chained_read_callback(const struct device *dev, rx_data_idx += evt->data.rx.len; break; case UART_RX_BUF_REQUEST: - err = uart_rx_buf_rsp(dev, - chained_read_buf[rx_buf_idx], - sizeof(chained_read_buf[0])); + err = uart_rx_buf_rsp(dev, chained_read_buf[rx_buf_idx], + sizeof(chained_read_buf_0)); zassert_equal(err, 0); rx_buf_idx = !rx_buf_idx ? 1 : 0; break; @@ -387,11 +393,10 @@ ZTEST_USER(uart_async_chain_read, test_chained_read) uint32_t rx_timeout_ms = 50; int err; - err = uart_rx_enable(uart_dev, - chained_read_buf[rx_buf_idx++], - sizeof(chained_read_buf[0]), + err = uart_rx_enable(uart_dev, chained_read_buf[rx_buf_idx++], sizeof(chained_read_buf_0), rx_timeout_ms * USEC_PER_MSEC); zassert_equal(err, 0); + rx_data_idx = 0; for (int i = 0; i < iter; i++) { zassert_not_equal(k_sem_take(&rx_disabled, K_MSEC(10)), @@ -406,7 +411,7 @@ ZTEST_USER(uart_async_chain_read, test_chained_read) "Unexpected amount of data received %d exp:%d", rx_data_idx, sizeof(tx_buf)); zassert_equal(memcmp(tx_buf, chained_cpy_buf, sizeof(tx_buf)), 0, - "Buffers not equal"); + "Buffers not equal exp %s, real %s", tx_buf, chained_cpy_buf); rx_data_idx = 0; } uart_rx_disable(uart_dev);