On Sun, Dec 20, 2020 at 3:24 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> 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? ;-)
No comment. ;)
> > 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.
Okay, let's add them automatically I guess. We can at least make them
dependent on having a 'clocks' property.
Rob
