[PATCH net-next v3 0/9] net/stmmac: Add Mediatek MT8189 support

[PATCH net-next v3 0/9] net/stmmac: Add Mediatek MT8189 support

[Louis-Alexis Eyraud]

This series adds the Ethernet support for Mediatek MT8189 SoC and its
variants (MT8371, MT8391). These SoC integrate a Gigabit Ethernet MAC
with RGMII/RMII/MII interface, with a similar design than previous SoCs
such MT8188 or MT8195.

This series is based on net-next tree (sha1: 6b3e73774815).
It has been tested on Mediatek Genio 520-EVK (MT8371) and 720-EVK
(MT8391) boards, which both integrate an Airoha AN8801R Ethernet
PHY, with hardware enablement series ([1]).
It has been also tested on Mediatek Genio 510-EVK (MT8370, variant of
MT8188) and 1200-EVK (MT8395, variant of MT8195).

[1]: https://lore.kernel.org/linux-mediatek/20260924-add-mediatek-genio-520-720-evk-v3-0-2ec5c80677d4@collabora.com/

---
Changes in v3:
- Rebased on net-next (sha1: 6b3e73774815)
- Added Reviewed-By trailers in patch 3 and 5
- Added patch 1 to fix unit in rx/tx delay property description
  (Sashiko)
- Patch 2:
  - Reworded patch description to add clock list details
  - Removed the specific MT8189 clocks list so the axi and apb clocks
    are also described for this compatible too and reworked the clock
    list and number check
  - Added mediatek,rmii-clk-from-mac as not allowed for MT8189 (Sashiko)
- Added patch 4 to simplify TX/RX delay handling in mt2712_set_delay,
  as suggested by M. Chevallier and modified following patches
  accordingly
- Fixed in patch 7 tx_rgmii_delay_stage_div null case and register value
  in mt8195_fix_mac_speed (Sashiko)
- Added in patch 9 the axi and apb clocks to MT8189 clock lists 
- Link to v2: https://lore.kernel.org/r/20260924-dwmac-mediatek-mt8189-v2-0-430bd74d5ef9@collabora.com

Changes in v2:
- Rebased on net-next (sha1: 528de6832b21)
- Dropped "net: stmmac: mediatek: rename MT2712 and MT8195 variant
  methods" and "net: stmmac: mediatek: add support for TX deallocation
   adjustment feature" patches, present in v1 (M. Chevallier)
- Patch 1:
  - Removed rmii_internal clock from required clock list (A. Lunn)
  - Added MT8189 stage delay divider values in "mediatek,rx-delay-ps" and
    "mediatek,rx-delay-ps" property description (Sashiko)
  - Updated commit description
- Added patch 2 to simplify TX/RX delay handling in mt8195_set_delay
- Added patch 3 to add RX/TX delay stage divider in platform data
- Patch 4:
  - Replace MT8195_PERI_ETH_CTRL_BASE by MT8195_PERI_ETH_CTRL_OFFSET to
    harmonize the variable and definition naming (A. Lunn)
  - Added Reviewed-By tag
- Added patch 5 to add the TX Clock phase shift use in
  RGMII and in 1G speed
- Patch 6:
  - Reworded flag and define names to explicit a bit more what the
    feature does
  - Updated commit description
- Patch 7:
  - Added the new definitions and needed fields in platform data
  - Updated commit description to add the info about MT8189 RX/TX delay
    stage divider and maximum values, and remove reference about 
    stage fine feature 
- Link to v1: https://lore.kernel.org/r/20260707-dwmac-mediatek-mt8189-v1-0-17f345eaaca3@collabora.com

---
Louis-Alexis Eyraud (9):
      dt-bindings: net: mediatek-dwmac: fix unit in rx/tx delay property description
      dt-bindings: net: mediatek-dwmac: add support for MT8189 SoC
      net: stmmac: mediatek: simplify TX/RX delay handling in mt8195_set_delay
      net: stmmac: mediatek: simplify TX/RX delay handling in mt2712_set_delay
      net: stmmac: mediatek: add RX/TX delay stage divider in platform data
      net: stmmac: mediatek: add PERI_ETH_CTRLx register offset in platform data
      net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed
      net: stmmac: mediatek: add support for TX clock output enable feature
      net: stmmac: mediatek: add support for MT8189 SoC

 .../devicetree/bindings/net/mediatek-dwmac.yaml    |  58 +++-
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 363 ++++++++++++++-------
 2 files changed, 287 insertions(+), 134 deletions(-)
---
base-commit: 6b3e73774815910a39d8ca75b9ad7df963f61d50
change-id: 20260703-dwmac-mediatek-mt8189-9dafe3984951

Best regards,
-- 
Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

[PATCH net-next v3 1/9] dt-bindings: net: mediatek-dwmac: fix unit in rx/tx delay property description

[Louis-Alexis Eyraud]

The "mediatek,rx-delay-ps" and "mediatek,tx-delay-ps" descriptions say
their values are in nanoseconds but their names with the "ps" suffix
suggest it is picoseconds.
They are in fact used in both driver and devicetree as picosecond.
So, modify the two properties unit description to fix their unit.

Fixes: 150b6adda6b1 ("net: dt-bindings: dwmac: Convert mediatek-dwmac to DT schema")
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 Documentation/devicetree/bindings/net/mediatek-dwmac.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
index 3aab21b8e8de..14f9683f20fe 100644
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -81,7 +81,7 @@ properties:
 
   mediatek,tx-delay-ps:
     description:
-      The internal TX clock delay (provided by this driver) in nanoseconds.
+      The internal TX clock delay (provided by this driver) in picoseconds.
       For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
@@ -91,7 +91,7 @@ properties:
 
   mediatek,rx-delay-ps:
     description:
-      The internal RX clock delay (provided by this driver) in nanoseconds.
+      The internal RX clock delay (provided by this driver) in picoseconds.
       For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,

-- 
2.56.0

Re: [PATCH net-next v3 1/9] dt-bindings: net: mediatek-dwmac: fix unit in rx/tx delay property description

[Andrew Lunn]

On Fri, Oct 16, 2026 at 12:31:05PM +0200, Louis-Alexis Eyraud wrote:
> The "mediatek,rx-delay-ps" and "mediatek,tx-delay-ps" descriptions say
> their values are in nanoseconds but their names with the "ps" suffix
> suggest it is picoseconds.
> They are in fact used in both driver and devicetree as picosecond.
> So, modify the two properties unit description to fix their unit.
> 
> Fixes: 150b6adda6b1 ("net: dt-bindings: dwmac: Convert mediatek-dwmac to DT schema")
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

Maybe this patch should be for net, not net-next. Documentation fixes
often are. Lets see what the Device Tree Maintainers say.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v3 1/9] dt-bindings: net: mediatek-dwmac: fix unit in rx/tx delay property description

[Krzysztof Kozlowski]

On Fri, Oct 16, 2026 at 05:33:42PM +0200, Andrew Lunn wrote:
> On Fri, Oct 16, 2026 at 12:31:05PM +0200, Louis-Alexis Eyraud wrote:
> > The "mediatek,rx-delay-ps" and "mediatek,tx-delay-ps" descriptions say
> > their values are in nanoseconds but their names with the "ps" suffix
> > suggest it is picoseconds.
> > They are in fact used in both driver and devicetree as picosecond.
> > So, modify the two properties unit description to fix their unit.
> > 
> > Fixes: 150b6adda6b1 ("net: dt-bindings: dwmac: Convert mediatek-dwmac to DT schema")
> > Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> 
> Maybe this patch should be for net, not net-next. Documentation fixes
> often are. Lets see what the Device Tree Maintainers say.

It has no real impact, because the schema is what matters, not open
text.

Anyway, regardless:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH net-next v3 2/9] dt-bindings: net: mediatek-dwmac: add support for MT8189 SoC

[Louis-Alexis Eyraud]

Add new compatible string and clock definitions for the Ethernet MAC
IP found in MT8189 SoC.

Regarding the required clocks, the MT8189 datasheets list in the
paragraph related to this IP clocks the mac main and ptp reference
clocks as the ones to turn on.

No specific peripheral clock for Ethernet MAC is described in the SoC
clock list.  But at the same time, the Ethernet MAC block diagram,
present in the datasheets, shows this IP is interconnected to the rest
of SoC with AXI and APB interfaces (and in MT8188 datasheet too).
There is no APB related clock, that is specific to the IP in MT8189,
contrary to MT8188 or MT8195 that have one. But since it is on an APB
bus (likely shared between multiple IPs), then there is a bus clock for
peripheral access. Describing the IP having it makes sense, same thing
for AXI too, so the clock lists for this compatible are reflecting
that.

Also, don't allow mediatek,rmii-clk-from-mac property for MT8189
compatible.

Finally, update "mediatek,rx-delay-ps" and "mediatek,tx-delay-ps" property
description to add MT8189 RX/TX delay stage divider values, as they
differ from existing supported SoC.

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../devicetree/bindings/net/mediatek-dwmac.yaml    | 54 ++++++++++++++++++----
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
index 14f9683f20fe..6acfe104ef09 100644
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -20,13 +20,11 @@ select:
         enum:
           - mediatek,mt2712-gmac
           - mediatek,mt8188-gmac
+          - mediatek,mt8189-gmac
           - mediatek,mt8195-gmac
   required:
     - compatible
 
-allOf:
-  - $ref: snps,dwmac.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -36,6 +34,7 @@ properties:
           - const: snps,dwmac-4.20a
       - items:
           - enum:
+              - mediatek,mt8189-gmac
               - mediatek,mt8195-gmac
           - const: snps,dwmac-5.10a
       - items:
@@ -45,7 +44,7 @@ properties:
           - const: snps,dwmac-5.10a
 
   clocks:
-    minItems: 5
+    minItems: 4
     items:
       - description: AXI clock
       - description: APB clock
@@ -55,7 +54,7 @@ properties:
       - description: MAC clock gate
 
   clock-names:
-    minItems: 5
+    minItems: 4
     items:
       - const: axi
       - const: apb
@@ -86,8 +85,10 @@ properties:
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
       or will round down. Range 0~31*550.
-      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple of 290,
-      or will round down. Range 0~31*290.
+      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a
+      multiple of 290, or will round down. Range 0~31*290.
+      For MT8189 RGMII/RMII/MII interface, Allowed value need to
+      be a multiple of 180, or will round down. Range 0~31*180.
 
   mediatek,rx-delay-ps:
     description:
@@ -96,8 +97,10 @@ properties:
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
       or will round down. Range 0~31*550.
-      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple
-      of 290, or will round down. Range 0~31*290.
+      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a
+      multiple of 290, or will round down. Range 0~31*290.
+      For MT8189 RGMII/RMII/MII interface, Allowed value need to
+      be a multiple of 180, or will round down. Range 0~31*180.
 
   mediatek,rmii-rxc:
     type: boolean
@@ -147,6 +150,39 @@ required:
   - phy-mode
   - mediatek,pericfg
 
+allOf:
+  - $ref: snps,dwmac.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt2712-gmac
+              - mediatek,mt8188-gmac
+              - mediatek,mt8195-gmac
+    then:
+      properties:
+        clocks:
+          minItems: 5
+
+        clock-names:
+          minItems: 5
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8189-gmac
+    then:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          maxItems: 4
+
+        mediatek,rmii-clk-from-mac: false
+
 unevaluatedProperties: false
 
 examples:

-- 
2.56.0

Re: [PATCH net-next v3 2/9] dt-bindings: net: mediatek-dwmac: add support for MT8189 SoC

[Andrew Lunn]

> Finally, update "mediatek,rx-delay-ps" and "mediatek,tx-delay-ps" property
> description to add MT8189 RX/TX delay stage divider values, as they
> differ from existing supported SoC.

These should be mark deprecated and the standard rx-internal-delay-ps
and tx-internal-delay-ps used in there place for new devices.

> @@ -86,8 +85,10 @@ properties:
>        or will round down. Range 0~31*170.
>        For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
>        or will round down. Range 0~31*550.
> -      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple of 290,
> -      or will round down. Range 0~31*290.
> +      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a
> +      multiple of 290, or will round down. Range 0~31*290.
> +      For MT8189 RGMII/RMII/MII interface, Allowed value need to
> +      be a multiple of 180, or will round down. Range 0~31*180.

Please make this:

For MT8189 RGMII/RMII/MII interface, this property is not used, the
standard tx-internal-delay-ps should be used instead.

> @@ -96,8 +97,10 @@ properties:
>        or will round down. Range 0~31*170.
>        For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
>        or will round down. Range 0~31*550.
> -      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple
> -      of 290, or will round down. Range 0~31*290.
> +      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a
> +      multiple of 290, or will round down. Range 0~31*290.
> +      For MT8189 RGMII/RMII/MII interface, Allowed value need to
> +      be a multiple of 180, or will round down. Range 0~31*180.

Same here.

     Andrew

[PATCH net-next v3 3/9] net: stmmac: mediatek: simplify TX/RX delay handling in mt8195_set_delay

[Louis-Alexis Eyraud]

The mt8195_set_delay function modifies at its beginning the TX and RX
internal delay variables, located in the driver data, by dividing
them by a constant (290) and restores their original values by
multiplying them again at the function end. It is done in order to
convert them into a step value, used by the hardware registers for
setting these delays.

But this is rather pointless to modify the driver data for that, while
it could be done locally in the function. The original delay values
cannot be used anymore (if needed) during mt8195_set_delay processing.
Finally, they are altered after the function call if they are not a
multiple of 290.

So, simplify these delay variable handling by using local variables to
convert them into the register value and use those in the write calls.
Also, remove the two private conversion functions, that are not useful
anymore and add definitions for MT8195 RX/TX delay maximum and divider
values.

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 77 ++++++++++------------
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 30ae0dba7fff..f7eb85110df0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -63,6 +63,11 @@
 #define MT8195_DLY_RMII_TXC_ENABLE	BIT(5)
 #define MT8195_DLY_RMII_TXC_STAGES	GENMASK(4, 0)
 
+#define MT8195_DLY_RXC_STAGE_DIV	290 /* 290ps per stage */
+#define MT8195_DLY_RXC_MAX		9280 /* 32 x 290ps */
+#define MT8195_DLY_TXC_STAGE_DIV	290 /* 290ps per stage */
+#define MT8195_DLY_TXC_MAX		9280 /* 32 x 290ps */
+
 struct mac_delay_struct {
 	u32 tx_delay;
 	u32 rx_delay;
@@ -293,39 +298,27 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
 	return 0;
 }
 
-static void mt8195_delay_ps2stage(struct mediatek_dwmac_plat_data *plat)
-{
-	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-
-	/* 290ps per stage */
-	mac_delay->tx_delay /= 290;
-	mac_delay->rx_delay /= 290;
-}
-
-static void mt8195_delay_stage2ps(struct mediatek_dwmac_plat_data *plat)
-{
-	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-
-	/* 290ps per stage */
-	mac_delay->tx_delay *= 290;
-	mac_delay->rx_delay *= 290;
-}
-
 static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 {
 	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-	u32 gtxc_delay_val = 0, delay_val = 0, rmii_delay_val = 0;
-
-	mt8195_delay_ps2stage(plat);
+	u32 rx_delay_stage_val = mac_delay->rx_delay / MT8195_DLY_RXC_STAGE_DIV;
+	u32 tx_delay_stage_val = mac_delay->tx_delay / MT8195_DLY_TXC_STAGE_DIV;
+	u32 gtxc_delay_val = 0;
+	u32 rmii_delay_val = 0;
+	u32 delay_val = 0;
 
 	switch (plat->phy_mode) {
 	case PHY_INTERFACE_MODE_MII:
-		delay_val |= FIELD_PREP(MT8195_DLY_TXC_ENABLE, !!mac_delay->tx_delay);
-		delay_val |= FIELD_PREP(MT8195_DLY_TXC_STAGES, mac_delay->tx_delay);
+		delay_val |= FIELD_PREP(MT8195_DLY_TXC_ENABLE,
+					!!tx_delay_stage_val);
+		delay_val |= FIELD_PREP(MT8195_DLY_TXC_STAGES,
+					tx_delay_stage_val);
 		delay_val |= FIELD_PREP(MT8195_DLY_TXC_INV, mac_delay->tx_inv);
 
-		delay_val |= FIELD_PREP(MT8195_DLY_RXC_ENABLE, !!mac_delay->rx_delay);
-		delay_val |= FIELD_PREP(MT8195_DLY_RXC_STAGES, mac_delay->rx_delay);
+		delay_val |= FIELD_PREP(MT8195_DLY_RXC_ENABLE,
+					!!rx_delay_stage_val);
+		delay_val |= FIELD_PREP(MT8195_DLY_RXC_STAGES,
+					rx_delay_stage_val);
 		delay_val |= FIELD_PREP(MT8195_DLY_RXC_INV, mac_delay->rx_inv);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
@@ -336,16 +329,16 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 			 * The ingress timing can be adjusted by RMII_RXC delay macro circuit.
 			 */
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_TXC_ENABLE,
-						     !!mac_delay->tx_delay);
+						     !!tx_delay_stage_val);
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_TXC_STAGES,
-						     mac_delay->tx_delay);
+						     tx_delay_stage_val);
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_TXC_INV,
 						     mac_delay->tx_inv);
 
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_RXC_ENABLE,
-						     !!mac_delay->rx_delay);
+						     !!rx_delay_stage_val);
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_RXC_STAGES,
-						     mac_delay->rx_delay);
+						     rx_delay_stage_val);
 			rmii_delay_val |= FIELD_PREP(MT8195_DLY_RMII_RXC_INV,
 						     mac_delay->rx_inv);
 		} else {
@@ -361,9 +354,9 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 				 * by RXC delay macro circuit.
 				 */
 				delay_val |= FIELD_PREP(MT8195_DLY_RXC_ENABLE,
-							!!mac_delay->rx_delay);
+							!!rx_delay_stage_val);
 				delay_val |= FIELD_PREP(MT8195_DLY_RXC_STAGES,
-							mac_delay->rx_delay);
+							rx_delay_stage_val);
 				delay_val |= FIELD_PREP(MT8195_DLY_RXC_INV,
 							mac_delay->rx_inv);
 			} else {
@@ -372,9 +365,9 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 				 * by TXC delay macro circuit.
 				 */
 				delay_val |= FIELD_PREP(MT8195_DLY_TXC_ENABLE,
-							!!mac_delay->rx_delay);
+							!!rx_delay_stage_val);
 				delay_val |= FIELD_PREP(MT8195_DLY_TXC_STAGES,
-							mac_delay->rx_delay);
+							rx_delay_stage_val);
 				delay_val |= FIELD_PREP(MT8195_DLY_TXC_INV,
 							mac_delay->rx_inv);
 			}
@@ -384,12 +377,16 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_ID:
-		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_ENABLE, !!mac_delay->tx_delay);
-		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_STAGES, mac_delay->tx_delay);
+		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_ENABLE,
+					     !!tx_delay_stage_val);
+		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_STAGES,
+					     tx_delay_stage_val);
 		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_INV, mac_delay->tx_inv);
 
-		delay_val |= FIELD_PREP(MT8195_DLY_RXC_ENABLE, !!mac_delay->rx_delay);
-		delay_val |= FIELD_PREP(MT8195_DLY_RXC_STAGES, mac_delay->rx_delay);
+		delay_val |= FIELD_PREP(MT8195_DLY_RXC_ENABLE,
+					!!rx_delay_stage_val);
+		delay_val |= FIELD_PREP(MT8195_DLY_RXC_STAGES,
+					rx_delay_stage_val);
 		delay_val |= FIELD_PREP(MT8195_DLY_RXC_INV, mac_delay->rx_inv);
 
 		break;
@@ -408,8 +405,6 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 	regmap_write(plat->peri_regmap, MT8195_PERI_ETH_CTRL1, delay_val);
 	regmap_write(plat->peri_regmap, MT8195_PERI_ETH_CTRL2, rmii_delay_val);
 
-	mt8195_delay_stage2ps(plat);
-
 	return 0;
 }
 
@@ -418,8 +413,8 @@ static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
 	.dwmac_set_delay = mt8195_set_delay,
 	.clk_list = mt8195_dwmac_clk_l,
 	.num_clks = ARRAY_SIZE(mt8195_dwmac_clk_l),
-	.rx_delay_max = 9280,
-	.tx_delay_max = 9280,
+	.rx_delay_max = MT8195_DLY_RXC_MAX,
+	.tx_delay_max = MT8195_DLY_TXC_MAX,
 	.dma_bit_mask = 35,
 };
 

-- 
2.56.0

[PATCH net-next v3 4/9] net: stmmac: mediatek: simplify TX/RX delay handling in mt2712_set_delay

[Louis-Alexis Eyraud]

The mt2712_set_delay function modifies at its beginning the TX and RX
internal delay variables, located in the driver data, by dividing
them by a constant (550 for RGMII, 170 for MII/RMII) and restores their
original values by multiplying them again at the function end. It is
done in order to convert them into a step value, used by the hardware
registers for setting these delays.

But this is rather pointless to modify the driver data for that, while
it could be done locally in the function. The original delay values
cannot be used anymore (if needed) during mt2712_set_delay processing.
Finally, they are altered after the function call if they are not a
multiple of the delay stage divider.

So, simplify these delay variable handling by using local variables to
convert them into the register value and use those in the write calls.
Also, remove the two private conversion functions, that are not useful
anymore and add definitions for MT2712 RX/TX delay maximum and divider
values.

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 137 +++++++++------------
 1 file changed, 60 insertions(+), 77 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index f7eb85110df0..596411d4a5b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -36,6 +36,14 @@
 #define ETH_FINE_DLY_GTXC	BIT(1)
 #define ETH_FINE_DLY_RXC	BIT(0)
 
+#define MT2712_RMII_DLY_RXC_STAGE_DIV	170 /* 170ps per stage */
+#define MT2712_RMII_DLY_TXC_STAGE_DIV	170 /* 170ps per stage */
+
+#define MT2712_RGMII_DLY_RXC_STAGE_DIV	550 /* 550ps per stage */
+#define MT2712_RGMII_DLY_RXC_MAX	17600 /* 32 x 550ps */
+#define MT2712_RGMII_DLY_TXC_STAGE_DIV	550 /* 550ps per stage */
+#define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */
+
 /* Peri Configuration register for mt8195 */
 #define MT8195_PERI_ETH_CTRL0		0xFD0
 #define MT8195_RMII_CLK_SRC_INTERNAL	BIT(28)
@@ -129,71 +137,36 @@ static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat,
 	return 0;
 }
 
-static void mt2712_delay_ps2stage(struct mediatek_dwmac_plat_data *plat)
-{
-	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-
-	switch (plat->phy_mode) {
-	case PHY_INTERFACE_MODE_MII:
-	case PHY_INTERFACE_MODE_RMII:
-		/* 550ps per stage for MII/RMII */
-		mac_delay->tx_delay /= 550;
-		mac_delay->rx_delay /= 550;
-		break;
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-		/* 170ps per stage for RGMII */
-		mac_delay->tx_delay /= 170;
-		mac_delay->rx_delay /= 170;
-		break;
-	default:
-		dev_err(plat->dev, "phy interface not supported\n");
-		break;
-	}
-}
-
-static void mt2712_delay_stage2ps(struct mediatek_dwmac_plat_data *plat)
-{
-	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-
-	switch (plat->phy_mode) {
-	case PHY_INTERFACE_MODE_MII:
-	case PHY_INTERFACE_MODE_RMII:
-		/* 550ps per stage for MII/RMII */
-		mac_delay->tx_delay *= 550;
-		mac_delay->rx_delay *= 550;
-		break;
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-		/* 170ps per stage for RGMII */
-		mac_delay->tx_delay *= 170;
-		mac_delay->rx_delay *= 170;
-		break;
-	default:
-		dev_err(plat->dev, "phy interface not supported\n");
-		break;
-	}
-}
-
 static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 {
 	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-	u32 delay_val = 0, fine_val = 0;
+	u32 rx_delay_stage_val, tx_delay_stage_val;
+	u16 rx_delay_stage_div, tx_delay_stage_div;
+	u32 delay_val = 0;
+	u32 fine_val = 0;
 
-	mt2712_delay_ps2stage(plat);
+	if (phy_interface_mode_is_rgmii(plat->phy_mode)) {
+		rx_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV;
+		tx_delay_stage_div = MT2712_RGMII_DLY_TXC_STAGE_DIV;
+	} else {
+		rx_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV;
+		tx_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV;
+	}
+	rx_delay_stage_val = mac_delay->rx_delay / rx_delay_stage_div;
+	tx_delay_stage_val = mac_delay->tx_delay / tx_delay_stage_div;
 
 	switch (plat->phy_mode) {
 	case PHY_INTERFACE_MODE_MII:
-		delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE, !!mac_delay->tx_delay);
-		delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES, mac_delay->tx_delay);
+		delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE,
+					!!tx_delay_stage_val);
+		delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES,
+					tx_delay_stage_val);
 		delay_val |= FIELD_PREP(ETH_DLY_TXC_INV, mac_delay->tx_inv);
 
-		delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE, !!mac_delay->rx_delay);
-		delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES, mac_delay->rx_delay);
+		delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE,
+					!!rx_delay_stage_val);
+		delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES,
+					rx_delay_stage_val);
 		delay_val |= FIELD_PREP(ETH_DLY_RXC_INV, mac_delay->rx_inv);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
@@ -203,12 +176,16 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 			 * The egress timing can be adjusted by GTXC delay macro circuit.
 			 * The ingress timing can be adjusted by TXC delay macro circuit.
 			 */
-			delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE, !!mac_delay->rx_delay);
-			delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES, mac_delay->rx_delay);
+			delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE,
+						!!rx_delay_stage_val);
+			delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES,
+						rx_delay_stage_val);
 			delay_val |= FIELD_PREP(ETH_DLY_TXC_INV, mac_delay->rx_inv);
 
-			delay_val |= FIELD_PREP(ETH_DLY_GTXC_ENABLE, !!mac_delay->tx_delay);
-			delay_val |= FIELD_PREP(ETH_DLY_GTXC_STAGES, mac_delay->tx_delay);
+			delay_val |= FIELD_PREP(ETH_DLY_GTXC_ENABLE,
+						!!tx_delay_stage_val);
+			delay_val |= FIELD_PREP(ETH_DLY_GTXC_STAGES,
+						tx_delay_stage_val);
 			delay_val |= FIELD_PREP(ETH_DLY_GTXC_INV, mac_delay->tx_inv);
 		} else {
 			/* case 2: the rmii reference clock is from external phy,
@@ -222,16 +199,20 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 				 * to RXC pin, the reference clock will be adjusted
 				 * by RXC delay macro circuit.
 				 */
-				delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE, !!mac_delay->rx_delay);
-				delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES, mac_delay->rx_delay);
+				delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE,
+							!!rx_delay_stage_val);
+				delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES,
+							rx_delay_stage_val);
 				delay_val |= FIELD_PREP(ETH_DLY_RXC_INV, mac_delay->rx_inv);
 			} else {
 				/* the rmii reference clock from outside is connected
 				 * to TXC pin, the reference clock will be adjusted
 				 * by TXC delay macro circuit.
 				 */
-				delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE, !!mac_delay->rx_delay);
-				delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES, mac_delay->rx_delay);
+				delay_val |= FIELD_PREP(ETH_DLY_TXC_ENABLE,
+							!!rx_delay_stage_val);
+				delay_val |= FIELD_PREP(ETH_DLY_TXC_STAGES,
+							rx_delay_stage_val);
 				delay_val |= FIELD_PREP(ETH_DLY_TXC_INV, mac_delay->rx_inv);
 			}
 			/* tx_inv will inverse the tx clock inside mac relateive to
@@ -248,12 +229,16 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 		fine_val = ETH_FINE_DLY_GTXC | ETH_FINE_DLY_RXC;
 
-		delay_val |= FIELD_PREP(ETH_DLY_GTXC_ENABLE, !!mac_delay->tx_delay);
-		delay_val |= FIELD_PREP(ETH_DLY_GTXC_STAGES, mac_delay->tx_delay);
+		delay_val |= FIELD_PREP(ETH_DLY_GTXC_ENABLE,
+					!!tx_delay_stage_val);
+		delay_val |= FIELD_PREP(ETH_DLY_GTXC_STAGES,
+					tx_delay_stage_val);
 		delay_val |= FIELD_PREP(ETH_DLY_GTXC_INV, mac_delay->tx_inv);
 
-		delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE, !!mac_delay->rx_delay);
-		delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES, mac_delay->rx_delay);
+		delay_val |= FIELD_PREP(ETH_DLY_RXC_ENABLE,
+					!!rx_delay_stage_val);
+		delay_val |= FIELD_PREP(ETH_DLY_RXC_STAGES,
+					rx_delay_stage_val);
 		delay_val |= FIELD_PREP(ETH_DLY_RXC_INV, mac_delay->rx_inv);
 		break;
 	default:
@@ -263,19 +248,17 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 	regmap_write(plat->peri_regmap, PERI_ETH_DLY, delay_val);
 	regmap_write(plat->peri_regmap, PERI_ETH_DLY_FINE, fine_val);
 
-	mt2712_delay_stage2ps(plat);
-
 	return 0;
 }
 
 static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
-		.dwmac_set_phy_interface = mt2712_set_interface,
-		.dwmac_set_delay = mt2712_set_delay,
-		.clk_list = mt2712_dwmac_clk_l,
-		.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
-		.rx_delay_max = 17600,
-		.tx_delay_max = 17600,
-		.dma_bit_mask = 33,
+	.dwmac_set_phy_interface = mt2712_set_interface,
+	.dwmac_set_delay = mt2712_set_delay,
+	.clk_list = mt2712_dwmac_clk_l,
+	.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
+	.rx_delay_max = MT2712_RGMII_DLY_RXC_MAX,
+	.tx_delay_max = MT2712_RGMII_DLY_TXC_MAX,
+	.dma_bit_mask = 33,
 };
 
 static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,

-- 
2.56.0

Re: [PATCH net-next v3 4/9] net: stmmac: mediatek: simplify TX/RX delay handling in mt2712_set_delay

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c,
  mt2712_set_delay() now uses the wrong picosecond-per-stage divisor for…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  mt2712_gmac_variant uses a single rx_delay_max/tx_delay_max of 17600ps…

--

net: stmmac: mediatek: simplify TX/RX delay handling in mt2712_set_delay

This commit removes mt2712_delay_ps2stage() and mt2712_delay_stage2ps().
Those helpers converted the TX/RX delays in the driver data in place. The
stage values are now computed in local variables inside mt2712_set_delay(),
and new defines are added for the MT2712 delay stage dividers and maximum
values.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index f7eb85110df08..596411d4a5b2b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -36,6 +36,14 @@
>  #define ETH_FINE_DLY_GTXC	BIT(1)
>  #define ETH_FINE_DLY_RXC	BIT(0)
>  
> +#define MT2712_RMII_DLY_RXC_STAGE_DIV	170 /* 170ps per stage */
> +#define MT2712_RMII_DLY_TXC_STAGE_DIV	170 /* 170ps per stage */
> +
> +#define MT2712_RGMII_DLY_RXC_STAGE_DIV	550 /* 550ps per stage */
> +#define MT2712_RGMII_DLY_RXC_MAX	17600 /* 32 x 550ps */
> +#define MT2712_RGMII_DLY_TXC_STAGE_DIV	550 /* 550ps per stage */
> +#define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */
> +

[ ... ]

> @@ -129,71 +137,36 @@ static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat,
>  	return 0;
>  }
>  
> -static void mt2712_delay_ps2stage(struct mediatek_dwmac_plat_data *plat)
> -{
> -	struct mac_delay_struct *mac_delay = &plat->mac_delay;
> -
> -	switch (plat->phy_mode) {
> -	case PHY_INTERFACE_MODE_MII:
> -	case PHY_INTERFACE_MODE_RMII:
> -		/* 550ps per stage for MII/RMII */
> -		mac_delay->tx_delay /= 550;
> -		mac_delay->rx_delay /= 550;
> -		break;
> -	case PHY_INTERFACE_MODE_RGMII:
> -	case PHY_INTERFACE_MODE_RGMII_TXID:
> -	case PHY_INTERFACE_MODE_RGMII_RXID:
> -	case PHY_INTERFACE_MODE_RGMII_ID:
> -		/* 170ps per stage for RGMII */
> -		mac_delay->tx_delay /= 170;
> -		mac_delay->rx_delay /= 170;
> -		break;

[ ... ]

>  static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
>  {
>  	struct mac_delay_struct *mac_delay = &plat->mac_delay;
> -	u32 delay_val = 0, fine_val = 0;
> +	u32 rx_delay_stage_val, tx_delay_stage_val;
> +	u16 rx_delay_stage_div, tx_delay_stage_div;
> +	u32 delay_val = 0;
> +	u32 fine_val = 0;
>  
> -	mt2712_delay_ps2stage(plat);
> +	if (phy_interface_mode_is_rgmii(plat->phy_mode)) {
> +		rx_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV;
> +		tx_delay_stage_div = MT2712_RGMII_DLY_TXC_STAGE_DIV;
> +	} else {
> +		rx_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV;
> +		tx_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV;
> +	}
> +	rx_delay_stage_val = mac_delay->rx_delay / rx_delay_stage_div;
> +	tx_delay_stage_val = mac_delay->tx_delay / tx_delay_stage_div;

[Severity: High]
Are the RGMII and MII/RMII stage dividers swapped here?

The removed mt2712_delay_ps2stage() divided MII/RMII delays by 550 and
RGMII delays by 170. The binding in
Documentation/devicetree/bindings/net/mediatek-dwmac.yaml says the same
thing for mediatek,tx-delay-ps and mediatek,rx-delay-ps:

    For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
    or will round down. Range 0~31*170.
    For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
    or will round down. Range 0~31*550.

The new defines use 550 for RGMII and 170 for MII/RMII. The commit message
describes the same reversed mapping:

    by dividing them by a constant (550 for RGMII, 170 for MII/RMII)

The in-tree arch/arm64/boot/dts/mediatek/mt2712-evb.dts uses
phy-mode = "rgmii-rxid" and mediatek,tx-delay-ps = <1530> (9 x 170ps).
For that board, the GTXC stage count written to PERI_ETH_DLY drops from 9
to 1530 / 550 = 2.

In MII/RMII mode, a binding-valid 1100ps (2 x 550ps) now becomes 6 stages,
which is roughly 3300ps on the hardware. Any value above 31 * 170 no longer
fits in the 5-bit ETH_DLY_*_STAGES fields, and FIELD_PREP() truncates it.

This path runs on every probe and resume through mediatek_dwmac_init() ->
mt2712_set_delay(). Could it keep the link from coming up, or corrupt
TX/RX data, on MT2712 boards that set a delay property?

The problem is still there at the end of the series. In the later patch
"net: stmmac: mediatek: add RX/TX delay stage divider in platform data",
mt2712_gmac_variant sets .rx_rgmii_delay_stage_div to
MT2712_RGMII_DLY_RXC_STAGE_DIV (550) and the rmii dividers to 170. It also
sets .tx_rgmii_delay_stage_div to MT2712_RMII_DLY_TXC_STAGE_DIV (170),
which looks like a typo.

That typo happens to restore the RGMII TX divider for mt2712-evb. RGMII RX
and all MII/RMII delays still appear to use the wrong divider.

[ ... ]

> @@ -263,19 +248,17 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)

[ ... ]

>  static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
> -		.dwmac_set_phy_interface = mt2712_set_interface,
> -		.dwmac_set_delay = mt2712_set_delay,
> -		.clk_list = mt2712_dwmac_clk_l,
> -		.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
> -		.rx_delay_max = 17600,
> -		.tx_delay_max = 17600,
> -		.dma_bit_mask = 33,
> +	.dwmac_set_phy_interface = mt2712_set_interface,
> +	.dwmac_set_delay = mt2712_set_delay,
> +	.clk_list = mt2712_dwmac_clk_l,
> +	.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
> +	.rx_delay_max = MT2712_RGMII_DLY_RXC_MAX,
> +	.tx_delay_max = MT2712_RGMII_DLY_TXC_MAX,
> +	.dma_bit_mask = 33,
>  };

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Should
the MT2712 delay limit depend on the phy mode?

mt2712_gmac_variant applies one 17600ps rx_delay_max/tx_delay_max to every
interface mode. mediatek_dwmac_config_dt() only rejects values at or above
that limit:

    if (tx_delay_ps < plat->variant->tx_delay_max) {
        mac_delay->tx_delay = tx_delay_ps;

The ETH_DLY_GTXC_STAGES, ETH_DLY_TXC_STAGES and ETH_DLY_RXC_STAGES fields
are 5 bits wide, so each holds at most 31 stages.

In the 170ps per stage mode (RGMII per the binding), any accepted value
above 31 * 170 = 5270ps gives a stage count between 32 and 103. FIELD_PREP()
on a runtime value just masks it (for example 100 & 0x1f = 4). The result
is an unrelated delay, written with the ENABLE bit still set and no error
reported.

The binding documents a separate range for each mode (RGMII 0~31*170,
MII/RMII 0~31*550).

This patch also moves the limit into macros named MT2712_RGMII_DLY_*_MAX
with the comment "32 x 550ps". That does not match the 170ps RGMII stage
given in the binding.

Because of the swapped dividers above, binding-valid MII/RMII values can
now overflow as well. That part goes away once the divider swap is fixed.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-dwmac-mediatek-mt8189-v3-0-25daf2bbcf35%40collabora.com

[PATCH net-next v3 5/9] net: stmmac: mediatek: add RX/TX delay stage divider in platform data

[Louis-Alexis Eyraud]

In preparation of newer SoC support, that have like MT8189 SoC
different RX and TX internal delay stage steps than MT8188/MT8195, add
the RX and TX delay stage divider values in the variant platform data
and replace the hardcoded value uses in mt2712_set_delay and
mt8195_set_delay.

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 46 ++++++++++++++++++----
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 596411d4a5b2..cb5bda53edb9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -108,6 +108,10 @@ struct mediatek_dwmac_variant {
 
 	u32 rx_delay_max;
 	u32 tx_delay_max;
+	u16 rx_rmii_delay_stage_div;
+	u16 tx_rmii_delay_stage_div;
+	u16 rx_rgmii_delay_stage_div;
+	u16 tx_rgmii_delay_stage_div;
 	u8 dma_bit_mask;
 };
 
@@ -146,14 +150,22 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
 	u32 fine_val = 0;
 
 	if (phy_interface_mode_is_rgmii(plat->phy_mode)) {
-		rx_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV;
-		tx_delay_stage_div = MT2712_RGMII_DLY_TXC_STAGE_DIV;
+		rx_delay_stage_div = plat->variant->rx_rgmii_delay_stage_div;
+		tx_delay_stage_div = plat->variant->tx_rgmii_delay_stage_div;
 	} else {
-		rx_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV;
-		tx_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV;
+		rx_delay_stage_div = plat->variant->rx_rmii_delay_stage_div;
+		tx_delay_stage_div = plat->variant->tx_rmii_delay_stage_div;
 	}
-	rx_delay_stage_val = mac_delay->rx_delay / rx_delay_stage_div;
-	tx_delay_stage_val = mac_delay->tx_delay / tx_delay_stage_div;
+
+	if (rx_delay_stage_div)
+		rx_delay_stage_val = mac_delay->rx_delay / rx_delay_stage_div;
+	else
+		rx_delay_stage_val = 0;
+
+	if (tx_delay_stage_div)
+		tx_delay_stage_val = mac_delay->tx_delay / tx_delay_stage_div;
+	else
+		tx_delay_stage_val = 0;
 
 	switch (plat->phy_mode) {
 	case PHY_INTERFACE_MODE_MII:
@@ -258,6 +270,10 @@ static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
 	.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
 	.rx_delay_max = MT2712_RGMII_DLY_RXC_MAX,
 	.tx_delay_max = MT2712_RGMII_DLY_TXC_MAX,
+	.rx_rmii_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV,
+	.tx_rmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,
+	.rx_rgmii_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV,
+	.tx_rgmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,
 	.dma_bit_mask = 33,
 };
 
@@ -283,13 +299,25 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
 
 static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 {
+	u16 rx_delay_stage_div = plat->variant->rx_rgmii_delay_stage_div;
+	u16 tx_delay_stage_div = plat->variant->tx_rgmii_delay_stage_div;
 	struct mac_delay_struct *mac_delay = &plat->mac_delay;
-	u32 rx_delay_stage_val = mac_delay->rx_delay / MT8195_DLY_RXC_STAGE_DIV;
-	u32 tx_delay_stage_val = mac_delay->tx_delay / MT8195_DLY_TXC_STAGE_DIV;
+	u32 rx_delay_stage_val;
+	u32 tx_delay_stage_val;
 	u32 gtxc_delay_val = 0;
 	u32 rmii_delay_val = 0;
 	u32 delay_val = 0;
 
+	if (rx_delay_stage_div)
+		rx_delay_stage_val = mac_delay->rx_delay / rx_delay_stage_div;
+	else
+		rx_delay_stage_val = 0;
+
+	if (tx_delay_stage_div)
+		tx_delay_stage_val = mac_delay->tx_delay / tx_delay_stage_div;
+	else
+		tx_delay_stage_val = 0;
+
 	switch (plat->phy_mode) {
 	case PHY_INTERFACE_MODE_MII:
 		delay_val |= FIELD_PREP(MT8195_DLY_TXC_ENABLE,
@@ -398,6 +426,8 @@ static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
 	.num_clks = ARRAY_SIZE(mt8195_dwmac_clk_l),
 	.rx_delay_max = MT8195_DLY_RXC_MAX,
 	.tx_delay_max = MT8195_DLY_TXC_MAX,
+	.rx_rgmii_delay_stage_div = MT8195_DLY_RXC_STAGE_DIV,
+	.tx_rgmii_delay_stage_div = MT8195_DLY_TXC_STAGE_DIV,
 	.dma_bit_mask = 35,
 };
 

-- 
2.56.0

Re: [PATCH net-next v3 5/9] net: stmmac: mediatek: add RX/TX delay stage divider in platform data

[Maxime Chevallier]

Hi Louis-Alexis,

On 10/16/26 12:31, Louis-Alexis Eyraud wrote:
> In preparation of newer SoC support, that have like MT8189 SoC
> different RX and TX internal delay stage steps than MT8188/MT8195, add
> the RX and TX delay stage divider values in the variant platform data
> and replace the hardcoded value uses in mt2712_set_delay and
> mt8195_set_delay.
> 
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 46 ++++++++++++++++++----
>  1 file changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 596411d4a5b2..cb5bda53edb9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -108,6 +108,10 @@ struct mediatek_dwmac_variant {
>  
>  	u32 rx_delay_max;
>  	u32 tx_delay_max;
> +	u16 rx_rmii_delay_stage_div;
> +	u16 tx_rmii_delay_stage_div;
> +	u16 rx_rgmii_delay_stage_div;
> +	u16 tx_rgmii_delay_stage_div;
>  	u8 dma_bit_mask;
>  };
>  
> @@ -146,14 +150,22 @@ static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
>  	u32 fine_val = 0;
>  
>  	if (phy_interface_mode_is_rgmii(plat->phy_mode)) {
> -		rx_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV;
> -		tx_delay_stage_div = MT2712_RGMII_DLY_TXC_STAGE_DIV;
> +		rx_delay_stage_div = plat->variant->rx_rgmii_delay_stage_div;
> +		tx_delay_stage_div = plat->variant->tx_rgmii_delay_stage_div;
>  	} else {
> -		rx_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV;
> -		tx_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV;
> +		rx_delay_stage_div = plat->variant->rx_rmii_delay_stage_div;
> +		tx_delay_stage_div = plat->variant->tx_rmii_delay_stage_div;

Looking closer at this, what's RMII delay ? There's no delays in RMII, and
it's a single clock not a RX and TX clock like RGMII, there's no RMII TXC/RXC.

I think it's the same as on allwinner, the delay handling has been mixed-up between
RMII and RGMII in the glue, you should be able to simplify this even more by
ditching the RMII case.

Maxime

Re: [PATCH net-next v3 5/9] net: stmmac: mediatek: add RX/TX delay stage divider in platform data

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In the variant data this patch adds for MT2712, the delay-stage
  dividers are the wrong way round for the hardware, and the RGMII TX…

--

net: stmmac: mediatek: add RX/TX delay stage divider in platform data

This commit adds RMII and RGMII RX/TX delay stage divider fields to
struct mediatek_dwmac_variant. It makes mt2712_set_delay() and
mt8195_set_delay() read the dividers from the variant data instead of
hardcoded constants, and it guards the divisions against a zero divider.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 596411d4a5b2b..cb5bda53edb94 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

[ ... ]

> @@ -258,6 +270,10 @@ static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
>  	.num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
>  	.rx_delay_max = MT2712_RGMII_DLY_RXC_MAX,
>  	.tx_delay_max = MT2712_RGMII_DLY_TXC_MAX,
> +	.rx_rmii_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV,
> +	.tx_rmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,
> +	.rx_rgmii_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV,
> +	.tx_rgmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,
                                    ^^^^

[Severity: High]

Should this be MT2712_RGMII_DLY_TXC_STAGE_DIV? The RX line above uses
the RGMII constant, and after this line MT2712_RGMII_DLY_TXC_STAGE_DIV
has no users.

Compared with the parent commit, this changes the RGMII TX (GTXC)
divider in mt2712_set_delay() from 550 to 170. The commit message
doesn't mention that change.

There also seems to be a wider problem with the values. The previous
patch in this series defined:

#define MT2712_RMII_DLY_RXC_STAGE_DIV	170 /* 170ps per stage */
#define MT2712_RMII_DLY_TXC_STAGE_DIV	170 /* 170ps per stage */

#define MT2712_RGMII_DLY_RXC_STAGE_DIV	550 /* 550ps per stage */
[ ... ]
#define MT2712_RGMII_DLY_TXC_STAGE_DIV	550 /* 550ps per stage */

The original mt2712_delay_ps2stage() used the opposite mapping:

	case PHY_INTERFACE_MODE_MII:
	case PHY_INTERFACE_MODE_RMII:
		/* 550ps per stage for MII/RMII */
		mac_delay->tx_delay /= 550;
		mac_delay->rx_delay /= 550;
		break;
	[ ... ]
		/* 170ps per stage for RGMII */
		mac_delay->tx_delay /= 170;
		mac_delay->rx_delay /= 170;

The binding in mediatek-dwmac.yaml matches the original code:

      For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
      or will round down. Range 0~31*170.
      For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
      or will round down. Range 0~31*550.

The commit message of that earlier patch also seems to describe the
original code the wrong way round (550 for RGMII, 170 for MII/RMII).

With those values now stored in mt2712_gmac_variant, mt2712_set_delay()
divides by:

  RGMII RX:        550 (originally 170)
  RGMII TX:        170 (correct only because of the constant above)
  MII/RMII RX/TX:  170 (originally 550)

Wouldn't this write the wrong stage counts to PERI_ETH_DLY?

For example, RGMII with mediatek,rx-delay-ps = <1700> gives 3 RXC
stages instead of 10. MII with 1100 ps gives 6 stages instead of 2.

RMII with 5500 ps is inside the binding's 0~31*550 range, but
5500 / 170 = 32. FIELD_PREP() masks that to 0 in the 5-bit
ETH_DLY_*_STAGES field, while the ENABLE bit is still set from
!!rx_delay_stage_val.

The in-tree mt2712-evb (rgmii-rxid, tx-delay-ps = 1530) gets the
expected 9 stages only because of the RMII constant on this line. The
parent commit gave it 2.

This still looks present at the end of the series. Should both parts
be fixed together? That would mean swapping the macro values back
(RGMII = 170, MII/RMII = 550) and using MT2712_RGMII_DLY_TXC_STAGE_DIV
here. Fixing only one of them still leaves RGMII TX at 550.

>  	.dma_bit_mask = 33,
>  };

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-dwmac-mediatek-mt8189-v3-0-25daf2bbcf35%40collabora.com

[PATCH net-next v3 6/9] net: stmmac: mediatek: add PERI_ETH_CTRLx register offset in platform data

[Louis-Alexis Eyraud]

In preparation of newer SoC support, that use like MT8195 the Ethernet
control registers from the peripheral configuration syscon but at a
different base offset, add a new base offset in the variant platform
data to access the PERI_ETH_CTRLx registers and use it in implemented
methods.

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 26 ++++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index cb5bda53edb9..7dd46cd5a49e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -45,7 +45,9 @@
 #define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */
 
 /* Peri Configuration register for mt8195 */
-#define MT8195_PERI_ETH_CTRL0		0xFD0
+#define MT8195_PERI_ETH_CTRL_OFFSET	0xFD0
+
+#define MT8195_PERI_ETH_CTRL0		0x0
 #define MT8195_RMII_CLK_SRC_INTERNAL	BIT(28)
 #define MT8195_RMII_CLK_SRC_RXC		BIT(27)
 #define MT8195_ETH_INTF_SEL		GENMASK(26, 24)
@@ -55,7 +57,7 @@
 #define MT8195_DLY_GTXC_ENABLE		BIT(5)
 #define MT8195_DLY_GTXC_STAGES		GENMASK(4, 0)
 
-#define MT8195_PERI_ETH_CTRL1		0xFD4
+#define MT8195_PERI_ETH_CTRL1		0x4
 #define MT8195_DLY_RXC_INV		BIT(25)
 #define MT8195_DLY_RXC_ENABLE		BIT(18)
 #define MT8195_DLY_RXC_STAGES		GENMASK(17, 13)
@@ -63,7 +65,7 @@
 #define MT8195_DLY_TXC_ENABLE		BIT(5)
 #define MT8195_DLY_TXC_STAGES		GENMASK(4, 0)
 
-#define MT8195_PERI_ETH_CTRL2		0xFD8
+#define MT8195_PERI_ETH_CTRL2		0x8
 #define MT8195_DLY_RMII_RXC_INV		BIT(25)
 #define MT8195_DLY_RMII_RXC_ENABLE	BIT(18)
 #define MT8195_DLY_RMII_RXC_STAGES	GENMASK(17, 13)
@@ -106,6 +108,7 @@ struct mediatek_dwmac_variant {
 	const char * const *clk_list;
 	int num_clks;
 
+	u32 peri_eth_ctrl_offset;
 	u32 rx_delay_max;
 	u32 tx_delay_max;
 	u16 rx_rmii_delay_stage_div;
@@ -281,6 +284,7 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
 				u8 phy_intf_sel)
 {
 	u32 intf_val = FIELD_PREP(MT8195_ETH_INTF_SEL, phy_intf_sel);
+	u32 reg_offset = plat->variant->peri_eth_ctrl_offset;
 
 	if (phy_intf_sel == PHY_INTF_SEL_RMII) {
 		if (plat->rmii_clk_from_mac)
@@ -292,7 +296,9 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
 	/* MT8195 only support external PHY */
 	intf_val |= MT8195_EXT_PHY_MODE;
 
-	regmap_write(plat->peri_regmap, MT8195_PERI_ETH_CTRL0, intf_val);
+	regmap_write(plat->peri_regmap,
+		     reg_offset + MT8195_PERI_ETH_CTRL0,
+		     intf_val);
 
 	return 0;
 }
@@ -302,6 +308,7 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 	u16 rx_delay_stage_div = plat->variant->rx_rgmii_delay_stage_div;
 	u16 tx_delay_stage_div = plat->variant->tx_rgmii_delay_stage_div;
 	struct mac_delay_struct *mac_delay = &plat->mac_delay;
+	u32 reg_offset = plat->variant->peri_eth_ctrl_offset;
 	u32 rx_delay_stage_val;
 	u32 tx_delay_stage_val;
 	u32 gtxc_delay_val = 0;
@@ -407,14 +414,18 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 	}
 
 	regmap_update_bits(plat->peri_regmap,
-			   MT8195_PERI_ETH_CTRL0,
+			   reg_offset + MT8195_PERI_ETH_CTRL0,
 			   MT8195_RGMII_TXC_PHASE_CTRL |
 			   MT8195_DLY_GTXC_INV |
 			   MT8195_DLY_GTXC_ENABLE |
 			   MT8195_DLY_GTXC_STAGES,
 			   gtxc_delay_val);
-	regmap_write(plat->peri_regmap, MT8195_PERI_ETH_CTRL1, delay_val);
-	regmap_write(plat->peri_regmap, MT8195_PERI_ETH_CTRL2, rmii_delay_val);
+	regmap_write(plat->peri_regmap,
+		     reg_offset + MT8195_PERI_ETH_CTRL1,
+		     delay_val);
+	regmap_write(plat->peri_regmap,
+		     reg_offset + MT8195_PERI_ETH_CTRL2,
+		     rmii_delay_val);
 
 	return 0;
 }
@@ -429,6 +440,7 @@ static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
 	.rx_rgmii_delay_stage_div = MT8195_DLY_RXC_STAGE_DIV,
 	.tx_rgmii_delay_stage_div = MT8195_DLY_TXC_STAGE_DIV,
 	.dma_bit_mask = 35,
+	.peri_eth_ctrl_offset = MT8195_PERI_ETH_CTRL_OFFSET,
 };
 
 static int mediatek_dwmac_config_dt(struct mediatek_dwmac_plat_data *plat)

-- 
2.56.0

[PATCH net-next v3 7/9] net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed

[Louis-Alexis Eyraud]

Currently, if the devicetree of a board based on MT8195 (or compatible)
configures the use of RGMII PHY mode and insert at MAC level additional
delay on TX clock at MAC (with "mediatek,tx-delay-ps" vendor property
presence), the driver inserts this delay by setting TX coarse delay
bits (bit 0 to 5) of the Ethernet control 0 register from the
peripheral configuration (pericfg) at probe time.

The HW delay macro circuit, that these bits enable, can be affected by
external factors such as environment temperature or vcore voltage and
thus applied delay value can change. For 10/100M link speed cases the
variations are not problematic, but it can be for 1G link speed cases,
that require much precise and stable timings (around 2ns).

The Ethernet control 0 register of MT8195 SoC has another bit (bit 22:
MT8195_RGMII_TXC_PHASE_CTR) to enable the TX clock phase shift and
insert a 2ns clock delay in a more consistence way, as it is not
affected like the HW delay macro circuit under the same conditions.
This only works though for 1G speed cases so it cannot be set
unconditionally at probe time, thus needs to be set after link speed
negotiation.

In order to allow the TX clock phase shift use in the driver, add in
the variant platform data and probe sequence the support of
fix_mac_speed callback, provided by dwmac core. This callback allows
driver configuration update when the link speed has been negotiated.
Implement it for MT8195 so that, when a delay needs to be inserted
for TX clock (due to the devicetree configuration), it enables the
TX clock phase shift in Ethernet control 0 register only and only if
the PHY mode allows it and negotiated link speed is 1G, and fallback
to the HW delay macro circuit otherwise.

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 7dd46cd5a49e..4b2d5280f703 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -104,6 +104,9 @@ struct mediatek_dwmac_variant {
 				       u8 phy_intf_sel);
 	int (*dwmac_set_delay)(struct mediatek_dwmac_plat_data *plat);
 
+	void (*dwmac_fix_mac_speed)(void *priv, phy_interface_t interface,
+				    int speed, unsigned int mode);
+
 	/* clock ids to be requested */
 	const char * const *clk_list;
 	int num_clks;
@@ -430,9 +433,65 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
 	return 0;
 }
 
+static void mt8195_fix_mac_speed(void *priv, phy_interface_t interface,
+				 int speed, unsigned int mode)
+{
+	struct mediatek_dwmac_plat_data *priv_plat = priv;
+	const struct mediatek_dwmac_variant *variant;
+	struct mac_delay_struct *mac_delay;
+	u32 tx_delay_stage_val, reg_offset;
+	u32 reg_val = 0;
+
+	if (!priv_plat)
+		return;
+
+	mac_delay = &priv_plat->mac_delay;
+	variant = priv_plat->variant;
+
+	if (!mac_delay->tx_delay ||
+	    (interface != PHY_INTERFACE_MODE_RGMII &&
+	     interface != PHY_INTERFACE_MODE_RGMII_RXID))
+		return;
+
+	/*
+	 * When link speed is 1Gbps with RGMII interface, and a TX internal
+	 * delay needs to be applied on MAC, prefer to override the delay
+	 * settings with a 2ns fixed delay which is controlled by
+	 * RGMII_TXC_PHASE_CTRL. Otherwise, fallback to HW delay macro circuit
+	 * for 10/100Mbps link speeds.
+	 */
+	if (speed == SPEED_1000) {
+		reg_val = MT8195_RGMII_TXC_PHASE_CTRL;
+	} else {
+		if (variant->tx_rgmii_delay_stage_div) {
+			tx_delay_stage_val = mac_delay->tx_delay /
+					     variant->tx_rgmii_delay_stage_div;
+		} else {
+			tx_delay_stage_val = 0;
+		}
+
+		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_ENABLE,
+				      !!tx_delay_stage_val);
+		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_STAGES,
+				      tx_delay_stage_val);
+		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_INV,
+				      mac_delay->tx_inv);
+	}
+
+	reg_offset = variant->peri_eth_ctrl_offset + MT8195_PERI_ETH_CTRL0;
+	regmap_update_bits(priv_plat->peri_regmap,
+			   reg_offset,
+			   MT8195_RGMII_TXC_PHASE_CTRL |
+			   MT8195_DLY_GTXC_ENABLE |
+			   MT8195_DLY_GTXC_INV |
+			   MT8195_DLY_GTXC_STAGES,
+			   reg_val);
+}
+
 static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
 	.dwmac_set_phy_interface = mt8195_set_interface,
 	.dwmac_set_delay = mt8195_set_delay,
