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

Hi Mathieu,

On Wed, 19 Aug 2026 at 17:12, Mathieu Dubois-Briand
<[email protected]> wrote:
> On Wed Aug 19, 2026 at 2:32 PM CEST, Geert Uytterhoeven wrote:
> > On Wed, 19 Aug 2026 at 07:39, Mathieu Dubois-Briand
> > <[email protected]> wrote:
> >> On Tue Aug 18, 2026 at 6:02 PM CEST, Geert Uytterhoeven wrote:
> >> > On Mon, 27 Jul 2026 at 09:44, Mathieu Dubois-Briand
> >> > <[email protected]> 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 <[email protected]>
> >> >
> >> > 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.

> > With omap2plus_defconfig and multi_v7_defconfig, it does boot.  However,
> > this commit does have an impact on /sys/kernel/debug/clk/clk_summary:
> >
> > --- summary-7.2.0-rc2-00106-g0d4d262c1664       2026-08-19
> > 10:56:51.622275674 +0200
> > +++ summary-7.2.0-rc2-00107-g667f420c09f1       2026-08-19
> > 10:58:13.929823890 +0200
> > ...
> >
> > I.e. wdt1_fck changed its parent from clk_rc32k_ck to
> > clk-24mhz-clkctrl:0000:0, which is probably not intended?
> >
>
> This was definitely not intended, but I believe this is a valid
> configuration.
>
> wdt1_fck has two potential parents. In the device tree
> (am33xx-clocks.dtsi), we have:
>
>         wdt1_fck: clock-wdt1-fck@538 {
>                 ...
>                 clocks = <&clk_rc32k_ck>, <&clk_24mhz_clkctrl AM3_CLK_24MHZ_CLKDIV32K_CLKCTRL 0>;
>                 ...
>         };
>
> As I understand it, the kernel is free to select any of the parents
> here. Is that correct?
>
> Previously, the link with the second parent was broken, and only the
> first parent was known. That was my initial issue that triggered my
> changes. Now, both parents can be selected. I assume the second one is
> seen as a better option for some reason, and so now it is selected.
>
> But maybe my assumption are wrongs, so feel free correct me here.

Thanks, that sounds like a good explanation!

> > However, that is not the cause of the crash.  My .config has
> > CONFIG_INIT_STACK_ALL_PATTERN enabled, so any uninitialized local
> > variables contain garbage.
> >
> >> @@ -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
> >
> > Init is not fully initialized, hence init.parent_names contains garbage
> > (before, .parent_data contained garbage).
> > As clk_core_populate_parent_map() prioritizes .parent_names over
> > .parent_data, the uninitialized .parent_data didn't cause any harm before.
>
> Nice catch. I didn't realise that while doing my changes.
>
> Thanks for taking the time to track this issue.

You're welcome!

I submitted a first set of fixes, incl. "[PATCH treewide 1/5] clk:
ti: Make sure clk_init_data is fully initialized"
https://lore.kernel.org/e6f52b34163c8e708c1100107b437af0b3b049a6.1787165329.git.geert+renesas@glider.be/
I will submit the remainder (way too many) later...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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