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

[netdev-bot+sashiko@kernel.org]

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

--

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