[PATCH 0/4] riscv: sophgo: Add ethernet support for SG2044

[PATCH 0/4] riscv: sophgo: Add ethernet support for SG2044

[Inochi Amaoto]

The ethernet controller of SG2044 is Synopsys DesignWare IP with
custom clock. Add glue layer for it.

Inochi Amaoto (4):
  dt-bindings: net: snps,dwmac: Add dwmac-5.30a version
  dt-bindings: net: Add support for Sophgo SG2044 dwmac
  net: stmmac: platform: Add snps,dwmac-5.30a IP compatible string
  net: stmmac: Add glue layer for Sophgo SG2044 SoC

 .../devicetree/bindings/net/snps,dwmac.yaml   |   4 +
 .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-sophgo.c    | 132 ++++++++++++++++
 .../ethernet/stmicro/stmmac/stmmac_platform.c |   3 +-
 6 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c

--
2.47.0

[PATCH 1/4] dt-bindings: net: snps,dwmac: Add dwmac-5.30a version

[Inochi Amaoto]

Add compatible string for dwmac-5.30a IP.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 4e2ba1bf788c..3c4007cb65f8 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -31,6 +31,7 @@ select:
           - snps,dwmac-4.20a
           - snps,dwmac-5.10a
           - snps,dwmac-5.20
+          - snps,dwmac-5.30a
           - snps,dwxgmac
           - snps,dwxgmac-2.10
 
@@ -95,6 +96,7 @@ properties:
         - snps,dwmac-4.20a
         - snps,dwmac-5.10a
         - snps,dwmac-5.20
+        - snps,dwmac-5.30a
         - snps,dwxgmac
         - snps,dwxgmac-2.10
         - starfive,jh7100-dwmac
@@ -627,6 +629,7 @@ allOf:
                 - snps,dwmac-4.20a
                 - snps,dwmac-5.10a
                 - snps,dwmac-5.20
+                - snps,dwmac-5.30a
                 - snps,dwxgmac
                 - snps,dwxgmac-2.10
                 - st,spear600-gmac
-- 
2.47.0

[PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Inochi Amaoto]

The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
with some extra clock.

Add necessary compatible string for this device.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
 .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 3c4007cb65f8..69f6bb36970b 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -99,6 +99,7 @@ properties:
         - snps,dwmac-5.30a
         - snps,dwxgmac
         - snps,dwxgmac-2.10
+        - sophgo,sg2044-dwmac
         - starfive,jh7100-dwmac
         - starfive,jh7110-dwmac
 
diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
new file mode 100644
index 000000000000..93c41550b0b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
@@ -0,0 +1,145 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 DWMAC glue layer
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - sophgo,sg2044-dwmac
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: sophgo,sg2044-dwmac
+      - const: snps,dwmac-5.30a
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: GMAC main clock
+      - description: PTP clock
+      - description: TX clock
+
+  clock-names:
+    items:
+      - const: stmmaceth
+      - const: ptp_ref
+      - const: tx
+
+  sophgo,syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to syscon that configures phy
+          - description: offset of phy mode register
+          - description: length of the phy mode register
+    description:
+      A phandle to syscon with two arguments that configure phy mode.
+      The argument one is the offset of phy mode register, the
+      argument two is the length of phy mode register.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: sophgo,sg2044-dwmac
+    then:
+      properties:
+        interrupts:
+          minItems: 1
+          maxItems: 1
+
+        interrupt-names:
+          minItems: 1
+          maxItems: 1
+
+        resets:
+          maxItems: 1
+
+        reset-names:
+          const: stmmaceth
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ethernet@30006000 {
+      compatible = "sophgo,sg2044-dwmac", "snps,dwmac-5.30a";
+      reg = <0x30006000 0x4000>;
+      clocks = <&clk 151>, <&clk 152>, <&clk 154>;
+      clock-names = "stmmaceth", "ptp_ref", "tx";
+      interrupt-parent = <&intc>;
+      interrupts = <296 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-names = "macirq";
+      resets = <&rst 30>;
+      reset-names = "stmmaceth";
+      snps,multicast-filter-bins = <0>;
+      snps,perfect-filter-entries = <1>;
+      snps,aal;
+      snps,tso;
+      snps,txpbl = <32>;
+      snps,rxpbl = <32>;
+      snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
+      snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
+      snps,axi-config = <&gmac0_stmmac_axi_setup>;
+      status = "disabled";
+
+      gmac0_mtl_rx_setup: rx-queues-config {
+        snps,rx-queues-to-use = <8>;
+        snps,rx-sched-wsp;
+        queue0 {};
+        queue1 {};
+        queue2 {};
+        queue3 {};
+        queue4 {};
+        queue5 {};
+        queue6 {};
+        queue7 {};
+      };
+
+      gmac0_mtl_tx_setup: tx-queues-config {
+        snps,tx-queues-to-use = <8>;
+        queue0 {};
+        queue1 {};
+        queue2 {};
+        queue3 {};
+        queue4 {};
+        queue5 {};
+        queue6 {};
+        queue7 {};
+      };
+
+      gmac0_stmmac_axi_setup: stmmac-axi-config {
+        snps,blen = <16 8 4 0 0 0 0>;
+        snps,wr_osr_lmt = <1>;
+        snps,rd_osr_lmt = <2>;
+      };
+    };
-- 
2.47.0

[PATCH 3/4] net: stmmac: platform: Add snps,dwmac-5.30a IP compatible string

[Inochi Amaoto]

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

