otsl

Race condition in MCUXpresso USART Driver

Discussion created by otsl on Mar 24, 2020
Latest reply on May 28, 2020 by Rong Shen

The code contained in fsl_usart.c driver contains a race condition on transmit.  The error will occur in systems that have non-blocking transmit and receive happening simultaneously.

 

Starting at line 588 of the file (function USART_TransferSendNonBlocking), the following is used to start a transfer:

handle->txData = xfer->data;
handle->txDataSize = xfer->dataSize;
handle->txDataSizeAll = xfer->dataSize;
handle->txState = (uint8_t)kUSART_TxBusy;
/* Enable transmiter interrupt. */
base->FIFOINTENSET |= USART_FIFOINTENSET_TXLVL_MASK;

If a USART interrupt (such as a receive interrupt, etc) happens between setting txDataSize, and enabling the interrupt, the ISR will execute.  Inside the ISR, a boolean (sendEnable) is set based on txDataSize.  The main 'while' loop that sends/receives data uses this flag in conjunction with the FIFOSTAT register to (correctly) transmit the requested byte, decrement txDataSize, and enable the completion interrupt (TX_IDLEEN). 

 

The ISR then completes, and returns to USART_TransferSendNonBlocking that now enables the TXLVL interrupt. 

 

When the TXLVL interrupt triggers, txDataSize is zero (because it was previously transmitted), and the ISR (as written) never clears it, because the sendEnable flag is never true.  This causes the CPU to lock up servicing the USART IRQ.  Depending on overall timing of my RTOS system, I've seen it lock up stuck with TXIDLEEN flag on (but txState not correctly set to kUSART_TxIdle) or with TXLVL interrupt enabled, but txDataSize == 0.

 

This whole mess comes about because the ISR uses multiple, non-atomic ways to determine what state the transmit engine is in (txDataSize, TXNOTFULL flag, txState).  The correct way to sort this out would be to clean up the ISR to only move between states based on interrupt flags (which can be set/cleared atomically with the SET and CLR registers).

 

As a temporary fix, you can protect the critical lines above by wrapping them with:

regPrimask = DisableGlobalIRQ();
...
EnableGlobalIRQ(regPrimask);

 

Outcomes