Hello Andrew,

On Fri, 10 Jan 2025 16:38:18 +0100
Andrew Lunn [off-list ref] wrote:

quoted

Do we need updates on this description. It doesn't talk about external PCB
level delays?

https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L77-L90

This is what you explained:

MAC driver reads following phy-mode from device tree. 95% of mac driver
directly
 pass it to PHY through phy_connect.

rgmii - PCB has long clock lines so delay is added by PCB
        On this mode PHY does nothing.
rgmii-id - PCB doesn't add delay. Either MAC or PHY needs to add the delay
           Add delays in both directions. Some PHY may not add delay in that
           case MAC needs to add the delay mask rgmii-id to rgmii.
rgmii-rxid - If there is an extra long TX clock line, but not RX clock,
             you would use rgmii-rxid
rgmii-txid - When there is an extra long RX clock line on the PCB, but not
             the TX clock line, you would use rgmii-txid  

The documentation is not great, that has been said a few times.  What
is described here is the view from the PHY, which is not ideal.

      # RX and TX delays are added by the MAC when required
      - rgmii

From the perspective of the PHY, this means it does not need to add
delays, because the MAC has added the delays, if required, e.g. the
PCB has not added the delays.

We have the problem that DT is supposed to describe the
hardware. Saying the PHY should add the delays, but if the MAC adds
the delays it needs to mask the value passed to the PHY does not
describe the hardware, it is Linix implementation details. The DT
Maintainers don't want that in the DT binding because other OSes might
decide to implement the details differently.

So your description becomes:

rgmii      - PCB has long clock lines so delays are added by the PCB
rgmii-id   - PCB doesn't add delay. Either MAC or PHY needs to add the delays
             in both directions.
rgmii-rxid - There is an extra long TX clock line on the PCB, but not the RX clock.
rgmii-txid - There is an extra long RX clock line on the PCB, but not the TX clock.

It is correct, but leaves so much unsaid developers will still get it
wrong.

I myself got it wrong, as the kernel doc explicitely says that the "rgmii"
phy-mode is the one to use to get MAC-side delay insertion, whereas the way I
understand it, mac-side delay insertion doesn't really depend on the phy-mode
passed from DT. Ideally we would even consider that these mac-side delay
insertion would have to be ignored in basic 'RGMII' mode, but I think that would
break quite some existing setups ?

Can we consider an update in the kernel doc along these lines :

---
 Documentation/networking/phy.rst | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst
index f64641417c54..7ab77f9867a0 100644
--- a/Documentation/networking/phy.rst
+++ b/Documentation/networking/phy.rst

@@ -106,14 +106,17 @@ Whenever possible, use the PHY side RGMII delay for these reasons:
   configure correctly a specified delay enables more designs with similar delay
   requirements to be operated correctly
 
-For cases where the PHY is not capable of providing this delay, but the
-Ethernet MAC driver is capable of doing so, the correct phy_interface_t value
-should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
-configured correctly in order to provide the required transmit and/or receive
-side delay from the perspective of the PHY device. Conversely, if the Ethernet
-MAC driver looks at the phy_interface_t value, for any other mode but
-PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are
-disabled.
+The MAC driver may add delays if the PCB doesn't include any. This can be
+detected based on firmware "rx-internal-delay-ps" and "tx-internal-delay-ps"
+properties.
+
+When the MAC driver can insert the delays, it should always do so when these
+properties are present and non-zero, regardless of the RGMII mode specified.
+
+However, the MAC driver must adjust the PHY_INTERFACE_MODE_RGMII_* mode it passes
+to the connected PHY device (through phy_attach or phylink_create() for example)
+to account for MAC-side delay insertion, so that the the PHY device knows
+if any delays still needs insertion on either TX or RX paths.
 
 In case neither the Ethernet MAC, nor the PHY are capable of providing the
 required delays, as defined per the RGMII standard, several options may be

-- 

Thanks,

Maxime