[PATCH] net: phy: Support overriding Auto Negotiation timeout with env variable

[Siddharth Vadapalli]

On Tue, Jul 22, 2025 at 10:49:04AM +0200, Quentin Schulz wrote:
> Hi Siddarth,
> 
> On 7/22/25 8:05 AM, Siddharth Vadapalli wrote:
> > The Auto Negotiation procedure between two Ethernet PHYs consists of
> > determining the best commonly supported parameters among Speed,
> > Duplex Mode and Flow Control.
> > 
> > The time taken for this procedure is not only dependent on the local
> > PHY used, but also on the link-partner PHY.
> > 
> > While a timeout can be specified in the form of a "CONFIG" on the basis
> > of the local PHY present on the device, since the timeout also depends
> > on the link-partner PHY, it might be necessary to modify the timeout.
> > 
> > To avoid rebuilding the bootloader for a given device, just because it
> > may be connected to various link-partner PHYs, each with a different
> > timeout, introduce an environment variable named "phy_aneg_timeout" and
> > override "CONFIG_PHY_ANEG_TIMEOUT" with "phy_aneg_timeout".
> > 
> 
> I'm not entirely convinced it makes sense to do that but I'm not against it,
> will let others decide. If we go forward with it, please see comments below.
> 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
> > ---
> > 
> > Hello,
> > 
> > This patch is based on commit
> > 7598b469c16 Merge tag 'u-boot-dfu-next-20250703' of https://source.denx.de/u-boot/custodians/u-boot-dfu into next
> > of the next branch of Mainline U-Boot.
> > 
> > Patch has been tested on J784S4-EVM validating the following cases:
> > 1. PHY Auto-Negotiation performed with "phy_aneg_timeout" unset.
> >     CONFIG_PHY_TIMEOUT with a value of 4,000 takes effect.
> >     => Auto Negotiation succeeds
> > 2. PHY Auto-Negotiation performed with "phy_aneg_timeout" set to 10,000.
> >     "phy_aneg_timeout" overrides CONFIG_PHY_TIMEOUT.
> >     [Higher value than default specified by CONFIG_PHY_ANEG_TIMEOUT]
> >     => Auto Negotiation succeeds
> > 3. PHY Auto-Negotiation performed with "phy_aneg_timeout" set to 2,000.
> >     "phy_aneg_timeout" overrides CONFIG_PHY_TIMEOUT.
> >     [Lower value than default specified by CONFIG_PHY_ANEG_TIMEOUT]
> >     => Auto Negotiation times out
> > 
> > Test Logs:
> > https://gist.github.com/Siddharth-Vadapalli-at-TI/697eadcbb26786435df48a834c968375
> > 
> > Regards,
> > Siddharth.
> > 
> >   doc/usage/environment.rst | 8 ++++++++
> >   drivers/net/phy/phy.c     | 6 +++++-
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> > index bb6c351b441..7447bb85d3e 100644
> > --- a/doc/usage/environment.rst
> > +++ b/doc/usage/environment.rst
> > @@ -335,6 +335,14 @@ netretry
> >       Useful on scripts which control the retry operation
> >       themselves.
> > +phy_aneg_timeout
> > +    If set, the specified value will override CONFIG_PHY_ANEG_TIMEOUT
> > +    which defaults to 4000. The default value of CONFIG_PHY_ANEG_TIMEOUT
> > +    may be sufficient for most use-cases, but certain link-partners
> > +    may require a larger timeout due to the Ethernet PHY they use.
> > +    Alternatively, the timeout can be reduced as well if the use-case
> > +    demands it.
> > +
> 
> Please also update the Kconfig symbol's help to mention this new variable
> can override the value in that symbol.

Sure, I will do so.

> 
> Please also mention the base and unit of that variable. Reading the code, it
> seems it's forced decimal and the unit is milliseconds?

The unit is the same as CONFIG_PHY_ANEG_TIMEOUT which it overrides.

> 
> >   rng_seed_size
> >       Size of random value added to device-tree node /chosen/rng-seed.
> >       This variable is given as a decimal number.
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index e6fed8c41d7..0f5c9685b59 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> 
> Please also update other users of CONFIG_PHY_ANEG_TIMEOUT. I would recommend
> to abstract the logic to get the timeout into a function so that consumers
> don't have to do the parsing themselves, they simply call that function to
> get the actual timeout.
> 
> Users are drivers/net/phy/aquantia.c and drivers/net/xilinx_axi_emac.c as
> well at the very least.
> 
> I'm wondering if we should only have this environment variable and make it
> default to CONFIG_PHY_ANEG_TIMEOUT instead? Then the logic would be to
> simply read this variable.

Would it be better to increase the default value of
CONFIG_PHY_ANEG_TIMEOUT to maybe 20,000 instead? If auto negotiation
completes earlier, then there won't be any issues for existing users.
The only problem could be if it is expected to fail earlier for
triggering a fallback mechanism. Please let me know what you think.

Regards,
Siddharth.