+	.dwmac_fix_mac_speed = mt8195_fix_mac_speed,
 	.clk_list = mt8195_dwmac_clk_l,
 	.num_clks = ARRAY_SIZE(mt8195_dwmac_clk_l),
 	.rx_delay_max = MT8195_DLY_RXC_MAX,
@@ -591,6 +650,9 @@ static int mediatek_dwmac_common_data(struct platform_device *pdev,
 	plat->resume = mediatek_dwmac_init;
 	plat->clks_config = mediatek_dwmac_clks_config;
 
+	if (priv_plat->variant->dwmac_fix_mac_speed)
+		plat->fix_mac_speed = priv_plat->variant->dwmac_fix_mac_speed;
+
 	plat->safety_feat_cfg = devm_kzalloc(&pdev->dev,
 					     sizeof(*plat->safety_feat_cfg),
 					     GFP_KERNEL);

-- 
2.56.0

Re: [PATCH net-next v3 7/9] net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed

[Maxime Chevallier]

Hi,

On 10/16/26 12:31, Louis-Alexis Eyraud wrote:
> Currently, if the devicetree of a board based on MT8195 (or compatible)
> configures the use of RGMII PHY mode and insert at MAC level additional
> delay on TX clock at MAC (with "mediatek,tx-delay-ps" vendor property
> presence), the driver inserts this delay by setting TX coarse delay
> bits (bit 0 to 5) of the Ethernet control 0 register from the
> peripheral configuration (pericfg) at probe time.
> 
> The HW delay macro circuit, that these bits enable, can be affected by
> external factors such as environment temperature or vcore voltage and
> thus applied delay value can change. For 10/100M link speed cases the
> variations are not problematic, but it can be for 1G link speed cases,
> that require much precise and stable timings (around 2ns).
> 
> The Ethernet control 0 register of MT8195 SoC has another bit (bit 22:
> MT8195_RGMII_TXC_PHASE_CTR) to enable the TX clock phase shift and
> insert a 2ns clock delay in a more consistence way, as it is not
> affected like the HW delay macro circuit under the same conditions.
> This only works though for 1G speed cases so it cannot be set
> unconditionally at probe time, thus needs to be set after link speed
> negotiation.
> 
> In order to allow the TX clock phase shift use in the driver, add in
> the variant platform data and probe sequence the support of
> fix_mac_speed callback, provided by dwmac core. This callback allows
> driver configuration update when the link speed has been negotiated.
> Implement it for MT8195 so that, when a delay needs to be inserted
> for TX clock (due to the devicetree configuration), it enables the
> TX clock phase shift in Ethernet control 0 register only and only if
> the PHY mode allows it and negotiated link speed is 1G, and fallback
> to the HW delay macro circuit otherwise.
> 
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 7dd46cd5a49e..4b2d5280f703 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -104,6 +104,9 @@ struct mediatek_dwmac_variant {
>  				       u8 phy_intf_sel);
>  	int (*dwmac_set_delay)(struct mediatek_dwmac_plat_data *plat);
>  
> +	void (*dwmac_fix_mac_speed)(void *priv, phy_interface_t interface,
> +				    int speed, unsigned int mode);
> +
>  	/* clock ids to be requested */
>  	const char * const *clk_list;
>  	int num_clks;
> @@ -430,9 +433,65 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
>  	return 0;
>  }
>  
> +static void mt8195_fix_mac_speed(void *priv, phy_interface_t interface,
> +				 int speed, unsigned int mode)
> +{
> +	struct mediatek_dwmac_plat_data *priv_plat = priv;
> +	const struct mediatek_dwmac_variant *variant;
> +	struct mac_delay_struct *mac_delay;
> +	u32 tx_delay_stage_val, reg_offset;
> +	u32 reg_val = 0;
> +
> +	if (!priv_plat)
> +		return;
> +
> +	mac_delay = &priv_plat->mac_delay;
> +	variant = priv_plat->variant;
> +
> +	if (!mac_delay->tx_delay ||
> +	    (interface != PHY_INTERFACE_MODE_RGMII &&
> +	     interface != PHY_INTERFACE_MODE_RGMII_RXID))
> +		return;

I don't think this is right. PHY_INTERFACE_MODE_RGMII in that context comes
from the devicetree, and therefore means "RGMII delays are implemented through
traces on the PCB", the MAC isn't supposed to insert any. RXID means "TX delays
are on PCB, RX delays are internal).

