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

[Quentin Schulz]

Hi Siddharth,

On 7/23/25 8:26 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".
> 
> 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.
> 

Between the release and -rc2 (excluded), please use master branch and 
not next as your base, see 
https://docs.u-boot.org/en/latest/develop/release_cycle.html

> v1 of this patch is at:
> https://patchwork.ozlabs.org/project/uboot/patch/20250722060554.352952-1-s-vadapalli@ti.com/
> Changes since v1:
> - Switched to env_get_ulong() based on feedback from Marek Vasut at:
>    https://patchwork.ozlabs.org/project/uboot/patch/20250722060554.352952-1-s-vadapalli@ti.com/#3550426
> - Addressed feedback from Quentin Schulz at:
>    https://patchwork.ozlabs.org/project/uboot/patch/20250722060554.352952-1-s-vadapalli@ti.com/#3550263
>    by performing the following changes:
>    1) Updated drivers/net/phy/Kconfig by documenting that phy_aneg_timeout
>       env variable can override the value of CONFIG_PHY_ANEG_TIMEOUT
>    2) Updated doc/usage/environment.rst by documenting the base and unit
>       of phy_aneg_timeout
>    3) Updated drivers/net/phy/aquantia.c and drivers/net/xilinx_axi_emac.c
>       to support overriding CONFIG_PHY_ANEG_TIMEOUT with phy_aneg_timeout
> 
> 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 20,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 200.
>     "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/3e9cbdfedc541032768b14c15244f590
> 
> Regards,
> Siddharth.
> 
>   doc/usage/environment.rst     | 9 +++++++++
>   drivers/net/phy/Kconfig       | 4 +++-
>   drivers/net/phy/aquantia.c    | 8 +++++---
>   drivers/net/phy/phy.c         | 7 +++++--
>   drivers/net/xilinx_axi_emac.c | 7 +++++--
>   5 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index bb6c351b441..b48c11faffa 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -335,6 +335,15 @@ 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 (decimal) millisecond. This variable has

Please do not specify the default here, it may be different per board 
and this would then be misleading.

> +    the same base and unit as CONFIG_PHY_ANEG_TIMEOUT.

+that is, decimal and milliseconds.

> +    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.
> +
>   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/Kconfig b/drivers/net/phy/Kconfig
> index 3132718e4f8..7fa1a89fca4 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -23,7 +23,9 @@ config PHY_ANEG_TIMEOUT
>   	int "PHY auto-negotiation timeout"
>   	default 4000
>   	help
> -	  Default PHY auto-negotiation timeout.
> +	  Default PHY auto-negotiation timeout in millisecond (decimal).

Not a native, but my gut says it should be "in milliseconds".

> +	  This can be overridden by the phy_aneg_timeout environment
> +	  variable that has the same base and unit.
>   
>   if PHY_ADDR_ENABLE
>   config PHY_ADDR
> diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
> index d2db8d9f792..72f29824468 100644
> --- a/drivers/net/phy/aquantia.c
> +++ b/drivers/net/phy/aquantia.c
> @@ -552,13 +552,15 @@ int aquantia_config(struct phy_device *phydev)
>   int aquantia_startup(struct phy_device *phydev)
>   {
>   	u32 speed;
> -	int i = 0;
> +	ulong i = 0;
>   	int reg;
>   
>   	phydev->duplex = DUPLEX_FULL;
>   
>   	/* if the AN is still in progress, wait till timeout. */
>   	if (!aquantia_link_is_up(phydev)) {
> +		ulong aneg_timeout = env_get_ulong("phy_aneg_timeout", 10,
> +						   CONFIG_PHY_ANEG_TIMEOUT);

Considering phy_aneg_timeout stores a value in milliseconds, I believe a 
u32 is plenty enough as that would mean 4294967295 milliseconds max 
which is almost 50 days, I think that's enough for a timeout :)

Up to you on the type of the variable but ulong seems overkill (it is 
what env_get_ulong will return but we don't need more). Since this comes 
from the user at runtime, I would suggest to *not* take a signed type so 
that a type overflow is defined behavior (it wraps and is minimally 0).

Same remark for other drivers. ulong is fine as is, just taking a lot of 
space in memory for no benefit.

Cheers,
Quentin