[PATCH v1 0/7] Add Ethernet driver for StarFive JH7110 SoC

[PATCH v1 0/7] Add Ethernet driver for StarFive JH7110 SoC

[Yanhong Wang]

This series adds ethernet support for the StarFive JH7110 RISC-V SoC.
The series includes PHY and MAC drivers. The PHY model is
YT8531 (from Motorcomm Inc), and the MAC version is dwmac-5.20
(from Synopsys DesignWare). For more information and support,
you can visit RVspace wiki[1].
	
This patchset should be applied after the patchset [2], [3], [4].
[1] https://wiki.rvspace.org/
[2] https://lore.kernel.org/all/20221118010627.70576-1-hal.feng@starfivetech.com/
[3] https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/
[4] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/

Yanhong Wang (7):
  dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20
    version.
  net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string
  dt-bindings: net: Add bindings for StarFive dwmac
  net: phy: motorcomm: Add YT8531 phy support
  net: stmmac: Add StarFive dwmac supoort
  riscv: dts: starfive: jh7110: Add ethernet device node
  riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

 .../devicetree/bindings/net/snps,dwmac.yaml   |   2 +
 .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++
 MAINTAINERS                                   |   6 +
 .../jh7110-starfive-visionfive-v2.dts         |  46 +++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  80 ++++++++
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-starfive-plat.c      | 147 ++++++++++++++
 .../ethernet/stmicro/stmmac/stmmac_platform.c |   3 +-
 drivers/net/phy/Kconfig                       |   3 +-
 drivers/net/phy/motorcomm.c                   | 185 ++++++++++++++++++
 11 files changed, 588 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c


base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
prerequisite-patch-id: 8ebfffa09b478904bf7c516f76e2d824ddb60140
prerequisite-patch-id: e8dd8258a4c4062eee2cf07c4607d52baea71f3a
prerequisite-patch-id: d050d884d7b091ff30508a70f5ce5164bb3b72e5
prerequisite-patch-id: 0e41f8cfd4861fcbf6f2e6a2559ce28f0450299e
prerequisite-patch-id: 6e1652501859b85f101ff3b15ced585d43c71c1b
prerequisite-patch-id: 587628a67adad5c655e5f998bf6c4a368ec07d3c
prerequisite-patch-id: 596490c0e397df6c0249c1306fbb1d5bf00b5b83
prerequisite-patch-id: dc873317826b50364344b25ac5cd74e811403f3d
prerequisite-patch-id: a50150f41d8e874553023187e22eb24dffae8d16
prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5
prerequisite-patch-id: 9d2e83a2dd43e193f534283fab73e90b4f435043
prerequisite-patch-id: 7a43e0849a9afa3c6f83547fd16d9271b07619e5
prerequisite-patch-id: e7aa6fb05314bad6d94c465f3f59969871bf3d2e
prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9
prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b
prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53
prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851
prerequisite-patch-id: ba61df106fbe2ada21e8f22c3d2cfaf7809c84b6
prerequisite-patch-id: 287572fb64f83f5d931034f7c75674907584a087
prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556
prerequisite-patch-id: 841cd71b556b480d6a5a5e332eeca70d6a76ec3f
prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c
prerequisite-patch-id: 5f59bc7cbbf1230e5ff4761fa7c1116d4e6e5d71
prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e
-- 
2.17.1

[PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[Yanhong Wang]

Add dwmac-5.20 version to snps.dwmac.yaml

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 13b984076af5..d8779d3de3d6 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -30,6 +30,7 @@ select:
           - snps,dwmac-4.10a
           - snps,dwmac-4.20a
           - snps,dwmac-5.10a
+          - snps,dwmac-5.20
           - snps,dwxgmac
           - snps,dwxgmac-2.10
 
-- 
2.17.1

[PATCH v1 2/7] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[Yanhong Wang]

Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
to define some platform data in the glue layer.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 50f6b4a14be4..cc3b701af802 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
 	    of_device_is_compatible(np, "snps,dwmac-4.10a") ||
 	    of_device_is_compatible(np, "snps,dwmac-4.20a") ||
-	    of_device_is_compatible(np, "snps,dwmac-5.10a")) {
+	    of_device_is_compatible(np, "snps,dwmac-5.10a") ||
+	    of_device_is_compatible(np, "snps,dwmac-5.20")) {
 		plat->has_gmac4 = 1;
 		plat->has_gmac = 0;
 		plat->pmt = 1;
-- 
2.17.1

[PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Yanhong Wang]

Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
 .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
 MAINTAINERS                                   |   5 +
 3 files changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index d8779d3de3d6..13c5928d7170 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -33,6 +33,7 @@ select:
           - snps,dwmac-5.20
           - snps,dwxgmac
           - snps,dwxgmac-2.10
+          - starfive,dwmac
 
           # Deprecated
           - st,spear600-gmac
diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
new file mode 100644
index 000000000000..561cf2a713ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: StarFive DWMAC glue layer
+
+maintainers:
+  - Yanhong Wang <yanhong.wang@starfivetech.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - starfive,dwmac
+  required:
+    - compatible
+
+allOf:
+  - $ref: "snps,dwmac.yaml#"
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+               - starfive,dwmac
+          - const: snps,dwmac-5.20
+
+  clocks:
+    items:
+      - description: GMAC main clock
+      - description: GMAC AHB clock
+      - description: PTP clock
+      - description: TX clock
+      - description: GTXC clock
+      - description: GTX clock
+
+  clock-names:
+    contains:
+      enum:
+        - stmmaceth
+        - pclk
+        - ptp_ref
+        - tx
+        - gtxc
+        - gtx
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/starfive-jh7110.h>
+    #include <dt-bindings/reset/starfive-jh7110.h>
+
+    stmmac_axi_setup: stmmac-axi-config {
+        snps,wr_osr_lmt = <4>;
+        snps,rd_osr_lmt = <4>;
+        snps,blen = <256 128 64 32 0 0 0>;
+    };
+
+    gmac0: ethernet@16030000 {
+        compatible = "starfive,dwmac", "snps,dwmac-5.20";
+        reg = <0x16030000 0x10000>;
+        clocks = <&aoncrg_clk JH7110_AONCLK_GMAC0_AXI>,
+                 <&aoncrg_clk JH7110_AONCLK_GMAC0_AHB>,
+                 <&syscrg_clk JH7110_SYSCLK_GMAC0_PTP>,
+                 <&aoncrg_clk JH7110_AONCLK_GMAC0_TX>,
+                 <&syscrg_clk JH7110_SYSCLK_GMAC0_GTXC>,
+                 <&syscrg_clk JH7110_SYSCLK_GMAC0_GTXCLK>;
+        clock-names = "stmmaceth",
+                      "pclk",
+                      "ptp_ref",
+                      "tx",
+                      "gtxc",
+                      "gtx";
+        resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
+                 <&aoncrg JH7110_AONRST_GMAC0_AHB>;
+        reset-names = "stmmaceth", "ahb";
+        interrupts = <7>, <6>, <5>;
+        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+        phy-mode = "rgmii-id";
+        snps,multicast-filter-bins = <64>;
+        snps,perfect-filter-entries = <8>;
+        rx-fifo-depth = <2048>;
+        tx-fifo-depth = <2048>;
+        snps,fixed-burst;
+        snps,no-pbl-x8;
+        snps,force_thresh_dma_mode;
+        snps,axi-config = <&stmmac_axi_setup>;
+        snps,tso;
+        snps,en-tx-lpi-clockgating;
+        snps,lpi_en;
+        snps,txpbl = <16>;
+        snps,rxpbl = <16>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a70c1d0f303e..7eaaec8d3b96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19606,6 +19606,11 @@ F:	Documentation/devicetree/bindings/clock/starfive*
 F:	drivers/clk/starfive/
 F:	include/dt-bindings/clock/starfive*
 
+STARFIVE DWMAC GLUE LAYER
+M:	Yanhong Wang <yanhong.wang@starfivetech.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
+
 STARFIVE PINCTRL DRIVER
 M:	Emil Renner Berthing <kernel@esmil.dk>
 M:	Jianlong Huang <jianlong.huang@starfivetech.com>
-- 
2.17.1

[PATCH v1 4/7] net: phy: motorcomm: Add YT8531 phy support

[Yanhong Wang]

This adds basic support for the Motorcomm YT8531
Gigabit Ethernet PHY.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 drivers/net/phy/Kconfig     |   3 +-
 drivers/net/phy/motorcomm.c | 185 ++++++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index c57a0262fb64..86399254d9ff 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -258,9 +258,10 @@ config MICROSEMI_PHY
 
 config MOTORCOMM_PHY
 	tristate "Motorcomm PHYs"
+	default SOC_STARFIVE
 	help
 	  Enables support for Motorcomm network PHYs.
-	  Currently supports the YT8511 gigabit PHY.
+	  Currently supports the YT8511 and YT8531 gigabit PHYs.
 
 config NATIONAL_PHY
 	tristate "National Semiconductor PHYs"
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 7e6ac2c5e27e..7f3e22879399 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -3,13 +3,17 @@
  * Driver for Motorcomm PHYs
  *
  * Author: Peter Geis <pgwipeout@gmail.com>
+ *
  */
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 
 #define PHY_ID_YT8511		0x0000010a
+#define PHY_ID_YT8531		0x4f51e91b
 
 #define YT8511_PAGE_SELECT	0x1e
 #define YT8511_PAGE		0x1f
@@ -17,6 +21,10 @@
 #define YT8511_EXT_DELAY_DRIVE	0x0d
 #define YT8511_EXT_SLEEP_CTRL	0x27
 
+#define YTPHY_EXT_SMI_SDS_PHY		0xa000
+#define YTPHY_EXT_CHIP_CONFIG		0xa001
+#define YTPHY_EXT_RGMII_CONFIG1	0xa003
+
 /* 2b00 25m from pll
  * 2b01 25m from xtl *default*
  * 2b10 62.m from pll
@@ -38,6 +46,34 @@
 #define YT8511_DELAY_FE_TX_EN	(0xf << 12)
 #define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
 
+struct ytphy_reg_field {
+	char *name;
+	u32 mask;
+	u8	dflt;	/* Default value */
+};
+
+struct ytphy_priv_t {
+	u32 tx_inverted_1000;
+	u32 tx_inverted_100;
+	u32 tx_inverted_10;
+};
+
+static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
+	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
+	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
+	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
+};
+
+static const struct ytphy_reg_field ytphy_txinver_grp[] = {
+	{ "tx_inverted_1000", BIT(14), 0x0 },
+	{ "tx_inverted_100", BIT(14), 0x0 },
+	{ "tx_inverted_10", BIT(14), 0x0 }
+};
+
+static const struct ytphy_reg_field ytphy_rxden_grp[] = {
+	{ "rxc_dly_en", BIT(8), 0x1 }
+};
+
 static int yt8511_read_page(struct phy_device *phydev)
 {
 	return __phy_read(phydev, YT8511_PAGE_SELECT);
@@ -48,6 +84,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page)
 	return __phy_write(phydev, YT8511_PAGE_SELECT, page);
 };
 
