Hi Rob,
On Fri, Dec 18, 2020 at 11:32 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Fri, Dec 18, 2020 at 5:42 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Fri, Dec 18, 2020 at 12:59 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> > > On Wed, Dec 16, 2020 at 03:52:31PM +0100, Geert Uytterhoeven wrote:
> > > > - Add reference to clock.yaml, and switch to unevaluatedProperties, to
> > > > stop complaining about the presence of "assigned-clock-rates" and
> > > > "assigned-clocks" in board DTS files,
> > > > Fixes: 45c940184b501fc6 ("dt-bindings: clk: versaclock5: convert to yaml")
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > > ---
> > > > Notes:
> > > > 1. The use of "idt,voltage-microvolts" (with trailing S) is a bit
> > > > unfortunate, as Documentation/devicetree/bindings/property-units.txt
> > > > suggests to not have the trailing edge.
> > > > Can we still fix the driver and bindings? While this entered
> > > > uptstream in v5.9, there are no users in next-20201216.
> > > >
> > > > 2. Due to "clock-output-names" being part of
> > > > dt-schema/schemas/clock/clock.yaml, the presence of this property
> > > > does not trigger an error. Adding "clock-output-names: false"
> > > > can fix that. But given this property is deprecated, except for
> > > > very specific use cases, explicitly allowing it for those few use
> > > > cases would be better.
> > > > ---
> > > > .../bindings/clock/idt,versaclock5.yaml | 53 ++++++++++---------
> > > > 1 file changed, 29 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > > > index 2ac1131fd9222a86..14851e76f6342095 100644
> > > > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > > > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > > > @@ -33,6 +33,9 @@ description: |
> > > > maintainers:
> > > > - Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
> > > >
> > > > +allOf:
> > > > + - $ref: clock.yaml#
> > >
> > > No, that's not right. clock.yaml is already applied unconditionally.
> >
> > But without that, it complains about unevaluatedProperties?
>
> By design. You can't have other properties outside your binding unless
> you have a $ref to other schemas. Also, note that there's not a single
> other ref to clock.yaml.
>
> > > You need to define assigned-clocks, etc. here just like 'clocks' and
> > > define how many entries. Or convince me they should be allowed on any
> > > node.
> >
> > They are handled by of_clk_set_defaults(), which is applied to all
> > clock providers.
>
> What does that Linux implementation detail have to do with the bindings?
I consider Linux the reference implementation.
Is there any other real reference implementation? ;-)
> The only other exception we have is pinctrl properties. They often
> aren't that interesting unless you have more than one (maybe we should
> only automatically allow the single case). That's maybe true in the
> assigned-clocks case too. However the big difference I see is pinctrl
> properties are almost always present whereas assign-clocks is more the
> exception. So I think it's good to be explicit where they are used.
The problem with the assigned-clock* properties is that the DT binding
writer has no idea if they will be ever used or not. These properties
come into play at an even higher level than the pinctrl properties.
While the DT binding writer usually[1] knows if there can be pinctrl
properties or not, this is not the case for the assigned-clock*
properties. The former are expected and mandatory, the latter are
optional, and are added only during the system integration phase, and
may appear everywhere.
So I think they should be allowed on any node. Unless we decide
assigned-clock* properties are a bad idea in general.
[1] Unless the bindings are written initially for an IP core in an SoC
that doesn't have pinctrl for the related pins, and the IP core is
reused later in an SoC that does have pinctrl.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
