Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec5c00be authored by Mike Tipton's avatar Mike Tipton Committed by Greg Kroah-Hartman
Browse files

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



[ Upstream commit 54baf56eaa40aa5cdcd02b3c20d593e4e1211220 ]

Before commit fc0c209c ("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: fc0c209c ("clk: Allow parents to be specified without string names")
Signed-off-by: default avatarMike 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: default avatarStephen Boyd <sboyd@kernel.org>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent f83ed203
Loading
Loading
Loading
Loading
+12 −3
Original line number Diff line number Diff line
@@ -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)