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

[RFC] stm32: clock_control: Enable device clock source selection #45053

Merged
merged 24 commits into from
May 10, 2022

Conversation

erwango
Copy link
Member

@erwango erwango commented Apr 22, 2022

EDIT 08'22:
This PR was some time ago and is now largely in use. But I can now see that "alternative/alternate/optional" wording is confusing and "domain" clock would have better a better choice. If you read these lines, please keep this in mind. Follow up change: #48679

TL;DR

STM32 clock_control driver update to provide a way to configure alternate clocks on peripherals that support it:

    clocks = <&rcc STM32_CLOCK_BUS_APB2 0x00001000>,  <- Existing, bus driven, "register" clock
             <&rcc STM32_SRC_PLL3_P SPI123_SEL(2)>;   <- Newly added alternate clock (PLL3_P output here)

This alternate clock source could be selected in peripheral driver thanks to new clock_control_configure() API:

	if (IS_ENABLED(STM32_SPI_OPT_CLOCK_SUPPORT) && (cfg->pclk_len > 1)) {
		if (clock_control_configure(DEVICE_DT_GET(STM32_CLOCK_CONTROL_NODE),
					    (clock_control_subsys_t) &cfg->pclken[1],
					    NULL) != 0) {
			LOG_ERR("Could not select alternate SPI source clock");
			return -EIO;
		}
	}

Follow up of #42097 which was limited to H7.

Goal

