Re: [PATCH] net: phy: realtek: Add tx/rx delay config for 8211e

[Ramon Fried <rfried.dev@gmail.com>]

On Sat, Oct 16, 2021 at 9:30 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Wed, Oct 13, 2021 at 5:07 AM Samuel Holland <samuel@sholland.org> wrote:
> >
> > Some boards need to change the tx/rx delay config in order for
> > gigabit Ethernet to work.
> >
> > In Linux commit bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx
> > delay config"), Realtek documented the bits for overriding the delays
> > from the hardware straps.
> >
> > Copy the logic from linux, so the delay config is set from the PHY's
> > interface type (the phy-mode property in the device tree).
> >
> > This removes the need for a one-off workaround for the Pine A64+ board.
> >
> > Signed-off-by: Samuel Holland <samuel@sholland.org>
> > ---
> > This change is needed for the Allwinner D1 Nezha board, which has
> > incorrect hardware straps.
> >
> >  configs/pine64_plus_defconfig |  1 -
> >  drivers/net/phy/Kconfig       | 10 -----
> >  drivers/net/phy/realtek.c     | 69 ++++++++++++++++++++++-------------
> >  3 files changed, 43 insertions(+), 37 deletions(-)
> >
> > diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig
> > index d1c2c3c3cc..f42f4e5923 100644
> > --- a/configs/pine64_plus_defconfig
> > +++ b/configs/pine64_plus_defconfig
> > @@ -8,7 +8,6 @@ CONFIG_PINE64_DT_SELECTION=y
> >  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> >  CONFIG_OF_LIST="sun50i-a64-pine64 sun50i-a64-pine64-plus"
> >  CONFIG_PHY_REALTEK=y
> > -CONFIG_RTL8211E_PINE64_GIGABIT_FIX=y
> >  CONFIG_SUN8I_EMAC=y
> >  CONFIG_USB_EHCI_HCD=y
> >  CONFIG_USB_OHCI_HCD=y
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 68ee7d7a2d..e69cd8a4b3 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -214,16 +214,6 @@ config PHY_NXP_C45_TJA11XX
> >  config PHY_REALTEK
> >         bool "Realtek Ethernet PHYs support"
> >
> > -config RTL8211E_PINE64_GIGABIT_FIX
> > -       bool "Fix gigabit throughput on some Pine64+ models"
> > -       depends on PHY_REALTEK
> > -       help
> > -         Configure the Realtek RTL8211E found on some Pine64+ models differently to
> > -         fix throughput on Gigabit links, turning off all internal delays in the
> > -         process. The settings that this touches are not documented in the CONFREG
> > -         section of the RTL8211E datasheet, but come from Realtek by way of the
> > -         Pine64 engineering team.
> > -
> >  config RTL8211X_PHY_FORCE_MASTER
> >         bool "Ethernet PHY RTL8211x: force 1000BASE-T master mode"
> >         depends on PHY_REALTEK
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index b1b1fa5080..84c755525f 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -12,7 +12,6 @@
> >  #include <linux/delay.h>
> >
> >  #define PHY_RTL8211x_FORCE_MASTER BIT(1)
> > -#define PHY_RTL8211E_PINE64_GIGABIT_FIX BIT(2)
> >  #define PHY_RTL8211F_FORCE_EEE_RXC_ON BIT(3)
> >  #define PHY_RTL8201F_S700_RMII_TIMINGS BIT(4)
> >
> > @@ -49,10 +48,10 @@
> >  #define MIIM_RTL8211F_PHYSTAT_SPDDONE  0x0800
> >  #define MIIM_RTL8211F_PHYSTAT_LINK     0x0004
> >
> > -#define MIIM_RTL8211E_CONFREG           0x1c
> > -#define MIIM_RTL8211E_CONFREG_TXD              0x0002
> > -#define MIIM_RTL8211E_CONFREG_RXD              0x0004
> > -#define MIIM_RTL8211E_CONFREG_MAGIC            0xb400  /* Undocumented */
> > +#define MIIM_RTL8211E_CONFREG          0x1c
> > +#define MIIM_RTL8211E_CTRL_DELAY       BIT(13)
> > +#define MIIM_RTL8211E_TX_DELAY         BIT(12)
> > +#define MIIM_RTL8211E_RX_DELAY         BIT(11)
> >
> >  #define MIIM_RTL8211E_EXT_PAGE_SELECT  0x1e
> >
> > @@ -108,10 +107,6 @@ static int rtl8211b_probe(struct phy_device *phydev)
> >
> >  static int rtl8211e_probe(struct phy_device *phydev)
> >  {
> > -#ifdef CONFIG_RTL8211E_PINE64_GIGABIT_FIX
> > -       phydev->flags |= PHY_RTL8211E_PINE64_GIGABIT_FIX;
> > -#endif
> > -
> >         return 0;
> >  }
> >
> > @@ -154,22 +149,6 @@ static int rtl8211x_config(struct phy_device *phydev)
> >                 reg |= MIIM_RTL8211x_CTRL1000T_MASTER;
> >                 phy_write(phydev, MDIO_DEVAD_NONE, MII_CTRL1000, reg);
> >         }
> > -       if (phydev->flags & PHY_RTL8211E_PINE64_GIGABIT_FIX) {
> > -               unsigned int reg;
> > -
> > -               phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_PAGE_SELECT,
> > -                         7);
> > -               phy_write(phydev, MDIO_DEVAD_NONE,
> > -                         MIIM_RTL8211E_EXT_PAGE_SELECT, 0xa4);
> > -               reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211E_CONFREG);
> > -               /* Ensure both internal delays are turned off */
> > -               reg &= ~(MIIM_RTL8211E_CONFREG_TXD | MIIM_RTL8211E_CONFREG_RXD);
> > -               /* Flip the magic undocumented bits */
> > -               reg |= MIIM_RTL8211E_CONFREG_MAGIC;
> > -               phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211E_CONFREG, reg);
> > -               phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_PAGE_SELECT,
> > -                         0);
> > -       }
> >         /* read interrupt status just to clear it */
> >         phy_read(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211x_PHY_INER);
> >
> > @@ -201,6 +180,44 @@ static int rtl8201f_config(struct phy_device *phydev)
> >         return 0;
> >  }
> >
> > +static int rtl8211e_config(struct phy_device *phydev)
> > +{
> > +       int reg, val;
> > +
> > +       /* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */
> > +       switch (phydev->interface) {
> > +       case PHY_INTERFACE_MODE_RGMII:
> > +               val = MIIM_RTL8211E_CTRL_DELAY;
> > +               break;
> > +       case PHY_INTERFACE_MODE_RGMII_ID:
> > +               val = MIIM_RTL8211E_CTRL_DELAY | MIIM_RTL8211E_TX_DELAY |
> > +                     MIIM_RTL8211E_RX_DELAY;
> > +               break;
> > +       case PHY_INTERFACE_MODE_RGMII_RXID:
> > +               val = MIIM_RTL8211E_CTRL_DELAY | MIIM_RTL8211E_TX_DELAY;
> > +               break;
> > +       case PHY_INTERFACE_MODE_RGMII_TXID:
> > +               val = MIIM_RTL8211E_CTRL_DELAY | MIIM_RTL8211E_RX_DELAY;
> > +               break;
> > +       default: /* the rest of the modes imply leaving delays as is. */
> > +               goto default_delay;
> > +       }
> > +
> > +       phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_PAGE_SELECT, 7);
> > +       phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211E_EXT_PAGE_SELECT, 0xa4);
> > +
> > +       reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211E_CONFREG);
> > +       reg &= ~(MIIM_RTL8211E_TX_DELAY | MIIM_RTL8211E_RX_DELAY);
> > +       phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211E_CONFREG, reg | val);
> > +
> > +       phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_PAGE_SELECT, 0);
> > +
> > +default_delay:
> > +       genphy_config_aneg(phydev);
> > +
> > +       return 0;
> > +}
> > +
> >  static int rtl8211f_config(struct phy_device *phydev)
> >  {
> >         u16 reg;
> > @@ -410,7 +427,7 @@ static struct phy_driver RTL8211E_driver = {
> >         .mask = 0xffffff,
> >         .features = PHY_GBIT_FEATURES,
> >         .probe = &rtl8211e_probe,
> > -       .config = &rtl8211x_config,
> > +       .config = &rtl8211e_config,
> >         .startup = &rtl8211e_startup,
> >         .shutdown = &genphy_shutdown,
> >  };
> > --
> > 2.32.0
> >
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
applied to u-boot-net/next
Thanks,
Ramon.