From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753101AbcKQKVC (ORCPT ); Thu, 17 Nov 2016 05:21:02 -0500 Received: from mail-wm0-f51.google.com ([74.125.82.51]:37404 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751287AbcKQKU6 (ORCPT ); Thu, 17 Nov 2016 05:20:58 -0500 Message-ID: <1479378055.17538.57.camel@baylibre.com> Subject: Re: [PATCH net 1/3] net: phy: realtek: add eee advertisement disable options From: Jerome Brunet To: Anand Moon Cc: netdev@vger.kernel.org, devicetree , Florian Fainelli , Alexandre TORGUE , Neil Armstrong , Martin Blumenstingl , Kevin Hilman , Linux Kernel , Andre Roth , linux-amlogic@lists.infradead.org, Carlo Caione , Giuseppe Cavallaro , linux-arm-kernel Date: Thu, 17 Nov 2016 11:20:55 +0100 In-Reply-To: References: <1479220154-25851-1-git-send-email-jbrunet@baylibre.com> <1479220154-25851-2-git-send-email-jbrunet@baylibre.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 (3.20.5-1.fc24) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-11-16 at 22:36 +0530, Anand Moon wrote: >  Hi Jerome. > > On 15 November 2016 at 19:59, Jerome Brunet > wrote: > > > > On some platforms, energy efficient ethernet with rtl8211 devices > > is > > causing issue, like throughput drop or broken link. > > > > This was reported on the OdroidC2 (DWMAC + RTL8211F). While the > > issue root > > cause is not fully understood yet, disabling EEE advertisement > > prevent auto > > negotiation from enabling EEE. > > > > This patch provides options to disable 1000T and 100TX EEE > > advertisement > > individually for the realtek phys supporting this feature. > > > > Reported-by: Martin Blumenstingl > m> > > Cc: Giuseppe Cavallaro > > Cc: Alexandre TORGUE > > Signed-off-by: Jerome Brunet > > Signed-off-by: Neil Armstrong > > Tested-by: Andre Roth > > --- > >  drivers/net/phy/realtek.c | 65 > > ++++++++++++++++++++++++++++++++++++++++++++++- > >  1 file changed, 64 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c > > index aadd6e9f54ad..77235fd5faaf 100644 > > --- a/drivers/net/phy/realtek.c > > +++ b/drivers/net/phy/realtek.c > > @@ -15,6 +15,12 @@ > >   */ > >  #include > >  #include > > +#include > > + > > +struct rtl8211x_phy_priv { > > +       bool eee_1000t_disable; > > +       bool eee_100tx_disable; > > +}; > > > >  #define RTL821x_PHYSR          0x11 > >  #define RTL821x_PHYSR_DUPLEX   0x2000 > > @@ -93,12 +99,44 @@ static int rtl8211f_config_intr(struct > > phy_device *phydev) > >         return err; > >  } > > > > +static void rtl8211x_clear_eee_adv(struct phy_device *phydev) > > +{ > > +       struct rtl8211x_phy_priv *priv = phydev->priv; > > +       u16 val; > > + > > +       if (priv->eee_1000t_disable || priv->eee_100tx_disable) { > > +               val = phy_read_mmd_indirect(phydev, > > MDIO_AN_EEE_ADV, > > +                                           MDIO_MMD_AN); > > + > > +               if (priv->eee_1000t_disable) > > +                       val &= ~MDIO_AN_EEE_ADV_1000T; > > +               if (priv->eee_100tx_disable) > > +                       val &= ~MDIO_AN_EEE_ADV_100TX; > > + > > +               phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, > > +                                      MDIO_MMD_AN, val); > > +       } > > +} > > + > > +static int rtl8211x_config_init(struct phy_device *phydev) > > +{ > > +       int ret; > > + > > +       ret = genphy_config_init(phydev); > > +       if (ret < 0) > > +               return ret; > > + > > +       rtl8211x_clear_eee_adv(phydev); > > + > > +       return 0; > > +} > > + > >  static int rtl8211f_config_init(struct phy_device *phydev) > >  { > >         int ret; > >         u16 reg; > > > > -       ret = genphy_config_init(phydev); > > +       ret = rtl8211x_config_init(phydev); > >         if (ret < 0) > >                 return ret; > > > > @@ -115,6 +153,26 @@ static int rtl8211f_config_init(struct > > phy_device *phydev) > >         return 0; > >  } > > > > +static int rtl8211x_phy_probe(struct phy_device *phydev) > > +{ > > +       struct device *dev = &phydev->mdio.dev; > > +       struct device_node *of_node = dev->of_node; > > +       struct rtl8211x_phy_priv *priv; > > + > > +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > +       if (!priv) > > +               return -ENOMEM; > > + > > +       priv->eee_1000t_disable = > > +               of_property_read_bool(of_node, "realtek,disable- > > eee-1000t"); > > +       priv->eee_100tx_disable = > > +               of_property_read_bool(of_node, "realtek,disable- > > eee-100tx"); > > + > > +       phydev->priv = priv; > > + > > +       return 0; > > +} > > + > >  static struct phy_driver realtek_drvs[] = { > >         { > >                 .phy_id         = 0x00008201, > > @@ -140,7 +198,9 @@ static struct phy_driver realtek_drvs[] = { > >                 .phy_id_mask    = 0x001fffff, > >                 .features       = PHY_GBIT_FEATURES, > >                 .flags          = PHY_HAS_INTERRUPT, > > +               .probe          = &rtl8211x_phy_probe, > >                 .config_aneg    = genphy_config_aneg, > > +               .config_init    = &rtl8211x_config_init, > >                 .read_status    = genphy_read_status, > >                 .ack_interrupt  = rtl821x_ack_interrupt, > >                 .config_intr    = rtl8211e_config_intr, > > @@ -152,7 +212,9 @@ static struct phy_driver realtek_drvs[] = { > >                 .phy_id_mask    = 0x001fffff, > >                 .features       = PHY_GBIT_FEATURES, > >                 .flags          = PHY_HAS_INTERRUPT, > > +               .probe          = &rtl8211x_phy_probe, > >                 .config_aneg    = &genphy_config_aneg, > > +               .config_init    = &rtl8211x_config_init, > >                 .read_status    = &genphy_read_status, > >                 .ack_interrupt  = &rtl821x_ack_interrupt, > >                 .config_intr    = &rtl8211e_config_intr, > > @@ -164,6 +226,7 @@ static struct phy_driver realtek_drvs[] = { > >                 .phy_id_mask    = 0x001fffff, > >                 .features       = PHY_GBIT_FEATURES, > >                 .flags          = PHY_HAS_INTERRUPT, > > +               .probe          = &rtl8211x_phy_probe, > >                 .config_aneg    = &genphy_config_aneg, > >                 .config_init    = &rtl8211f_config_init, > >                 .read_status    = &genphy_read_status, > > -- > > 2.7.4 > > > > How about adding callback functionality for .soft_reset to handle > BMCR > where we update the Auto-Negotiation for the phy, > as per the datasheet of the rtl8211f. I'm not sure I understand how this would help with our issue (and EEE). Am I missing something or is it something unrelated that you would like to see happening on this driver ?  > > -Best Regard > Anand Moon > > > > > > > _______________________________________________ > > linux-amlogic mailing list > > linux-amlogic@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-amlogic