linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] completely rework mediatek,mt7530 binding
@ 2022-07-30 14:26 Arınç ÜNAL
  2022-07-30 14:26 ` [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes Arınç ÜNAL
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-30 14:26 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, Arınç ÜNAL

Hello.

This patch series brings complete rework of the mediatek,mt7530 binding.

The binding is checked with "make dt_binding_check
DT_SCHEMA_FILES=mediatek,mt7530.yaml".

If anyone knows the GIC bit for interrupt for multi-chip module MT7530 in
MT7623AI SoC, let me know. I'll add it to the examples.

If anyone got a Unielec U7623 or another MT7623AI board, please reach out.

Arınç ÜNAL (4):
  dt-bindings: net: dsa: mediatek,mt7530: make trivial changes
  dt-bindings: net: dsa: mediatek,mt7530: update examples
  dt-bindings: net: dsa: mediatek,mt7530: update binding description
  dt-bindings: net: dsa: mediatek,mt7530: update json-schema

 .../bindings/net/dsa/mediatek,mt7530.yaml       | 1006 +++++++++++++-----
 1 file changed, 764 insertions(+), 242 deletions(-)



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes
  2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
@ 2022-07-30 14:26 ` Arınç ÜNAL
  2022-08-02  8:41   ` Krzysztof Kozlowski
  2022-07-30 14:26 ` [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples Arınç ÜNAL
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-30 14:26 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, Arınç ÜNAL

Make trivial changes on the binding.

- Update title to include MT7531 switch.
- Add me as a maintainer. List maintainers in alphabetical order by first
name.
- Add description to compatible strings.
- Fix MCM description. mediatek,mcm is not used on MT7623NI.
- Add description for reset-gpios.
- Remove quotes from $ref: "dsa.yaml#".

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 27 ++++++++++++++-----
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 17ab6c69ecc7..541984a7d2d4 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -4,12 +4,13 @@
 $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mediatek MT7530 Ethernet switch
+title: Mediatek MT7530 and MT7531 Ethernet Switches
 
 maintainers:
-  - Sean Wang <sean.wang@mediatek.com>
+  - Arınç ÜNAL <arinc.unal@arinc9.com>
   - Landen Chao <Landen.Chao@mediatek.com>
   - DENG Qingfang <dqfext@gmail.com>
+  - Sean Wang <sean.wang@mediatek.com>
 
 description: |
   Port 5 of mt7530 and mt7621 switch is muxed between:
@@ -66,6 +67,14 @@ properties:
       - mediatek,mt7531
       - mediatek,mt7621
 
+    description: |
+      mediatek,mt7530:
+        For standalone MT7530 and multi-chip module MT7530 in MT7623AI SoC.
+      mediatek,mt7531:
+        For standalone MT7531.
+      mediatek,mt7621:
+        For multi-chip module MT7530 in MT7621AT, MT7621DAT and MT7621ST SoCs.
+
   reg:
     maxItems: 1
 
@@ -79,7 +88,7 @@ properties:
   gpio-controller:
     type: boolean
     description:
-      if defined, MT7530's LED controller will run on GPIO mode.
+      If defined, MT7530's LED controller will run on GPIO mode.
 
   "#interrupt-cells":
     const: 1
@@ -98,11 +107,15 @@ properties:
   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.
+      Used for MT7621AT, MT7621DAT, MT7621ST and MT7623AI SoCs which the MT7530
+      switch is a part of the multi-chip module.
 
   reset-gpios:
+    description:
+      GPIO to reset the switch. Use this if mediatek,mcm is not used.
+      This property is optional because some boards share the reset line with
+      other components which makes it impossible to probe the switch if the
+      reset line is used.
     maxItems: 1
 
   reset-names:
@@ -148,7 +161,7 @@ required:
   - reg
 
 allOf:
-  - $ref: "dsa.yaml#"
+  - $ref: dsa.yaml#
   - if:
       required:
         - mediatek,mcm
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples
  2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
  2022-07-30 14:26 ` [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes Arınç ÜNAL
@ 2022-07-30 14:26 ` Arınç ÜNAL
  2022-08-02  8:43   ` Krzysztof Kozlowski
  2022-07-30 14:26 ` [PATCH 3/4] dt-bindings: net: dsa: mediatek,mt7530: update binding description Arınç ÜNAL
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-30 14:26 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, Arınç ÜNAL

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.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 661 +++++++++++++-----
 1 file changed, 500 insertions(+), 161 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 541984a7d2d4..479e292cb2af 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -186,144 +186,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 {
+            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 {
+                        #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
+  - |
+    #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";
+                };
+            };
         };
 
