cancel
Showing results for 
Search instead for 
Did you mean: 

HAL Bug in stm32l1xx_hal.c?

MSipo
Senior II

I'm using FirmwarePackage=STM32Cube FW_L1 V1.10.1

I'm looking at these functions in stm32l1xx_hal.c:

/**
  * @brief  Return the first word of the unique device identifier (UID based on 96 bits)
  * @retval Device identifier 31:0 bits
  */
uint32_t HAL_GetUIDw0(void)
{
  return(READ_REG(*((uint32_t *)UID_BASE)));
}
 
/**
  * @brief  Return the second word of the unique device identifier (UID based on 96 bits)
  * @retval Device identifier 63:32 bits
  */
uint32_t HAL_GetUIDw1(void)
{
  return(READ_REG(*((uint32_t *)(UID_BASE + 0x4U))));
}
 
/**
  * @brief  Return the third word of the unique device identifier (UID based on 96 bits)
  * @retval Device identifier 95:64 bits
  */
uint32_t HAL_GetUIDw2(void)
{
  return(READ_REG(*((uint32_t *)(UID_BASE + 0x14U))));
}

They do not use a volatile pointer. Shouldn't they? Could a compiler optimize this in a bad way?

I would write them as:

return *((__IO uint32_t *)UID_BASE);

4 REPLIES 4

Why would it need to be volatile? The content doesn't change between reads.

Tips, buy me a coffee, or three.. PayPal Venmo Up vote any posts that you find helpful, it shows what's working..

But the IDCODE register is defined volatile and that doesn't either change?

/** 
  * @brief Debug MCU
  */
 
typedef struct
{
  __IO uint32_t IDCODE;       /*!< MCU device ID code,                          Address offset: 0x00 */
  __IO uint32_t CR;           /*!< Debug MCU configuration register,            Address offset: 0x04 */
  __IO uint32_t APB1FZ;       /*!< Debug MCU APB1 freeze register,              Address offset: 0x08 */
  __IO uint32_t APB2FZ;       /*!< Debug MCU APB2 freeze register,              Address offset: 0x0C */
}DBGMCU_TypeDef;

And do those actually need to be volatile either? I think you're working this problem in the wrong direction.

A bug would be a situation where something changed outside of program control/flow and wasn't marked as volatile.

The blanket __IO in the structures avoids a lot of register level evaluation and maintenance on the library maintainers part, some of whom might not have a particularly strong understanding of the exact HW interactions at a bit/register level.

Tips, buy me a coffee, or three.. PayPal Venmo Up vote any posts that you find helpful, it shows what's working..

Thanks. I got confused why particularly these register wasn't defined as volatile. But I understand now. For non-volatile it reads it at least once but than maybe no more. And for volatile it always reads it again before evaluating.