Re: [PATCH] net: phy: Don't disable irqs on shutdown if WoL is enabled

On 8/9/2023 9:21 AM, Andrew Lunn wrote:

quoted

Thinking about this, I wonder whether we could solve your issue by
disabling interrupts when the PHY is probed, rather than disabling
them on shutdown - something like this? (not build tested)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3e9909b30938..4d1a37487923 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c

@@ -3216,6 +3216,8 @@ static int phy_probe(struct device *dev)
  			goto out;
  	}
  
+        phy_disable_interrupts(phydev);
+
  	/* Start out supporting everything. Eventually,
  	 * a controller will attach, and may modify one
  	 * or both of these values

At some point, the interrupt is going to be enabled again. And then
the WoL interrupt will fire. I think some PHY drivers actually need to
see that WoL interrupt. e.g. the marvell driver has this comment:

static int m88e1318_set_wol(struct phy_device *phydev,
                             struct ethtool_wolinfo *wol)
{
....
                 /* If WOL event happened once, the LED[2] interrupt pin
                  * will not be cleared unless we reading the interrupt status
                  * register. If interrupts are in use, the normal interrupt
                  * handling will clear the WOL event. Clear the WOL event
                  * before enabling it if !phy_interrupt_is_valid()
                  */

So it seems like just probing the marvell PHY is not enough to clear
the WoL interrupt.

Can we be sure that the other PHY has reached a state it can handle
and clear an interrupt when we come to enable the interrupt? I think
not, especially in cases like NFS root, where the interface will be
put into use as soon as it exists, maybe before the other interface
has probed.

Does it really make sense to have the PHY be interrupt driven for this 
specific board configuration if this causes so much hassle?
-- 
Florian