[PATCH net v2 0/5] net: stmmac: eic7700: fix delay calculation and initialization ordering

[PATCH net v2 0/5] net: stmmac: eic7700: fix delay calculation and initialization ordering

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

v1 -> v2:
  - Update eswin,eic7700-eth.yaml:
    - Limit the binding changes to adding optional TXD and RXD delay register
      offsets in eswin,hsp-sp-csr.
    - Restore the original enum-based definitions for rx-internal-delay-ps
      and tx-internal-delay-ps.
    - Keep rx-internal-delay-ps and tx-internal-delay-ps as required
      properties.
    - Restore the original example content, with only the additional optional
      TXD and RXD delay register offsets.
    - Restore Acked-by from Conor Dooley for the binding change, which was
      temporarily omitted in v1 during series restructuring and has been
      reinstated now that the change is stable and properly isolated.

  - Update dwmac-eic7700.c:
    - Split driver changes into smaller patches based on review feedback to
      improve reviewability and bisectability.
    - Keep the existing requirement that rx-internal-delay-ps and
      tx-internal-delay-ps must be present in the device tree.
    - Treat TXD/RXD delay register offsets as optional and only program them
      when provided by device tree.
    - Remove the previously proposed fix_mac_speed logic.

  - Link to v1:
    https://lore.kernel.org/lkml/20260507083037.152-1-lizhi2@eswincomputing.com/

Zhi Li (5):
  dt-bindings: ethernet: eswin: add optional TXD and RXD delay register
    offsets
  net: stmmac: eswin: fix HSP CSR init ordering after clock enable
  net: stmmac: eswin: clear TXD and RXD delay registers during
    initialization
  net: stmmac: eswin: correct RGMII delay granularity to 20 ps
  net: stmmac: eswin: validate RGMII delay values

 .../bindings/net/eswin,eic7700-eth.yaml       |  13 +-
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 126 +++++++++++++-----
 2 files changed, 101 insertions(+), 38 deletions(-)

-- 
2.25.1

[PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

Document two optional cells in eswin,hsp-sp-csr for the TXD and RXD
delay control register offsets.

These registers are used by the driver to clear any residual delay
configuration left by the bootloader, ensuring that MAC-side RGMII delay
settings are applied solely according to the kernel configuration.

Add a reference to the EIC7700X SoC Technical Reference Manual for
background information about the HSP CSR block.

Fixes: 888bd0eca93c ("dt-bindings: ethernet: eswin: Document for EIC7700 SoC")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/net/eswin,eic7700-eth.yaml  | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
index 91e8cd1db67b..b66ae6300faf 100644
--- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
+++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
@@ -73,6 +73,15 @@ properties:
       HSP CSR is to control and get status of different high-speed peripherals
       (such as Ethernet, USB, SATA, etc.) via register, which can tune
       board-level's parameters of PHY, etc.
+
+      Additional background information about the High-Speed Subsystem
+      and the HSP CSR block is available in Chapter 10 ("High-Speed Interface")
+      of the EIC7700X SoC Technical Reference Manual, Part 4
+      (EIC7700X_SoC_Technical_Reference_Manual_Part4.pdf). The manual is
+      publicly available at
+      https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases
+
+      This reference is provided for background information only.
     $ref: /schemas/types.yaml#/definitions/phandle-array
     items:
       - items:
@@ -82,6 +91,8 @@ properties:
           - description: Offset of AXI clock controller Low-Power request
                          register
           - description: Offset of register controlling TX/RX clock delay
+          - description: Optional offset of register controlling TXD delay
+          - description: Optional offset of register controlling RXD delay
 
 required:
   - compatible
@@ -116,7 +127,7 @@ examples:
         reset-names = "stmmaceth";
         rx-internal-delay-ps = <200>;
         tx-internal-delay-ps = <200>;
-        eswin,hsp-sp-csr = <&hsp_sp_csr 0x100 0x108 0x118>;
+        eswin,hsp-sp-csr = <&hsp_sp_csr 0x100 0x108 0x118 0x114 0x11c>;
         snps,axi-config = <&stmmac_axi_setup>;
         snps,aal;
         snps,fixed-burst;
-- 
2.25.1

[PATCH net v2 2/5] net: stmmac: eswin: fix HSP CSR init ordering after clock enable

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

Fix the initialization ordering of the HSP CSR configuration in the
EIC7700 DWMAC glue driver.

The HSP CSR registers control MAC-side RGMII delay behavior and must
only be accessed after the corresponding clocks are enabled. The
previous implementation could trigger register access before clock
enablement, leading to undefined behavior depending on boot state.

Move the HSP CSR configuration into the post-clock-enable initialization
path to ensure all register accesses occur under valid clock domains.

This change ensures deterministic initialization and prevents
clock-dependent register access failures during probe or resume.

Fixes: ea77dbbdbc4e ("net: stmmac: add Eswin EIC7700 glue driver")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 73 +++++++++++--------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
index bcb8e000e720..63001c4acdb7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -42,6 +42,11 @@ static const char * const eic7700_clk_names[] = {
 
 struct eic7700_qos_priv {
 	struct plat_stmmacenet_data *plat_dat;
+	struct regmap *eic7700_hsp_regmap;
+	u32 eth_axi_lp_ctrl_offset;
+	u32 eth_phy_ctrl_offset;
+	u32 eth_clk_offset;
+	u32 eth_clk_dly_param;
 };
 
 static int eic7700_clks_config(void *priv, bool enabled)
@@ -61,8 +66,28 @@ static int eic7700_clks_config(void *priv, bool enabled)
 static int eic7700_dwmac_init(struct device *dev, void *priv)
 {
 	struct eic7700_qos_priv *dwc = priv;
+	int ret;
+
+	ret = eic7700_clks_config(dwc, true);
+	if (ret)
+		return ret;
+
+	ret = regmap_set_bits(dwc->eic7700_hsp_regmap,
+			      dwc->eth_phy_ctrl_offset,
+			      EIC7700_ETH_TX_CLK_SEL |
+			      EIC7700_ETH_PHY_INTF_SELI);
+	if (ret) {
+		eic7700_clks_config(dwc, false);
+		return ret;
+	}
+
+	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_axi_lp_ctrl_offset,
+		     EIC7700_ETH_CSYSREQ_VAL);
 
-	return eic7700_clks_config(dwc, true);
+	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset,
+		     dwc->eth_clk_dly_param);
+
+	return 0;
 }
 
 static void eic7700_dwmac_exit(struct device *dev, void *priv)
@@ -93,12 +118,6 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
 	struct eic7700_qos_priv *dwc_priv;
-	struct regmap *eic7700_hsp_regmap;
-	u32 eth_axi_lp_ctrl_offset;
-	u32 eth_phy_ctrl_offset;
-	u32 eth_phy_ctrl_regset;
-	u32 eth_rxd_dly_offset;
-	u32 eth_dly_param = 0;
 	u32 delay_ps;
 	int i, ret;
 
@@ -121,8 +140,9 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 				  "rx-internal-delay-ps", &delay_ps)) {
 		u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
 
-		eth_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
-		eth_dly_param |= FIELD_PREP(EIC7700_ETH_RX_ADJ_DELAY, val);
+		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
+		dwc_priv->eth_clk_dly_param |=
+				 FIELD_PREP(EIC7700_ETH_RX_ADJ_DELAY, val);
 	} else {
 		return dev_err_probe(&pdev->dev, -EINVAL,
 			"missing required property rx-internal-delay-ps\n");
@@ -133,53 +153,42 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 				  "tx-internal-delay-ps", &delay_ps)) {
 		u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
 
-		eth_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
-		eth_dly_param |= FIELD_PREP(EIC7700_ETH_TX_ADJ_DELAY, val);
+		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
+		dwc_priv->eth_clk_dly_param |=
+				 FIELD_PREP(EIC7700_ETH_TX_ADJ_DELAY, val);
 	} else {
 		return dev_err_probe(&pdev->dev, -EINVAL,
 			"missing required property tx-internal-delay-ps\n");
 	}
 
