>> 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.
>>
>> Thank you all for your analysis.
>> I am using the Linux kernel version 6.6, the current system is
>> utilizing ACPI firmware. However, in terms of configuration,
>> the system only includes MAC layer configuration while lacking
>> PHY configuration. Furthermore, it has been observed that the
>> phydev->attached_dev is NULL
>>
>> Is it possible to add a judgment about netdev is NULL?
>> if (!netdev && phydev->wol_enabled &&
>>     !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))

>Comments like this should be placed below the --- so they don't make
>it into the commit message.

>Why is phydev->attached_dev NULL? Was a MAC never attached to the PHY?
>Has the MAC disconnected the PHY as part of the suspend? It would be
>odd that a device being used for WoL would disconnect the PHY.

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().

log:
[    5.932502] YT8521 Gigabit Ethernet stmmac-0:00: attached PHY driver
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
[    5.932512] YT8521 Gigabit Ethernet stmmac-0:01: attached PHY driver
(mii_bus:phy_addr=stmmac-0:01, irq=POLL)
[   24.566289] YT8521 Gigabit Ethernet stmmac-0:00: yt8521_read_status,
link down, media: UTP

>>
>> log:
>> [  322.631362] OOM killer disabled.
>> [  322.631364] Freezing remaining freezable tasks
>> [  322.632536] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
>> [  322.632540] printk: Suspending console(s) (use no_console_suspend to debug)
>> [  322.633052] YT8521 Gigabit Ethernet stmmac-0:01:
>> PM: dpm_run_callback(): mdio_bus_phy_suspend+0x0/0x110 [libphy] returns -16
>> [  322.633071] YT8521 Gigabit Ethernet stmmac-0:01:
>> PM: failed to suspend: error -16
>> [  322.669699] PM: Some devices failed to suspend, or early wake event detected
>> [  322.669949] OOM killer enabled.
>> [  322.669951] Restarting tasks ... done.
>> [  322.671008] random: crng reseeded on system resumption
>> [  322.671014] PM: suspend exit
>>
>> If the YT8521 driver adds phydrv->flags, ask the YT8521 driver to process
>> WOL at suspend and resume time, the phydev->suspended_by_mdio_bus=1
>> flag would cause the resume failure.
>>
>> log:
>> [  260.814763] YT8521 Gigabit Ethernet stmmac-0:01:
>> PM: dpm_run_callback():mdio_bus_phy_resume+0x0/0x160 [libphy] returns -95
>> [  260.814782] YT8521 Gigabit Ethernet stmmac-0:01:
>> PM: failed to resume: error -95

>-95 is EOPNOTSUPP. Where is that coming from?

yt8511_config_init() -> ret = -EOPNOTSUPP;

Signed-off-by: Youwan Wang <youwan@nfschina.com>
---
 drivers/net/phy/phy_device.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

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;
+
 	/* 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
-- 
2.25.1

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@armlinux.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@armlinux.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!

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().

init log:
[    5.932502] YT8521 Gigabit Ethernet stmmac-0:00: attached PHY driver
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
[    5.932512] YT8521 Gigabit Ethernet stmmac-0:01: attached PHY driver
(mii_bus:phy_addr=stmmac-0:01, irq=POLL)
[   24.566289] YT8521 Gigabit Ethernet stmmac-0:00: yt8521_read_status,
link down, media: UTP

suspend log:
[  322.631362] OOM killer disabled.
[  322.631364] Freezing remaining freezable tasks
[  322.632536] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[  322.632540] printk: Suspending console(s) (use no_console_suspend to debug)
[  322.633052] YT8521 Gigabit Ethernet stmmac-0:01:
PM: dpm_run_callback(): mdio_bus_phy_suspend+0x0/0x110 [libphy] returns -16
[  322.633071] YT8521 Gigabit Ethernet stmmac-0:01:
PM: failed to suspend: error -16
[  322.669699] PM: Some devices failed to suspend, or early wake event detected
[  322.669949] OOM killer enabled.
[  322.669951] Restarting tasks ... done.
[  322.671008] random: crng reseeded on system resumption
[  322.671014] PM: suspend exit

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@armlinux.org.uk>
Signed-off-by: Youwan Wang <youwan@nfschina.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 drivers/net/phy/phy_device.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

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))
-- 
2.25.1

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..

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.

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().

init log:
[    5.932502] YT8521 Gigabit Ethernet stmmac-0:00: attached PHY driver
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
[    5.932512] YT8521 Gigabit Ethernet stmmac-0:01: attached PHY driver
(mii_bus:phy_addr=stmmac-0:01, irq=POLL)
[   24.566289] YT8521 Gigabit Ethernet stmmac-0:00: yt8521_read_status,
link down, media: UTP

suspend log:
[  322.631362] OOM killer disabled.
[  322.631364] Freezing remaining freezable tasks
[  322.632536] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[  322.632540] printk: Suspending console(s) (use no_console_suspend to debug)
[  322.633052] YT8521 Gigabit Ethernet stmmac-0:01:
PM: dpm_run_callback(): mdio_bus_phy_suspend+0x0/0x110 [libphy] returns -16
[  322.633071] YT8521 Gigabit Ethernet stmmac-0:01:
PM: failed to suspend: error -16
[  322.669699] PM: Some devices failed to suspend, or early wake event detected
[  322.669949] OOM killer enabled.
[  322.669951] Restarting tasks ... done.
[  322.671008] random: crng reseeded on system resumption
[  322.671014] PM: suspend exit

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@armlinux.org.uk>
Signed-off-by: Youwan Wang <youwan@nfschina.com>
---
 drivers/net/phy/phy_device.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7752e9386b40..04a9987ac092 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))
-- 
2.25.1

On 30.07.2024 10:15, 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().
> 
> init log:
> [    5.932502] YT8521 Gigabit Ethernet stmmac-0:00: attached PHY driver
> (mii_bus:phy_addr=stmmac-0:00, irq=POLL)
> [    5.932512] YT8521 Gigabit Ethernet stmmac-0:01: attached PHY driver
> (mii_bus:phy_addr=stmmac-0:01, irq=POLL)
> [   24.566289] YT8521 Gigabit Ethernet stmmac-0:00: yt8521_read_status,
> link down, media: UTP
> 
> suspend log:
> [  322.631362] OOM killer disabled.
> [  322.631364] Freezing remaining freezable tasks
> [  322.632536] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
> [  322.632540] printk: Suspending console(s) (use no_console_suspend to debug)
> [  322.633052] YT8521 Gigabit Ethernet stmmac-0:01:
> PM: dpm_run_callback(): mdio_bus_phy_suspend+0x0/0x110 [libphy] returns -16
> [  322.633071] YT8521 Gigabit Ethernet stmmac-0:01:
> PM: failed to suspend: error -16
> [  322.669699] PM: Some devices failed to suspend, or early wake event detected
> [  322.669949] OOM killer enabled.
> [  322.669951] Restarting tasks ... done.
> [  322.671008] random: crng reseeded on system resumption
> [  322.671014] PM: suspend exit
> 
> 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@armlinux.org.uk>
> Signed-off-by: Youwan Wang <youwan@nfschina.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/phy/phy_device.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 7752e9386b40..04a9987ac092 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))

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

 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.

 Why is phydev->attached_dev NULL? Was a MAC never attached to the PHY?
 Has the MAC disconnected the PHY as part of the suspend? It would be
 odd that a device being used for WoL would disconnect the PHY.

 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().

 log:
 [    5.932502] YT8521 Gigabit Ethernet stmmac-0:00: attached PHY driver
 (mii_bus:phy_addr=stmmac-0:00, irq=POLL)
 [    5.932512] YT8521 Gigabit Ethernet stmmac-0:01: attached PHY driver
 (mii_bus:phy_addr=stmmac-0:01, irq=POLL)
 [   24.566289] YT8521 Gigabit Ethernet stmmac-0:00: yt8521_read_status,
 link down, media: UTP

 log:
 [  322.631362] OOM killer disabled.
 [  322.631364] Freezing remaining freezable tasks
 [  322.632536] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
 [  322.632540] printk: Suspending console(s) (use no_console_suspend to debug)
 [  322.633052] YT8521 Gigabit Ethernet stmmac-0:01:
 PM: dpm_run_callback(): mdio_bus_phy_suspend+0x0/0x110 [libphy] returns -16
 [  322.633071] YT8521 Gigabit Ethernet stmmac-0:01:
 PM: failed to suspend: error -16
 [  322.669699] PM: Some devices failed to suspend, or early wake event detected
 [  322.669949] OOM killer enabled.
 [  322.669951] Restarting tasks ... done.
 [  322.671008] random: crng reseeded on system resumption
 [  322.671014] PM: suspend exit

 If the YT8521 driver adds phydrv->flags, ask the YT8521 driver to process
 WOL at suspend and resume time, the phydev->suspended_by_mdio_bus=1
 flag would cause the resume failure.

 log:
 [  260.814763] YT8521 Gigabit Ethernet stmmac-0:01:
 PM: dpm_run_callback():mdio_bus_phy_resume+0x0/0x160 [libphy] returns -95
 [  260.814782] YT8521 Gigabit Ethernet stmmac-0:01:
 PM: failed to resume: error -95

 -95 is EOPNOTSUPP. Where is that coming from?

 yt8511_config_init() -> ret = -EOPNOTSUPP;

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Youwan Wang <youwan@nfschina.com>
---
 drivers/net/phy/phy_device.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2ce74593d6e4..c3ad6f6791ff 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -268,6 +268,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;
@@ -277,6 +286,13 @@ 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) &&
+	    !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
+		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
@@ -1850,7 +1866,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;
 	struct phy_driver *phydrv = phydev->drv;
 	int ret;
@@ -1858,8 +1873,7 @@ int phy_suspend(struct phy_device *phydev)
 	if (phydev->suspended)
 		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.25.1