SDK == fill-in-the-blanks test (aka. CMSIS I2C Driver incompletely implemented)

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

SDK == fill-in-the-blanks test (aka. CMSIS I2C Driver incompletely implemented)

Jump to solution
729 Views
danielholala
Senior Contributor II

Hi fellow developers,

I'm reporting on the quality (or rather the lack thereof) of the SDK again. This time it's about the CMSIS I2C driver, see fsl_i2c_cmsis.c. I think that I found another occurrence of the "incomplete implementation pattern". 

The ARM CMSIS specification states that a finished I2C transfer is signaled to the application through a callback function. The callback function passes the status as a parameter "event" to the application, see reference driver (scroll to the bottom).

The CMSIS event flags are:

ARM_I2C_EVENT_TRANSFER_DONE1UL << 0Occurs after Master/Slave Transmit/Receive operation has finished.
ARM_I2C_EVENT_TRANSFER_INCOMPLETE1UL << 1Occurs together with ARM_I2C_EVENT_TRANSFER_DONE when less data is transferred then requested.
ARM_I2C_EVENT_SLAVE_TRANSMIT1UL << 2Occurs when addressed as Slave Transmitter and ARM_I2C_SlaveTransmit has not been started.
ARM_I2C_EVENT_SLAVE_RECEIVE1UL << 3Occurs when addressed as Slave Receiver and ARM_I2C_SlaveReceive has not been started.
ARM_I2C_EVENT_ADDRESS_NACK1UL << 4Occurs in master mode when address is not acknowledged from slave.
ARM_I2C_EVENT_GENERAL_CALL1UL << 5Indicates General Call in slave mode together with ARM_I2C_EVENT_TRANSFER_DONE, ARM_I2C_EVENT_SLAVE_TRANSMIT and ARM_I2C_EVENT_SLAVE_RECEIVE.
ARM_I2C_EVENT_ARBITRATION_LOST1UL << 6Occurs in master mode when arbitration is lost.
ARM_I2C_EVENT_BUS_ERROR1UL << 7Occurs when bus error is detected.
ARM_I2C_EVENT_BUS_CLEAR1UL << 8Occurs after ARM_I2C_BUS_CLEAR Control operation has finished.

 

This callback function ((ARM_I2C_SignalEvent_t)userData)(event) is invoked in fsl_i2c_cmsis.c :

static void KSDK_I2C_MASTER_InterruptCallback(I2C_Type *base,
                                              i2c_master_handle_t *handle,
                                              status_t status,
                                              void *userData)
{
    uint32_t event;

    switch (status)
    {
        case kStatus_Success: /* Occurs after Master Transmit/Receive operation has finished. */
            event = ARM_I2C_EVENT_TRANSFER_DONE;
            break;
        case kStatus_I2C_ArbitrationLost: /*Occurs in master mode when arbitration is lost.*/
            event = ARM_I2C_EVENT_ARBITRATION_LOST;
            break;
        default:
            event = ARM_I2C_EVENT_TRANSFER_INCOMPLETE;
            break;
    }

    /* User data is actually CMSIS driver callback. */
    if (userData != NULL)
    {
        ((ARM_I2C_SignalEvent_t)userData)(event);
    }
}

KSDK_I2C_MASTER_InterruptCallback() receives the SDK I2C status, converts it to CMSIS event and invokes the callback. However, only a subset of the CMSIS events are supported/implemented. That's striking as the standard SDK peripheral drivers supports most of them. 

As you know, the CMSIS driver is built on top of standard SDK peripheral drivers. The standard I2C driver fsl_i2c.c invokes the callback function after receiving the SDK I2C status from I2C_RunTransferStateMachine(). There you find, e.g. the SDK I2C status kStatus_I2C_Addr_Nak that is used when there's no target device acknowledging the I2C address. The matching CMSIS event is ARM_I2C_EVENT_ADDRESS_NACK. But the CMSIS SDK driver (as implemented by NXP) returns  ARM_I2C_EVENT_TRANSFER_INCOMPLETE.

That's unsatisfying. It feels like the completion of the driver code "is left as an exercise for the reader".

While I applaud NXP that the Peripherals Tools generates code based on CMSIS API, I'd wish NXP would put more effort in code quality and API documentation.

Hope that helps future users of the CMSIS I2C driver.

Best regards,
Daniel

0 Kudos
1 Solution
449 Views
danielholala
Senior Contributor II

The driver now supports ARM_I2C_EVENT_ADDRESS_NACK, see SDK on Github

So this issue seems to have been fixed in SDK 2.12.0

View solution in original post

0 Kudos
1 Reply
450 Views
danielholala
Senior Contributor II

The driver now supports ARM_I2C_EVENT_ADDRESS_NACK, see SDK on Github

So this issue seems to have been fixed in SDK 2.12.0

0 Kudos