-	eic7700_hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
-							     "eswin,hsp-sp-csr");
-	if (IS_ERR(eic7700_hsp_regmap))
+	dwc_priv->eic7700_hsp_regmap =
+			syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							"eswin,hsp-sp-csr");
+	if (IS_ERR(dwc_priv->eic7700_hsp_regmap))
 		return dev_err_probe(&pdev->dev,
-				PTR_ERR(eic7700_hsp_regmap),
+				PTR_ERR(dwc_priv->eic7700_hsp_regmap),
 				"Failed to get hsp-sp-csr regmap\n");
 
 	ret = of_property_read_u32_index(pdev->dev.of_node,
 					 "eswin,hsp-sp-csr",
-					 1, &eth_phy_ctrl_offset);
+					 1, &dwc_priv->eth_phy_ctrl_offset);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "can't get eth_phy_ctrl_offset\n");
 
-	regmap_read(eic7700_hsp_regmap, eth_phy_ctrl_offset,
-		    &eth_phy_ctrl_regset);
-	eth_phy_ctrl_regset |=
-		(EIC7700_ETH_TX_CLK_SEL | EIC7700_ETH_PHY_INTF_SELI);
-	regmap_write(eic7700_hsp_regmap, eth_phy_ctrl_offset,
-		     eth_phy_ctrl_regset);
-
 	ret = of_property_read_u32_index(pdev->dev.of_node,
 					 "eswin,hsp-sp-csr",
-					 2, &eth_axi_lp_ctrl_offset);
+					 2, &dwc_priv->eth_axi_lp_ctrl_offset);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "can't get eth_axi_lp_ctrl_offset\n");
 
-	regmap_write(eic7700_hsp_regmap, eth_axi_lp_ctrl_offset,
-		     EIC7700_ETH_CSYSREQ_VAL);
-
 	ret = of_property_read_u32_index(pdev->dev.of_node,
 					 "eswin,hsp-sp-csr",
-					 3, &eth_rxd_dly_offset);
+					 3, &dwc_priv->eth_clk_offset);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
-				     "can't get eth_rxd_dly_offset\n");
-
-	regmap_write(eic7700_hsp_regmap, eth_rxd_dly_offset,
-		     eth_dly_param);
+				     "can't get eth_clk_offset\n");
 
 	plat_dat->num_clks = ARRAY_SIZE(eic7700_clk_names);
 	plat_dat->clks = devm_kcalloc(&pdev->dev,
-- 
2.25.1

[PATCH net v2 3/5] net: stmmac: eswin: clear TXD and RXD delay registers during initialization

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

Clear the TXD and RXD delay control registers during EIC7700 DWMAC
initialization.

These registers may retain values programmed by the bootloader. If left
unchanged, residual delays can alter the effective RGMII timing seen by
the MAC and override the configuration described by the device tree.

This may violate the expected RGMII timing model and can cause link
instability or prevent the Ethernet controller from operating correctly.

Explicitly clearing these registers ensures that the MAC delay settings
are determined solely by the kernel configuration.

The corresponding register offsets are optional, and the registers are
only cleared when the offsets are provided in the device tree.

Fixes: ea77dbbdbc4e ("net: stmmac: add Eswin EIC7700 glue driver")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
index 63001c4acdb7..541b279f08a1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -46,7 +46,11 @@ struct eic7700_qos_priv {
 	u32 eth_axi_lp_ctrl_offset;
 	u32 eth_phy_ctrl_offset;
 	u32 eth_clk_offset;
+	u32 eth_txd_offset;
+	u32 eth_rxd_offset;
 	u32 eth_clk_dly_param;
+	bool has_txd_offset;
+	bool has_rxd_offset;
 };
 
 static int eic7700_clks_config(void *priv, bool enabled)
@@ -84,6 +88,12 @@ static int eic7700_dwmac_init(struct device *dev, void *priv)
 	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_axi_lp_ctrl_offset,
 		     EIC7700_ETH_CSYSREQ_VAL);
 
+	if (dwc->has_txd_offset)
+		regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_txd_offset, 0);
+
+	if (dwc->has_rxd_offset)
+		regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_rxd_offset, 0);
+
 	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset,
 		     dwc->eth_clk_dly_param);
 
@@ -190,6 +200,18 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, ret,
 				     "can't get eth_clk_offset\n");
 
+	ret = of_property_read_u32_index(pdev->dev.of_node,
+					 "eswin,hsp-sp-csr",
+					 4, &dwc_priv->eth_txd_offset);
+	if (!ret)
+		dwc_priv->has_txd_offset = true;
+
+	ret = of_property_read_u32_index(pdev->dev.of_node,
+					 "eswin,hsp-sp-csr",
+					 5, &dwc_priv->eth_rxd_offset);
+	if (!ret)
+		dwc_priv->has_rxd_offset = true;
+
 	plat_dat->num_clks = ARRAY_SIZE(eic7700_clk_names);
 	plat_dat->clks = devm_kcalloc(&pdev->dev,
 				      plat_dat->num_clks,
-- 
2.25.1

[PATCH net v2 4/5] net: stmmac: eswin: correct RGMII delay granularity to 20 ps

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

The EIC7700 MAC implements programmable RGMII delay adjustment with a
granularity of 20 ps per hardware step.

The driver previously converted rx-internal-delay-ps and
tx-internal-delay-ps values using a 100 ps step size, resulting in
incorrect delay programming.

Update the conversion to use the correct 20 ps granularity so the
programmed delay matches the values described in the device tree.

Fixes: ea77dbbdbc4e ("net: stmmac: add Eswin EIC7700 glue driver")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
index 541b279f08a1..ef60cab24533 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -28,8 +28,8 @@
 
 /*
  * TX/RX Clock Delay Bit Masks:
- * - TX Delay: bits [14:8] — TX_CLK delay (unit: 0.1ns per bit)
- * - RX Delay: bits [30:24] — RX_CLK delay (unit: 0.1ns per bit)
+ * - TX Delay: bits [14:8] — TX_CLK delay (unit: 0.02ns per bit)
+ * - RX Delay: bits [30:24] — RX_CLK delay (unit: 0.02ns per bit)
  */
 #define EIC7700_ETH_TX_ADJ_DELAY	GENMASK(14, 8)
 #define EIC7700_ETH_RX_ADJ_DELAY	GENMASK(30, 24)
@@ -148,7 +148,7 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	/* Read rx-internal-delay-ps and update rx_clk delay */
 	if (!of_property_read_u32(pdev->dev.of_node,
 				  "rx-internal-delay-ps", &delay_ps)) {
-		u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
+		u32 val = min(delay_ps / 20, EIC7700_MAX_DELAY_UNIT);
 
 		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
 		dwc_priv->eth_clk_dly_param |=
@@ -161,7 +161,7 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	/* Read tx-internal-delay-ps and update tx_clk delay */
 	if (!of_property_read_u32(pdev->dev.of_node,
 				  "tx-internal-delay-ps", &delay_ps)) {
-		u32 val = min(delay_ps / 100, EIC7700_MAX_DELAY_UNIT);
+		u32 val = min(delay_ps / 20, EIC7700_MAX_DELAY_UNIT);
 
 		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
 		dwc_priv->eth_clk_dly_param |=
-- 
2.25.1

[PATCH net v2 5/5] net: stmmac: eswin: validate RGMII delay values

[lizhi2]

From: Zhi Li <lizhi2@eswincomputing.com>

Validate rx-internal-delay-ps and tx-internal-delay-ps against the
hardware capabilities of the EIC7700 MAC.

The programmable RGMII delay supports 20 ps steps and a maximum value of
2540 ps. The driver previously accepted arbitrary values and silently
truncated unsupported settings when converting them to hardware units.

As a result, invalid device tree values could lead to unexpected delay
programming and incorrect RGMII timing.

Reject delay values that are not multiples of 20 ps or exceed the
supported hardware range.

Fixes: ea77dbbdbc4e ("net: stmmac: add Eswin EIC7700 glue driver")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 29 ++++++++++++++++---
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
index ef60cab24533..4ac979d874d6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -34,7 +34,10 @@
 #define EIC7700_ETH_TX_ADJ_DELAY	GENMASK(14, 8)
 #define EIC7700_ETH_RX_ADJ_DELAY	GENMASK(30, 24)
 
-#define EIC7700_MAX_DELAY_UNIT 0x7F
+#define EIC7700_MAX_DELAY_STEPS		0x7F
+#define EIC7700_DELAY_STEP_PS		20
+#define EIC7700_MAX_DELAY_PS	\
+	(EIC7700_MAX_DELAY_STEPS * EIC7700_DELAY_STEP_PS)
 
 static const char * const eic7700_clk_names[] = {
 	"tx", "axi", "cfg",
@@ -128,7 +131,7 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
 	struct eic7700_qos_priv *dwc_priv;
-	u32 delay_ps;
+	u32 delay_ps, val;
 	int i, ret;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
@@ -148,7 +151,16 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	/* Read rx-internal-delay-ps and update rx_clk delay */
 	if (!of_property_read_u32(pdev->dev.of_node,
 				  "rx-internal-delay-ps", &delay_ps)) {
-		u32 val = min(delay_ps / 20, EIC7700_MAX_DELAY_UNIT);
+		if (delay_ps % EIC7700_DELAY_STEP_PS)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+				"rx delay must be multiple of %dps\n",
+				EIC7700_DELAY_STEP_PS);
+
+		if (delay_ps > EIC7700_MAX_DELAY_PS)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+				"rx delay out of range\n");
+
+		val = delay_ps / EIC7700_DELAY_STEP_PS;
 
 		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
 		dwc_priv->eth_clk_dly_param |=
@@ -161,7 +173,16 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	/* Read tx-internal-delay-ps and update tx_clk delay */
 	if (!of_property_read_u32(pdev->dev.of_node,
 				  "tx-internal-delay-ps", &delay_ps)) {
-		u32 val = min(delay_ps / 20, EIC7700_MAX_DELAY_UNIT);
+		if (delay_ps % EIC7700_DELAY_STEP_PS)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+				"tx delay must be multiple of %dps\n",
+				EIC7700_DELAY_STEP_PS);
+
+		if (delay_ps > EIC7700_MAX_DELAY_PS)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+				"tx delay out of range\n");
+
+		val = delay_ps / EIC7700_DELAY_STEP_PS;
 
 		dwc_priv->eth_clk_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
 		dwc_priv->eth_clk_dly_param |=
-- 
2.25.1

Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
--

commit 2992483df88af2823c275fc86187f7e38acbee5c
Author: Zhi Li <lizhi2@eswincomputing.com>

dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

This commit documents two optional cells in the eswin,hsp-sp-csr property
for the TXD and RXD delay control register offsets.

> diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml

[ ... ]

> @@ -82,6 +91,8 @@ properties:
>            - description: Offset of AXI clock controller Low-Power request
>                           register
>            - description: Offset of register controlling TX/RX clock delay
> +          - description: Optional offset of register controlling TXD delay
> +          - description: Optional offset of register controlling RXD delay

Will this make the two new offsets strictly required instead of optional?

In dt-schema, when an items list defines a tuple, minItems implicitly
defaults to the total number of items in that list. By increasing the list
from 4 to 6 items without adding a minItems: 4 constraint to the inner items
list, won't validation now strictly require exactly 6 items?

Could this cause dt_binding_check to fail for existing device trees that
correctly specify only the original 4 elements?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021919.404-1-lizhi2@eswincomputing.com?part=1

Re: [PATCH net v2 2/5] net: stmmac: eswin: fix HSP CSR init ordering after clock enable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware State Loss on Resume: The HSP CSR registers are not restored during system resume, contradicting the commit message.
--

commit 627f510ee556ff4322e0f4ed0b60fdd27337080f
Author: Zhi Li <lizhi2@eswincomputing.com>

net: stmmac: eswin: fix HSP CSR init ordering after clock enable

This commit moves the HSP CSR configuration in the EIC7700 DWMAC glue driver
into the post-clock-enable initialization path to ensure proper ordering.
The intent is to prevent register access before clock enablement and ensure
deterministic initialization during probe or resume.

>     This change ensures deterministic initialization and prevents
>     clock-dependent register access failures during probe or resume.

Are the HSP CSR registers actually restored during system resume?

The driver overrides the core resume callback by setting
plat_dat->resume = eic7700_dwmac_resume. Because a custom resume callback is
provided, the STMMAC core invokes this instead of the default
stmmac_plat_resume() (which would have called plat->init()).

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> index bcb8e000e720b..63001c4acdb7a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c

[ ... ]

> @@ -61,8 +66,28 @@ static int eic7700_clks_config(void *priv, bool enabled)
>  static int eic7700_dwmac_init(struct device *dev, void *priv)
>  {
>  	struct eic7700_qos_priv *dwc = priv;
> +	int ret;
> +
> +	ret = eic7700_clks_config(dwc, true);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(dwc->eic7700_hsp_regmap,
> +			      dwc->eth_phy_ctrl_offset,
> +			      EIC7700_ETH_TX_CLK_SEL |
> +			      EIC7700_ETH_PHY_INTF_SELI);
> +	if (ret) {
> +		eic7700_clks_config(dwc, false);
> +		return ret;
> +	}
> +
> +	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_axi_lp_ctrl_offset,
> +		     EIC7700_ETH_CSYSREQ_VAL);
>  
> -	return eic7700_clks_config(dwc, true);
> +	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset,
> +		     dwc->eth_clk_dly_param);
> +
> +	return 0;
>  }

