Message-ID: <aElArNHIwm1--GUn@shell.armlinux.org.uk>
Date: Wed, 11 Jun 2025 09:39:08 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Icenowy Zheng <uwu@...nowy.me>
Cc: Andrew Lunn <andrew@...n.ch>, 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 Wed, Jun 11, 2025 at 04:03:11PM +0800, Icenowy Zheng wrote:
> 在 2025-06-05星期四的 15:48 +0200，Andrew Lunn写道：
> > 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.
> 
> BTW I found that in some case the assumption of PHY-side delay being
> always better than MAC-side one is wrong -- modern MACs usually have
> adjustable delay line, but Realtek 8211-series PHYs have only on/off
> delay with a fixed 2ns value.

The only time that MACs may implement delays based on the
PHY_INTERFACE_MODE_RGMII* is if they also include code to pass
PHY_INTERFACE_MODE_RGMII (no suffixes) to phylink / phylib to ensure
that the PHY doesn't _also_ add delays. This isn't something we
encourage because it's more code, more review, and a different way
of implementing it - thus adding to maintainers workloads that are
already high enough.

> > 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.
> 
> Well I think "rgmii-*" shouldn't exist at all, if focusing on hardware.
> I prefer only "rgmii" with properties describing the delay numbers.

Yes, I think we as phylib maintainers have also come to the same
conclusion with all the hassle this causes, but we can't get rid
of this without breaking the kernel and breaking device-tree
compatibility. So, we're stuck with it.

> > You are not reading it carefully enough. The binding describes
> > hardware, the board. phy.rst describes the phylib interface. They are
> > different.
> 
> Well I can't find the reason of phy-mode being so designed except for
> leaky abstraction from phylib.

I have no idea what that sentence means, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists