[PATCH net-next v6 0/5] Add AST2600 RGMII delay into ftgmac100

[PATCH net-next v6 0/5] Add AST2600 RGMII delay into ftgmac100

[Jacky Chou]

This patch series adds support for configuring RGMII internal delays for the
Aspeed AST2600 FTGMAC100 Ethernet MACs. It introduces new compatible strings to
distinguish between MAC0/1 and MAC2/3, as their delay chains and configuration
units differ.
The device tree bindings are updated to restrict the allowed phy-mode and delay
properties for each MAC type. Corresponding changes are made to the device tree
source files and the FTGMAC100 driver to support the new delay configuration.

This enables precise RGMII timing configuration for AST2600-based platforms,
improving interoperability with various PHYs

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
Changes in v6:
- Adjustments made according to net: ftgmac100: Various probe cleanups.
- Split [net-next,v5,3/4] net: ftgmac100: Add RGMII delay support for AST2600.
  1. Confgure RGMII delay value according to rx/tx-internal-delay-ps from dts
  2. Keep RGMII delay value for old dts
- Link to v5: https://lore.kernel.org/r/20251205-rgmii_delay_2600-v5-0-bd2820ad3da7@aspeedtech.com

Changes in v5:
- Remove the new property, "aspeed,rgmii-delay-ps" from yaml and driver
- Add aspeed,scu to aspeed-g6 dtsi back
- Determine delay value from bootloader and tx/rx-internal-delay-ps to
  configure RGMII delay value with phy-mode
- Add a helper for AST2600 to get phy driver handle
- Link to v4: https://lore.kernel.org/r/20251110-rgmii_delay_2600-v4-0-5cad32c766f7@aspeedtech.com

Changes in v4:
- Remove the compatible "aspeed,ast2600-mac01" and
  "aspeed,ast2600-mac23"
- Add new property to specify the RGMII delay step for each MACs
- Add default value of rx/tx-internal-delay-ps
- For legacy dts, a warning message reminds users to update phy-mode
- If lack rx/tx-internal-delay-ps, driver will use default value to
  configure the RGMII delay
- Link to v3: https://lore.kernel.org/r/20251103-rgmii_delay_2600-v3-0-e2af2656f7d7@aspeedtech.com

Changes in v3:
- Add new item on compatible property for new compatible strings
- Remove the new compatible and scu handle of MAC from aspeed-g6.dtsi
- Add new compatible and scu handle to MAC node in
  aspeed-ast2600-evb.dts
- Change all phy-mode of MACs to "rgmii-id"
- Keep "aspeed,ast2600-mac" compatible in ftgmac100.c and configure the
  rgmii delay with "aspeed,ast2600-mac01" and "aspeed,ast2600-mac23"
- Link to v2: https://lore.kernel.org/r/20250813063301.338851-1-jacky_chou@aspeedtech.com

Changes in v2:
- added new compatible strings for MAC0/1 and MAC2/3
- updated device tree bindings to restrict phy-mode and delay properties
- refactored driver code to handle rgmii delay configuration
- Link to v1: https://lore.kernel.org/r/20250317025922.1526937-1-jacky_chou@aspeedtech.com

---
Jacky Chou (5):
      dt-bindings: net: ftgmac100: Add delay properties for AST2600
      ARM: dts: aspeed-g6: add aspeed,scu property for MAC
      net: ftgmac100: Add RGMII delay support for AST2600
      net: ftgmac100: Support rgmii delay in old dts with AST2600
      ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

 .../devicetree/bindings/net/faraday,ftgmac100.yaml |  42 ++-
 arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts    |  20 +-
 arch/arm/boot/dts/aspeed/aspeed-g6.dtsi            |   4 +
 drivers/net/ethernet/faraday/ftgmac100.c           | 318 ++++++++++++++++++++-
 drivers/net/ethernet/faraday/ftgmac100.h           |  25 ++
 5 files changed, 388 insertions(+), 21 deletions(-)
---
base-commit: 01857fc712f6469cab9cc578120cdc80f1c2a634
change-id: 20251031-rgmii_delay_2600-a00b0248c7e6

Best regards,
-- 
Jacky Chou <jacky_chou@aspeedtech.com>

[PATCH net-next v6 1/5] dt-bindings: net: ftgmac100: Add delay properties for AST2600

[Jacky Chou]

The AST2600 contains two dies, each with its own MAC, and these MACs
require different delay configurations.
Previously, these delay values were configured during the bootloader
stage rather than in the driver. This change introduces the use of the
standard properties defined in ethernet-controller.yaml to configure
the delay values directly in the driver.

Each Aspeed platform has its own delay step value. And for Aspeed platform,
the total steps of RGMII delay configuraion is 32 steps, so the total delay
is delay-step-ps * 32.
Default delay values are declared so that tx-internal-delay-ps and
rx-internal-delay-ps become optional. If these properties are not present,
the driver will use the default values instead.
Add conditional schema constraints for Aspeed AST2600 MAC controllers:
- For MAC0/1, per delay step for rgmii is 45 ps
- For MAC2/3, per delay step for rgmii is 250 ps
- Both require the "aspeed,scu" and "aspeed,rgmii-delay-ps" properties.
Other compatible values remain unrestricted.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 .../devicetree/bindings/net/faraday,ftgmac100.yaml | 42 +++++++++++++++++-----
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
index d14410018bcf..4cac216f7339 100644
--- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
+++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
@@ -69,6 +69,30 @@ properties:
   mdio:
     $ref: /schemas/net/mdio.yaml#
 
+  aspeed,scu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the SCU (System Control Unit) syscon node for Aspeed platform.
+      This reference is used by the MAC controller to configure the RGMII delays.
+
+  rx-internal-delay-ps:
+    description:
+      RGMII Receive Clock Delay defined in pico seconds. There are 32
+      steps of RGMII delay for Aspeed platform. Each Aspeed platform has its
+      own delay step value, it is fixed by hardware design. Total delay is
+      calculated by delay-step * 32. A value of 0 ps will disable any
+      delay. The Default is no delay.
+    default: 0
+
+  tx-internal-delay-ps:
+    description:
+      RGMII Transmit Clock Delay defined in pico seconds. There are 32
+      steps of RGMII delay for Aspeed platform. Each Aspeed platform has its
+      own delay step value, it is fixed by hardware design. Total delay is
+      calculated by delay-step * 32. A value of 0 ps will disable any
+      delay. The Default is no delay.
+    default: 0
+
 required:
   - compatible
   - reg
@@ -77,17 +101,17 @@ required:
 allOf:
   - $ref: ethernet-controller.yaml#
   - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - aspeed,ast2600-mac
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - aspeed,ast2600-mac
     then:
       properties:
-        resets: true
-    else:
-      properties:
-        resets: false
+        aspeed,scu: false
+        rx-internal-delay-ps: false
+        tx-internal-delay-ps: false
 
 unevaluatedProperties: false
 

-- 
2.34.1

[PATCH net-next v6 2/5] ARM: dts: aspeed-g6: add aspeed,scu property for MAC

[Jacky Chou]

Add aspeed,scu property to let MAC driver to configure RGMII delay with
scu register.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
index 189bc3bbb47c..cbcdc1c6eadb 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
@@ -235,6 +235,7 @@ mac0: ethernet@1e660000 {
 			reg = <0x1e660000 0x180>;
 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&syscon ASPEED_CLK_GATE_MAC1CLK>;
+			aspeed,scu = <&syscon>;
 			status = "disabled";
 		};
 
@@ -243,6 +244,7 @@ mac1: ethernet@1e680000 {
 			reg = <0x1e680000 0x180>;
 			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&syscon ASPEED_CLK_GATE_MAC2CLK>;
+			aspeed,scu = <&syscon>;
 			status = "disabled";
 		};
 
