diff mbox series

[v3,1/4] dt-bindings: net: ethernet-controller: Correct the definition of phy-mode

Message ID 20250121-dts_qcs615-v3-1-fa4496950d8a@quicinc.com (mailing list archive)
State New
Headers show
Series Enable ethernet on qcs615 | expand

Commit Message

Yijie Yang Jan. 21, 2025, 7:54 a.m. UTC
Correct the definition of 'phy-mode' to reflect that RX and TX delays are
added by the board, not the MAC, to prevent confusion and ensure accurate
documentation.

Signed-off-by: Yijie Yang <quic_yijiyang@quicinc.com>
---
 Documentation/devicetree/bindings/net/ethernet-controller.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Maxime Chevallier Jan. 21, 2025, 1:08 p.m. UTC | #1
On Tue, 21 Jan 2025 15:54:53 +0800
Yijie Yang <quic_yijiyang@quicinc.com> wrote:

> Correct the definition of 'phy-mode' to reflect that RX and TX delays are
> added by the board, not the MAC, to prevent confusion and ensure accurate
> documentation.

That's not entirely correct though. The purpose of the RGMII variants
(TXID, RXID, ID) are mostly to know whether or not the PHY must add
internal delays. That would be when the MAC can't AND there's no PCB
delay traces. Some MAC can insert delays.

There's documentation here as well on that point :

https://elixir.bootlin.com/linux/v6.13-rc3/source/Documentation/networking/phy.rst#L82

So, MACs may insert delays. A modification for the doc, if needed,
would rather be :

-      # RX and TX delays are added by the MAC when required
+      # RX and TX delays are added by the MAC or PCB traces when required

Thanks,

Maxime
Andrew Lunn Jan. 21, 2025, 5 p.m. UTC | #2
On Tue, Jan 21, 2025 at 02:08:40PM +0100, Maxime Chevallier wrote:
> On Tue, 21 Jan 2025 15:54:53 +0800
> Yijie Yang <quic_yijiyang@quicinc.com> wrote:
> 
> > Correct the definition of 'phy-mode' to reflect that RX and TX delays are
> > added by the board, not the MAC, to prevent confusion and ensure accurate
> > documentation.
> 
> That's not entirely correct though. The purpose of the RGMII variants
> (TXID, RXID, ID) are mostly to know whether or not the PHY must add
> internal delays. That would be when the MAC can't AND there's no PCB
> delay traces. Some MAC can insert delays.
> 
> There's documentation here as well on that point :
> 
> https://elixir.bootlin.com/linux/v6.13-rc3/source/Documentation/networking/phy.rst#L82

This is part of the problem. This describes
PHY_INTERFACE_MODE_RGMII_*, and the value passed to phylib. The
documentation even says:

   The values of phy_interface_t must be understood from the
   perspective of the PHY device itself,

But the value in DT is about the board as a whole, it describes the
hardware. Software gets to decide if the MAC or the PHY adds the
delays, if the board does not provide the delay.

> So, MACs may insert delays. A modification for the doc, if needed,
> would rather be :
> 
> -      # RX and TX delays are added by the MAC when required
> +      # RX and TX delays are added by the MAC or PCB traces when required

From the perspective of the board, this is wrong. 'rgmii' means the
board provides the delays.

There is a parallel discussion going on, about how aspeed have also
got there implementation wrong. See:

https://lore.kernel.org/netdev/0ee94fd3-d099-4d82-9ba8-eb1939450cc3@lunn.ch/

and the test of that thread.

	Andrew
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 45819b2358002bc75e876eddb4b2ca18017c04bd..d56f1a2e79c4656762c7397dc96408bdde19c52b 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -74,7 +74,7 @@  properties:
       - rev-rmii
       - moca
 
-      # RX and TX delays are added by the MAC when required
+      # RX and TX delays are added by the board when required
       - rgmii
 
       # RGMII with internal RX and TX delays provided by the PHY,