net: stmmac: yet more cleanups

PATCH Applied Russell King (Oracle) 7 participants 23 messages Sep 23, 2025 – Apr 17

Subsystems (4)

Networking Network drivers Arm STM32 stmmac Ethernet driver

lore

Share & tools

lore

Russell King (Oracle)

author

2025-09-23 11:25 UTC #0 lore raw

cover net: stmmac: yet more cleanups

Building on the previous cleanup series, this cleans up yet more stmmac code.

- Move stmmac_bus_clks_config() into stmmac_platform() which is where its onlny user is.

- Move the xpcs Clause 73 test into stmmac_init_phy(), resulting in simpler code in __stmmac_open().

- Move "can't attach PHY" error message into stmmac_init_phy().

We then start moving stuff out of __stmac_open() into stmmac_open() (and correspondingly __stmmac_release() into stmmac_release()) which is not necessary when re-initialising the interface on e.g. MTU change.

- Move initialisation of tx_lpi_timer
- Move PHY attachment/detachment
- Move PHY error message into stmmac_init_phy()

Finally, simplfy the paths in stmmac_init_phy().

 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 111 ++++++++-------------
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  32 ++++++
 3 files changed, 73 insertions(+), 71 deletions(-)

Signature · 2 lines

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

Russell King (Oracle)

2025-09-23 11:26 UTC #1 lore raw

PATCH 1/6 net: stmmac: move stmmac_bus_clks_config() to stmmac_platform.c

stmmac_bus_clks_config() is only used by stmmac_platform.c, so rather than having it in stmmac_main.c and needing to export the symbol, move it to where it's used.

Signed-off-by Russell King (Oracle) <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 33 -------------------
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 32 ++++++++++++++++++
 3 files changed, 32 insertions(+), 34 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 -1

index 151f08e5e85d..7ca5477be390 100644
@@ -404,7 +404,6 @@ int stmmac_dvr_probe(struct device *device,
 		     struct stmmac_resources *res);
 int stmmac_reinit_queues(struct net_device *dev, u32 rx_cnt, u32 tx_cnt);
 int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size);
-int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled);
 int stmmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
 			   phy_interface_t interface, int speed);

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0 ~~-33~~

index d17820d9e7f1..517b25b2bcae 100644
@@ -147,39 +147,6 @@ static void stmmac_exit_fs(struct net_device *dev);
 
 #define STMMAC_COAL_TIMER(x) (ns_to_ktime((x) * NSEC_PER_USEC))
 
-int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled)
-{
-	struct plat_stmmacenet_data *plat_dat = priv->plat;
-	int ret;
-
-	if (enabled) {
-		ret = clk_prepare_enable(plat_dat->stmmac_clk);
-		if (ret)
-			return ret;
-		ret = clk_prepare_enable(plat_dat->pclk);
-		if (ret) {
-			clk_disable_unprepare(plat_dat->stmmac_clk);
-			return ret;
-		}
-		if (plat_dat->clks_config) {
-			ret = plat_dat->clks_config(plat_dat->bsp_priv, enabled);
-			if (ret) {
-				clk_disable_unprepare(plat_dat->stmmac_clk);
-				clk_disable_unprepare(plat_dat->pclk);
-				return ret;
-			}
-		}
-	} else {
-		clk_disable_unprepare(plat_dat->stmmac_clk);
-		clk_disable_unprepare(plat_dat->pclk);
-		if (plat_dat->clks_config)
-			plat_dat->clks_config(plat_dat->bsp_priv, enabled);
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(stmmac_bus_clks_config);
-
 /**
  * stmmac_set_clk_tx_rate() - set the clock rate for the MAC transmit clock
  * @bsp_priv: BSP private data structure (unused)

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +32 -0

index 712ef235f0f4..27bcaae07a7f 100644
@@ -911,6 +911,38 @@ void stmmac_pltfr_remove(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(stmmac_pltfr_remove);
 
+static int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled)
+{
+	struct plat_stmmacenet_data *plat_dat = priv->plat;
+	int ret;
+
+	if (enabled) {
+		ret = clk_prepare_enable(plat_dat->stmmac_clk);
+		if (ret)
+			return ret;
+		ret = clk_prepare_enable(plat_dat->pclk);
+		if (ret) {
+			clk_disable_unprepare(plat_dat->stmmac_clk);
+			return ret;
+		}
+		if (plat_dat->clks_config) {
+			ret = plat_dat->clks_config(plat_dat->bsp_priv, enabled);
+			if (ret) {
+				clk_disable_unprepare(plat_dat->stmmac_clk);
+				clk_disable_unprepare(plat_dat->pclk);
+				return ret;
+			}
+		}
+	} else {
+		clk_disable_unprepare(plat_dat->stmmac_clk);
+		clk_disable_unprepare(plat_dat->pclk);
+		if (plat_dat->clks_config)
+			plat_dat->clks_config(plat_dat->bsp_priv, enabled);
+	}
+
+	return 0;
+}
+
 static int __maybe_unused stmmac_runtime_suspend(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);

Signature · 1 line

2.47.3

Russell King (Oracle)

2025-09-23 11:26 UTC #2 lore raw

PATCH 2/6 net: stmmac: move xpcs clause 73 test into stmmac_init_phy()

In reply to Russell King (Oracle)

We avoid binding a PHY if the XPCS is using clause 73 negotiation. Rather than having this complexity in __stmmac_open(), move it to stmmac_init_phy() instead. There is no point checking the XPCS state this unless phylink wants a PHY, so place this appropriately.

Signed-off-by Russell King (Oracle) <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 21 ++++++++++---------
 1 file changed, 11 insertions(+), 10 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +11 ~~-10~~

index 517b25b2bcae..3b47d4ca24ca 100644
@@ -1112,6 +1112,7 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
 static int stmmac_init_phy(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
+	int mode = priv->plat->phy_interface;
 	struct fwnode_handle *phy_fwnode;
 	struct fwnode_handle *fwnode;
 	int ret;

@@ -1119,6 +1120,10 @@ static int stmmac_init_phy(struct net_device *dev)
 	if (!phylink_expects_phy(priv->phylink))
 		return 0;
 
+	if (priv->hw->xpcs &&
+	    xpcs_get_an_mode(priv->hw->xpcs, mode) == DW_AN_C73)
+		return 0;
+
 	fwnode = priv->plat->port_node;
 	if (!fwnode)
 		fwnode = dev_fwnode(priv->device);

@@ -3926,7 +3931,6 @@ static int __stmmac_open(struct net_device *dev,
 			 struct stmmac_dma_conf *dma_conf)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
-	int mode = priv->plat->phy_interface;
 	u32 chan;
 	int ret;

@@ -3934,15 +3938,12 @@ static int __stmmac_open(struct net_device *dev,
 	if (!priv->tx_lpi_timer)
 		priv->tx_lpi_timer = eee_timer * 1000;
 
-	if ((!priv->hw->xpcs ||
-	     xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) {
-		ret = stmmac_init_phy(dev);
-		if (ret) {
-			netdev_err(priv->dev,
-				   "%s: Cannot attach to PHY (error: %d)\n",
-				   __func__, ret);
-			return ret;
-		}
+	ret = stmmac_init_phy(dev);
+	if (ret) {
+		netdev_err(priv->dev,
+			   "%s: Cannot attach to PHY (error: %d)\n",
+			   __func__, ret);
+		return ret;
 	}
 
 	for (int i = 0; i < MTL_MAX_TX_QUEUES; i++)

Signature · 1 line

2.47.3

Russell King (Oracle)

2025-09-23 11:26 UTC #3 lore raw

PATCH 3/6 net: stmmac: move PHY attachment error message into stmmac_init_phy()

In reply to Russell King (Oracle)

Move the "cannot attach to PHY" error message into stmmac_init_phy() so we don't end up with multiple error messages printed when things go wrong. Drop the function name from the message, and use %pe to print the error code description rather than just a number.

Signed-off-by Russell King (Oracle) <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +5 -6

index 3b47d4ca24ca..8831bbda964c 100644
@@ -1157,7 +1157,10 @@ static int stmmac_init_phy(struct net_device *dev)
 		ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0);
 	}
 
-	if (ret == 0) {
+	if (ret) {
+		netdev_err(priv->dev, "cannot attach to PHY (error: %pe)\n",
+			   ERR_PTR(ret));
+	} else {
 		struct ethtool_keee eee;
 
 		/* Configure phylib's copy of the LPI timer. Normally,

@@ -3939,12 +3942,8 @@ static int __stmmac_open(struct net_device *dev,
 		priv->tx_lpi_timer = eee_timer * 1000;
 
 	ret = stmmac_init_phy(dev);
-	if (ret) {
-		netdev_err(priv->dev,
-			   "%s: Cannot attach to PHY (error: %d)\n",
-			   __func__, ret);
+	if (ret)
 		return ret;
-	}
 
 	for (int i = 0; i < MTL_MAX_TX_QUEUES; i++)
 		if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN)

Signature · 1 line

2.47.3

Russell King (Oracle)

2025-09-23 11:26 UTC #4 lore raw

PATCH 4/6 net: stmmac: move initialisation of priv->tx_lpi_timer to stmmac_open()

In reply to Russell King (Oracle)

The initialisation of priv->tx_lpi_timer only happens once during the lifetime of the driver, which is during the initial administrative open of the device. Move this initialisation out of __stmmac_open() into stmmac_open().

Signed-off-by Russell King (Oracle) <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +4 -4

index 8831bbda964c..4acd180d2da8 100644
@@ -3937,10 +3937,6 @@ static int __stmmac_open(struct net_device *dev,
 	u32 chan;
 	int ret;
 
-	/* Initialise the tx lpi timer, converting from msec to usec */
-	if (!priv->tx_lpi_timer)
-		priv->tx_lpi_timer = eee_timer * 1000;
-
 	ret = stmmac_init_phy(dev);
 	if (ret)
 		return ret;

@@ -4004,6 +4000,10 @@ static int stmmac_open(struct net_device *dev)
 	struct stmmac_dma_conf *dma_conf;
 	int ret;
 
+	/* Initialise the tx lpi timer, converting from msec to usec */
+	if (!priv->tx_lpi_timer)
+		priv->tx_lpi_timer = eee_timer * 1000;
+
 	dma_conf = stmmac_setup_dma_desc(priv, dev->mtu);
 	if (IS_ERR(dma_conf))
 		return PTR_ERR(dma_conf);

Signature · 1 line

2.47.3

Russell King (Oracle)

2025-09-23 11:26 UTC #5 lore raw

PATCH 5/6 net: stmmac: move PHY handling out of __stmmac_open()/release()

In reply to Russell King (Oracle)

Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.

Signed-off-by Russell King (Oracle) <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +17 ~~-12~~

index 4acd180d2da8..4844d563e291 100644
@@ -3937,10 +3937,6 @@ static int __stmmac_open(struct net_device *dev,
 	u32 chan;
 	int ret;
 
-	ret = stmmac_init_phy(dev);
-	if (ret)
-		return ret;
-
 	for (int i = 0; i < MTL_MAX_TX_QUEUES; i++)
 		if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN)
 			dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs;

@@ -3990,7 +3986,6 @@ static int __stmmac_open(struct net_device *dev,
 
 	stmmac_release_ptp(priv);
 init_error:
-	phylink_disconnect_phy(priv->phylink);
 	return ret;
 }

@@ -4010,18 +4005,28 @@ static int stmmac_open(struct net_device *dev)
 
 	ret = pm_runtime_resume_and_get(priv->device);
 	if (ret < 0)
-		goto err;
+		goto err_dma_resources;
+
+	ret = stmmac_init_phy(dev);
+	if (ret)
+		goto err_runtime_pm;
 
 	ret = __stmmac_open(dev, dma_conf);
-	if (ret) {
-		pm_runtime_put(priv->device);
-err:
-		free_dma_desc_resources(priv, dma_conf);
-	}
+	if (ret)
+		goto err_disconnect_phy;
 
 	kfree(dma_conf);
 
 	return ret;
+
+err_disconnect_phy:
+	phylink_disconnect_phy(priv->phylink);
+err_runtime_pm:
+	pm_runtime_put(priv->device);
+err_dma_resources:
+	free_dma_desc_resources(priv, dma_conf);
+	kfree(dma_conf);
+	return ret;
 }
 
 static void __stmmac_release(struct net_device *dev)

@@ -4038,7 +4043,6 @@ static void __stmmac_release(struct net_device *dev)
 
 	/* Stop and disconnect the PHY */
 	phylink_stop(priv->phylink);
-	phylink_disconnect_phy(priv->phylink);
 
 	stmmac_disable_all_queues(priv);

@@ -4078,6 +4082,7 @@ static int stmmac_release(struct net_device *dev)
 
 	__stmmac_release(dev);
 
+	phylink_disconnect_phy(priv->phylink);
 	pm_runtime_put(priv->device);
 
 	return 0;

Signature · 1 line

2.47.3

Russell King (Oracle)

2025-09-23 11:26 UTC #19 lore raw

PATCH 6/6 net: stmmac: simplify stmmac_init_phy()

In reply to Russell King (Oracle)

If we fail to attach a PHY, there is no point trying to configure WoL settings. Exit the function after printing the "cannot attach to PHY" error, and remove the now unnecessary code indentation for configuring the LPI timer in phylink. Since we know that "ret" must be zero at this point, change the final return to use a constant rather than "ret".

Signed-off-by Russell King (Oracle) <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 25 +++++++++----------
 1 file changed, 12 insertions(+), 13 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 ~~-13~~

index 4844d563e291..be064f240895 100644
@@ -1115,6 +1115,7 @@ static int stmmac_init_phy(struct net_device *dev)
 	int mode = priv->plat->phy_interface;
 	struct fwnode_handle *phy_fwnode;
 	struct fwnode_handle *fwnode;
+	struct ethtool_keee eee;
 	int ret;
 
 	if (!phylink_expects_phy(priv->phylink))

@@ -1160,19 +1161,17 @@ static int stmmac_init_phy(struct net_device *dev)
 	if (ret) {
 		netdev_err(priv->dev, "cannot attach to PHY (error: %pe)\n",
 			   ERR_PTR(ret));
-	} else {
-		struct ethtool_keee eee;
+		return ret;
+	}
 
-		/* Configure phylib's copy of the LPI timer. Normally,
-		 * phylink_config.lpi_timer_default would do this, but there is
-		 * a chance that userspace could change the eee_timer setting
-		 * via sysfs before the first open. Thus, preserve existing
-		 * behaviour.
-		 */
-		if (!phylink_ethtool_get_eee(priv->phylink, &eee)) {
-			eee.tx_lpi_timer = priv->tx_lpi_timer;
-			phylink_ethtool_set_eee(priv->phylink, &eee);
-		}
+	/* Configure phylib's copy of the LPI timer. Normally,
+	 * phylink_config.lpi_timer_default would do this, but there is a
+	 * chance that userspace could change the eee_timer setting via sysfs
+	 * before the first open. Thus, preserve existing behaviour.
+	 */
+	if (!phylink_ethtool_get_eee(priv->phylink, &eee)) {
+		eee.tx_lpi_timer = priv->tx_lpi_timer;
+		phylink_ethtool_set_eee(priv->phylink, &eee);
 	}
 
 	if (!priv->plat->pmt) {

@@ -1183,7 +1182,7 @@ static int stmmac_init_phy(struct net_device *dev)
 		device_set_wakeup_enable(priv->device, !!wol.wolopts);
 	}
 
-	return ret;
+	return 0;
 }
 
 static int stmmac_phy_setup(struct stmmac_priv *priv)

Signature · 1 line

2.47.3

Russell King (Oracle)

author

2025-09-23 11:31 UTC #20 lore raw

In reply to Russell King (Oracle)

Quoted 19 lines · Russell King (Oracle)

Building on the previous cleanup series, this cleans up yet more stmmac code.
- Move stmmac_bus_clks_config() into stmmac_platform() which is where its onlny user is.
- Move the xpcs Clause 73 test into stmmac_init_phy(), resulting in simpler code in __stmmac_open().
- Move "can't attach PHY" error message into stmmac_init_phy().
We then start moving stuff out of __stmac_open() into stmmac_open() (and correspondingly __stmmac_release() into stmmac_release()) which is not necessary when re-initialising the interface on e.g. MTU change.
- Move initialisation of tx_lpi_timer
- Move PHY attachment/detachment
- Move PHY error message into stmmac_init_phy()

Quoted 6 lines

Finally, simplfy the paths in stmmac_init_phy().

 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 111 ++++++++-------------
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  32 ++++++
 3 files changed, 73 insertions(+), 71 deletions(-)

Should've added: tested on nVidia Jetson Xavier NX.

However, observed a failure changing the MTU with the link down - our old friend, failure to complete the DMA reset.

Once that's been triggered, taking the interface down or changing the MTU again results in more problems, with the thread spinning in napi_disable_locked() with RTNL held (as we effectively end up calling napi_disable() twice on the same napi struct.)

