[PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller

[PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller

[weishangjuan]

From: Shangjuan Wei <weishangjuan@eswincomputing.com>

Updates:

  dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  v1 -> v2:
    1. Remove the code related to PHY LED configuration from the MAC driver.
    2. Use phylib instead of the GPIO API in the driver to implement the PHY reset function.
    3. Align with the latest stmmac API, use the API provided by stmmac helper to refactor the driver,
       and replace or remove duplicate code.
    4. Adjust the code format and driver interfaces, such as replacing kzalloc with devm_kzalloc, etc.

  ethernet: eswin: Add eic7700 ethernet driver
  v1 -> v2:
    1. Significant errors have been corrected in the email reply for version v1.
    2. Add snps,dwmac.
    3. Chang the names of reset-names and phy-mode.
    4. Add descriptions of eswin, hsp_sp_csr, eswin, syscrg.csr, eswin, dly_hsp.reg.

  Regarding the question about delay parameters in the previous email reply, the explanation is as follows:
    Dly_hsp_reg: Configure the delay compensation register between MAC/PHY;
    Dly_param_ *: The value written to the dly_hsp_reg register at a rate of 1000/100/10, which varies due 
                  to the routing of the board;

  In addition, your bot found errors running 'make dt_binding_check' on our patch about yamllint warnings/errors,
  it looks like the validation failure is because missing eswin entry in vendor-prefixes.yaml. 
  When we run "make dt_binding_check", we get the same error. We have already added 'eswin' in the vendor-prefixes.yaml 
  file before, and the code has mentioned the community, but you have not yet integrated it.

Shangjuan Wei (2):
  dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  ethernet: eswin: Add eic7700 ethernet driver

 .../bindings/net/eswin,eic7700-eth.yaml       | 200 +++++++++
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 410 ++++++++++++++++++
 4 files changed, 622 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c

-- 
2.17.1

[PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC

[weishangjuan]

From: Shangjuan Wei <weishangjuan@eswincomputing.com>

Add ESWIN EIC7700 Ethernet controller, supporting
multi-rate (10M/100M/1G) auto-negotiation, clock/reset control,
and AXI bus parameter optimization.

Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
---
 .../bindings/net/eswin,eic7700-eth.yaml       | 200 ++++++++++++++++++
 1 file changed, 200 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml

diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
new file mode 100644
index 000000000000..c76d2fbf0ebd
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
@@ -0,0 +1,200 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/eswin,eic7700-eth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Eswin EIC7700 SOC Eth Controller
+
+maintainers:
+  - Shuang Liang <liangshuang@eswincomputing.com>
+  - Zhi Li <lizhi2@eswincomputing.com>
+  - Shangjuan Wei <weishangjuan@eswincomputing.com>
+
+description:
+  The eth controller registers are part of the syscrg block on
+  the EIC7700 SoC.
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - eswin,eic7700-qos-eth
+  required:
+    - compatible
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: eswin,eic7700-qos-eth
+      - const: snps,dwmac
+
+  reg:
+    description: Base address and size of the Ethernet controller registers
+    items:
+      - description: Register base address
+      - description: Register size
+
+  interrupt-names:
+    const: macirq
+
+  interrupts:
+    maxItems: 1
+
+  phy-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - rgmii
+      - rgmii-rxid
+      - rgmii-txid
+      - rgmii-id
+
+  phy-handle:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference to the PHY device
+
+  clocks:
+    minItems: 3
+    maxItems: 7
+
+  clock-names:
+    minItems: 3
+    contains:
+      enum:
+        - app
+        - stmmaceth
+        - tx
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+      - const: stmmaceth
+
+  dma-noncoherent: true
+
+  # Custom properties
+  eswin,hsp_sp_csr:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: HSP peripheral control register area
+          - description: Control registers (such as used to select TX clock
+                         source, PHY interface mode)
+          - description: AXI bus low-power control related registers
+          - description: Status register
+    description:
+      Configure TX clock source selection, set PHY interface mode,
+      and control low-power bus behavior
+
+  eswin,syscrg_csr:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: CRG's device tree node handle
+          - description: Enable and divide HSP ACLK control
+          - description: Behavior of configuring HSP controller
+    description:
+      Register that accesses the system clock controller.
+      Used to configure HSP clocks for Ethernet subsystems
+
+  eswin,dly_hsp_reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: TX delay control register offset
+          - description: RX delay control register offset
+          - description: Switch for controlling delay function
+    description:
+      Register for configuring delay compensation between MAC/PHY
+
+  snps,axi-config:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: AXI bus configuration
+
+  stmmac-axi-config:
+    type: object
+    unevaluatedProperties: false
+    properties:
+      snps,blen:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        description: Set the burst transmission length of AXI bus
+      snps,rd_osr_lmt:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Set OSR restrictions for read operations
+      snps,wr_osr_lmt:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Set OSR restrictions for write operations
+      snps,lpi_en:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description: Low Power Interface enable flag (true/false)
+    required:
+      - snps,blen
+      - snps,rd_osr_lmt
+      - snps,wr_osr_lmt
+      - snps,lpi_en
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupt-names
+  - interrupts
+  - phy-mode
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - eswin,hsp_sp_csr
+  - eswin,syscrg_csr
+  - eswin,dly_hsp_reg
+  - snps,axi-config
+
+additionalProperties: false
+
+examples:
+  - |
+    gmac0: ethernet@50400000 {
+        compatible = "eswin,eic7700-qos-eth", "snps,dwmac";
+        reg = <0x0 0x50400000 0x0 0x10000>;
+        interrupt-parent = <&plic>;
+        interrupt-names = "macirq";
+        interrupts = <61>;
+        phy-mode = "rgmii-txid";
+        phy-handle = <&phy0>;
+        status = "okay";
+        clocks = <&clock 550>,
+                 <&clock 551>,
+                 <&clock 552>;
+        clock-names = "app", "stmmaceth", "tx";
+        resets = <&reset 0x07 (1 << 26)>;
+        reset-names = "stmmaceth";
+        dma-noncoherent;
+        eswin,hsp_sp_csr = <&hsp_sp_csr 0x1030 0x100 0x108>;
+        eswin,syscrg_csr = <&sys_crg 0x148 0x14c>;
+        eswin,dly_hsp_reg = <0x114 0x118 0x11c>;
+        snps,axi-config = <&stmmac_axi_setup>;
+        stmmac_axi_setup: stmmac-axi-config {
+            snps,blen = <0 0 0 0 16 8 4>;
+            snps,rd_osr_lmt = <2>;
+            snps,wr_osr_lmt = <2>;
+            snps,lpi_en;
+        };
+        /* mdio {
+          compatible = "snps,dwmac-mdio";
+          status = "okay";
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          phy0: ethernet-phy@0 {
+            device_type = "ethernet-phy";
+            reg = <0>;
+            compatible = "ethernet-phy-id001c.c916", "realtek,rtl8211f";
+          };
+        }; */
+    };
-- 
2.17.1

[PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver

[weishangjuan]

From: Shangjuan Wei <weishangjuan@eswincomputing.com>

Add Ethernet controller support for Eswin's eic7700 SoC. The driver
provides management and control of Ethernet signals for the eiC7700
series chips.

Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 410 ++++++++++++++++++
 3 files changed, 422 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 67fa879b1e52..a13b15ce1abd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -67,6 +67,17 @@ config DWMAC_ANARION
 
 	  This selects the Anarion SoC glue layer support for the stmmac driver.
 
+config DWMAC_EIC7700
+	tristate "Support for Eswin eic7700 ethernet driver"
+	select CRC32
+	select MII
+	depends on OF && HAS_DMA && ARCH_ESWIN || COMPILE_TEST
+	help
+	  This driver supports the Eswin EIC7700 Ethernet controller,
+	  which integrates Synopsys DesignWare QoS features. It enables
+	  high-speed networking with DMA acceleration and is optimized
+	  for embedded systems.
+
 config DWMAC_INGENIC
 	tristate "Ingenic MAC support"
 	default MACH_INGENIC
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index b591d93f8503..f4ec5fc16571 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
 # Ordering matters. Generic driver must be last.
 obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
 obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
+obj-$(CONFIG_DWMAC_EIC7700)	+= dwmac-eic7700.o
 obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
 obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
new file mode 100644
index 000000000000..98b1e63913be
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Eswin DWC Ethernet linux driver
+ *
+ * Authors: Shuang Liang <liangshuang@eswincomputing.com>
+ * Shangjuan Wei <weishangjuan@eswincomputing.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/ethtool.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/stmmac.h>
+
+#include "stmmac_platform.h"
+#include "dwmac4.h"
+
+#include <linux/regmap.h>
+
+/* eth_phy_ctrl_offset eth0:0x100; eth1:0x200 */
+#define ETH_TX_CLK_SEL			BIT(16)
+#define ETH_PHY_INTF_SELI		BIT(0)
+
+/* eth_axi_lp_ctrl_offset eth0:0x108; eth1:0x208 */
+#define ETH_CSYSREQ_VAL			BIT(0)
+
+/* hsp_aclk_ctrl_offset (0x148) */
+#define HSP_ACLK_CLKEN				BIT(31)
+#define HSP_ACLK_DIVSOR				(0x2 << 4)
+
+/* hsp_cfg_ctrl_offset (0x14c) */
+#define HSP_CFG_CLKEN			BIT(31)
+#define SCU_HSP_PCLK_EN			BIT(30)
+#define HSP_CFG_CTRL_REGSET		(HSP_CFG_CLKEN | SCU_HSP_PCLK_EN)
+
+/* PHY default addr in mdio*/
+#define PHY_ADDR				-1
+
+struct eswin_qos_priv {
+	struct device *dev;
+	int dev_id;
+	struct regmap *crg_regmap;
+	struct regmap *hsp_regmap;
+	int phyaddr;
+	unsigned int dly_hsp_reg[3];
+	unsigned int dly_param_1000m[3];
+	unsigned int dly_param_100m[3];
+	unsigned int dly_param_10m[3];
+};
+
+static struct clk *dwc_eth_find_clk(struct plat_stmmacenet_data *plat_dat,
+				    const char *name)
+{
+	for (int i = 0; i < plat_dat->num_clks; i++)
+		if (strcmp(plat_dat->clks[i].id, name) == 0)
+			return plat_dat->clks[i].clk;
+
+	return NULL;
+}
+
+static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
+				   struct plat_stmmacenet_data *plat_dat)
+{
+	struct device *dev = &pdev->dev;
+	u32 burst_map = 0;
+	u32 bit_index = 0;
+	u32 a_index = 0;
+
+	if (!plat_dat->axi) {
+		plat_dat->axi = devm_kzalloc(&pdev->dev, sizeof(struct stmmac_axi), GFP_KERNEL);
+
+		if (!plat_dat->axi)
+			return -ENOMEM;
+	}
+
+	plat_dat->axi->axi_lpi_en = device_property_read_bool(dev,
+							      "snps,en-lpi");
+	if (device_property_read_u32(dev, "snps,write-requests",
+				     &plat_dat->axi->axi_wr_osr_lmt)) {
+		/**
+		 * Since the register has a reset value of 1, if property
+		 * is missing, default to 1.
+		 */
+		plat_dat->axi->axi_wr_osr_lmt = 1;
+	} else {
+		/**
+		 * If property exists, to keep the behavior from dwc_eth_qos,
+		 * subtract one after parsing.
+		 */
+		plat_dat->axi->axi_wr_osr_lmt--;
+	}
+
+	if (device_property_read_u32(dev, "snps,read-requests",
+				     &plat_dat->axi->axi_rd_osr_lmt)) {
+		/**
+		 * Since the register has a reset value of 1, if property
+		 * is missing, default to 1.
+		 */
+		plat_dat->axi->axi_rd_osr_lmt = 1;
+	} else {
+		/**
+		 * If property exists, to keep the behavior from dwc_eth_qos,
+		 * subtract one after parsing.
+		 */
+		plat_dat->axi->axi_rd_osr_lmt--;
+	}
+	device_property_read_u32(dev, "snps,burst-map", &burst_map);
+
+	/* converts burst-map bitmask to burst array */
+	for (bit_index = 0; bit_index < 7; bit_index++) {
+		if (burst_map & (1 << bit_index)) {
+			switch (bit_index) {
+			case 0:
+				plat_dat->axi->axi_blen[a_index] = 4; break;
+			case 1:
+				plat_dat->axi->axi_blen[a_index] = 8; break;
+			case 2:
+				plat_dat->axi->axi_blen[a_index] = 16; break;
+			case 3:
+				plat_dat->axi->axi_blen[a_index] = 32; break;
+			case 4:
+				plat_dat->axi->axi_blen[a_index] = 64; break;
+			case 5:
+				plat_dat->axi->axi_blen[a_index] = 128; break;
+			case 6:
+				plat_dat->axi->axi_blen[a_index] = 256; break;
+			default:
+				break;
+			}
+			a_index++;
+		}
+	}
+
+	/* dwc-qos needs GMAC4, AAL, TSO and PMT */
+	plat_dat->has_gmac4 = 1;
+	plat_dat->dma_cfg->aal = 1;
+	plat_dat->flags |= STMMAC_FLAG_TSO_EN;
+	plat_dat->pmt = 1;
+
+	return 0;
+}
+
+static int dwc_qos_probe(struct platform_device *pdev,
+			 struct plat_stmmacenet_data *plat_dat,
+			 struct stmmac_resources *stmmac_res)
+{
+	plat_dat->pclk = dwc_eth_find_clk(plat_dat, "phy_ref_clk");
+
+	return 0;
+}
+
+static void eswin_qos_fix_speed(void *priv, int speed, unsigned int mode)
+{
+	struct eswin_qos_priv *dwc_priv = priv;
+	int i;
+
+	switch (speed) {
+	case SPEED_1000:
+		for (i = 0; i < 3; i++)
+			regmap_write(dwc_priv->hsp_regmap,
+				     dwc_priv->dly_hsp_reg[i],
+				     dwc_priv->dly_param_1000m[i]);
+
+		break;
+	case SPEED_100:
+		for (i = 0; i < 3; i++) {
+			regmap_write(dwc_priv->hsp_regmap,
+				     dwc_priv->dly_hsp_reg[i],
+				     dwc_priv->dly_param_100m[i]);
+		}
+
+		break;
+	case SPEED_10:
+		for (i = 0; i < 3; i++) {
+			regmap_write(dwc_priv->hsp_regmap,
+				     dwc_priv->dly_hsp_reg[i],
+				     dwc_priv->dly_param_10m[i]);
+		}
+
+		break;
+	default:
+		dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
+		break;
+	}
+}
+
+static int eswin_qos_probe(struct platform_device *pdev,
+			   struct plat_stmmacenet_data *plat_dat,
+			   struct stmmac_resources *stmmac_res)
+{
+	struct eswin_qos_priv *dwc_priv;
+	u32 hsp_aclk_ctrl_offset;
+	u32 hsp_aclk_ctrl_regset;
+	u32 hsp_cfg_ctrl_offset;
+	u32 eth_axi_lp_ctrl_offset;
+	u32 eth_phy_ctrl_offset;
+	u32 eth_phy_ctrl_regset;
+	struct clk *clk_app;
+	int ret;
+	int err;
+
+	dwc_priv = devm_kzalloc(&pdev->dev, sizeof(*dwc_priv), GFP_KERNEL);
+	if (!dwc_priv)
+		return -ENOMEM;
+
+	if (device_property_read_u32(&pdev->dev, "id", &dwc_priv->dev_id))
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				"Can not read device id!\n");
+
+	dwc_priv->dev = &pdev->dev;
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,phyaddr", 0,
+					 &dwc_priv->phyaddr);
+	if (ret)
+		dev_warn(&pdev->dev, "can't get phyaddr (%d)\n", ret);
+
+	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "eswin,dly_hsp_reg",
+						  &dwc_priv->dly_hsp_reg[0], 3, 0);
+	if (ret != 3) {
+		dev_err(&pdev->dev, "can't get delay hsp reg.ret(%d)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-1000m",
+						  &dwc_priv->dly_param_1000m[0], 3, 0);
+	if (ret != 3) {
+		dev_err(&pdev->dev, "can't get delay param for 1Gbps mode (%d)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-100m",
+						  &dwc_priv->dly_param_100m[0], 3, 0);
+	if (ret != 3) {
+		dev_err(&pdev->dev, "can't get delay param for 100Mbps mode (%d)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-10m",
+						  &dwc_priv->dly_param_10m[0], 3, 0);
+	if (ret != 3) {
+		dev_err(&pdev->dev, "can't get delay param for 10Mbps mode (%d)\n", ret);
+		return ret;
+	}
+
+	dwc_priv->crg_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							       "eswin,syscrg_csr");
+	if (IS_ERR(dwc_priv->crg_regmap)) {
+		dev_dbg(&pdev->dev, "No syscrg_csr phandle specified\n");
+		return 0;
+	}
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 1,
+					 &hsp_aclk_ctrl_offset);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get syscrg_csr 1\n");
+
+	regmap_read(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, &hsp_aclk_ctrl_regset);
+	hsp_aclk_ctrl_regset |= (HSP_ACLK_CLKEN | HSP_ACLK_DIVSOR);
+	regmap_write(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, hsp_aclk_ctrl_regset);
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 2,
+					 &hsp_cfg_ctrl_offset);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get syscrg_csr 2\n");
+
+	regmap_write(dwc_priv->crg_regmap, hsp_cfg_ctrl_offset, HSP_CFG_CTRL_REGSET);
+
+	dwc_priv->hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							       "eswin,hsp_sp_csr");
+	if (IS_ERR(dwc_priv->hsp_regmap)) {
+		dev_dbg(&pdev->dev, "No hsp_sp_csr phandle specified\n");
+		return 0;
+	}
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 2,
+					 &eth_phy_ctrl_offset);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get hsp_sp_csr 2\n");
+
+	regmap_read(dwc_priv->hsp_regmap,
+		    eth_phy_ctrl_offset,
+		    &eth_phy_ctrl_regset);
+	eth_phy_ctrl_regset |= (ETH_TX_CLK_SEL | ETH_PHY_INTF_SELI);
+	regmap_write(dwc_priv->hsp_regmap,
+		     eth_phy_ctrl_offset,
+		     eth_phy_ctrl_regset);
+
+	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 3,
+					 &eth_axi_lp_ctrl_offset);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				"can't get hsp_sp_csr 3\n");
+
+	regmap_write(dwc_priv->hsp_regmap,
+		     eth_axi_lp_ctrl_offset,
+		     ETH_CSYSREQ_VAL);
+
+	clk_app = devm_clk_get_enabled(&pdev->dev, "app");
+	if (IS_ERR(clk_app))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk_app),
+				"error getting app clock\n");
+
+	plat_dat->clk_tx_i = devm_clk_get_enabled(&pdev->dev, "tx");
+	if (IS_ERR(plat_dat->clk_tx_i))
+		return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat->clk_tx_i),
+				"error getting tx clock\n");
+
+	plat_dat->fix_mac_speed = eswin_qos_fix_speed;
+	plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
+	plat_dat->bsp_priv = dwc_priv;
+	plat_dat->phy_addr = PHY_ADDR;
+
+	return 0;
+}
+
+struct dwc_eth_dwmac_data {
+	int (*probe)(struct platform_device *pdev,
+		     struct plat_stmmacenet_data *plat_dat,
+		     struct stmmac_resources *res);
+	const char *stmmac_clk_name;
+};
+
+static const struct dwc_eth_dwmac_data eswin_qos_data = {
+	.probe = eswin_qos_probe,
+	.stmmac_clk_name = "stmmaceth",
+};
+
+static int dwc_eth_dwmac_probe(struct platform_device *pdev)
+{
+	const struct dwc_eth_dwmac_data *data;
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	int ret;
+
+	data = device_get_match_data(&pdev->dev);
+
+	memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
+
+	/**
+	 * Since stmmac_platform supports name IRQ only, basic platform
+	 * resource initialization is done in the glue logic.
+	 */
+	stmmac_res.irq = platform_get_irq(pdev, 0);
+	if (stmmac_res.irq < 0)
+		return stmmac_res.irq;
+	stmmac_res.wol_irq = stmmac_res.irq;
+
+	stmmac_res.addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(stmmac_res.addr))
+		return PTR_ERR(stmmac_res.addr);
+
+	plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return PTR_ERR(plat_dat);
+
+	plat_dat->stmmac_clk = dwc_eth_find_clk(plat_dat,
+						data->stmmac_clk_name);
+
+	if (data->probe)
+		ret = data->probe(pdev, plat_dat, &stmmac_res);
+	if (ret < 0) {
+		return dev_err_probe(&pdev->dev, ret,
+				"failed to probe subdriver\n");
+	}
+
+	ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				"Failed to config dt\n");
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				"Failed to driver probe\n");
+
+	return ret;
+}
+
+static const struct of_device_id dwc_eth_dwmac_match[] = {
+	{ .compatible = "eswin,eic7700-qos-eth", .data = &eswin_qos_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
+
+static struct platform_driver eic7700_eth_dwmac_driver = {
+	.probe  = dwc_eth_dwmac_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name           = "eic7700-eth-dwmac",
+		.pm             = &stmmac_pltfr_pm_ops,
+		.of_match_table = dwc_eth_dwmac_match,
+	},
+};
+module_platform_driver(eic7700_eth_dwmac_driver);
+
+MODULE_AUTHOR("Eswin");
+MODULE_AUTHOR("Shuang Liang <liangshuang@eswincomputing.com>");
+MODULE_AUTHOR("Shangjuan Wei <weishangjuan@eswincomputing.com>");
+MODULE_DESCRIPTION("Eswin eic7700 qos ethernet driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1

Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC

[Rob Herring (Arm)]

On Wed, 28 May 2025 12:15:58 +0800, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
> 
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, clock/reset control,
> and AXI bus parameter optimization.
> 
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
> ---
>  .../bindings/net/eswin,eic7700-eth.yaml       | 200 ++++++++++++++++++
>  1 file changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/eswin,eic7700-eth.example.dtb: ethernet@50400000 (eswin,eic7700-qos-eth): 'eswin,dly_hsp_reg', 'eswin,hsp_sp_csr', 'eswin,syscrg_csr' do not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pciclass|pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^IBM,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aoly,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^ariaboard,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blaize,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csot,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^econet,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emcraft,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^gocontroll,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liontron,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netcube,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^nothing,.*', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pegatron,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pinctrl-[0-9]+$', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^pri,.*', '^primeview,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^retronix,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartfiber,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tcu,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^techwell,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^ultratronik,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winsen,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^yuridenki,.*', '^yuzukihd,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*'
	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250528041558.895-1-weishangjuan@eswincomputing.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller

[Michal Swiatkowski]

On Wed, May 28, 2025 at 12:14:42PM +0800, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
> 
> Updates:
> 
>   dt-bindings: ethernet: eswin: Document for EIC7700 SoC
>   v1 -> v2:
>     1. Remove the code related to PHY LED configuration from the MAC driver.
>     2. Use phylib instead of the GPIO API in the driver to implement the PHY reset function.
>     3. Align with the latest stmmac API, use the API provided by stmmac helper to refactor the driver,
>        and replace or remove duplicate code.
>     4. Adjust the code format and driver interfaces, such as replacing kzalloc with devm_kzalloc, etc.
> 
>   ethernet: eswin: Add eic7700 ethernet driver
>   v1 -> v2:
>     1. Significant errors have been corrected in the email reply for version v1.
>     2. Add snps,dwmac.
>     3. Chang the names of reset-names and phy-mode.
>     4. Add descriptions of eswin, hsp_sp_csr, eswin, syscrg.csr, eswin, dly_hsp.reg.
> 
>   Regarding the question about delay parameters in the previous email reply, the explanation is as follows:
>     Dly_hsp_reg: Configure the delay compensation register between MAC/PHY;
>     Dly_param_ *: The value written to the dly_hsp_reg register at a rate of 1000/100/10, which varies due 
>                   to the routing of the board;
> 
>   In addition, your bot found errors running 'make dt_binding_check' on our patch about yamllint warnings/errors,
>   it looks like the validation failure is because missing eswin entry in vendor-prefixes.yaml. 
>   When we run "make dt_binding_check", we get the same error. We have already added 'eswin' in the vendor-prefixes.yaml 
>   file before, and the code has mentioned the community, but you have not yet integrated it.

Usualy description is above the changelog. Please try to follow 72 line
length rule.

net-next is closed, you should resend it when open (after June 9th) [1]

[1] https://lore.kernel.org/netdev/20250527191710.7d94a61c@kernel.org/T/#m0bc90575288f5f1bcf5e50ecff59fb904b79505c

> 
> Shangjuan Wei (2):
>   dt-bindings: ethernet: eswin: Document for EIC7700 SoC
>   ethernet: eswin: Add eic7700 ethernet driver
> 
>  .../bindings/net/eswin,eic7700-eth.yaml       | 200 +++++++++
>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>  .../ethernet/stmicro/stmmac/dwmac-eic7700.c   | 410 ++++++++++++++++++
>  4 files changed, 622 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> 
> -- 
> 2.17.1

Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC

[Krzysztof Kozlowski]

On 28/05/2025 06:15, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
> 
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, clock/reset control,
> and AXI bus parameter optimization.
> 
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
> ---
>  .../bindings/net/eswin,eic7700-eth.yaml       | 200 ++++++++++++++++++
>  1 file changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> new file mode 100644
> index 000000000000..c76d2fbf0ebd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> @@ -0,0 +1,200 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/eswin,eic7700-eth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Eswin EIC7700 SOC Eth Controller
> +
> +maintainers:
> +  - Shuang Liang <liangshuang@eswincomputing.com>
> +  - Zhi Li <lizhi2@eswincomputing.com>
> +  - Shangjuan Wei <weishangjuan@eswincomputing.com>
> +
> +description:
> +  The eth controller registers are part of the syscrg block on
> +  the EIC7700 SoC.
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - eswin,eic7700-qos-eth
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: eswin,eic7700-qos-eth
> +      - const: snps,dwmac
> +
> +  reg:
> +    description: Base address and size of the Ethernet controller registers

Drop description, wasn't here.

> +    items:
> +      - description: Register base address
> +      - description: Register size

The second item makes no sense. Size is part of one entry. Looking at
your example you have only one item. Last time you said this is
extension region, so I really do not understand what is happening here.


> +
> +  interrupt-names:
> +    const: macirq
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  phy-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - rgmii
> +      - rgmii-rxid
> +      - rgmii-txid
> +      - rgmii-id
> +
> +  phy-handle:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference to the PHY device
> +
> +  clocks:
> +    minItems: 3
> +    maxItems: 7

No improvements. Read feedback given to other eswin patches. This cannot
be flexible.


> +
> +  clock-names:
> +    minItems: 3
> +    contains:
> +      enum:
> +        - app
> +        - stmmaceth
> +        - tx
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: stmmaceth
> +
> +  dma-noncoherent: true
> +
> +  # Custom properties

Drop

> +  eswin,hsp_sp_csr:

Nothing improved. Eswin upstreamed several bindings at once with same
issues. I commented on multiple of them, but here I stopped commenting
asking you to fix the same things I asked your colleagues. The point is
that it is beneficial if you learn together, not each repeat same
mistake and receive same feedback from reviewer.

Follow DTS coding style naming for property.


> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: HSP peripheral control register area

What is HSP?

> +          - description: Control registers (such as used to select TX clock
> +                         source, PHY interface mode)
> +          - description: AXI bus low-power control related registers
> +          - description: Status register
> +    description:
> +      Configure TX clock source selection, set PHY interface mode,
> +      and control low-power bus behavior
> +
> +  eswin,syscrg_csr:

Nothing improved.

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: CRG's device tree node handle

What is CRG? Anyway, use the same style. If previously phandle is
control register area, why this is phandle? Drop redundant parts,
because this cannot be anything else than phandle, and describe what is
the device you are pointing here.

> +          - description: Enable and divide HSP ACLK control
> +          - description: Behavior of configuring HSP controller
> +    description:
> +      Register that accesses the system clock controller.
> +      Used to configure HSP clocks for Ethernet subsystems
> +
> +  eswin,dly_hsp_reg:

Nothing improved.

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: TX delay control register offset
> +          - description: RX delay control register offset

No... These are not phandles. Look at your example - where is a phandle
there?

> +          - description: Switch for controlling delay function
> +    description:
> +      Register for configuring delay compensation between MAC/PHY
> +
> +  snps,axi-config:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: AXI bus configuration

Do not redefine properties. Include proper ref and fix your
additionalProps to unevaluateadProperties.

> +
> +  stmmac-axi-config:

Don't duplicate with snps dwmac.

> +    type: object
> +    unevaluatedProperties: false
> +    properties:
> +      snps,blen:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: Set the burst transmission length of AXI bus
> +      snps,rd_osr_lmt:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Set OSR restrictions for read operations
> +      snps,wr_osr_lmt:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Set OSR restrictions for write operations
> +      snps,lpi_en:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: Low Power Interface enable flag (true/false)
> +    required:
> +      - snps,blen
> +      - snps,rd_osr_lmt
> +      - snps,wr_osr_lmt
> +      - snps,lpi_en
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-names
> +  - interrupts
> +  - phy-mode
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - eswin,hsp_sp_csr
> +  - eswin,syscrg_csr
> +  - eswin,dly_hsp_reg
> +  - snps,axi-config
> +
> +additionalProperties: false

unevaluatedProperties instead

> +
> +examples:
> +  - |
> +    gmac0: ethernet@50400000 {
> +        compatible = "eswin,eic7700-qos-eth", "snps,dwmac";
> +        reg = <0x0 0x50400000 0x0 0x10000>;
> +        interrupt-parent = <&plic>;
> +        interrupt-names = "macirq";
> +        interrupts = <61>;
> +        phy-mode = "rgmii-txid";
> +        phy-handle = <&phy0>;
> +        status = "okay";

Drop

> +        clocks = <&clock 550>,
> +                 <&clock 551>,
> +                 <&clock 552>;
> +        clock-names = "app", "stmmaceth", "tx";
> +        resets = <&reset 0x07 (1 << 26)>;
> +        reset-names = "stmmaceth";
> +        dma-noncoherent;
> +        eswin,hsp_sp_csr = <&hsp_sp_csr 0x1030 0x100 0x108>;
> +        eswin,syscrg_csr = <&sys_crg 0x148 0x14c>;
> +        eswin,dly_hsp_reg = <0x114 0x118 0x11c>;
> +        snps,axi-config = <&stmmac_axi_setup>;
> +        stmmac_axi_setup: stmmac-axi-config {
> +            snps,blen = <0 0 0 0 16 8 4>;
> +            snps,rd_osr_lmt = <2>;
> +            snps,wr_osr_lmt = <2>;
> +            snps,lpi_en;
> +        };
> +        /* mdio {

Don't post dead code.

> +          compatible = "snps,dwmac-mdio";
> +          status = "okay";

Drop

> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          phy0: ethernet-phy@0 {
> +            device_type = "ethernet-phy";
> +            reg = <0>;
> +            compatible = "ethernet-phy-id001c.c916", "realtek,rtl8211f";
> +          };
> +        }; */
> +    };


Best regards,
Krzysztof

Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver

[Krzysztof Kozlowski]

On 28/05/2025 06:16, weishangjuan@eswincomputing.com wrote:
> +static int eswin_qos_probe(struct platform_device *pdev,
> +			   struct plat_stmmacenet_data *plat_dat,
> +			   struct stmmac_resources *stmmac_res)
> +{
> +	struct eswin_qos_priv *dwc_priv;
> +	u32 hsp_aclk_ctrl_offset;
> +	u32 hsp_aclk_ctrl_regset;
> +	u32 hsp_cfg_ctrl_offset;
> +	u32 eth_axi_lp_ctrl_offset;
> +	u32 eth_phy_ctrl_offset;
> +	u32 eth_phy_ctrl_regset;
> +	struct clk *clk_app;
> +	int ret;
> +	int err;
> +
> +	dwc_priv = devm_kzalloc(&pdev->dev, sizeof(*dwc_priv), GFP_KERNEL);
> +	if (!dwc_priv)
> +		return -ENOMEM;
> +
> +	if (device_property_read_u32(&pdev->dev, "id", &dwc_priv->dev_id))

NAK, you cannot have undocumented ABI. You did not test your DTS.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC

[Andrew Lunn]

> +examples:
> +  - |
> +    gmac0: ethernet@50400000 {
> +        compatible = "eswin,eic7700-qos-eth", "snps,dwmac";
> +        reg = <0x0 0x50400000 0x0 0x10000>;
> +        interrupt-parent = <&plic>;
> +        interrupt-names = "macirq";
> +        interrupts = <61>;
> +        phy-mode = "rgmii-txid";

Does the PCB has extra long clock lines in one direction? That is very
odd.

https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1

	Andrew

Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver

[Andrew Lunn]

> +/* PHY default addr in mdio*/
> +#define PHY_ADDR				-1

PHY addresses are 0 to 31. How can -1 be a default?

> +static struct clk *dwc_eth_find_clk(struct plat_stmmacenet_data *plat_dat,
> +				    const char *name)
> +{
> +	for (int i = 0; i < plat_dat->num_clks; i++)
> +		if (strcmp(plat_dat->clks[i].id, name) == 0)
> +			return plat_dat->clks[i].clk;
> +
> +	return NULL;
> +}

Please look at the cleanup work Russell King has been doing the last
couple of months. Is this still needed?

> +static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
> +				   struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct device *dev = &pdev->dev;
> +	u32 burst_map = 0;
> +	u32 bit_index = 0;
> +	u32 a_index = 0;
> +
> +	if (!plat_dat->axi) {
> +		plat_dat->axi = devm_kzalloc(&pdev->dev, sizeof(struct stmmac_axi), GFP_KERNEL);
> +
> +		if (!plat_dat->axi)
> +			return -ENOMEM;
> +	}
> +
> +	plat_dat->axi->axi_lpi_en = device_property_read_bool(dev,
> +							      "snps,en-lpi");
> +	if (device_property_read_u32(dev, "snps,write-requests",
> +				     &plat_dat->axi->axi_wr_osr_lmt)) {
> +		/**
> +		 * Since the register has a reset value of 1, if property
> +		 * is missing, default to 1.
> +		 */

Is that described in the binding? Please fully describe all the DT
properties, including what happens when they are not present.

> +	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,phyaddr", 0,
> +					 &dwc_priv->phyaddr);
> +	if (ret)
> +		dev_warn(&pdev->dev, "can't get phyaddr (%d)\n", ret);

Are we talking about the Ethernet PHY here or a generic PHY? You
should not need any vendor properties for an Ethernet phy, phy-handle
points to the PHY on an MDIO bus.

> +	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-1000m",
> +						  &dwc_priv->dly_param_1000m[0], 3, 0);
> +	if (ret != 3) {
> +		dev_err(&pdev->dev, "can't get delay param for 1Gbps mode (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-100m",
> +						  &dwc_priv->dly_param_100m[0], 3, 0);
> +	if (ret != 3) {
> +		dev_err(&pdev->dev, "can't get delay param for 100Mbps mode (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-10m",
> +						  &dwc_priv->dly_param_10m[0], 3, 0);
> +	if (ret != 3) {
> +		dev_err(&pdev->dev, "can't get delay param for 10Mbps mode (%d)\n", ret);
> +		return ret;
> +	}

        rx-internal-delay-ps:
          description:
            RGMII Receive Clock Delay defined in pico seconds. This is used for
            controllers that have configurable RX internal delays. If this
            property is present then the MAC applies the RX delay.
        tx-internal-delay-ps:
          description:
            RGMII Transmit Clock Delay defined in pico seconds. This is used for
            controllers that have configurable TX internal delays. If this
            property is present then the MAC applies the TX delay.

The RGMII standard only talks about 2ns delay. There is no delay per
link speed. This is something specific to your hardware. Please figure
out how you can map these standard properties to what you need for
100Mbps and 10Mbps.

    Andrew

---
pw-bot: cr

Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver

[Russell King (Oracle)]

On Wed, May 28, 2025 at 12:16:25PM +0800, weishangjuan@eswincomputing.com wrote:
> +static struct clk *dwc_eth_find_clk(struct plat_stmmacenet_data *plat_dat,
> +				    const char *name)
> +{
> +	for (int i = 0; i < plat_dat->num_clks; i++)
> +		if (strcmp(plat_dat->clks[i].id, name) == 0)
> +			return plat_dat->clks[i].clk;
> +
> +	return NULL;
> +}

Okay, I think this driver is mindless copying of dwmac-dwc-qos-eth.c
between 24th February and 9th April 2025. I can say this because I added
this function to that driver and later removed it.

Looking at the rest of the code, I doubt this even does anything useful
(hence "mindless copying") as you're not fetching any clocks into this
array, and plat_dat->num_clks will be zero here. Thus, this will return
NULL. Therefore, you haven't thought about whether you need this or not,
but have just copied dwmac-dwc-qos-eth.c and then modified it until it
works for you.

You haven't acknowledged where you derived this code from - you've cut
the header of your source file out, and basically are claiming it to be
all your own work. I know this is rubbish for the reason I've stated
above. This is quite simply plagiarism. I am not impressed.

Thus I will end the review here, and simply state that this is not
acceptable.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver

[kernel test robot]

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/main net/main linus/master v6.15 next-20250528]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/weishangjuan-eswincomputing-com/dt-bindings-ethernet-eswin-Document-for-EIC7700-SoC/20250528-121947
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250528041634.912-1-weishangjuan%40eswincomputing.com
patch subject: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250529/202505290202.daQ8Q8Xq-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505290202.daQ8Q8Xq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505290202.daQ8Q8Xq-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:210:6: warning: unused variable 'err' [-Wunused-variable]
     210 |         int err;
         |             ^~~
>> drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:369:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     369 |         if (data->probe)
         |             ^~~~~~~~~~~
   drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:371:6: note: uninitialized use occurs here
     371 |         if (ret < 0) {
         |             ^~~
   drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:369:2: note: remove the 'if' if its condition is always true
     369 |         if (data->probe)
         |         ^~~~~~~~~~~~~~~~
     370 |                 ret = data->probe(pdev, plat_dat, &stmmac_res);
   drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:343:9: note: initialize the variable 'ret' to silence this warning
     343 |         int ret;
         |                ^
         |                 = 0
   drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c:153:12: warning: unused function 'dwc_qos_probe' [-Wunused-function]
     153 | static int dwc_qos_probe(struct platform_device *pdev,
         |            ^~~~~~~~~~~~~
   3 warnings generated.


vim +/err +210 drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c

   196	
   197	static int eswin_qos_probe(struct platform_device *pdev,
   198				   struct plat_stmmacenet_data *plat_dat,
   199				   struct stmmac_resources *stmmac_res)
   200	{
   201		struct eswin_qos_priv *dwc_priv;
   202		u32 hsp_aclk_ctrl_offset;
   203		u32 hsp_aclk_ctrl_regset;
   204		u32 hsp_cfg_ctrl_offset;
   205		u32 eth_axi_lp_ctrl_offset;
   206		u32 eth_phy_ctrl_offset;
   207		u32 eth_phy_ctrl_regset;
   208		struct clk *clk_app;
   209		int ret;
 > 210		int err;
   211	
   212		dwc_priv = devm_kzalloc(&pdev->dev, sizeof(*dwc_priv), GFP_KERNEL);
   213		if (!dwc_priv)
   214			return -ENOMEM;
   215	
   216		if (device_property_read_u32(&pdev->dev, "id", &dwc_priv->dev_id))
   217			return dev_err_probe(&pdev->dev, -EINVAL,
   218					"Can not read device id!\n");
   219	
   220		dwc_priv->dev = &pdev->dev;
   221	
   222		ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,phyaddr", 0,
   223						 &dwc_priv->phyaddr);
   224		if (ret)
   225			dev_warn(&pdev->dev, "can't get phyaddr (%d)\n", ret);
   226	
   227		ret = of_property_read_variable_u32_array(pdev->dev.of_node, "eswin,dly_hsp_reg",
   228							  &dwc_priv->dly_hsp_reg[0], 3, 0);
   229		if (ret != 3) {
   230			dev_err(&pdev->dev, "can't get delay hsp reg.ret(%d)\n", ret);
   231			return ret;
   232		}
   233	
   234		ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-1000m",
   235							  &dwc_priv->dly_param_1000m[0], 3, 0);
   236		if (ret != 3) {
   237			dev_err(&pdev->dev, "can't get delay param for 1Gbps mode (%d)\n", ret);
   238			return ret;
   239		}
   240	
   241		ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-100m",
   242							  &dwc_priv->dly_param_100m[0], 3, 0);
   243		if (ret != 3) {
   244			dev_err(&pdev->dev, "can't get delay param for 100Mbps mode (%d)\n", ret);
   245			return ret;
   246		}
   247	
   248		ret = of_property_read_variable_u32_array(pdev->dev.of_node, "dly-param-10m",
   249							  &dwc_priv->dly_param_10m[0], 3, 0);
   250		if (ret != 3) {
   251			dev_err(&pdev->dev, "can't get delay param for 10Mbps mode (%d)\n", ret);
   252			return ret;
   253		}
   254	
   255		dwc_priv->crg_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
   256								       "eswin,syscrg_csr");
   257		if (IS_ERR(dwc_priv->crg_regmap)) {
   258			dev_dbg(&pdev->dev, "No syscrg_csr phandle specified\n");
   259			return 0;
   260		}
   261	
   262		ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 1,
   263						 &hsp_aclk_ctrl_offset);
   264		if (ret)
   265			return dev_err_probe(&pdev->dev, ret, "can't get syscrg_csr 1\n");
   266	
   267		regmap_read(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, &hsp_aclk_ctrl_regset);
   268		hsp_aclk_ctrl_regset |= (HSP_ACLK_CLKEN | HSP_ACLK_DIVSOR);
   269		regmap_write(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, hsp_aclk_ctrl_regset);
   270	
   271		ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 2,
   272						 &hsp_cfg_ctrl_offset);
   273		if (ret)
   274			return dev_err_probe(&pdev->dev, ret, "can't get syscrg_csr 2\n");
   275	
   276		regmap_write(dwc_priv->crg_regmap, hsp_cfg_ctrl_offset, HSP_CFG_CTRL_REGSET);
   277	
   278		dwc_priv->hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
   279								       "eswin,hsp_sp_csr");
   280		if (IS_ERR(dwc_priv->hsp_regmap)) {
   281			dev_dbg(&pdev->dev, "No hsp_sp_csr phandle specified\n");
   282			return 0;
   283		}
   284	
   285		ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 2,
   286						 &eth_phy_ctrl_offset);
   287		if (ret)
   288			return dev_err_probe(&pdev->dev, ret, "can't get hsp_sp_csr 2\n");
   289	
   290		regmap_read(dwc_priv->hsp_regmap,
   291			    eth_phy_ctrl_offset,
   292			    &eth_phy_ctrl_regset);
   293		eth_phy_ctrl_regset |= (ETH_TX_CLK_SEL | ETH_PHY_INTF_SELI);
   294		regmap_write(dwc_priv->hsp_regmap,
   295			     eth_phy_ctrl_offset,
   296			     eth_phy_ctrl_regset);
   297	
   298		ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 3,
   299						 &eth_axi_lp_ctrl_offset);
   300		if (ret)
   301			return dev_err_probe(&pdev->dev, ret,
   302					"can't get hsp_sp_csr 3\n");
   303	
   304		regmap_write(dwc_priv->hsp_regmap,
   305			     eth_axi_lp_ctrl_offset,
   306			     ETH_CSYSREQ_VAL);
   307	
   308		clk_app = devm_clk_get_enabled(&pdev->dev, "app");
   309		if (IS_ERR(clk_app))
   310			return dev_err_probe(&pdev->dev, PTR_ERR(clk_app),
   311					"error getting app clock\n");
   312	
   313		plat_dat->clk_tx_i = devm_clk_get_enabled(&pdev->dev, "tx");
   314		if (IS_ERR(plat_dat->clk_tx_i))
   315			return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat->clk_tx_i),
   316					"error getting tx clock\n");
   317	
   318		plat_dat->fix_mac_speed = eswin_qos_fix_speed;
   319		plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
   320		plat_dat->bsp_priv = dwc_priv;
   321		plat_dat->phy_addr = PHY_ADDR;
   322	
   323		return 0;
   324	}
   325	
   326	struct dwc_eth_dwmac_data {
   327		int (*probe)(struct platform_device *pdev,
   328			     struct plat_stmmacenet_data *plat_dat,
   329			     struct stmmac_resources *res);
   330		const char *stmmac_clk_name;
   331	};
   332	
   333	static const struct dwc_eth_dwmac_data eswin_qos_data = {
   334		.probe = eswin_qos_probe,
   335		.stmmac_clk_name = "stmmaceth",
   336	};
   337	
   338	static int dwc_eth_dwmac_probe(struct platform_device *pdev)
   339	{
   340		const struct dwc_eth_dwmac_data *data;
   341		struct plat_stmmacenet_data *plat_dat;
   342		struct stmmac_resources stmmac_res;
   343		int ret;
   344	
   345		data = device_get_match_data(&pdev->dev);
   346	
   347		memset(&stmmac_res, 0, sizeof(struct stmmac_resources));
   348	
   349		/**
   350		 * Since stmmac_platform supports name IRQ only, basic platform
   351		 * resource initialization is done in the glue logic.
   352		 */
   353		stmmac_res.irq = platform_get_irq(pdev, 0);
   354		if (stmmac_res.irq < 0)
   355			return stmmac_res.irq;
   356		stmmac_res.wol_irq = stmmac_res.irq;
   357	
   358		stmmac_res.addr = devm_platform_ioremap_resource(pdev, 0);
   359		if (IS_ERR(stmmac_res.addr))
   360			return PTR_ERR(stmmac_res.addr);
   361	
   362		plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
   363		if (IS_ERR(plat_dat))
   364			return PTR_ERR(plat_dat);
   365	
   366		plat_dat->stmmac_clk = dwc_eth_find_clk(plat_dat,
   367							data->stmmac_clk_name);
   368	
 > 369		if (data->probe)
   370			ret = data->probe(pdev, plat_dat, &stmmac_res);
   371		if (ret < 0) {
   372			return dev_err_probe(&pdev->dev, ret,
   373					"failed to probe subdriver\n");
   374		}
   375	
   376		ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
   377		if (ret)
   378			return dev_err_probe(&pdev->dev, ret,
   379					"Failed to config dt\n");
   380	
   381		ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
   382		if (ret)
   383			return dev_err_probe(&pdev->dev, ret,
   384					"Failed to driver probe\n");
   385	
   386		return ret;
   387	}
   388	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

