[PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

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

---
base-commit: d4cb1ecc22908ef46f2885ee2978a4f22e90f365
change-id: 20250429-v6-15-rc3-net-rgmii-delays-8a00c4788fa7

Best regards,
-- 
Andrew Lunn <andrew@lunn.ch>

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

On Wed, Apr 30, 2025 at 11:21:35AM -0500, Andrew Lunn wrote:
> 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@lunn.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

I'm not sure that "provided" is the correct word to describe what's
meant here (implemented might be better), but it's perfectly
understandable as-is and I don't think worth respinning or splitting
hairs over...

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 30 Apr 2025 11:21:35 -0500 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net,v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays
    https://git.kernel.org/netdev/net/c/c360eb0c3ccb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Matthias Schiffer]

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@lunn.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/

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-04-30星期三的 11:21 -0500，Andrew Lunn写道：
> 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.

Sorry for my late disturbance (so late that this patch is already
included in released version), but I have some questions about this...

> 
> Fixes: 9d3de3c58347 ("dt-bindings: net: Add YAML schemas for the
> generic Ethernet options")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> 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(-)
> 
> 
> ---
> base-commit: d4cb1ecc22908ef46f2885ee2978a4f22e90f365
> change-id: 20250429-v6-15-rc3-net-rgmii-delays-8a00c4788fa7
> 
> Best regards,
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-
> controller.yaml b/Documentation/devicetree/bindings/net/ethernet-
> controller.yaml
> index
> 45819b2358002bc75e876eddb4b2ca18017c04bd..a2d4c626f659a57fc7dcd39301f
> 322c28afed69d 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

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.

In addition, considering this is set as a property of the MAC device
tree node, it's more natural to represents the perspective of MAC,
instead of only describing what's on the PCB.

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

Usually, in this case, this isn't pure software configuration but
already described as '?x-internal-delay-ps' (or in some legacy cases,
'vendor,?x-delay-ps') property in the MAC.

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

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.

Take Realtek GbE PHYs, which I am most familiar with, as examples. The
Linux realtek netphy driver supports overriding delay configuration
only for RTL8211E/F. From Internet, RTL8211B/C/E/F datasheets could be
found. All of them contain RXDLY/TXDLY strap pins, but neither of them
mention any register to config the delays, which means that on
RTL8211B/C there's no known way to override them (the only known , and
on RTL8211E/F the way to override them are undocumented registers from
unknown origin (which means no support from Realtek themselves are
available). I do partly participate a dramatic case about RTL8211E:
there were some Pine A64+ boards with broken RTL8211E that have some
delay-chain related issues, and Pine64 only got magic numbers from
Realtek to fix them (the magic numbers happen to match current RTL8211E
delay configuration code in realtek driver and show that RXDLY is
overriden to disabled).

In addition, the Linux kernel contains a "Generic PHY" driver for any
802.1 c22 PHYs to work, without setting any delays. With this driver
it's impossible to let the PHY "always be reconfigured", and
enabling/disabling the PHY-specific driver may lead to behavior change
(when enabling the specific driver, the configuration is overriden;
when disabling it, the strap pin configuration is used). Well
personally I think the driver should give out a warning when it can
read out strip pin status and it does not match the DT configuration).

The DT binding (and phylib) currently also lacks any way to describe
"respect the hardware strapping delay configuration of PHY".

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

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

In case of the MAC implementing a '?x-internal-delay-ps' property, when
should the MAC stripping delays from phy-mode? Should the phy-mode be
changed when MAC delaying 100ps? Should it be changed when MAC delaying
1000ps? And if the PHY could be reprogrammed to do the delay and the
property contains a big value that is higher than 2000ps, should the
software switch to do the delay in PHY and decrease the MAC side delay
by 2000ps?

Besides this, do we have any existing MAC driver that implements
stripping the delay from passing to PHY? How should we maintain
compatibilty between a DT that does not consider MAC driver delay
stripping and a driver that considers this? DTs are expected to be
backward compatible, and if this problem could not be solved, we can
never implement this intended MAC delay stripping.

My personal idea here, which reflects the current practice of most
drivers/DTs I read and leads to no known breaking change, is to make
the "phy-mode" property of the MAC node represents how the MAC expects
external components (PCB+PHY), and, assuming no significant PCB delay,
just pass this value to PHY instead. The MAC side delay shouldn't be
considered and represented in this property, but extra properties, like
'?x-internal-delay-ps' should be used instead.

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

BTW the MAC might be very powerful on introducing delays, e.g. it can
have delay lines on data pins in addition to clock pins (in case of
data pins are delayed, this could be considered as a "negative clock
delay"), or some hardware might be able to do per-data-pin data delay
(to compensate HW trace length inconsistency). Should these be
considered by the standard binding, or should these be left to vendored
properties?

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

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

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

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-04星期三的 14:23 +0200，Andrew Lunn写道：
> > > -      # 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.

Yes, I saw many related discussions.

I have the same question with [1], what's the answer?

[1]
https://lore.kernel.org/netdev/271c15a45f41a110416f65d1f8a44b896aa01e33.camel@ew.tq-group.com/

In addition, analyzing existing Ethernet drivers, I found two drivers
with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
ti/icssg/icssg_prueth.c .

The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode is
rgmii or rgmii-rxid, and the PRU ETH driver, which works on some MAC
with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
rgmii-id or rgmii-txid to remove the txid part.

The logic of QCOM ETHQOS clearly follows the original DT binding, which
describes "rgmii-id" as `RGMII with internal RX and TX delays provided
by the PHY, the MAC should not add the RX or TX delays in this case`
(the driver skips the delay for rgmii-id). The logic of PRU ETH follows
the logic of the new DT binding. This shows that the DT binding patch
is not a simple clarification, but a change of meanings.

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

This sounds weird, and may introduce side effect on the MAC side.

Well we might need to allow PHY to have phy-mode property in addition
to MAC, in this case MAC phy-mode='rgmii*' and PHY phy-mode='internal'
might work?

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

Well this sounds unfortunate but reasonable.

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

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Russell King (Oracle)]

On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> In addition, analyzing existing Ethernet drivers, I found two drivers
> with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> ti/icssg/icssg_prueth.c .
> 
> The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode is
> rgmii or rgmii-rxid, and the PRU ETH driver, which works on some MAC
> with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
> rgmii-id or rgmii-txid to remove the txid part.

No, this is wrong.

First, it does not reject any RGMII mode. See qcom_ethqos_probe() and
the switch() in there. All four RGMII modes are accepted.

The code in ethqos_rgmii_macro_init() is the questionable bit, but
again, does _not_ do any rejection of any RGMII mode. It simply sets
the transmit clock phase shift according to the mode, and the only
way this can work is if the board does not provide the required delay.

This code was not reviewed by phylib maintainers, so has slipped
through the review process. It ought to be using the delay properties
to configure the MAC.

> The logic of QCOM ETHQOS clearly follows the original DT binding, which

Let's make this clear. "original DT binding" - no, nothing has
*actually* changed with the DT binding - the meaning of the RGMII
modes have not changed. The problem is one of interpretation, and
I can tell you from personal experience that getting stuff documented
so that everyone gets the same understanding is nigh on impossible.
People will pick holes, and deliberately interpret whatever is written
in ways that it isn't meant to - and the more words that are used the
more this happens.

The RGMII modes have been documented in Documentation/networking/phy.rst
(Documentation/networking/phy.txt predating) since:

commit bf8f6952a233f5084431b06f49dc0e1d8907969e
Author: Florian Fainelli <f.fainelli@gmail.com>
Date:   Sun Nov 27 18:45:14 2016 -0800

    Documentation: net: phy: Add blurb about RGMII

    RGMII is a recurring source of pain for people with Gigabit Ethernet
    hardware since it may require PHY driver and MAC driver level
    configuration hints. Document what are the expectations from PHYLIB and
    what options exist.

    Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
    Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

> describes "rgmii-id" as `RGMII with internal RX and TX delays provided
> by the PHY, the MAC should not add the RX or TX delays in this case`
> (the driver skips the delay for rgmii-id). The logic of PRU ETH follows
> the logic of the new DT binding. This shows that the DT binding patch
> is not a simple clarification, but a change of meanings.

Let me say again. Nothing has changed. There is no "old binding" or
"new binding". If you think there is, then it's down to
misinterpretation.

This is precisely why I've been opposed to documenting these properties
in the binding document _and_ Documentation/networking/phy.* because
keeping them both in sync is going to be a pain, leading to ambiguity
and misinterpretation.

> > If you want the kernel to not touch the PHY, use
> > 
> > phy-mode = 'internal'
> 
> This sounds weird, and may introduce side effect on the MAC side.
> 
> Well we might need to allow PHY to have phy-mode property in addition
> to MAC, in this case MAC phy-mode='rgmii*' and PHY phy-mode='internal'
> might work?

I'm not convinced that adding more possibilities to the problem (i.o.w.
the idea that phy=mode = "internal" can be used to avoid the delays
being messed with) is a good idea - not at this point, because as you
point out MACs (and PHYs) won't know that they need to be configured
for RGMII mode. "internal" doesn't state this, and if we do start doing
this, we'll end up with "internal" selecting RGMII mode which may work
for some platforms but not all.

So, IMHO this is a bad idea.

> > > 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.
> 
> Well this sounds unfortunate but reasonable.

We're already in this state with PHYs faster than gigabit, because
IEEE 802.3 in their wisdom did not define where the 1000BASE-T
autoneg parameters appear in the register space. As a result, vendors
have done their own thing, and every vendor / PHY is different.
Without access to this key data, phylib has no way to know the
negotiation results. Thus, a generic PHY driver that works correctly
for PHYs > 1G just isn't possible.

I expect that in years to come, we'll see IEEE 802.3 updated with
the 1G registers for Clause 45 PHYs, but the boat has already sailed
so this would be totally pointless as there will be too many PHYs
out there doing their own thing for whatever IEEE 802.3 says about
this to have any relevence what so ever. Just like they did with
2500BASE-X, which is a similar mess due to IEEE 802.3 being way too
late.

I hope that there isn't going to be more of this, because each time
it happens, the IEEE 802.3 "standard" less relevant.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-05星期四的 10:41 +0100，Russell King (Oracle)写道：
> On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> > In addition, analyzing existing Ethernet drivers, I found two
> > drivers
> > with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> > ti/icssg/icssg_prueth.c .
> > 
> > The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode
> > is
> > rgmii or rgmii-rxid, and the PRU ETH driver, which works on some
> > MAC
> > with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
> > rgmii-id or rgmii-txid to remove the txid part.
> 
> No, this is wrong.
> 
> First, it does not reject any RGMII mode. See qcom_ethqos_probe() and
> the switch() in there. All four RGMII modes are accepted.

Well my sentence have its subject switched here. I mean the TI PRU ETH
driver is rejecting modes.

> 
> The code in ethqos_rgmii_macro_init() is the questionable bit, but
> again, does _not_ do any rejection of any RGMII mode. It simply sets
> the transmit clock phase shift according to the mode, and the only
> way this can work is if the board does not provide the required
> delay.
> 
> This code was not reviewed by phylib maintainers, so has slipped
> through the review process. It ought to be using the delay properties
> to configure the MAC.
> 
> > The logic of QCOM ETHQOS clearly follows the original DT binding,
> > which
> 
> Let's make this clear. "original DT binding" - no, nothing has
> *actually* changed with the DT binding - the meaning of the RGMII
> modes have not changed. The problem is one of interpretation, and
> I can tell you from personal experience that getting stuff documented
> so that everyone gets the same understanding is nigh on impossible.
> People will pick holes, and deliberately interpret whatever is
> written
> in ways that it isn't meant to - and the more words that are used the
> more this happens.

Well I am not sure, considering two examples I raised here (please note
I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
contrary handling of RGMII modes, and one matches the old binding
document, one matches the new one).

> 
> The RGMII modes have been documented in
> Documentation/networking/phy.rst
> (Documentation/networking/phy.txt predating) since:

I checked the document here, and it seems that it's against the changed
binding document (it matches the original one):

The phy.rst document says:
```
* PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting
any
  internal delay by itself, it assumes that either the Ethernet MAC (if
capable)
  or the PCB traces insert the correct 1.5-2ns delay
```

The changed binding document says:
```
# 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.
```

In the case of MAC inserting delays, the phy.rst document assumes it's
PHY_INTERFACE_MODE_RGMII but the changed binding document assumes it's
'rgmii-id'.

> commit bf8f6952a233f5084431b06f49dc0e1d8907969e
> Author: Florian Fainelli <f.fainelli@gmail.com>
> Date:   Sun Nov 27 18:45:14 2016 -0800
> 
>     Documentation: net: phy: Add blurb about RGMII
> 
>     RGMII is a recurring source of pain for people with Gigabit
> Ethernet
>     hardware since it may require PHY driver and MAC driver level
>     configuration hints. Document what are the expectations from
> PHYLIB and
>     what options exist.
> 
>     Reviewed-by: Martin Blumenstingl
> <martin.blumenstingl@googlemail.com>
>     Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> > describes "rgmii-id" as `RGMII with internal RX and TX delays
> > provided
> > by the PHY, the MAC should not add the RX or TX delays in this
> > case`
> > (the driver skips the delay for rgmii-id). The logic of PRU ETH
> > follows
> > the logic of the new DT binding. This shows that the DT binding
> > patch
> > is not a simple clarification, but a change of meanings.
> 
> Let me say again. Nothing has changed. There is no "old binding" or
> "new binding". If you think there is, then it's down to
> misinterpretation.
> 
> This is precisely why I've been opposed to documenting these
> properties
> in the binding document _and_ Documentation/networking/phy.* because
> keeping them both in sync is going to be a pain, leading to ambiguity
> and misinterpretation.
> 
> > > If you want the kernel to not touch the PHY, use
> > > 
> > > phy-mode = 'internal'
> > 
> > This sounds weird, and may introduce side effect on the MAC side.
> > 
> > Well we might need to allow PHY to have phy-mode property in
> > addition
> > to MAC, in this case MAC phy-mode='rgmii*' and PHY phy-
> > mode='internal'
> > might work?
> 
> I'm not convinced that adding more possibilities to the problem
> (i.o.w.
> the idea that phy=mode = "internal" can be used to avoid the delays
> being messed with) is a good idea - not at this point, because as you
> point out MACs (and PHYs) won't know that they need to be configured
> for RGMII mode. "internal" doesn't state this, and if we do start
> doing
> this, we'll end up with "internal" selecting RGMII mode which may
> work
> for some platforms but not all.
> 
> So, IMHO this is a bad idea.
> 
> > > > 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.
> > 
> > Well this sounds unfortunate but reasonable.
> 
> We're already in this state with PHYs faster than gigabit, because
> IEEE 802.3 in their wisdom did not define where the 1000BASE-T
> autoneg parameters appear in the register space. As a result, vendors
> have done their own thing, and every vendor / PHY is different.
> Without access to this key data, phylib has no way to know the
> negotiation results. Thus, a generic PHY driver that works correctly
> for PHYs > 1G just isn't possible.
> 
> I expect that in years to come, we'll see IEEE 802.3 updated with
> the 1G registers for Clause 45 PHYs, but the boat has already sailed
> so this would be totally pointless as there will be too many PHYs
> out there doing their own thing for whatever IEEE 802.3 says about
> this to have any relevence what so ever. Just like they did with
> 2500BASE-X, which is a similar mess due to IEEE 802.3 being way too
> late.
> 
> I hope that there isn't going to be more of this, because each time
> it happens, the IEEE 802.3 "standard" less relevant.
> 

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Russell King (Oracle)]

On Thu, Jun 05, 2025 at 06:51:43PM +0800, Icenowy Zheng wrote:
> 在 2025-06-05星期四的 10:41 +0100，Russell King (Oracle)写道：
> > On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> > > In addition, analyzing existing Ethernet drivers, I found two
> > > drivers
> > > with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> > > ti/icssg/icssg_prueth.c .
> > > 
> > > The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode
> > > is
> > > rgmii or rgmii-rxid, and the PRU ETH driver, which works on some
> > > MAC
> > > with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
> > > rgmii-id or rgmii-txid to remove the txid part.
> > 
> > No, this is wrong.
> > 
> > First, it does not reject any RGMII mode. See qcom_ethqos_probe() and
> > the switch() in there. All four RGMII modes are accepted.
> 
> Well my sentence have its subject switched here. I mean the TI PRU ETH
> driver is rejecting modes.
> 
> > 
> > The code in ethqos_rgmii_macro_init() is the questionable bit, but
> > again, does _not_ do any rejection of any RGMII mode. It simply sets
> > the transmit clock phase shift according to the mode, and the only
> > way this can work is if the board does not provide the required
> > delay.
> > 
> > This code was not reviewed by phylib maintainers, so has slipped
> > through the review process. It ought to be using the delay properties
> > to configure the MAC.
> > 
> > > The logic of QCOM ETHQOS clearly follows the original DT binding,
> > > which
> > 
> > Let's make this clear. "original DT binding" - no, nothing has
> > *actually* changed with the DT binding - the meaning of the RGMII
> > modes have not changed. The problem is one of interpretation, and
> > I can tell you from personal experience that getting stuff documented
> > so that everyone gets the same understanding is nigh on impossible.
> > People will pick holes, and deliberately interpret whatever is
> > written
> > in ways that it isn't meant to - and the more words that are used the
> > more this happens.
> 
> Well I am not sure, considering two examples I raised here (please note
> I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
> contrary handling of RGMII modes, and one matches the old binding
> document, one matches the new one).

