[PATCH 0/4] RTL8211E-specified hacks

[PATCH 0/4] RTL8211E-specified hacks

[Icenowy Zheng]

Some Pine64 boards are reported to have broken RTL8211E PHYs, which will fail
to work at 1000BASE-T mode if not workarounded.

The workaround is retrieved from Pine64, and is said to be from Realtek
engineer. It's undocumented but effective. (Tested on my Pine64 with GbE
broken)

The first patch is a small tweak for Realtek PHY driver, which removed the
"F" in page select register name, as RTL8211E also uses the same register
as page select (although with some different difinition).

The second patch adds a binding for the PHY, specified for this hack.

The third patch is the real driver part of this hack, which contains
some magic numbers from Pine64/Realtek.

The fourth patch is for reference only and should not be merged -- to
use it you will need sun8i-emac or dwmac-sun8i patchset applied.

Icenowy Zheng (4):
  net: phy: realtek: change macro name for page select register
  dt-bindings: add binding for RTL8211E Ethernet PHY
  net: phy: realtek: add disable RX delay hack for RTL8211E
  [DO NOT MERGE] arm64: allwinner: a64: enable RTL8211E PHY workaround

 .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22 ++++++++++
 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts  |  4 ++
 drivers/net/phy/realtek.c                          | 48 +++++++++++++++++++---
 3 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl8211e.txt

-- 
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] net: phy: realtek: change macro name for page select register

[Icenowy Zheng]

From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

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-ymACFijhrKM@public.gmane.org>
---
 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.12.2

[PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Icenowy Zheng]

From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

Some RTL8211E Ethernet PHY have an issue that needs a workaround
indicated with device tree.

Add the binding for a property that indicates this workaround.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl8211e.txt

diff --git a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
new file mode 100644
index 000000000000..c1913301bfe8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
@@ -0,0 +1,22 @@
+Realtek RTL8211E Ethernet PHY
+
+One batch of RTL8211E is slight broken, that needs some special (and
+full of magic numbers) tweaking in order to make GbE to operate properly.
+The only well-known board that used the broken batch is Pine64+.
+Configure it through an Ethernet OF device node.
+
+Optional properties:
+
+- realtek,disable-rx-delay:
+  If set, RX delay will be completely disabled (according to Realtek). This
+  will affect the performance on non-broken boards.
+  default: do not disable RX delay.
+
+Examples:
+Pine64+ with broken RTL8211E:
+&mdio {
+	ext_phy: ethernet-phy@0 {
+		reg = <0>;
+		realtek,disable-rx-delay;
+	};
+};
-- 
2.12.2

[PATCH 3/4] net: phy: realtek: add disable RX delay hack for RTL8211E

[Icenowy Zheng]

From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

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.

Enable this hack when a certain device tree property is set.

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-ymACFijhrKM@public.gmane.org>
---
 drivers/net/phy/realtek.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index d820d00addf6..880022160cd2 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,38 @@ 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 (of_node &&
+	    of_property_read_bool(of_node, "realtek,disable-rx-delay")) {
+		/* All these magic numbers are retrieved from Pine64, and
+		 * they're said to be originated from Realtek.
+		 *
+		 * The datasheet of RTL8211E didn't cover this ext page.
+		 *
+		 * Select extension page 0xa4 here.
+		 */
+		phy_write(phydev, RTL8211_PAGE_SELECT, RTL8211E_EXT_PAGE)
+		phy_write(phydev, RTL8211E_EXT_PAGE_SELECT, 0xa4);
+
+		/* Write the magic number */
+		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 +194,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.12.2

[PATCH 4/4] [DO NOT MERGE] arm64: allwinner: a64: enable RTL8211E PHY workaround

[Icenowy Zheng]

From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

Some Pine64+ boards are said to have broken RTL8211E PHY.

Enable the workaround in Pine64+ device tree file.

Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts | 4 ++++
 1 file changed, 4 insertions(+)

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 790d14daaa6a..1f59ee4f8b45 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
@@ -48,3 +48,7 @@
 
 	/* TODO: Camera, Ethernet PHY, touchscreen, etc. */
 };
+
+&ext_phy {
+	realtek,disable-rx-delay;
+};
-- 
2.12.2

Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Florian Fainelli]

