[PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D

Sebastian Reichel posted 1 patch 2 months, 1 week ago
arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Sebastian Reichel 2 months, 1 week ago
According to the Ethernet controller device tree binding "rgmii-id"
means, that the PCB does not have extra long lines to add the required
delays. This is indeed the case for the ROCK 4D.

The problem is, that the Rockchip MAC Linux driver interprets the
interface type differently and abuses the information to configure
RX and TX delays in the MAC using (vendor) properties 'rx_delay' and
'tx_delay'.

When Detlev Casanova upstreamed the ROCK 4D device tree, he used the
correct description for the board ("rgmii-id"). This results in no delays
being configured in the MAC. At the same time the PHY will provide
some delays.

This works to some degree, but is not a stable configuration. All five
ROCK 4D production boards, which have recently been added to the Collabora
LAVA lab for CI purposes have trouble with data not getting through
after a connection has been established.

Using the same delay setup as the vendor device tree fixes the
functionality (at the cost of not properly following the DT binding).
As we cannot fix the driver behavior for RK3576 (some other boards
already depend on this), let's update the ROCK 4D DT instead.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
There is an additional problem, that the link does not come up in the
first place. This has been handled by the recently merged (and less
questionable) DT changes [0] combined with a PHY driver patch to trigger
a PHY reset after the clock has been enabled [1] (I will send a new
version of that patch after this one). Stable network support on the
ROCK 4D requires all patches.

[0] https://lore.kernel.org/linux-rockchip/20250704-rk3576-rock4d-phy-handling-fixes-v1-1-1d64130c4139@kernel.org/
[1] https://lore.kernel.org/netdev/20250704-phy-realtek-clock-fix-v1-1-63b33d204537@kernel.org/
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 0388d72076bf79ee4f20ac1d3bc04fe1859f857b..f105ed675d3671e93916c830a9a17dc240ca5fda 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -237,7 +237,7 @@ &cpu_l3 {
 &gmac0 {
 	clock_in_out = "output";
 	phy-handle = <&rgmii_phy0>;
-	phy-mode = "rgmii-id";
+	phy-mode = "rgmii-rxid";
 	pinctrl-names = "default";
 	pinctrl-0 = <&eth0m0_miim
 		     &eth0m0_tx_bus2
@@ -246,6 +246,8 @@ &eth0m0_rgmii_clk
 		     &eth0m0_rgmii_bus
 		     &ethm0_clk0_25m_out>;
 	status = "okay";
+	tx_delay = <0x20>;
+	rx_delay = <0x00>;
 };
 
 &gpu {

---
base-commit: e05818ef75bee755fc56811cb54febf4174d7cf2
change-id: 20250724-rk3576-rock4d-phy-timings-6b4eb4ba82fc

Best regards,
-- 
Sebastian Reichel <sre@kernel.org>
Re: [PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Russell King (Oracle) 2 months, 1 week ago
On Thu, Jul 24, 2025 at 04:31:25PM +0200, Sebastian Reichel wrote:
> According to the Ethernet controller device tree binding "rgmii-id"
> means, that the PCB does not have extra long lines to add the required
> delays. This is indeed the case for the ROCK 4D.
> 
> The problem is, that the Rockchip MAC Linux driver interprets the
> interface type differently and abuses the information to configure
> RX and TX delays in the MAC using (vendor) properties 'rx_delay' and
> 'tx_delay'.
> 
> When Detlev Casanova upstreamed the ROCK 4D device tree, he used the
> correct description for the board ("rgmii-id"). This results in no delays
> being configured in the MAC. At the same time the PHY will provide
> some delays.

So why can't the MAC be fixed to conform to the requirements that are
well documented for RGMII?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Sebastian Reichel 2 months, 1 week ago
Hi,

On Thu, Jul 24, 2025 at 07:16:53PM +0100, Russell King (Oracle) wrote:
> On Thu, Jul 24, 2025 at 04:31:25PM +0200, Sebastian Reichel wrote:
> > According to the Ethernet controller device tree binding "rgmii-id"
> > means, that the PCB does not have extra long lines to add the required
> > delays. This is indeed the case for the ROCK 4D.
> > 
> > The problem is, that the Rockchip MAC Linux driver interprets the
> > interface type differently and abuses the information to configure
> > RX and TX delays in the MAC using (vendor) properties 'rx_delay' and
> > 'tx_delay'.
> > 
> > When Detlev Casanova upstreamed the ROCK 4D device tree, he used the
> > correct description for the board ("rgmii-id"). This results in no delays
> > being configured in the MAC. At the same time the PHY will provide
> > some delays.
> 
> So why can't the MAC be fixed to conform to the requirements that are
> well documented for RGMII?

Fixing the MAC driver without introducing regressions is not so
easy. It currently abuses the RGMII mode to detect if MAC delays
should be applied:

PHY_INTERFACE_MODE_RGMII => apply MAC delays
PHY_INTERFACE_MODE_RGMII_ID => do not apply MAC delays
PHY_INTERFACE_MODE_RGMII_RXID => apply TX MAC delay
PHY_INTERFACE_MODE_RGMII_TXID => apply RX MAC delay

As the delay values are sourced from DT, in theory in would be
possible to just always apply the MAC delays. The problem with
that is, that the default (missing DT properties) is not 0. So
this would result in regressions.

Greetings,

-- Sebastian
Re: [PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Andrew Lunn 2 months, 1 week ago
>  &gmac0 {
>  	clock_in_out = "output";
>  	phy-handle = <&rgmii_phy0>;
> -	phy-mode = "rgmii-id";
> +	phy-mode = "rgmii-rxid";
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&eth0m0_miim
>  		     &eth0m0_tx_bus2
> @@ -246,6 +246,8 @@ &eth0m0_rgmii_clk
>  		     &eth0m0_rgmii_bus
>  		     &ethm0_clk0_25m_out>;
>  	status = "okay";
> +	tx_delay = <0x20>;
> +	rx_delay = <0x00>;

What does 0x20 mean? Is it less than 2ns, or greater than 2ns?

Have you tried "rgmii-id" and small values for tx_delay? If the
hardware needs 2.1ns, for example, the MAC could add 0.1ns and the PHY
adds the default 2ns. That would allow you to conform to the DT
binding.

What PHY is this? Have you looked it you can control the delays the
PHY adds? If you actually need a delay of 1.9ns, maybe the PHY can be
configured to do this? That would also allow you to conform to the DT
binding.

	Andrew
Re: [PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Sebastian Reichel 2 months, 1 week ago
Hi,

On Thu, Jul 24, 2025 at 05:51:16PM +0200, Andrew Lunn wrote:
> >  &gmac0 {
> >  	clock_in_out = "output";
> >  	phy-handle = <&rgmii_phy0>;
> > -	phy-mode = "rgmii-id";
> > +	phy-mode = "rgmii-rxid";
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&eth0m0_miim
> >  		     &eth0m0_tx_bus2
> > @@ -246,6 +246,8 @@ &eth0m0_rgmii_clk
> >  		     &eth0m0_rgmii_bus
> >  		     &ethm0_clk0_25m_out>;
> >  	status = "okay";
> > +	tx_delay = <0x20>;
> > +	rx_delay = <0x00>;
> 
> What does 0x20 mean? Is it less than 2ns, or greater than 2ns?

Unfortunately I don't know. This part is not documented in the TRM I
have access to. Also my scope is not good enough for measuring
delays in the pico-second range. Previous measurements for RK3588
(RK3576 uses most of its IP and probably shared most of its quirks)
suggested, that mapping this to the standard properties may not
easily be possible:

https://lore.kernel.org/all/bb3486c6-93df-4453-acc6-deba3c8f7f0e@lunn.ch/

> Have you tried "rgmii-id" and small values for tx_delay? If the
> hardware needs 2.1ns, for example, the MAC could add 0.1ns and the PHY
> adds the default 2ns. That would allow you to conform to the DT
> binding.

The MAC code (drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c) looks
like this, so "rgmii-id" results in no MAC delays being applied:

	case PHY_INTERFACE_MODE_RGMII:
		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay,
					    bsp_priv->rx_delay);
		break;
	case PHY_INTERFACE_MODE_RGMII_ID:
		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
		break;
	case PHY_INTERFACE_MODE_RGMII_RXID:
		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay, 0);
		break;
	case PHY_INTERFACE_MODE_RGMII_TXID:
		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, bsp_priv->rx_delay);
		break;
    ...

Also the default values (if properties are missing in DT) are
rx_delay=0x10 and tx_delay=0x30, so changing this logic risks
breaking some boards :(

> What PHY is this? Have you looked it you can control the delays the
> PHY adds? If you actually need a delay of 1.9ns, maybe the PHY can be
> configured to do this? That would also allow you to conform to the DT
> binding.

The Rock 4D uses a RTL8211F. As far as I can see that just offers
one delay control knob.

Greetings,

-- Sebastian
Re: [PATCH] arm64: dts: rockchip: use MAC TX delay for ROCK 4D
Posted by Andrew Lunn 2 months, 1 week ago
> > Have you tried "rgmii-id" and small values for tx_delay? If the
> > hardware needs 2.1ns, for example, the MAC could add 0.1ns and the PHY
> > adds the default 2ns. That would allow you to conform to the DT
> > binding.
> 
> The MAC code (drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c) looks
> like this, so "rgmii-id" results in no MAC delays being applied:
> 
> 	case PHY_INTERFACE_MODE_RGMII:
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay,
> 					    bsp_priv->rx_delay);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_ID:
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_RXID:
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay, 0);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_TXID:
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, bsp_priv->rx_delay);
> 		break;
>     ...
> 
> Also the default values (if properties are missing in DT) are
> rx_delay=0x10 and tx_delay=0x30, so changing this logic risks
> breaking some boards :(

Just as an experiment, could you enable setting delays for
PHY_INTERFACE_MODE_RGMII_ID and see if small values do work for you.

If they do, we can then figure out a way to actually do it without
breaking other boards.

	Andrew