Problem with I2C on STM32H7A3IIK6

[Miglio]

Hi, i'm developing a project on STM32H7A3IIK6 [cortex M7] and i'm having a problem using the I2C peripheral with the ST HAL functions. I've initialized the peripheral and enabled the Event and Error Inpterrupt with TX and RX DMA and, sometimes, i have a problem when the HAL function "I2C_Mem_ISR_DMA" [placed in stm32h7xx_hal_i2c.c] is called. If the peripheral is locked [lock and unlock of the peripheral are managed by the HAL functions] the "I2C_Mem_ISR_DMA" always fails [is not able to clean the interrupt flag] and the interrupt handler [HAL_I2C_EV_IRQHandler] is not able to detect that the "I2C_Mem_ISR_DMA" is failing [resource locked] because its return value is not checked. I think it's a bug in the H7 package. Will it be fixed in a next revision?

[S.Ma]

I2C Master or Slave mode?

[Miglio]

Master Mode

[Miglio]

I have a problem using the I2C HAL functions [stm32h7xx_hal_i2c.c] with event and error interrupt enabled. If the function "HAL_I2C_Mem_Write_DMA" is called by the application task to enable the I2C write the I2C resource is locked and, after that, the TXDR register is written. Sometimes the data, written in the TXDR register, is sent before the resource unlock and so a new event interrupt is generated. When the new interrupt calls its handler the I2C resource is still locked and the handler is not able to serve the interrupt anymore.

I've noticed this problem porting an application firmware from F4 family to H7 family without changing the priority level for application task and I2C interrupt [in the F4 family there was not a check regarding the I2C lock state inside the interrupt handler]. Will it be fixed in a new version of the H7 firmware package?

Otherwise the only way to fix this problem is to have, for the I2C Event interrupt, a priority lower than the task which calls the write and read functions.

[Pavel A.]

Nothing will be fixed unless you provide a good test project (small as possible, clear).

How an interrupt can be lower priority than a task?

​

Note that the HAL "locks" are not really locks. They are used only to check for reentrancy between thread and interrupt context code. If you want to use I2C from several RTOS tasks, this needs a real RTOS-level lock (or other sync method).

[Piranha]

> They are used only to check for reentrancy between thread and interrupt context code.

That was the intention, but in reality they do not provide even that, because they are broken.

https://community.st.com/s/question/0D50X0000C5Tns8SQC/bug-stm32-hal-driver-lock-mechanism-is-not-interrupt-safe

[Miglio]

What do you mean for "good test project"? Sorry for the "lower", what i meant was that to fix this problem the solution is to give to the I2C Event interrupt a priority value of 5 or higher

[Pavel A.]

Aha, then it is the well known trivia of FreeRTOS: if an interrupt handler calls a RTOS function, the interrupt priority must be lower (value greater) than a threshold parameter in the FreeRTOS config file. Maybe this threshold is different in your F4 and H7 projects.

>What do you mean for "good test project"?

https://stackoverflow.com/help/minimal-reproducible-example

[Miglio]

My problem is that the I2C hal, auto generated for the H7 device, is not "interrupt safe". In particular, sometimes, i have a problem when my "eeprom manager task" [the H7 MCU talks with an external EEPROM via I2C] calls the "HAL_I2C_Mem_Read_DMA".

This function makes the following actions :

1. Locks the I2C handle
   1. Prepares the parameters for the transfer [like the function "I2C_Mem_ISR_DMA" to manage the transfer end]
   2. Writes the TXDR register
   3. Enables the I2C transfer calling the function "I2C_TransferConfig"
   4. Unlocks the handle.
   5. Enables the transmission interrupt [NEVER DISABLED BEFORE] with an internal commet which says that the interrupt must be enabled after the "unlock" to avoid the risk of an interrupt execution

Sometimes, after point 4, the TX I2C Event Interrupt is generated but, finding the resource locked, is not able to serve the interrupt request and so the interrupt will be generated forever blocking the device.

