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: <5b97f70c-45c3-41c2-ba7b-d383864f0003@lunn.ch>
Date: Thu, 30 Oct 2025 13:57:58 +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

On Thu, Oct 30, 2025 at 10:25:09AM +0800, Yi Cong wrote:
> On Wed, 29 Oct 2025 13:07:35 +0100, Andrew Lunn <andrew@...n.ch> wrote:
> >
> > On Wed, Oct 29, 2025 at 11:00:42AM +0800, Yi Cong wrote:
> > > From: Yi Cong <yicong@...inos.cn>
> > >
> > > According to the dataSheet, rx delay default value is set to 0.
> >
> > You need to be careful here, or you will break working boards. Please
> > add to the commit message why this is safe.
> >
> > Also, motorcomm,yt8xxx.yaml says:
> >
> >   rx-internal-delay-ps:
> >     description: |
> >       RGMII RX Clock Delay used only when PHY operates in RGMII mode with
> >       internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
> >     enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650,
> >             1800, 1900, 1950, 2050, 2100, 2200, 2250, 2350, 2500, 2650, 2800,
> >             2950, 3100, 3250, 3400, 3550, 3700, 3850, 4000, 4150 ]
> >     default: 1950
> 
> Hi, Andrew, thanks for your reply!
> 
> Let me add the following information:
> 
> The chip documentation I have for the YT8521 and YT8531S:
> "YT8521SH/YT8521SC Application Note, Version v1.7, Release Date: January 3, 2024"
> "YT8531SH/YT8531SC Application Note, Version v1.2, Release Date: November 21, 2023"
> 
> Both documents specify the RGMII delay configuration as follows:
> The RX delay value can be set via Ext Reg 0xA003[13:10], where each
> increment of 1 adds 150ps. After power-on, the default value of
> bits [13:10] is 0.
> The TX delay value can be set via Ext Reg 0xA003[3:0], with the
> default value of bits [3:0] being 1 after power-on.
> 
> I reviewed the commit history of this driver code. When YT8521 support
> was initially added, the code configuration matched the chip manual:
> 70479a40954c ("net: phy: Add driver for Motorcomm yt8521 gigabit ethernet phy")
> 
> However, later when DTS support was added:
> a6e68f0f8769 ("net: phy: Add dts support for Motorcomm yt8521 gigabit ethernet phy")
> the default values were changed to 1.950ns.

Read carefully...

> >       RGMII RX Clock Delay used only when PHY operates in RGMII mode with
> >       internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.

What do rgmii-id and rgmii-rxid mean?

> Indeed, the RGMII standard specifies that the clock signal should be
> delayed by 1-2ns relative to the data signals to ensure proper setup/hold
> timing, which is likely the origin of the 1.950ns value in the YAML and
> current code.

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

> More importantly, the current Motorcomm driver's delay configuration logic
> is incomplete. In the projects I've worked on, some configurations are
> obtained from DTS, some from ACPI, but many manufacturers prefer to directly
> set the delay values in BIOS based on their hardware design.

Which is somewhat fine. If you want to the driver to not touch the PHY
configuration you use PHY_INTERFACE_MODE_NA

https://elixir.bootlin.com/linux/v6.16.5/source/include/linux/phy.h#L72

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.

> In fact, Motorcomm's Linux 5.4 driver versions guided PC motherboard
> manufacturers to configure the delay values through BIOS, and the driver
> code did not touch the delay registers (Ext Reg 0xA003). This means that
> upgrading to a newer kernel version, where the driver writes 1.950ns
> by default, could cause communication failures.

Only if the MAC driver requests the PHY to adds delays.

> To summarize, the current issues with the Motorcomm driver are:
> 1. It only supports configuration via DTS, not via ACPI
>     —— I may implement this myself or coordinate with Motorcomm
>        in the future.

FYI:

https://elixir.bootlin.com/linux/v6.16.5/source/Documentation/firmware-guide/acpi/dsd/phy.rst

You need to work within this framework.

    Andrew