This is a note to let you know that I've just added the patch titled
net: phy: fix resume handling
to the 4.14-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: net-phy-fix-resume-handling.patch and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Wed Feb 28 16:23:28 CET 2018
From: Russell King rmk+kernel@armlinux.org.uk Date: Tue, 12 Dec 2017 10:45:36 +0000 Subject: net: phy: fix resume handling
From: Russell King rmk+kernel@armlinux.org.uk
[ Upstream commit f5e64032a799d4f54decc7eb6aafcdffb67f9ad9 ]
When a PHY has the BMCR_PDOWN bit set, it may decide to ignore writes to other registers, or reset the registers to power-on defaults. Micrel PHYs do this for their interrupt registers.
The current structure of phylib tries to enable interrupts before resuming (and releasing) the BMCR_PDOWN bit. This fails, causing Micrel PHYs to stop working after a suspend/resume sequence if they are using interrupts.
Fix this by ensuring that the PHY driver resume methods do not take the phydev->lock mutex themselves, but the callers of phy_resume() take that lock. This then allows us to move the call to phy_resume() before we enable interrupts in phy_start().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk Reviewed-by: Andrew Lunn andrew@lunn.ch Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Sasha Levin alexander.levin@verizon.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/net/phy/at803x.c | 4 ---- drivers/net/phy/phy.c | 9 +++------ drivers/net/phy/phy_device.c | 10 ++++++---- 3 files changed, 9 insertions(+), 14 deletions(-)
--- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -239,14 +239,10 @@ static int at803x_resume(struct phy_devi { int value;
- mutex_lock(&phydev->lock); - value = phy_read(phydev, MII_BMCR); value &= ~(BMCR_PDOWN | BMCR_ISOLATE); phy_write(phydev, MII_BMCR, value);
- mutex_unlock(&phydev->lock); - return 0; }
--- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -828,7 +828,6 @@ EXPORT_SYMBOL(phy_stop); */ void phy_start(struct phy_device *phydev) { - bool do_resume = false; int err = 0;
mutex_lock(&phydev->lock); @@ -841,6 +840,9 @@ void phy_start(struct phy_device *phydev phydev->state = PHY_UP; break; case PHY_HALTED: + /* if phy was suspended, bring the physical link up again */ + phy_resume(phydev); + /* make sure interrupts are re-enabled for the PHY */ if (phydev->irq != PHY_POLL) { err = phy_enable_interrupts(phydev); @@ -849,17 +851,12 @@ void phy_start(struct phy_device *phydev }
phydev->state = PHY_RESUMING; - do_resume = true; break; default: break; } mutex_unlock(&phydev->lock);
- /* if phy was suspended, bring the physical link up again */ - if (do_resume) - phy_resume(phydev); - phy_trigger_machine(phydev, true); } EXPORT_SYMBOL(phy_start); --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -135,7 +135,9 @@ static int mdio_bus_phy_resume(struct de if (!mdio_bus_phy_may_suspend(phydev)) goto no_resume;
+ mutex_lock(&phydev->lock); ret = phy_resume(phydev); + mutex_unlock(&phydev->lock); if (ret < 0) return ret;
@@ -1026,7 +1028,9 @@ int phy_attach_direct(struct net_device if (err) goto error;
+ mutex_lock(&phydev->lock); phy_resume(phydev); + mutex_unlock(&phydev->lock); phy_led_triggers_register(phydev);
return err; @@ -1157,6 +1161,8 @@ int phy_resume(struct phy_device *phydev struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver); int ret = 0;
+ WARN_ON(!mutex_is_locked(&phydev->lock)); + if (phydev->drv && phydrv->resume) ret = phydrv->resume(phydev);
@@ -1639,13 +1645,9 @@ int genphy_resume(struct phy_device *phy { int value;
- mutex_lock(&phydev->lock); - value = phy_read(phydev, MII_BMCR); phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
- mutex_unlock(&phydev->lock); - return 0; } EXPORT_SYMBOL(genphy_resume);
Patches currently in stable-queue which might be from rmk+kernel@armlinux.org.uk are
queue-4.14/arm-8731-1-fix-csum_partial_copy_from_user-stack-mismatch.patch queue-4.14/net-phy-fix-resume-handling.patch
When a PHY has the BMCR_PDOWN bit set, it may decide to ignore writes to other registers, or reset the registers to power-on defaults. Micrel PHYs do this for their interrupt registers.
The current structure of phylib tries to enable interrupts before resuming (and releasing) the BMCR_PDOWN bit. This fails, causing Micrel PHYs to stop working after a suspend/resume sequence if they are using interrupts.
Fix this by ensuring that the PHY driver resume methods do not take the phydev->lock mutex themselves, but the callers of phy_resume() take that lock. This then allows us to move the call to phy_resume() before we enable interrupts in phy_start().
Hi Greg
This patch introduces some issues. You don't want this patch on its own, you want it in combination with:
Fixes: 9c2c2e62df3f ("net: phy: Restore phy_resume() locking assumption")
Which went into David Millers net tree yesterday and should be making its way towards stable.
I suggest you hold off on this patch, until you can do both at the same time.
Andrew
On Wed, Feb 28, 2018 at 04:43:48PM +0100, Andrew Lunn wrote:
When a PHY has the BMCR_PDOWN bit set, it may decide to ignore writes to other registers, or reset the registers to power-on defaults. Micrel PHYs do this for their interrupt registers.
The current structure of phylib tries to enable interrupts before resuming (and releasing) the BMCR_PDOWN bit. This fails, causing Micrel PHYs to stop working after a suspend/resume sequence if they are using interrupts.
Fix this by ensuring that the PHY driver resume methods do not take the phydev->lock mutex themselves, but the callers of phy_resume() take that lock. This then allows us to move the call to phy_resume() before we enable interrupts in phy_start().
Hi Greg
This patch introduces some issues. You don't want this patch on its own, you want it in combination with:
Fixes: 9c2c2e62df3f ("net: phy: Restore phy_resume() locking assumption")
Which went into David Millers net tree yesterday and should be making its way towards stable.
I suggest you hold off on this patch, until you can do both at the same time.
Thanks for letting me know, I've now dropped this.
greg k-h
On Wed, Feb 28, 2018 at 05:27:34PM +0100, Greg KH wrote:
On Wed, Feb 28, 2018 at 04:43:48PM +0100, Andrew Lunn wrote:
When a PHY has the BMCR_PDOWN bit set, it may decide to ignore writes to other registers, or reset the registers to power-on defaults. Micrel PHYs do this for their interrupt registers.
The current structure of phylib tries to enable interrupts before resuming (and releasing) the BMCR_PDOWN bit. This fails, causing Micrel PHYs to stop working after a suspend/resume sequence if they are using interrupts.
Fix this by ensuring that the PHY driver resume methods do not take the phydev->lock mutex themselves, but the callers of phy_resume() take that lock. This then allows us to move the call to phy_resume() before we enable interrupts in phy_start().
Hi Greg
This patch introduces some issues. You don't want this patch on its own, you want it in combination with:
Fixes: 9c2c2e62df3f ("net: phy: Restore phy_resume() locking assumption")
Which went into David Millers net tree yesterday and should be making its way towards stable.
I suggest you hold off on this patch, until you can do both at the same time.
Thanks for letting me know, I've now dropped this.
And now both are queued up for 4.14.y, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
Greg KH -
gregkh＠linuxfoundation.org