STM32 G4 HAL i2c not clearing interrupts

[photon]

**Problem:**

- When the I2C is enabled, the part can hit the WWDG.

- It looks like the pointer to the function `XferISR` is getting set to NULL somehow.

  - When this happens, the interrupts are not cleared, and it continually fires the interrupt because it is never cleared.

  - This is hard to troubleshoot because, in my system, there are many higher priority interrupts, making it hard to know that this is the problem.

- On another note, it looks like the interrupts are not always cleared in accordance with **RM0440 Table 394**:

  - In the error section, I2C_FLAG_PECERR, I2C_FLAG_TIMEOUT, and I2C_FLAG_ALERT are never cleared.

  - Also in I2C_Master_ISR_IT.

**Background:**

- Using an STM32G4 with HAL v1.2.3.

- Using DMA or IT (different builds).

- Connected to many I2C sensors.

- It is in a pretty noisy environment.

- All calls are from the main loop (except what was generated by HAL); all reading of buffers is in the main loop.

- If I have an I2C error, I re-init the I2C to clear out problems.

**Recommended Solutions:**

```c

void HAL_I2C_EV_IRQHandler(I2C_HandleTypeDef *hi2c) /* Derogation MISRAC2012-Rule-8.13 */

{

  /* Get current IT Flags and IT sources value */

  uint32_t itflags   = READ_REG(hi2c->Instance->ISR);

  uint32_t itsources = READ_REG(hi2c->Instance->CR1);

  /* I2C events treatment -------------------------------------*/

  if (hi2c->XferISR != NULL)

  {

    hi2c->XferISR(hi2c, itflags, itsources);

  }

  /* Add this */

  else

  {

    /* if you are very confident in the HAL */

    ASSERT();

    /* else */

    ClearAllI2cInterrupts(hi2c);

  }

  /* End Add */

}

```

- When there is a problem, you want to bring that to the attention of the developer to deal with it.

- I would think clearing all I2C interrupts would be more palatable.

- Also, where `XferISR = NULL`, you could set `XferISR = ClearAllI2cInterrupts;` this way, it better covers the bases.

**Questions:**

- Have you seen `XferISR` set to NULL while getting interrupts?

- Do you have any ideas why this might happen?

**Final Remarks:**

- I am going to try this out. Maybe I am smashing memory and setting `XferISR = NULL`. If after initialization it should never be NULL and then it got set to NULL, then likely I did something wrong...

[Saket_Om]

Hello @photon 

These flags "I2C_FLAG_PECERR, I2C_FLAG_TIMEOUT, and I2C_FLAG_ALERT" cannot be cleared by software. They are cleared by hardware when disabling the peripheral. 

 

 

[photon]

More background

