Re: [PATCH net-next v5 09/13] net: phylink: Use phy_caps_lookup for fixed-link configuration

[Alexander H Duyck]

On Fri, 2025-03-07 at 18:36 +0100, Maxime Chevallier wrote:
> When phylink creates a fixed-link configuration, it finds a matching
> linkmode to set as the advertised, lp_advertising and supported modes
> based on the speed and duplex of the fixed link.
> 
> Use the newly introduced phy_caps_lookup to get these modes instead of
> phy_lookup_settings(). This has the side effect that the matched
> settings and configured linkmodes may now contain several linkmodes (the
> intersection of supported linkmodes from the phylink settings and the
> linkmodes that match speed/duplex) instead of the one from
> phy_lookup_settings().
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> ---
>  drivers/net/phy/phylink.c | 44 +++++++++++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index cf9f019382ad..8e2b7d647a92 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -802,12 +802,26 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
>  	return phylink_validate_mac_and_pcs(pl, supported, state);
>  }
>  
> +static void phylink_fill_fixedlink_supported(unsigned long *supported)
> +{
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
> +}
> +

Any chance we can go with a different route here than just locking
fixed mode to being only for BaseT configurations?

I am currently working on getting the fbnic driver up and running and I
am using fixed-link mode as a crutch until I can finish up enabling
QSFP module support for phylink and this just knocked out the supported
link modes as I was using 25CR, 50CR, 50CR2, and 100CR2.

Seems like this should really be something handled by some sort of
validation function rather than just forcing all devices using fixed
link to assume that they are BaseT. I know in our direct attached
copper case we aren't running autoneg so that plan was to use fixed
link until we can add more flexibility by getting QSFP support going.