public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/3] net: stmmac: eic7700: fix EIC7700 eth1 RX sampling timing
@ 2026-03-13  7:52 lizhi2
  2026-03-13  7:53 ` [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control lizhi2
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: lizhi2 @ 2026-03-13  7:52 UTC (permalink / raw)
  To: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, wens, pjw, palmer, aou, alex, linux-riscv,
	linux-stm32, linux-arm-kernel, linux-kernel
  Cc: ningyu, linmin, pinkesh.vaghela, pritesh.patel, weishangjuan,
	Zhi Li

From: Zhi Li <lizhi2@eswincomputing.com>

v3 -> v4:
  - Update eswin,eic7700-eth.yaml:
    - Improve commit message in dt-bindings patch to clarify the
      hardware difference of the eth1 MAC and why a new compatible
      string is required.
    - Move the newly added eswin,hsp-sp-csr item to the end of the list
      to avoid inserting entries in the middle of the binding schema.
    - Simplify the compatible schema by replacing the previous oneOf
      construct with an enum.

  - Update dwmac-eic7700.c:
    - Fix build issues.
    - Adjust code to match the updated binding definition.

  - Update DTS/DTSI descriptions:
    - Move SoC-level descriptions to the .dtsi file.
    - Keep board-specific configuration in the .dts file.

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

v2 -> v3:
  - Update eswin,eic7700-eth.yaml:
    - Extend rx-internal-delay-ps and tx-internal-delay-ps range
      from 0-2400 to 0-2540 to match the full 7-bit hardware delay
      field (127 * 20 ps).
    - Add "multipleOf: 20" constraint to reflect the 20 ps hardware
      step size.
    - Make rx-internal-delay-ps and tx-internal-delay-ps optional.
      A well-designed board should not require internal delay tuning.
    - Remove rx-internal-delay-ps and tx-internal-delay-ps from the
      example to avoid encouraging blind copy into board DTs.

  - Update dwmac-eic7700.c:
    - Treat rx-internal-delay-ps and tx-internal-delay-ps as optional
      DT properties.
    - Apply delay configuration only when properties are present.
    - Keep TX/RX delay registers cleared by default to ensure a
      deterministic state when no delay is specified.

  - Describe Ethernet configuration for the HiFive Premier P550 board:
    - Add GMAC controller nodes for the HiFive Premier P550 board
      to describe the on-board Ethernet configuration.

      The Ethernet controller depends on clock, reset, pinctrl
      and HSP subsystem providers which are currently under
      upstream review. These dependent nodes will be submitted
      separately once the corresponding drivers are merged.

      Due to these missing dependencies, dt-binding-check may
      report warnings or failures for this series.

  - No functional changes to RX clock inversion logic.

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

  - This series is based on the EIC7700 clock support series:
    https://lore.kernel.org/all/20260210095008.726-1-dongxuyang@eswincomputing.com/
    The clock series is currently under review.

v1 -> v2:
  - Update eswin,eic7700-eth.yaml:
    - Drop the vendor-specific properties eswin,rx-clk-invert and
      eswin,tx-clk-invert.
    - Introduce a distinct compatible string
      "eswin,eic7700-qos-eth-clk-inversion" to describe MAC instances that
      require internal RGMII clock inversion.
      This models the SoC-specific hardware difference directly via the
      compatible string and avoids per-board configuration properties.
    - Change rx-internal-delay-ps and tx-internal-delay-ps from enum to
      minimum/maximum to reflect the actual delay range (0-2400 ps)
    - Add reference to High-Speed Subsystem documentation in eswin,hsp-sp-csr
      description. The HSP CSR block is described in Chapter 10
      ("High-Speed Interface") of the EIC7700X SoC Technical Reference Manual,
      Part 4 (EIC7700X_SoC_Technical_Reference_Manual_Part4.pdf):
      https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases

  - Update dwmac-eic7700.c:
    - Remove handling of eswin,rx-clk-invert and eswin,tx-clk-invert
      properties.
    - Select RX clock inversion based on the new
      "eswin,eic7700-qos-eth-clk-inversion" compatible string, using
      match data to apply the required configuration for affected MAC
      instances (eth1).

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

Zhi Li (3):
  dt-bindings: ethernet: eswin: add clock sampling control
  net: stmmac: eic7700: enable clocks before syscon access and correct
    RX sampling timing
  riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet
    controller

 .../bindings/net/eswin,eic7700-eth.yaml       |  69 +++++--
 .../dts/eswin/eic7700-hifive-premier-p550.dts |  42 ++++
 arch/riscv/boot/dts/eswin/eic7700.dtsi        |  66 +++++++
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 181 +++++++++++++-----
 4 files changed, 301 insertions(+), 57 deletions(-)

-- 
2.25.1


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

* [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control
  2026-03-13  7:52 [PATCH net-next v4 0/3] net: stmmac: eic7700: fix EIC7700 eth1 RX sampling timing lizhi2
@ 2026-03-13  7:53 ` lizhi2
  2026-03-13 17:39   ` Conor Dooley
  2026-03-13  7:54 ` [PATCH net-next v4 2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing lizhi2
  2026-03-13  7:54 ` [PATCH net-next v4 3/3] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller lizhi2
  2 siblings, 1 reply; 8+ messages in thread
From: lizhi2 @ 2026-03-13  7:53 UTC (permalink / raw)
  To: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, wens, pjw, palmer, aou, alex, linux-riscv,
	linux-stm32, linux-arm-kernel, linux-kernel
  Cc: ningyu, linmin, pinkesh.vaghela, pritesh.patel, weishangjuan,
	Zhi Li

From: Zhi Li <lizhi2@eswincomputing.com>

Due to chip backend reasons, there is already an approximately 4-5 ns
skew between the RX clock and data of the eth1 MAC controller inside
the silicon.

For 1000M, the RX clock must be inverted since it is not possible to
meet the RGMII timing requirements using only rx-internal-delay-ps on
the MAC together with the standard 2 ns delay on the PHY. Therefore,
even on a properly designed board, eth1 still requires RX clock
inversion.

This behaviour effectively breaks the RGMII timing assumptions at the
SoC level.

For the TX path of eth1, there is also a skew between the TX clock
and data on the MAC controller inside the silicon. This skew happens
to be approximately 2 ns. Therefore, it can be considered that the
2 ns delay of TX is provided by the MAC, so the TX is compliant with
the RGMII standard.

For 10/100 operation, the approximately 4-5 ns skew in the chip does
not break the standard. The RGMII timing table (Section 3.3) specifies
that for 10/100 operation the maximum value is unspecified:
https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/20655/1/RGMIIv2_0_final_hp.pdf

Due to the eth1 silicon behavior described above, a new compatible
string "eswin,eic7700-qos-eth-clk-inversion" is added to the device
tree. This allows the driver to handle the differences between eth1
and eth0 through dedicated logic.

The rx-internal-delay-ps and tx-internal-delay-ps properties now use
minimum and maximum constraints to reflect the actual hardware delay
range (0-2540 ps) applied in 20 ps steps. This relaxes the binding
validation compared to the previous enum-based definition and avoids
regressions for existing DTBs while keeping the same hardware limits.

Treat the RX/TX internal delay properties as optional, board-specific
tuning knobs and remove them from the example to avoid encouraging
their use.

In addition, the binding now includes additional background information
about the HSP CSR registers accessed by the MAC. The TXD and RXD delay
control registers are included so the driver can explicitly clear any
residual configuration left by the bootloader.

Background reference for the High-Speed Subsystem and 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):
https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases

There are currently no in-tree users of the EIC7700 Ethernet driver, so
these changes are safe.

Fixes: 888bd0eca93c ("dt-bindings: ethernet: eswin: Document for EIC7700 SoC")
Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 .../bindings/net/eswin,eic7700-eth.yaml       | 69 +++++++++++++++----
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
index 91e8cd1db67b..0b27719feb7d 100644
--- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
+++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
@@ -20,6 +20,7 @@ select:
       contains:
         enum:
           - eswin,eic7700-qos-eth
+          - eswin,eic7700-qos-eth-clk-inversion
   required:
     - compatible
 
@@ -29,7 +30,9 @@ allOf:
 properties:
   compatible:
     items:
-      - const: eswin,eic7700-qos-eth
+      - enum:
+          - eswin,eic7700-qos-eth
+          - eswin,eic7700-qos-eth-clk-inversion
       - const: snps,dwmac-5.20
 
   reg:
@@ -63,16 +66,29 @@ properties:
       - const: stmmaceth
 
   rx-internal-delay-ps:
-    enum: [0, 200, 600, 1200, 1600, 1800, 2000, 2200, 2400]
+    minimum: 0
+    maximum: 2540
+    multipleOf: 20
 
   tx-internal-delay-ps:
-    enum: [0, 200, 600, 1200, 1600, 1800, 2000, 2200, 2400]
+    minimum: 0
+    maximum: 2540
+    multipleOf: 20
 
   eswin,hsp-sp-csr:
     description:
       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 +98,8 @@ properties:
           - description: Offset of AXI clock controller Low-Power request
                          register
           - description: Offset of register controlling TX/RX clock delay
+          - description: Offset of register controlling TXD delay
+          - description: Offset of register controlling RXD delay
 
 required:
   - compatible
@@ -93,8 +111,6 @@ required:
   - phy-mode
   - resets
   - reset-names
-  - rx-internal-delay-ps
-  - tx-internal-delay-ps
   - eswin,hsp-sp-csr
 
 unevaluatedProperties: false
@@ -104,24 +120,49 @@ examples:
     ethernet@50400000 {
         compatible = "eswin,eic7700-qos-eth", "snps,dwmac-5.20";
         reg = <0x50400000 0x10000>;
+        interrupt-parent = <&plic>;
+        interrupts = <61>;
+        interrupt-names = "macirq";
         clocks = <&d0_clock 186>, <&d0_clock 171>, <&d0_clock 40>,
                 <&d0_clock 193>;
         clock-names = "axi", "cfg", "stmmaceth", "tx";
+        resets = <&reset 95>;
+        reset-names = "stmmaceth";
+        eswin,hsp-sp-csr = <&hsp_sp_csr 0x100 0x108 0x118 0x114 0x11c>;
+        phy-handle = <&gmac0_phy0>;
+        phy-mode = "rgmii-id";
+        snps,aal;
+        snps,fixed-burst;
+        snps,tso;
+        snps,axi-config = <&stmmac_axi_setup_gmac0>;
+
+        stmmac_axi_setup_gmac0: stmmac-axi-config {
+            snps,blen = <0 0 0 0 16 8 4>;
+            snps,rd_osr_lmt = <2>;
+            snps,wr_osr_lmt = <2>;
+        };
+    };
+
+    ethernet@50410000 {
+        compatible = "eswin,eic7700-qos-eth-clk-inversion", "snps,dwmac-5.20";
+        reg = <0x50410000 0x10000>;
         interrupt-parent = <&plic>;
-        interrupts = <61>;
+        interrupts = <70>;
         interrupt-names = "macirq";
-        phy-mode = "rgmii-id";
-        phy-handle = <&phy0>;
-        resets = <&reset 95>;
+        clocks = <&d0_clock 186>, <&d0_clock 171>, <&d0_clock 40>,
+                <&d0_clock 194>;
+        clock-names = "axi", "cfg", "stmmaceth", "tx";
+        resets = <&reset 94>;
         reset-names = "stmmaceth";
-        rx-internal-delay-ps = <200>;
-        tx-internal-delay-ps = <200>;
-        eswin,hsp-sp-csr = <&hsp_sp_csr 0x100 0x108 0x118>;
-        snps,axi-config = <&stmmac_axi_setup>;
+        eswin,hsp-sp-csr = <&hsp_sp_csr 0x200 0x208 0x218 0x214 0x21c>;
+        phy-handle = <&gmac1_phy0>;
+        phy-mode = "rgmii-id";
         snps,aal;
         snps,fixed-burst;
         snps,tso;
-        stmmac_axi_setup: stmmac-axi-config {
+        snps,axi-config = <&stmmac_axi_setup_gmac1>;
+
+        stmmac_axi_setup_gmac1: stmmac-axi-config {
             snps,blen = <0 0 0 0 16 8 4>;
             snps,rd_osr_lmt = <2>;
             snps,wr_osr_lmt = <2>;
-- 
2.25.1


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

* [PATCH net-next v4 2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
  2026-03-13  7:52 [PATCH net-next v4 0/3] net: stmmac: eic7700: fix EIC7700 eth1 RX sampling timing lizhi2
  2026-03-13  7:53 ` [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control lizhi2
@ 2026-03-13  7:54 ` lizhi2
  2026-03-15 16:27   ` [net-next,v4,2/3] " Simon Horman
  2026-03-13  7:54 ` [PATCH net-next v4 3/3] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller lizhi2
  2 siblings, 1 reply; 8+ messages in thread
From: lizhi2 @ 2026-03-13  7:54 UTC (permalink / raw)
  To: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, wens, pjw, palmer, aou, alex, linux-riscv,
	linux-stm32, linux-arm-kernel, linux-kernel
  Cc: ningyu, linmin, pinkesh.vaghela, pritesh.patel, weishangjuan,
	Zhi Li

From: Zhi Li <lizhi2@eswincomputing.com>

The second Ethernet controller (eth1) on the Eswin EIC7700 SoC may fail
to sample RX data correctly at Gigabit speed due to EIC7700-specific
receive clock to data skew at the MAC input in the silicon.

The existing internal delay configuration does not provide sufficient
adjustment range to compensate for this condition at 1000Mbps.
Update the EIC7700 DWMAC glue driver to apply EIC7700-specific clock
sampling inversion only during Gigabit operation on MAC instances
that require it.

TXD and RXD delay registers are explicitly cleared during initialization
to override any residual configuration left by the bootloader. All HSP
CSR register accesses are performed only after the required clocks are
enabled.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
index bcb8e000e720..b230bc5b902a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -28,20 +28,40 @@
 
 /*
  * 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)
+ * - TX Invert : bit  [15]
+ * - RX Delay: bits [30:24] — RX_CLK delay (unit: 0.02ns per bit)
+ * - RX Invert : bit  [31]
  */
 #define EIC7700_ETH_TX_ADJ_DELAY	GENMASK(14, 8)
 #define EIC7700_ETH_RX_ADJ_DELAY	GENMASK(30, 24)
+#define EIC7700_ETH_TX_INV_DELAY	BIT(15)
+#define EIC7700_ETH_RX_INV_DELAY	BIT(31)
 
-#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",
 };
 
+struct eic7700_dwmac_data {
+	bool rgmii_rx_clk_invert;
+};
+
 struct eic7700_qos_priv {
+	struct device *dev;
 	struct plat_stmmacenet_data *plat_dat;
+	struct regmap *eic7700_hsp_regmap;
+	u32 eth_axi_lp_ctrl_offset;
+	u32 eth_phy_ctrl_offset;
+	u32 eth_txd_offset;
+	u32 eth_clk_offset;
+	u32 eth_rxd_offset;
+	u32 eth_clk_dly_param;
+	bool eth_rx_clk_inv;
 };
 
 static int eic7700_clks_config(void *priv, bool enabled)
@@ -61,8 +81,26 @@ 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)
+		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_txd_offset, 0);
+	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_rxd_offset, 0);
+
+	return 0;
 }
 
 static void eic7700_dwmac_exit(struct device *dev, void *priv)
@@ -88,18 +126,35 @@ static int eic7700_dwmac_resume(struct device *dev, void *priv)
 	return ret;
 }
 
+static void eic7700_dwmac_fix_speed(void *priv, phy_interface_t interface,
+				    int speed, unsigned int mode)
+{
+	struct eic7700_qos_priv *dwc = (struct eic7700_qos_priv *)priv;
+	u32 dly_param = dwc->eth_clk_dly_param;
+
+	switch (speed) {
+	case SPEED_1000:
+		if (dwc->eth_rx_clk_inv)
+			dly_param |= EIC7700_ETH_RX_INV_DELAY;
+		break;
+	case SPEED_100:
+	case SPEED_10:
+		break;
+	default:
+		dev_err(dwc->dev, "invalid speed %u\n", speed);
+		break;
+	}
+
+	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_clk_offset, dly_param);
+}
+
 static int eic7700_dwmac_probe(struct platform_device *pdev)
 {
+	const struct eic7700_dwmac_data *data;
 	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;
+	u32 delay_ps, val;
 	int i, ret;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
@@ -116,70 +171,95 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	if (!dwc_priv)
 		return -ENOMEM;
 
+	dwc_priv->dev = &pdev->dev;
+
+	data = device_get_match_data(&pdev->dev);
+	if (!data)
+		return dev_err_probe(&pdev->dev,
+				     -EINVAL, "no match data found\n");
+
+	dwc_priv->eth_rx_clk_inv = data->rgmii_rx_clk_invert;
+
 	/* 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);
+		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);
 
-		eth_dly_param &= ~EIC7700_ETH_RX_ADJ_DELAY;
-		eth_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");
+		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 |=
+				 FIELD_PREP(EIC7700_ETH_RX_ADJ_DELAY, val);
 	}
 
 	/* 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);
+		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");
 
-		eth_dly_param &= ~EIC7700_ETH_TX_ADJ_DELAY;
-		eth_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");
+		val = delay_ps / EIC7700_DELAY_STEP_PS;
+
+		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);
 	}
 
-	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, &dwc_priv->eth_clk_offset);
+	if (ret)
+		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",
-					 3, &eth_rxd_dly_offset);
+					 4, &dwc_priv->eth_txd_offset);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
-				     "can't get eth_rxd_dly_offset\n");
+				     "can't get eth_txd_offset\n");
 
-	regmap_write(eic7700_hsp_regmap, eth_rxd_dly_offset,
-		     eth_dly_param);
+	ret = of_property_read_u32_index(pdev->dev.of_node,
+					 "eswin,hsp-sp-csr",
+					 5, &dwc_priv->eth_rxd_offset);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "can't get eth_rxd_offset\n");
 
 	plat_dat->num_clks = ARRAY_SIZE(eic7700_clk_names);
 	plat_dat->clks = devm_kcalloc(&pdev->dev,
@@ -208,12 +288,27 @@ static int eic7700_dwmac_probe(struct platform_device *pdev)
 	plat_dat->exit = eic7700_dwmac_exit;
 	plat_dat->suspend = eic7700_dwmac_suspend;
 	plat_dat->resume = eic7700_dwmac_resume;
+	plat_dat->fix_mac_speed = eic7700_dwmac_fix_speed;
 
 	return devm_stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
 }
 
+static const struct eic7700_dwmac_data eic7700_dwmac_data = {
+	.rgmii_rx_clk_invert = false,
+};
+
+static const struct eic7700_dwmac_data eic7700_dwmac_data_clk_inversion = {
+	.rgmii_rx_clk_invert = true,
+};
+
 static const struct of_device_id eic7700_dwmac_match[] = {
-	{ .compatible = "eswin,eic7700-qos-eth" },
+	{	.compatible = "eswin,eic7700-qos-eth",
+		.data = &eic7700_dwmac_data,
+	},
+	{
+		.compatible = "eswin,eic7700-qos-eth-clk-inversion",
+		.data = &eic7700_dwmac_data_clk_inversion,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, eic7700_dwmac_match);
-- 
2.25.1


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

* [PATCH net-next v4 3/3] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller
  2026-03-13  7:52 [PATCH net-next v4 0/3] net: stmmac: eic7700: fix EIC7700 eth1 RX sampling timing lizhi2
  2026-03-13  7:53 ` [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control lizhi2
  2026-03-13  7:54 ` [PATCH net-next v4 2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing lizhi2
@ 2026-03-13  7:54 ` lizhi2
  2 siblings, 0 replies; 8+ messages in thread
From: lizhi2 @ 2026-03-13  7:54 UTC (permalink / raw)
  To: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, wens, pjw, palmer, aou, alex, linux-riscv,
	linux-stm32, linux-arm-kernel, linux-kernel
  Cc: ningyu, linmin, pinkesh.vaghela, pritesh.patel, weishangjuan,
	Zhi Li

From: Zhi Li <lizhi2@eswincomputing.com>

Enable the on-board Gigabit Ethernet controller on the
HiFive Premier P550 development board.

Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
---
 .../dts/eswin/eic7700-hifive-premier-p550.dts | 42 ++++++++++++
 arch/riscv/boot/dts/eswin/eic7700.dtsi        | 66 +++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
index 131ed1fc6b2e..5a40be1d2a25 100644
--- a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
+++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
@@ -13,6 +13,8 @@ / {
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
 	};
 
 	chosen {
@@ -20,6 +22,46 @@ chosen {
 	};
 };
 
+&gmac0 {
+	phy-handle = <&gmac0_phy0>;
+	phy-mode = "rgmii-id";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gpio106_pins>;
+	rx-internal-delay-ps = <20>;
+	tx-internal-delay-ps = <100>;
+	status = "okay";
+
+	mdio {
+		gmac0_phy0: ethernet-phy@0 {
+			compatible = "ethernet-phy-id001c.c916";
+			reg = <0>;
+			reset-gpios = <&gpioD 10 GPIO_ACTIVE_LOW>;
+			reset-assert-us = <10000>;
+			reset-deassert-us = <80000>;
+		};
+	};
+};
+
+&gmac1 {
+	phy-handle = <&gmac1_phy0>;
+	phy-mode = "rgmii-rxid";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gpio111_pins>;
+	rx-internal-delay-ps = <200>;
+	tx-internal-delay-ps = <200>;
+	status = "okay";
+
+	mdio {
+		gmac1_phy0: ethernet-phy@0 {
+			compatible = "ethernet-phy-id001c.c916";
+			reg = <0>;
+			reset-gpios = <&gpioD 15 GPIO_ACTIVE_LOW>;
+			reset-assert-us = <10000>;
+			reset-deassert-us = <80000>;
+		};
+	};
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/riscv/boot/dts/eswin/eic7700.dtsi b/arch/riscv/boot/dts/eswin/eic7700.dtsi
index c3ed93008bca..f1a01d5736a1 100644
--- a/arch/riscv/boot/dts/eswin/eic7700.dtsi
+++ b/arch/riscv/boot/dts/eswin/eic7700.dtsi
@@ -5,6 +5,8 @@
 
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
+
 / {
 	#address-cells = <2>;
 	#size-cells = <2>;
@@ -295,6 +297,70 @@ uart4: serial@50940000 {
 			status = "disabled";
 		};
 
+		gmac0: ethernet@50400000 {
+			compatible = "eswin,eic7700-qos-eth", "snps,dwmac-5.20";
+			reg = <0x0 0x50400000 0x0 0x10000>;
+			interrupts = <61>;
+			interrupt-names = "macirq";
+			clocks = <&clk 186>,
+				 <&clk 171>,
+				 <&clk 40>,
+				 <&clk 193>;
+			clock-names = "axi", "cfg", "stmmaceth", "tx";
+			resets = <&reset 95>;
+			reset-names = "stmmaceth";
+			eswin,hsp-sp-csr = <&hsp_sp_csr 0x100 0x108 0x118 0x114 0x11c>;
+			snps,aal;
+			snps,fixed-burst;
+			snps,tso;
+			snps,axi-config = <&stmmac_axi_setup_gmac0>;
+			status = "disabled";
+
+			stmmac_axi_setup_gmac0: stmmac-axi-config {
+				snps,blen = <0 0 0 0 16 8 4>;
+				snps,rd_osr_lmt = <2>;
+				snps,wr_osr_lmt = <2>;
+			};
+
+			mdio {
+				compatible = "snps,dwmac-mdio";
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
+		gmac1: ethernet@50410000 {
+			compatible = "eswin,eic7700-qos-eth-clk-inversion", "snps,dwmac-5.20";
+			reg = <0x0 0x50410000 0x0 0x10000>;
+			interrupts = <70>;
+			interrupt-names = "macirq";
+			clocks = <&clk 186>,
+				 <&clk 171>,
+				 <&clk 40>,
+				 <&clk 194>;
+			clock-names = "axi", "cfg", "stmmaceth", "tx";
+			resets = <&reset 94>;
+			reset-names = "stmmaceth";
+			eswin,hsp-sp-csr = <&hsp_sp_csr 0x200 0x208 0x218 0x214 0x21c>;
+			snps,aal;
+			snps,fixed-burst;
+			snps,tso;
+			snps,axi-config = <&stmmac_axi_setup_gmac1>;
+			status = "disabled";
+
+			stmmac_axi_setup_gmac1: stmmac-axi-config {
+				snps,blen = <0 0 0 0 16 8 4>;
+				snps,rd_osr_lmt = <2>;
+				snps,wr_osr_lmt = <2>;
+			};
+
+			mdio {
+				compatible = "snps,dwmac-mdio";
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		gpio@51600000 {
 			compatible = "snps,dw-apb-gpio";
 			reg = <0x0 0x51600000 0x0 0x80>;
-- 
2.25.1


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

* Re: [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control
  2026-03-13  7:53 ` [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control lizhi2
@ 2026-03-13 17:39   ` Conor Dooley
  2026-03-19  9:55     ` 李志
  0 siblings, 1 reply; 8+ messages in thread
From: Conor Dooley @ 2026-03-13 17:39 UTC (permalink / raw)
  To: lizhi2
  Cc: devicetree, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
	rmk+kernel, wens, pjw, palmer, aou, alex, linux-riscv,
	linux-stm32, linux-arm-kernel, linux-kernel, ningyu, linmin,
	pinkesh.vaghela, pritesh.patel, weishangjuan

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

On Fri, Mar 13, 2026 at 03:53:51PM +0800, lizhi2@eswincomputing.com wrote:
> From: Zhi Li <lizhi2@eswincomputing.com>
> 
> Due to chip backend reasons, there is already an approximately 4-5 ns
> skew between the RX clock and data of the eth1 MAC controller inside
> the silicon.
> 
> For 1000M, the RX clock must be inverted since it is not possible to
> meet the RGMII timing requirements using only rx-internal-delay-ps on
> the MAC together with the standard 2 ns delay on the PHY. Therefore,
> even on a properly designed board, eth1 still requires RX clock
> inversion.
> 
> This behaviour effectively breaks the RGMII timing assumptions at the
> SoC level.
> 
> For the TX path of eth1, there is also a skew between the TX clock
> and data on the MAC controller inside the silicon. This skew happens
> to be approximately 2 ns. Therefore, it can be considered that the
> 2 ns delay of TX is provided by the MAC, so the TX is compliant with
> the RGMII standard.
> 
> For 10/100 operation, the approximately 4-5 ns skew in the chip does
> not break the standard. The RGMII timing table (Section 3.3) specifies
> that for 10/100 operation the maximum value is unspecified:
> https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/20655/1/RGMIIv2_0_final_hp.pdf
> 
> Due to the eth1 silicon behavior described above, a new compatible
> string "eswin,eic7700-qos-eth-clk-inversion" is added to the device
> tree. This allows the driver to handle the differences between eth1
> and eth0 through dedicated logic.
> 
> The rx-internal-delay-ps and tx-internal-delay-ps properties now use
> minimum and maximum constraints to reflect the actual hardware delay
> range (0-2540 ps) applied in 20 ps steps. This relaxes the binding
> validation compared to the previous enum-based definition and avoids
> regressions for existing DTBs while keeping the same hardware limits.
> 
> Treat the RX/TX internal delay properties as optional, board-specific
> tuning knobs and remove them from the example to avoid encouraging
> their use.
> 
> In addition, the binding now includes additional background information
> about the HSP CSR registers accessed by the MAC. The TXD and RXD delay
> control registers are included so the driver can explicitly clear any
> residual configuration left by the bootloader.
> 
> Background reference for the High-Speed Subsystem and 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):
> https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases
> 
> There are currently no in-tree users of the EIC7700 Ethernet driver, so
> these changes are safe.
> 
> Fixes: 888bd0eca93c ("dt-bindings: ethernet: eswin: Document for EIC7700 SoC")
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>

Krzysztof might not yet be happy with the compatible naming, but from my
pov:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

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

* Re: [net-next,v4,2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
  2026-03-13  7:54 ` [PATCH net-next v4 2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing lizhi2
@ 2026-03-15 16:27   ` Simon Horman
  2026-03-16  2:10     ` 李志
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Horman @ 2026-03-15 16:27 UTC (permalink / raw)
  To: lizhi2
  Cc: Simon Horman, alexandre.torgue, devicetree, alex,
	linux-arm-kernel, linux-stm32, ningyu, linux-riscv, krzk+dt,
	davem, andrew+netdev, conor+dt, weishangjuan, kuba, robh,
	edumazet, pjw, rmk+kernel, palmer, mcoquelin.stm32,
	pinkesh.vaghela, linux-kernel, pritesh.patel, pabeni, aou, wens,
	netdev, linmin

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing

The second Ethernet controller (eth1) on the Eswin EIC7700 SoC may fail
to sample RX data correctly at Gigabit speed due to EIC7700-specific
receive clock to data skew at the MAC input in the silicon. The existing
internal delay configuration does not provide sufficient adjustment range
to compensate for this condition at 1000Mbps. Update the EIC7700 DWMAC
glue driver to apply EIC7700-specific clock sampling inversion only
during Gigabit operation on MAC instances that require it.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> index bcb8e000e720..b230bc5b902a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c

[ ... ]

> @@ -61,8 +81,26 @@ 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)
> +		return ret;

Does this code leak the enabled clocks? The function calls
eic7700_clks_config(dwc, true) to enable the clocks, then later calls
regmap_set_bits(). If regmap_set_bits() fails, the function returns
without disabling the clocks.

Looking at the caller in stmmac_main.c:stmmac_dvr_probe():

    if (priv->plat->init) {
        ret = priv->plat->init(priv->device, priv->plat->bsp_priv);
        if (ret)
            return ret;
    }

    ret = __stmmac_dvr_probe(priv->device, priv->plat, &res);
    if (ret)
        goto error_exit_init;

The exit handler is only called when __stmmac_dvr_probe() fails, not
when init() fails. This leaves the bulk clocks in the prepared-and-enabled
state on this error path.

> +
> +	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_txd_offset, 0);
> +	regmap_write(dwc->eic7700_hsp_regmap, dwc->eth_rxd_offset, 0);
> +
> +	return 0;
>  }

[ ... ]

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

* Re: Re: [net-next,v4,2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
  2026-03-15 16:27   ` [net-next,v4,2/3] " Simon Horman
@ 2026-03-16  2:10     ` 李志
  0 siblings, 0 replies; 8+ messages in thread
From: 李志 @ 2026-03-16  2:10 UTC (permalink / raw)
  To: Simon Horman
  Cc: alexandre.torgue, devicetree, alex, linux-arm-kernel, linux-stm32,
	ningyu, linux-riscv, krzk+dt, davem, andrew+netdev, conor+dt,
	weishangjuan, kuba, robh, edumazet, pjw, rmk+kernel, palmer,
	mcoquelin.stm32, pinkesh.vaghela, linux-kernel, pritesh.patel,
	pabeni, aou, wens, netdev, linmin




> -----原始邮件-----
> 发件人: "Simon Horman" <horms@kernel.org>
> 发送时间:2026-03-16 00:27:35 (星期一)
> 收件人: lizhi2@eswincomputing.com
> 抄送: "Simon Horman" <horms@kernel.org>, alexandre.torgue@foss.st.com, devicetree@vger.kernel.org, alex@ghiti.fr, linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, ningyu@eswincomputing.com, linux-riscv@lists.infradead.org, krzk+dt@kernel.org, davem@davemloft.net, andrew+netdev@lunn.ch, conor+dt@kernel.org, weishangjuan@eswincomputing.com, kuba@kernel.org, robh@kernel.org, edumazet@google.com, pjw@kernel.org, rmk+kernel@armlinux.org.uk, palmer@dabbelt.com, mcoquelin.stm32@gmail.com, pinkesh.vaghela@einfochips.com, linux-kernel@vger.kernel.org, pritesh.patel@einfochips.com, pabeni@redhat.com, aou@eecs.berkeley.edu, wens@kernel.org, netdev@vger.kernel.org, linmin@eswincomputing.com
> 主题: Re: [net-next,v4,2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
> 
> The second Ethernet controller (eth1) on the Eswin EIC7700 SoC may fail
> to sample RX data correctly at Gigabit speed due to EIC7700-specific
> receive clock to data skew at the MAC input in the silicon. The existing
> internal delay configuration does not provide sufficient adjustment range
> to compensate for this condition at 1000Mbps. Update the EIC7700 DWMAC
> glue driver to apply EIC7700-specific clock sampling inversion only
> during Gigabit operation on MAC instances that require it.
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> > index bcb8e000e720..b230bc5b902a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> 
> [ ... ]
> 
> > @@ -61,8 +81,26 @@ 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)
> > +		return ret;
> 
> Does this code leak the enabled clocks? The function calls
> eic7700_clks_config(dwc, true) to enable the clocks, then later calls
> regmap_set_bits(). If regmap_set_bits() fails, the function returns
> without disabling the clocks.
> 
> Looking at the caller in stmmac_main.c:stmmac_dvr_probe():
> 
>     if (priv->plat->init) {
>         ret = priv->plat->init(priv->device, priv->plat->bsp_priv);
>         if (ret)
>             return ret;
>     }
> 
>     ret = __stmmac_dvr_probe(priv->device, priv->plat, &res);
>     if (ret)
>         goto error_exit_init;
> 
> The exit handler is only called when __stmmac_dvr_probe() fails, not
> when init() fails. This leaves the bulk clocks in the prepared-and-enabled
> state on this error path.
> 
Thanks for the review and for catching this.

You're right, the error path in eic7700_dwmac_init() would leak the enabled
clocks if regmap_set_bits() fails. This was an oversight during my own review.

I'll fix the error handling and include the update in the next version (v5)
of the patch series.

Thanks again.

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

* Re: Re: [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control
  2026-03-13 17:39   ` Conor Dooley
@ 2026-03-19  9:55     ` 李志
  0 siblings, 0 replies; 8+ messages in thread
From: 李志 @ 2026-03-19  9:55 UTC (permalink / raw)
  To: krzk+dt, Conor Dooley
  Cc: devicetree, andrew+netdev, davem, edumazet, kuba, robh, conor+dt,
	netdev, pabeni, mcoquelin.stm32, alexandre.torgue, rmk+kernel,
	wens, pjw, palmer, aou, alex, linux-riscv, linux-stm32,
	linux-arm-kernel, linux-kernel, ningyu, linmin, pinkesh.vaghela,
	pritesh.patel, weishangjuan

Hi Krzysztof,

Could you please take a look at Conor’s feedback on the compatible naming?

Conor has reviewed the patch and provided his Acked-by, but also noted
that there might be concerns regarding the compatible string.

Please let me know if the current naming is acceptable, or if any changes
are required.

Thanks,
Zhi Li


> -----原始邮件-----
> 发件人: "Conor Dooley" <conor@kernel.org>
> 发送时间:2026-03-14 01:39:58 (星期六)
> 收件人: lizhi2@eswincomputing.com
> 抄送: devicetree@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, wens@kernel.org, pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr, linux-riscv@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, pritesh.patel@einfochips.com, weishangjuan@eswincomputing.com
> 主题: Re: [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control
> 
> On Fri, Mar 13, 2026 at 03:53:51PM +0800, lizhi2@eswincomputing.com wrote:
> > From: Zhi Li <lizhi2@eswincomputing.com>
> > 
> > Due to chip backend reasons, there is already an approximately 4-5 ns
> > skew between the RX clock and data of the eth1 MAC controller inside
> > the silicon.
> > 
> > For 1000M, the RX clock must be inverted since it is not possible to
> > meet the RGMII timing requirements using only rx-internal-delay-ps on
> > the MAC together with the standard 2 ns delay on the PHY. Therefore,
> > even on a properly designed board, eth1 still requires RX clock
> > inversion.
> > 
> > This behaviour effectively breaks the RGMII timing assumptions at the
> > SoC level.
> > 
> > For the TX path of eth1, there is also a skew between the TX clock
> > and data on the MAC controller inside the silicon. This skew happens
> > to be approximately 2 ns. Therefore, it can be considered that the
> > 2 ns delay of TX is provided by the MAC, so the TX is compliant with
> > the RGMII standard.
> > 
> > For 10/100 operation, the approximately 4-5 ns skew in the chip does
> > not break the standard. The RGMII timing table (Section 3.3) specifies
> > that for 10/100 operation the maximum value is unspecified:
> > https://community.nxp.com/pwmxy87654/attachments/pwmxy87654/imx-processors/20655/1/RGMIIv2_0_final_hp.pdf
> > 
> > Due to the eth1 silicon behavior described above, a new compatible
> > string "eswin,eic7700-qos-eth-clk-inversion" is added to the device
> > tree. This allows the driver to handle the differences between eth1
> > and eth0 through dedicated logic.
> > 
> > The rx-internal-delay-ps and tx-internal-delay-ps properties now use
> > minimum and maximum constraints to reflect the actual hardware delay
> > range (0-2540 ps) applied in 20 ps steps. This relaxes the binding
> > validation compared to the previous enum-based definition and avoids
> > regressions for existing DTBs while keeping the same hardware limits.
> > 
> > Treat the RX/TX internal delay properties as optional, board-specific
> > tuning knobs and remove them from the example to avoid encouraging
> > their use.
> > 
> > In addition, the binding now includes additional background information
> > about the HSP CSR registers accessed by the MAC. The TXD and RXD delay
> > control registers are included so the driver can explicitly clear any
> > residual configuration left by the bootloader.
> > 
> > Background reference for the High-Speed Subsystem and 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):
> > https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases
> > 
> > There are currently no in-tree users of the EIC7700 Ethernet driver, so
> > these changes are safe.
> > 
> > Fixes: 888bd0eca93c ("dt-bindings: ethernet: eswin: Document for EIC7700 SoC")
> > Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> 
> Krzysztof might not yet be happy with the compatible naming, but from my
> pov:
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

end of thread, other threads:[~2026-03-19  9:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13  7:52 [PATCH net-next v4 0/3] net: stmmac: eic7700: fix EIC7700 eth1 RX sampling timing lizhi2
2026-03-13  7:53 ` [PATCH net-next v4 1/3] dt-bindings: ethernet: eswin: add clock sampling control lizhi2
2026-03-13 17:39   ` Conor Dooley
2026-03-19  9:55     ` 李志
2026-03-13  7:54 ` [PATCH net-next v4 2/3] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing lizhi2
2026-03-15 16:27   ` [net-next,v4,2/3] " Simon Horman
2026-03-16  2:10     ` 李志
2026-03-13  7:54 ` [PATCH net-next v4 3/3] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller lizhi2

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox