2021-01-22 14:50:08

by Badel, Laurent

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

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

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


2021-01-22 15:24:37

by Heiner Kallweit

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

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

2021-01-22 16:16:10

by Badel, Laurent

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

> From: Heiner Kallweit <[email protected]>
> 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

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