public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
@ 2026-02-03  0:24 Stephen Boyd
  2026-02-03  2:57 ` Peng Fan
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Stephen Boyd @ 2026-02-03  0:24 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, patches, Alexander Stein, Mark Brown,
	Nicolas Frattaroli, Brian Masney, AngeloGioacchino Del Regno,
	Chen-Yu Tsai

This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
It's been shown to cause problems on i.MX and STM32 platforms
where the board doesn't boot. In one case, a clk with
CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
causing the parent to be enabled, the rate recalculated, and then
the parent is disabled causing the critical clk being registered
to stop clocking.

A fix for that would be to calculate the rate of the clk after
enabling the critical clk itself, but that wouldn't fix another
problem where a clk with CLK_OPS_PARENT_ENABLE is registered
before the parent is registered. In this case the hardware access
in the clk_ops::recalc_rate() function would fail if the parent
is disabled.

There are even more problems exposed by this patch because it
introduces logic that disables clks earlier in system boot than
has existed previously. Historically we've not disabled clks
until late init (clk_disable_unused) under the assumption that
clks have been registered enough to have a consistent view of the
clk tree. The clk_disable_unused logic doesn't work very well
though, leading to quite a few devices booting with
clk_ignore_unused on the kernel command line.

Long story short, disabling clks during clk registration is full
of pitfalls. Revert this commit until a proper solution can be
found.

Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/r/036da7ce-6487-4a6e-9b15-97c6d3bcdcec@sirena.org.uk
Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Cc: Brian Masney <bmasney@redhat.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Chen-Yu Tsai <wenst@chromium.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 1b0f9d567f48..85d2f2481acf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1921,14 +1921,7 @@ static unsigned long clk_recalc(struct clk_core *core,
 	unsigned long rate = parent_rate;
 
 	if (core->ops->recalc_rate && !clk_pm_runtime_get(core)) {
-		if (core->flags & CLK_OPS_PARENT_ENABLE)
-			clk_core_prepare_enable(core->parent);
-
 		rate = core->ops->recalc_rate(core->hw, parent_rate);
-
-		if (core->flags & CLK_OPS_PARENT_ENABLE)
-			clk_core_disable_unprepare(core->parent);
-
 		clk_pm_runtime_put(core);
 	}
 	return rate;
@@ -4038,9 +4031,6 @@ static int __clk_core_init(struct clk_core *core)
 	 */
 	clk_core_update_duty_cycle_nolock(core);
 
-	if (core->flags & CLK_OPS_PARENT_ENABLE)
-		clk_core_prepare_enable(core->parent);
-
 	/*
 	 * Set clk's rate.  The preferred method is to use .recalc_rate.  For
 	 * simple clocks and lazy developers the default fallback is to use the
@@ -4056,9 +4046,6 @@ static int __clk_core_init(struct clk_core *core)
 		rate = 0;
 	core->rate = core->req_rate = rate;
 
-	if (core->flags & CLK_OPS_PARENT_ENABLE)
-		clk_core_disable_unprepare(core->parent);
-
 	/*
 	 * Enable CLK_IS_CRITICAL clocks so newly added critical clocks
 	 * don't get accidentally disabled when walking the orphan tree and

base-commit: aa2ad19210a6a444111bce55e8b69579f29318fb
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
  2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
@ 2026-02-03  2:57 ` Peng Fan
  2026-02-03  7:32 ` Alexander Stein
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Peng Fan @ 2026-02-03  2:57 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, patches,
	Alexander Stein, Mark Brown, Nicolas Frattaroli, Brian Masney,
	AngeloGioacchino Del Regno, Chen-Yu Tsai

On Mon, Feb 02, 2026 at 04:24:38PM -0800, Stephen Boyd wrote:
>This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
>It's been shown to cause problems on i.MX and STM32 platforms
>where the board doesn't boot. In one case, a clk with
>CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
>causing the parent to be enabled, the rate recalculated, and then
>the parent is disabled causing the critical clk being registered
>to stop clocking.
>
>A fix for that would be to calculate the rate of the clk after
>enabling the critical clk itself, but that wouldn't fix another
>problem where a clk with CLK_OPS_PARENT_ENABLE is registered
>before the parent is registered. In this case the hardware access
>in the clk_ops::recalc_rate() function would fail if the parent
>is disabled.
>
>There are even more problems exposed by this patch because it
>introduces logic that disables clks earlier in system boot than
>has existed previously. Historically we've not disabled clks
>until late init (clk_disable_unused) under the assumption that
>clks have been registered enough to have a consistent view of the
>clk tree. The clk_disable_unused logic doesn't work very well
>though, leading to quite a few devices booting with
>clk_ignore_unused on the kernel command line.
>
>Long story short, disabling clks during clk registration is full
>of pitfalls. Revert this commit until a proper solution can be
>found.
>
>Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w
>Reported-by: Mark Brown <broonie@kernel.org>
>Closes: https://lore.kernel.org/r/036da7ce-6487-4a6e-9b15-97c6d3bcdcec@sirena.org.uk
>Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>Cc: Brian Masney <bmasney@redhat.com>
>Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>Cc: Chen-Yu Tsai <wenst@chromium.org>
>Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Tested-by: Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
  2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
  2026-02-03  2:57 ` Peng Fan
@ 2026-02-03  7:32 ` Alexander Stein
  2026-02-03 15:15 ` Brian Masney
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Alexander Stein @ 2026-02-03  7:32 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Stephen Boyd
  Cc: linux-kernel, linux-clk, patches, Mark Brown, Nicolas Frattaroli,
	Brian Masney, AngeloGioacchino Del Regno, Chen-Yu Tsai

Am Dienstag, 3. Februar 2026, 01:24:38 CET schrieb Stephen Boyd:
> This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
> It's been shown to cause problems on i.MX and STM32 platforms
> where the board doesn't boot. In one case, a clk with
> CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
> causing the parent to be enabled, the rate recalculated, and then
> the parent is disabled causing the critical clk being registered
> to stop clocking.
> 
> A fix for that would be to calculate the rate of the clk after
> enabling the critical clk itself, but that wouldn't fix another
> problem where a clk with CLK_OPS_PARENT_ENABLE is registered
> before the parent is registered. In this case the hardware access
> in the clk_ops::recalc_rate() function would fail if the parent
> is disabled.
> 
> There are even more problems exposed by this patch because it
> introduces logic that disables clks earlier in system boot than
> has existed previously. Historically we've not disabled clks
> until late init (clk_disable_unused) under the assumption that
> clks have been registered enough to have a consistent view of the
> clk tree. The clk_disable_unused logic doesn't work very well
> though, leading to quite a few devices booting with
> clk_ignore_unused on the kernel command line.
> 
> Long story short, disabling clks during clk registration is full
> of pitfalls. Revert this commit until a proper solution can be
> found.
> 
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/r/036da7ce-6487-4a6e-9b15-97c6d3bcdcec@sirena.org.uk
> Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
>  drivers/clk/clk.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 1b0f9d567f48..85d2f2481acf 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1921,14 +1921,7 @@ static unsigned long clk_recalc(struct clk_core *core,
>  	unsigned long rate = parent_rate;
>  
>  	if (core->ops->recalc_rate && !clk_pm_runtime_get(core)) {
> -		if (core->flags & CLK_OPS_PARENT_ENABLE)
> -			clk_core_prepare_enable(core->parent);
> -
>  		rate = core->ops->recalc_rate(core->hw, parent_rate);
> -
> -		if (core->flags & CLK_OPS_PARENT_ENABLE)
> -			clk_core_disable_unprepare(core->parent);
> -
>  		clk_pm_runtime_put(core);
>  	}
>  	return rate;
> @@ -4038,9 +4031,6 @@ static int __clk_core_init(struct clk_core *core)
>  	 */
>  	clk_core_update_duty_cycle_nolock(core);
>  
> -	if (core->flags & CLK_OPS_PARENT_ENABLE)
> -		clk_core_prepare_enable(core->parent);
> -
>  	/*
>  	 * Set clk's rate.  The preferred method is to use .recalc_rate.  For
>  	 * simple clocks and lazy developers the default fallback is to use the
> @@ -4056,9 +4046,6 @@ static int __clk_core_init(struct clk_core *core)
>  		rate = 0;
>  	core->rate = core->req_rate = rate;
>  
> -	if (core->flags & CLK_OPS_PARENT_ENABLE)
> -		clk_core_disable_unprepare(core->parent);
> -
>  	/*
>  	 * Enable CLK_IS_CRITICAL clocks so newly added critical clocks
>  	 * don't get accidentally disabled when walking the orphan tree and
> 
> base-commit: aa2ad19210a6a444111bce55e8b69579f29318fb
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
  2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
  2026-02-03  2:57 ` Peng Fan
  2026-02-03  7:32 ` Alexander Stein
@ 2026-02-03 15:15 ` Brian Masney
  2026-02-03 17:43 ` Mark Brown
  2026-02-05 11:43 ` AngeloGioacchino Del Regno
  4 siblings, 0 replies; 6+ messages in thread
From: Brian Masney @ 2026-02-03 15:15 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, patches,
	Alexander Stein, Mark Brown, Nicolas Frattaroli,
	AngeloGioacchino Del Regno, Chen-Yu Tsai

On Mon, Feb 02, 2026 at 04:24:38PM -0800, Stephen Boyd wrote:
> This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
> It's been shown to cause problems on i.MX and STM32 platforms
> where the board doesn't boot. In one case, a clk with
> CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
> causing the parent to be enabled, the rate recalculated, and then
> the parent is disabled causing the critical clk being registered
> to stop clocking.
> 
> A fix for that would be to calculate the rate of the clk after
> enabling the critical clk itself, but that wouldn't fix another
> problem where a clk with CLK_OPS_PARENT_ENABLE is registered
> before the parent is registered. In this case the hardware access
> in the clk_ops::recalc_rate() function would fail if the parent
> is disabled.
> 
> There are even more problems exposed by this patch because it
> introduces logic that disables clks earlier in system boot than
> has existed previously. Historically we've not disabled clks
> until late init (clk_disable_unused) under the assumption that
> clks have been registered enough to have a consistent view of the
> clk tree. The clk_disable_unused logic doesn't work very well
> though, leading to quite a few devices booting with
> clk_ignore_unused on the kernel command line.
> 
> Long story short, disabling clks during clk registration is full
> of pitfalls. Revert this commit until a proper solution can be
> found.
> 
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/r/036da7ce-6487-4a6e-9b15-97c6d3bcdcec@sirena.org.uk
> Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Reviewed-by: Brian Masney <bmasney@redhat.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
  2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
                   ` (2 preceding siblings ...)
  2026-02-03 15:15 ` Brian Masney
@ 2026-02-03 17:43 ` Mark Brown
  2026-02-05 11:43 ` AngeloGioacchino Del Regno
  4 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2026-02-03 17:43 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, patches,
	Alexander Stein, Nicolas Frattaroli, Brian Masney,
	AngeloGioacchino Del Regno, Chen-Yu Tsai

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Mon, Feb 02, 2026 at 04:24:38PM -0800, Stephen Boyd wrote:
> This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
> It's been shown to cause problems on i.MX and STM32 platforms
> where the board doesn't boot. In one case, a clk with
> CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
> causing the parent to be enabled, the rate recalculated, and then
> the parent is disabled causing the critical clk being registered
> to stop clocking.

Tested-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc"
  2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
                   ` (3 preceding siblings ...)
  2026-02-03 17:43 ` Mark Brown
@ 2026-02-05 11:43 ` AngeloGioacchino Del Regno
  4 siblings, 0 replies; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-02-05 11:43 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: linux-kernel, linux-clk, patches, Alexander Stein, Mark Brown,
	Nicolas Frattaroli, Brian Masney, Chen-Yu Tsai

Il 03/02/26 01:24, Stephen Boyd ha scritto:
> This reverts commit 669917676e93fca5ea3c66fc9539830312bec58e.
> It's been shown to cause problems on i.MX and STM32 platforms
> where the board doesn't boot. In one case, a clk with
> CLK_IS_CRITICAL and CLK_OPS_PARENT_ENABLE is being registered
> causing the parent to be enabled, the rate recalculated, and then
> the parent is disabled causing the critical clk being registered
> to stop clocking.
> 
> A fix for that would be to calculate the rate of the clk after
> enabling the critical clk itself, but that wouldn't fix another
> problem where a clk with CLK_OPS_PARENT_ENABLE is registered
> before the parent is registered. In this case the hardware access
> in the clk_ops::recalc_rate() function would fail if the parent
> is disabled.
> 
> There are even more problems exposed by this patch because it
> introduces logic that disables clks earlier in system boot than
> has existed previously. Historically we've not disabled clks
> until late init (clk_disable_unused) under the assumption that
> clks have been registered enough to have a consistent view of the
> clk tree. The clk_disable_unused logic doesn't work very well
> though, leading to quite a few devices booting with
> clk_ignore_unused on the kernel command line.
> 
> Long story short, disabling clks during clk registration is full
> of pitfalls. Revert this commit until a proper solution can be
> found.
> 
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/r/036da7ce-6487-4a6e-9b15-97c6d3bcdcec@sirena.org.uk
> Cc: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Makes sense, and I agree.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-05 11:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-03  0:24 [PATCH] Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" Stephen Boyd
2026-02-03  2:57 ` Peng Fan
2026-02-03  7:32 ` Alexander Stein
2026-02-03 15:15 ` Brian Masney
2026-02-03 17:43 ` Mark Brown
2026-02-05 11:43 ` AngeloGioacchino Del Regno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox