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

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

[Samuel Holland]

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

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

[Ramon Fried]

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>

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

[Ramon Fried]

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.

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Tue, Oct 12, 2021 at 09:07:32PM -0500, Samuel Holland 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.

OK, so this breaks my pine64+.  It's the 1GB memory model from the
original Kickstarter with an 8211e on it.  My dhcp just times out now,
with this applied.  Let me know if there's some specifics you want me to
try for debug or talk to me off-list and I can try and setup access to
the board for you.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Andre Przywara]

On Thu, 4 Nov 2021 09:13:49 -0400
Tom Rini <trini@konsulko.com> wrote:

Hi Tom,

> On Tue, Oct 12, 2021 at 09:07:32PM -0500, Samuel Holland 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.  
> 
> OK, so this breaks my pine64+.  It's the 1GB memory model from the
> original Kickstarter with an 8211e on it.  My dhcp just times out now,
> with this applied.  Let me know if there's some specifics you want me to
> try for debug or talk to me off-list and I can try and setup access to
> the board for you.

Many thanks for testing and the report. I will have a closer look tonight,
I should have quite some boards to be able to reproduce it.

Thanks,
Andre

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]

On Thu, Nov 04, 2021 at 01:40:42PM +0000, Andre Przywara wrote:
> On Thu, 4 Nov 2021 09:13:49 -0400
> Tom Rini <trini@konsulko.com> wrote:
> 
> Hi Tom,
> 
> > On Tue, Oct 12, 2021 at 09:07:32PM -0500, Samuel Holland 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.  
> > 
> > OK, so this breaks my pine64+.  It's the 1GB memory model from the
> > original Kickstarter with an 8211e on it.  My dhcp just times out now,
> > with this applied.  Let me know if there's some specifics you want me to
> > try for debug or talk to me off-list and I can try and setup access to
> > the board for you.
> 
> Many thanks for testing and the report. I will have a closer look tonight,
> I should have quite some boards to be able to reproduce it.

Ah good.  And yes, good news / bad news, this board is now in my
every-PR local CI HW loop :)

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Ramon Fried]

On Thu, Nov 4, 2021 at 3:42 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Nov 04, 2021 at 01:40:42PM +0000, Andre Przywara wrote:
> > On Thu, 4 Nov 2021 09:13:49 -0400
> > Tom Rini <trini@konsulko.com> wrote:
> >
> > Hi Tom,
> >
> > > On Tue, Oct 12, 2021 at 09:07:32PM -0500, Samuel Holland 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.
> > >
> > > OK, so this breaks my pine64+.  It's the 1GB memory model from the
> > > original Kickstarter with an 8211e on it.  My dhcp just times out now,
> > > with this applied.  Let me know if there's some specifics you want me to
> > > try for debug or talk to me off-list and I can try and setup access to
> > > the board for you.
> >
> > Many thanks for testing and the report. I will have a closer look tonight,
> > I should have quite some boards to be able to reproduce it.
>
> Ah good.  And yes, good news / bad news, this board is now in my
> every-PR local CI HW loop :)
>
> --
> Tom
I'm dropping the patch, will check it again once v2 is uploaded.