-        mdio: mdio-bus {
+        ethernet {
             #address-cells = <1>;
             #size-cells = <0>;
 
-            /* Internal phy */
-            phy4: ethernet-phy@4 {
-                reg = <4>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&example5_rgmii2_pins>;
+
+            mac@1 {
+                compatible = "mediatek,eth-mac";
+                reg = <1>;
+
+                phy-mode = "rgmii";
+                phy-handle = <&example5_ethphy4>;
             };
 
-            mt7530: switch@1f {
-                compatible = "mediatek,mt7621";
-                reg = <0x1f>;
-                mediatek,mcm;
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
 
-                resets = <&rstctrl 2>;
-                reset-names = "mcm";
+                /* MT7530's phy4 */
+                example5_ethphy4: ethernet-phy@4 {
+                    reg = <4>;
+                };
 
-                ethernet-ports {
-                    #address-cells = <1>;
-                    #size-cells = <0>;
+                switch@0 {
+                    compatible = "mediatek,mt7621";
+                    reg = <0>;
 
-                    port@0 {
-                        reg = <0>;
-                        label = "lan0";
-                    };
+                    mediatek,mcm;
+                    resets = <&sysc MT7621_RST_MCM>;
+                    reset-names = "mcm";
 
-                    port@1 {
-                        reg = <1>;
-                        label = "lan1";
-                    };
+                    interrupt-controller;
+                    #interrupt-cells = <1>;
+                    interrupt-parent = <&gic>;
+                    interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>;
 
-                    port@2 {
-                        reg = <2>;
-                        label = "lan2";
-                    };
+                    ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
 
-                    port@3 {
-                        reg = <3>;
-                        label = "lan3";
-                    };
+                        port@0 {
+                            reg = <0>;
+                            label = "lan1";
+                        };
 
-                    /* Commented out. Port 4 is handled by 2nd GMAC.
-                    port@4 {
-                        reg = <4>;
-                        label = "lan4";
-                    };
-                    */
+                        port@1 {
+                            reg = <1>;
+                            label = "lan2";
+                        };
 
-                    port@6 {
-                        reg = <6>;
-                        label = "cpu";
-                        ethernet = <&gmac0>;
-                        phy-mode = "rgmii";
+                        port@2 {
+                            reg = <2>;
+                            label = "lan3";
+                        };
 
-                        fixed-link {
-                            speed = <1000>;
-                            full-duplex;
-                            pause;
+                        port@3 {
+                            reg = <3>;
+                            label = "lan4";
+                        };
+
+                        /* Commented out, phy4 is muxed to gmac1.
+                        port@4 {
+                            reg = <4>;
+                            label = "wan";
+                        };
+                        */
+
+                        port@6 {
+                            reg = <6>;
+                            label = "cpu";
+                            ethernet = <&gmac0>;
+                            phy-mode = "trgmii";
+
+                            fixed-link {
+                                speed = <1000>;
+                                full-duplex;
+                                pause;
+                            };
                         };
                     };
                 };
@@ -331,87 +561,196 @@ examples:
         };
     };
 
+  # Example 6: MT7621: mux external phy to SoC's gmac1
   - |
-    //Example 3: MT7621: Port 5 is connected to external PHY: Port 5 -> external PHY.
-
-    ethernet {
-        #address-cells = <1>;
-        #size-cells = <0>;
-        gmac_0: 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 {
+        pinctrl {
+            example6_rgmii2_pins: rgmii2-pins {
+                pinmux {
+                    groups = "rgmii2";
+                    function = "rgmii2";
+                };
             };
         };
 
-        mdio0: mdio-bus {
+        ethernet {
             #address-cells = <1>;
             #size-cells = <0>;
 
-            /* External phy */
-            ephy5: ethernet-phy@7 {
-                reg = <7>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&example6_rgmii2_pins>;
+
+            mac@1 {
+                compatible = "mediatek,eth-mac";
+                reg = <1>;
+
+                phy-mode = "rgmii";
             };
 
-            switch@1f {
-                compatible = "mediatek,mt7621";
-                reg = <0x1f>;
-                mediatek,mcm;
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
 
-                resets = <&rstctrl 2>;
-                reset-names = "mcm";
+                /* External PHY */
+                ethernet-phy@7 {
+                    reg = <7>;
+                    phy-mode = "rgmii";
+                };
 
-                ethernet-ports {
-                    #address-cells = <1>;
-                    #size-cells = <0>;
+                switch@0 {
+                    compatible = "mediatek,mt7621";
+                    reg = <0>;
 
-                    port@0 {
-                        reg = <0>;
-                        label = "lan0";
-                    };
+                    mediatek,mcm;
+                    resets = <&sysc MT7621_RST_MCM>;
+                    reset-names = "mcm";
 
-                    port@1 {
-                        reg = <1>;
-                        label = "lan1";
-                    };
+                    interrupt-controller;
+                    #interrupt-cells = <1>;
+                    interrupt-parent = <&gic>;
+                    interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>;
 
-                    port@2 {
-                        reg = <2>;
-                        label = "lan2";
-                    };
+                    ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
 
-                    port@3 {
-                        reg = <3>;
-                        label = "lan3";
-                    };
+                        port@0 {
+                            reg = <0>;
+                            label = "lan1";
+                        };
 
-                    port@4 {
-                        reg = <4>;
-                        label = "lan4";
-                    };
+                        port@1 {
+                            reg = <1>;
+                            label = "lan2";
+                        };
+
+                        port@2 {
+                            reg = <2>;
+                            label = "lan3";
+                        };
 
-                    port@5 {
-                        reg = <5>;
-                        label = "lan5";
-                        phy-mode = "rgmii";
-                        phy-handle = <&ephy5>;
+                        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 7: MT7621: mux external phy to MT7530's port 5
+  - |
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+    #include <dt-bindings/reset/mt7621-reset.h>
+
+    platform {
+        pinctrl {
+            example7_rgmii2_pins: rgmii2-pins {
+                pinmux {
+                    groups = "rgmii2";
+                    function = "gpio";
+                };
+            };
+        };
+
+        ethernet {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            pinctrl-names = "default";
+            pinctrl-0 = <&example7_rgmii2_pins>;
 
-                    cpu_port0: port@6 {
-                        reg = <6>;
-                        label = "cpu";
-                        ethernet = <&gmac_0>;
-                        phy-mode = "rgmii";
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                /* External PHY */
+                ethernet-phy@7 {
+                    reg = <7>;
+                    phy-mode = "rgmii";
+                };
+
+                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@5 {
+                            reg = <5>;
+                            label = "extphy";
+                            phy-mode = "rgmii-txid";
+                        };
 
-                        fixed-link {
-                            speed = <1000>;
-                            full-duplex;
-                            pause;
+                        port@6 {
+                            reg = <6>;
+                            label = "cpu";
+                            ethernet = <&gmac0>;
+                            phy-mode = "trgmii";
+
+                            fixed-link {
+                                speed = <1000>;
+                                full-duplex;
+                                pause;
+                            };
                         };
                     };
                 };
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] dt-bindings: net: dsa: mediatek,mt7530: update binding description
  2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
  2022-07-30 14:26 ` [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes Arınç ÜNAL
  2022-07-30 14:26 ` [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples Arınç ÜNAL
@ 2022-07-30 14:26 ` Arınç ÜNAL
  2022-07-30 14:26 ` [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema Arınç ÜNAL
  2022-07-31  8:53 ` [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
  4 siblings, 0 replies; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-30 14:26 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, Arınç ÜNAL

Update the description of the binding.

- Describe the switches, which SoCs they are in, or if they are standalone.
- Explain the various ways of configuring MT7530's port 5.
- Remove phy-mode = "rgmii-txid" from description. Same code path is
followed for delayed rgmii and rgmii phy-mode on mtk_eth_soc.c.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 98 ++++++++++---------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 479e292cb2af..a88e650e910b 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -13,52 +13,60 @@ maintainers:
   - Sean Wang <sean.wang@mediatek.com>
 
 description: |
-  Port 5 of mt7530 and mt7621 switch is muxed between:
-  1. GMAC5: GMAC5 can interface with another external MAC or PHY.
-  2. PHY of port 0 or port 4: PHY interfaces with an external MAC like 2nd GMAC
-     of the SOC. Used in many setups where port 0/4 becomes the WAN port.
-     Note: On a MT7621 SOC with integrated switch: 2nd GMAC can only connected to
-       GMAC5 when the gpios for RGMII2 (GPIO 22-33) are not used and not
-       connected to external component!
-
-  Port 5 modes/configurations:
-  1. Port 5 is disabled and isolated: An external phy can interface to the 2nd
-     GMAC of the SOC.
-     In the case of a build-in MT7530 switch, port 5 shares the RGMII bus with 2nd
-     GMAC and an optional external phy. Mind the GPIO/pinctl settings of the SOC!
-  2. Port 5 is muxed to PHY of port 0/4: Port 0/4 interfaces with 2nd GMAC.
-     It is a simple MAC to PHY interface, port 5 needs to be setup for xMII mode
-     and RGMII delay.
-  3. Port 5 is muxed to GMAC5 and can interface to an external phy.
-     Port 5 becomes an extra switch port.
-     Only works on platform where external phy TX<->RX lines are swapped.
-     Like in the Ubiquiti ER-X-SFP.
-  4. Port 5 is muxed to GMAC5 and interfaces with the 2nd GAMC as 2nd CPU port.
-     Currently a 2nd CPU port is not supported by DSA code.
-
-  Depending on how the external PHY is wired:
-  1. normal: The PHY can only connect to 2nd GMAC but not to the switch
-  2. swapped: RGMII TX, RX are swapped; external phy interface with the switch as
-     a ethernet port. But can't interface to the 2nd GMAC.
-
-    Based on the DT the port 5 mode is configured.
-
-  Driver tries to lookup the phy-handle of the 2nd GMAC of the master device.
-  When phy-handle matches PHY of port 0 or 4 then port 5 set-up as mode 2.
-  phy-mode must be set, see also example 2 below!
-  * mt7621: phy-mode = "rgmii-txid";
-  * mt7623: phy-mode = "rgmii";
-
-  CPU-Ports need a phy-mode property:
-    Allowed values on mt7530 and mt7621:
-      - "rgmii"
-      - "trgmii"
-    On mt7531:
-      - "1000base-x"
-      - "2500base-x"
-      - "rgmii"
-      - "sgmii"
+  There are two versions of MT7530, standalone and in a multi-chip module.
 
+  MT7530 is a part of the multi-chip module in MT7620AN, MT7620DA, MT7620DAN,
+  MT7620NN, MT7621AT, MT7621DAT, MT7621ST and MT7623AI SoCs.
+
+  MT7530 in MT7620AN, MT7620DA, MT7620DAN and MT7620NN SoCs has got 10/100 PHYs
+  and the switch registers are directly mapped into SoC's memory map rather than
+  using MDIO. There is currently no support for this.
+
+  There is only the standalone version of MT7531.
+
+  Port 5 on MT7530 has got various ways of configuration.
+
+  For standalone MT7530:
+
+    - Port 5 can be used as a DSA master.
+
+    - PHY 0 or 4 of the switch can be muxed to connect to the gmac of the SoC
+      which port 5 is wired to. Usually used for connecting the wan port
+      directly to the CPU to achieve 2 Gbps routing in total.
+      The phy-handle property on the gmac node must refer to the phy node.
+
+      The driver requires the gmac of the SoC to have "mediatek,eth-mac" as the
+      compatible string and the reg must be 1. So, for now, only gmac1 of an
+      MediaTek SoC can benefit this. Banana Pi BPI-R2 for example.
+      Check out example 5 for a similar configuration.
+
+    - Port 5 can be wired to an external phy. Port 5 becomes a DSA slave.
+      Check out example 7 for a similar configuration.
+
+  For multi-chip module MT7530:
+
+    - Port 5 can be used as a DSA master.
+
+    - PHY 0 or 4 of the switch can be muxed to connect to gmac1 of the SoC.
+      Usually used for connecting the wan port directly to the CPU to achieve 2
+      Gbps routing in total.
+      The phy-handle property on the gmac node must refer to the phy node.
+
+      For the MT7621 SoCs, rgmii2 group must be claimed with rgmii2 function.
+      Check out example 5.
+
+    - In case of an external phy wired to gmac1 of the SoC, port 5 must not be
+      enabled.
+
+      For the MT7621 SoCs, rgmii2 group must be claimed with rgmii2 function.
+      Check out example 6.
+
+    - Port 5 can be muxed to an external phy. Port 5 becomes a DSA slave.
+      The external phy must be wired TX to TX to gmac1 of the SoC for this to
+      work. Ubiquiti EdgeRouter X SFP for example.
+
+      For the MT7621 SoCs, rgmii2 group must be claimed with gpio function.
+      Check out example 7.
 
 properties:
   compatible:
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
                   ` (2 preceding siblings ...)
  2022-07-30 14:26 ` [PATCH 3/4] dt-bindings: net: dsa: mediatek,mt7530: update binding description Arınç ÜNAL
@ 2022-07-30 14:26 ` Arınç ÜNAL
  2022-08-02  8:46   ` Krzysztof Kozlowski
  2022-07-31  8:53 ` [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
  4 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-30 14:26 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, Arınç ÜNAL

Update the json-schema for compatible devices.

- Define acceptable phy-mode values for CPU port of each compatible device.
- Remove requiring the "reg" property since the referred dsa-port.yaml
already does that.
- Require mediatek,mcm for the described MT7621 SoCs as the compatible
string is only used for MT7530 which is a part of the multi-chip module.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 220 +++++++++++++++---
 1 file changed, 191 insertions(+), 29 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index a88e650e910b..a37a14fba9f6 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -135,35 +135,6 @@ properties:
       the ethsys.
     maxItems: 1
 
-patternProperties:
-  "^(ethernet-)?ports$":
-    type: object
-
-    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
-
 required:
   - compatible
   - reg
@@ -187,10 +158,201 @@ allOf:
           items:
             - const: mediatek,mt7530
     then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          type: object
+
+          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:
+                    allOf:
+                      - if:
+                          properties:
+                            reg:
+                              const: 5
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - gmii
+                                - mii
+                                - rgmii
+
+                      - if:
+                          properties:
+                            reg:
+                              const: 6
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - rgmii
+                                - trgmii
+
+                    properties:
+                      reg:
+                        enum:
+                          - 5
+                          - 6
+
+                    required:
+                      - phy-mode
+
       required:
         - core-supply
         - io-supply
 
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: mediatek,mt7531
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          type: object
+
+          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:
+                    allOf:
+                      - if:
+                          properties:
+                            reg:
+                              const: 5
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - 1000base-x
+                                - 2500base-x
+                                - rgmii
+                                - sgmii
+
+                      - if:
+                          properties:
+                            reg:
+                              const: 6
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - 1000base-x
+                                - 2500base-x
+                                - sgmii
+
+                    properties:
+                      reg:
+                        enum:
+                          - 5
+                          - 6
+
+                    required:
+                      - phy-mode
+
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: mediatek,mt7621
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          type: object
+
+          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:
+                    allOf:
+                      - if:
+                          properties:
+                            reg:
+                              const: 5
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - gmii
+                                - mii
+                                - rgmii
+
+                      - if:
+                          properties:
+                            reg:
+                              const: 6
+                        then:
+                          properties:
+                            phy-mode:
+                              enum:
+                                - rgmii
+                                - trgmii
+
+                    properties:
+                      reg:
+                        enum:
+                          - 5
+                          - 6
+
+                    required:
+                      - phy-mode
+
+      required:
+        - mediatek,mcm
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/4] completely rework mediatek,mt7530 binding
  2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
                   ` (3 preceding siblings ...)
  2022-07-30 14:26 ` [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema Arınç ÜNAL
@ 2022-07-31  8:53 ` Arınç ÜNAL
  4 siblings, 0 replies; 17+ messages in thread
From: Arınç ÜNAL @ 2022-07-31  8:53 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Frank Wunderlich,
	Luiz Angelo Daros de Luca, Sander Vanheule, René van Dorst,
	Daniel Golle, erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

This is for net-next, I forgot to include it in the subject.

Arınç

On 30.07.2022 17:26, Arınç ÜNAL wrote:
> Hello.
> 
> This patch series brings complete rework of the mediatek,mt7530 binding.
> 
> The binding is checked with "make dt_binding_check
> DT_SCHEMA_FILES=mediatek,mt7530.yaml".
> 
> If anyone knows the GIC bit for interrupt for multi-chip module MT7530 in
> MT7623AI SoC, let me know. I'll add it to the examples.
> 
> If anyone got a Unielec U7623 or another MT7623AI board, please reach out.
> 
> Arınç ÜNAL (4):
>    dt-bindings: net: dsa: mediatek,mt7530: make trivial changes
>    dt-bindings: net: dsa: mediatek,mt7530: update examples
>    dt-bindings: net: dsa: mediatek,mt7530: update binding description
>    dt-bindings: net: dsa: mediatek,mt7530: update json-schema
> 
>   .../bindings/net/dsa/mediatek,mt7530.yaml       | 1006 +++++++++++++-----
>   1 file changed, 764 insertions(+), 242 deletions(-)
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes
  2022-07-30 14:26 ` [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes Arınç ÜNAL
@ 2022-08-02  8:41   ` Krzysztof Kozlowski
  2022-08-11 22:09     ` Arınç ÜNAL
  0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-02  8:41 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 30/07/2022 16:26, Arınç ÜNAL wrote:
> Make trivial changes on the binding.
> 
> - Update title to include MT7531 switch.
> - Add me as a maintainer. List maintainers in alphabetical order by first
> name.
> - Add description to compatible strings.
> - Fix MCM description. mediatek,mcm is not used on MT7623NI.
> - Add description for reset-gpios.
> - Remove quotes from $ref: "dsa.yaml#".
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/net/dsa/mediatek,mt7530.yaml     | 27 ++++++++++++++-----
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index 17ab6c69ecc7..541984a7d2d4 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -4,12 +4,13 @@
>  $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Mediatek MT7530 Ethernet switch
> +title: Mediatek MT7530 and MT7531 Ethernet Switches
>  
>  maintainers:
> -  - Sean Wang <sean.wang@mediatek.com>
> +  - Arınç ÜNAL <arinc.unal@arinc9.com>
>    - Landen Chao <Landen.Chao@mediatek.com>
>    - DENG Qingfang <dqfext@gmail.com>
> +  - Sean Wang <sean.wang@mediatek.com>
>  
>  description: |
>    Port 5 of mt7530 and mt7621 switch is muxed between:
> @@ -66,6 +67,14 @@ properties:
>        - mediatek,mt7531
>        - mediatek,mt7621
>  
> +    description: |
> +      mediatek,mt7530:
> +        For standalone MT7530 and multi-chip module MT7530 in MT7623AI SoC.
> +      mediatek,mt7531:
> +        For standalone MT7531.
> +      mediatek,mt7621:
> +        For multi-chip module MT7530 in MT7621AT, MT7621DAT and MT7621ST SoCs.

If compatible: is changed to oneOf, you can use description for each
item. Look at board compatibles (arm/fsl.yaml)

> +
>    reg:
>      maxItems: 1
>  
> @@ -79,7 +88,7 @@ properties:
>    gpio-controller:
>      type: boolean
>      description:
> -      if defined, MT7530's LED controller will run on GPIO mode.
> +      If defined, MT7530's LED controller will run on GPIO mode.
>  
>    "#interrupt-cells":
>      const: 1
> @@ -98,11 +107,15 @@ properties:
>    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.
> +      Used for MT7621AT, MT7621DAT, MT7621ST and MT7623AI SoCs which the MT7530
> +      switch is a part of the multi-chip module.

Does this mean it is valid only on these variants? If yes, this should
have a "mediatek,mcm:false" in allOf:if:then as separate patch (with
this change in description).

>  
>    reset-gpios:
> +    description:
> +      GPIO to reset the switch. Use this if mediatek,mcm is not used.

The same. Example:
https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/mfd/samsung,s5m8767.yaml#L155

> +      This property is optional because some boards share the reset line with
> +      other components which makes it impossible to probe the switch if the
> +      reset line is used.
>      maxItems: 1
>  
>    reset-names:
> @@ -148,7 +161,7 @@ required:
>    - reg
>  
>  allOf:
> -  - $ref: "dsa.yaml#"
> +  - $ref: dsa.yaml#
>    - if:
>        required:
>          - mediatek,mcm


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples
  2022-07-30 14:26 ` [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples Arınç ÜNAL
@ 2022-08-02  8:43   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-02  8:43 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 30/07/2022 16:26, 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.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/net/dsa/mediatek,mt7530.yaml     | 661 +++++++++++++-----
>  1 file changed, 500 insertions(+), 161 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index 541984a7d2d4..479e292cb2af 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -186,144 +186,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 {
> +            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 {
> +                        #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
> +  - |
> +    #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

This is the same as previous... actually several examples are similar.
Do not add examples for each compatible - this is not DTS. Add only one
example for significant differences.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-07-30 14:26 ` [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema Arınç ÜNAL
@ 2022-08-02  8:46   ` Krzysztof Kozlowski
  2022-08-11 22:09     ` Arınç ÜNAL
  0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-02  8:46 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 30/07/2022 16:26, Arınç ÜNAL wrote:
> Update the json-schema for compatible devices.
> 
> - Define acceptable phy-mode values for CPU port of each compatible device.
> - Remove requiring the "reg" property since the referred dsa-port.yaml
> already does that.
> - Require mediatek,mcm for the described MT7621 SoCs as the compatible
> string is only used for MT7530 which is a part of the multi-chip module.

3 separate patches.

> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/net/dsa/mediatek,mt7530.yaml     | 220 +++++++++++++++---
>  1 file changed, 191 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index a88e650e910b..a37a14fba9f6 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -135,35 +135,6 @@ properties:
>        the ethsys.
>      maxItems: 1
>  
> -patternProperties:
> -  "^(ethernet-)?ports$":
> -    type: object

Actually four patches...

I don't find this change explained in commit msg. What is more, it looks
incorrect. All properties and patternProperties should be explained in
top-level part.

Defining such properties (with big piece of YAML) in each if:then: is no
readable.

> -
> -    patternProperties:


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes
  2022-08-02  8:41   ` Krzysztof Kozlowski
@ 2022-08-11 22:09     ` Arınç ÜNAL
  0 siblings, 0 replies; 17+ messages in thread
From: Arınç ÜNAL @ 2022-08-11 22:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 2.08.2022 11:41, Krzysztof Kozlowski wrote:
> On 30/07/2022 16:26, Arınç ÜNAL wrote:
>> Make trivial changes on the binding.
>>
>> - Update title to include MT7531 switch.
>> - Add me as a maintainer. List maintainers in alphabetical order by first
>> name.
>> - Add description to compatible strings.
>> - Fix MCM description. mediatek,mcm is not used on MT7623NI.
>> - Add description for reset-gpios.
>> - Remove quotes from $ref: "dsa.yaml#".
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>>   .../bindings/net/dsa/mediatek,mt7530.yaml     | 27 ++++++++++++++-----
>>   1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> index 17ab6c69ecc7..541984a7d2d4 100644
>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> @@ -4,12 +4,13 @@
>>   $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>   
>> -title: Mediatek MT7530 Ethernet switch
>> +title: Mediatek MT7530 and MT7531 Ethernet Switches
>>   
>>   maintainers:
>> -  - Sean Wang <sean.wang@mediatek.com>
>> +  - Arınç ÜNAL <arinc.unal@arinc9.com>
>>     - Landen Chao <Landen.Chao@mediatek.com>
>>     - DENG Qingfang <dqfext@gmail.com>
>> +  - Sean Wang <sean.wang@mediatek.com>
>>   
>>   description: |
>>     Port 5 of mt7530 and mt7621 switch is muxed between:
>> @@ -66,6 +67,14 @@ properties:
>>         - mediatek,mt7531
>>         - mediatek,mt7621
>>   
>> +    description: |
>> +      mediatek,mt7530:
>> +        For standalone MT7530 and multi-chip module MT7530 in MT7623AI SoC.
>> +      mediatek,mt7531:
>> +        For standalone MT7531.
>> +      mediatek,mt7621:
>> +        For multi-chip module MT7530 in MT7621AT, MT7621DAT and MT7621ST SoCs.
> 
> If compatible: is changed to oneOf, you can use description for each
> item. Look at board compatibles (arm/fsl.yaml)

Will do, thanks for the example.

> 
>> +
>>     reg:
>>       maxItems: 1
>>   
>> @@ -79,7 +88,7 @@ properties:
>>     gpio-controller:
>>       type: boolean
>>       description:
>> -      if defined, MT7530's LED controller will run on GPIO mode.
>> +      If defined, MT7530's LED controller will run on GPIO mode.
>>   
>>     "#interrupt-cells":
>>       const: 1
>> @@ -98,11 +107,15 @@ properties:
>>     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.
>> +      Used for MT7621AT, MT7621DAT, MT7621ST and MT7623AI SoCs which the MT7530
>> +      switch is a part of the multi-chip module.
> 
> Does this mean it is valid only on these variants? If yes, this should
> have a "mediatek,mcm:false" in allOf:if:then as separate patch (with
> this change in description).

Only valid for those, yes. I'll make it false in allOf:if:then for 
mediatek,mt7531. It either can or can't be used for mediatek,mt7530 so 
nothing to do there.

> 
>>   
>>     reset-gpios:
>> +    description:
>> +      GPIO to reset the switch. Use this if mediatek,mcm is not used.
> 
> The same. Example:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/mfd/samsung,s5m8767.yaml#L155

Thanks, I'll make it false in allOf:if:then for mediatek,mcm.

Arınç

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-02  8:46   ` Krzysztof Kozlowski
@ 2022-08-11 22:09     ` Arınç ÜNAL
  2022-08-12  6:57       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-08-11 22:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 2.08.2022 11:46, Krzysztof Kozlowski wrote:
> On 30/07/2022 16:26, Arınç ÜNAL wrote:
>> Update the json-schema for compatible devices.
>>
>> - Define acceptable phy-mode values for CPU port of each compatible device.
>> - Remove requiring the "reg" property since the referred dsa-port.yaml
>> already does that.
>> - Require mediatek,mcm for the described MT7621 SoCs as the compatible
>> string is only used for MT7530 which is a part of the multi-chip module.
> 
> 3 separate patches.

Roger.

> 
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>>   .../bindings/net/dsa/mediatek,mt7530.yaml     | 220 +++++++++++++++---
>>   1 file changed, 191 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> index a88e650e910b..a37a14fba9f6 100644
>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>> @@ -135,35 +135,6 @@ properties:
>>         the ethsys.
>>       maxItems: 1
>>   
>> -patternProperties:
>> -  "^(ethernet-)?ports$":
>> -    type: object
> 
> Actually four patches...
> 
> I don't find this change explained in commit msg. What is more, it looks
> incorrect. All properties and patternProperties should be explained in
> top-level part.
> 
> Defining such properties (with big piece of YAML) in each if:then: is no
> readable.

I can't figure out another way. I need to require certain properties for 
a compatible string AND certain enum/const for certain properties which 
are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading 
the compatible string.

If I put allOf:if:then under patternProperties, I can't do the latter.

Other than readability to human eyes, binding check works as intended, 
in case there's no other way to do it.

Arınç

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-11 22:09     ` Arınç ÜNAL
@ 2022-08-12  6:57       ` Krzysztof Kozlowski
  2022-08-12  7:01         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-12  6:57 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>> -patternProperties:
>>> -  "^(ethernet-)?ports$":
>>> -    type: object
>>
>> Actually four patches...
>>
>> I don't find this change explained in commit msg. What is more, it looks
>> incorrect. All properties and patternProperties should be explained in
>> top-level part.
>>
>> Defining such properties (with big piece of YAML) in each if:then: is no
>> readable.
> 
> I can't figure out another way. I need to require certain properties for 
> a compatible string AND certain enum/const for certain properties which 
> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading 
> the compatible string.

requiring properties is not equal to defining them and nothing stops you
from defining all properties top-level and requiring them in
allOf:if:then:patternProperties.


> If I put allOf:if:then under patternProperties, I can't do the latter.

You can.

> 
> Other than readability to human eyes, binding check works as intended, 
> in case there's no other way to do it.

I don't see the problem in doing it and readability is one of main
factors of code admission to Linux kernel.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-12  6:57       ` Krzysztof Kozlowski
@ 2022-08-12  7:01         ` Krzysztof Kozlowski
  2022-08-12 13:41           ` Arınç ÜNAL
  0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-12  7:01 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>> -patternProperties:
>>>> -  "^(ethernet-)?ports$":
>>>> -    type: object
>>>
>>> Actually four patches...
>>>
>>> I don't find this change explained in commit msg. What is more, it looks
>>> incorrect. All properties and patternProperties should be explained in
>>> top-level part.
>>>
>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>> readable.
>>
>> I can't figure out another way. I need to require certain properties for 
>> a compatible string AND certain enum/const for certain properties which 
>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading 
>> the compatible string.
> 
> requiring properties is not equal to defining them and nothing stops you
> from defining all properties top-level and requiring them in
> allOf:if:then:patternProperties.
> 
> 
>> If I put allOf:if:then under patternProperties, I can't do the latter.
> 
> You can.
> 
>>
>> Other than readability to human eyes, binding check works as intended, 
>> in case there's no other way to do it.
> 
> I don't see the problem in doing it and readability is one of main
> factors of code admission to Linux kernel.

One more thought - if your schema around allOf:if:then grows too much,
it is actually a sign that it might benefit from splitting. Either into
two separate schemas or into common+two separate.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-12  7:01         ` Krzysztof Kozlowski
@ 2022-08-12 13:41           ` Arınç ÜNAL
  2022-08-12 13:48             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-08-12 13:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
>> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>>> -patternProperties:
>>>>> -  "^(ethernet-)?ports$":
>>>>> -    type: object
>>>>
>>>> Actually four patches...
>>>>
>>>> I don't find this change explained in commit msg. What is more, it looks
>>>> incorrect. All properties and patternProperties should be explained in
>>>> top-level part.
>>>>
>>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>>> readable.
>>>
>>> I can't figure out another way. I need to require certain properties for
>>> a compatible string AND certain enum/const for certain properties which
>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
>>> the compatible string.
>>
>> requiring properties is not equal to defining them and nothing stops you
>> from defining all properties top-level and requiring them in
>> allOf:if:then:patternProperties.
>>
>>
>>> If I put allOf:if:then under patternProperties, I can't do the latter.
>>
>> You can.

Am I supposed to do something like this:

patternProperties:
   "^(ethernet-)?ports$":
     type: object

     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:
               allOf:
                 - if:
                     properties:
                       compatible:
                         items:
                           - const: mediatek,mt7530
                           - const: mediatek,mt7621
                   then:
                     allOf:
                       - if:
                           properties:
                             reg:
                               const: 5
                         then:
                           properties:
                             phy-mode:
                               enum:
                                 - gmii
                                 - mii
                                 - rgmii

                       - if:
                           properties:
                             reg:
                               const: 6
                         then:
                           properties:
                             phy-mode:
                               enum:
                                 - rgmii
                                 - trgmii

                 - if:
                     properties:
                       compatible:
                         items:
                           - const: mediatek,mt7531
                   then:
                     allOf:
                       - if:
                           properties:
                             reg:
                               const: 5
                         then:
                           properties:
                             phy-mode:
                               enum:
                                 - 1000base-x
                                 - 2500base-x
                                 - rgmii
                                 - sgmii

                       - if:
                           properties:
                             reg:
                               const: 6
                         then:
                           properties:
                             phy-mode:
                               enum:
                                 - 1000base-x
                                 - 2500base-x
                                 - sgmii

               properties:
                 reg:
                   enum:
                     - 5
                     - 6

               required:
                 - phy-mode

>>
>>>
>>> Other than readability to human eyes, binding check works as intended,
>>> in case there's no other way to do it.
>>
>> I don't see the problem in doing it and readability is one of main
>> factors of code admission to Linux kernel.
> 
> One more thought - if your schema around allOf:if:then grows too much,
> it is actually a sign that it might benefit from splitting. Either into
> two separate schemas or into common+two separate.
> 
> Best regards,
> Krzysztof

Arınç

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-12 13:41           ` Arınç ÜNAL
@ 2022-08-12 13:48             ` Krzysztof Kozlowski
  2022-08-12 14:06               ` Arınç ÜNAL
  0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-12 13:48 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 12/08/2022 16:41, Arınç ÜNAL wrote:
> On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
>> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
>>> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>>>> -patternProperties:
>>>>>> -  "^(ethernet-)?ports$":
>>>>>> -    type: object
>>>>>
>>>>> Actually four patches...
>>>>>
>>>>> I don't find this change explained in commit msg. What is more, it looks
>>>>> incorrect. All properties and patternProperties should be explained in
>>>>> top-level part.
>>>>>
>>>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>>>> readable.
>>>>
>>>> I can't figure out another way. I need to require certain properties for
>>>> a compatible string AND certain enum/const for certain properties which
>>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
>>>> the compatible string.
>>>
>>> requiring properties is not equal to defining them and nothing stops you
>>> from defining all properties top-level and requiring them in
>>> allOf:if:then:patternProperties.
>>>
>>>
>>>> If I put allOf:if:then under patternProperties, I can't do the latter.
>>>
>>> You can.
> 
> Am I supposed to do something like this:
> 
> patternProperties:
>    "^(ethernet-)?ports$":
>      type: object
> 
>      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:
>                allOf:
>                  - if:
>                      properties:

Not really, this is absolutely unreadable.

Usually the way it is handled is:

patternProperties:
   "^(ethernet-)?ports$":
     type: object

     patternProperties:
       "^(ethernet-)?port@[0-9]+$":
         type: object
         description: Ethernet switch ports
         unevaluatedProperties: false
         ... regular stuff follows

allOf:
 - if:
     properties:
       compatible:
         .....
   then:
     patternProperties:
       "^(ethernet-)?ports$":
         patternProperties:
           "^(ethernet-)?port@[0-9]+$":
             properties:
               reg:
                 const: 5


I admit that it is still difficult to parse, which could justify
splitting to separate schema. Anyway the point of my comment was to
define all properties in top level, not in allOf.

allOf should be used to constrain these properties.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-12 13:48             ` Krzysztof Kozlowski
@ 2022-08-12 14:06               ` Arınç ÜNAL
  2022-08-19 12:40                 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 17+ messages in thread
From: Arınç ÜNAL @ 2022-08-12 14:06 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel



On 12.08.2022 16:48, Krzysztof Kozlowski wrote:
> On 12/08/2022 16:41, Arınç ÜNAL wrote:
>> On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
>>> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
>>>> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>>>>> -patternProperties:
>>>>>>> -  "^(ethernet-)?ports$":
>>>>>>> -    type: object
>>>>>>
>>>>>> Actually four patches...
>>>>>>
>>>>>> I don't find this change explained in commit msg. What is more, it looks
>>>>>> incorrect. All properties and patternProperties should be explained in
>>>>>> top-level part.
>>>>>>
>>>>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>>>>> readable.
>>>>>
>>>>> I can't figure out another way. I need to require certain properties for
>>>>> a compatible string AND certain enum/const for certain properties which
>>>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
>>>>> the compatible string.
>>>>
>>>> requiring properties is not equal to defining them and nothing stops you
>>>> from defining all properties top-level and requiring them in
>>>> allOf:if:then:patternProperties.
>>>>
>>>>
>>>>> If I put allOf:if:then under patternProperties, I can't do the latter.
>>>>
>>>> You can.
>>
>> Am I supposed to do something like this:
>>
>> patternProperties:
>>     "^(ethernet-)?ports$":
>>       type: object
>>
>>       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:
>>                 allOf:
>>                   - if:
>>                       properties:
> 
> Not really, this is absolutely unreadable.
> 
> Usually the way it is handled is:
> 
> patternProperties:
>     "^(ethernet-)?ports$":
>       type: object
> 
>       patternProperties:
>         "^(ethernet-)?port@[0-9]+$":
>           type: object
>           description: Ethernet switch ports
>           unevaluatedProperties: false
>           ... regular stuff follows
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           .....
>     then:
>       patternProperties:
>         "^(ethernet-)?ports$":
>           patternProperties:
>             "^(ethernet-)?port@[0-9]+$":
>               properties:
>                 reg:
>                   const: 5
> 
> 
> I admit that it is still difficult to parse, which could justify
> splitting to separate schema. Anyway the point of my comment was to
> define all properties in top level, not in allOf.
> 
> allOf should be used to constrain these properties.

The problem is:
- only specific values of reg are allowed if label is cpu.
- only specific values of phy-mode are allowed if reg is 5 or 6.

This forces me to define properties under allOf:if:then. Splitting to 
separate schema (per compatible string?) wouldn't help in this case.

I can split patternProperties to two sections, but I can't directly 
define the reg property like you put above.

I can at least split mediatek,mt7531 to a separate schema to have less 
patternProperties on a single binding.

What do you think?

Arınç

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
  2022-08-12 14:06               ` Arınç ÜNAL
@ 2022-08-19 12:40                 ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-19 12:40 UTC (permalink / raw)
  To: Arınç ÜNAL, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Frank Wunderlich, Luiz Angelo Daros de Luca,
	Sander Vanheule, René van Dorst, Daniel Golle,
	erkin.bozoglu, Sergio Paracuellos
  Cc: netdev, devicetree, linux-arm-kernel, linux-mediatek, linux-kernel

On 12/08/2022 17:06, Arınç ÜNAL wrote:
> 
> 
> On 12.08.2022 16:48, Krzysztof Kozlowski wrote:
>> On 12/08/2022 16:41, Arınç ÜNAL wrote:
>>> On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
>>>> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
>>>>> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>>>>>> -patternProperties:
>>>>>>>> -  "^(ethernet-)?ports$":
>>>>>>>> -    type: object
>>>>>>>
>>>>>>> Actually four patches...
>>>>>>>
>>>>>>> I don't find this change explained in commit msg. What is more, it looks
>>>>>>> incorrect. All properties and patternProperties should be explained in
>>>>>>> top-level part.
>>>>>>>
>>>>>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>>>>>> readable.
>>>>>>
>>>>>> I can't figure out another way. I need to require certain properties for
>>>>>> a compatible string AND certain enum/const for certain properties which
>>>>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
>>>>>> the compatible string.
>>>>>
>>>>> requiring properties is not equal to defining them and nothing stops you
>>>>> from defining all properties top-level and requiring them in
>>>>> allOf:if:then:patternProperties.
>>>>>
>>>>>
>>>>>> If I put allOf:if:then under patternProperties, I can't do the latter.
>>>>>
>>>>> You can.
>>>
>>> Am I supposed to do something like this:
>>>
>>> patternProperties:
>>>     "^(ethernet-)?ports$":
>>>       type: object
>>>
>>>       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:
>>>                 allOf:
>>>                   - if:
>>>                       properties:
>>
>> Not really, this is absolutely unreadable.
>>
>> Usually the way it is handled is:
>>
>> patternProperties:
>>     "^(ethernet-)?ports$":
>>       type: object
>>
>>       patternProperties:
>>         "^(ethernet-)?port@[0-9]+$":
>>           type: object
>>           description: Ethernet switch ports
>>           unevaluatedProperties: false
>>           ... regular stuff follows
>>
>> allOf:
>>   - if:
>>       properties:
>>         compatible:
>>           .....
>>     then:
>>       patternProperties:
>>         "^(ethernet-)?ports$":
>>           patternProperties:
>>             "^(ethernet-)?port@[0-9]+$":
>>               properties:
>>                 reg:
>>                   const: 5
>>
>>
>> I admit that it is still difficult to parse, which could justify
>> splitting to separate schema. Anyway the point of my comment was to
>> define all properties in top level, not in allOf.
>>
>> allOf should be used to constrain these properties.
> 
> The problem is:
> - only specific values of reg are allowed if label is cpu.
> - only specific values of phy-mode are allowed if reg is 5 or 6.
> 
> This forces me to define properties under allOf:if:then. 

None of the reasons above force you to define properties in some
allOf:if:then subblock. These force you to constrain the properties in
allOf:if:then, but not define.

> Splitting to 
> separate schema (per compatible string?) wouldn't help in this case.

True.

> 
> I can split patternProperties to two sections, but I can't directly 
> define the reg property like you put above.

Of course you can and original bindings were doing it.

Let me ask specific questions (yes, no):
1. Are ethernet-ports and ethernet-port present in each variant?

2. Is dsa-port.yaml applicable to each variant? (looks like that - three
compatibles, three all:if:then)
3. If reg appearing in each variant?
4. If above is true, if reg is maximum one item in each variant?

Looking at your patch, I think answer is 4x yes, which means you can
define them in one place and constrain in allOf:if:then, just like all
other schemas, because this one is not different.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-08-19 12:42 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-30 14:26 [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL
2022-07-30 14:26 ` [PATCH 1/4] dt-bindings: net: dsa: mediatek,mt7530: make trivial changes Arınç ÜNAL
2022-08-02  8:41   ` Krzysztof Kozlowski
2022-08-11 22:09     ` Arınç ÜNAL
2022-07-30 14:26 ` [PATCH 2/4] dt-bindings: net: dsa: mediatek,mt7530: update examples Arınç ÜNAL
2022-08-02  8:43   ` Krzysztof Kozlowski
2022-07-30 14:26 ` [PATCH 3/4] dt-bindings: net: dsa: mediatek,mt7530: update binding description Arınç ÜNAL
2022-07-30 14:26 ` [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema Arınç ÜNAL
2022-08-02  8:46   ` Krzysztof Kozlowski
2022-08-11 22:09     ` Arınç ÜNAL
2022-08-12  6:57       ` Krzysztof Kozlowski
2022-08-12  7:01         ` Krzysztof Kozlowski
2022-08-12 13:41           ` Arınç ÜNAL
2022-08-12 13:48             ` Krzysztof Kozlowski
2022-08-12 14:06               ` Arınç ÜNAL
2022-08-19 12:40                 ` Krzysztof Kozlowski
2022-07-31  8:53 ` [PATCH 0/4] completely rework mediatek,mt7530 binding Arınç ÜNAL

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).