On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> 
> Some RTL8211E Ethernet PHY have an issue that needs a workaround
> indicated with device tree.
> 
> Add the binding for a property that indicates this workaround.
> 
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> ---
>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
> new file mode 100644
> index 000000000000..c1913301bfe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
> @@ -0,0 +1,22 @@
> +Realtek RTL8211E Ethernet PHY
> +
> +One batch of RTL8211E is slight broken, that needs some special (and
> +full of magic numbers) tweaking in order to make GbE to operate properly.
> +The only well-known board that used the broken batch is Pine64+.
> +Configure it through an Ethernet OF device node.
> +
> +Optional properties:
> +
> +- realtek,disable-rx-delay:
> +  If set, RX delay will be completely disabled (according to Realtek). This
> +  will affect the performance on non-broken boards.
> +  default: do not disable RX delay.

Please don't introduce custom properties to do that, instead correct
specify the "phy-mode" such that it is e.g: "rgmii-txid" which indicates
that there should be no RX internal delay, but a TX internal delay added
by the PHY.
-- 
Florian

Re: [PATCH 3/4] net: phy: realtek: add disable RX delay hack for RTL8211E

[Florian Fainelli]

On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> 
> 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.
> 
> Enable this hack when a certain device tree property is set.
> 
> 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-ymACFijhrKM@public.gmane.org>
> ---
>  drivers/net/phy/realtek.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index d820d00addf6..880022160cd2 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,38 @@ 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 (of_node &&
> +	    of_property_read_bool(of_node, "realtek,disable-rx-delay")) {
> +		/* All these magic numbers are retrieved from Pine64, and
> +		 * they're said to be originated from Realtek.
> +		 *
> +		 * The datasheet of RTL8211E didn't cover this ext page.
> +		 *
> +		 * Select extension page 0xa4 here.
> +		 */
> +		phy_write(phydev, RTL8211_PAGE_SELECT, RTL8211E_EXT_PAGE)
> +		phy_write(phydev, RTL8211E_EXT_PAGE_SELECT, 0xa4);
> +
> +		/* Write the magic number */
> +		phy_write(phydev, 0x1c, 0xb591);
> +
> +		/* Restore to default page 0 */
> +		phy_write(phydev, RTL8211_PAGE_SELECT, 0);
> +	}

The code itself is probably fine, but the way you are checking whether
the RX delay should be disabled is not. As mentioned in patch 2, using
the correct "phy-mode" property would translate in the proper
phydev->interface value here (presumably PHY_INTERFACE_MODE_RGMII_TXID)
is that you want here that would allow you to check whether the RX delay
should, or should not be disabled.

Thank you
-- 
Florian

Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[icenowy-h8G6r0blFSE]

在 2017-04-22 08:22，Florian Fainelli 写道：
> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> 
>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>> indicated with device tree.
>> 
>> Add the binding for a property that indicates this workaround.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> ---
>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22 
>> ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt 
>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> new file mode 100644
>> index 000000000000..c1913301bfe8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> @@ -0,0 +1,22 @@
>> +Realtek RTL8211E Ethernet PHY
>> +
>> +One batch of RTL8211E is slight broken, that needs some special (and
>> +full of magic numbers) tweaking in order to make GbE to operate 
>> properly.
>> +The only well-known board that used the broken batch is Pine64+.
>> +Configure it through an Ethernet OF device node.
>> +
>> +Optional properties:
>> +
>> +- realtek,disable-rx-delay:
>> +  If set, RX delay will be completely disabled (according to 
>> Realtek). This
>> +  will affect the performance on non-broken boards.
>> +  default: do not disable RX delay.
> 
> Please don't introduce custom properties to do that, instead correct
> specify the "phy-mode" such that it is e.g: "rgmii-txid" which 
> indicates
> that there should be no RX internal delay, but a TX internal delay 
> added
> by the PHY.

According to Realtek and Pine64 people, set that register is equal to
hardwire RDDLY line to low.

Is it the standard definition of RGMII-TXID?

(P.S. as it's only provided as a workaround, there's no implementation
known for RGMII-RXID and RGMII-ID)

Thanks,
Icenowy

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 3/4] net: phy: realtek: add disable RX delay hack for RTL8211E

[kbuild test robot]

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

Hi Icenowy,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Icenowy-Zheng/net-phy-realtek-change-macro-name-for-page-select-register/20170422-144641
config: i386-randconfig-x070-201716 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/net/phy/realtek.c: In function 'rtl8211e_config_init':
>> drivers/net/phy/realtek.c:147:3: error: expected ';' before 'phy_write'
      phy_write(phydev, RTL8211E_EXT_PAGE_SELECT, 0xa4);
      ^~~~~~~~~

