Message-ID: <2e42f2f7985fb036bec6ab085432a49961c8dc42.camel@icenowy.me>
Date: Thu, 05 Jun 2025 17:06:43 +0800
From: Icenowy Zheng <uwu@...nowy.me>
To: Andrew Lunn <andrew@...n.ch>
Cc: 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>, "Russell King (Oracle)"
 <linux@...linux.org.uk>,  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

在 2025-06-04星期三的 14:23 +0200，Andrew Lunn写道：
> > > -      # RX and TX delays are added by the MAC when required
> > > +      # RX and TX delays are provided by the PCB. See below
> > 
> > This really sounds like a breaking change that changes the meaning
> > of
> > the definition of this item instead of simply rewording.
> > 
> > Everything written according to the original description is broken
> > by
> > this change.
> 
> Please give some examples. What has broken, which was not already
> broken. There has been a lot of discussion about this over the last
> year, so please do some careful research about what has been said,
> and
> try not to repeat past discussion.

Yes, I saw many related discussions.

I have the same question with [1], what's the answer?

[1]
https://lore.kernel.org/netdev/271c15a45f41a110416f65d1f8a44b896aa01e33.camel@ew.tq-group.com/

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.

The logic of QCOM ETHQOS clearly follows the original DT binding, which
describes "rgmii-id" as `RGMII with internal RX and TX delays provided
by the PHY, the MAC should not add the RX or TX delays in this case`
(the driver skips the delay for rgmii-id). The logic of PRU ETH follows
the logic of the new DT binding. This shows that the DT binding patch
is not a simple clarification, but a change of meanings.

> 
> The whole point of this change is this is often wrongly interpreted,
> and there are a lot of broken .dts files. By including a lot of text,
> explaining both the pure OS agnostic DT meaning, and how Linux
> systems
> should implement it, i hope i have made it less ambiguous.
> 
> > Although these PHYs are able to implement (or not to implement) the
> > delay, it's not promised that this could be overriden by the kernel
> > instead of being set up as strap pins.
> 
> If you want the kernel to not touch the PHY, use
> 
> phy-mode = 'internal'

This sounds weird, and may introduce side effect on the MAC side.

Well we might need to allow PHY to have phy-mode property in addition
to MAC, in this case MAC phy-mode='rgmii*' and PHY phy-mode='internal'
might work?

> 
> > In addition, the Linux kernel contains a "Generic PHY" driver for
> > any
> > 802.1 c22 PHYs to work, without setting any delays.
> 
> genphy is best effort, cross your fingers, it might work if you are
> luckily. Given the increasing complexity of PHYs, it is becoming less
> and less likely to work. From a Maintainers perspective, i only care
> if the system works with the proper PHY driver for the
> hardware. Anything else is unmaintainable.

Well this sounds unfortunate but reasonable.

> 
> > > +#
> > > +# There are a small number of cases where the MAC has hard coded
> > > +# delays which cannot be disabled. The 'phy-mode' only describes
> > > the
> > > +# PCB.  The inability to disable the delays in the MAC does not
> > > change
> > > +# the meaning of 'phy-mode'. It does however mean that a 'phy-
> > > mode'
> > > of
> > > +# 'rgmii' is now invalid, it cannot be supported, since both the
> > > PCB
> > > +# and the MAC and PHY adding delays cannot result in a
> > > functional
> > > +# link. Thus the MAC should report a fatal error for any modes
> > > which
> > 
> > Considering compatibilty, should this be just a warning (which
> > usually
> > means a wrong phy-mode setup) instead of a fatal error?
> 
> As i said, there are a large number of broken DT blobs. In order to
> fix them, but not break backwards compatibility, some MAC and PHY
> drivers are going to have to check the strapping/bootloader
> configuration and issue a warning if phy-mode seems wrong, telling
> the
> user to update there DT blob. So, yes it is just a warning for
> systems
> that are currently broken, but i would consider it an error for
> correctly implemented systems.
> 
>         Andrew

Powered by blists - more mailing lists