Skip to content

drivers: serial: silabs: bug correction on high baudrate for series 2 #97721

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 22, 2025
Merged

drivers: serial: silabs: bug correction on high baudrate for series 2 #97721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98bf595
drivers: serial: silabs: Correction of dma/peripheral isr exec order
Martinhoff-maker Oct 16, 2025
a8de046
drivers: serial: silabs: Fix uart tx abort at high baudrate
Martinhoff-maker Oct 16, 2025
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
4 changes: 3 additions & 1 deletion drivers/serial/uart_silabs_eusart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,12 @@ __maybe_unused static void eusart_dma_tx_cb(const struct device *dma_dev, void *
{
const struct device *uart_dev = user_data;
struct eusart_data *data = uart_dev->data;
const struct eusart_config *config = uart_dev->config;

dma_stop(data->dma_tx.dma_dev, data->dma_tx.dma_channel);
data->dma_tx.enabled = false;

EUSART_IntEnable(config->eusart, EUSART_IF_TXC);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the transfer already completed when we get here (e.g. because the DMA interrupt is delayed by higher-priority interrupts), TXC will already be set at this point, and the ISR will fire immediately. I assume that's the intent?

If so, shouldn't this be the last thing to happen in the function, in order to robustly tolerate a scenario where the UART ISR has higher priority than the DMA ISR, and will preempt at this point? Otherwise, interrupt priority configuration will determine whether data->dma_tx.enabled = false happens before or after the UART ISR runs.

I assume you have also verified that the UART ISR actually fires if TXC is set prior to enabling the IF.

Copy link
Member Author

@Martinhoff-maker Martinhoff-maker Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For the last part, async uart test are working so it means that we managed to fire the callback otherwise we will not have our TX done event. Moreover, if I strictly quote the rf: "If USART interrupts are enabled, an interrupt will be made if one or more of the interrupt flags in USART_IF and their corresponding bits in USART_IEN are set.", I understand it as : the NVIC line is asserted only when both condition are meet (in any order). I assume it's working but that's a valid question. I test it with a baudrate up to 2000000.

Good point about the preemption of the ISR and yes it should be the last thing to happen in the function.

I first thinking about calling the tx event done directly here but it could lead to situation where we say to the user that the transfer is done but that's not technically right (especially with low baudrate) and i'm not okay with that. That's why i'm going for enabling the interrupt here.

Copy link
Member Author

Choose a reason for hiding this comment

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

btw I've done it correctly in usart driver, don't know why I switch in eusart ...

}

static int eusart_async_tx(const struct device *dev, const uint8_t *tx_data, size_t buf_size,
Expand Down Expand Up @@ -500,7 +503,6 @@ static int eusart_async_tx(const struct device *dev, const uint8_t *tx_data, siz
eusart_pm_lock_get(dev, EUSART_PM_LOCK_TX);

EUSART_IntClear(config->eusart, EUSART_IF_TXC);
EUSART_IntEnable(config->eusart, EUSART_IF_TXC);

ret = dma_config(data->dma_tx.dma_dev, data->dma_tx.dma_channel, &data->dma_tx.dma_cfg);
if (ret) {
Expand Down
7 changes: 5 additions & 2 deletions drivers/serial/uart_silabs_usart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,12 @@ void uart_silabs_dma_tx_cb(const struct device *dma_dev, void *user_data, uint32
{
const struct device *uart_dev = user_data;
struct uart_silabs_data *data = uart_dev->data;
const struct uart_silabs_config *config = uart_dev->config;

dma_stop(data->dma_tx.dma_dev, data->dma_tx.dma_channel);
data->dma_tx.enabled = false;

USART_IntEnable(config->base, USART_IF_TXC);
}

static int uart_silabs_async_tx(const struct device *dev, const uint8_t *tx_data, size_t buf_size,
Expand Down Expand Up @@ -499,7 +502,6 @@ static int uart_silabs_async_tx(const struct device *dev, const uint8_t *tx_data

(void)uart_silabs_pm_lock_get(dev, UART_SILABS_PM_LOCK_TX);
USART_IntClear(config->base, USART_IF_TXC | USART_IF_TCMP2);
USART_IntEnable(config->base, USART_IF_TXC);
if (timeout >= 0) {
USART_IntEnable(config->base, USART_IF_TCMP2);
}
Expand Down Expand Up @@ -537,11 +539,12 @@ static int uart_silabs_async_tx_abort(const struct device *dev)
USART_IntClear(config->base, USART_IF_TXC | USART_IF_TCMP2);
(void)uart_silabs_pm_lock_put(dev, UART_SILABS_PM_LOCK_TX);

dma_stop(data->dma_tx.dma_dev, data->dma_tx.dma_channel);

if (!dma_get_status(data->dma_tx.dma_dev, data->dma_tx.dma_channel, &stat)) {
data->dma_tx.counter = tx_buffer_length - stat.pending_length;
}

dma_stop(data->dma_tx.dma_dev, data->dma_tx.dma_channel);
data->dma_tx.enabled = false;

async_evt_tx_abort(data);
Expand Down