Re: [PATCH RFC net-next 8/9] net: stmmac: qcom-ethqos: support external PCS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: unsupported PCS interfaces are silently accepted without a PCS
--

commit 7e5af7c53413fc3b964ab675b20db937601457fb
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

net: stmmac: qcom-ethqos: support external PCS

Select the XPCS instance initialized by stmmac when firmware provides
a pcs-handle. Reject interfaces that are not supported by the selected
PCS.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index ac7d6d3e205a1..a99a78880c0fb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

[ ... ]

> @@ -681,6 +683,21 @@ static void ethqos_ptp_clk_freq_config(struct stmmac_priv *priv)
>  	netdev_dbg(priv->dev, "PTP rate %lu\n", plat_dat->clk_ptp_rate);
>  }
>  
> +static struct phylink_pcs *
> +qcom_ethqos_select_pcs(struct stmmac_priv *priv, phy_interface_t interface)
> +{
> +	struct phylink_pcs *pcs;
> +
> +	if (!priv->hw->xpcs)
> +		return ERR_PTR(-ENODEV);
> +
> +	pcs = xpcs_to_phylink_pcs(priv->hw->xpcs);
> +	if (!test_bit(interface, pcs->supported_interfaces))
> +		return ERR_PTR(-EOPNOTSUPP);

[Severity: High]
Does returning an error pointer here actually reject the unsupported
interface?

Looking at stmmac_mac_select_pcs() in
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:

	if (priv->plat->select_pcs) {
		pcs = priv->plat->select_pcs(priv, interface);
		if (!IS_ERR(pcs))
			return pcs;
	}
	...
	return NULL;

It appears that if select_pcs() returns an error pointer like
ERR_PTR(-EOPNOTSUPP), stmmac_mac_select_pcs() will ignore the error and
eventually fall through to return NULL.

Since phylink interprets a NULL return from stmmac_mac_select_pcs() as "no PCS
required" rather than a failure, won't this result in silently accepting the
unsupported interface mode and configuring the link without a PCS instead of
failing the initialization?

> +
> +	return pcs;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-qcom_xpcs_nord_emac-v1-0-4b1c682af70f@oss.qualcomm.com?part=8