netdev - Re: [net-next,v2] net: phy: phy_device: fix PHY WOL enabled, PM failed to suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZqdM1rwbmIED/0WC@shell.armlinux.org.uk>
Date: Mon, 29 Jul 2024 09:03:34 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Youwan Wang <youwan@...china.com>
Cc: andrew@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	hkallweit1@...il.com, kuba@...nel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, pabeni@...hat.com
Subject: Re: [net-next,v2] net: phy: phy_device: fix PHY WOL enabled, PM
 failed to suspend

On Tue, Jul 09, 2024 at 07:37:35PM +0800, Youwan Wang wrote:
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 2ce74593d6e4..0564decf701f 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -270,6 +270,7 @@ static DEFINE_MUTEX(phy_fixup_lock);
>  
>  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  {
> +	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
>  	struct device_driver *drv = phydev->mdio.dev.driver;
>  	struct phy_driver *phydrv = to_phy_driver(drv);
>  	struct net_device *netdev = phydev->attached_dev;
> @@ -277,6 +278,15 @@ 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.
> +	 */
> +	phy_ethtool_get_wol(phydev, &wol);
> +	phydev->wol_enabled = !!(wol.wolopts);
> +	if (!netdev && phydev->wol_enabled &&
> +	    !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
> +		return false;
> +

We now end up with two places that do this phy_ethtool_get_wol()
dance. Rather than duplicating code, we should use a function to
avoid the duplication:

8<===

From: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Subject: [PATCH net-next] net: phy: add phy_drv_wol_enabled()

Add a function that phylib can inquire of the driver whether WoL has
been enabled at the PHY.

Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
---
 drivers/net/phy/phy_device.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 19f8ae113dd3..09f57181b8a6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1433,6 +1433,15 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+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;
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1975,7 +1984,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 +1991,9 @@ int phy_suspend(struct phy_device *phydev)
 	if (phydev->suspended || !phydrv)
 		return 0;
 
-	phy_ethtool_get_wol(phydev, &wol);
-	phydev->wol_enabled = wol.wolopts || (netdev && netdev->wol_enabled);
+	phydev->wol_enabled = phy_drv_wol_enabled(phydev) ||
+			      (netdev && netdev->wol_enabled);
+
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
 		return -EBUSY;
-- 
2.30.2


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