Bug report: CMSIS Driver issues invalid ARM_I2C_EVENT_ADDRESS_NACK after a WriteRead transaction.

[dirkjurjen]

Posted on October 14, 2016 at 20:04

I'd like to report a bug in the CMSIS I2C driver. The issue is in the file ''ARM\PACK\Keil\STM32F4xx_DFP\2.0\CMSIS\DriverI2C_STM32F4xx.c'' in the function ''I2C_ER_IRQHandler''.

Description: The used MCU is the STM32F407VGT6 (on a STM322F4 Discovery board). This is the DUT. The I2C driver on the DUT is configured as a slave. An external master I2C issues a WriteRead type of command using the restart condition. A single byte is written to the DUT and 4 bytes are read from it. The order of I2C bus events is: 1) Master issues [START] condition. 2) Master writes DUT address (7-bit) with write bit set. 3) DUT issues ACK - CMSIS lib in the DUT issues the ''ARM_I2C_EVENT_SLAVE_RECEIVE'' event. 4) Master transmits a single byte. 5) DUT issues ACK 6) Master issues [RESTART] condition 7) Master sends out DUT address with read bit set. 😎 DUT issues ACK - CMSIS driver in DUT issues ''ARM_I2C_EVENT_TRANSFER_DONE'' event. 9) (3 times) DUT transmits 3 bytes and master issues ACK for each. - CMSIS driver in DUT issues ''ARM_I2C_EVENT_SLAVE_TRANSMIT''. 10) DUT transmits 1 byte and master issues NACK (in accordance with the I2C spec). - CMSIS driver in DUT issues ''ARM_I2C_EVENT_TRANSFER_DONE'' event AND also the ''ARM_I2C_EVENT_ADDRESS_NACK'' event and this is the issue (see below). 11) Master issues [STOP] The event ''ARM_I2C_EVENT_ADDRESS_NACK'' after step 10 should not have been issued in the DUT. The NACK is expected and completely valid. The issue lies in this piece of code where a flag is erased before it is checked:

if (sr1 & I2C_SR1_AF) {
/* Acknowledge failure */
err |= I2C_SR1_AF;
/* Reset the communication */
i2c->reg->CR1 |= I2C_CR1_STOP;
i2c->info->status.busy = 0U;
i2c->info->status.mode = 0U;
i2c->info->xfer.data = NULL;
i2c->info->xfer.ctrl = 0U;
evt = ARM_I2C_EVENT_TRANSFER_DONE;
if ((i2c->info->xfer.ctrl & XFER_CTRL_ADDR_DONE) == 0U) {
/* Addressing not done */
evt |= ARM_I2C_EVENT_ADDRESS_NACK;
}
}

The fix is simple:

if (sr1 & I2C_SR1_AF) {
/* Acknowledge failure */
err |= I2C_SR1_AF;
/* Reset the communication */
i2c->reg->CR1 |= I2C_CR1_STOP;
i2c->info->status.busy = 0U;
i2c->info->status.mode = 0U;
i2c->info->xfer.data = NULL;
evt = ARM_I2C_EVENT_TRANSFER_DONE;
if ((i2c->info->xfer.ctrl & XFER_CTRL_ADDR_DONE) == 0U) {
/* Addressing not done */
evt |= ARM_I2C_EVENT_ADDRESS_NACK;
} 
i2c->info->xfer.ctrl = 0U;
}

Additionally I have some worries about the moment when the events are issues: It seems to me that the ''ARM_I2C_EVENT_SLAVE_TRANSMIT'' should be issued when the address + read bit is encountered right after the restart condition. Can someone comment on this as well? Cheers, - Dirk PS: an image is attached with the Device clock in blue and the moment of events being issued by the CMSIS I2C driver in Red with a label. #cmsis-i2c-stm32-slave-writeread