Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: clock_control: stm32 ll common: fix RCC pll disable #83062

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bbyers-UBCO
Copy link

The clock_stm32_ll_common.c function set_up_plls calls LL_RCC_PLL_Disable(); and it was not waiting for the disable to complete before trying to configure the pll sysclock which creates a race condition for pll configuration. The wait for re-enabling the RCC pll is already there, it was just missing the wait for the disable before configuration.

Copy link

Hello @bbyers-UBCO, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bbyers-UBCO !

Please fix the compliance issues reported by ci here.

Comment on lines 572 to 574
/* Disable PLL */
LL_RCC_PLL_Disable();
while (LL_RCC_PLL_IsReady() != 0U) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix indentation (we're using 8 char tabs)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Also added the LL_RCC_PLL2_IsReady() check as @FRASTM mentioned.

Copy link
Collaborator

@FRASTM FRASTM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing that
LGTM after formatting the code

Probably that control loop should also apply anywhere a PLL is being disabled by LL_RCC_PLL1_Disable() or LL_RCC_PLL2_Disable() before any access to the PLL config register.

The clock_stm32_ll_common.c function set_up_plls calls LL_RCC_PLL_Disable();
and it was not waiting for the disable to complete before trying to configure
the pll sysclock which creates a race condition for pll configuration.
The wait for re-enabling the RCC pll is already there, it was just missing
the wait for the disable before configuration. Also added the wait for PLL2.

Signed-off-by: Benjamin Curtis Byers <[email protected]>
@bbyers-UBCO bbyers-UBCO force-pushed the stm32-pll-clock-disable-fix branch from b070786 to 9294bae Compare December 17, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants