[PATCH net-next 0/5] net: stmmac: socfpga: fix init ordering and cleanups

[PATCH net-next 0/5] net: stmmac: socfpga: fix init ordering and cleanups

[Russell King (Oracle)]

Hi,

This series fixes the init ordering of the socfpga probe function.
The standard rule is to do all setup before publishing any device,
and socfpga violates that. I can see no reason for this, but these
patches have not been tested on hardware.

Address this by moving the initialisation of dwmac->stmmac_rst
along with all the other dwmac initialisers - there's no reason
for this to be late as plat_dat->stmmac_rst has already been
populated.

Next, replace the call to ops->set_phy_mode() with an init function
socfpga_dwmac_init() which will then be linked in to plat_dat->init.

Then, add this to plat_dat->init, and switch to stmmac_pltfr_pm_ops
from the private ops. The runtime suspend/resume socfpga implementations
are identical to the platform ones, but misses the noirq versions
which this will add.

Next, swap the order of socfpga_dwmac_init() and stmmac_dvr_probe().

Finally, convert to devm_stmmac_pltfr_probe() by moving the call
to ops->set_phy_mode() into an init function appropriately populating
plat_dat->init.

 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 79 +++++-----------------
 1 file changed, 16 insertions(+), 63 deletions(-)

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

[PATCH net-next 1/5] net: stmmac: socfpga: init dwmac->stmmac_rst before registration

[Russell King (Oracle)]

Initialisation/setup after registration is a bug. This is the first of
two patches fixing this in socfpga.

dwmac->stmmac_rst is initialised from the stmmac plat_dat's stmmac_rst
member, which is itself initialised by devm_stmmac_probe_config_dt().
Therefore, this can be initialised before we call stmmac_dvr_probe().
Move it there.

dwmac->stmmac_rst is used by the set_phy_mode() method.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 116855658559..bcdb25ee2a33 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -442,8 +442,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	struct device		*dev = &pdev->dev;
 	int			ret;
 	struct socfpga_dwmac	*dwmac;
-	struct net_device	*ndev;
-	struct stmmac_priv	*stpriv;
 	const struct socfpga_dwmac_ops *ops;
 
 	ops = device_get_match_data(&pdev->dev);
@@ -479,7 +477,13 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* The socfpga driver needs to control the stmmac reset to set the phy
+	 * mode. Create a copy of the core reset handle so it can be used by
+	 * the driver later.
+	 */
+	dwmac->stmmac_rst = plat_dat->stmmac_rst;
 	dwmac->ops = ops;
+
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
 	plat_dat->pcs_init = socfpga_dwmac_pcs_init;
@@ -493,15 +497,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ndev = platform_get_drvdata(pdev);
-	stpriv = netdev_priv(ndev);
-
-	/* The socfpga driver needs to control the stmmac reset to set the phy
-	 * mode. Create a copy of the core reset handle so it can be used by
-	 * the driver later.
-	 */
-	dwmac->stmmac_rst = stpriv->plat->stmmac_rst;
-
 	ret = ops->set_phy_mode(dwmac);
 	if (ret)
 		goto err_dvr_remove;
-- 
2.30.2

[PATCH net-next 2/5] net: stmmac: socfpga: provide init function

[Russell King (Oracle)]

Both the resume and probe path needs to configure the phy mode, so
provide a common function to do this which can later be hooked into
plat_dat->init.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index bcdb25ee2a33..000d349a6d4c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -435,6 +435,13 @@ static struct phylink_pcs *socfpga_dwmac_select_pcs(struct stmmac_priv *priv,
 	return priv->hw->phylink_pcs;
 }
 
+static int socfpga_dwmac_init(struct platform_device *pdev, void *bsp_priv)
+{
+	struct socfpga_dwmac *dwmac = bsp_priv;
+
+	return dwmac->ops->set_phy_mode(dwmac);
+}
+
 static int socfpga_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -497,7 +504,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = ops->set_phy_mode(dwmac);
+	ret = socfpga_dwmac_init(pdev, dwmac);
 	if (ret)
 		goto err_dvr_remove;
 
