Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717010 > unrolled thread

[PATCH v2 0/4] Workaround broken RTL8211E on some Pine64+ boards

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] Workaround broken RTL8211E on some Pine64+ boards Icenowy Zheng <icenowy@aosc.io> - 2017-08-22 06:10 +0200
    [PATCH v2 4/4] arm64: allwinner: a64: disable the RTL8211E internal RX delay on Pine64+ Icenowy Zheng <icenowy@aosc.io> - 2017-08-22 06:10 +0200
    [PATCH v2 2/4] net: phy: realtek: change macro name for page select register Icenowy Zheng <icenowy@aosc.io> - 2017-08-22 06:10 +0200
    [PATCH v2 1/4] net: stmmac: dwmac-sun8i: support RGMII modes with PHY internal delay Icenowy Zheng <icenowy@aosc.io> - 2017-08-22 06:10 +0200
    [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay mode Icenowy Zheng <icenowy@aosc.io> - 2017-08-22 06:20 +0200
      Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay  mode Andrew Lunn <andrew@lunn.ch> - 2017-08-22 15:50 +0200
        Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay  mode icenowy@aosc.io - 2017-08-24 10:10 +0200
          Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay  mode Andrew Lunn <andrew@lunn.ch> - 2017-08-24 15:20 +0200

#1717010 — [PATCH v2 0/4] Workaround broken RTL8211E on some Pine64+ boards

Some Pine64+ boards come with bad RTL8211E PHYs, which cannot work reliably
unless do some hack. According to Pine64 people, Realtek describes the hack
as totally disabling RX delay, and it's not documented at all.

This patchset introduces the workaround on Pine64+.

The first patch adds RGMII variants' support to the dwmac-sun8i driver.

The second patch renames some macros in RTL PHY driver, and the third
patch introduces the hack as the "RGMII-TXID" mode of the PHY.

The fourth patch enables the hack in the device tree.

Icenowy Zheng (4):
  net: stmmac: dwmac-sun8i: support RGMII modes with PHY internal delay
  net: phy: realtek: change macro name for page select register
  net: phy: realtek: add disable RX internal delay mode
  arm64: allwinner: a64: disable the RTL8211E internal RX delay on
    Pine64+

 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts  |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  |  3 ++
 drivers/net/phy/realtek.c                          | 43 +++++++++++++++++++---
 3 files changed, 42 insertions(+), 6 deletions(-)

-- 
2.13.5

[toc] | [next] | [standalone]

#1717011 — [PATCH v2 4/4] arm64: allwinner: a64: disable the RTL8211E internal RX delay on Pine64+

Some Pine64+ boards have a broken RTL8211E PHY, which cannot work
reliably in 1000Base-T mode with default configuration.

A solution is passed to Pine64, which is said to be disabling the
internal RX delay of the PHY.

Enable the hack by set the PHY mode to RGMII-TXID.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- It can be merged now as dwmac-sun8i entered mainline.
- Use phy-mode rgmii-txid instead of custom property.

 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
index 24f1aac366d6..ed715426fffc 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
@@ -52,7 +52,7 @@
 &emac {
 	pinctrl-names = "default";
 	pinctrl-0 = <&rgmii_pins>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-txid";
 	phy-handle = <&ext_rgmii_phy>;
 	status = "okay";
 };
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1717012 — [PATCH v2 2/4] net: phy: realtek: change macro name for page select register

From: Icenowy Zheng <icenowy@aosc.xyz>

The page select register also exists on RTL8211E PHY (although it
behaves slightly differently).

Change the register macro name to remove the F.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 drivers/net/phy/realtek.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645e3d89..d820d00addf6 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -22,11 +22,13 @@
 #define RTL821x_INER		0x12
 #define RTL821x_INER_INIT	0x6400
 #define RTL821x_INSR		0x13
+
+#define RTL8211_PAGE_SELECT	0x1f
+
 #define RTL8211E_INER_LINK_STATUS 0x400
 
 #define RTL8211F_INER_LINK_STATUS 0x0010
 #define RTL8211F_INSR		0x1d
-#define RTL8211F_PAGE_SELECT	0x1f
 #define RTL8211F_TX_DELAY	0x100
 
 MODULE_DESCRIPTION("Realtek PHY driver");
@@ -46,10 +48,10 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
 
-	phy_write(phydev, RTL8211F_PAGE_SELECT, 0xa43);
+	phy_write(phydev, RTL8211_PAGE_SELECT, 0xa43);
 	err = phy_read(phydev, RTL8211F_INSR);
 	/* restore to default page 0 */
-	phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+	phy_write(phydev, RTL8211_PAGE_SELECT, 0x0);
 
 	return (err < 0) ? err : 0;
 }
