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

[Marek Vasut]

On 4/17/25 2:34 PM, Adam Ford wrote:
> 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'

This here is not about finding out the clock NAME, this is about 
resolving the parent clock driver instance .

> 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.

Have a look at my reply to "[PATCH v2] clk: fixed-rate: Use 
"clock-output-names" to name fixed clocks" , maybe that clarifies how 
clock resolution is supposed to work. Clock "NAME" does never enter that 
resolution, except for mapping of local "clock-names" array offset to 
"clocks" array offset, the rest uses DT phandles to other clock nodes.

> 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.

Linux does NOT use clock names to resolve clock in DT, Linux does use 
"clocks" and "clock-names" properties and phandles.

> Another possibiltiy, is to change the name of the clock at the of
> registration to match the name returned from clock-output-names.
Sorry, but no, this is plain wrong.

... and let me ask again, how do I reproduce this issue you have ?