Unverified Commit 448b50b5 authored by Stephen Boyd's avatar Stephen Boyd
Browse files

Revert "clk: Respect CLK_OPS_PARENT_ENABLE during recalc" 



This reverts commit 66991767.
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: default avatarAlexander Stein <alexander.stein@ew.tq-group.com>
Closes: https://lore.kernel.org/r/6239343.lOV4Wx5bFT@steina-w


Reported-by: default avatarMark 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>
Tested-by: default avatarPeng Fan <peng.fan@nxp.com>
Tested-by: default avatarAlexander Stein <alexander.stein@ew.tq-group.com>
Reviewed-by: default avatarBrian Masney <bmasney@redhat.com>
Tested-by: default avatarMark Brown <broonie@kernel.org>
Signed-off-by: default avatarStephen Boyd <sboyd@kernel.org>
parent aa2ad192

drivers/clk/clk.c

+0 −13
Original line number Diff line number Diff line
@@ -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