STMicroelectronics / STM32CubeG4

STM32Cube MCU Full Package for the STM32G4 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
192 stars 103 forks source link

UTILS_ConfigureSystemClock example not functioning correctly #11

Closed fivdi closed 3 years ago

fivdi commented 4 years ago

Describe the set-up

Describe the bug The UTILS_ConfigureSystemClock example for the NUCLEO-G431RB which can be found here doesn't function correctly.

There are two problems.

Problem 1

When the example runs, LL_PLL_ConfigSystemClock_HSI which is called here returns ERROR. It returns ERROR because the PLL is enabled when LL_PLL_ConfigSystemClock_HSI is called. In order for LL_PLL_ConfigSystemClock_HSI to function correctly, the application must ensure that the PLL is disabled. The PLL was enabled by calling SystemClock_Config directly before calling LL_PLL_ConfigSystemClock_HSI.

To workaround this problem the call to SystemClock_Config can be commented out as it's not actually needed here. However, this leads to the second problem.

Problem 2

After commenting out the call to SystemClock_Config, the call to LL_PLL_ConfigSystemClock_HSI hangs and never returns. This occurs because the data passed to LL_PLL_ConfigSystemClock_HSI in sUTILS_PLLInitStruct is incorrect. Currently, sUTILS_PLLInitStruct is initialized as follows which is incorrect:

/* Variable to store PLL parameters */
/* Configuration will allow to reach a SYSCLK frequency set to 170MHz:
   Syst freq = ((HSI_VALUE / PLLM) * PLLN)/ PLLP)
               ((16MHz /4) * 85)/ 2)             = 170MHz             */
LL_UTILS_PLLInitTypeDef sUTILS_PLLInitStruct = {LL_RCC_PLLM_DIV_4, 85, LL_RCC_PLLP_DIV_2};

Here is how sUTILS_PLLInitStruct should be initialized:

/* Variable to store PLL parameters */
/* Configuration will allow to reach a SYSCLK frequency set to 170MHz:
   Syst freq = ((HSI_VALUE / PLLM) * PLLN)/ PLLR)
               ((16MHz /4) * 85)/ 2)             = 170MHz             */
LL_UTILS_PLLInitTypeDef sUTILS_PLLInitStruct = {LL_RCC_PLLM_DIV_4, 85, LL_RCC_PLLR_DIV_2};

Notice how PLLP has been replaced with PLLR in both the comment and the code.

A similar example for the NUCLEO-G474RE which can be found here most likely suffers from the same problems.

ALABSTM commented 4 years ago

Hi @fivdi,

First of all thank you for this detailed analysis and the solution suggested. We do appreciate.

From our side, we tried to reproduce the issue you described but could not. We used both NUCLEO-G431RB and NUCLEO-G474RE boards. We also used EWARM v8.20.2, v8.30.1 and Cube IDE v1.0.2.

May I ask whether you introduced any change to the original source code, the project configuration, or any other aspect of the example? It would be helpful if you could provide us with the project you used to obtain the issue.

Thank you,

fivdi commented 4 years ago

First of all thank you for this detailed analysis and the solution suggested. We do appreciate.

You're welcome.

From our side, we tried to reproduce the issue you described but could not.

I can still reproduce the issue today.

We used both NUCLEO-G431RB and NUCLEO-G474RE boards. We also used EWARM v8.20.2, v8.30.1 and Cube IDE v1.0.2.

I used a NUCLEO-G431RB and arm-none-eabi-gcc-xpack and did everything from the command line.

$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (xPack GNU Arm Embedded GCC, 32-bit) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

May I ask whether you introduced any change to the original source code

No changes whatsoever were made to the original source code.

the project configuration, or any other aspect of the example?

Yes, I changed everything. I used my own projects.

It would be helpful if you could provide us with the project you used to obtain the issue.

I've created a repository with four projects that show how the issue can be reproduced here.

project1

project1 contains exactly the same code as the UTILS_ConfigureSystemClock example. This will be what you looked at. You think it works and it looks like it works but internally it does not work as it should. It looks like it works by accident. I would imagine that you are using a scope connected to the MCO pin and seeing what you expect to see. However, what you see on the MCO pin was set by the call to SystemClock_Config and not by the call to LL_PLL_ConfigSystemClock_HSI. The call to LL_PLL_ConfigSystemClock_HSI is not functioning correctly.

project2

project2 contains two modifications to main.c. The first change is here and the second change is here. If you run project2 you will notice that the on board LED blinks at 5Hz indicating that LL_PLL_ConfigSystemClock_HSI returned an error.

project3

project3 contains one modification to main.c. The call to SystemClock_Config here has been commented out. Commenting this line out should be ok and the program should continue to function correctly. However, the program no longer functions because LL_PLL_ConfigSystemClock_HSI never returns. This can be verified by running the program and observing that the on board LED does not blink at 0.5Hz as it should.

project 4

project4 contains four modification to main.c. The changes are here, here, here, and here. The call to LL_PLL_ConfigSystemClock_HSI functions correctly and does not return an error. The on board LED blinks at 0.5Hz as expected.

If you have any further questions please let me know.

ALABSTM commented 4 years ago

ST Internal Reference: 90961

RKOUSTM commented 3 years ago

Hi @fivdi,

Thank you for your contribution. The fix you requested has been implemented and is available in the frame of the STM32CubeG4 package V1.4.0 release.

This issue can be closed now. Thank you again for your contribution.

With regards,