linux-kernel - Re: [PATCH v2] arm64: dts: rockchip: support Ethernet Switch adapter for RK3588 Jaguar

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: <bb3486c6-93df-4453-acc6-deba3c8f7f0e@lunn.ch>
Date: Sun, 15 Jun 2025 16:53:52 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Quentin Schulz <quentin.schulz@...rry.de>
Cc: Jakob Unterwurzacher <jakobunt@...il.com>, foss+kernel@...il.net,
	conor+dt@...nel.org, devicetree@...r.kernel.org, heiko@...ech.de,
	jakob.unterwurzacher@...rry.de, krzk+dt@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-rockchip@...ts.infradead.org, robh@...nel.org,
	Kever Yang <kever.yang@...k-chips.com>
Subject: Re: [PATCH v2] arm64: dts: rockchip: support Ethernet Switch adapter
 for RK3588 Jaguar

On Fri, Jun 13, 2025 at 04:27:54PM +0200, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 5/28/25 3:09 PM, Andrew Lunn wrote:
> > On Wed, May 28, 2025 at 09:56:51AM +0200, Quentin Schulz wrote:
> > > Hi Andrew,
> > > 
> > > On 5/27/25 6:18 PM, Andrew Lunn wrote:
> > > > On Tue, May 27, 2025 at 03:11:42PM +0200, Jakob Unterwurzacher wrote:
> > > > > > @Jakob, is this something you could check? devmem2 0xfd58c31c w 0x3c0000
> > > > > > should do the trick to disable the circuitry according to the TRM?
> > > > > 
> > > > > I measured TXCLK vs TXD3 on an oscilloscope on gmac1:
> > > > > 
> > > > > 	Setting	Decimal	Actual TXCLK delay (ps)
> > > > > 	00	0	47
> > > > > 	0a	10	283
> > > > > 	10	16	440
> > > > > 	20	32	893
> > > > > 	30	48	1385
> > > > > 	40	64	1913
> > > > > 	50	80	2514
> > > > > 	60	96	3077
> > > > > 	70	112	3565
> > > > > 	7f	127	4009
> > > > > 
> > > > > 	off	x	-315
> > > > > 
> > > > > Setting = tx_delay (hex)
> > > > > Decimal = tx_delay (dec)
> > > > > Actual TXCLK delay (ps) = Measurement from oscilloscope
> > > > > 
> > > > > Plotting this we can deduce that one tx_delay unit is about 31ps.
> > > > 
> > > > Nice to see somebody actually do the measurements. Based on this, it
> > > > would be good to implement:
> > > > 
> > > >           tx-internal-delay-ps:
> > > >             description:
> > > >               RGMII Transmit Clock Delay defined in pico seconds. This is used for
> > > >               controllers that have configurable TX internal delays. If this
> > > >               property is present then the MAC applies the TX delay.
> > > > 
> > > > For the moment, please limit it to just the device you measured it on.
> > > > 
> > > 
> > > What exactly do you mean with "limit it to just the device you measured it
> > > on"?
> > 
> > Nobody seems to know if rx_delay & tx_delay operate the same across
> > the whole range of SoCs. I don't particularly care if these properties
> > are difference between SoC, they are vendor properties, with
> > undocumented magic values. However 'tx-internal-delay-ps' is
> > standardised, and has a very clear meaning. I don't want it used
> > unless somebody has performed a measurement and we know that 2000
> > produces a 2ns delay.
> > 
> > > I'll need to implement reading the delay from the stmmac driver to use this
> > > property, do I need to restrict reading this property to the SoC we tested
> > > (RK3588)?
> > 
> > Yes, please only allow it to be used on RK3588, and any other SoC you
> > can test and verify its behaviour.
> 
> Coming back to this topic, I'm unfortunately the bearer of some bad news.
> 
> I implemented the suggested logic (see at the end of this mail) and then
> went to validate it with Jakob's help. Unfortunately, it seems that the
> delay value really isn't stable or reliable.
> 
> We tested the same adapter with two different main boards (the same product,
> just two different units). With a value of 0x40 for tx_delay (which should
> be ~2000ps if we have a 31ps per tx_delay unit as empirically decided), we
> have one board with 1778ps and one with 1391ps. Following a hunch, we
> started to stress (or cool) the device (with stress-ng/a fan) and it did
> slightly change the result too. Changing the CPU operating points (and by
> extension at least CPU clocks) didn't impact the result though.

Thanks for taking such a scientific approach to this. Most developers
try values until it works, and call it done. It is nice to see
somebody doing some real study.

Russell quoted the standard, which says the delay needs to be between
1ns and 2.6ns, which is quite a wide range. So for a tx_delay value of
0x40, nominally 2000ps, your two values are within that range, and so
conform to the standard.

> While this could be observed with tx_delay property too, this property
> doesn't claim to provide a value in picoseconds that tx-internal-delay-ps
> would (but at the same time this didn't stop it to be implemented for the
> DSA switch we have which claims "more than 1.5ns" and nothing more, so maybe
> that would be acceptable?).
> 
> I feel uncomfortable contributing this considering the wildly different
> results across our very small test sample pool of two units and slightly
> different operating temperature.

I can understand that. But there is another way to look at this. I am
making a big jump from just two boards, but it seems to me, tx_delay
and rx_delay are pointless, if they produce such a wide range of
values from what should be identical boards. They cannot be used for
fine tuning because the same value has a 387ps difference, which is
huge compared to the 31ps step.

It seems to me, rx_delay and tx_delay should be deprecated, set to 0,
and let the PHY do they delays. If i remember correctly, that is what
you ended up with for you board?

	Andrew