Hi Rob,
On Fri, Nov 15, 2024 at 4:02 PM Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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@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
