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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251030022509.267938-1-cong.yi@linux.dev>
Date: Thu, 30 Oct 2025 10:25:09 +0800
From: Yi Cong <cong.yi@...ux.dev>
To: andrew@...n.ch
Cc: Frank.Sae@...or-comm.com,
	cong.yi@...ux.dev,
	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 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.

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.

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.

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.

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.
2. When no DTS configuration is available, the default values
   do not match the chip manual
    —— this is the issue I'm currently fixing.
3. Regardless of whether the configuration comes from DTS, ACPI, or defaults,
   the driver overwrites any BIOS settings.
    —— In similar past cases, I would first check if the register holds its
     default value; if not, it indicates a deliberate configuration, and
     the driver should leave it unchanged.

Issues 1 and 3 will be addressed as my project progresses,
including development, testing, and validation.
Currently, issue 2 has been verified as effective with the present patch,
and has also been confirmed by Motorcomm.

Thank you again! Please feel free to raise any further questions,
I will continue to communicate promptly with Motorcomm.

Regards,
    Yi Cong