Hi > Gesendet: Donnerstag, 05. Mai 2022 um 22:29 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@xxxxxxxxxx> > An: "Frank Wunderlich" <linux@xxxxxxxxx>, linux-mediatek@xxxxxxxxxxxxxxxxxxx > Cc: "Frank Wunderlich" <frank-w@xxxxxxxxxxxxxxx>, "Andrew Lunn" <andrew@xxxxxxx>, "Vivien Didelot" <vivien.didelot@xxxxxxxxx>, "Florian Fainelli" <f.fainelli@xxxxxxxxx>, "Vladimir Oltean" <olteanv@xxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, "Jakub Kicinski" <kuba@xxxxxxxxxx>, "Paolo Abeni" <pabeni@xxxxxxxxxx>, "Rob Herring" <robh+dt@xxxxxxxxxx>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@xxxxxxxxxx>, "Matthias Brugger" <matthias.bgg@xxxxxxxxx>, "Sean Wang" <sean.wang@xxxxxxxxxxxx>, "Landen Chao" <Landen.Chao@xxxxxxxxxxxx>, "DENG Qingfang" <dqfext@xxxxxxxxx>, netdev@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, "Greg Ungerer" <gerg@xxxxxxxxxx>, "René van Dorst" <opensource@xxxxxxxxxx>, "Mauro Carvalho Chehab" <mchehab+samsung@xxxxxxxxxx> > Betreff: Re: [RFC v2] dt-bindings: net: dsa: convert binding for mediatek switches > > On 05/05/2022 17:00, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> > > > > Convert txt binding to yaml binding for Mediatek switches. > > > > Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> > > Thank you for your patch. There is something to discuss/improve. > > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + core-supply: > > + description: | > > Drop | everywhere where it is not needed (so in all places, AFAICT) is it necessary for multiline-descriptions or is indentation enough? > > + Phandle to the regulator node necessary for the core power. > > + > > + "#gpio-cells": > > + description: | > > + Must be 2 if gpio-controller is defined. > > Skip description, it's obvious from the GPIO controller schema. ok > > + const: 2 > > + > > + gpio-controller: > > + type: boolean > > + description: | > > + if defined, MT7530's LED controller will run on GPIO mode. > > + > > + "#interrupt-cells": > > + const: 1 > > + > > + interrupt-controller: > > + type: boolean > > + > > + interrupts: > > + maxItems: 1 > > + > > + io-supply: > > + description: | > > + Phandle to the regulator node necessary for the I/O power. > > + See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt > > + for details for the regulator setup on these boards. > > + > > + mediatek,mcm: > > + type: boolean > > + description: | > > + if defined, indicates that either MT7530 is the part on multi-chip > > + module belong to MT7623A has or the remotely standalone chip as the > > + function MT7623N reference board provided for. > > + > > + reset-gpios: > > + description: | > > + Should be a gpio specifier for a reset line. > > Skip description. ok > > + maxItems: 1 > > + > > + reset-names: > > + description: | > > + Should be set to "mcm". > > Skip description. ok > > + const: mcm > > + > > + resets: > > + description: | > > + Phandle pointing to the system reset controller with line index for > > + the ethsys. > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +allOf: > > + - $ref: "dsa.yaml#" > > + - if: > > + required: > > + - mediatek,mcm > > + then: > > + required: > > + - resets > > + - reset-names > > + else: > > + required: > > + - reset-gpios > > + > > + - if: > > + required: > > + - interrupt-controller > > + then: > > + required: > > + - interrupts > > + > > + - if: > > + properties: > > + compatible: > > + items: > > + - const: mediatek,mt7530 > > + then: > > + required: > > + - core-supply > > + - io-supply > > + > > + > > +patternProperties: > > patternProperties go before allOf, just after regular properties. after required, right? > > + "^(ethernet-)?ports$": > > + type: object > > Also on this level: > unevaluatedProperties: false this is imho a bit redundant because in dsa.yaml (which is included now after patternProperties) it is already set on both levels. Adding it here will fail in examples because of size/address-cells which are already defined in dsa.yaml... so i need to define them here again. > > + > > + patternProperties: > > + "^(ethernet-)?port@[0-9]+$": > > + type: object > > + description: Ethernet switch ports > > + > > + properties: > > + reg: > > + description: | > > + Port address described must be 6 for CPU port and from 0 to 5 for user ports. > > This looks like not wrapped @80 character. i fix this > > + > > + unevaluatedProperties: false > > + > > + allOf: > > + - $ref: dsa-port.yaml# > > + - if: > > + properties: > > + label: > > + items: > > + - const: cpu > > + then: > > + required: > > + - reg > > + - phy-mode > > + > > +unevaluatedProperties: false > > Best regards, > Krzysztof regards Frank
