[PATCH 1/2] arm64: dts: meson-g12: Fix clock order for amlogic,axg-tdm-iface devices

[Kevin Hilman]

Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org> writes:

> On 10/08/2023 19:15, Kevin Hilman wrote:
>> DT maintainers,
>> 
>> Alexander Stein <alexander.stein at mailbox.org> writes:
>> 
>>> Binding specify order of clocks as:
>>> 1. "sclk"
>>> 2. "lrclk"
>>> 3. "mclk"
>>> Adjust clocks accordingly. Fixes warnings:
>> 
>> I understand this patch is to fix DT warnings (and thank you Alexander
>> for fixing warnings!) ... *but* the underlying requirement being
>> enforced by the schema here seems completely wrong to me, and a step
>> backwards.
>> 
>> Sorry if this is a FAQ someplace, but I couldn't find an explanation for
>> this.  One of the main goals of introducing names in the first place was
>> to get rid of ordering requirements.
>
> Not entirely. The names was just a helper for cases when order is not
> fixed,

Exactly, "when order is not fixed."  This is the case for lots of
hardware today (not just clocks), and is precisely what we need to
describe multiple, optional and independent (unordered) clocks.

> but even with the names for every regular case the order was
> always strict. We always expect these to be ordered.

I'm not sure who the "we" is you're referring to, but this expectation
is new to me, and honestly a bit surprising.  Before DT schema, driver &
DT writers could happily describe their hardware using names for
optional and unordered resources.

>> Now the DT schema is enforcing
>> ordering requirements, but the drivers don't need ordering, so what is
>> the point of enforcing ordering requirements?
>
> Because names are not everything.  One OS implementation might still
> take by indices, even if names are provided, so you cannot change the
> order.

Wait, isn't this an "OS-ism" imposing requirements on the DT that are
not at all about describing hardware?

Strictly ordering resources in DT that are completely independent (and
unordered!) in hardware seems to be a big step away from the general
guidance of "describe the hardware, not OS-isms".

And so far, we've only been talking about clocks, but the ordering
implications here apply to resets, pinctrl, regulators and probably
others as well.  All of these subsystems today have some way to describe
unordered & independent resources using names.  Yet, what you are
implying here applies to all of these subsystems: even where names are
used, these resources must be strictly ordered in DT.  From my
perspective, this is a new requirement.  Do you have any pointers to
where this was discussed & decided?  Admittedly, I do not follow DT
schema developments closely, but new requirements like this have
implications that I hope were discussed publically.

> Few bindings allow relaxed approach here, but these are written that way
> to allow mixing order.

Right, "mixing order" is another way of saying unordered, which is an
accurate description of lots of hardware out there.

> For few other bindings (e.g. newer Qualcomm clock controllers) we just
> dropped the names entirely, because they bring little value and also
> code for lookup by name is slower than by index.

I can see that names might bring little value if there aren't
independent optional clocks.  I can also see that making that choice for
perf reasons being a design choice, but that also a case of an OS design
choice impacting the DT, and not really about describing the hardware.

IMO, this design should be a choice of the driver writer who is most
likely to best understand the hardware.  Being forced into that strict
ordering requirement by DT schema when that is not an accurate
description of the hardware seems to be enforcing the wrong thing for
the wrong reasons.

Now that I've ranted a bit, maybe our time would be better spent if we
get practical and discuss a concrete example.  This other thread[1] has
a specific example, and ends with a specific question from Jerome about
how one should actually model a specific piece of hardware with current
DT schema.

Kevin

[1] https://lore.kernel.org/linux-amlogic/1jsf8r9v1v.fsf@starbuckisacylon.baylibre.com/