[PATCH] net: phy: motorcomm: keep chip delay defaults when firmware provides none

[PATCH] net: phy: motorcomm: keep chip delay defaults when firmware provides none

[guoqi0226]

From: cuiguoqi <cuiguoqi@kylinos.cn>

ytphy_rgmii_clk_delay_config() unconditionally programs the YT8521
RGMII delay registers. When firmware describes neither
rx-internal-delay-ps nor tx-internal-delay-ps,
ytphy_get_delay_reg_value() falls back to the default 1950 ps values
and those are still written into CCR and RC1R, overwriting the chip
strap/OTP settings. With phy-mode "rgmii-rxid" this clears the RX
internal delay the MAC relies on, causing CRC errors, packet loss and
link training failures at gigabit speed until the link falls back to
100M.

The fallback only supplies a value and does not skip the register
write, so the check belongs in the caller. Since the hardware links
cleanly at gigabit with the chip defaults untouched, return early when
firmware provides neither delay property. Delays supplied via DT or
ACPI _DSD are still honored.

Since commit fffedfece2b4 ("net: phy: motorcomm: use device properties
for firmware tuning") switched the driver to the generic device
property API, this applies to both DT and ACPI platforms.

Fixes: a6e68f0f8769 ("net: phy: Add dts support for Motorcomm yt8521 gigabit ethernet phy")
Signed-off-by: cuiguoqi <cuiguoqi@kylinos.cn>
---
 drivers/net/phy/motorcomm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 5071605a1a11..a8b8ebbd4d6b 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -895,6 +895,18 @@ static int ytphy_rgmii_clk_delay_config(struct phy_device *phydev)
 	u16 mask, val = 0;
 	int ret;
 
+	/* When firmware provides no delay properties, keep the chip's
+	 * strap/OTP defaults untouched. Rewriting the delay registers here
+	 * would zero the RX internal delay that rgmii-rxid relies on,
+	 * which breaks gigabit links (CRC errors and intermittent link
+	 * training failures).
+	 */
+	if (!device_property_present(&phydev->mdio.dev,
+				     "rx-internal-delay-ps") &&
+	    !device_property_present(&phydev->mdio.dev,
+				     "tx-internal-delay-ps"))
+		return 0;
+
 	rx_reg = ytphy_get_delay_reg_value(phydev, "rx-internal-delay-ps",
 					   ytphy_rgmii_delays, tb_size,
 					   &rxc_dly_en,
-- 
2.25.1

Re: [PATCH] net: phy: motorcomm: keep chip delay defaults when firmware provides none

[Andrew Lunn]

On Tue, Sep 01, 2026 at 02:19:15PM +0800, guoqi0226 wrote:
> From: cuiguoqi <cuiguoqi@kylinos.cn>
> 
> ytphy_rgmii_clk_delay_config() unconditionally programs the YT8521
> RGMII delay registers. When firmware describes neither
> rx-internal-delay-ps nor tx-internal-delay-ps,
> ytphy_get_delay_reg_value() falls back to the default 1950 ps values
> and those are still written into CCR and RC1R, overwriting the chip
> strap/OTP settings. With phy-mode "rgmii-rxid" this clears the RX
> internal delay the MAC relies on, causing CRC errors, packet loss and
> link training failures at gigabit speed until the link falls back to
> 100M.
> 
> The fallback only supplies a value and does not skip the register
> write, so the check belongs in the caller. Since the hardware links
> cleanly at gigabit with the chip defaults untouched, return early when
> firmware provides neither delay property. Delays supplied via DT or
> ACPI _DSD are still honored.
> 
> Since commit fffedfece2b4 ("net: phy: motorcomm: use device properties
> for firmware tuning") switched the driver to the generic device
> property API, this applies to both DT and ACPI platforms.

This seems wrong.

phydev->interface describes the basic actions.
PHY_INTERFACE_MODE_RGMII means no delays are added by the
PHY. PHY_INTERFACE_MODE_RGMII_ID means delays are added for both RX
and TX, by PHY.

rx-internal-delay-ps and tx-internal-delay-ps are about how big the
delays are. The RGMII standard says the delays should be 2ns, so if
these properties are not present, if delays should be added, 2ns
should be added.

The chips strapping does not matter, the PHY should do what it is told
to do, via phydev->interface.

So, does your PCB have extra long clock lines? That decides on the
phy-mode value, you are describing the hardware.

https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287

You say "rgmii-rxid". That would be a very odd value, it means you
have an extra long clock line for one, and a regular clock line for
the other. It does happen, but it is very unusual.

If you really want to have the PHY not touch the delay configuration:

/**
 * enum phy_interface_t - Interface Mode definitions
 *
 * @PHY_INTERFACE_MODE_NA: Not Applicable - don't touch

and you need to patch ytphy_rgmii_clk_delay_config() to support this.

    Andrew

---
pw-bot: cr