Code sometimes sneaks in that hasn't been reviewed by the phylib
maintainers. That seems to be the case with the qcom ethqos driver.
Note the lack of tags from a phylib maintainer. However, I haven't
checked the mailing list history, maybe they put forward a good
reason for the code being as it is.

However, the fundamental fact is that the PHY interface mode is
passed into phylink and phylib unchanged, which instructs the PHY
driver to set the delays at the PHY as per that mode - as per the
phylib documentation.

One reason the code in qcom ethqos may exist is that all boards do
not insert the transmit delay, so the MAC needs to if the PHY
isn't. That may have been covered on the mailing list. I don't
know without checking, and I'm not able to check at the moment.

> > The RGMII modes have been documented in
> > Documentation/networking/phy.rst
> > (Documentation/networking/phy.txt predating) since:
> 
> I checked the document here, and it seems that it's against the changed
> binding document (it matches the original one):

I repeatedly raised the issue that the phylib documentation is the
definitive documentation, and we shouldn't be duplicating it. If you
think that the binding document is contrary to what the phylib doc
says, then the binding document is wrong.

I stand by my comment in the review. We should *not* be documenting
the same thing in two different places. You've proven my point.

I suggest we get rid of the "clarification" in the binding document
because it's just adding to the confusion. Document stuff in one
place, and one place only. Anything else is madness and leads to
exactly this problem.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

> > > If you want the kernel to not touch the PHY, use
> > > 
> > > phy-mode = 'internal'
> > 
> > This sounds weird, and may introduce side effect on the MAC side.
> > 
> > Well we might need to allow PHY to have phy-mode property in addition
> > to MAC, in this case MAC phy-mode='rgmii*' and PHY phy-mode='internal'
> > might work?
> 
> I'm not convinced that adding more possibilities to the problem (i.o.w.
> the idea that phy=mode = "internal" can be used to avoid the delays
> being messed with) is a good idea - not at this point, because as you
> point out MACs (and PHYs) won't know that they need to be configured
> for RGMII mode. "internal" doesn't state this, and if we do start doing
> this, we'll end up with "internal" selecting RGMII mode which may work
> for some platforms but not all.
> 
> So, IMHO this is a bad idea.

I agree, and if actually see a MAC/PHY pair doing this, i will ask
why. Using "internal" like is currently mostly hypothetical, i don't
actually know of any real users. Those DT blobs which do use
"internal" actually are internal interfaces within a SoC and it is
some vendor proprietary interface which does not need any
configuration. That is the original meaning for "internal", but it has
been noticed it can be used to side step RGMII delay configuration.

It _might_ play a role when developers decide to clean up a mess with
RGMII delays, and decide to just go with the strapping resistors
rather than try to figure out what is actually happening vs what DT
says should be happening, and issue warning the DT blob is out of
date. But still, i would consider "internal" a lazy hack, not a proper
fix.

	Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

On Thu, Jun 05, 2025 at 06:51:43PM +0800, Icenowy Zheng wrote:
> 在 2025-06-05星期四的 10:41 +0100，Russell King (Oracle)写道：
> > On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> > > In addition, analyzing existing Ethernet drivers, I found two
> > > drivers
> > > with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> > > ti/icssg/icssg_prueth.c .
> > > 
> > > The QCOM ETHQOS driver enables the MAC's TX delay if the phy_mode
> > > is
> > > rgmii or rgmii-rxid, and the PRU ETH driver, which works on some
> > > MAC
> > > with hardcoded TX delay, rejects rgmii and rgmii-rxid, and patches
> > > rgmii-id or rgmii-txid to remove the txid part.
> > 
> > No, this is wrong.
> > 
> > First, it does not reject any RGMII mode. See qcom_ethqos_probe() and
> > the switch() in there. All four RGMII modes are accepted.
> 
> Well my sentence have its subject switched here. I mean the TI PRU ETH
> driver is rejecting modes.

Which is theoretically fine. I've not looked at this driver in
particular, but there are some MACs were you cannot disable the delay.
The MAC always imposes 2ns delay. That would mean a PCB which also has
extra long clock lines is simply FUBAR, cannot work, and 'rgmii' is
invalid, so reject it.

> Well I am not sure, considering two examples I raised here (please note
> I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
> contrary handling of RGMII modes, and one matches the old binding
> document, one matches the new one).

Nope, i fully agree with Russell, the binding has not changed, just the
words to explain the binding.

Just for a minute, consider your interpretation of the old text is
wrong. Read the old text again and again, and see if you can find an
interpretation which is the same as the new text. If you do:

* It proves our point that describing what this means is hard, and
  developers will get it wrong.

* There is an interpretation of both the old and new where nothing
  changed.

* You have to be careful looking at drivers, because some percent of
  developers also interpreted it wrongly, and have broken
  implementations as a result.  You cannot say the binding means X,
  not Y, because there is a driver using meaning X.

My hope with the new text is that it focuses on hardware, which is
what DT is about. You can look at the schematic, see if there is extra
long clock lines or not, and then decided on 'rgmii-id' if there are
not, and 'rgmii' is there are. The rest then follows from that.

And if you look at the questions i've been asking for the last year or
more, i always start with, "Does the PCB have extra long clock
lines?".

> > The RGMII modes have been documented in
> > Documentation/networking/phy.rst
> > (Documentation/networking/phy.txt predating) since:
> 
> I checked the document here, and it seems that it's against the changed
> binding document (it matches the original one):
> 
> The phy.rst document says:
> ```
> * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting
> any
>   internal delay by itself, it assumes that either the Ethernet MAC (if
> capable)
>   or the PCB traces insert the correct 1.5-2ns delay
> ```
> 
> The changed binding document says:

You are not reading it carefully enough. The binding describes
hardware, the board. phy.rst describes the phylib interface. They are
different.

	Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-05星期四的 15:48 +0200，Andrew Lunn写道：
> On Thu, Jun 05, 2025 at 06:51:43PM +0800, Icenowy Zheng wrote:
> > 在 2025-06-05星期四的 10:41 +0100，Russell King (Oracle)写道：
> > > On Thu, Jun 05, 2025 at 05:06:43PM +0800, Icenowy Zheng wrote:
> > > > In addition, analyzing existing Ethernet drivers, I found two
> > > > drivers
> > > > with contradition: stmicro/stmmac/dwmac-qcom-ethqos.c and
> > > > ti/icssg/icssg_prueth.c .
> > > > 
> > > > The QCOM ETHQOS driver enables the MAC's TX delay if the
> > > > phy_mode
> > > > is
> > > > rgmii or rgmii-rxid, and the PRU ETH driver, which works on
> > > > some
> > > > MAC
> > > > with hardcoded TX delay, rejects rgmii and rgmii-rxid, and
> > > > patches
> > > > rgmii-id or rgmii-txid to remove the txid part.
> > > 
> > > No, this is wrong.
> > > 
> > > First, it does not reject any RGMII mode. See qcom_ethqos_probe()
> > > and
> > > the switch() in there. All four RGMII modes are accepted.
> > 
> > Well my sentence have its subject switched here. I mean the TI PRU
> > ETH
> > driver is rejecting modes.
> 
> Which is theoretically fine. I've not looked at this driver in
> particular, but there are some MACs were you cannot disable the
> delay.
> The MAC always imposes 2ns delay. That would mean a PCB which also
> has
> extra long clock lines is simply FUBAR, cannot work, and 'rgmii' is
> invalid, so reject it.

BTW I found that in some case the assumption of PHY-side delay being
always better than MAC-side one is wrong -- modern MACs usually have
adjustable delay line, but Realtek 8211-series PHYs have only on/off
delay with a fixed 2ns value.

> 
> > Well I am not sure, considering two examples I raised here (please
> > note
> > I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
> > contrary handling of RGMII modes, and one matches the old binding
> > document, one matches the new one).
> 
> Nope, i fully agree with Russell, the binding has not changed, just
> the
> words to explain the binding.

Well I read about phy.rst, and I found my understanding of the old
binding matches my understanding of phy.rst, but does not match the new
binding.

> 
> Just for a minute, consider your interpretation of the old text is
> wrong. Read the old text again and again, and see if you can find an
> interpretation which is the same as the new text. If you do:
> 
> * It proves our point that describing what this means is hard, and
>   developers will get it wrong.
> 
> * There is an interpretation of both the old and new where nothing
>   changed.
> 
> * You have to be careful looking at drivers, because some percent of
>   developers also interpreted it wrongly, and have broken
>   implementations as a result.  You cannot say the binding means X,
>   not Y, because there is a driver using meaning X.
> 
> My hope with the new text is that it focuses on hardware, which is
> what DT is about. You can look at the schematic, see if there is
> extra
> long clock lines or not, and then decided on 'rgmii-id' if there are
> not, and 'rgmii' is there are. The rest then follows from that.

Well I think "rgmii-*" shouldn't exist at all, if focusing on hardware.
I prefer only "rgmii" with properties describing the delay numbers.

> 
> And if you look at the questions i've been asking for the last year
> or
> more, i always start with, "Does the PCB have extra long clock
> lines?".
> 
> > > The RGMII modes have been documented in
> > > Documentation/networking/phy.rst
> > > (Documentation/networking/phy.txt predating) since:
> > 
> > I checked the document here, and it seems that it's against the
> > changed
> > binding document (it matches the original one):
> > 
> > The phy.rst document says:
> > ```
> > * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for
> > inserting
> > any
> >   internal delay by itself, it assumes that either the Ethernet MAC
> > (if
> > capable)
> >   or the PCB traces insert the correct 1.5-2ns delay
> > ```
> > 
> > The changed binding document says:
> 
> You are not reading it carefully enough. The binding describes
> hardware, the board. phy.rst describes the phylib interface. They are
> different.

Well I can't find the reason of phy-mode being so designed except for
leaky abstraction from phylib.

> 
>         Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Russell King (Oracle)]

On Wed, Jun 11, 2025 at 04:03:11PM +0800, Icenowy Zheng wrote:
> 在 2025-06-05星期四的 15:48 +0200，Andrew Lunn写道：
> > Which is theoretically fine. I've not looked at this driver in
> > particular, but there are some MACs were you cannot disable the
> > delay.
> > The MAC always imposes 2ns delay. That would mean a PCB which also
> > has
> > extra long clock lines is simply FUBAR, cannot work, and 'rgmii' is
> > invalid, so reject it.
> 
> BTW I found that in some case the assumption of PHY-side delay being
> always better than MAC-side one is wrong -- modern MACs usually have
> adjustable delay line, but Realtek 8211-series PHYs have only on/off
> delay with a fixed 2ns value.

The only time that MACs may implement delays based on the
PHY_INTERFACE_MODE_RGMII* is if they also include code to pass
PHY_INTERFACE_MODE_RGMII (no suffixes) to phylink / phylib to ensure
that the PHY doesn't _also_ add delays. This isn't something we
encourage because it's more code, more review, and a different way
of implementing it - thus adding to maintainers workloads that are
already high enough.

> > Just for a minute, consider your interpretation of the old text is
> > wrong. Read the old text again and again, and see if you can find an
> > interpretation which is the same as the new text. If you do:
> > 
> > * It proves our point that describing what this means is hard, and
> >   developers will get it wrong.
> > 
> > * There is an interpretation of both the old and new where nothing
> >   changed.
> > 
> > * You have to be careful looking at drivers, because some percent of
> >   developers also interpreted it wrongly, and have broken
> >   implementations as a result.  You cannot say the binding means X,
> >   not Y, because there is a driver using meaning X.
> > 
> > My hope with the new text is that it focuses on hardware, which is
> > what DT is about. You can look at the schematic, see if there is
> > extra
> > long clock lines or not, and then decided on 'rgmii-id' if there are
> > not, and 'rgmii' is there are. The rest then follows from that.
> 
> Well I think "rgmii-*" shouldn't exist at all, if focusing on hardware.
> I prefer only "rgmii" with properties describing the delay numbers.

