From: Mike Tipton quic_mdtipton@quicinc.com
[ Upstream commit 54baf56eaa40aa5cdcd02b3c20d593e4e1211220 ]
Before commit fc0c209c147f ("clk: Allow parents to be specified without string names") child clks couldn't find their parent until the parent clk was added to a list in __clk_core_init(). After that commit, child clks can reference their parent clks directly via a clk_hw pointer, or they can lookup that clk_hw pointer via DT if the parent clk is registered with an OF clk provider.
The common clk framework treats hw->core being non-NULL as "the clk is registered" per the logic within clk_core_fill_parent_index():
parent = entry->hw->core; /* * We have a direct reference but it isn't registered yet? * Orphan it and let clk_reparent() update the orphan status * when the parent is registered. */ if (!parent)
Therefore we need to be extra careful to not set hw->core until the clk is fully registered with the clk framework. Otherwise we can get into a situation where a child finds a parent clk and we move the child clk off the orphan list when the parent isn't actually registered, wrecking our enable accounting and breaking critical clks.
Consider the following scenario:
CPU0 CPU1 ---- ---- struct clk_hw clkBad; struct clk_hw clkA;
clkA.init.parent_hws = { &clkBad };
clk_hw_register(&clkA) clk_hw_register(&clkBad) ... __clk_register() hw->core = core ... __clk_register() __clk_core_init() clk_prepare_lock() __clk_init_parent() clk_core_get_parent_by_index() clk_core_fill_parent_index() if (entry->hw) { parent = entry->hw->core;
At this point, 'parent' points to clkBad even though clkBad hasn't been fully registered yet. Ouch! A similar problem can happen if a clk controller registers orphan clks that are referenced in the DT node of another clk controller.
Let's fix all this by only setting the hw->core pointer underneath the clk prepare lock in __clk_core_init(). This way we know that clk_core_fill_parent_index() can't see hw->core be non-NULL until the clk is fully registered.
Fixes: fc0c209c147f ("clk: Allow parents to be specified without string names") Signed-off-by: Mike Tipton quic_mdtipton@quicinc.com Link: https://lore.kernel.org/r/20211109043438.4639-1-quic_mdtipton@quicinc.com [sboyd@kernel.org: Reword commit text, update comment] Signed-off-by: Stephen Boyd sboyd@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/clk/clk.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 6ff87cd867121..e4e1b4e94a67b 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3299,6 +3299,14 @@ static int __clk_core_init(struct clk_core *core)
clk_prepare_lock();
+ /* + * Set hw->core after grabbing the prepare_lock to synchronize with + * callers of clk_core_fill_parent_index() where we treat hw->core + * being NULL as the clk not being registered yet. This is crucial so + * that clks aren't parented until their parent is fully registered. + */ + core->hw->core = core; + ret = clk_pm_runtime_get(core); if (ret) goto unlock; @@ -3452,8 +3460,10 @@ static int __clk_core_init(struct clk_core *core) out: clk_pm_runtime_put(core); unlock: - if (ret) + if (ret) { hlist_del_init(&core->child_node); + core->hw->core = NULL; + }
clk_prepare_unlock();
@@ -3699,7 +3709,6 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) core->num_parents = init->num_parents; core->min_rate = 0; core->max_rate = ULONG_MAX; - hw->core = core;
ret = clk_core_populate_parent_map(core, init); if (ret) @@ -3717,7 +3726,7 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) goto fail_create_clk; }
- clk_core_link_consumer(hw->core, hw->clk); + clk_core_link_consumer(core, hw->clk);
ret = __clk_core_init(core); if (!ret)