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

driver/clock_control: Clean up stm32h7 and stm32u5 drivers #41585

Merged

Conversation

erwango
Copy link
Member

@erwango erwango commented Jan 5, 2022

Attempt to clean up STM32 clock_control driver, starting on H7 and U5 series.
Main target is to clean up clock_stm32_ll_common.c which is used by all the other STM32 series,
but I first want to validate ideas on these 2 first ones.

In the end, there are not much changes, aside from:

  • factorization when possible
  • use of IS_ENABLED when possible to ease readability

Changes were tested thanks to new dedicated tests available in tests/drivers/clock_control/stm32_clock_configuration.

Based on:

EDIT (01/18):
Following changes were made on top of existing proposal. Reason not to split this in another PR is that I prefer to group breaking changes in one big PR and then be done with it. I think this is also preferable for driver users.

  • Reworked various clocks sources configuration and enabling. They're now configured independently and in logical order (fixed clocks, then PLLs) at startup based on device tree status. Then they are used if required when configuring sysclock
  • Reworked some STM32_CLOCK dt symbols in order to simplify usage in clock_control drivers.

The rationale behind this new changes is to prepare introduction of alternate / optional clock sources (cf #41650), which requires to be able to enable each source clock independently from each others.

@erwango erwango requested review from ABOSTM and FRASTM January 5, 2022 10:19
@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32 labels Jan 5, 2022
@erwango erwango requested a review from gmarull January 5, 2022 10:20
@erwango erwango force-pushed the dev_stm32_clean_clock_u5h7 branch 3 times, most recently from 5e2b36a to df120be Compare January 10, 2022 15:04
Copy link
Collaborator

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

Few inline comments, otherwise, LGTM

drivers/clock_control/clock_stm32_ll_h7.c Outdated Show resolved Hide resolved
drivers/clock_control/clock_stm32_ll_h7.c Outdated Show resolved Hide resolved
@erwango erwango force-pushed the dev_stm32_clean_clock_u5h7 branch from df120be to a8b57d0 Compare January 10, 2022 16:21
@erwango
Copy link
Member Author

erwango commented Jan 10, 2022

@ABOSTM Thanks for the review. Comments fixed.

@erwango erwango requested a review from ABOSTM January 10, 2022 16:37
@erwango erwango marked this pull request as ready for review January 10, 2022 16:37
ABOSTM
ABOSTM previously approved these changes Jan 10, 2022
Copy link
Collaborator

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

LGTM

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.

The stm32_clock_control_init function return code seems not tested yet, however if it could be <>0 , then we could see in the function something like

if (IS_ENABLED(STM32_PLL_SRC_HSE)) {
...
if (IS_ENABLED(STM32_PLL3_P_ENABLE)) {
...
else
return -ENOTSUP

drivers/clock_control/clock_stm32_ll_u5.c Outdated Show resolved Hide resolved
drivers/clock_control/clock_stm32_ll_u5.c Outdated Show resolved Hide resolved
drivers/clock_control/clock_stm32_ll_h7.c Outdated Show resolved Hide resolved
FRASTM
FRASTM previously approved these changes Jan 13, 2022
@erwango erwango added this to the v3.0.0 milestone Jan 17, 2022
erwango added 7 commits March 1, 2022 11:09
Remove conditions around some definitions since these symbols
and the node itself are mandatory.

Signed-off-by: Erwan Gouriou <[email protected]>
Group definitions by related node.

Signed-off-by: Erwan Gouriou <[email protected]>
Tweak some macros definitions so that they can be used in
IS_ENABLED utility macro.
Finality is to rework STM32 clock_control driver to a more
readable format.

Signed-off-by: Erwan Gouriou <[email protected]>
Use benefits of IS_ENABLED.

Signed-off-by: Erwan Gouriou <[email protected]>
Re-arrange code using benefits of IS_ENABLED.
Change some #if to #ifdef when possible.

Signed-off-by: Erwan Gouriou <[email protected]>
- Factorize elementary clocks setup code.
- Put conditional logic on CONFIG_CPU_CORTEX_M4 outside init function

Signed-off-by: Erwan Gouriou <[email protected]>
PLL3 setting should also be protected CFG_HW_RCC_SEMID.
Move semaphore unlock after we're done with PLL3.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango erwango force-pushed the dev_stm32_clean_clock_u5h7 branch from a390f0e to ea36487 Compare March 1, 2022 10:12
@erwango erwango removed the DNM This PR should not be merged (Do Not Merge) label Mar 1, 2022
drivers/clock_control/clock_stm32_ll_h7.c Outdated Show resolved Hide resolved
@erwango erwango force-pushed the dev_stm32_clean_clock_u5h7 branch from ea36487 to 444ca6c Compare March 1, 2022 13:49
ABOSTM
ABOSTM previously approved these changes Mar 1, 2022
FRASTM
FRASTM previously approved these changes Mar 2, 2022
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.

change commit msg to drivers/clock_control: stm32u5: Use LL API for LSESYS programming

erwango added 4 commits March 2, 2022 14:14
Use LL API when possible.

Signed-off-by: Erwan Gouriou <[email protected]>
Add STM32_FOO_ENABLED and STM32_FOO_FREQ to STM32 fixed clocks:
HSI, HSE, MSI(S), CSI, LSI, LSE..

Replace STM32_LSE_CLOCK by STM32_LSE_FREQ and when possible
replace by new STM32_LSE_ENABLED when making sense.

Fix STM32_PLL3_FOO_ENABLE to STM32_PLL3_FOO_ENABLED

Additionally, add STM32_PLL_FOO_ENABLED definitions.

Signed-off-by: Erwan Gouriou <[email protected]>
Rework clock start up functions in order to allow configuration
and enabling of individual clocks.
This way, each clock defined with a "okay" status will be enabled
even if not part of the sysclock clock tree.

Signed-off-by: Erwan Gouriou <[email protected]>
Instead of reading registers query the info on sysclock configuration
from existing configuration symbols.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango erwango dismissed stale reviews from FRASTM and ABOSTM via 7ec6df3 March 2, 2022 13:14
@erwango erwango force-pushed the dev_stm32_clean_clock_u5h7 branch from 444ca6c to 7ec6df3 Compare March 2, 2022 13:14
@erwango
Copy link
Member Author

erwango commented Mar 2, 2022

change commit msg to drivers/clock_control: stm32u5: Use LL API for LSESYS programming

Fixed, Thanks

@erwango erwango requested review from ABOSTM and FRASTM March 2, 2022 13:14
@erwango erwango changed the title [RFC] driver/clock_control: Clean up stm32h7 and stm32u5 drivers driver/clock_control: Clean up stm32h7 and stm32u5 drivers Mar 7, 2022
@carlescufi carlescufi merged commit d74cb2a into zephyrproject-rtos:main Mar 7, 2022
@erwango erwango deleted the dev_stm32_clean_clock_u5h7 branch January 19, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Boards area: Clock Control area: Devicetree area: Documentation area: Samples Samples area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants