Message-ID: <aAe2NFFrcXDice2Z@shell.armlinux.org.uk>
Date: Tue, 22 Apr 2025 16:31:00 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Matthias Schiffer <matthias.schiffer@...tq-group.com>,
	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>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Andy Whitcroft <apw@...onical.com>,
	Dwaipayan Ray <dwaipayanray1@...il.com>,
	Lukas Bulwahn <lukas.bulwahn@...il.com>,
	Joe Perches <joe@...ches.com>, Jonathan Corbet <corbet@....net>,
	Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
	Siddharth Vadapalli <s-vadapalli@...com>,
	Roger Quadros <rogerq@...nel.org>, Tero Kristo <kristo@...nel.org>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux@...tq-group.com
Subject: Re: [PATCH net-next 1/4] dt-bindings: net: ethernet-controller:
 update descriptions of RGMII modes

On Tue, Apr 22, 2025 at 05:00:37PM +0200, Andrew Lunn wrote:
> On Mon, Apr 21, 2025 at 08:20:29PM +0100, Russell King (Oracle) wrote:
> > On Tue, Apr 15, 2025 at 12:18:01PM +0200, Matthias Schiffer wrote:
> > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > index 45819b2358002..2ddc1ce2439a6 100644
> > > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > @@ -74,19 +74,21 @@ properties:
> > >        - rev-rmii
> > >        - moca
> > >  
> > > -      # RX and TX delays are added by the MAC when required
> > > +      # RX and TX delays are part of the board design (through PCB traces). MAC
> > > +      # and PHY must not add delays.
> > >        - rgmii
> > >  
> > > -      # RGMII with internal RX and TX delays provided by the PHY,
> > > -      # the MAC should not add the RX or TX delays in this case
> > > +      # RGMII with internal RX and TX delays provided by the MAC or PHY. No
> > > +      # delays are included in the board design; this is the most common case
> > > +      # in modern designs.
> > >        - rgmii-id
> > >  
> > > -      # RGMII with internal RX delay provided by the PHY, the MAC
> > > -      # should not add an RX delay in this case
> > > +      # RGMII with internal RX delay provided by the MAC or PHY. TX delay is
> > > +      # part of the board design.
> > >        - rgmii-rxid
> > >  
> > > -      # RGMII with internal TX delay provided by the PHY, the MAC
> > > -      # should not add an TX delay in this case
> > > +      # RGMII with internal TX delay provided by the MAC or PHY. RX delay is
> > > +      # part of the board design.
> > >        - rgmii-txid
> > >        - rtbi
> > >        - smii
> > 
> > Sorry, but I don't think this wording improves the situation - in fact,
> > I think it makes the whole thing way more confusing.
> > 
> > Scenario 1: I'm a network device driver author. I read the above, Okay,
> > I have a RGMII interface, but I need delays to be added. I'll detect
> > when RGMII-ID is used, and cause the MAC driver to add the delays, but
> > still pass PHY_INTERFACE_MODE_RGMII_ID to phylib.
> > 
> > Scenario 2: I'm writing a DT file for a board. Hmm, so if I specify
> > rgmii because the delays are implemented in the traces, but I need to
> > fine-tune them. However, the documentation says that delays must not
> > be added by the MAC or the PHY so how do I adjust them. I know, I'll
> > use rgmii-id because that allows delays!
> > 
> > I suspect neither of these two are really want you mean, but given
> > this wording, that's exactly where it leads - which is more
> > confusion and less proper understanding.
> 
> These DT documents are supposed to be normative and OS agnostic. I
> wounder what the DT Maintainers will say if we add an Informative
> section afterwards giving a detailed description of how Linux actually
> implements these normative statements? It will need to open with a
> clear statement that it is describing Linux behaviour, and other OSes
> can implement the normative part in other ways and still be compliant,
> but that Linux has seen a lot of broken implementations and so wants
> to add Informative information to guide Linux developers.

Well, looking at ePAPR, the only thing that was defined back then was
the presence of a property to describe the interface type between the
ethernet device and PHY. The values were left to the implementation
to decide upon, but with some recommendations.

What that means is that the values to this property are not part of
the DT standard, but are a matter for the implementation.

However, with the yaml stuff, if that is basically becoming "DT
specification" then it needs to be clearly defined what each value
actually means for the system, and not this vague airy-fairy thing
we have now.

We've learnt the hard way in the kernel where that gets us with
the number of back-compat breaking changes we've had to make where
the RGMII delays have been totally wrongly interpreted and leaving
it vague means that other implementations will suffer the same pain.

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