Yes, I think we as phylib maintainers have also come to the same
conclusion with all the hassle this causes, but we can't get rid
of this without breaking the kernel and breaking device-tree
compatibility. So, we're stuck with it.

> > You are not reading it carefully enough. The binding describes
> > hardware, the board. phy.rst describes the phylib interface. They are
> > different.
> 
> Well I can't find the reason of phy-mode being so designed except for
> leaky abstraction from phylib.

I have no idea what that sentence means, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-11星期三的 09:39 +0100，Russell King (Oracle)写道：
> On Wed, Jun 11, 2025 at 04:03:11PM +0800, Icenowy Zheng wrote:
> > 在 2025-06-05星期四的 15:48 +0200，Andrew Lunn写道：
> > > Which is theoretically fine. I've not looked at this driver in
> > > particular, but there are some MACs were you cannot disable the
> > > delay.
> > > The MAC always imposes 2ns delay. That would mean a PCB which
> > > also
> > > has
> > > extra long clock lines is simply FUBAR, cannot work, and 'rgmii'
> > > is
> > > invalid, so reject it.
> > 
> > BTW I found that in some case the assumption of PHY-side delay
> > being
> > always better than MAC-side one is wrong -- modern MACs usually
> > have
> > adjustable delay line, but Realtek 8211-series PHYs have only
> > on/off
> > delay with a fixed 2ns value.
> 
> The only time that MACs may implement delays based on the
> PHY_INTERFACE_MODE_RGMII* is if they also include code to pass
> PHY_INTERFACE_MODE_RGMII (no suffixes) to phylink / phylib to ensure
> that the PHY doesn't _also_ add delays. This isn't something we
> encourage because it's more code, more review, and a different way
> of implementing it - thus adding to maintainers workloads that are
> already high enough.

Well in fact I have an additional question: when the MAC has any extra
[tr]x-internal-delay-ps property, what's the threshold of MAC
triggering patching phy mode? (The property might be only used for a
slight a few hundred ps delay for tweak instead of the full 2ns one)

> 
> > > Just for a minute, consider your interpretation of the old text
> > > is
> > > wrong. Read the old text again and again, and see if you can find
> > > an
> > > interpretation which is the same as the new text. If you do:
> > > 
> > > * It proves our point that describing what this means is hard,
> > > and
> > >   developers will get it wrong.
> > > 
> > > * There is an interpretation of both the old and new where
> > > nothing
> > >   changed.
> > > 
> > > * You have to be careful looking at drivers, because some percent
> > > of
> > >   developers also interpreted it wrongly, and have broken
> > >   implementations as a result.  You cannot say the binding means
> > > X,
> > >   not Y, because there is a driver using meaning X.
> > > 
> > > My hope with the new text is that it focuses on hardware, which
> > > is
> > > what DT is about. You can look at the schematic, see if there is
> > > extra
> > > long clock lines or not, and then decided on 'rgmii-id' if there
> > > are
> > > not, and 'rgmii' is there are. The rest then follows from that.
> > 
> > Well I think "rgmii-*" shouldn't exist at all, if focusing on
> > hardware.
> > I prefer only "rgmii" with properties describing the delay numbers.
> 
> Yes, I think we as phylib maintainers have also come to the same
> conclusion with all the hassle this causes, but we can't get rid
> of this without breaking the kernel and breaking device-tree
> compatibility. So, we're stuck with it.
> 
> > > You are not reading it carefully enough. The binding describes
> > > hardware, the board. phy.rst describes the phylib interface. They
> > > are
> > > different.
> > 
> > Well I can't find the reason of phy-mode being so designed except
> > for
> > leaky abstraction from phylib.
> 
> I have no idea what that sentence means, sorry.

Well, I mean the existence of rgmii-* modes is coupled with the
internal of phylib, did I get it right?

> 

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

> BTW I found that in some case the assumption of PHY-side delay being
> always better than MAC-side one is wrong -- modern MACs usually have
> adjustable delay line, but Realtek 8211-series PHYs have only on/off
> delay with a fixed 2ns value.

There is another email conversation last month about this. The RGMII
standard says a 2ns delay is needed. If your hardware needs something
other than 2ns, it is breaking the standard. It probably means
somebody has been lazy and made a bad design. The PCB track lengths
have not been balanced, or the MAC or PHY internal delays are poorly
designed. Whatever, it is breaking the standard. We should not be
encouraging such bad design by making it easy to work around broken
designs.

There is also another thread about Broadcom MAC/PHY combinations, and
somebody doing tests and showing that combination is very robust to
delays, it will happily work with 1ns or 3ns, not just 2.00ns.

So in general, a fixed 2ns value should just work if you are
compliment to the standard.

> > > Well I am not sure, considering two examples I raised here (please
> > > note
> > > I am comparing QCOM ETHQOS and TI PRUETH two drivers, they have
> > > contrary handling of RGMII modes, and one matches the old binding
> > > document, one matches the new one).
> > 
> > Nope, i fully agree with Russell, the binding has not changed, just
> > the
> > words to explain the binding.
> 
> Well I read about phy.rst, and I found my understanding of the old
> binding matches my understanding of phy.rst, but does not match the new
> binding.

So please quote the text, explain how you interpret it, and maybe we
can then tell you how you have it wrong. Or i might admit that the
text needs more work to remove more ambiguities.

> Well I think "rgmii-*" shouldn't exist at all, if focusing on hardware.
> I prefer only "rgmii" with properties describing the delay numbers.

As Russell said, we cannot easily change it without breaking
systems. And i'm not convinced a new set of properties would be any
better. Developers are often lazy. They try various things until it
works and call it done. There are multiple ways of getting a MAC/PHY
link to work given the available parameters, and there are multiple
combination which works, but are not correct according to the
definitions. I expect the same to be true for a new set of
parameters. Laziness wins out over correctness.

And the real issue is more subtle. Working incorrect implementations
sometimes turn into broken incorrect implementations. We have seen
cases where somebody correctly describes their hardware in DT and it
did not work. Digging into the details, how the implementation was
broken was found, and fixed, so that it correctly implemented DT, and
made that correctly described board work. But in the process it broken
lots of incorrectly described but working boards.

	Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Andrew Lunn]

> Well in fact I have an additional question: when the MAC has any extra
> [tr]x-internal-delay-ps property, what's the threshold of MAC
> triggering patching phy mode? (The property might be only used for a
> slight a few hundred ps delay for tweak instead of the full 2ns one)

Maybe you should read the text.

The text says:

  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.

So a few hundred ps delay is fine. The MAC is not providing the 2ns
delay, the PHY needs to do that, so you don't mask the value.

> > > Well I can't find the reason of phy-mode being so designed except
> > > for
> > > leaky abstraction from phylib.
> > 
> > I have no idea what that sentence means, sorry.
> 
> Well, I mean the existence of rgmii-* modes is coupled with the
> internal of phylib, did I get it right?

This is the external API of phylib, it has nothing to do with the
internals of phylib.

/**
 * phy_attach - attach a network device to a particular PHY device
 * @dev: network device to attach
 * @bus_id: Bus ID of PHY device to attach
 * @interface: PHY device's interface
 *
 * Description: Same as phy_attach_direct() except that a PHY bus_id
 *     string is passed instead of a pointer to a struct phy_device.
 */
struct phy_device *phy_attach(struct net_device *dev, const char *bus_id,
			      phy_interface_t interface)

interface tells the PHY how it should configure its interface.

If you follow the guidelines, the PHY adds the delay if needed, you
get interface == phy-mode. However, interface and phy-mode are
different things. phy-mode describes the hardware, the PCB. interface
tells the PHY what to do. There are legitimate cases where
interface != phy-mode.

	Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-11星期三的 17:28 +0200，Andrew Lunn写道：
> > Well in fact I have an additional question: when the MAC has any
> > extra
> > [tr]x-internal-delay-ps property, what's the threshold of MAC
> > triggering patching phy mode? (The property might be only used for
> > a
> > slight a few hundred ps delay for tweak instead of the full 2ns
> > one)
> 
> Maybe you should read the text.
> 
> The text says:
> 
>   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.
> 
> So a few hundred ps delay is fine. The MAC is not providing the 2ns
> delay, the PHY needs to do that, so you don't mask the value.

Thus if the MAC delay is set to 1xxx ps (e.g. 1800ps), should the MAC
do the masking?

What should be the threshold? 1ns?

> 
> > > > Well I can't find the reason of phy-mode being so designed
> > > > except
> > > > for
> > > > leaky abstraction from phylib.
> > > 
> > > I have no idea what that sentence means, sorry.
> > 
> > Well, I mean the existence of rgmii-* modes is coupled with the
> > internal of phylib, did I get it right?
> 
> This is the external API of phylib, it has nothing to do with the
> internals of phylib.
> 
> /**
>  * phy_attach - attach a network device to a particular PHY device
>  * @dev: network device to attach
>  * @bus_id: Bus ID of PHY device to attach
>  * @interface: PHY device's interface
>  *
>  * Description: Same as phy_attach_direct() except that a PHY bus_id
>  *     string is passed instead of a pointer to a struct phy_device.
>  */
> struct phy_device *phy_attach(struct net_device *dev, const char
> *bus_id,
>                               phy_interface_t interface)
> 
> interface tells the PHY how it should configure its interface.
> 
> If you follow the guidelines, the PHY adds the delay if needed, you
> get interface == phy-mode. However, interface and phy-mode are
> different things. phy-mode describes the hardware, the PCB. interface
> tells the PHY what to do. There are legitimate cases where
> interface != phy-mode.
> 
>         Andrew

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Russell King (Oracle)]

On Fri, Jun 13, 2025 at 04:01:37PM +0800, Icenowy Zheng wrote:
> 在 2025-06-11星期三的 17:28 +0200，Andrew Lunn写道：
> > > Well in fact I have an additional question: when the MAC has any
> > > extra
> > > [tr]x-internal-delay-ps property, what's the threshold of MAC
> > > triggering patching phy mode? (The property might be only used for
> > > a
> > > slight a few hundred ps delay for tweak instead of the full 2ns
> > > one)
> > 
> > Maybe you should read the text.
> > 
> > The text says:
> > 
> >   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.
> > 
> > So a few hundred ps delay is fine. The MAC is not providing the 2ns
> > delay, the PHY needs to do that, so you don't mask the value.
> 
> Thus if the MAC delay is set to 1xxx ps (e.g. 1800ps), should the MAC
> do the masking?
> 
> What should be the threshold? 1ns?

Why should there be a "threshold" ? It's really a case by case issue
where the capabilities of the hardware need to be provided and
considered before a decision can be made.

In order to first understand this, one needs to understand the
requirements of RGMII. RGMII v1.3 states:

Symbol	Parameter		Min	Typ	Max	Units
TskewT	Data to Clock output	-500	0	500	ps
	skew at clock tx
TskewR	Data to Clock input	1		2.6	ns
	skew at clock rx

The RGMII specification is written based upon the clock transmitter
and receiver having no built-in delays, and the delay is achieved
purely by trace routing. So, where delays are provided by the
transmitter or receiver (whether that's the MAC or the PHY depends
on whether TXC or RXC is being examined) these figures need to be
thought about.

However, the range for the delay at the receiver is -1ns to +0.6ns.

In your example, you're talking about needing a 1800ps delay. I
would suggest that, *assuming the PCB tracks introduce a 200ps skew
between the data and clock*, then using the PHY's built-in 2ns delay
is perfectly within the requirements of the RGMII specification.

That bit "assuming" is where the discussion needs to happen, and why
it would be case by case. If the skew due to trace routing were
800ps, then enabling the PHY's built-in 2ns delay would take the
delay out of spec.

Thrown into this would also be temperature effects, so trying to get
to as near as the 2ns delay as possible is probably a good idea.

Lastly, there's the question whether the software engineer even
knows what the skew provided by the hardware actually is.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Icenowy Zheng]