I'd expect the condition here to be:

> +	if (!mac_delay->tx_delay ||
> +	    (interface != PHY_INTERFACE_MODE_RGMII_ID &&
> +	     interface != PHY_INTERFACE_MODE_RGMII_TXID))
> +		return;

Maxime

Re: [PATCH net-next v3 7/9] net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed

[Andrew Lunn]

> > +static void mt8195_fix_mac_speed(void *priv, phy_interface_t interface,
> > +				 int speed, unsigned int mode)
> > +{
> > +	struct mediatek_dwmac_plat_data *priv_plat = priv;
> > +	const struct mediatek_dwmac_variant *variant;
> > +	struct mac_delay_struct *mac_delay;
> > +	u32 tx_delay_stage_val, reg_offset;
> > +	u32 reg_val = 0;
> > +
> > +	if (!priv_plat)
> > +		return;
> > +
> > +	mac_delay = &priv_plat->mac_delay;
> > +	variant = priv_plat->variant;
> > +
> > +	if (!mac_delay->tx_delay ||
> > +	    (interface != PHY_INTERFACE_MODE_RGMII &&
> > +	     interface != PHY_INTERFACE_MODE_RGMII_RXID))
> > +		return;
> 
> I don't think this is right. PHY_INTERFACE_MODE_RGMII in that context comes
> from the devicetree, and therefore means "RGMII delays are implemented through
> traces on the PCB", the MAC isn't supposed to insert any. RXID means "TX delays
> are on PCB, RX delays are internal).
> 
> I'd expect the condition here to be:
> 
> > +	if (!mac_delay->tx_delay ||
> > +	    (interface != PHY_INTERFACE_MODE_RGMII_ID &&
> > +	     interface != PHY_INTERFACE_MODE_RGMII_TXID))
> > +		return;

