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] [day] [month] [year] [list]

Message-ID: <3303d8d4-ec5a-4cdc-8391-ab6e35d76b33@lunn.ch>
Date: Wed, 28 May 2025 15:09:10 +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 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.

> I assume you're then expecting tx-internal-delay-ps only on this new DTSO's
> gmac1?

I would like rx_delay and tx_delay to be marked deprecated, with the
end goal they are no longer used, the standard properties take there
place. But to get there, we need more measurements from real hardware,
or some other way to be sure what we have the correct delay.

> Would you still want rx_delay/tx_delay to be set to 0x00? Maybe only
> rx_delay since we won't have a companion rx-internal-delay-ps for now (until
> someone from Rockchip answers :); adding Kever back to the Cc for that)? Or
> should I remove both of them?

rx-internal-delay-ps and rx_delay should be mutually exclusive. If
both are present -EINVAL. We have to keep supporting rx_delay for
rk3588 otherwise we break backwards compatibility, but ideally no more
instances of it should be added once rx-internal-delay-ps is added.

	Andrew