Since eic7700_dwmac_resume() only calls pm_runtime_force_resume(dev) and
doesn't invoke eic7700_dwmac_init(), the HSP CSR configuration won't be
re-applied upon system resume. If the hardware loses state during suspend,
will the RGMII delays be permanently lost, leading to a MAC hang or silent
network failure?

Also, if we try to fix this by naively calling init() from resume(), would
it cause a clock reference leak? pm_runtime_force_resume() already
re-enables the clocks via the runtime PM callbacks, so the CSR configuration
might need to be factored out into a separate helper function.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021919.404-1-lizhi2@eswincomputing.com?part=2

Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On Tue, May 19, 2026 at 02:23:33AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
> --
> 
> commit 2992483df88af2823c275fc86187f7e38acbee5c
> Author: Zhi Li <lizhi2@eswincomputing.com>
> 
> dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> 
> This commit documents two optional cells in the eswin,hsp-sp-csr property
> for the TXD and RXD delay control register offsets.
> 
> > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> 
> [ ... ]
> 
> > @@ -82,6 +91,8 @@ properties:
> >            - description: Offset of AXI clock controller Low-Power request
> >                           register
> >            - description: Offset of register controlling TX/RX clock delay
> > +          - description: Optional offset of register controlling TXD delay
> > +          - description: Optional offset of register controlling RXD delay
> 
> Will this make the two new offsets strictly required instead of optional?
> 
> In dt-schema, when an items list defines a tuple, minItems implicitly
> defaults to the total number of items in that list. By increasing the list
> from 4 to 6 items without adding a minItems: 4 constraint to the inner items
> list, won't validation now strictly require exactly 6 items?
> 
> Could this cause dt_binding_check to fail for existing device trees that
> correctly specify only the original 4 elements?