@@ -102,7 +104,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
+	phy_write(phydev, RTL8211_PAGE_SELECT, 0xd08);
 	reg = phy_read(phydev, 0x11);
 
 	/* enable TX-delay for rgmii-id and rgmii-txid, otherwise disable it */
@@ -114,7 +116,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 
 	phy_write(phydev, 0x11, reg);
 	/* restore to default page 0 */
-	phy_write(phydev, RTL8211F_PAGE_SELECT, 0x0);
+	phy_write(phydev, RTL8211_PAGE_SELECT, 0x0);
 
 	return 0;
 }
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1717013 — [PATCH v2 1/4] net: stmmac: dwmac-sun8i: support RGMII modes with PHY internal delay

Some boards uses a PHY with internal delay with an Allwinner SoC.

Support these PHY modes in the driver.

As the driver has no configuration registers for these modes, just treat
them as ordinary RGMII.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index fffd6d5fc907..2af680cac497 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -723,6 +723,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 		/* default */
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
 		reg |= SYSCON_EPIT | SYSCON_ETCS_INT_GMII;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1717015 — [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay mode

From: Icenowy Zheng <icenowy@aosc.xyz>

Some RTL8211E chips have broken GbE function, which needs a hack to
fix. It's said that this fix will affect the performance on not-buggy
PHYs, so it should only be enabled on boards with the broken PHY.
Currently only some Pine64+ boards are known to have this issue.

This hack is said to disable RX relay for RTL8211E according to Realtek.
So implement it as RGMII-TXID mode.

As this hack is not documented on the datasheet at all, it contains
magic numbers, and could not be revealed. These magic numbers are
received from Realtek via Pine64.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
Changes in v2:
- Used RGMII_TXID phy mode.

 drivers/net/phy/realtek.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index d820d00addf6..8306b6abaaa8 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -13,6 +13,7 @@
  * option) any later version.
  *
  */
+#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/module.h>
 
@@ -26,6 +27,8 @@
 #define RTL8211_PAGE_SELECT	0x1f
 
 #define RTL8211E_INER_LINK_STATUS 0x400
+#define RTL8211E_EXT_PAGE_SELECT 0x1e
+#define RTL8211E_EXT_PAGE	0x7
 
 #define RTL8211F_INER_LINK_STATUS 0x0010
 #define RTL8211F_INSR		0x1d
