[PATCH 1/2] clk: clk_core_get() can also return NULL

[PATCH 1/2] clk: clk_core_get() can also return NULL

[Stephen Boyd]

Nothing stops a clk controller from registering an OF clk provider
before registering those clks with the clk framework. This is not great
but we deal with it in the clk framework by refusing to hand out struct
clk pointers when 'hw->core' is NULL, the indication that clk_register()
has been called.

Within clk_core_fill_parent_index() we considered this case when a
clk_hw pointer is referenced directly by filling in the parent cache
with an -EPROBE_DEFER pointer when the core pointer is NULL. When we
lookup a parent with clk_core_get() we don't care about the return value
being NULL though, because that was considered largely impossible, but
it's been proven now that it can be NULL if two clk providers are
probing in parallel and the parent provider has been registered before
the clk has. Let's check for NULL here as well and treat it the same as
direct clk_hw references.

Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f467d63bbf1e..add86a4b8e8c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -424,19 +424,20 @@ static void clk_core_fill_parent_index(struct clk_core *core, u8 index)
 
 	if (entry->hw) {
 		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)
-			parent = ERR_PTR(-EPROBE_DEFER);
 	} else {
 		parent = clk_core_get(core, index);
 		if (PTR_ERR(parent) == -ENOENT && entry->name)
 			parent = clk_core_lookup(entry->name);
 	}
 
+	/*
+	 * 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)
+		parent = ERR_PTR(-EPROBE_DEFER);
+
 	/* Only cache it if it's not an error */
 	if (!IS_ERR(parent))
 		entry->core = parent;

base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git

[PATCH 2/2] clk: __clk_core_init() never takes NULL

[Stephen Boyd]

The only caller of __clk_core_init() allocates the pointer and checks
the allocation for NULL so this check is impossible. Remove it.

Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index add86a4b8e8c..d9414a7d585b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3414,9 +3414,6 @@ static int __clk_core_init(struct clk_core *core)
 	unsigned long rate;
 	int phase;
 
-	if (!core)
-		return -EINVAL;
-
 	clk_prepare_lock();
 
 	ret = clk_pm_runtime_get(core);
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git

Re: [PATCH 2/2] clk: __clk_core_init() never takes NULL

[Stephen Boyd]

Quoting Stephen Boyd (2021-12-07 20:15:34)
> The only caller of __clk_core_init() allocates the pointer and checks
> the allocation for NULL so this check is impossible. Remove it.
> 
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

Re: [PATCH 1/2] clk: clk_core_get() can also return NULL

[Stephen Boyd]

Quoting Stephen Boyd (2021-12-07 20:15:33)
> Nothing stops a clk controller from registering an OF clk provider
> before registering those clks with the clk framework. This is not great
> but we deal with it in the clk framework by refusing to hand out struct
> clk pointers when 'hw->core' is NULL, the indication that clk_register()
> has been called.
> 
> Within clk_core_fill_parent_index() we considered this case when a
> clk_hw pointer is referenced directly by filling in the parent cache
> with an -EPROBE_DEFER pointer when the core pointer is NULL. When we
> lookup a parent with clk_core_get() we don't care about the return value
> being NULL though, because that was considered largely impossible, but
> it's been proven now that it can be NULL if two clk providers are
> probing in parallel and the parent provider has been registered before
> the clk has. Let's check for NULL here as well and treat it the same as
> direct clk_hw references.
> 
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next