[PATCH v3 00/13] further mt7988 devicetree work

[PATCH v3 00/13] further mt7988 devicetree work

[Frank Wunderlich]

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

    This series continues mt7988 devicetree work
    
    - Extend cpu frequency scaling with CCI
    - Basic network-support (ethernet controller + builtin switch + SFP Cages)
    
    depencies (i hope this list is complete and latest patches/series linked):
    
    for 2.5g phy function (currently disabled):
    - net: ethernet: mtk_eth_soc: add support for MT7988 internal 2.5G PHY (already merged to 6.15-net-next)
    - net: phy: mediatek: add driver for built-in 2.5G ethernet PHY on MT7988
      https://patchwork.kernel.org/project/netdevbpf/patch/20250516102327.2014531-3-SkyLake.Huang@mediatek.com/ (v4)
    
    for SFP-Function (macs currently disabled):
    
    PCS clearance which is a 1.5 year discussion currently ongoing
    
    e.g. something like this (one of):
    * https://patchwork.kernel.org/project/netdevbpf/patch/20250511201250.3789083-4-ansuelsmth@gmail.com/ (v4)
    * https://patchwork.kernel.org/project/netdevbpf/patch/20250523203339.1993685-4-sean.anderson@linux.dev/ (v5)
    * https://patchwork.kernel.org/project/netdevbpf/patch/ba4e359584a6b3bc4b3470822c42186d5b0856f9.1721910728.git.daniel@makrotopia.org/
    
    full usxgmii driver:
    https://patchwork.kernel.org/project/netdevbpf/patch/07845ec900ba41ff992875dce12c622277592c32.1702352117.git.daniel@makrotopia.org/
    
    first PCS-discussion is here:
    https://patchwork.kernel.org/project/netdevbpf/patch/8aa905080bdb6760875d62cb3b2b41258837f80e.1702352117.git.daniel@makrotopia.org/
    
    and then dts nodes for sgmiisys+usxgmii+2g5 firmware
    
    when above depencies are solved the mac1/2 can be enabled and 2.5G phy/SFP slots will work.
    
    changes:
    v3:
      - dropped patches already applied (SPI+thermal)
      - added soc specific cci compatible (new binding patch + changed dts)
      - enable 2g5 phy because driver is now merged
      - add patch for cleaning up unnecessary pins
      - add patch for gpio-leds
      - add patch for adding ethernet aliases
    
    v2:
      - change reg to list of items in eth binding
      - changed mt7530 binding:
        - unevaluatedProperties=false
        - mediatek,pio subproperty
        - from patternProperty to property
      - board specific properties like led function and labels moved to bpi-r4 dtsi

Frank Wunderlich (13):
  dt-bindings: net: mediatek,net: update for mt7988
  dt-bindings: net: dsa: mediatek,mt7530: add dsa-port definition for
    mt7988
  dt-bindings: net: dsa: mediatek,mt7530: add internal mdio bus
  dt-bindings: interconnect: add mt7988-cci compatible
  arm64: dts: mediatek: mt7988: add cci node
  arm64: dts: mediatek: mt7988: add basic ethernet-nodes
  arm64: dts: mediatek: mt7988: add switch node
  arm64: dts: mediatek: mt7988a-bpi-r4: add proc-supply for cci
  arm64: dts: mediatek: mt7988a-bpi-r4: drop unused pins
  arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds
  arm64: dts: mediatek: mt7988a-bpi-r4: add aliase for ethernet
  arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac
  arm64: dts: mediatek: mt7988a-bpi-r4: configure switch phys and leds

 .../bindings/interconnect/mediatek,cci.yaml   |  11 +-
 .../bindings/net/dsa/mediatek,mt7530.yaml     |  24 +-
 .../devicetree/bindings/net/mediatek,net.yaml |  10 +-
 .../mediatek/mt7988a-bananapi-bpi-r4-2g5.dts  |  12 +
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dts  |  18 +
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 197 ++++++-----
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi     | 310 +++++++++++++++++-
 7 files changed, 484 insertions(+), 98 deletions(-)

-- 
2.43.0

[PATCH v3 01/13] dt-bindings: net: mediatek,net: update for mt7988

[Frank Wunderlich]

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

Update binding for mt7988 which has 3 gmac and 2 reg items.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
- change reg to list of items
---
 .../devicetree/bindings/net/mediatek,net.yaml          | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
index 9e02fd80af83..e5dc35cfe4e4 100644
--- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
@@ -28,7 +28,10 @@ properties:
       - ralink,rt5350-eth
 
   reg:
-    maxItems: 1
+    items:
+      - description: Register for accessing the MACs.
+      - description: SoC internal SRAM used for DMA operations.
+    minItems: 1
 
   clocks:
     minItems: 2
@@ -381,8 +384,11 @@ allOf:
             - const: xgp2
             - const: xgp3
 
+        reg:
+          minItems: 2
+
 patternProperties:
-  "^mac@[0-1]$":
+  "^mac@[0-2]$":
     type: object
     unevaluatedProperties: false
     allOf:
-- 
2.43.0

[PATCH v3 02/13] dt-bindings: net: dsa: mediatek,mt7530: add dsa-port definition for mt7988

[Frank Wunderlich]

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

Add own dsa-port binding for SoC with internal switch where only phy-mode
'internal' is valid.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml          | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 51205f9f2985..9b983fdbf3c7 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -190,6 +190,18 @@ required:
   - reg
 
 $defs:
+  builtin-dsa-port:
+    patternProperties:
+      "^(ethernet-)?ports$":
+        patternProperties:
+          "^(ethernet-)?port@[0-6]$":
+            if:
+              required: [ ethernet ]
+            then:
+              properties:
+                phy-mode:
+                  const: internal
+
   mt7530-dsa-port:
     patternProperties:
       "^(ethernet-)?ports$":
@@ -297,7 +309,7 @@ allOf:
             - airoha,en7581-switch
             - airoha,an7583-switch
     then:
-      $ref: "#/$defs/mt7530-dsa-port"
+      $ref: "#/$defs/builtin-dsa-port"
       properties:
         gpio-controller: false
         mediatek,mcm: false
-- 
2.43.0

[PATCH v3 03/13] dt-bindings: net: dsa: mediatek,mt7530: add internal mdio bus

[Frank Wunderlich]

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

Mt7988 buildin switch has own mdio bus where ge-phys are connected.
Add related property for this.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
- change from patternproperty to property
- add unevaluatedProperties and mediatek,pio subproperty
---
 .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml   | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 9b983fdbf3c7..815a90808901 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -136,6 +136,16 @@ properties:
       See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt for
       details for the regulator setup on these boards.
 
+  mdio:
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      mediatek,pio:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description:
+          Phandle pointing to the mediatek pinctrl node.
+
   mediatek,mcm:
     type: boolean
     description:
-- 
2.43.0

[PATCH v3 04/13] dt-bindings: interconnect: add mt7988-cci compatible

[Frank Wunderlich]

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

Add compatible for Mediatek MT7988 SoC with mediatek,mt8183-cci fallback
which is taken by driver.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
v2:
- no RFC
- drop "items" as sugested by conor
---
 .../bindings/interconnect/mediatek,cci.yaml           | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml
index 58611ba2a0f4..4d72525f407e 100644
--- a/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml
+++ b/Documentation/devicetree/bindings/interconnect/mediatek,cci.yaml
@@ -17,9 +17,14 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - mediatek,mt8183-cci
-      - mediatek,mt8186-cci
+    oneOf:
+      - enum:
+          - mediatek,mt8183-cci
+          - mediatek,mt8186-cci
+      - items:
+          - enum:
+              - mediatek,mt7988-cci
+          - const: mediatek,mt8183-cci
 
   clocks:
     items:
-- 
2.43.0

[PATCH v3 05/13] arm64: dts: mediatek: mt7988: add cci node

[Frank Wunderlich]

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

