Re: [PATCH] net: phy: mxl-gpy: add 2.5G for MXL68211C

[Rouven Czerwinski]

Hi,

On Thu, 24 Sept 2026 at 04:27, <netdev-bot+sashiko@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] Missing `Fixes:` tag — a user-visible bug fix is submitted as
>   a net-next feature.

My guess would be that we can add this, I suspect that stable would pick up
this patch automatically  either way.

> - [Low] Commit message names the wrong part number: the subject ("net:
>   phy: mxl-gpy: add 2.5G for MXL68211C") and the body sentence ("the…

I'll fix the typos in a v2.

>
> --
>
> net: phy: mxl-gpy: add 2.5G for MXL68211C
>
> This switches the MxL86211C phy_driver entry from gpy_config_init() to
> gpy21x_config_init(), so that PHY_INTERFACE_MODE_2500BASEX and
> PHY_INTERFACE_MODE_SGMII get advertised in phydev->possible_interfaces.
> The changelog reports that 2.5G then negotiates against a Mikrotik
> CRS310-G+2S+IN.
>
> > net: phy: mxl-gpy: add 2.5G for MXL68211C
>
> [Severity: Low]
> The subject says "MXL68211C", and the body repeats it in "the 2.5G speed
> indication is the same between GPY211 & MXL68211C", while the first body
> sentence and the code use MXL86211C.
>
> The driver only knows the latter spelling:
>
> drivers/net/phy/mxl-gpy.c:
>         #define PHY_ID_MXL86211C        0xC1335400
>         ...
>                 .name           = "Maxlinear Ethernet MxL86211C",
>
> Could the digits be un-transposed in the subject and in that sentence on a
> resend?
>
> > diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
> > index 0da2d4e9d8548..089a1a9e5bc61 100644
> > --- a/drivers/net/phy/mxl-gpy.c
> > +++ b/drivers/net/phy/mxl-gpy.c
> > @@ -1405,7 +1405,7 @@ static struct phy_driver gpy_drivers[] = {
> >               PHY_ID_MATCH_MODEL(PHY_ID_MXL86211C),
> >               .name           = "Maxlinear Ethernet MxL86211C",
> >               .get_features   = genphy_c45_pma_read_abilities,
> > -             .config_init    = gpy_config_init,
> > +             .config_init    = gpy21x_config_init,
>
> [Severity: Medium]
> Should this carry a Fixes: tag rather than going in as a feature?
>
> The only functional delta is the possible_interfaces advertisement:
>
> drivers/net/phy/mxl-gpy.c:
>         static int gpy21x_config_init(struct phy_device *phydev)
>         {
>                 __set_bit(PHY_INTERFACE_MODE_2500BASEX, phydev->possible_interfaces);
>                 __set_bit(PHY_INTERFACE_MODE_SGMII, phydev->possible_interfaces);
>
>                 return gpy_config_init(phydev);
>         }
>
> git blame shows the MxL86211C entry, including .config_init =
> gpy_config_init, came in with:
>
> 9d844da693d6d0 ("net: phy: mxl-gpy: add support for MxL86211C")
>
> and every other GPY21x entry (GPY211B/C, GPY212B/C, GPY215B/C) already uses
> gpy21x_config_init. Together with the changelog note that 2.5G only
> negotiates "with this change", that reads like the original entry was
> incomplete rather than an intentional 1G restriction.
>
> Would it make sense to add:
>
>         Fixes: 9d844da693d6 ("net: phy: mxl-gpy: add support for MxL86211C")
>
> so that boards with this PHY, which are limited to 1G on released kernels,
> get the fix through stable?
>
> >               .probe          = gpy_probe,
> >               .inband_caps    = gpy_inband_caps,
> >               .config_inband  = gpy_config_inband,
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-mainline-rouven-mxl2g5-v1-1-a0ed53631a5f%40linaro.org

pw-bot: cr