netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro
@ 2022-05-07 17:04 Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

This Series add Support for the mt7531 switch on Bananapi R2 Pro board.

This board uses port5 of the switch to conect to the gmac0 of the
rk3568 SoC.

Currently CPU-Port is hardcoded in the mt7530 driver to port 6.

Compared to v1 the reset-Patch was dropped as it was not needed and
CPU-Port-changes are completely rewriten based on suggestions/code from
Vladimir Oltean (many thanks to this).
In DTS Patch i only dropped the status-property that was not
needed/ignored by driver.

Due to the Changes i also made a regression Test on mt7623 bpi-r2 using
mt7530 with cpu-port 6. Tests were done directly (ipv4 config on dsa user
port) and with vlan-aware bridge including vlan that was tagged outgoing
on dsa user port.

Main change is that i now include the binding-changes into the patchset which
i posted separately.

Frank Wunderlich (6):
  dt-bindings: net: dsa: convert binding for mediatek switches
  net: dsa: mt7530: rework mt7530_hw_vlan_{add,del}
  net: dsa: mt7530: rework mt753[01]_setup
  net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant
  dt-bindings: net: dsa: make reset optional and add rgmii-mode to
    mt7531
  arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board

 .../bindings/net/dsa/mediatek,mt7530.yaml     | 421 ++++++++++++++++++
 .../devicetree/bindings/net/dsa/mt7530.txt    | 327 --------------
 .../boot/dts/rockchip/rk3568-bpi-r2-pro.dts   |  48 ++
 drivers/net/dsa/mt7530.c                      |  82 ++--
 drivers/net/dsa/mt7530.h                      |   1 -
 5 files changed, 522 insertions(+), 357 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

-- 
2.25.1


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

