netdev - Re: [PATCH] [RFC] dt-bindings: net: micrel: Convert to json-schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdV01hv-riCFEBD024pX6jL37C6hp7Cjjy1rtaUnrhvK3w@mail.gmail.com>
Date: Mon, 18 Nov 2024 11:39:22 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Rob Herring <robh@...nel.org>
Cc: Ben Dooks <ben.dooks@...ethink.co.uk>, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, netdev@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH] [RFC] dt-bindings: net: micrel: Convert to json-schema

Hi Rob,

On Fri, Nov 15, 2024 at 4:02 PM Rob Herring <robh@...nel.org> wrote:
> On Thu, Nov 14, 2024 at 11:42:50AM +0100, Geert Uytterhoeven wrote:
> > Convert the Micrel PHY Device Tree binding documentation to json-schema.
> >
> > Add a simple example.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> > ---
> > Notes:
> >   1. I specified Ben Dooks as the maintainer, as he wrote the original
> >      bindings. Ben, are you OK with that?
> >   2. This schema is never applied, as there is no compatible value or
> >      select statement. Adding
> >
> >       select:
> >         properties:
> >           $nodename:
> >             pattern: "^ethernet-phy(@[a-f0-9]+)?$"
> >
> >         required:
> >           - $nodename
> >
> >      and changing
> >
> >       -unevaluatedProperties: false
> >       +additionalProperties: true
> >
> >      would fix that, and is mostly harmless, except for possible
> >      conflicts with other Ethernet PHYs having more than one clock, or
> >      using different clock-names.
> >      Documentation/devicetree/bindings/net/qca,ar803x.yaml has the same
> >      issue.
> >      Is there a proper way to handle this?  Are there other options than
> >      mandating specific compatible values for Ethernet PHYs?
>
> The proper way is simply, if you need to describe your phy in DT, it
> needs a compatible string. MDIO phys are not special.

So that's gonna be a bunch of "ethernet-phy-id0022.*" values,
especially as the least significant nibble is the revision number...

> We really need to split ethernet-phy.yaml into common properties and a
> specific schema for the compatibles it contains so that we can change
> 'additionalProperties: true'. That's one reason why all these properties
> and typos didn't get flagged.
>
> If you don't want to retro-actively add a compatible, you can also do
> something like this:
>
> select:
>   anyOf:
>     - required: ['micrel,led-mode']
>     - required: ['micrel,rmii-reference-clock-select-25-mhz']
>     - required: ['micrel,fiber-mode']
>     - required: ['coma-mode-gpios']
>
> That doesn't catch every case nor if you have a typo in the property
> names.

Indeed.

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/micrel,phy.yaml

> > +  micrel,rmii-reference-clock-select-25-mhz:
> > +    description: |
> > +      RMII Reference Clock Select bit selects 25 MHz mode
> > +
> > +      Setting the RMII Reference Clock Select bit enables 25 MHz rather
> > +      than 50 MHz clock mode.
> > +
> > +      Note that this option in only needed for certain PHY revisions with a
> > +      non-standard, inverted function of this configuration bit.
> > +      Specifically, a clock reference ("rmii-ref" below) is always needed to
> > +      actually select a mode.
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: rmii-ref
> > +    description: |
> > +      supported clocks:
> > +        - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII reference
> > +          input clock. Used to determine the XI input clock.
>
> Don't repeat the clock name in the description.

Actually I kept it on purpose, as the driver treats the "rmii-ref" clock
differently than any other (unnamed) clock.  Obviously I failed to
relay that information, so I should enhance the description ;-)

> > +  coma-mode-gpios:
> > +    description: |
> > +      If present the given gpio will be deasserted when the PHY is probed.
> > +
> > +      Some PHYs have a COMA mode input pin which puts the PHY into
> > +      isolate and power-down mode. On some boards this input is connected
> > +      to a GPIO of the SoC.
> > +
> > +      Supported on the LAN8814.
>
> Another reason to add compatible. You have per device properties.

So I have to increase my datasheet library first, to discover all
the PHY IDs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

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