vim +147 drivers/net/phy/realtek.c

   141			 *
   142			 * The datasheet of RTL8211E didn't cover this ext page.
   143			 *
   144			 * Select extension page 0xa4 here.
   145			 */
   146			phy_write(phydev, RTL8211_PAGE_SELECT, RTL8211E_EXT_PAGE)
 > 147			phy_write(phydev, RTL8211E_EXT_PAGE_SELECT, 0xa4);
   148	
   149			/* Write the magic number */
   150			phy_write(phydev, 0x1c, 0xb591);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24280 bytes --]

Re: [PATCH 4/4] [DO NOT MERGE] arm64: allwinner: a64: enable RTL8211E PHY workaround

[kbuild test robot]

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

Hi Icenowy,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Icenowy-Zheng/net-phy-realtek-change-macro-name-for-page-select-register/20170422-144641
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts:52.1-9 Label or path ext_phy not found
>> FATAL ERROR: Syntax error parsing input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34529 bytes --]

Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[icenowy-h8G6r0blFSE]

在 2017-04-22 08:22，Florian Fainelli 写道：
> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> 
>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>> indicated with device tree.
>> 
>> Add the binding for a property that indicates this workaround.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> ---
>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22 
>> ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt 
>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> new file mode 100644
>> index 000000000000..c1913301bfe8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>> @@ -0,0 +1,22 @@
>> +Realtek RTL8211E Ethernet PHY
>> +
>> +One batch of RTL8211E is slight broken, that needs some special (and
>> +full of magic numbers) tweaking in order to make GbE to operate 
>> properly.
>> +The only well-known board that used the broken batch is Pine64+.
>> +Configure it through an Ethernet OF device node.
>> +
>> +Optional properties:
>> +
>> +- realtek,disable-rx-delay:
>> +  If set, RX delay will be completely disabled (according to 
>> Realtek). This
>> +  will affect the performance on non-broken boards.
>> +  default: do not disable RX delay.
> 
> Please don't introduce custom properties to do that, instead correct
> specify the "phy-mode" such that it is e.g: "rgmii-txid" which 
> indicates
> that there should be no RX internal delay, but a TX internal delay 
> added
> by the PHY.

Checked the document, the meaning of "rgmii-txid" is not correct here.

This doesn't effect the MAC, and the MAC should still add TX delay.

The definition of "rgmii-txid" in
Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
internal TX delay provided by the PHY, the MAC should not add an TX 
delay
in this case". However, this do not indicate that the MAC doesn't add TX
delay; in fact that just totally disabled the PHY to provide the RX 
delay.
MAC still should to add delay on both TX/RX, which is the semantic of
standard "rgmii".

So I cannot used "rgmii-txid" here, but should continue to use this
custom property.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Florian Fainelli]

On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
> 在 2017-04-22 08:22，Florian Fainelli 写道：
>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>
>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>> indicated with device tree.
>>>
>>> Add the binding for a property that indicates this workaround.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>> ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>> new file mode 100644
>>> index 000000000000..c1913301bfe8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>> @@ -0,0 +1,22 @@
>>> +Realtek RTL8211E Ethernet PHY
>>> +
>>> +One batch of RTL8211E is slight broken, that needs some special (and
>>> +full of magic numbers) tweaking in order to make GbE to operate
>>> properly.
>>> +The only well-known board that used the broken batch is Pine64+.
>>> +Configure it through an Ethernet OF device node.
>>> +
>>> +Optional properties:
>>> +
>>> +- realtek,disable-rx-delay:
>>> +  If set, RX delay will be completely disabled (according to
>>> Realtek). This
>>> +  will affect the performance on non-broken boards.
>>> +  default: do not disable RX delay.
>>
>> Please don't introduce custom properties to do that, instead correct
>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which indicates
>> that there should be no RX internal delay, but a TX internal delay added
>> by the PHY.
> 
> Checked the document, the meaning of "rgmii-txid" is not correct here.
> 
> This doesn't effect the MAC, and the MAC should still add TX delay.
> 
> The definition of "rgmii-txid" in
> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
> internal TX delay provided by the PHY, the MAC should not add an TX delay
> in this case". However, this do not indicate that the MAC doesn't add TX
> delay; in fact that just totally disabled the PHY to provide the RX delay.
> MAC still should to add delay on both TX/RX, which is the semantic of
> standard "rgmii".
> 
> So I cannot used "rgmii-txid" here, but should continue to use this
> custom property.

