Possible error in fsl_usart.h for LPC5460x SDK ?

[carstengroen]

In the file fsl_usart.h for the SDK (2.2) for the LPC5460x device, there is a function:

USART_DisableInterrupts()

which is implemented like this in fsl_usart.h:

/*!
 * @brief Disables USART interrupts according to a provided mask.
 *
 * This function disables the USART interrupts according to a provided mask. The mask
 * is a logical OR of enumeration members. See @ref _usart_interrupt_enable.
 * This example shows how to disable the TX empty interrupt and RX full interrupt:
 * @code
 *     USART_DisableInterrupts(USART1, kUSART_TxLevelInterruptEnable | kUSART_RxLevelInterruptEnable);
 * @endcode
 *
 * @param base USART peripheral base address.
 * @param mask The interrupts to disable. Logical OR of @ref _usart_interrupt_enable.
 */
static inline void USART_DisableInterrupts(USART_Type *base, uint32_t mask)
{
    base->FIFOINTENSET = ~(mask & 0xF);
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

I think that there is an error in that implementation. According to the UM for the LPC5460x, to disable one of the FIFO interrupts one must write a '1' to the corresponding bit in register FIFOINTENCLR. The above should not have any effect according to the UM!

In my view, the code should instead read:

/*!
 * @brief Disables USART interrupts according to a provided mask.
 *
 * This function disables the USART interrupts according to a provided mask. The mask
 * is a logical OR of enumeration members. See @ref _usart_interrupt_enable.
 * This example shows how to disable the TX empty interrupt and RX full interrupt:
 * @code
 *     USART_DisableInterrupts(USART1, kUSART_TxLevelInterruptEnable | kUSART_RxLevelInterruptEnable);
 * @endcode
 *
 * @param base USART peripheral base address.
 * @param mask The interrupts to disable. Logical OR of @ref _usart_interrupt_enable.
 */
static inline void USART_DisableInterrupts(USART_Type *base, uint32_t mask)
{
    base->FIFOINTENCLR = mask & 0xF;
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

Isn't that correct, or am I mistaken ?

Similar error I think in line 9 below (file is fsl_usart.c):

void USART_TransferAbortReceive(USART_Type *base, usart_handle_t *handle)
{
    assert(NULL != handle);

    /* Only abort the receive to handle->rxData, the RX ring buffer is still working. */
    if (!handle->rxRingBuffer)
    {
        /* Disable interrupts */
        base->FIFOINTENSET &= ~USART_FIFOINTENSET_RXLVL_MASK;
        /* Empty rxFIFO */
        base->FIFOCFG |= USART_FIFOCFG_EMPTYRX_MASK;
    }

    handle->rxDataSize = 0U;
    handle->rxState = kUSART_RxIdle;
}
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

[andyjackson]

Hello Carsten,

I can confirm that we have the same fix for

USART_DisableInterrupts()

in our local copy of the SDK. We haven't tripped up on the other one as we don't use that function.

Best wishes,

   Andy

[carstengroen]

Thanks for feedback Andy, 

I'll continue using my fix then :smileyhappy:

[Carlos_Mendoza]

Hi Carsten,

You are correct, I have reported this to the SDK development team.

Thanks!

Best Regards,
Carlos Mendoza
Technical Support Engineer

[danielholala]

Hi @Carlos_Mendoza ,

Greeting from 2022. While the above mentioned bug has been fixed, I think the SDK team missed some more code where FIFOINTENSET is used to clear bits in the interrupt register.

In SDK 2.10.1 for LPC5526, the CMSIS driver for the USART interface fsl_usart_cmsis.c contains in function USART_NonBlockingControl():

    switch (control & ARM_USART_CONTROL_Msk)
    {
        /* Abort Send */
        case ARM_USART_ABORT_SEND:
            usart->resource->base->FIFOINTENSET &= ~USART_FIFOINTENSET_TXLVL_MASK;
            usart->handle->txDataSize = 0;
            usart->handle->txState    = (uint8_t)kUSART_TxIdle;
            result                    = ARM_DRIVER_OK;
            break;

        /* Abort receive */
        case ARM_USART_ABORT_RECEIVE:
            usart->resource->base->FIFOINTENSET &= ~USART_FIFOINTENSET_RXLVL_MASK;
            usart->handle->rxDataSize = 0U;
            usart->handle->rxState    = (uint8_t)kUSART_RxIdle;
            result                    = ARM_DRIVER_OK;
            break;

A quick search for this code pattern revealed similar code in fsl_spi.c:

void SPI_MasterTransferAbort(SPI_Type *base, spi_master_handle_t *handle)
{
    assert(NULL != handle);

    /* Disable interrupt requests*/
    base->FIFOINTENSET &= ~(SPI_FIFOINTENSET_TXLVL_MASK | SPI_FIFOINTENSET_RXLVL_MASK);
    /* Empty FIFOs */
    base->FIFOCFG |= SPI_FIFOCFG_EMPTYTX_MASK | SPI_FIFOCFG_EMPTYRX_MASK;

    handle->state            = (uint32_t)kStatus_SPI_Idle;
    handle->txRemainingBytes = 0U;
    handle->rxRemainingBytes = 0U;
}

I guess these parts need the fix, too.

Thanks.

Best regards,
Daniel