+static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
+{
+	int ret;
+	int val;
+
+	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	val = __phy_read(phydev, YT8511_PAGE);
+
+	return val;
+}
+
+static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val)
+{
+	int ret;
+
+	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	ret = __phy_write(phydev, YT8511_PAGE, val);
+
+	return ret;
+}
+
 static int yt8511_config_init(struct phy_device *phydev)
 {
 	int oldpage, ret = 0;
@@ -111,6 +174,116 @@ static int yt8511_config_init(struct phy_device *phydev)
 	return phy_restore_page(phydev, oldpage, ret);
 }
 
+static int ytphy_config_init(struct phy_device *phydev)
+{
+	struct device_node *of_node;
+	u32 val;
+	u32 mask;
+	u32 cfg;
+	int ret;
+	int i = 0;
+
+	of_node = phydev->mdio.dev.of_node;
+	if (of_node) {
+		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
+		if (!ret) {
+			mask = ytphy_rxden_grp[0].mask;
+			val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
+
+			/* check the cfg overflow or not */
+			cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
+
+			val &= ~mask;
+			val |= FIELD_PREP(mask, cfg);
+			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);
+		}
+
+		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
+		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
+			ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
+			if (!ret) {
+				mask = ytphy_rxtxd_grp[i].mask;
+
+				/* check the cfg overflow or not */
+				cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
+
+				val &= ~mask;
+				val |= cfg << (ffs(mask) - 1);
+			}
+		}
+		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
+	}
+
+	phydev_err(phydev, "Get of node fail\n");
+
+	return -EINVAL;
+}
+
+static void ytphy_link_change_notify(struct phy_device *phydev)
+{
+	u32 val;
+	struct ytphy_priv_t *ytphy_priv = phydev->priv;
+
+	if (phydev->speed < 0)
+		return;
+
+	val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
+	switch (phydev->speed) {
+	case SPEED_1000:
+		val  &= ~ytphy_txinver_grp[0].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[0].mask,
+				ytphy_priv->tx_inverted_1000);
+		break;
+
+	case SPEED_100:
+		val  &= ~ytphy_txinver_grp[1].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[1].mask,
+				ytphy_priv->tx_inverted_100);
+		break;
+
+	case SPEED_10:
+		val  &= ~ytphy_txinver_grp[2].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[2].mask,
+				ytphy_priv->tx_inverted_10);
+		break;
+
+	default:
+		break;
+	}
+
+	ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
+}
+
+static int yt8531_probe(struct phy_device *phydev)
+{
+	struct ytphy_priv_t *priv;
+	const struct device_node *of_node;
+	u32 val;
+	int ret;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	of_node = phydev->mdio.dev.of_node;
+	if (of_node) {
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val);
+		if (!ret)
+			priv->tx_inverted_1000 = val;
+
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val);
+		if (!ret)
+			priv->tx_inverted_100 = val;
+
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val);
+		if (!ret)
+			priv->tx_inverted_10 = val;
+	}
+	phydev->priv = priv;
+
+	return 0;
+}
+
 static struct phy_driver motorcomm_phy_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
@@ -120,6 +293,17 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 		.resume		= genphy_resume,
 		.read_page	= yt8511_read_page,
 		.write_page	= yt8511_write_page,
+	}, {
+		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
+		.name		= "YT8531 Gigabit Ethernet",
+		.probe		= yt8531_probe,
+		.config_init	= ytphy_config_init,
+		.read_status	= genphy_read_status,
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+		.read_page	= yt8511_read_page,
+		.write_page	= yt8511_write_page,
+		.link_change_notify = ytphy_link_change_notify,
 	},
 };
 
@@ -131,6 +315,7 @@ MODULE_LICENSE("GPL");
 
 static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8511) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
 	{ /* sentinal */ }
 };
 
-- 
2.17.1

[PATCH v1 5/7] net: stmmac: Add StarFive dwmac supoort

[Yanhong Wang]

This adds StarFive dwmac driver support on the StarFive JH7110 SoCs.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 MAINTAINERS                                   |   1 +
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-starfive-plat.c      | 147 ++++++++++++++++++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7eaaec8d3b96..36cb00cf860b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19610,6 +19610,7 @@ STARFIVE DWMAC GLUE LAYER
 M:	Yanhong Wang <yanhong.wang@starfivetech.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
+F:	drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
 
 STARFIVE PINCTRL DRIVER
 M:	Emil Renner Berthing <kernel@esmil.dk>
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 31ff35174034..1e29cd3770b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -235,6 +235,17 @@ config DWMAC_INTEL_PLAT
 	  the stmmac device driver. This driver is used for the Intel Keem Bay
 	  SoC.
 
+config DWMAC_STARFIVE_PLAT
+	tristate "StarFive dwmac support"
+	depends on OF && COMMON_CLK
+	depends on STMMAC_ETH
+	default SOC_STARFIVE
+	help
+	  Support for ethernet controllers on StarFive RISC-V SoCs
+
+	  This selects the StarFive platform specific glue layer support for
+	  the stmmac device driver. This driver is used for StarFive RISC-V SoCs.
+
 config DWMAC_VISCONTI
 	tristate "Toshiba Visconti DWMAC support"
 	default ARCH_VISCONTI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..a63ab0ab5071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
 obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
 obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
 obj-$(CONFIG_DWMAC_IMX8)	+= dwmac-imx.o
+obj-$(CONFIG_DWMAC_STARFIVE_PLAT)	+= dwmac-starfive-plat.o
 obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
 stmmac-platform-objs:= stmmac_platform.o
 dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
new file mode 100644
index 000000000000..8fbf584d4e19
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * StarFive DWMAC platform driver
+ *
+ * Copyright(C) 2022 StarFive Technology Co., Ltd.
+ *
+ */
+
+#include <linux/of_device.h>
+#include "stmmac_platform.h"
+
+struct starfive_dwmac {
+	struct device *dev;
+	struct clk *clk_tx;
+	struct clk *clk_gtx;
+	struct clk *clk_gtxc;
+};
+
+static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
+{
+	struct starfive_dwmac *dwmac = priv;
+	unsigned long rate;
+	int err;
+
+	switch (speed) {
+	case SPEED_1000:
+		rate = 125000000;
+		break;
+	case SPEED_100:
+		rate = 25000000;
+		break;
+	case SPEED_10:
+		rate = 2500000;
+		break;
+	default:
+		dev_err(dwmac->dev, "invalid speed %u\n", speed);
+		return;
+	}
+
+	err = clk_set_rate(dwmac->clk_gtx, rate);
+	if (err)
+		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
+}
+
+static void dwmac_starfive_clk_disable(void *clk)
+{
+	clk_disable_unprepare(clk);
+}
+
+static int starfive_eth_plat_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct starfive_dwmac *dwmac;
+	int err;
+
+	err = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (err)
+		return err;
+
+	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat)) {
+		dev_err(&pdev->dev, "dt configuration failed\n");
+		return PTR_ERR(plat_dat);
+	}
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	dwmac->clk_tx = devm_clk_get(&pdev->dev, "tx");
+	if (IS_ERR(dwmac->clk_tx))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
+						"error getting tx clock\n");
+
+	err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
+			      dwmac->clk_tx);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(dwmac->clk_tx);
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "error enabling tx clock\n");
+
+	dwmac->clk_gtx = devm_clk_get(&pdev->dev, "gtx");
+	if (IS_ERR(dwmac->clk_gtx))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
+						"error getting gtx clock\n");
+
+	err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
+			      dwmac->clk_gtx);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(dwmac->clk_gtx);
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "error enabling gtx clock\n");
+
+	dwmac->clk_gtxc = devm_clk_get(&pdev->dev, "gtxc");
+	if (IS_ERR(dwmac->clk_gtxc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtxc),
+						"error getting gtxc clock\n");
+
+	err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
+			      dwmac->clk_gtxc);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(dwmac->clk_gtxc);
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "error enabling gtxc clock\n");
+
+	dwmac->dev = &pdev->dev;
+	plat_dat->fix_mac_speed = starfive_eth_fix_mac_speed;
+	plat_dat->init = NULL;
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->dma_cfg->dche = true;
+
+	err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (err) {
+		stmmac_remove_config_dt(pdev, plat_dat);
+		return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id starfive_eth_plat_match[] = {
+	{.compatible = "starfive,dwmac"},
+	{ }
+};
+
+static struct platform_driver starfive_eth_plat_driver = {
+	.probe  = starfive_eth_plat_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name = "starfive-eth-plat",
+		.pm = &stmmac_pltfr_pm_ops,
+		.of_match_table = starfive_eth_plat_match,
+	},
+};
+
+module_platform_driver(starfive_eth_plat_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("StarFive DWMAC platform driver");
+MODULE_AUTHOR("Yanhong Wang <yanhong.wang@starfivetech.com>");
-- 
2.17.1

[PATCH v1 6/7] riscv: dts: starfive: jh7110: Add ethernet device node

[Yanhong Wang]

Add JH7110 ethernet device node to support gmac driver for the JH7110
RISC-V SoC.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 80 ++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index c22e8f1d2640..97ed5418d91f 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -433,5 +433,85 @@
 			reg-shift = <2>;
 			status = "disabled";
 		};
