AnsweredAssumed Answered

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

Question asked by buijsman.dirk on Oct 14, 2016
I'd like to report a bug in the CMSIS I2C driver. The issue is in the file "ARM\PACK\Keil\STM32F4xx_DFP\2.10.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.
8) 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.

Attachments

Outcomes