Looks like a valid complaint to me.
pw-bot: changes-requested

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[李志]

> -----Original Messages-----
> From: "Conor Dooley" <conor@kernel.org>
> Send time:Tuesday, 19/05/2026 17:11:59
> To: sashiko-reviews@lists.linux.dev
> Cc: lizhi2@eswincomputing.com, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> Subject: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> 
> On Tue, May 19, 2026 at 02:23:33AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
> > --
> > 
> > commit 2992483df88af2823c275fc86187f7e38acbee5c
> > Author: Zhi Li <lizhi2@eswincomputing.com>
> > 
> > dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > 
> > This commit documents two optional cells in the eswin,hsp-sp-csr property
> > for the TXD and RXD delay control register offsets.
> > 
> > > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > 
> > [ ... ]
> > 
> > > @@ -82,6 +91,8 @@ properties:
> > >            - description: Offset of AXI clock controller Low-Power request
> > >                           register
> > >            - description: Offset of register controlling TX/RX clock delay
> > > +          - description: Optional offset of register controlling TXD delay
> > > +          - description: Optional offset of register controlling RXD delay
> > 
> > Will this make the two new offsets strictly required instead of optional?
> > 
> > In dt-schema, when an items list defines a tuple, minItems implicitly
> > defaults to the total number of items in that list. By increasing the list
> > from 4 to 6 items without adding a minItems: 4 constraint to the inner items
> > list, won't validation now strictly require exactly 6 items?
> > 
> > Could this cause dt_binding_check to fail for existing device trees that
> > correctly specify only the original 4 elements?
> 
> Looks like a valid complaint to me.
> pw-bot: changes-requested
> 
You're right, this is a valid issue.

