From: Ramon Fried <rfried.dev@gmail.com>
To: Samuel Holland <samuel@sholland.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
Joe Hershberger <joe.hershberger@ni.com>,
Jagan Teki <jagan@amarulasolutions.com>,
Andre Przywara <andre.przywara@arm.com>,
Abbie Chang <abbie.chang@cortina-access.com>,
Bin Meng <bmeng.cn@gmail.com>,
Kuldeep Singh <kuldeep.singh@nxp.com>,
Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>,
Michal Simek <michal.simek@xilinx.com>,
Priyanka Jain <priyanka.jain@nxp.com>,
"Radu Pirea (NXP OSS)" <radu-nicolae.pirea@oss.nxp.com>
Subject: Re: [PATCH] net: phy: realtek: Add tx/rx delay config for 8211e
Date: Thu, 28 Oct 2021 21:48:45 +0300 [thread overview]
Message-ID: <CAGi-RULGTio60fXZbOq7a13J8H8Lx5vDs2SPOusEer2kwBKb_A@mail.gmail.com> (raw)
In-Reply-To: <CAGi-RUL+axtYwCLcT+T6Vx7rwOiwu3oTv2V4wyHOqfaOEKSF_Q@mail.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.
next prev parent reply other threads:[~2021-10-28 18:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-13 2:07 [PATCH] net: phy: realtek: Add tx/rx delay config for 8211e Samuel Holland
2021-10-16 18:30 ` Ramon Fried
2021-10-28 18:48 ` Ramon Fried [this message]
2021-11-04 13:13 ` Tom Rini
2021-11-04 13:40 ` Andre Przywara
2021-11-04 13:42 ` Tom Rini
2021-11-10 16:31 ` Ramon Fried
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGi-RULGTio60fXZbOq7a13J8H8Lx5vDs2SPOusEer2kwBKb_A@mail.gmail.com \
--to=rfried.dev@gmail.com \
--cc=abbie.chang@cortina-access.com \
--cc=andre.przywara@arm.com \
--cc=bmeng.cn@gmail.com \
--cc=jagan@amarulasolutions.com \
--cc=joe.hershberger@ni.com \
--cc=kuldeep.singh@nxp.com \
--cc=meenakshi.aggarwal@nxp.com \
--cc=michal.simek@xilinx.com \
--cc=priyanka.jain@nxp.com \
--cc=radu-nicolae.pirea@oss.nxp.com \
--cc=samuel@sholland.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).