Signed-off-by: Inochi Amaoto <inochiama@gmail.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 ad868e8d195d..3c4e78b10dd6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -555,7 +555,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 	    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.20")) {
+	    of_device_is_compatible(np, "snps,dwmac-5.20") ||
+	    of_device_is_compatible(np, "snps,dwmac-5.30a")) {
 		plat->has_gmac4 = 1;
 		plat->has_gmac = 0;
 		plat->pmt = 1;
-- 
2.47.0

[PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

Adds Sophgo dwmac driver support on the Sophgo SG2044 SoC.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-sophgo.c    | 132 ++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 05cc07b8f48c..bc44b21c593f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -169,6 +169,17 @@ config DWMAC_SOCFPGA
 	  for the stmmac device driver. This driver is used for
 	  arria5 and cyclone5 FPGA SoCs.
 
+config DWMAC_SOPHGO
+	tristate "Sophgo dwmac support"
+	depends on OF && (ARCH_SOPHGO || COMPILE_TEST)
+	default m if ARCH_SOPHGO
+	help
+	  Support for ethernet controllers on Sophgo RISC-V SoCs
+
+	  This selects the Sophgo SoC specific glue layer support
+	  for the stmmac device driver. This driver is used for the
+	  ethernet controllers on various Sophgo SoCs.
+
 config DWMAC_STARFIVE
 	tristate "StarFive dwmac support"
 	depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index c2f0e91f6bf8..e1287b53047b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_QCOM_ETHQOS)	+= dwmac-qcom-ethqos.o
 obj-$(CONFIG_DWMAC_ROCKCHIP)	+= dwmac-rk.o
 obj-$(CONFIG_DWMAC_RZN1)	+= dwmac-rzn1.o
 obj-$(CONFIG_DWMAC_SOCFPGA)	+= dwmac-altr-socfpga.o
+obj-$(CONFIG_DWMAC_SOPHGO)	+= dwmac-sophgo.o
 obj-$(CONFIG_DWMAC_STARFIVE)	+= dwmac-starfive.o
 obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
 obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c
new file mode 100644
index 000000000000..83c67c061182
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Sophgo DWMAC platform driver
+ *
+ * Copyright (C) 2024 Inochi Amaoto <inochiama@gmail.com>
+ *
+ */
+
+#include <linux/bits.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "stmmac_platform.h"
+
+struct sophgo_dwmac {
+	struct device *dev;
+	struct clk *clk_tx;
+};
+
+#define DWMAC_SG2044_FLAG_USE_RX_DELAY		BIT(16)
+
+static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
+{
+	struct sophgo_dwmac *dwmac = priv;
+	unsigned long rate;
+	int ret;
+
+	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);
+		break;
+	}
+
+	ret = clk_set_rate(dwmac->clk_tx, rate);
+	if (ret)
+		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
+}
+
+static int sophgo_sg2044_dwmac_init(struct platform_device *pdev,
+				    struct plat_stmmacenet_data *plat_dat,
+				    struct stmmac_resources *stmmac_res)
+{
+	struct sophgo_dwmac *dwmac;
+	struct regmap *regmap;
+	unsigned int args[2];
+	int ret;
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
+	if (IS_ERR(dwmac->clk_tx))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
+				     "failed to get tx clock\n");
+
+	regmap = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
+						      "sophgo,syscon",
+						      2, args);
+	if (IS_ERR(regmap))
+		return dev_err_probe(&pdev->dev, PTR_ERR(regmap),
+				     "failed to get the regmap\n");
+
+	ret = regmap_set_bits(regmap, args[0], DWMAC_SG2044_FLAG_USE_RX_DELAY);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to set the phy rx delay\n");
+
+	dwmac->dev = &pdev->dev;
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->flags |= STMMAC_FLAG_SPH_DISABLE;
+	plat_dat->fix_mac_speed = sophgo_dwmac_fix_mac_speed;
+	plat_dat->multicast_filter_bins = 0;
+	plat_dat->unicast_filter_entries = 1;
+
+	return 0;
+}
+
+static int sophgo_dwmac_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	int ret;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to get resources\n");
+
+	plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
+				     "dt configuration failed\n");
+
+	ret = sophgo_sg2044_dwmac_init(pdev, plat_dat, &stmmac_res);
+	if (ret)
+		return ret;
+
+	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+}
+
+static const struct of_device_id sophgo_dwmac_match[] = {
+	{ .compatible = "sophgo,sg2044-dwmac" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sophgo_dwmac_match);
+
+static struct platform_driver sophgo_dwmac_driver = {
+	.probe  = sophgo_dwmac_probe,
+	.remove_new = stmmac_pltfr_remove,
+	.driver = {
+		.name = "sophgo-dwmac",
+		.pm = &stmmac_pltfr_pm_ops,
+		.of_match_table = sophgo_dwmac_match,
+	},
+};
+module_platform_driver(sophgo_dwmac_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Sophgo DWMAC platform driver");
-- 
2.47.0

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

> +static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
> +{
> +	struct sophgo_dwmac *dwmac = priv;
> +	unsigned long rate;
> +	int ret;
> +
> +	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);
> +		break;
> +	}

There was a helper added recently for this, since it appears
repeatedly in drivers.

> +	ret = regmap_set_bits(regmap, args[0], DWMAC_SG2044_FLAG_USE_RX_DELAY);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to set the phy rx delay\n");

Please could you explain what this delay is for. Is it the 2ns RGMII
delay?

	Andrew

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Mon, Oct 21, 2024 at 02:22:47PM +0200, Andrew Lunn wrote:
> > +static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
> > +{
> > +	struct sophgo_dwmac *dwmac = priv;
> > +	unsigned long rate;
> > +	int ret;
> > +
> > +	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);
> > +		break;
> > +	}
> 
> There was a helper added recently for this, since it appears
> repeatedly in drivers.
> 

OK, I will change it.

> > +	ret = regmap_set_bits(regmap, args[0], DWMAC_SG2044_FLAG_USE_RX_DELAY);
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret,
> > +				     "failed to set the phy rx delay\n");
> 
> Please could you explain what this delay is for. Is it the 2ns RGMII
> delay?
> 
> 	Andrew

It is related to the RGMII delay. On sg2044, when the phy 
sets rx-delay, the interal mac is not set the same delay, 
so this is needed to be set.

Regards,
Inochi

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

> It is related to the RGMII delay. On sg2044, when the phy 
> sets rx-delay, the interal mac is not set the same delay, 
> so this is needed to be set.