This is absolutely not a correct understanding. The 'phy-mode' property
defines the contract between the MAC and PHY. It is defined from the
PHY's perspective of the delay, which means that the MAC has to either
also provide an adequate delay (RX or TX) or not (RX or TX). So if you
specified 'phy-mode' = "rgmii" this means that the MAC needs to adds the
TX and RX delay, so implcitly this means that your MAC operates in
"rgmii-id", if the property was defined from the perspective of the MAC,
which it is not.

Both the Ethernet PHY driver and the MAC driver need to take care of
adjusting the delays based on the phydev->interface value.

The property you are introducing here is absolutely not appropriate
because it is entirely redundant with what 'phy-mode' already defines,
except the latter also covers a lot more cases.
-- 
Florian

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Icenowy Zheng]

于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>> 在 2017-04-22 08:22，Florian Fainelli 写道：
>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>
>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>> indicated with device tree.
>>>>
>>>> Add the binding for a property that indicates this workaround.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>> ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> new file mode 100644
>>>> index 000000000000..c1913301bfe8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> @@ -0,0 +1,22 @@
>>>> +Realtek RTL8211E Ethernet PHY
>>>> +
>>>> +One batch of RTL8211E is slight broken, that needs some special
>(and
>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>> properly.
>>>> +The only well-known board that used the broken batch is Pine64+.
>>>> +Configure it through an Ethernet OF device node.
>>>> +
>>>> +Optional properties:
>>>> +
>>>> +- realtek,disable-rx-delay:
>>>> +  If set, RX delay will be completely disabled (according to
>>>> Realtek). This
>>>> +  will affect the performance on non-broken boards.
>>>> +  default: do not disable RX delay.
>>>
>>> Please don't introduce custom properties to do that, instead correct
>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>indicates
>>> that there should be no RX internal delay, but a TX internal delay
>added
>>> by the PHY.
>> 
>> Checked the document, the meaning of "rgmii-txid" is not correct
>here.
>> 
>> This doesn't effect the MAC, and the MAC should still add TX delay.
>> 
>> The definition of "rgmii-txid" in
>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>> internal TX delay provided by the PHY, the MAC should not add an TX
>delay
>> in this case". However, this do not indicate that the MAC doesn't add
>TX
>> delay; in fact that just totally disabled the PHY to provide the RX
>delay.
>> MAC still should to add delay on both TX/RX, which is the semantic of
>> standard "rgmii".
>> 
>> So I cannot used "rgmii-txid" here, but should continue to use this
>> custom property.
>
>This is absolutely not a correct understanding. The 'phy-mode' property
>defines the contract between the MAC and PHY. It is defined from the
>PHY's perspective of the delay, which means that the MAC has to either
>also provide an adequate delay (RX or TX) or not (RX or TX). So if you
>specified 'phy-mode' = "rgmii" this means that the MAC needs to adds
>the
>TX and RX delay, so implcitly this means that your MAC operates in

The MAC doesn't lose its responsibility to tweak RX/TX delays with this property set.

This situation is that, the PHY's RX delay tweaking function is broken. But it doesn't mean that the PHY can take over *all* responsibility to tweak TX, it still needs MAC to tweak TX.

>"rgmii-id", if the property was defined from the perspective of the
>MAC,
>which it is not.
>
>Both the Ethernet PHY driver and the MAC driver need to take care of
>adjusting the delays based on the phydev->interface value.
>
>The property you are introducing here is absolutely not appropriate
>because it is entirely redundant with what 'phy-mode' already defines,
>except the latter also covers a lot more cases.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Icenowy Zheng]