@@ -516,7 +523,7 @@ static int socfpga_dwmac_resume(struct device *dev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	struct socfpga_dwmac *dwmac_priv = get_stmmac_bsp_priv(dev);
 
-	dwmac_priv->ops->set_phy_mode(priv->plat->bsp_priv);
+	socfpga_dwmac_init(to_platform_device(dev), dwmac_priv);
 
 	return stmmac_resume(dev);
 }
-- 
2.30.2

[PATCH net-next 3/5] net: stmmac: socfpga: convert to stmmac_pltfr_pm_ops

[Russell King (Oracle)]

Convert socfpga to use the generic stmmac_pltfr_pm_ops, which can be
achieved by adding an appropriate plat_dat->init function to do the
setup.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../ethernet/stmicro/stmmac/dwmac-socfpga.c   | 39 +------------------
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 000d349a6d4c..69ffc52c0275 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -493,6 +493,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
+	plat_dat->init = socfpga_dwmac_init;
 	plat_dat->pcs_init = socfpga_dwmac_pcs_init;
 	plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
 	plat_dat->select_pcs = socfpga_dwmac_select_pcs;
@@ -516,42 +517,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int socfpga_dwmac_resume(struct device *dev)
-{
-	struct net_device *ndev = dev_get_drvdata(dev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
-	struct socfpga_dwmac *dwmac_priv = get_stmmac_bsp_priv(dev);
-
-	socfpga_dwmac_init(to_platform_device(dev), dwmac_priv);
-
-	return stmmac_resume(dev);
-}
-#endif /* CONFIG_PM_SLEEP */
-
-static int __maybe_unused socfpga_dwmac_runtime_suspend(struct device *dev)
-{
-	struct net_device *ndev = dev_get_drvdata(dev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
-
-	stmmac_bus_clks_config(priv, false);
-
-	return 0;
-}
-
-static int __maybe_unused socfpga_dwmac_runtime_resume(struct device *dev)
-{
-	struct net_device *ndev = dev_get_drvdata(dev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
-
-	return stmmac_bus_clks_config(priv, true);
-}
-
-static const struct dev_pm_ops socfpga_dwmac_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(stmmac_suspend, socfpga_dwmac_resume)
-	SET_RUNTIME_PM_OPS(socfpga_dwmac_runtime_suspend, socfpga_dwmac_runtime_resume, NULL)
-};
-
 static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
 	.set_phy_mode = socfpga_gen5_set_phy_mode,
 };
@@ -572,7 +537,7 @@ static struct platform_driver socfpga_dwmac_driver = {
 	.remove = stmmac_pltfr_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
-		.pm		= &socfpga_dwmac_pm_ops,
+		.pm		= &stmmac_pltfr_pm_ops,
 		.of_match_table = socfpga_dwmac_match,
 	},
 };
-- 
2.30.2

[PATCH net-next 4/5] net: stmmac: socfpga: call set_phy_mode() before registration

[Russell King (Oracle)]

Initialisation/setup after registration is a bug. This is the second
of two patches fixing this in socfpga.

The set_phy_mode() does various hardware setup that would interfere
with a netdev that has been published, and thus available to be opened
by the kernel/userspace.

Move the call to set_phy_mode() before calling stmmac_dvr_probe().
This also simplifies the probe function as there is no need to
unregister the netdev if set_phy_mode() fails.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 69ffc52c0275..251d8b72bed5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -501,20 +501,11 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 
 	plat_dat->riwt_off = 1;
 
-	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
-	if (ret)
-		return ret;
-
 	ret = socfpga_dwmac_init(pdev, dwmac);
 	if (ret)
-		goto err_dvr_remove;
-
-	return 0;
-
-err_dvr_remove:
-	stmmac_dvr_remove(&pdev->dev);
+		return ret;
 
-	return ret;
+	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 }
 
 static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
-- 
2.30.2

[PATCH net-next 5/5] net: stmmac: socfpga: convert to devm_stmmac_pltfr_probe()

[Russell King (Oracle)]