This is the wrong way to do it. Please look at how phy-mode should be
used, the four different "rgmii" values. Nearly everybody gets this
wrong, so there are plenty of emails from me in the netdev list about
how it should be done.

    Andrew

---
pw-bot: cr

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Chen Wang]

On 2024/10/21 18:36, Inochi Amaoto wrote:
[......]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c
> new file mode 100644
> index 000000000000..83c67c061182
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c
> @@ -0,0 +1,132 @@
[......]
> +module_platform_driver(sophgo_dwmac_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Sophgo DWMAC platform driver");

Missing MODULE_AUTHOR ......

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Mon, Oct 21, 2024 at 03:27:18PM +0200, Andrew Lunn wrote:
> > It is related to the RGMII delay. On sg2044, when the phy 
> > sets rx-delay, the interal mac is not set the same delay, 
> > so this is needed to be set.
> 
> This is the wrong way to do it. Please look at how phy-mode should be
> used, the four different "rgmii" values. Nearly everybody gets this
> wrong, so there are plenty of emails from me in the netdev list about
> how it should be done.
> 

The phy-mode is alreay set to the "rgmii-id" and a rx delay is already
set (a default tx delay is set by the phy driver). In the scenario 
the extra bit is used to fix 2ns difference between the sampling clock
and data. It is more like an extra setting and the kernel can not handle
it by only setting the phy-mode.

This is draft dts patch for the sg2044 gmac.
https://github.com/project-inochi/linux/commit/381cb6000044a89cb13d6d9c839e9bbc7b9d2e5a

Regards,
Inochi

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[kernel test robot]

Hi Inochi,

kernel test robot noticed the following build warnings:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Inochi-Amaoto/dt-bindings-net-snps-dwmac-Add-dwmac-5-30a-version/20241021-184301
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241021103617.653386-5-inochiama%40gmail.com
patch subject: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241022/202410221853.0nt4WyvW-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241022/202410221853.0nt4WyvW-lkp@intel.com/reproduce)

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

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c:41:2: warning: variable 'rate' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
      41 |         default:
         |         ^~~~~~~
   drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c:46:36: note: uninitialized use occurs here
      46 |         ret = clk_set_rate(dwmac->clk_tx, rate);
         |                                           ^~~~
   drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c:28:20: note: initialize the variable 'rate' to silence this warning
      28 |         unsigned long rate;
         |                           ^
         |                            = 0
   1 warning generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]


vim +/rate +41 drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c

    24	
    25	static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
    26	{
    27		struct sophgo_dwmac *dwmac = priv;
    28		unsigned long rate;
    29		int ret;
    30	
    31		switch (speed) {
    32		case SPEED_1000:
    33			rate = 125000000;
    34			break;
    35		case SPEED_100:
    36			rate = 25000000;
    37			break;
    38		case SPEED_10:
    39			rate = 2500000;
    40			break;
  > 41		default:
    42			dev_err(dwmac->dev, "invalid speed %u\n", speed);
    43			break;
    44		}
    45	
    46		ret = clk_set_rate(dwmac->clk_tx, rate);
    47		if (ret)
    48			dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
    49	}
    50	

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

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

On Tue, Oct 22, 2024 at 06:21:49PM +0800, Inochi Amaoto wrote:
> On Mon, Oct 21, 2024 at 03:27:18PM +0200, Andrew Lunn wrote:
> > > It is related to the RGMII delay. On sg2044, when the phy 
> > > sets rx-delay, the interal mac is not set the same delay, 
> > > so this is needed to be set.
> > 
> > This is the wrong way to do it. Please look at how phy-mode should be
> > used, the four different "rgmii" values. Nearly everybody gets this
> > wrong, so there are plenty of emails from me in the netdev list about
> > how it should be done.
> > 
> 
> The phy-mode is alreay set to the "rgmii-id" and a rx delay is already
> set (a default tx delay is set by the phy driver). In the scenario 
> the extra bit is used to fix 2ns difference between the sampling clock
> and data. It is more like an extra setting and the kernel can not handle
> it by only setting the phy-mode.

This sounds wrong.

So in DT you have rgmii-id? You say the PHY is doing TX delay. So you
pass PHY_INTERFACE_MODE_RGMII_TXID to the PHY? It is not clear from
this patch, i don't see any code mentioning
PHY_INTERFACE_MODE_RGMII_TXID. Could you point me at that code.

	Andrew

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Conor Dooley]

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

On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> with some extra clock.
> 
> Add necessary compatible string for this device.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
>  2 files changed, 146 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 3c4007cb65f8..69f6bb36970b 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -99,6 +99,7 @@ properties:
>          - snps,dwmac-5.30a
>          - snps,dwxgmac
>          - snps,dwxgmac-2.10
> +        - sophgo,sg2044-dwmac
>          - starfive,jh7100-dwmac
>          - starfive,jh7110-dwmac
>  
> diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> new file mode 100644
> index 000000000000..93c41550b0b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 DWMAC glue layer
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - sophgo,sg2044-dwmac
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: sophgo,sg2044-dwmac
> +      - const: snps,dwmac-5.30a
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: GMAC main clock
> +      - description: PTP clock
> +      - description: TX clock
> +
> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: ptp_ref
> +      - const: tx
> +
> +  sophgo,syscon:

How many dwmac instances does the sg2044 have?

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that configures phy
> +          - description: offset of phy mode register
> +          - description: length of the phy mode register
> +    description:
> +      A phandle to syscon with two arguments that configure phy mode.
> +      The argument one is the offset of phy mode register, the
> +      argument two is the length of phy mode register.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - resets
> +  - reset-names
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: sophgo,sg2044-dwmac

Why does this have to be applied conditionally? There's only one
compatible in the binding, can't you apply these unconditionally?


Cheers,
Conor.

> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          maxItems: 1
> +
> +        interrupt-names:
> +          minItems: 1
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +
> +        reset-names:
> +          const: stmmaceth
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    ethernet@30006000 {
> +      compatible = "sophgo,sg2044-dwmac", "snps,dwmac-5.30a";
> +      reg = <0x30006000 0x4000>;
> +      clocks = <&clk 151>, <&clk 152>, <&clk 154>;
> +      clock-names = "stmmaceth", "ptp_ref", "tx";
> +      interrupt-parent = <&intc>;
> +      interrupts = <296 IRQ_TYPE_LEVEL_HIGH>;
> +      interrupt-names = "macirq";
> +      resets = <&rst 30>;
> +      reset-names = "stmmaceth";
> +      snps,multicast-filter-bins = <0>;
> +      snps,perfect-filter-entries = <1>;
> +      snps,aal;
> +      snps,tso;
> +      snps,txpbl = <32>;
> +      snps,rxpbl = <32>;
> +      snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
> +      snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
> +      snps,axi-config = <&gmac0_stmmac_axi_setup>;
> +      status = "disabled";
> +
> +      gmac0_mtl_rx_setup: rx-queues-config {
> +        snps,rx-queues-to-use = <8>;
> +        snps,rx-sched-wsp;
> +        queue0 {};
> +        queue1 {};
> +        queue2 {};
> +        queue3 {};
> +        queue4 {};
> +        queue5 {};
> +        queue6 {};
> +        queue7 {};
> +      };
> +
> +      gmac0_mtl_tx_setup: tx-queues-config {
> +        snps,tx-queues-to-use = <8>;
> +        queue0 {};
> +        queue1 {};
> +        queue2 {};
> +        queue3 {};
> +        queue4 {};
> +        queue5 {};
> +        queue6 {};
> +        queue7 {};
> +      };
> +
> +      gmac0_stmmac_axi_setup: stmmac-axi-config {
> +        snps,blen = <16 8 4 0 0 0 0>;
> +        snps,wr_osr_lmt = <1>;
> +        snps,rd_osr_lmt = <2>;
> +      };
> +    };
> -- 
> 2.47.0
> 

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

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Inochi Amaoto]

On Tue, Oct 22, 2024 at 06:28:06PM +0100, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > with some extra clock.
> > 
> > Add necessary compatible string for this device.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> >  2 files changed, 146 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 3c4007cb65f8..69f6bb36970b 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -99,6 +99,7 @@ properties:
> >          - snps,dwmac-5.30a
> >          - snps,dwxgmac
> >          - snps,dwxgmac-2.10
> > +        - sophgo,sg2044-dwmac
> >          - starfive,jh7100-dwmac
> >          - starfive,jh7110-dwmac
> >  
> > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > new file mode 100644
> > index 000000000000..93c41550b0b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > @@ -0,0 +1,145 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive JH7110 DWMAC glue layer
> > +
> > +maintainers:
> > +  - Inochi Amaoto <inochiama@gmail.com>
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - sophgo,sg2044-dwmac
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: sophgo,sg2044-dwmac
> > +      - const: snps,dwmac-5.30a
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: GMAC main clock
> > +      - description: PTP clock
> > +      - description: TX clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: stmmaceth
> > +      - const: ptp_ref
> > +      - const: tx
> > +
> > +  sophgo,syscon:
> 
> How many dwmac instances does the sg2044 have?
> 

Only one, there is another 100G dwxgmac instance, but it does not
use this syscon.

> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      - items:
> > +          - description: phandle to syscon that configures phy
> > +          - description: offset of phy mode register
> > +          - description: length of the phy mode register
> > +    description:
> > +      A phandle to syscon with two arguments that configure phy mode.
> > +      The argument one is the offset of phy mode register, the
> > +      argument two is the length of phy mode register.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - resets
> > +  - reset-names
> > +
> > +allOf:
> > +  - $ref: snps,dwmac.yaml#
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: sophgo,sg2044-dwmac
> 
> Why does this have to be applied conditionally? There's only one
> compatible in the binding, can't you apply these unconditionally?
> 
> 
> Cheers,
> Conor.
> 

I think it can apply it unconditionally. I will fix it.

Regards,
Inochi

> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 1
> > +          maxItems: 1
> > +
> > +        interrupt-names:
> > +          minItems: 1
> > +          maxItems: 1
> > +
> > +        resets:
> > +          maxItems: 1
> > +
> > +        reset-names:
> > +          const: stmmaceth
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    ethernet@30006000 {
> > +      compatible = "sophgo,sg2044-dwmac", "snps,dwmac-5.30a";
> > +      reg = <0x30006000 0x4000>;
> > +      clocks = <&clk 151>, <&clk 152>, <&clk 154>;
> > +      clock-names = "stmmaceth", "ptp_ref", "tx";
> > +      interrupt-parent = <&intc>;
> > +      interrupts = <296 IRQ_TYPE_LEVEL_HIGH>;
> > +      interrupt-names = "macirq";
> > +      resets = <&rst 30>;
> > +      reset-names = "stmmaceth";
> > +      snps,multicast-filter-bins = <0>;
> > +      snps,perfect-filter-entries = <1>;
> > +      snps,aal;
> > +      snps,tso;
> > +      snps,txpbl = <32>;
> > +      snps,rxpbl = <32>;
> > +      snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
> > +      snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
> > +      snps,axi-config = <&gmac0_stmmac_axi_setup>;
> > +      status = "disabled";
> > +
> > +      gmac0_mtl_rx_setup: rx-queues-config {
> > +        snps,rx-queues-to-use = <8>;
> > +        snps,rx-sched-wsp;
> > +        queue0 {};
> > +        queue1 {};
> > +        queue2 {};
> > +        queue3 {};
> > +        queue4 {};
> > +        queue5 {};
> > +        queue6 {};
> > +        queue7 {};
> > +      };
> > +
> > +      gmac0_mtl_tx_setup: tx-queues-config {
> > +        snps,tx-queues-to-use = <8>;
> > +        queue0 {};
> > +        queue1 {};
> > +        queue2 {};
> > +        queue3 {};
> > +        queue4 {};
> > +        queue5 {};
> > +        queue6 {};
> > +        queue7 {};
> > +      };
> > +
> > +      gmac0_stmmac_axi_setup: stmmac-axi-config {
> > +        snps,blen = <16 8 4 0 0 0 0>;
> > +        snps,wr_osr_lmt = <1>;
> > +        snps,rd_osr_lmt = <2>;
> > +      };
> > +    };
> > -- 
> > 2.47.0
> > 

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Tue, Oct 22, 2024 at 03:51:08PM +0200, Andrew Lunn wrote:
> On Tue, Oct 22, 2024 at 06:21:49PM +0800, Inochi Amaoto wrote:
> > On Mon, Oct 21, 2024 at 03:27:18PM +0200, Andrew Lunn wrote:
> > > > It is related to the RGMII delay. On sg2044, when the phy 
> > > > sets rx-delay, the interal mac is not set the same delay, 
> > > > so this is needed to be set.
> > > 
> > > This is the wrong way to do it. Please look at how phy-mode should be
> > > used, the four different "rgmii" values. Nearly everybody gets this
> > > wrong, so there are plenty of emails from me in the netdev list about
> > > how it should be done.
> > > 
> > 
> > The phy-mode is alreay set to the "rgmii-id" and a rx delay is already
> > set (a default tx delay is set by the phy driver). In the scenario 
> > the extra bit is used to fix 2ns difference between the sampling clock
> > and data. It is more like an extra setting and the kernel can not handle
> > it by only setting the phy-mode.
> 
> This sounds wrong.
> 
> So in DT you have rgmii-id? You say the PHY is doing TX delay. So you
> pass PHY_INTERFACE_MODE_RGMII_TXID to the PHY? It is not clear from
> this patch, i don't see any code mentioning
> PHY_INTERFACE_MODE_RGMII_TXID. Could you point me at that code.
> 
> 	Andrew

