Bug in LPSPI_MasterTransferBlocking()?

cancel
Showing results for 
Search instead for 
Did you mean: 

Bug in LPSPI_MasterTransferBlocking()?

455 Views
kevin_haake
Contributor II

I believe there is a potential issue in LPSPI_MasterTransferBlocking() (as found in fsl_lpspi.c / SDK 2.6, MKE14F512VLL16). What I have observed is that if this function is preempted/interrupted for a substantial length of time at the wrong time, an overrun can occur in the Rx FIFO which causes the function to get stuck in an endless loop and never return.

The issue appears to reside in the Tx loop, and is exposed in the following circumstance:

Precondition: The Tx FIFO is full, and the Rx FIFO is still empty (i.e. we are waiting for data to be clocked out)

1. The code waits for room to open up in the Tx FIFO.

2. Once room is available (note that the Rx FIFO will not be empty at this point), another word is written to the Tx FIFO.


3. The code then checks the Rx FIFO, and reads any data that is available.

IF the driver is interrupted for a substantial amount of time between steps (2) and (3) above, it is possible that FIFOSIZE + 1 words will be clocked out before we read anything. If this happens, an Rx FIFO overrun will occur and a word will be dropped. This drop will cause the Rx loop at the end of the function (not shown) to never complete, because rxRemainingByteCount will never reach zero.

I think that switching the order of steps (2) and (3) above would address this issue, as shown in the below snippet. Notice that the write step was moved from before the read step to after the read step:

pastedImage_2.png

Thank you.

Labels (1)
3 Replies

162 Views
Alexis_A
NXP TechSupport
NXP TechSupport

Hello kevin.haake@licor.com‌,

Thanks for your comments, as you mention this looks like a bug but I think this being checked in the new SDK revision:

while (txRemainingByteCount > 0U)
    {
        if (txRemainingByteCount < bytesEachWrite)
        {
            bytesEachWrite = (uint8_t)txRemainingByteCount;
        }

        /*Wait until TX FIFO is not full*/
        while (LPSPI_GetTxFifoCount(base) == fifoSize)
        {
        }

        /* To prevent rxfifo overflow, ensure transmitting and receiving are executed in parallel */
        if (((NULL == rxData) || (rxRemainingByteCount - txRemainingByteCount) < rxFifoMaxBytes))
        {
            if (txData != NULL)
            {
                wordToSend = LPSPI_CombineWriteData(txData, bytesEachWrite, isByteSwap);
                txData += bytesEachWrite;
            }

            LPSPI_WriteData(base, wordToSend);
            txRemainingByteCount -= bytesEachWrite;
        }

        /*Check whether there is RX data in RX FIFO . Read out the RX data so that the RX FIFO would not overrun.*/
        if (rxData != NULL)
        {
            while (LPSPI_GetRxFifoCount(base) != 0U)
            {
                readData = LPSPI_ReadData(base);
                if (rxRemainingByteCount < bytesEachRead)
                {
                    bytesEachRead = (uint8_t)rxRemainingByteCount;
                }

                LPSPI_SeparateReadData(rxData, readData, bytesEachRead, isByteSwap);
                rxData += bytesEachRead;

                rxRemainingByteCount -= bytesEachRead;
            }
        }
    }

In the previous code, the driver checks the number of bytes in the RX and TX to send before sending another word so I think the overrun will be avoided.

Best Regards,

Alexis Andalon

162 Views
kevin_haake
Contributor II

Actually, I don't think that rxRemainingByteCount is being initialized properly in all cases. In SDK 2.7 fsl_lpspi.c, it is set as follows:

uint32_t rxFifoMaxBytes = 4U * fifoSize;

Perhaps I am missing something, but shouldn't it take bytesPerFrame into consideration? Something like:

uint32_t rxFifoMaxBytes = ((bytesPerFrame <= 4U) ? bytesPerFrame : 4U) * fifoSize;

I ran into the lockup issue again in LPSPI_MasterTransferBlocking() (SDK 2.7) using a bitsPerFrame setting of 8 before modifying it as above.

0 Kudos

162 Views
kevin_haake
Contributor II

Thank you, Alexis. Yes, it appears that should work. Serves me right for not updating to SDK 2.7.