[李志]

Dear Andrew Lunn,
Thank you for your professional and valuable suggestions.
We have carefully reviewed your comments and made the corresponding changes. Could you please help us evaluate whether the updates we mentioned in our previous email properly address your concerns and meet the expected standards?
At the same time, we still have some questions that need clarification. We have included these in the original email — we would appreciate it if you could also take a moment to review those points.


@Krzysztof Kozlowski
We noticed your review comment on the following page, but we did not receive it via email:
🔗 https://lore.kernel.org/lkml/f096afa1-260e-4f8c-8595-3b41425b2964@kernel.org/
Thank you for your professional feedback on our Ethernet driver.
Regarding the issue you raised:
"There is no such property. I already said at v2 you cannot have undocumented ABI."
Upon rechecking, we realized that during verification, we mistakenly used underscore-separated property names in the driver, while dash-separated names were used in the YAML bindings. We have now synchronized the property naming.
Could you please confirm if this mismatch was the root cause of your concern?


Best regards,

Li Zhi
Eswin Computing

> -----原始邮件-----
> 发件人: "Andrew Lunn" <andrew@lunn.ch>
> 发送时间:2025-07-04 00:12:29 (星期五)
> 收件人: weishangjuan@eswincomputing.com
> 抄送: 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, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, yong.liang.choong@linux.intel.com, vladimir.oltean@nxp.com, jszhang@kernel.org, jan.petrous@oss.nxp.com, prabhakar.mahadev-lad.rj@bp.renesas.com, inochiama@gmail.com, boon.khai.ng@altera.com, dfustini@tenstorrent.com, 0x1207@gmail.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, ningyu@eswincomputing.com, linmin@eswincomputing.com, lizhi2@eswincomputing.com
> 主题: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
> 
> > +/* Default delay value*/
> > +#define EIC7700_DELAY_VALUE0 0x20202020
> > +#define EIC7700_DELAY_VALUE1 0x96205A20
> 
> We need a better explanation of what is going on here. What do these
> numbers mean?
> 

