STM32CubeG4: Wrong comment in HAL_RCC_DeInit()?

[TwelveSquared]

The function HAL_RCC_DeInit() in STM32G4xx_HAL_Driver/Src/stm32g4xx_hal_rcc.c contains the following:

  /* Clear CR register in 2 steps: first to clear HSEON in case bypass was enabled */
  RCC->CR = RCC_CR_HSION;
 
  /* Then again to HSEBYP in case bypass was enabled */
  RCC->CR = RCC_CR_HSION;

The comments suggest this clears HSEON, followed by HSEBYP.

However, the code appears to set the same value to CR register twice, RCC_CR_HSION.

Is the comment incorrect?

[TDK]

I'd wager it's the correct comment but the incorrect code. Definitely a bug since overwriting the entire CR register is not the intention, in either case.

Probably should be:

/* Clear CR register in 2 steps: first to clear HSEON in case bypass was enabled */
  CLEAR_BIT(RCC->CR, RCC_CR_HSEON);
 
  /* Then again to HSEBYP in case bypass was enabled */
  CLEAR_BIT(RCC->CR, RCC_CR_HSEBYP);

 Not sure why two steps are needed. The F4 simply does:

  /* Clear HSEON, HSEBYP and CSSON bits */
  CLEAR_BIT(RCC->CR, RCC_CR_HSEON | RCC_CR_HSEBYP | RCC_CR_CSSON);

Although I doubt many people use this function. Don't often want to de-initialize the clock settings.

[Imen.D]

Hello,

I tracked this internally for analysis. I will come back to you soon with update.

Thank you for your contribution.

Best Regards,

Imen

[TwelveSquared]

Thank you for clearing that up. I thought there must be something weird in the reference manual that I didn't notice. Now I know I wasn't losing my mind. :)

[TwelveSquared]

Thank you for looking into it.

[Imen.D]

Hello,

Sorry for the delay to answer you.

After check, the code is correct and is optimized. We use a direct register access into RCC CR register to clear HSEON and HSEBYP bits.

The following Line will set HSION bit and enforce the reset on all other bits in CR register (HSEON will be cleared automatically by writing 0)

RCC->CR = RCC_CR_HSION;

equivalent to

RCC->CR &= RCC_CR_HSEON;

Best Regards,

Imen