I'll fix it in v3 by adding only minItems: 4 to preserve backward
compatibility with existing 4-cell device trees.

Should I drop your Acked-by in v3 due to the schema change, or should
it be retained?

Thanks.

Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2822 bytes --]

On Tue, May 19, 2026 at 05:51:05PM +0800, 李志 wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Conor Dooley" <conor@kernel.org>
> > Send time:Tuesday, 19/05/2026 17:11:59
> > To: sashiko-reviews@lists.linux.dev
> > Cc: lizhi2@eswincomputing.com, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> > Subject: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > 
> > On Tue, May 19, 2026 at 02:23:33AM +0000, sashiko-bot@kernel.org wrote:
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
> > > --
> > > 
> > > commit 2992483df88af2823c275fc86187f7e38acbee5c
> > > Author: Zhi Li <lizhi2@eswincomputing.com>
> > > 
> > > dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > > 
> > > This commit documents two optional cells in the eswin,hsp-sp-csr property
> > > for the TXD and RXD delay control register offsets.
> > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > 
> > > [ ... ]
> > > 
> > > > @@ -82,6 +91,8 @@ properties:
> > > >            - description: Offset of AXI clock controller Low-Power request
> > > >                           register
> > > >            - description: Offset of register controlling TX/RX clock delay
> > > > +          - description: Optional offset of register controlling TXD delay
> > > > +          - description: Optional offset of register controlling RXD delay
> > > 
> > > Will this make the two new offsets strictly required instead of optional?
> > > 
> > > In dt-schema, when an items list defines a tuple, minItems implicitly
> > > defaults to the total number of items in that list. By increasing the list
> > > from 4 to 6 items without adding a minItems: 4 constraint to the inner items
> > > list, won't validation now strictly require exactly 6 items?
> > > 
> > > Could this cause dt_binding_check to fail for existing device trees that
> > > correctly specify only the original 4 elements?
> > 
> > Looks like a valid complaint to me.
> > pw-bot: changes-requested
> > 
> You're right, this is a valid issue.
> 
> I'll fix it in v3 by adding only minItems: 4 to preserve backward
> compatibility with existing 4-cell device trees.
> 
> Should I drop your Acked-by in v3 due to the schema change, or should
> it be retained?