In response to your suggestion, we added the following more detailed comments to the code. Is this appropriate?
+/*
+ * Default delay register values for different signals:
+ *
+ * EIC7700_DELAY_VALUE0: Used for TXD and RXD signals delay configuration.
+ * Bits layout:
+ *   Byte 0 (bits 0-7)   : TXD0 / RXD0 delay (0x20 = 3.2 ns)
+ *   Byte 1 (bits 8-15)  : TXD1 / RXD1 delay (0x20 = 3.2 ns)
+ *   Byte 2 (bits 16-23) : TXD2 / RXD2 delay (0x20 = 3.2 ns)
+ *   Byte 3 (bits 24-31) : TXD3 / RXD3 delay (0x20 = 3.2 ns)
+ *
+ * EIC7700_DELAY_VALUE1: Used for control signals delay configuration.
+ * Bits layout:
+ *   Bits 0-6     : TXEN delay
+ *   Bits 8-14    : TXCLK delay
+ *   Bit  15      : TXCLK invert (1 = invert)
+ *   Bits 16-22   : RXDV delay
+ *   Bits 24-30   : RXCLK delay
+ *   Bit  31      : RXCLK invert (1 = invert)
+ */
+#define EIC7700_DELAY_VALUE0 0x20202020
+#define EIC7700_DELAY_VALUE1 0x96205A20

> > +	dwc_priv->dly_param_1000m[0] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_1000m[1] = EIC7700_DELAY_VALUE1;
> > +	dwc_priv->dly_param_1000m[2] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_100m[0] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_100m[1] = EIC7700_DELAY_VALUE1;
> > +	dwc_priv->dly_param_100m[2] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_10m[0] = 0x0;
> > +	dwc_priv->dly_param_10m[1] = 0x0;
> > +	dwc_priv->dly_param_10m[2] = 0x0;
> 
> What are the three different values for?

In response to your question, we have added the following more detailed comments to the code. Is this appropriate?
+        /* Initialize default delay parameters for 1000Mbps and 100Mbps speeds */
+        dwc_priv->dly_param_1000m[0] = EIC7700_DELAY_VALUE0; /* TXD delay */
+        dwc_priv->dly_param_1000m[1] = EIC7700_DELAY_VALUE1; /* Control signals delay */
+        dwc_priv->dly_param_1000m[2] = EIC7700_DELAY_VALUE0; /* RXD delay */
+        dwc_priv->dly_param_100m[0] = EIC7700_DELAY_VALUE0;
+        dwc_priv->dly_param_100m[1] = EIC7700_DELAY_VALUE1;
+        dwc_priv->dly_param_100m[2] = EIC7700_DELAY_VALUE0;
+        /* For 10Mbps, no delay by default */
+        dwc_priv->dly_param_10m[0] = 0x0;
+        dwc_priv->dly_param_10m[1] = 0x0;
+        dwc_priv->dly_param_10m[2] = 0x0;

> 
> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps",
> > +				   &dwc_priv->rx_delay_ps);
> > +	if (ret)
> > +		dev_dbg(&pdev->dev, "can't get rx-internal-delay-ps, ret(%d).", ret);
> > +	else
> > +		has_rx_dly = true;
> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "tx-internal-delay-ps",
> > +				   &dwc_priv->tx_delay_ps);
> > +	if (ret)
> > +		dev_dbg(&pdev->dev, "can't get tx-internal-delay-ps, ret(%d).", ret);
> > +	else
> > +		has_tx_dly = true;
> > +	if (has_rx_dly && has_tx_dly)
> 
> What if i only to set a TX delay? I want the RX delay to default to
> 0ps.
> 

Regarding your question, we have added default values ​​for tx delay and rx delay in the code, and as long as one of the two delays is configured in DTS, the original configuration can be overwritten. Does this process meet your suggestion?
+        /* Default delays in picoseconds */
+        dwc_priv->rx_delay_ps = 0;
+        dwc_priv->tx_delay_ps = 0;
+
+        if (has_rx_dly || has_tx_dly) {

> {
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_1000m[1]);
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_100m[1]);
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_10m[1]);
> > +	} else {
> > +		dev_dbg(&pdev->dev, " use default dly\n");
> 
> What is the default? It should be 0ps. So there is no point printing
> this message.
> 

Our original strategy was to use the value used when initializing dly_param_*m as the default value, so should we continue to follow your suggestion and use 0ps as the default value?

> 	Andrew

Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

[李志]

Dear Andrew Lunn,
Thank you for your professional and valuable suggestions.
Our questions are embedded below your comments in the original email below.


Best regards,

Li Zhi
Eswin Computing


> -----原始邮件-----
> 发件人: "Andrew Lunn" <andrew@lunn.ch>
> 发送时间:2025-07-04 00:12:29 (星期五)
> 收件人: weishangjuan@eswincomputing.com
> 抄送: 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, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, yong.liang.choong@linux.intel.com, vladimir.oltean@nxp.com, jszhang@kernel.org, jan.petrous@oss.nxp.com, prabhakar.mahadev-lad.rj@bp.renesas.com, inochiama@gmail.com, boon.khai.ng@altera.com, dfustini@tenstorrent.com, 0x1207@gmail.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, ningyu@eswincomputing.com, linmin@eswincomputing.com, lizhi2@eswincomputing.com
> 主题: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
> 
> > +/* Default delay value*/
> > +#define EIC7700_DELAY_VALUE0 0x20202020
> > +#define EIC7700_DELAY_VALUE1 0x96205A20
> 
> We need a better explanation of what is going on here. What do these
> numbers mean?
> 

Let me clarify:
  EIC7700_DELAY_VALUE0 (0x20202020) is used to configure delay taps for TXD[3:0] signals. Each byte represents the delay value for one data line.
  EIC7700_DELAY_VALUE1 (0x96205A20) configures control signal delays, such as TX_EN, RX_DV, and others. Again, each byte corresponds to a specific signal line.
More detailed inline comments will be added in the next patch to explain the bit layout and purpose of each byte in these default values. Is this understanding correct?

> > +	dwc_priv->dly_param_1000m[0] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_1000m[1] = EIC7700_DELAY_VALUE1;
> > +	dwc_priv->dly_param_1000m[2] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_100m[0] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_100m[1] = EIC7700_DELAY_VALUE1;
> > +	dwc_priv->dly_param_100m[2] = EIC7700_DELAY_VALUE0;
> > +	dwc_priv->dly_param_10m[0] = 0x0;
> > +	dwc_priv->dly_param_10m[1] = 0x0;
> > +	dwc_priv->dly_param_10m[2] = 0x0;
> 
> What are the three different values for?
> 

Let me clarify the purpose of the three elements in each dly_param_* array:
  dly_param_[x][0]: Delay configuration for TXD signals
  dly_param_[x][1]: Delay configuration for control signals (e.g., TX_EN, RX_DV, RX_CLK)
  dly_param_[x][2]: Delay configuration for RXD signals
These values are defined separately for different link speeds: 1000 Mbps, 100 Mbps, and 10 Mbps. During PHY initialization or when the link speed changes, the corresponding delay parameters are selected and applied to the hardware registers.
Inline comments will be added in the next patch to clarify the meaning and usage of each element. Is this understanding correct?

> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps",
> > +				   &dwc_priv->rx_delay_ps);
> > +	if (ret)
> > +		dev_dbg(&pdev->dev, "can't get rx-internal-delay-ps, ret(%d).", ret);
> > +	else
> > +		has_rx_dly = true;
> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "tx-internal-delay-ps",
> > +				   &dwc_priv->tx_delay_ps);
> > +	if (ret)
> > +		dev_dbg(&pdev->dev, "can't get tx-internal-delay-ps, ret(%d).", ret);
> > +	else
> > +		has_tx_dly = true;
> > +	if (has_rx_dly && has_tx_dly)
> 
> What if i only to set a TX delay? I want the RX delay to default to
> 0ps.
> 

Yes, this can be handled separately by calling eic7700_set_rgmii_rx_dly() and eic7700_set_rgmii_tx_dly() in the next patch. Is this correct?

> {
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_1000m[1]);
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_100m[1]);
> > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > +				  &dwc_priv->dly_param_10m[1]);
> > +	} else {
> > +		dev_dbg(&pdev->dev, " use default dly\n");
> 
> What is the default? It should be 0ps. So there is no point printing
> this message.
> 

The default value is EIC7700_DELAY_VALUE1, which is used in the absence of the DTS attribute. The print message will be removed in the next patch. Is this correct?

> 	Andrew

Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

[李志]

Dear Krzysztof Kozlowski,

Thank you for your professional and valuable suggestions.
Our question is embedded below your comment.

Best regards,


Li Zhi
Eswin Computing

> Subject: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
> Date: Thu, 3 Jul 2025 11:53:33 +0200	[thread overview]
> Message-ID: <f096afa1-260e-4f8c-8595-3b41425b2964@kernel.org> (raw)
> In-Reply-To: <20250703092015.1200-1-weishangjuan@eswincomputing.com>
> On 03/07/2025 11:20, weishangjuan@eswincomputing.com wrote:
> > +	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 1,
> > +					 &hsp_aclk_ctrl_offset);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "can't get hsp_aclk_ctrl_offset\n");
> > +
> > +	regmap_read(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, &hsp_aclk_ctrl_regset);
> > +	hsp_aclk_ctrl_regset |= (EIC7700_HSP_ACLK_CLKEN | EIC7700_HSP_ACLK_DIVSOR);
> > +	regmap_write(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, hsp_aclk_ctrl_regset);
> > +
> > > +	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 2,
> > +					 &hsp_cfg_ctrl_offset);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "can't get hsp_cfg_ctrl_offset\n");
> > +
> > +	regmap_write(dwc_priv->crg_regmap, hsp_cfg_ctrl_offset, EIC7700_HSP_CFG_CTRL_REGSET);
> > +
> > +	dwc_priv->hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> > +							       "eswin,hsp_sp_csr");
> 
> There is no such property. I already said at v2 you cannot have
> undocumented ABI.
> 

The properties in the YAML file use dashes, while the driver uses underscores, resulting in an inconsistency. This will be corrected in the next patch. Is this correct?

> > +	if (IS_ERR(dwc_priv->hsp_regmap))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(dwc_priv->hsp_regmap),
> > +				"Failed to get hsp_sp_csr regmap\n");
> > +
> > +	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 2,
> 
> NAK
> 
> > +					 &eth_phy_ctrl_offset);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "can't get eth_phy_ctrl_offset\n");
> > +
> > +	regmap_read(dwc_priv->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(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, eth_phy_ctrl_regset);
> > +
> > +	ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 3,
> > +					 &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(dwc_priv->hsp_regmap, eth_axi_lp_ctrl_offset, EIC7700_ETH_CSYSREQ_VAL);
> > +
> > +	plat_dat->clk_tx_i = devm_clk_get_enabled(&pdev->dev, "tx");
> > +	if (IS_ERR(plat_dat->clk_tx_i))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat->clk_tx_i),
> > +				"error getting tx clock\n");
> > +
> > +	plat_dat->fix_mac_speed = eic7700_qos_fix_speed;
> > +	plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> > +	plat_dat->bsp_priv = dwc_priv;
> > +
> > +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret, "Failed to driver probe\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct of_device_id eic7700_dwmac_match[] = {
> > +	{ .compatible = "eswin,eic7700-qos-eth" },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, eic7700_dwmac_match);
> > +
> > +static struct platform_driver eic7700_dwmac_driver = {
> +	.probe  = eic7700_dwmac_probe,
> +	.remove = stmmac_pltfr_remove,
> +	.driver = {
> +		.name           = "eic7700-eth-dwmac",
> +		.pm             = &stmmac_pltfr_pm_ops,
> +		.of_match_table = eic7700_dwmac_match,
> +	},
> +};
> +module_platform_driver(eic7700_dwmac_driver);
> +
> +MODULE_AUTHOR("Eswin");
> 
> Drop, that's not a person.
> 
> 
> Best regards,
> Krzysztof

Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

[Andrew Lunn]

> > > +	dwc_priv->dly_param_1000m[0] = EIC7700_DELAY_VALUE0;
> > > +	dwc_priv->dly_param_1000m[1] = EIC7700_DELAY_VALUE1;
> > > +	dwc_priv->dly_param_1000m[2] = EIC7700_DELAY_VALUE0;
> > > +	dwc_priv->dly_param_100m[0] = EIC7700_DELAY_VALUE0;
> > > +	dwc_priv->dly_param_100m[1] = EIC7700_DELAY_VALUE1;
> > > +	dwc_priv->dly_param_100m[2] = EIC7700_DELAY_VALUE0;
> > > +	dwc_priv->dly_param_10m[0] = 0x0;
> > > +	dwc_priv->dly_param_10m[1] = 0x0;
> > > +	dwc_priv->dly_param_10m[2] = 0x0;
> > 
> > What are the three different values for?
> > 
> 
> Let me clarify the purpose of the three elements in each dly_param_* array:
>   dly_param_[x][0]: Delay configuration for TXD signals
>   dly_param_[x][1]: Delay configuration for control signals (e.g., TX_EN, RX_DV, RX_CLK)
>   dly_param_[x][2]: Delay configuration for RXD signals

Maybe add a #define or an enum for the index.

Do these delays represent the RGMII 2ns delay?

> > {
> > > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > > +				  &dwc_priv->dly_param_1000m[1]);
> > > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > > +				  &dwc_priv->dly_param_100m[1]);
> > > +		eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> > > +				  &dwc_priv->dly_param_10m[1]);
> > > +	} else {
> > > +		dev_dbg(&pdev->dev, " use default dly\n");
> > 
> > What is the default? It should be 0ps. So there is no point printing
> > this message.
> > 
> 
> The default value is EIC7700_DELAY_VALUE1

But what does EIC7700_DELAY_VALUE1 mean? It should mean 0ps? But i'm
not sure it does.

	Andrew