Convert socfpga to use devm_stmmac_pltfr_probe() to further simplify
the probe function, wrapping the call to the set_phy_mode() method
into socfpga_dwmac_init() which can be called from the plat_dat->init()
method. Also call this from socfpga_dwmac_resume() thereby simplifying
that function.

Using the devm variant also means we can remove the call to
stmmac_pltfr_remove().

Unfortunately, we can't also convert to stmmac_pltfr_pm_ops as there is
extra work done in socfpga_dwmac_resume().

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 251d8b72bed5..8e6d780669b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -501,11 +501,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 
 	plat_dat->riwt_off = 1;
 
-	ret = socfpga_dwmac_init(pdev, dwmac);
-	if (ret)
-		return ret;
-
-	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	return devm_stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
 }
 
 static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
@@ -525,7 +521,6 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
 
 static struct platform_driver socfpga_dwmac_driver = {
 	.probe  = socfpga_dwmac_probe,
-	.remove = stmmac_pltfr_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
 		.pm		= &stmmac_pltfr_pm_ops,
-- 
2.30.2

Re: [PATCH net-next 0/5] net: stmmac: socfpga: fix init ordering and cleanups

[Maxime Chevallier]

Hello Russell,

On Tue, 15 Apr 2025 17:29:28 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> Hi,
> 
> This series fixes the init ordering of the socfpga probe function.
> The standard rule is to do all setup before publishing any device,
> and socfpga violates that. I can see no reason for this, but these
> patches have not been tested on hardware.
> 
> Address this by moving the initialisation of dwmac->stmmac_rst
> along with all the other dwmac initialisers - there's no reason
> for this to be late as plat_dat->stmmac_rst has already been
> populated.
> 
> Next, replace the call to ops->set_phy_mode() with an init function
> socfpga_dwmac_init() which will then be linked in to plat_dat->init.
> 
> Then, add this to plat_dat->init, and switch to stmmac_pltfr_pm_ops
> from the private ops. The runtime suspend/resume socfpga implementations
> are identical to the platform ones, but misses the noirq versions
> which this will add.
> 
> Next, swap the order of socfpga_dwmac_init() and stmmac_dvr_probe().

I've given this a try and unfortunately :

[    1.005290] 8<--- cut here ---
[    1.008378] Unable to handle kernel paging request at virtual address 00003c40 when read
[    1.016449] [00003c40] *pgd=00000000
[    1.020057] Internal error: Oops: 5 [#1] SMP ARM
[    1.024668] Modules linked in:
[    1.027726] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.15.0-rc1-00379-g4552d2fc04f5-dirty #304 PREEMPT 
[    1.037790] Hardware name: Altera SOCFPGA
[    1.041790] PC is at socfpga_gen5_set_phy_mode+0x44/0x338
[    1.047189] LR is at stmmac_pltfr_probe+0x24/0x6c
[    1.051893] pc : [<c010b82c>]    lr : [<c0611680>]    psr: a0000013
[    1.058142] sp : f0831d58  ip : 00000001  fp : 00000000
[    1.063353] r10: c0b4a508  r9 : 00000000  r8 : c093a170
[    1.068563] r7 : 00000060  r6 : c1880c00  r5 : c191a040  r4 : c203e9c0
[    1.075071] r3 : 00003000  r2 : 00000119  r1 : c0939ed0  r0 : c0a45f70
[    1.081579] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    1.088697] Control: 10c5387d  Table: 0000404a  DAC: 00000051
[    1.094424] Register r0 information: non-slab/vmalloc memory
[    1.100075] Register r1 information: non-slab/vmalloc memory
[    1.105723] Register r2 information: non-paged memory
[    1.110765] Register r3 information: non-paged memory
[    1.115807] Register r4 information: slab kmalloc-128 start c203e980 pointer offset 64 size 128
[    1.124508] Register r5 information: slab kmalloc-1k start c191a000 pointer offset 64 size 1024
[    1.133207] Register r6 information: slab kmalloc-512 start c1880c00 pointer offset 0 size 512
[    1.141816] Register r7 information: non-paged memory
[    1.146858] Register r8 information: non-slab/vmalloc memory
[    1.152506] Register r9 information: NULL pointer
[    1.157202] Register r10 information: non-slab/vmalloc memory
[    1.162937] Register r11 information: NULL pointer
[    1.167719] Register r12 information: non-paged memory
[    1.172848] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[    1.178840] Stack: (0xf0831d58 to 0xf0832000)
[    1.183192] 1d40:                                                       c0508cfc c050a250
[    1.191348] 1d60: c203e9c0 fb79b606 c18f6010 00000000 c191a040 f0831db0 c18f6000 c093a170
[    1.199504] 1d80: c203e9c0 c0611680 00000000 c18f6000 c18f6010 c191a040 c093a170 c0611918
[    1.207658] 1da0: 00000000 c18f6000 c18f6010 c061309c f0960000 00000000 00000000 0000001e
[    1.215811] 1dc0: fffffffa 0000001e fffffffa 00000000 00000000 00000000 00000000 00000000
[    1.223963] 1de0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.232117] 1e00: 00000000 00000000 00000000 00000000 00000000 fb79b606 c18f6010 00000000
[    1.240270] 1e20: c18f6010 c173b5b4 00000000 c203a1b8 c0b3f858 c0551410 c18f6010 00000000
[    1.248425] 1e40: c173b5b4 c054ed84 c18f6010 c173b5b4 c18f6010 00000000 c203a1b8 c054f008
[    1.256580] 1e60: 60000013 c0b3f858 c176b75c c173b5b4 c18f6010 00000000 c203a1b8 c054f1e8
[    1.264733] 1e80: c18f6010 c173b5b4 c18f6054 c1833600 c203a1b8 c054f3cc 00000000 c173b5b4
[    1.272888] 1ea0: c054f33c c054cdc0 c174b000 c1833658 c19b9f34 fb79b606 c1833600 c173b5b4
[    1.281041] 1ec0: c203a180 00000000 c1833600 c054e164 c0a46214 00000006 c173b5b4 c18b3600
[    1.289193] 1ee0: 00000006 00000000 c174b000 c05500c8 c0b25bd4 c18b3600 00000006 c010d244
[    1.297346] 1f00: c19a8bc6 c19a8bcc 00000000 00000000 000000e4 c19a8bcc c19a8be3 c014a2a0
[    1.305499] 1f20: c0aa4490 c0a46d70 00000000 00000000 00000006 00000006 00000064 c19a8bcc
[    1.313653] 1f40: c19a8bd1 fb79b606 c0b4a064 fb79b606 00000064 c19a8b80 00000064 00000006
[    1.321806] 1f60: c0b3f834 c0a62760 c0b3f858 c0b01340 00000006 00000006 00000000 c0b004d0
[    1.329958] 1f80: c1703e40 c08c3d08 00000000 00000000 00000000 00000000 00000000 c08c3d24
[    1.338113] 1fa0: 00000000 c08c3d08 00000000 c010014c 00000000 00000000 00000000 00000000
[    1.346266] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.354420] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[    1.362568] Call trace: 
[    1.362581]  socfpga_gen5_set_phy_mode from stmmac_pltfr_probe+0x24/0x6c
[    1.371804]  stmmac_pltfr_probe from devm_stmmac_pltfr_probe+0xc/0x78
[    1.378243]  devm_stmmac_pltfr_probe from socfpga_dwmac_probe+0x144/0x19c
[    1.385027]  socfpga_dwmac_probe from platform_probe+0x5c/0xb0
[    1.390868]  platform_probe from really_probe+0xb8/0x2b4
[    1.396184]  really_probe from __driver_probe_device+0x88/0x1a0
[    1.402100]  __driver_probe_device from driver_probe_device+0x30/0xc0
[    1.408536]  driver_probe_device from __driver_attach+0x90/0x13c
[    1.414539]  __driver_attach from bus_for_each_dev+0x70/0xc0
[    1.420194]  bus_for_each_dev from bus_add_driver+0xcc/0x1ec
[    1.425848]  bus_add_driver from driver_register+0x7c/0x114
[    1.431418]  driver_register from do_one_initcall+0x44/0x270
[    1.437080]  do_one_initcall from kernel_init_freeable+0x208/0x260
[    1.443258]  kernel_init_freeable from kernel_init+0x1c/0x12c
[    1.449000]  kernel_init from ret_from_fork+0x14/0x28

From what I see the issue is :

socfpga_dwmac_init()
  socfpga_gen5_set_phy_mode()
    socfpga_get_plat_phymode() {
      struct net_device *ndev = dev_get_drvdata(dwmac->dev);
      [...]
    }

At that moment, the netdev doesn't exist yet as the ->init() is called
before the netdev is created.

This is only to get the phymode, maybe we should do like dwmac_imx
and store a pointer to plat_dat into struct dwmac_socfpga, so that we
can get it back in dwmac_init ? I've tried with the patch below and it
does solve the issue, but maybe you have a better approach.

Maxime

---

From 4cad86cec2023030ae86126b436e4b08b9b99212 Mon Sep 17 00:00:00 2001
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Wed, 16 Apr 2025 09:49:50 +0200
Subject: [PATCH] net: stmmac: socfpga: Don't grab plat_data from netdev_priv
 in init path

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 8e6d780669b9..b866054808d6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -43,7 +43,7 @@
 
 struct socfpga_dwmac;
 struct socfpga_dwmac_ops {
-	int (*set_phy_mode)(struct socfpga_dwmac *dwmac_priv);
+	int (*set_phy_mode)(struct socfpga_dwmac *dwmac_priv, int phymode);
 };
 
 struct socfpga_dwmac {
@@ -59,6 +59,7 @@ struct socfpga_dwmac {
 	bool f2h_ptp_ref_clk;
 	const struct socfpga_dwmac_ops *ops;
 	struct mdio_device *pcs_mdiodev;
+	struct plat_stmmacenet_data *plat_dat;
 };
 
 static void socfpga_dwmac_fix_mac_speed(void *priv, int speed, unsigned int mode)
@@ -269,10 +270,9 @@ static int socfpga_set_phy_mode_common(int phymode, u32 *val)
 	return 0;
 }
 