+
+		stmmac_axi_setup: stmmac-axi-config {
+			snps,wr_osr_lmt = <4>;
+			snps,rd_osr_lmt = <4>;
+			snps,blen = <256 128 64 32 0 0 0>;
+		};
+
+		gmac0: ethernet@16030000 {
+			compatible = "starfive,dwmac", "snps,dwmac-5.20";
+			reg = <0x0 0x16030000 0x0 0x10000>;
+			clocks = <&aoncrg JH7110_AONCLK_GMAC0_AXI>,
+				 <&aoncrg JH7110_AONCLK_GMAC0_AHB>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_PTP>,
+				 <&aoncrg JH7110_AONCLK_GMAC0_TX>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_GTXC>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>;
+			clock-names = "stmmaceth",
+					"pclk",
+					"ptp_ref",
+					"tx",
+					"gtxc",
+					"gtx";
+			resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
+				 <&aoncrg JH7110_AONRST_GMAC0_AHB>;
+			reset-names = "stmmaceth", "ahb";
+			interrupts = <7>, <6>, <5> ;
+			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+			phy-mode = "rgmii-id";
+			snps,multicast-filter-bins = <64>;
+			snps,perfect-filter-entries = <8>;
+			rx-fifo-depth = <2048>;
+			tx-fifo-depth = <2048>;
+			snps,fixed-burst;
+			snps,no-pbl-x8;
+			snps,force_thresh_dma_mode;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,tso;
+			snps,en-tx-lpi-clockgating;
+			snps,lpi_en;
+			snps,txpbl = <16>;
+			snps,rxpbl = <16>;
+			status = "disabled";
+		};
+
+		gmac1: ethernet@16040000 {
+			compatible = "starfive,dwmac", "snps,dwmac-5.20";
+			reg = <0x0 0x16040000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_GMAC1_AXI>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_AHB>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_PTP>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_TX>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_GTXC>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_GTXCLK>;
+			clock-names = "stmmaceth",
+					"pclk",
+					"ptp_ref",
+					"tx",
+					"gtxc",
+					"gtx";
+			resets = <&syscrg JH7110_SYSRST_GMAC1_AXI>,
+				 <&syscrg JH7110_SYSRST_GMAC1_AHB>;
+			reset-names = "stmmaceth", "ahb";
+			interrupts = <78>, <77>, <76> ;
+			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+			phy-mode = "rgmii-id";
+			snps,multicast-filter-bins = <64>;
+			snps,perfect-filter-entries = <8>;
+			rx-fifo-depth = <2048>;
+			tx-fifo-depth = <2048>;
+			snps,fixed-burst;
+			snps,no-pbl-x8;
+			snps,force_thresh_dma_mode;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,tso;
+			snps,en-tx-lpi-clockgating;
+			snps,lpi_en;
+			snps,txpbl = <16>;
+			snps,rxpbl = <16>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.17.1

[PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[Yanhong Wang]

Add phy delay_chain configuration to support motorcomm phy driver for
StarFive VisionFive 2 board.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../jh7110-starfive-visionfive-v2.dts         | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index c8946cf3a268..2868ef4c74ef 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
@@ -15,6 +15,8 @@
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0=&gmac0;
+		ethernet1=&gmac1;
 	};
 
 	chosen {
@@ -114,3 +116,47 @@
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
+
+&gmac0 {
+	status = "okay";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	phy-handle = <&phy0>;
+	status = "okay";
+	mdio0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		phy0: ethernet-phy@0 {
+			reg = <0>;
+			rxc_dly_en = <1>;
+			tx_delay_sel_fe = <5>;
+			tx_delay_sel = <0xa>;
+			tx_inverted_10 = <0x1>;
+			tx_inverted_100 = <0x1>;
+			tx_inverted_1000 = <0x1>;
+		};
+	};
+};
+
+&gmac1 {
+	status = "okay";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	phy-handle = <&phy1>;
+	status = "okay";
+	mdio1 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		phy1: ethernet-phy@1 {
+			reg = <1>;
+			tx_delay_sel_fe = <5>;
+			tx_delay_sel = <0>;
+			rxc_dly_en = <0>;
+			tx_inverted_10 = <0x1>;
+			tx_inverted_100 = <0x1>;
+			tx_inverted_1000 = <0x0>;
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Rob Herring]

On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
>  MAINTAINERS                                   |   5 +
>  3 files changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
   21 |         #include <dt-bindings/clock/starfive-jh7110.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221201090242.2381-4-yanhong.wang@starfivetech.com

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

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

pip3 install dtschema --upgrade

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

Re: [PATCH v1 4/7] net: phy: motorcomm: Add YT8531 phy support

[Andrew Lunn]

> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
> +	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
> +	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
> +	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
> +};
> +
> +static const struct ytphy_reg_field ytphy_txinver_grp[] = {
> +	{ "tx_inverted_1000", BIT(14), 0x0 },
> +	{ "tx_inverted_100", BIT(14), 0x0 },
> +	{ "tx_inverted_10", BIT(14), 0x0 }
> +};
> +
> +static const struct ytphy_reg_field ytphy_rxden_grp[] = {
> +	{ "rxc_dly_en", BIT(8), 0x1 }
> +};
> +
> +static int ytphy_config_init(struct phy_device *phydev)
> +{
> +	struct device_node *of_node;
> +	u32 val;
> +	u32 mask;
> +	u32 cfg;
> +	int ret;
> +	int i = 0;
> +
> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {
> +		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);

You need to document the device tree binding.

Frank Sae always gets the locking wrong in this driver. Have you
tested your patch with lockdep enabled?

    Andrew

Re: [PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[Krzysztof Kozlowski]

On 01/12/2022 10:02, Yanhong Wang wrote:
> Add dwmac-5.20 version to snps.dwmac.yaml

Drop full stop from subject and add it here instead.

> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>

Two people contributed this one single line?

Best regards,
Krzysztof

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Krzysztof Kozlowski]

On 01/12/2022 10:02, Yanhong Wang wrote:
> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.

Subject: drop second, redundant "bindings".

> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
>  MAINTAINERS                                   |   5 +
>  3 files changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index d8779d3de3d6..13c5928d7170 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -33,6 +33,7 @@ select:
>            - snps,dwmac-5.20
>            - snps,dwxgmac
>            - snps,dwxgmac-2.10
> +          - starfive,dwmac
>  
>            # Deprecated
>            - st,spear600-gmac
> diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> new file mode 100644
> index 000000000000..561cf2a713ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml

Filename should base on compatible.

> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes from both lines.

> +
> +title: StarFive DWMAC glue layer
> +
> +maintainers:
> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - starfive,dwmac
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: "snps,dwmac.yaml#"

Drop quotes.

> +
> +properties:
> +  compatible:
> +    oneOf:

Drop oneOf. You do not have more cases here.

> +      - items:
> +          - enum:
> +               - starfive,dwmac

Wrong indentation.... kind of expected since you did not test the bindings.

> +          - const: snps,dwmac-5.20
> +
> +  clocks:
> +    items:
> +      - description: GMAC main clock
> +      - description: GMAC AHB clock
> +      - description: PTP clock
> +      - description: TX clock
> +      - description: GTXC clock
> +      - description: GTX clock
> +
> +  clock-names:
> +    contains:
> +      enum:
> +        - stmmaceth
> +        - pclk
> +        - ptp_ref
> +        - tx
> +        - gtxc
> +        - gtx

Names should be specific and with fixed order, just like clocks are.

> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false
> +

Best regards,
Krzysztof

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Conor Dooley]

Hey Yanhong,

On Thu, Dec 01, 2022 at 07:36:29AM -0600, Rob Herring wrote:
> 
> On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> > 
> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> >  .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
> >  MAINTAINERS                                   |   5 +
> >  3 files changed, 112 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)
> 
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
> Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
>    21 |         #include <dt-bindings/clock/starfive-jh7110.h>

Perhaps, rather than putting a long list of "prerequisite-patch-id" in
your cover letters etc, you drop the need for headers from your bindings
entirely? Otherwise, you need to wait for the clock bindings to be applied
before any of your other peripherals etc can have drivers/bindings upstream.

AFAIU, and Rob/Krzk please correct me, the example in a dt-binding
really is an *example* and there's no requirement for it to match the
jh7110 dts exactly. Because of that you can drop the header & just do
something like `clocks = <&clk 7>, <&clk 77>;` etc and the example is
still valid. Same goes for all of the other driver patchsets for new
StarFive stuff, like the pmu or crypto, that also have dt-bindings.

The only person who has to worry then about dependencies is me when I
apply the .dts patches :)

>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221201090242.2381-4-yanhong.wang@starfivetech.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Conor Dooley]

On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
> On 01/12/2022 10:02, Yanhong Wang wrote:
> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> 
> Subject: drop second, redundant "bindings".
> 
> > 
> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > +properties:
> > +  compatible:
> > +    oneOf:
> 
> Drop oneOf. You do not have more cases here.
> 
> > +      - items:
> > +          - enum:
> > +               - starfive,dwmac
> 
> Wrong indentation.... kind of expected since you did not test the bindings.
> 
> > +          - const: snps,dwmac-5.20

Disclaimer: no familiarity with the version info with DW stuff

Is it a bit foolish to call this binding "starfive,dwmac"? Could there
not be another StarFive SoC in the future that uses another DW mac IP
version & this would be better off as "starfive,jh7110-dwmac" or similar?

Thanks,
Conor.

Re: [PATCH v1 6/7] riscv: dts: starfive: jh7110: Add ethernet device node

[Conor Dooley]

On Thu, Dec 01, 2022 at 05:02:41PM +0800, Yanhong Wang wrote:
> Add JH7110 ethernet device node to support gmac driver for the JH7110
> RISC-V SoC.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 80 ++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index c22e8f1d2640..97ed5418d91f 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -433,5 +433,85 @@
>  			reg-shift = <2>;
>  			status = "disabled";
>  		};
> +
> +		stmmac_axi_setup: stmmac-axi-config {
> +			snps,wr_osr_lmt = <4>;
> +			snps,rd_osr_lmt = <4>;
> +			snps,blen = <256 128 64 32 0 0 0>;
> +		};
> +
> +		gmac0: ethernet@16030000 {
> +			compatible = "starfive,dwmac", "snps,dwmac-5.20";
> +			reg = <0x0 0x16030000 0x0 0x10000>;
> +			clocks = <&aoncrg JH7110_AONCLK_GMAC0_AXI>,
> +				 <&aoncrg JH7110_AONCLK_GMAC0_AHB>,
> +				 <&syscrg JH7110_SYSCLK_GMAC0_PTP>,
> +				 <&aoncrg JH7110_AONCLK_GMAC0_TX>,
> +				 <&syscrg JH7110_SYSCLK_GMAC0_GTXC>,
> +				 <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>;
> +			clock-names = "stmmaceth",
> +					"pclk",
> +					"ptp_ref",
> +					"tx",
> +					"gtxc",
> +					"gtx";

Can you sort this into fewer lines please?

> +			resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
> +				 <&aoncrg JH7110_AONRST_GMAC0_AHB>;
> +			reset-names = "stmmaceth", "ahb";
> +			interrupts = <7>, <6>, <5> ;

Please also remove the space before the ;

> +			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

The answer is probably "the dw driver needs this" but my OCD really
hates "macirq" vs "eth_wake_irq"..

> +			phy-mode = "rgmii-id";
> +			snps,multicast-filter-bins = <64>;
> +			snps,perfect-filter-entries = <8>;
> +			rx-fifo-depth = <2048>;
> +			tx-fifo-depth = <2048>;
> +			snps,fixed-burst;
> +			snps,no-pbl-x8;
> +			snps,force_thresh_dma_mode;
> +			snps,axi-config = <&stmmac_axi_setup>;
> +			snps,tso;
> +			snps,en-tx-lpi-clockgating;
> +			snps,lpi_en;
> +			snps,txpbl = <16>;
> +			snps,rxpbl = <16>;
> +			status = "disabled";
> +		};
> +
> +		gmac1: ethernet@16040000 {
> +			compatible = "starfive,dwmac", "snps,dwmac-5.20";
> +			reg = <0x0 0x16040000 0x0 0x10000>;
> +			clocks = <&syscrg JH7110_SYSCLK_GMAC1_AXI>,
> +				 <&syscrg JH7110_SYSCLK_GMAC1_AHB>,
> +				 <&syscrg JH7110_SYSCLK_GMAC1_PTP>,
> +				 <&syscrg JH7110_SYSCLK_GMAC1_TX>,
> +				 <&syscrg JH7110_SYSCLK_GMAC1_GTXC>,
> +				 <&syscrg JH7110_SYSCLK_GMAC1_GTXCLK>;
> +			clock-names = "stmmaceth",
> +					"pclk",
> +					"ptp_ref",
> +					"tx",
> +					"gtxc",
> +					"gtx";
> +			resets = <&syscrg JH7110_SYSRST_GMAC1_AXI>,
> +				 <&syscrg JH7110_SYSRST_GMAC1_AHB>;
> +			reset-names = "stmmaceth", "ahb";
> +			interrupts = <78>, <77>, <76> ;

Same comments for this node.

> +			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> +			phy-mode = "rgmii-id";
> +			snps,multicast-filter-bins = <64>;
> +			snps,perfect-filter-entries = <8>;
> +			rx-fifo-depth = <2048>;
> +			tx-fifo-depth = <2048>;
> +			snps,fixed-burst;
> +			snps,no-pbl-x8;
> +			snps,force_thresh_dma_mode;
> +			snps,axi-config = <&stmmac_axi_setup>;
> +			snps,tso;
> +			snps,en-tx-lpi-clockgating;
> +			snps,lpi_en;
> +			snps,txpbl = <16>;
> +			snps,rxpbl = <16>;
> +			status = "disabled";
> +		};
>  	};
>  };
> -- 
> 2.17.1
> 