在 2025-06-13星期五的 09:35 +0100，Russell King (Oracle)写道：
> On Fri, Jun 13, 2025 at 04:01:37PM +0800, Icenowy Zheng wrote:
> > 在 2025-06-11星期三的 17:28 +0200，Andrew Lunn写道：
> > > > Well in fact I have an additional question: when the MAC has
> > > > any
> > > > extra
> > > > [tr]x-internal-delay-ps property, what's the threshold of MAC
> > > > triggering patching phy mode? (The property might be only used
> > > > for
> > > > a
> > > > slight a few hundred ps delay for tweak instead of the full 2ns
> > > > one)
> > > 
> > > Maybe you should read the text.
> > > 
> > > The text says:
> > > 
> > >   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.
> > > 
> > > So a few hundred ps delay is fine. The MAC is not providing the
> > > 2ns
> > > delay, the PHY needs to do that, so you don't mask the value.
> > 
> > Thus if the MAC delay is set to 1xxx ps (e.g. 1800ps), should the
> > MAC
> > do the masking?
> > 
> > What should be the threshold? 1ns?
> 
> Why should there be a "threshold" ? It's really a case by case issue
> where the capabilities of the hardware need to be provided and
> considered before a decision can be made.
> 
> In order to first understand this, one needs to understand the
> requirements of RGMII. RGMII v1.3 states:
> 
> Symbol  Parameter               Min     Typ     Max     Units
> TskewT  Data to Clock output    -500    0       500     ps
>         skew at clock tx
> TskewR  Data to Clock input     1               2.6     ns
>         skew at clock rx
> 
> The RGMII specification is written based upon the clock transmitter
> and receiver having no built-in delays, and the delay is achieved
> purely by trace routing. So, where delays are provided by the
> transmitter or receiver (whether that's the MAC or the PHY depends
> on whether TXC or RXC is being examined) these figures need to be
> thought about.
> 
> However, the range for the delay at the receiver is -1ns to +0.6ns.
> 
> In your example, you're talking about needing a 1800ps delay. I
> would suggest that, *assuming the PCB tracks introduce a 200ps skew
> between the data and clock*, then using the PHY's built-in 2ns delay
> is perfectly within the requirements of the RGMII specification.
> 
> That bit "assuming" is where the discussion needs to happen, and why
> it would be case by case. If the skew due to trace routing were
> 800ps, then enabling the PHY's built-in 2ns delay would take the
> delay out of spec.
> 
> Thrown into this would also be temperature effects, so trying to get
> to as near as the 2ns delay as possible is probably a good idea.
> 
> Lastly, there's the question whether the software engineer even
> knows what the skew provided by the hardware actually is.

Sigh, my experience is that the only thing I can get is some magic
numbers from HW vendor... *facepalm*

> 

Re: [PATCH net v2] dt-bindings: net: ethernet-controller: Add informative text about RGMII delays

[Russell King (Oracle)]

On Fri, Jun 13, 2025 at 04:43:14PM +0800, Icenowy Zheng wrote:
> 在 2025-06-13星期五的 09:35 +0100，Russell King (Oracle)写道：
> > On Fri, Jun 13, 2025 at 04:01:37PM +0800, Icenowy Zheng wrote:
> > > 在 2025-06-11星期三的 17:28 +0200，Andrew Lunn写道：
> > > > > Well in fact I have an additional question: when the MAC has
> > > > > any
> > > > > extra
> > > > > [tr]x-internal-delay-ps property, what's the threshold of MAC
> > > > > triggering patching phy mode? (The property might be only used
> > > > > for
> > > > > a
> > > > > slight a few hundred ps delay for tweak instead of the full 2ns
> > > > > one)
> > > > 
> > > > Maybe you should read the text.
> > > > 
> > > > The text says:
> > > > 
> > > >   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.
> > > > 
> > > > So a few hundred ps delay is fine. The MAC is not providing the
> > > > 2ns
> > > > delay, the PHY needs to do that, so you don't mask the value.
> > > 
> > > Thus if the MAC delay is set to 1xxx ps (e.g. 1800ps), should the
> > > MAC
> > > do the masking?
> > > 
> > > What should be the threshold? 1ns?
> > 
> > Why should there be a "threshold" ? It's really a case by case issue
> > where the capabilities of the hardware need to be provided and
> > considered before a decision can be made.
> > 
> > In order to first understand this, one needs to understand the
> > requirements of RGMII. RGMII v1.3 states:
> > 
> > Symbol  Parameter               Min     Typ     Max     Units
> > TskewT  Data to Clock output    -500    0       500     ps
> >         skew at clock tx
> > TskewR  Data to Clock input     1               2.6     ns
> >         skew at clock rx
> > 
> > The RGMII specification is written based upon the clock transmitter
> > and receiver having no built-in delays, and the delay is achieved
> > purely by trace routing. So, where delays are provided by the
> > transmitter or receiver (whether that's the MAC or the PHY depends
> > on whether TXC or RXC is being examined) these figures need to be
> > thought about.
> > 
> > However, the range for the delay at the receiver is -1ns to +0.6ns.
> > 
> > In your example, you're talking about needing a 1800ps delay. I
> > would suggest that, *assuming the PCB tracks introduce a 200ps skew
> > between the data and clock*, then using the PHY's built-in 2ns delay
> > is perfectly within the requirements of the RGMII specification.
> > 
> > That bit "assuming" is where the discussion needs to happen, and why
> > it would be case by case. If the skew due to trace routing were
> > 800ps, then enabling the PHY's built-in 2ns delay would take the
> > delay out of spec.
> > 
> > Thrown into this would also be temperature effects, so trying to get
> > to as near as the 2ns delay as possible is probably a good idea.
> > 
> > Lastly, there's the question whether the software engineer even
> > knows what the skew provided by the hardware actually is.
> 
> Sigh, my experience is that the only thing I can get is some magic
> numbers from HW vendor... *facepalm*

I may have missed it if you've given the details, but tell us what
information you have, what the capabilities of the hardware is, and
we should be able to make suggestions.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!