* [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-05-07 19:59   ` Krzysztof Kozlowski
  2022-05-10 18:44   ` Rob Herring
  2022-05-07 17:04 ` [PATCH v3 2/6] net: dsa: mt7530: rework mt7530_hw_vlan_{add,del} Frank Wunderlich
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Convert txt binding to yaml binding for Mediatek switches.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v3:
- include standalone patch in mt7530 series
- drop some descriptions (gpio-cells,reset-gpios,reset-names)
- drop | from descriptions
- move patternProperties above allOf

v2:
- rename mediatek.yaml => mediatek,mt7530.yaml
- drop "boolean" in description
- drop description for interrupt-properties
- drop #interrupt-cells as requirement
- example: eth=>ethernet,mdio0=>mdio,comment indention
- replace 0 by GPIO_ACTIVE_HIGH in first example
- use port(s)-pattern from dsa.yaml
- adress/size-cells not added to required because this
  is defined at mdio-level inc current dts , not switch level
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 423 ++++++++++++++++++
 .../devicetree/bindings/net/dsa/mt7530.txt    | 327 --------------
 2 files changed, 423 insertions(+), 327 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
new file mode 100644
index 000000000000..a7696d1b4a8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -0,0 +1,423 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MT7530 Ethernet switch
+
+maintainers:
+  - Sean Wang <sean.wang@mediatek.com>
+  - Landen Chao <Landen.Chao@mediatek.com>
+  - DENG Qingfang <dqfext@gmail.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"
+      - "sgmii"
+
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt7530
+      - mediatek,mt7531
+      - mediatek,mt7621
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  core-supply:
+    description:
+      Phandle to the regulator node necessary for the core power.
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller:
+    type: boolean
+    description:
+      if defined, MT7530's LED controller will run on GPIO mode.
+
+  "#interrupt-cells":
+    const: 1
+
+  interrupt-controller:
+    type: boolean
+
+  interrupts:
+    maxItems: 1
+
+  io-supply:
+    description:
+      Phandle to the regulator node necessary for the I/O power.
+      See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
+      for details for the regulator setup on these boards.
+
+  mediatek,mcm:
+    type: boolean
+    description:
+      if defined, indicates that either MT7530 is the part on multi-chip
+      module belong to MT7623A has or the remotely standalone chip as the
+      function MT7623N reference board provided for.
+
+  reset-gpios:
+    maxItems: 1
+
+  reset-names:
+    const: mcm
+
+  resets:
+    description:
+      Phandle pointing to the system reset controller with line index for
+      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
+
+allOf:
+  - $ref: "dsa.yaml#"
+  - if:
+      required:
+        - mediatek,mcm
+    then:
+      required:
+        - resets
+        - reset-names
+    else:
+      required:
+        - reset-gpios
+
+  - if:
+      required:
+        - interrupt-controller
+    then:
+      required:
+        - interrupts
+
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: mediatek,mt7530
+    then:
+      required:
+        - core-supply
+        - io-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        switch@0 {
+            compatible = "mediatek,mt7530";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <0>;
+
+            core-supply = <&mt6323_vpa_reg>;
+            io-supply = <&mt6323_vemc3v3_reg>;
+            reset-gpios = <&pio 33 GPIO_ACTIVE_HIGH>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                port@0 {
+                    reg = <0>;
+                    label = "lan0";
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "lan1";
+                };
+
+                port@2 {
+                    reg = <2>;
+                    label = "lan2";
+                };
+
+                port@3 {
+                    reg = <3>;
+                    label = "lan3";
+                };
+
+                port@4 {
+                    reg = <4>;
+                    label = "wan";
+                };
+
+                port@6 {
+                    reg = <6>;
+                    label = "cpu";
+                    ethernet = <&gmac0>;
+                    phy-mode = "trgmii";
+                    fixed-link {
+                        speed = <1000>;
+                        full-duplex;
+                    };
+                };
+            };
+        };
+    };
+
+  - |
+    //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;
+            };
+        };
+
+        gmac1: mac@1 {
+            compatible = "mediatek,eth-mac";
+            reg = <1>;
+            phy-mode = "rgmii-txid";
+            phy-handle = <&phy4>;
+        };
+
+        mdio: mdio-bus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            /* Internal phy */
+            phy4: ethernet-phy@4 {
+                reg = <4>;
+            };
+
+            mt7530: switch@1f {
+                compatible = "mediatek,mt7621";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <0x1f>;
+                mediatek,mcm;
+
+                resets = <&rstctrl 2>;
+                reset-names = "mcm";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+                        label = "lan0";
+                    };
+
+                    port@1 {
+                        reg = <1>;
+                        label = "lan1";
+                    };
+
+                    port@2 {
+                        reg = <2>;
+                        label = "lan2";
+                    };
+
+                    port@3 {
+                        reg = <3>;
+                        label = "lan3";
+                    };
+
+                    /* Commented out. Port 4 is handled by 2nd GMAC.
+                    port@4 {
+                        reg = <4>;
+                        label = "lan4";
+                    };
+                    */
+
+                    port@6 {
+                        reg = <6>;
+                        label = "cpu";
+                        ethernet = <&gmac0>;
+                        phy-mode = "rgmii";
+
+                        fixed-link {
+                            speed = <1000>;
+                            full-duplex;
+                            pause;
+                        };
+                    };
+                };
+            };
+        };
+    };
+
+  - |
+    //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;
+            };
+        };
+
+        mdio0: mdio-bus {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            /* External phy */
+            ephy5: ethernet-phy@7 {
+                reg = <7>;
+            };
+
+            switch@1f {
+                compatible = "mediatek,mt7621";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <0x1f>;
+                mediatek,mcm;
+
+                resets = <&rstctrl 2>;
+                reset-names = "mcm";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+                        label = "lan0";
+                    };
+
+                    port@1 {
+                        reg = <1>;
+                        label = "lan1";
+                    };
+
+                    port@2 {
+                        reg = <2>;
+                        label = "lan2";
+                    };
+
+                    port@3 {
+                        reg = <3>;
+                        label = "lan3";
+                    };
+
+                    port@4 {
+                        reg = <4>;
+                        label = "lan4";
+                    };
+
+                    port@5 {
+                        reg = <5>;
+                        label = "lan5";
+                        phy-mode = "rgmii";
+                        phy-handle = <&ephy5>;
+                    };
+
+                    cpu_port0: port@6 {
+                        reg = <6>;
+                        label = "cpu";
+                        ethernet = <&gmac_0>;
+                        phy-mode = "rgmii";
+
+                        fixed-link {
+                            speed = <1000>;
+                            full-duplex;
+                            pause;
+                        };
+                    };
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
deleted file mode 100644
index 18247ebfc487..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
+++ /dev/null
@@ -1,327 +0,0 @@
-Mediatek MT7530 Ethernet switch
-================================
-
-Required properties:
-
-- compatible: may be compatible = "mediatek,mt7530"
-	or compatible = "mediatek,mt7621"
-	or compatible = "mediatek,mt7531"
-- #address-cells: Must be 1.
-- #size-cells: Must be 0.
-- mediatek,mcm: Boolean; 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.
-
-If compatible mediatek,mt7530 is set then the following properties are required
-
-- core-supply: Phandle to the regulator node necessary for the core power.
-- io-supply: Phandle to the regulator node necessary for the I/O power.
-	See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
-	for details for the regulator setup on these boards.
-
-If the property mediatek,mcm isn't defined, following property is required
-
-- reset-gpios: Should be a gpio specifier for a reset line.
-
-Else, following properties are required
-
-- resets : Phandle pointing to the system reset controller with
-	line index for the ethsys.
-- reset-names : Should be set to "mcm".
-
-Required properties for the child nodes within ports container:
-
-- reg: Port address described must be 6 for CPU port and from 0 to 5 for
-	user ports.
-- phy-mode: String, the following values are acceptable for port labeled
-	"cpu":
-	If compatible mediatek,mt7530 or mediatek,mt7621 is set,
-	must be either "trgmii" or "rgmii"
-	If compatible mediatek,mt7531 is set,
-	must be either "sgmii", "1000base-x" or "2500base-x"
-
-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";
-
-Optional properties:
-
-- gpio-controller: Boolean; if defined, MT7530's LED controller will run on
-	GPIO mode.
-- #gpio-cells: Must be 2 if gpio-controller is defined.
-- interrupt-controller: Boolean; Enables the internal interrupt controller.
-
-If interrupt-controller is defined, the following properties are required.
-
-- #interrupt-cells: Must be 1.
-- interrupts: Parent interrupt for the interrupt controller.
-
-See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional
-required, optional properties and how the integrated switch subnodes must
-be specified.
-
-Example:
-
-	&mdio0 {
-		switch@0 {
-			compatible = "mediatek,mt7530";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			reg = <0>;
-
-			core-supply = <&mt6323_vpa_reg>;
-			io-supply = <&mt6323_vemc3v3_reg>;
-			reset-gpios = <&pio 33 0>;
-
-			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				reg = <0>;
-				port@0 {
-					reg = <0>;
-					label = "lan0";
-				};
-
-				port@1 {
-					reg = <1>;
-					label = "lan1";
-				};
-
-				port@2 {
-					reg = <2>;
-					label = "lan2";
-				};
-
-				port@3 {
-					reg = <3>;
-					label = "lan3";
-				};
-
-				port@4 {
-					reg = <4>;
-					label = "wan";
-				};
-
-				port@6 {
-					reg = <6>;
-					label = "cpu";
-					ethernet = <&gmac0>;
-					phy-mode = "trgmii";
-					fixed-link {
-						speed = <1000>;
-						full-duplex;
-					};
-				};
-			};
-		};
-	};
-
-Example 2: MT7621: Port 4 is WAN port: 2nd GMAC -> Port 5 -> PHY port 4.
-
-&eth {
-	gmac0: mac@0 {
-		compatible = "mediatek,eth-mac";
-		reg = <0>;
-		phy-mode = "rgmii";
-
-		fixed-link {
-			speed = <1000>;
-			full-duplex;
-			pause;
-		};
-	};
-
-	gmac1: mac@1 {
-		compatible = "mediatek,eth-mac";
-		reg = <1>;
-		phy-mode = "rgmii-txid";
-		phy-handle = <&phy4>;
-	};
-
-	mdio: mdio-bus {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		/* Internal phy */
-		phy4: ethernet-phy@4 {
-			reg = <4>;
-		};
-
-		mt7530: switch@1f {
-			compatible = "mediatek,mt7621";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			reg = <0x1f>;
-			pinctrl-names = "default";
-			mediatek,mcm;
-
-			resets = <&rstctrl 2>;
-			reset-names = "mcm";
-
-			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-
-				port@0 {
-					reg = <0>;
-					label = "lan0";
-				};
-
-				port@1 {
-					reg = <1>;
-					label = "lan1";
-				};
-
-				port@2 {
-					reg = <2>;
-					label = "lan2";
-				};
-
-				port@3 {
-					reg = <3>;
-					label = "lan3";
-				};
-
-/* Commented out. Port 4 is handled by 2nd GMAC.
-				port@4 {
-					reg = <4>;
-					label = "lan4";
-				};
-*/
-
-				cpu_port0: port@6 {
-					reg = <6>;
-					label = "cpu";
-					ethernet = <&gmac0>;
-					phy-mode = "rgmii";
-
-					fixed-link {
-						speed = <1000>;
-						full-duplex;
-						pause;
-					};
-				};
-			};
-		};
-	};
-};
-
-Example 3: MT7621: Port 5 is connected to external PHY: Port 5 -> external PHY.
-
-&eth {
-	gmac0: mac@0 {
-		compatible = "mediatek,eth-mac";
-		reg = <0>;
-		phy-mode = "rgmii";
-
-		fixed-link {
-			speed = <1000>;
-			full-duplex;
-			pause;
-		};
-	};
-
-	mdio: mdio-bus {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		/* External phy */
-		ephy5: ethernet-phy@7 {
-			reg = <7>;
-		};
-
-		mt7530: switch@1f {
-			compatible = "mediatek,mt7621";
-			#address-cells = <1>;
-			#size-cells = <0>;
-			reg = <0x1f>;
-			pinctrl-names = "default";
-			mediatek,mcm;
-
-			resets = <&rstctrl 2>;
-			reset-names = "mcm";
-
-			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-
-				port@0 {
-					reg = <0>;
-					label = "lan0";
-				};
-
-				port@1 {
-					reg = <1>;
-					label = "lan1";
-				};
-
-				port@2 {
-					reg = <2>;
-					label = "lan2";
-				};
-
-				port@3 {
-					reg = <3>;
-					label = "lan3";
-				};
-
-				port@4 {
-					reg = <4>;
-					label = "lan4";
-				};
-
-				port@5 {
-					reg = <5>;
-					label = "lan5";
-					phy-mode = "rgmii";
-					phy-handle = <&ephy5>;
-				};
-
-				cpu_port0: port@6 {
-					reg = <6>;
-					label = "cpu";
-					ethernet = <&gmac0>;
-					phy-mode = "rgmii";
-
-					fixed-link {
-						speed = <1000>;
-						full-duplex;
-						pause;
-					};
-				};
-			};
-		};
-	};
-};
-- 
2.25.1


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

* [PATCH v3 2/6] net: dsa: mt7530: rework mt7530_hw_vlan_{add,del}
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 3/6] net: dsa: mt7530: rework mt753[01]_setup Frank Wunderlich
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Rework vlan_add/vlan_del functions in preparation for dynamic cpu port.

Currently BIT(MT7530_CPU_PORT) is added to new_members, even though
mt7530_port_vlan_add() will be called on the CPU port too.

Let DSA core decide when to call port_vlan_add for the CPU port, rather
than doing it implicitly.

We can do autonomous forwarding in a certain VLAN, but not add br0 to that
VLAN and avoid flooding the CPU with those packets, if software knows it
doesn't need to process them.

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
---
v2: new patch
---
 drivers/net/dsa/mt7530.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 19f0035d4410..46dee0714382 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1522,11 +1522,11 @@ static void
 mt7530_hw_vlan_add(struct mt7530_priv *priv,
 		   struct mt7530_hw_vlan_entry *entry)
 {
+	struct dsa_port *dp = dsa_to_port(priv->ds, entry->port);
 	u8 new_members;
 	u32 val;
 
-	new_members = entry->old_members | BIT(entry->port) |
-		      BIT(MT7530_CPU_PORT);
+	new_members = entry->old_members | BIT(entry->port);
 
 	/* Validate the entry with independent learning, create egress tag per
 	 * VLAN and joining the port as one of the port members.
@@ -1537,22 +1537,20 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
 
 	/* Decide whether adding tag or not for those outgoing packets from the
 	 * port inside the VLAN.
-	 */
-	val = entry->untagged ? MT7530_VLAN_EGRESS_UNTAG :
-				MT7530_VLAN_EGRESS_TAG;
-	mt7530_rmw(priv, MT7530_VAWD2,
-		   ETAG_CTRL_P_MASK(entry->port),
-		   ETAG_CTRL_P(entry->port, val));
-
-	/* CPU port is always taken as a tagged port for serving more than one
+	 * CPU port is always taken as a tagged port for serving more than one
 	 * VLANs across and also being applied with egress type stack mode for
 	 * that VLAN tags would be appended after hardware special tag used as
 	 * DSA tag.
 	 */
+	if (dsa_port_is_cpu(dp))
+		val = MT7530_VLAN_EGRESS_STACK;
+	else if (entry->untagged)
+		val = MT7530_VLAN_EGRESS_UNTAG;
+	else
+		val = MT7530_VLAN_EGRESS_TAG;
 	mt7530_rmw(priv, MT7530_VAWD2,
-		   ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
-		   ETAG_CTRL_P(MT7530_CPU_PORT,
-			       MT7530_VLAN_EGRESS_STACK));
+		   ETAG_CTRL_P_MASK(entry->port),
+		   ETAG_CTRL_P(entry->port, val));
 }
 
 static void
@@ -1571,11 +1569,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
 		return;
 	}
 
-	/* If certain member apart from CPU port is still alive in the VLAN,
-	 * the entry would be kept valid. Otherwise, the entry is got to be
-	 * disabled.
-	 */
-	if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
+	if (new_members) {
 		val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
 		      VLAN_VALID;
 		mt7530_write(priv, MT7530_VAWD1, val);
-- 
2.25.1


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

* [PATCH v3 3/6] net: dsa: mt7530: rework mt753[01]_setup
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 2/6] net: dsa: mt7530: rework mt7530_hw_vlan_{add,del} Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 4/6] net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant Frank Wunderlich
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Enumerate available cpu-ports instead of using hardcoded constant.

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/mt7530.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 46dee0714382..144c29f8fefc 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2087,11 +2087,12 @@ static int
 mt7530_setup(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
+	struct device_node *dn = NULL;
 	struct device_node *phy_node;
 	struct device_node *mac_np;
 	struct mt7530_dummy_poll p;
 	phy_interface_t interface;
-	struct device_node *dn;
+	struct dsa_port *cpu_dp;
 	u32 id, val;
 	int ret, i;
 
@@ -2099,7 +2100,19 @@ mt7530_setup(struct dsa_switch *ds)
 	 * controller also is the container for two GMACs nodes representing
 	 * as two netdev instances.
 	 */
-	dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
+	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
+		dn = cpu_dp->master->dev.of_node->parent;
+		/* It doesn't matter which CPU port is found first,
+		 * their masters should share the same parent OF node
+		 */
+		break;
+	}
+
+	if (!dn) {
+		dev_err(ds->dev, "parent OF node of DSA master not found");
+		return -EINVAL;
+	}
+
 	ds->assisted_learning_on_cpu_port = true;
 	ds->mtu_enforcement_ingress = true;
 
@@ -2260,6 +2273,7 @@ mt7531_setup(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
 	struct mt7530_dummy_poll p;
+	struct dsa_port *cpu_dp;
 	u32 val, id;
 	int ret, i;
 
@@ -2332,8 +2346,11 @@ mt7531_setup(struct dsa_switch *ds)
 				 CORE_PLL_GROUP4, val);
 
 	/* BPDU to CPU port */
-	mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
-		   BIT(MT7530_CPU_PORT));
+	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
+		mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
+			   BIT(cpu_dp->index));
+		break;
+	}
 	mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
 		   MT753X_BPDU_CPU_ONLY);
 
-- 
2.25.1


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

* [PATCH v3 4/6] net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
                   ` (2 preceding siblings ...)
  2022-05-07 17:04 ` [PATCH v3 3/6] net: dsa: mt7530: rework mt753[01]_setup Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-05-07 17:04 ` [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531 Frank Wunderlich
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Replace last occurences of hardcoded cpu-port by cpu_dp member of
dsa_port struct.

Now the constant can be dropped.

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/mt7530.c | 27 ++++++++++++++++++++-------
 drivers/net/dsa/mt7530.h |  1 -
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 144c29f8fefc..8bf27937e577 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1033,6 +1033,7 @@ static int
 mt7530_port_enable(struct dsa_switch *ds, int port,
 		   struct phy_device *phy)
 {
+	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct mt7530_priv *priv = ds->priv;
 
 	mutex_lock(&priv->reg_mutex);
@@ -1041,7 +1042,11 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
 	 * restore the port matrix if the port is the member of a certain
 	 * bridge.
 	 */
-	priv->ports[port].pm |= PCR_MATRIX(BIT(MT7530_CPU_PORT));
+	if (dsa_port_is_user(dp)) {
+		struct dsa_port *cpu_dp = dp->cpu_dp;
+
+		priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
+	}
 	priv->ports[port].enable = true;
 	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
 		   priv->ports[port].pm);
@@ -1190,7 +1195,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 			struct netlink_ext_ack *extack)
 {
 	struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
-	u32 port_bitmap = BIT(MT7530_CPU_PORT);
+	struct dsa_port *cpu_dp = dp->cpu_dp;
+	u32 port_bitmap = BIT(cpu_dp->index);
 	struct mt7530_priv *priv = ds->priv;
 
 	mutex_lock(&priv->reg_mutex);
@@ -1267,9 +1273,12 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 	 * the CPU port get out of VLAN filtering mode.
 	 */
 	if (all_user_ports_removed) {
-		mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
+		struct dsa_port *dp = dsa_to_port(ds, port);
+		struct dsa_port *cpu_dp = dp->cpu_dp;
+
+		mt7530_write(priv, MT7530_PCR_P(cpu_dp->index),
 			     PCR_MATRIX(dsa_user_ports(priv->ds)));
-		mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT), PORT_SPEC_TAG
+		mt7530_write(priv, MT7530_PVC_P(cpu_dp->index), PORT_SPEC_TAG
 			     | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
 	}
 }
@@ -1307,6 +1316,7 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			 struct dsa_bridge bridge)
 {
 	struct dsa_port *dp = dsa_to_port(ds, port), *other_dp;
+	struct dsa_port *cpu_dp = dp->cpu_dp;
 	struct mt7530_priv *priv = ds->priv;
 
 	mutex_lock(&priv->reg_mutex);
@@ -1335,8 +1345,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 	 */
 	if (priv->ports[port].enable)
 		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
-			   PCR_MATRIX(BIT(MT7530_CPU_PORT)));
-	priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
+			   PCR_MATRIX(BIT(cpu_dp->index)));
+	priv->ports[port].pm = PCR_MATRIX(BIT(cpu_dp->index));
 
 	/* When a port is removed from the bridge, the port would be set up
 	 * back to the default as is at initial boot which is a VLAN-unaware
@@ -1503,6 +1513,9 @@ static int
 mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 			   struct netlink_ext_ack *extack)
 {
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_port *cpu_dp = dp->cpu_dp;
+
 	if (vlan_filtering) {
 		/* The port is being kept as VLAN-unaware port when bridge is
 		 * set up with vlan_filtering not being set, Otherwise, the
@@ -1510,7 +1523,7 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 		 * for becoming a VLAN-aware port.
 		 */
 		mt7530_port_set_vlan_aware(ds, port);
-		mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+		mt7530_port_set_vlan_aware(ds, cpu_dp->index);
 	} else {
 		mt7530_port_set_vlan_unaware(ds, port);
 	}
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 91508e2feef9..5895bcfc0f7d 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -8,7 +8,6 @@
 
 #define MT7530_NUM_PORTS		7
 #define MT7530_NUM_PHYS			5
-#define MT7530_CPU_PORT			6
 #define MT7530_NUM_FDB_RECORDS		2048
 #define MT7530_ALL_MEMBERS		0xff
 
-- 
2.25.1


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

* [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
                   ` (3 preceding siblings ...)
  2022-05-07 17:04 ` [PATCH v3 4/6] net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-05-07 20:01   ` Krzysztof Kozlowski
  2022-05-07 17:04 ` [PATCH v3 6/6] arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board Frank Wunderlich
  2022-06-08 17:38 ` Aw: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
  6 siblings, 1 reply; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Make reset optional as driver already supports it, allow port 5 as
cpu-port and phy-mode rgmii for mt7531 cpu-port.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml          | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index a7696d1b4a8c..d02faed41b2a 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -55,6 +55,7 @@ description: |
     On mt7531:
       - "1000base-x"
       - "2500base-x"
+      - "rgmii"
       - "sgmii"
 
 
@@ -159,9 +160,6 @@ allOf:
       required:
         - resets
         - reset-names
-    else:
-      required:
-        - reset-gpios
 
   - if:
       required:
-- 
2.25.1


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

* [PATCH v3 6/6] arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
                   ` (4 preceding siblings ...)
  2022-05-07 17:04 ` [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531 Frank Wunderlich
@ 2022-05-07 17:04 ` Frank Wunderlich
  2022-06-08 17:38 ` Aw: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
  6 siblings, 0 replies; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-07 17:04 UTC (permalink / raw)
  To: linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

From: Frank Wunderlich <frank-w@public-files.de>

Add Device Tree node for mt7531 switch connected to gmac0.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v2:
- drop status=disabled
---
 .../boot/dts/rockchip/rk3568-bpi-r2-pro.dts   | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
index 235cb7405d9b..e517712f5d8d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
@@ -454,6 +454,54 @@ &i2c5 {
 	status = "disabled";
 };
 
+&mdio0 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	switch@0 {
+		compatible = "mediatek,mt7531";
+		reg = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@1 {
+				reg = <1>;
+				label = "lan0";
+			};
+
+			port@2 {
+				reg = <2>;
+				label = "lan1";
+			};
+
+			port@3 {
+				reg = <3>;
+				label = "lan2";
+			};
+
+			port@4 {
+				reg = <4>;
+				label = "lan3";
+			};
+
+			port@5 {
+				reg = <5>;
+				label = "cpu";
+				ethernet = <&gmac0>;
+				phy-mode = "rgmii";
+
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+					pause;
+				};
+			};
+		};
+	};
+};
+
 &mdio1 {
 	rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.25.1


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

* Re: [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches
  2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
@ 2022-05-07 19:59   ` Krzysztof Kozlowski
  2022-05-10 18:44   ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-07 19:59 UTC (permalink / raw)
  To: Frank Wunderlich, linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

On 07/05/2022 19:04, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Convert txt binding to yaml binding for Mediatek switches.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v3:
> - include standalone patch in mt7530 series
> - drop some descriptions (gpio-cells,reset-gpios,reset-names)
> - drop | from descriptions
> - move patternProperties above allOf
> 
> v2:
> - rename mediatek.yaml => mediatek,mt7530.yaml
> - drop "boolean" in description
> - drop description for interrupt-properties
> - drop #interrupt-cells as requirement
> - example: eth=>ethernet,mdio0=>mdio,comment indention
> - replace 0 by GPIO_ACTIVE_HIGH in first example
> - use port(s)-pattern from dsa.yaml
> - adress/size-cells not added to required because this
>   is defined at mdio-level inc current dts , not switch level
> ---
>  .../bindings/net/dsa/mediatek,mt7530.yaml     | 423 ++++++++++++++++++
>  .../devicetree/bindings/net/dsa/mt7530.txt    | 327 --------------
>  2 files changed, 423 insertions(+), 327 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> new file mode 100644
> index 000000000000..a7696d1b4a8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -0,0 +1,423 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

I don't see any acks here so that part is unresolved.

Rest looks good:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

* Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-07 17:04 ` [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531 Frank Wunderlich
@ 2022-05-07 20:01   ` Krzysztof Kozlowski
  2022-05-08  6:24     ` Frank Wunderlich
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-07 20:01 UTC (permalink / raw)
  To: Frank Wunderlich, linux-rockchip, linux-mediatek
  Cc: Frank Wunderlich, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab

On 07/05/2022 19:04, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Make reset optional as driver already supports it, 

I do not see the connection between hardware needing or not needing a
reset GPIO and a driver supporting it or not... What does it mean?

> allow port 5 as
> cpu-port 

How do you allow it here?

> and phy-mode rgmii for mt7531 cpu-port.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml          | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index a7696d1b4a8c..d02faed41b2a 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -55,6 +55,7 @@ description: |
>      On mt7531:
>        - "1000base-x"
>        - "2500base-x"
> +      - "rgmii"
>        - "sgmii"


Best regards,
Krzysztof

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

* Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-07 20:01   ` Krzysztof Kozlowski
@ 2022-05-08  6:24     ` Frank Wunderlich
  2022-05-08  9:41       ` Heiko Stuebner
  0 siblings, 1 reply; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-08  6:24 UTC (permalink / raw)
  To: linux-rockchip, Krzysztof Kozlowski, Frank Wunderlich, linux-mediatek
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Heiko Stuebner, Sean Wang,
	Landen Chao, DENG Qingfang, Peter Geis, netdev, devicetree,
	linux-arm-kernel, linux-kernel, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>On 07/05/2022 19:04, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Make reset optional as driver already supports it, 
>
>I do not see the connection between hardware needing or not needing a
>reset GPIO and a driver supporting it or not... What does it mean?

My board has a shared gpio-reset between gmac and switch, so both will resetted if it is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:

- due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
- if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.

Using reset only on gmac side brings the switch up.

>> allow port 5 as
>> cpu-port 
>
>How do you allow it here?

Argh, seems i accidentally removed this part and have not recognized while checking :(

It should only change description of reg for ports to:

"Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."

regards Frank

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

* Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-08  6:24     ` Frank Wunderlich
@ 2022-05-08  9:41       ` Heiko Stuebner
  2022-05-08 12:12         ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 20+ messages in thread
From: Heiko Stuebner @ 2022-05-08  9:41 UTC (permalink / raw)
  To: linux-rockchip, Krzysztof Kozlowski, Frank Wunderlich,
	linux-mediatek, frank-w
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Peter Geis, netdev, devicetree, linux-arm-kernel,
	linux-kernel, Greg Ungerer, René van Dorst,
	Mauro Carvalho Chehab

Am Sonntag, 8. Mai 2022, 08:24:37 CEST schrieb Frank Wunderlich:
> Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> >On 07/05/2022 19:04, Frank Wunderlich wrote:
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >> 
> >> Make reset optional as driver already supports it, 
> >
> >I do not see the connection between hardware needing or not needing a
> >reset GPIO and a driver supporting it or not... What does it mean?
> 
> My board has a shared gpio-reset between gmac and switch, so both will resetted if it is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:
> 
> - due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
> - if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.
> 
> Using reset only on gmac side brings the switch up.

I think the issue is more for the description itself.

Devicetree is only meant to describe the hardware and does in general don't
care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
"the kernel does it this way" is not a valid reason for a binding change ;-) .

Instead in general want to reason that there are boards without this reset
facility and thus make it optional for those.

Heiko

> >> allow port 5 as
> >> cpu-port 
> >
> >How do you allow it here?
> 
> Argh, seems i accidentally removed this part and have not recognized while checking :(
> 
> It should only change description of reg for ports to:
> 
> "Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."
> 
> regards Frank
> 





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

* Aw: Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-08  9:41       ` Heiko Stuebner
@ 2022-05-08 12:12         ` Frank Wunderlich
  2022-05-08 17:04           ` Peter Geis
  0 siblings, 1 reply; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-08 12:12 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip, Krzysztof Kozlowski, Frank Wunderlich,
	linux-mediatek, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger, Sean Wang,
	Landen Chao, DENG Qingfang, Peter Geis, netdev, devicetree,
	linux-arm-kernel, linux-kernel, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

Hi Heiko

> Gesendet: Sonntag, 08. Mai 2022 um 11:41 Uhr
> Von: "Heiko Stuebner" <heiko@sntech.de>
> Am Sonntag, 8. Mai 2022, 08:24:37 CEST schrieb Frank Wunderlich:
> > Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> > >On 07/05/2022 19:04, Frank Wunderlich wrote:
> > >> From: Frank Wunderlich <frank-w@public-files.de>
> > >>
> > >> Make reset optional as driver already supports it,
> > >
> > >I do not see the connection between hardware needing or not needing a
> > >reset GPIO and a driver supporting it or not... What does it mean?
> >
> > My board has a shared gpio-reset between gmac and switch, so both will resetted if it
> > is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:
> >
> > - due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
> > - if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch
> > to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.
> >
> > Using reset only on gmac side brings the switch up.
>
> I think the issue is more for the description itself.
>
> Devicetree is only meant to describe the hardware and does in general don't
> care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
> "the kernel does it this way" is not a valid reason for a binding change ;-) .
>
> Instead in general want to reason that there are boards without this reset
> facility and thus make it optional for those.

if only the wording is the problem i try to rephrase it from hardware PoV.

maybe something like this?

https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml

Another way is maybe increasing the delay after the reset (to give more time all
come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
child device.

have not looked into the gmac driver if this always  does the initial reset to
have a "clean state". In this initial reset the switch will be resetted too
and does not need an additional one which needs the gmac/mdio initialization
to be done again.

> > >> allow port 5 as
> > >> cpu-port
> > >
> > >How do you allow it here?
> >
> > Argh, seems i accidentally removed this part and have not recognized while checking :(
> >
> > It should only change description of reg for ports to:
> >
> > "Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."

noticed that the target-phase is not removed but squashed in the first bindings-patch.
This was a rebasing error and not intented...will fix in next version.

regards Frank

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

* Re: Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-08 12:12         ` Aw: " Frank Wunderlich
@ 2022-05-08 17:04           ` Peter Geis
  2022-05-09  6:48             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Geis @ 2022-05-08 17:04 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Heiko Stuebner, open list:ARM/Rockchip SoC...,
	Krzysztof Kozlowski, Frank Wunderlich, linux-mediatek,
	Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Matthias Brugger, Sean Wang, Landen Chao,
	DENG Qingfang, Linux Kernel Network Developers, devicetree,
	arm-mail-list, Linux Kernel Mailing List, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

On Sun, May 8, 2022 at 8:12 AM Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Hi Heiko
>
> > Gesendet: Sonntag, 08. Mai 2022 um 11:41 Uhr
> > Von: "Heiko Stuebner" <heiko@sntech.de>
> > Am Sonntag, 8. Mai 2022, 08:24:37 CEST schrieb Frank Wunderlich:
> > > Am 7. Mai 2022 22:01:22 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> > > >On 07/05/2022 19:04, Frank Wunderlich wrote:
> > > >> From: Frank Wunderlich <frank-w@public-files.de>
> > > >>
> > > >> Make reset optional as driver already supports it,
> > > >
> > > >I do not see the connection between hardware needing or not needing a
> > > >reset GPIO and a driver supporting it or not... What does it mean?
> > >
> > > My board has a shared gpio-reset between gmac and switch, so both will resetted if it
> > > is asserted. Currently it is set to the gmac and is aquired exclusive. Adding it to switch results in 2 problems:
> > >
> > > - due to exclusive and already mapped to gmac, switch driver exits as it cannot get the reset-gpio again.
> > > - if i drop the reset from gmac and add to switch, it resets the gmac and this takes too long for switch
> > > to get up. Of course i can increase the wait time after reset,but dropping reset here was the easier way.
> > >
> > > Using reset only on gmac side brings the switch up.
> >
> > I think the issue is more for the description itself.
> >
> > Devicetree is only meant to describe the hardware and does in general don't
> > care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
> > "the kernel does it this way" is not a valid reason for a binding change ;-) .
> >
> > Instead in general want to reason that there are boards without this reset
> > facility and thus make it optional for those.
>
> if only the wording is the problem i try to rephrase it from hardware PoV.
>
> maybe something like this?
>
> https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml
>
> Another way is maybe increasing the delay after the reset (to give more time all
> come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
> child device.
>
> have not looked into the gmac driver if this always  does the initial reset to
> have a "clean state". In this initial reset the switch will be resetted too
> and does not need an additional one which needs the gmac/mdio initialization
> to be done again.

For clarification, the reset gpio line is purely to reset the phy.
If having the switch driver own the reset gpio instead of the gmac
breaks initialization that means there's a bug in the gmac driver
handling phy init.
In testing I've seen issues moving the reset line to the mdio node
with other phys and the stmmac gmac driver, so I do believe this is
the case.

>
> > > >> allow port 5 as
> > > >> cpu-port
> > > >
> > > >How do you allow it here?
> > >
> > > Argh, seems i accidentally removed this part and have not recognized while checking :(
> > >
> > > It should only change description of reg for ports to:
> > >
> > > "Port address described must be 5 or 6 for CPU port and from 0 to 5 for user ports."
>
> noticed that the target-phase is not removed but squashed in the first bindings-patch.
> This was a rebasing error and not intented...will fix in next version.
>
> regards Frank

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

* Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-08 17:04           ` Peter Geis
@ 2022-05-09  6:48             ` Krzysztof Kozlowski
  2022-05-09 11:22               ` Peter Geis
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2022-05-09  6:48 UTC (permalink / raw)
  To: Peter Geis, Frank Wunderlich
  Cc: Heiko Stuebner, open list:ARM/Rockchip SoC...,
	Frank Wunderlich, linux-mediatek, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Matthias Brugger, Sean Wang, Landen Chao, DENG Qingfang,
	Linux Kernel Network Developers, devicetree, arm-mail-list,
	Linux Kernel Mailing List, Greg Ungerer, René van Dorst,
	Mauro Carvalho Chehab

On 08/05/2022 19:04, Peter Geis wrote:
> On Sun, May 8, 2022 at 8:12 AM Frank Wunderlich <frank-w@public-files.de> wrote:
>>>
>>> I think the issue is more for the description itself.
>>>
>>> Devicetree is only meant to describe the hardware and does in general don't
>>> care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
>>> "the kernel does it this way" is not a valid reason for a binding change ;-) .

Exactly. The argument in commit msg was not matching the change, because
driver implementation should not be (mostly) a reason for such changes.

>>>
>>> Instead in general want to reason that there are boards without this reset
>>> facility and thus make it optional for those.
>>
>> if only the wording is the problem i try to rephrase it from hardware PoV.
>>
>> maybe something like this?
>>
>> https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml

Looks ok.

>>
>> Another way is maybe increasing the delay after the reset (to give more time all
>> come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
>> child device.
>>
>> have not looked into the gmac driver if this always  does the initial reset to
>> have a "clean state". In this initial reset the switch will be resetted too
>> and does not need an additional one which needs the gmac/mdio initialization
>> to be done again.
> 
> For clarification, the reset gpio line is purely to reset the phy.
> If having the switch driver own the reset gpio instead of the gmac
> breaks initialization that means there's a bug in the gmac driver
> handling phy init.
> In testing I've seen issues moving the reset line to the mdio node
> with other phys and the stmmac gmac driver, so I do believe this is
> the case.

Yes, this seems reasonable, although Frank mentioned that reset is
shared with gmac, so it resets some part of it as well?




Best regards,
Krzysztof

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

* Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531
  2022-05-09  6:48             ` Krzysztof Kozlowski
@ 2022-05-09 11:22               ` Peter Geis
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Geis @ 2022-05-09 11:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Frank Wunderlich, Heiko Stuebner, open list:ARM/Rockchip SoC...,
	Frank Wunderlich, linux-mediatek, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Matthias Brugger, Sean Wang, Landen Chao, DENG Qingfang,
	Linux Kernel Network Developers, devicetree, arm-mail-list,
	Linux Kernel Mailing List, Greg Ungerer, René van Dorst,
	Mauro Carvalho Chehab

On Mon, May 9, 2022 at 2:48 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/05/2022 19:04, Peter Geis wrote:
> > On Sun, May 8, 2022 at 8:12 AM Frank Wunderlich <frank-w@public-files.de> wrote:
> >>>
> >>> I think the issue is more for the description itself.
> >>>
> >>> Devicetree is only meant to describe the hardware and does in general don't
> >>> care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
> >>> "the kernel does it this way" is not a valid reason for a binding change ;-) .
>
> Exactly. The argument in commit msg was not matching the change, because
> driver implementation should not be (mostly) a reason for such changes.
>
> >>>
> >>> Instead in general want to reason that there are boards without this reset
> >>> facility and thus make it optional for those.
> >>
> >> if only the wording is the problem i try to rephrase it from hardware PoV.
> >>
> >> maybe something like this?
> >>
> >> https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml
>
> Looks ok.
>
> >>
> >> Another way is maybe increasing the delay after the reset (to give more time all
> >> come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
> >> child device.
> >>
> >> have not looked into the gmac driver if this always  does the initial reset to
> >> have a "clean state". In this initial reset the switch will be resetted too
> >> and does not need an additional one which needs the gmac/mdio initialization
> >> to be done again.
> >
> > For clarification, the reset gpio line is purely to reset the phy.
> > If having the switch driver own the reset gpio instead of the gmac
> > breaks initialization that means there's a bug in the gmac driver
> > handling phy init.
> > In testing I've seen issues moving the reset line to the mdio node
> > with other phys and the stmmac gmac driver, so I do believe this is
> > the case.
>
> Yes, this seems reasonable, although Frank mentioned that reset is
> shared with gmac, so it resets some part of it as well?

No, the gpio-reset line is purely to reset the phy. The stmmac gmac
driver handles it because it seems initialization failures occur if
it's handled by the mdio drivers. I suspect this is due to a
difference between when the driver initializes the phy vs when the
driver triggers the reset.
They had tried to attach the gpio binding to both the gmac node and
the mdio node at the same time when only one can own it. Having it
owned by the switch driver on the mdio node leads to the same
initialization failures we see in other mdio drivers.

>
>
>
>
> Best regards,
> Krzysztof

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

* Re: [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches
  2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
  2022-05-07 19:59   ` Krzysztof Kozlowski
@ 2022-05-10 18:44   ` Rob Herring
  2022-05-11  8:02     ` Aw: " Frank Wunderlich
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-05-10 18:44 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-rockchip, linux-mediatek, Frank Wunderlich, Andrew Lunn,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Krzysztof Kozlowski, Matthias Brugger, Heiko Stuebner, Sean Wang,
	Landen Chao, DENG Qingfang, Peter Geis, netdev, devicetree,
	linux-arm-kernel, linux-kernel, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

On Sat, May 07, 2022 at 07:04:35PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Convert txt binding to yaml binding for Mediatek switches.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v3:
> - include standalone patch in mt7530 series
> - drop some descriptions (gpio-cells,reset-gpios,reset-names)
> - drop | from descriptions
> - move patternProperties above allOf
> 
> v2:
> - rename mediatek.yaml => mediatek,mt7530.yaml
> - drop "boolean" in description
> - drop description for interrupt-properties
> - drop #interrupt-cells as requirement
> - example: eth=>ethernet,mdio0=>mdio,comment indention
> - replace 0 by GPIO_ACTIVE_HIGH in first example
> - use port(s)-pattern from dsa.yaml
> - adress/size-cells not added to required because this
>   is defined at mdio-level inc current dts , not switch level
> ---
>  .../bindings/net/dsa/mediatek,mt7530.yaml     | 423 ++++++++++++++++++
>  .../devicetree/bindings/net/dsa/mt7530.txt    | 327 --------------
>  2 files changed, 423 insertions(+), 327 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> new file mode 100644
> index 000000000000..a7696d1b4a8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -0,0 +1,423 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek MT7530 Ethernet switch
> +
> +maintainers:
> +  - Sean Wang <sean.wang@mediatek.com>
> +  - Landen Chao <Landen.Chao@mediatek.com>
> +  - DENG Qingfang <dqfext@gmail.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"
> +      - "sgmii"
> +
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt7530
> +      - mediatek,mt7531
> +      - mediatek,mt7621
> +

> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

I don't see any child nodes with addresses, so these can be removed.

> +
> +  core-supply:
> +    description:
> +      Phandle to the regulator node necessary for the core power.
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller:
> +    type: boolean
> +    description:
> +      if defined, MT7530's LED controller will run on GPIO mode.
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  interrupt-controller:
> +    type: boolean

Already has a type. Just:

interrupt-controller: true

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  io-supply:
> +    description:
> +      Phandle to the regulator node necessary for the I/O power.
> +      See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
> +      for details for the regulator setup on these boards.
> +
> +  mediatek,mcm:
> +    type: boolean
> +    description:
> +      if defined, indicates that either MT7530 is the part on multi-chip
> +      module belong to MT7623A has or the remotely standalone chip as the
> +      function MT7623N reference board provided for.
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  reset-names:
> +    const: mcm
> +
> +  resets:
> +    description:
> +      Phandle pointing to the system reset controller with line index for
> +      the ethsys.
> +    maxItems: 1
> +
> +patternProperties:
> +  "^(ethernet-)?ports$":
> +    type: object

       additionalProperties: false

> +
> +    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
> +
> +allOf:
> +  - $ref: "dsa.yaml#"
> +  - if:
> +      required:
> +        - mediatek,mcm
> +    then:
> +      required:
> +        - resets
> +        - reset-names
> +    else:
> +      required:
> +        - reset-gpios
> +
> +  - if:
> +      required:
> +        - interrupt-controller
> +    then:
> +      required:
> +        - interrupts

This can be expressed as:

dependencies:
  interrupt-controller: [ interrupts ]
    
> +
> +  - if:
> +      properties:
> +        compatible:
> +          items:
> +            - const: mediatek,mt7530
> +    then:
> +      required:
> +        - core-supply
> +        - io-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        switch@0 {
> +            compatible = "mediatek,mt7530";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            reg = <0>;
> +
> +            core-supply = <&mt6323_vpa_reg>;
> +            io-supply = <&mt6323_vemc3v3_reg>;
> +            reset-gpios = <&pio 33 GPIO_ACTIVE_HIGH>;
> +
> +            ports {

Use the preferred form: ethernet-ports

> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                port@0 {


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

* Aw: Re: [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches
  2022-05-10 18:44   ` Rob Herring
@ 2022-05-11  8:02     ` Frank Wunderlich
  2022-05-18  1:54       ` Rob Herring
  0 siblings, 1 reply; 20+ messages in thread
From: Frank Wunderlich @ 2022-05-11  8:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Frank Wunderlich, linux-rockchip, linux-mediatek, Andrew Lunn,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Krzysztof Kozlowski, Matthias Brugger, Heiko Stuebner, Sean Wang,
	Landen Chao, DENG Qingfang, Peter Geis, netdev, devicetree,
	linux-arm-kernel, linux-kernel, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

Hi

thanks for review

> Gesendet: Dienstag, 10. Mai 2022 um 20:44 Uhr
> Von: "Rob Herring" <robh@kernel.org>
> On Sat, May 07, 2022 at 07:04:35PM +0200, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>

> > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml

> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mediatek,mt7530
> > +      - mediatek,mt7531
> > +      - mediatek,mt7621
> > +
>
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
>
> I don't see any child nodes with addresses, so these can be removed.

dropping this (and address-cells/size-cells from examples) causes errors like this (address-/size-cells set in mdio
node, so imho it should inherite):

Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dts:34.25-35: Warning (reg_format):
/example-0/mdio/switch@0/ports/port@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'

> > +  interrupt-controller:
> > +    type: boolean
>
> Already has a type. Just:
>
> interrupt-controller: true
>
> > +
> > +  interrupts:
> > +    maxItems: 1

> > +patternProperties:
> > +  "^(ethernet-)?ports$":
> > +    type: object
>
>        additionalProperties: false

imho this will block address-/size-cells from this level too. looks like it is needed here too (for port-regs).

> > +
> > +    patternProperties:
> > +      "^(ethernet-)?port@[0-9]+$":
> > +        type: object
> > +        description: Ethernet switch ports
> > +
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            description:
> > +              Port address described must be 5 or 6 for CPU port and from 0
> > +              to 5 for user ports.
> > +
> > +        allOf:
> > +          - $ref: dsa-port.yaml#
> > +          - if:
> > +              properties:
> > +                label:
> > +                  items:
> > +                    - const: cpu
> > +            then:
> > +              required:
> > +                - reg
> > +                - phy-mode
> > +

> > +  - if:
> > +      required:
> > +        - interrupt-controller
> > +    then:
> > +      required:
> > +        - interrupts
>
> This can be expressed as:
>
> dependencies:
>   interrupt-controller: [ interrupts ]

ok, i will change this

> > +            ports {
>
> Use the preferred form: ethernet-ports

current implementation in all existing dts and examples from old binding are "ports" only.
should they changed too?

> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +                port@0 {

regards Frank

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

* Re: Re: [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches
  2022-05-11  8:02     ` Aw: " Frank Wunderlich
@ 2022-05-18  1:54       ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2022-05-18  1:54 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Frank Wunderlich, linux-rockchip, linux-mediatek, Andrew Lunn,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, Paolo Abeni,
	Krzysztof Kozlowski, Matthias Brugger, Heiko Stuebner, Sean Wang,
	Landen Chao, DENG Qingfang, Peter Geis, netdev, devicetree,
	linux-arm-kernel, linux-kernel, Greg Ungerer,
	René van Dorst, Mauro Carvalho Chehab

On Wed, May 11, 2022 at 10:02:45AM +0200, Frank Wunderlich wrote:
> Hi
> 
> thanks for review
> 
> > Gesendet: Dienstag, 10. Mai 2022 um 20:44 Uhr
> > Von: "Rob Herring" <robh@kernel.org>
> > On Sat, May 07, 2022 at 07:04:35PM +0200, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
> 
> > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> 
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - mediatek,mt7530
> > > +      - mediatek,mt7531
> > > +      - mediatek,mt7621
> > > +
> >
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> >
> > I don't see any child nodes with addresses, so these can be removed.
> 
> dropping this (and address-cells/size-cells from examples) causes errors like this (address-/size-cells set in mdio
> node, so imho it should inherite):

I think you are off a level.


> Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dts:34.25-35: Warning (reg_format):
> /example-0/mdio/switch@0/ports/port@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)

That's for yet another level where 'ports' node need the properties.

> Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> 
> > > +  interrupt-controller:
> > > +    type: boolean
> >
> > Already has a type. Just:
> >
> > interrupt-controller: true
> >
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> 
> > > +patternProperties:
> > > +  "^(ethernet-)?ports$":
> > > +    type: object
> >
> >        additionalProperties: false
> 
> imho this will block address-/size-cells from this level too. looks like it is needed here too (for port-regs).
> 
> > > +
> > > +    patternProperties:
> > > +      "^(ethernet-)?port@[0-9]+$":
> > > +        type: object
> > > +        description: Ethernet switch ports
> > > +
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          reg:
> > > +            description:
> > > +              Port address described must be 5 or 6 for CPU port and from 0
> > > +              to 5 for user ports.
> > > +
> > > +        allOf:
> > > +          - $ref: dsa-port.yaml#
> > > +          - if:
> > > +              properties:
> > > +                label:
> > > +                  items:
> > > +                    - const: cpu
> > > +            then:
> > > +              required:
> > > +                - reg
> > > +                - phy-mode
> > > +
> 
> > > +  - if:
> > > +      required:
> > > +        - interrupt-controller
> > > +    then:
> > > +      required:
> > > +        - interrupts
> >
> > This can be expressed as:
> >
> > dependencies:
> >   interrupt-controller: [ interrupts ]
> 
> ok, i will change this
> 
> > > +            ports {
> >
> > Use the preferred form: ethernet-ports
> 
> current implementation in all existing dts and examples from old binding are "ports" only.
> should they changed too?

They don't have to be the schema allows both, but the example should be 
best practice.

Rob


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

* Aw: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro
  2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
                   ` (5 preceding siblings ...)
  2022-05-07 17:04 ` [PATCH v3 6/6] arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board Frank Wunderlich
@ 2022-06-08 17:38 ` Frank Wunderlich
  2022-06-08 17:54   ` Jakub Kicinski
  6 siblings, 1 reply; 20+ messages in thread
From: Frank Wunderlich @ 2022-06-08 17:38 UTC (permalink / raw)
  To: linux-mediatek
  Cc: linux-rockchip, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab,
	Frank Wunderlich

just a gentle ping, is anything missing/wrong?

regards Frank

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

* Re: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro
  2022-06-08 17:38 ` Aw: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
@ 2022-06-08 17:54   ` Jakub Kicinski
  0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2022-06-08 17:54 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-mediatek, linux-rockchip, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Matthias Brugger,
	Heiko Stuebner, Sean Wang, Landen Chao, DENG Qingfang,
	Peter Geis, netdev, devicetree, linux-arm-kernel, linux-kernel,
	Greg Ungerer, René van Dorst, Mauro Carvalho Chehab,
	Frank Wunderlich

On Wed, 8 Jun 2022 19:38:30 +0200 Frank Wunderlich wrote:
> just a gentle ping, is anything missing/wrong?

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#how-can-i-tell-the-status-of-a-patch-i-ve-sent

In your case:

https://patchwork.kernel.org/project/netdevbpf/list/?series=639420&state=*

I haven't double checked but even if the feedback you received was
waved the patch didn't apply when it was posted, so you gotta repost.

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

end of thread, other threads:[~2022-06-08 17:54 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-07 17:04 [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
2022-05-07 17:04 ` [PATCH v3 1/6] dt-bindings: net: dsa: convert binding for mediatek switches Frank Wunderlich
2022-05-07 19:59   ` Krzysztof Kozlowski
2022-05-10 18:44   ` Rob Herring
2022-05-11  8:02     ` Aw: " Frank Wunderlich
2022-05-18  1:54       ` Rob Herring
2022-05-07 17:04 ` [PATCH v3 2/6] net: dsa: mt7530: rework mt7530_hw_vlan_{add,del} Frank Wunderlich
2022-05-07 17:04 ` [PATCH v3 3/6] net: dsa: mt7530: rework mt753[01]_setup Frank Wunderlich
2022-05-07 17:04 ` [PATCH v3 4/6] net: dsa: mt7530: get cpu-port via dp->cpu_dp instead of constant Frank Wunderlich
2022-05-07 17:04 ` [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531 Frank Wunderlich
2022-05-07 20:01   ` Krzysztof Kozlowski
2022-05-08  6:24     ` Frank Wunderlich
2022-05-08  9:41       ` Heiko Stuebner
2022-05-08 12:12         ` Aw: " Frank Wunderlich
2022-05-08 17:04           ` Peter Geis
2022-05-09  6:48             ` Krzysztof Kozlowski
2022-05-09 11:22               ` Peter Geis
2022-05-07 17:04 ` [PATCH v3 6/6] arm64: dts: rockchip: Add mt7531 dsa node to BPI-R2-Pro board Frank Wunderlich
2022-06-08 17:38 ` Aw: [PATCH v3 0/6] Support mt7531 on BPI-R2 Pro Frank Wunderlich
2022-06-08 17:54   ` Jakub Kicinski

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