search

STMicroelectronics / STM32CubeF4

STM32Cube MCU Full Package for the STM32F4 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
869 stars 418 forks source link

stm32f4xx_hal_rcc.c has a broken check for clock init #84

Closed InconsolableCellist closed 3 years ago

InconsolableCellist commented 3 years ago

I believe this is still broken. In stm32f4xx_hal_rcc.c on line 553 the HEAD in master from git shows:

(READ_BIT(pll_config, RCC_PLLCFGR_PLLSRC) != RCC_OscInitStruct->PLL.PLLSource)

https://github.com/STMicroelectronics/STM32CubeF4/blob/2f3b26f16559f7af495727a98253067a31182cfc/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_rcc.c#L553

There was a partial fix in d599a824d, but you need to also shift the PLL.PLLSource over by RCC_PLLCFGR_PLLSRC_Pos like the other ones, or perhaps better, use READ_BIT.

I believe this error may be repeated on lines 545 and 1043, but I didn't check those codepaths.

https://github.com/STMicroelectronics/STM32CubeF4/blob/2f3b26f16559f7af495727a98253067a31182cfc/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_rcc.c#L545

https://github.com/STMicroelectronics/STM32CubeF4/blob/2f3b26f16559f7af495727a98253067a31182cfc/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_rcc.c#L1043

Additionally, there seems to be a regression introduced by 55c4943bb8 where it returns HAL_ERROR despite a valid PLL config, generated by STM32CubeMX 6.3.0 on the STM32F407 DISC-1 board using the following (and apparently valid) clock configuration: https://i.imgur.com/Bz2ifz1.png

In the latest git version of the code (2f3b26f1, master branch) there's a discrepancy between RCC->PLLCFGR_PLLN and the requested RCC_OscInitStruct->PLL.PLLN. In this case, I'm requesting the valid PLLN of 336 (RM0090 Rev. 19, p. 227), but the old PLLN is something different. The RCC->PLLCFGR is never updated by anything (like it is on line 78, but only in the case of it not being a system clock already).

I think the fundamental issue may be that STM32CubeMX is generating init code for using a PLL as a system clock, while the safety checks in that HAL file assume you're on a non-PLL and trying to switch to it--and it only lets you if your configuration is identical to what it was before.

Originally posted by @InconsolableCellist in https://github.com/STMicroelectronics/STM32CubeF4/issues/17#issuecomment-887977421

RKOUSTM commented 3 years ago

Hi @InconsolableCellist,

Thank you for this clear and concise report. It has been forwarded to our development teams. I will get to you as soon as they provide their feedback.

With regards,

RKOUSTM commented 3 years ago

Hi @InconsolableCellist,

First we would like to thank you for your report.

We tried to reproduce the issue you have described using the STM32CubeMX v6.3.0 and the IAR v8.50.6 running on STM32F407VGT6 board, with no success. I really hope you could find the root cause of this problem and, ideally, solve it.

Regarding the point for which you opened the issue, as it is irrelevant, allow me to close the issue. Anyway, if you ever become able to reproduce systematically the faulty behavior and find out that the problem is related to our drivers, do not hesitate to recontract me. We could reopen this issue again.

Thank you for your comprehension and thank you again for your report.

With regards,