[PATCH] clk: ti: mux: resolve parent clocks by DT index, not by name

[PATCH] clk: ti: mux: resolve parent clocks by DT index, not by name

[Mathieu Dubois-Briand]

Resolve parent clocks by their index into the device tree "clocks"
property rather than matching names as strings. Name-based matching is
fragile because a clock's "clock-output-names" value in its provider
node can differ from the name used to reference it in a consumer node,
and because names must be globally unique across all clock providers.

On AM335x, this caused broken clock trees where some clocks failed to
enable because their parents could not be found.

Replace of_clk_parent_fill() with a clk_parent_data array that sets
.index to the array position.

Fixes: ec7aa25fa483 ("ARM: dts: Use clock-output-names for am3")
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
This commit aims to solve an issue I've been describing a few months ago
on AM335x SoC [1]. I believe using the parent_data field of
clk_init_data structure is now the preferred way to convey that data,
and it should be more reliable than string comparisons.

[1]: https://lore.kernel.org/all/DI4RUFQNSSNP.2QMSSQWJW9I2O@bootlin.com/
---
 drivers/clk/ti/mux.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c
index d6a0ccfd81db..ded4432f7528 100644
--- a/drivers/clk/ti/mux.c
+++ b/drivers/clk/ti/mux.c
@@ -119,7 +119,7 @@ const struct clk_ops ti_clk_mux_ops = {
 };
 
 static struct clk *_register_mux(struct device_node *node, const char *name,
-				 const char * const *parent_names,
+				 const struct clk_parent_data *parent_data,
 				 u8 num_parents, unsigned long flags,
 				 struct clk_omap_reg *reg, u8 shift, u32 mask,
 				 s8 latch, u8 clk_mux_flags, u32 *table)
@@ -136,7 +136,7 @@ static struct clk *_register_mux(struct device_node *node, const char *name,
 	init.name = name;
 	init.ops = &ti_clk_mux_ops;
 	init.flags = flags;
-	init.parent_names = parent_names;
+	init.parent_data = parent_data;
 	init.num_parents = num_parents;
 
 	/* struct clk_mux assignments */
@@ -167,24 +167,26 @@ static void of_mux_clk_setup(struct device_node *node)
 	struct clk *clk;
 	struct clk_omap_reg reg;
 	unsigned int num_parents;
-	const char **parent_names;
+	struct clk_parent_data *parent_data;
 	const char *name;
 	u8 clk_mux_flags = 0;
 	u32 mask = 0;
 	u32 shift = 0;
 	s32 latch = -EINVAL;
 	u32 flags = CLK_SET_RATE_NO_REPARENT;
+	int i;
 
 	num_parents = of_clk_get_parent_count(node);
 	if (num_parents < 2) {
 		pr_err("mux-clock %pOFn must have parents\n", node);
 		return;
 	}
-	parent_names = kcalloc(num_parents, sizeof(char *), GFP_KERNEL);
-	if (!parent_names)
-		goto cleanup;
+	parent_data = kcalloc(num_parents, sizeof(*parent_data), GFP_KERNEL);
+	if (!parent_data)
+		return;
 
-	of_clk_parent_fill(node, parent_names, num_parents);
+	for (i = 0; i < num_parents; i++)
+		parent_data[i].index = i;
 
 	if (ti_clk_get_reg_addr(node, 0, &reg))
 		goto cleanup;
@@ -207,7 +209,7 @@ static void of_mux_clk_setup(struct device_node *node)
 	mask = (1 << fls(mask)) - 1;
 
 	name = ti_dt_clk_name(node);
-	clk = _register_mux(node, name, parent_names, num_parents,
+	clk = _register_mux(node, name, parent_data, num_parents,
 			    flags, &reg, shift, mask, latch, clk_mux_flags,
 			    NULL);
 
@@ -215,7 +217,7 @@ static void of_mux_clk_setup(struct device_node *node)
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
 
 cleanup:
-	kfree(parent_names);
+	kfree(parent_data);
 }
 CLK_OF_DECLARE(mux_clk, "ti,mux-clock", of_mux_clk_setup);
 

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260713-mathieu-wdt-clock-theo-f0c5ba58e258

Best regards,
-- 
Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>

Re: [PATCH] clk: ti: mux: resolve parent clocks by DT index, not by name

[Brian Masney]

Hi Mathieu,

On Wed, Jul 15, 2026 at 05:09:56PM +0200, Mathieu Dubois-Briand wrote:
> Resolve parent clocks by their index into the device tree "clocks"
> property rather than matching names as strings. Name-based matching is
> fragile because a clock's "clock-output-names" value in its provider
> node can differ from the name used to reference it in a consumer node,
> and because names must be globally unique across all clock providers.
> 
> On AM335x, this caused broken clock trees where some clocks failed to
> enable because their parents could not be found.
> 
> Replace of_clk_parent_fill() with a clk_parent_data array that sets
> .index to the array position.
> 
> Fixes: ec7aa25fa483 ("ARM: dts: Use clock-output-names for am3")
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>

So this only updates of_mux_clk_setup(). of_ti_composite_mux_clk_setup()
calls ti_clk_add_component(), which does a name based matching as well.
It would be nice to have this consistent within the same driver. I know
this is going to require a larger refactor.

Otherwise your fix looks correct to me.

Brian

Re: [PATCH] clk: ti: mux: resolve parent clocks by DT index, not by name

[Mathieu Dubois-Briand]

On Thu Jul 16, 2026 at 7:49 PM CEST, Brian Masney wrote:
> Hi Mathieu,
>
> On Wed, Jul 15, 2026 at 05:09:56PM +0200, Mathieu Dubois-Briand wrote:
>> Resolve parent clocks by their index into the device tree "clocks"
>> property rather than matching names as strings. Name-based matching is
>> fragile because a clock's "clock-output-names" value in its provider
>> node can differ from the name used to reference it in a consumer node,
>> and because names must be globally unique across all clock providers.
>> 
>> On AM335x, this caused broken clock trees where some clocks failed to
>> enable because their parents could not be found.
>> 
>> Replace of_clk_parent_fill() with a clk_parent_data array that sets
>> .index to the array position.
>> 
>> Fixes: ec7aa25fa483 ("ARM: dts: Use clock-output-names for am3")
>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>
> So this only updates of_mux_clk_setup(). of_ti_composite_mux_clk_setup()
> calls ti_clk_add_component(), which does a name based matching as well.
> It would be nice to have this consistent within the same driver. I know
> this is going to require a larger refactor.
>
> Otherwise your fix looks correct to me.
>
> Brian

Hi Brian,

Thanks for your comment.

Yes, it does make sense to make everything consistent here. I'm not
really comfortable with this driver, but I can give it a try.

Just to be sure, are you talking only about drivers/clk/ti/composite.c
or is it about more files in drivers/clk/ti/? I'm not sure to understand
the whole interactions between each components here.

Fixing drivers/clk/ti/composite.c to use clk_parent_data looks easy, I
can definitely send a patch for that. But you are talking about a larger
refactor, so I suspect you would like to see more changes.

I had a look at the other files in drivers/clk/ti/, here are my first
thoughts:
- drivers/clk/ti/apll.c, drivers/clk/ti/dpll.c, drivers/clk/ti/fapll.c:
  Changes would be similar to what was made here.
- drivers/clk/ti/divider.c, drivers/clk/ti/gate.c,
  drivers/clk/ti/interface.c, drivers/clk/ti/adpll.c: I believe we have
  to stick with matching parents by name. This could be made using
  clk_parent_data .name field if we really want to switch to
  clk_parent_data structure. Is that a target?
- drivers/clk/ti/clkctrl.c: similarly, I believe we have to stick to
  clock names. Using clk_parent_data structure requires a lot of changes
  in drivers/clk/ti/clk-* files, but these changes should be trivial.

What would be the relevant changes here?

Thanks,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] clk: ti: mux: resolve parent clocks by DT index, not by name