I would maybe base it on the value of tx-internal-delay-ps. If it is
2000, then add the delay here. But that has implications for the
interface mode value passed to the PHY.

https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287

We could keep mt8195 KISS, and reject all these properties, and let
the PHY add the delays.

	Andrew

Re: [PATCH net-next v3 7/9] net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] At 1Gbps on RGMII, mt8195_fix_mac_speed()
  (drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c) treats the DT…
- [Medium] At 1Gbps, mt8195_fix_mac_speed()
  (drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c) silently clears…

--

net: stmmac: mediatek: use TX clock phase shift in RGMII mode with 1Gbps speed

This commit adds a fix_mac_speed callback to the MediaTek dwmac glue and
implements it for MT8195. When an RGMII link comes up at 1Gbps and the DT
requests a TX delay, the callback turns on the fixed 2ns TX clock phase
shift (RGMII_TXC_PHASE_CTRL). At other speeds it falls back to the coarse
delay macro circuit.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 7dd46cd5a49e1..4b2d5280f7030 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

[ ... ]

> @@ -430,9 +433,65 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
>  	return 0;
>  }
>  
> +static void mt8195_fix_mac_speed(void *priv, phy_interface_t interface,
> +				 int speed, unsigned int mode)
> +{

[ ... ]

> +	if (!mac_delay->tx_delay ||
> +	    (interface != PHY_INTERFACE_MODE_RGMII &&
> +	     interface != PHY_INTERFACE_MODE_RGMII_RXID))
> +		return;
> +
> +	/*
> +	 * When link speed is 1Gbps with RGMII interface, and a TX internal
> +	 * delay needs to be applied on MAC, prefer to override the delay
> +	 * settings with a 2ns fixed delay which is controlled by
> +	 * RGMII_TXC_PHASE_CTRL. Otherwise, fallback to HW delay macro circuit
> +	 * for 10/100Mbps link speeds.
> +	 */
> +	if (speed == SPEED_1000) {
> +		reg_val = MT8195_RGMII_TXC_PHASE_CTRL;

[Severity: Medium]
At 1Gbps, does this treat mediatek,tx-delay-ps as an on/off switch rather
than as a delay amount?

The gate checks the raw picosecond value in mac_delay->tx_delay.
mediatek_dwmac_config_dt() stores that value without rounding:

    if (tx_delay_ps < plat->variant->tx_delay_max) {
        mac_delay->tx_delay = tx_delay_ps;

Both mt8195_set_delay() and the new 10/100 branch round the value down to
290ps stages (180ps on MT8189). So a value of 1..289ps means no delay at
probe time and at 10/100. At 1G, the same DT value now gets the full ~2ns
phase shift.

For every other value (290ps, 870ps, 8990ps, ...), the configured amount is
replaced by a fixed ~2ns at 1G. GTXC_ENABLE and GTXC_STAGES are in the
update mask, so the coarse delay is also cleared, with no warning.

The same DT setting therefore gives a different MAC TX delay depending on
link speed. Could boards that tuned tx-delay-ps to their PCB skew end up
with the wrong total skew at gigabit?

The binding in Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
still describes the property as:

      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a
      multiple of 290, or will round down. Range 0~31*290.

The commit message also doesn't say that the configured value is ignored at
1G.

None of the in-tree boards seem to hit this. mt8395-genio-common and
mt8395-radxa-nio-12l use rgmii-rxid with 2030ps. mt8390-grinn-genio-sbc uses
30ps, but its phy-mode is rgmii-id, which skips this path.

The later commit "net: stmmac: mediatek: add support for MT8189 SoC" also
wires mt8195_fix_mac_speed() into the MT8189 variant, so MT8189 gets the
same behaviour.

> +	} else {
> +		if (variant->tx_rgmii_delay_stage_div) {
> +			tx_delay_stage_val = mac_delay->tx_delay /
> +					     variant->tx_rgmii_delay_stage_div;
> +		} else {
> +			tx_delay_stage_val = 0;
> +		}
> +
> +		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_ENABLE,
> +				      !!tx_delay_stage_val);
> +		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_STAGES,
> +				      tx_delay_stage_val);
> +		reg_val |= FIELD_PREP(MT8195_DLY_GTXC_INV,
> +				      mac_delay->tx_inv);
> +	}
> +
> +	reg_offset = variant->peri_eth_ctrl_offset + MT8195_PERI_ETH_CTRL0;
> +	regmap_update_bits(priv_plat->peri_regmap,
> +			   reg_offset,
> +			   MT8195_RGMII_TXC_PHASE_CTRL |
> +			   MT8195_DLY_GTXC_ENABLE |
> +			   MT8195_DLY_GTXC_INV |
> +			   MT8195_DLY_GTXC_STAGES,
> +			   reg_val);
> +}

[Severity: Medium]
Does the 1G path clear the TX clock inversion requested by
mediatek,txc-inverse?

MT8195_DLY_GTXC_INV is in the regmap_update_bits() mask. The SPEED_1000
branch, however, sets only MT8195_RGMII_TXC_PHASE_CTRL in reg_val. GTXC_INV
is therefore written as 0 whatever mac_delay->tx_inv says.

The 10/100 branch above keeps the inversion. mt8195_set_delay() also
applies it for all RGMII modes at probe and resume:

    gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_INV, mac_delay->tx_inv);

The 1G case is also inconsistent with itself. With tx-delay-ps = 0, the
early return leaves the inversion in place. With any non-zero tx-delay-ps,
it is cleared.

The binding describes mediatek,txc-inverse as "tx clock will be inversed in
MII/RGMII case", with no exception for any speed. Should the SPEED_1000
branch also include FIELD_PREP(MT8195_DLY_GTXC_INV, mac_delay->tx_inv)?

No in-tree MediaTek board uses txc-inverse today. MT8189 is affected too,
once "net: stmmac: mediatek: add support for MT8189 SoC" wires in the same
callback.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-dwmac-mediatek-mt8189-v3-0-25daf2bbcf35%40collabora.com

[PATCH net-next v3 8/9] net: stmmac: mediatek: add support for TX clock output enable feature

[Louis-Alexis Eyraud]

The MT8189 SoC has in the Ethernet control 0 register from the
peripheral configuration (pericfg) an additional bit to enable the TX
clock signal output.

In preparation of MT8189 SoC support, add its definition, use in the
mt8195_set_interface callback, and a support flag in the platform
data.

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 4b2d5280f703..147e217b8662 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -44,6 +44,9 @@
 #define MT2712_RGMII_DLY_TXC_STAGE_DIV	550 /* 550ps per stage */
 #define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */
 
+/* Peri Configuration register for mt8189 */
+#define MT8189_CTRL0_TXC_OUT_OP_EN	BIT(20)
+
 /* Peri Configuration register for mt8195 */
 #define MT8195_PERI_ETH_CTRL_OFFSET	0xFD0
 
@@ -119,6 +122,7 @@ struct mediatek_dwmac_variant {
 	u16 rx_rgmii_delay_stage_div;
 	u16 tx_rgmii_delay_stage_div;
 	u8 dma_bit_mask;
+	bool mac_txclk_out_en;
 };
 
 /* list of clocks required for mac */
@@ -299,6 +303,9 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
 	/* MT8195 only support external PHY */
 	intf_val |= MT8195_EXT_PHY_MODE;
 
+	if (plat->variant->mac_txclk_out_en)
+		intf_val |= MT8189_CTRL0_TXC_OUT_OP_EN;
+
 	regmap_write(plat->peri_regmap,
 		     reg_offset + MT8195_PERI_ETH_CTRL0,
 		     intf_val);

-- 
2.56.0

[PATCH net-next v3 9/9] net: stmmac: mediatek: add support for MT8189 SoC

[Louis-Alexis Eyraud]

Like MT8195 SoC, MT8189 SoC uses the Ethernet control registers from
the peripheral configuration (pericfg) but at a different offset.
It also has different RX and TX internal delay stage dividers (290ps
for MT8195, 180ps for MT8189), and by consequence a reduced value range
for these delays. It finally needs additional feature such as the
TX clock output enable one.

Thus, add compatible, clock definitions and specific platform data to
support the MT8189 SoC.

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 147e217b8662..d480c339a16d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -45,8 +45,15 @@
 #define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */
 
 /* Peri Configuration register for mt8189 */
+#define MT8189_PERI_ETH_CTRL_OFFSET	0x270
+
 #define MT8189_CTRL0_TXC_OUT_OP_EN	BIT(20)
 
+#define MT8189_DLY_RXC_STAGE_DIV	180  /* 180ps per stage */
+#define MT8189_DLY_RXC_MAX		5760 /* 32 * 180ps */
+#define MT8189_DLY_TXC_STAGE_DIV	180  /* 180ps per stage */
+#define MT8189_DLY_TXC_MAX		5760 /* 32 * 180ps */
+
 /* Peri Configuration register for mt8195 */
 #define MT8195_PERI_ETH_CTRL_OFFSET	0xFD0
 
@@ -134,6 +141,10 @@ static const char * const mt8195_dwmac_clk_l[] = {
 	"axi", "apb", "mac_cg", "mac_main", "ptp_ref"
 };
 
+static const char * const mt8189_dwmac_clk_l[] = {
+	"axi", "apb", "mac_main", "ptp_ref"
+};
+
 static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat,
 				u8 phy_intf_sel)
 {
@@ -495,6 +506,21 @@ static void mt8195_fix_mac_speed(void *priv, phy_interface_t interface,
 			   reg_val);
 }
 
+static const struct mediatek_dwmac_variant mt8189_gmac_variant = {
+	.dwmac_set_phy_interface = mt8195_set_interface,
+	.dwmac_set_delay = mt8195_set_delay,
+	.dwmac_fix_mac_speed = mt8195_fix_mac_speed,
+	.clk_list = mt8189_dwmac_clk_l,
+	.num_clks = ARRAY_SIZE(mt8189_dwmac_clk_l),
+	.dma_bit_mask = 35,
+	.rx_delay_max = MT8189_DLY_RXC_MAX,
+	.tx_delay_max = MT8189_DLY_TXC_MAX,
+	.rx_rgmii_delay_stage_div = MT8189_DLY_RXC_STAGE_DIV,
+	.tx_rgmii_delay_stage_div = MT8189_DLY_TXC_STAGE_DIV,
+	.peri_eth_ctrl_offset = MT8189_PERI_ETH_CTRL_OFFSET,
+	.mac_txclk_out_en = true,
+};
+
 static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
 	.dwmac_set_phy_interface = mt8195_set_interface,
 	.dwmac_set_delay = mt8195_set_delay,
@@ -751,6 +777,8 @@ static void mediatek_dwmac_remove(struct platform_device *pdev)
 static const struct of_device_id mediatek_dwmac_match[] = {
 	{ .compatible = "mediatek,mt2712-gmac",
 	  .data = &mt2712_gmac_variant },
+	{ .compatible = "mediatek,mt8189-gmac",
+	  .data = &mt8189_gmac_variant },
 	{ .compatible = "mediatek,mt8195-gmac",
 	  .data = &mt8195_gmac_variant },
 	{ }

-- 
2.56.0

Re: [PATCH net-next v3 9/9] net: stmmac: mediatek: add support for MT8189 SoC

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The series reverses the MT2712 delay-stage dividers compared
  with the baseline driver and the binding.

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but in
  mediatek_dwmac_clks_config(), if clk_bulk_prepare_enable() succeeds…

--

net: stmmac: mediatek: add support for MT8189 SoC

This patch adds the mediatek,mt8189-gmac compatible to dwmac-mediatek,
along with a clock list and variant platform data. The new variant reuses
the MT8195 callbacks with a 0x270 pericfg offset and 180ps RX/TX delay
stage dividers, and it enables the TX clock output.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 147e217b86620..d480c339a16d0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -45,8 +45,15 @@
>  #define MT2712_RGMII_DLY_TXC_MAX	17600 /* 32 x 550ps */

[Severity: Medium]
The context here shows the MT2712 defines added earlier in this series. Are
the MT2712 RGMII and RMII stage dividers swapped?

The defines currently read:

#define MT2712_RMII_DLY_RXC_STAGE_DIV	170 /* 170ps per stage */
#define MT2712_RMII_DLY_TXC_STAGE_DIV	170 /* 170ps per stage */

#define MT2712_RGMII_DLY_RXC_STAGE_DIV	550 /* 550ps per stage */
#define MT2712_RGMII_DLY_RXC_MAX	17600 /* 32 x 550ps */
#define MT2712_RGMII_DLY_TXC_STAGE_DIV	550 /* 550ps per stage */

Before the series, mt2712_delay_ps2stage() divided RGMII delays by 170 and
MII/RMII delays by 550. The mediatek-dwmac.yaml binding says the same:

  For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
  ...
  For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,

mt2712_gmac_variant then uses:

	.rx_rmii_delay_stage_div = MT2712_RMII_DLY_RXC_STAGE_DIV,
	.tx_rmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,
	.rx_rgmii_delay_stage_div = MT2712_RGMII_DLY_RXC_STAGE_DIV,
	.tx_rgmii_delay_stage_div = MT2712_RMII_DLY_TXC_STAGE_DIV,

mt2712_set_delay() picks these dividers based on the phy mode. RGMII TX
still divides by 170, but only because it uses the RMII define. RGMII RX
now divides by 550 instead of 170. MII/RMII TX and RX now divide by 170
instead of 550.

For example, an RGMII mediatek,rx-delay-ps = <1530> used to program 9
stages and now programs 2. An MII/RMII delay of 1650 used to program 3
stages and now programs 9.

This came from two earlier patches in this series, not from this one:
"net: stmmac: mediatek: simplify TX/RX delay handling in mt2712_set_delay"
and "net: stmmac: mediatek: add RX/TX delay stage divider in platform
data". The first one's commit message also describes the old code as
"550 for RGMII, 170 for MII/RMII".

Changing only tx_rgmii_delay_stage_div to MT2712_RGMII_DLY_TXC_STAGE_DIV
would make things worse. mt2712-evb.dts (phy-mode "rgmii-rxid",
mediatek,tx-delay-ps = <1530>) would drop from 9 stages to 2.

Could the define values be swapped instead (RGMII = 170, RMII = 550), with
tx_rgmii_delay_stage_div then using the RGMII define?

[ ... ]

> @@ -751,6 +777,8 @@ static void mediatek_dwmac_remove(struct platform_device *pdev)
>  static const struct of_device_id mediatek_dwmac_match[] = {
>  	{ .compatible = "mediatek,mt2712-gmac",
>  	  .data = &mt2712_gmac_variant },
> +	{ .compatible = "mediatek,mt8189-gmac",
> +	  .data = &mt8189_gmac_variant },

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch, but
MT8189 now uses the same probe path. In mediatek_dwmac_clks_config(), if
clk_bulk_prepare_enable() succeeds and clk_prepare_enable() then fails on
plat->rmii_internal_clk, do the bulk clocks stay enabled?

drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c:mediatek_dwmac_clks_config() {
    ...
	ret = clk_bulk_prepare_enable(variant->num_clks, plat->clks);
	...
	ret = clk_prepare_enable(plat->rmii_internal_clk);
	if (ret) {
		dev_err(plat->dev, "failed to enable rmii internal clk, err = %d\n", ret);
		return ret;
	}
    ...
}

This path never calls clk_bulk_disable_unprepare(), and
mediatek_dwmac_probe() returns the error directly:

	ret = mediatek_dwmac_clks_config(priv_plat, true);
	if (ret)
		return ret;

The resume path behaves the same way.

For MT8189, rmii_internal_clk is only obtained when
mediatek,rmii-clk-from-mac is present, and the MT8189 binding forbids that
property. A binding-compliant MT8189 DT therefore gets a NULL clock and
cannot hit this path, but the driver does not enforce the binding.

>  	{ .compatible = "mediatek,mt8195-gmac",
>  	  .data = &mt8195_gmac_variant },
>  	{ }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-dwmac-mediatek-mt8189-v3-0-25daf2bbcf35%40collabora.com