-static int socfpga_gen5_set_phy_mode(struct socfpga_dwmac *dwmac)
+static int socfpga_gen5_set_phy_mode(struct socfpga_dwmac *dwmac, int phymode)
 {
 	struct regmap *sys_mgr_base_addr = dwmac->sys_mgr_base_addr;
-	int phymode = socfpga_get_plat_phymode(dwmac);
 	u32 reg_offset = dwmac->reg_offset;
 	u32 reg_shift = dwmac->reg_shift;
 	u32 ctrl, val, module;
@@ -327,10 +327,9 @@ static int socfpga_gen5_set_phy_mode(struct socfpga_dwmac *dwmac)
 	return 0;
 }
 
-static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac)
+static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac, int phymode)
 {
 	struct regmap *sys_mgr_base_addr = dwmac->sys_mgr_base_addr;
-	int phymode = socfpga_get_plat_phymode(dwmac);
 	u32 reg_offset = dwmac->reg_offset;
 	u32 reg_shift = dwmac->reg_shift;
 	u32 ctrl, val, module;
@@ -438,8 +437,9 @@ static struct phylink_pcs *socfpga_dwmac_select_pcs(struct stmmac_priv *priv,
 static int socfpga_dwmac_init(struct platform_device *pdev, void *bsp_priv)
 {
 	struct socfpga_dwmac *dwmac = bsp_priv;
+	struct plat_stmmacenet_data *plat_dat = dwmac->plat_dat;
 
-	return dwmac->ops->set_phy_mode(dwmac);
+	return dwmac->ops->set_phy_mode(dwmac, plat_dat->mac_interface);
 }
 
 static int socfpga_dwmac_probe(struct platform_device *pdev)
@@ -490,6 +490,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	 */
 	dwmac->stmmac_rst = plat_dat->stmmac_rst;
 	dwmac->ops = ops;
+	dwmac->plat_dat = plat_dat;
 
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
-- 
2.49.0

Re: [PATCH net-next 0/5] net: stmmac: socfpga: fix init ordering and cleanups

[Russell King (Oracle)]

On Wed, Apr 16, 2025 at 09:53:43AM +0200, Maxime Chevallier wrote:
> I've given this a try and unfortunately :

Great, someone with hardware, and who responds to patches! :)

> This is only to get the phymode, maybe we should do like dwmac_imx
> and store a pointer to plat_dat into struct dwmac_socfpga, so that we
> can get it back in dwmac_init ? I've tried with the patch below and it
> does solve the issue, but maybe you have a better approach.

Yes, but I don't think we need such a big patch:

 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 8e6d780669b9..59f90b123c5b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -50,6 +50,7 @@ struct socfpga_dwmac {
 	u32	reg_offset;
 	u32	reg_shift;
 	struct	device *dev;
+	struct plat_stmmacenet_data *plat_dat;
 	struct regmap *sys_mgr_base_addr;
 	struct reset_control *stmmac_rst;
 	struct reset_control *stmmac_ocp_rst;
@@ -233,10 +234,7 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 
 static int socfpga_get_plat_phymode(struct socfpga_dwmac *dwmac)
 {
-	struct net_device *ndev = dev_get_drvdata(dwmac->dev);
-	struct stmmac_priv *priv = netdev_priv(ndev);
-
-	return priv->plat->mac_interface;
+	return dwmac->plat_dat->mac_interface;
 }
 
 static void socfpga_sgmii_config(struct socfpga_dwmac *dwmac, bool enable)
@@ -490,6 +488,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	 */
 	dwmac->stmmac_rst = plat_dat->stmmac_rst;
 	dwmac->ops = ops;
+	dwmac->plat_dat = plat_dat;
 
 	plat_dat->bsp_priv = dwmac;
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;

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

Re: [PATCH net-next 0/5] net: stmmac: socfpga: fix init ordering and cleanups

[Maxime Chevallier]

On Wed, 16 Apr 2025 09:20:38 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 16, 2025 at 09:53:43AM +0200, Maxime Chevallier wrote:
> > I've given this a try and unfortunately :  
> 
> Great, someone with hardware, and who responds to patches! :)
> 
> > This is only to get the phymode, maybe we should do like dwmac_imx
> > and store a pointer to plat_dat into struct dwmac_socfpga, so that we
> > can get it back in dwmac_init ? I've tried with the patch below and it
> > does solve the issue, but maybe you have a better approach.  
> 
> Yes, but I don't think we need such a big patch:
> 
>  drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index 8e6d780669b9..59f90b123c5b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -50,6 +50,7 @@ struct socfpga_dwmac {
>  	u32	reg_offset;
>  	u32	reg_shift;
>  	struct	device *dev;
> +	struct plat_stmmacenet_data *plat_dat;
>  	struct regmap *sys_mgr_base_addr;
>  	struct reset_control *stmmac_rst;
>  	struct reset_control *stmmac_ocp_rst;
> @@ -233,10 +234,7 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
>  
>  static int socfpga_get_plat_phymode(struct socfpga_dwmac *dwmac)
>  {
> -	struct net_device *ndev = dev_get_drvdata(dwmac->dev);
> -	struct stmmac_priv *priv = netdev_priv(ndev);
> -
> -	return priv->plat->mac_interface;
> +	return dwmac->plat_dat->mac_interface;
>  }
>  
>  static void socfpga_sgmii_config(struct socfpga_dwmac *dwmac, bool enable)
> @@ -490,6 +488,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
>  	 */
>  	dwmac->stmmac_rst = plat_dat->stmmac_rst;
>  	dwmac->ops = ops;
> +	dwmac->plat_dat = plat_dat;
>  
>  	plat_dat->bsp_priv = dwmac;
>  	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
> 

Even better indeed ! I've tested it and it works.

I'll be happy to test any followup :)

Maxime