This basically makes the platforms networking unusable - and needs to be hard-reset.

These issues pre-exist all my cleanups.

Signature · 2 lines

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

Maxime Chevallier

maintainer Reviewed-by on the v1 series

2025-09-23 14:10 UTC #21 lore raw

In reply to Russell King (Oracle)

Hi Russell,

Quoted 19 lines · Russell King (Oracle)

Building on the previous cleanup series, this cleans up yet more stmmac code.
- Move stmmac_bus_clks_config() into stmmac_platform() which is where its onlny user is.
- Move the xpcs Clause 73 test into stmmac_init_phy(), resulting in simpler code in __stmmac_open().
- Move "can't attach PHY" error message into stmmac_init_phy().
We then start moving stuff out of __stmac_open() into stmmac_open() (and correspondingly __stmmac_release() into stmmac_release()) which is not necessary when re-initialising the interface on e.g. MTU change.
- Move initialisation of tx_lpi_timer
- Move PHY attachment/detachment
- Move PHY error message into stmmac_init_phy()

Quoted 7 lines

Finally, simplfy the paths in stmmac_init_phy().

  drivers/net/ethernet/stmicro/stmmac/stmmac.h       |   1 -
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 111 ++++++++-------------
  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  32 ++++++
  3 files changed, 73 insertions(+), 71 deletions(-)

For the series,

Reviewed-by Maxime Chevallier <[email protected]>

Thanks for the cleanup,

Maxime

[email protected]

CI Applied

2025-09-25 00:50 UTC #22 lore raw

In reply to Russell King (Oracle)

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Tue, 23 Sep 2025 12:25:30 +0100 you wrote:

Building on the previous cleanup series, this cleans up yet more stmmac code.
- Move stmmac_bus_clks_config() into stmmac_platform() which is where its onlny user is.
- Move the xpcs Clause 73 test into stmmac_init_phy(), resulting in simpler code in __stmmac_open().
[...]

Here is the summary with links:
- [net-next,1/6] net: stmmac: move stmmac_bus_clks_config() to stmmac_platform.c

    https://git.kernel.org/netdev/net-next/c/79d6e14e9cb3
  - [net-next,2/6] net: stmmac: move xpcs clause 73 test into stmmac_init_phy()
    https://git.kernel.org/netdev/net-next/c/f005ec4a3d6b
  - [net-next,3/6] net: stmmac: move PHY attachment error message into stmmac_init_phy()
    https://git.kernel.org/netdev/net-next/c/9641d727162d
  - [net-next,4/6] net: stmmac: move initialisation of priv->tx_lpi_timer to stmmac_open()
    https://git.kernel.org/netdev/net-next/c/bae62989a31b
  - [net-next,5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()
    https://git.kernel.org/netdev/net-next/c/db299a0c09e9
  - [net-next,6/6] net: stmmac: simplify stmmac_init_phy()
    https://git.kernel.org/netdev/net-next/c/50acea3662bf

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Alexander Stein

2026-04-15 06:08 UTC #6 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

In reply to Russell King (Oracle)

Hi,

Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):

Quoted 5 lines

Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.

Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:

$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
imx-dwmac 30bf0000.ethernet end1: __stmmac_open: Hw setup failed
imx-dwmac 30bf0000.ethernet end1: failed reopening the interface after MTU change

Using the command above bisecting was straight forward.
For some reason detach and re-attaching the PHY seems necessary on this platform. There already too much changes for simply reverting this commit.

Best regards,
Alexander

Quoted 81 lines

Signed-off-by Russell King (Oracle) <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +17 ~~-12~~

index 4acd180d2da8..4844d563e291 100644
@@ -3937,10 +3937,6 @@ static int __stmmac_open(struct net_device *dev,
 	u32 chan;
 	int ret;
 
-	ret = stmmac_init_phy(dev);
-	if (ret)
-		return ret;
-
 	for (int i = 0; i < MTL_MAX_TX_QUEUES; i++)
 		if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN)
 			dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs;

@@ -3990,7 +3986,6 @@ static int __stmmac_open(struct net_device *dev,
 
 	stmmac_release_ptp(priv);
 init_error:
-	phylink_disconnect_phy(priv->phylink);
 	return ret;
 }

@@ -4010,18 +4005,28 @@ static int stmmac_open(struct net_device *dev)
 
 	ret = pm_runtime_resume_and_get(priv->device);
 	if (ret < 0)
-		goto err;
+		goto err_dma_resources;
+
+	ret = stmmac_init_phy(dev);
+	if (ret)
+		goto err_runtime_pm;
 
 	ret = __stmmac_open(dev, dma_conf);
-	if (ret) {
-		pm_runtime_put(priv->device);
-err:
-		free_dma_desc_resources(priv, dma_conf);
-	}
+	if (ret)
+		goto err_disconnect_phy;
 
 	kfree(dma_conf);
 
 	return ret;
+
+err_disconnect_phy:
+	phylink_disconnect_phy(priv->phylink);
+err_runtime_pm:
+	pm_runtime_put(priv->device);
+err_dma_resources:
+	free_dma_desc_resources(priv, dma_conf);
+	kfree(dma_conf);
+	return ret;
 }
 
 static void __stmmac_release(struct net_device *dev)

@@ -4038,7 +4043,6 @@ static void __stmmac_release(struct net_device *dev)
 
 	/* Stop and disconnect the PHY */
 	phylink_stop(priv->phylink);
-	phylink_disconnect_phy(priv->phylink);
 
 	stmmac_disable_all_queues(priv);

@@ -4078,6 +4082,7 @@ static int stmmac_release(struct net_device *dev)
 
 	__stmmac_release(dev);
 
+	phylink_disconnect_phy(priv->phylink);
 	pm_runtime_put(priv->device);
 
 	return 0;

Signature · 4 lines

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Russell King (Oracle)

author

2026-04-15 12:59 UTC #7 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 11 lines · Alexander Stein

Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:

Quoted 9 lines

$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed

This basically means that a clock is missing. Please provide more information:

- what kernel version are you using?
- has EEE been negotiated?
- does the problem persist when EEE is disabled?
- which PHY is attached to stmmac?
- which PHY interface mode is being used to connect the PHY to stmmac?

Thanks.

Signature · 2 lines

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

Alexander Stein

2026-04-16 06:20 UTC #8 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):

Quoted 22 lines · Alexander Stein

Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed

Quoted 4 lines

This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?

Currently I am using v6.18.22.

$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no

Quoted 1 line

- has EEE been negotiated?

No. It is marked as not supported

$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported

Quoted 1 line

- does the problem persist when EEE is disabled?

As EEE is not supported the problem occurs even with EEE disabled.

Quoted 1 line

- which PHY is attached to stmmac?

It is a TI DP83867.

imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)

Quoted 1 line

- which PHY interface mode is being used to connect the PHY to stmmac?

For this interface

Quoted 1 line

phy-mode = "rgmii-id";

is set.

In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.

Thanks and best regards
Alexander

Signature · 4 lines

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Russell King (Oracle)

author

2026-04-16 10:49 UTC #9 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 59 lines · Alexander Stein

Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)

Quoted 8 lines

Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.

Thanks.

So, as best I can determine at the moment, we end up with the following sequence:

stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)

With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.

Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)

However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.

As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.

Thanks.

Signature · 2 lines

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

Alexander Stein

2026-04-16 12:02 UTC #10 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Hi Russel,

Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):

Quoted 99 lines · Alexander Stein

Quoted 59 lines
Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
Quoted 8 lines
Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.
Thanks.
So, as best I can determine at the moment, we end up with the following sequence:
stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.
Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)
However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.

Quoted 4 lines

As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.

I tried the following patch. This works as you suspected.

Thanks and best regards
Alexander

---8<---

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -0

@@ -5875,6 +5875,8 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
 
                __stmmac_release(dev);
 
+               phylink_prepare_resume(priv->phylink);
+
                ret = __stmmac_open(dev, dma_conf);
                if (ret) {
                        free_dma_desc_resources(priv, dma_conf);

---8<---

Signature · 4 lines

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Russell King (Oracle)

author

2026-04-16 12:13 UTC #11 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 102 lines · Alexander Stein

Hi Russel,
Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):
Quoted 99 lines · Alexander Stein
Quoted 59 lines
Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
Quoted 8 lines
Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.
Thanks.
So, as best I can determine at the moment, we end up with the following sequence:
stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.
Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)
However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.

Quoted 6 lines

Quoted 4 lines
As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.
I tried the following patch. This works as you suspected.

Brilliant, thanks for proving the theory why it broke.

I'll have a think about the best way to solve this, because phylink_prepare_resume() is supposed to be paired with phylink_resume() and that isn't the case here.

Please bear with me as my availability for looking at the kernel is very unpredictable at present (family health issues.)

Signature · 2 lines

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

Russell King (Oracle)

author

2026-04-16 13:48 UTC #12 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 110 lines · Russell King (Oracle)

Quoted 102 lines · Alexander Stein
Hi Russel,
Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):
Quoted 99 lines · Alexander Stein
Quoted 59 lines
Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
Quoted 8 lines
Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.
Thanks.
So, as best I can determine at the moment, we end up with the following sequence:
stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.
Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)
However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.
Quoted 6 lines
Quoted 4 lines
As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.
I tried the following patch. This works as you suspected.

Quoted 8 lines

Brilliant, thanks for proving the theory why it broke.
I'll have a think about the best way to solve this, because phylink_prepare_resume() is supposed to be paired with phylink_resume() and that isn't the case here.
Please bear with me as my availability for looking at the kernel is very unpredictable at present (family health issues.)

I have some patches which passed build testing, but more chaos means I can't post them nor test them. I'll do something when I'm next able to, whenever that will be.

The next problem will be netdev's policy over reviews vs patches balance which I'm already in deficit, and I have *NO* *TIME* what so ever to review patches - let alone propose patches to fix people's problems.

So I'm going to say this plainly: if netdev wants to enforce that rule, then I won't be fixing people's problems.

Signature · 2 lines

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

Jakub Kicinski

maintainer

2026-04-16 16:08 UTC #13 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 7 lines · On Thu, 16 Apr 2026 14:47:57 +0100 Russell King (Oracle) wrote:

The next problem will be netdev's policy over reviews vs patches balance which I'm already in deficit, and I have *NO* *TIME* what so ever to review patches - let alone propose patches to fix people's problems.
So I'm going to say this plainly: if netdev wants to enforce that rule, then I won't be fixing people's problems.

Do you have a better proposal?
I'm under the same pressure of million stupid projects from my employer as you are. Do y'all think that upstream maintainers have time given by their employers to do the reviews? SMH.

Russell King (Oracle)

author

2026-04-16 19:46 UTC #14 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 6 lines · Jakub Kicinski

