Merge patch series "clk: support arbitrary clk_register() sequence"

Yang Xiwen <forbidden405@outlook.com> says:

Currently, the U-Boot clk framework mandates that clock registration
begins at the root and proceeds to children. This creates an additional
requirement that does not exist in the Linux kernel, making the porting
of clk drivers more difficult.

This series handles the dependency entirely within the clk framework,
allowing drivers the freedom to register clocks in any order.

This is achieved by assigning the parent "lazily". The framework caches
the parent name in the core clk struct and attempts to resolve the
actual parent when clk consumers call clk_get_parent(). The process is
transparent to clk consumers as long as they use standard clk framework
APIs.

I've run `ut dm clk*` and verified these commits do not break any
existing test cases. It also passes the new test case.

This feature is disabled for xPLs by default. I have not found a clean
way to enable this separately for xPLs without introducing a repetitive
Kconfig entry (e.g., xPL_CLK_LAZY_REPARENT), which looks very ugly.

Link: https://lore.kernel.org/r/20260120-clk-reparent-v3-0-0d43d4b362ac@outlook.com

drivers/clk/Kconfig

@@ -51,6 +51,18 @@ config VPL_CLK
 	  setting up clocks within TPL, and allows the same drivers to be
 	  used as U-Boot proper.
 
+config CLK_LAZY_REPARENT
+	bool "Enable clock lazy reparenting feature"
+	depends on CLK_CCF
+	default n if SPL_CLK || TPL_CLK || VPL_CLK
+	default y
+	help
+	  This option allows registering clocks in a less strict order that
+	  Parent clocks can be registered before their children. The clock subsystem
+	  will cache the parent's name and resolve it to the real parent device "lazily".
+	  This is the default behavior in Linux clock subsystem. Enabling this feature
+	  should simplifies the porting of Linux clock drivers to U-Boot.
+
 config CLK_BCM6345
 	bool "Clock controller driver for BCM6345"
 	depends on CLK && ARCH_BMIPS

drivers/clk/clk-uclass.c

@@ -496,6 +496,32 @@ ulong clk_get_rate(struct clk *clk)
 	return ops->get_rate(clk);
 }
 
+static struct udevice *clk_reparent(struct clk *clk, const char *parent_name)
+{
+	struct udevice *pdev;
+	int ret;
+
+	if (!clk_valid(clk))
+		return NULL;
+
+	if (!parent_name)
+		return NULL;
+
+	debug("%s(clk=%p) reparenting to %s\n", __func__, clk, parent_name);
+
+	ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, &pdev);
+	if (ret) {
+		log_err("%s(clk=%p) failed to find parent \"%s\"\n", __func__, clk, parent_name);
+		return NULL;
+	}
+
+	ret = device_reparent(clk->dev, pdev);
+	if (ret)
+		return NULL;
+
+	return pdev;
+}
+
 struct clk *clk_get_parent(struct clk *clk)
 {
 	struct udevice *pdev;
@@ -506,8 +532,22 @@ struct clk *clk_get_parent(struct clk *clk)
 		return NULL;
 
 	pdev = dev_get_parent(clk->dev);
-	if (!pdev)
+	if (!pdev) {
+		if (CONFIG_IS_ENABLED(CLK_LAZY_REPARENT)) {
+			pdev = clk_reparent(clk, clk->parent_name);
+			free(clk->parent_name);
+			clk->parent_name = NULL;
+
+			if (!pdev)
+				return ERR_PTR(-ENODEV);
+		} else {
+			return ERR_PTR(-ENODEV);
+		}
+	}
+
+	if (device_get_uclass_id(pdev) != UCLASS_CLK)
 		return ERR_PTR(-ENODEV);
+
 	pclk = dev_get_clk_ptr(pdev);
 	if (!pclk)
 		return ERR_PTR(-ENODEV);
@@ -627,6 +667,10 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 	debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent);
 	if (!clk_valid(clk))
 		return 0;
+
+	free(clk->parent_name);
+	clk->parent_name = NULL;
+
 	ops = clk_dev_ops(clk->dev);
 
 	if (!ops->set_parent)
