linux-kernel - Re: [PATCH 2/6] arm64: dts: rockchip: Fix the 1Ghz ethernet on Qnap TS433

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e4d3127b-c879-4931-9ea0-de7449bc508c@lunn.ch>
Date: Thu, 25 Sep 2025 19:11:29 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jonas Karlman <jonas@...boo.se>
Cc: Heiko Stuebner <heiko@...ech.de>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	"ukleinek@...ian.org" <ukleinek@...ian.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-rockchip@...ts.infradead.org" <linux-rockchip@...ts.infradead.org>
Subject: Re: [PATCH 2/6] arm64: dts: rockchip: Fix the 1Ghz ethernet on Qnap
 TS433

On Thu, Sep 25, 2025 at 06:58:06PM +0200, Jonas Karlman wrote:
> Hi Heiko,
> 
> On 9/25/2025 11:29 AM, Heiko Stuebner wrote:
> > While I want to remember that the dwmac on the TS433 was working at some
> > point, it seems I had my network always connected to the 2.5G nic after
> > that "point". And testing now revealed that the gmac does not actually
> > manages to transfer data.
> > 
> > Currently the gmac is set to rgmii-id with no rx-/tx-delay values set
> > which makes the driver use default values. Setting the delays to 0
> > also does not provide a working interface.
> > 
> > The vendor kernel is running with phy-mode set to rgmii and delays of
> >     tx_delay = 0x3c, rx_delay = 0x2f
> > 
> > As Andrew points out often, those delay values "are magic" and rgmii-id
> > should definitly be used "with small values" for delays, if really needed.
> > 
> > The Rockchip vendor-kernel actually contains additional code in the dwmac
> > driver to use the loopback function of a phy to find a window of usable
> > delay values. Code can be found for example on [0] and the process is
> > described in a document called "Rockchip GMAC RGMII Delayline Guide"
> > which has made its way onto the internet in a lot of places [1].
> > 
> > So I used this process, with the interface set to rgmii-id to get values
> > for this mode, which are in face lower than the ones for rgmii with
> >     tx_delay = 0x21, rx_delay = 0x15
> > and results in a working interface on the dwmac.
> > 
> > [0] https://github.com/armbian/linux-rockchip/blob/rk-6.1-rkr6.1/drivers/net/ethernet/stmicro/stmmac/dwmac-rk-tool.c
> > [1] https://gitlab.com/firefly-linux/docs/-/blob/rk356x/firefly/Common/GMAC/Rockchip_Developer_Guide_Linux_GMAC_RGMII_Delayline_EN.pdf
> > 
> > Cc: Andrew Lunn <andrew@...n.ch>
> > Signed-off-by: Heiko Stuebner <heiko@...ech.de>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> > index 5656554ca284..e8af92a011d6 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-qnap-ts433.dts
> > @@ -257,6 +257,8 @@ &gmac0_tx_bus2
> >  		     &gmac0_rx_bus2
> >  		     &gmac0_rgmii_clk
> >  		     &gmac0_rgmii_bus>;
> > +	rx_delay = <0x15>;
> > +	tx_delay = <0x21>;
> 
> I do not understand why defining rx/tx_delay would change anything.

I was also wondering why these two properties are added but no change
to phy-mode.

> 
> Setting these should currently not have any effect on the driver code
> when phy-mode=rgmii-id is used, see below (next-20250924, dwmac-rk.c):
> 
> 
> 	switch (bsp_priv->phy_iface) {
> 	case PHY_INTERFACE_MODE_RGMII:
> 		dev_info(dev, "init for RGMII\n");
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay,
> 					    bsp_priv->rx_delay);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_ID:
> 		dev_info(dev, "init for RGMII_ID\n");
> 		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
> 		break;

And this explains it, thanks.

> I have played around with a few patches that changes this and apply the
> rx/tx_delay for rgmii-id modes (both Linux and U-Boot), see top of [2]
> for Linux patches. Will try to get them on ML in a few days.

We should not really be expanding the use of rx_delay and
tx_delay. The standardized properties {tx|rx}-internal-delay-ps should
be used.

	Andrew