Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

clk: Don't parent clks until the parent is fully registered

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>

authored by

Mike Tipton and committed by
Stephen Boyd
54baf56e 2d4fcc5a

+12 -3
+12 -3
drivers/clk/clk.c
··· 3418 3418 3419 3419 clk_prepare_lock(); 3420 3420 3421 + /* 3422 + * Set hw->core after grabbing the prepare_lock to synchronize with 3423 + * callers of clk_core_fill_parent_index() where we treat hw->core 3424 + * being NULL as the clk not being registered yet. This is crucial so 3425 + * that clks aren't parented until their parent is fully registered. 3426 + */ 3427 + core->hw->core = core; 3428 + 3421 3429 ret = clk_pm_runtime_get(core); 3422 3430 if (ret) 3423 3431 goto unlock; ··· 3590 3582 out: 3591 3583 clk_pm_runtime_put(core); 3592 3584 unlock: 3593 - if (ret) 3585 + if (ret) { 3594 3586 hlist_del_init(&core->child_node); 3587 + core->hw->core = NULL; 3588 + } 3595 3589 3596 3590 clk_prepare_unlock(); 3597 3591 ··· 3857 3847 core->num_parents = init->num_parents; 3858 3848 core->min_rate = 0; 3859 3849 core->max_rate = ULONG_MAX; 3860 - hw->core = core; 3861 3850 3862 3851 ret = clk_core_populate_parent_map(core, init); 3863 3852 if (ret) ··· 3874 3865 goto fail_create_clk; 3875 3866 } 3876 3867 3877 - clk_core_link_consumer(hw->core, hw->clk); 3868 + clk_core_link_consumer(core, hw->clk); 3878 3869 3879 3870 ret = __clk_core_init(core); 3880 3871 if (!ret)