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@xxxxxxxxxx>
> 
> 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