Add cci devicetree node for cpu frequency scaling.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v3:
- add mt7988-cci compatible as suggested by angelo
---
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 33 +++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index c46b31f8d653..560ec86dbec0 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -12,6 +12,35 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	cci: cci {
+		compatible = "mediatek,mt7988-cci", "mediatek,mt8183-cci";
+		clocks = <&mcusys CLK_MCU_BUS_DIV_SEL>,
+			 <&topckgen CLK_TOP_XTAL>;
+		clock-names = "cci", "intermediate";
+		operating-points-v2 = <&cci_opp>;
+	};
+
+	cci_opp: opp-table-cci {
+		compatible = "operating-points-v2";
+		opp-shared;
+		opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			opp-microvolt = <850000>;
+		};
+		opp-660000000 {
+			opp-hz = /bits/ 64 <660000000>;
+			opp-microvolt = <850000>;
+		};
+		opp-900000000 {
+			opp-hz = /bits/ 64 <900000000>;
+			opp-microvolt = <850000>;
+		};
+		opp-1080000000 {
+			opp-hz = /bits/ 64 <1080000000>;
+			opp-microvolt = <900000>;
+		};
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -25,6 +54,7 @@ cpu0: cpu@0 {
 				 <&topckgen CLK_TOP_XTAL>;
 			clock-names = "cpu", "intermediate";
 			operating-points-v2 = <&cluster0_opp>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu1: cpu@1 {
@@ -36,6 +66,7 @@ cpu1: cpu@1 {
 				 <&topckgen CLK_TOP_XTAL>;
 			clock-names = "cpu", "intermediate";
 			operating-points-v2 = <&cluster0_opp>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu2: cpu@2 {
@@ -47,6 +78,7 @@ cpu2: cpu@2 {
 				 <&topckgen CLK_TOP_XTAL>;
 			clock-names = "cpu", "intermediate";
 			operating-points-v2 = <&cluster0_opp>;
+			mediatek,cci = <&cci>;
 		};
 
 		cpu3: cpu@3 {
@@ -58,6 +90,7 @@ cpu3: cpu@3 {
 				 <&topckgen CLK_TOP_XTAL>;
 			clock-names = "cpu", "intermediate";
 			operating-points-v2 = <&cluster0_opp>;
+			mediatek,cci = <&cci>;
 		};
 
 		cluster0_opp: opp-table-0 {
-- 
2.43.0

[PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Frank Wunderlich]

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

Add basic ethernet related nodes.

Mac1+2 needs pcs (sgmii+usxgmii) to work correctly which will be linked
later when driver is merged.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 124 +++++++++++++++++++++-
 1 file changed, 121 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index 560ec86dbec0..ee1e01d720fe 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -680,7 +680,28 @@ xphyu3port0: usb-phy@11e13000 {
 			};
 		};
 
-		clock-controller@11f40000 {
+		xfi_tphy0: phy@11f20000 {
+			compatible = "mediatek,mt7988-xfi-tphy";
+			reg = <0 0x11f20000 0 0x10000>;
+			resets = <&watchdog 14>;
+			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
+				 <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>;
+			clock-names = "xfipll", "topxtal";
+			mediatek,usxgmii-performance-errata;
+			#phy-cells = <0>;
+		};
+
+		xfi_tphy1: phy@11f30000 {
+			compatible = "mediatek,mt7988-xfi-tphy";
+			reg = <0 0x11f30000 0 0x10000>;
+			resets = <&watchdog 15>;
+			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
+				 <&topckgen CLK_TOP_XFI_PHY_1_XTAL_SEL>;
+			clock-names = "xfipll", "topxtal";
+			#phy-cells = <0>;
+		};
+
+		xfi_pll: clock-controller@11f40000 {
 			compatible = "mediatek,mt7988-xfi-pll";
 			reg = <0 0x11f40000 0 0x1000>;
 			resets = <&watchdog 16>;
@@ -714,19 +735,116 @@ phy_calibration_p3: calib@97c {
 			};
 		};
 
-		clock-controller@15000000 {
+		ethsys: clock-controller@15000000 {
 			compatible = "mediatek,mt7988-ethsys", "syscon";
 			reg = <0 0x15000000 0 0x1000>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
 
-		clock-controller@15031000 {
+		ethwarp: clock-controller@15031000 {
 			compatible = "mediatek,mt7988-ethwarp";
 			reg = <0 0x15031000 0 0x1000>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
+
+		eth: ethernet@15100000 {
+			compatible = "mediatek,mt7988-eth";
+			reg = <0 0x15100000 0 0x80000>,
+			      <0 0x15400000 0 0x200000>;
+			interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ethsys CLK_ETHDMA_CRYPT0_EN>,
+				 <&ethsys CLK_ETHDMA_FE_EN>,
+				 <&ethsys CLK_ETHDMA_GP2_EN>,
+				 <&ethsys CLK_ETHDMA_GP1_EN>,
+				 <&ethsys CLK_ETHDMA_GP3_EN>,
+				 <&ethwarp CLK_ETHWARP_WOCPU2_EN>,
+				 <&ethwarp CLK_ETHWARP_WOCPU1_EN>,
+				 <&ethwarp CLK_ETHWARP_WOCPU0_EN>,
+				 <&ethsys CLK_ETHDMA_ESW_EN>,
+				 <&topckgen CLK_TOP_ETH_GMII_SEL>,
+				 <&topckgen CLK_TOP_ETH_REFCK_50M_SEL>,
+				 <&topckgen CLK_TOP_ETH_SYS_200M_SEL>,
+				 <&topckgen CLK_TOP_ETH_SYS_SEL>,
+				 <&topckgen CLK_TOP_ETH_XGMII_SEL>,
+				 <&topckgen CLK_TOP_ETH_MII_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_500M_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_PAO_2X_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_SYNC_250M_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_PPEFB_250M_SEL>,
+				 <&topckgen CLK_TOP_NETSYS_WARP_SEL>,
+				 <&ethsys CLK_ETHDMA_XGP1_EN>,
+				 <&ethsys CLK_ETHDMA_XGP2_EN>,
+				 <&ethsys CLK_ETHDMA_XGP3_EN>;
+			clock-names = "crypto", "fe", "gp2", "gp1",
+				      "gp3",
+				      "ethwarp_wocpu2", "ethwarp_wocpu1",
+				      "ethwarp_wocpu0", "esw", "top_eth_gmii_sel",
+				      "top_eth_refck_50m_sel", "top_eth_sys_200m_sel",
+				      "top_eth_sys_sel", "top_eth_xgmii_sel",
+				      "top_eth_mii_sel", "top_netsys_sel",
+				      "top_netsys_500m_sel", "top_netsys_pao_2x_sel",
+				      "top_netsys_sync_250m_sel",
+				      "top_netsys_ppefb_250m_sel",
+				      "top_netsys_warp_sel","xgp1", "xgp2", "xgp3";
+			assigned-clocks = <&topckgen CLK_TOP_NETSYS_2X_SEL>,
+					  <&topckgen CLK_TOP_NETSYS_GSW_SEL>,
+					  <&topckgen CLK_TOP_USXGMII_SBUS_0_SEL>,
+					  <&topckgen CLK_TOP_USXGMII_SBUS_1_SEL>,
+					  <&topckgen CLK_TOP_SGM_0_SEL>,
+					  <&topckgen CLK_TOP_SGM_1_SEL>;
+			assigned-clock-parents = <&apmixedsys CLK_APMIXED_NET2PLL>,
+						 <&topckgen CLK_TOP_NET1PLL_D4>,
+						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
+						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
+						 <&apmixedsys CLK_APMIXED_SGMPLL>,
+						 <&apmixedsys CLK_APMIXED_SGMPLL>;
+			mediatek,ethsys = <&ethsys>;
+			mediatek,infracfg = <&topmisc>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			gmac0: mac@0 {
+				compatible = "mediatek,eth-mac";
+				reg = <0>;
+				phy-mode = "internal";
+
+				fixed-link {
+					speed = <10000>;
+					full-duplex;
+					pause;
+				};
+			};
+
+			gmac1: mac@1 {
+				compatible = "mediatek,eth-mac";
+				reg = <1>;
+				status = "disabled";
+			};
+
+			gmac2: mac@2 {
+				compatible = "mediatek,eth-mac";
+				reg = <2>;
+				status = "disabled";
+			};
+
+			mdio_bus: mdio-bus {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/* internal 2.5G PHY */
+				int_2p5g_phy: ethernet-phy@f {
+					reg = <15>;
+					compatible = "ethernet-phy-ieee802.3-c45";
+					phy-mode = "internal";
+				};
+			};
+		};
 	};
 
 	thermal-zones {
-- 
2.43.0

[PATCH v3 07/13] arm64: dts: mediatek: mt7988: add switch node

[Frank Wunderlich]

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

Add mt7988 builtin mt753x switch nodes.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v2:
- drop labels and led-function too (have to be in board)
---
 arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 153 ++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
index ee1e01d720fe..0b35a32b9c89 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
@@ -742,6 +742,159 @@ ethsys: clock-controller@15000000 {
 			#reset-cells = <1>;
 		};
 
+		switch: switch@15020000 {
+			compatible = "mediatek,mt7988-switch";
+			reg = <0 0x15020000 0 0x8000>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&ethwarp MT7988_ETHWARP_RST_SWITCH>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				gsw_port0: port@0 {
+					reg = <0>;
+					phy-mode = "internal";
+					phy-handle = <&gsw_phy0>;
+				};
+
+				gsw_port1: port@1 {
+					reg = <1>;
+					phy-mode = "internal";
+					phy-handle = <&gsw_phy1>;
+				};
+
+				gsw_port2: port@2 {
+					reg = <2>;
+					phy-mode = "internal";
+					phy-handle = <&gsw_phy2>;
+				};
+
+				gsw_port3: port@3 {
+					reg = <3>;
+					phy-mode = "internal";
+					phy-handle = <&gsw_phy3>;
+				};
+
+				port@6 {
+					reg = <6>;
+					ethernet = <&gmac0>;
+					phy-mode = "internal";
+
+					fixed-link {
+						speed = <10000>;
+						full-duplex;
+						pause;
+					};
+				};
+			};
+
+			mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				mediatek,pio = <&pio>;
+
+				gsw_phy0: ethernet-phy@0 {
+					compatible = "ethernet-phy-ieee802.3-c22";
+					reg = <0>;
+					interrupts = <0>;
+					phy-mode = "internal";
+					nvmem-cells = <&phy_calibration_p0>;
+					nvmem-cell-names = "phy-cal-data";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						gsw_phy0_led0: led@0 {
+							reg = <0>;
+							status = "disabled";
+						};
+
+						gsw_phy0_led1: led@1 {
+							reg = <1>;
+							status = "disabled";
+						};
+					};
+				};
+
+				gsw_phy1: ethernet-phy@1 {
+					compatible = "ethernet-phy-ieee802.3-c22";
+					reg = <1>;
+					interrupts = <1>;
+					phy-mode = "internal";
+					nvmem-cells = <&phy_calibration_p1>;
+					nvmem-cell-names = "phy-cal-data";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						gsw_phy1_led0: led@0 {
+							reg = <0>;
+							status = "disabled";
+						};
+
+						gsw_phy1_led1: led@1 {
+							reg = <1>;
+							status = "disabled";
+						};
+					};
+				};
+
+				gsw_phy2: ethernet-phy@2 {
+					compatible = "ethernet-phy-ieee802.3-c22";
+					reg = <2>;
+					interrupts = <2>;
+					phy-mode = "internal";
+					nvmem-cells = <&phy_calibration_p2>;
+					nvmem-cell-names = "phy-cal-data";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						gsw_phy2_led0: led@0 {
+							reg = <0>;
+							status = "disabled";
+						};
+
+						gsw_phy2_led1: led@1 {
+							reg = <1>;
+							status = "disabled";
+						};
+					};
+				};
+
+				gsw_phy3: ethernet-phy@3 {
+					compatible = "ethernet-phy-ieee802.3-c22";
+					reg = <3>;
+					interrupts = <3>;
+					phy-mode = "internal";
+					nvmem-cells = <&phy_calibration_p3>;
+					nvmem-cell-names = "phy-cal-data";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						gsw_phy3_led0: led@0 {
+							reg = <0>;
+							status = "disabled";
+						};
+
+						gsw_phy3_led1: led@1 {
+							reg = <1>;
+							status = "disabled";
+						};
+					};
+				};
+			};
+		};
+
 		ethwarp: clock-controller@15031000 {
 			compatible = "mediatek,mt7988-ethwarp";
 			reg = <0 0x15031000 0 0x1000>;
-- 
2.43.0

[PATCH v3 08/13] arm64: dts: mediatek: mt7988a-bpi-r4: add proc-supply for cci

[Frank Wunderlich]

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

CCI requires proc-supply. Add it on board level.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index 81ba045e0e0e..afa9e3b2b16a 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -40,6 +40,10 @@ reg_3p3v: regulator-3p3v {
 	};
 };
 
+&cci {
+	proc-supply = <&rt5190_buck3>;
+};
+
 &cpu0 {
 	proc-supply = <&rt5190_buck3>;
 };
-- 
2.43.0

[PATCH v3 09/13] arm64: dts: mediatek: mt7988a-bpi-r4: drop unused pins

[Frank Wunderlich]

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

Pins were moved from SoC dtsi to Board level dtsi without cleaning up
to needed ones. Drop the unused pins now.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 89 -------------------
 1 file changed, 89 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index afa9e3b2b16a..30affedf84d4 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -223,18 +223,6 @@ &pcie3 {
 };
 
 &pio {
-	mdio0_pins: mdio0-pins {
-		mux {
-			function = "eth";
-			groups = "mdc_mdio0";
-		};
-
-		conf {
-			pins = "SMI_0_MDC", "SMI_0_MDIO";
-			drive-strength = <8>;
-		};
-	};
-
 	i2c0_pins: i2c0-g0-pins {
 		mux {
 			function = "i2c";
@@ -249,20 +237,6 @@ mux {
 		};
 	};
 
-	i2c1_sfp_pins: i2c1-sfp-g0-pins {
-		mux {
-			function = "i2c";
-			groups = "i2c1_sfp";
-		};
-	};
-
-	i2c2_0_pins: i2c2-g0-pins {
-		mux {
-			function = "i2c";
-			groups = "i2c2_0";
-		};
-	};
-
 	i2c2_1_pins: i2c2-g1-pins {
 		mux {
 			function = "i2c";
@@ -298,34 +272,6 @@ mux {
 		};
 	};
 
-	gbe0_led1_pins: gbe0-led1-pins {
-		mux {
-			function = "led";
-			groups = "gbe0_led1";
-		};
-	};
-
-	gbe1_led1_pins: gbe1-led1-pins {
-		mux {
-			function = "led";
-			groups = "gbe1_led1";
-		};
-	};
-
-	gbe2_led1_pins: gbe2-led1-pins {
-		mux {
-			function = "led";
-			groups = "gbe2_led1";
-		};
-	};
-
-	gbe3_led1_pins: gbe3-led1-pins {
-		mux {
-			function = "led";
-			groups = "gbe3_led1";
-		};
-	};
-
 	i2p5gbe_led0_pins: 2p5gbe-led0-pins {
 		mux {
 			function = "led";
@@ -333,13 +279,6 @@ mux {
 		};
 	};
 
-	i2p5gbe_led1_pins: 2p5gbe-led1-pins {
-		mux {
-			function = "led";
-			groups = "2p5gbe_led1";
-		};
-	};
-
 	mmc0_pins_emmc_45: mmc0-emmc-45-pins {
 		mux {
 			function = "flash";
@@ -361,40 +300,12 @@ mux {
 		};
 	};
 
-	snfi_pins: snfi-pins {
-		mux {
-			function = "flash";
-			groups = "snfi";
-		};
-	};
-
-	spi0_pins: spi0-pins {
-		mux {
-			function = "spi";
-			groups = "spi0";
-		};
-	};
-
 	spi0_flash_pins: spi0-flash-pins {
 		mux {
 			function = "spi";
 			groups = "spi0", "spi0_wp_hold";
 		};
 	};
-
-	spi2_pins: spi2-pins {
-		mux {
-			function = "spi";
-			groups = "spi2";
-		};
-	};
-
-	spi2_flash_pins: spi2-flash-pins {
-		mux {
-			function = "spi";
-			groups = "spi2", "spi2_wp_hold";
-		};
-	};
 };
 
 &pwm {
-- 
2.43.0

[PATCH v3 10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds

[Frank Wunderlich]

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

Bananapi R4 has a green and a blue led which can be switched by gpio.
Green led is for running state so default on.

Green led also shares pin with eeprom writeprotect where led off allows
writing to eeprom.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index 30affedf84d4..21eb91c8609f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -21,6 +21,25 @@ fan: pwm-fan {
 		status = "okay";
 	};
 
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led_green: led-green {
+			function = LED_FUNCTION_STATUS;
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&pio 79 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+
+		led_blue: led-blue {
+			function = LED_FUNCTION_WPS;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 63 GPIO_ACTIVE_HIGH>;
+			default-state = "off";
+		};
+	};
+
+
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
 		regulator-name = "fixed-1.8V";
-- 
2.43.0

[PATCH v3 11/13] arm64: dts: mediatek: mt7988a-bpi-r4: add aliase for ethernet

[Frank Wunderlich]

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

Add aliase for gmacs to allow bootloader setting mac-adresses.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index 21eb91c8609f..20073eb4d1bd 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -8,6 +8,12 @@
 #include "mt7988a.dtsi"
 
 / {
+	aliases {
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
+		ethernet2 = &gmac2;
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
-- 
2.43.0

[PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Frank Wunderlich]

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

Add SFP cages to Bananapi-R4 board. The 2.5g phy variant only contains the
wan-SFP, so add this to common dtsi and the lan-sfp only to the dual-SFP
variant.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v3:
- enable mac with 2.5g phy on r4 phy variant because driver is now mainline
---
 .../mediatek/mt7988a-bananapi-bpi-r4-2g5.dts   | 12 ++++++++++++
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dts   | 18 ++++++++++++++++++
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi  | 18 ++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
index 53de9c113f60..e63e17ae35a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
@@ -9,3 +9,15 @@ / {
 	model = "Banana Pi BPI-R4 (1x SFP+, 1x 2.5GbE)";
 	chassis-type = "embedded";
 };
+
+&gmac1 {
+	phy-mode = "internal";
+	phy-connection-type = "internal";
+	phy = <&int_2p5g_phy>;
+	status = "okay";
+};
+
+&int_2p5g_phy {
+	pinctrl-names = "i2p5gbe-led";
+	pinctrl-0 = <&i2p5gbe_led0_pins>;
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
index 36bd1ef2efab..3136dc4ba4cc 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
@@ -8,6 +8,24 @@ / {
 	compatible = "bananapi,bpi-r4", "mediatek,mt7988a";
 	model = "Banana Pi BPI-R4 (2x SFP+)";
 	chassis-type = "embedded";
+
+	/* SFP2 cage (LAN) */
+	sfp2: sfp2 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c_sfp2>;
+		los-gpios = <&pio 2 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&pio 83 GPIO_ACTIVE_LOW>;
+		tx-disable-gpios = <&pio 0 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpios = <&pio 1 GPIO_ACTIVE_HIGH>;
+		rate-select0-gpios = <&pio 3 GPIO_ACTIVE_LOW>;
+		maximum-power-milliwatt = <3000>;
+	};
+};
+
+&gmac1 {
+	sfp = <&sfp2>;
+	managed = "in-band-status";
+	phy-mode = "usxgmii";
 };
 
 &pca9545 {
diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index 20073eb4d1bd..d8b9cd794ee3 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -63,6 +63,18 @@ reg_3p3v: regulator-3p3v {
 		regulator-boot-on;
 		regulator-always-on;
 	};
+
+	/* SFP1 cage (WAN) */
+	sfp1: sfp1 {
+		compatible = "sff,sfp";
+		i2c-bus = <&i2c_sfp1>;
+		los-gpios = <&pio 54 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&pio 82 GPIO_ACTIVE_LOW>;
+		tx-disable-gpios = <&pio 70 GPIO_ACTIVE_HIGH>;
+		tx-fault-gpios = <&pio 69 GPIO_ACTIVE_HIGH>;
+		rate-select0-gpios = <&pio 21 GPIO_ACTIVE_LOW>;
+		maximum-power-milliwatt = <3000>;
+	};
 };
 
 &cci {
@@ -133,6 +145,12 @@ map-cpu-active-low {
 	};
 };
 
+&gmac2 {
+	sfp = <&sfp1>;
+	managed = "in-band-status";
+	phy-mode = "usxgmii";
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0_pins>;
-- 
2.43.0

[PATCH v3 13/13] arm64: dts: mediatek: mt7988a-bpi-r4: configure switch phys and leds

[Frank Wunderlich]

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

Assign pinctrl to switch phys and leds.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
v2:
- add labels and led-function and include after dropping from soc dtsi
---
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index d8b9cd794ee3..f10d3617dcac 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -4,6 +4,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/regulator/richtek,rt5190a-regulator.h>
+#include <dt-bindings/leds/common.h>
 
 #include "mt7988a.dtsi"
 
@@ -151,6 +152,66 @@ &gmac2 {
 	phy-mode = "usxgmii";
 };
 
+&gsw_phy0 {
+	pinctrl-names = "gbe-led";
+	pinctrl-0 = <&gbe0_led0_pins>;
+};
+
+&gsw_phy0_led0 {
+	status = "okay";
+	function = LED_FUNCTION_WAN;
+	color = <LED_COLOR_ID_GREEN>;
+};
+
+&gsw_port0 {
+	label = "wan";
+};
+
+&gsw_phy1 {
+	pinctrl-names = "gbe-led";
+	pinctrl-0 = <&gbe1_led0_pins>;
+};
+
+&gsw_phy1_led0 {
+	status = "okay";
+	function = LED_FUNCTION_LAN;
+	color = <LED_COLOR_ID_GREEN>;
+};
+
+&gsw_port1 {
+	label = "lan1";
+};
+
+&gsw_phy2 {
+	pinctrl-names = "gbe-led";
+	pinctrl-0 = <&gbe2_led0_pins>;
+};
+
+&gsw_phy2_led0 {
+	status = "okay";
+	function = LED_FUNCTION_LAN;
+	color = <LED_COLOR_ID_GREEN>;
+};
+
+&gsw_port2 {
+	label = "lan2";
+};
+
+&gsw_phy3 {
+	pinctrl-names = "gbe-led";
+	function = LED_FUNCTION_LAN;
+	pinctrl-0 = <&gbe3_led0_pins>;
+};
+
+&gsw_phy3_led0 {
+	status = "okay";
+	color = <LED_COLOR_ID_GREEN>;
+};
+
+&gsw_port3 {
+	label = "lan3";
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0_pins>;
-- 
2.43.0

Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Andrew Lunn]

> +			gmac0: mac@0 {
> +				compatible = "mediatek,eth-mac";
> +				reg = <0>;
> +				phy-mode = "internal";
> +
> +				fixed-link {
> +					speed = <10000>;
> +					full-duplex;
> +					pause;
> +				};

Maybe i've asked this before? What is on the other end of this link?
phy-mode internal and fixed link seems an odd combination. It might
just need some comments, if this is internally connected to a switch.

> +			mdio_bus: mdio-bus {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				/* internal 2.5G PHY */
> +				int_2p5g_phy: ethernet-phy@f {
> +					reg = <15>;

It is a bit odd mixing hex and decimal.

> +					compatible = "ethernet-phy-ieee802.3-c45";

I _think_ the coding standard say the compatible should be first.

> +					phy-mode = "internal";

A phy should not have a phy-mode.

	Andrew

Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Daniel Golle]

On Sun, Jun 08, 2025 at 11:14:39PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add basic ethernet related nodes.
> 
> Mac1+2 needs pcs (sgmii+usxgmii) to work correctly which will be linked
> later when driver is merged.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 124 +++++++++++++++++++++-
>  1 file changed, 121 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> index 560ec86dbec0..ee1e01d720fe 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> +
> +		eth: ethernet@15100000 {
> +			compatible = "mediatek,mt7988-eth";
> +			reg = <0 0x15100000 0 0x80000>,
> +			      <0 0x15400000 0 0x200000>;
> +			interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;

It would be better to use MT7988 with RSS and add the additional
interrupts for doing so before introducing support for this SoC without
RSS. In this way we would avoid having to deal with keeping the DT
support compatible with the old (ie. 4 IRQs) way while also supporting the
RSS-way (with a total of 8 IRQs).
Alternatively, if we really want to support MT7988 with and without RSS we
should use 'interrupt-names' instead of unnamed interrupts imho.

Re: [PATCH v3 07/13] arm64: dts: mediatek: mt7988: add switch node

[Andrew Lunn]

> +			mdio {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				mediatek,pio = <&pio>;
> +
> +				gsw_phy0: ethernet-phy@0 {
> +					compatible = "ethernet-phy-ieee802.3-c22";
> +					reg = <0>;
> +					interrupts = <0>;
> +					phy-mode = "internal";

More phys with a phy-mode?

	Andrew

Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Andrew Lunn]

> +&gmac1 {
> +	phy-mode = "internal";
> +	phy-connection-type = "internal";

ethernet-controller.yaml says:

  phy-connection-type:
    description:
      Specifies interface type between the Ethernet device and a physical
      layer (PHY) device.
    enum:
      # There is not a standard bus between the MAC and the PHY,
      # something proprietary is being used to embed the PHY in the
      # MAC.
      - internal
      - mii
      - gmii
  ...

  phy-mode:
    $ref: "#/properties/phy-connection-type"


so phy-mode and phy-connection-type are the same thing.

> +	/* SFP2 cage (LAN) */
> +	sfp2: sfp2 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c_sfp2>;
> +		los-gpios = <&pio 2 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&pio 83 GPIO_ACTIVE_LOW>;
> +		tx-disable-gpios = <&pio 0 GPIO_ACTIVE_HIGH>;
> +		tx-fault-gpios = <&pio 1 GPIO_ACTIVE_HIGH>;
> +		rate-select0-gpios = <&pio 3 GPIO_ACTIVE_LOW>;
> +		maximum-power-milliwatt = <3000>;

sff,sfp.yaml says:

  maximum-power-milliwatt:
    minimum: 1000
    default: 1000
    description:
      Maximum module power consumption Specifies the maximum power consumption
      allowable by a module in the slot, in milli-Watts. Presently, modules can
      be up to 1W, 1.5W or 2W.

I've no idea what will happen when the SFP core sees 3000. Is the
comment out of date?

	Andrew

Aw: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Frank Wunderlich]

Hi Andrew

> Gesendet: Sonntag, 8. Juni 2025 um 23:23
> Von: "Andrew Lunn" <andrew@lunn.ch>
> An: "Frank Wunderlich" <linux@fw-web.de>
> Betreff: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes
>
> > +			gmac0: mac@0 {
> > +				compatible = "mediatek,eth-mac";
> > +				reg = <0>;
> > +				phy-mode = "internal";
> > +
> > +				fixed-link {
> > +					speed = <10000>;
> > +					full-duplex;
> > +					pause;
> > +				};
> 
> Maybe i've asked this before? What is on the other end of this link?
> phy-mode internal and fixed link seems an odd combination. It might
> just need some comments, if this is internally connected to a switch.

yes you've asked in v1 and i responded :)

https://patchwork.kernel.org/project/linux-mediatek/patch/20250511141942.10284-9-linux@fw-web.de/

connected to internal (mt7530) switch. Which kind of comment do you want here? Only "connected to internal switch"
or some more details?

> > +			mdio_bus: mdio-bus {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				/* internal 2.5G PHY */
> > +				int_2p5g_phy: ethernet-phy@f {
> > +					reg = <15>;
> 
> It is a bit odd mixing hex and decimal.

do you prefer hex or decimal for both? for r3mini i used decimal for both, so i would change unit-address
to 15.

> > +					compatible = "ethernet-phy-ieee802.3-c45";
> 
> I _think_ the coding standard say the compatible should be first.

i can move this up of course

> > +					phy-mode = "internal";
> 
> A phy should not have a phy-mode.

not sure if this is needed for mt7988 internal 2.5g phy driver, but seems not when i look at the driver
(drivers/net/phy/mediatek/mtk-2p5ge.c). The switch phys also use this and also here i do not see any
access in the driver (drivers/net/dsa/mt7530-mmio.c + mt7530.c) on a quick look.
Afaik binding required the property and should be read by phylink (to be not unknown, but looks like
handled the same way).

Maybe daniel can describe a bit deeper.

> 	Andrew

regards Frank

Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Andrew Lunn]

> > > +			gmac0: mac@0 {
> > > +				compatible = "mediatek,eth-mac";
> > > +				reg = <0>;
> > > +				phy-mode = "internal";
> > > +
> > > +				fixed-link {
> > > +					speed = <10000>;
> > > +					full-duplex;
> > > +					pause;
> > > +				};
> > 
> > Maybe i've asked this before? What is on the other end of this link?
> > phy-mode internal and fixed link seems an odd combination. It might
> > just need some comments, if this is internally connected to a switch.
> 
> yes you've asked in v1 and i responded :)
> 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20250511141942.10284-9-linux@fw-web.de/
> 
> connected to internal (mt7530) switch. Which kind of comment do you want here? Only "connected to internal switch"
> or some more details?

"Connected to internal switch" will do. The word switch explains the
fixed-link, and internal the phy-mode.

It is not the case here, but i've seen DT misused like this because
the MAC is connected to a PHY and there is no PHY driver yet, so a
fixed link is used instead.

> > > +			mdio_bus: mdio-bus {
> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +
> > > +				/* internal 2.5G PHY */
> > > +				int_2p5g_phy: ethernet-phy@f {
> > > +					reg = <15>;
> > 
> > It is a bit odd mixing hex and decimal.
> 
> do you prefer hex or decimal for both? for r3mini i used decimal for both, so i would change unit-address
> to 15.

I suspect decimal is more common, but i don't care.

> 
> > > +					compatible = "ethernet-phy-ieee802.3-c45";
> > 
> > I _think_ the coding standard say the compatible should be first.
> 
> i can move this up of course
> 
> > > +					phy-mode = "internal";
> > 
> > A phy should not have a phy-mode.
> 
> not sure if this is needed for mt7988 internal 2.5g phy driver, but seems not when i look at the driver
> (drivers/net/phy/mediatek/mtk-2p5ge.c). The switch phys also use this and also here i do not see any
> access in the driver (drivers/net/dsa/mt7530-mmio.c + mt7530.c) on a quick look.
> Afaik binding required the property and should be read by phylink (to be not unknown, but looks like
> handled the same way).

Which binding requires this? This is a PHY node, but i don't see
anything about it in ethernet-phy.yaml.

	Andrew

Aw: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Frank Wunderlich]

Hi Andrew

> Gesendet: Sonntag, 8. Juni 2025 um 23:31
> Von: "Andrew Lunn" <andrew@lunn.ch>
> Betreff: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac
>
> > +&gmac1 {
> > +	phy-mode = "internal";
> > +	phy-connection-type = "internal";
> 
> ethernet-controller.yaml says:
> 
>   phy-connection-type:
>     description:
>       Specifies interface type between the Ethernet device and a physical
>       layer (PHY) device.
>     enum:
>       # There is not a standard bus between the MAC and the PHY,
>       # something proprietary is being used to embed the PHY in the
>       # MAC.
>       - internal
>       - mii
>       - gmii
>   ...
> 
>   phy-mode:
>     $ref: "#/properties/phy-connection-type"
> 
> 
> so phy-mode and phy-connection-type are the same thing.

phy-connection-type seems not needed, tested without it and 2.5G phy works without this property in the 2g5 dts.

> > +	/* SFP2 cage (LAN) */
> > +	sfp2: sfp2 {
> > +		compatible = "sff,sfp";
> > +		i2c-bus = <&i2c_sfp2>;
> > +		los-gpios = <&pio 2 GPIO_ACTIVE_HIGH>;
> > +		mod-def0-gpios = <&pio 83 GPIO_ACTIVE_LOW>;
> > +		tx-disable-gpios = <&pio 0 GPIO_ACTIVE_HIGH>;
> > +		tx-fault-gpios = <&pio 1 GPIO_ACTIVE_HIGH>;
> > +		rate-select0-gpios = <&pio 3 GPIO_ACTIVE_LOW>;
> > +		maximum-power-milliwatt = <3000>;
> 
> sff,sfp.yaml says:
> 
>   maximum-power-milliwatt:
>     minimum: 1000
>     default: 1000
>     description:
>       Maximum module power consumption Specifies the maximum power consumption
>       allowable by a module in the slot, in milli-Watts. Presently, modules can
>       be up to 1W, 1.5W or 2W.
> 
> I've no idea what will happen when the SFP core sees 3000. Is the
> comment out of date?

at least sfp-core has no issue with the setting

root@bpi-r4-phy-8G:~# dmesg | grep sfp
[    1.269437] sfp sfp1: Host maximum power 3.0W
[    1.613749] sfp sfp1: module CISCO-FINISAR    FTLX8571D3BCL-C2 rev A    sn S2209167650      dc 220916  

imho some modules require more than 2W (some gpon/xpon and 10G copper ethernet).

> 	Andrew
> 

regards Frank

Aw: Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Frank Wunderlich]

Hi


> Gesendet: Montag, 9. Juni 2025 um 14:12
> Von: "Andrew Lunn" <andrew@lunn.ch>
> An: "Frank Wunderlich" <frank-w@public-files.de>
> Betreff: Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes
>
> > > > +			gmac0: mac@0 {
> > > > +				compatible = "mediatek,eth-mac";
> > > > +				reg = <0>;
> > > > +				phy-mode = "internal";
> > > > +
> > > > +				fixed-link {
> > > > +					speed = <10000>;
> > > > +					full-duplex;
> > > > +					pause;
> > > > +				};
> > > 
> > > Maybe i've asked this before? What is on the other end of this link?
> > > phy-mode internal and fixed link seems an odd combination. It might
> > > just need some comments, if this is internally connected to a switch.
> > 
> > yes you've asked in v1 and i responded :)
> > 
> > https://patchwork.kernel.org/project/linux-mediatek/patch/20250511141942.10284-9-linux@fw-web.de/
> > 
> > connected to internal (mt7530) switch. Which kind of comment do you want here? Only "connected to internal switch"
> > or some more details?
> 
> "Connected to internal switch" will do. The word switch explains the
> fixed-link, and internal the phy-mode.
> 
> It is not the case here, but i've seen DT misused like this because
> the MAC is connected to a PHY and there is no PHY driver yet, so a
> fixed link is used instead.
> 
> > > > +			mdio_bus: mdio-bus {
> > > > +				#address-cells = <1>;
> > > > +				#size-cells = <0>;
> > > > +
> > > > +				/* internal 2.5G PHY */
> > > > +				int_2p5g_phy: ethernet-phy@f {
> > > > +					reg = <15>;
> > > 
> > > It is a bit odd mixing hex and decimal.
> > 
> > do you prefer hex or decimal for both? for r3mini i used decimal for both, so i would change unit-address
> > to 15.
> 
> I suspect decimal is more common, but i don't care.
> 
> > 
> > > > +					compatible = "ethernet-phy-ieee802.3-c45";
> > > 
> > > I _think_ the coding standard say the compatible should be first.
> > 
> > i can move this up of course
> > 
> > > > +					phy-mode = "internal";
> > > 
> > > A phy should not have a phy-mode.
> > 
> > not sure if this is needed for mt7988 internal 2.5g phy driver, but seems not when i look at the driver
> > (drivers/net/phy/mediatek/mtk-2p5ge.c). The switch phys also use this and also here i do not see any
> > access in the driver (drivers/net/dsa/mt7530-mmio.c + mt7530.c) on a quick look.
> > Afaik binding required the property and should be read by phylink (to be not unknown, but looks like
> > handled the same way).
> 
> Which binding requires this? This is a PHY node, but i don't see
> anything about it in ethernet-phy.yaml.

seems like only the cpu-port on switch requires the phy-mode for binding...

  DTC [C] arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtb
arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtb: switch@15020000: ports:port@6: 'phy-mode' is a required property
	from schema $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#

tested on hardware with the others disabled:

- phy-mode on gsw-ports are needed else i get this:
[    1.386586] mt7530-mmio 15020000.switch wan (uninitialized): validation of gmii with support 0000000,00000000,00000000,000062ef and advertisement 0000000,00000000,00000000,000062ef failed: -EINVAL
[    1.408209] mt7530-mmio 15020000.switch wan (uninitialized): failed to connect to PHY: -EINVAL
[    1.421308] mt7530-mmio 15020000.switch wan (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 0
- phy-mode on gmac0 is needed alse ethernet-controller (and switch) does not get up

the phy-modes (gsw_phyX) can be dropped, the 2g5 phy-mode is currently set twice (mt7988a.dtsi and in 2g5 dts), so i would leave this in mt7988a.dtsi

> 
> 	Andrew

regards Frank

Re: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Andrew Lunn]

> > sff,sfp.yaml says:
> > 
> >   maximum-power-milliwatt:
> >     minimum: 1000
> >     default: 1000
> >     description:
> >       Maximum module power consumption Specifies the maximum power consumption
> >       allowable by a module in the slot, in milli-Watts. Presently, modules can
> >       be up to 1W, 1.5W or 2W.
> > 
> > I've no idea what will happen when the SFP core sees 3000. Is the
> > comment out of date?
> 
> at least sfp-core has no issue with the setting
> 
> root@bpi-r4-phy-8G:~# dmesg | grep sfp
> [    1.269437] sfp sfp1: Host maximum power 3.0W
> [    1.613749] sfp sfp1: module CISCO-FINISAR    FTLX8571D3BCL-C2 rev A    sn S2209167650      dc 220916  
> 
> imho some modules require more than 2W (some gpon/xpon and 10G copper ethernet).

Looking at the code:

static int sfp_module_parse_power(struct sfp *sfp)
{
        u32 power_mW = 1000;
        bool supports_a2;

        if (sfp->id.ext.sff8472_compliance >= SFP_SFF8472_COMPLIANCE_REV10_2 &&
            sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_POWER_DECL))
                power_mW = 1500;
        /* Added in Rev 11.9, but there is no compliance code for this */
        if (sfp->id.ext.sff8472_compliance >= SFP_SFF8472_COMPLIANCE_REV11_4 &&
            sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_HIGH_POWER_LEVEL))
                power_mW = 2000;

How does your module indicate it needs 3000 mW? Does this bit of code
need extending to read additional bits?

	Andrew

Re: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Frank Wunderlich]

Am 10. Juni 2025 14:58:44 MESZ schrieb Andrew Lunn <andrew@lunn.ch>:
>> > sff,sfp.yaml says:
>> > 
>> >   maximum-power-milliwatt:
>> >     minimum: 1000
>> >     default: 1000
>> >     description:
>> >       Maximum module power consumption Specifies the maximum power consumption
>> >       allowable by a module in the slot, in milli-Watts. Presently, modules can
>> >       be up to 1W, 1.5W or 2W.
>> > 
>> > I've no idea what will happen when the SFP core sees 3000. Is the
>> > comment out of date?
>> 
>> at least sfp-core has no issue with the setting
>> 
>> root@bpi-r4-phy-8G:~# dmesg | grep sfp
>> [    1.269437] sfp sfp1: Host maximum power 3.0W
>> [    1.613749] sfp sfp1: module CISCO-FINISAR    FTLX8571D3BCL-C2 rev A    sn S2209167650      dc 220916  
>> 
>> imho some modules require more than 2W (some gpon/xpon and 10G copper ethernet).
>
>Looking at the code:
>
>static int sfp_module_parse_power(struct sfp *sfp)
>{
>        u32 power_mW = 1000;
>        bool supports_a2;
>
>        if (sfp->id.ext.sff8472_compliance >= SFP_SFF8472_COMPLIANCE_REV10_2 &&
>            sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_POWER_DECL))
>                power_mW = 1500;
>        /* Added in Rev 11.9, but there is no compliance code for this */
>        if (sfp->id.ext.sff8472_compliance >= SFP_SFF8472_COMPLIANCE_REV11_4 &&
>            sfp->id.ext.options & cpu_to_be16(SFP_OPTIONS_HIGH_POWER_LEVEL))
>                power_mW = 2000;
>
>How does your module indicate it needs 3000 mW? Does this bit of code
>need extending to read additional bits?

Message says "host maximum power",not that sfp needs the 3w.

>	Andrew


regards Frank

Re: [PATCH v3 13/13] arm64: dts: mediatek: mt7988a-bpi-r4: configure switch phys and leds

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Assign pinctrl to switch phys and leds.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v2:
> - add labels and led-function and include after dropping from soc dtsi
> ---
>   .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 61 +++++++++++++++++++
>   1 file changed, 61 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> index d8b9cd794ee3..f10d3617dcac 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> @@ -4,6 +4,7 @@
>   
>   #include <dt-bindings/gpio/gpio.h>
>   #include <dt-bindings/regulator/richtek,rt5190a-regulator.h>
> +#include <dt-bindings/leds/common.h>
>   
>   #include "mt7988a.dtsi"
>   
> @@ -151,6 +152,66 @@ &gmac2 {
>   	phy-mode = "usxgmii";
>   };
>   
> +&gsw_phy0 {
> +	pinctrl-names = "gbe-led";
> +	pinctrl-0 = <&gbe0_led0_pins>;

pinctrl-0
pinctrl-names (here and everywhere else)

> +};
> +
> +&gsw_phy0_led0 {

function
color
status

> +	status = "okay";
> +	function = LED_FUNCTION_WAN;
> +	color = <LED_COLOR_ID_GREEN>;
> +};
> +
> +&gsw_port0 {
> +	label = "wan";
> +};
> +
> +&gsw_phy1 {
> +	pinctrl-names = "gbe-led";
> +	pinctrl-0 = <&gbe1_led0_pins>;
> +};
> +
> +&gsw_phy1_led0 {
> +	status = "okay";
> +	function = LED_FUNCTION_LAN;
> +	color = <LED_COLOR_ID_GREEN>;
> +};
> +
> +&gsw_port1 {
> +	label = "lan1";
> +};
> +
> +&gsw_phy2 {
> +	pinctrl-names = "gbe-led";
> +	pinctrl-0 = <&gbe2_led0_pins>;
> +};
> +
> +&gsw_phy2_led0 {
> +	status = "okay";
> +	function = LED_FUNCTION_LAN;
> +	color = <LED_COLOR_ID_GREEN>;
> +};
> +
> +&gsw_port2 {
> +	label = "lan2";
> +};
> +
> +&gsw_phy3 {

function

pinctrl-0
pinctrl-names


> +	pinctrl-names = "gbe-led";
> +	function = LED_FUNCTION_LAN;
> +	pinctrl-0 = <&gbe3_led0_pins>;
> +};
> +
> +&gsw_phy3_led0 {

color
status

> +	status = "okay";
> +	color = <LED_COLOR_ID_GREEN>;
> +};

...after which

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add SFP cages to Bananapi-R4 board. The 2.5g phy variant only contains the
> wan-SFP, so add this to common dtsi and the lan-sfp only to the dual-SFP
> variant.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v3:
> - enable mac with 2.5g phy on r4 phy variant because driver is now mainline
> ---
>   .../mediatek/mt7988a-bananapi-bpi-r4-2g5.dts   | 12 ++++++++++++
>   .../dts/mediatek/mt7988a-bananapi-bpi-r4.dts   | 18 ++++++++++++++++++
>   .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi  | 18 ++++++++++++++++++
>   3 files changed, 48 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> index 53de9c113f60..e63e17ae35a0 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> @@ -9,3 +9,15 @@ / {
>   	model = "Banana Pi BPI-R4 (1x SFP+, 1x 2.5GbE)";
>   	chassis-type = "embedded";
>   };
> +
> +&gmac1 {

phy = ...
phy-c..onnection-type
phy-m...ode

> +	phy-mode = "internal";
> +	phy-connection-type = "internal";
> +	phy = <&int_2p5g_phy>;
> +	status = "okay";
> +};
> +
> +&int_2p5g_phy {
> +	pinctrl-names = "i2p5gbe-led";
> +	pinctrl-0 = <&i2p5gbe_led0_pins>;

pinctrl-names
pinctrl-0

> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
> index 36bd1ef2efab..3136dc4ba4cc 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
> @@ -8,6 +8,24 @@ / {
>   	compatible = "bananapi,bpi-r4", "mediatek,mt7988a";
>   	model = "Banana Pi BPI-R4 (2x SFP+)";
>   	chassis-type = "embedded";
> +
> +	/* SFP2 cage (LAN) */
> +	sfp2: sfp2 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c_sfp2>;

maximum-power-milliwatt here
(blank line)
...then gpios

> +		los-gpios = <&pio 2 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&pio 83 GPIO_ACTIVE_LOW>;

(r)ate-select0-gpios before (t)x-disable....

> +		tx-disable-gpios = <&pio 0 GPIO_ACTIVE_HIGH>;
> +		tx-fault-gpios = <&pio 1 GPIO_ACTIVE_HIGH>;
> +		rate-select0-gpios = <&pio 3 GPIO_ACTIVE_LOW>;
> +		maximum-power-milliwatt = <3000>;
> +	};
> +};
> +
> +&gmac1 {
managed
phy-mode
sfp

> +	sfp = <&sfp2>;
> +	managed = "in-band-status";
> +	phy-mode = "usxgmii";
>   };
>   
>   &pca9545 {
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> index 20073eb4d1bd..d8b9cd794ee3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
> @@ -63,6 +63,18 @@ reg_3p3v: regulator-3p3v {
>   		regulator-boot-on;
>   		regulator-always-on;
>   	};
> +
> +	/* SFP1 cage (WAN) */
> +	sfp1: sfp1 {
> +		compatible = "sff,sfp";
> +		i2c-bus = <&i2c_sfp1>;

(same comments from sfp2)

> +		los-gpios = <&pio 54 GPIO_ACTIVE_HIGH>;
> +		mod-def0-gpios = <&pio 82 GPIO_ACTIVE_LOW>;
> +		tx-disable-gpios = <&pio 70 GPIO_ACTIVE_HIGH>;
> +		tx-fault-gpios = <&pio 69 GPIO_ACTIVE_HIGH>;
> +		rate-select0-gpios = <&pio 21 GPIO_ACTIVE_LOW>;
> +		maximum-power-milliwatt = <3000>;
> +	};
>   };
>   
>   &cci {
> @@ -133,6 +145,12 @@ map-cpu-active-low {
>   	};
>   };
>   
> +&gmac2 {

(same comments from gmac1)

> +	sfp = <&sfp1>;
> +	managed = "in-band-status";
> +	phy-mode = "usxgmii";
> +};
> +
>   &i2c0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&i2c0_pins>;


Cheers,
Angelo

Re: [PATCH v3 11/13] arm64: dts: mediatek: mt7988a-bpi-r4: add aliase for ethernet

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add aliase for gmacs to allow bootloader setting mac-adresses.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

s/aliase/aliases/g

after which

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Bananapi R4 has a green and a blue led which can be switched by gpio.
> Green led is for running state so default on.
> 
> Green led also shares pin with eeprom writeprotect where led off allows
> writing to eeprom.

This is extremely ugly from a hardware perspective and I'm sad to see such
a thing here.

Though, I don't see any other solution, so I can only give you an advice:
don't ever write to the eeprom from the kernel. Things will eventually go
south otherwise.

I would also advise to add a read-only property to the eeprom node.

Regardless (and sadly),

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 09/13] arm64: dts: mediatek: mt7988a-bpi-r4: drop unused pins

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Pins were moved from SoC dtsi to Board level dtsi without cleaning up
> to needed ones. Drop the unused pins now.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 08/13] arm64: dts: mediatek: mt7988a-bpi-r4: add proc-supply for cci

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> CCI requires proc-supply. Add it on board level.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 07/13] arm64: dts: mediatek: mt7988: add switch node

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add mt7988 builtin mt753x switch nodes.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> v2:
> - drop labels and led-function too (have to be in board)
> ---
>   arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 153 ++++++++++++++++++++++
>   1 file changed, 153 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> index ee1e01d720fe..0b35a32b9c89 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> @@ -742,6 +742,159 @@ ethsys: clock-controller@15000000 {
>   			#reset-cells = <1>;
>   		};
>   
> +		switch: switch@15020000 {
> +			compatible = "mediatek,mt7988-switch";
> +			reg = <0 0x15020000 0 0x8000>;
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +			interrupt-parent = <&gic>;

You don't need interrupt-parent, it's already GIC here... :-)

> +			interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> +			resets = <&ethwarp MT7988_ETHWARP_RST_SWITCH>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				gsw_port0: port@0 {
> +					reg = <0>;

Keep it ordered alphabetically - phy-handle (h) before phy-mode (m)

> +					phy-mode = "internal";
> +					phy-handle = <&gsw_phy0>;
> +				};
> +
> +				gsw_port1: port@1 {
> +					reg = <1>;
> +					phy-mode = "internal";
> +					phy-handle = <&gsw_phy1>;
> +				};
> +
> +				gsw_port2: port@2 {
> +					reg = <2>;
> +					phy-mode = "internal";
> +					phy-handle = <&gsw_phy2>;
> +				};
> +
> +				gsw_port3: port@3 {
> +					reg = <3>;
> +					phy-mode = "internal";
> +					phy-handle = <&gsw_phy3>;
> +				};
> +
> +				port@6 {
> +					reg = <6>;
> +					ethernet = <&gmac0>;
> +					phy-mode = "internal";
> +
> +					fixed-link {
> +						speed = <10000>;
> +						full-duplex;
> +						pause;
> +					};
> +				};
> +			};
> +
> +			mdio {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				mediatek,pio = <&pio>;
> +
> +				gsw_phy0: ethernet-phy@0 {
> +					compatible = "ethernet-phy-ieee802.3-c22";
> +					reg = <0>;
> +					interrupts = <0>;
> +					phy-mode = "internal";
> +					nvmem-cells = <&phy_calibration_p0>;
> +					nvmem-cell-names = "phy-cal-data";

phy-mode (p) after nvmem-cell-names (n) please (here and everywhere else)

> +
> +					leds {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +
> +						gsw_phy0_led0: led@0 {
> +							reg = <0>;
> +							status = "disabled";
> +						};
> +
> +						gsw_phy0_led1: led@1 {
> +							reg = <1>;
> +							status = "disabled";
> +						};
> +					};
> +				};
> +
> +				gsw_phy1: ethernet-phy@1 {
> +					compatible = "ethernet-phy-ieee802.3-c22";
> +					reg = <1>;
> +					interrupts = <1>;
> +					phy-mode = "internal";
> +					nvmem-cells = <&phy_calibration_p1>;
> +					nvmem-cell-names = "phy-cal-data";
> +
> +					leds {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +
> +						gsw_phy1_led0: led@0 {
> +							reg = <0>;
> +							status = "disabled";
> +						};
> +
> +						gsw_phy1_led1: led@1 {
> +							reg = <1>;
> +							status = "disabled";
> +						};
> +					};
> +				};
> +
> +				gsw_phy2: ethernet-phy@2 {
> +					compatible = "ethernet-phy-ieee802.3-c22";
> +					reg = <2>;
> +					interrupts = <2>;
> +					phy-mode = "internal";
> +					nvmem-cells = <&phy_calibration_p2>;
> +					nvmem-cell-names = "phy-cal-data";
> +
> +					leds {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +
> +						gsw_phy2_led0: led@0 {
> +							reg = <0>;
> +							status = "disabled";
> +						};
> +
> +						gsw_phy2_led1: led@1 {
> +							reg = <1>;
> +							status = "disabled";
> +						};
> +					};
> +				};
> +
> +				gsw_phy3: ethernet-phy@3 {
> +					compatible = "ethernet-phy-ieee802.3-c22";
> +					reg = <3>;
> +					interrupts = <3>;
> +					phy-mode = "internal";
> +					nvmem-cells = <&phy_calibration_p3>;
> +					nvmem-cell-names = "phy-cal-data";
> +
> +					leds {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +
> +						gsw_phy3_led0: led@0 {
> +							reg = <0>;
> +							status = "disabled";
> +						};
> +
> +						gsw_phy3_led1: led@1 {
> +							reg = <1>;
> +							status = "disabled";
> +						};
> +					};
> +				};
> +			};
> +		};
> +
>   		ethwarp: clock-controller@15031000 {
>   			compatible = "mediatek,mt7988-ethwarp";
>   			reg = <0 0x15031000 0 0x1000>;

Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add basic ethernet related nodes.
> 
> Mac1+2 needs pcs (sgmii+usxgmii) to work correctly which will be linked
> later when driver is merged.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 124 +++++++++++++++++++++-
>   1 file changed, 121 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> index 560ec86dbec0..ee1e01d720fe 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> @@ -680,7 +680,28 @@ xphyu3port0: usb-phy@11e13000 {
>   			};
>   		};
>   
> -		clock-controller@11f40000 {
> +		xfi_tphy0: phy@11f20000 {
> +			compatible = "mediatek,mt7988-xfi-tphy";
> +			reg = <0 0x11f20000 0 0x10000>;
> +			resets = <&watchdog 14>;
> +			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
> +				 <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>;
> +			clock-names = "xfipll", "topxtal";

resets here please, not after reg.

> +			mediatek,usxgmii-performance-errata;
> +			#phy-cells = <0>;
> +		};
> +
> +		xfi_tphy1: phy@11f30000 {
> +			compatible = "mediatek,mt7988-xfi-tphy";
> +			reg = <0 0x11f30000 0 0x10000>;
> +			resets = <&watchdog 15>;
> +			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
> +				 <&topckgen CLK_TOP_XFI_PHY_1_XTAL_SEL>;
> +			clock-names = "xfipll", "topxtal";

ditto

> +			#phy-cells = <0>;
> +		};
> +
> +		xfi_pll: clock-controller@11f40000 {
>   			compatible = "mediatek,mt7988-xfi-pll";
>   			reg = <0 0x11f40000 0 0x1000>;
>   			resets = <&watchdog 16>;
> @@ -714,19 +735,116 @@ phy_calibration_p3: calib@97c {
>   			};
>   		};
>   
> -		clock-controller@15000000 {
> +		ethsys: clock-controller@15000000 {
>   			compatible = "mediatek,mt7988-ethsys", "syscon";
>   			reg = <0 0x15000000 0 0x1000>;
>   			#clock-cells = <1>;
>   			#reset-cells = <1>;
>   		};
>   
> -		clock-controller@15031000 {
> +		ethwarp: clock-controller@15031000 {
>   			compatible = "mediatek,mt7988-ethwarp";
>   			reg = <0 0x15031000 0 0x1000>;
>   			#clock-cells = <1>;
>   			#reset-cells = <1>;
>   		};
> +
> +		eth: ethernet@15100000 {
> +			compatible = "mediatek,mt7988-eth";
> +			reg = <0 0x15100000 0 0x80000>,
> +			      <0 0x15400000 0 0x200000>;

reg = <0 0x15100000 0 0x80000>, <0 0x15400000 0 0x200000>;

it's 83 columns - it's fine.

> +			interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ethsys CLK_ETHDMA_CRYPT0_EN>,
> +				 <&ethsys CLK_ETHDMA_FE_EN>,
> +				 <&ethsys CLK_ETHDMA_GP2_EN>,
> +				 <&ethsys CLK_ETHDMA_GP1_EN>,
> +				 <&ethsys CLK_ETHDMA_GP3_EN>,
> +				 <&ethwarp CLK_ETHWARP_WOCPU2_EN>,
> +				 <&ethwarp CLK_ETHWARP_WOCPU1_EN>,
> +				 <&ethwarp CLK_ETHWARP_WOCPU0_EN>,
> +				 <&ethsys CLK_ETHDMA_ESW_EN>,
> +				 <&topckgen CLK_TOP_ETH_GMII_SEL>,
> +				 <&topckgen CLK_TOP_ETH_REFCK_50M_SEL>,
> +				 <&topckgen CLK_TOP_ETH_SYS_200M_SEL>,
> +				 <&topckgen CLK_TOP_ETH_SYS_SEL>,
> +				 <&topckgen CLK_TOP_ETH_XGMII_SEL>,
> +				 <&topckgen CLK_TOP_ETH_MII_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_500M_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_PAO_2X_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_SYNC_250M_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_PPEFB_250M_SEL>,
> +				 <&topckgen CLK_TOP_NETSYS_WARP_SEL>,
> +				 <&ethsys CLK_ETHDMA_XGP1_EN>,
> +				 <&ethsys CLK_ETHDMA_XGP2_EN>,
> +				 <&ethsys CLK_ETHDMA_XGP3_EN>;
> +			clock-names = "crypto", "fe", "gp2", "gp1",
> +				      "gp3",

clock-names = "crypto", "fe", "gp2", "gp1", "gp3",

:-)

> +				      "ethwarp_wocpu2", "ethwarp_wocpu1",
> +				      "ethwarp_wocpu0", "esw", "top_eth_gmii_sel",
> +				      "top_eth_refck_50m_sel", "top_eth_sys_200m_sel",
> +				      "top_eth_sys_sel", "top_eth_xgmii_sel",
> +				      "top_eth_mii_sel", "top_netsys_sel",
> +				      "top_netsys_500m_sel", "top_netsys_pao_2x_sel",
> +				      "top_netsys_sync_250m_sel",
> +				      "top_netsys_ppefb_250m_sel",
> +				      "top_netsys_warp_sel","xgp1", "xgp2", "xgp3";
> +			assigned-clocks = <&topckgen CLK_TOP_NETSYS_2X_SEL>,
> +					  <&topckgen CLK_TOP_NETSYS_GSW_SEL>,
> +					  <&topckgen CLK_TOP_USXGMII_SBUS_0_SEL>,
> +					  <&topckgen CLK_TOP_USXGMII_SBUS_1_SEL>,
> +					  <&topckgen CLK_TOP_SGM_0_SEL>,
> +					  <&topckgen CLK_TOP_SGM_1_SEL>;
> +			assigned-clock-parents = <&apmixedsys CLK_APMIXED_NET2PLL>,
> +						 <&topckgen CLK_TOP_NET1PLL_D4>,
> +						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
> +						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
> +						 <&apmixedsys CLK_APMIXED_SGMPLL>,
> +						 <&apmixedsys CLK_APMIXED_SGMPLL>;

Address and size cells must go *before* vendor-specific properties

> +			mediatek,ethsys = <&ethsys>;
> +			mediatek,infracfg = <&topmisc>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +

Cheers!
Angelo

Re: [PATCH v3 05/13] arm64: dts: mediatek: mt7988: add cci node

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add cci devicetree node for cpu frequency scaling.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 04/13] dt-bindings: interconnect: add mt7988-cci compatible

[AngeloGioacchino Del Regno]

Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add compatible for Mediatek MT7988 SoC with mediatek,mt8183-cci fallback
> which is taken by driver.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Aw: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

[Frank Wunderlich]

Hi Angelo,

thanks for review.

> Gesendet: Mittwoch, 11. Juni 2025 um 11:33
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Betreff: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes
>
> Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Add basic ethernet related nodes.
> > 
> > Mac1+2 needs pcs (sgmii+usxgmii) to work correctly which will be linked
> > later when driver is merged.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt7988a.dtsi | 124 +++++++++++++++++++++-
> >   1 file changed, 121 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> > index 560ec86dbec0..ee1e01d720fe 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt7988a.dtsi
> > @@ -680,7 +680,28 @@ xphyu3port0: usb-phy@11e13000 {
> >   			};
> >   		};
> >   
> > -		clock-controller@11f40000 {
> > +		xfi_tphy0: phy@11f20000 {
> > +			compatible = "mediatek,mt7988-xfi-tphy";
> > +			reg = <0 0x11f20000 0 0x10000>;
> > +			resets = <&watchdog 14>;
> > +			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
> > +				 <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>;
> > +			clock-names = "xfipll", "topxtal";
> 
> resets here please, not after reg.

OK

> > +			mediatek,usxgmii-performance-errata;
> > +			#phy-cells = <0>;

maybe #phy-cells above vendor-specific property like below?

> > +		};
> > +
> > +		xfi_tphy1: phy@11f30000 {
> > +			compatible = "mediatek,mt7988-xfi-tphy";
> > +			reg = <0 0x11f30000 0 0x10000>;
> > +			resets = <&watchdog 15>;
> > +			clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
> > +				 <&topckgen CLK_TOP_XFI_PHY_1_XTAL_SEL>;
> > +			clock-names = "xfipll", "topxtal";
> 
> ditto

ok, #phy-cells last?

> > +			#phy-cells = <0>;
> > +		};
> > +
> > +		xfi_pll: clock-controller@11f40000 {
> >   			compatible = "mediatek,mt7988-xfi-pll";
> >   			reg = <0 0x11f40000 0 0x1000>;
> >   			resets = <&watchdog 16>;
> > @@ -714,19 +735,116 @@ phy_calibration_p3: calib@97c {
> >   			};
> >   		};
> >   
> > -		clock-controller@15000000 {
> > +		ethsys: clock-controller@15000000 {
> >   			compatible = "mediatek,mt7988-ethsys", "syscon";
> >   			reg = <0 0x15000000 0 0x1000>;
> >   			#clock-cells = <1>;
> >   			#reset-cells = <1>;
> >   		};
> >   
> > -		clock-controller@15031000 {
> > +		ethwarp: clock-controller@15031000 {
> >   			compatible = "mediatek,mt7988-ethwarp";
> >   			reg = <0 0x15031000 0 0x1000>;
> >   			#clock-cells = <1>;
> >   			#reset-cells = <1>;
> >   		};
> > +
> > +		eth: ethernet@15100000 {
> > +			compatible = "mediatek,mt7988-eth";
> > +			reg = <0 0x15100000 0 0x80000>,
> > +			      <0 0x15400000 0 0x200000>;
> 
> reg = <0 0x15100000 0 0x80000>, <0 0x15400000 0 0x200000>;
> 
> it's 83 columns - it's fine.

ok, thought 80 columns is the limit...

> > +			interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&ethsys CLK_ETHDMA_CRYPT0_EN>,
> > +				 <&ethsys CLK_ETHDMA_FE_EN>,
> > +				 <&ethsys CLK_ETHDMA_GP2_EN>,
> > +				 <&ethsys CLK_ETHDMA_GP1_EN>,
> > +				 <&ethsys CLK_ETHDMA_GP3_EN>,
> > +				 <&ethwarp CLK_ETHWARP_WOCPU2_EN>,
> > +				 <&ethwarp CLK_ETHWARP_WOCPU1_EN>,
> > +				 <&ethwarp CLK_ETHWARP_WOCPU0_EN>,
> > +				 <&ethsys CLK_ETHDMA_ESW_EN>,
> > +				 <&topckgen CLK_TOP_ETH_GMII_SEL>,
> > +				 <&topckgen CLK_TOP_ETH_REFCK_50M_SEL>,
> > +				 <&topckgen CLK_TOP_ETH_SYS_200M_SEL>,
> > +				 <&topckgen CLK_TOP_ETH_SYS_SEL>,
> > +				 <&topckgen CLK_TOP_ETH_XGMII_SEL>,
> > +				 <&topckgen CLK_TOP_ETH_MII_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_500M_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_PAO_2X_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_SYNC_250M_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_PPEFB_250M_SEL>,
> > +				 <&topckgen CLK_TOP_NETSYS_WARP_SEL>,
> > +				 <&ethsys CLK_ETHDMA_XGP1_EN>,
> > +				 <&ethsys CLK_ETHDMA_XGP2_EN>,
> > +				 <&ethsys CLK_ETHDMA_XGP3_EN>;
> > +			clock-names = "crypto", "fe", "gp2", "gp1",
> > +				      "gp3",
> 
> clock-names = "crypto", "fe", "gp2", "gp1", "gp3",
> 
> :-)

do not know how this was happen...of course i change this ;)

> > +				      "ethwarp_wocpu2", "ethwarp_wocpu1",
> > +				      "ethwarp_wocpu0", "esw", "top_eth_gmii_sel",
> > +				      "top_eth_refck_50m_sel", "top_eth_sys_200m_sel",
> > +				      "top_eth_sys_sel", "top_eth_xgmii_sel",
> > +				      "top_eth_mii_sel", "top_netsys_sel",
> > +				      "top_netsys_500m_sel", "top_netsys_pao_2x_sel",
> > +				      "top_netsys_sync_250m_sel",
> > +				      "top_netsys_ppefb_250m_sel",
> > +				      "top_netsys_warp_sel","xgp1", "xgp2", "xgp3";
> > +			assigned-clocks = <&topckgen CLK_TOP_NETSYS_2X_SEL>,
> > +					  <&topckgen CLK_TOP_NETSYS_GSW_SEL>,
> > +					  <&topckgen CLK_TOP_USXGMII_SBUS_0_SEL>,
> > +					  <&topckgen CLK_TOP_USXGMII_SBUS_1_SEL>,
> > +					  <&topckgen CLK_TOP_SGM_0_SEL>,
> > +					  <&topckgen CLK_TOP_SGM_1_SEL>;
> > +			assigned-clock-parents = <&apmixedsys CLK_APMIXED_NET2PLL>,
> > +						 <&topckgen CLK_TOP_NET1PLL_D4>,
> > +						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
> > +						 <&topckgen CLK_TOP_NET1PLL_D8_D4>,
> > +						 <&apmixedsys CLK_APMIXED_SGMPLL>,
> > +						 <&apmixedsys CLK_APMIXED_SGMPLL>;
> 
> Address and size cells must go *before* vendor-specific properties

OK, same for #phy-cells above?

> > +			mediatek,ethsys = <&ethsys>;
> > +			mediatek,infracfg = <&topmisc>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> 
> Cheers!
> Angelo
> 

Aw: Re: [PATCH v3 10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds

[Frank Wunderlich]

Hi
> Gesendet: Mittwoch, 11. Juni 2025 um 11:33
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Betreff: Re: [PATCH v3 10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds
>
> Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Bananapi R4 has a green and a blue led which can be switched by gpio.
> > Green led is for running state so default on.
> > 
> > Green led also shares pin with eeprom writeprotect where led off allows
> > writing to eeprom.
> 
> This is extremely ugly from a hardware perspective and I'm sad to see such
> a thing here.

i know, but cannot change this...found this out while testing the eeprom...

where uboot allows the writing of eeprom currently without having this pin configured, linux does not and require green-led switch off.

> Though, I don't see any other solution, so I can only give you an advice:
> don't ever write to the eeprom from the kernel. Things will eventually go
> south otherwise.
> 
> I would also advise to add a read-only property to the eeprom node.

the eeprom is empty from factory, but can be used to store e.g. mac-adress which are random by default.

i tested a bit with tlv_eeprom command in uboot and some additional patches to archive this.

imho green-led as running indicator on (=write-protect of eeprom) is a good solution
=> readonly by default, but allowing to write it if user is really sure :)

> Regardless (and sadly),
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Thanks for your opinion and RB

regards Frank

Aw: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac

[Frank Wunderlich]

Hi

> Gesendet: Mittwoch, 11. Juni 2025 um 11:33
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Betreff: Re: [PATCH v3 12/13] arm64: dts: mediatek: mt7988a-bpi-r4: add sfp cages and link to gmac
>
> Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Add SFP cages to Bananapi-R4 board. The 2.5g phy variant only contains the
> > wan-SFP, so add this to common dtsi and the lan-sfp only to the dual-SFP
> > variant.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> > v3:
> > - enable mac with 2.5g phy on r4 phy variant because driver is now mainline
> > ---
> >   .../mediatek/mt7988a-bananapi-bpi-r4-2g5.dts   | 12 ++++++++++++
> >   .../dts/mediatek/mt7988a-bananapi-bpi-r4.dts   | 18 ++++++++++++++++++
> >   .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi  | 18 ++++++++++++++++++
> >   3 files changed, 48 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> > index 53de9c113f60..e63e17ae35a0 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-2g5.dts
> > @@ -9,3 +9,15 @@ / {
> >   	model = "Banana Pi BPI-R4 (1x SFP+, 1x 2.5GbE)";
> >   	chassis-type = "embedded";
> >   };
> > +
> > +&gmac1 {
> 
> phy = ...
> phy-c..onnection-type
> phy-m...ode

phy-connection-type is dropped in next version due to comment from andrew, but i order alphabetically

> > +	phy-mode = "internal";
> > +	phy-connection-type = "internal";
> > +	phy = <&int_2p5g_phy>;
> > +	status = "okay";
> > +};
> > +
> > +&int_2p5g_phy {
> > +	pinctrl-names = "i2p5gbe-led";
> > +	pinctrl-0 = <&i2p5gbe_led0_pins>;
> 
> pinctrl-names
> pinctrl-0

what the difference? i don't see it :(

the others i change in v4

regards Frank