[PATCH net v3 0/2] net: bcmgenet: fixes for a failed resume and the ring rebuild

[PATCH net v3 0/2] net: bcmgenet: fixes for a failed resume and the ring rebuild

[Nicolai Buchwitz]

This is v3 of my resume and ring fixes for genet.

Mostly findings sashiko raised during my recent review sessions.

Patch 1 is a resume that fails in bcmgenet_init_dma(), which leaves the
interface up with no datapath and the next ifdown hanging in
napi_disable() with RTNL held. It is one patch because nothing that
resume leaves behind is reachable on its own, the second suspend never
returns until the hang is fixed.

Patch 2 (was patch 4 in v1) is the Tx coalescing threshold going back
to the driver default whenever the rings are rebuilt.

Patch 1 fixes the rebuild path, not the root cause. bcmasp for example
parks the PHY in its WoL suspend and has one deinit with a flag, which
looks like the direction for genet too. But one step at a time :)

It is also prep for the next version of my MTU series [1], which
drops the "hallmark of a bad design" flag in favour of the ring pointers
it clears.

[1] https://patch.msgid.link/20261013-nb-genet-mtu-nn-v2-v5-0-076b6fc240be@tipi-net.de

For backports: the WoL hunks in patch 1 need 254f3239dd07, on older trees
they can be dropped. The rest should apply on its own.

Tested on BCM2711 (Raspberry Pi CM4) with the resume allocation forced to
fail. BCM2711 has no WoL irq, so I had to force the capability.
With this a magic packet wakes the board and the interface
recovers on the next suspend and resume. Thanks to Florian for pointing
out that s2idle can do this at all.

WoL doesn't wake the board after a failed resume though: as sashiko
pointed out init_umac() left UMAC_CMD at 10 Mb/s and only adjust_link puts
it back, so it arms fine and then misses the packet. But this is
pre-existing, arguably even better since it no longer hangs the system
like before. IMHO the proper fix is to let resume allocate before it
touches the hardware ... I'll do that after the MTU series.

Florian, Justin: if you have a board where the WoL irq is actually
wired, I'd appreciate a run on it too.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
Changes in v3:
- Arm only the configured wake filters instead of turning HFB off (sashiko)
- Share that selection with the normal WoL suspend path
- Fix the clock wording in patch 1 (sashiko)
- Tested the WoL paths on a CM4, see above
- Link to v2: https://patch.msgid.link/20261019-nb-genet-fini-dma-net-v2-0-b85f6b92654a@tipi-net.de

Changes in v2:
- Squashed v1 patches 1 to 3 into one, each was either a no-op or left a
  broken tree on its own
- Turn HFB off on the suspend early return, it stayed armed as a wake
  source (sashiko)
- Second Fixes tag for the WoL hunks
- Let PHY_ERROR reach phy_stop() too (sashiko)
- Link to v1: https://patch.msgid.link/20261017-nb-genet-fini-dma-net-v1-0-47e7c57f6834@tipi-net.de

To: Doug Berger <opendmb@gmail.com>
To: Florian Fainelli <florian.fainelli@broadcom.com>
To: Nicolai Buchwitz <nb@tipi-net.de>
To: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Nicolai Buchwitz (2):
      net: bcmgenet: recover from a failed resume
      net: bcmgenet: restore the Tx coalescing setting when the rings are rebuilt

 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 102 +++++++++++++++++--------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |   1 +
 2 files changed, 71 insertions(+), 32 deletions(-)
---
base-commit: 82e31923c0553fdbfcddcd910ca07952880ca9b9
change-id: 20261015-nb-genet-fini-dma-net-62a39ee14e65

Best regards,
--  
Nicolai Buchwitz <nb@tipi-net.de>

[PATCH net v3 1/2] net: bcmgenet: recover from a failed resume

[Nicolai Buchwitz]

A resume that fails in bcmgenet_init_dma() returns with the interface
still IFF_UP and the datapath gone. The next ifdown runs the teardown
a second time and napi_disable() waits for a NAPI_STATE_SCHED bit that
nothing will clear:

  task:ip  state:D
    usleep_range_state+0xec/0x158
    napi_disable_locked+0x254/0x318
    napi_disable+0x3c/0x68
    bcmgenet_netif_stop+0x1c8/0x318
    bcmgenet_close+0x24/0xd0
    __dev_close_many+0x18c/0x310

RTNL is held across the wait, so everything else blocks behind it.

The same resume gates the clock and detaches the device. Close releases
the clock a second time and ifup fails with -ENODEV. The hardware filters
stay armed, so the next WoL suspend wakes on any frame. That suspend sets
CMD_RX_EN for magic packet detection, so the WoL resume reuses rings that
are gone.

