Message-ID: <b7d039b1-7554-47bb-93f4-98d2b08a5882@beims.me>
Date: Tue, 4 Nov 2025 11:56:49 -0300
From: Rafael Beims <rafael@...ms.me>
To: Geert Uytterhoeven <geert@...ux-m68k.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,
 Rafael Beims <rafael.beims@...adex.com>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH] [RFC] dt-bindings: net: micrel: Convert to json-schema

Hello Gert,

On 11/18/24 07:39, Geert Uytterhoeven wrote:
> 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

I would like to add a new property to the micrel dt-bindings. I 
understand that
the conversion to json-schema has to be finished before new properties 
are added.

You had this patch posted a while back, and reading the feedback, I'm 
not sure how to proceed.
It seems that forcing the use of compatible strings could be a bit 
risky. At least the "micrel,led-mode"
is used in many device-trees. If I understand the requirement correctly, 
it means we would have to find out
the exact PHY model that every one of these boards uses to not create a 
breaking change.
Other flags like "micrel,rmii-reference-clock-select-25-mhz" or 
"coma-mode-gpios" are more contained
and should be easier, but there would still be some risk to do this 
change without testing on actual hardware.
I couldn't find any use of the "micrel,fiber-mode"  flag in current 
device trees.

Did you find other issues that prevented you from moving forward with 
this? Is my thinking correct regarding
this, or am I missing something?

I appreciate any insight you could give me on this,

Rafael

Powered by blists - more mailing lists