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

[Geert Uytterhoeven]

Hi Mathieu,

On Mon, 27 Jul 2026 at 09:44, Mathieu Dubois-Briand
<mathieu.dubois-briand@xxxxxxxxxxx> 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@xxxxxxxxxxx>

Thanks for your patch, which is now commit 667f420c09f1417c ("clk: ti:
mux: resolve parent clocks by DT index, not by name") in clk/clk-next.

This breaks booting BeagleBone Black for me: I don't get any output
on the serial console, even with "earlycon keep_bootcon".  When the
watchdog kicks in (or on kernel panic?), the system reboots.

Reverting this commit fixes the issue.
After reverting, I added debug code to print all parents, and compared
them to the DTB, but didn't see any discrepancies.

Your similar change to drivers/clk/ti/composite.c does not cause any
issues for me.

Do you have a clue?
Thanks!

> --- 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);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds