[PATCH] clk: s2mps11: Propagate OF provider registration failures

[PATCH] clk: s2mps11: Propagate OF provider registration failures

[Pengpeng Hou]

s2mps11_clk_probe() ignores of_clk_add_hw_provider() failures, stores
driver data and returns success even though DT consumers cannot acquire the
registered clocks.

Return the provider error through the existing lookup cleanup path. Skip
empty lookup slots while unwinding because S2MPS14 omits its CP clock.

The issue was found by our static-analysis tool and manually reviewed.

Fixes: b228fad50c00 ("clk: s2mps11: Migrate to clk_hw based OF and registration APIs")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
 drivers/clk/clk-s2mps11.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index fa5ac8f673f6..94969d97d6e9 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -192,8 +192,10 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 		clk_data->hws[i] = &s2mps11_clks[i].hw;
 	}
 
-	of_clk_add_hw_provider(s2mps11_clks->clk_np, of_clk_hw_onecell_get,
-			       clk_data);
+	ret = of_clk_add_hw_provider(s2mps11_clks->clk_np,
+				     of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		goto err_reg;
 
 	platform_set_drvdata(pdev, s2mps11_clks);
 
@@ -201,8 +203,10 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
 
 err_reg:
 	of_node_put(s2mps11_clks[0].clk_np);
-	while (--i >= 0)
-		clkdev_drop(s2mps11_clks[i].lookup);
+	while (--i >= 0) {
+		if (s2mps11_clks[i].lookup)
+			clkdev_drop(s2mps11_clks[i].lookup);
+	}
 
 	return ret;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] clk: s2mps11: Propagate OF provider registration failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] of_clk_add_hw_provider() unconditionally marks fwnode as initialized on failure.
--

commit 70cc7ee16e2987b440f1878c683c89d530c69a34
Author: Pengpeng Hou <hppiscas@163.com>

clk: s2mps11: Propagate OF provider registration failures

This commit updates s2mps11_clk_probe() to correctly check the return value
of of_clk_add_hw_provider() and propagate registration failures. It also
adjusts the cleanup path to handle omitted clocks.

> diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
> index fa5ac8f673f6e..94969d97d6e99 100644
> --- a/drivers/clk/clk-s2mps11.c
> +++ b/drivers/clk/clk-s2mps11.c
[ ... ]
> @@ -192,8 +192,10 @@ static int s2mps11_clk_probe(struct platform_device *pdev)
>  		clk_data->hws[i] = &s2mps11_clks[i].hw;
>  	}
>  
> -	of_clk_add_hw_provider(s2mps11_clks->clk_np, of_clk_hw_onecell_get,
> -			       clk_data);
> +	ret = of_clk_add_hw_provider(s2mps11_clks->clk_np,
> +				     of_clk_hw_onecell_get, clk_data);
> +	if (ret)
> +		goto err_reg;

[Severity: High]
This is a pre-existing issue, but by properly propagating the error from
of_clk_add_hw_provider(), could this cause dependent consumers to fail their
probe instead of deferring?

Looking at of_clk_add_hw_provider() in drivers/clk/clk.c, if it fails, it
unconditionally marks the fwnode as initialized before returning the error:

drivers/clk/clk.c:of_clk_add_hw_provider() {
    ...
	ret = of_clk_set_defaults(np, true);
	if (ret < 0)
		of_clk_del_provider(np);

	fwnode_dev_initialized(&np->fwnode, true);

	return ret;
}

If of_clk_set_defaults() fails (e.g., returning -EPROBE_DEFER),
of_clk_del_provider() unregisters the provider and correctly marks the fwnode
uninitialized, but it is then unconditionally forced to true.

Since fw_devlink relies on this initialized state, does this trick consumers
into thinking the supplier is ready, thereby breaking the EPROBE_DEFER
mechanism for dependent devices when this failure path is taken?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906033651.84082-1-hppiscas@163.com?part=1

Re: [PATCH] clk: s2mps11: Propagate OF provider registration failures

[Brian Masney]

On Sun, Sep 06, 2026 at 11:36:04AM +0800, Pengpeng Hou wrote:
> s2mps11_clk_probe() ignores of_clk_add_hw_provider() failures, stores
> driver data and returns success even though DT consumers cannot acquire the
> registered clocks.
> 
> Return the provider error through the existing lookup cleanup path. Skip
> empty lookup slots while unwinding because S2MPS14 omits its CP clock.
> 
> The issue was found by our static-analysis tool and manually reviewed.
> 
> Fixes: b228fad50c00 ("clk: s2mps11: Migrate to clk_hw based OF and registration APIs")
> Assisted-by: gpt 5

Assisted-by: LLM

I'll fix this up on merge.

Brian

Re: [PATCH] clk: s2mps11: Propagate OF provider registration failures

[Brian Masney]

On Sun, 06 Sep 2026 11:36:04 +0800, Pengpeng Hou wrote:
> clk: s2mps11: Propagate OF provider registration failures

Applied, thanks!

[1/1] clk: s2mps11: Propagate OF provider registration failures
      commit: 276741aac3b56ba50bef59715ba18ba3f0b953a2

Best regards,
-- 
Brian Masney <bmasney@redhat.com>