| Message ID | 20240731091537.771391-1-youwan@nfschina.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4f534b7f0c8d2a9ec557f9c7d77f96d29518c666 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v4] net: phy: phy_device: fix PHY WOL enabled, PM failed to suspend | expand |
On Wed, 31 Jul 2024 17:15:37 +0800 Youwan Wang wrote: > If the PHY of the mido bus is enabled with Wake-on-LAN (WOL), > we cannot suspend the PHY. Although the WOL status has been > checked in phy_suspend(), returning -EBUSY(-16) would cause > the Power Management (PM) to fail to suspend. Since > phy_suspend() is an exported symbol (EXPORT_SYMBOL), > timely error reporting is needed. Therefore, an additional > check is performed here. If the PHY of the mido bus is enabled > with WOL, we skip calling phy_suspend() to avoid PM failure. > > From the following logs, it has been observed that the phydev->attached_dev > is NULL, phydev is "stmmac-0:01", it not attached, but it will affect suspend > and resume.The actually attached "stmmac-0:00" will not dpm_run_callback(): > mdio_bus_phy_suspend(). Are you just reposting this to add a review tag? You don't have to do that. Please let me know if you're doing so based on some documentation, if such documentation exists I'll go and update it.. Please include a changelog in the future. And also please don't post new versions in-reply-to an existing thread.
On Wed, 31 Jul 2024 17:15:37 +0800 Youwan Wang wrote: > + /* If the PHY on the mido bus is not attached but has WOL enabled > + * we cannot suspend the PHY. > + */ > + if (!netdev && phy_drv_wol_enabled(phydev)) > + return false; Not sure why you stopped setting phydev->wol_enabled between v2 and v3 but let's hear from phy maintainers..
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 31 Jul 2024 17:15:37 +0800 you wrote: > If the PHY of the mido bus is enabled with Wake-on-LAN (WOL), > we cannot suspend the PHY. Although the WOL status has been > checked in phy_suspend(), returning -EBUSY(-16) would cause > the Power Management (PM) to fail to suspend. Since > phy_suspend() is an exported symbol (EXPORT_SYMBOL), > timely error reporting is needed. Therefore, an additional > check is performed here. If the PHY of the mido bus is enabled > with WOL, we skip calling phy_suspend() to avoid PM failure. > > [...] Here is the summary with links: - [net-next,v4] net: phy: phy_device: fix PHY WOL enabled, PM failed to suspend https://git.kernel.org/netdev/net-next/c/4f534b7f0c8d You are awesome, thank you!
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 7752e9386b40..34752a87f98f 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -279,6 +279,15 @@ static struct phy_driver genphy_driver; static LIST_HEAD(phy_fixup_list); static DEFINE_MUTEX(phy_fixup_lock); +static bool phy_drv_wol_enabled(struct phy_device *phydev) +{ + struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL }; + + phy_ethtool_get_wol(phydev, &wol); + + return wol.wolopts != 0; +} + static bool mdio_bus_phy_may_suspend(struct phy_device *phydev) { struct device_driver *drv = phydev->mdio.dev.driver; @@ -288,6 +297,12 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev) if (!drv || !phydrv->suspend) return false; + /* If the PHY on the mido bus is not attached but has WOL enabled + * we cannot suspend the PHY. + */ + if (!netdev && phy_drv_wol_enabled(phydev)) + return false; + /* PHY not attached? May suspend if the PHY has not already been * suspended as part of a prior call to phy_disconnect() -> * phy_detach() -> phy_suspend() because the parent netdev might be the @@ -1975,7 +1990,6 @@ EXPORT_SYMBOL(phy_detach); int phy_suspend(struct phy_device *phydev) { - struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL }; struct net_device *netdev = phydev->attached_dev; const struct phy_driver *phydrv = phydev->drv; int ret; @@ -1983,8 +1997,7 @@ int phy_suspend(struct phy_device *phydev) if (phydev->suspended || !phydrv) return 0; - phy_ethtool_get_wol(phydev, &wol); - phydev->wol_enabled = wol.wolopts || + phydev->wol_enabled = phy_drv_wol_enabled(phydev) || (netdev && netdev->ethtool->wol_enabled); /* If the device has WOL enabled, we cannot suspend the PHY */ if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))