Re: [PATCH net v2] net: pcs: enable autonegotiation for 10g-usxgmii

[Vladimir Oltean <olteanv@gmail.com>]

Hi,

On Mon, Aug 24, 2026 at 12:36:12PM +0200, Patryk Biel wrote:
> The Lynx PCS USXGMII setup programs the replicator advertisement, but
> does not explicitly enable and restart in-band autonegotiation or program
> the replicator link timers.
> 
> This leaves the PCS dependent on firmware or bootloader state. Systems
> which do not get the USXGMII replicator preconfigured before Linux may
> therefore fail to negotiate the link correctly.
> 
> After programming the USXGMII device ability, configure the replicator
> BMCR with reset, autonegotiation enable and autonegotiation restart. Also
> program the replicator link timer registers with the values used by the
> ENETC/Felix setup.
> 
> Signed-off-by: Patryk Biel <pbiel7@gmail.com>
> ---
> This is a follow-up to the discussion started here:
> 
> Link: https://lore.kernel.org/netdev/CA+DkFDaW_wJ5p9_P7pMpz-8iE6xeKkdF-MQcd2m2GcyVUE3S4Q@mail.gmail.com/
> 
> To summarize: on systems that don't rely on the U-Boot Felix switch
> driver to pre-configure 10G-QXGMII in-band autonegotiation, the Lynx
> PCS USXGMII code programs the replicator device ability but never
> actually enables/restarts autonegotiation nor sets up the replicator
> link timers. This leaves link establishment dependent on bootloader
> state that isn't guaranteed to exist.
> 
> This series only addresses the PCS side of the problem: it configures
> the USXGMII replicator BMCR (reset/AN enable/AN restart) and the link
> timer registers whenever lynx_pcs_config_usxgmii() is called, so that
> in-band AN comes up correctly regardless of what the bootloader did.
> 
> It intentionally does NOT yet remove the "only supports in-band AN for
> now" limitation, nor does it wire up neg_mode-based configuration to
> support the managed = "in-band-status" property being absent from the
> device tree. That part still needs more work/testing on my side (in
> particular the in-band-disable path isn't behaving as expected yet
> with the PHY I'm testing against), and I'd like to discuss the right
> approach for it separately before sending a follow-up series.
> 
> Feedback welcome, especially on whether this is an acceptable
> incremental step or whether it should be bundled together with the
> neg_mode/in-band-disable work
> ---
> Changes in v2:
> - Reorder local variable declarations in lynx_pcs_config_usxgmii().
> - Move USXGMII replicator link timer configuration before the autonegotiation restart.
> - Use phylink_get_link_timer_ns() instead of hardcoded USXGMII 
>   replicator link timer values, converting to 3.2 ns register step.
> - Link to v1: https://lore.kernel.org/r/20260820-b4-fix-pcs-lynx-an-v1-1-62d66391eaff@gmail.com
> ---
>  drivers/net/pcs/pcs-lynx.c | 47 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
> index a92081560e641ad2b297b7395cc72bf2f59ea16c..564c8a3d2b06fde92a249308f955566d6baac36f 100644
> --- a/drivers/net/pcs/pcs-lynx.c
> +++ b/drivers/net/pcs/pcs-lynx.c
> @@ -20,6 +20,9 @@
>  #define IF_MODE_SPEED_MSK		GENMASK(3, 2)
>  #define IF_MODE_HALF_DUPLEX		BIT(4)
>  
> +/* USXGMII replicator link timer step is 3.2 ns (312.5 MHz clock) */
> +#define USXGMII_LINK_TIMER_VAL(ns)	((u32)((ns) * 10 / 32))
> +
>  struct lynx_pcs {
>  	struct phylink_pcs pcs;
>  	struct mdio_device *mdio;
> @@ -156,6 +159,9 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs,
>  {
>  	struct mii_bus *bus = pcs->bus;
>  	int addr = pcs->addr;
> +	int link_timer_ns;
> +	u32 link_timer;
> +	int ret;
>  
>  	if (neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) {
>  		dev_err(&pcs->dev, "%s only supports in-band AN for now\n",
> @@ -164,10 +170,43 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs,
>  	}
>  
>  	/* Configure device ability for the USXGMII Replicator */
> -	return mdiobus_c45_write(bus, addr, MDIO_MMD_VEND2, MII_ADVERTISE,
> -				 MDIO_USXGMII_10G | MDIO_USXGMII_LINK |
> -				 MDIO_USXGMII_FULL_DUPLEX |
> -				 ADVERTISE_SGMII | ADVERTISE_LPACK);
> +	ret = mdiobus_c45_write(bus, addr, MDIO_MMD_VEND2, MII_ADVERTISE,
> +				MDIO_USXGMII_10G | MDIO_USXGMII_LINK |
> +				MDIO_USXGMII_FULL_DUPLEX |
> +				ADVERTISE_SGMII | ADVERTISE_LPACK);
> +	if (ret < 0) {
> +		dev_err(&pcs->dev, "could not set USXGMII replicator config\n");
> +		return ret;
> +	}
> +
> +	link_timer_ns = phylink_get_link_timer_ns(interface);
> +	if (link_timer_ns > 0) {
> +		link_timer = USXGMII_LINK_TIMER_VAL(link_timer_ns);
> +
> +		ret = mdiobus_c45_write(bus, addr, MDIO_MMD_VEND2,
> +					LINK_TIMER_LO, link_timer & 0xffff);
> +		if (ret < 0) {
> +			dev_err(&pcs->dev, "could not set USXGMII Link Timer 1\n");
> +			return ret;
> +		}
> +
> +		ret = mdiobus_c45_write(bus, addr, MDIO_MMD_VEND2,
> +					LINK_TIMER_HI, (link_timer >> 16) & 0x1f);
> +		if (ret < 0) {
> +			dev_err(&pcs->dev, "could not set USXGMII Link Timer 2\n");
> +			return ret;
> +		}
> +	}
> +
> +	/* Configure autonegotiation */
> +	ret = mdiobus_c45_write(bus, addr, MDIO_MMD_VEND2, MII_BMCR,
> +				BMCR_RESET | BMCR_ANENABLE | BMCR_ANRESTART);
> +	if (ret < 0) {
> +		dev_err(&pcs->dev, "could not set USXGMII replicator control config\n");
> +		return ret;
> +	}
> +
> +	return ret;
>  }
>  
>  static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> 
> ---
> base-commit: 7cbfb180945ce529608e4d4e24a6d483699fab1e
> change-id: 20260820-b4-fix-pcs-lynx-an-3fd1d5e94ce6
> 
> Best regards,
> -- 
> Patryk Biel <pbiel7@gmail.com>
> 
> 

There are some problems with this patch on
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds-13bb.dtso (AQR412C PHY).

[   63.529426] mscc_felix 0000:00:00.5: Unsupported speed on port 0: -1
[   63.529454] mscc_felix 0000:00:00.5 swp0: Link is Up - Unknown/Unknown - flow control off

(and traffic doesn't pass)

There are multiple differences compared to U-Boot does. I haven't yet
been able to definitively determine the cause for regression. I'll debug
more tomorrow. Preliminary testing shows that any link timer values > 0.84 ms
will cause in-band autoneg to not complete with this combination.

To be clear, does your board work with this patch?