Message-ID: <271c15a45f41a110416f65d1f8a44b896aa01e33.camel@ew.tq-group.com>
Date: Wed, 07 May 2025 09:17:45 +0200
From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
To: Andrew Lunn <andrew@...n.ch>, Rob Herring <robh@...nel.org>
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>,  Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Chaoyi Chen
 <chaoyi.chen@...k-chips.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

On Wed, 2025-04-30 at 11:21 -0500, Andrew Lunn wrote:
> ********************
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
> Attention external email: Open attachments and links only if you know that they are from a secure source and are safe. In doubt forward the email to the IT-Helpdesk to check it.
> ********************
> 
> Device Tree and Ethernet MAC driver writers often misunderstand RGMII
> delays. Rewrite the Normative section in terms of the PCB, is the PCB
> adding the 2ns delay. This meaning was previous implied by the
> definition, but often wrongly interpreted due to the ambiguous wording
> and looking at the definition from the wrong perspective. The new
> definition concentrates clearly on the hardware, and should be less
> ambiguous.
> 
> Add an Informative section to the end of the binding describing in
> detail what the four RGMII delays mean. This expands on just the PCB
> meaning, adding in the implications for the MAC and PHY.
> 
> Additionally, when the MAC or PHY needs to add a delay, which is
> software configuration, describe how Linux does this, in the hope of
> reducing errors. Make it clear other users of device tree binding may
> implement the software configuration in other ways while still
> conforming to the binding.
> 
> Fixes: 9d3de3c58347 ("dt-bindings: net: Add YAML schemas for the generic Ethernet options")
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
> Changes in v2:
> Reword Normative section
> manor->manner
> add when using phylib/phylink
> request details in the commit message and .dts comments
> clarify PHY -internal-delay-ps values being depending on rgmii-X mode.
> Link to v1: https://lore.kernel.org/r/20250429-v6-15-rc3-net-rgmii-delays-v1-1-f52664945741@lunn.ch
> ---
>  .../bindings/net/ethernet-controller.yaml          | 97 ++++++++++++++++++++--
>  1 file changed, 90 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index 45819b2358002bc75e876eddb4b2ca18017c04bd..a2d4c626f659a57fc7dcd39301f322c28afed69d 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -74,19 +74,17 @@ properties:
>        - rev-rmii
>        - moca
>  
> -      # RX and TX delays are added by the MAC when required
> +      # RX and TX delays are provided by the PCB. See below
>        - 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
> +      # RX and TX delays are not provided by the PCB. This is the most
> +      # frequent case. See below
>        - rgmii-id
>  
> -      # RGMII with internal RX delay provided by the PHY, the MAC
> -      # should not add an RX delay in this case
> +      # TX delay is provided by the PCB. See below
>        - rgmii-rxid
>  
> -      # RGMII with internal TX delay provided by the PHY, the MAC
> -      # should not add an TX delay in this case
> +      # RX delay is provided by the PCB. See below
>        - rgmii-txid
>        - rtbi
>        - smii
> @@ -286,4 +284,89 @@ allOf:
>  
>  additionalProperties: true
>  
> +# Informative
> +# ===========
> +#
> +# 'phy-modes' & 'phy-connection-type' properties 'rgmii', 'rgmii-id',
> +# 'rgmii-rxid', and 'rgmii-txid' are frequently used wrongly by
> +# developers. This informative section clarifies their usage.
> +#
> +# The RGMII specification requires a 2ns delay between the data and
> +# clock signals on the RGMII bus. How this delay is implemented is not
> +# specified.
> +#
> +# One option is to make the clock traces on the PCB longer than the
> +# data traces. A sufficiently difference in length can provide the 2ns
> +# delay. If both the RX and TX delays are implemented in this manner,
> +# 'rgmii' should be used, so indicating the PCB adds the delays.
> +#
> +# If the PCB does not add these delays via extra long traces,
> +# 'rgmii-id' should be used. Here, 'id' refers to 'internal delay',
> +# where either the MAC or PHY adds the delay.
> +#
> +# If only one of the two delays are implemented via extra long clock
> +# lines, either 'rgmii-rxid' or 'rgmii-txid' should be used,
> +# indicating the MAC or PHY should implement one of the delays
> +# internally, while the PCB implements the other delay.
> +#
> +# Device Tree describes hardware, and in this case, it describes the
> +# PCB between the MAC and the PHY, if the PCB implements delays or
> +# not.
> +#
> +# In practice, very few PCBs make use of extra long clock lines. Hence
> +# any RGMII phy mode other than 'rgmii-id' is probably wrong, and is
> +# unlikely to be accepted during review without details provided in
> +# the commit description and comments in the .dts file.
> +#
> +# When the PCB does not implement the delays, the MAC or PHY must.  As
> +# such, this is software configuration, and so not described in Device
> +# Tree.
> +#
> +# The following describes how Linux implements the configuration of
> +# the MAC and PHY to add these delays when the PCB does not. As stated
> +# above, developers often get this wrong, and the aim of this section
> +# is reduce the frequency of these errors by Linux developers. Other
> +# users of the Device Tree may implement it differently, and still be
> +# consistent with both the normative and informative description
> +# above.
> +#
> +# By default in Linux, when using phylib/phylink, the MAC is expected
> +# to read the 'phy-mode' from Device Tree, not implement any delays,
> +# and pass the value to the PHY. The PHY will then implement delays as
> +# specified by the 'phy-mode'. The PHY should always be reconfigured
> +# to implement the needed delays, replacing any setting performed by
> +# strapping or the bootloader, etc.
> +#
> +# Experience to date is that all PHYs which implement RGMII also
> +# implement the ability to add or not add the needed delays. Hence
> +# this default is expected to work in all cases. Ignoring this default
> +# is likely to be questioned by Reviews, and require a strong argument
> +# to be accepted.
> +#
> +# 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
> +# cannot be supported. When the MAC implements the delay, it must
> +# ensure that the PHY does not also implement the same delay. So it
> +# must modify the phy-mode it passes to the PHY, removing the delay it
> +# has added. Failure to remove the delay will result in a
> +# non-functioning link.
> +#
> +# Sometimes there is a need to fine tune the delays. Often the MAC or
> +# PHY can perform this fine tuning. In the MAC node, the Device Tree
> +# properties 'rx-internal-delay-ps' and 'tx-internal-delay-ps' should
> +# be used to indicate fine tuning performed by the MAC. The values
> +# expected here are small. A value of 2000ps, i.e 2ns, and a phy-mode
> +# of 'rgmii' will not be accepted by Reviewers.
> +#
> +# If the PHY is to perform fine tuning, the properties
> +# 'rx-internal-delay-ps' and 'tx-internal-delay-ps' in the PHY node
> +# should be used. When the PHY is implementing delays, e.g. 'rgmii-id'
> +# these properties should have a value near to 2000ps. If the PCB is
> +# implementing delays, e.g. 'rgmii', a small value can be used to fine
> +# tune the delay added by the PCB.

Sorry for not having a look at this earlier, I got busy with something else.

This section doesn't really make sense to me. It is described what values
*-internal-delay-ps should have /when the PHY is implementing delays/ - but at
the same time the above description leaves it open whether the MAC or the PHY
should implement the delays in rgmii-id mode, so the finetuning setting would
break the delays completely if another OS decides to have the MAC instead of the
PHY add the delay by default.

Best,
Matthias


>  ...
> 
> ---
> base-commit: d4cb1ecc22908ef46f2885ee2978a4f22e90f365
> change-id: 20250429-v6-15-rc3-net-rgmii-delays-8a00c4788fa7
> 
> Best regards,

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