A failed resume should leave a state the ordinary paths can handle. Clear
rx_cbs and tx_cbs as they are freed so they answer whether a datapath
exists, and leave the clock alone so close and suspend release it once.
The WoL resume rebuilds instead, so stop the PHY first, phy_start() only
takes PHY_READY or PHY_HALTED.

Flagged in a sashiko review and reproduced on a CM4 by forcing the resume
allocation to fail.

Fixes: b6e978e50444 ("net: bcmgenet: add suspend/resume callbacks")
Fixes: 254f3239dd07 ("net: bcmgenet: revise suspend/resume")	# for the WoL parts
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 92 +++++++++++++++++---------
 1 file changed, 62 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index f4cfbff49a1b8..e7dec73d5b1c0 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3117,7 +3117,9 @@ static void bcmgenet_fini_dma(struct bcmgenet_priv *priv)
 	bcmgenet_free_rx_buffers(priv);
 	bcmgenet_destroy_rx_page_pools(priv);
 	kfree(priv->rx_cbs);
+	priv->rx_cbs = NULL;
 	kfree(priv->tx_cbs);
+	priv->tx_cbs = NULL;
 }
 
 /* init_edma: Initialize DMA control register */
@@ -3175,6 +3177,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv, bool flush_rx)
 	priv->tx_cbs = kzalloc_objs(struct enet_cb, priv->num_tx_bds);
 	if (!priv->tx_cbs) {
 		kfree(priv->rx_cbs);
+		priv->rx_cbs = NULL;
 		return -ENOMEM;
 	}
 
@@ -3194,7 +3197,9 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv, bool flush_rx)
 		bcmgenet_free_rx_buffers(priv);
 		bcmgenet_destroy_rx_page_pools(priv);
 		kfree(priv->rx_cbs);
+		priv->rx_cbs = NULL;
 		kfree(priv->tx_cbs);
+		priv->tx_cbs = NULL;
 		return ret;
 	}
 
@@ -3503,7 +3508,9 @@ static int bcmgenet_close(struct net_device *dev)
 
 	netif_dbg(priv, ifdown, dev, "bcmgenet_close\n");
 
-	bcmgenet_netif_stop(dev, false);
+	/* A failed resume has torn the datapath down already */
+	if (priv->tx_cbs)
+		bcmgenet_netif_stop(dev, false);
 
 	/* Really kill the PHY state machine and disconnect from it */
 	phy_disconnect(dev->phydev);
@@ -3511,6 +3518,9 @@ static int bcmgenet_close(struct net_device *dev)
 	free_irq(priv->irq0, priv);
 	free_irq(priv->irq1, priv);
 
+	/* A failed resume left the device detached, __dev_open() refuses that */
+	netif_device_attach(dev);
+
 	if (priv->internal_phy)
 		ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
 