Why the interrupt, enabled in point 6, has never been disabled before?

To fix just this problem [the HAL I2C module is for sure "NOT INTERRUPT SAFE"] my idea is to modify the HAL I2C module [not the "LOCK/UNLOCK" macros]. Are you planning to fix the autogenerated HAL in order to make it "interrupt safe" or not? Is it possible to share with you the modification i'm talking about [really quite simple] to understand with you if you see any kind of other problems in this modification?

[Pavel A.]

Maybe this is a bug. I haven't deeply analyzed this driver. So you're saying that at the point 3 the TX interrupt is already enabled in the I2C CR1 register?

The HAL drivers are not "autogenerated". They are copied into your project (optionally) from the "repository". If you indeed think there's a bug, open a issue here.

Of course you can have your own copy of the library with a patch.

Maybe a simpler workaround is to disable the global I2C interrupt in NVIC before calling HAL_I2C_Mem_Read_DMA and enable after it returns. No changes in the HAL files.

[Miglio]

Yeah you're right, The HAL are not auto generated of course but copied from the repository. Anyway i'll open an issue at the link to gave me to ask to fix this bug. Of course i can disable and anable again the I2C interrupt in my task but i'd like to fix it where it should be fixed [at HAL level]

[Pavel A.]

So do you have a firm evidence that TX interrupt bit (or other interrupt bits) is enabled in the I2C CR1 register before 1st time calling HAL_I2C_Mem_Read_DMA?

[Miglio]

Looking at the function "HAL_I2C_Mem_Read_DMA" [in stm32h7xx_hal_i2c.c - H7 firmware package rev. 1.11] you can see that this function :

1. Locks the i2c handle
2. Prepares the parameters for the transfer [like the function "I2C_Mem_ISR_DMA" to manage the transfer end]
3. Writes the TXDR register
4. Enables the I2C transfer calling the function "I2C_TransferConfig". In particular this function writes the CR1 register to enable the I2C event interrupt
5. Unlocks the handle
6. Enable I2C interrupt

If the I2C interrupt is generated between point 4 and 5 with the I2C resource still locked the interrupt will never be served and the system will crash.

The problem is that the I2C interrupt has never been disabled before locking the I2C resource but it's been enabled at the end [of course it's a bug. there's no sense to enable an interrupt never disabled].

For the moment i've managed it modifying the I2C HAL module but i'll open an issue to ask to ST to fix it in the next version of this package.

HAL_StatusTypeDef HAL_I2C_Mem_Read_DMA(I2C_HandleTypeDef *hi2c, uint16_t DevAddress, uint16_t MemAddress,

                                      uint16_t MemAddSize, uint8_t *pData, uint16_t Size)