Quoted 5 lines · On Thu, 16 Apr 2026 14:47:57 +0100 Russell King (Oracle) wrote:
The next problem will be netdev's policy over reviews vs patches balance which I'm already in deficit, and I have *NO* *TIME* what so ever to review patches - let alone propose patches to fix people's problems.

Quoted 7 lines

Quoted 2 lines
So I'm going to say this plainly: if netdev wants to enforce that rule, then I won't be fixing people's problems.
Do you have a better proposal?
I'm under the same pressure of million stupid projects from my employer as you are. Do y'all think that upstream maintainers have time given by their employers to do the reviews? SMH.

Are you really under the same pressure? I have one of my parents in hospital right now, and was in A&E yesterday afternoon through into the evening. I've been down at the hospital since 2pm today, only just come back to feed the other parent and head back down for what could be a long night. Then there's supposed to be an appointment that will take up to 3 hours tomorrow morning...

Yea, I'm sure you have the same pressures and worry from your employer - except my pressures are medical, looking after my parents.

Thank you for your lack of understanding.

Signature · 2 lines

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

Jakub Kicinski

maintainer

2026-04-16 20:16 UTC #15 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 22 lines · On Thu, 16 Apr 2026 20:46:34 +0100 Russell King (Oracle) wrote:

Quoted 6 lines · Jakub Kicinski
Quoted 5 lines · On Thu, 16 Apr 2026 14:47:57 +0100 Russell King (Oracle) wrote:
The next problem will be netdev's policy over reviews vs patches balance which I'm already in deficit, and I have *NO* *TIME* what so ever to review patches - let alone propose patches to fix people's problems.
Quoted 7 lines
Quoted 2 lines
So I'm going to say this plainly: if netdev wants to enforce that rule, then I won't be fixing people's problems.
Do you have a better proposal?
I'm under the same pressure of million stupid projects from my employer as you are. Do y'all think that upstream maintainers have time given by their employers to do the reviews? SMH.
Are you really under the same pressure? I have one of my parents in hospital right now, and was in A&E yesterday afternoon through into the evening. I've been down at the hospital since 2pm today, only just come back to feed the other parent and head back down for what could be a long night. Then there's supposed to be an appointment that will take up to 3 hours tomorrow morning...

Quoted 4 lines

Yea, I'm sure you have the same pressures and worry from your employer - except my pressures are medical, looking after my parents.
Thank you for your lack of understanding.

Not my point. Sorry to hear about the issues you're facing.

I don't think making vague complaints about the development process is going to make anything better.

Maxime Chevallier

maintainer

2026-04-17 07:12 UTC #16 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

In reply to Russell King (Oracle)

Hi,

Quoted 110 lines · Russell King (Oracle)

Quoted 102 lines · Alexander Stein
Hi Russel,
Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):
Quoted 99 lines · Alexander Stein
Quoted 59 lines
Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
Quoted 8 lines
Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.
Thanks.
So, as best I can determine at the moment, we end up with the following sequence:
stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.
Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)
However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.
Quoted 6 lines
Quoted 4 lines
As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.
I tried the following patch. This works as you suspected.

Quoted 8 lines

Brilliant, thanks for proving the theory why it broke.
I'll have a think about the best way to solve this, because phylink_prepare_resume() is supposed to be paired with phylink_resume() and that isn't the case here.
Please bear with me as my availability for looking at the kernel is very unpredictable at present (family health issues.)

FWIW I am able to reproduce this with imx8mp + ksz9131

I can give this a try as Russell isn't available.

Maxime

Quoted 1 line

Alexander Stein

2026-04-17 08:47 UTC #17 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Am Freitag, 17. April 2026, 09:11:59 CEST schrieb Maxime Chevallier:

Quoted 119 lines

Hi,
Quoted 116 lines · Russell King (Oracle)
Quoted 102 lines · Alexander Stein
Hi Russel,
Am Donnerstag, 16. April 2026, 12:49:25 CEST schrieb Russell King (Oracle):
Quoted 99 lines · Alexander Stein
Quoted 59 lines
Am Mittwoch, 15. April 2026, 14:59:32 CEST schrieb Russell King (Oracle):
Quoted 22 lines · Alexander Stein
Quoted 11 lines
Hi,
Am Dienstag, 23. September 2025, 13:26:19 CEST schrieb Russell King (Oracle):
Quoted 5 lines
Move the PHY attachment/detachment from the network driver out of __stmmac_open() and __stmmac_release() into stmmac_open() and stmmac_release() where these actions will only happen when the interface is administratively brought up or down. It does not make sense to detach and re-attach the PHY during a change of MTU.
Sorry for coming up now. But I recently noticed this commit breaks changing the MTU on i.MX8MP. Once I simply change the MTU I run into some DMA error:
Quoted 9 lines
$ ip link set dev end1 mtu 1400
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-0
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-1
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-2
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-3
imx-dwmac 30bf0000.ethernet end1: Register MEM_TYPE_PAGE_POOL RxQ-4
imx-dwmac 30bf0000.ethernet end1: Link is Down
imx-dwmac 30bf0000.ethernet end1: Failed to reset the dma
imx-dwmac 30bf0000.ethernet end1: stmmac_hw_setup: DMA engine initialization failed
Quoted 4 lines
This basically means that a clock is missing. Please provide more information:
- what kernel version are you using?
Currently I am using v6.18.22.
$ ethtool -i end1
driver: st_gmac
version: 6.18.22
firmware-version: 
expansion-rom-version: 
bus-info: 30bf0000.ethernet
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: yes
supports-priv-flags: no
Quoted 1 line
- has EEE been negotiated?
No. It is marked as not supported
$ ethtool --show-eee end1
EEE settings for end1:
        EEE status: not supported
Quoted 1 line
- does the problem persist when EEE is disabled?
As EEE is not supported the problem occurs even with EEE disabled.
Quoted 1 line
- which PHY is attached to stmmac?
It is a TI DP83867.
imx-dwmac 30bf0000.ethernet eth1: PHY [stmmac-1:03] driver [TI DP83867] (irq=136)
Quoted 8 lines
Quoted 1 line
- which PHY interface mode is being used to connect the PHY to stmmac?
For this interface
Quoted 1 line
phy-mode = "rgmii-id";
is set.
In case it is helpful. My platform is arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts Thanks for assisting. If there a further questions, don't hesitate to ask.
Thanks.
So, as best I can determine at the moment, we end up with the following sequence:
stmmac_change_mtu()
__stmmac_release()
phylink_stop()
phy_stop()
phy->state = PHY_HALTED _phy_state_machine() returns PHY_STATE_WORK_SUSPEND _phy_state_machine_post_work()
phy_suspend()
genphy_suspend()
phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN)
With the DP83867, this causes most of the PHY to be powered down, thus stopping the clocks, and this causes the stmmac reset to time out.
Prior to this commit, we would have called phylink_disconnect_phy() immediately after phylink_stop(), but I can see nothing that would be affected by this change there (since that also calls phy_suspend(), but as the PHY is already suspended, this becomes a no-op.)
However, __stmmac_open() would have called stmmac_init_phy(), which would reattach the PHY. This would have called phy_init_hw(), resetting the PHY, and phy_resume() which would ensure that the PDOWN bit is clear - thus clocks would be running.
Quoted 6 lines
Quoted 4 lines
As a hack, please can you try calling phylink_prepare_resume() between the __stmmac_release() and __stmmac_open() in stmmac_change_mtu(). This should resume the PHY, thus restoring the clocks necessary for stmmac to reset.
I tried the following patch. This works as you suspected.
Brilliant, thanks for proving the theory why it broke.
I'll have a think about the best way to solve this, because phylink_prepare_resume() is supposed to be paired with phylink_resume() and that isn't the case here.

Quoted 6 lines

Quoted 2 lines
Please bear with me as my availability for looking at the kernel is very unpredictable at present (family health issues.)
FWIW I am able to reproduce this with imx8mp + ksz9131
I can give this a try as Russell isn't available.

Thanks for conforming this for another PHY. What I'm wondering right now:
Why is the PHY stopped in the first place? We are just changing the MTU, no? This has no effect on the PHY itself. "all" we need to do is reconfiguring the DMA. I have a proof of concept running, but it needs more cleanup due to code duplication.

Best regards
Alexander

Signature · 4 lines

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Andrew Lunn

2026-04-17 17:30 UTC #18 lore raw

Re: [PATCH net-next 5/6] net: stmmac: move PHY handling out of __stmmac_open()/release()

Quoted 2 lines

Thanks for conforming this for another PHY. What I'm wondering right now:
Why is the PHY stopped in the first place? We are just changing the MTU, no?

It is not too uncommon to see an MTU change destroying everything and rebuilding it, especially when it was retrofitted into an older driver which had fixed MTU.

Quoted 2 lines

I have a proof of concept running, but it needs more cleanup due to code duplication.

You probably also want to take a look at the ethtool code for configuring rings. Oh, it is even worse:

int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size)

{
        struct stmmac_priv *priv = netdev_priv(dev);
        int ret = 0;

        if (netif_running(dev))
                stmmac_release(dev);

        priv->dma_conf.dma_rx_size = rx_size;
        priv->dma_conf.dma_tx_size = tx_size;

        if (netif_running(dev))
                ret = stmmac_open(dev);

        return ret;
}

So not even using __stmmac_release() or __stmmac_open(), and leaving you with a dead interface if there is not enough memory to allocate the rings.

These paths should really share the same code.

Andrew