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

[Rob Herring <robh@kernel.org>]

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