Re: [PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[Conor Dooley]

On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote:
> Add phy delay_chain configuration to support motorcomm phy driver for
> StarFive VisionFive 2 board.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-v2.dts         | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> index c8946cf3a268..2868ef4c74ef 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> @@ -15,6 +15,8 @@
>  
>  	aliases {
>  		serial0 = &uart0;
> +		ethernet0=&gmac0;
> +		ethernet1=&gmac1;

Please match the whitespace usage of the existing entry.

>  	};
>  
>  	chosen {
> @@ -114,3 +116,47 @@
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";
>  };
> +
> +&gmac0 {
> +	status = "okay";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	phy-handle = <&phy0>;
> +	status = "okay";
> +	mdio0 {

A line of whitespace before the child nodes too please :)

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +		phy0: ethernet-phy@0 {
> +			reg = <0>;
> +			rxc_dly_en = <1>;
> +			tx_delay_sel_fe = <5>;
> +			tx_delay_sel = <0xa>;
> +			tx_inverted_10 = <0x1>;
> +			tx_inverted_100 = <0x1>;
> +			tx_inverted_1000 = <0x1>;
> +		};
> +	};
> +};
> +
> +&gmac1 {
> +	status = "okay";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	phy-handle = <&phy1>;
> +	status = "okay";
> +	mdio1 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +		phy1: ethernet-phy@1 {
> +			reg = <1>;
> +			tx_delay_sel_fe = <5>;
> +			tx_delay_sel = <0>;
> +			rxc_dly_en = <0>;
> +			tx_inverted_10 = <0x1>;
> +			tx_inverted_100 = <0x1>;
> +			tx_inverted_1000 = <0x0>;
> +		};
> +	};
> +};
> -- 
> 2.17.1
> 

Re: [PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[Conor Dooley]

On Thu, Dec 01, 2022 at 05:49:08PM +0000, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote:
> > riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration
> > 
> > Add phy delay_chain configuration to support motorcomm phy driver for
> > StarFive VisionFive 2 board.

nit: please re-word this commit next time around to actually say what
you're doing here. I didn't notice it initially, but this patch is doing
a lot more than adding `delay_chain` configuration. To my dwmac unaware
brain, there's nothing hits for that term outside of the changelog :(

Thanks,
Conor.

> > 
> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> > ---
> >  .../jh7110-starfive-visionfive-v2.dts         | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > index c8946cf3a268..2868ef4c74ef 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > @@ -15,6 +15,8 @@
> >  
> >  	aliases {
> >  		serial0 = &uart0;
> > +		ethernet0=&gmac0;
> > +		ethernet1=&gmac1;
> 
> Please match the whitespace usage of the existing entry.
> 
> >  	};
> >  
> >  	chosen {
> > @@ -114,3 +116,47 @@
> >  	pinctrl-0 = <&uart0_pins>;
> >  	status = "okay";
> >  };
> > +
> > +&gmac0 {
> > +	status = "okay";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	phy-handle = <&phy0>;
> > +	status = "okay";
> > +	mdio0 {
> 
> A line of whitespace before the child nodes too please :)
> 
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "snps,dwmac-mdio";
> > +		phy0: ethernet-phy@0 {
> > +			reg = <0>;
> > +			rxc_dly_en = <1>;
> > +			tx_delay_sel_fe = <5>;
> > +			tx_delay_sel = <0xa>;
> > +			tx_inverted_10 = <0x1>;
> > +			tx_inverted_100 = <0x1>;
> > +			tx_inverted_1000 = <0x1>;
> > +		};
> > +	};
> > +};
> > +
> > +&gmac1 {
> > +	status = "okay";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	phy-handle = <&phy1>;
> > +	status = "okay";
> > +	mdio1 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "snps,dwmac-mdio";
> > +		phy1: ethernet-phy@1 {
> > +			reg = <1>;
> > +			tx_delay_sel_fe = <5>;
> > +			tx_delay_sel = <0>;
> > +			rxc_dly_en = <0>;
> > +			tx_inverted_10 = <0x1>;
> > +			tx_inverted_100 = <0x1>;
> > +			tx_inverted_1000 = <0x0>;
> > +		};
> > +	};
> > +};
> > -- 
> > 2.17.1
> > 

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Rob Herring]

On Thu, Dec 01, 2022 at 05:35:37PM +0000, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
> > On 01/12/2022 10:02, Yanhong Wang wrote:
> > > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> > 
> > Subject: drop second, redundant "bindings".
> > 
> > > 
> > > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> > > ---
> > >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > 
> > Drop oneOf. You do not have more cases here.
> > 
> > > +      - items:
> > > +          - enum:
> > > +               - starfive,dwmac
> > 
> > Wrong indentation.... kind of expected since you did not test the bindings.
> > 
> > > +          - const: snps,dwmac-5.20
> 
> Disclaimer: no familiarity with the version info with DW stuff
> 
> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
> not be another StarFive SoC in the future that uses another DW mac IP
> version & this would be better off as "starfive,jh7110-dwmac" or similar?

Yes. Really, *only* "starfive,jh7110-dwmac" is enough IMO. 

The question is what would the OS do with only understanding 
"snps,dwmac-5.20"? The answer is typically nothing because it isn't 
enough information to act on. So that compatible is not needed. Maybe 
the driver can do some things based on version, but that can be implied 
from the compatible (if not read from a register). And often, the exact 
version is not known, so do you want to hardcode a guess in DT? For 
these reasons, we've moved away from using these generic IP compatibles 
(with or without versions).

Rob

Re: [PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[Andrew Lunn]

On Thu, Dec 01, 2022 at 07:30:16PM +0000, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:49:08PM +0000, Conor Dooley wrote:
> > On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote:
> > > riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration
> > > 
> > > Add phy delay_chain configuration to support motorcomm phy driver for
> > > StarFive VisionFive 2 board.
> 
> nit: please re-word this commit next time around to actually say what
> you're doing here. I didn't notice it initially, but this patch is doing
> a lot more than adding `delay_chain` configuration. To my dwmac unaware
> brain, there's nothing hits for that term outside of the changelog :(

Hi Conor

I suspect once we see the documentation of the binding, it will get
rejected and implemented differently. So i would not worry too much
about this at the moment.

      Andrew

Re: [PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[yanhong wang]

On 2022/12/2 0:18, Krzysztof Kozlowski wrote:
> On 01/12/2022 10:02, Yanhong Wang wrote:
>> Add dwmac-5.20 version to snps.dwmac.yaml
> 
> Drop full stop from subject and add it here instead.
> 

Will update in the next version.

>> 
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> 
> Two people contributed this one single line?
> 

Emil made this patch and I submitted it.

> Best regards,
> Krzysztof
> 

Re: [PATCH v1 6/7] riscv: dts: starfive: jh7110: Add ethernet device node

[yanhong wang]

On 2022/12/2 1:45, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:02:41PM +0800, Yanhong Wang wrote:
>> Add JH7110 ethernet device node to support gmac driver for the JH7110
>> RISC-V SoC.
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 80 ++++++++++++++++++++++++
>>  1 file changed, 80 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index c22e8f1d2640..97ed5418d91f 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -433,5 +433,85 @@
>>  			reg-shift = <2>;
>>  			status = "disabled";
>>  		};
>> +
>> +		stmmac_axi_setup: stmmac-axi-config {
>> +			snps,wr_osr_lmt = <4>;
>> +			snps,rd_osr_lmt = <4>;
>> +			snps,blen = <256 128 64 32 0 0 0>;
>> +		};
>> +
>> +		gmac0: ethernet@16030000 {
>> +			compatible = "starfive,dwmac", "snps,dwmac-5.20";
>> +			reg = <0x0 0x16030000 0x0 0x10000>;
>> +			clocks = <&aoncrg JH7110_AONCLK_GMAC0_AXI>,
>> +				 <&aoncrg JH7110_AONCLK_GMAC0_AHB>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC0_PTP>,
>> +				 <&aoncrg JH7110_AONCLK_GMAC0_TX>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC0_GTXC>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>;
>> +			clock-names = "stmmaceth",
>> +					"pclk",
>> +					"ptp_ref",
>> +					"tx",
>> +					"gtxc",
>> +					"gtx";
> 
> Can you sort this into fewer lines please?

Will sort in the next version.

> 
>> +			resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
>> +				 <&aoncrg JH7110_AONRST_GMAC0_AHB>;
>> +			reset-names = "stmmaceth", "ahb";
>> +			interrupts = <7>, <6>, <5> ;
> 
> Please also remove the space before the ;

Will remove the space.

> 
>> +			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> 
> The answer is probably "the dw driver needs this" but my OCD really
> hates "macirq" vs "eth_wake_irq"..

The definition of  "macirq" and "eth_wake_irq"  is to reuse stmmac_get_platform_resources() API. 

> 
>> +			phy-mode = "rgmii-id";
>> +			snps,multicast-filter-bins = <64>;
>> +			snps,perfect-filter-entries = <8>;
>> +			rx-fifo-depth = <2048>;
>> +			tx-fifo-depth = <2048>;
>> +			snps,fixed-burst;
>> +			snps,no-pbl-x8;
>> +			snps,force_thresh_dma_mode;
>> +			snps,axi-config = <&stmmac_axi_setup>;
>> +			snps,tso;
>> +			snps,en-tx-lpi-clockgating;
>> +			snps,lpi_en;
>> +			snps,txpbl = <16>;
>> +			snps,rxpbl = <16>;
>> +			status = "disabled";
>> +		};
>> +
>> +		gmac1: ethernet@16040000 {
>> +			compatible = "starfive,dwmac", "snps,dwmac-5.20";
>> +			reg = <0x0 0x16040000 0x0 0x10000>;
>> +			clocks = <&syscrg JH7110_SYSCLK_GMAC1_AXI>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC1_AHB>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC1_PTP>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC1_TX>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC1_GTXC>,
>> +				 <&syscrg JH7110_SYSCLK_GMAC1_GTXCLK>;
>> +			clock-names = "stmmaceth",
>> +					"pclk",
>> +					"ptp_ref",
>> +					"tx",
>> +					"gtxc",
>> +					"gtx";
>> +			resets = <&syscrg JH7110_SYSRST_GMAC1_AXI>,
>> +				 <&syscrg JH7110_SYSRST_GMAC1_AHB>;
>> +			reset-names = "stmmaceth", "ahb";
>> +			interrupts = <78>, <77>, <76> ;
> 
> Same comments for this node.

Will remove the space.

> 
>> +			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
>> +			phy-mode = "rgmii-id";
>> +			snps,multicast-filter-bins = <64>;
>> +			snps,perfect-filter-entries = <8>;
>> +			rx-fifo-depth = <2048>;
>> +			tx-fifo-depth = <2048>;
>> +			snps,fixed-burst;
>> +			snps,no-pbl-x8;
>> +			snps,force_thresh_dma_mode;
>> +			snps,axi-config = <&stmmac_axi_setup>;
>> +			snps,tso;
>> +			snps,en-tx-lpi-clockgating;
>> +			snps,lpi_en;
>> +			snps,txpbl = <16>;
>> +			snps,rxpbl = <16>;
>> +			status = "disabled";
>> +		};
>>  	};
>>  };
>> -- 
>> 2.17.1
>> 

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[yanhong wang]