The phy on the board I have is YT8531, The config code is here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/motorcomm.c#n868

As the syscon only has a config on rx delay. I have
already fix the code and only set the bit when the
mac is rgmii-rxid/id.

Regards,
Inochi.

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

On Wed, Oct 23, 2024 at 08:41:36AM +0800, Inochi Amaoto wrote:
> On Tue, Oct 22, 2024 at 03:51:08PM +0200, Andrew Lunn wrote:
> > On Tue, Oct 22, 2024 at 06:21:49PM +0800, Inochi Amaoto wrote:
> > > On Mon, Oct 21, 2024 at 03:27:18PM +0200, Andrew Lunn wrote:
> > > > > It is related to the RGMII delay. On sg2044, when the phy 
> > > > > sets rx-delay, the interal mac is not set the same delay, 
> > > > > so this is needed to be set.
> > > > 
> > > > This is the wrong way to do it. Please look at how phy-mode should be
> > > > used, the four different "rgmii" values. Nearly everybody gets this
> > > > wrong, so there are plenty of emails from me in the netdev list about
> > > > how it should be done.
> > > > 
> > > 
> > > The phy-mode is alreay set to the "rgmii-id" and a rx delay is already
> > > set (a default tx delay is set by the phy driver). In the scenario 
> > > the extra bit is used to fix 2ns difference between the sampling clock
> > > and data. It is more like an extra setting and the kernel can not handle
> > > it by only setting the phy-mode.
> > 
> > This sounds wrong.
> > 
> > So in DT you have rgmii-id? You say the PHY is doing TX delay. So you
> > pass PHY_INTERFACE_MODE_RGMII_TXID to the PHY? It is not clear from
> > this patch, i don't see any code mentioning
> > PHY_INTERFACE_MODE_RGMII_TXID. Could you point me at that code.
> > 
> > 	Andrew
> 
> The phy on the board I have is YT8531, The config code is here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/motorcomm.c#n868

This PHY should be able to do rgmii-id, so there is no need for the
MAC to add delays. We encourage that setup in linux, so all RGMII
MAC/PHY pairs are the same, the PHY add the delays.

	Andrew

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Wed, Oct 23, 2024 at 03:08:52AM +0200, Andrew Lunn wrote:
> On Wed, Oct 23, 2024 at 08:41:36AM +0800, Inochi Amaoto wrote:
> > On Tue, Oct 22, 2024 at 03:51:08PM +0200, Andrew Lunn wrote:
> > > On Tue, Oct 22, 2024 at 06:21:49PM +0800, Inochi Amaoto wrote:
> > > > On Mon, Oct 21, 2024 at 03:27:18PM +0200, Andrew Lunn wrote:
> > > > > > It is related to the RGMII delay. On sg2044, when the phy 
> > > > > > sets rx-delay, the interal mac is not set the same delay, 
> > > > > > so this is needed to be set.
> > > > > 
> > > > > This is the wrong way to do it. Please look at how phy-mode should be
> > > > > used, the four different "rgmii" values. Nearly everybody gets this
> > > > > wrong, so there are plenty of emails from me in the netdev list about
> > > > > how it should be done.
> > > > > 
> > > > 
> > > > The phy-mode is alreay set to the "rgmii-id" and a rx delay is already
> > > > set (a default tx delay is set by the phy driver). In the scenario 
> > > > the extra bit is used to fix 2ns difference between the sampling clock
> > > > and data. It is more like an extra setting and the kernel can not handle
> > > > it by only setting the phy-mode.
> > > 
> > > This sounds wrong.
> > > 
> > > So in DT you have rgmii-id? You say the PHY is doing TX delay. So you
> > > pass PHY_INTERFACE_MODE_RGMII_TXID to the PHY? It is not clear from
> > > this patch, i don't see any code mentioning
> > > PHY_INTERFACE_MODE_RGMII_TXID. Could you point me at that code.
> > > 
> > > 	Andrew
> > 
> > The phy on the board I have is YT8531, The config code is here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/motorcomm.c#n868
> 
> This PHY should be able to do rgmii-id, so there is no need for the
> MAC to add delays. We encourage that setup in linux, so all RGMII
> MAC/PHY pairs are the same, the PHY add the delays.
> 

