[PATCH net 0/1] net: phy: Fix interrupt mask loss on resume from hibernation

[PATCH net 0/1] net: phy: Fix interrupt mask loss on resume from hibernation

[Laurent Badel]

Some PHYs such as SMSC LAN87xx clear the interrupt mask register on
software reset. Since mdio_bus_phy_restore() calls phy_init_hw() which
does a software reset of the PHY, these PHYs will lose their interrupt 
mask configuration on resuming from hibernation.

I initially reconfigured only the PHY interrupt mask using 
phydev->config_intr(), which worked fine with PM_DEBUG/test_resume, but
there seems to be an issue when resuming from a real hibernation, by which
the interrupt type is not set appropriately (in this case 
IRQ_TYPE_LEVEL_LOW). Calling irq_set_irq_type() directly from sysfs 
restored the PHY functionality immediately suggesting that everything is
otherwise well configured. Therefore this patch suggests freeing and
re-requesting the interrupt, to guarantee proper interrupt configuration.

Laurent Badel (1):
  net: phy: Reconfigure PHY interrupt in mdio_bus_phy_restore()

 drivers/net/phy/phy_device.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.17.1



-----------------------------
Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland 

-----------------------------

[PATCH net 1/1] net: phy: Reconfigure PHY interrupt in mdio_bus_phy_restore()

[Laurent Badel]

Some PHY (e.g. SMSC LAN87xx) clear their interrupt mask on software
reset. This breaks the ethernet interface on resuming from hibernation,
if the PHY is running in interrupt mode, so reconfigure interrupts
after the software reset in mdio_bus_phy_restore().

Signed-off-by: Laurent Badel <laurentbadel@eaton.com>
---
 drivers/net/phy/phy_device.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 80c2e646c093..5070eed55447 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -324,6 +324,15 @@ static int mdio_bus_phy_restore(struct device *dev)
 	if (ret < 0)
 		return ret;
 
+	if (phydev->drv->config_intr && phy_interrupt_is_valid(phydev))
+	{
+		/* Some PHYs (e.g. SMSC LAN8720) clear their
+		 * interrupt mask on software reset.
+		 */
+		phy_free_interrupt(phydev);
+		phy_request_interrupt(phydev);
+	}
+
 	if (phydev->attached_dev && phydev->adjust_link)
 		phy_start_machine(phydev);
 
-- 
2.17.1



-----------------------------
Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland 

-----------------------------

Re: [PATCH net 0/1] net: phy: Fix interrupt mask loss on resume from hibernation

[Heiner Kallweit]

On 22.01.2021 15:35, Laurent Badel wrote:
> Some PHYs such as SMSC LAN87xx clear the interrupt mask register on
> software reset. Since mdio_bus_phy_restore() calls phy_init_hw() which
> does a software reset of the PHY, these PHYs will lose their interrupt 
> mask configuration on resuming from hibernation.

The (optional) software reset is done via soft_reset callback.
So if the PHY in question needs special treatment after a soft reset,
why not add it to the soft_reset callback?

> 
> I initially reconfigured only the PHY interrupt mask using 
> phydev->config_intr(), which worked fine with PM_DEBUG/test_resume, but
> there seems to be an issue when resuming from a real hibernation, by which
> the interrupt type is not set appropriately (in this case 
> IRQ_TYPE_LEVEL_LOW). Calling irq_set_irq_type() directly from sysfs 

This sounds to me like a lower level driver (e.g. for GPIO / interrupt
controller) not resuming properly from hibernation. Supposedly things
like edge/level high/low/both are stored per interrupt line in a register
of the interrupt controller, and the controller would have to restore
the register value on resume from hibernation. You may want to have
a look at that driver.

> restored the PHY functionality immediately suggesting that everything is
> otherwise well configured. Therefore this patch suggests freeing and
> re-requesting the interrupt, to guarantee proper interrupt configuration.
> 
> Laurent Badel (1):
>   net: phy: Reconfigure PHY interrupt in mdio_bus_phy_restore()
> 
>  drivers/net/phy/phy_device.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

RE: [EXTERNAL] Re: [PATCH net 0/1] net: phy: Fix interrupt mask loss on resume from hibernation

[Badel, Laurent]

> From: Heiner Kallweit <hkallweit1@gmail.com>
> Sent: Friday, January 22, 2021 4:20 PM

> The (optional) software reset is done via soft_reset callback.
> So if the PHY in question needs special treatment after a soft reset,
> why not add it to the soft_reset callback?

Thank you very much for the fast reply. This makes sense, I will 
modify the patch in this direction. 
 
> This sounds to me like a lower level driver (e.g. for GPIO / interrupt
> controller) not resuming properly from hibernation. Supposedly things
> like edge/level high/low/both are stored per interrupt line in a
> register of the interrupt controller, and the controller would have to
> restore the register value on resume from hibernation. You may want to
> have a look at that driver.

I think you are right, the gpio-mxs driver has no PM operations, so 
if it responsible for restoring the interrupt level, no wonder it 
doesn't. This would require implementing the PM ops, which would 
take some additional work, I'll see if I can get around to doing this.
 
Best regards,

Laurent



-----------------------------
Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland 

-----------------------------