mbox series

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

Message ID 20181229143556.27339-1-martin.blumenstingl@googlemail.com (mailing list archive)
Headers show
Series Meson8b RGMII Ethernet pin fixes | expand

Message

Martin Blumenstingl Dec. 29, 2018, 2:35 p.m. UTC
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(-)

Comments

Emiliano Ingrassia Dec. 30, 2018, 6:15 p.m. UTC | #1
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
>
Martin Blumenstingl Dec. 31, 2018, 10:59 a.m. UTC | #2
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