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 e373529a18..64edbde8c9 100644 --- a/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv1/hal_serial_lld.c @@ -728,7 +728,7 @@ void sd_lld_stop(SerialDriver *sdp) { */ void sd_lld_serve_interrupt(SerialDriver *sdp) { USART_TypeDef *u = sdp->usart; - uint16_t cr1 = u->CR1; + uint16_t cr1; uint16_t sr = u->SR; /* Special case, LIN break detection.*/ @@ -755,6 +755,9 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { } osalSysUnlockFromISR(); + /* Caching CR1.*/ + cr1 = u->CR1; + /* Transmission buffer empty.*/ if ((cr1 & USART_CR1_TXEIE) && (sr & USART_SR_TXE)) { msg_t b; @@ -762,7 +765,7 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { b = oqGetI(&sdp->oqueue); if (b < MSG_OK) { chnAddFlagsI(sdp, CHN_OUTPUT_EMPTY); - u->CR1 = cr1 & ~USART_CR1_TXEIE; + cr1 &= ~USART_CR1_TXEIE; } else u->DR = b; @@ -774,10 +777,13 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { osalSysLockFromISR(); if (oqIsEmptyI(&sdp->oqueue)) { chnAddFlagsI(sdp, CHN_TRANSMISSION_END); - u->CR1 = cr1 & ~USART_CR1_TCIE; + cr1 &= ~USART_CR1_TCIE; } osalSysUnlockFromISR(); } + + /* Writing CR1 once.*/ + u->CR1 = cr1; } #endif /* HAL_USE_SERIAL */ 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 a020ed5ad5..c90d63336e 100644 --- a/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv2/hal_serial_lld.c @@ -843,7 +843,7 @@ void sd_lld_stop(SerialDriver *sdp) { */ void sd_lld_serve_interrupt(SerialDriver *sdp) { USART_TypeDef *u = sdp->usart; - uint32_t cr1 = u->CR1; + uint32_t cr1; uint32_t isr; /* Reading and clearing status.*/ @@ -874,6 +874,9 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { isr = u->ISR; } + /* Caching CR1.*/ + cr1 = u->CR1; + /* Transmission buffer empty, note it is a while in order to handle two situations: 1) The data registers has been emptied immediately after writing it, this @@ -888,7 +891,7 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { b = oqGetI(&sdp->oqueue); if (b < MSG_OK) { chnAddFlagsI(sdp, CHN_OUTPUT_EMPTY); - u->CR1 = cr1 & ~USART_CR1_TXEIE; + cr1 &= ~USART_CR1_TXEIE; osalSysUnlockFromISR(); break; } @@ -904,10 +907,13 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { osalSysLockFromISR(); if (oqIsEmptyI(&sdp->oqueue)) { chnAddFlagsI(sdp, CHN_TRANSMISSION_END); - u->CR1 = cr1 & ~USART_CR1_TCIE; + cr1 &= ~USART_CR1_TCIE; } osalSysUnlockFromISR(); } + + /* Writing CR1 once.*/ + u->CR1 = cr1; } #endif /* HAL_USE_SERIAL */ 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 d8dc340fe3..fca0443cff 100644 --- a/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c +++ b/os/hal/ports/STM32/LLD/USARTv3/hal_serial_lld.c @@ -971,7 +971,7 @@ void sd_lld_stop(SerialDriver *sdp) { */ void sd_lld_serve_interrupt(SerialDriver *sdp) { USART_TypeDef *u = sdp->usart; - uint32_t cr1 = u->CR1; + uint32_t cr1; uint32_t isr; /* Reading and clearing status.*/ @@ -1002,6 +1002,9 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { isr = u->ISR; } + /* Caching CR1.*/ + cr1 = u->CR1; + /* Transmission buffer empty, note it is a while in order to handle two situations: 1) The data registers has been emptied immediately after writing it, this @@ -1016,7 +1019,7 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { b = oqGetI(&sdp->oqueue); if (b < MSG_OK) { chnAddFlagsI(sdp, CHN_OUTPUT_EMPTY); - u->CR1 = cr1 & ~USART_CR1_TXEIE; + cr1 &= ~USART_CR1_TXEIE; osalSysUnlockFromISR(); break; } @@ -1032,10 +1035,13 @@ void sd_lld_serve_interrupt(SerialDriver *sdp) { osalSysLockFromISR(); if (oqIsEmptyI(&sdp->oqueue)) { chnAddFlagsI(sdp, CHN_TRANSMISSION_END); - u->CR1 = cr1 & ~USART_CR1_TCIE; + cr1 &= ~USART_CR1_TCIE; } osalSysUnlockFromISR(); } + + /* Writing CR1 once.*/ + u->CR1 = cr1; } #endif /* HAL_USE_SERIAL */ diff --git a/readme.txt b/readme.txt index eac6d9c95f..05c8dd2bd2 100644 --- a/readme.txt +++ b/readme.txt @@ -86,6 +86,7 @@ - NEW: Reworked STM32 SDMMCv1 and SDMMCv2 drivers, better timeout and clock handling, improved speed for aligned buffers. - FIX: Fixed wrong STM32 LSI activation check (bug #1279). +- FIX: Fixed STM32 HAL UART ISR flaw (bug #1278). - FIX: Fixed race condition caused by chGuardedPoolAllocI() (bug #1277). - FIX: Fixed avoid shadowing with build-in pow10 function in chprintf.c (bug #1274).