Re: [PATCH net 1/3] net: phy: realtek: add eee advertisement disable options

[Jerome Brunet]

On Wed, 2016-11-16 at 22:36 +0530, Anand Moon wrote:
> ÂHi Jerome.
> 
> On 15 November 2016 at 19:59, Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> 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 <martin.blumenstingl@xxxxxxxxxxxxx
> > m>
> > Cc: Giuseppe Cavallaro <peppe.cavallaro@xxxxxx>
> > Cc: Alexandre TORGUE <alexandre.torgue@xxxxxx>
> > Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> > Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> > Tested-by: Andre Roth <neolynx@xxxxxxxxx>
> > ---
> > Â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 <linux/phy.h>
> > Â#include <linux/module.h>
> > +#include <linux/of.h>
> > +
> > +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@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-amlogic