On Wed, May 11, 2022 at 10:02:45AM +0200, Frank Wunderlich wrote: > Hi > > thanks for review > > > Gesendet: Dienstag, 10. Mai 2022 um 20:44 Uhr > > Von: "Rob Herring" <robh@xxxxxxxxxx> > > On Sat, May 07, 2022 at 07:04:35PM +0200, Frank Wunderlich wrote: > > > From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> > > > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > > > +properties: > > > + compatible: > > > + enum: > > > + - mediatek,mt7530 > > > + - mediatek,mt7531 > > > + - mediatek,mt7621 > > > + > > > > > + "#address-cells": > > > + const: 1 > > > + > > > + "#size-cells": > > > + const: 0 > > > > I don't see any child nodes with addresses, so these can be removed. > > dropping this (and address-cells/size-cells from examples) causes errors like this (address-/size-cells set in mdio > node, so imho it should inherite): I think you are off a level. > Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dts:34.25-35: Warning (reg_format): > /example-0/mdio/switch@0/ports/port@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) That's for yet another level where 'ports' node need the properties. > Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' > > > > + interrupt-controller: > > > + type: boolean > > > > Already has a type. Just: > > > > interrupt-controller: true > > > > > + > > > + interrupts: > > > + maxItems: 1 > > > > +patternProperties: > > > + "^(ethernet-)?ports$": > > > + type: object > > > > additionalProperties: false > > imho this will block address-/size-cells from this level too. looks like it is needed here too (for port-regs). > > > > + > > > + patternProperties: > > > + "^(ethernet-)?port@[0-9]+$": > > > + type: object > > > + description: Ethernet switch ports > > > + > > > + unevaluatedProperties: false > > > + > > > + properties: > > > + reg: > > > + description: > > > + Port address described must be 5 or 6 for CPU port and from 0 > > > + to 5 for user ports. > > > + > > > + allOf: > > > + - $ref: dsa-port.yaml# > > > + - if: > > > + properties: > > > + label: > > > + items: > > > + - const: cpu > > > + then: > > > + required: > > > + - reg > > > + - phy-mode > > > + > > > > + - if: > > > + required: > > > + - interrupt-controller > > > + then: > > > + required: > > > + - interrupts > > > > This can be expressed as: > > > > dependencies: > > interrupt-controller: [ interrupts ] > > ok, i will change this > > > > + ports { > > > > Use the preferred form: ethernet-ports > > current implementation in all existing dts and examples from old binding are "ports" only. > should they changed too? They don't have to be the schema allows both, but the example should be best practice. Rob