stm32 clock mux for CK48 definition #96353

FRASTM · 2025-09-22T09:02:52Z

Define the STM32_CK48_ENABLED especially for the stm32F4 series

etienne-lms · 2025-10-01T09:26:57Z

...s/clock_control/stm32_clock_configuration/stm32_common_devices/boards/f4_sdmmc48_pll.overlay

+ &pll {
+	div-m = <4>;
+	mul-n = <192>;
+	div-p = <4>;
+	div-q = <8>;
+	clocks = <&clk_hse>;
+	status = "okay";
+};
+
+&rcc {
+	clocks = <&pll>;
+	clock-frequency = <DT_FREQ_M(96)>;
+	ahb-prescaler = <1>;
+	apb1-prescaler = <2>;
+	apb2-prescaler = <1>;
+};


Intentional change?
Would it be worth to add an alternate test with its own DTS overlay file?

yes, when the sdmmc is clocked by the PLL_q, a freq = 48MHz is required.
This overlay file "f4_sdmmc48_pll.overlay"is for running this testcase by un-commenting.
The alternate test is "drivers.clock.stm32_clock_configuration.common_device.f4.sdmmc_48".
Two possible config are in this overlay (selection is by commenting/un-commenting). Is there any reason today to change with 2 different testcases/overlay files, if it was your question ?

Ok, fine. Already having a test for SDMMC over CK48 seems enough.
I just found strange this added inline comments in this commit.

etienne-lms

LGTM.

Maybe the commit message could be more verbose on why the change, e.g.:

 Define the STM32_CK48_ENABLED especially for the stm32F4 series
+when ck48 node is enabled to leverage its already implemented support.

etienne-lms · 2025-10-09T14:20:28Z

...s/clock_control/stm32_clock_configuration/stm32_common_devices/boards/f4_sdmmc48_pll.overlay

+ &pll {
+	div-m = <4>;
+	mul-n = <192>;
+	div-p = <4>;
+	div-q = <8>;
+	clocks = <&clk_hse>;
+	status = "okay";
+};
+
+&rcc {
+	clocks = <&pll>;
+	clock-frequency = <DT_FREQ_M(96)>;
+	ahb-prescaler = <1>;
+	apb1-prescaler = <2>;
+	apb2-prescaler = <1>;
+};


Ok, fine. Already having a test for SDMMC over CK48 seems enough.
I just found strange this added inline comments in this commit.

erwango

Nack until #94934 (comment) is fixed

FRASTM · 2025-10-13T07:57:30Z

Add a commit to enable the clk48

Define the STM32_CK48_ENABLED especially for the stm32F4 series when ck48 node is enabled to leverage its already implemented support. Signed-off-by: Francois Ramu <[email protected]>

FRASTM · 2025-10-14T06:18:45Z

LGTM.

Maybe the commit message could be more verbose on why the change, e.g.:
 Define the STM32_CK48_ENABLED especially for the stm32F4 series
+when ck48 node is enabled to leverage its already implemented support.

comment updated

etienne-lms

Comment below is not blocking.

etienne-lms · 2025-10-14T06:44:14Z

drivers/clock_control/clock_stm32_ll_common.c

 		}
 		break;
 #endif /* STM32_SRC_MSI */
+#if defined(STM32_CK48_ENABLED)


Implementation looks good to me, but for consistency one may prefer here:

Suggested change

#if defined(STM32_CK48_ENABLED)

#if defined(STM32_SRC_CK48)

unless what it seems this could be simplified to:

#if defined(STM32_CK48_ENABLED) case STM32_SRC_CK48: break; #endif

That said, I think it's rather the other case that could simplified:

int enabled_clock(uint32_t src_clk) { int r = 0; switch (src_clk) { /* Supported source clocks */ case STM32_SRC_SYSCLK: #if defined(STM32_SRC_PCLK) case STM32_SRC_PCLK: #endif #if IS_ENABLED(STM32_HSE_ENABLED) case STM32_SRC_HSE: #endif #if defined(STM32_SRC_EXT_HSE) /* EXT_HSE is the raw OSC_IN signal, so it is always available * regardless of the clocks configuration. */ case STM32_SRC_EXT_HSE: #endif #if IS_ENABLED(STM32_HSI_ENABLED) case STM32_SRC_HSI: #endif #if IS_ENABLED(STM32_LSE_ENABLED) case STM32_SRC_LSE: #endif #if IS_ENABLED(STM32_LSI_ENABLED) case STM32_SRC_LSI: #endif #if IS_ENABLED(STM32_HSI14_ENABLED) case STM32_SRC_HSI14: #endif #if IS_ENABLED(STM32_HSI48_ENABLED) case STM32_SRC_HSI48: #endif (...) #if defined(STM32_SRC_TIMPLLCLK) case STM32_SRC_TIMPLLCLK: #endif return 0; /* All not listed sources above are implicitly not supported */ default: return -ENOTSUP; } }

changed to #if defined(STM32_SRC_CK48)

Other suggestions are probably out of this PR/issue

Other suggestions are probably out of this PR/issue

Indeed.

This commit enables the clk48 clock mux if STM32_CK48_ENABLED is set by the device tree. Signed-off-by: Francois Ramu <[email protected]>

sonarqubecloud · 2025-10-14T07:21:29Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

etienne-lms · 2025-10-14T10:08:45Z

@FRASTM, I think you need to rebase to get the CI tests to all succeed.

FRASTM changed the title ~~stminclude: drivers: stm32 clock mux CK48 definition~~ stm32 clock mux for CK48 definition Sep 22, 2025

FRASTM force-pushed the issue94934 branch 2 times, most recently from d4d97b6 to 6c38115 Compare September 22, 2025 15:44

FRASTM marked this pull request as ready for review September 23, 2025 06:59

zephyrbot added the area: Clock Control label Sep 23, 2025

zephyrbot requested a review from nordic-krch September 23, 2025 07:01

zephyrbot assigned nordic-krch Sep 23, 2025

FRASTM requested review from djiatsaf-st, erwango, etienne-lms, gautierg-st and mathieuchopstm October 1, 2025 08:17

etienne-lms reviewed Oct 1, 2025

View reviewed changes

FRASTM added the platform: STM32 ST Micro STM32 label Oct 8, 2025

mathieuchopstm previously approved these changes Oct 9, 2025

View reviewed changes

etienne-lms previously approved these changes Oct 9, 2025

View reviewed changes

erwango mentioned this pull request Oct 10, 2025

stm42f412 "Failed to enable SDMMC domain clock" #94934

Open

1 task

erwango requested changes Oct 13, 2025

View reviewed changes

erwango assigned erwango and unassigned nordic-krch Oct 13, 2025

FRASTM dismissed stale reviews from etienne-lms and mathieuchopstm via efda26e October 13, 2025 07:56

zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 13, 2025

zephyrbot requested review from GeorgeCGV, marwaiehm-st and nashif October 13, 2025 07:58

FRASTM force-pushed the issue94934 branch from efda26e to d08a2c8 Compare October 13, 2025 09:12

FRASTM added this to the 4.3 milestone Oct 13, 2025

include: drivers: stm32 clock mux CK48 definition

b3dde1a

Define the STM32_CK48_ENABLED especially for the stm32F4 series when ck48 node is enabled to leverage its already implemented support. Signed-off-by: Francois Ramu <[email protected]>

FRASTM force-pushed the issue94934 branch from d08a2c8 to 0e1a5d6 Compare October 13, 2025 14:46

mathieuchopstm mentioned this pull request Oct 13, 2025

boards: st: stm32u83c_dk: Add USB support #97486

Open

etienne-lms previously approved these changes Oct 14, 2025

View reviewed changes

drivers: clock control: stm32 common clock enables clk48

d9708c8

This commit enables the clk48 clock mux if STM32_CK48_ENABLED is set by the device tree. Signed-off-by: Francois Ramu <[email protected]>

FRASTM dismissed etienne-lms’s stale review via d9708c8 October 14, 2025 06:55

FRASTM force-pushed the issue94934 branch from 0e1a5d6 to d9708c8 Compare October 14, 2025 06:55

mathieuchopstm modified the milestones: 4.3, v4.3.0 Oct 14, 2025

stm32 clock mux for CK48 definition #96353

Are you sure you want to change the base?

stm32 clock mux for CK48 definition #96353

Conversation

FRASTM commented Sep 22, 2025

Uh oh!

etienne-lms Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

FRASTM Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etienne-lms Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

etienne-lms Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

FRASTM commented Oct 13, 2025

Uh oh!

FRASTM commented Oct 14, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

etienne-lms Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

FRASTM Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

FRASTM Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

etienne-lms Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 14, 2025

Quality Gate passed

Uh oh!

etienne-lms commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

FRASTM Oct 1, 2025 •

edited

Loading