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

[Chris Packham <Chris.Packham@alliedtelesis.co.nz>]

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
>>
>>