Messages in this thread | | | Date | Tue, 17 May 2022 20:54:59 -0500 | From | Rob Herring <> | Subject | Re: Re: [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches |
| |
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@kernel.org> > > On Sat, May 07, 2022 at 07:04:35PM +0200, Frank Wunderlich wrote: > > > From: Frank Wunderlich <frank-w@public-files.de> > > > > +++ 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
| |