Just fix it, and you can retain.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: [PATCH net v2 2/5] net: stmmac: eswin: fix HSP CSR init ordering after clock enable

[李志]

> -----Original Messages-----
> From: sashiko-bot@kernel.org
> Send time:Tuesday, 19/05/2026 10:23:35
> To: lizhi2@eswincomputing.com
> Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
> Subject: Re: [PATCH net v2 2/5] net: stmmac: eswin: fix HSP CSR init ordering after clock enable
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Hardware State Loss on Resume: The HSP CSR registers are not restored during system resume, contradicting the commit message.
> --
> 
> commit 627f510ee556ff4322e0f4ed0b60fdd27337080f
> Author: Zhi Li <lizhi2@eswincomputing.com>
> 
> net: stmmac: eswin: fix HSP CSR init ordering after clock enable
> 
> This commit moves the HSP CSR configuration in the EIC7700 DWMAC glue driver
> into the post-clock-enable initialization path to ensure proper ordering.
> The intent is to prevent register access before clock enablement and ensure
> deterministic initialization during probe or resume.
> 
> >     This change ensures deterministic initialization and prevents
> >     clock-dependent register access failures during probe or resume.
> 
> Are the HSP CSR registers actually restored during system resume?
> 
> The driver overrides the core resume callback by setting
> plat_dat->resume = eic7700_dwmac_resume. Because a custom resume callback is
> provided, the STMMAC core invokes this instead of the default
> stmmac_plat_resume() (which would have called plat->init()).
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> > index bcb8e000e720b..63001c4acdb7a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> 
> [ ... ]
> 
> > @@ -61,8 +66,28 @@ static int eic7700_clks_config(void *priv, bool enabled)
> >  static int eic7700_dwmac_init(struct device *dev, void *priv)
> >  {
> >  	struct eic7700_qos_priv *dwc = priv;
> > +	int ret;
> > +
> > +	ret = eic7700_clks_config(dwc, true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_set_bits(dwc->eic7700_hsp_regmap,
> > +			      dwc->eth_phy_ctrl_offset,
> > +			      EIC7700_ETH_TX_CLK_SEL |
> > +			      EIC7700_ETH_PHY_INTF_SELI);
> > +	if (ret) {
> > +		eic7700_clks_config(dwc, false);
> > +		return ret;
> > +	}
> > +
> > +	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_axi_lp_ctrl_offset,
> > +		     EIC7700_ETH_CSYSREQ_VAL);
> >  
> > -	return eic7700_clks_config(dwc, true);
> > +	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset,
> > +		     dwc->eth_clk_dly_param);
> > +
> > +	return 0;
> >  }
> 
> Since eic7700_dwmac_resume() only calls pm_runtime_force_resume(dev) and
> doesn't invoke eic7700_dwmac_init(), the HSP CSR configuration won't be
> re-applied upon system resume. If the hardware loses state during suspend,
> will the RGMII delays be permanently lost, leading to a MAC hang or silent
> network failure?
> 
> Also, if we try to fix this by naively calling init() from resume(), would
> it cause a clock reference leak? pm_runtime_force_resume() already
> re-enables the clocks via the runtime PM callbacks, so the CSR configuration
> might need to be factored out into a separate helper function.
> 

You're right, this was simply a mistake in the commit message.

There is no resume path involved — only probe calls
eic7700_dwmac_init(), where the HSP CSR configuration is applied
after clock enablement.

I'll fix the commit message in v3 accordingly.

Thanks for the review.