On 2022/12/2 0:21, Krzysztof Kozlowski wrote:
> On 01/12/2022 10:02, Yanhong Wang wrote:
>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> 
> Subject: drop second, redundant "bindings".
> 

I'll fix the title in the next version.

>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>>  .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
>>  MAINTAINERS                                   |   5 +
>>  3 files changed, 112 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index d8779d3de3d6..13c5928d7170 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -33,6 +33,7 @@ select:
>>            - snps,dwmac-5.20
>>            - snps,dwxgmac
>>            - snps,dwxgmac-2.10
>> +          - starfive,dwmac
>>  
>>            # Deprecated
>>            - st,spear600-gmac
>> diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>> new file mode 100644
>> index 000000000000..561cf2a713ab
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> 
> Filename should base on compatible.
> 

Will update file name.

>> @@ -0,0 +1,106 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"
> 
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> 
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> 
> Drop quotes from both lines.
> 

Will remove quotes.

>> +
>> +title: StarFive DWMAC glue layer
>> +
>> +maintainers:
>> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - starfive,dwmac
>> +  required:
>> +    - compatible
>> +
>> +allOf:
>> +  - $ref: "snps,dwmac.yaml#"
> 
> Drop quotes.
> 

Will remove quotes.

>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
> 
> Drop oneOf. You do not have more cases here.
> 

Will remove oneOf.

>> +      - items:
>> +          - enum:
>> +               - starfive,dwmac
> 
> Wrong indentation.... kind of expected since you did not test the bindings.
> 

Will fix.

>> +          - const: snps,dwmac-5.20
>> +
>> +  clocks:
>> +    items:
>> +      - description: GMAC main clock
>> +      - description: GMAC AHB clock
>> +      - description: PTP clock
>> +      - description: TX clock
>> +      - description: GTXC clock
>> +      - description: GTX clock
>> +
>> +  clock-names:
>> +    contains:
>> +      enum:
>> +        - stmmaceth
>> +        - pclk
>> +        - ptp_ref
>> +        - tx
>> +        - gtxc
>> +        - gtx
> 
> Names should be specific and with fixed order, just like clocks are.
> 

Will fix.

>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +
>> +unevaluatedProperties: false
>> +
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[yanhong wang]

On 2022/12/2 1:35, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
>> On 01/12/2022 10:02, Yanhong Wang wrote:
>> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>> 
>> Subject: drop second, redundant "bindings".
>> 
>> > 
>> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> > ---
>> >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>> > +properties:
>> > +  compatible:
>> > +    oneOf:
>> 
>> Drop oneOf. You do not have more cases here.
>> 
>> > +      - items:
>> > +          - enum:
>> > +               - starfive,dwmac
>> 
>> Wrong indentation.... kind of expected since you did not test the bindings.
>> 
>> > +          - const: snps,dwmac-5.20
> 
> Disclaimer: no familiarity with the version info with DW stuff
> 
> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
> not be another StarFive SoC in the future that uses another DW mac IP
> version & this would be better off as "starfive,jh7110-dwmac" or similar?
> 

The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac".

> Thanks,
> Conor.
> 

Re: [PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[Krzysztof Kozlowski]

On 02/12/2022 03:53, yanhong wang wrote:
> 
> 
> On 2022/12/2 0:18, Krzysztof Kozlowski wrote:
>> On 01/12/2022 10:02, Yanhong Wang wrote:
>>> Add dwmac-5.20 version to snps.dwmac.yaml
>>
>> Drop full stop from subject and add it here instead.
>>
> 
> Will update in the next version.
> 
>>>
>>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>
>> Two people contributed this one single line?
>>
> 
> Emil made this patch and I submitted it.

If Emil made this patch, then your From field is incorrect.

Best regards,
Krzysztof

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Krzysztof Kozlowski]

On 02/12/2022 08:16, yanhong wang wrote:
> 
> 
> On 2022/12/2 1:35, Conor Dooley wrote:
>> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
>>> On 01/12/2022 10:02, Yanhong Wang wrote:
>>>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>>>
>>> Subject: drop second, redundant "bindings".
>>>
>>>>
>>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>>> ---
>>>>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>
>>> Drop oneOf. You do not have more cases here.
>>>
>>>> +      - items:
>>>> +          - enum:
>>>> +               - starfive,dwmac
>>>
>>> Wrong indentation.... kind of expected since you did not test the bindings.
>>>
>>>> +          - const: snps,dwmac-5.20
>>
>> Disclaimer: no familiarity with the version info with DW stuff
>>
>> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
>> not be another StarFive SoC in the future that uses another DW mac IP
>> version & this would be better off as "starfive,jh7110-dwmac" or similar?
>>
> 
> The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac".

It's not a reason to make compatible generic.

Best regards,
Krzysztof

Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

[Krzysztof Kozlowski]

On 01/12/2022 17:45, Conor Dooley wrote:
> Hey Yanhong,
> 
> On Thu, Dec 01, 2022 at 07:36:29AM -0600, Rob Herring wrote:
>>
>> On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
>>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>>>
>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>> ---
>>>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>>>  .../bindings/net/starfive,dwmac-plat.yaml     | 106 ++++++++++++++++++
>>>  MAINTAINERS                                   |   5 +
>>>  3 files changed, 112 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)
>>
>> dtschema/dtc warnings/errors:
>> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
>> 	expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
>> Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
>>    21 |         #include <dt-bindings/clock/starfive-jh7110.h>
> 
> Perhaps, rather than putting a long list of "prerequisite-patch-id" in
> your cover letters etc, you drop the need for headers from your bindings
> entirely? Otherwise, you need to wait for the clock bindings to be applied
> before any of your other peripherals etc can have drivers/bindings upstream.
> 
> AFAIU, and Rob/Krzk please correct me, the example in a dt-binding
> really is an *example* and there's no requirement for it to match the
> jh7110 dts exactly. Because of that you can drop the header & just do
> something like `clocks = <&clk 7>, <&clk 77>;` etc and the example is
> still valid. Same goes for all of the other driver patchsets for new
> StarFive stuff, like the pmu or crypto, that also have dt-bindings.
> 
> The only person who has to worry then about dependencies is me when I
> apply the .dts patches :)

It's nice when example looks good and matching final DTS, but that's not
a requirement. Pretty often clocks/resets/domains are mocked for the
example.

Best regards,
Krzysztof

Re: [PATCH v1 4/7] net: phy: motorcomm: Add YT8531 phy support

[yanhong wang]

On 2022/12/1 23:40, Andrew Lunn wrote:
>> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
>> +	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
>> +	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
>> +	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
>> +};
>> +
>> +static const struct ytphy_reg_field ytphy_txinver_grp[] = {
>> +	{ "tx_inverted_1000", BIT(14), 0x0 },
>> +	{ "tx_inverted_100", BIT(14), 0x0 },
>> +	{ "tx_inverted_10", BIT(14), 0x0 }
>> +};
>> +
>> +static const struct ytphy_reg_field ytphy_rxden_grp[] = {
>> +	{ "rxc_dly_en", BIT(8), 0x1 }
>> +};
>> +
>> +static int ytphy_config_init(struct phy_device *phydev)
>> +{
>> +	struct device_node *of_node;
>> +	u32 val;
>> +	u32 mask;
>> +	u32 cfg;
>> +	int ret;
>> +	int i = 0;
>> +
>> +	of_node = phydev->mdio.dev.of_node;
>> +	if (of_node) {
>> +		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
> 
> You need to document the device tree binding.
> 
> Frank Sae always gets the locking wrong in this driver. Have you
> tested your patch with lockdep enabled?
> 

I will add document to describe details in the next version, such as "rx_delay_sel","tx_inverted_10" etc.

>     Andrew

Re: [PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[Emil Renner Berthing]

On Fri, 2 Dec 2022 at 09:04, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/12/2022 03:53, yanhong wang wrote:
> >
> >
> > On 2022/12/2 0:18, Krzysztof Kozlowski wrote:
> >> On 01/12/2022 10:02, Yanhong Wang wrote:
> >>> Add dwmac-5.20 version to snps.dwmac.yaml
> >>
> >> Drop full stop from subject and add it here instead.
> >>
> >
> > Will update in the next version.
> >
> >>>
> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> >>
> >> Two people contributed this one single line?
> >>
> >
> > Emil made this patch and I submitted it.
>
> If Emil made this patch, then your From field is incorrect.

Yes, please don't change the author of the commits you cherry-picked
from my tree.

But now I'm curious. Did you check with your colleagues that the dwmac
IP on the SoC is in fact version 5.20?
This was just an educated guess from my side.

/Emil

> Best regards,
> Krzysztof
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 2/7] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[Emil Renner Berthing]

On Thu, 1 Dec 2022 at 10:05, Yanhong Wang <yanhong.wang@starfivetech.com> wrote:
>
> Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
> to define some platform data in the glue layer.
>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>

Hi Yanhong.

Thanks for submitting this.
But just as a reminder. Please don't change the author of the commits
you cherry-picked from my tree.

/Emil

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 50f6b4a14be4..cc3b701af802 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>         if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
>             of_device_is_compatible(np, "snps,dwmac-4.10a") ||
>             of_device_is_compatible(np, "snps,dwmac-4.20a") ||
> -           of_device_is_compatible(np, "snps,dwmac-5.10a")) {
> +           of_device_is_compatible(np, "snps,dwmac-5.10a") ||
> +           of_device_is_compatible(np, "snps,dwmac-5.20")) {
>                 plat->has_gmac4 = 1;
>                 plat->has_gmac = 0;
>                 plat->pmt = 1;
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 5/7] net: stmmac: Add StarFive dwmac supoort

[Emil Renner Berthing]

Hi Yanhong,

Thanks for submitting this. Again please don't change the author of
the commits you cherry-picked from my tree. This is why we have the
Co-developed-by: tag so you can mark that you added or in this case
removed code.
I'd hoped you would still include the support for the JH7100 though.

Also you seem to have changed the name of some of the functions.
Please at least keep the prefix consistent if you do that. Now it's a
mix of dwmac_starfive_, starfive_eth_ and starfive_eth_plat.

On Thu, 1 Dec 2022 at 10:07, Yanhong Wang <yanhong.wang@starfivetech.com> wrote:
>
> This adds StarFive dwmac driver support on the StarFive JH7110 SoCs.
>
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  MAINTAINERS                                   |   1 +
>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>  .../stmicro/stmmac/dwmac-starfive-plat.c      | 147 ++++++++++++++++++
>  4 files changed, 160 insertions(+)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7eaaec8d3b96..36cb00cf860b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19610,6 +19610,7 @@ STARFIVE DWMAC GLUE LAYER
>  M:     Yanhong Wang <yanhong.wang@starfivetech.com>
>  S:     Maintained
>  F:     Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> +F:     drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>
>  STARFIVE PINCTRL DRIVER
>  M:     Emil Renner Berthing <kernel@esmil.dk>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 31ff35174034..1e29cd3770b9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -235,6 +235,17 @@ config DWMAC_INTEL_PLAT
>           the stmmac device driver. This driver is used for the Intel Keem Bay
>           SoC.
>
> +config DWMAC_STARFIVE_PLAT

Why did you add the _PLAT suffix? None of the other SoC wrappers have this..

> +       tristate "StarFive dwmac support"
> +       depends on OF && COMMON_CLK
> +       depends on STMMAC_ETH
> +       default SOC_STARFIVE
> +       help
> +         Support for ethernet controllers on StarFive RISC-V SoCs
> +
> +         This selects the StarFive platform specific glue layer support for
> +         the stmmac device driver. This driver is used for StarFive RISC-V SoCs.
> +
>  config DWMAC_VISCONTI
>         tristate "Toshiba Visconti DWMAC support"
>         default ARCH_VISCONTI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index d4e12e9ace4f..a63ab0ab5071 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)       += dwmac-dwc-qos-eth.o
>  obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>  obj-$(CONFIG_DWMAC_IMX8)       += dwmac-imx.o
> +obj-$(CONFIG_DWMAC_STARFIVE_PLAT)      += dwmac-starfive-plat.o
>  obj-$(CONFIG_DWMAC_VISCONTI)   += dwmac-visconti.o
>  stmmac-platform-objs:= stmmac_platform.o
>  dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
> new file mode 100644
> index 000000000000..8fbf584d4e19
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * StarFive DWMAC platform driver
> + *
> + * Copyright(C) 2022 StarFive Technology Co., Ltd.

Hmm.. where did my copyright go?

> + *
> + */
> +
> +#include <linux/of_device.h>
> +#include "stmmac_platform.h"
> +
> +struct starfive_dwmac {
> +       struct device *dev;
> +       struct clk *clk_tx;
> +       struct clk *clk_gtx;
> +       struct clk *clk_gtxc;
> +};
> +
> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
> +{
> +       struct starfive_dwmac *dwmac = priv;
> +       unsigned long rate;
> +       int err;
> +
> +       switch (speed) {
> +       case SPEED_1000:
> +               rate = 125000000;
> +               break;
> +       case SPEED_100:
> +               rate = 25000000;
> +               break;
> +       case SPEED_10:
> +               rate = 2500000;
> +               break;
> +       default:
> +               dev_err(dwmac->dev, "invalid speed %u\n", speed);
> +               return;
> +       }
> +
> +       err = clk_set_rate(dwmac->clk_gtx, rate);
> +       if (err)
> +               dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
> +}
> +
> +static void dwmac_starfive_clk_disable(void *clk)
> +{
> +       clk_disable_unprepare(clk);
> +}
> +
> +static int starfive_eth_plat_probe(struct platform_device *pdev)
> +{
> +       struct plat_stmmacenet_data *plat_dat;
> +       struct stmmac_resources stmmac_res;
> +       struct starfive_dwmac *dwmac;
> +       int err;
> +
> +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
> +       if (err)
> +               return err;
> +
> +       plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> +       if (IS_ERR(plat_dat)) {
> +               dev_err(&pdev->dev, "dt configuration failed\n");
> +               return PTR_ERR(plat_dat);
> +       }
> +
> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +       if (!dwmac)
> +               return -ENOMEM;
> +
> +       dwmac->clk_tx = devm_clk_get(&pdev->dev, "tx");
> +       if (IS_ERR(dwmac->clk_tx))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
> +                                               "error getting tx clock\n");
> +
> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
> +                             dwmac->clk_tx);
> +       if (err)
> +               return err;
> +
> +       err = clk_prepare_enable(dwmac->clk_tx);
> +       if (err)
> +               return dev_err_probe(&pdev->dev, err, "error enabling tx clock\n");
> +
> +       dwmac->clk_gtx = devm_clk_get(&pdev->dev, "gtx");
> +       if (IS_ERR(dwmac->clk_gtx))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
> +                                               "error getting gtx clock\n");
> +
> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
> +                             dwmac->clk_gtx);
> +       if (err)
> +               return err;
> +
> +       err = clk_prepare_enable(dwmac->clk_gtx);
> +       if (err)
> +               return dev_err_probe(&pdev->dev, err, "error enabling gtx clock\n");

I think the 3 calls above can be simplified to devm_clk_get_enabled().

> +       dwmac->clk_gtxc = devm_clk_get(&pdev->dev, "gtxc");
> +       if (IS_ERR(dwmac->clk_gtxc))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtxc),
> +                                               "error getting gtxc clock\n");
> +
> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
> +                             dwmac->clk_gtxc);
> +       if (err)
> +               return err;
> +
> +       err = clk_prepare_enable(dwmac->clk_gtxc);
> +       if (err)
> +               return dev_err_probe(&pdev->dev, err, "error enabling gtxc clock\n");

Same here.

> +
> +       dwmac->dev = &pdev->dev;
> +       plat_dat->fix_mac_speed = starfive_eth_fix_mac_speed;
> +       plat_dat->init = NULL;
> +       plat_dat->bsp_priv = dwmac;
> +       plat_dat->dma_cfg->dche = true;
> +
> +       err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +       if (err) {
> +               stmmac_remove_config_dt(pdev, plat_dat);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id starfive_eth_plat_match[] = {
> +       {.compatible = "starfive,dwmac"},
> +       { }
> +};
> +
> +static struct platform_driver starfive_eth_plat_driver = {
> +       .probe  = starfive_eth_plat_probe,
> +       .remove = stmmac_pltfr_remove,
> +       .driver = {
> +               .name = "starfive-eth-plat",
> +               .pm = &stmmac_pltfr_pm_ops,
> +               .of_match_table = starfive_eth_plat_match,
> +       },
> +};
> +
> +module_platform_driver(starfive_eth_plat_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");

Here you also seem to have removed me.

> +MODULE_AUTHOR("Yanhong Wang <yanhong.wang@starfivetech.com>");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 1/7] dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version.

[yanhong wang]

On 2022/12/7 21:56, Emil Renner Berthing wrote:
> On Fri, 2 Dec 2022 at 09:04, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 02/12/2022 03:53, yanhong wang wrote:
>> >
>> >
>> > On 2022/12/2 0:18, Krzysztof Kozlowski wrote:
>> >> On 01/12/2022 10:02, Yanhong Wang wrote:
>> >>> Add dwmac-5.20 version to snps.dwmac.yaml
>> >>
>> >> Drop full stop from subject and add it here instead.
>> >>
>> >
>> > Will update in the next version.
>> >
>> >>>
>> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> >>
>> >> Two people contributed this one single line?
>> >>
>> >
>> > Emil made this patch and I submitted it.
>>
>> If Emil made this patch, then your From field is incorrect.
> 
> Yes, please don't change the author of the commits you cherry-picked
> from my tree.
> 
> But now I'm curious. Did you check with your colleagues that the dwmac
> IP on the SoC is in fact version 5.20?

I can confirm that the IP version is 5.20 on JH7110 SoC.

> This was just an educated guess from my side.
> 
> /Emil
> 
>> Best regards,
>> Krzysztof
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 2/7] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[yanhong wang]

On 2022/12/7 21:58, Emil Renner Berthing wrote:
> On Thu, 1 Dec 2022 at 10:05, Yanhong Wang <yanhong.wang@starfivetech.com> wrote:
>>
>> Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
>> to define some platform data in the glue layer.
>>
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> 
> Hi Yanhong.
> 
> Thanks for submitting this.
> But just as a reminder. Please don't change the author of the commits
> you cherry-picked from my tree.
> 

I will recover the author is you in the next version.

> /Emil
> 
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 50f6b4a14be4..cc3b701af802 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>         if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
>>             of_device_is_compatible(np, "snps,dwmac-4.10a") ||
>>             of_device_is_compatible(np, "snps,dwmac-4.20a") ||
>> -           of_device_is_compatible(np, "snps,dwmac-5.10a")) {
>> +           of_device_is_compatible(np, "snps,dwmac-5.10a") ||
>> +           of_device_is_compatible(np, "snps,dwmac-5.20")) {
>>                 plat->has_gmac4 = 1;
>>                 plat->has_gmac = 0;
>>                 plat->pmt = 1;
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 5/7] net: stmmac: Add StarFive dwmac supoort

[yanhong wang]

On 2022/12/7 22:16, Emil Renner Berthing wrote:
> Hi Yanhong,
> 
> Thanks for submitting this. Again please don't change the author of
> the commits you cherry-picked from my tree. This is why we have the
> Co-developed-by: tag so you can mark that you added or in this case
> removed code.

This commit is not cherry-picked from your tree.

> I'd hoped you would still include the support for the JH7100 though.
> 

If necessary, would include the support for the JH7100 in the next version.

