devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller
@ 2025-05-28  4:14 weishangjuan
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-28  4:14 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel
  Cc: ningyu, linmin, lizhi2, Shangjuan Wei

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


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

* [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  2025-05-28  4:14 [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
@ 2025-05-28  4:15 ` weishangjuan
  2025-05-28  5:21   ` Rob Herring (Arm)
                     ` (2 more replies)
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
  2025-05-28  5:24 ` [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller Michal Swiatkowski
  2 siblings, 3 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-28  4:15 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel
  Cc: ningyu, linmin, lizhi2, Shangjuan Wei

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


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

* [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  4:14 [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
@ 2025-05-28  4:16 ` weishangjuan
  2025-05-28  5:50   ` Krzysztof Kozlowski
                     ` (3 more replies)
  2025-05-28  5:24 ` [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller Michal Swiatkowski
  2 siblings, 4 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-28  4:16 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel
  Cc: ningyu, linmin, lizhi2, Shangjuan Wei

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


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

* Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
@ 2025-05-28  5:21   ` Rob Herring (Arm)
  2025-05-28  5:48   ` Krzysztof Kozlowski
  2025-05-28 13:34   ` Andrew Lunn
  2 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-05-28  5:21 UTC (permalink / raw)
  To: weishangjuan
  Cc: boon.khai.ng, linux-kernel, netdev, alexandre.torgue,
	yong.liang.choong, linmin, linux-stm32, jan.petrous,
	linux-arm-kernel, rmk+kernel, kuba, davem,
	prabhakar.mahadev-lad.rj, inochiama, pabeni, p.zabel, krzk+dt,
	devicetree, vladimir.oltean, mcoquelin.stm32, andrew+netdev,
	lizhi2, ningyu, jszhang, 0x1207, edumazet, conor+dt


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.


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

* Re: [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller
  2025-05-28  4:14 [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
@ 2025-05-28  5:24 ` Michal Swiatkowski
  2 siblings, 0 replies; 12+ messages in thread
From: Michal Swiatkowski @ 2025-05-28  5:24 UTC (permalink / raw)
  To: weishangjuan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel,
	ningyu, linmin, lizhi2

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

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

* Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
  2025-05-28  5:21   ` Rob Herring (Arm)
@ 2025-05-28  5:48   ` Krzysztof Kozlowski
  2025-05-28 13:34   ` Andrew Lunn
  2 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-28  5:48 UTC (permalink / raw)
  To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
	krzk+dt, conor+dt, netdev, devicetree, linux-kernel,
	mcoquelin.stm32, alexandre.torgue, vladimir.oltean, rmk+kernel,
	yong.liang.choong, prabhakar.mahadev-lad.rj, inochiama,
	jan.petrous, jszhang, p.zabel, 0x1207, boon.khai.ng, linux-stm32,
	linux-arm-kernel
  Cc: ningyu, linmin, lizhi2

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

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

* Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
@ 2025-05-28  5:50   ` Krzysztof Kozlowski
  2025-07-15 10:09     ` Re: [PATCH v3 " 李志
  2025-05-28 13:44   ` [PATCH v2 " Andrew Lunn
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-28  5:50 UTC (permalink / raw)
  To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
	krzk+dt, conor+dt, netdev, devicetree, linux-kernel,
	mcoquelin.stm32, alexandre.torgue, vladimir.oltean, rmk+kernel,
	yong.liang.choong, prabhakar.mahadev-lad.rj, inochiama,
	jan.petrous, jszhang, p.zabel, 0x1207, boon.khai.ng, linux-stm32,
	linux-arm-kernel
  Cc: ningyu, linmin, lizhi2

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

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

* Re: [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC
  2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
  2025-05-28  5:21   ` Rob Herring (Arm)
  2025-05-28  5:48   ` Krzysztof Kozlowski
@ 2025-05-28 13:34   ` Andrew Lunn
  2 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-05-28 13:34 UTC (permalink / raw)
  To: weishangjuan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel,
	ningyu, linmin, lizhi2

> +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

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

* Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
  2025-05-28  5:50   ` Krzysztof Kozlowski
@ 2025-05-28 13:44   ` Andrew Lunn
  2025-05-28 14:32   ` Russell King (Oracle)
  2025-05-28 18:37   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-05-28 13:44 UTC (permalink / raw)
  To: weishangjuan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, rmk+kernel, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel,
	ningyu, linmin, lizhi2

> +/* 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

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

* Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
  2025-05-28  5:50   ` Krzysztof Kozlowski
  2025-05-28 13:44   ` [PATCH v2 " Andrew Lunn
@ 2025-05-28 14:32   ` Russell King (Oracle)
  2025-05-28 18:37   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: Russell King (Oracle) @ 2025-05-28 14:32 UTC (permalink / raw)
  To: weishangjuan
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, vladimir.oltean, yong.liang.choong,
	prabhakar.mahadev-lad.rj, inochiama, jan.petrous, jszhang,
	p.zabel, 0x1207, boon.khai.ng, linux-stm32, linux-arm-kernel,
	ningyu, linmin, lizhi2

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!

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

* Re: [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
                     ` (2 preceding siblings ...)
  2025-05-28 14:32   ` Russell King (Oracle)
@ 2025-05-28 18:37   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-05-28 18:37 UTC (permalink / raw)
  To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
	krzk+dt, conor+dt, netdev, devicetree, linux-kernel,
	mcoquelin.stm32, alexandre.torgue, vladimir.oltean, rmk+kernel,
	yong.liang.choong, prabhakar.mahadev-lad.rj, inochiama,
	jan.petrous, jszhang, p.zabel, 0x1207, boon.khai.ng, linux-stm32,
	linux-arm-kernel
  Cc: llvm, oe-kbuild-all, ningyu, linmin, lizhi2, Shangjuan Wei

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

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

* Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
  2025-05-28  5:50   ` Krzysztof Kozlowski
@ 2025-07-15 10:09     ` 李志
  0 siblings, 0 replies; 12+ messages in thread
From: 李志 @ 2025-07-15 10:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: weishangjuan, andrew+netdev, davem, edumazet, kuba, robh, krzk+dt,
	conor+dt, netdev, devicetree, linux-kernel, mcoquelin.stm32,
	alexandre.torgue, rmk+kernel, yong.liang.choong, vladimir.oltean,
	jszhang, jan.petrous, prabhakar.mahadev-lad.rj, inochiama,
	boon.khai.ng, dfustini, 0x1207, linux-stm32, linux-arm-kernel,
	ningyu, linmin, pinkesh.vaghela

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

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

end of thread, other threads:[~2025-07-15 10:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28  4:14 [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
2025-05-28  4:15 ` [PATCH v2 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC weishangjuan
2025-05-28  5:21   ` Rob Herring (Arm)
2025-05-28  5:48   ` Krzysztof Kozlowski
2025-05-28 13:34   ` Andrew Lunn
2025-05-28  4:16 ` [PATCH v2 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
2025-05-28  5:50   ` Krzysztof Kozlowski
2025-07-15 10:09     ` Re: [PATCH v3 " 李志
2025-05-28 13:44   ` [PATCH v2 " Andrew Lunn
2025-05-28 14:32   ` Russell King (Oracle)
2025-05-28 18:37   ` kernel test robot
2025-05-28  5:24 ` [PATCH v2 0/2] Add driver support for Eswin eic7700 SoC ethernet controller Michal Swiatkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).