Yes, this is what I have done at the beginning. At first I only
set up the phy setting and not set the config in the syscon. 
But I got a weird thing: the phy lookback test is timeout. 
Although the datasheet told it just adds a internal delay for 
the phy, I suspect sophgo does something more to set this delay.

Regards,
Inochi

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

> Yes, this is what I have done at the beginning. At first I only
> set up the phy setting and not set the config in the syscon. 
> But I got a weird thing: the phy lookback test is timeout. 
> Although the datasheet told it just adds a internal delay for 
> the phy, I suspect sophgo does something more to set this delay.

You need to understand what is going on here. Just because it works
does not mean it is correct.

	Andrew

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Conor Dooley]

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

On Wed, Oct 23, 2024 at 08:31:24AM +0800, Inochi Amaoto wrote:
> On Tue, Oct 22, 2024 at 06:28:06PM +0100, Conor Dooley wrote:
> > On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> > > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > > with some extra clock.
> > > 
> > > Add necessary compatible string for this device.
> > > 
> > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > ---
> > >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> > >  2 files changed, 146 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > index 3c4007cb65f8..69f6bb36970b 100644
> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > @@ -99,6 +99,7 @@ properties:
> > >          - snps,dwmac-5.30a
> > >          - snps,dwxgmac
> > >          - snps,dwxgmac-2.10
> > > +        - sophgo,sg2044-dwmac
> > >          - starfive,jh7100-dwmac
> > >          - starfive,jh7110-dwmac
> > >  
> > > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > new file mode 100644
> > > index 000000000000..93c41550b0b6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > @@ -0,0 +1,145 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: StarFive JH7110 DWMAC glue layer
> > > +
> > > +maintainers:
> > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > +
> > > +select:
> > > +  properties:
> > > +    compatible:
> > > +      contains:
> > > +        enum:
> > > +          - sophgo,sg2044-dwmac
> > > +  required:
> > > +    - compatible
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: sophgo,sg2044-dwmac
> > > +      - const: snps,dwmac-5.30a
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: GMAC main clock
> > > +      - description: PTP clock
> > > +      - description: TX clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: stmmaceth
> > > +      - const: ptp_ref
> > > +      - const: tx
> > > +
> > > +  sophgo,syscon:
> > 
> > How many dwmac instances does the sg2044 have?
> > 
> 
> Only one, there is another 100G dwxgmac instance, but it does not
> use this syscon.

That dwxgmac is a different device, with a different compatible etc?

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

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Wed, Oct 23, 2024 at 02:42:16PM +0200, Andrew Lunn wrote:
> > Yes, this is what I have done at the beginning. At first I only
> > set up the phy setting and not set the config in the syscon. 
> > But I got a weird thing: the phy lookback test is timeout. 
> > Although the datasheet told it just adds a internal delay for 
> > the phy, I suspect sophgo does something more to set this delay.
> 
> You need to understand what is going on here. Just because it works
> does not mean it is correct.
> 

It seems like there is a missing info in the SG2044 doc: setting the
syscon internal delay bit is not enabling the internal mac delay, but
disable it. Now everything seems like normal: the mac adds no delay,
and the phy adds its delay. 

The sophgo have already confirmed this is a firmware issue that does 
not set up the mac delay correctly and will fix this in the firmware,
so the kernal can always have not mac delay. Since this will be fixed
in the firmware and this interface is not exposed to the kernel, I will
remove the code setting the syscon bit.

Thanks for your effort on this.

Regards,
Inochi

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Inochi Amaoto]

On Wed, Oct 23, 2024 at 09:49:34PM +0100, Conor Dooley wrote:
> On Wed, Oct 23, 2024 at 08:31:24AM +0800, Inochi Amaoto wrote:
> > On Tue, Oct 22, 2024 at 06:28:06PM +0100, Conor Dooley wrote:
> > > On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> > > > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > > > with some extra clock.
> > > > 
> > > > Add necessary compatible string for this device.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > ---
> > > >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > > >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> > > >  2 files changed, 146 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > index 3c4007cb65f8..69f6bb36970b 100644
> > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > @@ -99,6 +99,7 @@ properties:
> > > >          - snps,dwmac-5.30a
> > > >          - snps,dwxgmac
> > > >          - snps,dwxgmac-2.10
> > > > +        - sophgo,sg2044-dwmac
> > > >          - starfive,jh7100-dwmac
> > > >          - starfive,jh7110-dwmac
> > > >  
> > > > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > new file mode 100644
> > > > index 000000000000..93c41550b0b6
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > @@ -0,0 +1,145 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: StarFive JH7110 DWMAC glue layer
> > > > +
> > > > +maintainers:
> > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > +
> > > > +select:
> > > > +  properties:
> > > > +    compatible:
> > > > +      contains:
> > > > +        enum:
> > > > +          - sophgo,sg2044-dwmac
> > > > +  required:
> > > > +    - compatible
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - const: sophgo,sg2044-dwmac
> > > > +      - const: snps,dwmac-5.30a
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: GMAC main clock
> > > > +      - description: PTP clock
> > > > +      - description: TX clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: stmmaceth
> > > > +      - const: ptp_ref
> > > > +      - const: tx
> > > > +
> > > > +  sophgo,syscon:
> > > 
> > > How many dwmac instances does the sg2044 have?
> > > 
> > 
> > Only one, there is another 100G dwxgmac instance, but it does not
> > use this syscon.
> 
> That dwxgmac is a different device, with a different compatible etc?

Yes, it needs a different compatiable, and maybe a new binding is needed
since the 100G and 1G IP are different.

Regards,
Inochi

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Emil Renner Berthing]

Inochi Amaoto wrote:
> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> with some extra clock.
>
> Add necessary compatible string for this device.
>
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
>  2 files changed, 146 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 3c4007cb65f8..69f6bb36970b 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -99,6 +99,7 @@ properties:
>          - snps,dwmac-5.30a
>          - snps,dwxgmac
>          - snps,dwxgmac-2.10
> +        - sophgo,sg2044-dwmac
>          - starfive,jh7100-dwmac
>          - starfive,jh7110-dwmac
>
> diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> new file mode 100644
> index 000000000000..93c41550b0b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 DWMAC glue layer