> Also you seem to have changed the name of some of the functions.
> Please at least keep the prefix consistent if you do that. Now it's a
> mix of dwmac_starfive_, starfive_eth_ and starfive_eth_plat.
> 
> On Thu, 1 Dec 2022 at 10:07, Yanhong Wang <yanhong.wang@starfivetech.com> wrote:
>>
>> This adds StarFive dwmac driver support on the StarFive JH7110 SoCs.
>>
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  MAINTAINERS                                   |   1 +
>>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
>>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>>  .../stmicro/stmmac/dwmac-starfive-plat.c      | 147 ++++++++++++++++++
>>  4 files changed, 160 insertions(+)
>>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7eaaec8d3b96..36cb00cf860b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19610,6 +19610,7 @@ STARFIVE DWMAC GLUE LAYER
>>  M:     Yanhong Wang <yanhong.wang@starfivetech.com>
>>  S:     Maintained
>>  F:     Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>> +F:     drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>>
>>  STARFIVE PINCTRL DRIVER
>>  M:     Emil Renner Berthing <kernel@esmil.dk>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index 31ff35174034..1e29cd3770b9 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -235,6 +235,17 @@ config DWMAC_INTEL_PLAT
>>           the stmmac device driver. This driver is used for the Intel Keem Bay
>>           SoC.
>>
>> +config DWMAC_STARFIVE_PLAT
> 
> Why did you add the _PLAT suffix? None of the other SoC wrappers have this..

The _PLAT suffix definition reference the other SoC,such as DWMAC_INTEL_PLAT.

> 
>> +       tristate "StarFive dwmac support"
>> +       depends on OF && COMMON_CLK
>> +       depends on STMMAC_ETH
>> +       default SOC_STARFIVE
>> +       help
>> +         Support for ethernet controllers on StarFive RISC-V SoCs
>> +
>> +         This selects the StarFive platform specific glue layer support for
>> +         the stmmac device driver. This driver is used for StarFive RISC-V SoCs.
>> +
>>  config DWMAC_VISCONTI
>>         tristate "Toshiba Visconti DWMAC support"
>>         default ARCH_VISCONTI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index d4e12e9ace4f..a63ab0ab5071 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)       += dwmac-dwc-qos-eth.o
>>  obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
>>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>>  obj-$(CONFIG_DWMAC_IMX8)       += dwmac-imx.o
>> +obj-$(CONFIG_DWMAC_STARFIVE_PLAT)      += dwmac-starfive-plat.o
>>  obj-$(CONFIG_DWMAC_VISCONTI)   += dwmac-visconti.o
>>  stmmac-platform-objs:= stmmac_platform.o
>>  dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>> new file mode 100644
>> index 000000000000..8fbf584d4e19
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>> @@ -0,0 +1,147 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * StarFive DWMAC platform driver
>> + *
>> + * Copyright(C) 2022 StarFive Technology Co., Ltd.
> 
> Hmm.. where did my copyright go?
> 
>> + *
>> + */
>> +
>> +#include <linux/of_device.h>
>> +#include "stmmac_platform.h"
>> +
>> +struct starfive_dwmac {
>> +       struct device *dev;
>> +       struct clk *clk_tx;
>> +       struct clk *clk_gtx;
>> +       struct clk *clk_gtxc;
>> +};
>> +
>> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
>> +{
>> +       struct starfive_dwmac *dwmac = priv;
>> +       unsigned long rate;
>> +       int err;
>> +
>> +       switch (speed) {
>> +       case SPEED_1000:
>> +               rate = 125000000;
>> +               break;
>> +       case SPEED_100:
>> +               rate = 25000000;
>> +               break;
>> +       case SPEED_10:
>> +               rate = 2500000;
>> +               break;
>> +       default:
>> +               dev_err(dwmac->dev, "invalid speed %u\n", speed);
>> +               return;
>> +       }
>> +
>> +       err = clk_set_rate(dwmac->clk_gtx, rate);
>> +       if (err)
>> +               dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>> +}
>> +
>> +static void dwmac_starfive_clk_disable(void *clk)
>> +{
>> +       clk_disable_unprepare(clk);
>> +}
>> +
>> +static int starfive_eth_plat_probe(struct platform_device *pdev)
>> +{
>> +       struct plat_stmmacenet_data *plat_dat;
>> +       struct stmmac_resources stmmac_res;
>> +       struct starfive_dwmac *dwmac;
>> +       int err;
>> +
>> +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +       if (err)
>> +               return err;
>> +
>> +       plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
>> +       if (IS_ERR(plat_dat)) {
>> +               dev_err(&pdev->dev, "dt configuration failed\n");
>> +               return PTR_ERR(plat_dat);
>> +       }
>> +
>> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> +       if (!dwmac)
>> +               return -ENOMEM;
>> +
>> +       dwmac->clk_tx = devm_clk_get(&pdev->dev, "tx");
>> +       if (IS_ERR(dwmac->clk_tx))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
>> +                                               "error getting tx clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_tx);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_tx);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling tx clock\n");
>> +
>> +       dwmac->clk_gtx = devm_clk_get(&pdev->dev, "gtx");
>> +       if (IS_ERR(dwmac->clk_gtx))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
>> +                                               "error getting gtx clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_gtx);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_gtx);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling gtx clock\n");
> 
> I think the 3 calls above can be simplified to devm_clk_get_enabled().

I'll simplify it to devm_ clk_ get_ enabled() in the next version.

