Message-ID: <debcb2e1-b7ef-493b-a4c4-e13d4aaf0223@lunn.ch>
Date: Wed, 4 Jun 2025 14:23:08 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Icenowy Zheng <uwu@...nowy.me>
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

> > -      # 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.

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'

> 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.

> > +#
> > +# 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