On Sat, Aug 13, 2022 at 06:44:11PM +0300, Arınç ÜNAL wrote: > Update the examples on the binding. > > - Add examples which include a wide variation of configurations. > - Make example comments YAML comment instead of DT binding comment. > - Define examples from platform to make the bindings clearer. > - Add interrupt controller to the examples. Include header file for > interrupt. > - Change reset line for MT7621 examples. > - Pretty formatting for the examples. > - Change switch reg to 0. > - Change port labels to fit the example, change port 4 label to wan. > - Change ethernet-ports to ports. Again, why? > > Signed-off-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx> > --- > .../bindings/net/dsa/mediatek,mt7530.yaml | 663 +++++++++++++----- > 1 file changed, 502 insertions(+), 161 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 4c99266ce82a..cc87f48d4d07 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -210,144 +210,374 @@ allOf: > unevaluatedProperties: false > > examples: > + # Example 1: Standalone MT7530 > - | > #include <dt-bindings/gpio/gpio.h> > - mdio { > - #address-cells = <1>; > - #size-cells = <0>; > - switch@0 { > - compatible = "mediatek,mt7530"; > - reg = <0>; > - > - core-supply = <&mt6323_vpa_reg>; > - io-supply = <&mt6323_vemc3v3_reg>; > - reset-gpios = <&pio 33 GPIO_ACTIVE_HIGH>; > - > - ethernet-ports { > + > + platform { > + ethernet { Don't need these nodes. > + mdio { > #address-cells = <1>; > #size-cells = <0>; > - port@0 { > + > + switch@0 { > + compatible = "mediatek,mt7530"; > reg = <0>; > - label = "lan0"; > - }; > > - port@1 { > - reg = <1>; > - label = "lan1"; > - }; > + reset-gpios = <&pio 33 0>; > > - port@2 { > - reg = <2>; > - label = "lan2"; > - }; > + core-supply = <&mt6323_vpa_reg>; > + io-supply = <&mt6323_vemc3v3_reg>; > + > + ports { 'ports' is for the DT graph binding. 'ethernet-ports' is for DSA binding. The former is allowed due to existing users. Don't add more. > + #address-cells = <1>; > + #size-cells = <0>; > > - port@3 { > - reg = <3>; > - label = "lan3"; > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > + > + port@4 { > + reg = <4>; > + label = "wan"; > + }; > + > + port@6 { > + reg = <6>; > + label = "cpu"; > + ethernet = <&gmac0>; > + phy-mode = "rgmii"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + pause; > + }; > + }; > + }; > }; > + }; > + }; > + }; > > - port@4 { > - reg = <4>; > - label = "wan"; > + # Example 2: MT7530 in MT7623AI SoC Looks almost the same as example 1. Examples are not an enumeration of every possible DT. Limit them to cases which are significantly different. > + - | > + #include <dt-bindings/reset/mt2701-resets.h> > + > + platform { > + ethernet { > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + switch@0 { > + compatible = "mediatek,mt7530"; > + reg = <0>; > + > + mediatek,mcm; > + resets = <ðsys MT2701_ETHSYS_MCM_RST>; > + reset-names = "mcm"; > + > + core-supply = <&mt6323_vpa_reg>; > + io-supply = <&mt6323_vemc3v3_reg>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > + > + port@4 { > + reg = <4>; > + label = "wan"; > + }; > + > + port@6 { > + reg = <6>; > + label = "cpu"; > + ethernet = <&gmac0>; > + phy-mode = "trgmii"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + pause; > + }; > + }; > + }; > }; > + }; > + }; > + }; > + > + # Example 3: Standalone MT7531 > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + platform { > + ethernet { > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + switch@0 { > + compatible = "mediatek,mt7531"; > + reg = <0>; > + > + reset-gpios = <&pio 54 0>; > + > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupt-parent = <&pio>; > + interrupts = <53 IRQ_TYPE_LEVEL_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > > - port@6 { > - reg = <6>; > - label = "cpu"; > - ethernet = <&gmac0>; > - phy-mode = "trgmii"; > - fixed-link { > - speed = <1000>; > - full-duplex; > + port@4 { > + reg = <4>; > + label = "wan"; > + }; > + > + port@6 { > + reg = <6>; > + label = "cpu"; > + ethernet = <&gmac0>; > + phy-mode = "2500base-x"; > + > + fixed-link { > + speed = <2500>; > + full-duplex; > + pause; > + }; > + }; > }; > }; > }; > }; > }; > > + # Example 4: MT7530 in MT7621AT, MT7621DAT and MT7621ST SoCs > - | > - //Example 2: MT7621: Port 4 is WAN port: 2nd GMAC -> Port 5 -> PHY port 4. > - > - ethernet { > - #address-cells = <1>; > - #size-cells = <0>; > - gmac0: mac@0 { > - compatible = "mediatek,eth-mac"; > - reg = <0>; > - phy-mode = "rgmii"; > - > - fixed-link { > - speed = <1000>; > - full-duplex; > - pause; > + #include <dt-bindings/interrupt-controller/mips-gic.h> > + #include <dt-bindings/reset/mt7621-reset.h> > + > + platform { > + ethernet { > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + switch@0 { > + compatible = "mediatek,mt7621"; > + reg = <0>; > + > + mediatek,mcm; > + resets = <&sysc MT7621_RST_MCM>; > + reset-names = "mcm"; > + > + interrupt-controller; > + #interrupt-cells = <1>; > + interrupt-parent = <&gic>; > + interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + label = "lan1"; > + }; > + > + port@1 { > + reg = <1>; > + label = "lan2"; > + }; > + > + port@2 { > + reg = <2>; > + label = "lan3"; > + }; > + > + port@3 { > + reg = <3>; > + label = "lan4"; > + }; > + > + port@4 { > + reg = <4>; > + label = "wan"; > + }; > + > + port@6 { > + reg = <6>; > + label = "cpu"; > + ethernet = <&gmac0>; > + phy-mode = "trgmii"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + pause; > + }; > + }; > + }; > + }; > }; > }; > + }; > > - gmac1: mac@1 { > - compatible = "mediatek,eth-mac"; > - reg = <1>; > - phy-mode = "rgmii-txid"; > - phy-handle = <&phy4>; > + # Example 5: MT7621: mux MT7530's phy4 to SoC's gmac1 > + - | > + #include <dt-bindings/interrupt-controller/mips-gic.h> > + #include <dt-bindings/reset/mt7621-reset.h> > + > + platform { > + pinctrl { > + example5_rgmii2_pins: rgmii2-pins { > + pinmux { > + groups = "rgmii2"; > + function = "rgmii2"; > + }; > + }; No need to put this in the example. We don't put provide nodes in the examples of the consumers. It's also incomplete and can't be validated. > }; > > - mdio: mdio-bus { > + ethernet { > #address-cells = <1>; > #size-cells = <0>;