I think you forgot to change this when you copied the binding.

/Emil

> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - sophgo,sg2044-dwmac
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: sophgo,sg2044-dwmac
> +      - const: snps,dwmac-5.30a
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: GMAC main clock
> +      - description: PTP clock
> +      - description: TX clock
> +
> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: ptp_ref
> +      - const: tx
> +
> +  sophgo,syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that configures phy
> +          - description: offset of phy mode register
> +          - description: length of the phy mode register
> +    description:
> +      A phandle to syscon with two arguments that configure phy mode.
> +      The argument one is the offset of phy mode register, the
> +      argument two is the length of phy mode register.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - resets
> +  - reset-names
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: sophgo,sg2044-dwmac
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          maxItems: 1
> +
> +        interrupt-names:
> +          minItems: 1
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +
> +        reset-names:
> +          const: stmmaceth
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    ethernet@30006000 {
> +      compatible = "sophgo,sg2044-dwmac", "snps,dwmac-5.30a";
> +      reg = <0x30006000 0x4000>;
> +      clocks = <&clk 151>, <&clk 152>, <&clk 154>;
> +      clock-names = "stmmaceth", "ptp_ref", "tx";
> +      interrupt-parent = <&intc>;
> +      interrupts = <296 IRQ_TYPE_LEVEL_HIGH>;
> +      interrupt-names = "macirq";
> +      resets = <&rst 30>;
> +      reset-names = "stmmaceth";
> +      snps,multicast-filter-bins = <0>;
> +      snps,perfect-filter-entries = <1>;
> +      snps,aal;
> +      snps,tso;
> +      snps,txpbl = <32>;
> +      snps,rxpbl = <32>;
> +      snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
> +      snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
> +      snps,axi-config = <&gmac0_stmmac_axi_setup>;
> +      status = "disabled";
> +
> +      gmac0_mtl_rx_setup: rx-queues-config {
> +        snps,rx-queues-to-use = <8>;
> +        snps,rx-sched-wsp;
> +        queue0 {};
> +        queue1 {};
> +        queue2 {};
> +        queue3 {};
> +        queue4 {};
> +        queue5 {};
> +        queue6 {};
> +        queue7 {};
> +      };
> +
> +      gmac0_mtl_tx_setup: tx-queues-config {
> +        snps,tx-queues-to-use = <8>;
> +        queue0 {};
> +        queue1 {};
> +        queue2 {};
> +        queue3 {};
> +        queue4 {};
> +        queue5 {};
> +        queue6 {};
> +        queue7 {};
> +      };
> +
> +      gmac0_stmmac_axi_setup: stmmac-axi-config {
> +        snps,blen = <16 8 4 0 0 0 0>;
> +        snps,wr_osr_lmt = <1>;
> +        snps,rd_osr_lmt = <2>;
> +      };
> +    };
> --
> 2.47.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Andrew Lunn]

On Thu, Oct 24, 2024 at 06:36:06AM +0800, Inochi Amaoto wrote:
> On Wed, Oct 23, 2024 at 02:42:16PM +0200, Andrew Lunn wrote:
> > > Yes, this is what I have done at the beginning. At first I only
> > > set up the phy setting and not set the config in the syscon. 
> > > But I got a weird thing: the phy lookback test is timeout. 
> > > Although the datasheet told it just adds a internal delay for 
> > > the phy, I suspect sophgo does something more to set this delay.
> > 
> > You need to understand what is going on here. Just because it works
> > does not mean it is correct.
> > 
> 
> It seems like there is a missing info in the SG2044 doc: setting the
> syscon internal delay bit is not enabling the internal mac delay, but
> disable it. Now everything seems like normal: the mac adds no delay,
> and the phy adds its delay. 

That makes a lot more sense.

Thanks for digging into the details.

	Andrew

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Inochi Amaoto]

On Wed, Oct 23, 2024 at 07:41:28PM -0400, Emil Renner Berthing wrote:
> Inochi Amaoto wrote:
> > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > with some extra clock.
> >
> > Add necessary compatible string for this device.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> >  2 files changed, 146 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 3c4007cb65f8..69f6bb36970b 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -99,6 +99,7 @@ properties:
> >          - snps,dwmac-5.30a
> >          - snps,dwxgmac
> >          - snps,dwxgmac-2.10
> > +        - sophgo,sg2044-dwmac
> >          - starfive,jh7100-dwmac
> >          - starfive,jh7110-dwmac
> >
> > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > new file mode 100644
> > index 000000000000..93c41550b0b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > @@ -0,0 +1,145 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive JH7110 DWMAC glue layer
> 
> I think you forgot to change this when you copied the binding.
> 
> /Emil
> 

Thanks, I will fix it.

Regards,
Inochi

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Uwe Kleine-König]

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

Hello,

On Mon, Oct 21, 2024 at 06:36:17PM +0800, Inochi Amaoto wrote:
> +static struct platform_driver sophgo_dwmac_driver = {
> +	.probe  = sophgo_dwmac_probe,
> +	.remove_new = stmmac_pltfr_remove,
> +	.driver = {
> +		.name = "sophgo-dwmac",
> +		.pm = &stmmac_pltfr_pm_ops,
> +		.of_match_table = sophgo_dwmac_match,
> +	},
> +};

After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
return void") .remove() is (again) the right callback to implement for
platform drivers. Please just drop "_new".

Best regards
Uwe

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

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Conor Dooley]

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