于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>> 在 2017-04-22 08:22，Florian Fainelli 写道：
>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>
>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>> indicated with device tree.
>>>>
>>>> Add the binding for a property that indicates this workaround.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>> ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> new file mode 100644
>>>> index 000000000000..c1913301bfe8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>> @@ -0,0 +1,22 @@
>>>> +Realtek RTL8211E Ethernet PHY
>>>> +
>>>> +One batch of RTL8211E is slight broken, that needs some special
>(and
>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>> properly.
>>>> +The only well-known board that used the broken batch is Pine64+.
>>>> +Configure it through an Ethernet OF device node.
>>>> +
>>>> +Optional properties:
>>>> +
>>>> +- realtek,disable-rx-delay:
>>>> +  If set, RX delay will be completely disabled (according to
>>>> Realtek). This
>>>> +  will affect the performance on non-broken boards.
>>>> +  default: do not disable RX delay.
>>>
>>> Please don't introduce custom properties to do that, instead correct
>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>indicates
>>> that there should be no RX internal delay, but a TX internal delay
>added
>>> by the PHY.
>> 
>> Checked the document, the meaning of "rgmii-txid" is not correct
>here.
>> 
>> This doesn't effect the MAC, and the MAC should still add TX delay.
>> 
>> The definition of "rgmii-txid" in
>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>> internal TX delay provided by the PHY, the MAC should not add an TX
>delay
>> in this case". However, this do not indicate that the MAC doesn't add
>TX
>> delay; in fact that just totally disabled the PHY to provide the RX
>delay.
>> MAC still should to add delay on both TX/RX, which is the semantic of
>> standard "rgmii".
>> 
>> So I cannot used "rgmii-txid" here, but should continue to use this
>> custom property.
>
>This is absolutely not a correct understanding. The 'phy-mode' property
>defines the contract between the MAC and PHY. It is defined from the
>PHY's perspective of the delay, which means that the MAC has to either
>also provide an adequate delay (RX or TX) or not (RX or TX). So if you
>specified 'phy-mode' = "rgmii" this means that the MAC needs to adds
>the
>TX and RX delay, so implcitly this means that your MAC operates in

The MAC doesn't lose its responsibility to tweak RX/TX delays with this property set.

This situation is that, the PHY's RX delay tweaking function is broken. But it doesn't mean that the PHY can take over *all* responsibility to tweak TX, it still needs MAC to tweak TX.

>"rgmii-id", if the property was defined from the perspective of the
>MAC,
>which it is not.
>
>Both the Ethernet PHY driver and the MAC driver need to take care of
>adjusting the delays based on the phydev->interface value.
>
>The property you are introducing here is absolutely not appropriate
>because it is entirely redundant with what 'phy-mode' already defines,
>except the latter also covers a lot more cases.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Florian Fainelli]

