[PATCH net v2] net: pcs: xpcs: make supported_interfaces authoritative

[PATCH net v2] net: pcs: xpcs: make supported_interfaces authoritative

[Coia Prant]

pcs.supported_interfaces is the mask of interfaces a given xpcs
instance actually supports. The compat table describes what the
Synopsys IP can do in general, but a specific integration may support
a different subset: the RK3568 XPCS supports 1000BASE-X, SGMII and
QSGMII in hardware, but does not implement the C73 10G or 2500BASE-X
modes that the Synopsys compat table advertises.

The Rockchip glue currently advertises only SGMII, since that is what
the RK3568 MAC side supports today. 1000BASE-X and QSGMII can be added
later when the MAC side gains support.

Without narrowing the mask, stmmac_phylink_setup() merges the PCS
supported_interfaces into the MAC mask with phy_interface_or(), so the
XPCS would contribute 1000BASE-X, 2500BASE-X, USXGMII and the 10G/25G
modes to the combined set. The MAC side forces the phy modes and
phylink start passes the mode explicitly, so this does not currently
make phylink switch to an unsupported mode, but the combined mask still
advertises interfaces the SoC cannot use.

However, xpcs_find_compat() only walks desc->compat[] and does not look
at pcs.supported_interfaces. If a caller ever passes an interface
outside the mask, the core would still find and configure a compat
entry for it, contradicting the mask the glue already set. This was a
known gap: the mask was written but never consulted by the core.

Fix both sides:

- In xpcs_find_compat(), return NULL when the interface is not in
  pcs.supported_interfaces, so the mask is authoritative for what the
  core will configure.
- In the Rockchip glue, clear the mask inherited from the compat table
  and advertise only SGMII, matching what the RK3568 integration
  exposes today.

This is a logical inconsistency rather than a user-visible bug in a
released kernel: the affected Rockchip platform is not present in any
released version, and the MAC side forces the phy mode, so phylink does
not normally pass an interface outside the mask. The patch makes the
mask the single source of truth for both the core and the glue.

1000BASE-X and QSGMII can be enabled later when the MAC side supports
them. The C73 10G and 2500BASE-X modes are not implemented on RK3568
and are cleared by the mask.

Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 drivers/net/pcs/pcs-xpcs-rk.c | 6 ++++++
 drivers/net/pcs/pcs-xpcs.c    | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
index 6f0a110e32d2..cc11a70d624f 100644
--- a/drivers/net/pcs/pcs-xpcs-rk.c
+++ b/drivers/net/pcs/pcs-xpcs-rk.c
@@ -536,6 +536,12 @@ struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *pcs, int
 		return xpcs;
 	}
 
+	/*
+	 * The RK3568 XPCS supports 1000BASE-X, SGMII and QSGMII.
+	 */
+	phy_interface_zero(xpcs->pcs.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_SGMII, xpcs->pcs.supported_interfaces);
+
 	xpcs_config_eee_mult_fact(xpcs, pxpcs->eee_mult_fact);
 	put_device(&pdev->dev);
 	return xpcs;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 6466e0ff2a98..46878a66064e 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -169,6 +169,9 @@ xpcs_find_compat(struct dw_xpcs *xpcs, phy_interface_t interface)
 {
 	const struct dw_xpcs_compat *compat;
 
+	if (!test_bit(interface, xpcs->pcs.supported_interfaces))
+		return NULL;
+
 	for (compat = xpcs->desc->compat; compat->supported; compat++)
 		if (compat->interface == interface)
 			return compat;
-- 
2.47.3

Re: [PATCH net v2] net: pcs: xpcs: make supported_interfaces authoritative

[Andrew Lunn]

> +	/*
> +	 * The RK3568 XPCS supports 1000BASE-X, SGMII and QSGMII.
> +	 */
> +	phy_interface_zero(xpcs->pcs.supported_interfaces);
> +	__set_bit(PHY_INTERFACE_MODE_SGMII, xpcs->pcs.supported_interfaces);

The comment and the code does not really match. Why not set the other
two interface modes as well?

    Andrew

Re: [PATCH net v2] net: pcs: xpcs: make supported_interfaces authoritative

[Andrew Lunn]

On Thu, Oct 29, 2026 at 06:27:29PM +0800, Coia Prant wrote:
> pcs.supported_interfaces is the mask of interfaces a given xpcs
> instance actually supports. The compat table describes what the
> Synopsys IP can do in general, but a specific integration may support
> a different subset: the RK3568 XPCS supports 1000BASE-X, SGMII and
> QSGMII in hardware, but does not implement the C73 10G or 2500BASE-X
> modes that the Synopsys compat table advertises.
> 
> The Rockchip glue currently advertises only SGMII, since that is what
> the RK3568 MAC side supports today. 1000BASE-X and QSGMII can be added
> later when the MAC side gains support.
> 
> Without narrowing the mask, stmmac_phylink_setup() merges the PCS
> supported_interfaces into the MAC mask with phy_interface_or(), so the
> XPCS would contribute 1000BASE-X, 2500BASE-X, USXGMII and the 10G/25G
> modes to the combined set. The MAC side forces the phy modes and
> phylink start passes the mode explicitly, so this does not currently
> make phylink switch to an unsupported mode, but the combined mask still
> advertises interfaces the SoC cannot use.

I don't know this code too well, but something does not feel right
here. It might be due to the way stmmac does PCS with phylink, which
is often considered broken.

What i would expect to happen is that each piece of hardware indicates
what it can do, and then phylink finds the common modes.

So maybe stmmac_phylink_setup() is wrong when it ORs them together?

Maybe look at other MAC drivers using pcs and phylink.

      Andrew