[RFC v1 0/2] dwmac-meson8b Ethernet RX delay configuration

[RFC v1 0/2] dwmac-meson8b Ethernet RX delay configuration

[Martin Blumenstingl]

The Ethernet TX performance has been historically bad on Meson8b and
Meson8m2 SoCs because high packet loss was seen. I found out that this
was related (yet again) to the RGMII TX delay configuration.
In the process of discussing the big picture (and not just a single
patch) [0] with Andrew I discovered that the IP block behind the
dwmac-meson8b driver actually seems to support the configuration of the
RGMII RX delay (at least on the Meson8b SoC generation).

The goal of this series is to start the discussion around how to
implement the RGMII RX delay on this IP block. Additionally it seems
that the RX delay can also be applied for RMII PHYs?

@Jianxin: can you please add the Amlogic internal Ethernet team to this
discussion? My questions are documented in the patch description of
patch #2.

Dependencies: this series is based on my other series [1]
"net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs"


@David: please do NOT merge this series yet, it's only meant for
discussion in it's current state!


[0] https://patchwork.kernel.org/patch/11309891/
[1] https://patchwork.kernel.org/patch/11310669/


Martin Blumenstingl (2):
  net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it
  net: stmmac: dwmac-meson8b: add support for the RX delay configuration

 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 60 ++++++++++---------
 1 file changed, 33 insertions(+), 27 deletions(-)

-- 
2.24.1


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

[RFC v1 1/2] net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it

[Martin Blumenstingl]

Use FIELD_PREP() to shift a value to the correct offset based on a
bitmask instead of open-coding the logic.
No functional changes.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 0e2fa14f1423..1483761ab1e6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2016 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -32,7 +33,6 @@
 #define PRG_ETH0_CLK_M250_SEL_SHIFT	4
 #define PRG_ETH0_CLK_M250_SEL_MASK	GENMASK(4, 4)
 
-#define PRG_ETH0_TXDLY_SHIFT		5
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
 
 /* divider for the result of m250_sel */
@@ -261,7 +261,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+					FIELD_PREP(PRG_ETH0_TXDLY_MASK,
+						   tx_dly_val));
 
 		/* Configure the 125MHz RGMII TX clock, the IP block changes
 		 * the output automatically (= without us having to configure
-- 
2.24.1


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

[RFC v1 2/2] net: stmmac: dwmac-meson8b: add support for the RX delay configuration

[Martin Blumenstingl]

DO NOT MERGE THIS!

Test results:
- Akaso M8S (not upstream yet) Meson8m2 with RMII PHY: no change
- Odroid-C1 Meson8b with RTL8211F RGMII PHY:
  phy-mode = "rgmii" still works
  phy-mode = "rgmii-id" works now
- Khadas VIM2 GXM with RTL8211F RGMII PHY:
  phy-mode = "rgmii" is now broken!
  phy-mode = "rgmii-id" works

The public A311D datasheet [0] mentions these bits in the PRG_ETH0 (also
call PRG_ETH0_REG) register:
- bit[13]: RMII & RGMII mode, Enable data delay adjustment and
  calibration logic.
- bit[14]: Set RXDV and RXD setup time, data is aligned with index 0.
  When set to 1, auto delay and skew.
- bit[19:15]:  Set bit14 to 0. RMII & RGMII mode. Capture input data at
  clock index equal to adj_delay.
- bit[24:20]: RMII & RGMII mode. 5 Bits correspondent to
  {RXDV, RXD[3:0]}, set to 1 will delay the data capture by 1 cycle.
(do all of these bits have the same meaning for all supported SoCs:
Meson8b, Meson8m2, GXBB, GXL, GXM, AXG, G12A, G12B, SM1?)

The public "Amlogic Ethernet controller user guide" recommends the
following settings.
RMII: adj_enable = 1, adj_setup = 0, adj_delay = 18, adj_skew = 0x0
RGMII: adj_enable = 1, adj_setup = 1, adj_delay = 4, adj_skew = 0xc
(shouldn't adj_delay be 0 for the RGMII mode based on the register
description above?)

Do we need to expose adj_delay and adj_skew to devicetree?
How do I know which value to set for each board?

[0] https://dl.khadas.com/Hardware/VIM3/Datasheet/A311D_Datasheet_01_Wesion.pdf
[1] http://openlinux.amlogic.com/@api/deki/files/75/=Amlogic_Ethenet_controller_user_Guide.pdf

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 1483761ab1e6..af25eb1aea41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -33,6 +33,10 @@
 #define PRG_ETH0_CLK_M250_SEL_SHIFT	4
 #define PRG_ETH0_CLK_M250_SEL_MASK	GENMASK(4, 4)
 
+/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where 8ns are exactly one
+ * cycle of the 125MHz RGMII TX clock):
+ * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+ */
 #define PRG_ETH0_TXDLY_MASK		GENMASK(6, 5)
 
 /* divider for the result of m250_sel */
