Hi Heiner,
On Tue, 18 Dec 2018 19:15:39 +0100 <hkallwe...@gmail.com> wrote:
> On 18.12.2018 08:57, Kunihiko Hayashi wrote:
> > Even though the link is down before entering hibernation,
> > there is an issue that the network interface always links up after resuming
> > from hibernation.
> >
> > If the link is still down before enabling the network interface,
> > and after resuming from hibernation, the phydev->state is forcibly set
> > to PHY_UP in mdio_bus_phy_restore(), and the link becomes up.
> >
> > In suspend sequence, only if the PHY is attached, mdio_bus_phy_suspend()
> > calls phy_stop_machine(), and mdio_bus_phy_resume() calls
> > phy_start_machine().
> > In resume sequence, it's enough to do the same as mdio_bus_phy_resume()
> > because the state has been preserved.
> >
> > This patch fixes the issue by calling phy_start_machine() in
> > mdio_bus_phy_restore() in the same way as mdio_bus_phy_resume().
> >
> The patch itself is fine and also that you tagged it as "net".
> What's missing is a "Fixes:" line (before the Suggested-by:).
> I think it should be:
>
> Fixes: bc87922ff59d ("phy: Move PHY PM operations into phy_device")
>
> This commit didn't actually introduce the issue, but moved it to today's
> place.
Thanks for pointing out.
I was concerned about that and which commit would be fixed if added.
Thank you,
>
> > Suggested-by: Heiner Kallweit <hkallwe...@gmail.com>
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunih...@socionext.com>
> > ---
> > drivers/net/phy/phy_device.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > This patch is based on the RFC patch discussion [1].
> > [1] https://www.spinics.net/lists/netdev/msg537326.html
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 7d5d698..3685be4 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -315,11 +315,8 @@ static int mdio_bus_phy_restore(struct device *dev)
> > if (ret < 0)
> > return ret;
> >
> > - /* The PHY needs to renegotiate. */
> > - phydev->link = 0;
> > - phydev->state = PHY_UP;
> > -
> > - phy_start_machine(phydev);
> > + if (phydev->attached_dev && phydev->adjust_link)
> > + phy_start_machine(phydev);
> >
> > return 0;
> > }
> >
---
Best Regards,
Kunihiko Hayashi
