Re: [PATCH v2 3/7] dt-bindings: net: dsa: mediatek,mt7530: update examples

From: Arınç ÜNAL
Date: Tue Aug 16 2022 - 18:16:56 EST

On 17.08.2022 00:02, Rob Herring wrote:
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?

For wrong reasons as it seems. Will revert that one.



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.

Will remove.


+ 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.

Will fix.


+ #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.

It seemed to me it would be useful to reference the reset line for the MT7623AI SoC. Using mediatek,mcm and especially MT2701_ETHSYS_MCM_RST in dt-bindings/reset/mt2701-resets.h.

Should I remove anyway?


+ - |
+ #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 = <&ethsys 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.

Will remove.


};
- mdio: mdio-bus {
+ ethernet {
#address-cells = <1>;
#size-cells = <0>;