Re: [PATCH v8 3/7] clk: nuvoton: ma35d1: Retrieve HXT/LXT from DT when possible

[Jerome Brunet <jbrunet@baylibre.com>]

On Sun 04 Oct 2026 at 23:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

>> This commit modifies the clock driver to retrieve the external HXT and LXT
>> clocks from the device tree instead of registering them directly. It falls
>> back to registering LXT for backward compatibility with older device trees.
>>
>>> diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c
>>> index ceebcbd8c18b3..ed61a990870e4 100644
>>> --- a/drivers/clk/nuvoton/clk-ma35d1.c
>>> +++ b/drivers/clk/nuvoton/clk-ma35d1.c
>> [ ... ]
>>> @@ -240,12 +257,18 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> -	hws[HXT] = ma35d1_clk_fixed("hxt", 24000000);
>>
>> [Severity: Critical]
>> Does this unconditional removal break ABI compatibility with older device
>> trees?
>>
>> Because HXT (index 0) is exported as a public ABI in the DT bindings,
>> removing its explicit registration leaves hws[HXT] as NULL in the provider's
>> clock array. If an existing downstream or older device tree consumer explicitly
>> references this clock via clocks = <&clk HXT>, won't the provider's
>> of_clk_hw_onecell_get() fail to resolve the clock and return an error?
>
> Jerome, your wisdom matters here :-) For me this is a false positive,
> because nobody outside this driver should access hws[HXT] and the clock
> is still referenced through the DT, just not provided by this
> driver. For me this is not an issue we should consider, let me know
> otherwise.

Strictly speaking there is (used to be) a clock at this index so it is
not a false positive. Someone crazy enough could have gone through here
to get the OSC even if does not make a lot of sense.

This clock should never have existed to start with but it got in the
bindings ... and that is the pain of dealing with bad bindings :/

You've got 2 options:
* Removing the ID from the bindings explaining clearly that it conflicts
  with the fact this clock is actually an input according to the same
  bindings. I suspect the DT maintainers won't like this much ...

* Keep the bad support binding in the driver: You'll need a pass-through
  clock element where the parent data is the fw_name and a fallback to
  an `hxt-fake-internal' fixed clock. Feel free to add a comment
  explaining how bad it is to use that clock and what user should be
  doing instead.

You'll probably need to play that last trick with the lxt anyway ?

>
> Miquèl

-- 
Jerome