On 05/04/2017 11:26 AM, Icenowy Zheng wrote:
> 
> 
> 于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>> On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>>> 在 2017-04-22 08:22，Florian Fainelli 写道：
>>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>
>>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>>> indicated with device tree.
>>>>>
>>>>> Add the binding for a property that indicates this workaround.
>>>>>
>>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>> ---
>>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>>> ++++++++++++++++++++++
>>>>>  1 file changed, 22 insertions(+)
>>>>>  create mode 100644
>>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>> new file mode 100644
>>>>> index 000000000000..c1913301bfe8
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>> @@ -0,0 +1,22 @@
>>>>> +Realtek RTL8211E Ethernet PHY
>>>>> +
>>>>> +One batch of RTL8211E is slight broken, that needs some special
>> (and
>>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>>> properly.
>>>>> +The only well-known board that used the broken batch is Pine64+.
>>>>> +Configure it through an Ethernet OF device node.
>>>>> +
>>>>> +Optional properties:
>>>>> +
>>>>> +- realtek,disable-rx-delay:
>>>>> +  If set, RX delay will be completely disabled (according to
>>>>> Realtek). This
>>>>> +  will affect the performance on non-broken boards.
>>>>> +  default: do not disable RX delay.
>>>>
>>>> Please don't introduce custom properties to do that, instead correct
>>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>> indicates
>>>> that there should be no RX internal delay, but a TX internal delay
>> added
>>>> by the PHY.
>>>
>>> Checked the document, the meaning of "rgmii-txid" is not correct
>> here.
>>>
>>> This doesn't effect the MAC, and the MAC should still add TX delay.
>>>
>>> The definition of "rgmii-txid" in
>>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>>> internal TX delay provided by the PHY, the MAC should not add an TX
>> delay
>>> in this case". However, this do not indicate that the MAC doesn't add
>> TX
>>> delay; in fact that just totally disabled the PHY to provide the RX
>> delay.
>>> MAC still should to add delay on both TX/RX, which is the semantic of
>>> standard "rgmii".
>>>
>>> So I cannot used "rgmii-txid" here, but should continue to use this
>>> custom property.
>>
>> This is absolutely not a correct understanding. The 'phy-mode' property
>> defines the contract between the MAC and PHY. It is defined from the
>> PHY's perspective of the delay, which means that the MAC has to either
>> also provide an adequate delay (RX or TX) or not (RX or TX). So if you
>> specified 'phy-mode' = "rgmii" this means that the MAC needs to adds
>> the
>> TX and RX delay, so implcitly this means that your MAC operates in
> 
> The MAC doesn't lose its responsibility to tweak RX/TX delays with this property set.

No it does not but now there is no contract binding the MAC and the PHY
together was to what an appropriate delay configuration there should be.
This is why using phydev->interface (directly inherited from 'phy-mode')
is important because it binds the PHY and MAC on a contract.

> 
> This situation is that, the PHY's RX delay tweaking function is broken. But it doesn't mean that the PHY can take over *all* responsibility to tweak TX, it still needs MAC to tweak TX.

Correct, so what part of my answer was not clear in that sense?

> 
>> "rgmii-id", if the property was defined from the perspective of the
>> MAC,
>> which it is not.
>>
>> Both the Ethernet PHY driver and the MAC driver need to take care of
>> adjusting the delays based on the phydev->interface value.
>>
>> The property you are introducing here is absolutely not appropriate
>> because it is entirely redundant with what 'phy-mode' already defines,
>> except the latter also covers a lot more cases.


-- 
Florian

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[icenowy-h8G6r0blFSE]

在 2017-05-05 02:29，Florian Fainelli 写道：
> On 05/04/2017 11:26 AM, Icenowy Zheng wrote:
>> 
>> 
>> 于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli 
>> <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>>> On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>>>> 在 2017-04-22 08:22，Florian Fainelli 写道：
>>>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>> 
>>>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>>>> indicated with device tree.
>>>>>> 
>>>>>> Add the binding for a property that indicates this workaround.
>>>>>> 
>>>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>>>> ++++++++++++++++++++++
>>>>>>  1 file changed, 22 insertions(+)
>>>>>>  create mode 100644
>>>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>> 
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..c1913301bfe8
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>> @@ -0,0 +1,22 @@
>>>>>> +Realtek RTL8211E Ethernet PHY
>>>>>> +
>>>>>> +One batch of RTL8211E is slight broken, that needs some special
>>> (and
>>>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>>>> properly.
>>>>>> +The only well-known board that used the broken batch is Pine64+.
>>>>>> +Configure it through an Ethernet OF device node.
>>>>>> +
>>>>>> +Optional properties:
>>>>>> +
>>>>>> +- realtek,disable-rx-delay:
>>>>>> +  If set, RX delay will be completely disabled (according to
>>>>>> Realtek). This
>>>>>> +  will affect the performance on non-broken boards.
>>>>>> +  default: do not disable RX delay.
>>>>> 
>>>>> Please don't introduce custom properties to do that, instead 
>>>>> correct
>>>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>>> indicates
>>>>> that there should be no RX internal delay, but a TX internal delay
>>> added
>>>>> by the PHY.
>>>> 
>>>> Checked the document, the meaning of "rgmii-txid" is not correct
>>> here.
>>>> 
>>>> This doesn't effect the MAC, and the MAC should still add TX delay.
>>>> 
>>>> The definition of "rgmii-txid" in
>>>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>>>> internal TX delay provided by the PHY, the MAC should not add an TX
>>> delay
>>>> in this case". However, this do not indicate that the MAC doesn't 
>>>> add
>>> TX
>>>> delay; in fact that just totally disabled the PHY to provide the RX
>>> delay.
>>>> MAC still should to add delay on both TX/RX, which is the semantic 
>>>> of
>>>> standard "rgmii".
>>>> 
>>>> So I cannot used "rgmii-txid" here, but should continue to use this
>>>> custom property.

Sorry for replying an old email, but it's because the driver of the MAC 
I
used is merged (dwmac-sun8i).

The driver of the MAC currently only supports "mii", "rmii", and 
"rgmii",
and according to the SoC's user manual, the MAC cannot has its delays
disabled.

How should it handle this "rgmii-txid" here? Just treat it as "rgmii"?

>>> 
>>> This is absolutely not a correct understanding. The 'phy-mode' 
>>> property
>>> defines the contract between the MAC and PHY. It is defined from the
>>> PHY's perspective of the delay, which means that the MAC has to 
>>> either
>>> also provide an adequate delay (RX or TX) or not (RX or TX). So if 
>>> you
>>> specified 'phy-mode' = "rgmii" this means that the MAC needs to adds
>>> the
>>> TX and RX delay, so implcitly this means that your MAC operates in
>> 
>> The MAC doesn't lose its responsibility to tweak RX/TX delays with 
>> this property set.
> 
> No it does not but now there is no contract binding the MAC and the PHY
> together was to what an appropriate delay configuration there should 
> be.
> This is why using phydev->interface (directly inherited from 
> 'phy-mode')
> is important because it binds the PHY and MAC on a contract.
> 
>> 
>> This situation is that, the PHY's RX delay tweaking function is 
>> broken. But it doesn't mean that the PHY can take over *all* 
>> responsibility to tweak TX, it still needs MAC to tweak TX.
> 
> Correct, so what part of my answer was not clear in that sense?
> 
>> 
>>> "rgmii-id", if the property was defined from the perspective of the
>>> MAC,
>>> which it is not.
>>> 
>>> Both the Ethernet PHY driver and the MAC driver need to take care of
>>> adjusting the delays based on the phydev->interface value.
>>> 
>>> The property you are introducing here is absolutely not appropriate
>>> because it is entirely redundant with what 'phy-mode' already 
>>> defines,
>>> except the latter also covers a lot more cases.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY

[Florian Fainelli]

On 08/21/2017 07:53 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
> 在 2017-05-05 02:29，Florian Fainelli 写道：
>> On 05/04/2017 11:26 AM, Icenowy Zheng wrote:
>>>
>>>
>>> 于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli
>>> <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>>>> On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>>>>> 在 2017-04-22 08:22，Florian Fainelli 写道：
>>>>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>>>
>>>>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>>>>> indicated with device tree.
>>>>>>>
>>>>>>> Add the binding for a property that indicates this workaround.
>>>>>>>
>>>>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>>>>> ++++++++++++++++++++++
>>>>>>>  1 file changed, 22 insertions(+)
>>>>>>>  create mode 100644
>>>>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..c1913301bfe8
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> @@ -0,0 +1,22 @@
>>>>>>> +Realtek RTL8211E Ethernet PHY
>>>>>>> +
>>>>>>> +One batch of RTL8211E is slight broken, that needs some special
>>>> (and
>>>>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>>>>> properly.
>>>>>>> +The only well-known board that used the broken batch is Pine64+.
>>>>>>> +Configure it through an Ethernet OF device node.
>>>>>>> +
>>>>>>> +Optional properties:
>>>>>>> +
>>>>>>> +- realtek,disable-rx-delay:
>>>>>>> +  If set, RX delay will be completely disabled (according to
>>>>>>> Realtek). This
>>>>>>> +  will affect the performance on non-broken boards.
>>>>>>> +  default: do not disable RX delay.
>>>>>>
>>>>>> Please don't introduce custom properties to do that, instead correct
>>>>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>>>> indicates
>>>>>> that there should be no RX internal delay, but a TX internal delay
>>>> added
>>>>>> by the PHY.
>>>>>
>>>>> Checked the document, the meaning of "rgmii-txid" is not correct
>>>> here.
>>>>>
>>>>> This doesn't effect the MAC, and the MAC should still add TX delay.
>>>>>
>>>>> The definition of "rgmii-txid" in
>>>>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>>>>> internal TX delay provided by the PHY, the MAC should not add an TX
>>>> delay
>>>>> in this case". However, this do not indicate that the MAC doesn't add
>>>> TX
>>>>> delay; in fact that just totally disabled the PHY to provide the RX
>>>> delay.
>>>>> MAC still should to add delay on both TX/RX, which is the semantic of
>>>>> standard "rgmii".
>>>>>
>>>>> So I cannot used "rgmii-txid" here, but should continue to use this
>>>>> custom property.
> 
> Sorry for replying an old email, but it's because the driver of the MAC I
> used is merged (dwmac-sun8i).
> 
> The driver of the MAC currently only supports "mii", "rmii", and "rgmii",
> and according to the SoC's user manual, the MAC cannot has its delays
> disabled.
> 
> How should it handle this "rgmii-txid" here? Just treat it as "rgmii"?

Considering there are no configurable delays on the MAC side, all you
can do is treat all RGMII variants the same by configuring the MAC for
RGMII mode (with no additional capabilities and as opposed to MII, RMII
which are other clocking/data pins modes) and let the PHY configure the
delay accordingly based on "phy-mode"/phy_interface_t. You can use
phy_interface_is_rgmii() as a helper function to cover all 4 variants.
-- 
Florian

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.