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

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

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