@@ -660,7 +704,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 int clk_enable(struct clk *clk)
 {
 	const struct clk_ops *ops;
-	struct clk *clkp = NULL;
+	struct clk *clkp = NULL, *clk_parent;
 	int ret;
 
 	debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL");
@@ -676,9 +720,10 @@ int clk_enable(struct clk *clk)
 				clkp->enable_count++;
 				return 0;
 			}
-			if (clkp->dev->parent &&
+
-			    device_get_uclass_id(clkp->dev->parent) == UCLASS_CLK) {
+			clk_parent = clk_get_parent(clkp);
-				ret = clk_enable(dev_get_clk_ptr(clkp->dev->parent));
+			if (!IS_ERR_OR_NULL(clk_parent)) {
+				ret = clk_enable(clk_parent);
 				if (ret) {
 					printf("Enable %s failed\n",
 					       clkp->dev->parent->name);
@@ -751,13 +796,16 @@ int clk_disable(struct clk *clk)
 				return ret;
 		}
 
-		if (clkp && clkp->dev->parent &&
+		if (clkp) {
-		    device_get_uclass_id(clkp->dev->parent) == UCLASS_CLK) {
+			struct clk *clk_parent = clk_get_parent(clkp);
-			ret = clk_disable(dev_get_clk_ptr(clkp->dev->parent));
+
-			if (ret) {
+			if (!IS_ERR_OR_NULL(clk_parent)) {
-				printf("Disable %s failed\n",
+				ret = clk_disable(clk_parent);
-				       clkp->dev->parent->name);
+				if (ret) {
-				return ret;
+					printf("Disable %s failed\n",
+					       clkp->dev->parent->name);
+					return ret;
+				}
 			}
 		}
 	} else {

drivers/clk/clk.c

@@ -24,8 +24,18 @@ int clk_register(struct clk *clk, const char *drv_name,
 	if (parent_name) {
 		ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, &parent);
 		if (ret) {
-			log_err("%s: failed to get %s device (parent of %s)\n",
+			log_debug("%s: failed to get %s device (parent of %s)\n",
-				__func__, parent_name, name);
+				  __func__, parent_name, name);
+
+			if (CONFIG_IS_ENABLED(CLK_LAZY_REPARENT)) {
+				/*
+				 * The parent is not yet registered.
+				 * Cache the parent name and resolve it later.
+				 */
+				clk->parent_name = strdup(parent_name);
+				if (!clk->parent_name)
+					return -ENOMEM;
+			}
 		} else {
 			log_debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
 				  parent->name, parent);

drivers/clk/clk_sandbox_ccf.c

@@ -229,6 +229,7 @@ static const struct udevice_id sandbox_clk_ccf_test_ids[] = {
 
 static const char *const usdhc_sels[] = { "pll3_60m", "pll3_80m", };
 static const char *const i2c_sels[] = { "pll3_60m", "pll3_80m", };
+static const char *const i2s_sels[] = { "pll3_60m", "pll3_80m", };
 
 static int sandbox_clk_ccf_probe(struct udevice *dev)
 {
@@ -277,6 +278,15 @@ static int sandbox_clk_ccf_probe(struct udevice *dev)
 	dev_clk_dm(dev, SANDBOX_CLK_I2C_ROOT,
 		   sandbox_clk_gate2("i2c_root", "i2c", base + 0x7c, 0));
 
+	/* Register i2s_root(child) and i2s(parent) in reverse order to test CLK_LAZY_REPARENT */
+	dev_clk_dm(dev, SANDBOX_CLK_I2S_ROOT,
+		   sandbox_clk_gate2("i2s_root", "i2s", base + 0x80, 0));
+
+	reg = BIT(29) | BIT(25) | BIT(17);
+	dev_clk_dm(dev, SANDBOX_CLK_I2S,
+		   sandbox_clk_composite("i2s", i2s_sels, ARRAY_SIZE(i2s_sels),
+					 &reg, CLK_SET_RATE_UNGATE));
+
 	return 0;
 }
 

drivers/core/device.c

@@ -285,6 +285,14 @@ int device_reparent(struct udevice *dev, struct udevice *new_parent)
 	assert(dev);
 	assert(new_parent);
 
+	if (!dev->parent) {
+		assert(list_empty(&dev->sibling_node));
+
+		list_add_tail(&dev->sibling_node, &new_parent->child_head);
+		dev->parent = new_parent;
+		return 0;
+	}
+
 	device_foreach_child_safe(pos, n, dev->parent) {
 		if (pos->driver != dev->driver)
 			continue;

include/clk.h

@@ -47,6 +47,7 @@ struct udevice;
 /**
  * struct clk - A handle to (allowing control of) a single clock.
  * @dev: The device which implements the clock signal.
+ * @parent_name: The name of the parent.
  * @rate: The clock rate (in HZ).
  * @flags: Flags used across common clock structure (e.g. %CLK_)
  *         Clock IP blocks specific flags (i.e. mux, div, gate, etc) are defined
@@ -72,6 +73,7 @@ struct udevice;
  */
 struct clk {
 	struct udevice *dev;
+	char *parent_name;
 	long long rate;	/* in HZ */
 	u32 flags;
 	int enable_count;

include/sandbox-clk.h

@@ -21,6 +21,8 @@ enum {
 	SANDBOX_CLK_USDHC2_SEL,
 	SANDBOX_CLK_I2C,
 	SANDBOX_CLK_I2C_ROOT,
+	SANDBOX_CLK_I2S,
+	SANDBOX_CLK_I2S_ROOT,
 };
 
 enum sandbox_pllv3_type {

test/dm/clk_ccf.c

@@ -35,12 +35,14 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_ECSPI_ROOT), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("ecspi_root", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_PARENT, clk->flags);
 
 	/* Test for clk_get_parent_rate() */
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_ECSPI1), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("ecspi1", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_PARENT, clk->flags);
 
 	rate = clk_get_parent_rate(clk);
@@ -50,6 +52,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_ECSPI0), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("ecspi0", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_PARENT, clk->flags);
 
 	rate = clk_get_parent_rate(clk);
@@ -59,6 +62,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_USDHC1_SEL), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("usdhc1_sel", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_NO_REPARENT, clk->flags);
 
 	rate = clk_get_parent_rate(clk);
@@ -71,6 +75,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ut_asserteq_64(60000000, rate);
 
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_PLL3_80M), &pclk);
+	ut_assertnull(clk->parent_name);
 	ut_assertok(ret);
 
 	ret = clk_set_parent(clk, pclk);
@@ -82,6 +87,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_USDHC2_SEL), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("usdhc2_sel", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_NO_REPARENT, clk->flags);
 
 	rate = clk_get_parent_rate(clk);
@@ -98,6 +104,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ut_asserteq_64(80000000, rate);
 
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_PLL3_60M), &pclk);
+	ut_assertnull(clk->parent_name);
 	ut_assertok(ret);
 
 	ret = clk_set_parent(clk, pclk);
