Re: [PATCH net-next v2 05/14] net: pcs: pcs-xpcs: select operating mode for 10G-baseR capable PCS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] False detection of 10Gbase-R capability on 1G-only devices lacking MMD 3 (PCS) due to unhandled `0xffff` MDIO read response.
--

commit 8962f48f9cb6674f919016e2d22817c0d0ed132d
Author: Daniel Thompson <daniel@riscstar.com>

net: pcs: pcs-xpcs: select operating mode for 10G-baseR capable PCS

This commit adds generic logic to change the operating mode on XPCS
devices that support 10Gbase-R when the default mode is inappropriate.
It sets PCS_TYPE_SEL to a reserved value to fix an issue where modal
switching is otherwise suppressed.

> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 76c04372b5b50..e58103ae8dadd 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -705,10 +705,49 @@ static void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces)
>  static int xpcs_switch_interface_mode(struct dw_xpcs *xpcs,
>  				      phy_interface_t interface)
>  {
> +	int mdio_stat2, ret;
> +
>  	/* Wangxun provides a full alternative implementation to handle quirks */
>  	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID)
>  		return txgbe_xpcs_switch_mode(xpcs, interface);
>  
> +	mdio_stat2 = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_STAT2);
> +	if (mdio_stat2 < 0)
> +		return mdio_stat2;
> +
> +	/*
> +	 * If this XPCS supports 10Gbase-R then that will be the default
> +	 * operating mode. There are several interface modes where this default
> +	 * is unhelpful. Change the operating mode for interfaces were we know
> +	 * the default is wrong, and restore the default otherwise.
> +	 */
> +	if (mdio_stat2 & MDIO_PCS_STAT2_10GBR) {

[Severity: Medium]
Will this falsely detect 10GBASE-R support on 1G-only devices?

If a device lacks the PCS MMD (MMD 3), the MDIO bus read in xpcs_read()
naturally returns 0xffff due to floating bus pull-ups. Because 0xffff is
greater than 0, it bypasses the mdio_stat2 < 0 error check above.

Since 0xffff & MDIO_PCS_STAT2_10GBR (0x0001) evaluates to true, the driver
will falsely identify the device as 10GBASE-R capable and perform unintended
writes to MDIO_CTRL2 in the unpopulated MMD 3. Does this check need to
account for 0xffff responses?

> +		switch (interface) {
> +		case PHY_INTERFACE_MODE_SGMII:

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605010022.968612-1-elder@riscstar.com?part=5