netdev - Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

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: <ba7b290d-0cd1-4809-822a-bfe902684d7e@lunn.ch>
Date: Thu, 5 Jun 2025 15:48:37 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Icenowy Zheng <uwu@...nowy.me>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
	Rob Herring <robh@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Chaoyi Chen <chaoyi.chen@...k-chips.com>,
	Matthias Schiffer <matthias.schiffer@...tq-group.com>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add
 informative text about RGMII delays

On Thu, Jun 05, 2025 at 06:51:43PM +0800, Icenowy Zheng wrote:
> 在 2025-06-05星期四的 10:41 +0100，Russell King (Oracle)写道：
> > On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> > > In addition, analyzing existing Ethernet drivers, I found two
> > > drivers
> > > with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> > > ti/icssg/icssg_prueth.c .
> > > 
> > > The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode
> > > is
> > > rgmii or rgmii-rxid, and the PRU ETH driver, which works on some
> > > MAC
> > > with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
> > > rgmii-id or rgmii-txid to remove the txid part.
> > 
> > No, this is wrong.
> > 
> > First, it does not reject any RGMII mode. See qcom_ethqos_probe() and
> > the switch() in there. All four RGMII modes are accepted.
> 
> Well my sentence have its subject switched here. I mean the TI PRU ETH
> driver is rejecting modes.

Which is theoretically fine. I've not looked at this driver in
particular, but there are some MACs were you cannot disable the delay.
The MAC always imposes 2ns delay. That would mean a PCB which also has
extra long clock lines is simply FUBAR, cannot work, and 'rgmii' is
invalid, so reject it.

> Well I am not sure, considering two examples I raised here (please note
> I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
> contrary handling of RGMII modes, and one matches the old binding
> document, one matches the new one).

Nope, i fully agree with Russell, the binding has not changed, just the
words to explain the binding.

Just for a minute, consider your interpretation of the old text is
wrong. Read the old text again and again, and see if you can find an
interpretation which is the same as the new text. If you do:

* It proves our point that describing what this means is hard, and
  developers will get it wrong.

* There is an interpretation of both the old and new where nothing
  changed.

* You have to be careful looking at drivers, because some percent of
  developers also interpreted it wrongly, and have broken
  implementations as a result.  You cannot say the binding means X,
  not Y, because there is a driver using meaning X.

My hope with the new text is that it focuses on hardware, which is
what DT is about. You can look at the schematic, see if there is extra
long clock lines or not, and then decided on 'rgmii-id' if there are
not, and 'rgmii' is there are. The rest then follows from that.

And if you look at the questions i've been asking for the last year or
more, i always start with, "Does the PCB have extra long clock
lines?".

> > The RGMII modes have been documented in
> > Documentation/networking/phy.rst
> > (Documentation/networking/phy.txt predating) since:
> 
> I checked the document here, and it seems that it's against the changed
> binding document (it matches the original one):
> 
> The phy.rst document says:
> ```
> * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting
> any
>   internal delay by itself, it assumes that either the Ethernet MAC (if
> capable)
>   or the PCB traces insert the correct 1.5-2ns delay
> ```
> 
> The changed binding document says:

You are not reading it carefully enough. The binding describes
hardware, the board. phy.rst describes the phylib interface. They are
different.

	Andrew