Request for comments: add a timeout to the LPCOpen I2C stack

[danielholala]

Hi all,

as some of you have found out before, LPCOpen I2C stack does not implement timeouts. If your I2C lines are blocked or pulled to GND, the LPCOpen transfer function will not return.

To add a timeout to the LPCOpen I2C stack, I suggest the following changes to i2c_17xx_40xx.c:

+volatile uint32_t i2c_busy_timeouts = 0;
+const uint32_t i2c_busy_limit = 9999; // adapt value to hardware setup and I2C clock rate
 void Chip_I2C_EventHandler(I2C_ID_T id, I2C_EVENT_T event)
 {
      struct i2c_interface *iic = &i2c[id];
      volatile I2C_STATUS_T *stat;
+     uint32_t busy_counter = 0;
 
      /* Only WAIT event needs to be handled */
      if (event != I2C_EVENT_WAIT) {
           return;
      }
 
      stat = &iic->mXfer->status;
      /* Wait for the status to change */
-     while (*stat == I2C_STATUS_BUSY) {}
+     while (*stat == I2C_STATUS_BUSY) {
+          busy_counter++;
+          if (busy_counter > i2c_busy_limit) {
+               // I2C bus hang-up identified...
+               i2c_busy_timeouts++;
+               *stat = I2C_STATUS_BUSERR;
+               i2c[id].ip->CONSET = I2C_CON_STO; // set stop bit for forced access to the I2C bus
+               break;
+          }
+     }
 }
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

and further down in Chip_I2C_MasterTransfer():

      iic->mEvent(id, I2C_EVENT_WAIT);
      iic->mXfer = 0;
 
-     /* Wait for stop condition to appear on bus */
-     while (!isI2CBusFree(iic->ip)) {}
+     if (xfer->status != I2C_STATUS_BUSERR) {
+          /* Wait for stop condition to appear on bus */
+          while (!isI2CBusFree(iic->ip)) {}
+     }
 
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

The timeout breaks the busy wait loop which otherwise would not exit on bus obstruction (e.g., due to holding either the SDA or SCL line LOW by any device on the bus) or bus hang-ups (e.g., due to intermittant signals by an uncontrolled source that generate a superfluous START condition or mask a STOP condition).

While we are at it, you can see that in Chip_I2C_MasterTransfer() after the event I2C_EVENT_WAIT has been handled, the pointer iic->mXfer is set to NULL. If thereafter an I2C interrupt occurs (maybe due to intermittent signals), a hard fault will generated. This is because the I2C interrupt service routine (at least in master mode) will call Chip_I2C_MasterStateHandler() and this function will then call  handleMasterXferState() with mXfer as parameter. handleMasterXferState() will then happily dereference this NULL pointer and consequently generate a hard fault.

To fix this issue, I suggest the following changes to Chip_I2C_MasterStateHandler():

 /* State change handler for master transfer */
 void Chip_I2C_MasterStateHandler(I2C_ID_T id)
 {
+     if (!i2c[id].mXfer) {
+          // Sometimes (e.g., when I2C is blocked intermittently)
+          // something (e.g., Chip_I2C_MasterTransfer())
+          // clears i2c[id].mXfer. In this case don't call
+          // handleMasterXferState() as it dereferences mXfer
+          // and this leads to a hard fault.
+          i2c[id].ip->CONCLR = I2C_CON_SI; // clear interrupt bit to stop isr from being called repeatedly
+          return;
+     }
      if (!handleMasterXferState(i2c[id].ip, i2c[id].mXfer)) {
           i2c[id].mEvent(id, I2C_EVENT_DONE);
      }
 }
‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

I appreciate any comments. Thank you.

Daniel

[Alice_Yang]

Hello Daniel Kabs,

Thanks for your sharing.

I think your suggestion is good. About timeout, we can see in the User Manual for LPC chip( for example LPC176x):

In some applications, it may be possible for an uncontrolled source to cause a bus
hang-up.

If the STA flag is set and bus access is not obtained within a reasonable amount of time, then a forced access to the I2C-bus is possible. This is achieved by setting the STO flag, just like your code on interrupt function Chip_I2C_EventHandler().

BR

Alice