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.