netdev - Re: [PATCH net-next 1/2] net: phy: motorcomm: correct the default rx delay config for the rgmii

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: <ccbf01a4-71e7-446f-b13b-5c090d5a621e@lunn.ch>
Date: Fri, 31 Oct 2025 13:21:22 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Yi Cong <cong.yi@...ux.dev>
Cc: Frank.Sae@...or-comm.com, davem@...emloft.net, hkallweit1@...il.com,
	kuba@...nel.org, linux@...linux.org.uk, netdev@...r.kernel.org,
	yicong@...inos.cn
Subject: Re: [PATCH net-next 1/2] net: phy: motorcomm: correct the default rx
 delay config for the rgmii

> > However, in general, Linux should not rely on the BIOS, Linux should
> > correctly configure the hardware, and in this case, it is trivial to
> > do, since it is already all implemented.
> 
> The phy-mode is read by the MAC from ACPI. The same MAC paired with
> different PHYs, or different vendors using the same MAC+PHY combination
> but with varying motherboard designs, may result in differences here
> (I've encountered both rgmii-id and rgmii-rxid).

There are two basic PCB designs. The PCB provides the 2ns delay by
using extra long clock lines. If that is the case, DT says 'rgmii'.
If the PCB does not have extra long clock lines, the MAC/PHY pair
needs to provide the delays, so DT says 'rgmii-id'. Very few designs
have the PCB add delays, so 95% of boards use 'rgmii-id'. There is one
board i know of which has one extra long clock line, one not, so it
uses rgmii-rxid, or maybe rgmii-txid in DT. I don't remember which.

If you are doing an ACPI binding the same should apply.

> Returning to the current patch: changing the default value to match the
> datasheet will not affect any phy-mode directly. This is because the delay value
> is first obtained, and then its application is determined based on the phy-mode:
> ```
> static int ytphy_rgmii_clk_delay_config(struct phy_device *phydev)
> {
> 	...
> 	/* get rx/tx delay value */
> 	rx_reg = ytphy_get_delay_reg_value(phydev, "rx-internal-delay-ps",
> 					   ytphy_rgmii_delays, tb_size,
> 					   &rxc_dly_en,
> 					   YT8521_RC1R_RGMII_0_000_NS);
> 	tx_reg = ytphy_get_delay_reg_value(phydev, "tx-internal-delay-ps",
> 					   ytphy_rgmii_delays, tb_size, NULL,
> 					   YT8521_RC1R_RGMII_NONLINEAR_0_750_NS);
> 
> 	/* Set it based on different phy-mode */
> 	switch (phydev->interface) {
> 	case PHY_INTERFACE_MODE_RGMII:
> 		rxc_dly_en = 0;
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_RXID:
> 		val |= FIELD_PREP(YT8521_RC1R_RX_DELAY_MASK, rx_reg);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_TXID:
> 		rxc_dly_en = 0;
> 		val |= FIELD_PREP(YT8521_RC1R_GE_TX_DELAY_MASK, tx_reg);
> 		break;
> 	case PHY_INTERFACE_MODE_RGMII_ID:
> 		val |= FIELD_PREP(YT8521_RC1R_RX_DELAY_MASK, rx_reg) |
> 		       FIELD_PREP(YT8521_RC1R_GE_TX_DELAY_MASK, tx_reg);
> 		break;
> 	default: /* do not support other modes */
> 		return -EOPNOTSUPP;
> 	}

Now think about what PHY_INTERFACE_MODE_RGMII_ID means. It means, the
PCB does not add the 2ns delays. The MAC/PHY combination must add the
delays, and when the MAC has called phy_connect(), it has passed
PHY_INTERFACE_MODE_RGMII_ID, asking the PHY to add the 2ns delay. It
can add a range of delays, but the default is 1.95ns, which is the
closest it can get to the 2ns defined in the RGMII standard.

If the PHY actually added 0ns delay, the board is broken, because it
is missing the delays. The default is correct and there are shipping
boards making use of it.

Any issues you are having are because you are not using the APIs
correctly. If the PCB is adding the delays, use 'rgmii' in DT. If the
MAC is adding delays, which we do not recommend, pass
PHY_INTERFACE_MODE_RGMII to the PHY. This is all explained in one of
the links i gave you.

   Andrew