From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH RFC 13/18] r8168: replace speed_down with genphy_restart_aneg Date: Fri, 22 Dec 2017 11:14:41 +0100 Message-ID: <20171222101441.GK2431@lunn.ch> References: <83321b2e-8402-26c5-9703-3fe795cc893d@gmail.com> <704cc35b-d54a-4220-2a24-dec9521e05af@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Realtek linux nic maintainers , Chun-Hao Lin , David Miller , "netdev@vger.kernel.org" To: Heiner Kallweit Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:43037 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756264AbdLVKOs (ORCPT ); Fri, 22 Dec 2017 05:14:48 -0500 Content-Disposition: inline In-Reply-To: <704cc35b-d54a-4220-2a24-dec9521e05af@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Dec 21, 2017 at 09:50:39PM +0100, Heiner Kallweit wrote: > Dealing with link partner abilities is handled by phylib, so let's > just trigger autonegotiation here. > > Signed-off-by: Heiner Kallweit > --- > drivers/net/ethernet/realtek/r8168.c | 26 +------------------------- > 1 file changed, 1 insertion(+), 25 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8168.c b/drivers/net/ethernet/realtek/r8168.c > index d33f93a31..6b398915f 100644 > --- a/drivers/net/ethernet/realtek/r8168.c > +++ b/drivers/net/ethernet/realtek/r8168.c > @@ -4360,30 +4360,6 @@ static void rtl_init_mdio_ops(struct rtl8168_private *tp) > } > } > > -static void rtl_speed_down(struct rtl8168_private *tp) > -{ > - u32 adv; > - int lpa; > - > - rtl_writephy(tp, 0x1f, 0x0000); > - lpa = rtl_readphy(tp, MII_LPA); > - > - if (lpa & (LPA_10HALF | LPA_10FULL)) > - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full; > - else if (lpa & (LPA_100HALF | LPA_100FULL)) > - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full | > - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full; > - else > - adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full | > - ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full | > - (tp->mii.supports_gmii ? > - ADVERTISED_1000baseT_Half | > - ADVERTISED_1000baseT_Full : 0); > - > - rtl8168_set_speed(tp->dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL, > - adv); > -} > - > static void rtl_wol_suspend_quirk(struct rtl8168_private *tp) > { > void __iomem *ioaddr = tp->mmio_addr; > @@ -4424,7 +4400,7 @@ static bool rtl_wol_pll_power_down(struct rtl8168_private *tp) > if (!(__rtl8168_get_wol(tp) & WAKE_ANY)) > return false; > > - rtl_speed_down(tp); > + genphy_restart_aneg(tp->dev->phydev); > rtl_wol_suspend_quirk(tp); > > return true; I'm not too clear what is going on here? Is this suspend while WOL is enabled? There should be no need to change the PHY settings. The PHY driver should leave the PHY running in whatever state it was configured to. The only danger here is that the MAC driver has called phy_stop() during suspend. That should not be done when WOL is enabled. Is the wol being passed to the phylib? phy_ethtool_set_wol() and phy_ethtool_get_wol()? Andrew