• There is high amounts of high priority interrupts happening in this system (that you don't see)
• Here are some of my interrupt priorities:

 

#define NVIC_PRI_WWDG (1UL)
#define NVIC_PRI_PWR (1UL)
...
#define NVIC_PRI_SYS_TICK (3UL)
#define NVIC_PRI_EXTI (4UL)
#define NVIC_PRI_DMA_I2C_RX (4UL)
#define NVIC_PRI_DMA_I2C_TX (4UL)
#define NVIC_PRI_DMA_I2C_RXERR (4UL)
#define NVIC_PRI_I2C2_EV_IRQn (11UL)
#define NVIC_PRI_I2C2_ER_IRQn (11UL)

 

Further investigation and fixes

Potential thread safety violation

• Compare function pointer to NULL then call it, but it could have been changed between the check and the call.
• This can be fixed by creating a local variable that grabs the value compares that value then calls the stored value.
• Was:

 

void HAL_I2C_EV_IRQHandler(I2C_HandleTypeDef *hi2c) /* Derogation MISRAC2012-Rule-8.13 */
{
 /* Get current IT Flags and IT sources value */
 uint32_t itflags = READ_REG(hi2c->Instance->ISR);
 uint32_t itsources = READ_REG(hi2c->Instance->CR1);

 /* I2C events treatment -------------------------------------*/
 if (hi2c->XferISR != NULL)
 {
 hi2c->XferISR(hi2c, itflags, itsources);
 }
}

 

• Fix:

 

void HAL_I2C_EV_IRQHandler(I2C_HandleTypeDef *hi2c) /* Derogation MISRAC2012-Rule-8.13 */
{
 /* Get current IT Flags and IT sources value */
 uint32_t itflags = READ_REG(hi2c->Instance->ISR);
 uint32_t itsources = READ_REG(hi2c->Instance->CR1);
 HAL_StatusTypeDef(*pFoo)(struct __I2C_HandleTypeDef *hi2c, uint32_t ITFlags, uint32_t ITSources);
 pFoo = hi2c->XferISR;

 /* I2C events treatment -------------------------------------*/
 if (NULL != pFoo)
 {
 pFoo(hi2c, itflags & I2C_EV_MASK_IT, itsources);
 }
}

 

 

Create a pointer to function will clear un-expected interrupts

 

#define I2C_EV_MASK_IT ( I2C_FLAG_TXE | I2C_FLAG_TXIS | I2C_FLAG_RXNE | I2C_FLAG_ADDR \
 | I2C_FLAG_AF | I2C_FLAG_STOPF | I2C_FLAG_TC | I2C_FLAG_TCR )
#define I2C_ER_MASK_IT ( I2C_FLAG_BERR | I2C_FLAG_ARLO | I2C_FLAG_OVR \
 | I2C_FLAG_PECERR | I2C_FLAG_TIMEOUT | I2C_FLAG_ALERT )

HAL_StatusTypeDef I2C_Null_IT(struct __I2C_HandleTypeDef *hi2c, uint32_t ITFlags,
 uint32_t ITSources)
{
 __HAL_LOCK(hi2c);

 /* Disable interupts */
 __HAL_I2C_DISABLE_IT(hi2c, ITFlags & (I2C_EV_MASK_IT | I2C_ER_MASK_IT ));
 __HAL_I2C_CLEAR_FLAG(hi2c, ITFlags & (I2C_EV_MASK_IT | I2C_ER_MASK_IT ));

 __HAL_UNLOCK(hi2c);
 return (HAL_OK);
}

 

• Changes
  • Was: hi2c->XferISR = NULL;
  • Fix: hi2c->XferISR = I2C_Null_IT;
  • Change applied to
    • static void I2C_ITMasterSeqCplt(I2C_HandleTypeDef *hi2c)
    • static void I2C_ITMasterCplt(I2C_HandleTypeDef *hi2c, uint32_t ITFlags)
    • static void I2C_ITError(I2C_HandleTypeDef *hi2c, uint32_t ErrorCode)
    • Likely should be done in all the i2c interrupt functions.

Where I stand now

• I have not determined all the changes are needed. As I never set XferISR back to NULL doesn't allow for the opportunity to create a thead safety violation.
  • Fixing the thead safety seems important and I would recommend should be done for keeping good practices.
  • Fixing thead safety alone might not be enough to prevent a lockup.
• These change seem to mitigate the WWDG from firing because i2c interrupts locking up.
• I do not accept your solution as a fix, as it does not fix anything.
  • Pointing out that you don't need to clear these errors does change the fact that the i2c library was hanging in an interrupt when XferISR was set to NULL.
  • Making it not able to clear the interrupt
  • The interrupt continue to fire causing starving of the main loop
  • Creating a WWDG event
• Possible "Accept as Solution" include fixing my lockup problem that could include:
  • Explain how to use HAL so I don't get that problem.
  • Change the HAL so I don't get this problem.

[Saket_Om]

Hello @photon 

For the moment, it is up to the user to manage the race condition on HAL access. This can be done using a software semaphore. By implementing a software semaphore, you can control access to the HAL, ensuring that only one process or thread can access it at a time, thus preventing race conditions.