Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mcux lpuart async fixes #79325

Merged
merged 2 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions drivers/serial/uart_mcux_lpuart.c
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,11 @@
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");
}
Expand Down Expand Up @@ -584,7 +585,7 @@
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;
EmilioCBen marked this conversation as resolved.
Show resolved Hide resolved
}

static int configure_and_start_rx_dma(
Expand Down Expand Up @@ -615,20 +616,38 @@
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;
EmilioCBen marked this conversation as resolved.
Show resolved Hide resolved
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;
}

Expand Down Expand Up @@ -673,16 +692,9 @@
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 ");
Expand Down Expand Up @@ -832,6 +844,8 @@
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);
Expand All @@ -841,11 +855,10 @@
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);

Check notice on line 861 in drivers/serial/uart_mcux_lpuart.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

drivers/serial/uart_mcux_lpuart.c:861 - kLPUART_FramingErrorFlag | - kLPUART_NoiseErrorFlag); + kLPUART_FramingErrorFlag | + kLPUART_NoiseErrorFlag);
irq_unlock(key);
return ret;
}
Expand All @@ -853,13 +866,14 @@
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;
}

Expand Down Expand Up @@ -921,6 +935,10 @@
data->async.rx_dma_params.timeout_us);
LPUART_ClearStatusFlags(config->base, kLPUART_IdleLineFlag);
}

if (status & kLPUART_RxOverrunFlag) {
LPUART_ClearStatusFlags(config->base, kLPUART_RxOverrunFlag);
}
}
#endif

Expand Down
23 changes: 14 additions & 9 deletions tests/drivers/uart/uart_async_api/src/test_uart_async.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,17 +324,24 @@ 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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question that's unrelated to your changes, does __used have any utility here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JarmouniA , __used is to force compiler to emit symbol, but I do not know whether it is a must in Zephyr, so I copy the line headless.

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;
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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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)),
Expand All @@ -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);
Expand Down
Loading