[REGRESSION] Re: [PATCH v2 16/24] clk: imx: Convert clock-osc-* back to osc_*

[Adam Ford]

On Thu, Apr 17, 2025 at 6:31 AM Marek Vasut <marex at denx.de> wrote:
>
> On 4/17/25 12:51 PM, Adam Ford wrote:
> > On Thu, Apr 17, 2025 at 2:24 AM Marek Vasut <marex at denx.de> wrote:
> >>
> >> On 4/17/25 1:58 AM, Fabio Estevam wrote:
> >>> On Wed, Apr 16, 2025 at 8:47 PM Marek Vasut <marex at denx.de> wrote:
> >>>
> >>>> You are not supposed to use "clock-output-names" for clock look up.
> >>>> You are supposed to use "clocks"/"clock-names" DT properties and then
> >>>> resolve the remote clock from information in those.
> >>>
> >>> What do you think about registering the osc clocks like this?
> >>>
> >>> https://paste.debian.net/1369857/
> >>
> >> It would be good to include the patch inline in this email, not in a
> >> link to some paste site which will go away.
> >>
> >> This also does not work in case 24 MHz or 32 kHz clock are provided by
> >> something else than an xtal , and it will also lead to instantiation of
> >> the same clock twice. The clock are already described in DT as
> >> fixed-clock , there is fixed clock driver which binds to those clock
> >> based on their DT description , the problem here seems to be related to
> >> the look up of those 24 MHz or 32 kHz clock, not their (re)registration.
> >
> >
> > Since that patch changed the lookup of the clock name broke USB on
> > multiple boards, I would like to recommend we revert it until a proper
> > solution is found.
> This would also re-introduce the bogus clock-osc-24m look up, which
> requires workarounds in DT, so no. Let's actually fix this instead of
> reinstating broken workarounds.
>
> Does clk_resolve_parent_clk() in clk_mux_fetch_parent_index() not
> resolve the mux index correctly or something ? Do you by any chance have
> some -u-boot.dtsi modification to the clock-controller at 30380000 node
> which changes clocks/clock-names somehow ? Which mux does not get
> correctly resolved here ? Is there a simple reproducer ?

I am not modifying my clock-controller node, but I did spend a little
time looking at how Linux reviews the parent clock names.

In Linux, of_clk_get_parent_name() will look for 'clock-output-names'
and return that value if it's present. If we go with your proposal, I
feel like we'd have to traverse to the top of the clock tree to get
the CCM's clock associated with osc_24m, and then compare the clock it
returns with the clock we want to see if they match.

If we follow the Linux method where we try to fetch the parent clock
name and use the name associated with it, we could simplify this.
Another possibiltiy, is to change the name of the clock at the of
registration to match the name returned from clock-output-names.

adam