> 
>> +       dwmac->clk_gtxc = devm_clk_get(&pdev->dev, "gtxc");
>> +       if (IS_ERR(dwmac->clk_gtxc))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtxc),
>> +                                               "error getting gtxc clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_gtxc);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_gtxc);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling gtxc clock\n");
> 
> Same here.
> 
>> +
>> +       dwmac->dev = &pdev->dev;
>> +       plat_dat->fix_mac_speed = starfive_eth_fix_mac_speed;
>> +       plat_dat->init = NULL;
>> +       plat_dat->bsp_priv = dwmac;
>> +       plat_dat->dma_cfg->dche = true;
>> +
>> +       err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +       if (err) {
>> +               stmmac_remove_config_dt(pdev, plat_dat);
>> +               return err;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id starfive_eth_plat_match[] = {
>> +       {.compatible = "starfive,dwmac"},
>> +       { }
>> +};
>> +
>> +static struct platform_driver starfive_eth_plat_driver = {
>> +       .probe  = starfive_eth_plat_probe,
>> +       .remove = stmmac_pltfr_remove,
>> +       .driver = {
>> +               .name = "starfive-eth-plat",
>> +               .pm = &stmmac_pltfr_pm_ops,
>> +               .of_match_table = starfive_eth_plat_match,
>> +       },
>> +};
>> +
>> +module_platform_driver(starfive_eth_plat_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
> 
> Here you also seem to have removed me.
> 
>> +MODULE_AUTHOR("Yanhong Wang <yanhong.wang@starfivetech.com>");
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[yanhong wang]

On 2022/12/2 1:49, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote:
>> Add phy delay_chain configuration to support motorcomm phy driver for
>> StarFive VisionFive 2 board.
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-v2.dts         | 46 +++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> index c8946cf3a268..2868ef4c74ef 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> @@ -15,6 +15,8 @@
>>  
>>  	aliases {
>>  		serial0 = &uart0;
>> +		ethernet0=&gmac0;
>> +		ethernet1=&gmac1;
> 
> Please match the whitespace usage of the existing entry.
> 

Will fix in the next version.

>>  	};
>>  
>>  	chosen {
>> @@ -114,3 +116,47 @@
>>  	pinctrl-0 = <&uart0_pins>;
>>  	status = "okay";
>>  };
>> +
>> +&gmac0 {
>> +	status = "okay";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	phy-handle = <&phy0>;
>> +	status = "okay";
>> +	mdio0 {
> 
> A line of whitespace before the child nodes too please :)
> 

Will fix.

>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		compatible = "snps,dwmac-mdio";
>> +		phy0: ethernet-phy@0 {
>> +			reg = <0>;
>> +			rxc_dly_en = <1>;
>> +			tx_delay_sel_fe = <5>;
>> +			tx_delay_sel = <0xa>;
>> +			tx_inverted_10 = <0x1>;
>> +			tx_inverted_100 = <0x1>;
>> +			tx_inverted_1000 = <0x1>;
>> +		};
>> +	};
>> +};
>> +
>> +&gmac1 {
>> +	status = "okay";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	phy-handle = <&phy1>;
>> +	status = "okay";
>> +	mdio1 {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		compatible = "snps,dwmac-mdio";
>> +		phy1: ethernet-phy@1 {
>> +			reg = <1>;
>> +			tx_delay_sel_fe = <5>;
>> +			tx_delay_sel = <0>;
>> +			rxc_dly_en = <0>;
>> +			tx_inverted_10 = <0x1>;
>> +			tx_inverted_100 = <0x1>;
>> +			tx_inverted_1000 = <0x0>;
>> +		};
>> +	};
>> +};
>> -- 
>> 2.17.1
>> 

Re: [PATCH v1 7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

[yanhong wang]

On 2022/12/2 3:30, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:49:08PM +0000, Conor Dooley wrote:
>> On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote:
>> > riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration
>> > 
>> > Add phy delay_chain configuration to support motorcomm phy driver for
>> > StarFive VisionFive 2 board.
> 
> nit: please re-word this commit next time around to actually say what
> you're doing here. I didn't notice it initially, but this patch is doing
> a lot more than adding `delay_chain` configuration. To my dwmac unaware
> brain, there's nothing hits for that term outside of the changelog :(
> 

I will re-word the commit message and add another dt-binding to describe the details that such as "rxc_dly_en","tx_inverted_10" etc.

> Thanks,
> Conor.
> 
>> > 
>> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> > ---
>> >  .../jh7110-starfive-visionfive-v2.dts         | 46 +++++++++++++++++++
>> >  1 file changed, 46 insertions(+)
>> > 
>> > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> > index c8946cf3a268..2868ef4c74ef 100644
>> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> > @@ -15,6 +15,8 @@
>> >  
>> >  	aliases {
>> >  		serial0 = &uart0;
>> > +		ethernet0=&gmac0;
>> > +		ethernet1=&gmac1;
>> 
>> Please match the whitespace usage of the existing entry.
>> 
>> >  	};
>> >  
>> >  	chosen {
>> > @@ -114,3 +116,47 @@
>> >  	pinctrl-0 = <&uart0_pins>;
>> >  	status = "okay";
>> >  };
>> > +
>> > +&gmac0 {
>> > +	status = "okay";
>> > +	#address-cells = <1>;
>> > +	#size-cells = <0>;
>> > +	phy-handle = <&phy0>;
>> > +	status = "okay";
>> > +	mdio0 {
>> 
>> A line of whitespace before the child nodes too please :)
>> 
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +		compatible = "snps,dwmac-mdio";
>> > +		phy0: ethernet-phy@0 {
>> > +			reg = <0>;
>> > +			rxc_dly_en = <1>;
>> > +			tx_delay_sel_fe = <5>;
>> > +			tx_delay_sel = <0xa>;
>> > +			tx_inverted_10 = <0x1>;
>> > +			tx_inverted_100 = <0x1>;
>> > +			tx_inverted_1000 = <0x1>;
>> > +		};
>> > +	};
>> > +};
>> > +
>> > +&gmac1 {
>> > +	status = "okay";
>> > +	#address-cells = <1>;
>> > +	#size-cells = <0>;
>> > +	phy-handle = <&phy1>;
>> > +	status = "okay";
>> > +	mdio1 {
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +		compatible = "snps,dwmac-mdio";
>> > +		phy1: ethernet-phy@1 {
>> > +			reg = <1>;
>> > +			tx_delay_sel_fe = <5>;
>> > +			tx_delay_sel = <0>;
>> > +			rxc_dly_en = <0>;
>> > +			tx_inverted_10 = <0x1>;
>> > +			tx_inverted_100 = <0x1>;
>> > +			tx_inverted_1000 = <0x0>;
>> > +		};
>> > +	};
>> > +};
>> > -- 
>> > 2.17.1
>> > 

Re: [PATCH v1 2/7] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[Ben Dooks]

On 2022-12-01 09:02, Yanhong Wang wrote:
> Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
> to define some platform data in the glue layer.
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 50f6b4a14be4..cc3b701af802 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device
> *pdev, u8 *mac)
>  	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
>  	    of_device_is_compatible(np, "snps,dwmac-4.10a") ||
>  	    of_device_is_compatible(np, "snps,dwmac-4.20a") ||
> -	    of_device_is_compatible(np, "snps,dwmac-5.10a")) {
> +	    of_device_is_compatible(np, "snps,dwmac-5.10a") ||
> +	    of_device_is_compatible(np, "snps,dwmac-5.20")) {
>  		plat->has_gmac4 = 1;
>  		plat->has_gmac = 0;
>  		plat->pmt = 1;

out of interest, is the version of the ip autodetectable yet?
also, we would be better off if having an if (version > 4) check if we 
use the standard snps ip block code headers

Re: [PATCH v1 5/7] net: stmmac: Add StarFive dwmac supoort

[yanhong wang]

On 2022/12/7 22:16, Emil Renner Berthing wrote:
> Hi Yanhong,
> 
> Thanks for submitting this. Again please don't change the author of
> the commits you cherry-picked from my tree. This is why we have the
> Co-developed-by: tag so you can mark that you added or in this case
> removed code.
> I'd hoped you would still include the support for the JH7100 though.
>

Hi Emil,

  Thank you for your suggestion. If the dwmac-starfive-plat driver supports JH7100 and JH7110, the dt-binding calls "starfive,jh71x0-dwmac.yaml", is this ok?

 
> Also you seem to have changed the name of some of the functions.
> Please at least keep the prefix consistent if you do that. Now it's a
> mix of dwmac_starfive_, starfive_eth_ and starfive_eth_plat.
> 

I will named all the functions with starfive_eth_plat prefix in the next version.

> On Thu, 1 Dec 2022 at 10:07, Yanhong Wang <yanhong.wang@starfivetech.com> wrote:
>>
>> This adds StarFive dwmac driver support on the StarFive JH7110 SoCs.
>>
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  MAINTAINERS                                   |   1 +
>>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
>>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>>  .../stmicro/stmmac/dwmac-starfive-plat.c      | 147 ++++++++++++++++++
>>  4 files changed, 160 insertions(+)
>>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7eaaec8d3b96..36cb00cf860b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19610,6 +19610,7 @@ STARFIVE DWMAC GLUE LAYER
>>  M:     Yanhong Wang <yanhong.wang@starfivetech.com>
>>  S:     Maintained
>>  F:     Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>> +F:     drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>>
>>  STARFIVE PINCTRL DRIVER
>>  M:     Emil Renner Berthing <kernel@esmil.dk>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index 31ff35174034..1e29cd3770b9 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -235,6 +235,17 @@ config DWMAC_INTEL_PLAT
>>           the stmmac device driver. This driver is used for the Intel Keem Bay
>>           SoC.
>>
>> +config DWMAC_STARFIVE_PLAT
> 
> Why did you add the _PLAT suffix? None of the other SoC wrappers have this..
> 
>> +       tristate "StarFive dwmac support"
>> +       depends on OF && COMMON_CLK
>> +       depends on STMMAC_ETH
>> +       default SOC_STARFIVE
>> +       help
>> +         Support for ethernet controllers on StarFive RISC-V SoCs
>> +
>> +         This selects the StarFive platform specific glue layer support for
>> +         the stmmac device driver. This driver is used for StarFive RISC-V SoCs.
>> +
>>  config DWMAC_VISCONTI
>>         tristate "Toshiba Visconti DWMAC support"
>>         default ARCH_VISCONTI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index d4e12e9ace4f..a63ab0ab5071 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)       += dwmac-dwc-qos-eth.o
>>  obj-$(CONFIG_DWMAC_INTEL_PLAT) += dwmac-intel-plat.o
>>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>>  obj-$(CONFIG_DWMAC_IMX8)       += dwmac-imx.o
>> +obj-$(CONFIG_DWMAC_STARFIVE_PLAT)      += dwmac-starfive-plat.o
>>  obj-$(CONFIG_DWMAC_VISCONTI)   += dwmac-visconti.o
>>  stmmac-platform-objs:= stmmac_platform.o
>>  dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>> new file mode 100644
>> index 000000000000..8fbf584d4e19
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
>> @@ -0,0 +1,147 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * StarFive DWMAC platform driver
>> + *
>> + * Copyright(C) 2022 StarFive Technology Co., Ltd.
> 
> Hmm.. where did my copyright go?
> 
>> + *
>> + */
>> +
>> +#include <linux/of_device.h>
>> +#include "stmmac_platform.h"
>> +
>> +struct starfive_dwmac {
>> +       struct device *dev;
>> +       struct clk *clk_tx;
>> +       struct clk *clk_gtx;
>> +       struct clk *clk_gtxc;
>> +};
>> +
>> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
>> +{
>> +       struct starfive_dwmac *dwmac = priv;
>> +       unsigned long rate;
>> +       int err;
>> +
>> +       switch (speed) {
>> +       case SPEED_1000:
>> +               rate = 125000000;
>> +               break;
>> +       case SPEED_100:
>> +               rate = 25000000;
>> +               break;
>> +       case SPEED_10:
>> +               rate = 2500000;
>> +               break;
>> +       default:
>> +               dev_err(dwmac->dev, "invalid speed %u\n", speed);
>> +               return;
>> +       }
>> +
>> +       err = clk_set_rate(dwmac->clk_gtx, rate);
>> +       if (err)
>> +               dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>> +}
>> +
>> +static void dwmac_starfive_clk_disable(void *clk)
>> +{
>> +       clk_disable_unprepare(clk);
>> +}
>> +
>> +static int starfive_eth_plat_probe(struct platform_device *pdev)
>> +{
>> +       struct plat_stmmacenet_data *plat_dat;
>> +       struct stmmac_resources stmmac_res;
>> +       struct starfive_dwmac *dwmac;
>> +       int err;
>> +
>> +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +       if (err)
>> +               return err;
>> +
>> +       plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
>> +       if (IS_ERR(plat_dat)) {
>> +               dev_err(&pdev->dev, "dt configuration failed\n");
>> +               return PTR_ERR(plat_dat);
>> +       }
>> +
>> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> +       if (!dwmac)
>> +               return -ENOMEM;
>> +
>> +       dwmac->clk_tx = devm_clk_get(&pdev->dev, "tx");
>> +       if (IS_ERR(dwmac->clk_tx))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
>> +                                               "error getting tx clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_tx);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_tx);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling tx clock\n");
>> +
>> +       dwmac->clk_gtx = devm_clk_get(&pdev->dev, "gtx");
>> +       if (IS_ERR(dwmac->clk_gtx))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
>> +                                               "error getting gtx clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_gtx);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_gtx);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling gtx clock\n");
> 
> I think the 3 calls above can be simplified to devm_clk_get_enabled().
> 
>> +       dwmac->clk_gtxc = devm_clk_get(&pdev->dev, "gtxc");
>> +       if (IS_ERR(dwmac->clk_gtxc))
>> +               return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtxc),
>> +                                               "error getting gtxc clock\n");
>> +
>> +       err = devm_add_action(&pdev->dev, dwmac_starfive_clk_disable,
>> +                             dwmac->clk_gtxc);
>> +       if (err)
>> +               return err;
>> +
>> +       err = clk_prepare_enable(dwmac->clk_gtxc);
>> +       if (err)
>> +               return dev_err_probe(&pdev->dev, err, "error enabling gtxc clock\n");
> 
> Same here.
> 
>> +
>> +       dwmac->dev = &pdev->dev;
>> +       plat_dat->fix_mac_speed = starfive_eth_fix_mac_speed;
>> +       plat_dat->init = NULL;
>> +       plat_dat->bsp_priv = dwmac;
>> +       plat_dat->dma_cfg->dche = true;
>> +
>> +       err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +       if (err) {
>> +               stmmac_remove_config_dt(pdev, plat_dat);
>> +               return err;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id starfive_eth_plat_match[] = {
>> +       {.compatible = "starfive,dwmac"},
>> +       { }
>> +};
>> +
>> +static struct platform_driver starfive_eth_plat_driver = {
>> +       .probe  = starfive_eth_plat_probe,
>> +       .remove = stmmac_pltfr_remove,
>> +       .driver = {
>> +               .name = "starfive-eth-plat",
>> +               .pm = &stmmac_pltfr_pm_ops,
>> +               .of_match_table = starfive_eth_plat_match,
>> +       },
>> +};
>> +
>> +module_platform_driver(starfive_eth_plat_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
> 
> Here you also seem to have removed me.
> 
>> +MODULE_AUTHOR("Yanhong Wang <yanhong.wang@starfivetech.com>");
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v1 2/7] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[yanhong wang]

On 2022/12/9 9:59, Ben Dooks wrote:
> 
> 
> On 2022-12-01 09:02, Yanhong Wang wrote:
>> Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
>> to define some platform data in the glue layer.
>>
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 50f6b4a14be4..cc3b701af802 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device
>> *pdev, u8 *mac)
>>      if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
>>          of_device_is_compatible(np, "snps,dwmac-4.10a") ||
>>          of_device_is_compatible(np, "snps,dwmac-4.20a") ||
>> -        of_device_is_compatible(np, "snps,dwmac-5.10a")) {
>> +        of_device_is_compatible(np, "snps,dwmac-5.10a") ||
>> +        of_device_is_compatible(np, "snps,dwmac-5.20")) {
>>          plat->has_gmac4 = 1;
>>          plat->has_gmac = 0;
>>          plat->pmt = 1;
> 
> out of interest, is the version of the ip autodetectable yet?
> also, we would be better off if having an if (version > 4) check if we use the standard snps ip block code headers
> 

Yes, the version of the ip is autodetectable. It is also possible to use the standard "snps, dwmac-5.10a" definition on JH7110（has been tested on the VisionFive v2 boards and works normally）, do you have any better suggestions?