linux-amlogic.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes
@ 2018-12-29 14:35 Martin Blumenstingl
  2018-12-29 14:35 ` [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins Martin Blumenstingl
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Martin Blumenstingl @ 2018-12-29 14:35 UTC (permalink / raw)
  To: linux-amlogic, khilman, ingrassia
  Cc: Martin Blumenstingl, linux-arm-kernel, jianxin.pan, linus.luessing

Hi Emiliano, Hi Linus,

could you please test the patch from this series on your Odroid-C1?
I tested it on mine and Ethernet seems to be improve Ethernet transmit
speeds greatly (at the cost of a small drop in receive performance).


Changes since v2 at [1]:
- Thanks to Jianxin for providing the documentation for the two missing
  bits in pin mux register 7
- added a patch to add the missing eth_rxd2 and eth_rxd3 pin groups to
  the pinctrl-meson8b driver
- update the meson8.dtsi patch to include the eth_rxd2 and eth_rxd3 pin
  groups on top of the previous change which only removed eth_txd0_1
  and eth_txd1_1 (including the patch description)
- update subject of the cover-letter and the meson8b.dtsi patch


Changes since v1 at [0]:
- rebased so it applies on top of "ARM: dts: meson: meson8b: add the CPU
  OPP tables" which will be part of v4.20-rc1 once the arm-soc tree is
  merged into mainline
- updated patch description with more details


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-May/007274.html
[1] https://patchwork.kernel.org/cover/10744709/


Martin Blumenstingl (2):
  pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  ARM: dts: meson8b: fix the Ethernet data line signals in
    eth_rgmii_pins

 arch/arm/boot/dts/meson8b.dtsi          | 6 +++---
 drivers/pinctrl/meson/pinctrl-meson8b.c | 6 +++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.20.1


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2018-12-29 14:35 [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Martin Blumenstingl
@ 2018-12-29 14:35 ` Martin Blumenstingl
  2019-01-11  1:08   ` Kevin Hilman
  2018-12-29 14:35 ` [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins Martin Blumenstingl
  2018-12-30 18:15 ` [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Emiliano Ingrassia
  2 siblings, 1 reply; 17+ messages in thread
From: Martin Blumenstingl @ 2018-12-29 14:35 UTC (permalink / raw)
  To: linux-amlogic, khilman, ingrassia
  Cc: Martin Blumenstingl, linux-arm-kernel, jianxin.pan, linus.luessing

Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
rely on the bootloader to set them up correctly.

The vendor u-boot sources for Odroid-C1 use the following Ethernet
pinmux configuration:
  SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
  SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
This translates to the following pin groups in the mainline kernel:
- register 6 bit  0: eth_rxd1 (DIF_0_P)
- register 6 bit  1: eth_rxd0 (DIF_0_N)
- register 6 bit  2: eth_rx_dv (DIF_1_P)
- register 6 bit  3: eth_rx_clk (DIF_1_N)
- register 6 bit  6: eth_tx_en (DIF_3_P)
- register 6 bit  8: eth_ref_clk (DIF_3_N)
- register 6 bit  9: eth_mdc (DIF_4_P)
- register 6 bit 10: eth_mdio_en (DIF_4_N)
- register 6 bit 11: eth_tx_clk (GPIOH_9)
- register 6 bit 12: eth_txd2 (GPIOH_8)
- register 6 bit 13: eth_txd3 (GPIOH_7)
- register 7 bit 20: eth_txd0_0 (GPIOH_6)
- register 7 bit 21: eth_txd1_0 (GPIOH_5)
- register 7 bit 22: eth_rxd3 (DIF_2_P)
- register 7 bit 23: eth_rxd2 (DIF_2_N)

All functions except eth_rxd2 and eth_rxd3 are already supported by the
pinctrl-meson8b driver.

Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/pinctrl/meson/pinctrl-meson8b.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
index c69ca95b1ad5..84938bd73ac2 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8b.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
@@ -346,6 +346,8 @@ static const unsigned int eth_rx_dv_pins[]	= { DIF_1_P };
 static const unsigned int eth_rx_clk_pins[]	= { DIF_1_N };
 static const unsigned int eth_txd0_1_pins[]	= { DIF_2_P };
 static const unsigned int eth_txd1_1_pins[]	= { DIF_2_N };
+static const unsigned int eth_rxd3_pins[]	= { DIF_2_P };
+static const unsigned int eth_rxd2_pins[]	= { DIF_2_N };
 static const unsigned int eth_tx_en_pins[]	= { DIF_3_P };
 static const unsigned int eth_ref_clk_pins[]	= { DIF_3_N };
 static const unsigned int eth_mdc_pins[]	= { DIF_4_P };
@@ -599,6 +601,8 @@ static struct meson_pmx_group meson8b_cbus_groups[] = {
 	GROUP(eth_ref_clk,	6,	8),
 	GROUP(eth_mdc,		6,	9),
 	GROUP(eth_mdio_en,	6,	10),
+	GROUP(eth_rxd3,		7,	22),
+	GROUP(eth_rxd2,		7,	23),
 };
 
 static struct meson_pmx_group meson8b_aobus_groups[] = {
@@ -748,7 +752,7 @@ static const char * const ethernet_groups[] = {
 	"eth_tx_clk", "eth_tx_en", "eth_txd1_0", "eth_txd1_1",
 	"eth_txd0_0", "eth_txd0_1", "eth_rx_clk", "eth_rx_dv",
 	"eth_rxd1", "eth_rxd0", "eth_mdio_en", "eth_mdc", "eth_ref_clk",
-	"eth_txd2", "eth_txd3"
+	"eth_txd2", "eth_txd3", "eth_rxd3", "eth_rxd2"
 };
 
 static const char * const i2c_a_groups[] = {
-- 
2.20.1


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2018-12-29 14:35 [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Martin Blumenstingl
  2018-12-29 14:35 ` [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins Martin Blumenstingl
@ 2018-12-29 14:35 ` Martin Blumenstingl
  2019-01-11  1:09   ` Kevin Hilman
  2018-12-30 18:15 ` [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Emiliano Ingrassia
  2 siblings, 1 reply; 17+ messages in thread
From: Martin Blumenstingl @ 2018-12-29 14:35 UTC (permalink / raw)
  To: linux-amlogic, khilman, ingrassia
  Cc: Martin Blumenstingl, linux-arm-kernel, jianxin.pan, linus.luessing

According to the Odroid-C1+ schematics the Ethernet TXD1 signal is
routed to GPIOH_5 and the TXD0 signal is routed to GPIOH_6.
The public S805 datasheet shows that TXD0 can be routed to DIF_2_P and
TXD1 can be routed to DIF_2_N instead.

The pin groups eth_txd0_0 (GPIOH_6) and eth_txd0_1 (DIF_2_P) are both
configured as Ethernet TXD0 and TXD1 data lines in meson8b.dtsi. At the
same time eth_txd1_0 (GPIOH_5) and eth_txd1_1 (DIF_2_N) are configured
as TXD0 and TXD1 data lines as well.
This results in a bad Ethernet receive performance. Presumably this is
due to the eth_txd0 and eth_txd1 signal being routed to the wrong pins.
As a result of that data can only be transmitted on eth_txd2 and
eth_txd3. However, I have no scope to fully confirm this assumption.

The vendor u-boot sources for Odroid-C1 use the following Ethernet
pinmux configuration:
  SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
  SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
This translates to the following pin groups in the mainline kernel:
- register 6 bit  0: eth_rxd1 (DIF_0_P)
- register 6 bit  1: eth_rxd0 (DIF_0_N)
- register 6 bit  2: eth_rx_dv (DIF_1_P)
- register 6 bit  3: eth_rx_clk (DIF_1_N)
- register 6 bit  6: eth_tx_en (DIF_3_P)
- register 6 bit  8: eth_ref_clk (DIF_3_N)
- register 6 bit  9: eth_mdc (DIF_4_P)
- register 6 bit 10: eth_mdio_en (DIF_4_N)
- register 6 bit 11: eth_tx_clk (GPIOH_9)
- register 6 bit 12: eth_txd2 (GPIOH_8)
- register 6 bit 13: eth_txd3 (GPIOH_7)
- register 7 bit 20: eth_txd0_0 (GPIOH_6)
- register 7 bit 21: eth_txd1_0 (GPIOH_5)
- register 7 bit 22: eth_rxd3 (DIF_2_P)
- register 7 bit 23: eth_rxd2 (DIF_2_N)

Drop the eth_txd0_1 and eth_txd1_1 groups from eth_rgmii_pins to fix the
Ethernet transmit performance on Odroid-C1. Also add the eth_rxd2 and
eth_rxd3 groups so we don't rely on the bootloader to set them up.

iperf3 statistics before this change:
- transmitting from Odroid-C1: 741 Mbits/sec (0 retries)
- receiving on Odroid-C1: 199 Mbits/sec (1713 retries)

iperf3 statistics after this change:
- transmitting from Odroid-C1: 667 Mbits/sec (0 retries)
- receiving on Odroid-C1: 750 Mbits/sec (0 retries)

Fixes: b96446541d8390 ("ARM: dts: meson8b: extend ethernet controller description")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
Cc: Linus Lüssing <linus.luessing@c0d3.blue>
---
 arch/arm/boot/dts/meson8b.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 22d775460767..dc125769fe85 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -270,9 +270,7 @@
 				groups = "eth_tx_clk",
 					 "eth_tx_en",
 					 "eth_txd1_0",
-					 "eth_txd1_1",
 					 "eth_txd0_0",
-					 "eth_txd0_1",
 					 "eth_rx_clk",
 					 "eth_rx_dv",
 					 "eth_rxd1",
@@ -281,7 +279,9 @@
 					 "eth_mdc",
 					 "eth_ref_clk",
 					 "eth_txd2",
-					 "eth_txd3";
+					 "eth_txd3",
+					 "eth_rxd3",
+					 "eth_rxd2";
 				function = "ethernet";
 				bias-disable;
 			};
-- 
2.20.1


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes
  2018-12-29 14:35 [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Martin Blumenstingl
  2018-12-29 14:35 ` [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins Martin Blumenstingl
  2018-12-29 14:35 ` [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins Martin Blumenstingl
@ 2018-12-30 18:15 ` Emiliano Ingrassia
  2018-12-31 10:59   ` Martin Blumenstingl
  2 siblings, 1 reply; 17+ messages in thread
From: Emiliano Ingrassia @ 2018-12-30 18:15 UTC (permalink / raw)
  To: Martin Blumenstingl; +Cc: linux-amlogic, khilman

Hi Martin,

On Sat, Dec 29, 2018 at 03:35:54PM +0100, Martin Blumenstingl wrote:
> Hi Emiliano, Hi Linus,
>
> could you please test the patch from this series on your Odroid-C1?
> I tested it on mine and Ethernet seems to be improve Ethernet transmit
> speeds greatly (at the cost of a small drop in receive performance).
>

I redo the tests described in [0] with your patches applied.
Here is what I got:

1) Start packet generator on laptop:

              | incoming traffic | outgoing traffic
=====================================================
nload (board) |     ~940 Mbps    |       0 Mbps
-----------------------------------------------------
nload (laptop)|       0 Mbps     |     ~940 Mbps
=====================================================

2) Start packet generator on board:

              | incoming traffic  | outgoing traffic
==============+===================+==================
nload (board) |     ~460 Mbps     |     ~256 Mbps
--------------+-------------------+------------------
nload (laptop)|     ~256 Mbps     |     ~940 Mbps
=====================================================

3) Stop packet generator on laptop:

              | incoming traffic | outgoing traffic
=====================================================
nload (board) |       0 Mbps     |    ~940 Mbps
-----------------------------------------------------
nload (laptop)|       ~940 Mbps  |      0 Mbps
=====================================================

4) Restart packet generator on laptop:

              | incoming traffic  | outgoing traffic
==============+===================+==================
nload (board) |     ~460 Mbps     |     ~256 Mbps
--------------+-------------------+------------------
nload (laptop)|     ~256 Mbps     |     ~940 Mbps
=====================================================

So, now the board does not drop the entire incoming traffic while
transmitting at full speed, but gently decreases the transmit rate
as one would expect.
Sure it's not working as full-duplex yet, but this is a huge step. Good catch!

Just a question about the patch 1/2: why you didn't remove the variables
"eth_txd0_1_pins" and "eth_txd1_1_pins" in "pinctrl-meson8b" driver?

Apart from this doubt, your patch is fine for me.

Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>

Regards,

Emiliano

[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-December/009397.html
>
> Changes since v2 at [1]:
> - Thanks to Jianxin for providing the documentation for the two missing
>   bits in pin mux register 7
> - added a patch to add the missing eth_rxd2 and eth_rxd3 pin groups to
>   the pinctrl-meson8b driver
> - update the meson8.dtsi patch to include the eth_rxd2 and eth_rxd3 pin
>   groups on top of the previous change which only removed eth_txd0_1
>   and eth_txd1_1 (including the patch description)
> - update subject of the cover-letter and the meson8b.dtsi patch
>
>
> Changes since v1 at [0]:
> - rebased so it applies on top of "ARM: dts: meson: meson8b: add the CPU
>   OPP tables" which will be part of v4.20-rc1 once the arm-soc tree is
>   merged into mainline
> - updated patch description with more details
>
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2018-May/007274.html
> [1] https://patchwork.kernel.org/cover/10744709/
>
>
> Martin Blumenstingl (2):
>   pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
>   ARM: dts: meson8b: fix the Ethernet data line signals in
>     eth_rgmii_pins
>
>  arch/arm/boot/dts/meson8b.dtsi          | 6 +++---
>  drivers/pinctrl/meson/pinctrl-meson8b.c | 6 +++++-
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> --
> 2.20.1
>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes
  2018-12-30 18:15 ` [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Emiliano Ingrassia
@ 2018-12-31 10:59   ` Martin Blumenstingl
  0 siblings, 0 replies; 17+ messages in thread
From: Martin Blumenstingl @ 2018-12-31 10:59 UTC (permalink / raw)
  To: Emiliano Ingrassia; +Cc: linux-amlogic, khilman

Hi Emiliano,

On Sun, Dec 30, 2018 at 7:15 PM Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
>
> Hi Martin,
>
> On Sat, Dec 29, 2018 at 03:35:54PM +0100, Martin Blumenstingl wrote:
> > Hi Emiliano, Hi Linus,
> >
> > could you please test the patch from this series on your Odroid-C1?
> > I tested it on mine and Ethernet seems to be improve Ethernet transmit
> > speeds greatly (at the cost of a small drop in receive performance).
> >
>
> I redo the tests described in [0] with your patches applied.
> Here is what I got:
>
> 1) Start packet generator on laptop:
>
>               | incoming traffic | outgoing traffic
> =====================================================
> nload (board) |     ~940 Mbps    |       0 Mbps
> -----------------------------------------------------
> nload (laptop)|       0 Mbps     |     ~940 Mbps
> =====================================================
>
> 2) Start packet generator on board:
>
>               | incoming traffic  | outgoing traffic
> ==============+===================+==================
> nload (board) |     ~460 Mbps     |     ~256 Mbps
> --------------+-------------------+------------------
> nload (laptop)|     ~256 Mbps     |     ~940 Mbps
> =====================================================
>
> 3) Stop packet generator on laptop:
>
>               | incoming traffic | outgoing traffic
> =====================================================
> nload (board) |       0 Mbps     |    ~940 Mbps
> -----------------------------------------------------
> nload (laptop)|       ~940 Mbps  |      0 Mbps
> =====================================================
>
> 4) Restart packet generator on laptop:
>
>               | incoming traffic  | outgoing traffic
> ==============+===================+==================
> nload (board) |     ~460 Mbps     |     ~256 Mbps
> --------------+-------------------+------------------
> nload (laptop)|     ~256 Mbps     |     ~940 Mbps
> =====================================================
>
> So, now the board does not drop the entire incoming traffic while
> transmitting at full speed, but gently decreases the transmit rate
> as one would expect.
awesome, great that it works on your board as well!

> Sure it's not working as full-duplex yet, but this is a huge step. Good catch!
I'm not sure what could cause it to not use full-duplex (whether it's
the PHY, MAC, SoC implementation, PRG_ETHERNET registers, ...)

> Just a question about the patch 1/2: why you didn't remove the variables
> "eth_txd0_1_pins" and "eth_txd1_1_pins" in "pinctrl-meson8b" driver?
(10/100 Ethernet only requires TXD0 and TXD1, there's no TXD2 and TXD3 there)
I didn't drop them because there may be devices out there where the
board designer decided to route TXD0 and TXD1 to the
eth_txd0_1_pins/eth_txd1_1_pins

> Apart from this doubt, your patch is fine for me.
>
> Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
> Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
thank you for these!


Regards
Martin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2018-12-29 14:35 ` [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins Martin Blumenstingl
@ 2019-01-11  1:08   ` Kevin Hilman
  2019-01-11  9:36     ` Emiliano Ingrassia
                       ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Kevin Hilman @ 2019-01-11  1:08 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, ingrassia
  Cc: Martin Blumenstingl, linux-arm-kernel, jianxin.pan, linus.luessing

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
> Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
> rely on the bootloader to set them up correctly.
>
> The vendor u-boot sources for Odroid-C1 use the following Ethernet
> pinmux configuration:
>   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
>   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> This translates to the following pin groups in the mainline kernel:
> - register 6 bit  0: eth_rxd1 (DIF_0_P)
> - register 6 bit  1: eth_rxd0 (DIF_0_N)
> - register 6 bit  2: eth_rx_dv (DIF_1_P)
> - register 6 bit  3: eth_rx_clk (DIF_1_N)
> - register 6 bit  6: eth_tx_en (DIF_3_P)
> - register 6 bit  8: eth_ref_clk (DIF_3_N)
> - register 6 bit  9: eth_mdc (DIF_4_P)
> - register 6 bit 10: eth_mdio_en (DIF_4_N)
> - register 6 bit 11: eth_tx_clk (GPIOH_9)
> - register 6 bit 12: eth_txd2 (GPIOH_8)
> - register 6 bit 13: eth_txd3 (GPIOH_7)
> - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> - register 7 bit 22: eth_rxd3 (DIF_2_P)
> - register 7 bit 23: eth_rxd2 (DIF_2_N)
>
> All functions except eth_rxd2 and eth_rxd3 are already supported by the
> pinctrl-meson8b driver.
>
> Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2018-12-29 14:35 ` [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins Martin Blumenstingl
@ 2019-01-11  1:09   ` Kevin Hilman
  2019-01-11 10:13     ` Emiliano Ingrassia
  2019-02-04 14:26     ` Martin Blumenstingl
  0 siblings, 2 replies; 17+ messages in thread
From: Kevin Hilman @ 2019-01-11  1:09 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, ingrassia
  Cc: Martin Blumenstingl, linux-arm-kernel, jianxin.pan, linus.luessing

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> According to the Odroid-C1+ schematics the Ethernet TXD1 signal is
> routed to GPIOH_5 and the TXD0 signal is routed to GPIOH_6.
> The public S805 datasheet shows that TXD0 can be routed to DIF_2_P and
> TXD1 can be routed to DIF_2_N instead.
>
> The pin groups eth_txd0_0 (GPIOH_6) and eth_txd0_1 (DIF_2_P) are both
> configured as Ethernet TXD0 and TXD1 data lines in meson8b.dtsi. At the
> same time eth_txd1_0 (GPIOH_5) and eth_txd1_1 (DIF_2_N) are configured
> as TXD0 and TXD1 data lines as well.
> This results in a bad Ethernet receive performance. Presumably this is
> due to the eth_txd0 and eth_txd1 signal being routed to the wrong pins.
> As a result of that data can only be transmitted on eth_txd2 and
> eth_txd3. However, I have no scope to fully confirm this assumption.
>
> The vendor u-boot sources for Odroid-C1 use the following Ethernet
> pinmux configuration:
>   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
>   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> This translates to the following pin groups in the mainline kernel:
> - register 6 bit  0: eth_rxd1 (DIF_0_P)
> - register 6 bit  1: eth_rxd0 (DIF_0_N)
> - register 6 bit  2: eth_rx_dv (DIF_1_P)
> - register 6 bit  3: eth_rx_clk (DIF_1_N)
> - register 6 bit  6: eth_tx_en (DIF_3_P)
> - register 6 bit  8: eth_ref_clk (DIF_3_N)
> - register 6 bit  9: eth_mdc (DIF_4_P)
> - register 6 bit 10: eth_mdio_en (DIF_4_N)
> - register 6 bit 11: eth_tx_clk (GPIOH_9)
> - register 6 bit 12: eth_txd2 (GPIOH_8)
> - register 6 bit 13: eth_txd3 (GPIOH_7)
> - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> - register 7 bit 22: eth_rxd3 (DIF_2_P)
> - register 7 bit 23: eth_rxd2 (DIF_2_N)
>
> Drop the eth_txd0_1 and eth_txd1_1 groups from eth_rgmii_pins to fix the
> Ethernet transmit performance on Odroid-C1. Also add the eth_rxd2 and
> eth_rxd3 groups so we don't rely on the bootloader to set them up.
>
> iperf3 statistics before this change:
> - transmitting from Odroid-C1: 741 Mbits/sec (0 retries)
> - receiving on Odroid-C1: 199 Mbits/sec (1713 retries)
>
> iperf3 statistics after this change:
> - transmitting from Odroid-C1: 667 Mbits/sec (0 retries)
> - receiving on Odroid-C1: 750 Mbits/sec (0 retries)
>
> Fixes: b96446541d8390 ("ARM: dts: meson8b: extend ethernet controller description")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
> Cc: Linus Lüssing <linus.luessing@c0d3.blue>

Queued for v5.1 (branch: v5.1/dt)

Kevin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11  1:08   ` Kevin Hilman
@ 2019-01-11  9:36     ` Emiliano Ingrassia
  2019-01-11 10:09     ` Emiliano Ingrassia
  2019-01-11 19:52     ` Martin Blumenstingl
  2 siblings, 0 replies; 17+ messages in thread
From: Emiliano Ingrassia @ 2019-01-11  9:36 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Martin Blumenstingl, khilman, linux-amlogic

On Thu, Jan 10, 2019 at 05:08:39PM -0800, Kevin Hilman wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
> > rely on the bootloader to set them up correctly.
> >
> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> > pinmux configuration:
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> > This translates to the following pin groups in the mainline kernel:
> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> > - register 6 bit  9: eth_mdc (DIF_4_P)
> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >
> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
> > pinctrl-meson8b driver.
> >
> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11  1:08   ` Kevin Hilman
  2019-01-11  9:36     ` Emiliano Ingrassia
@ 2019-01-11 10:09     ` Emiliano Ingrassia
  2019-01-11 18:06       ` Kevin Hilman
  2019-01-11 19:52     ` Martin Blumenstingl
  2 siblings, 1 reply; 17+ messages in thread
From: Emiliano Ingrassia @ 2019-01-11 10:09 UTC (permalink / raw)
  To: Kevin Hilman, Martin Blumenstingl, linux-amlogic
  Cc: linux-arm-kernel, jianxin.pan, linus.luessing

Hi Kevin,

sorry to bother you.

On Thu, Jan 10, 2019 at 05:08:39PM -0800, Kevin Hilman wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
> > rely on the bootloader to set them up correctly.
> >
> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> > pinmux configuration:
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> > This translates to the following pin groups in the mainline kernel:
> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> > - register 6 bit  9: eth_mdc (DIF_4_P)
> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >
> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
> > pinctrl-meson8b driver.
> >
> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>

For both patches of this series I gave:
Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>

They were in the answer to the cover letter of this series.
I note that they were not included in the commit message of the first patch.

Did I miss something or did something wrong?
Please, let me know.

Thank you,

Emiliano

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2019-01-11  1:09   ` Kevin Hilman
@ 2019-01-11 10:13     ` Emiliano Ingrassia
  2019-02-04 14:26     ` Martin Blumenstingl
  1 sibling, 0 replies; 17+ messages in thread
From: Emiliano Ingrassia @ 2019-01-11 10:13 UTC (permalink / raw)
  To: Kevin Hilman, Martin Blumenstingl, linux-amlogic
  Cc: linux-arm-kernel, jianxin.pan, linus.luessing

On Thu, Jan 10, 2019 at 05:09:47PM -0800, Kevin Hilman wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > According to the Odroid-C1+ schematics the Ethernet TXD1 signal is
> > routed to GPIOH_5 and the TXD0 signal is routed to GPIOH_6.
> > The public S805 datasheet shows that TXD0 can be routed to DIF_2_P and
> > TXD1 can be routed to DIF_2_N instead.
> >
> > The pin groups eth_txd0_0 (GPIOH_6) and eth_txd0_1 (DIF_2_P) are both
> > configured as Ethernet TXD0 and TXD1 data lines in meson8b.dtsi. At the
> > same time eth_txd1_0 (GPIOH_5) and eth_txd1_1 (DIF_2_N) are configured
> > as TXD0 and TXD1 data lines as well.
> > This results in a bad Ethernet receive performance. Presumably this is
> > due to the eth_txd0 and eth_txd1 signal being routed to the wrong pins.
> > As a result of that data can only be transmitted on eth_txd2 and
> > eth_txd3. However, I have no scope to fully confirm this assumption.
> >
> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> > pinmux configuration:
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> > This translates to the following pin groups in the mainline kernel:
> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> > - register 6 bit  9: eth_mdc (DIF_4_P)
> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >
> > Drop the eth_txd0_1 and eth_txd1_1 groups from eth_rgmii_pins to fix the
> > Ethernet transmit performance on Odroid-C1. Also add the eth_rxd2 and
> > eth_rxd3 groups so we don't rely on the bootloader to set them up.
> >
> > iperf3 statistics before this change:
> > - transmitting from Odroid-C1: 741 Mbits/sec (0 retries)
> > - receiving on Odroid-C1: 199 Mbits/sec (1713 retries)
> >
> > iperf3 statistics after this change:
> > - transmitting from Odroid-C1: 667 Mbits/sec (0 retries)
> > - receiving on Odroid-C1: 750 Mbits/sec (0 retries)
> >
> > Fixes: b96446541d8390 ("ARM: dts: meson8b: extend ethernet controller description")
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
> > Cc: Linus Lüssing <linus.luessing@c0d3.blue>
>
> Queued for v5.1 (branch: v5.1/dt)
>
> Kevin

Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>

Emiliano

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11 10:09     ` Emiliano Ingrassia
@ 2019-01-11 18:06       ` Kevin Hilman
  2019-01-11 18:21         ` Emiliano Ingrassia
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Hilman @ 2019-01-11 18:06 UTC (permalink / raw)
  To: Emiliano Ingrassia, Martin Blumenstingl, linux-amlogic
  Cc: linux-arm-kernel, jianxin.pan, linus.luessing

Emiliano Ingrassia <ingrassia@epigenesys.com> writes:

> Hi Kevin,
>
> sorry to bother you.

No need to apologize.

> On Thu, Jan 10, 2019 at 05:08:39PM -0800, Kevin Hilman wrote:
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>>
>> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
>> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
>> > rely on the bootloader to set them up correctly.
>> >
>> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
>> > pinmux configuration:
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
>> > This translates to the following pin groups in the mainline kernel:
>> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
>> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
>> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
>> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
>> > - register 6 bit  6: eth_tx_en (DIF_3_P)
>> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
>> > - register 6 bit  9: eth_mdc (DIF_4_P)
>> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
>> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
>> > - register 6 bit 12: eth_txd2 (GPIOH_8)
>> > - register 6 bit 13: eth_txd3 (GPIOH_7)
>> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
>> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
>> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
>> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
>> >
>> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
>> > pinctrl-meson8b driver.
>> >
>> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
>
> For both patches of this series I gave:
> Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
> Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
>
> They were in the answer to the cover letter of this series.
> I note that they were not included in the commit message of the first patch.
>
> Did I miss something or did something wrong?
> Please, let me know.

You did everything fine, I just missed them.

I've started relying a bit more heavily on patchwork to collect
review/test tags, and right now it doesn't notice tags to the
cover-letter, only to individual patches, so I typically add those by
hand, but in this case I missed them.

Sorry about that, and thanks for the reminder.  I'll add them and
repush.

And, while I'm thinking about it, I'll request that feature to patchwork
developers.

Kevin



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11 18:06       ` Kevin Hilman
@ 2019-01-11 18:21         ` Emiliano Ingrassia
  0 siblings, 0 replies; 17+ messages in thread
From: Emiliano Ingrassia @ 2019-01-11 18:21 UTC (permalink / raw)
  To: Kevin Hilman, Martin Blumenstingl, linux-amlogic
  Cc: linux-arm-kernel, jianxin.pan, linus.luessing

Hi Kevin,

On Fri, Jan 11, 2019 at 10:06:54AM -0800, Kevin Hilman wrote:
> Emiliano Ingrassia <ingrassia@epigenesys.com> writes:
>
> > Hi Kevin,
> >
> > sorry to bother you.
>
> No need to apologize.
>
> > On Thu, Jan 10, 2019 at 05:08:39PM -0800, Kevin Hilman wrote:
> >> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> >>
> >> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
> >> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
> >> > rely on the bootloader to set them up correctly.
> >> >
> >> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> >> > pinmux configuration:
> >> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> >> > This translates to the following pin groups in the mainline kernel:
> >> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> >> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> >> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> >> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> >> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> >> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> >> > - register 6 bit  9: eth_mdc (DIF_4_P)
> >> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> >> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> >> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> >> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> >> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> >> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> >> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> >> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >> >
> >> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
> >> > pinctrl-meson8b driver.
> >> >
> >> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
> >> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >>
> >> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> >
> > For both patches of this series I gave:
> > Tested-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
> > Reviewed-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
> >
> > They were in the answer to the cover letter of this series.
> > I note that they were not included in the commit message of the first patch.
> >
> > Did I miss something or did something wrong?
> > Please, let me know.
>
> You did everything fine, I just missed them.
>
> I've started relying a bit more heavily on patchwork to collect
> review/test tags, and right now it doesn't notice tags to the
> cover-letter, only to individual patches, so I typically add those by
> hand, but in this case I missed them.
>

Good to know, I'll keep it in mind next time.

> Sorry about that, and thanks for the reminder.  I'll add them and
> repush.
>
> And, while I'm thinking about it, I'll request that feature to patchwork
> developers.
>
> Kevin
>
>

Thank you for the explanation!

Best regards,

Emiliano

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11  1:08   ` Kevin Hilman
  2019-01-11  9:36     ` Emiliano Ingrassia
  2019-01-11 10:09     ` Emiliano Ingrassia
@ 2019-01-11 19:52     ` Martin Blumenstingl
  2019-01-11 22:42       ` Kevin Hilman
  2 siblings, 1 reply; 17+ messages in thread
From: Martin Blumenstingl @ 2019-01-11 19:52 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linus.luessing, jianxin.pan, linus.walleij, linux-gpio,
	ingrassia, linux-amlogic, linux-arm-kernel

Hi Kevin,

On Fri, Jan 11, 2019 at 2:08 AM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
> > rely on the bootloader to set them up correctly.
> >
> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> > pinmux configuration:
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> > This translates to the following pin groups in the mainline kernel:
> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> > - register 6 bit  9: eth_mdc (DIF_4_P)
> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >
> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
> > pinctrl-meson8b driver.
> >
> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
thank you for reviewing this!

I just realized that I forgot to CC the linux-gpio mailing list as
well as Linus Walleij on this patch (fixing that with this mail) -
sorry for that.
right now this patch is applied to linux-amlogic.git's v5.1/dt branch.
please let me know whether you would like to keep it in your tree or
if you would like me to re-send it (including Linus and the linux-gpio
list this time...)


Regards
Martin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins
  2019-01-11 19:52     ` Martin Blumenstingl
@ 2019-01-11 22:42       ` Kevin Hilman
  0 siblings, 0 replies; 17+ messages in thread
From: Kevin Hilman @ 2019-01-11 22:42 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linus.luessing, jianxin.pan, linus.walleij, linux-gpio,
	ingrassia, linux-amlogic, linux-arm-kernel

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Kevin,
>
> On Fri, Jan 11, 2019 at 2:08 AM Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>>
>> > Gigabit Ethernet requires the Ethernet TXD0..3 and RXD0..3 data lines.
>> > Add the missing eth_rxd2 and eth_rxd3 definitions so we don't have to
>> > rely on the bootloader to set them up correctly.
>> >
>> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
>> > pinmux configuration:
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
>> > This translates to the following pin groups in the mainline kernel:
>> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
>> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
>> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
>> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
>> > - register 6 bit  6: eth_tx_en (DIF_3_P)
>> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
>> > - register 6 bit  9: eth_mdc (DIF_4_P)
>> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
>> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
>> > - register 6 bit 12: eth_txd2 (GPIOH_8)
>> > - register 6 bit 13: eth_txd3 (GPIOH_7)
>> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
>> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
>> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
>> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
>> >
>> > All functions except eth_rxd2 and eth_rxd3 are already supported by the
>> > pinctrl-meson8b driver.
>> >
>> > Suggested-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>
>> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> thank you for reviewing this!
>
> I just realized that I forgot to CC the linux-gpio mailing list as
> well as Linus Walleij on this patch (fixing that with this mail) -
> sorry for that.
> right now this patch is applied to linux-amlogic.git's v5.1/dt branch.

That was a mistake I rectified earlier today. I meant to only apply the
DT patch not the driver also.

> please let me know whether you would like to keep it in your tree or
> if you would like me to re-send it (including Linus and the linux-gpio
> list this time...)

Please resend including Linus, linux-gpio and my review tag.

Thanks,

Kevin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2019-01-11  1:09   ` Kevin Hilman
  2019-01-11 10:13     ` Emiliano Ingrassia
@ 2019-02-04 14:26     ` Martin Blumenstingl
  2019-02-07  3:04       ` Kevin Hilman
  1 sibling, 1 reply; 17+ messages in thread
From: Martin Blumenstingl @ 2019-02-04 14:26 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-arm-kernel, linux-amlogic, jianxin.pan, ingrassia, linus.luessing

Hi Kevin,

On Fri, Jan 11, 2019 at 2:09 AM Kevin Hilman <khilman@baylibre.com> wrote:
>
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > According to the Odroid-C1+ schematics the Ethernet TXD1 signal is
> > routed to GPIOH_5 and the TXD0 signal is routed to GPIOH_6.
> > The public S805 datasheet shows that TXD0 can be routed to DIF_2_P and
> > TXD1 can be routed to DIF_2_N instead.
> >
> > The pin groups eth_txd0_0 (GPIOH_6) and eth_txd0_1 (DIF_2_P) are both
> > configured as Ethernet TXD0 and TXD1 data lines in meson8b.dtsi. At the
> > same time eth_txd1_0 (GPIOH_5) and eth_txd1_1 (DIF_2_N) are configured
> > as TXD0 and TXD1 data lines as well.
> > This results in a bad Ethernet receive performance. Presumably this is
> > due to the eth_txd0 and eth_txd1 signal being routed to the wrong pins.
> > As a result of that data can only be transmitted on eth_txd2 and
> > eth_txd3. However, I have no scope to fully confirm this assumption.
> >
> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
> > pinmux configuration:
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
> > This translates to the following pin groups in the mainline kernel:
> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
> > - register 6 bit  6: eth_tx_en (DIF_3_P)
> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
> > - register 6 bit  9: eth_mdc (DIF_4_P)
> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
> > - register 6 bit 12: eth_txd2 (GPIOH_8)
> > - register 6 bit 13: eth_txd3 (GPIOH_7)
> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
> >
> > Drop the eth_txd0_1 and eth_txd1_1 groups from eth_rgmii_pins to fix the
> > Ethernet transmit performance on Odroid-C1. Also add the eth_rxd2 and
> > eth_rxd3 groups so we don't rely on the bootloader to set them up.
> >
> > iperf3 statistics before this change:
> > - transmitting from Odroid-C1: 741 Mbits/sec (0 retries)
> > - receiving on Odroid-C1: 199 Mbits/sec (1713 retries)
> >
> > iperf3 statistics after this change:
> > - transmitting from Odroid-C1: 667 Mbits/sec (0 retries)
> > - receiving on Odroid-C1: 750 Mbits/sec (0 retries)
> >
> > Fixes: b96446541d8390 ("ARM: dts: meson8b: extend ethernet controller description")
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
> > Cc: Linus Lüssing <linus.luessing@c0d3.blue>
>
> Queued for v5.1 (branch: v5.1/dt)
do you know what happened to this patch? I can't find it in v5.1/dt

let me know if I should re-send it


Regards
Martin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2019-02-04 14:26     ` Martin Blumenstingl
@ 2019-02-07  3:04       ` Kevin Hilman
  2019-02-08 19:38         ` Martin Blumenstingl
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Hilman @ 2019-02-07  3:04 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-arm-kernel, linux-amlogic, jianxin.pan, ingrassia, linus.luessing

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Kevin,
>
> On Fri, Jan 11, 2019 at 2:09 AM Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>>
>> > According to the Odroid-C1+ schematics the Ethernet TXD1 signal is
>> > routed to GPIOH_5 and the TXD0 signal is routed to GPIOH_6.
>> > The public S805 datasheet shows that TXD0 can be routed to DIF_2_P and
>> > TXD1 can be routed to DIF_2_N instead.
>> >
>> > The pin groups eth_txd0_0 (GPIOH_6) and eth_txd0_1 (DIF_2_P) are both
>> > configured as Ethernet TXD0 and TXD1 data lines in meson8b.dtsi. At the
>> > same time eth_txd1_0 (GPIOH_5) and eth_txd1_1 (DIF_2_N) are configured
>> > as TXD0 and TXD1 data lines as well.
>> > This results in a bad Ethernet receive performance. Presumably this is
>> > due to the eth_txd0 and eth_txd1 signal being routed to the wrong pins.
>> > As a result of that data can only be transmitted on eth_txd2 and
>> > eth_txd3. However, I have no scope to fully confirm this assumption.
>> >
>> > The vendor u-boot sources for Odroid-C1 use the following Ethernet
>> > pinmux configuration:
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_6, 0x3f4f);
>> >   SET_CBUS_REG_MASK(PERIPHS_PIN_MUX_7, 0xf00000);
>> > This translates to the following pin groups in the mainline kernel:
>> > - register 6 bit  0: eth_rxd1 (DIF_0_P)
>> > - register 6 bit  1: eth_rxd0 (DIF_0_N)
>> > - register 6 bit  2: eth_rx_dv (DIF_1_P)
>> > - register 6 bit  3: eth_rx_clk (DIF_1_N)
>> > - register 6 bit  6: eth_tx_en (DIF_3_P)
>> > - register 6 bit  8: eth_ref_clk (DIF_3_N)
>> > - register 6 bit  9: eth_mdc (DIF_4_P)
>> > - register 6 bit 10: eth_mdio_en (DIF_4_N)
>> > - register 6 bit 11: eth_tx_clk (GPIOH_9)
>> > - register 6 bit 12: eth_txd2 (GPIOH_8)
>> > - register 6 bit 13: eth_txd3 (GPIOH_7)
>> > - register 7 bit 20: eth_txd0_0 (GPIOH_6)
>> > - register 7 bit 21: eth_txd1_0 (GPIOH_5)
>> > - register 7 bit 22: eth_rxd3 (DIF_2_P)
>> > - register 7 bit 23: eth_rxd2 (DIF_2_N)
>> >
>> > Drop the eth_txd0_1 and eth_txd1_1 groups from eth_rgmii_pins to fix the
>> > Ethernet transmit performance on Odroid-C1. Also add the eth_rxd2 and
>> > eth_rxd3 groups so we don't rely on the bootloader to set them up.
>> >
>> > iperf3 statistics before this change:
>> > - transmitting from Odroid-C1: 741 Mbits/sec (0 retries)
>> > - receiving on Odroid-C1: 199 Mbits/sec (1713 retries)
>> >
>> > iperf3 statistics after this change:
>> > - transmitting from Odroid-C1: 667 Mbits/sec (0 retries)
>> > - receiving on Odroid-C1: 750 Mbits/sec (0 retries)
>> >
>> > Fixes: b96446541d8390 ("ARM: dts: meson8b: extend ethernet controller description")
>> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> > Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
>> > Cc: Linus Lüssing <linus.luessing@c0d3.blue>
>>
>> Queued for v5.1 (branch: v5.1/dt)
> do you know what happened to this patch? I can't find it in v5.1/dt

Hmm, I'm not sure what happened to it, but I (re)added it to v5.1/dt
just now.

Kevin

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins
  2019-02-07  3:04       ` Kevin Hilman
@ 2019-02-08 19:38         ` Martin Blumenstingl
  0 siblings, 0 replies; 17+ messages in thread
From: Martin Blumenstingl @ 2019-02-08 19:38 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-arm-kernel, linux-amlogic, jianxin.pan, ingrassia, linus.luessing

On Thu, Feb 7, 2019 at 4:05 AM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> >> Queued for v5.1 (branch: v5.1/dt)
> > do you know what happened to this patch? I can't find it in v5.1/dt
>
> Hmm, I'm not sure what happened to it, but I (re)added it to v5.1/dt
> just now.
perfect - thank you!

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-02-08 19:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-29 14:35 [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Martin Blumenstingl
2018-12-29 14:35 ` [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins Martin Blumenstingl
2019-01-11  1:08   ` Kevin Hilman
2019-01-11  9:36     ` Emiliano Ingrassia
2019-01-11 10:09     ` Emiliano Ingrassia
2019-01-11 18:06       ` Kevin Hilman
2019-01-11 18:21         ` Emiliano Ingrassia
2019-01-11 19:52     ` Martin Blumenstingl
2019-01-11 22:42       ` Kevin Hilman
2018-12-29 14:35 ` [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins Martin Blumenstingl
2019-01-11  1:09   ` Kevin Hilman
2019-01-11 10:13     ` Emiliano Ingrassia
2019-02-04 14:26     ` Martin Blumenstingl
2019-02-07  3:04       ` Kevin Hilman
2019-02-08 19:38         ` Martin Blumenstingl
2018-12-30 18:15 ` [PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes Emiliano Ingrassia
2018-12-31 10:59   ` Martin Blumenstingl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).