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

[Quentin Schulz <quentin.schulz@cherry.de>]

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"?

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)? Or should I just apply it indiscriminately (considering 
that no Rockchip board actually set this property in its DT?) and let 
future users fix up the scale for the other SoCs whenever they want to 
use this property?

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

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?

Is this request blocking the merging of this DTSO patch or would a 
follow-up series be okay?

Cheers,
Quentin