{

 HAL_StatusTypeDef dmaxferstatus;

 /* Check the parameters */

 assert_param(IS_I2C_MEMADD_SIZE(MemAddSize));

 if (hi2c->State == HAL_I2C_STATE_READY)

 {

   if ((pData == NULL) || (Size == 0U))

   {

     hi2c->ErrorCode = HAL_I2C_ERROR_INVALID_PARAM;

     return HAL_ERROR;

   }

   if (__HAL_I2C_GET_FLAG(hi2c, I2C_FLAG_BUSY) == SET)

   {

     return HAL_BUSY;

   }

   /* Process Locked */

   __HAL_LOCK(hi2c);

   hi2c->State      = HAL_I2C_STATE_BUSY_RX;

   hi2c->Mode       = HAL_I2C_MODE_MEM;

   hi2c->ErrorCode  = HAL_I2C_ERROR_NONE;

   /* Prepare transfer parameters */

   hi2c->pBuffPtr   = pData;

   hi2c->XferCount  = Size;

   hi2c->XferOptions = I2C_NO_OPTION_FRAME;

   hi2c->XferISR    = I2C_Mem_ISR_DMA;

   hi2c->Devaddress = DevAddress;

   if (hi2c->XferCount > MAX_NBYTE_SIZE)

   {

     hi2c->XferSize = MAX_NBYTE_SIZE;

   }

   else

   {

     hi2c->XferSize = hi2c->XferCount;

   }

   /* If Memory address size is 8Bit */

   if (MemAddSize == I2C_MEMADD_SIZE_8BIT)

   {

     /* Prefetch Memory Address */

     hi2c->Instance->TXDR = I2C_MEM_ADD_LSB(MemAddress);

     /* Reset Memaddress content */

     hi2c->Memaddress = 0xFFFFFFFFU;

   }

   /* If Memory address size is 16Bit */

   else

   {

     /* Prefetch Memory Address (MSB part, LSB will be manage through interrupt) */

     hi2c->Instance->TXDR = I2C_MEM_ADD_MSB(MemAddress);

     /* Prepare Memaddress buffer for LSB part */

     hi2c->Memaddress = I2C_MEM_ADD_LSB(MemAddress);

   }

   if (hi2c->hdmarx != NULL)

   {

     /* Set the I2C DMA transfer complete callback */

     hi2c->hdmarx->XferCpltCallback = I2C_DMAMasterReceiveCplt;

     /* Set the DMA error callback */

     hi2c->hdmarx->XferErrorCallback = I2C_DMAError;

     /* Set the unused DMA callbacks to NULL */

     hi2c->hdmarx->XferHalfCpltCallback = NULL;

     hi2c->hdmarx->XferAbortCallback = NULL;

     /* Enable the DMA stream or channel depends on Instance */

     dmaxferstatus = HAL_DMA_Start_IT(hi2c->hdmarx, (uint32_t)&hi2c->Instance->RXDR, (uint32_t)pData,

                                      hi2c->XferSize);

   }

   else

   {

     /* Update I2C state */

     hi2c->State    = HAL_I2C_STATE_READY;

     hi2c->Mode     = HAL_I2C_MODE_NONE;

     /* Update I2C error code */

     hi2c->ErrorCode |= HAL_I2C_ERROR_DMA_PARAM;

     /* Process Unlocked */

     __HAL_UNLOCK(hi2c);

     return HAL_ERROR;

   }

   if (dmaxferstatus == HAL_OK)

   {

     /* Send Slave Address and Memory Address */

     I2C_TransferConfig(hi2c, DevAddress, (uint8_t)MemAddSize, I2C_SOFTEND_MODE, I2C_GENERATE_START_WRITE);

     /* Process Unlocked */

     __HAL_UNLOCK(hi2c);

     /* Note : The I2C interrupts must be enabled after unlocking current process

               to avoid the risk of I2C interrupt handle execution before current

               process unlock */

     /* Enable ERR, TC, STOP, NACK, TXI interrupt */

     /* possible to enable all of these */

     /* I2C_IT_ERRI | I2C_IT_TCI | I2C_IT_STOPI | I2C_IT_NACKI |

       I2C_IT_ADDRI | I2C_IT_RXI | I2C_IT_TXI */

     I2C_Enable_IRQ(hi2c, I2C_XFER_TX_IT);

   }

   else

   {

     /* Update I2C state */

     hi2c->State    = HAL_I2C_STATE_READY;

     hi2c->Mode     = HAL_I2C_MODE_NONE;

     /* Update I2C error code */

     hi2c->ErrorCode |= HAL_I2C_ERROR_DMA;

     /* Process Unlocked */

     __HAL_UNLOCK(hi2c);

     return HAL_ERROR;

   }

   return HAL_OK;

 }

 else

 {

   return HAL_BUSY;

 }

}

Post deleted to adhere community guidelines.

[Miglio]

This is exactly the same problem i'm having and i'm talking about in this post. Also in my case the project, developed for a F4 family device [STM32F469BET6], had this kind of problem in the porting to the H7 family device [STM32H7A3IIK6].

For the moment i've fixed it working on the HAL module by myself [stm32h7xx_hal_i2c.c] but i'll open an issue as soon as possible with ST to ask them to fix it in the next firmware package release [now i'm working with v1.11]