| Message ID | 20250214094414.1418174-1-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] Documentation: net: phy: Elaborate on RGMII delay handling | expand |
On Fri, Feb 14, 2025 at 10:44:13AM +0100, Maxime Chevallier wrote: > @@ -73,8 +73,16 @@ The Reduced Gigabit Medium Independent Interface (RGMII) is a 12-pin > electrical signal interface using a synchronous 125Mhz clock signal and several > data lines. Due to this design decision, a 1.5ns to 2ns delay must be added > between the clock line (RXC or TXC) and the data lines to let the PHY (clock > -sink) have a large enough setup and hold time to sample the data lines correctly. The > -PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let > +sink) have a large enough setup and hold time to sample the data lines correctly. > + > +The device tree property phy-mode describes the hardware. When used Please don't make this document device-tree centric - it isn't currently, and in fact phylink can be used with other implementations even statically defined. Nothing about the phy mode is device-tree centric. > +with RGMII, its value indicates if the hardware, i.e. the PCB, > +provides the 2ns delay required for RGMII. A phy-mode of 'rgmii' > +indicates the PCB is adding the 2ns delay. For other values, the > +MAC/PHY pair must insert the needed 2ns delay, with the strong > +preference the PHY adds the delay. This gets confusing. The documentation already lists each RGMII mode describing each in detail in terms of the PHY. I'm not sure we need to turn it on its head and start talking about "it's the PCB property". > + > +The PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let > the PHY driver and optionally the MAC driver, implement the required delay. The > values of phy_interface_t must be understood from the perspective of the PHY > device itself, leading to the following: > @@ -106,14 +114,22 @@ 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 fine tune the delays. This can be configured > +based on firmware "rx-internal-delay-ps" and "tx-internal-delay-ps" > +properties. These values are expected to be small, not the full 2ns > +delay. > + > +A MAC driver inserting these fine tuning delays should always do so > +when these properties are present and non-zero, regardless of the > +RGMII mode specified. > + > +For cases where the PHY is not capable of providing the 2ns delay, > +the MAC must provide it, No, this is inaccurate. One may have a PHY that is not capable of providing the delay, but the PCB does. It also brings up the question "how does the MAC know that the PHY isn't capable of providing the delay" and "how does the MAC know that the PCB is not providing the delay". This is a can of worms... > if the phy-mode indicates the PCB is not > +providing the delays. The MAC driver must adjust the > +PHY_INTERFACE_MODE_RGMII_* mode it passes to the connected PHY > +device (through :c:func:`phy_connect <phy_connect>` for example) to > +account for MAC-side delay insertion, so that the PHY device > +does not add additional delays. The intention of the paragraph you're trying to clarify (but I'm not sure it is any clearer) is: - If the MAC driver is providing the delays, it should pass PHY_INTERFACE_MODE_RGMII to phylib. It should interpret the individual RGMII modes for its own delay setting. - If the MAC driver is not providing the delays, it should pass the appropriate PHY_INTERFACE_MODE_RGMII* to phylib so the PHY can add the appropriate delays (which will depend on the PCB and other design parameters.) The MAC driver must *not* interpret the individual RGMII modes for its own delay setting. In both cases, the MAC can fine-tune the delays using whatever mechanism it sees fit. Whether the PHY is capable of providing the delay or not is up to the board designer and up to the author providing the RGMII configuration (e.g. board firmware (DT / ACPI) to sort out.) There is no mechanism in the kernel that the MAC can discover whether the PHY its going to connect to supports any particular RGMII delay setting.
Hi Russell, On Fri, 14 Feb 2025 10:08:12 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Fri, Feb 14, 2025 at 10:44:13AM +0100, Maxime Chevallier wrote: > > @@ -73,8 +73,16 @@ The Reduced Gigabit Medium Independent Interface (RGMII) is a 12-pin > > electrical signal interface using a synchronous 125Mhz clock signal and several > > data lines. Due to this design decision, a 1.5ns to 2ns delay must be added > > between the clock line (RXC or TXC) and the data lines to let the PHY (clock > > -sink) have a large enough setup and hold time to sample the data lines correctly. The > > -PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let > > +sink) have a large enough setup and hold time to sample the data lines correctly. > > + > > +The device tree property phy-mode describes the hardware. When used > > Please don't make this document device-tree centric - it isn't > currently, and in fact phylink can be used with other implementations > even statically defined. Nothing about the phy mode is device-tree > centric. Would "firmware" be more appropriate, or do you want to simply drop the whole thing ? > > > +with RGMII, its value indicates if the hardware, i.e. the PCB, > > +provides the 2ns delay required for RGMII. A phy-mode of 'rgmii' > > +indicates the PCB is adding the 2ns delay. For other values, the > > +MAC/PHY pair must insert the needed 2ns delay, with the strong > > +preference the PHY adds the delay. > > This gets confusing. The documentation already lists each RGMII mode > describing each in detail in terms of the PHY. I'm not sure we need to > turn it on its head and start talking about "it's the PCB property". What I'm trying to convey here is that this description in terms of PHY-side perspective leads people (me included, but not only) to wrongly assume that if the phy-mode passed by the firmware is "rgmii", then MAC inserts the delays, and if it's "rgmii-[tx|rx]id", then the PHY inserts them. Which, as you state later, is wrong as "rgmii" means "No need for delay insertion in either MAC of PHY" and the other modes means "MAC or PHY needs to insert the 2ns delay". > > + > > +The PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let > > the PHY driver and optionally the MAC driver, implement the required delay. The > > values of phy_interface_t must be understood from the perspective of the PHY > > device itself, leading to the following: > > @@ -106,14 +114,22 @@ 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 fine tune the delays. This can be configured > > +based on firmware "rx-internal-delay-ps" and "tx-internal-delay-ps" > > +properties. These values are expected to be small, not the full 2ns > > +delay. > > + > > +A MAC driver inserting these fine tuning delays should always do so > > +when these properties are present and non-zero, regardless of the > > +RGMII mode specified. > > + > > +For cases where the PHY is not capable of providing the 2ns delay, > > +the MAC must provide it, > > No, this is inaccurate. One may have a PHY that is not capable of > providing the delay, but the PCB does. Sure thing, I can add a mention of the fact that this whole logic of 'who gets to insert the delay' only applies when the firmware phy-mode isn't rgmii. > It also brings up the question "how does the MAC know that the PHY > isn't capable of providing the delay" and "how does the MAC know that > the PCB is not providing the delay". This is a can of worms... Agreed, but indeed that's a whole can of worms with so much users than It's hard to introduce some proper support for that without breaking existing setups. > > if the phy-mode indicates the PCB is not > > +providing the delays. The MAC driver must adjust the > > +PHY_INTERFACE_MODE_RGMII_* mode it passes to the connected PHY > > +device (through :c:func:`phy_connect <phy_connect>` for example) to > > +account for MAC-side delay insertion, so that the PHY device > > +does not add additional delays. > > The intention of the paragraph you're trying to clarify (but I'm not > sure it is any clearer) is: > > - If the MAC driver is providing the delays, it should pass > PHY_INTERFACE_MODE_RGMII to phylib. It should interpret the > individual RGMII modes for its own delay setting. > > - If the MAC driver is not providing the delays, it should pass > the appropriate PHY_INTERFACE_MODE_RGMII* to phylib so the PHY > can add the appropriate delays (which will depend on the PCB and > other design parameters.) The MAC driver must *not* interpret the > individual RGMII modes for its own delay setting. That's indeed what I'm trying to say :) > In both cases, the MAC can fine-tune the delays using whatever mechanism > it sees fit. > > Whether the PHY is capable of providing the delay or not is up to the > board designer and up to the author providing the RGMII configuration > (e.g. board firmware (DT / ACPI) to sort out.) There is no mechanism > in the kernel that the MAC can discover whether the PHY its going to > connect to supports any particular RGMII delay setting. Just to clarify, do you see that patch as useful ? seems to me like the original version is clear enough to you Thanks for reviewing, Maxime
> Just to clarify, do you see that patch as useful ? seems to me like the > original version is clear enough to you > > Thanks for reviewing, Is the problem that the documentation is confusing? Or that developers don't actually read the documentation, nor the mailing list? There is a point of diminishing returns with working on Documentation. There might be more value in working on checkpatch, add a warning about any patch adding phy-mode == 'rmgii' without a comment on the line. Andrew
diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst index f64641417c54..c6b8fa611548 100644 --- a/Documentation/networking/phy.rst +++ b/Documentation/networking/phy.rst @@ -73,8 +73,16 @@ The Reduced Gigabit Medium Independent Interface (RGMII) is a 12-pin electrical signal interface using a synchronous 125Mhz clock signal and several data lines. Due to this design decision, a 1.5ns to 2ns delay must be added between the clock line (RXC or TXC) and the data lines to let the PHY (clock -sink) have a large enough setup and hold time to sample the data lines correctly. The -PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let +sink) have a large enough setup and hold time to sample the data lines correctly. + +The device tree property phy-mode describes the hardware. When used +with RGMII, its value indicates if the hardware, i.e. the PCB, +provides the 2ns delay required for RGMII. A phy-mode of 'rgmii' +indicates the PCB is adding the 2ns delay. For other values, the +MAC/PHY pair must insert the needed 2ns delay, with the strong +preference the PHY adds the delay. + +The PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let the PHY driver and optionally the MAC driver, implement the required delay. The values of phy_interface_t must be understood from the perspective of the PHY device itself, leading to the following: @@ -106,14 +114,22 @@ 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 fine tune the delays. This can be configured +based on firmware "rx-internal-delay-ps" and "tx-internal-delay-ps" +properties. These values are expected to be small, not the full 2ns +delay. + +A MAC driver inserting these fine tuning delays should always do so +when these properties are present and non-zero, regardless of the +RGMII mode specified. + +For cases where the PHY is not capable of providing the 2ns delay, +the MAC must provide it, if the phy-mode indicates the PCB is not +providing the delays. The MAC driver must adjust the +PHY_INTERFACE_MODE_RGMII_* mode it passes to the connected PHY +device (through :c:func:`phy_connect <phy_connect>` for example) to +account for MAC-side delay insertion, so that the PHY device +does not add additional delays. 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
As discussed here [1], the RGMII delays may be inserted by either the MAC, the PHY or the Board through the PCB traces. Elaborate more on what the firmware properties represent, and what is the expected role of MAC and PHY in delay insertion, with a preference on PHY-side delay insertion. [1] : https://lore.kernel.org/netdev/c83f0193-ce24-4a3e-87d1-f52587e13ca4@lunn.ch/ Suggested-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- Documentation/networking/phy.rst | 36 +++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 10 deletions(-)