* [PATCH v1 0/2] Add driver support for Eswin eic7700 SoC ethernet controller
@ 2025-05-16 1:08 weishangjuan
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
0 siblings, 2 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-16 1:08 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
linux-stm32, linux-arm-kernel
Cc: ningyu, linmin, lizhi2, Shangjuan Wei
From: Shangjuan Wei <weishangjuan@eswincomputing.com>
Introduce a driver for the Eswin eic7700 series SoC ethernet controller,
adding support for the ethernet functionality in the Linux kernel. The
driver provides basic functionality to manage and control the ethernet
signals for the eic7700 series chips, which are part of the Eswin SoC family.
Supported chips:
Eswin eic7700 series SoC.
Test:
I tested this patch on the Sifive HiFive Premier P550 (which uses the EIC7700 SoC),
including system boot and ethernet.
Shangjuan Wei (2):
ethernet: eswin: Document for eic7700 SoC
ethernet: eswin: Add eic7700 ethernet driver
.../bindings/net/eswin,eic7700-eth.yaml | 142 +++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../ethernet/stmicro/stmmac/dwmac-eic7700.c | 521 ++++++++++++++++++
4 files changed, 675 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 v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:08 [PATCH v1 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
@ 2025-05-16 1:10 ` weishangjuan
2025-05-16 2:23 ` Rob Herring (Arm)
` (4 more replies)
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
1 sibling, 5 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-16 1:10 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
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, PHY LED configuration,
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 | 142 ++++++++++++++++++
1 file changed, 142 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..6cb9c109c036
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
@@ -0,0 +1,142 @@
+# 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.
+
+properties:
+ compatible:
+ const: eswin,eic7700-qos-eth
+
+ reg:
+ minItems: 1
+ items:
+ - description: Base address and size
+ - description: Extension region (optional)
+
+ interrupt-names:
+ const: macirq
+
+ interrupts:
+ maxItems: 1
+
+ phy-mode:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [mii, gmii, rgmii, rmii, sgmii]
+
+ id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Controller instance ID
+
+ clocks:
+ minItems: 3
+ maxItems: 7
+
+ clock-names:
+ minItems: 3
+ items:
+ - const: app
+ - const: stmmaceth
+ - const: tx
+ - const: slave_bus
+ - const: master_bus
+ - const: ptp_ref
+ - const: phy_ref_clk
+
+ resets:
+ maxItems: 1
+
+ reset-names:
+ items:
+ - const: ethrst
+
+ dma-noncoherent: true
+
+ # Custom properties
+ eswin,hsp_sp_csr:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: HSP SP control registers
+
+ eswin,syscrg_csr:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: System clock registers
+
+ eswin,dly_hsp_reg:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: HSP delay control registers
+
+ snps,axi-config:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: AXI bus configuration
+
+ stmmac-axi-config:
+ type: object
+ unevaluatedProperties: false
+ properties:
+ snps,lpi_en:
+ type: boolean
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: Low Power Interface enable flag (true/false)
+
+required:
+ - compatible
+ - reg
+ - interrupt-names
+ - interrupts
+ - phy-mode
+ - id
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - eswin,hsp_sp_csr
+ - eswin,syscrg_csr
+ - eswin,dly_hsp_reg
+ - snps,axi-config
+ - snps,blen
+ - snps,rd_osr_lmt
+ - snps,wr_osr_lmt
+ - snps,lpi_en
+
+additionalProperties: false
+
+examples:
+ - |
+ gmac0: ethernet@50400000 {
+ compatible = "eswin,eic7700-qos-eth";
+ reg = <0x0 0x50400000 0x0 0x10000>;
+ interrupt-parent = <&plic>;
+ interrupt-names = "macirq";
+ interrupts = <61>;
+ phy-mode = "rgmii";
+ id = <0>;
+ status = "disabled";
+ clocks = <&clock 550>,
+ <&clock 551>,
+ <&clock 552>;
+ clock-names = "app", "stmmaceth", "tx";
+ resets = <&reset 0x07 (1 << 26)>;
+ reset-names = "ethrst";
+ 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;
+ };
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver
2025-05-16 1:08 [PATCH v1 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
@ 2025-05-16 1:11 ` weishangjuan
2025-05-16 14:32 ` Jakub Kicinski
` (3 more replies)
1 sibling, 4 replies; 12+ messages in thread
From: weishangjuan @ 2025-05-16 1:11 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
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 | 521 ++++++++++++++++++
3 files changed, 533 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 3c820ef56775..6a3970c92db7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -66,6 +66,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 594883fb4164..c9279bafdbb1 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..3483827e5652
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
@@ -0,0 +1,521 @@
+// 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_device.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/mfd/syscon.h>
+#include <linux/bitfield.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.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)
+
+/* RTL8211F PHY Configurations for LEDs */
+#define PHY_ADDR 0
+#define PHY_PAGE_SWITCH_REG 31
+#define PHY_LED_CFG_REG 16
+#define PHY_LED_PAGE_CFG 0xd04
+
+struct dwc_qos_priv {
+ struct device *dev;
+ int dev_id;
+ struct regmap *crg_regmap;
+ struct regmap *hsp_regmap;
+ struct reset_control *rst;
+ struct clk *clk_app;
+ struct clk *clk_tx;
+ struct gpio_desc *phy_reset;
+ struct stmmac_priv *stmpriv;
+ int phyled_cfgs[3];
+ 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 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 = kzalloc(sizeof(*plat_dat->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 void dwc_qos_fix_speed(void *priv, int speed, unsigned int mode)
+{
+ unsigned long rate = 125000000;
+ int i, err, data = 0;
+ struct dwc_qos_priv *dwc_priv = (struct dwc_qos_priv *)priv;
+
+ switch (speed) {
+ case SPEED_1000:
+ rate = 125000000;
+
+ for (i = 0; i < 3; i++)
+ regmap_write(dwc_priv->hsp_regmap,
+ dwc_priv->dly_hsp_reg[i],
+ dwc_priv->dly_param_1000m[i]);
+
+ if (dwc_priv->stmpriv) {
+ data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[0]);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, data);
+ }
+
+ break;
+ case SPEED_100:
+ rate = 25000000;
+
+ for (i = 0; i < 3; i++) {
+ regmap_write(dwc_priv->hsp_regmap,
+ dwc_priv->dly_hsp_reg[i],
+ dwc_priv->dly_param_100m[i]);
+ }
+
+ if (dwc_priv->stmpriv) {
+ data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[1]);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, data);
+ }
+
+ break;
+ case SPEED_10:
+ rate = 2500000;
+
+ for (i = 0; i < 3; i++) {
+ regmap_write(dwc_priv->hsp_regmap,
+ dwc_priv->dly_hsp_reg[i],
+ dwc_priv->dly_param_10m[i]);
+ }
+
+ if (dwc_priv->stmpriv) {
+ data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[2]);
+ mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
+ PHY_PAGE_SWITCH_REG, data);
+ }
+
+ break;
+ default:
+ dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
+ break;
+ }
+
+ err = clk_set_rate(dwc_priv->clk_tx, rate);
+ if (err < 0)
+ dev_err(dwc_priv->dev, "failed to set TX rate: %d\n", err);
+}
+
+static int dwc_qos_probe(struct platform_device *pdev,
+ struct plat_stmmacenet_data *plat_dat,
+ struct stmmac_resources *stmmac_res)
+{
+ struct dwc_qos_priv *dwc_priv;
+ int ret;
+ int err;
+ 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;
+
+ 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)) {
+ dev_err(&pdev->dev, "Can not read device id!\n");
+ return -EINVAL;
+ }
+
+ dwc_priv->dev = &pdev->dev;
+ dwc_priv->phy_reset = devm_gpiod_get(&pdev->dev, "rst", GPIOD_OUT_LOW);
+ if (IS_ERR(dwc_priv->phy_reset)) {
+ dev_err(&pdev->dev, "Reset gpio not specified\n");
+ return -EINVAL;
+ }
+
+ gpiod_set_value(dwc_priv->phy_reset, 0);
+
+ ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 0,
+ &dwc_priv->phyled_cfgs[0]);
+ if (ret)
+ dev_warn(&pdev->dev, "can't get phyaddr (%d)\n", ret);
+ ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 0,
+ &dwc_priv->phyled_cfgs[0]);
+ if (ret)
+ dev_warn(&pdev->dev, "can't get led cfgs for 1Gbps mode (%d)\n", ret);
+
+ ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 1,
+ &dwc_priv->phyled_cfgs[1]);
+ if (ret)
+ dev_warn(&pdev->dev, "can't get led cfgs for 100Mbps mode (%d)\n", ret);
+
+ ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 2,
+ &dwc_priv->phyled_cfgs[2]);
+ if (ret)
+ dev_warn(&pdev->dev, "can't get led cfgs for 10Mbps mode (%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) {
+ dev_err(&pdev->dev, "can't get hsp_aclk_ctrl_offset (%d)\n", ret);
+ return ret;
+ }
+ 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) {
+ dev_err(&pdev->dev, "can't get hsp_cfg_ctrl_offset (%d)\n", ret);
+ return ret;
+ }
+ 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,
+ ð_phy_ctrl_offset);
+ if (ret) {
+ dev_err(&pdev->dev, "can't get eth_phy_ctrl_offset (%d)\n", ret);
+ return ret;
+ }
+ regmap_read(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, ð_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,
+ ð_axi_lp_ctrl_offset);
+ if (ret) {
+ dev_err(&pdev->dev, "can't get eth_axi_lp_ctrl_offset (%d)\n", ret);
+ return ret;
+ }
+ regmap_write(dwc_priv->hsp_regmap, eth_axi_lp_ctrl_offset, ETH_CSYSREQ_VAL);
+
+ dwc_priv->clk_app = devm_clk_get(&pdev->dev, "app");
+ if (IS_ERR(dwc_priv->clk_app)) {
+ dev_err(&pdev->dev, "app clock not found.\n");
+ return PTR_ERR(dwc_priv->clk_app);
+ }
+
+ err = clk_prepare_enable(dwc_priv->clk_app);
+ if (err < 0) {
+ dev_err(&pdev->dev, "failed to enable app clock: %d\n",
+ err);
+ return err;
+ }
+
+ dwc_priv->clk_tx = devm_clk_get(&pdev->dev, "tx");
+ if (IS_ERR(plat_dat->pclk)) {
+ dev_err(&pdev->dev, "tx clock not found.\n");
+ return PTR_ERR(dwc_priv->clk_tx);
+ }
+
+ err = clk_prepare_enable(dwc_priv->clk_tx);
+ if (err < 0) {
+ dev_err(&pdev->dev, "failed to enable tx clock: %d\n", err);
+ return err;
+ }
+ dwc_priv->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "ethrst");
+ if (IS_ERR(dwc_priv->rst))
+ return PTR_ERR(dwc_priv->rst);
+
+ ret = reset_control_assert(dwc_priv->rst);
+ WARN_ON(ret != 0);
+ ret = reset_control_deassert(dwc_priv->rst);
+ WARN_ON(ret != 0);
+
+ plat_dat->fix_mac_speed = dwc_qos_fix_speed;
+ plat_dat->bsp_priv = dwc_priv;
+ plat_dat->phy_addr = PHY_ADDR;
+
+ return 0;
+}
+
+static int dwc_qos_remove(struct platform_device *pdev)
+{
+ struct dwc_qos_priv *dwc_priv = get_stmmac_bsp_priv(&pdev->dev);
+
+ reset_control_assert(dwc_priv->rst);
+ clk_disable_unprepare(dwc_priv->clk_tx);
+ clk_disable_unprepare(dwc_priv->clk_app);
+
+ devm_gpiod_put(&pdev->dev, dwc_priv->phy_reset);
+
+ return 0;
+}
+
+struct dwc_eth_dwmac_data {
+ int (*probe)(struct platform_device *pdev,
+ struct plat_stmmacenet_data *data,
+ struct stmmac_resources *res);
+ int (*remove)(struct platform_device *pdev);
+};
+
+static const struct dwc_eth_dwmac_data dwc_qos_data = {
+ .probe = dwc_qos_probe,
+ .remove = dwc_qos_remove,
+};
+
+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;
+ struct net_device *ndev = NULL;
+ struct stmmac_priv *stmpriv = NULL;
+ struct dwc_qos_priv *dwc_priv = NULL;
+ 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);
+
+ ret = data->probe(pdev, plat_dat, &stmmac_res);
+ if (ret < 0) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to probe subdriver: %d\n",
+ ret);
+
+ return ret;
+ }
+
+ ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
+ if (ret)
+ goto remove;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret)
+ goto remove;
+
+ ndev = dev_get_drvdata(&pdev->dev);
+ stmpriv = netdev_priv(ndev);
+ dwc_priv = (struct dwc_qos_priv *)plat_dat->bsp_priv;
+ dwc_priv->stmpriv = stmpriv;
+
+ return ret;
+
+remove:
+ data->remove(pdev);
+
+ return ret;
+}
+
+static void dwc_eth_dwmac_remove(struct platform_device *pdev)
+{
+ const struct dwc_eth_dwmac_data *data;
+ int err;
+
+ data = device_get_match_data(&pdev->dev);
+
+ stmmac_dvr_remove(&pdev->dev);
+
+ err = data->remove(pdev);
+ if (err < 0)
+ dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
+}
+
+static const struct of_device_id dwc_eth_dwmac_match[] = {
+ { .compatible = "eswin,eic7700-qos-eth", .data = &dwc_qos_data },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
+
+static struct platform_driver eic7700_eth_dwmac_driver = {
+ .probe = dwc_eth_dwmac_probe,
+ .remove = dwc_eth_dwmac_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 v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
@ 2025-05-16 2:23 ` Rob Herring (Arm)
2025-05-16 13:19 ` Krzysztof Kozlowski
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-05-16 2:23 UTC (permalink / raw)
To: weishangjuan
Cc: devicetree, inochiama, alexandre.torgue, davem, conor+dt,
rmk+kernel, krzk+dt, pabeni, p.zabel, andrew+netdev,
mcoquelin.stm32, linux-kernel, linux-arm-kernel, lizhi2, edumazet,
linux-stm32, yong.liang.choong, kuba, jan.petrous, dfustini,
linmin, ningyu, 0x1207, netdev, richardcochran, jszhang
On Fri, 16 May 2025 09:10:38 +0800, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
>
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, PHY LED configuration,
> 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 | 142 ++++++++++++++++++
> 1 file changed, 142 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,.*', '^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/20250516011040.801-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 v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
2025-05-16 2:23 ` Rob Herring (Arm)
@ 2025-05-16 13:19 ` Krzysztof Kozlowski
2025-05-18 1:07 ` Inochi Amaoto
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-16 13:19 UTC (permalink / raw)
To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
krzk+dt, conor+dt, richardcochran, netdev, devicetree,
linux-kernel, mcoquelin.stm32, alexandre.torgue, p.zabel,
yong.liang.choong, rmk+kernel, jszhang, inochiama, jan.petrous,
dfustini, 0x1207, linux-stm32, linux-arm-kernel
Cc: ningyu, linmin, lizhi2
On 16/05/2025 03:10, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
>
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, PHY LED configuration,
> clock/reset control, and AXI bus parameter optimization.
>
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> ---
> .../bindings/net/eswin,eic7700-eth.yaml | 142 ++++++++++++++++++
> 1 file changed, 142 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..6cb9c109c036
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> @@ -0,0 +1,142 @@
> +# 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: |
Same comments apply as for all of your patches.
> + The eth controller registers are part of the syscrg block on
> + the EIC7700 SoC.
> +
> +properties:
> + compatible:
> + const: eswin,eic7700-qos-eth
> +
> + reg:
> + minItems: 1
> + items:
> + - description: Base address and size
> + - description: Extension region (optional)
How it can be optional? This is SoC. It is strictly defined, isn't it?
> +
> + interrupt-names:
> + const: macirq
> +
> + interrupts:
> + maxItems: 1
> +
> + phy-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [mii, gmii, rgmii, rmii, sgmii]
> +
> + id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Controller instance ID
No, drop. IDs are not allowed.
> +
> + clocks:
> + minItems: 3
> + maxItems: 7
No.
I am supposed to repeat the same comments... So no.
All my comments apply to all eswin patches. For driver, bindings,
everything. I suggest to slow down and learn from one review.
I finish review here.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
@ 2025-05-16 14:32 ` Jakub Kicinski
2025-05-17 14:11 ` Simon Horman
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2025-05-16 14:32 UTC (permalink / raw)
To: weishangjuan
Cc: andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
richardcochran, netdev, devicetree, linux-kernel, mcoquelin.stm32,
alexandre.torgue, p.zabel, yong.liang.choong, rmk+kernel, jszhang,
inochiama, jan.petrous, dfustini, 0x1207, linux-stm32,
linux-arm-kernel, ningyu, linmin, lizhi2
On Fri, 16 May 2025 09:11:28 +0800 weishangjuan@eswincomputing.com
wrote:
> + dwc_priv->clk_tx = devm_clk_get(&pdev->dev, "tx");
> + if (IS_ERR(plat_dat->pclk)) {
you're checking the wrong pointer here
> + dev_err(&pdev->dev, "tx clock not found.\n");
> + return PTR_ERR(dwc_priv->clk_tx);
--
pw-bot: cr
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
2025-05-16 14:32 ` Jakub Kicinski
@ 2025-05-17 14:11 ` Simon Horman
2025-05-18 1:12 ` Inochi Amaoto
2025-05-18 22:45 ` Andrew Lunn
3 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2025-05-17 14:11 UTC (permalink / raw)
To: weishangjuan
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
linux-stm32, linux-arm-kernel, ningyu, linmin, lizhi2
On Fri, May 16, 2025 at 09:11:28AM +0800, weishangjuan@eswincomputing.com wrote:
> 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>
...
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
...
d
> +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 = kzalloc(sizeof(*plat_dat->axi), GFP_KERNEL);
It is unclear to me where this memory is freed, both on error and removal.
For consistency perhaps it is appropriate to use devm_kzalloc().
> +
> + 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;
I think the preferred coding style for the above would be:
switch(...) {
case 0:
...
break;
...
default:
...
}
> + }
> + a_index++;
> + }
But I also think the code above can be more succinctly expressed something
like this:
for (bit_index = 0; bit_index < 7; bit_index++)
if (burst_map & (1 << bit_index)
plat_dat->axi->axi_blen[a_index++] = 1 << i + 2;
> + }
> +
> + /* 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 void dwc_qos_fix_speed(void *priv, int speed, unsigned int mode)
> +{
> + unsigned long rate = 125000000;
> + int i, err, data = 0;
> + struct dwc_qos_priv *dwc_priv = (struct dwc_qos_priv *)priv;
Please arrange local variables in networking code in reverse xmas tree
order - longest line to shortest.
To this end, this tool can be useful.
https://github.com/ecree-solarflare/xmastree
Also priv is void *. And there is usually no need to explicitly cast
to or from a void *.
So, something like this (completely intested!)
struct dwc_qos_priv *dwc_priv = priv;
unsigned long rate = 125000000;
int i, err, data = 0;
> +
> + switch (speed) {
> + case SPEED_1000:
> + rate = 125000000;
> +
> + for (i = 0; i < 3; i++)
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_1000m[i]);
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[0]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + case SPEED_100:
> + rate = 25000000;
> +
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_100m[i]);
> + }
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[1]);
For Networking code please limit lines to 80 columns wide or less in
Networking code where it can be done without reducing readability (which is
the case here).
checkpatch.pl --max-line-length=80 is your friend.
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + case SPEED_10:
> + rate = 2500000;
> +
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_10m[i]);
> + }
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[2]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + default:
Maybe set rate here rather than along with it's declaration, and provide
a comment regarding a default value being used in the case of an
invalid speed.
> + dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
> + break;
> + }
> +
> + err = clk_set_rate(dwc_priv->clk_tx, rate);
> + if (err < 0)
> + dev_err(dwc_priv->dev, "failed to set TX rate: %d\n", err);
> +}
> +
> +static int dwc_qos_probe(struct platform_device *pdev,
> + struct plat_stmmacenet_data *plat_dat,
> + struct stmmac_resources *stmmac_res)
> +{
> + struct dwc_qos_priv *dwc_priv;
> + int ret;
> + int err;
> + 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;
> +
> + 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)) {
> + dev_err(&pdev->dev, "Can not read device id!\n");
> + return -EINVAL;
This and other error paths below look like candidates for:
return dev_err_probe(...)
> + }
> +
> + dwc_priv->dev = &pdev->dev;
> + dwc_priv->phy_reset = devm_gpiod_get(&pdev->dev, "rst", GPIOD_OUT_LOW);
> + if (IS_ERR(dwc_priv->phy_reset)) {
> + dev_err(&pdev->dev, "Reset gpio not specified\n");
> + return -EINVAL;
> + }
...
> + err = clk_prepare_enable(dwc_priv->clk_app);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to enable app clock: %d\n",
> + err);
> + return err;
> + }
Can devm_clk_get_enabled() be used in place of devm_clk_get() +
clk_prepare_enable(). It seems odd to me to let devm handle part of
the setup but not all of it.
If not, I think you need to make sure clk_disable_unprepare() is
called in the error paths below.
Flagged by Smatch.
> +
> + dwc_priv->clk_tx = devm_clk_get(&pdev->dev, "tx");
> + if (IS_ERR(plat_dat->pclk)) {
> + dev_err(&pdev->dev, "tx clock not found.\n");
> + return PTR_ERR(dwc_priv->clk_tx);
> + }
> +
> + err = clk_prepare_enable(dwc_priv->clk_tx);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to enable tx clock: %d\n", err);
> + return err;
> + }
Likewise here.
> + dwc_priv->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "ethrst");
> + if (IS_ERR(dwc_priv->rst))
> + return PTR_ERR(dwc_priv->rst);
> +
> + ret = reset_control_assert(dwc_priv->rst);
> + WARN_ON(ret != 0);
> + ret = reset_control_deassert(dwc_priv->rst);
> + WARN_ON(ret != 0);
> +
> + plat_dat->fix_mac_speed = dwc_qos_fix_speed;
> + plat_dat->bsp_priv = dwc_priv;
> + plat_dat->phy_addr = PHY_ADDR;
> +
> + return 0;
> +}
...
> +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;
> + struct net_device *ndev = NULL;
> + struct stmmac_priv *stmpriv = NULL;
> + struct dwc_qos_priv *dwc_priv = NULL;
> + 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);
> +
> + ret = data->probe(pdev, plat_dat, &stmmac_res);
> + if (ret < 0) {
This also looks like a candidate for dev_err_probe().
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to probe subdriver: %d\n",
> + ret);
> +
> + return ret;
> + }
> +
> + ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
> + if (ret)
> + goto remove;
> +
> + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> + if (ret)
> + goto remove;
> +
> + ndev = dev_get_drvdata(&pdev->dev);
> + stmpriv = netdev_priv(ndev);
> + dwc_priv = (struct dwc_qos_priv *)plat_dat->bsp_priv;
> + dwc_priv->stmpriv = stmpriv;
> +
> + return ret;
> +
> +remove:
> + data->remove(pdev);
> +
> + return ret;
> +}
> +
> +static void dwc_eth_dwmac_remove(struct platform_device *pdev)
> +{
> + const struct dwc_eth_dwmac_data *data;
> + int err;
> +
> + data = device_get_match_data(&pdev->dev);
> +
> + stmmac_dvr_remove(&pdev->dev);
> +
> + err = data->remove(pdev);
> + if (err < 0)
> + dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
> +}
> +
> +static const struct of_device_id dwc_eth_dwmac_match[] = {
> + { .compatible = "eswin,eic7700-qos-eth", .data = &dwc_qos_data },
Checkpatch warns that:
.../dwmac-eic7700.c:501: WARNING: DT compatible string vendor "eswin" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
> +
> +static struct platform_driver eic7700_eth_dwmac_driver = {
> + .probe = dwc_eth_dwmac_probe,
> + .remove = dwc_eth_dwmac_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");
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
2025-05-16 2:23 ` Rob Herring (Arm)
2025-05-16 13:19 ` Krzysztof Kozlowski
@ 2025-05-18 1:07 ` Inochi Amaoto
2025-05-18 22:38 ` Andrew Lunn
2025-05-26 5:41 ` Bo Gan
4 siblings, 0 replies; 12+ messages in thread
From: Inochi Amaoto @ 2025-05-18 1:07 UTC (permalink / raw)
To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
krzk+dt, conor+dt, richardcochran, netdev, devicetree,
linux-kernel, mcoquelin.stm32, alexandre.torgue, p.zabel,
yong.liang.choong, rmk+kernel, jszhang, inochiama, jan.petrous,
dfustini, 0x1207, linux-stm32, linux-arm-kernel
Cc: ningyu, linmin, lizhi2
On Fri, May 16, 2025 at 09:10:38AM +0800, weishangjuan@eswincomputing.com wrote:
> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
>
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, PHY LED configuration,
> 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 | 142 ++++++++++++++++++
> 1 file changed, 142 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..6cb9c109c036
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> @@ -0,0 +1,142 @@
> +# 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.
> +
> +properties:
> + compatible:
> + const: eswin,eic7700-qos-eth
Please set the related dwmac version as basic compatible,
it should be something like snps,dwmac-xxx.
> +
> + reg:
> + minItems: 1
> + items:
> + - description: Base address and size
> + - description: Extension region (optional)
> +
> + interrupt-names:
> + const: macirq
> +
> + interrupts:
> + maxItems: 1
> +
> + phy-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [mii, gmii, rgmii, rmii, sgmii]
> +
> + id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Controller instance ID
> +
> + clocks:
> + minItems: 3
> + maxItems: 7
> +
> + clock-names:
> + minItems: 3
> + items:
> + - const: app
> + - const: stmmaceth
> + - const: tx
> + - const: slave_bus
> + - const: master_bus
> + - const: ptp_ref
> + - const: phy_ref_clk
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + items:
> + - const: ethrst
Please refer to snps,dwmac.yaml and set a matching name.
This applies to all properties with snp prefix.
> +
> + dma-noncoherent: true
> +
> + # Custom properties
> + eswin,hsp_sp_csr:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: HSP SP control registers
> +
> + eswin,syscrg_csr:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: System clock registers
> +
> + eswin,dly_hsp_reg:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: HSP delay control registers
> +
> + snps,axi-config:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: AXI bus configuration
> +
> + stmmac-axi-config:
> + type: object
> + unevaluatedProperties: false
> + properties:
> + snps,lpi_en:
> + type: boolean
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: Low Power Interface enable flag (true/false)
> +
> +required:
> + - compatible
> + - reg
> + - interrupt-names
> + - interrupts
> + - phy-mode
> + - id
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> + - eswin,hsp_sp_csr
> + - eswin,syscrg_csr
> + - eswin,dly_hsp_reg
> + - snps,axi-config
> + - snps,blen
> + - snps,rd_osr_lmt
> + - snps,wr_osr_lmt
> + - snps,lpi_en
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + gmac0: ethernet@50400000 {
> + compatible = "eswin,eic7700-qos-eth";
> + reg = <0x0 0x50400000 0x0 0x10000>;
> + interrupt-parent = <&plic>;
> + interrupt-names = "macirq";
> + interrupts = <61>;
> + phy-mode = "rgmii";
> + id = <0>;
> + status = "disabled";
> + clocks = <&clock 550>,
> + <&clock 551>,
> + <&clock 552>;
> + clock-names = "app", "stmmaceth", "tx";
> + resets = <&reset 0x07 (1 << 26)>;
> + reset-names = "ethrst";
> + 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;
> + };
> + };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
2025-05-16 14:32 ` Jakub Kicinski
2025-05-17 14:11 ` Simon Horman
@ 2025-05-18 1:12 ` Inochi Amaoto
2025-05-18 22:45 ` Andrew Lunn
3 siblings, 0 replies; 12+ messages in thread
From: Inochi Amaoto @ 2025-05-18 1:12 UTC (permalink / raw)
To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
krzk+dt, conor+dt, richardcochran, netdev, devicetree,
linux-kernel, mcoquelin.stm32, alexandre.torgue, p.zabel,
yong.liang.choong, rmk+kernel, jszhang, inochiama, jan.petrous,
dfustini, 0x1207, linux-stm32, linux-arm-kernel
Cc: ningyu, linmin, lizhi2
On Fri, May 16, 2025 at 09:11:28AM +0800, weishangjuan@eswincomputing.com wrote:
> 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 | 521 ++++++++++++++++++
> 3 files changed, 533 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 3c820ef56775..6a3970c92db7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -66,6 +66,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 594883fb4164..c9279bafdbb1 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..3483827e5652
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-eic7700.c
> @@ -0,0 +1,521 @@
> +// 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_device.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/mfd/syscon.h>
> +#include <linux/bitfield.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio/consumer.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)
> +
> +/* RTL8211F PHY Configurations for LEDs */
> +#define PHY_ADDR 0
> +#define PHY_PAGE_SWITCH_REG 31
> +#define PHY_LED_CFG_REG 16
> +#define PHY_LED_PAGE_CFG 0xd04
> +
> +struct dwc_qos_priv {
> + struct device *dev;
> + int dev_id;
> + struct regmap *crg_regmap;
> + struct regmap *hsp_regmap;
> + struct reset_control *rst;
> + struct clk *clk_app;
> + struct clk *clk_tx;
> + struct gpio_desc *phy_reset;
> + struct stmmac_priv *stmpriv;
> + int phyled_cfgs[3];
> + 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 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 = kzalloc(sizeof(*plat_dat->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 void dwc_qos_fix_speed(void *priv, int speed, unsigned int mode)
> +{
> + unsigned long rate = 125000000;
> + int i, err, data = 0;
> + struct dwc_qos_priv *dwc_priv = (struct dwc_qos_priv *)priv;
> +
> + switch (speed) {
> + case SPEED_1000:
> + rate = 125000000;
> +
> + for (i = 0; i < 3; i++)
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_1000m[i]);
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[0]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + case SPEED_100:
> + rate = 25000000;
> +
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_100m[i]);
> + }
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[1]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + case SPEED_10:
> + rate = 2500000;
> +
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_10m[i]);
> + }
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[2]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
> + }
> +
> + break;
> + default:
> + dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
> + break;
> + }
> +
> + err = clk_set_rate(dwc_priv->clk_tx, rate);
> + if (err < 0)
> + dev_err(dwc_priv->dev, "failed to set TX rate: %d\n", err);
> +}
check set_clk_tx_rate, and replace this with stmmac_set_clk_tx_rate
if possible.
> +
> +static int dwc_qos_probe(struct platform_device *pdev,
> + struct plat_stmmacenet_data *plat_dat,
> + struct stmmac_resources *stmmac_res)
> +{
> + struct dwc_qos_priv *dwc_priv;
> + int ret;
> + int err;
> + 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;
> +
> + 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)) {
> + dev_err(&pdev->dev, "Can not read device id!\n");
> + return -EINVAL;
> + }
> +
> + dwc_priv->dev = &pdev->dev;
> + dwc_priv->phy_reset = devm_gpiod_get(&pdev->dev, "rst", GPIOD_OUT_LOW);
> + if (IS_ERR(dwc_priv->phy_reset)) {
> + dev_err(&pdev->dev, "Reset gpio not specified\n");
> + return -EINVAL;
> + }
> +
> + gpiod_set_value(dwc_priv->phy_reset, 0);
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 0,
> + &dwc_priv->phyled_cfgs[0]);
> + if (ret)
> + dev_warn(&pdev->dev, "can't get phyaddr (%d)\n", ret);
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 0,
> + &dwc_priv->phyled_cfgs[0]);
> + if (ret)
> + dev_warn(&pdev->dev, "can't get led cfgs for 1Gbps mode (%d)\n", ret);
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 1,
> + &dwc_priv->phyled_cfgs[1]);
> + if (ret)
> + dev_warn(&pdev->dev, "can't get led cfgs for 100Mbps mode (%d)\n", ret);
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,led-cfgs", 2,
> + &dwc_priv->phyled_cfgs[2]);
> + if (ret)
> + dev_warn(&pdev->dev, "can't get led cfgs for 10Mbps mode (%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) {
> + dev_err(&pdev->dev, "can't get hsp_aclk_ctrl_offset (%d)\n", ret);
> + return ret;
> + }
> + 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) {
> + dev_err(&pdev->dev, "can't get hsp_cfg_ctrl_offset (%d)\n", ret);
> + return ret;
> + }
> + 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,
> + ð_phy_ctrl_offset);
> + if (ret) {
> + dev_err(&pdev->dev, "can't get eth_phy_ctrl_offset (%d)\n", ret);
> + return ret;
> + }
> + regmap_read(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, ð_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,
> + ð_axi_lp_ctrl_offset);
> + if (ret) {
> + dev_err(&pdev->dev, "can't get eth_axi_lp_ctrl_offset (%d)\n", ret);
> + return ret;
> + }
> + regmap_write(dwc_priv->hsp_regmap, eth_axi_lp_ctrl_offset, ETH_CSYSREQ_VAL);
> +
> + dwc_priv->clk_app = devm_clk_get(&pdev->dev, "app");
> + if (IS_ERR(dwc_priv->clk_app)) {
> + dev_err(&pdev->dev, "app clock not found.\n");
> + return PTR_ERR(dwc_priv->clk_app);
> + }
> +
> + err = clk_prepare_enable(dwc_priv->clk_app);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to enable app clock: %d\n",
> + err);
> + return err;
> + }
> +
> + dwc_priv->clk_tx = devm_clk_get(&pdev->dev, "tx");
> + if (IS_ERR(plat_dat->pclk)) {
> + dev_err(&pdev->dev, "tx clock not found.\n");
> + return PTR_ERR(dwc_priv->clk_tx);
> + }
> +
> + err = clk_prepare_enable(dwc_priv->clk_tx);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to enable tx clock: %d\n", err);
> + return err;
> + }
> + dwc_priv->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "ethrst");
> + if (IS_ERR(dwc_priv->rst))
> + return PTR_ERR(dwc_priv->rst);
> +
> + ret = reset_control_assert(dwc_priv->rst);
> + WARN_ON(ret != 0);
> + ret = reset_control_deassert(dwc_priv->rst);
> + WARN_ON(ret != 0);
> +
> + plat_dat->fix_mac_speed = dwc_qos_fix_speed;
> + plat_dat->bsp_priv = dwc_priv;
> + plat_dat->phy_addr = PHY_ADDR;
> +
> + return 0;
> +}
> +
> +static int dwc_qos_remove(struct platform_device *pdev)
> +{
> + struct dwc_qos_priv *dwc_priv = get_stmmac_bsp_priv(&pdev->dev);
> +
> + reset_control_assert(dwc_priv->rst);
> + clk_disable_unprepare(dwc_priv->clk_tx);
> + clk_disable_unprepare(dwc_priv->clk_app);
> +
> + devm_gpiod_put(&pdev->dev, dwc_priv->phy_reset);
> +
> + return 0;
> +}
> +
> +struct dwc_eth_dwmac_data {
> + int (*probe)(struct platform_device *pdev,
> + struct plat_stmmacenet_data *data,
> + struct stmmac_resources *res);
> + int (*remove)(struct platform_device *pdev);
> +};
> +
> +static const struct dwc_eth_dwmac_data dwc_qos_data = {
> + .probe = dwc_qos_probe,
> + .remove = dwc_qos_remove,
> +};
> +
> +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;
> + struct net_device *ndev = NULL;
> + struct stmmac_priv *stmpriv = NULL;
> + struct dwc_qos_priv *dwc_priv = NULL;
> + 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);
> +
> + ret = data->probe(pdev, plat_dat, &stmmac_res);
> + if (ret < 0) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to probe subdriver: %d\n",
> + ret);
> +
> + return ret;
> + }
> +
> + ret = dwc_eth_dwmac_config_dt(pdev, plat_dat);
> + if (ret)
> + goto remove;
> +
> + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> + if (ret)
> + goto remove;
> +
> + ndev = dev_get_drvdata(&pdev->dev);
> + stmpriv = netdev_priv(ndev);
> + dwc_priv = (struct dwc_qos_priv *)plat_dat->bsp_priv;
> + dwc_priv->stmpriv = stmpriv;
> +
> + return ret;
> +
> +remove:
> + data->remove(pdev);
> +
> + return ret;
> +}
> +
> +static void dwc_eth_dwmac_remove(struct platform_device *pdev)
> +{
> + const struct dwc_eth_dwmac_data *data;
> + int err;
> +
> + data = device_get_match_data(&pdev->dev);
> +
> + stmmac_dvr_remove(&pdev->dev);
> +
> + err = data->remove(pdev);
> + if (err < 0)
> + dev_err(&pdev->dev, "failed to remove subdriver: %d\n", err);
> +}
> +
> +static const struct of_device_id dwc_eth_dwmac_match[] = {
> + { .compatible = "eswin,eic7700-qos-eth", .data = &dwc_qos_data },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
> +
Use common stammac helper for you code, do not do this by yourself!!!
The only thing you should add is the vendor specific logic, not the
common one for the stmmac driver.
> +static struct platform_driver eic7700_eth_dwmac_driver = {
> + .probe = dwc_eth_dwmac_probe,
> + .remove = dwc_eth_dwmac_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 [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
` (2 preceding siblings ...)
2025-05-18 1:07 ` Inochi Amaoto
@ 2025-05-18 22:38 ` Andrew Lunn
2025-05-26 5:41 ` Bo Gan
4 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-05-18 22:38 UTC (permalink / raw)
To: weishangjuan
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
linux-stm32, linux-arm-kernel, ningyu, linmin, lizhi2
> + phy-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [mii, gmii, rgmii, rmii, sgmii]
In theory, all four rgmii modes should be listed. In practice, only
rgmii-id is likely to be used.
> +examples:
> + - |
> + gmac0: ethernet@50400000 {
> + compatible = "eswin,eic7700-qos-eth";
> + reg = <0x0 0x50400000 0x0 0x10000>;
> + interrupt-parent = <&plic>;
> + interrupt-names = "macirq";
> + interrupts = <61>;
> + phy-mode = "rgmii";
Please don't use rgmii in an example. It is probably wrong, unless you
have an unusual PCB design.
Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
` (2 preceding siblings ...)
2025-05-18 1:12 ` Inochi Amaoto
@ 2025-05-18 22:45 ` Andrew Lunn
3 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-05-18 22:45 UTC (permalink / raw)
To: weishangjuan
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, robh, krzk+dt,
conor+dt, richardcochran, netdev, devicetree, linux-kernel,
mcoquelin.stm32, alexandre.torgue, p.zabel, yong.liang.choong,
rmk+kernel, jszhang, inochiama, jan.petrous, dfustini, 0x1207,
linux-stm32, linux-arm-kernel, ningyu, linmin, lizhi2
> +/* RTL8211F PHY Configurations for LEDs */
> +#define PHY_ADDR 0
> +#define PHY_PAGE_SWITCH_REG 31
> +#define PHY_LED_CFG_REG 16
> +#define PHY_LED_PAGE_CFG 0xd04
The PHY driver is responsible for the PHY LEDs, not the MAC driver.
> +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 = kzalloc(sizeof(*plat_dat->axi), GFP_KERNEL);
> +
> + if (!plat_dat->axi)
> + return -ENOMEM;
> + }
> +
> + plat_dat->axi->axi_lpi_en = device_property_read_bool(dev,
> + "snps,en-lpi");
Please look at the work Russell King has been doing recently, and make
sure you are not adding stuff he has been busy cleaning up.
> +static void dwc_qos_fix_speed(void *priv, int speed, unsigned int mode)
> +{
> + unsigned long rate = 125000000;
> + int i, err, data = 0;
> + struct dwc_qos_priv *dwc_priv = (struct dwc_qos_priv *)priv;
> +
> + switch (speed) {
> + case SPEED_1000:
> + rate = 125000000;
> +
> + for (i = 0; i < 3; i++)
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_1000m[i]);
> +
> + if (dwc_priv->stmpriv) {
> + data = mdiobus_read(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, PHY_LED_PAGE_CFG);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_LED_CFG_REG, dwc_priv->phyled_cfgs[0]);
> + mdiobus_write(dwc_priv->stmpriv->mii, PHY_ADDR,
> + PHY_PAGE_SWITCH_REG, data);
Please remove all this LED code.
> + dwc_priv->dev = &pdev->dev;
> + dwc_priv->phy_reset = devm_gpiod_get(&pdev->dev, "rst", GPIOD_OUT_LOW);
> + if (IS_ERR(dwc_priv->phy_reset)) {
> + dev_err(&pdev->dev, "Reset gpio not specified\n");
> + return -EINVAL;
> + }
> +
> + gpiod_set_value(dwc_priv->phy_reset, 0);
Please allow phylib to control the PHY reset line.
> + 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;
> + }
What are these delay parameters?
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
` (3 preceding siblings ...)
2025-05-18 22:38 ` Andrew Lunn
@ 2025-05-26 5:41 ` Bo Gan
4 siblings, 0 replies; 12+ messages in thread
From: Bo Gan @ 2025-05-26 5:41 UTC (permalink / raw)
To: weishangjuan, andrew+netdev, davem, edumazet, kuba, pabeni, robh,
krzk+dt, conor+dt, richardcochran, netdev, devicetree,
linux-kernel, mcoquelin.stm32, alexandre.torgue, p.zabel,
yong.liang.choong, rmk+kernel, jszhang, inochiama, jan.petrous,
dfustini, 0x1207, linux-stm32, linux-arm-kernel
Cc: ningyu, linmin, lizhi2
On 5/15/25 18:10, weishangjuan@eswincomputing.com wrote:> From: Shangjuan Wei <weishangjuan@eswincomputing.com>
>
> Add ESWIN EIC7700 Ethernet controller, supporting
> multi-rate (10M/100M/1G) auto-negotiation, PHY LED configuration,
> clock/reset control, and AXI bus parameter optimization.
>
> Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
> ---...> + # Custom properties
> + eswin,hsp_sp_csr:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: HSP SP control register> +...> +additionalProperties: false
> +
> + eswin,syscrg_csr:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: System clock registers
> +
> + eswin,dly_hsp_reg:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: HSP delay control registers
...
> +examples:
> + - |
> + gmac0: ethernet@50400000 {...> + 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>;
Please help explain the meaning of eswin,<reg> array, and also the expected
number of elements in it, like what starfive did to their JH71x0 device-
tree bindings. E.g., this is what net/starfive,jh7110-dwmac.yaml looks like:
...
starfive,syscon:
$ref: /schemas/types.yaml#/definitions/phandle-array
items:
- items:
- description: phandle to syscon that configures phy mode
- description: Offset of phy mode selection
- description: Shift of phy mode selection
description:
A phandle to syscon with two arguments that configure phy mode.
The argument one is the offset of phy mode selection, the
argument two is the shift of phy mode selection.
...
Otherwise, there's no way for people to reason about the driver code.
The same should apply for your sdhci/usb/pcie/... patchsets as well.
Also there's no reference to the first element of the hsp_sp_csr array.
From the vendor code, I'm reading that you are using the first element
as the register to set the stream ID of the device to tag the memory
transactions for SMMU, but in the patch, there's no mentioning of it.
I'm guessing you are planning to upstream that part later. If so, I
think it's better to put that register index at the end of the array,
and make it optional. It should then be properly documented as well.
Bo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-05-26 5:39 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-16 1:08 [PATCH v1 0/2] Add driver support for Eswin eic7700 SoC ethernet controller weishangjuan
2025-05-16 1:10 ` [PATCH v1 1/2] ethernet: eswin: Document for eic7700 SoC weishangjuan
2025-05-16 2:23 ` Rob Herring (Arm)
2025-05-16 13:19 ` Krzysztof Kozlowski
2025-05-18 1:07 ` Inochi Amaoto
2025-05-18 22:38 ` Andrew Lunn
2025-05-26 5:41 ` Bo Gan
2025-05-16 1:11 ` [PATCH v1 2/2] ethernet: eswin: Add eic7700 ethernet driver weishangjuan
2025-05-16 14:32 ` Jakub Kicinski
2025-05-17 14:11 ` Simon Horman
2025-05-18 1:12 ` Inochi Amaoto
2025-05-18 22:45 ` Andrew Lunn
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).