Message-ID: <20250122150542.55d483e6@fedora.home>
Date: Wed, 22 Jan 2025 15:05:42 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Ninad Palsule <ninad@...ux.ibm.com>, Jacky Chou
 <jacky_chou@...eedtech.com>, "andrew+netdev@...n.ch"
 <andrew+netdev@...n.ch>, "andrew@...econstruct.com.au"
 <andrew@...econstruct.com.au>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
 "davem@...emloft.net" <davem@...emloft.net>, "devicetree@...r.kernel.org"
 <devicetree@...r.kernel.org>, "eajames@...ux.ibm.com"
 <eajames@...ux.ibm.com>, "edumazet@...gle.com" <edumazet@...gle.com>,
 "joel@....id.au" <joel@....id.au>, "krzk+dt@...nel.org"
 <krzk+dt@...nel.org>, "kuba@...nel.org" <kuba@...nel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
 <linux-aspeed@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "minyard@....org" <minyard@....org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "openipmi-developer@...ts.sourceforge.net"
 <openipmi-developer@...ts.sourceforge.net>, "pabeni@...hat.com"
 <pabeni@...hat.com>, "ratbert@...aday-tech.com" <ratbert@...aday-tech.com>,
 "robh@...nel.org" <robh@...nel.org>
Subject: Re: 回覆: 回覆: [PATCH v2 05/10] ARM: dts:
 aspeed: system1: Add RGMII support

> > 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.  
> 
> You dropped:
> 
>    For cases where the PHY is not capable of providing this delay...
> 
> This is something i would like to keep, to strengthen that we really
> do want the PHY to add the delays. Many MACs are capable of adding
> delays, but we don't want them to, the PHY should do it, so we have
> consistency.
> 
> The language i've tried to use is that "rx-internal-delay-ps" and
> "tx-internal-delay-ps" can be used to fine tune the delays, so i'm
> expecting their values to be small, because the PHY is adding the 2ns,
> and the MAC is just adding/removing 0-200ps etc. I've also used the
> same terminology for PHY drivers, the PHY DT properties for delays are
> used for fine tuning, but the basic 2ns on/off comes from the phy-mode
> passed to phylib.
> 
> If it is just fine tuning, and not adding the full 2ns, it should just
> pass phy-mode straight through.
> 
> So your text becomes something like:
> 
>   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.
> 
> Then we can address when the MAC adds the full 2ns.
> 
>   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 phy_attach or phylink_create() for example) to
>   account for MAC-side delay insertion, so that the the PHY device
>   does not add additional delays.
> 
> I also think we need something near the beginning like:
> 
>   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.

Thanks Andrew for the suggestions, your wording is definitely better
than mine :) I'll queue that for when net-next re-opens.

Maxime

Powered by blists - more mailing lists