On Thu, Oct 24, 2024 at 06:38:29AM +0800, Inochi Amaoto wrote:
> On Wed, Oct 23, 2024 at 09:49:34PM +0100, Conor Dooley wrote:
> > On Wed, Oct 23, 2024 at 08:31:24AM +0800, Inochi Amaoto wrote:
> > > On Tue, Oct 22, 2024 at 06:28:06PM +0100, Conor Dooley wrote:
> > > > On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> > > > > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > > > > with some extra clock.
> > > > > 
> > > > > Add necessary compatible string for this device.
> > > > > 
> > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > ---
> > > > >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > > > >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> > > > >  2 files changed, 146 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > index 3c4007cb65f8..69f6bb36970b 100644
> > > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > @@ -99,6 +99,7 @@ properties:
> > > > >          - snps,dwmac-5.30a
> > > > >          - snps,dwxgmac
> > > > >          - snps,dwxgmac-2.10
> > > > > +        - sophgo,sg2044-dwmac
> > > > >          - starfive,jh7100-dwmac
> > > > >          - starfive,jh7110-dwmac
> > > > >  
> > > > > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..93c41550b0b6
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > @@ -0,0 +1,145 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: StarFive JH7110 DWMAC glue layer
> > > > > +
> > > > > +maintainers:
> > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > +
> > > > > +select:
> > > > > +  properties:
> > > > > +    compatible:
> > > > > +      contains:
> > > > > +        enum:
> > > > > +          - sophgo,sg2044-dwmac
> > > > > +  required:
> > > > > +    - compatible
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    items:
> > > > > +      - const: sophgo,sg2044-dwmac
> > > > > +      - const: snps,dwmac-5.30a
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    items:
> > > > > +      - description: GMAC main clock
> > > > > +      - description: PTP clock
> > > > > +      - description: TX clock
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: stmmaceth
> > > > > +      - const: ptp_ref
> > > > > +      - const: tx
> > > > > +
> > > > > +  sophgo,syscon:
> > > > 
> > > > How many dwmac instances does the sg2044 have?
> > > > 
> > > 
> > > Only one, there is another 100G dwxgmac instance, but it does not
> > > use this syscon.
> > 
> > That dwxgmac is a different device, with a different compatible etc?
> 
> Yes, it needs a different compatiable, and maybe a new binding is needed
> since the 100G and 1G IP are different.

In that case, you don't /need/ a syscon property at all, much less one
with offsets. You can just look up the syscon by compatible and hard
code the offset in the driver.

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

Re: [PATCH 2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

[Inochi Amaoto]

On Thu, Oct 24, 2024 at 06:04:31PM +0100, Conor Dooley wrote:
> On Thu, Oct 24, 2024 at 06:38:29AM +0800, Inochi Amaoto wrote:
> > On Wed, Oct 23, 2024 at 09:49:34PM +0100, Conor Dooley wrote:
> > > On Wed, Oct 23, 2024 at 08:31:24AM +0800, Inochi Amaoto wrote:
> > > > On Tue, Oct 22, 2024 at 06:28:06PM +0100, Conor Dooley wrote:
> > > > > On Mon, Oct 21, 2024 at 06:36:15PM +0800, Inochi Amaoto wrote:
> > > > > > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > > > > > with some extra clock.
> > > > > > 
> > > > > > Add necessary compatible string for this device.
> > > > > > 
> > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
> > > > > >  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 145 ++++++++++++++++++
> > > > > >  2 files changed, 146 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > > index 3c4007cb65f8..69f6bb36970b 100644
> > > > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > > > @@ -99,6 +99,7 @@ properties:
> > > > > >          - snps,dwmac-5.30a
> > > > > >          - snps,dwxgmac
> > > > > >          - snps,dwxgmac-2.10
> > > > > > +        - sophgo,sg2044-dwmac
> > > > > >          - starfive,jh7100-dwmac
> > > > > >          - starfive,jh7110-dwmac
> > > > > >  
> > > > > > diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..93c41550b0b6
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
> > > > > > @@ -0,0 +1,145 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: StarFive JH7110 DWMAC glue layer
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > +
> > > > > > +select:
> > > > > > +  properties:
> > > > > > +    compatible:
> > > > > > +      contains:
> > > > > > +        enum:
> > > > > > +          - sophgo,sg2044-dwmac
> > > > > > +  required:
> > > > > > +    - compatible
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    items:
> > > > > > +      - const: sophgo,sg2044-dwmac
> > > > > > +      - const: snps,dwmac-5.30a
> > > > > > +
> > > > > > +  reg:
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    items:
> > > > > > +      - description: GMAC main clock
> > > > > > +      - description: PTP clock
> > > > > > +      - description: TX clock
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +    items:
> > > > > > +      - const: stmmaceth
> > > > > > +      - const: ptp_ref
> > > > > > +      - const: tx
> > > > > > +
> > > > > > +  sophgo,syscon:
> > > > > 
> > > > > How many dwmac instances does the sg2044 have?
> > > > > 
> > > > 
> > > > Only one, there is another 100G dwxgmac instance, but it does not
> > > > use this syscon.
> > > 
> > > That dwxgmac is a different device, with a different compatible etc?
> > 
> > Yes, it needs a different compatiable, and maybe a new binding is needed
> > since the 100G and 1G IP are different.
> 
> In that case, you don't /need/ a syscon property at all, much less one
> with offsets. You can just look up the syscon by compatible and hard
> code the offset in the driver.

Good, look up the syscon is a good idea. Thanks for this suggestion.

Regards,
Inochi

Re: [PATCH 4/4] net: stmmac: Add glue layer for Sophgo SG2044 SoC

[Inochi Amaoto]

On Thu, Oct 24, 2024 at 05:37:03PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Mon, Oct 21, 2024 at 06:36:17PM +0800, Inochi Amaoto wrote:
> > +static struct platform_driver sophgo_dwmac_driver = {
> > +	.probe  = sophgo_dwmac_probe,
> > +	.remove_new = stmmac_pltfr_remove,
> > +	.driver = {
> > +		.name = "sophgo-dwmac",
> > +		.pm = &stmmac_pltfr_pm_ops,
> > +		.of_match_table = sophgo_dwmac_match,
> > +	},
> > +};
> 
> After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
> return void") .remove() is (again) the right callback to implement for
> platform drivers. Please just drop "_new".
> 
> Best regards
> Uwe


Thanks, I will fix it.

Regards,
Inochi