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

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

[Geert Uytterhoeven]

Convert the Micrel PHY Device Tree binding documentation to json-schema.

Add a simple example.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.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?

Thanks for your comments!
---
 .../devicetree/bindings/net/micrel,phy.yaml   | 93 +++++++++++++++++++
 .../devicetree/bindings/net/micrel.txt        | 57 ------------
 2 files changed, 93 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel,phy.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/micrel.txt

diff --git a/Documentation/devicetree/bindings/net/micrel,phy.yaml b/Documentation/devicetree/bindings/net/micrel,phy.yaml
new file mode 100644
index 0000000000000000..609bbd9729efe516
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/micrel,phy.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/micrel,phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Micrel PHY properties
+
+maintainers:
+  - Ben Dooks <ben.dooks@codethink.co.uk>
+
+properties:
+  micrel,led-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3 ]
+    description: |
+      LED mode value to set for PHYs with configurable LEDs.
+
+      Configure the LED mode with single value. The list of PHYs and the
+      bits that are currently supported:
+
+      KSZ8001: register 0x1e, bits 15..14
+      KSZ8041: register 0x1e, bits 15..14
+      KSZ8021: register 0x1f, bits 5..4
+      KSZ8031: register 0x1f, bits 5..4
+      KSZ8051: register 0x1f, bits 5..4
+      KSZ8081: register 0x1f, bits 5..4
+      KSZ8091: register 0x1f, bits 5..4
+      LAN8814: register EP5.0, bit 6
+
+      See the respective PHY datasheet for the mode values.
+
+  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.
+
+  micrel,fiber-mode:
+    type: boolean
+    description: |
+      If present the PHY is configured to operate in fiber mode.
+
+      Some PHYs, such as the KSZ8041FTL variant, support fiber mode, enabled
+      by the FXEN boot strapping pin. It can't be determined from the PHY
+      registers whether the PHY is in fiber mode, so this boolean device tree
+      property can be used to describe it.
+
+      In fiber mode, auto-negotiation is disabled and the PHY can only work in
+      100base-fx (full and half duplex) modes.
+
+  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.
+
+dependencies:
+  micrel,rmii-reference-clock-select-25-mhz: [ clock-names ]
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy@1 {
+            reg = <1>;
+            micrel,led-mode = <1>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
deleted file mode 100644
index a407dd1b461459a6..0000000000000000
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-Micrel PHY properties.
-
-These properties cover the base properties Micrel PHYs.
-
-Optional properties:
-
- - micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
-
-	Configure the LED mode with single value. The list of PHYs and the
-	bits that are currently supported:
-
-	KSZ8001: register 0x1e, bits 15..14
-	KSZ8041: register 0x1e, bits 15..14
-	KSZ8021: register 0x1f, bits 5..4
-	KSZ8031: register 0x1f, bits 5..4
-	KSZ8051: register 0x1f, bits 5..4
-	KSZ8081: register 0x1f, bits 5..4
-	KSZ8091: register 0x1f, bits 5..4
-	LAN8814: register EP5.0, bit 6
-
-	See the respective PHY datasheet for the mode values.
-
- - micrel,rmii-reference-clock-select-25-mhz: 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, clock-names: contains clocks according to the common clock bindings.
-
-	supported clocks:
-	- KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII reference
-	  input clock. Used to determine the XI input clock.
-
- - micrel,fiber-mode: If present the PHY is configured to operate in fiber mode
-
-	Some PHYs, such as the KSZ8041FTL variant, support fiber mode, enabled
-	by the FXEN boot strapping pin. It can't be determined from the PHY
-	registers whether the PHY is in fiber mode, so this boolean device tree
-	property can be used to describe it.
-
-	In fiber mode, auto-negotiation is disabled and the PHY can only work in
-	100base-fx (full and half duplex) modes.
-
- - coma-mode-gpios: 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.
-- 
2.34.1

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

[Rob Herring]

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@glider.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.

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.

> Thanks for your comments!
> ---
>  .../devicetree/bindings/net/micrel,phy.yaml   | 93 +++++++++++++++++++
>  .../devicetree/bindings/net/micrel.txt        | 57 ------------
>  2 files changed, 93 insertions(+), 57 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/micrel,phy.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/micrel.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/micrel,phy.yaml b/Documentation/devicetree/bindings/net/micrel,phy.yaml
> new file mode 100644
> index 0000000000000000..609bbd9729efe516
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/micrel,phy.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/micrel,phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Micrel PHY properties
> +
> +maintainers:
> +  - Ben Dooks <ben.dooks@codethink.co.uk>
> +
> +properties:
> +  micrel,led-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [ 0, 1, 2, 3 ]
> +    description: |
> +      LED mode value to set for PHYs with configurable LEDs.
> +
> +      Configure the LED mode with single value. The list of PHYs and the
> +      bits that are currently supported:
> +
> +      KSZ8001: register 0x1e, bits 15..14
> +      KSZ8041: register 0x1e, bits 15..14
> +      KSZ8021: register 0x1f, bits 5..4
> +      KSZ8031: register 0x1f, bits 5..4
> +      KSZ8051: register 0x1f, bits 5..4
> +      KSZ8081: register 0x1f, bits 5..4
> +      KSZ8091: register 0x1f, bits 5..4
> +      LAN8814: register EP5.0, bit 6
> +
> +      See the respective PHY datasheet for the mode values.
> +
> +  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.

> +
> +  micrel,fiber-mode:
> +    type: boolean
> +    description: |
> +      If present the PHY is configured to operate in fiber mode.
> +
> +      Some PHYs, such as the KSZ8041FTL variant, support fiber mode, enabled
> +      by the FXEN boot strapping pin. It can't be determined from the PHY
> +      registers whether the PHY is in fiber mode, so this boolean device tree
> +      property can be used to describe it.
> +
> +      In fiber mode, auto-negotiation is disabled and the PHY can only work in
> +      100base-fx (full and half duplex) modes.
> +
> +  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.

> +
> +dependencies:
> +  micrel,rmii-reference-clock-select-25-mhz: [ clock-names ]
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    ethernet {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ethernet-phy@1 {
> +            reg = <1>;
> +            micrel,led-mode = <1>;
> +        };
> +    };

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

[Geert Uytterhoeven]

Hi Rob,

On Fri, Nov 15, 2024 at 4:02 PM Rob Herring <robh@kernel.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@glider.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@linux-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

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

[Rafael Beims]

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@kernel.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@glider.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