From 7a3576b61b5c4dd943af07d5375246ee68a84238 Mon Sep 17 00:00:00 2001 From: bugobliterator Date: Thu, 10 Sep 2026 16:24:28 +1000 Subject: [PATCH] STM32 USARTv1/v2/v3: add SerialConfig.external_rx_buffer When set, the driver leaves RXNEIE clear and its interrupt handler never reads the data register to take data (USARTv1 reads it only to clear an error flag). Without this, an IDLE or TX interrupt that finds a byte the external receive path has not taken yet diverts it into the unused input queue, losing it from the stream. Found by inspection while chasing a byte-loss bug that turned out to be a different issue. Issue found and fix developed with the assistance of Claude (Anthropic). --- .../ports/STM32/LLD/USARTv1/hal_serial_lld.c | 17 ++++++++++--- .../ports/STM32/LLD/USARTv1/hal_serial_lld.h | 8 +++++++ .../ports/STM32/LLD/USARTv2/hal_serial_lld.c | 22 ++++++++++------- .../ports/STM32/LLD/USARTv2/hal_serial_lld.h | 7 ++++++ .../ports/STM32/LLD/USARTv3/hal_serial_lld.c | 24 ++++++++++++------- .../ports/STM32/LLD/USARTv3/hal_serial_lld.h | 7 ++++++ 6 files changed, 65 insertions(+), 20 deletions(-) diff --git a/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c b/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c index 578046e487..b87636df03 100644 --- a/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c @@ -96,7 +96,8 @@ static const SerialConfig default_config = USART_CR2_STOP1_BITS, 0, NULL, - NULL + NULL, + false }; /*===========================================================================*/ @@ -132,8 +133,8 @@ static void usart_init(SerialDriver *sdp, const SerialConfig *config) { u->CR2 = config->cr2 | USART_CR2_LBDIE; u->CR3 = config->cr3 | USART_CR3_EIE; u->CR1 = config->cr1 | USART_CR1_UE | USART_CR1_PEIE | - USART_CR1_RXNEIE | USART_CR1_TE | - USART_CR1_RE; + (config->external_rx_buffer ? 0U : USART_CR1_RXNEIE) | + USART_CR1_TE | USART_CR1_RE; u->SR = 0; (void)u->SR; /* SR reset step 1.*/ (void)u->DR; /* SR reset step 2.*/ @@ -752,6 +753,16 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { /* Error condition detection.*/ if (sr & (USART_SR_ORE | USART_SR_NE | USART_SR_FE | USART_SR_PE)) set_error(sdp, sr); + if (sdp->config->external_rx_buffer) { + /* Receive path is external: DR is read only to clear an error flag, + never to take data. A pending byte with no error is left for the + external path.*/ + if ((sr & (USART_SR_ORE | USART_SR_NE | USART_SR_FE | USART_SR_PE)) == 0U) + break; + (void)u->DR; + sr = u->SR; + continue; + } b = (uint8_t)u->DR & sdp->rxmask; if (sr & USART_SR_RXNE) sdIncomingDataI(sdp, b); diff --git a/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.h b/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.h index bfce02432c..e8e1e3b5a1 100644 --- a/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.h +++ b/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.h @@ -429,6 +429,14 @@ typedef struct hal_serial_config { * @pointer to ctx */ void* ctx; + /** + * @brief Receive data is moved by an external path. + * @details When true the driver does not enable RXNEIE and its interrupt + * handler reads the data register only to clear an error flag, + * never to take data, so bytes cannot be diverted into the + * (unused) input queue. + */ + bool external_rx_buffer; } SerialConfig; /** diff --git a/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c b/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c index f1983ebb23..6d862dc1d2 100644 --- a/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c @@ -145,7 +145,8 @@ static const SerialConfig default_config = USART_CR2_STOP1_BITS, 0, NULL, - NULL + NULL, + false }; #if STM32_SERIAL_USE_USART1 || defined(__DOXYGEN__) @@ -269,8 +270,8 @@ static void usart_init(SerialDriver *sdp, u->CR2 = config->cr2 | USART_CR2_LBDIE; u->CR3 = config->cr3 | USART_CR3_EIE; u->CR1 = config->cr1 | USART_CR1_UE | USART_CR1_PEIE | - USART_CR1_RXNEIE | USART_CR1_TE | - USART_CR1_RE; + (config->external_rx_buffer ? 0U : USART_CR1_RXNEIE) | + USART_CR1_TE | USART_CR1_RE; u->ICR = 0xFFFFFFFFU; /* Deciding mask to be applied on the data register on receive, this is @@ -870,12 +871,17 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { an extra interrupt to serve. 2) FIFO mode is enabled on devices that support it, we need to empty the FIFO.*/ - while (isr & USART_ISR_RXNE) { - osalSysLockFromISR(); - sdIncomingDataI(sdp, (uint8_t)u->RDR & sdp->rxmask); - osalSysUnlockFromISR(); + /* Skipped entirely when the receive path is external: an IDLE or TX + interrupt that finds a byte the external path has not taken yet must not + divert it into the input queue.*/ + if (!sdp->config->external_rx_buffer) { + while (isr & USART_ISR_RXNE) { + osalSysLockFromISR(); + sdIncomingDataI(sdp, (uint8_t)u->RDR & sdp->rxmask); + osalSysUnlockFromISR(); - isr = u->ISR; + isr = u->ISR; + } } /* Caching CR1.*/ diff --git a/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.h b/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.h index 8b0475cb4e..3ca3b38e3a 100644 --- a/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.h +++ b/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.h @@ -542,6 +542,13 @@ typedef struct hal_serial_config { * @pointer to ctx */ void* ctx; + /** + * @brief Receive data is moved by an external path. + * @details When true the driver does not enable RXNEIE and its interrupt + * handler never reads the data register to take data, so bytes + * cannot be diverted into the (unused) input queue. + */ + bool external_rx_buffer; } SerialConfig; diff --git a/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c b/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c index fccf39b611..2cea9c9ce1 100644 --- a/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c @@ -155,7 +155,8 @@ static const SerialConfig default_config = USART_CR2_STOP1_BITS, 0, NULL, - NULL + NULL, + false }; #if STM32_SERIAL_USE_USART1 || defined(__DOXYGEN__) @@ -295,8 +296,8 @@ static void usart_init(SerialDriver *sdp, u->CR2 = config->cr2 | USART_CR2_LBDIE; u->CR3 = config->cr3 | USART_CR3_EIE; u->CR1 = config->cr1 | USART_CR1_UE | USART_CR1_PEIE | - USART_CR1_RXNEIE | USART_CR1_TE | - USART_CR1_RE; + (config->external_rx_buffer ? 0U : USART_CR1_RXNEIE) | + USART_CR1_TE | USART_CR1_RE; u->ICR = 0xFFFFFFFFU; /* Deciding mask to be applied on the data register on receive, this is @@ -997,13 +998,18 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { 1) Another byte arrived after removing the previous one, this would cause an extra interrupt to serve. 2) FIFO mode is enabled on devices that support it, we need to empty - the FIFO.*/ - while (isr & USART_ISR_RXNE) { - osalSysLockFromISR(); - sdIncomingDataI(sdp, (uint8_t)u->RDR & sdp->rxmask); - osalSysUnlockFromISR(); + the FIFO. + Skipped entirely when the receive path is external: an IDLE or TX + interrupt that finds a byte the external path has not taken yet must not + divert it into the input queue.*/ + if (!sdp->config->external_rx_buffer) { + while (isr & USART_ISR_RXNE) { + osalSysLockFromISR(); + sdIncomingDataI(sdp, (uint8_t)u->RDR & sdp->rxmask); + osalSysUnlockFromISR(); - isr = u->ISR; + isr = u->ISR; + } } /* Caching CR1.*/ diff --git a/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.h b/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.h index b50388a744..e3f9ae5060 100644 --- a/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.h +++ b/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.h @@ -638,6 +638,13 @@ typedef struct hal_serial_config { * @pointer to ctx */ void* ctx; + /** + * @brief Receive data is moved by an external path. + * @details When true the driver does not enable RXNEIE and its interrupt + * handler never reads the data register to take data, so bytes + * cannot be diverted into the (unused) input queue. + */ + bool external_rx_buffer; } SerialConfig; /**