[PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes

[PATCH v3 0/2] Meson8b RGMII Ethernet pin fixes

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Kevin Hilman]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Kevin Hilman]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Emiliano Ingrassia]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Emiliano Ingrassia]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Kevin Hilman]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Emiliano Ingrassia]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] pinctrl: meson: meson8b: add the eth_rxd2 and eth_rxd3 pins

[Kevin Hilman]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Kevin Hilman]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/2] ARM: dts: meson8b: fix the Ethernet data line signals in eth_rgmii_pins

[Martin Blumenstingl]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel