lkml.org 
[lkml]   [2021]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 28/99] clk: Dont parent clks until the parent is fully registered
    Date
    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 61c78714c0957..515ef39c4610c 100644
    --- a/drivers/clk/clk.c
    +++ b/drivers/clk/clk.c
    @@ -3389,6 +3389,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;
    @@ -3557,8 +3565,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();

    @@ -3804,7 +3814,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)
    @@ -3822,7 +3831,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)
    --
    2.33.0


    \
     
     \ /
      Last update: 2021-12-20 15:57    [W:4.840 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site