2024-08-27 11:27 AM - edited 2024-08-27 01:06 PM
Can you see the glaring bug, or is it? I left a clue *** some fail returns do not unlock HAL
HAL_StatusTypeDef HAL_I2C_Mem_Write(I2C_HandleTypeDef *hi2c, uint16_t DevAddress, uint16_t MemAddress,
uint16_t MemAddSize, uint8_t *pData, uint16_t Size, uint32_t Timeout)
{
uint32_t tickstart;
/* 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;
}
/* Process Locked ***/
__HAL_LOCK(hi2c);
/* Init tickstart for timeout management*/
tickstart = HAL_GetTick();
if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BUSY, SET, I2C_TIMEOUT_BUSY, tickstart) != HAL_OK)
{
return HAL_ERROR;
}
hi2c->State = HAL_I2C_STATE_BUSY_TX;
hi2c->Mode = HAL_I2C_MODE_MEM;
hi2c->ErrorCode = HAL_I2C_ERROR_NONE;
/* Prepare transfer parameters */
hi2c->pBuffPtr = pData;
hi2c->XferCount = Size;
hi2c->XferISR = NULL;
/* Send Slave Address and Memory Address */
if (I2C_RequestMemoryWrite(hi2c, DevAddress, MemAddress, MemAddSize, Timeout, tickstart) != HAL_OK)
{
/* Process Unlocked */
__HAL_UNLOCK(hi2c);
return HAL_ERROR;
}
...
Solved! Go to Solution.
2024-09-03 02:47 AM
Checked this point internally and seems the Unlock is done on each function call in return of HAL_I2C_Mem_Write() in this list of function:
I2C_WaitOnFlagUntilTimeout()
I2C_WaitOnTXISFlagUntilTimeout()
I2C_WaitOnSTOPFlagUntilTimeout()
Example. In this function the unlock is done in line 24 before returning error.
static HAL_StatusTypeDef I2C_WaitOnFlagUntilTimeout(I2C_HandleTypeDef *hi2c, uint32_t Flag, FlagStatus Status,
uint32_t Timeout, uint32_t Tickstart)
{
while (__HAL_I2C_GET_FLAG(hi2c, Flag) == Status)
{
/* Check if an error is detected */
if (I2C_IsErrorOccurred(hi2c, Timeout, Tickstart) != HAL_OK)
{
return HAL_ERROR;
}
/* Check for the Timeout */
if (Timeout != HAL_MAX_DELAY)
{
if (((HAL_GetTick() - Tickstart) > Timeout) || (Timeout == 0U))
{
if ((__HAL_I2C_GET_FLAG(hi2c, Flag) == Status))
{
hi2c->ErrorCode |= HAL_I2C_ERROR_TIMEOUT;
hi2c->State = HAL_I2C_STATE_READY;
hi2c->Mode = HAL_I2C_MODE_NONE;
/* Process Unlocked */
__HAL_UNLOCK(hi2c);
return HAL_ERROR;
}
}
}
}
return HAL_OK;
}
2024-08-27 11:38 AM - edited 2024-08-27 11:42 AM
There is also another fail here in this function definition, one arm has a return, the other does not. Sloppy really as the compiler even reports the issue "Return has value in function returning void".
#define __HAL_LOCK(__HANDLE__) \
do{ \
if((__HANDLE__)->Lock == HAL_LOCKED) \
{ \
return HAL_BUSY; \
} \
else \
{ \
(__HANDLE__)->Lock = HAL_LOCKED; \
} \
}while (0)
2024-08-27 12:24 PM
I admit up front I am no fan of the "return hidden in a macro" coding style of the __HAL_LOCK() macro.
That said, you should not see that compiler warning for the __HAL_LOCK() call in HAL_I2C_Mem_Write() as that function DOES have a return value. So where DID you see that compiler warning?
Which CPU? I presume the "V1.11.2" refer to the Cube library version?
2024-08-27 01:33 PM - edited 2024-08-27 01:33 PM
The defined function has no return specification so the compiler assumes its void, then one arm of the "if" returns a value, of course its going to give a warning! I´m using the latest version of CubeIDE and the warning is on the tool tip when hovering over the use of the defined function __HAL_LOCK and in the compiler warnings.
MCU is H743, Repository is the latest V1.11.2 at -> STM32Cube\Repository\STM32Cube_FW_H7_V1.11.2\Drivers\STM32H7xx_HAL_Driver\Src
2024-08-27 01:36 PM - edited 2024-08-27 01:54 PM
Escalated internally to be fixed.
Internal ticket number for tracking 189599.
2024-08-27 06:49 PM
There were quite a lot of changes from HAL V1.11.1 to 1.11.2, stm32h7xx_hal_i2c.c has quite a few changes, and our application is no longer stable communicating over I2C to a PLL SI5351, though we are still investigating the reason.
Is there any information on I2C bugs in this release?
2024-08-27 07:04 PM - edited 2024-08-27 07:13 PM
Do you mean that return on line 25 after I2C_WaitOnFlagUntilTimeout does not call __HAL_UNLOCK?
No bug there. I2C_WaitOnFlagUntilTimeout calls unlock when it returns error.
Neither __HAL_LOCK macro will cause "Return has value in function returning void" - when used properly (in functions that return HAL_StatusTypedef.
/* It's only a matter of taste, as one my friend said after watching the Paris Olympics opening */
> Is there any information on I2C bugs in this release?
STM32H7 has some I2C related errata. Please check the errata document. For the HAL library bugs - see open issues on github.
2024-08-27 08:27 PM
As far as I see there are three error return points in HAL_I2C_Mem_Write, only 1 unlocks HAL.
No reference to any I2C bugs in HAL on the link you sent, what a pain this all is.
2024-09-03 02:47 AM
Checked this point internally and seems the Unlock is done on each function call in return of HAL_I2C_Mem_Write() in this list of function:
I2C_WaitOnFlagUntilTimeout()
I2C_WaitOnTXISFlagUntilTimeout()
I2C_WaitOnSTOPFlagUntilTimeout()
Example. In this function the unlock is done in line 24 before returning error.
static HAL_StatusTypeDef I2C_WaitOnFlagUntilTimeout(I2C_HandleTypeDef *hi2c, uint32_t Flag, FlagStatus Status,
uint32_t Timeout, uint32_t Tickstart)
{
while (__HAL_I2C_GET_FLAG(hi2c, Flag) == Status)
{
/* Check if an error is detected */
if (I2C_IsErrorOccurred(hi2c, Timeout, Tickstart) != HAL_OK)
{
return HAL_ERROR;
}
/* Check for the Timeout */
if (Timeout != HAL_MAX_DELAY)
{
if (((HAL_GetTick() - Tickstart) > Timeout) || (Timeout == 0U))
{
if ((__HAL_I2C_GET_FLAG(hi2c, Flag) == Status))
{
hi2c->ErrorCode |= HAL_I2C_ERROR_TIMEOUT;
hi2c->State = HAL_I2C_STATE_READY;
hi2c->Mode = HAL_I2C_MODE_NONE;
/* Process Unlocked */
__HAL_UNLOCK(hi2c);
return HAL_ERROR;
}
}
}
}
return HAL_OK;
}
2024-09-03 02:56 AM
@Robmar wrote:The defined function has no return specification so the compiler assumes it's void
Really?
Wouldn't it assume int ?
🤔