Problem in HAL_I2C_Mem_Read, no NAK generated in rare cases

[ks000]

Describe the set-up

• The board: custom board with STM32F407ZET6
• IDE: STM32CubeIDE 1.9.0

Describe the bug (skip if none)

In some cases HAL_I2C_Mem_Read will finish reading given number of bytes from I2C slave, but will not NAK the last byte. This may cause the slave to pull SDA low if its next byte's MSb is 0, thus freezing the I2C bus.

How to reproduce the bug (skip if none)

1. My code is reading the data from slave via I2C bus using HAL_I2C_Mem_Read function.

2. List the modules that you suspect to be the cause of the problem:

   • stm32f4xx_hal_i2c.c

3. Describe the use case that generates the problem The problem appears when receiving >3 data bytes and HAL_I2C_Mem_Read function can be pre-empted. This could be also caused by an interrupt (I did not check this).

4. How we can reproduce the problem: Run HAL_I2C_Mem_Read from a task which can be pre-empted by FreeRTOS (or some interrupt) in order to read >3 data bytes via I2C.

Additional context

It seems that I localized the problem to lines 2812-2823 in file stm32f4xx_hal_i2c.c (looking at the latest commit 3d6be4b)

• The check of BTF flag and DR readout in these lines seems to be wrong and causing the erroneous behavior.
• Under normal circumstances (when task is not pre-empted by RTOS) BTF flag should be never high immediately after DR readout (line 2803). Even if BTF was high before DR read, according to reference manual it will be cleared when DR is read. So, this check seems unnecessary.
• Under normal circumstances last 3 data bytes will be handled by code in lines 2744-2792 which disables ACK for correct ending of data reception (with NAK).
• When there are exactly 4 bytes left to receive, the code will go to RXNE flag loop as usual in lines 2795-2824. If RTOS will switch to another task (or some interrupt happens) while code in lines 2806-2810 is being executed (after receiving 4th last byte) and some other task (or ISR) takes sufficient time for I2C DR and the shift register to be filled by incoming I2C data (3rd and 2nd last bytes), the problem will manifest itself:
  • BTF flag will be set and the following check in line 2812 will yield TRUE, so the code inside IF clause gets executed.
  • The code will read 3rd last data byte from I2C DR register and reduce the counter to 2.
  • On the next loop the data reception will not be handled in lines 2744-2792 which disable ACK, but in lines 2711-2741 which will only generate STOP (if possible), but not disable ACK. This causes the described behavior on I2C bus when slave does not get NAK and prepares to send the next byte which can freeze the bus.
  • My solution: remove the code in lines 2812-2823.

Screenshots

I used logic analyzer to drill down to the reason of the no-NAK behavior and used 4 extra GPIO outputs to understand what the code is doing. On screenshots, there is I2C bus SCL and SDA signals, as well as 4 GPIO lines:

• DBG0 (yellow) corresponds to execution of code in lines 2795-2824 (RXNE flag poll loop)
• DBG1 (green) corresponds to execution of code in lines 2692-2709 (1 byte left to receive)
• DBG2 (blue) corresponds to execution of code in lines 2711-2741 (2 bytes left to receive)
• DBG3 (purple) corresponds to execution of code in lines 2744-2792 (3 bytes left to receive)

On screenshot with normal data reception the 6 data bytes are received as expected: first three bytes with RXNE loop (DBG0 high), last three bytes in lines 2744-2792 (DBG3 high), NAK in the end.

On screenshot with the problematic behavior, the last bytes are handled in code lines 2711-2741 (DBG2 high), no NAK in the end.

[ghost]

Hi. Any idea when this issue will be solved in the STMCubeF4? I'm facing a similar issue and most likely the root cause is the same... The I2C slave is the lowest priority RTOS task which means that it is preempted quite often. We have situations where the slave puts the SDA line low as described in the ticket. To overcome, we implemented a workaround which sends nine clock pulses to unfreeze the slave (see chapter 3.1.16 from UM10204 document).

[ASELSTM]

ST Internal Reference: 156225

[ASELSTM]

Hi @ks000,

Would you please try this patch ( update in HAL_I2C_Mem_Read() ) and check whether you are still having the same issue or not.