@@ -44,6 +48,11 @@
 #define PRG_ETH0_INVERTED_RMII_CLK	BIT(11)
 #define PRG_ETH0_TX_AND_PHY_REF_CLK	BIT(12)
 
+#define PRG_ETH0_ADJ_ENABLE		BIT(13)
+#define PRG_ETH0_ADJ_SETUP		BIT(14)
+#define PRG_ETH0_ADJ_DELAY		GENMASK(19, 15)
+#define PRG_ETH0_ADJ_SKEW		GENMASK(24, 20)
+
 #define MUX_CLK_NUM_PARENTS		2
 
 struct meson8b_dwmac;
@@ -241,29 +250,38 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac *dwmac)
 
 static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 {
+	u8 tx_dly_val = dwmac->tx_delay_ns >> 1;
+	u32 delay_config;
 	int ret;
-	u8 tx_dly_val = 0;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
+		delay_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK, tx_dly_val);
+		delay_config |= PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
+		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
-		/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
-		 * 8ns are exactly one cycle of the 125MHz RGMII TX clock):
-		 * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
-		 */
-		tx_dly_val = dwmac->tx_delay_ns >> 1;
-		/* fall through */
-
-	case PHY_INTERFACE_MODE_RGMII_ID:
+		delay_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK, tx_dly_val);
+		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
+		delay_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RMII:
+	default:
+		delay_config = 0;
+		break;
+	};
+
+	meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK |
+				PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP |
+				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
+				delay_config);
+
+	if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
 		/* only relevant for RMII mode -> disable in RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK, 0);
 
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					FIELD_PREP(PRG_ETH0_TXDLY_MASK,
-						   tx_dly_val));
-
 		/* Configure the 125MHz RGMII TX clock, the IP block changes
 		 * the output automatically (= without us having to configure
 		 * a register) based on the line-speed (125MHz for Gbit speeds,
@@ -286,24 +304,11 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 		devm_add_action_or_reset(dwmac->dev,
 					(void(*)(void *))clk_disable_unprepare,
 					dwmac->rgmii_tx_clk);
-		break;
-
-	case PHY_INTERFACE_MODE_RMII:
+	} else {
 		/* invert internal clk_rmii_i to generate 25/2.5 tx_rx_clk */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
 					PRG_ETH0_INVERTED_RMII_CLK,
 					PRG_ETH0_INVERTED_RMII_CLK);
-
-		/* TX clock delay cannot be configured in RMII mode */
-		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
-					0);
-
-		break;
-
-	default:
-		dev_err(dwmac->dev, "unsupported phy-mode %s\n",
-			phy_modes(dwmac->phy_mode));
-		return -EINVAL;
 	}
 
 	/* enable TX_CLK and PHY_REF_CLK generator */
-- 
2.24.1


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

Re: [RFC v1 0/2] dwmac-meson8b Ethernet RX delay configuration

[Martin Blumenstingl]

Hello Jianxin,

On Thu, Dec 26, 2019 at 9:37 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> The Ethernet TX performance has been historically bad on Meson8b and
> Meson8m2 SoCs because high packet loss was seen. I found out that this
> was related (yet again) to the RGMII TX delay configuration.
> In the process of discussing the big picture (and not just a single
> patch) [0] with Andrew I discovered that the IP block behind the
> dwmac-meson8b driver actually seems to support the configuration of the
> RGMII RX delay (at least on the Meson8b SoC generation).
>
> The goal of this series is to start the discussion around how to
> implement the RGMII RX delay on this IP block. Additionally it seems
> that the RX delay can also be applied for RMII PHYs?
>
> @Jianxin: can you please add the Amlogic internal Ethernet team to this
> discussion? My questions are documented in the patch description of
> patch #2.
do you already have an update for me on this topic?

while we're discussing unknown bits of the Ethernet controller I also
remembered that we're currently not describing the relation between
the "fclk_div2" clock and the Ethernet controller. however, as
described in commit 72e1f230204039 ("clk: meson: meson8b: mark
fclk_div2 gate clocks as CLK_IS_CRITICAL") this is needed for RGMII
mode.
it would be great to know the relation between fclk_div2 and RGMII
mode on the Ethernet controller!


Thank you!
Martin

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