Message-ID: <b53fba84c8435859a40288f3a12db40685b8863a.camel@ew.tq-group.com>
Date: Wed, 16 Apr 2025 09:41:57 +0200
From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
To: Siddharth Vadapalli <s-vadapalli@...com>
Cc: 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>, 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, 2025-04-15 at 17:25 +0530, Siddharth Vadapalli wrote:
> 
> On Tue, Apr 15, 2025 at 01:28:48PM +0200, Matthias Schiffer wrote:
> > On Tue, 2025-04-15 at 16:06 +0530, Siddharth Vadapalli wrote:
> > > 
> > > On Tue, Apr 15, 2025 at 12:18:01PM +0200, Matthias Schiffer wrote:
> > > > As discussed [1], the comments for the different rgmii(-*id) modes do not
> > > > accurately describe what these values mean.
> > > > 
> > > > As the Device Tree is primarily supposed to describe the hardware and not
> > > > its configuration, the different modes need to distinguish board designs
> > > 
> > > If the Ethernet-Controller (MAC) is integrated in an SoC (as is the case
> > > with CPSW Ethernet Switch), and, given that "phy-mode" is a property
> > > added within the device-tree node of the MAC, I fail to understand how
> > > the device-tree can continue "describing" hardware for different board
> > > designs using the same SoC (unchanged MAC HW).
> > 
> > The setting is part of the MAC node, but it is always set in the board DTS,
> > together with assigning a PHY to the MAC.
> 
> The MAC is the same independent of which board it is used in. So are we
> really describing the "MAC" or configuring the "MAC"? Isn't it the PHY
> along with the PCB lines on a given board that determine how the "MAC"
> should be "configured" to make the combination of "MAC" + "PHY"
> functional together?
> 
> > 
> > > How do we handle situations where a given MAC supports various
> > > "phy-modes" in HW? Shouldn't "phy-modes" then be a "list" to technically
> > > descibe the HW? Even if we set aside the "rgmii" variants that this
> > > series is attempting to address, the CPSW MAC supports "sgmii", "qsgmii"
> > > and "usxgmii/xfi" as well.
> > 
> > This is not about PHY mode support of the MAC, but the mode to be used on a
> > particular board. I would not expect a board to use multiple different
> > interfaces with a single PHY (and if such cases exist, I consider them out of
> 
> For a fixed PHY, the MAC will be "configured" to operate in a set of
> modes supported by the PHY. The HW description is coming from the PHY
> that has been "fixed", and not the MAC. But the "phy-mode" property
> resides within the device-tree node of the MAC and not the PHY. So are
> we still "describing" the MAC when it is the "PHY" that introduces the
> limitation or requires the MAC to be configured for a particular
> "phy-mode"?

The phy-mode property does not describe the MAC, but how MAC and PHY are
connected. The MAC node just happens to be where this information is placed in
the Device Tree (Using graph nodes to describe the connection between MAC and
PHY seems like overkill...)

Also note that (as I understand it) I'm not changing anything, I'm updating the
documentation to reflect what has been the intended behavior already. Please see
the previous discussion with Andrew that I linked, where he convinced me that
this is the correct approach.

> 
> > scope for this patch series).
> > 
> > > 
> > > > (if a delay is built into the PCB using different trace lengths); whether
> > > > a delay is added on the MAC or the PHY side when needed should not matter.
> > > > 
> > > > Unfortunately, implementation in MAC drivers is somewhat inconsistent
> > > > where a delay is fixed or configurable on the MAC side. As a first step
> > > > towards sorting this out, improve the documentation.
> 
> While this patch is improving the documentation and making it consistent
> when it comes to the description of "rgmii" by stating that the "MAC"
> shouldn't add a delay, for the remaining cases, as to who adds the delay
> and whether or not the MAC should add a delay has been left open.
> Existing documentation clarifies what the MAC should do for each case
> except "rgmii" which is being fixed by your patch.

Andrew specifically asked to leave it open in the DT bindings whether MAC or PHY
add the delay, and it might differ between drivers (and different operating
systems using the same Device Tree).

Whether the MAC should add a required delay in cases where it's configurable is
an interesting question - not one of the Device Tree bindings, but of driver
implementation.

On Linux, there currently isn't a way for the MAC driver to query from the PHY
whether it could include the delays itself. My assumption is that most PHYs
either don't have internal delays, or the delays are configurable. If this is
the case, having the MAC add them in internal-delay modes and not adding them on
the PHY side would be the best default (also for PHY-less/fixed-link setups,
which should be handled like a PHY without internal delay capabilities.)

@Andrew, does the above seem correct to you?

Best,
Matthias


> 
> > > > 
> > > > Link: https://lore.kernel.org/lkml/d25b1447-c28b-4998-b238-92672434dc28@lunn.ch/ [1]
> > > > Signed-off-by: Matthias Schiffer <matthias.schiffer@...tq-group.com>
> > > > ---
> > > >  .../bindings/net/ethernet-controller.yaml        | 16 +++++++++-------
> > > >  1 file changed, 9 insertions(+), 7 deletions(-)
> > > > 
> > > > 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.
> 
> [...]
> 
> Regards,
> Siddharth.

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Powered by blists - more mailing lists