diff options
author | Maxime Ripard <maxime@cerno.tech> | 2022-08-16 13:25:18 +0200 |
---|---|---|
committer | Stephen Boyd <sboyd@kernel.org> | 2022-09-15 09:31:09 -0700 |
commit | cb1b1dd96241f37ea41d241946d5153c48141cd5 (patch) | |
tree | a13346389da096ba710c88177b752a416bdd9e2f /drivers/clk/clk.c | |
parent | 3afb07231d603d51dca6a5d5e16d9d8f422f9b5f (diff) |
clk: Set req_rate on reparenting
If a non-rate clock started by default with a parent that never
registered, core->req_rate will be 0. The expectation is that whenever
the parent will be registered, req_rate will be updated with the new
value that has just been computed.
However, if that clock is a mux, clk_set_parent() can also make that
clock no longer orphan. In this case however, we never update req_rate.
The natural solution to this would be to update core->rate and
core->req_rate in clk_reparent() by calling clk_recalc().
However, this doesn't work in all cases. Indeed, clk_recalc() is called
by __clk_set_parent_before(), __clk_set_parent() and
clk_core_reparent(). Both __clk_set_parent_before() and __clk_set_parent
will call clk_recalc() with the enable_lock taken through a call to
clk_enable_lock(), the underlying locking primitive being a spinlock.
clk_recalc() calls the backing driver .recalc_rate hook, and that
implementation might sleep if the underlying device uses a bus with
accesses that might sleep, such as i2c.
In such a situation, we would end up sleeping while holding a spinlock,
and thus in an atomic section.
In order to work around this, we can move the core->rate and
core->req_rate update to the clk_recalc() calling sites, after the
enable_lock has been released if it was taken.
The only situation that could still be problematic is the
clk_core_reparent() -> clk_reparent() case that doesn't have any
locking. clk_core_reparent() is itself called by clk_hw_reparent(),
which is then called by 4 drivers:
* clk-stm32mp1.c, stm32/clk-stm32-core.c and tegra/clk-tegra210-emc.c
use it in their set_parent implementation. The set_parent hook is
only called by __clk_set_parent() and clk_change_rate(), both of
them calling it without the enable_lock taken.
* clk/tegra/clk-tegra124-emc.c calls it as part of its set_rate
implementation. set_rate is only called by clk_change_rate(), again
without the enable_lock taken.
In both cases we can't end up in a situation where the clk_hw_reparent()
caller would hold a spinlock, so it seems like this is a good
workaround.
Let's also add some unit tests to make sure we cover the original bug.
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220816112530.1837489-14-maxime@cerno.tech
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Diffstat (limited to 'drivers/clk/clk.c')
-rw-r--r-- | drivers/clk/clk.c | 22 |
1 files changed, 22 insertions, 0 deletions
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 53b28e63deae..91bb1ea0e147 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1765,6 +1765,23 @@ static void clk_core_update_orphan_status(struct clk_core *core, bool is_orphan) clk_core_update_orphan_status(child, is_orphan); } +/* + * Update the orphan rate and req_rate of @core and all its children. + */ +static void clk_core_update_orphan_child_rates(struct clk_core *core) +{ + struct clk_core *child; + unsigned long parent_rate = 0; + + if (core->parent) + parent_rate = core->parent->rate; + + core->rate = core->req_rate = clk_recalc(core, parent_rate); + + hlist_for_each_entry(child, &core->children, child_node) + clk_core_update_orphan_child_rates(child); +} + static void clk_reparent(struct clk_core *core, struct clk_core *new_parent) { bool was_orphan = core->orphan; @@ -1834,6 +1851,8 @@ static struct clk_core *__clk_set_parent_before(struct clk_core *core, clk_reparent(core, parent); clk_enable_unlock(flags); + clk_core_update_orphan_child_rates(core); + return old_parent; } @@ -1878,6 +1897,8 @@ static int __clk_set_parent(struct clk_core *core, struct clk_core *parent, flags = clk_enable_lock(); clk_reparent(core, old_parent); clk_enable_unlock(flags); + + clk_core_update_orphan_child_rates(core); __clk_set_parent_after(core, old_parent, parent); return ret; @@ -2506,6 +2527,7 @@ static void clk_core_reparent(struct clk_core *core, struct clk_core *new_parent) { clk_reparent(core, new_parent); + clk_core_update_orphan_child_rates(core); __clk_recalc_accuracies(core); __clk_recalc_rates(core, POST_RATE_CHANGE); } |