Merge patch series "clk: Return value calculated by ERR_PTR"

Andrew Goodbody <andrew.goodbody@linaro.org> says:

Smatch reported an error where a value calculated by ERR_PTR was not
used. Fixing this to return the generated value led to a test failure
which meant updating the sandbox clock code so that it would still cause
the tests to pass with the above correction.
Debugging this problem led to a SIGSEGV which is addressed in 1/3.
Possible memory leaks noticed are addressed in 3/3.

Link: https://lore.kernel.org/r/20251121-clk_uclass_fix-v2-0-74f4ea10e194@linaro.org

4 files changed, 26 insertions, 7 deletions

diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h
index 37fe49c7fcf..2d3318cdcc4 100644
--- a/arch/sandbox/include/asm/clk.h
+++ b/arch/sandbox/include/asm/clk.h
@@ -50,6 +50,7 @@ enum sandbox_clk_test_id {
 
 struct sandbox_clk_priv {
 	bool probed;
+	struct clk clk;
 	ulong rate[SANDBOX_CLK_ID_COUNT];
 	bool enabled[SANDBOX_CLK_ID_COUNT];
 	bool requested[SANDBOX_CLK_ID_COUNT];
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ae787b88510..4a438181ba9 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -199,7 +199,7 @@ static struct clk *clk_set_default_get_by_id(struct clk *clk)
 		if (ret) {
 			debug("%s(): could not get parent clock pointer, id %lu\n",
 			      __func__, clk->id);
-			ERR_PTR(ret);
+			return ERR_PTR(ret);
 		}
 	}
 
@@ -338,7 +338,7 @@ static int clk_set_default_rates(struct udevice *dev,
 				continue;
 			}
 
-			return ret;
+			goto fail;
 		}
 
 		/* This is clk provider device trying to program itself
@@ -354,7 +354,7 @@ static int clk_set_default_rates(struct udevice *dev,
 
 		c = clk_set_default_get_by_id(&clk);
 		if (IS_ERR(c))
-			return PTR_ERR(c);
+			continue;
 
 		ret = clk_set_rate(c, rates[index]);
 
@@ -662,7 +662,7 @@ int clk_enable(struct clk *clk)
 	struct clk *clkp = NULL;
 	int ret;
 
-	debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name);
+	debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL");
 	if (!clk_valid(clk))
 		return 0;
 	ops = clk_dev_ops(clk->dev);
@@ -723,7 +723,7 @@ int clk_disable(struct clk *clk)
 	struct clk *clkp = NULL;
 	int ret;
 
-	debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name);
+	debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL");
 	if (!clk_valid(clk))
 		return 0;
 	ops = clk_dev_ops(clk->dev);
diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c
index c8c5a88c52d..cd92a6a29f5 100644
--- a/drivers/clk/clk_sandbox.c
+++ b/drivers/clk/clk_sandbox.c
@@ -8,8 +8,23 @@
 #include <errno.h>
 #include <malloc.h>
 #include <asm/clk.h>
+#include <dm/device-internal.h>
 #include <linux/clk-provider.h>
 
+static int sandbox_clk_of_to_plat(struct udevice *dev)
+{
+	struct clk *clk;
+	struct sandbox_clk_priv *priv = dev_get_priv(dev);
+
+	clk = &priv->clk;
+
+	/* FIXME: This is not allowed */
+	dev_set_uclass_priv(dev, clk);
+
+	clk->dev = dev;
+	return 0;
+}
+
 static ulong sandbox_clk_get_rate(struct clk *clk)
 {
 	struct sandbox_clk_priv *priv = dev_get_priv(clk->dev);
@@ -102,6 +117,7 @@ static int sandbox_clk_request(struct clk *clk)
 	if (id >= SANDBOX_CLK_ID_COUNT)
 		return -EINVAL;
 
+	priv->clk.id = id;
 	priv->requested[id] = true;
 	return 0;
 }
@@ -132,6 +148,7 @@ U_BOOT_DRIVER(sandbox_clk) = {
 	.name		= "sandbox_clk",
 	.id		= UCLASS_CLK,
 	.of_match	= sandbox_clk_ids,
+	.of_to_plat	= sandbox_clk_of_to_plat,
 	.ops		= &sandbox_clk_ops,
 	.probe		= sandbox_clk_probe,
 	.priv_auto	= sizeof(struct sandbox_clk_priv),
diff --git a/test/dm/clk.c b/test/dm/clk.c
index 790968e6477..c3363796498 100644
--- a/test/dm/clk.c
+++ b/test/dm/clk.c
@@ -89,8 +89,9 @@ static int dm_test_clk(struct unit_test_state *uts)
 						 SANDBOX_CLK_TEST_ID_SPI));
 	ut_asserteq(0, sandbox_clk_test_get_rate(dev_test,
 						 SANDBOX_CLK_TEST_ID_I2C));
-	ut_asserteq(321, sandbox_clk_test_get_rate(dev_test,
-						   SANDBOX_CLK_TEST_ID_DEVM1));
+	if (!CONFIG_IS_ENABLED(CLK_AUTO_ID))
+		ut_asserteq(321, sandbox_clk_test_get_rate(dev_test,
+							   SANDBOX_CLK_TEST_ID_DEVM1));
 	ut_asserteq(0, sandbox_clk_test_get_rate(dev_test,
 						 SANDBOX_CLK_TEST_ID_DEVM2));