@@ -110,6 +117,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2C), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("i2c", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_SET_RATE_UNGATE, clk->flags);
 
 	rate = clk_get_rate(clk);
@@ -124,11 +132,13 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_index(test_dev, SANDBOX_CLK_TEST_ID_I2C_ROOT, &clk_ccf);
 	ut_assertok(ret);
 	ut_asserteq_str("clk-ccf", clk_ccf.dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(CLK_ID(clk_ccf.dev, SANDBOX_CLK_I2C_ROOT), clk_ccf.id);
 
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2C_ROOT), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("i2c_root", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 	ut_asserteq(SANDBOX_CLK_I2C_ROOT, clk_get_id(clk));
 
 	ret = clk_enable(&clk_ccf);
@@ -138,6 +148,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ut_asserteq(1, ret);
 
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2C), &pclk);
+	ut_assertnull(clk->parent_name);
 	ut_assertok(ret);
 
 	ret = sandbox_clk_enable_count(pclk);
@@ -156,6 +167,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_USDHC1_SEL), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("usdhc1_sel", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 
 	pclk = clk_get_parent(clk);
 	ut_assertok_ptr(pclk);
@@ -169,6 +181,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 
 	ret = clk_get_by_id(CLK_ID(dev, clkid), &pclk);
 	ut_assertok(ret);
+	ut_assertnull(clk->parent_name);
 	ret = clk_set_parent(clk, pclk);
 	ut_assertok(ret);
 	pclk = clk_get_parent(clk);
@@ -179,6 +192,7 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2C_ROOT), &clk);
 	ut_assertok(ret);
 	ut_asserteq_str("i2c_root", clk->dev->name);
+	ut_assertnull(clk->parent_name);
 
 	/* Disable it, if any. */
 	ret = sandbox_clk_enable_count(clk);
@@ -209,3 +223,29 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
 	return 1;
 }
 DM_TEST(dm_test_clk_ccf, UTF_SCAN_FDT);
+
+#if CONFIG_IS_ENABLED(CLK_LAZY_REPARENT)
+/* Test CLK_LAZY_REPARENT feature */
+static int dm_test_clk_lazy_reparent(struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	struct clk *clk_i2s, *clk_i2s_root;
+
+	/* Get the device using the clk device */
+	ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-ccf", &dev));
+
+	ut_assertok(clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2S_ROOT), &clk_i2s_root));
+	ut_assertok(clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_I2S), &clk_i2s));
+
+	ut_asserteq_str(clk_i2s_root->parent_name, "i2s");
+	ut_assertnull(clk_i2s_root->dev->parent);
+
+	ut_assertok(clk_enable(clk_i2s_root));
+
+	ut_assertnull(clk_i2s_root->parent_name);
+	ut_asserteq_ptr(clk_i2s_root->dev->parent, clk_i2s->dev);
+
+	return 0;
+}
+DM_TEST(dm_test_clk_lazy_reparent, UTF_SCAN_FDT);
+#endif

test/dm/core.c

@@ -696,9 +696,10 @@ DM_TEST(dm_test_children, 0);
 static int dm_test_device_reparent(struct unit_test_state *uts)
 {
 	struct udevice *top[NODE_COUNT];
-	struct udevice *child[NODE_COUNT];
+	struct udevice *child[NODE_COUNT], *temp_child = NULL;
 	struct udevice *grandchild[NODE_COUNT];
 	struct udevice *dev;
+	struct udevice *orphan;
 	int total;
 	int ret;
 	int i;
@@ -720,8 +721,11 @@ static int dm_test_device_reparent(struct unit_test_state *uts)
 		ut_assertok(create_children(uts, child[i], NODE_COUNT, 50 * i,
 					    i == 2 ? grandchild : NULL));
 
+	/* Create an orphan device */
+	ut_assertok(create_children(uts, NULL, 1, 49, &orphan));
+
 	/* Check total number of devices */
-	total = NODE_COUNT * (3 + NODE_COUNT);
+	total = NODE_COUNT * (3 + NODE_COUNT) + 1;
 	ut_asserteq(total, dm_testdrv_op_count[DM_TEST_OP_BIND]);
 
 	/* Probe everything */
@@ -738,6 +742,14 @@ static int dm_test_device_reparent(struct unit_test_state *uts)
 
 	ut_assertok(device_reparent(top[4], top[0]));
 
+	/* Ensure it's reparented */
+	ut_asserteq_ptr(top[4]->parent, top[0]);
+	device_foreach_child(temp_child, top[0]) {
+		if (temp_child == top[4])
+			break;
+	}
+	ut_asserteq_ptr(temp_child, top[4]);
+
 	/* try to get devices */
 	ret = uclass_find_first_device(UCLASS_TEST, &dev);
 	ut_assert(!ret);
@@ -773,6 +785,24 @@ static int dm_test_device_reparent(struct unit_test_state *uts)
 	ut_assert(!ret);
 	ut_assertnonnull(dev);
 
+	/* Re-parent orphant device */
+	ut_assertok(device_reparent(orphan, top[0]));
+
+	/* try to get the device */
+	ret = uclass_find_first_device(UCLASS_TEST, &dev);
+	ut_assert(!ret);
+	ut_assertnonnull(dev);
+
+	/* ensure it's reparented */
+	ut_asserteq_ptr(orphan->parent, top[0]);
+
+	temp_child = NULL;
+	device_foreach_child(temp_child, top[0]) {
+		if (temp_child == orphan)
+			break;
+	}
+	ut_asserteq_ptr(temp_child, orphan);
+
 	/* Remove re-pareneted devices. */
 	ut_assertok(device_remove(top[3], DM_REMOVE_NORMAL));