@@ -251,6 +253,7 @@ mac2: ethernet@1e670000 {
 			reg = <0x1e670000 0x180>;
 			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&syscon ASPEED_CLK_GATE_MAC3CLK>;
+			aspeed,scu = <&syscon>;
 			status = "disabled";
 		};
 
@@ -259,6 +262,7 @@ mac3: ethernet@1e690000 {
 			reg = <0x1e690000 0x180>;
 			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&syscon ASPEED_CLK_GATE_MAC4CLK>;
+			aspeed,scu = <&syscon>;
 			status = "disabled";
 		};
 

-- 
2.34.1

[PATCH net-next v6 3/5] net: ftgmac100: Add RGMII delay support for AST2600

[Jacky Chou]

On the AST2600 platform, the RGMII delay is controlled via the
SCU registers. The delay chain configuration differs between MAC0/1
and MAC2/3, even though all four MACs use a 32-stage delay chain.
+------+----------+-----------+-------------+-------------+
|      |Delay Unit|Delay Stage|TX Edge Stage|RX Edge Stage|
+------+----------+-----------+-------------+-------------+
|MAC0/1|     45 ps|        32 |           0 |           0 |
+------+----------+-----------+-------------+-------------+
|MAC2/3|    250 ps|        32 |           0 |          26 |
+------+----------+-----------+-------------+-------------+
For MAC2/3, the "no delay" condition starts from stage 26.
Setting the RX delay stage to 26 means that no additional RX
delay is applied.
Here lists the RX delay setting of MAC2/3 below.
26 -> 0   ns, 27 -> 0.25 ns, ... , 31 -> 1.25 ns,
0  -> 1.5 ns, 1  -> 1.75 ns, ... , 25 -> 7.75 ns

Therefore, we calculate the delay stage from the
rx-internal-delay-ps of MAC2/3 to add 26. If the stage is equel
to or bigger than 32, the delay stage will be mask 0x1f to get
the correct setting.
The delay chain is like a ring for configuration.
Example for the rx-internal-delay-ps of MAC2/3 is 2000 ps,
we will get the delay stage is 2.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 162 ++++++++++++++++++++++++++++++-
 drivers/net/ethernet/faraday/ftgmac100.h |  20 ++++
 2 files changed, 178 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 1e91e79c8134..0b2a0bb8a4a9 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -26,6 +26,9 @@
 #include <linux/if_vlan.h>
 #include <linux/of_net.h>
 #include <linux/phy_fixed.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/bitfield.h>
 #include <net/ip.h>
 #include <net/ncsi.h>
 
@@ -42,6 +45,7 @@ enum ftgmac100_mac_id {
 
 struct ftgmac100_match_data {
 	enum ftgmac100_mac_id mac_id;
+	bool rgmii_delay_conf;
 };
 
 /* Arbitrary values, I am not sure the HW has limits */
@@ -1919,10 +1923,148 @@ static int ftgmac100_probe_dt(struct net_device *netdev,
 	return 0;
 }
 
+static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
+					     u32 rgmii_tx_delay,
+					     u32 rgmii_rx_delay,
+					     phy_interface_t phy_intf)
+{
+	struct device *dev = priv->dev;
+	struct device_node *np;
+	u32 rgmii_delay_unit;
+	u32 rx_delay_index;
+	u32 tx_delay_index;
+	struct regmap *scu;
+	int dly_mask;
+	int dly_reg;
+	int mac_id;
+
+	np = dev->of_node;
+
+	scu = syscon_regmap_lookup_by_phandle(np, "aspeed,scu");
+	if (IS_ERR(scu)) {
+		dev_err(dev, "failed to get aspeed,scu");
+		return PTR_ERR(scu);
+	}
+
+	/* According to the register base address to specify the corresponding
+	 * values.
+	 */
+	switch (priv->res->start) {
+	case AST2600_MAC0_BASE_ADDR:
+		mac_id = 0;
+		rgmii_delay_unit = AST2600_MAC01_CLK_DLY_UNIT;
+		dly_reg = AST2600_MAC01_CLK_DLY;
+		break;
+	case AST2600_MAC1_BASE_ADDR:
+		mac_id = 1;
+		rgmii_delay_unit = AST2600_MAC01_CLK_DLY_UNIT;
+		dly_reg = AST2600_MAC01_CLK_DLY;
+		break;
+	case AST2600_MAC2_BASE_ADDR:
+		mac_id = 2;
+		rgmii_delay_unit = AST2600_MAC23_CLK_DLY_UNIT;
+		dly_reg = AST2600_MAC23_CLK_DLY;
+		break;
+	case AST2600_MAC3_BASE_ADDR:
+		mac_id = 3;
+		rgmii_delay_unit = AST2600_MAC23_CLK_DLY_UNIT;
+		dly_reg = AST2600_MAC23_CLK_DLY;
+		break;
+	default:
+		dev_err(dev, "Invalid mac base address");
+		return -EINVAL;
+	}
+
+	/* Please refer to ethernet-controller.yaml. */
+	if (phy_intf == PHY_INTERFACE_MODE_RGMII &&
+	    (rgmii_tx_delay == 2000 || rgmii_rx_delay == 2000)) {
+		dev_warn(dev, "RX/TX delay cannot set to 2000 on 'rgmii'\n");
+		return -EINVAL;
+	}
+
+	tx_delay_index = DIV_ROUND_CLOSEST(rgmii_tx_delay, rgmii_delay_unit);
+	if (tx_delay_index >= 32) {
+		dev_err(dev, "The %u ps of TX delay is out of range\n",
+			rgmii_tx_delay);
+		return -EINVAL;
+	}
+
+	rx_delay_index = DIV_ROUND_CLOSEST(rgmii_rx_delay, rgmii_delay_unit);
+	if (rx_delay_index >= 32) {
+		dev_err(dev, "The %u ps of RX delay is out of range\n",
+			rgmii_rx_delay);
+		return -EINVAL;
+	}
+
+	/* Due to the hardware design reason, for MAC2/3 on AST2600, the zero
+	 * delay ns on RX is configured by setting value 0x1a.
+	 * List as below:
+	 * 0x1a -> 0   ns, 0x1b -> 0.25 ns, ... , 0x1f -> 1.25 ns,
+	 * 0x00 -> 1.5 ns, 0x01 -> 1.75 ns, ... , 0x19 -> 7.75 ns, 0x1a -> 0 ns
+	 */
+	if (mac_id == 2 || mac_id == 3)
+		rx_delay_index = (AST2600_MAC23_RX_DLY_0_NS + rx_delay_index) &
+				 AST2600_MAC_TX_RX_DLY_MASK;
+
+	if (mac_id == 0 || mac_id == 2) {
+		dly_mask = ASPEED_MAC0_2_TX_DLY | ASPEED_MAC0_2_RX_DLY;
+		tx_delay_index = FIELD_PREP(ASPEED_MAC0_2_TX_DLY, tx_delay_index);
+		rx_delay_index = FIELD_PREP(ASPEED_MAC0_2_RX_DLY, rx_delay_index);
+	} else {
+		dly_mask = ASPEED_MAC1_3_TX_DLY | ASPEED_MAC1_3_RX_DLY;
+		tx_delay_index = FIELD_PREP(ASPEED_MAC1_3_TX_DLY, tx_delay_index);
+		rx_delay_index = FIELD_PREP(ASPEED_MAC1_3_RX_DLY, rx_delay_index);
+	}
+
+	regmap_update_bits(scu, dly_reg, dly_mask, tx_delay_index | rx_delay_index);
+
+	return 0;
+}
+
+static int ftgmac100_config_rgmii_delay(struct ftgmac100 *priv)
+{
+	struct device_node *np = priv->dev->of_node;
+	phy_interface_t phy_intf;
+	u32 rgmii_tx_delay;
+	u32 rgmii_rx_delay;
+	int err = 0;
+
+	/* Because some old dts using NC-SI mode does not include phy-mode
+	 * property, here need to skip RGMII delay configuration and prevent
+	 * of_get_phy_mode() from returning error.
+	 */
+	if (of_get_property(np, "use-ncsi", NULL))
+		return 0;
+
+	err = of_get_phy_mode(np, &phy_intf);
+	if (err) {
+		dev_err(priv->dev, "Failed to get phy mode: %d\n", err);
+		return err;
+	}
+
+	/* RMII does not need to configure RGMII delay */
+	if (!phy_interface_mode_is_rgmii(phy_intf))
+		return 0;
+
+	if (of_property_read_u32(np, "tx-internal-delay-ps", &rgmii_tx_delay))
+		rgmii_tx_delay = 0;
+	if (of_property_read_u32(np, "rx-internal-delay-ps", &rgmii_rx_delay))
+		rgmii_rx_delay = 0;
+
+	if (priv->mac_id == FTGMAC100_AST2600)
+		err = ftgmac100_set_ast2600_rgmii_delay(priv,
+							rgmii_tx_delay,
+							rgmii_rx_delay,
+							phy_intf);
+
+	return err;
+}
+
 static int ftgmac100_probe(struct platform_device *pdev)
 {
 	const struct ftgmac100_match_data *match_data;
 	enum ftgmac100_mac_id mac_id;
+	bool rgmii_delay_conf;
 	struct resource *res;
 	int irq;
 	struct net_device *netdev;
@@ -1936,8 +2078,10 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		if (!match_data)
 			return -EINVAL;
 		mac_id = match_data->mac_id;
+		rgmii_delay_conf = match_data->rgmii_delay_conf;
 	} else {
 		mac_id = FTGMAC100_FARADAY;
+		rgmii_delay_conf = false;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2006,6 +2150,12 @@ static int ftgmac100_probe(struct platform_device *pdev)
 		priv->txdes0_edotr_mask = BIT(15);
 	}
 
