Re: [PATCH net-next 1/4] dt-bindings: net: ethernet-controller: update descriptions of RGMII modes

[Siddharth Vadapalli <s-vadapalli@ti.com>]

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

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

> > > 
> > > Link: https://lore.kernel.org/lkml/d25b1447-c28b-4998-b238-92672434dc28@lunn.ch/ [1]
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.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.