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

[Andrew Lunn <andrew@lunn.ch>]

On Thu, May 22, 2025 at 10:18:12AM +0200, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 5/21/25 6:25 PM, Andrew Lunn wrote:
> > > +&gmac1 {
> > > +	clock_in_out = "output";
> > > +	phy-mode = "rgmii";
> > 
> > Does the PCB have extra long clock lines to implement the 2ns delays?
> > 
> 
> Not that I am aware no.

So 'rgmii-id' describes the hardware.

> 
> The issue here is that I believe the Linux driver actually got the whole
> phy-mode thing wrong?

Quite possible, a few drivers do.

> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> 
> First, tx_delay defaults to 0x30 if absent, rx_delay to 0x10 if absent,
> which seems a bit odd but why not.
> 
> Then you have rk_gmac_powerup() handling the delays.
> 
> If RGMII, then use rx_delay and tx_delay. If RGMII-ID, use neither. If
> RGMII-RXID use tx_delay. If RGMII-TXID use rx_delay.
> 
> This is the complete opposite of what I was expecting?

This driver, and the aspeed driver cause a lot of problems....

> > Since this has a switch on the other end, its a bit more complicated
> > with RGMII delays. Normally, the MAC does nothing and passed rgmii-id
> > to the PHY, and the PHY then does the delays. However, here you don't
> > have a PHY. So you have the MAC add the delays. This looks O.K. I
> 
> The switch actually supports adding delays on the port used for DSA conduit.

That actually looks to be the simplest and correct solution. Set the
MAC to 'rgmii-id', rx_delay and tx_delay to 0, even if they are
ignored. And in the switch, also 'rgmii-id' and let it insert the 2ns
delay. You can use rx-internal-delay-ps and tx-internal-delay-ps if
you want, but it seems to default to sensible values.

> I'm a bit confused by the following sentence:
> 
> """
> Normally, the MAC does nothing and passed rgmii-id
> """
> 
> is this something that the MAC driver is supposed to do or is the subsystem
> handling that somehow? How do I know how/when to rewrite the phy-mode passed
> to the PHY?

A small number of MACs have hard coded delays. You cannot turn the
delay off. So the MAC has no choice but to do the delay. 'rgmii' is
simply not possible, so -EINVAL. For 'rgmii-id', if you pass that to
the PHY, it will also add a delay, and 4ns in total does not work. So
when the MAC is adding delays, it needs to mask out the delays it is
adding before calling phy_attach() passing an rgmii mode.

	Andrew