+	if (rgmii_delay_conf) {
+		err = ftgmac100_config_rgmii_delay(priv);
+		if (err)
+			return err;
+	}
+
 	if (priv->mac_id == FTGMAC100_FARADAY ||
 	    priv->mac_id == FTGMAC100_AST2400 ||
 	    priv->mac_id == FTGMAC100_AST2500) {
@@ -2100,19 +2250,23 @@ static void ftgmac100_remove(struct platform_device *pdev)
 }
 
 static const struct ftgmac100_match_data ftgmac100_match_data_ast2400 = {
-	.mac_id = FTGMAC100_AST2400
+	.mac_id = FTGMAC100_AST2400,
+	.rgmii_delay_conf = false
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_ast2500 = {
-	.mac_id = FTGMAC100_AST2500
+	.mac_id = FTGMAC100_AST2500,
+	.rgmii_delay_conf = false
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_ast2600 = {
-	.mac_id = FTGMAC100_AST2600
+	.mac_id = FTGMAC100_AST2600,
+	.rgmii_delay_conf = true
 };
 
 static const struct ftgmac100_match_data ftgmac100_match_data_faraday = {
-	.mac_id = FTGMAC100_FARADAY
+	.mac_id = FTGMAC100_FARADAY,
+	.rgmii_delay_conf = false
 };
 
 static const struct of_device_id ftgmac100_of_match[] = {
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index 4968f6f0bdbc..d19d44d1b8e0 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -271,4 +271,24 @@ struct ftgmac100_rxdes {
 #define FTGMAC100_RXDES1_UDP_CHKSUM_ERR	(1 << 26)
 #define FTGMAC100_RXDES1_IP_CHKSUM_ERR	(1 << 27)
 
+/* Aspeed SCU */
+#define AST2600_MAC01_CLK_DLY	0x340
+#define AST2600_MAC23_CLK_DLY	0x350
+#define AST2600_MAC01_CLK_DLY_UNIT	45	/* ps */
+#define AST2600_MAC01_TX_DLY_0_NS	0
+#define AST2600_MAC01_RX_DLY_0_NS	0
+#define AST2600_MAC23_CLK_DLY_UNIT	250	/* ps */
+#define AST2600_MAC23_TX_DLY_0_NS	0
+#define AST2600_MAC23_RX_DLY_0_NS	0x1a
+#define AST2600_MAC_TX_RX_DLY_MASK	0x1f
+#define ASPEED_MAC0_2_TX_DLY		GENMASK(5, 0)
+#define ASPEED_MAC0_2_RX_DLY		GENMASK(17, 12)
+#define ASPEED_MAC1_3_TX_DLY		GENMASK(11, 6)
+#define ASPEED_MAC1_3_RX_DLY		GENMASK(23, 18)
+
+#define AST2600_MAC0_BASE_ADDR		0x1e660000
+#define AST2600_MAC1_BASE_ADDR		0x1e680000
+#define AST2600_MAC2_BASE_ADDR		0x1e670000
+#define AST2600_MAC3_BASE_ADDR		0x1e690000
+
 #endif /* __FTGMAC100_H */

-- 
2.34.1

[PATCH net-next v6 4/5] net: ftgmac100: Support rgmii delay in old dts with AST2600

[Jacky Chou]

Strating to this patch, driver will remind the legacy dts to update the
"phy-mode" to "rgmii-id, and if necessary, add small
"rx-internal-delay-ps" and "tx-internal-delay-ps.
If lack the two properties, driver will accord to the original delay
value from bootloader to disable RGMII delay and to change the phy
interface to phy driver.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 190 +++++++++++++++++++++++++++----
 drivers/net/ethernet/faraday/ftgmac100.h |   5 +
 2 files changed, 174 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 0b2a0bb8a4a9..5f5b9199a9ef 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1871,10 +1871,40 @@ static int ftgmac100_probe_ncsi(struct net_device *netdev,
 	return err;
 }
 
+static struct phy_device *ftgmac100_ast2600_phy_get(struct net_device *dev,
+						    struct device_node *np,
+						    void (*hndlr)(struct net_device *),
+						    phy_interface_t phy_intf)
+{
+	struct device_node *phy_np;
+	struct phy_device *phy;
+	int ret;
+
+	if (of_phy_is_fixed_link(np)) {
+		ret = of_phy_register_fixed_link(np);
+		if (ret < 0) {
+			netdev_err(dev, "broken fixed-link specification\n");
+			return NULL;
+		}
+		phy_np = of_node_get(np);
+	} else {
+		phy_np = of_parse_phandle(np, "phy-handle", 0);
+		if (!phy_np)
+			return NULL;
+	}
+
+	phy = of_phy_connect(dev, phy_np, hndlr, 0, phy_intf);
+
+	of_node_put(phy_np);
+
+	return phy;
+}
+
 static int ftgmac100_probe_dt(struct net_device *netdev,
 			      struct platform_device *pdev,
 			      struct ftgmac100 *priv,
-			      struct device_node *np)
+			      struct device_node *np,
+			      phy_interface_t phy_intf)
 {
 	struct phy_device *phy;
 	int err;
@@ -1890,8 +1920,16 @@ static int ftgmac100_probe_dt(struct net_device *netdev,
 		 * them. 2600 has an independent MDIO controller, not
 		 * part of the MAC.
 		 */
-		phy = of_phy_get_and_connect(priv->netdev, np,
-					     &ftgmac100_adjust_link);
+		if (priv->mac_id == FTGMAC100_AST2600)
+			/* Because AST2600 will use the RGMII delay to determine
+			 * which phy interface to use.
+			 */
+			phy = ftgmac100_ast2600_phy_get(priv->netdev, np,
+							&ftgmac100_adjust_link,
+							phy_intf);
+		else
+			phy = of_phy_get_and_connect(priv->netdev, np,
+						     &ftgmac100_adjust_link);
 		if (!phy) {
 			dev_err(&pdev->dev, "Failed to connect to phy\n");
 			return -EINVAL;
@@ -1923,10 +1961,62 @@ static int ftgmac100_probe_dt(struct net_device *netdev,
 	return 0;
 }
 
+static int ftgmac100_get_ast2600_rgmii_flag(u32 delay)
+{
+	if ((delay > 500 && delay < 1500) ||
+	    (delay > 2500 && delay < 7500))
+		return AST2600_RGMII_KEEP_DELAY;
+
+	return AST2600_RGMII_DIS_DELAY;
+}
+
+static int ftgmac100_check_ast2600_rgmii_delay(struct regmap *scu,
+					       u32 delay_unit,
+					       int mac_id, int dly_reg)
+{
+	u32 delay_value;
+	u32 tx_delay;
+	u32 rx_delay;
+	int tx_flag;
+	int rx_flag;
+
+	regmap_read(scu, dly_reg, &delay_value);
+	if (mac_id == 0 || mac_id == 2) {
+		tx_delay = FIELD_GET(ASPEED_MAC0_2_TX_DLY, delay_value);
+		rx_delay = FIELD_GET(ASPEED_MAC0_2_RX_DLY, delay_value);
+	} else {
+		tx_delay = FIELD_GET(ASPEED_MAC1_3_TX_DLY, delay_value);
+		rx_delay = FIELD_GET(ASPEED_MAC1_3_RX_DLY, delay_value);
+	}
+
+	/* Due to the hardware design reason, for MAC2/3 on AST2600,
+	 * the zero delay ns on RX is configured by setting value 0x1a.
+	 * List as below:
+	 * 0x1a, 0x1b, ... , 0x1f, 0x00, 0x01, ... , 0x19
+	 * Covert for calculation purpose.
+	 * 0x00, 0x01, ... , 0x19, 0x1a, 0x1b, ... , 0x1f
+	 */
+	if (mac_id == 2 || mac_id == 3)
+		rx_delay = (rx_delay + 0x06) & 0x1f;
+
+	tx_delay *= delay_unit;
+	rx_delay *= delay_unit;
+
+	tx_flag = ftgmac100_get_ast2600_rgmii_flag(tx_delay);
+	rx_flag = ftgmac100_get_ast2600_rgmii_flag(rx_delay);
+
+	if (tx_flag == AST2600_RGMII_KEEP_DELAY ||
+	    rx_flag == AST2600_RGMII_KEEP_DELAY) {
+		return AST2600_RGMII_KEEP_DELAY;
+	}
+
+	return AST2600_RGMII_DIS_DELAY;
+}
+
 static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
-					     u32 rgmii_tx_delay,
-					     u32 rgmii_rx_delay,
-					     phy_interface_t phy_intf)
+					     s32 rgmii_tx_delay,
+					     s32 rgmii_rx_delay,
+					     phy_interface_t *phy_intf)
 {
 	struct device *dev = priv->dev;
 	struct device_node *np;
@@ -1975,13 +2065,59 @@ static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
 		return -EINVAL;
 	}
 
-	/* Please refer to ethernet-controller.yaml. */
-	if (phy_intf == PHY_INTERFACE_MODE_RGMII &&
-	    (rgmii_tx_delay == 2000 || rgmii_rx_delay == 2000)) {
-		dev_warn(dev, "RX/TX delay cannot set to 2000 on 'rgmii'\n");
-		return -EINVAL;
+	if (of_phy_is_fixed_link(np)) {
+		if (rgmii_tx_delay < 0 || rgmii_rx_delay < 0) {
+			dev_err(dev,
+				"Add rx/tx-internal-delay-ps for fixed-link\n");
+			/* Keep original RGMII delay value*/
+			return 0;
+		}
+
+		/* Must have both of rx/tx-internal-delay-ps for fixed-link */
+		goto conf_delay;
+	}
+
+	if (*phy_intf == PHY_INTERFACE_MODE_RGMII_RXID ||
+	    *phy_intf == PHY_INTERFACE_MODE_RGMII_TXID)
+		goto out_warn;
+
+	/* Both rx/tx-internal-delay-ps are not existed. */
+	if (rgmii_tx_delay < 0 && rgmii_rx_delay < 0) {
+		int flag;
+
+		flag = ftgmac100_check_ast2600_rgmii_delay(scu,
+							   rgmii_delay_unit,
+							   mac_id,
+							   dly_reg);
+		if (flag == AST2600_RGMII_KEEP_DELAY)
+			goto out_warn;
+
+		if (*phy_intf == PHY_INTERFACE_MODE_RGMII) {
+			dev_err(dev, "Update phy-mode to 'rgmii-id'\n");
+			/* Forced phy interface to RGMII_ID and MAC will disable
+			 * RGMII delay.
+			 */
+			*phy_intf = PHY_INTERFACE_MODE_RGMII_ID;
+		}
+	} else {
+		/* Please refer to ethernet-controller.yaml. */
+		if (*phy_intf == PHY_INTERFACE_MODE_RGMII &&
+		    (rgmii_tx_delay == 2000 || rgmii_rx_delay == 2000)) {
+			dev_warn(dev,
+				 "RX/TX delay cannot set to 2000 on 'rgmii'\n");
+			return -EINVAL;
+		}
 	}
 
+	/* The value is negative, which means the rx/tx-internal-delay-ps
+	 * property is not existed in dts. Therefore, set to default 0.
+	 */
+	if (rgmii_tx_delay < 0)
+		rgmii_tx_delay = 0;
+	if (rgmii_rx_delay < 0)
+		rgmii_rx_delay = 0;
+
+conf_delay:
 	tx_delay_index = DIV_ROUND_CLOSEST(rgmii_tx_delay, rgmii_delay_unit);
 	if (tx_delay_index >= 32) {
 		dev_err(dev, "The %u ps of TX delay is out of range\n",
@@ -2018,15 +2154,21 @@ static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
 
 	regmap_update_bits(scu, dly_reg, dly_mask, tx_delay_index | rx_delay_index);
 
+	return 0;
+
+out_warn:
+	/* Print the warning message. Keep the phy-mode and the RGMII delay value. */
+	dev_warn(dev, "Update phy-mode to 'rgmii-id' and add rx/tx-internal-delay-ps\n");
+
 	return 0;
 }
 
-static int ftgmac100_config_rgmii_delay(struct ftgmac100 *priv)
+static int ftgmac100_config_rgmii_delay(struct ftgmac100 *priv,
+					phy_interface_t *phy_intf)
 {
 	struct device_node *np = priv->dev->of_node;
-	phy_interface_t phy_intf;
-	u32 rgmii_tx_delay;
-	u32 rgmii_rx_delay;
+	s32 rgmii_tx_delay;
+	s32 rgmii_rx_delay;
 	int err = 0;
 
 	/* Because some old dts using NC-SI mode does not include phy-mode
@@ -2036,20 +2178,23 @@ static int ftgmac100_config_rgmii_delay(struct ftgmac100 *priv)
 	if (of_get_property(np, "use-ncsi", NULL))
 		return 0;
 
-	err = of_get_phy_mode(np, &phy_intf);
+	err = of_get_phy_mode(np, phy_intf);
 	if (err) {
 		dev_err(priv->dev, "Failed to get phy mode: %d\n", err);
 		return err;
 	}
 
 	/* RMII does not need to configure RGMII delay */
-	if (!phy_interface_mode_is_rgmii(phy_intf))
+	if (!phy_interface_mode_is_rgmii(*phy_intf))
 		return 0;
 
+	/* AST2600 needs to know if the "tx/rx-internal-delay-ps" properties
+	 * are existed in dts. If not existed, set -1 and delay is equal to 0.
+	 */
 	if (of_property_read_u32(np, "tx-internal-delay-ps", &rgmii_tx_delay))
-		rgmii_tx_delay = 0;
+		rgmii_tx_delay = -1;
 	if (of_property_read_u32(np, "rx-internal-delay-ps", &rgmii_rx_delay))
-		rgmii_rx_delay = 0;
+		rgmii_rx_delay = -1;
 
 	if (priv->mac_id == FTGMAC100_AST2600)
 		err = ftgmac100_set_ast2600_rgmii_delay(priv,
@@ -2068,10 +2213,13 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	struct resource *res;
 	int irq;
 	struct net_device *netdev;
+	phy_interface_t phy_intf;
 	struct ftgmac100 *priv;
 	struct device_node *np;
 	int err = 0;
 
+	phy_intf = PHY_INTERFACE_MODE_NA;
+
 	np = pdev->dev.of_node;
 	if (np) {
 		match_data = of_device_get_match_data(&pdev->dev);
@@ -2151,7 +2299,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	}
 
 	if (rgmii_delay_conf) {
-		err = ftgmac100_config_rgmii_delay(priv);
+		err = ftgmac100_config_rgmii_delay(priv, &phy_intf);
 		if (err)
 			return err;
 	}
@@ -2165,7 +2313,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	}
 
 	if (np) {
-		err = ftgmac100_probe_dt(netdev, pdev, priv, np);
+		err = ftgmac100_probe_dt(netdev, pdev, priv, np, phy_intf);
 		if (err)
 			goto err;
 	}
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index d19d44d1b8e0..1b2f79a104ea 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -291,4 +291,9 @@ struct ftgmac100_rxdes {
 #define AST2600_MAC2_BASE_ADDR		0x1e670000
 #define AST2600_MAC3_BASE_ADDR		0x1e690000
 
+/* Keep original delay */
+#define AST2600_RGMII_KEEP_DELAY	0x01
+/* Need to disable delay on MAC side */
+#define AST2600_RGMII_DIS_DELAY		0x02
+
 #endif /* __FTGMAC100_H */

-- 
2.34.1

[PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Jacky Chou]

This change sets the rx-internal-delay-ps and tx-internal-delay-ps
properties to control the RGMII signal delay.
The phy-mode for MAC0–MAC3 is updated to "rgmii-id" to enable TX/RX
internal delay on the PHY and disable the corresponding delay
on the MAC.

Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
index 3f2ca9da0be2..a2a1c1dbb830 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
@@ -123,42 +123,54 @@ ethphy3: ethernet-phy@0 {
 &mac0 {
 	status = "okay";
 
-	phy-mode = "rgmii-rxid";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy0>;
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_rgmii1_default>;
+
+	rx-internal-delay-ps = <0>;
+	tx-internal-delay-ps = <0>;
 };
 
 
 &mac1 {
 	status = "okay";
 
-	phy-mode = "rgmii-rxid";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy1>;
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_rgmii2_default>;
+
+	rx-internal-delay-ps = <0>;
+	tx-internal-delay-ps = <0>;
 };
 
 &mac2 {
 	status = "okay";
 
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy2>;
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_rgmii3_default>;
+
+	rx-internal-delay-ps = <0>;
+	tx-internal-delay-ps = <0>;
 };
 
 &mac3 {
 	status = "okay";
 
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy3>;
 
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_rgmii4_default>;
+
+	rx-internal-delay-ps = <0>;
+	tx-internal-delay-ps = <0>;
 };
 
 &emmc_controller {

-- 
2.34.1

Re: [PATCH net-next v6 3/5] net: ftgmac100: Add RGMII delay support for AST2600

[Andrew Lunn]

On Mon, Mar 02, 2026 at 06:24:30PM +0800, Jacky Chou wrote:
> On the AST2600 platform, the RGMII delay is controlled via the
> SCU registers. The delay chain configuration differs between MAC0/1
> and MAC2/3, even though all four MACs use a 32-stage delay chain.
> +------+----------+-----------+-------------+-------------+
> |      |Delay Unit|Delay Stage|TX Edge Stage|RX Edge Stage|
> +------+----------+-----------+-------------+-------------+
> |MAC0/1|     45 ps|        32 |           0 |           0 |
> +------+----------+-----------+-------------+-------------+
> |MAC2/3|    250 ps|        32 |           0 |          26 |
> +------+----------+-----------+-------------+-------------+
> For MAC2/3, the "no delay" condition starts from stage 26.
> Setting the RX delay stage to 26 means that no additional RX
> delay is applied.
> Here lists the RX delay setting of MAC2/3 below.
> 26 -> 0   ns, 27 -> 0.25 ns, ... , 31 -> 1.25 ns,
> 0  -> 1.5 ns, 1  -> 1.75 ns, ... , 25 -> 7.75 ns
> 
> Therefore, we calculate the delay stage from the
> rx-internal-delay-ps of MAC2/3 to add 26. If the stage is equel
> to or bigger than 32, the delay stage will be mask 0x1f to get
> the correct setting.

Please return -EINVAL. Asking for more than 7.75ns is clearly broken.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v6 4/5] net: ftgmac100: Support rgmii delay in old dts with AST2600

[Andrew Lunn]

>  static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
> -					     u32 rgmii_tx_delay,
> -					     u32 rgmii_rx_delay,
> -					     phy_interface_t phy_intf)
> +					     s32 rgmii_tx_delay,
> +					     s32 rgmii_rx_delay,

You introduced these parameters in the previous patch. Please get the
types correct from the start.

	Andrew

Re: [PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Andrew Lunn]

On Mon, Mar 02, 2026 at 06:24:32PM +0800, Jacky Chou wrote:
> This change sets the rx-internal-delay-ps and tx-internal-delay-ps
> properties to control the RGMII signal delay.
> The phy-mode for MAC0–MAC3 is updated to "rgmii-id" to enable TX/RX
> internal delay on the PHY and disable the corresponding delay
> on the MAC.
> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> ---
>  arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> index 3f2ca9da0be2..a2a1c1dbb830 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> @@ -123,42 +123,54 @@ ethphy3: ethernet-phy@0 {
>  &mac0 {
>  	status = "okay";
>  
> -	phy-mode = "rgmii-rxid";
> +	phy-mode = "rgmii-id";
>  	phy-handle = <&ethphy0>;
>  
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_rgmii1_default>;
> +
> +	rx-internal-delay-ps = <0>;
> +	tx-internal-delay-ps = <0>;

In the binding, you said these default to 0. So you don't need them.

It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are
0?

What was the bootloader doing? This is worth a comment in the commit
messages.

	Andrew

Re: [PATCH net-next v6 0/5] Add AST2600 RGMII delay into ftgmac100

[Andrew Lunn]

On Mon, Mar 02, 2026 at 06:24:27PM +0800, Jacky Chou wrote:
> This patch series adds support for configuring RGMII internal delays for the
> Aspeed AST2600 FTGMAC100 Ethernet MACs. It introduces new compatible strings to
> distinguish between MAC0/1 and MAC2/3, as their delay chains and configuration
> units differ.
> The device tree bindings are updated to restrict the allowed phy-mode and delay
> properties for each MAC type. Corresponding changes are made to the device tree
> source files and the FTGMAC100 driver to support the new delay configuration.
> 
> This enables precise RGMII timing configuration for AST2600-based platforms,
> improving interoperability with various PHYs

Please add more about backwards compatibility. With just a quick look,
i'm not convinced it is. So it is missing text in the commit message
and probably comments in the code explaining how it works.

    Andrew

Re: [PATCH net-next v6 0/5] Add AST2600 RGMII delay into ftgmac100

[Andrew Lunn]

On Mon, Mar 02, 2026 at 06:24:27PM +0800, Jacky Chou wrote:
> This patch series adds support for configuring RGMII internal delays for the
> Aspeed AST2600 FTGMAC100 Ethernet MACs.

I've been thinking about this some more. And i would like to propose a
completely different solution.

What we are trying to achieve is allowing the correct phy-mode to be
used in DT, rgmii-id. Being able to specify internal delays is nice to
have, but for most platforms should not be needed. It is only needed
for badly designed boards where the designer of the PCB did not take
care with the length of the traces.

Part of the problem is that the MAC driver is not actually
"broken". It does what we recommend, read the phy-mode value from DT,
and pass it to the PHY. The real issue is that the bootloader enabled
delays in the MAC, behind the MAC drivers back. Because the MAC driver
is not "broken", it is hard to "fix" the issue in the MAC driver.

So lets solve this in the bootloader. I suggest you patch the
bootloader to no longer enable the delays in the MAC. It also needs to
patch the DT blob. If the blob says "rgmii", change it to
"rgmii-id". u-boot should be able to do this.

That should give backwards compatibility:

* Existing DT blobs with old bootloader gain the delays in the MAC.

* Existing DT blobs with the new bootloader gain delays in the PHY.

* For new boards being added to Linux, we NACK them with "rgmii", tell
  the developer to upgrade the bootloader, and use the correct
  rgmii-id. This is a power we do have.

* Developers of existing .dts files can submit patches to replace
  "rgmii" with "rgmii-id" once they are happy all platforms have had
  their bootloader upgraded. That might be never, since developers are
  lazy, and we cannot force this on them.

	Andrew

回覆: [PATCH net-next v6 0/5] Add AST2600 RGMII delay into ftgmac100

[Jacky Chou]

Hi Andrew,

Thank you for your reply.

> On Mon, Mar 02, 2026 at 06:24:27PM +0800, Jacky Chou wrote:
> > This patch series adds support for configuring RGMII internal delays
> > for the Aspeed AST2600 FTGMAC100 Ethernet MACs.
> 
> I've been thinking about this some more. And i would like to propose a
> completely different solution.
> 
> What we are trying to achieve is allowing the correct phy-mode to be used in
> DT, rgmii-id. Being able to specify internal delays is nice to have, but for most
> platforms should not be needed. It is only needed for badly designed boards
> where the designer of the PCB did not take care with the length of the traces.
> 
> Part of the problem is that the MAC driver is not actually "broken". It does
> what we recommend, read the phy-mode value from DT, and pass it to the PHY.
> The real issue is that the bootloader enabled delays in the MAC, behind the
> MAC drivers back. Because the MAC driver is not "broken", it is hard to "fix"
> the issue in the MAC driver.
> 
> So lets solve this in the bootloader. I suggest you patch the bootloader to no
> longer enable the delays in the MAC. It also needs to patch the DT blob. If the
> blob says "rgmii", change it to "rgmii-id". u-boot should be able to do this.
> 
> That should give backwards compatibility:
> 
> * Existing DT blobs with old bootloader gain the delays in the MAC.
> 
> * Existing DT blobs with the new bootloader gain delays in the PHY.
> 
> * For new boards being added to Linux, we NACK them with "rgmii", tell
>   the developer to upgrade the bootloader, and use the correct
>   rgmii-id. This is a power we do have.
> 
> * Developers of existing .dts files can submit patches to replace
>   "rgmii" with "rgmii-id" once they are happy all platforms have had
>   their bootloader upgraded. That might be never, since developers are
>   lazy, and we cannot force this on them.
> 

In the current ftgmac100 driver, there is no logic to configure MAC internal delay.
All MAC delay settings are configured in U-Boot, and those delay values are derived from the DT used in U-Boot.

As a result:
*The phy-mode in the Linux DT matches the one used in U-Boot.
*Existing AST2600 device trees — including Aspeed’s EVB DT — do not use "rgmii-id".

Before v6 of this series, we discussed introducing MAC internal delay configuration into the ftgmac100 driver.
However, this change would affect all existing DTs, because:
If rx-internal-delay-ns / tx-internal-delay-ns are not present, according to the YAML binding defaults,
MAC internal delay would be disabled.

This would unintentionally change the behavior of existing platforms.
Therefore, starting from v6, the approach evolved into:
* Checking the relationship between the current MAC delay and the phy-mode
* Deciding whether to:
 * Modify the MAC delay, or
 * Preserve the existing MAC delay setting

In fact, in our SDK (U-Boot):
The AST2600 EVB DT has already been updated to use "rgmii-id".
MAC internal delay is explicitly disabled (set to 0 ns).
However, existing deployed platforms will not update their MAC delay configuration simply because of these changes.

For AST2600, we need to maintain backward compatibility in the Linux ftgmac100 driver.
The behavior is:
For old DTs:
The driver determines whether the existing MAC delay should be preserved.
It may attempt to convert to "rgmii-id" and disable MAC internal delay only when safe.
However, if the old DT uses fixed-link, we must preserve the original MAC delay,
Because we cannot propagate phy-mode to modify the external device’s state.
For new DTs:
The driver configures MAC internal delay based on:
rx-internal-delay-ns
tx-internal-delay-ns
The driver then propagates the phy-mode to the PHY.

After completing AST2600 RGMII delay handling, we plan to patch the AST2700 platform.
For AST2700:
U-Boot will consistently use "rgmii-id".
MAC internal delay will be disabled in U-Boot.
Linux will follow the AST2600 "new DT" model:
* Configure MAC internal delay according to rx/tx-internal-delay-ns
* Propagate phy-mode to the PHY
This ensures the platform is implemented cleanly according to the discussed direction, without legacy compatibility constraints.

Thanks,
Jacky

回覆: [PATCH net-next v6 3/5] net: ftgmac100: Add RGMII delay support for AST2600

[Jacky Chou]

Hi Andrew,

Thank you for your reply.

> > On the AST2600 platform, the RGMII delay is controlled via the SCU
> > registers. The delay chain configuration differs between MAC0/1 and
> > MAC2/3, even though all four MACs use a 32-stage delay chain.
> > +------+----------+-----------+-------------+-------------+
> > |      |Delay Unit|Delay Stage|TX Edge Stage|RX Edge Stage|
> > +------+----------+-----------+-------------+-------------+
> > |MAC0/1|     45 ps|        32 |           0 |           0 |
> > +------+----------+-----------+-------------+-------------+
> > |MAC2/3|    250 ps|        32 |           0 |          26 |
> > +------+----------+-----------+-------------+-------------+
> > For MAC2/3, the "no delay" condition starts from stage 26.
> > Setting the RX delay stage to 26 means that no additional RX delay is
> > applied.
> > Here lists the RX delay setting of MAC2/3 below.
> > 26 -> 0   ns, 27 -> 0.25 ns, ... , 31 -> 1.25 ns,
> > 0  -> 1.5 ns, 1  -> 1.75 ns, ... , 25 -> 7.75 ns
> >
> > Therefore, we calculate the delay stage from the rx-internal-delay-ps
> > of MAC2/3 to add 26. If the stage is equel to or bigger than 32, the
> > delay stage will be mask 0x1f to get the correct setting.
> 
> Please return -EINVAL. Asking for more than 7.75ns is clearly broken.

...
	tx_delay_index = DIV_ROUND_CLOSEST(rgmii_tx_delay, rgmii_delay_unit);
	if (tx_delay_index >= 32) {
		dev_err(dev, "The %u ps of TX delay is out of range\n",
			rgmii_tx_delay);
		return -EINVAL;
	}

	rx_delay_index = DIV_ROUND_CLOSEST(rgmii_rx_delay, rgmii_delay_unit);
	if (rx_delay_index >= 32) {
		dev_err(dev, "The %u ps of RX delay is out of range\n",
			rgmii_rx_delay);
		return -EINVAL;
	}
...

These codes will calculate the ns delay to MAC delay index.
(rgmii_delay_unit is 250 ps on MAC2/3)
If set tx delay to 8 ns on MAC2 or MAC3, it will get index is 32.
It is over the delay configuration range, here will return -EINVAL.

Thanks,
Jacky

回覆: [PATCH net-next v6 4/5] net: ftgmac100: Support rgmii delay in old dts with AST2600

[Jacky Chou]

Hi Andrew,

Thank you for your reply.

> >  static int ftgmac100_set_ast2600_rgmii_delay(struct ftgmac100 *priv,
> > -					     u32 rgmii_tx_delay,
> > -					     u32 rgmii_rx_delay,
> > -					     phy_interface_t phy_intf)
> > +					     s32 rgmii_tx_delay,
> > +					     s32 rgmii_rx_delay,
> 
> You introduced these parameters in the previous patch. Please get the types
> correct from the start.

Agreed.
I will adjust in next version.

Thanks,
Jacky

Re: 回覆: [PATCH net-next v6 3/5] net: ftgmac100: Add RGMII delay support for AST2600

[Andrew Lunn]

> > > Therefore, we calculate the delay stage from the rx-internal-delay-ps
> > > of MAC2/3 to add 26. If the stage is equel to or bigger than 32, the
> > > delay stage will be mask 0x1f to get the correct setting.
> > 
> > Please return -EINVAL. Asking for more than 7.75ns is clearly broken.
> 
> ...
> 	tx_delay_index = DIV_ROUND_CLOSEST(rgmii_tx_delay, rgmii_delay_unit);
> 	if (tx_delay_index >= 32) {
> 		dev_err(dev, "The %u ps of TX delay is out of range\n",
> 			rgmii_tx_delay);
> 		return -EINVAL;
> 	}
> 
> 	rx_delay_index = DIV_ROUND_CLOSEST(rgmii_rx_delay, rgmii_delay_unit);
> 	if (rx_delay_index >= 32) {
> 		dev_err(dev, "The %u ps of RX delay is out of range\n",
> 			rgmii_rx_delay);
> 		return -EINVAL;
> 	}
> ...
> 
> These codes will calculate the ns delay to MAC delay index.
> (rgmii_delay_unit is 250 ps on MAC2/3)
> If set tx delay to 8 ns on MAC2 or MAC3, it will get index is 32.
> It is over the delay configuration range, here will return -EINVAL.

So is the comment wrong? No masking is actually done?

   Andrew

[PATCH net-next v6 3/5] net: ftgmac100: Add RGMII delay support for AST2600

[Jacky Chou]

> > > > Therefore, we calculate the delay stage from the
> > > > rx-internal-delay-ps of MAC2/3 to add 26. If the stage is equel to
> > > > or bigger than 32, the delay stage will be mask 0x1f to get the correct
> setting.
> > >
> > > Please return -EINVAL. Asking for more than 7.75ns is clearly broken.
> >
> > ...
> > 	tx_delay_index = DIV_ROUND_CLOSEST(rgmii_tx_delay,
> rgmii_delay_unit);
> > 	if (tx_delay_index >= 32) {
> > 		dev_err(dev, "The %u ps of TX delay is out of range\n",
> > 			rgmii_tx_delay);
> > 		return -EINVAL;
> > 	}
> >
> > 	rx_delay_index = DIV_ROUND_CLOSEST(rgmii_rx_delay,
> rgmii_delay_unit);
> > 	if (rx_delay_index >= 32) {
> > 		dev_err(dev, "The %u ps of RX delay is out of range\n",
> > 			rgmii_rx_delay);
> > 		return -EINVAL;
> > 	}
> > ...
> >
> > These codes will calculate the ns delay to MAC delay index.
> > (rgmii_delay_unit is 250 ps on MAC2/3) If set tx delay to 8 ns on MAC2
> > or MAC3, it will get index is 32.
> > It is over the delay configuration range, here will return -EINVAL.
> 
> So is the comment wrong? No masking is actually done?
> 

Yes. I will update the comment in next version.
Thank you for reminding me.

Thanks,
Jacky

[PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Jacky Chou]

Hi Andrew,

Thank you for yor reply.

> On Mon, Mar 02, 2026 at 06:24:32PM +0800, Jacky Chou wrote:
> > This change sets the rx-internal-delay-ps and tx-internal-delay-ps
> > properties to control the RGMII signal delay.
> > The phy-mode for MAC0–MAC3 is updated to "rgmii-id" to enable TX/RX
> > internal delay on the PHY and disable the corresponding delay on the
> > MAC.
> >
> > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> > ---
> >  arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts | 20
> > ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > index 3f2ca9da0be2..a2a1c1dbb830 100644
> > --- a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > @@ -123,42 +123,54 @@ ethphy3: ethernet-phy@0 {
> >  &mac0 {
> >  	status = "okay";
> >
> > -	phy-mode = "rgmii-rxid";
> > +	phy-mode = "rgmii-id";
> >  	phy-handle = <&ethphy0>;
> >
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&pinctrl_rgmii1_default>;
> > +
> > +	rx-internal-delay-ps = <0>;
> > +	tx-internal-delay-ps = <0>;
> 
> In the binding, you said these default to 0. So you don't need them.
> 
> It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are 0?
> 
> What was the bootloader doing? This is worth a comment in the commit
> messages.
> 

Before this patch, aspeed-ast2600-evb.dts is an existed old dts in mainline kernel.
In this series, ftgmac100 for AST2600 will configure the MAC RGMII internal delay
via SCU register, so this patch is changing this dts as a NEW dts for driver to configure
RGMII delay from the properties of MAC nodes.

Old dts: generally, leak tx/rx-internal-delay-ps -> Calculate the RGMII delay that is configured
from bootloader and decide whether keep the original value

New dts: In AST2600, we expect the MAC node includes the rx/tx-internal-delay-ns properties
and the driver directly uses these properties to configure RGMII delay.

We hope this series can keep the old dts works fine and push the new mechanism for the
next generation, AST2700. So, in AST2700, if the rx/tx-internal-delay-ps are ZERO, it is 
unnecessary including these properties, according to the binding these default are ZERO.

Thanks,
Jacky

Re: [PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Andrew Lunn]

On Thu, Mar 05, 2026 at 05:05:03AM +0000, Jacky Chou wrote:
> Hi Andrew,
> 
> Thank you for yor reply.
> 
> > On Mon, Mar 02, 2026 at 06:24:32PM +0800, Jacky Chou wrote:
> > > This change sets the rx-internal-delay-ps and tx-internal-delay-ps
> > > properties to control the RGMII signal delay.
> > > The phy-mode for MAC0–MAC3 is updated to "rgmii-id" to enable TX/RX
> > > internal delay on the PHY and disable the corresponding delay on the
> > > MAC.
> > >
> > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> > > ---
> > >  arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts | 20
> > > ++++++++++++++++----
> > >  1 file changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > > b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > > index 3f2ca9da0be2..a2a1c1dbb830 100644
> > > --- a/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > > +++ b/arch/arm/boot/dts/aspeed/aspeed-ast2600-evb.dts
> > > @@ -123,42 +123,54 @@ ethphy3: ethernet-phy@0 {
> > >  &mac0 {
> > >  	status = "okay";
> > >
> > > -	phy-mode = "rgmii-rxid";
> > > +	phy-mode = "rgmii-id";
> > >  	phy-handle = <&ethphy0>;
> > >
> > >  	pinctrl-names = "default";
> > >  	pinctrl-0 = <&pinctrl_rgmii1_default>;
> > > +
> > > +	rx-internal-delay-ps = <0>;
> > > +	tx-internal-delay-ps = <0>;
> > 
> > In the binding, you said these default to 0. So you don't need them.
> > 
> > It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are 0?
> > 
> > What was the bootloader doing? This is worth a comment in the commit
> > messages.
> > 
> 
> Before this patch, aspeed-ast2600-evb.dts is an existed old dts in mainline kernel.
> In this series, ftgmac100 for AST2600 will configure the MAC RGMII internal delay
> via SCU register, so this patch is changing this dts as a NEW dts for driver to configure
> RGMII delay from the properties of MAC nodes.
> 
> Old dts: generally, leak tx/rx-internal-delay-ps -> Calculate the RGMII delay that is configured
> from bootloader and decide whether keep the original value
> 
> New dts: In AST2600, we expect the MAC node includes the rx/tx-internal-delay-ns properties
> and the driver directly uses these properties to configure RGMII delay.

You did not answer my question...

> It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are 0?

I could understand "rgmii" becoming "rgmii-id" with both delays being
0. But that is not what you have here. It is starting as
"rgmii-rxid". So the PHY is being asked to insert the RX delay, but
not the TX delay. Where is the TX delay coming from?

I assume the MAC? Has the bootloader configured the MAC to insert the
TX delay? But look at all the board vendors who have been submitting
DT patches, and i've been rejecting them. They all seem to use rgmii,
not rgmii-rxid.

Why is this different to all the other boards? This is what i would
like to see in the commit message, an explanation of this oddness.

> We hope this series can keep the old dts works fine

Hope is not sufficient. Backwards compatibility is required. You need
to convince the reviewer the code changes are backwards
compatible. You can use the commit messages and comments in the code
to explain how backwards compatibility is maintained, while adding
this new functionality.

And once you have convinced me, i will probably ask you to post the
changes to the BMC mailing list, and ask for a few board vendors to
test the patches with old DT blobs, current DT blobs and updated DT
blobs.

And this is why i think fixing the issue in the bootloader is
better. It is much easier to convince a reviewer the changes are
backwards compatible.

	Andrew

[PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Jacky Chou]

> > > > -	phy-mode = "rgmii-rxid";
> > > > +	phy-mode = "rgmii-id";
> > > >  	phy-handle = <&ethphy0>;
> > > >
> > > >  	pinctrl-names = "default";
> > > >  	pinctrl-0 = <&pinctrl_rgmii1_default>;
> > > > +
> > > > +	rx-internal-delay-ps = <0>;
> > > > +	tx-internal-delay-ps = <0>;
> > >
> > > In the binding, you said these default to 0. So you don't need them.
> > >
> > > It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are 0?
> > >
> > > What was the bootloader doing? This is worth a comment in the commit
> > > messages.
> > >
> >
> > Before this patch, aspeed-ast2600-evb.dts is an existed old dts in mainline
> kernel.
> > In this series, ftgmac100 for AST2600 will configure the MAC RGMII
> > internal delay via SCU register, so this patch is changing this dts as
> > a NEW dts for driver to configure RGMII delay from the properties of MAC
> nodes.
> >
> > Old dts: generally, leak tx/rx-internal-delay-ps -> Calculate the
> > RGMII delay that is configured from bootloader and decide whether keep
> > the original value
> >
> > New dts: In AST2600, we expect the MAC node includes the
> > rx/tx-internal-delay-ns properties and the driver directly uses these
> properties to configure RGMII delay.
> 
> You did not answer my question...
> 
> > It is also odd that rgmii-rxid becomes rmgii-id, yet both delays are 0?
> 
> I could understand "rgmii" becoming "rgmii-id" with both delays being 0. But
> that is not what you have here. It is starting as "rgmii-rxid". So the PHY is being
> asked to insert the RX delay, but not the TX delay. Where is the TX delay
> coming from?
> 
> I assume the MAC? Has the bootloader configured the MAC to insert the TX
> delay? But look at all the board vendors who have been submitting DT patches,
> and i've been rejecting them. They all seem to use rgmii, not rgmii-rxid.
> 
> Why is this different to all the other boards? This is what i would like to see in
> the commit message, an explanation of this oddness.
> 
> > We hope this series can keep the old dts works fine
> 
> Hope is not sufficient. Backwards compatibility is required. You need to
> convince the reviewer the code changes are backwards compatible. You can
> use the commit messages and comments in the code to explain how
> backwards compatibility is maintained, while adding this new functionality.
> 
> And once you have convinced me, i will probably ask you to post the changes to
> the BMC mailing list, and ask for a few board vendors to test the patches with
> old DT blobs, current DT blobs and updated DT blobs.
> 
> And this is why i think fixing the issue in the bootloader is better. It is much
> easier to convince a reviewer the changes are backwards compatible.

The current ftgmac100 driver does not support to configure the MAC RGMII delay and 
these tx/rx internal delay on MAC is configured in bootloader stage.
So, the phy-mode of MAC node in current aspeed-ast2600-evb.dts will use the same
from U-boot dts.

Could you please help confirm how we should proceed with this project?

1. Discard this series.
2. Fix the bootloader that we provide to our customers.
3. Send a patch to correct the phy-mode settings in aspeed-ast2600-evb.dts so that they 
match the bootloader after the fix.

mac0: "rgmii-rxid" -> "rgmii-id"
mac1: "rgmii-rxid" -> "rgmii-id"
mac2: "rgmii" -> "rgmii-id"
mac3: "rgmii" -> "rgmii-id"

We will also add a detailed comment in the patch to clarify why these phy-mode settings 
are changed and when the MAC RGMII internal delay should be configured.

Thanks,

Jacky

Re: [PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Andrew Lunn]

> Could you please help confirm how we should proceed with this project?

That is up to you.

If you do lots of small patches with good comments, you probably can
do it in Linux. But you need to convince reviewers you are not
breaking backwards compatibility.

I think the bootloader change is simpler. But i've not worked on
bootloaders for a long time.

> 3. Send a patch to correct the phy-mode settings in aspeed-ast2600-evb.dts so that they 
> match the bootloader after the fix.
> 
> mac0: "rgmii-rxid" -> "rgmii-id"
> mac1: "rgmii-rxid" -> "rgmii-id"
> mac2: "rgmii" -> "rgmii-id"
> mac3: "rgmii" -> "rgmii-id"

You will do this anyway. However, just so that backwards comparability
gets tested all the time, i would suggest you only change mac0 and
mac3. Leave mac1 and mac2 with old settings, and add comments that old
values are being used for testing, and they should not be cut/paste
into new .dts files. And add a comment that mac0 and mac3 require the
new bootloader, if you decide to solve it that way.

	Andrew

[PATCH net-next v6 5/5] ARM: dts: aspeed: ast2600-evb: Configure RGMII delay for MAC

[Jacky Chou]

> > Could you please help confirm how we should proceed with this project?
> 
> That is up to you.
> 
> If you do lots of small patches with good comments, you probably can do it in
> Linux. But you need to convince reviewers you are not breaking backwards
> compatibility.
> 
> I think the bootloader change is simpler. But i've not worked on bootloaders for
> a long time.
> 
> > 3. Send a patch to correct the phy-mode settings in
> > aspeed-ast2600-evb.dts so that they match the bootloader after the fix.
> >
> > mac0: "rgmii-rxid" -> "rgmii-id"
> > mac1: "rgmii-rxid" -> "rgmii-id"
> > mac2: "rgmii" -> "rgmii-id"
> > mac3: "rgmii" -> "rgmii-id"
> 
> You will do this anyway. However, just so that backwards comparability gets
> tested all the time, i would suggest you only change mac0 and mac3. Leave
> mac1 and mac2 with old settings, and add comments that old values are being
> used for testing, and they should not be cut/paste into new .dts files. And add a
> comment that mac0 and mac3 require the new bootloader, if you decide to
> solve it that way.
> 

Thank you for your patience and for the detailed guidance.

I will discuss the possible approaches internally with the team and decide on the 
direction for the fix. 
Thanks again for your feedback.

Thanks,
Jacky