HAL_StatusTypeDef HAL_I2C_Mem_Read(I2C_HandleTypeDef *hi2c, uint16_t DevAddress, uint16_t MemAddress, uint16_t MemAddSize, uint8_t *pData, uint16_t Size, uint32_t Timeout)
{
  /* Init tickstart for timeout management*/
  uint32_t tickstart = HAL_GetTick();

  /* Check the parameters */
  assert_param(IS_I2C_MEMADD_SIZE(MemAddSize));

  if (hi2c->State == HAL_I2C_STATE_READY)
  {
    /* Wait until BUSY flag is reset */
    if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BUSY, SET, I2C_TIMEOUT_BUSY_FLAG, tickstart) != HAL_OK)
    {
      return HAL_BUSY;
    }

    /* Process Locked */
    __HAL_LOCK(hi2c);

    /* Check if the I2C is already enabled */
    if ((hi2c->Instance->CR1 & I2C_CR1_PE) != I2C_CR1_PE)
    {
      /* Enable I2C peripheral */
      __HAL_I2C_ENABLE(hi2c);
    }

    /* Disable Pos */
    CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_POS);

    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->XferSize    = hi2c->XferCount;
    hi2c->XferOptions = I2C_NO_OPTION_FRAME;

    /* Send Slave Address and Memory Address */
    if (I2C_RequestMemoryRead(hi2c, DevAddress, MemAddress, MemAddSize, Timeout, tickstart) != HAL_OK)
    {
      return HAL_ERROR;
    }

    if (hi2c->XferSize == 0U)
    {
      /* Clear ADDR flag */
      __HAL_I2C_CLEAR_ADDRFLAG(hi2c);

      /* Generate Stop */
      SET_BIT(hi2c->Instance->CR1, I2C_CR1_STOP);
    }
    else if (hi2c->XferSize == 1U)
    {
      /* Disable Acknowledge */
      CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK);

      /* Clear ADDR flag */
      __HAL_I2C_CLEAR_ADDRFLAG(hi2c);

      /* Generate Stop */
      SET_BIT(hi2c->Instance->CR1, I2C_CR1_STOP);
    }
    else if (hi2c->XferSize == 2U)
    {
      /* Disable Acknowledge */
      CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK);

      /* Enable Pos */
      SET_BIT(hi2c->Instance->CR1, I2C_CR1_POS);

      /* Clear ADDR flag */
      __HAL_I2C_CLEAR_ADDRFLAG(hi2c);
    }
    else
    {
      /* Clear ADDR flag */
      __HAL_I2C_CLEAR_ADDRFLAG(hi2c);
    }

    while (hi2c->XferSize > 0U)
    {
      if (hi2c->XferSize <= 3U)
      {
        /* One byte */
        if (hi2c->XferSize == 1U)
        {
          /* Wait until RXNE flag is set */
          if (I2C_WaitOnRXNEFlagUntilTimeout(hi2c, Timeout, tickstart) != HAL_OK)
          {
            return HAL_ERROR;
          }

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
        /* Two bytes */
        else if (hi2c->XferSize == 2U)
        {
          /* Wait until BTF flag is set */
          if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BTF, RESET, Timeout, tickstart) != HAL_OK)
          {
            return HAL_ERROR;
          }

          /* Generate Stop */
          SET_BIT(hi2c->Instance->CR1, I2C_CR1_STOP);

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
        /* 3 Last bytes */
        else
        {
          /* Wait until BTF flag is set */
          if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BTF, RESET, Timeout, tickstart) != HAL_OK)
          {
            return HAL_ERROR;
          }

          /* Disable Acknowledge */
          CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK);

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;

          /* Wait until BTF flag is set */
          if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BTF, RESET, Timeout, tickstart) != HAL_OK)
          {
            return HAL_ERROR;
          }

          /* Generate Stop */
          SET_BIT(hi2c->Instance->CR1, I2C_CR1_STOP);

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
      }
      else
      {
        /* Wait until RXNE flag is set */
        if (I2C_WaitOnRXNEFlagUntilTimeout(hi2c, Timeout, tickstart) != HAL_OK)
        {
          return HAL_ERROR;
        }

        /* Read data from DR */
        *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

        /* Increment Buffer pointer */
        hi2c->pBuffPtr++;

        /* Update counter */
        hi2c->XferSize--;
        hi2c->XferCount--;

        if (__HAL_I2C_GET_FLAG(hi2c, I2C_FLAG_BTF) == SET)
        {

+         if (hi2c->XferSize == 3U)
+         {
+           /* Disable Acknowledge */
+          CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK);
+          }

          /* Read data from DR */
          *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;

          /* Increment Buffer pointer */
          hi2c->pBuffPtr++;

          /* Update counter */
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
      }
    }

    hi2c->State = HAL_I2C_STATE_READY;
    hi2c->Mode = HAL_I2C_MODE_NONE;

    /* Process Unlocked */
    __HAL_UNLOCK(hi2c);

    return HAL_OK;
  }
  else
  {
    return HAL_BUSY;
  }
}

With regards,

[RJMSTM]

I hope you are fine. The issue you reported has been fixed in the frame of version v1.28.0 of the STM32CubeF4 published recently on GitHub. Thank you again for having reported.