Aim of this work is to enable the use of an optional clock in a device configuration.
This is particularly required in case of STM32H7 based platform on which device clock configuration is usually based on 2 different clocks (Fixes #41650).

  • A register clock, required for device configuration, usually coming from APB/AHB bus.
  • A kernel clock, which will be used to clock the hardware block (and hence is the one driving the device frequency), typically derived from an alternate clck source (PLL output, fixed clock, ..)

On other STM32 series, provide the possibility to provide an alternate clock to a peripheral. For instance, configuring LSE as the LPUART or LPTIM clock, or HSI48 as the USB clock.

Update of clock_control API (03/14):

Reworked to make use of new clock_control_configure() API, cf #43790

Implementation options:

Alternate clocks bindings

Additional/Alternate clocks could be fixed clocks (HSI, HSE, ..), PLL outputs (PLLX_P/Q/R), sysclock, ....
For all these clocks, I've decided to keep existing RCC device driver as the clock controller. As a consequence, the dt binding used to select a specific clock looks to:

	clocks = <&rcc STM32_CLOCK_BUS_APB2 0x00001000>,  <- Existing, bus driven, "register" clock
		 <&rcc STM32_SRC_PLL3_P SPI123_SEL(2)>;   <- Newly added alternate clock (PLL3_P output here)

Another way to support this would have been to set up each potential clock source as a clock controler on its own and use the following binding:

	clocks = <&rcc STM32_CLOCK_BUS_APB2 0x00001000>,  <- Existing, bus driven, "register" clock
		 <&pll3 pll_p_output SPI123_SEL(2)>;      <- Discarded binding option

I've discarded this second option because, to be consistent with the binding, this second option would require to instantiate each potential source clock as a device driver, each one implementing clock_control API. This would have had negative impact on code footprint, which could have been considered ok for STM32H7 series but problematic on "smaller" STM32 series.

Use of clock-names

In #42097, I tried to comply with H7 reference manuals (clock-names = "reg", "kernel";). But extending to other series this was hard to find a single nomenclature matching all series. Besides, usage of clock-names didn't show any benefit of code simplification.
As a consequence, in this PR, I'm setting the use of clock-names property to optional. It is consumer driver responsibility to access and correctly use clock source cell.

Clock mux bindings

Complete solution requires to find a way to configure clock mux (as per_ck).

First option is to add a dedicated node descibing the clock mux. Its only property is to configure its clock input

&perck {
	clocks = <&rcc STM32_SRC_HSI_KER CKPER_SEL(0)>;
	status = "okay";
};

Works fine, main drawback is the cost (~220 bytes of flash)

A much simpler alternative was to add clock configuration directly to the clock consumer end node:

	clocks = <&rcc STM32_CLOCK_BUS_APB2 0x00001000>,
		 <&rcc STM32_SRC_HSI_KER CKPER_SEL(0)>,
		 <&rcc STM32_SRC_CKPER SPI123_SEL(4)>;

Though, this proposal has 2 main drawbacks:

  • If board dts is poorly configured, 2 consumers of the same internal clock (per_ck here) could set conflicting configurations
  • This adds complexity on the consumers driver implementation side that should be able to differentiate between all these clocks

In the end the addition of a clock mux node device felt saner.

Limitations and next steps:

  • Only STM32 SPI driver is made aware of this. All STM32 drivers should be updated to take this new clock config into account.
  • It is not possible today to clock_off supplementary clocks as they potentially clocks multiple peripherals (or even the core itself). So, this most likely requires the use of power-domains
  • On some series using "common driver" some source clocks are not yet implemented (specifically the sources based on a specific PLL output, like PLL_P, this is left as a future work). For this series, I'm focusing on a common subset of source clocks: HSE, HIS, MSI, LSE, LSI, "main" PLL output and SYSCLK.

@github-actions github-actions bot added area: API Changes to public APIs area: Boards area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: SPI SPI bus platform: STM32 ST Micro STM32 labels Apr 22, 2022
@erwango erwango force-pushed the dev_full_alt_clocks branch from 41a8f59 to 6755ad0 Compare April 22, 2022 12:58
@erwango
Copy link
Member Author

erwango commented Apr 22, 2022

CI reported issues are not related: fixed

@erwango
Copy link
Member Author

erwango commented Apr 25, 2022

@benediktibk, @r2r0, @abxy, @lmajewski, @GeorgeCGV, @ycsin, @heinwessels, @nfaurant, @str4t0m, @lochej, @nandojve

@lmajewski
Copy link
Collaborator

I'm just wondering if this approach to configure clocks in a generic way for STM32 MCU series is somewhat standard approach for other SoCs from this manufacturer?

In other words - is it only one off for STM32 or can it be applied to other M3/M4 ARM CPUs from other manufacturers (like e.g. NXP) ?

@erwango
Copy link
Member Author

erwango commented Apr 25, 2022

I'm just wondering if this approach to configure clocks in a generic way for STM32 MCU series is somewhat standard approach for other SoCs from this manufacturer?

The change is based on a new clock_control API which is generic and proposed in standalone here. I guess the feature itself is also generic as alternate clock source is also something common across vendors (with some exceptions).
This being said, the binding proposal is STM32 specific. It also differs from STM32 Linux bindings (which also differs from STM32 uboot bindings). Aim is to make a first step in direction of a more flexible clock framework as the lack of this feature is blocking in some cases (cf #41650 but also wake-up pin selection) and has to be implemented today per driver (cf #44988), which has limitations.

@erwango
Copy link
Member Author

erwango commented Apr 26, 2022

EDIT: I'm finding some test issues, it might not work on some cases.

erwango added 14 commits May 9, 2022 12:00
Add support for CKPER clock mux.

Signed-off-by: Erwan Gouriou <[email protected]>
Add 2 scenarios to test CKPER used as a clock source.

Signed-off-by: Erwan Gouriou <[email protected]>
Add a stm32u5_devices test which aims at testing devices
clock control configuration on stm32u5 targets

Signed-off-by: Erwan Gouriou <[email protected]>
This new binding allows to work on providing stm32u5 specific
alternate and complementary device clocks.

Signed-off-by: Erwan Gouriou <[email protected]>
This change updates stm32u5 driver to support configuration of
optional clocks on peripherals.

Signed-off-by: Erwan Gouriou <[email protected]>
PLL input should be between 4 and 16MHz, so when MSI is set to 4MHz
fix PLLM can't be higher than 1.
Fix PLL1-NQR in consequence.

Signed-off-by: Erwan Gouriou <[email protected]>
…ssors

Rename and factorize clock source bindings accessors by moving them
in common header file stm32_clock_control and remove them from
include/dt-bindings/clock/stm32XY_clock.h files

Signed-off-by: Erwan Gouriou <[email protected]>
Add clock sources bindings on F0/F3/G0/G4/L0/L1/L4/WB/WL series.
Due to inconsistencies, some common bindings are now split:
F1 -> F0/F1/F3
L4 -> L4/G4/WB
Update .dtsi files when required

In a first step, allowed sources are limited to already supported
clocks: LSI/LSE/HSI/HSE/MSI/PCLK/PLLCLK/SYSCLK
Support for other clocks such as HSI48, SAIXCLK, ... is left for a
next step.

Signed-off-by: Erwan Gouriou <[email protected]>
STM32WL_DUAL_CORE and RCC_CALC_MSI_RUN_FREQ are not used anymore.
Clean up those definitions

Signed-off-by: Erwan Gouriou <[email protected]>
Similarly to what was done on U5 and H7 clock_control drivers, enable
device clock source selection.
This is done by:
-providing implementation for clock_control_configure().
-updating clock_control_get_rate() to support various possible clock
sources (SYSCLK, PLLCLK, LSE, LSI, HSI, HSE).
-providing enable_clock() to verify requested clock source exists and
is enabled.
-adding LSI and LSE device tree based initialization to
set_up_fixed_clock_sources().

Signed-off-by: Erwan Gouriou <[email protected]>
Move stm32_common tests to stm32_common_core before adding new folder
for device source selection tests.

Signed-off-by: Erwan Gouriou <[email protected]>
Add a test section to enable device clock source selection testing.
Test targets I2C1 device which supports clock source selection
on all SOCs using this driver except L1
Initial test done on wb target.

Signed-off-by: Erwan Gouriou <[email protected]>
…etting

Since implementation of clock source selection in consumer device drivers
could be achieved without usage of a clock-names property and no
example of usage is provided up to now, remove this property from existing
examples.
Additionally, make it clear in stm32 clock control binding that it is
driver's responsibility to correctly access clock source information
and use it as required.

Signed-off-by: Erwan Gouriou <[email protected]>
Follow up of what was done in main branch during this development.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango
Copy link
Member Author

erwango commented May 10, 2022

@gmarull Thanks for the review. I've taken into account the comments and pushed once more. Agreed on the pinctrl like macro, but I prefer to introduce it in a second step.

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

I haven't gone into implementation details, but the overall approach looks reasonable to me. Improvements can be made later, e.g. clocks definition.

@erwango erwango added this to the v3.1.0 milestone May 10, 2022
@carlescufi carlescufi merged commit b520211 into zephyrproject-rtos:main May 10, 2022
MaureenHelm pushed a commit that referenced this pull request May 11, 2022
Add support for an alternate clock. If available,
alternate clock is enabled and used to get the
device clock rate.
Based on: #45053.

Signed-off-by: Artur Lipowski <[email protected]>
laxiLang pushed a commit to laxiLang/zephyr that referenced this pull request May 30, 2022
Add support for an alternate clock. If available,
alternate clock is enabled and used to get the
device clock rate.
Based on: zephyrproject-rtos#45053.

Signed-off-by: Artur Lipowski <[email protected]>
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: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: SPI SPI bus platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

STM32H7 SPI123 incorrect clock source used for prescaler calculation
6 participants