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

[Alexander H Duyck <alexander.duyck@gmail.com>]

On Mon, 2025-03-31 at 18:20 +0200, Maxime Chevallier wrote:
> On Mon, 31 Mar 2025 15:54:20 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

...

> I was hoping Alexander could give option 1 a try, but let me know if
> you think we should instead adopt option 2, which is probably the safer
> on.
> 
> Maxime

So I gave it a try, but the results weren't promising. I ended up
getting the lp_advertised spammed with all the modes:

    Link partner advertised link modes:  100000baseKR4/Full
                                         100000baseSR4/Full
                                         100000baseCR4/Full
                                         100000baseLR4_ER4/Full
                                         100000baseKR2/Full
                                         100000baseSR2/Full
                                         100000baseCR2/Full
                                         100000baseLR2_ER2_FR2/Full
                                         100000baseDR2/Full
                                         100000baseKR/Full
                                         100000baseSR/Full
                                         100000baseLR_ER_FR/Full
                                         100000baseCR/Full
                                         100000baseDR/Full


In order to resolve it I just made the following change:
@@ -713,9 +700,7 @@ static int phylink_parse_fixedlink(struct phylink
*pl,
                phylink_warn(pl, "fixed link specifies half duplex for
%dMbps link?\n",
                             pl->link_config.speed);
 
-       linkmode_zero(pl->supported);
-       phylink_fill_fixedlink_supported(pl->supported);
-
+       linkmode_fill(pl->supported);
        linkmode_copy(pl->link_config.advertising, pl->supported);
        phylink_validate(pl, pl->supported, &pl->link_config);



Basically the issue is that I am using the pcs_validate to cleanup my
link modes. So the code below this point worked correctly for me. The
only issue was the dropping of the other bits.

That is why I mentioned the possibility of maybe adding some sort of
follow-on filter function that would go through the upper bits and or
them into the filter being run after the original one.

For example there is mask which is used to filter out everything but
the pause and autoneg bits. Perhaps we should assemble bits there
depending on the TP, FIBER, and BACKPLANE bits to clean out everything
but CR, KR, and TP types if those bits are set.