Re: [PATCH net v2 2/3] net: stmmac: advertise integrated PCS interfaces to phylink

Re: [PATCH net v2 2/3] net: stmmac: advertise integrated PCS interfaces to phylink

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

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] The Fixes tag names 68cff4fff61f ("net: stmmac: add BASE-X
  support to integrated PCS", 2026-03-18) and the stable Cc is…
- [Medium] The patch makes PHY_INTERFACE_MODE_1000BASEX runtime-
  selectable by phylink on every platform with an integrated PCS (the…

--

net: stmmac: advertise integrated PCS interfaces to phylink

This adds the integrated PCS supported_interfaces bitmap into
config->supported_interfaces in stmmac_phylink_setup(), so that
interfaces handled by the integrated PCS are advertised to phylink.
The stated symptom is SGMII missing from the bitmap on qcom-ethqos when
firmware selects 2500BASE-X, which makes phylink drop the lower-speed
modes of a QCA8081.

>     Fixes: 68cff4fff61f ("net: stmmac: add BASE-X support to integrated PCS")
>     Cc: stable@vger.kernel.org # 7.0+

[Severity: Medium]

Is 68cff4fff61f the right commit to blame here, and is the stable range
wide enough?

The thing being fixed is that stmmac_phylink_setup() never folded
priv->integrated_pcs->pcs.supported_interfaces into
config->supported_interfaces.  Both priv->integrated_pcs and the SGMII bit
appear to come from 2c81f3357136a ("net: stmmac: convert to phylink PCS
support"), which already had:

drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c:stmmac_integrated_pcs_init()
	__set_bit(PHY_INTERFACE_MODE_SGMII, spcs->pcs.supported_interfaces);

git blame attributes that SGMII line to 2c81f3357136a, while only the
surrounding 1000BASEX/2500BASEX/support_tbi_rtbi lines come from
68cff4fff61f.

The described symptom also doesn't seem to need anything 68cff4fff61f
added.  dwmac-qcom-ethqos implements neither get_interfaces() nor
select_pcs(), and has no xpcs or hw->phylink_pcs, so
config->supported_interfaces was just {2500BASEX} via the fallback:

	if (phy_interface_empty(config->supported_interfaces))
		__set_bit(priv->plat->phy_interface,
			  config->supported_interfaces);

phylink_validate_phy() then does phy_interface_and() against
phy->possible_interfaces, and qca808x_fill_possible_interfaces() only sets
SGMII + 2500BASEX, so the intersection collapsed to {2500BASEX} on kernels
containing 2c81f3357136a but not 68cff4fff61f as well.

Should the Fixes tag point at 2c81f3357136a and the stable Cc cover the
corresponding range, so those kernels get the fix too?  The patch applies
unchanged to any tree containing 2c81f3357136a.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 1350e92943ac..f956e0d2ec64 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1414,6 +1414,12 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
>  		__set_bit(priv->plat->phy_interface,
>  			  config->supported_interfaces);
>  
> +	/* If we have an integrated PCS, add the interfaces it supports. */
> +	if (priv->integrated_pcs)
> +		phy_interface_or(config->supported_interfaces,
> +				 config->supported_interfaces,
> +				 priv->integrated_pcs->pcs.supported_interfaces);
> +

[Severity: Medium]

Does this make 1000BASE-X selectable on platforms that cannot program
their SerDes for it?

stmmac_integrated_pcs_init() sets the 1000BASEX bit unconditionally, so
after this change config->supported_interfaces contains 1000BASEX on every
platform with an integrated PCS.  On qcom-ethqos, though, only SGMII and
2500BASEX reach the SerDes:

drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c:ethqos_mac_finish_serdes()
	if (interface == PHY_INTERFACE_MODE_SGMII ||
	    interface == PHY_INTERFACE_MODE_2500BASEX)
		ret = phy_set_mode_ext(ethqos->serdes_phy, PHY_MODE_ETHERNET,
				       interface);

	return ret;

The SerDes driver does distinguish the rates, so a stale setting is a real
mismatch rather than a don't-care:

drivers/phy/qualcomm/phy-qcom-sgmii-eth.c:qcom_dwmac_sgmii_phy_calibrate()
	case PHY_INTERFACE_MODE_SGMII:
	case PHY_INTERFACE_MODE_1000BASEX:
		/* 1.25Gbps mode */
		qcom_dwmac_sgmii_phy_init_1g(data->regmap);
		break;
	case PHY_INTERFACE_MODE_2500BASEX:
		/* 3.125Gbps mode */
		qcom_dwmac_sgmii_phy_init_2p5g(data->regmap);

So if 1000BASE-X is selected after a 2500BASE-X link, the QSERDES stays at
3.125 Gb/s while the integrated PCS runs 1000BASE-X at 1.25 Gb/s, and
ethqos_mac_finish_serdes() still returns 0, so nothing reports an error.

One way to reach that selection is an optical 1000BASE-X SFP module, since
phylink intersects the module's interfaces with the config bitmap:

drivers/net/phy/phylink.c:phylink_sfp_config_optical()
	phy_interface_and(pl->sfp_interfaces, pl->config->supported_interfaces,
			  pl->sfp_interfaces);

Previously such a module was rejected with "unsupported SFP module: no
common interface modes".

I could not find an in-tree PHY driver that exports 1000BASEX in
possible_interfaces, nor a qcom arm64 DT that pairs ethqos with an sfp
node, so this may not be reachable with in-tree configurations today.  Is
an ethqos board DT with an SFP cage expected to work here, and if so should
ethqos_mac_finish_serdes() also handle PHY_INTERFACE_MODE_1000BASEX (or
should the interface be masked out for platforms that can't program it)?

>  	/* If we have an xpcs, it defines which PHY interfaces are supported. */
>  	if (priv->hw->xpcs)
>  		pcs = xpcs_to_phylink_pcs(priv->hw->xpcs);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/SJ2PR11MB8401F5C3A075DE2A0013E063E4872%40SJ2PR11MB8401.namprd11.prod.outlook.com

Re: [PATCH net v2 2/3] net: stmmac: advertise integrated PCS interfaces to phylink

[Maxime Chevallier]

Hello Thomas,

On 9/18/26 06:50, Thomas Karpiniec wrote:
> From: Thomas Karpiniec <tom@wendy.sh>
> 
> stmmac_phylink_setup() adds interfaces reported by platform
> get_interfaces() and non-integrated PCS instances to
> config->supported_interfaces, but omits the integrated PCS.
> 
> On qcom-ethqos with 2500BASE-X selected by firmware, this leaves SGMII
> out of the supported interface bitmap, causing phylink to remove the
> lower-speed modes of a switching PHY such as QCA8081.
> 
> Add the integrated PCS interface bitmap to config->supported_interfaces.
> 
> Fixes: 68cff4fff61f ("net: stmmac: add BASE-X support to integrated PCS")
> Cc: stable@vger.kernel.org # 7.0+
> Assisted-by: LLM sparse
> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>

Looks good to me, thanks !

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime


> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 1350e92943ac..f956e0d2ec64 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1414,6 +1414,12 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
>  		__set_bit(priv->plat->phy_interface,
>  			  config->supported_interfaces);
>  
> +	/* If we have an integrated PCS, add the interfaces it supports. */
> +	if (priv->integrated_pcs)
> +		phy_interface_or(config->supported_interfaces,
> +				 config->supported_interfaces,
> +				 priv->integrated_pcs->pcs.supported_interfaces);
> +
>  	/* If we have an xpcs, it defines which PHY interfaces are supported. */
>  	if (priv->hw->xpcs)
>  		pcs = xpcs_to_phylink_pcs(priv->hw->xpcs);

[PATCH net v2 2/3] net: stmmac: advertise integrated PCS interfaces to phylink

[Thomas Karpiniec]

From: Thomas Karpiniec <tom@wendy.sh>

stmmac_phylink_setup() adds interfaces reported by platform
get_interfaces() and non-integrated PCS instances to
config->supported_interfaces, but omits the integrated PCS.

On qcom-ethqos with 2500BASE-X selected by firmware, this leaves SGMII
out of the supported interface bitmap, causing phylink to remove the
lower-speed modes of a switching PHY such as QCA8081.

Add the integrated PCS interface bitmap to config->supported_interfaces.

Fixes: 68cff4fff61f ("net: stmmac: add BASE-X support to integrated PCS")
Cc: stable@vger.kernel.org # 7.0+
Assisted-by: LLM sparse
Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1350e92943ac..f956e0d2ec64 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1414,6 +1414,12 @@ static int stmmac_phylink_setup(struct stmmac_priv *priv)
 		__set_bit(priv->plat->phy_interface,
 			  config->supported_interfaces);
 
+	/* If we have an integrated PCS, add the interfaces it supports. */
+	if (priv->integrated_pcs)
+		phy_interface_or(config->supported_interfaces,
+				 config->supported_interfaces,
+				 priv->integrated_pcs->pcs.supported_interfaces);
+
 	/* If we have an xpcs, it defines which PHY interfaces are supported. */
 	if (priv->hw->xpcs)
 		pcs = xpcs_to_phylink_pcs(priv->hw->xpcs);
-- 
2.54.0 (Apple Git-157)