[net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle TX clock delay. This seems to work fine for many boards (for
example Odroid-C2 or Amlogic's reference boards) but there are some
others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

This raises a question though:
Which device is supposed to enable the TX delay when both MAC and PHY
support it? And should we implement it for each PHY / MAC separately
or should we think about a more generic solution (currently it's not
possible to disable the TX delay generated by the RTL8211F PHY via
devicetree when using phy-mode "rgmii")?

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R          
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901 Mbits/sec                  
[  4]   1.00-2.00   sec  94.2 MBytes   791 Mbits/sec                  
[  4]   2.00-3.00   sec  96.5 MBytes   810 Mbits/sec                  
[  4]   3.00-4.00   sec  96.2 MBytes   808 Mbits/sec                  
[  4]   4.00-5.00   sec  96.6 MBytes   810 Mbits/sec                  
[  4]   5.00-6.00   sec  96.5 MBytes   810 Mbits/sec                  
[  4]   6.00-7.00   sec  96.6 MBytes   810 Mbits/sec                  
[  4]   7.00-8.00   sec  96.5 MBytes   809 Mbits/sec                  
[  4]   8.00-9.00   sec   105 MBytes   884 Mbits/sec                  
[  4]   9.00-10.00  sec   111 MBytes   934 Mbits/sec                  
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  1000 MBytes   839 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   998 MBytes   837 Mbits/sec                  receiver

iperf Done.
$ iperf3 -c 192.168.1.100   
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139 KBytes       
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7 KBytes       
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187 KBytes       
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143 KBytes       
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129 KBytes       
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195 KBytes       
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7 KBytes       
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6 KBytes       
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107 KBytes       
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874 Mbits/sec  1189             sender
[  4]   0.00-10.01  sec  1.02 GBytes   873 Mbits/sec                  receiver

iperf Done.


Martin Blumenstingl (2):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

 Documentation/devicetree/bindings/net/meson-dwmac.txt | 11 +++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16 +++++++++++-----
 include/dt-bindings/net/dwmac-meson8b.h               | 18 ++++++++++++++++++
 3 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 include/dt-bindings/net/dwmac-meson8b.h

-- 
2.10.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

[net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

This allows configuring the RGMII TX clock delay. This clock is
generated by the Meson 8b / GXBB DWMAC glue. The configuration depends
on the actual hardware (no delay may be needed due to the design of the
actual circuit, the PHY might add this delay, etc.).
The configuration values are provided as preprocessor macros to make the
devicetree files easier to read.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/net/meson-dwmac.txt | 11 +++++++++++
 include/dt-bindings/net/dwmac-meson8b.h               | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 include/dt-bindings/net/dwmac-meson8b.h

diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62dd..fe526d0 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,17 @@ Required properties on Meson8b and newer:
 		- "clkin0" - first parent clock of the internal mux
 		- "clkin1" - second parent clock of the internal mux
 
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay:	The internal RGMII TX clock delay configuration.
+			Defaults to DWMAC_MESON8B_TXDLY_QUARTER_CYCLE
+			when not given. All possible values are defined
+			as preprocessor macro in
+			<dt-bindings/net/dwmac-meson8b.h>.
+			The delay is specified as divider for the
+			internal clock (RGMII typically uses a 125MHz
+			clock clock (= 8ns per cycle), so setting
+			DWMAC_MESON8B_TXDLY_QUARTER_CYCLE
+			results in a TX delay of 8ns/4 = 2ns.
 
 Example for Meson6:
 
diff --git a/include/dt-bindings/net/dwmac-meson8b.h b/include/dt-bindings/net/dwmac-meson8b.h
new file mode 100644
index 0000000..4fc149e
--- /dev/null
+++ b/include/dt-bindings/net/dwmac-meson8b.h
@@ -0,0 +1,18 @@
+/*
+ * Devicetree constants for the Amlogic Meson8b and GXBB DWMAC glue layer
+ *
+ * Copyright (C) 2016 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* TX delay configuration */
+#define DWMAC_MESON8B_TXDLY_OFF				0x0
+#define DWMAC_MESON8B_TXDLY_QUARTER_CYCLE		0x1
+#define DWMAC_MESON8B_TXDLY_HALF_CYCLE			0x2
+#define DWMAC_MESON8B_TXDLY_THREE_QUARTER_CYCLE		0x3
-- 
2.10.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

Re: [net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Andrew Lunn]

> The configuration values are provided as preprocessor macros to make the
> devicetree files easier to read.

Hi Martin

If i'm reading the code/comments correctly, you can set the delay to
0, 2, 4 or 6ns? So calling this property amlogic,tx-delay-ns would be
even easier to read.

     Andrew

Re: [net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

Hi Andrew,

On Thu, Nov 24, 2016 at 4:48 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> The configuration values are provided as preprocessor macros to make the
>> devicetree files easier to read.
>
> Hi Martin
>
> If i'm reading the code/comments correctly, you can set the delay to
> 0, 2, 4 or 6ns? So calling this property amlogic,tx-delay-ns would be
> even easier to read.
indeed, this sounds like a very nice idea (as it moves the calculation
from the programmer's brain to dwmac-meson8b.c)!

I'll send an updated version once I received enough feedback (in case
something else is wrong with the patches)
--
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

[net-next PATCH v1 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Prior to this patch we were using a hardcoded RGMII TX clock delay of
1/4 cycle (= 2ns). This value works for many boards, but unfortunately
not for all (due to the way the actual circuit is designed, sometimes
because the TX delay is enabled in the PHY, etc.).
Making the TX delay on the MAC side configurable allows us to support
all possible hardware combinations (which may or not be out there).

This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..1697d1a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -23,6 +23,8 @@
 #include <linux/platform_device.h>
 #include <linux/stmmac.h>
 
+#include <dt-bindings/net/dwmac-meson8b.h>
+
 #include "stmmac_platform.h"
 
 #define PRG_ETH0			0x0
@@ -35,10 +37,6 @@
 
 #define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF		(0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER		(0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF		(0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS	(0x3 << PRG_ETH0_TXDLY_SHIFT)
 
 /* divider for the result of m250_sel */
 #define PRG_ETH0_CLK_M250_DIV_SHIFT	7
@@ -69,6 +67,8 @@ struct meson8b_dwmac {
 
 	struct clk_divider	m25_div;
 	struct clk		*m25_div_clk;
+
+	u32			tx_dly;
 };
 
 static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -198,7 +198,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 
 		/* TX clock delay - all known boards use a 1/4 cycle delay */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					PRG_ETH0_TXDLY_QUARTER);
+					dwmac->tx_dly << PRG_ETH0_TXDLY_SHIFT);
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
@@ -279,6 +279,12 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	ret = of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay",
+				   &dwmac->tx_dly);
+	if (ret)
+		/* default to 1/4 cycle (= 2ns for RGMII) */
+		dwmac->tx_dly = DWMAC_MESON8B_TXDLY_QUARTER_CYCLE;
+
 	ret = meson8b_init_clk(dwmac);
 	if (ret)
 		return ret;
-- 
2.10.2

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Jerome Brunet]

On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
> 
> This raises a question though:
> Which device is supposed to enable the TX delay when both MAC and PHY
> support it? And should we implement it for each PHY / MAC separately
> or should we think about a more generic solution (currently it's not
> possible to disable the TX delay generated by the RTL8211F PHY via
> devicetree when using phy-mode "rgmii")?

Actually you can skip the part which activate the Tx-delay on the phy
by setting "phy-mode = "rgmii-id" instead of "rgmii"

phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
but PHY_INTERFACE_MODE_RGMII_ID.

> 
> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
> TX clock delay disabled on the MAC (as it's enabled in the PHY
> driver).
> TX throughput was virtually zero before:
> $ iperf3 -c 192.168.1.100 -R          
> Connecting to host 192.168.1.100, port 5201
> Reverse mode, remote host 192.168.1.100 is sending
> [  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-1.00   sec   108 MBytes   901
> Mbits/sec                  
> [  4]   1.00-2.00   sec  94.2 MBytes   791
> Mbits/sec                  
> [  4]   2.00-3.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   3.00-4.00   sec  96.2 MBytes   808
> Mbits/sec                  
> [  4]   4.00-5.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   5.00-6.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   6.00-7.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   7.00-8.00   sec  96.5 MBytes   809
> Mbits/sec                  
> [  4]   8.00-9.00   sec   105 MBytes   884
> Mbits/sec                  
> [  4]   9.00-10.00  sec   111 MBytes   934
> Mbits/sec                  
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.00  sec  1000 MBytes   839
> Mbits/sec    0             sender
> [  4]   0.00-10.00  sec   998 MBytes   837
> Mbits/sec                  receiver
> 
> iperf Done.
> $ iperf3 -c 192.168.1.100   
> Connecting to host 192.168.1.100, port 5201
> [  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139
> KBytes       
> [  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
> KBytes       
> [  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187
> KBytes       
> [  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143
> KBytes       
> [  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129
> KBytes       
> [  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195
> KBytes       
> [  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
> KBytes       
> [  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
> KBytes       
> [  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107
> KBytes       
> [  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139
> KBytes       
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.01  sec  1.02 GBytes   874
> Mbits/sec  1189             sender
> [  4]   0.00-10.01  sec  1.02 GBytes   873
> Mbits/sec                  receiver
> 
> iperf Done.

> 
> 
> Martin Blumenstingl (2):
>   net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
>   net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
> 
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
> +++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
> +++++++++++-----
>  include/dt-bindings/net/dwmac-meson8b.h               | 18
> ++++++++++++++++++
>  3 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
> 

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>> cycle TX clock delay. This seems to work fine for many boards (for
>> example Odroid-C2 or Amlogic's reference boards) but there are some
>> others where TX traffic is simply broken.
>> There are probably multiple reasons why it's working on some boards
>> while it's broken on others:
>> - some of Amlogic's reference boards are using a Micrel PHY
>> - hardware circuit design
>> - maybe more...
>>
>> This raises a question though:
>> Which device is supposed to enable the TX delay when both MAC and PHY
>> support it? And should we implement it for each PHY / MAC separately
>> or should we think about a more generic solution (currently it's not
>> possible to disable the TX delay generated by the RTL8211F PHY via
>> devicetree when using phy-mode "rgmii")?
>
> Actually you can skip the part which activate the Tx-delay on the phy
> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>
> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
> but PHY_INTERFACE_MODE_RGMII_ID.
unfortunately this is not true for RTL8211F (I did my previous tests
with the same expectation in mind)!
the code seems to suggest that TX-delay is disabled whenever mode !=
PHY_INTERFACE_MODE_RGMII.
BUT: on my device RTL8211F_TX_DELAY is set even before
"phy_write(phydev, 0x11, reg);"!

Based on what I found it seems that rgmii-id, rgmii-txid and
rgmii-rxid are supposed to be handled by the PHY.
That would mean that we have two problems here:
1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
enable the TX-delay in that case - otherwise explicitly disable it
2) dwmac-meson8b.c should only use the configured TX-delay for
PHY_INTERFACE_MODE_RGMII
@Florian: could you please share your thoughts on this (who handles
the TX delay in which case)?


Regards,
Martin
--
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

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Florian Fainelli]

Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>>> cycle TX clock delay. This seems to work fine for many boards (for
>>> example Odroid-C2 or Amlogic's reference boards) but there are some
>>> others where TX traffic is simply broken.
>>> There are probably multiple reasons why it's working on some boards
>>> while it's broken on others:
>>> - some of Amlogic's reference boards are using a Micrel PHY
>>> - hardware circuit design
>>> - maybe more...
>>>
>>> This raises a question though:
>>> Which device is supposed to enable the TX delay when both MAC and PHY
>>> support it? And should we implement it for each PHY / MAC separately
>>> or should we think about a more generic solution (currently it's not
>>> possible to disable the TX delay generated by the RTL8211F PHY via
>>> devicetree when using phy-mode "rgmii")?
>>
>> Actually you can skip the part which activate the Tx-delay on the phy
>> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>>
>> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
>> but PHY_INTERFACE_MODE_RGMII_ID.
> unfortunately this is not true for RTL8211F (I did my previous tests
> with the same expectation in mind)!
> the code seems to suggest that TX-delay is disabled whenever mode !=
> PHY_INTERFACE_MODE_RGMII.
> BUT: on my device RTL8211F_TX_DELAY is set even before
> "phy_write(phydev, 0x11, reg);"!

(Adding Sebastian (and Mans, and Andrew) since he raised the same
question a while ago. I think I now understand a bit better what
Sebastian was after a couple of weeks ago)

> 
> Based on what I found it seems that rgmii-id, rgmii-txid and
> rgmii-rxid are supposed to be handled by the PHY.

Correct, the meaning of PHY_INTERFACE_MODE should be from the
perspective of the PHY device:

- PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
- PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)

> That would mean that we have two problems here:
> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
> enable the TX-delay in that case - otherwise explicitly disable it

Agreed.

> 2) dwmac-meson8b.c should only use the configured TX-delay for
> PHY_INTERFACE_MODE_RGMII
> @Florian: could you please share your thoughts on this (who handles
> the TX delay in which case)?

This also seems reasonable to do, provided that the PHY is also properly
configured not to add delays in both directions, and therefore assumes
that the MAC does it.

We have a fairly large problem with how RGMII delays are done in PHYLIB
and Ethernet MAC drivers (or just in general), where we can't really
intersect properly what a PHY is supporting (in terms of internal
delays), and what the MAC supports either. One possible approach could
be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
support (ideally, even with normalized nanosecond delay values), and
then intersect that with the requested phy_interface_t during
phy_{attach,connect} time, and feed this back to the MAC with a special
error code/callback, so we could gracefully try to choose another
PHY_INTERFACE_MODE_* value that the MAC supports....

A larger problem is that a number of drivers have been deployed, and
Device Trees, possibly with the meaning of "phy-mode" and
"phy-connection-type" being from the MAC perspective, and not the PHY
perspective *sigh*, good luck auditing those.

So from there, here is possibly what we could do

- submit a series of patches that update the PHYLIB documentation (there
are other things missing here) and make it clear from which entity (PHY
or MAC) does the delay apply to, document the "intersection" problem here

- have you document the configured behavior for dwmac-meson8b that we
just discussed here in v2 of this patch series

Sorry for the long post, here is a virtual potato: 0
-- 
Florian
--
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

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

On Thu, Nov 24, 2016 at 7:55 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>>> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>>>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>>>> cycle TX clock delay. This seems to work fine for many boards (for
>>>> example Odroid-C2 or Amlogic's reference boards) but there are some
>>>> others where TX traffic is simply broken.
>>>> There are probably multiple reasons why it's working on some boards
>>>> while it's broken on others:
>>>> - some of Amlogic's reference boards are using a Micrel PHY
>>>> - hardware circuit design
>>>> - maybe more...
>>>>
>>>> This raises a question though:
>>>> Which device is supposed to enable the TX delay when both MAC and PHY
>>>> support it? And should we implement it for each PHY / MAC separately
>>>> or should we think about a more generic solution (currently it's not
>>>> possible to disable the TX delay generated by the RTL8211F PHY via
>>>> devicetree when using phy-mode "rgmii")?
>>>
>>> Actually you can skip the part which activate the Tx-delay on the phy
>>> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>>>
>>> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
>>> but PHY_INTERFACE_MODE_RGMII_ID.
>> unfortunately this is not true for RTL8211F (I did my previous tests
>> with the same expectation in mind)!
>> the code seems to suggest that TX-delay is disabled whenever mode !=
>> PHY_INTERFACE_MODE_RGMII.
>> BUT: on my device RTL8211F_TX_DELAY is set even before
>> "phy_write(phydev, 0x11, reg);"!
>
> (Adding Sebastian (and Mans, and Andrew) since he raised the same
> question a while ago. I think I now understand a bit better what
> Sebastian was after a couple of weeks ago)
>
>>
>> Based on what I found it seems that rgmii-id, rgmii-txid and
>> rgmii-rxid are supposed to be handled by the PHY.
>
> Correct, the meaning of PHY_INTERFACE_MODE should be from the
> perspective of the PHY device:
>
> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
and PHY_INTERFACE_MODE_RGMII_ID is basically _TXID and _RXID combined
(meaning that the PHY is responsible for the TX and RX delays)

>> That would mean that we have two problems here:
>> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
>> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
>> enable the TX-delay in that case - otherwise explicitly disable it
>
> Agreed.
(on a side-not: it seems that the RTL8211F's TX-delay setting is
either untouched by a hardware reset via GPIO or enabled automatically
during hardware reset via GPIO)

>> 2) dwmac-meson8b.c should only use the configured TX-delay for
>> PHY_INTERFACE_MODE_RGMII
>> @Florian: could you please share your thoughts on this (who handles
>> the TX delay in which case)?
>
> This also seems reasonable to do, provided that the PHY is also properly
> configured not to add delays in both directions, and therefore assumes
> that the MAC does it.
on Amlogic Meson systems (at least on the ARM64 ones) all customer
devices with Gbit ethernet are using the RTL8211F PHY. The only
exception are some development/reference boards from Amlogic
themselves, which seem to be using a Micrel RGMII PHY.

> We have a fairly large problem with how RGMII delays are done in PHYLIB
> and Ethernet MAC drivers (or just in general), where we can't really
> intersect properly what a PHY is supporting (in terms of internal
> delays), and what the MAC supports either. One possible approach could
> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
> support (ideally, even with normalized nanosecond delay values), and
> then intersect that with the requested phy_interface_t during
> phy_{attach,connect} time, and feed this back to the MAC with a special
> error code/callback, so we could gracefully try to choose another
> PHY_INTERFACE_MODE_* value that the MAC supports....
>
> A larger problem is that a number of drivers have been deployed, and
> Device Trees, possibly with the meaning of "phy-mode" and
> "phy-connection-type" being from the MAC perspective, and not the PHY
> perspective *sigh*, good luck auditing those.
>
> So from there, here is possibly what we could do
>
> - submit a series of patches that update the PHYLIB documentation (there
> are other things missing here) and make it clear from which entity (PHY
> or MAC) does the delay apply to, document the "intersection" problem here
sounds like a good idea, maybe we should move this to a separate thread (I guess
this is the part which is especially interesting for Sebastian, Mans
and Andrew)?

> - have you document the configured behavior for dwmac-meson8b that we
> just discussed here in v2 of this patch series
I would add something like "...using the phy-modes rgmii-id or
rgmii-txid means that the TX-delay will be added by the PHY, thus no
TX-delay should be configured for the MAC/dwmac-meson8b glue" (I'll
improve this, I just want a quick confirmation that I get your idea
right)

Thanks for the quick and helpful answer Florian!


Regards,
Martin

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Sebastian Frias]

On 24/11/16 19:55, Florian Fainelli wrote:
> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>>> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>>>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>>>> cycle TX clock delay. This seems to work fine for many boards (for
>>>> example Odroid-C2 or Amlogic's reference boards) but there are some
>>>> others where TX traffic is simply broken.
>>>> There are probably multiple reasons why it's working on some boards
>>>> while it's broken on others:
>>>> - some of Amlogic's reference boards are using a Micrel PHY
>>>> - hardware circuit design
>>>> - maybe more...
>>>>
>>>> This raises a question though:
>>>> Which device is supposed to enable the TX delay when both MAC and PHY
>>>> support it? And should we implement it for each PHY / MAC separately
>>>> or should we think about a more generic solution (currently it's not
>>>> possible to disable the TX delay generated by the RTL8211F PHY via
>>>> devicetree when using phy-mode "rgmii")?
>>>
>>> Actually you can skip the part which activate the Tx-delay on the phy
>>> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>>>
>>> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
>>> but PHY_INTERFACE_MODE_RGMII_ID.
>> unfortunately this is not true for RTL8211F (I did my previous tests
>> with the same expectation in mind)!
>> the code seems to suggest that TX-delay is disabled whenever mode !=
>> PHY_INTERFACE_MODE_RGMII.
>> BUT: on my device RTL8211F_TX_DELAY is set even before
>> "phy_write(phydev, 0x11, reg);"!

If you look at the Atheros 803x PHY and its driver
'drivers/net/phy/at803x.c':
- by default (as HW reset preset) the PHY has RX delay enabled, TX
delay disabled
- the driver only enables RX, or TX, or both, according to "rgmii-rxid",
"rgmii-txid", or "rgmii-id" respectively, but does not alter HW reset
presets. In other words:
  a "rgmii-rxid" results in RX enabled (expected)
  b "rgmii-txid" results in RX *and* TX enabled (unexpected?)
  c "rgmii-id"   results in RX *and* TX enabled (expected)
  d "rgmii"      results in RX enabled (unexpected?)

This is a bit surprising and I think that some boards and PHY<->MAC
combinations are working a little bit by chance, unless I'm missing
something.

> 
> (Adding Sebastian (and Mans, and Andrew) since he raised the same
> question a while ago. I think I now understand a bit better what
> Sebastian was after a couple of weeks ago)
> 

Thanks for CCing us, it is indeed a very similar issue.

>>
>> Based on what I found it seems that rgmii-id, rgmii-txid and
>> rgmii-rxid are supposed to be handled by the PHY.
> 
> Correct, the meaning of PHY_INTERFACE_MODE should be from the
> perspective of the PHY device:
> 
> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
> 

Thanks for the explanation.
Actually I had thought that the delay was to account for board routing
(wires) between the MAC and the PHY.
From your explanation it appears that the delay is to account for board
routing (wires) between the PHY and the RJ45 socket.

>> That would mean that we have two problems here:
>> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
>> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
>> enable the TX-delay in that case - otherwise explicitly disable it
> 
> Agreed.
> 
>> 2) dwmac-meson8b.c should only use the configured TX-delay for
>> PHY_INTERFACE_MODE_RGMII
>> @Florian: could you please share your thoughts on this (who handles
>> the TX delay in which case)?
> 
> This also seems reasonable to do, provided that the PHY is also properly
> configured not to add delays in both directions, and therefore assumes
> that the MAC does it.
> 
> We have a fairly large problem with how RGMII delays are done in PHYLIB
> and Ethernet MAC drivers (or just in general), where we can't really
> intersect properly what a PHY is supporting (in terms of internal
> delays), and what the MAC supports either. One possible approach could
> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
> support (ideally, even with normalized nanosecond delay values), 

Just to make sure I understood this, the DT would say something like:

phy-connection-type = "rgmii-txid";
txid-delay-ns = <3>;

For a 3ns TX delay, would that be good?

>and
> then intersect that with the requested phy_interface_t during
> phy_{attach,connect} time, and feed this back to the MAC with a special
> error code/callback, so we could gracefully try to choose another
> PHY_INTERFACE_MODE_* value that the MAC supports....
> 
> A larger problem is that a number of drivers have been deployed, and
> Device Trees, possibly with the meaning of "phy-mode" and
> "phy-connection-type" being from the MAC perspective, and not the PHY
> perspective *sigh*, good luck auditing those.
> 
> So from there, here is possibly what we could do
> 
> - submit a series of patches that update the PHYLIB documentation (there
> are other things missing here) and make it clear from which entity (PHY
> or MAC) does the delay apply to, document the "intersection" problem here

I think documenting is necessary, thanks in advance!

However, I'm wondering if there's a way to make this work in all cases.
Indeed, if we consider for example that TX delay is required, we have 4
cases:

       PHY         |       MAC          | Who applies?
TXID supported     | TXID supported     | PHY
TXID supported     | TXID not supported | PHY
TXID not supported | TXID supported     | MAC
TXID not supported | TXID not supported | cannot be done

That is basically what my patch:

https://marc.info/?l=linux-netdev&m=147869658031783&w=2

attempted to achieve. That would allow more combinations of MAC<->PHY to
work, right?

Nevertheless, I think we also need to keep in mind that most of this
discussion assumes the case where both, MAC and PHY have equal capabilities.
Could it happen that the PHY supports only 2ns delay, and the MAC only
1ns delay?
Could it happen that the delay is bigger than what is supported by
either the PHY or MAC alone? maybe if combined it could work, for example
a 3ns delay required and the PHY supporting 2ns and the MAC 1ns, combined
it could work?

I don't know if these are cases worth supporting, nor if they are valid.

> 
> - have you document the configured behavior for dwmac-meson8b that we
> just discussed here in v2 of this patch series
> 
> Sorry for the long post, here is a virtual potato: 0
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Måns Rullgård]

Sebastian Frias <sf84@laposte.net> writes:

> On 24/11/16 19:55, Florian Fainelli wrote:
>> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>>> Based on what I found it seems that rgmii-id, rgmii-txid and
>>> rgmii-rxid are supposed to be handled by the PHY.
>> 
>> Correct, the meaning of PHY_INTERFACE_MODE should be from the
>> perspective of the PHY device:
>> 
>> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
>> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
>> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
>> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
>> 
>
> Thanks for the explanation.
> Actually I had thought that the delay was to account for board routing
> (wires) between the MAC and the PHY.
> From your explanation it appears that the delay is to account for board
> routing (wires) between the PHY and the RJ45 socket.

The delay pertains to the RGMII link between MAC and PHY.  The external
connection is self-clocking.

-- 
Måns Rullgård

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Florian Fainelli]

On 11/25/2016 03:13 AM, Sebastian Frias wrote:
> On 24/11/16 19:55, Florian Fainelli wrote:
>> Le 24/11/2016 à 09:05, Martin Blumenstingl a écrit :
>>> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>>>> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>>>>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>>>>> cycle TX clock delay. This seems to work fine for many boards (for
>>>>> example Odroid-C2 or Amlogic's reference boards) but there are some
>>>>> others where TX traffic is simply broken.
>>>>> There are probably multiple reasons why it's working on some boards
>>>>> while it's broken on others:
>>>>> - some of Amlogic's reference boards are using a Micrel PHY
>>>>> - hardware circuit design
>>>>> - maybe more...
>>>>>
>>>>> This raises a question though:
>>>>> Which device is supposed to enable the TX delay when both MAC and PHY
>>>>> support it? And should we implement it for each PHY / MAC separately
>>>>> or should we think about a more generic solution (currently it's not
>>>>> possible to disable the TX delay generated by the RTL8211F PHY via
>>>>> devicetree when using phy-mode "rgmii")?
>>>>
>>>> Actually you can skip the part which activate the Tx-delay on the phy
>>>> by setting "phy-mode = "rgmii-id" instead of "rgmii"
>>>>
>>>> phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
>>>> but PHY_INTERFACE_MODE_RGMII_ID.
>>> unfortunately this is not true for RTL8211F (I did my previous tests
>>> with the same expectation in mind)!
>>> the code seems to suggest that TX-delay is disabled whenever mode !=
>>> PHY_INTERFACE_MODE_RGMII.
>>> BUT: on my device RTL8211F_TX_DELAY is set even before
>>> "phy_write(phydev, 0x11, reg);"!
> 
> If you look at the Atheros 803x PHY and its driver
> 'drivers/net/phy/at803x.c':
> - by default (as HW reset preset) the PHY has RX delay enabled, TX
> delay disabled
> - the driver only enables RX, or TX, or both, according to "rgmii-rxid",
> "rgmii-txid", or "rgmii-id" respectively, but does not alter HW reset
> presets. In other words:
>   a "rgmii-rxid" results in RX enabled (expected)
>   b "rgmii-txid" results in RX *and* TX enabled (unexpected?)
>   c "rgmii-id"   results in RX *and* TX enabled (expected)
>   d "rgmii"      results in RX enabled (unexpected?)
> 
> This is a bit surprising and I think that some boards and PHY<->MAC
> combinations are working a little bit by chance, unless I'm missing
> something.
> 
>>
>> (Adding Sebastian (and Mans, and Andrew) since he raised the same
>> question a while ago. I think I now understand a bit better what
>> Sebastian was after a couple of weeks ago)
>>
> 
> Thanks for CCing us, it is indeed a very similar issue.
> 
>>>
>>> Based on what I found it seems that rgmii-id, rgmii-txid and
>>> rgmii-rxid are supposed to be handled by the PHY.
>>
>> Correct, the meaning of PHY_INTERFACE_MODE should be from the
>> perspective of the PHY device:
>>
>> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
>> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
>> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
>> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
>>
> 
> Thanks for the explanation.
> Actually I had thought that the delay was to account for board routing
> (wires) between the MAC and the PHY.
> From your explanation it appears that the delay is to account for board
> routing (wires) between the PHY and the RJ45 socket.

The placement of the (delay) was not meant to be exact, but it was
wrongly place anyway, so it should be between the MAC and PHY, always.
This is why you see people either fixing the need for a delay by
appropriately programming the PHY, or the MAC, or by just inserting a
fixed delay on the PCB between the PHY and the MAC and programming no
delays (or using the default values and hoping this works).

> 
>>> That would mean that we have two problems here:
>>> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
>>> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
>>> enable the TX-delay in that case - otherwise explicitly disable it
>>
>> Agreed.
>>
>>> 2) dwmac-meson8b.c should only use the configured TX-delay for
>>> PHY_INTERFACE_MODE_RGMII
>>> @Florian: could you please share your thoughts on this (who handles
>>> the TX delay in which case)?
>>
>> This also seems reasonable to do, provided that the PHY is also properly
>> configured not to add delays in both directions, and therefore assumes
>> that the MAC does it.
>>
>> We have a fairly large problem with how RGMII delays are done in PHYLIB
>> and Ethernet MAC drivers (or just in general), where we can't really
>> intersect properly what a PHY is supporting (in terms of internal
>> delays), and what the MAC supports either. One possible approach could
>> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
>> support (ideally, even with normalized nanosecond delay values), 
> 
> Just to make sure I understood this, the DT would say something like:
> 
> phy-connection-type = "rgmii-txid";
> txid-delay-ns = <3>;
> 
> For a 3ns TX delay, would that be good?

That's one possibility, although, see below, some PHYs support
sub-nanosecond values, but in general, that seems like a good
representation. If the "txid-delay-ns" property is omitted, a standard
2ns delay is assumed.

> 
>> and
>> then intersect that with the requested phy_interface_t during
>> phy_{attach,connect} time, and feed this back to the MAC with a special
>> error code/callback, so we could gracefully try to choose another
>> PHY_INTERFACE_MODE_* value that the MAC supports....
>>
>> A larger problem is that a number of drivers have been deployed, and
>> Device Trees, possibly with the meaning of "phy-mode" and
>> "phy-connection-type" being from the MAC perspective, and not the PHY
>> perspective *sigh*, good luck auditing those.
>>
>> So from there, here is possibly what we could do
>>
>> - submit a series of patches that update the PHYLIB documentation (there
>> are other things missing here) and make it clear from which entity (PHY
>> or MAC) does the delay apply to, document the "intersection" problem here
> 
> I think documenting is necessary, thanks in advance!
> 
> However, I'm wondering if there's a way to make this work in all cases.
> Indeed, if we consider for example that TX delay is required, we have 4
> cases:
> 
>        PHY         |       MAC          | Who applies?
> TXID supported     | TXID supported     | PHY
> TXID supported     | TXID not supported | PHY
> TXID not supported | TXID supported     | MAC
> TXID not supported | TXID not supported | cannot be done
> 
> That is basically what my patch:
> 
> https://marc.info/?l=linux-netdev&m=147869658031783&w=2
> 
> attempted to achieve. That would allow more combinations of MAC<->PHY to
> work, right?

Yes, indeed.

> 
> Nevertheless, I think we also need to keep in mind that most of this
> discussion assumes the case where both, MAC and PHY have equal capabilities.
> Could it happen that the PHY supports only 2ns delay, and the MAC only
> 1ns delay?

I doubt this exists at the MAC level what we should have is either a 2ns
delay, in either RX or TX path, or nothing, because that's the value
that results in shifting the data lines and the RX/TX lines by 90
degrees at 125Mhz (1/125^6 = 8 ns, one quarter shift is 90 degrees =
2ns). The PHY may have a similar set of pre-programmed, fixed 2ns
delays, but it is not uncommon to see 0.X ns resolution available:

drivers/net/phy/mscc.c
drivers/net/phy/dp83867.c w/ arch/arm/boot/dts/dra72-evm-revc.dts

In these cases, if you end-up using a non 2ns delay, you are fixing a
PCB problem more than an interoperability problem between your MAC and PHY.

> Could it happen that the delay is bigger than what is supported by
> either the PHY or MAC alone? maybe if combined it could work, for example
> a 3ns delay required and the PHY supporting 2ns and the MAC 1ns, combined
> it could work?

I suppose such a thing would work yes, but it would be difficult to
report correctly to the core PHYLIB how this can work considering the
vast array of options available to introduce delays in that case:
MAC-level, PHY-level, pinctrl/pad level and possibly at the PCB itself.

Once we can't rely on the fixed 2ns delay to work, we are going to have
people do various experiments until they can either measure what the
right delay value is for the specific PCB, or they just found the value
that happens to work. I don't think we can do much at that point from a
core PHYLIB perspective other than tell the network driver that the PHY
supports delay in either RX, TX or both directions, and have the MAC
decide what to apply that makes sense here, considering that this is
already kind of an exceptional situation to be in.
-- 
Florian
--
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

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Sebastian Frias]

On 25/11/16 18:44, Florian Fainelli wrote:
> On 11/25/2016 03:13 AM, Sebastian Frias wrote:
>> On 24/11/16 19:55, Florian Fainelli wrote:
<snip>
>>> Correct, the meaning of PHY_INTERFACE_MODE should be from the
>>> perspective of the PHY device:
>>>
>>> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
>>> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
>>> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
>>> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
>>>
>>
>> Thanks for the explanation.
>> Actually I had thought that the delay was to account for board routing
>> (wires) between the MAC and the PHY.
>> From your explanation it appears that the delay is to account for board
>> routing (wires) between the PHY and the RJ45 socket.
> 
> The placement of the (delay) was not meant to be exact, but it was
> wrongly place anyway, so it should be between the MAC and PHY, always.
> This is why you see people either fixing the need for a delay by
> appropriately programming the PHY, or the MAC, or by just inserting a
> fixed delay on the PCB between the PHY and the MAC and programming no
> delays (or using the default values and hoping this works).

Thanks.
Your patch "[PATCH net-next 3/4] Documentation: net: phy: Add blurb about
RGMII" on the documentation makes it clear.

>>>
>>> This also seems reasonable to do, provided that the PHY is also properly
>>> configured not to add delays in both directions, and therefore assumes
>>> that the MAC does it.
>>>
>>> We have a fairly large problem with how RGMII delays are done in PHYLIB
>>> and Ethernet MAC drivers (or just in general), where we can't really
>>> intersect properly what a PHY is supporting (in terms of internal
>>> delays), and what the MAC supports either. One possible approach could
>>> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
>>> support (ideally, even with normalized nanosecond delay values), 
>>
>> Just to make sure I understood this, the DT would say something like:
>>
>> phy-connection-type = "rgmii-txid";
>> txid-delay-ns = <3>;
>>
>> For a 3ns TX delay, would that be good?
> 
> That's one possibility, although, see below, some PHYs support
> sub-nanosecond values, but in general, that seems like a good
> representation. If the "txid-delay-ns" property is omitted, a standard
> 2ns delay is assumed.

Sounds good.
I did not see the "txid-delay-ns" property documented in your patches, if
it is not too late, maybe it could be "txid-delay-ps" using picoseconds as
unit, right?

>>> and
>>> then intersect that with the requested phy_interface_t during
>>> phy_{attach,connect} time, and feed this back to the MAC with a special
>>> error code/callback, so we could gracefully try to choose another
>>> PHY_INTERFACE_MODE_* value that the MAC supports....
>>>
>>> A larger problem is that a number of drivers have been deployed, and
>>> Device Trees, possibly with the meaning of "phy-mode" and
>>> "phy-connection-type" being from the MAC perspective, and not the PHY
>>> perspective *sigh*, good luck auditing those.
>>>
>>> So from there, here is possibly what we could do
>>>
>>> - submit a series of patches that update the PHYLIB documentation (there
>>> are other things missing here) and make it clear from which entity (PHY
>>> or MAC) does the delay apply to, document the "intersection" problem here
>>
>> I think documenting is necessary, thanks in advance!
>>
>> However, I'm wondering if there's a way to make this work in all cases.
>> Indeed, if we consider for example that TX delay is required, we have 4
>> cases:
>>
>>        PHY         |       MAC          | Who applies?
>> TXID supported     | TXID supported     | PHY
>> TXID supported     | TXID not supported | PHY
>> TXID not supported | TXID supported     | MAC
>> TXID not supported | TXID not supported | cannot be done
>>
>> That is basically what my patch:
>>
>> https://marc.info/?l=linux-netdev&m=147869658031783&w=2
>>
>> attempted to achieve. That would allow more combinations of MAC<->PHY to
>> work, right?
> 
> Yes, indeed.

Just one thing, from your patch "[PATCH net-next 3/4] Documentation: net:
phy: Add blurb about RGMII" I have the impression that the 3rd option from
the table above, would be a little bit more complex to implement.

I will comment on the patch.

>> Nevertheless, I think we also need to keep in mind that most of this
>> discussion assumes the case where both, MAC and PHY have equal capabilities.
>> Could it happen that the PHY supports only 2ns delay, and the MAC only
>> 1ns delay?
> 
> I doubt this exists at the MAC level what we should have is either a 2ns
> delay, in either RX or TX path, or nothing, because that's the value
> that results in shifting the data lines and the RX/TX lines by 90
> degrees at 125Mhz (1/125^6 = 8 ns, one quarter shift is 90 degrees =
> 2ns). The PHY may have a similar set of pre-programmed, fixed 2ns
> delays, but it is not uncommon to see 0.X ns resolution available:
> 
> drivers/net/phy/mscc.c
> drivers/net/phy/dp83867.c w/ arch/arm/boot/dts/dra72-evm-revc.dts
> 
> In these cases, if you end-up using a non 2ns delay, you are fixing a
> PCB problem more than an interoperability problem between your MAC and PHY.

I see, thanks.

>> Could it happen that the delay is bigger than what is supported by
>> either the PHY or MAC alone? maybe if combined it could work, for example
>> a 3ns delay required and the PHY supporting 2ns and the MAC 1ns, combined
>> it could work?
> 
> I suppose such a thing would work yes, but it would be difficult to
> report correctly to the core PHYLIB how this can work considering the
> vast array of options available to introduce delays in that case:
> MAC-level, PHY-level, pinctrl/pad level and possibly at the PCB itself.
> 
> Once we can't rely on the fixed 2ns delay to work, we are going to have
> people do various experiments until they can either measure what the
> right delay value is for the specific PCB, or they just found the value
> that happens to work. I don't think we can do much at that point from a
> core PHYLIB perspective other than tell the network driver that the PHY
> supports delay in either RX, TX or both directions, and have the MAC
> decide what to apply that makes sense here, considering that this is
> already kind of an exceptional situation to be in.


Fair enough.

And thanks again for documenting this.
--
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

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Jerome Brunet]

On Thu, 2016-11-24 at 18:05 +0100, Martin Blumenstingl wrote:
> On Thu, Nov 24, 2016 at 4:56 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> wrote:
> > 
> > On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> > > 
> > > Currently the dwmac-meson8b stmmac glue driver uses a hardcoded
> > > 1/4
> > > cycle TX clock delay. This seems to work fine for many boards
> > > (for
> > > example Odroid-C2 or Amlogic's reference boards) but there are
> > > some
> > > others where TX traffic is simply broken.
> > > There are probably multiple reasons why it's working on some
> > > boards
> > > while it's broken on others:
> > > - some of Amlogic's reference boards are using a Micrel PHY
> > > - hardware circuit design
> > > - maybe more...
> > > 
> > > This raises a question though:
> > > Which device is supposed to enable the TX delay when both MAC and
> > > PHY
> > > support it? And should we implement it for each PHY / MAC
> > > separately
> > > or should we think about a more generic solution (currently it's
> > > not
> > > possible to disable the TX delay generated by the RTL8211F PHY
> > > via
> > > devicetree when using phy-mode "rgmii")?
> > 
> > Actually you can skip the part which activate the Tx-delay on the
> > phy
> > by setting "phy-mode = "rgmii-id" instead of "rgmii"
> > 
> > phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
> > but PHY_INTERFACE_MODE_RGMII_ID.
> unfortunately this is not true for RTL8211F (I did my previous tests
> with the same expectation in mind)!
> the code seems to suggest that TX-delay is disabled whenever mode !=
> PHY_INTERFACE_MODE_RGMII.
> BUT: on my device RTL8211F_TX_DELAY is set even before
> "phy_write(phydev, 0x11, reg);"!

Thx a lot for pointing this out. I wrongly assumed that is was off by
default. It turns out it is ON on the OdroidC2 as well.

So I re-did all my test:
On the OdroidC2, as long as eee is not disabled (using ethtool or
through my patch) I can't get iperf to work properly. Disabling EEE,
the link works for the following configurations:

PHY TX Delay bit set : MAC delay 0ns, 2ns.
PHY TX Delay bit cleared : MAC delay 2ns, 4ns.

This seems to confirm that the PHY adds a 2ns delay when the bit is
set.

I could not test on my MXQPRO2.1 ... the PHY is no longer found during
the boot. Something died overnight :( 

> 
> Based on what I found it seems that rgmii-id, rgmii-txid and
> rgmii-rxid are supposed to be handled by the PHY.
> That would mean that we have two problems here:
> 1) drivers/net/phy/realtek.c:rtl8211f_config_init should check for
> PHY_INTERFACE_MODE_RGMII_ID or PHY_INTERFACE_MODE_RGMII_TXID and
> enable the TX-delay in that case - otherwise explicitly disable it
> 2) dwmac-meson8b.c should only use the configured TX-delay for
> PHY_INTERFACE_MODE_RGMII
> @Florian: could you please share your thoughts on this (who handles
> the TX delay in which case)?
> 
> 
> Regards,
> Martin
--
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

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Jerome Brunet]

On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
> 
> This raises a question though:
> Which device is supposed to enable the TX delay when both MAC and PHY
> support it? And should we implement it for each PHY / MAC separately
> or should we think about a more generic solution (currently it's not
> possible to disable the TX delay generated by the RTL8211F PHY via
> devicetree when using phy-mode "rgmii")?
> 
> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
> TX clock delay disabled on the MAC (as it's enabled in the PHY
> driver).
> TX throughput was virtually zero before:
> $ iperf3 -c 192.168.1.100 -R          
> Connecting to host 192.168.1.100, port 5201
> Reverse mode, remote host 192.168.1.100 is sending
> [  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-1.00   sec   108 MBytes   901
> Mbits/sec                  
> [  4]   1.00-2.00   sec  94.2 MBytes   791
> Mbits/sec                  
> [  4]   2.00-3.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   3.00-4.00   sec  96.2 MBytes   808
> Mbits/sec                  
> [  4]   4.00-5.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   5.00-6.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   6.00-7.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   7.00-8.00   sec  96.5 MBytes   809
> Mbits/sec                  
> [  4]   8.00-9.00   sec   105 MBytes   884
> Mbits/sec                  
> [  4]   9.00-10.00  sec   111 MBytes   934
> Mbits/sec                  
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.00  sec  1000 MBytes   839
> Mbits/sec    0             sender
> [  4]   0.00-10.00  sec   998 MBytes   837
> Mbits/sec                  receiver
> 
> iperf Done.
> $ iperf3 -c 192.168.1.100   
> Connecting to host 192.168.1.100, port 5201
> [  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139
> KBytes       
> [  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
> KBytes       
> [  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187
> KBytes       
> [  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143
> KBytes       
> [  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129
> KBytes       
> [  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195
> KBytes       
> [  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
> KBytes       
> [  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
> KBytes       
> [  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107
> KBytes       
> [  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139
> KBytes       
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.01  sec  1.02 GBytes   874
> Mbits/sec  1189             sender
> [  4]   0.00-10.01  sec  1.02 GBytes   873
> Mbits/sec                  receiver
> 

Cool, one more board working ;)
I tried your patch on another board (MXQ_V2.1, with sheep printed on
the PCB). It 's not improving the situation for this one unfortunately.
Actually I already tried playing with the TX delay on the MAC and PHY
but I could get any good results with the boards I have.

It is strange that we can adjust the delay by 2ns steps, when delay
seens by the phy should be 2ns ...

> iperf Done.
> 
> 
> Martin Blumenstingl (2):
>   net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
>   net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
> 
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
> +++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
> +++++++++++-----
>  include/dt-bindings/net/dwmac-meson8b.h               | 18
> ++++++++++++++++++
>  3 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
> 

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Giuseppe CAVALLARO]

On 11/24/2016 5:08 PM, Jerome Brunet wrote:
> On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>> cycle TX clock delay. This seems to work fine for many boards (for
>> example Odroid-C2 or Amlogic's reference boards) but there are some
>> others where TX traffic is simply broken.
>> There are probably multiple reasons why it's working on some boards
>> while it's broken on others:
>> - some of Amlogic's reference boards are using a Micrel PHY
>> - hardware circuit design
>> - maybe more...
>>
>> This raises a question though:
>> Which device is supposed to enable the TX delay when both MAC and PHY
>> support it? And should we implement it for each PHY / MAC separately
>> or should we think about a more generic solution (currently it's not
>> possible to disable the TX delay generated by the RTL8211F PHY via
>> devicetree when using phy-mode "rgmii")?
>>
>> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
>> TX clock delay disabled on the MAC (as it's enabled in the PHY
>> driver).
>> TX throughput was virtually zero before:
>> $ iperf3 -c 192.168.1.100 -R
>> Connecting to host 192.168.1.100, port 5201
>> Reverse mode, remote host 192.168.1.100 is sending
>> [  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
>> 5201
>> [ ID] Interval           Transfer     Bandwidth
>> [  4]   0.00-1.00   sec   108 MBytes   901
>> Mbits/sec
>> [  4]   1.00-2.00   sec  94.2 MBytes   791
>> Mbits/sec
>> [  4]   2.00-3.00   sec  96.5 MBytes   810
>> Mbits/sec
>> [  4]   3.00-4.00   sec  96.2 MBytes   808
>> Mbits/sec
>> [  4]   4.00-5.00   sec  96.6 MBytes   810
>> Mbits/sec
>> [  4]   5.00-6.00   sec  96.5 MBytes   810
>> Mbits/sec
>> [  4]   6.00-7.00   sec  96.6 MBytes   810
>> Mbits/sec
>> [  4]   7.00-8.00   sec  96.5 MBytes   809
>> Mbits/sec
>> [  4]   8.00-9.00   sec   105 MBytes   884
>> Mbits/sec
>> [  4]   9.00-10.00  sec   111 MBytes   934
>> Mbits/sec
>> - - - - - - - - - - - - - - - - - - - - - - - - -
>> [ ID] Interval           Transfer     Bandwidth       Retr
>> [  4]   0.00-10.00  sec  1000 MBytes   839
>> Mbits/sec    0             sender
>> [  4]   0.00-10.00  sec   998 MBytes   837
>> Mbits/sec                  receiver
>>
>> iperf Done.
>> $ iperf3 -c 192.168.1.100
>> Connecting to host 192.168.1.100, port 5201
>> [  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
>> 5201
>> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
>> [  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139
>> KBytes
>> [  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
>> KBytes
>> [  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187
>> KBytes
>> [  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143
>> KBytes
>> [  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129
>> KBytes
>> [  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195
>> KBytes
>> [  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
>> KBytes
>> [  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
>> KBytes
>> [  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107
>> KBytes
>> [  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139
>> KBytes
>> - - - - - - - - - - - - - - - - - - - - - - - - -
>> [ ID] Interval           Transfer     Bandwidth       Retr
>> [  4]   0.00-10.01  sec  1.02 GBytes   874
>> Mbits/sec  1189             sender
>> [  4]   0.00-10.01  sec  1.02 GBytes   873
>> Mbits/sec                  receiver
>>
>
> Cool, one more board working ;)
> I tried your patch on another board (MXQ_V2.1, with sheep printed on
> the PCB). It 's not improving the situation for this one unfortunately.
> Actually I already tried playing with the TX delay on the MAC and PHY
> but I could get any good results with the boards I have.
>
> It is strange that we can adjust the delay by 2ns steps, when delay
> seens by the phy should be 2ns ...
>

Ok, as said, I could expect that extra delay was needed on GiGa.
FYI, on ST platforms, this extra delay was added in the pintctrl
dtsi. I mean, in some way, ad-hoc setup was solved at device-tree
level. Usually, extra delay could depend on the PCB.

peppe

>> iperf Done.
>>
>>
>> Martin Blumenstingl (2):
>>   net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
>>   net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
>>
>>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
>> +++++++++++
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
>> +++++++++++-----
>>  include/dt-bindings/net/dwmac-meson8b.h               | 18
>> ++++++++++++++++++
>>  3 files changed, 40 insertions(+), 5 deletions(-)
>>  create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
>>
>

--
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 v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle TX clock delay. This seems to work fine for many boards (for
example Odroid-C2 or Amlogic's reference boards) but there are some
others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901 Mbits/sec
[  4]   1.00-2.00   sec  94.2 MBytes   791 Mbits/sec
[  4]   2.00-3.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   3.00-4.00   sec  96.2 MBytes   808 Mbits/sec
[  4]   4.00-5.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   5.00-6.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   6.00-7.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   7.00-8.00   sec  96.5 MBytes   809 Mbits/sec
[  4]   8.00-9.00   sec   105 MBytes   884 Mbits/sec
[  4]   9.00-10.00  sec   111 MBytes   934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  1000 MBytes   839 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   998 MBytes   837 Mbits/sec                  receiver

iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139 KBytes
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7 KBytes
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187 KBytes
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143 KBytes
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129 KBytes
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195 KBytes
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7 KBytes
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6 KBytes
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107 KBytes
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874 Mbits/sec  1189             sender
[  4]   0.00-10.01  sec  1.02 GBytes   873 Mbits/sec                  receiver

iperf Done.

I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
So changes to at least the RTL8211F PHY driver are needed to get it
working properly in all situations.


NOTE: patches 3-7 should be taken though the Amlogic tree. patches 1
and 2 can be taken through the net-tree or through the Amlogic tree.
There shouldn't be a runtime dependency as long as phy-mode "rgmii"
(or the not-relevant-for-this-case "rmii") is used (which is currently
the case for all ARM64 meson-gx boards) due to the dwmac-meson8b's
default 2ns TX-delay.


Changes since v1:
- renamed the devicetree property "amlogic,tx-delay" to
  "amlogic,tx-delay-ns", which makes the .dts easier to read as we can
  simply specify human-readable values instead of having "preprocessor
  defines and calculation in human brain". Thanks to Andrew Lunn for
  the suggestion!
- improved documentation to indicate when the MAC TX-delay should be
  configured and how to use the PHY's TX-delay
- changed the default TX-delay in the dwmac-meson8b driver from 2ns
  to 0ms when any of the rgmii-*id modes are used (the 2ns default
  value still applies for phy-mode "rgmii")
- added patches to properly reset the PHY on Meson GXBB devices and to
  use a similar configuration than the one we use on Meson GXL devices
  (by passing a phy-handle to stmmac and defining the PHY in the mdio0
  bus - patch 3-6)
- add the "amlogic,tx-delay-ns" property to all boards which are using
  the RGMII PHY (patch 7)


Martin Blumenstingl (7):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
  ARM64: dts: meson-gx: move the MDIO node to meson-gx
  ARM64: dts: meson-gxbb-odroidc2: add reset for the ethernet PHY
  ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
  ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY
  ARM64: dts: amlogic: add the ethernet TX delay configuration

 .../devicetree/bindings/net/meson-dwmac.txt        | 14 ++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi          |  6 +++++
 .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 17 ++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   | 17 ++++++++++++++
 .../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi      | 17 ++++++++++++++
 .../boot/dts/amlogic/meson-gxl-s905d-p230.dts      |  2 ++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  6 -----
 .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  2 ++
 .../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts |  2 ++
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 26 +++++++++++++++++-----
 10 files changed, 97 insertions(+), 12 deletions(-)

-- 
2.10.2

[PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

This allows configuring the RGMII TX clock delay. The RGMII clock is
generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
The configuration depends on the actual hardware (no delay may be
needed due to the design of the actual circuit, the PHY might add this
delay, etc.).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62dd..f8bc540 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
 		- "clkin0" - first parent clock of the internal mux
 		- "clkin1" - second parent clock of the internal mux
 
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
+			by this driver) in nanoseconds. Allowed values
+			are: 0ns, 2ns, 4ns, 6ns.
+			This must be configured when the phy-mode is
+			"rgmii" (typically a value of 2ns is used in
+			this case).
+			When phy-mode is set to "rgmii-id" or
+			"rgmii-txid" the TX clock delay is already
+			provided by the PHY. In that case this
+			property should be set to 0ns (which disables
+			the TX clock delay in the MAC to prevent the
+			clock from going off because both PHY and MAC
+			are adding a delay).
 
 Example for Meson6:
 
-- 
2.10.2

Re: [PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Rob Herring]

On Fri, Nov 25, 2016 at 02:01:50PM +0100, Martin Blumenstingl wrote:
> This allows configuring the RGMII TX clock delay. The RGMII clock is
> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
> The configuration depends on the actual hardware (no delay may be
> needed due to the design of the actual circuit, the PHY might add this
> delay, etc.).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
> index 89e62dd..f8bc540 100644
> --- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
> @@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
>  		- "clkin0" - first parent clock of the internal mux
>  		- "clkin1" - second parent clock of the internal mux
>  
> +Optional properties on Meson8b and newer:
> +- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
> +			by this driver) in nanoseconds. Allowed values
> +			are: 0ns, 2ns, 4ns, 6ns.
> +			This must be configured when the phy-mode is
> +			"rgmii" (typically a value of 2ns is used in
> +			this case).
> +			When phy-mode is set to "rgmii-id" or
> +			"rgmii-txid" the TX clock delay is already
> +			provided by the PHY. In that case this
> +			property should be set to 0ns (which disables
> +			the TX clock delay in the MAC to prevent the
> +			clock from going off because both PHY and MAC
> +			are adding a delay).

What's the default? Why can't no property mean 0ns delay?

>  
>  Example for Meson6:
>  
> -- 
> 2.10.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

Re: [PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

On Wed, Nov 30, 2016 at 10:44 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Nov 25, 2016 at 02:01:50PM +0100, Martin Blumenstingl wrote:
>> This allows configuring the RGMII TX clock delay. The RGMII clock is
>> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
>> The configuration depends on the actual hardware (no delay may be
>> needed due to the design of the actual circuit, the PHY might add this
>> delay, etc.).
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> index 89e62dd..f8bc540 100644
>> --- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> +++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> @@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
>>               - "clkin0" - first parent clock of the internal mux
>>               - "clkin1" - second parent clock of the internal mux
>>
>> +Optional properties on Meson8b and newer:
>> +- amlogic,tx-delay-ns:       The internal RGMII TX clock delay (provided
>> +                     by this driver) in nanoseconds. Allowed values
>> +                     are: 0ns, 2ns, 4ns, 6ns.
>> +                     This must be configured when the phy-mode is
>> +                     "rgmii" (typically a value of 2ns is used in
>> +                     this case).
>> +                     When phy-mode is set to "rgmii-id" or
>> +                     "rgmii-txid" the TX clock delay is already
>> +                     provided by the PHY. In that case this
>> +                     property should be set to 0ns (which disables
>> +                     the TX clock delay in the MAC to prevent the
>> +                     clock from going off because both PHY and MAC
>> +                     are adding a delay).
>
> What's the default? Why can't no property mean 0ns delay?
This value (2ns) was previously hardcoded. Having a default of 0ns
means that patch 7 ("ARM64: dts: amlogic: add the ethernet TX delay
configuration") becomes mandatory (otherwise we'll have broken Gbit
ethernet again because the TX delay is now disabled in the PHY when
phy-mode is "rgmii" and additionally we don't apply the 2ns TX delay
on the MAC side when the property is missing).
I'm fine with either way though - just let me know so I can adjust the
code accordingly.

[PATCH v2 2/7] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Prior to this patch we were using a hardcoded RGMII TX clock delay of
2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
many boards, but unfortunately not for all (due to the way the actual
circuit is designed, sometimes because the TX delay is enabled in the
PHY, etc.). Making the TX delay on the MAC side configurable allows us
to support all possible hardware combinations.

This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 26 +++++++++++++++++-----
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..8ba33be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -35,10 +35,6 @@
 
 #define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF		(0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER		(0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF		(0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS	(0x3 << PRG_ETH0_TXDLY_SHIFT)
 
 /* divider for the result of m250_sel */
 #define PRG_ETH0_CLK_M250_DIV_SHIFT	7
@@ -69,6 +65,8 @@ struct meson8b_dwmac {
 
 	struct clk_divider	m25_div;
 	struct clk		*m25_div_clk;
+
+	u32			tx_delay_ns;
 };
 
 static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -179,6 +177,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
 	unsigned long clk_rate;
+	u8 tx_dly_val;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
@@ -196,9 +195,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		/* TX clock delay - all known boards use a 1/4 cycle delay */
+		/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
+		 * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
+		 * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+		 */
+		tx_dly_val = dwmac->tx_delay_ns >> 1;
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					PRG_ETH0_TXDLY_QUARTER);
+					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
@@ -277,6 +280,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 	if (dwmac->phy_mode < 0) {
 		dev_err(&pdev->dev, "missing phy-mode property\n");
 		return -EINVAL;
+	} else if (dwmac->phy_mode != PHY_INTERFACE_MODE_RMII) {
+		ret = of_property_read_u32(pdev->dev.of_node,
+					   "amlogic,tx-delay-ns",
+					   &dwmac->tx_delay_ns);
+		if (ret && dwmac->phy_mode == PHY_INTERFACE_MODE_RGMII)
+			/* default to a TX clock delay of 2ns when the PHY is
+			 * connected via RGMII (with RGMII_ID and RGMII_TXID
+			 * the TX clock delay is generated by the PHY and thus
+			 * we use the default 0ns delay in these case).
+			 */
+			dwmac->tx_delay_ns = 2;
 	}
 
 	ret = meson8b_init_clk(dwmac);
-- 
2.10.2

[PATCH v2 3/7] ARM64: dts: meson-gx: move the MDIO node to meson-gx

[Martin Blumenstingl]

stmmac's MDIO bus is currently only defined in meson-gxl.dtsi. Move it
up to meson-gx to allow us to keep the stmmac configuration for
meson-gxbb similar to the configuration on meson-gxl.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 6 ++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 47ab306..a2c3ca6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -371,6 +371,12 @@
 			interrupt-names = "macirq";
 			phy-mode = "rgmii";
 			status = "disabled";
+
+			mdio0: mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "snps,dwmac-mdio";
+			};
 		};
 
 		apb: apb@d0000000 {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 3af54dc..aa3cd80 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -57,12 +57,6 @@
 		 <&clkc CLKID_FCLK_DIV2>,
 		 <&clkc CLKID_MPLL2>;
 	clock-names = "stmmaceth", "clkin0", "clkin1";
-
-	mdio0: mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		compatible = "snps,dwmac-mdio";
-	};
 };
 
 &aobus {
-- 
2.10.2

RE: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay

[David Laight]

From: Martin Blumenstingl
> Sent: 25 November 2016 13:02
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
...

Interesting thought.
Can you test phy loopback with different delays?
It might be then possible to default to 'auto'.

	David

[PATCH v2 4/7] ARM64: dts: meson-gxbb-odroidc2: add reset for the ethernet PHY

[Martin Blumenstingl]

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..cbaf024 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -143,10 +143,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &ir {
-- 
2.10.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 v2 5/7] ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY

[Martin Blumenstingl]

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..2abc553 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -134,10 +134,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &ir {
-- 
2.10.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 v2 6/7] ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY

[Martin Blumenstingl]

This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index e59ad30..a0e92e3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -113,10 +113,25 @@
 	pinctrl-names = "default";
 };
 
+&mdio0 {
+	ethernet_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+	};
+};
+
 &ethmac {
 	status = "okay";
 	pinctrl-0 = <&eth_rgmii_pins>;
 	pinctrl-names = "default";
+
+	phy-handle = <&ethernet_phy0>;
+
+	snps,reset-gpio = <&gpio GPIOZ_14 0>;
+	snps,reset-delays-us = <0 10000 1000000>;
+	snps,reset-active-low;
+
+	phy-mode = "rgmii";
 };
 
 &usb0_phy {
-- 
2.10.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 v2 7/7] ARM64: dts: amlogic: add the ethernet TX delay configuration

[Martin Blumenstingl]

This adds the amlogic,tx-delay-ns with the old (hardcoded) default value
of 2ns to all boards which are using an RGMII ethernet PHY.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts  | 2 ++
 arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi     | 2 ++
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 ++
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 ++
 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts  | 2 ++
 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts  | 2 ++
 6 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index cbaf024..fdade07 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -161,6 +161,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	phy-mode = "rgmii";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 2abc553..8172e12 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -152,6 +152,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	phy-mode = "rgmii";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index a0e92e3..ab49712 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -131,6 +131,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	phy-mode = "rgmii";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index f66939c..7fd11c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -64,6 +64,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	/* External PHY is in RGMII */
 	phy-mode = "rgmii";
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index d320727..f83d6dc 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -156,6 +156,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	/* External PHY is in RGMII */
 	phy-mode = "rgmii";
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
index 5dbc660..e428e29 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
@@ -64,6 +64,8 @@
 	snps,reset-delays-us = <0 10000 1000000>;
 	snps,reset-active-low;
 
+	amlogic,tx-delay-ns = <2>;
+
 	/* External PHY is in RGMII */
 	phy-mode = "rgmii";
 };
-- 
2.10.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

Re: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay

[David Miller]

From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Date: Fri, 25 Nov 2016 14:01:49 +0100

> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...

The ARM arch file changes do not apply cleanly to net-next, you probably
want to merge them via the ARM tree instead of mine, and respin this series
to be without the .dts file changes.
--
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

Re: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

On Mon, Nov 28, 2016 at 2:33 AM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Date: Fri, 25 Nov 2016 14:01:49 +0100
>
>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>> cycle TX clock delay. This seems to work fine for many boards (for
>> example Odroid-C2 or Amlogic's reference boards) but there are some
>> others where TX traffic is simply broken.
>> There are probably multiple reasons why it's working on some boards
>> while it's broken on others:
>> - some of Amlogic's reference boards are using a Micrel PHY
>> - hardware circuit design
>> - maybe more...
>
> The ARM arch file changes do not apply cleanly to net-next, you probably
> want to merge them via the ARM tree instead of mine, and respin this series
> to be without the .dts file changes.
done, v3 contains only the net-next changes while the dts changes can
be found here: [0]


Regards,
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-December/001836.html
--
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 net-next v3 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle (= 2ns) TX clock delay. This seems to work fine for many boards
(for example Odroid-C2 or Amlogic's reference boards) but there are
some others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901 Mbits/sec
[  4]   1.00-2.00   sec  94.2 MBytes   791 Mbits/sec
[  4]   2.00-3.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   3.00-4.00   sec  96.2 MBytes   808 Mbits/sec
[  4]   4.00-5.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   5.00-6.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   6.00-7.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   7.00-8.00   sec  96.5 MBytes   809 Mbits/sec
[  4]   8.00-9.00   sec   105 MBytes   884 Mbits/sec
[  4]   9.00-10.00  sec   111 MBytes   934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  1000 MBytes   839 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   998 MBytes   837 Mbits/sec                  receiver

iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139 KBytes
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7 KBytes
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187 KBytes
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143 KBytes
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129 KBytes
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195 KBytes
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7 KBytes
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6 KBytes
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107 KBytes
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874 Mbits/sec  1189             sender
[  4]   0.00-10.01  sec  1.02 GBytes   873 Mbits/sec                  receiver

iperf Done.

I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
So changes to at least the RTL8211F PHY driver are needed to get it
working properly in all situations.

Changes since v2:
- moved all .dts patches (3-7) to a separate series
- removed the default 2ns TX delay when phy-mode RGMII is specified
- (rebased against current net-next)

Changes since v1:
- renamed the devicetree property "amlogic,tx-delay" to
  "amlogic,tx-delay-ns", which makes the .dts easier to read as we can
  simply specify human-readable values instead of having "preprocessor
  defines and calculation in human brain". Thanks to Andrew Lunn for
  the suggestion!
- improved documentation to indicate when the MAC TX-delay should be
  configured and how to use the PHY's TX-delay
- changed the default TX-delay in the dwmac-meson8b driver from 2ns
  to 0ms when any of the rgmii-*id modes are used (the 2ns default
  value still applies for phy-mode "rgmii")
- added patches to properly reset the PHY on Meson GXBB devices and to
  use a similar configuration than the one we use on Meson GXL devices
  (by passing a phy-handle to stmmac and defining the PHY in the mdio0
  bus - patch 3-6)
- add the "amlogic,tx-delay-ns" property to all boards which are using
  the RGMII PHY (patch 7)


Martin Blumenstingl (2):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

 .../devicetree/bindings/net/meson-dwmac.txt         | 14 ++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.10.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 net-next v3 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

This allows configuring the RGMII TX clock delay. The RGMII clock is
generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
The configuration depends on the actual hardware (no delay may be
needed due to the design of the actual circuit, the PHY might add this
delay, etc.).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62dd..f8bc540 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
 		- "clkin0" - first parent clock of the internal mux
 		- "clkin1" - second parent clock of the internal mux
 
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
+			by this driver) in nanoseconds. Allowed values
+			are: 0ns, 2ns, 4ns, 6ns.
+			This must be configured when the phy-mode is
+			"rgmii" (typically a value of 2ns is used in
+			this case).
+			When phy-mode is set to "rgmii-id" or
+			"rgmii-txid" the TX clock delay is already
+			provided by the PHY. In that case this
+			property should be set to 0ns (which disables
+			the TX clock delay in the MAC to prevent the
+			clock from going off because both PHY and MAC
+			are adding a delay).
 
 Example for Meson6:
 
-- 
2.10.2

Re: [PATCH net-next v3 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Rob Herring]

On Sat, Dec 03, 2016 at 12:32:37AM +0100, Martin Blumenstingl wrote:
> This allows configuring the RGMII TX clock delay. The RGMII clock is
> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
> The configuration depends on the actual hardware (no delay may be
> needed due to the design of the actual circuit, the PHY might add this
> delay, etc.).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 net-next v3 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Prior to this patch we were using a hardcoded RGMII TX clock delay of
2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
many boards, but unfortunately not for all (due to the way the actual
circuit is designed, sometimes because the TX delay is enabled in the
PHY, etc.). Making the TX delay on the MAC side configurable allows us
to support all possible hardware combinations.

This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 250e4ce..dad31b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -35,10 +35,6 @@
 
 #define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF		(0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER		(0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF		(0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS	(0x3 << PRG_ETH0_TXDLY_SHIFT)
 
 /* divider for the result of m250_sel */
 #define PRG_ETH0_CLK_M250_DIV_SHIFT	7
@@ -69,6 +65,8 @@ struct meson8b_dwmac {
 
 	struct clk_divider	m25_div;
 	struct clk		*m25_div_clk;
+
+	u32			tx_delay_ns;
 };
 
 static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -179,6 +177,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
 	unsigned long clk_rate;
+	u8 tx_dly_val;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
@@ -196,9 +195,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		/* TX clock delay - all known boards use a 1/4 cycle delay */
+		/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
+		 * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
+		 * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+		 */
+		tx_dly_val = dwmac->tx_delay_ns >> 1;
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					PRG_ETH0_TXDLY_QUARTER);
+					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
@@ -277,6 +280,12 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 	if (dwmac->phy_mode < 0) {
 		dev_err(&pdev->dev, "missing phy-mode property\n");
 		return -EINVAL;
+	} else if (dwmac->phy_mode != PHY_INTERFACE_MODE_RMII) {
+		/* ignore errors as this is an optional property - by default
+		 * we assume a TX delay of 0ns.
+		 */
+		of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay-ns",
+				     &dwmac->tx_delay_ns);
 	}
 
 	ret = meson8b_init_clk(dwmac);
-- 
2.10.2

[PATCH net-next v4 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle (= 2ns) TX clock delay. This seems to work fine for many boards
(for example Odroid-C2 or Amlogic's reference boards) but there are
some others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901 Mbits/sec
[  4]   1.00-2.00   sec  94.2 MBytes   791 Mbits/sec
[  4]   2.00-3.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   3.00-4.00   sec  96.2 MBytes   808 Mbits/sec
[  4]   4.00-5.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   5.00-6.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   6.00-7.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   7.00-8.00   sec  96.5 MBytes   809 Mbits/sec
[  4]   8.00-9.00   sec   105 MBytes   884 Mbits/sec
[  4]   9.00-10.00  sec   111 MBytes   934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  1000 MBytes   839 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   998 MBytes   837 Mbits/sec                  receiver

iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139 KBytes
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7 KBytes
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187 KBytes
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143 KBytes
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129 KBytes
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195 KBytes
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7 KBytes
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6 KBytes
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107 KBytes
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874 Mbits/sec  1189             sender
[  4]   0.00-10.01  sec  1.02 GBytes   873 Mbits/sec                  receiver

iperf Done.

I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
So changes to at least the RTL8211F PHY driver are needed to get it
working properly in all situations.

Changes since v3:
- rebased to apply against current net-next branch (fixes a conflict
  with d2ed0a7755fe14c7 "net: ethernet: stmmac: fix of-node and
  fixed-link-phydev leaks")
- 

Changes since v2:
- moved all .dts patches (3-7) to a separate series
- removed the default 2ns TX delay when phy-mode RGMII is specified
- (rebased against current net-next)

Changes since v1:
- renamed the devicetree property "amlogic,tx-delay" to
  "amlogic,tx-delay-ns", which makes the .dts easier to read as we can
  simply specify human-readable values instead of having "preprocessor
  defines and calculation in human brain". Thanks to Andrew Lunn for
  the suggestion!
- improved documentation to indicate when the MAC TX-delay should be
  configured and how to use the PHY's TX-delay
- changed the default TX-delay in the dwmac-meson8b driver from 2ns
  to 0ms when any of the rgmii-*id modes are used (the 2ns default
  value still applies for phy-mode "rgmii")
- added patches to properly reset the PHY on Meson GXBB devices and to
  use a similar configuration than the one we use on Meson GXL devices
  (by passing a phy-handle to stmmac and defining the PHY in the mdio0
  bus - patch 3-6)
- add the "amlogic,tx-delay-ns" property to all boards which are using
  the RGMII PHY (patch 7)

Martin Blumenstingl (2):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

 .../devicetree/bindings/net/meson-dwmac.txt         | 14 ++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.11.0

--
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 net-next v4 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

This allows configuring the RGMII TX clock delay. The RGMII clock is
generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
The configuration depends on the actual hardware (no delay may be
needed due to the design of the actual circuit, the PHY might add this
delay, etc.).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62ddc69ca..f8bc54094e3c 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
 		- "clkin0" - first parent clock of the internal mux
 		- "clkin1" - second parent clock of the internal mux
 
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
+			by this driver) in nanoseconds. Allowed values
+			are: 0ns, 2ns, 4ns, 6ns.
+			This must be configured when the phy-mode is
+			"rgmii" (typically a value of 2ns is used in
+			this case).
+			When phy-mode is set to "rgmii-id" or
+			"rgmii-txid" the TX clock delay is already
+			provided by the PHY. In that case this
+			property should be set to 0ns (which disables
+			the TX clock delay in the MAC to prevent the
+			clock from going off because both PHY and MAC
+			are adding a delay).
 
 Example for Meson6:
 
-- 
2.11.0

[PATCH net-next v4 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Prior to this patch we were using a hardcoded RGMII TX clock delay of
2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
many boards, but unfortunately not for all (due to the way the actual
circuit is designed, sometimes because the TX delay is enabled in the
PHY, etc.). Making the TX delay on the MAC side configurable allows us
to support all possible hardware combinations.

This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index ffaed1f35efe..db970cd6600f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -35,10 +35,6 @@
 
 #define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF		(0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER		(0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF		(0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS	(0x3 << PRG_ETH0_TXDLY_SHIFT)
 
 /* divider for the result of m250_sel */
 #define PRG_ETH0_CLK_M250_DIV_SHIFT	7
@@ -69,6 +65,8 @@ struct meson8b_dwmac {
 
 	struct clk_divider	m25_div;
 	struct clk		*m25_div_clk;
+
+	u32			tx_delay_ns;
 };
 
 static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -179,6 +177,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
 	unsigned long clk_rate;
+	u8 tx_dly_val;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
@@ -196,9 +195,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		/* TX clock delay - all known boards use a 1/4 cycle delay */
+		/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
+		 * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
+		 * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+		 */
+		tx_dly_val = dwmac->tx_delay_ns >> 1;
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					PRG_ETH0_TXDLY_QUARTER);
+					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
@@ -282,6 +285,12 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "missing phy-mode property\n");
 		ret = -EINVAL;
 		goto err_remove_config_dt;
+	} else if (dwmac->phy_mode != PHY_INTERFACE_MODE_RMII) {
+		/* ignore errors as this is an optional property - by default
+		 * we assume a TX delay of 0ns.
+		 */
+		of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay-ns",
+				     &dwmac->tx_delay_ns);
 	}
 
 	ret = meson8b_init_clk(dwmac);
-- 
2.11.0

Re: [PATCH net-next v4 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[David Miller]

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Sat, 17 Dec 2016 19:21:19 +0100

> Prior to this patch we were using a hardcoded RGMII TX clock delay of
> 2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
> many boards, but unfortunately not for all (due to the way the actual
> circuit is designed, sometimes because the TX delay is enabled in the
> PHY, etc.). Making the TX delay on the MAC side configurable allows us
> to support all possible hardware combinations.
> 
> This allows fixing a compatibility issue on some boards, where the
> RTL8211F PHY is configured to generate the TX delay. We can now turn
> off the TX delay in the MAC, because otherwise we would be applying the
> delay twice (which results in non-working TX traffic).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Tested-by: Neil Armstrong <narmstrong@baylibre.com>

Is this really the safest thing to do?

If you say the existing hard-coded setting of 1/4 cycle works on most
boards, and what you're trying to do is override it with an OF
property value for boards where the existing setting does not work,
then you _must_ use a default value that corresponds to what the
existing code does not when you don't see this new OF property.

So please retain the current behavior of the 1/4 cycle TX delay
setting when you don't see the amlogic,tx-delay-ns property.

I really think you risk breaking existing boards by not doing so,
unless you can have this patch tested on every such board that exists
and I don't think you really can feasibly and rigorously do that.

Thanks.

Re: [PATCH net-next v4 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

On Sun, Dec 18, 2016 at 4:49 PM, David Miller <davem@davemloft.net> wrote:
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Date: Sat, 17 Dec 2016 19:21:19 +0100
>
>> Prior to this patch we were using a hardcoded RGMII TX clock delay of
>> 2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
>> many boards, but unfortunately not for all (due to the way the actual
>> circuit is designed, sometimes because the TX delay is enabled in the
>> PHY, etc.). Making the TX delay on the MAC side configurable allows us
>> to support all possible hardware combinations.
>>
>> This allows fixing a compatibility issue on some boards, where the
>> RTL8211F PHY is configured to generate the TX delay. We can now turn
>> off the TX delay in the MAC, because otherwise we would be applying the
>> delay twice (which results in non-working TX traffic).
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Is this really the safest thing to do?
>
> If you say the existing hard-coded setting of 1/4 cycle works on most
> boards, and what you're trying to do is override it with an OF
> property value for boards where the existing setting does not work,
> then you _must_ use a default value that corresponds to what the
> existing code does not when you don't see this new OF property.
it's a bit more complicated in reality: 1/4 cycle works when the TX
delay of the RTL8211F PHY is turned off (until recently it was always
enabled for phy-mode RGMII).

> So please retain the current behavior of the 1/4 cycle TX delay
> setting when you don't see the amlogic,tx-delay-ns property.
>
> I really think you risk breaking existing boards by not doing so,
> unless you can have this patch tested on every such board that exists
> and I don't think you really can feasibly and rigorously do that.
there's a patch in my follow-up series which adds the 2ns to the .dts
for all RGMII based boards: [0] (and I would keep these even if we had
a default value, just to make it explicit and thus easier to
understand for other people).
however, we can add the 2ns default back (I can do this if you want -
Rob Herring was unhappy with the missing documentation of this default
value [1] - so note to myself: take care of that as well). but then we
have to decide when to apply this default value: only when we're in
RGMII mode or also in any of the RGMII_*ID modes?

please let me know how we should proceed


Regards,
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-December/001838.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001817.html

Re: [PATCH net-next v4 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Hi David,

On Sun, Dec 18, 2016 at 5:13 PM, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Sun, Dec 18, 2016 at 4:49 PM, David Miller <davem@davemloft.net> wrote:
>> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Date: Sat, 17 Dec 2016 19:21:19 +0100
>>
>>> Prior to this patch we were using a hardcoded RGMII TX clock delay of
>>> 2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
>>> many boards, but unfortunately not for all (due to the way the actual
>>> circuit is designed, sometimes because the TX delay is enabled in the
>>> PHY, etc.). Making the TX delay on the MAC side configurable allows us
>>> to support all possible hardware combinations.
>>>
>>> This allows fixing a compatibility issue on some boards, where the
>>> RTL8211F PHY is configured to generate the TX delay. We can now turn
>>> off the TX delay in the MAC, because otherwise we would be applying the
>>> delay twice (which results in non-working TX traffic).
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> Is this really the safest thing to do?
>>
>> If you say the existing hard-coded setting of 1/4 cycle works on most
>> boards, and what you're trying to do is override it with an OF
>> property value for boards where the existing setting does not work,
>> then you _must_ use a default value that corresponds to what the
>> existing code does not when you don't see this new OF property.
> it's a bit more complicated in reality: 1/4 cycle works when the TX
> delay of the RTL8211F PHY is turned off (until recently it was always
> enabled for phy-mode RGMII).
>
>> So please retain the current behavior of the 1/4 cycle TX delay
>> setting when you don't see the amlogic,tx-delay-ns property.
>>
>> I really think you risk breaking existing boards by not doing so,
>> unless you can have this patch tested on every such board that exists
>> and I don't think you really can feasibly and rigorously do that.
> there's a patch in my follow-up series which adds the 2ns to the .dts
> for all RGMII based boards: [0] (and I would keep these even if we had
> a default value, just to make it explicit and thus easier to
> understand for other people).
> however, we can add the 2ns default back (I can do this if you want -
> Rob Herring was unhappy with the missing documentation of this default
> value [1] - so note to myself: take care of that as well). but then we
> have to decide when to apply this default value: only when we're in
> RGMII mode or also in any of the RGMII_*ID modes?
>
> please let me know how we should proceed
gentle ping - what is your opinion on this?


Regards,
Martin

Re: [PATCH net-next v4 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Hi David,

On Sun, Dec 18, 2016 at 5:13 PM, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Sun, Dec 18, 2016 at 4:49 PM, David Miller <davem@davemloft.net> wrote:
>> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Date: Sat, 17 Dec 2016 19:21:19 +0100
>>
>>> Prior to this patch we were using a hardcoded RGMII TX clock delay of
>>> 2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
>>> many boards, but unfortunately not for all (due to the way the actual
>>> circuit is designed, sometimes because the TX delay is enabled in the
>>> PHY, etc.). Making the TX delay on the MAC side configurable allows us
>>> to support all possible hardware combinations.
>>>
>>> This allows fixing a compatibility issue on some boards, where the
>>> RTL8211F PHY is configured to generate the TX delay. We can now turn
>>> off the TX delay in the MAC, because otherwise we would be applying the
>>> delay twice (which results in non-working TX traffic).
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> Is this really the safest thing to do?
>>
>> If you say the existing hard-coded setting of 1/4 cycle works on most
>> boards, and what you're trying to do is override it with an OF
>> property value for boards where the existing setting does not work,
>> then you _must_ use a default value that corresponds to what the
>> existing code does not when you don't see this new OF property.
> it's a bit more complicated in reality: 1/4 cycle works when the TX
> delay of the RTL8211F PHY is turned off (until recently it was always
> enabled for phy-mode RGMII).
>
>> So please retain the current behavior of the 1/4 cycle TX delay
>> setting when you don't see the amlogic,tx-delay-ns property.
>>
>> I really think you risk breaking existing boards by not doing so,
>> unless you can have this patch tested on every such board that exists
>> and I don't think you really can feasibly and rigorously do that.
> there's a patch in my follow-up series which adds the 2ns to the .dts
> for all RGMII based boards: [0] (and I would keep these even if we had
> a default value, just to make it explicit and thus easier to
> understand for other people).
> however, we can add the 2ns default back (I can do this if you want -
> Rob Herring was unhappy with the missing documentation of this default
> value [1] - so note to myself: take care of that as well). but then we
> have to decide when to apply this default value: only when we're in
> RGMII mode or also in any of the RGMII_*ID modes?
could you please let me know if I should add a a fallback (2ns which
was the old hardcoded value) to the driver or if I should simply leave
it out (that's the way it is right now).
I'm fine with either way, I would just like to avoid duplicate work.
So please let me know how you prefer it.


Regards,
Martin

[PATCH net-next v5 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Martin Blumenstingl]

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle (= 2ns) TX clock delay. This seems to work fine for many boards
(for example Odroid-C2 or Amlogic's reference boards) but there are
some others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901 Mbits/sec
[  4]   1.00-2.00   sec  94.2 MBytes   791 Mbits/sec
[  4]   2.00-3.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   3.00-4.00   sec  96.2 MBytes   808 Mbits/sec
[  4]   4.00-5.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   5.00-6.00   sec  96.5 MBytes   810 Mbits/sec
[  4]   6.00-7.00   sec  96.6 MBytes   810 Mbits/sec
[  4]   7.00-8.00   sec  96.5 MBytes   809 Mbits/sec
[  4]   8.00-9.00   sec   105 MBytes   884 Mbits/sec
[  4]   9.00-10.00  sec   111 MBytes   934 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  1000 MBytes   839 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   998 MBytes   837 Mbits/sec                  receiver

iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139 KBytes
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7 KBytes
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187 KBytes
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143 KBytes
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129 KBytes
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195 KBytes
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7 KBytes
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6 KBytes
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107 KBytes
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874 Mbits/sec  1189             sender
[  4]   0.00-10.01  sec  1.02 GBytes   873 Mbits/sec                  receiver

iperf Done.

I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
So changes to at least the RTL8211F PHY driver are needed to get it
working properly in all situations.

Changes since v4:
- add a fallback of 2ns (the value which was previously hardcoded) for
  the TX delay so we are backwards-compatible with older .dts'
- update the documentation with the new fallback value and add a small
  note that the "amlogic,tx-delay" property is ignored when the phy-mode
  is "rmii".

Changes since v3:
- rebased to apply against current net-next branch (fixes a conflict
  with d2ed0a7755fe14c7 "net: ethernet: stmmac: fix of-node and
  fixed-link-phydev leaks")

Changes since v2:
- moved all .dts patches (3-7) to a separate series
- removed the default 2ns TX delay when phy-mode RGMII is specified
- (rebased against current net-next)

Changes since v1:
- renamed the devicetree property "amlogic,tx-delay" to
  "amlogic,tx-delay-ns", which makes the .dts easier to read as we can
  simply specify human-readable values instead of having "preprocessor
  defines and calculation in human brain". Thanks to Andrew Lunn for
  the suggestion!
- improved documentation to indicate when the MAC TX-delay should be
  configured and how to use the PHY's TX-delay
- changed the default TX-delay in the dwmac-meson8b driver from 2ns
  to 0ms when any of the rgmii-*id modes are used (the 2ns default
  value still applies for phy-mode "rgmii")
- added patches to properly reset the PHY on Meson GXBB devices and to
  use a similar configuration than the one we use on Meson GXL devices
  (by passing a phy-handle to stmmac and defining the PHY in the mdio0
  bus - patch 3-6)
- add the "amlogic,tx-delay-ns" property to all boards which are using
  the RGMII PHY (patch 7)


Martin Blumenstingl (2):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

 .../devicetree/bindings/net/meson-dwmac.txt          | 16 ++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c  | 20 ++++++++++++++------
 2 files changed, 30 insertions(+), 6 deletions(-)

-- 
2.11.0

[PATCH net-next v5 2/2] net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

[Martin Blumenstingl]

Prior to this patch we were using a hardcoded RGMII TX clock delay of
2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for
many boards, but unfortunately not for all (due to the way the actual
circuit is designed, sometimes because the TX delay is enabled in the
PHY, etc.). Making the TX delay on the MAC side configurable allows us
to support all possible hardware combinations.

This allows fixing a compatibility issue on some boards, where the
RTL8211F PHY is configured to generate the TX delay. We can now turn
off the TX delay in the MAC, because otherwise we would be applying the
delay twice (which results in non-working TX traffic).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index ffaed1f35efe..8840a360a0b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -35,10 +35,6 @@
 
 #define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
-#define PRG_ETH0_TXDLY_OFF		(0x0 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_QUARTER		(0x1 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_HALF		(0x2 << PRG_ETH0_TXDLY_SHIFT)
-#define PRG_ETH0_TXDLY_THREE_QUARTERS	(0x3 << PRG_ETH0_TXDLY_SHIFT)
 
 /* divider for the result of m250_sel */
 #define PRG_ETH0_CLK_M250_DIV_SHIFT	7
@@ -69,6 +65,8 @@ struct meson8b_dwmac {
 
 	struct clk_divider	m25_div;
 	struct clk		*m25_div_clk;
+
+	u32			tx_delay_ns;
 };
 
 static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
@@ -179,6 +177,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
 	int ret;
 	unsigned long clk_rate;
+	u8 tx_dly_val;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
@@ -196,9 +195,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		/* TX clock delay - all known boards use a 1/4 cycle delay */
+		/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
+		 * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
+		 * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+		 */
+		tx_dly_val = dwmac->tx_delay_ns >> 1;
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					PRG_ETH0_TXDLY_QUARTER);
+					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
 		break;
 
 	case PHY_INTERFACE_MODE_RMII:
@@ -284,6 +287,11 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	/* use 2ns as fallback since this value was previously hardcoded */
+	if (of_property_read_u32(pdev->dev.of_node, "amlogic,tx-delay-ns",
+				 &dwmac->tx_delay_ns))
+		dwmac->tx_delay_ns = 2;
+
 	ret = meson8b_init_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
-- 
2.11.0

[PATCH net-next v5 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac

[Martin Blumenstingl]

This allows configuring the RGMII TX clock delay. The RGMII clock is
generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
The configuration depends on the actual hardware (no delay may be
needed due to the design of the actual circuit, the PHY might add this
delay, etc.).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/net/meson-dwmac.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
index 89e62ddc69ca..0703ad3f3c1e 100644
--- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
@@ -25,6 +25,22 @@ Required properties on Meson8b and newer:
 		- "clkin0" - first parent clock of the internal mux
 		- "clkin1" - second parent clock of the internal mux
 
+Optional properties on Meson8b and newer:
+- amlogic,tx-delay-ns:	The internal RGMII TX clock delay (provided
+			by this driver) in nanoseconds. Allowed values
+			are: 0ns, 2ns, 4ns, 6ns.
+			When phy-mode is set to "rgmii" then the TX
+			delay should be explicitly configured. When
+			not configured a fallback of 2ns is used.
+			When the phy-mode is set to either "rgmii-id"
+			or "rgmii-txid" the TX clock delay is already
+			provided by the PHY. In that case this
+			property should be set to 0ns (which disables
+			the TX clock delay in the MAC to prevent the
+			clock from going off because both PHY and MAC
+			are adding a delay).
+			Any configuration is ignored when the phy-mode
+			is set to "rmii".
 
 Example for Meson6:
 
-- 
2.11.0

--
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

Re: [PATCH net-next v5 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[David Miller]

From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Date: Sun, 22 Jan 2017 23:02:44 +0100

> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle (= 2ns) TX clock delay. This seems to work fine for many boards
> (for example Odroid-C2 or Amlogic's reference boards) but there are
> some others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
> 
> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
> TX clock delay disabled on the MAC (as it's enabled in the PHY driver).
> TX throughput was virtually zero before:
 ...
> I get similar TX throughput on my Meson GXBB "MXQ Pro+" board when I
> disable the PHY's TX-delay and configure a 4ms TX-delay on the MAC.
> So changes to at least the RTL8211F PHY driver are needed to get it
> working properly in all situations.

Series applied, thanks.
--
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