Thread (11 messages) 11 messages, 6 authors, 4h ago

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

flat view

From: Nicolai Buchwitz <nb@tipi-net.de>
Date: 2026-10-23 12:04:39
Also in: lkml

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?
quoted
 	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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help