@@ -4268,7 +4278,11 @@ static int bcmgenet_resume(struct device *d)
 
 	if (device_may_wakeup(d) && priv->wolopts) {
 		reg = bcmgenet_umac_readl(priv, UMAC_CMD);
-		if (reg & CMD_RX_EN) {
+
+		/* The WoL power down sets CMD_RX_EN itself, so the bit
+		 * alone does not mean the rings survived
+		 */
+		if (priv->tx_cbs && (reg & CMD_RX_EN)) {
 			/* Successfully exited WoL, just resume data flows */
 			list_for_each_entry(rule, &priv->rxnfc_list, list)
 				if (rule->state == BCMGENET_RXNFC_STATE_ENABLED)
@@ -4298,6 +4312,9 @@ static int bcmgenet_resume(struct device *d)
 		bcmgenet_rdma_disable(priv);
 		bcmgenet_intr_disable(priv);
 		bcmgenet_fini_dma(priv);
+		if (phy_is_started(dev->phydev) ||
+		    dev->phydev->state == PHY_ERROR)
+			phy_stop(dev->phydev);
 		enable_irq(priv->irq1);
 	}
 
@@ -4320,8 +4337,9 @@ static int bcmgenet_resume(struct device *d)
 	/* Reinitialize TDMA and RDMA and SW housekeeping */
 	ret = bcmgenet_init_dma(priv, false);
 	if (ret) {
+		/* Leave the block clocked, close and suspend tear it down */
 		netdev_err(dev, "failed to initialize DMA\n");
-		goto out_clk_disable;
+		return ret;
 	}
 
 	if (!device_may_wakeup(d))
@@ -4332,24 +4350,57 @@ static int bcmgenet_resume(struct device *d)
 	netif_device_attach(dev);
 
 	return 0;
+}
 
-out_clk_disable:
-	if (priv->internal_phy)
-		bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
-	clk_disable_unprepare(priv->clk);
-	return ret;
+/* Arm the configured wake filters and drop the rest, then clear old
+ * matches so that only a new one wakes the system
+ */
+static void bcmgenet_wol_arm_filters(struct bcmgenet_priv *priv)
+{
+	struct bcmgenet_rxnfc_rule *rule;
+	u32 reg, hfb_enable = 0;
+
+	if (priv->wolopts & WAKE_FILTER)
+		list_for_each_entry(rule, &priv->rxnfc_list, list)
+			if (rule->fs.ring_cookie == RX_CLS_FLOW_WAKE &&
+			    rule->state == BCMGENET_RXNFC_STATE_ENABLED)
+				hfb_enable |= 1 << rule->fs.location;
+
+	reg = bcmgenet_hfb_reg_readl(priv, HFB_CTRL);
+	if (GENET_IS_V1(priv) || GENET_IS_V2(priv)) {
+		reg &= ~RBUF_HFB_FILTER_EN_MASK;
+		reg |= hfb_enable << (RBUF_HFB_FILTER_EN_SHIFT + 1);
+	} else {
+		bcmgenet_hfb_reg_writel(priv, hfb_enable << 1,
+					HFB_FLT_ENABLE_V3PLUS + 4);
+	}
+	if (!hfb_enable)
+		reg &= ~RBUF_HFB_EN;
+	bcmgenet_hfb_reg_writel(priv, reg, HFB_CTRL);
+
+	bcmgenet_intrl2_0_writel(priv, 0xFFFFFFFF, INTRL2_CPU_MASK_SET);
+	bcmgenet_intrl2_0_writel(priv, 0xFFFFFFFF, INTRL2_CPU_CLEAR);
 }
 
 static int bcmgenet_suspend(struct device *d)
 {
 	struct net_device *dev = dev_get_drvdata(d);
 	struct bcmgenet_priv *priv = netdev_priv(dev);
-	struct bcmgenet_rxnfc_rule *rule;
-	u32 reg, hfb_enable = 0;
 
 	if (!netif_running(dev))
 		return 0;
 
+	/* A failed resume has torn the datapath down already. Resume still
+	 * enables irq1 on the WoL path, so it has to be disabled here.
+	 */
+	if (!priv->tx_cbs) {
+		if (device_may_wakeup(d) && priv->wolopts) {
+			bcmgenet_wol_arm_filters(priv);
+			disable_irq(priv->irq1);
+		}
+		return 0;
+	}
+
 	netif_device_detach(dev);
 
 	if (device_may_wakeup(d) && priv->wolopts) {
@@ -4358,26 +4409,7 @@ static int bcmgenet_suspend(struct device *d)
 		netif_tx_disable(dev);
 
 		/* Suspend non-wake Rx data flows */
-		if (priv->wolopts & WAKE_FILTER)
-			list_for_each_entry(rule, &priv->rxnfc_list, list)
-				if (rule->fs.ring_cookie == RX_CLS_FLOW_WAKE &&
-				    rule->state == BCMGENET_RXNFC_STATE_ENABLED)
-					hfb_enable |= 1 << rule->fs.location;
-		reg = bcmgenet_hfb_reg_readl(priv, HFB_CTRL);
-		if (GENET_IS_V1(priv) || GENET_IS_V2(priv)) {
-			reg &= ~RBUF_HFB_FILTER_EN_MASK;
-			reg |= hfb_enable << (RBUF_HFB_FILTER_EN_SHIFT + 1);
-		} else {
-			bcmgenet_hfb_reg_writel(priv, hfb_enable << 1,
-						HFB_FLT_ENABLE_V3PLUS + 4);
-		}
-		if (!hfb_enable)
-			reg &= ~RBUF_HFB_EN;
-		bcmgenet_hfb_reg_writel(priv, reg, HFB_CTRL);
-
-		/* Clear any old filter matches so only new matches wake */
-		bcmgenet_intrl2_0_writel(priv, 0xFFFFFFFF, INTRL2_CPU_MASK_SET);
-		bcmgenet_intrl2_0_writel(priv, 0xFFFFFFFF, INTRL2_CPU_CLEAR);
+		bcmgenet_wol_arm_filters(priv);
 
 		if (-ETIMEDOUT == bcmgenet_tdma_disable(priv))
 			netdev_warn(priv->dev,

-- 
2.53.0

[PATCH net v3 2/2] net: bcmgenet: restore the Tx coalescing setting when the rings are rebuilt

[Nicolai Buchwitz]

bcmgenet_set_coalesce() writes tx_max_coalesced_frames straight into
DMA_MBUF_DONE_THRESH and keeps no copy of it, while
bcmgenet_init_tx_ring() programs a fixed 1. An ifdown and ifup or a resume
then puts the Tx coalescing back to one frame per interrupt.

Rx keeps its value per ring and reapplies it on init. Do the same for Tx.

Fixes: 2f9130709d2c ("net: bcmgenet: Implement TX coalescing control knobs")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++++--
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e7dec73d5b1c0..f258997f20a9f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -931,10 +931,13 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
 	/* Program all TX queues with the same values, as there is no
 	 * ethtool knob to do coalescing on a per-queue basis
 	 */
-	for (i = 0; i <= priv->hw_params->tx_queues; i++)
+	for (i = 0; i <= priv->hw_params->tx_queues; i++) {
+		priv->tx_rings[i].tx_max_coalesced_frames =
+			ec->tx_max_coalesced_frames;
 		bcmgenet_tdma_ring_writel(priv, i,
 					  ec->tx_max_coalesced_frames,
 					  DMA_MBUF_DONE_THRESH);
+	}
 
 	for (i = 0; i <= priv->hw_params->rx_queues; i++)
 		bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[i], ec);
@@ -2758,7 +2761,8 @@ static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_PROD_INDEX);
 	bcmgenet_tdma_ring_writel(priv, index, 0, TDMA_CONS_INDEX);
-	bcmgenet_tdma_ring_writel(priv, index, 1, DMA_MBUF_DONE_THRESH);
+	bcmgenet_tdma_ring_writel(priv, index, ring->tx_max_coalesced_frames,
+				  DMA_MBUF_DONE_THRESH);
 	/* Disable rate control for now */
 	bcmgenet_tdma_ring_writel(priv, index, flow_period_val,
 				  TDMA_FLOW_PERIOD);
@@ -4176,6 +4180,8 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		priv->rx_rings[i].index = i;
 		priv->rx_rings[i].rx_max_coalesced_frames = 1;
 	}
+	for (i = 0; i <= priv->hw_params->tx_queues; i++)
+		priv->tx_rings[i].tx_max_coalesced_frames = 1;
 
 	/* Initialize u64 stats seq counter for 32bit machines */
 	for (i = 0; i <= GENET_MAX_MQ_CNT; i++) {
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 86f2aed20dbe8..e48ce39d3e7c8 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -553,6 +553,7 @@ struct bcmgenet_tx_ring {
 	unsigned int	prod_index;	/* Tx ring producer index SW copy */
 	unsigned int	cb_ptr;		/* Tx ring initial CB ptr */
 	unsigned int	end_ptr;	/* Tx ring end CB ptr */
+	unsigned int	tx_max_coalesced_frames;
 	struct bcmgenet_priv *priv;
 };
 

-- 
2.53.0

Re: [PATCH net v3 1/2] net: bcmgenet: recover from a failed resume

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] After a failed non-WoL resume, the next non-WoL system suspend
  leaves the external PHY powered and linked.

--

net: bcmgenet: recover from a failed resume

This commit lets bcmgenet cope with a resume that fails in
bcmgenet_init_dma(). The driver now sets rx_cbs and tx_cbs to NULL when
it frees them, so they show whether a datapath exists, and the failure
path no longer disables the clock. Close, suspend and the WoL resume now
check that state, so they skip the datapath teardown that was already
done.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index f4cfbff49a1b..e7dec73d5b1c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c

[ ... ]

> @@ -4320,8 +4337,9 @@ static int bcmgenet_resume(struct device *d)
>  	/* Reinitialize TDMA and RDMA and SW housekeeping */
>  	ret = bcmgenet_init_dma(priv, false);
>  	if (ret) {
> +		/* Leave the block clocked, close and suspend tear it down */
>  		netdev_err(dev, "failed to initialize DMA\n");
> -		goto out_clk_disable;
> +		return ret;
>  	}

[ ... ]

>  static int bcmgenet_suspend(struct device *d)
>  {
>  	struct net_device *dev = dev_get_drvdata(d);
>  	struct bcmgenet_priv *priv = netdev_priv(dev);
> -	struct bcmgenet_rxnfc_rule *rule;
> -	u32 reg, hfb_enable = 0;
>
>  	if (!netif_running(dev))
>  		return 0;
>
> +	/* A failed resume has torn the datapath down already. Resume still
> +	 * enables irq1 on the WoL path, so it has to be disabled here.
> +	 */
> +	if (!priv->tx_cbs) {
> +		if (device_may_wakeup(d) && priv->wolopts) {
> +			bcmgenet_wol_arm_filters(priv);
> +			disable_irq(priv->irq1);
> +		}
> +		return 0;
> +	}

[Severity: Medium]

After a failed non-WoL resume, can the next non-WoL suspend leave an
external PHY powered up with its link active?

On the normal non-WoL path, bcmgenet_suspend() halts and suspends the
PHY:

bcmgenet_netif_stop(dev, true)
  phy_stop()
    PHY_HALTED -> phy_suspend()   (BMCR power-down)

The failed resume has already done this before bcmgenet_init_dma()
fails:

bcmgenet_resume()
  phy_init_hw()          drv->soft_reset() clears BMCR_PDOWN and sets
                         phydev->suspended = 0
  genphy_config_aneg()   restarts autoneg
  bcmgenet_init_dma()    fails -> return ret
                         (phy_resume() is never reached)

This new early return then skips any PHY power-down. Calling phy_stop()
here would not work either, because the PHY is still in PHY_HALTED.

Nothing else seems to power the PHY down. bcmgenet_mii_probe() sets
mac_managed_pm = true, so mdio_bus_phy_suspend() returns early, and
bcmgenet_suspend_noirq() only powers down an internal GPHY via
GENET_POWER_PASSIVE.

So an external PHY whose driver has both .soft_reset and .suspend (for
example genphy_soft_reset with genphy_suspend) would stay powered with
its link up through system sleep.

This would not show up with the bcm54xx PHY on CM4, which has no
.soft_reset, or with the Generic PHY driver.

Does the !priv->tx_cbs path need a phy_suspend() call (or similar) when
WoL is not in use?

>
>  	netif_device_detach(dev);
>
>  	if (device_may_wakeup(d) && priv->wolopts) {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-nb-genet-fini-dma-net-v3-0-6e9fc89c0156%40tipi-net.de

Re: [PATCH net v3 1/2] net: bcmgenet: recover from a failed resume

[Nicolai Buchwitz]

On 23.10.2026 13:55, netdev-bot+sashiko@kernel.org wrote:
> [...]

> [Severity: Medium]
> 
> After a failed non-WoL resume, can the next non-WoL suspend leave an
> external PHY powered up with its link active?
> 
> On the normal non-WoL path, bcmgenet_suspend() halts and suspends the
> PHY:
> 
> bcmgenet_netif_stop(dev, true)
>   phy_stop()
>     PHY_HALTED -> phy_suspend()   (BMCR power-down)
> 
> The failed resume has already done this before bcmgenet_init_dma()
> fails:
> 
> bcmgenet_resume()
>   phy_init_hw()          drv->soft_reset() clears BMCR_PDOWN and sets
>                          phydev->suspended = 0
>   genphy_config_aneg()   restarts autoneg
>   bcmgenet_init_dma()    fails -> return ret
>                          (phy_resume() is never reached)
> 
> This new early return then skips any PHY power-down. Calling phy_stop()
> here would not work either, because the PHY is still in PHY_HALTED.
> 
> Nothing else seems to power the PHY down. bcmgenet_mii_probe() sets
> mac_managed_pm = true, so mdio_bus_phy_suspend() returns early, and
> bcmgenet_suspend_noirq() only powers down an internal GPHY via
> GENET_POWER_PASSIVE.
> 
> So an external PHY whose driver has both .soft_reset and .suspend (for
> example genphy_soft_reset with genphy_suspend) would stay powered with
> its link up through system sleep.
> 
> This would not show up with the bcm54xx PHY on CM4, which has no
> .soft_reset, or with the Generic PHY driver.
> 
> Does the !priv->tx_cbs path need a phy_suspend() call (or similar) when
> WoL is not in use?
> 
>> 
>>  	netif_device_detach(dev);
>> 
>>  	if (device_may_wakeup(d) && priv->wolopts) {
> 
> [ ... ]

Same as in v2: pre-existing, phy_init_hw() powers it up in the failed
resume and nothing starts it again. Fair that it's reachable now, before
this series that suspend just hung. Nobody seems to have hit it either, 
it
needs a failed resume plus a PHY with soft_reset, so pretty much an edge
case, don't you think?

I'd rather not add a phy_suspend() I can't test ... the CM4's bcm54xx 
has
no soft_reset, so the PHY never gets powered up there anyway. The proper
fix is to stop leaving the half state behind, that's on my list after 
the
MTU series.

Thanks,
Nicolai