@@ -121,6 +124,33 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8211e_config_init(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	int ret;
+
+	ret = genphy_config_init(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+		/* Disable the RX internal delay here.
+		 *
+		 * All the magic numbers are not documented on RTL8211E
+		 * datasheet. They're said to be from Realtek by Pine64.
+		 */
+		phy_write(phydev, RTL8211_PAGE_SELECT, RTL8211E_EXT_PAGE);
+		phy_write(phydev, RTL8211E_EXT_PAGE_SELECT, 0xa4);
+		phy_write(phydev, 0x1c, 0xb591);
+
+		/* Restore to default page 0 */
+		phy_write(phydev, RTL8211_PAGE_SELECT, 0);
+	}
+
+	return 0;
+}
+
 static struct phy_driver realtek_drvs[] = {
 	{
 		.phy_id         = 0x00008201,
@@ -159,6 +189,7 @@ static struct phy_driver realtek_drvs[] = {
 		.features	= PHY_GBIT_FEATURES,
 		.flags		= PHY_HAS_INTERRUPT,
 		.config_aneg	= &genphy_config_aneg,
+		.config_init	= rtl8211e_config_init,
 		.read_status	= &genphy_read_status,
 		.ack_interrupt	= &rtl821x_ack_interrupt,
 		.config_intr	= &rtl8211e_config_intr,
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1717429 — Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay mode

On Tue, Aug 22, 2017 at 12:03:59PM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Some RTL8211E chips have broken GbE function, which needs a hack to
> fix. It's said that this fix will affect the performance on not-buggy
> PHYs, so it should only be enabled on boards with the broken PHY.

I would not call this a broken PHY. It is a question of board
design. If you have shorter tracks, you need a delay. If you have long
tracks, you don't. Hence the four RGMII modes, so you can select if
the delay is needed or not, depending on your board design.

> Currently only some Pine64+ boards are known to have this issue.

Design feature.

Please remove all reference to bug.
 
> This hack is said to disable RX relay for RTL8211E according to Realtek.
> So implement it as RGMII-TXID mode.

Do we know that the TX lines do have a delay after making this change?
We need to be careful here. We will get into a mess if this is
actually RGMII not RGMII_TXID, and somebody designs a board which
needs RGMII-TXID.

> +#include <linux/of.h>
>  #include <linux/phy.h>
>  #include <linux/module.h>

> +static int rtl8211e_config_init(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct device_node *of_node = dev->of_node;

You don't appear to use of_node, nor dev. Please remove, along with
the header file.

    Andrew

[toc] | [prev] | [next] | [standalone]

#1718942 — Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay mode

在 2017-08-22 21:39，Andrew Lunn 写道：
> On Tue, Aug 22, 2017 at 12:03:59PM +0800, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy@aosc.xyz>
>> 
>> Some RTL8211E chips have broken GbE function, which needs a hack to
>> fix. It's said that this fix will affect the performance on not-buggy
>> PHYs, so it should only be enabled on boards with the broken PHY.
> 
> I would not call this a broken PHY. It is a question of board
> design. If you have shorter tracks, you need a delay. If you have long
> tracks, you don't. Hence the four RGMII modes, so you can select if
> the delay is needed or not, depending on your board design.
> 
>> Currently only some Pine64+ boards are known to have this issue.
> 
> Design feature.
> 
> Please remove all reference to bug.
> 
>> This hack is said to disable RX relay for RTL8211E according to 
>> Realtek.
>> So implement it as RGMII-TXID mode.
> 
> Do we know that the TX lines do have a delay after making this change?
> We need to be careful here. We will get into a mess if this is
> actually RGMII not RGMII_TXID, and somebody designs a board which
> needs RGMII-TXID.

In fact I'm not sure -- I'm even not sure that after making this change
there's no TX delay. The change is totally not documented, so I choose
to use a custom property in v1.

But Florian required me to use RGMII-TXID instead.

> 
>> +#include <linux/of.h>
>>  #include <linux/phy.h>
>>  #include <linux/module.h>
> 
>> +static int rtl8211e_config_init(struct phy_device *phydev)
>> +{
>> +	struct device *dev = &phydev->mdio.dev;
>> +	struct device_node *of_node = dev->of_node;
> 
> You don't appear to use of_node, nor dev. Please remove, along with
> the header file.
> 
>     Andrew
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [prev] | [next] | [standalone]

#1719229 — Re: [PATCH v2 3/4] net: phy: realtek: add disable RX internal delay mode

> >Do we know that the TX lines do have a delay after making this change?
> >We need to be careful here. We will get into a mess if this is
> >actually RGMII not RGMII_TXID, and somebody designs a board which
> >needs RGMII-TXID.
> 
> In fact I'm not sure -- I'm even not sure that after making this change
> there's no TX delay. The change is totally not documented, so I choose
> to use a custom property in v1.

Can you get in touch with Realtek and ask? Or can you get a hardware
guy to do some measurements?

Thanks
    Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web