diff mbox series

[v2] dt-bindings: net: orion-mdio: Convert to JSON schema

Message ID 20220505210621.3637268-1-chris.packham@alliedtelesis.co.nz (mailing list archive)
State Accepted
Commit 0781434af811f29a0808521533bedc4027685751
Delegated to: Netdev Maintainers
Headers show
Series [v2] dt-bindings: net: orion-mdio: Convert to JSON schema | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Chris Packham May 5, 2022, 9:06 p.m. UTC
Convert the marvell,orion-mdio binding to JSON schema.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    This does throw up the following dtbs_check warnings for turris-mox:
    
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@10:reg: [[16], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@2:reg: [[2], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@11:reg: [[17], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@2:reg: [[2], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@12:reg: [[18], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@2:reg: [[2], [0]] is too long
            From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
    
    I think they're all genuine but I'm hesitant to leap in and fix them
    without being able to test them.
    
    I also need to set unevaluatedProperties: true to cater for the L2
    switch on turris-mox (and probably others). That might be better tackled
    in the core mdio.yaml schema but I wasn't planning on touching that.
    
    Changes in v2:
    - Add Andrew as maintainer (thanks for volunteering)

 .../bindings/net/marvell,orion-mdio.yaml      | 60 +++++++++++++++++++
 .../bindings/net/marvell-orion-mdio.txt       | 54 -----------------
 2 files changed, 60 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt

Comments

Jakub Kicinski May 10, 2022, 12:28 a.m. UTC | #1
On Fri,  6 May 2022 09:06:20 +1200 Chris Packham wrote:
> Subject: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema

JSON or YAML?
Andrew Lunn May 10, 2022, 12:41 a.m. UTC | #2
On Mon, May 09, 2022 at 05:28:14PM -0700, Jakub Kicinski wrote:
> On Fri,  6 May 2022 09:06:20 +1200 Chris Packham wrote:
> > Subject: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema
> 
> JSON or YAML?

JAML is a superset of JSON. So it is not completely wrong. I've no
idea if this particular binding sticks to the subset which is JSON.

     Andrew
Jakub Kicinski May 10, 2022, 12:43 a.m. UTC | #3
On Tue, 10 May 2022 02:41:26 +0200 Andrew Lunn wrote:
> On Mon, May 09, 2022 at 05:28:14PM -0700, Jakub Kicinski wrote:
> > On Fri,  6 May 2022 09:06:20 +1200 Chris Packham wrote:  
> > > Subject: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema  
> > 
> > JSON or YAML?  
> 
> JAML is a superset of JSON. So it is not completely wrong. I've no
> idea if this particular binding sticks to the subset which is JSON.

I hope you mean s/JAML/YAML/ otherwise I'm completely confused :)
Chris Packham May 10, 2022, 12:47 a.m. UTC | #4
On 10/05/22 12:28, Jakub Kicinski wrote:
> On Fri,  6 May 2022 09:06:20 +1200 Chris Packham wrote:
>> Subject: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema
> JSON or YAML?

I based that on other commits at that do something similar and say 
"Convert to JSON schema", although I might be operating on old data.

Looking again now with `git log --oneline --no-merges --grep onvert -- 
Documentation/devicetree/bindings/` I see "Convert binding to YAML" and 
"convert to dtschema" (still a few json-schema in there).

I guess technically the binding is written in YAML but forms part of 
schema which is ultimately consumed by jsonschema but perhaps the use of 
jsonschema is an implementation detail that doesn't require mentioning.

I'm happy to reword the subject and send a v3 if people feel strongly 
about it.
Andrew Lunn May 10, 2022, 11:12 p.m. UTC | #5
On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
> Convert the marvell,orion-mdio binding to JSON schema.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
patchwork-bot+netdevbpf@kernel.org May 11, 2022, 2:20 a.m. UTC | #6
Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  6 May 2022 09:06:20 +1200 you wrote:
> Convert the marvell,orion-mdio binding to JSON schema.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>     This does throw up the following dtbs_check warnings for turris-mox:
> 
> [...]

Here is the summary with links:
  - [v2] dt-bindings: net: orion-mdio: Convert to JSON schema
    https://git.kernel.org/netdev/net-next/c/0781434af811

You are awesome, thank you!
Rob Herring May 16, 2022, 6:14 p.m. UTC | #7
On Tue, May 10, 2022 at 02:41:26AM +0200, Andrew Lunn wrote:
> On Mon, May 09, 2022 at 05:28:14PM -0700, Jakub Kicinski wrote:
> > On Fri,  6 May 2022 09:06:20 +1200 Chris Packham wrote:
> > > Subject: [PATCH v2] dt-bindings: net: orion-mdio: Convert to JSON schema
> > 
> > JSON or YAML?

I actually prefer 'DT schema format' which implicitly means json-schema 
vocabulary in YAML formatted files (among other constraints we place on 
them).

> 
> JAML is a superset of JSON. So it is not completely wrong. I've no
> idea if this particular binding sticks to the subset which is JSON.

I think you just invented a new term for 'JSON compatible YAML subset'.

Rob
Rob Herring May 16, 2022, 6:16 p.m. UTC | #8
On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
> Convert the marvell,orion-mdio binding to JSON schema.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>     This does throw up the following dtbs_check warnings for turris-mox:
>     
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@10:reg: [[16], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@2:reg: [[2], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@11:reg: [[17], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@2:reg: [[2], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@12:reg: [[18], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@2:reg: [[2], [0]] is too long
>             From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>     
>     I think they're all genuine but I'm hesitant to leap in and fix them
>     without being able to test them.
>     
>     I also need to set unevaluatedProperties: true to cater for the L2
>     switch on turris-mox (and probably others). That might be better tackled
>     in the core mdio.yaml schema but I wasn't planning on touching that.
>     
>     Changes in v2:
>     - Add Andrew as maintainer (thanks for volunteering)
> 
>  .../bindings/net/marvell,orion-mdio.yaml      | 60 +++++++++++++++++++
>  .../bindings/net/marvell-orion-mdio.txt       | 54 -----------------
>  2 files changed, 60 insertions(+), 54 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> new file mode 100644
> index 000000000000..fe3a3412f093
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,orion-mdio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell MDIO Ethernet Controller interface
> +
> +maintainers:
> +  - Andrew Lunn <andrew@lunn.ch>
> +
> +description: |
> +  The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, MV78xx0,
> +  Armada 370, Armada XP, Armada 7k and Armada 8k have an identical unit that
> +  provides an interface with the MDIO bus. Additionally, Armada 7k and Armada
> +  8k has a second unit which provides an interface with the xMDIO bus. This
> +  driver handles these interfaces.
> +
> +allOf:
> +  - $ref: "mdio.yaml#"
> +
> +properties:
> +  compatible:
> +    enum:
> +      - marvell,orion-mdio
> +      - marvell,xmdio
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 4

Really this should be better defined, but the original was not.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: true

This must be false.

> +
> +examples:
> +  - |
> +    mdio@d0072004 {
> +      compatible = "marvell,orion-mdio";
> +      reg = <0xd0072004 0x4>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      interrupts = <30>;
> +
> +      phy0: ethernet-phy@0 {
> +        reg = <0>;
> +      };
> +
> +      phy1: ethernet-phy@1 {
> +        reg = <1>;
> +      };
> +    };
> diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> deleted file mode 100644
> index 3f3cfc1d8d4d..000000000000
> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Marvell MDIO Ethernet Controller interface
> -
> -The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
> -MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
> -identical unit that provides an interface with the MDIO bus.
> -Additionally, Armada 7k and Armada 8k has a second unit which
> -provides an interface with the xMDIO bus. This driver handles
> -these interfaces.
> -
> -Required properties:
> -- compatible: "marvell,orion-mdio" or "marvell,xmdio"
> -- reg: address and length of the MDIO registers.  When an interrupt is
> -  not present, the length is the size of the SMI register (4 bytes)
> -  otherwise it must be 0x84 bytes to cover the interrupt control
> -  registers.
> -
> -Optional properties:
> -- interrupts: interrupt line number for the SMI error/done interrupt
> -- clocks: phandle for up to four required clocks for the MDIO instance
> -
> -The child nodes of the MDIO driver are the individual PHY devices
> -connected to this MDIO bus. They must have a "reg" property given the
> -PHY address on the MDIO bus.
> -
> -Example at the SoC level without an interrupt property:
> -
> -mdio {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	compatible = "marvell,orion-mdio";
> -	reg = <0xd0072004 0x4>;
> -};
> -
> -Example with an interrupt property:
> -
> -mdio {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	compatible = "marvell,orion-mdio";
> -	reg = <0xd0072004 0x84>;
> -	interrupts = <30>;
> -};
> -
> -And at the board level:
> -
> -mdio {
> -	phy0: ethernet-phy@0 {
> -		reg = <0>;
> -	};
> -
> -	phy1: ethernet-phy@1 {
> -		reg = <1>;
> -	};
> -}
> -- 
> 2.36.0
> 
>
Chris Packham May 16, 2022, 10:18 p.m. UTC | #9
Hi Rob,

On 17/05/22 06:16, Rob Herring wrote:
> On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
>> Convert the marvell,orion-mdio binding to JSON schema.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      This does throw up the following dtbs_check warnings for turris-mox:
>>      
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@10:reg: [[16], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch0@2:reg: [[2], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@11:reg: [[17], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch1@2:reg: [[2], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@12:reg: [[18], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: switch2@2:reg: [[2], [0]] is too long
>>              From schema: Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>      
>>      I think they're all genuine but I'm hesitant to leap in and fix them
>>      without being able to test them.
>>      
>>      I also need to set unevaluatedProperties: true to cater for the L2
>>      switch on turris-mox (and probably others). That might be better tackled
>>      in the core mdio.yaml schema but I wasn't planning on touching that.
>>      
>>      Changes in v2:
>>      - Add Andrew as maintainer (thanks for volunteering)
>>
>>   .../bindings/net/marvell,orion-mdio.yaml      | 60 +++++++++++++++++++
>>   .../bindings/net/marvell-orion-mdio.txt       | 54 -----------------
>>   2 files changed, 60 insertions(+), 54 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> new file mode 100644
>> index 000000000000..fe3a3412f093
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_bkDWJOhEQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fnet%2fmarvell%2corion-mdio%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_e4CDcetEw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Marvell MDIO Ethernet Controller interface
>> +
>> +maintainers:
>> +  - Andrew Lunn <andrew@lunn.ch>
>> +
>> +description: |
>> +  The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, MV78xx0,
>> +  Armada 370, Armada XP, Armada 7k and Armada 8k have an identical unit that
>> +  provides an interface with the MDIO bus. Additionally, Armada 7k and Armada
>> +  8k has a second unit which provides an interface with the xMDIO bus. This
>> +  driver handles these interfaces.
>> +
>> +allOf:
>> +  - $ref: "mdio.yaml#"
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - marvell,orion-mdio
>> +      - marvell,xmdio
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 4
> Really this should be better defined, but the original was not.
>
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +unevaluatedProperties: true
> This must be false.

Right now there is no way (that I have found) of dealing with non-PHY 
devices like the dsa switches so setting this to false generates 
warnings on turris-mox:

arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: 
Unevaluated properties are not allowed ('#address-cells', '#size-cells', 
'ethernet-phy@1', 'switch0@10', 'switch0@2', 'switch1@11', 'switch1@2', 
'switch2@12', 'switch2@2' were unexpected)
         From schema: 
/home/chrisp/src/linux/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml

There are also warnings about the size of the reg property but these 
seem to be genuine problems that Marek is looking at.

This change has already been picked up for net-next but if you have 
suggestions for improvement I'm happy to submit them as additional 
changes on-top of that.

>
>> +
>> +examples:
>> +  - |
>> +    mdio@d0072004 {
>> +      compatible = "marvell,orion-mdio";
>> +      reg = <0xd0072004 0x4>;
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      interrupts = <30>;
>> +
>> +      phy0: ethernet-phy@0 {
>> +        reg = <0>;
>> +      };
>> +
>> +      phy1: ethernet-phy@1 {
>> +        reg = <1>;
>> +      };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>> deleted file mode 100644
>> index 3f3cfc1d8d4d..000000000000
>> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>> +++ /dev/null
>> @@ -1,54 +0,0 @@
>> -* Marvell MDIO Ethernet Controller interface
>> -
>> -The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
>> -MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
>> -identical unit that provides an interface with the MDIO bus.
>> -Additionally, Armada 7k and Armada 8k has a second unit which
>> -provides an interface with the xMDIO bus. This driver handles
>> -these interfaces.
>> -
>> -Required properties:
>> -- compatible: "marvell,orion-mdio" or "marvell,xmdio"
>> -- reg: address and length of the MDIO registers.  When an interrupt is
>> -  not present, the length is the size of the SMI register (4 bytes)
>> -  otherwise it must be 0x84 bytes to cover the interrupt control
>> -  registers.
>> -
>> -Optional properties:
>> -- interrupts: interrupt line number for the SMI error/done interrupt
>> -- clocks: phandle for up to four required clocks for the MDIO instance
>> -
>> -The child nodes of the MDIO driver are the individual PHY devices
>> -connected to this MDIO bus. They must have a "reg" property given the
>> -PHY address on the MDIO bus.
>> -
>> -Example at the SoC level without an interrupt property:
>> -
>> -mdio {
>> -	#address-cells = <1>;
>> -	#size-cells = <0>;
>> -	compatible = "marvell,orion-mdio";
>> -	reg = <0xd0072004 0x4>;
>> -};
>> -
>> -Example with an interrupt property:
>> -
>> -mdio {
>> -	#address-cells = <1>;
>> -	#size-cells = <0>;
>> -	compatible = "marvell,orion-mdio";
>> -	reg = <0xd0072004 0x84>;
>> -	interrupts = <30>;
>> -};
>> -
>> -And at the board level:
>> -
>> -mdio {
>> -	phy0: ethernet-phy@0 {
>> -		reg = <0>;
>> -	};
>> -
>> -	phy1: ethernet-phy@1 {
>> -		reg = <1>;
>> -	};
>> -}
>> -- 
>> 2.36.0
>>
>>
Chris Packham May 16, 2022, 10:27 p.m. UTC | #10
On 17/05/22 10:18, Chris Packham wrote:
> Hi Rob,
>
> On 17/05/22 06:16, Rob Herring wrote:
>> On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
>>> Convert the marvell,orion-mdio binding to JSON schema.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>>      This does throw up the following dtbs_check warnings for 
>>> turris-mox:
>>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: 
>>> switch0@10:reg: [[16], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
>>> mdio@32004: switch0@2:reg: [[2], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
>>> mdio@32004: switch1@11:reg: [[17], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
>>> mdio@32004: switch1@2:reg: [[2], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
>>> mdio@32004: switch2@12:reg: [[18], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
>>> mdio@32004: switch2@2:reg: [[2], [0]] is too long
>>>              From schema: 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>           I think they're all genuine but I'm hesitant to leap in 
>>> and fix them
>>>      without being able to test them.
>>>           I also need to set unevaluatedProperties: true to cater 
>>> for the L2
>>>      switch on turris-mox (and probably others). That might be 
>>> better tackled
>>>      in the core mdio.yaml schema but I wasn't planning on touching 
>>> that.
>>>           Changes in v2:
>>>      - Add Andrew as maintainer (thanks for volunteering)
>>>
>>>   .../bindings/net/marvell,orion-mdio.yaml      | 60 
>>> +++++++++++++++++++
>>>   .../bindings/net/marvell-orion-mdio.txt       | 54 -----------------
>>>   2 files changed, 60 insertions(+), 54 deletions(-)
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>>   delete mode 100644 
>>> Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml 
>>> b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> new file mode 100644
>>> index 000000000000..fe3a3412f093
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>>> @@ -0,0 +1,60 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: 
>>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_bkDWJOhEQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fnet%2fmarvell%2corion-mdio%2eyaml%23
>>> +$schema: 
>>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_e4CDcetEw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>> +
>>> +title: Marvell MDIO Ethernet Controller interface
>>> +
>>> +maintainers:
>>> +  - Andrew Lunn <andrew@lunn.ch>
>>> +
>>> +description: |
>>> +  The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, 
>>> MV78xx0,
>>> +  Armada 370, Armada XP, Armada 7k and Armada 8k have an identical 
>>> unit that
>>> +  provides an interface with the MDIO bus. Additionally, Armada 7k 
>>> and Armada
>>> +  8k has a second unit which provides an interface with the xMDIO 
>>> bus. This
>>> +  driver handles these interfaces.
>>> +
>>> +allOf:
>>> +  - $ref: "mdio.yaml#"
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - marvell,orion-mdio
>>> +      - marvell,xmdio
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    minItems: 1
>>> +    maxItems: 4
>> Really this should be better defined, but the original was not.
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +unevaluatedProperties: true
>> This must be false.
>
> Right now there is no way (that I have found) of dealing with non-PHY 
> devices like the dsa switches so setting this to false generates 
> warnings on turris-mox:
>
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: 
> Unevaluated properties are not allowed ('#address-cells', 
> '#size-cells', 'ethernet-phy@1', 'switch0@10', 'switch0@2', 
> 'switch1@11', 'switch1@2', 'switch2@12', 'switch2@2' were unexpected)
>         From schema: 
> /home/chrisp/src/linux/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
>
> There are also warnings about the size of the reg property but these 
> seem to be genuine problems that Marek is looking at.
Actually it looks if I fix the reg problem the need for 
unevaluatedProperties goes away. I'll whip up a small patch series on 
top of net-next.
>
> This change has already been picked up for net-next but if you have 
> suggestions for improvement I'm happy to submit them as additional 
> changes on-top of that.
>
>>
>>> +
>>> +examples:
>>> +  - |
>>> +    mdio@d0072004 {
>>> +      compatible = "marvell,orion-mdio";
>>> +      reg = <0xd0072004 0x4>;
>>> +      #address-cells = <1>;
>>> +      #size-cells = <0>;
>>> +      interrupts = <30>;
>>> +
>>> +      phy0: ethernet-phy@0 {
>>> +        reg = <0>;
>>> +      };
>>> +
>>> +      phy1: ethernet-phy@1 {
>>> +        reg = <1>;
>>> +      };
>>> +    };
>>> diff --git 
>>> a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt 
>>> b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>> deleted file mode 100644
>>> index 3f3cfc1d8d4d..000000000000
>>> --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
>>> +++ /dev/null
>>> @@ -1,54 +0,0 @@
>>> -* Marvell MDIO Ethernet Controller interface
>>> -
>>> -The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
>>> -MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
>>> -identical unit that provides an interface with the MDIO bus.
>>> -Additionally, Armada 7k and Armada 8k has a second unit which
>>> -provides an interface with the xMDIO bus. This driver handles
>>> -these interfaces.
>>> -
>>> -Required properties:
>>> -- compatible: "marvell,orion-mdio" or "marvell,xmdio"
>>> -- reg: address and length of the MDIO registers.  When an interrupt is
>>> -  not present, the length is the size of the SMI register (4 bytes)
>>> -  otherwise it must be 0x84 bytes to cover the interrupt control
>>> -  registers.
>>> -
>>> -Optional properties:
>>> -- interrupts: interrupt line number for the SMI error/done interrupt
>>> -- clocks: phandle for up to four required clocks for the MDIO instance
>>> -
>>> -The child nodes of the MDIO driver are the individual PHY devices
>>> -connected to this MDIO bus. They must have a "reg" property given the
>>> -PHY address on the MDIO bus.
>>> -
>>> -Example at the SoC level without an interrupt property:
>>> -
>>> -mdio {
>>> -    #address-cells = <1>;
>>> -    #size-cells = <0>;
>>> -    compatible = "marvell,orion-mdio";
>>> -    reg = <0xd0072004 0x4>;
>>> -};
>>> -
>>> -Example with an interrupt property:
>>> -
>>> -mdio {
>>> -    #address-cells = <1>;
>>> -    #size-cells = <0>;
>>> -    compatible = "marvell,orion-mdio";
>>> -    reg = <0xd0072004 0x84>;
>>> -    interrupts = <30>;
>>> -};
>>> -
>>> -And at the board level:
>>> -
>>> -mdio {
>>> -    phy0: ethernet-phy@0 {
>>> -        reg = <0>;
>>> -    };
>>> -
>>> -    phy1: ethernet-phy@1 {
>>> -        reg = <1>;
>>> -    };
>>> -}
>>> -- 
>>> 2.36.0
>>>
>>>
Rob Herring May 18, 2022, 4:18 p.m. UTC | #11
On Mon, May 16, 2022 at 10:27:42PM +0000, Chris Packham wrote:
> 
> On 17/05/22 10:18, Chris Packham wrote:
> > Hi Rob,
> >
> > On 17/05/22 06:16, Rob Herring wrote:
> >> On Fri, May 06, 2022 at 09:06:20AM +1200, Chris Packham wrote:
> >>> Convert the marvell,orion-mdio binding to JSON schema.
> >>>
> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> >>> ---
> >>>
> >>> Notes:
> >>>      This does throw up the following dtbs_check warnings for 
> >>> turris-mox:
> >>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: 
> >>> switch0@10:reg: [[16], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
> >>> mdio@32004: switch0@2:reg: [[2], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
> >>> mdio@32004: switch1@11:reg: [[17], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
> >>> mdio@32004: switch1@2:reg: [[2], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
> >>> mdio@32004: switch2@12:reg: [[18], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>      arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: 
> >>> mdio@32004: switch2@2:reg: [[2], [0]] is too long
> >>>              From schema: 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>           I think they're all genuine but I'm hesitant to leap in 
> >>> and fix them
> >>>      without being able to test them.
> >>>           I also need to set unevaluatedProperties: true to cater 
> >>> for the L2
> >>>      switch on turris-mox (and probably others). That might be 
> >>> better tackled
> >>>      in the core mdio.yaml schema but I wasn't planning on touching 
> >>> that.
> >>>           Changes in v2:
> >>>      - Add Andrew as maintainer (thanks for volunteering)
> >>>
> >>>   .../bindings/net/marvell,orion-mdio.yaml      | 60 
> >>> +++++++++++++++++++
> >>>   .../bindings/net/marvell-orion-mdio.txt       | 54 -----------------
> >>>   2 files changed, 60 insertions(+), 54 deletions(-)
> >>>   create mode 100644 
> >>> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>>   delete mode 100644 
> >>> Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
> >>>
> >>> diff --git 
> >>> a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml 
> >>> b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>> new file mode 100644
> >>> index 000000000000..fe3a3412f093
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >>> @@ -0,0 +1,60 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: 
> >>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_bkDWJOhEQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fnet%2fmarvell%2corion-mdio%2eyaml%23
> >>> +$schema: 
> >>> http://scanmail.trustwave.com/?c=20988&d=j5WC4r_pZnDMILajH1kaKLL9oC7kQjgv_e4CDcetEw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
> >>> +
> >>> +title: Marvell MDIO Ethernet Controller interface
> >>> +
> >>> +maintainers:
> >>> +  - Andrew Lunn <andrew@lunn.ch>
> >>> +
> >>> +description: |
> >>> +  The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, 
> >>> MV78xx0,
> >>> +  Armada 370, Armada XP, Armada 7k and Armada 8k have an identical 
> >>> unit that
> >>> +  provides an interface with the MDIO bus. Additionally, Armada 7k 
> >>> and Armada
> >>> +  8k has a second unit which provides an interface with the xMDIO 
> >>> bus. This
> >>> +  driver handles these interfaces.
> >>> +
> >>> +allOf:
> >>> +  - $ref: "mdio.yaml#"
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - marvell,orion-mdio
> >>> +      - marvell,xmdio
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  interrupts:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    minItems: 1
> >>> +    maxItems: 4
> >> Really this should be better defined, but the original was not.
> >>
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +
> >>> +unevaluatedProperties: true
> >> This must be false.
> >
> > Right now there is no way (that I have found) of dealing with non-PHY 
> > devices like the dsa switches so setting this to false generates 
> > warnings on turris-mox:
> >
> > arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dtb: mdio@32004: 
> > Unevaluated properties are not allowed ('#address-cells', 
> > '#size-cells', 'ethernet-phy@1', 'switch0@10', 'switch0@2', 
> > 'switch1@11', 'switch1@2', 'switch2@12', 'switch2@2' were unexpected)
> >         From schema: 
> > /home/chrisp/src/linux/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
> >
> > There are also warnings about the size of the reg property but these 
> > seem to be genuine problems that Marek is looking at.
> Actually it looks if I fix the reg problem the need for 
> unevaluatedProperties goes away. I'll whip up a small patch series on 
> top of net-next.

Yeah, it's confusing. It seems 'evaluated and failed' counts as 
unevaluated. Seems to be a quirk of json-schema itself rather than the 
specific implementation AFAICT.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
new file mode 100644
index 000000000000..fe3a3412f093
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml
@@ -0,0 +1,60 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,orion-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell MDIO Ethernet Controller interface
+
+maintainers:
+  - Andrew Lunn <andrew@lunn.ch>
+
+description: |
+  The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x, MV78xx0,
+  Armada 370, Armada XP, Armada 7k and Armada 8k have an identical unit that
+  provides an interface with the MDIO bus. Additionally, Armada 7k and Armada
+  8k has a second unit which provides an interface with the xMDIO bus. This
+  driver handles these interfaces.
+
+allOf:
+  - $ref: "mdio.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - marvell,orion-mdio
+      - marvell,xmdio
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: true
+
+examples:
+  - |
+    mdio@d0072004 {
+      compatible = "marvell,orion-mdio";
+      reg = <0xd0072004 0x4>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      interrupts = <30>;
+
+      phy0: ethernet-phy@0 {
+        reg = <0>;
+      };
+
+      phy1: ethernet-phy@1 {
+        reg = <1>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
deleted file mode 100644
index 3f3cfc1d8d4d..000000000000
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ /dev/null
@@ -1,54 +0,0 @@ 
-* Marvell MDIO Ethernet Controller interface
-
-The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
-MV78xx0, Armada 370, Armada XP, Armada 7k and Armada 8k have an
-identical unit that provides an interface with the MDIO bus.
-Additionally, Armada 7k and Armada 8k has a second unit which
-provides an interface with the xMDIO bus. This driver handles
-these interfaces.
-
-Required properties:
-- compatible: "marvell,orion-mdio" or "marvell,xmdio"
-- reg: address and length of the MDIO registers.  When an interrupt is
-  not present, the length is the size of the SMI register (4 bytes)
-  otherwise it must be 0x84 bytes to cover the interrupt control
-  registers.
-
-Optional properties:
-- interrupts: interrupt line number for the SMI error/done interrupt
-- clocks: phandle for up to four required clocks for the MDIO instance
-
-The child nodes of the MDIO driver are the individual PHY devices
-connected to this MDIO bus. They must have a "reg" property given the
-PHY address on the MDIO bus.
-
-Example at the SoC level without an interrupt property:
-
-mdio {
-	#address-cells = <1>;
-	#size-cells = <0>;
-	compatible = "marvell,orion-mdio";
-	reg = <0xd0072004 0x4>;
-};
-
-Example with an interrupt property:
-
-mdio {
-	#address-cells = <1>;
-	#size-cells = <0>;
-	compatible = "marvell,orion-mdio";
-	reg = <0xd0072004 0x84>;
-	interrupts = <30>;
-};
-
-And at the board level:
-
-mdio {
-	phy0: ethernet-phy@0 {
-		reg = <0>;
-	};
-
-	phy1: ethernet-phy@1 {
-		reg = <1>;
-	};
-}