2020-05-12 21:53:56

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 0/2] ARM: dts: meson8b/m2: RGMII improvements

Hi Kevin,

the fist patch in this series connects FCLK_DIV2 to the PRG_ETH
"additional" registers for the dwmac Ethernet controller.
Now that we know how RGMII and FCLK_DIV2 are connected we can
add this dependency to get rid of CLK_IS_CRITICAL for FCLK_DIV2
at some point.

The second patch fixes the RX and TX delay. The 4ns TX delay which
we have used so far is incorrect and only worked because we were
using an unsupported clock divider in the PRG_ETH registers. That
divider has been fixed with commit bd6f48546b9c ("net: stmmac:
dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs").
Instead of "just" fixing the TX delay we can even do better and
switch to phy-mode = "rgmii-id" to let the PHY generate the RX
and TX delay. However, previously we didn't know that there was
an RX delay applied by the MAC on these boards. Only the additional
information from Jianxin in the other series [0] made us aware
of this. Without the other series there will be a 4ns RX delay
(2ns from the MAC and additional 2ns from the PHY). Due to this
dependency I did not add a Fixes tag, because backporting these
.dts patches without their runtime dependency will break stable
kernels.

TL;DR:
Ethernet TX throughput on my Odroid-C1 improved from <200Mbit/s
to >700Mbit/s (measured with iperf3)


Dependencies:
This series has a runtime dependency on v7 of the series called
"dwmac-meson8b Ethernet RX delay configuration" [0]


[0] https://patchwork.kernel.org/cover/11543997/


Martin Blumenstingl (2):
ARM: dts: meson: Add the Ethernet "timing-adjustment" clock
ARM: dts: meson: Switch existing boards with RGMII PHY to "rgmii-id"

arch/arm/boot/dts/meson8b-odroidc1.dts | 3 +--
arch/arm/boot/dts/meson8b.dtsi | 5 +++--
arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 4 +---
arch/arm/boot/dts/meson8m2.dtsi | 5 +++--
4 files changed, 8 insertions(+), 9 deletions(-)

--
2.26.2


2020-05-12 21:53:57

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 1/2] ARM: dts: meson: Add the Ethernet "timing-adjustment" clock

Add the "timing-adjusment" clock now that we now that this is connected
to the PRG_ETHERNET registers. It is used internally to generate the
RGMII RX delay no the MAC side (if needed).

Signed-off-by: Martin Blumenstingl <[email protected]>
---
arch/arm/boot/dts/meson8b.dtsi | 5 +++--
arch/arm/boot/dts/meson8m2.dtsi | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index e34b039b9357..ba36168b9c1b 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -425,8 +425,9 @@ &ethmac {

clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_MPLL2>,
- <&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+ <&clkc CLKID_MPLL2>,
+ <&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;

diff --git a/arch/arm/boot/dts/meson8m2.dtsi b/arch/arm/boot/dts/meson8m2.dtsi
index 5bde7f502007..96b37d5e9afd 100644
--- a/arch/arm/boot/dts/meson8m2.dtsi
+++ b/arch/arm/boot/dts/meson8m2.dtsi
@@ -30,8 +30,9 @@ &ethmac {
0xc1108140 0x8>;
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_MPLL2>,
- <&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+ <&clkc CLKID_MPLL2>,
+ <&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
resets = <&reset RESET_ETHERNET>;
reset-names = "stmmaceth";
};
--
2.26.2

2020-05-12 21:56:21

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dts: meson: Switch existing boards with RGMII PHY to "rgmii-id"

Let the PHY generate the RX and TX delay on the Odroid-C1 and MXIII
Plus.

Previously we did not know that these boards used an RX delay. We
assumed that setting the TX delay on the MAC side It turns out that
these boards also require an RX delay of 2ns (verified on Odroid-C1,
but the u-boot code uses the same setup on both boards). Ethernet only
worked because u-boot added this RX delay on the MAC side.

The 4ns TX delay was also wrong and the result of using an unsupported
RGMII TX clock divider setting. This has been fixed in the driver with
commit bd6f48546b9cb7 ("net: stmmac: dwmac-meson8b: Fix the RGMII TX
delay on Meson8b/8m2 SoCs").

Switch to phy-mode "rgmii-id" to let the PHY side handle all the delays,
(as recommended by the Ethernet maintainers anyways) to correctly
describe the need for a 2ns RX as well as 2ns TX delay on these boards.
This fixes the Ethernet performance on Odroid-C1 where there was a huge
amount of packet loss when transmitting data due to the incorrect TX
delay.

Signed-off-by: Martin Blumenstingl <[email protected]>
---
arch/arm/boot/dts/meson8b-odroidc1.dts | 3 +--
arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
index a2a47804fc4a..cb21ac9f517c 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -202,9 +202,8 @@ &ethmac {
pinctrl-0 = <&eth_rgmii_pins>;
pinctrl-names = "default";

- phy-mode = "rgmii";
phy-handle = <&eth_phy>;
- amlogic,tx-delay-ns = <4>;
+ phy-mode = "rgmii-id";

nvmem-cells = <&ethernet_mac_address>;
nvmem-cell-names = "mac-address";
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index d54477b1001c..cc498191ddd1 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -69,9 +69,7 @@ &ethmac {
pinctrl-names = "default";

phy-handle = <&eth_phy0>;
- phy-mode = "rgmii";
-
- amlogic,tx-delay-ns = <4>;
+ phy-mode = "rgmii-id";

mdio {
compatible = "snps,dwmac-mdio";
--
2.26.2

2020-05-20 00:10:05

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: dts: meson8b/m2: RGMII improvements

On Tue, 12 May 2020 23:51:46 +0200, Martin Blumenstingl wrote:
> the fist patch in this series connects FCLK_DIV2 to the PRG_ETH
> "additional" registers for the dwmac Ethernet controller.
> Now that we know how RGMII and FCLK_DIV2 are connected we can
> add this dependency to get rid of CLK_IS_CRITICAL for FCLK_DIV2
> at some point.
>
> The second patch fixes the RX and TX delay. The 4ns TX delay which
> we have used so far is incorrect and only worked because we were
> using an unsupported clock divider in the PRG_ETH registers. That
> divider has been fixed with commit bd6f48546b9c ("net: stmmac:
> dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs").
> Instead of "just" fixing the TX delay we can even do better and
> switch to phy-mode = "rgmii-id" to let the PHY generate the RX
> and TX delay. However, previously we didn't know that there was
> an RX delay applied by the MAC on these boards. Only the additional
> information from Jianxin in the other series [0] made us aware
> of this. Without the other series there will be a 4ns RX delay
> (2ns from the MAC and additional 2ns from the PHY). Due to this
> dependency I did not add a Fixes tag, because backporting these
> .dts patches without their runtime dependency will break stable
> kernels.
>
> [...]

Applied, thanks!

[1/2] ARM: dts: meson: Add the Ethernet "timing-adjustment" clock
commit: b632506c5af22a9a7c63674fc605d24cf94d585b
[2/2] ARM: dts: meson: Switch existing boards with RGMII PHY to "rgmii-id"
commit: 005231128e9e97461e81fa32421957a7664317ca

Best regards,
--
Kevin Hilman <[email protected]>