[Brian Masney]

Hi Mathieu,

On Fri, Jul 24, 2026 at 04:56:01PM +0200, Mathieu Dubois-Briand wrote:
> On Thu Jul 16, 2026 at 7:49 PM CEST, Brian Masney wrote:
> > On Wed, Jul 15, 2026 at 05:09:56PM +0200, Mathieu Dubois-Briand wrote:
> >> Resolve parent clocks by their index into the device tree "clocks"
> >> property rather than matching names as strings. Name-based matching is
> >> fragile because a clock's "clock-output-names" value in its provider
> >> node can differ from the name used to reference it in a consumer node,
> >> and because names must be globally unique across all clock providers.
> >> 
> >> On AM335x, this caused broken clock trees where some clocks failed to
> >> enable because their parents could not be found.
> >> 
> >> Replace of_clk_parent_fill() with a clk_parent_data array that sets
> >> .index to the array position.
> >> 
> >> Fixes: ec7aa25fa483 ("ARM: dts: Use clock-output-names for am3")
> >> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> >
> > So this only updates of_mux_clk_setup(). of_ti_composite_mux_clk_setup()
> > calls ti_clk_add_component(), which does a name based matching as well.
> > It would be nice to have this consistent within the same driver. I know
> > this is going to require a larger refactor.
> >
> > Otherwise your fix looks correct to me.
> >
> > Brian
> 
> Hi Brian,
> 
> Thanks for your comment.
> 
> Yes, it does make sense to make everything consistent here. I'm not
> really comfortable with this driver, but I can give it a try.
> 
> Just to be sure, are you talking only about drivers/clk/ti/composite.c
> or is it about more files in drivers/clk/ti/? I'm not sure to understand
> the whole interactions between each components here.
> 
> Fixing drivers/clk/ti/composite.c to use clk_parent_data looks easy, I
> can definitely send a patch for that. But you are talking about a larger
> refactor, so I suspect you would like to see more changes.

I think we should just get drivers/clk/ti/composite.c consistent with
itself.

Brian