Hello Andrew, On Fri, 10 Jan 2025 16:38:18 +0100 Andrew Lunn <andrew@xxxxxxx> wrote: > > 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
