[PATCH net-next 0/2] net: pcs: xpcs: fix operating mode selection

[PATCH net-next 0/2] net: pcs: xpcs: fix operating mode selection

[Alex Elder]

The XPCS found on Toshiba TC9564 is unable to operate at 2500base-X
and slower with a PHY connected using SGMII/2500base-X.  This XPCS
supports 10Gbase-R, which is the default operating mode, and this
causes mode selection to fail for slower interfaces (SGMII, 1000baseX,
and 2500baseX).  Writing a reserved value to the MDIO_CTRL2 register
avoids this failure.

Wangxun already addresses a similar scenario, but includes custom
quirk handling.

This series implements handling for slower rates for an XPCS that
supports 10Gbase-R.  The first patch rearranges code that performs
soft reset so that it is done *before* the interface mode is
switched.  The second adds code to xpcs_switch_interface_mode()
to update the MDIO_CTRL2 register for interfaces that support
10Gbase-R so that the desired rate change works as intended.

					-Alex

Daniel Thompson (2):
  net: pcs: xpcs: re-order xpcs_pre_config() to update after the reset
  net: pcs: xpcs: select operating mode for 10G-baseR capable PCS

 drivers/net/pcs/pcs-xpcs.c | 94 +++++++++++++++++++++++++++-----------
 1 file changed, 67 insertions(+), 27 deletions(-)


base-commit: 05b5019b01fb9443b09ae4ed34af1135a80a84c8
-- 
2.53.0

[PATCH net-next 1/2] net: pcs: xpcs: re-order xpcs_pre_config() to update after the reset

[Alex Elder]

From: Daniel Thompson <daniel@riscstar.com>

Currently, on Wangxun platforms, the XPCS is configured during the call
to xpcs_switch_interface_mode() and, if the need_reset flag is set, the
XPCS is reset and the configuration will be lost. This is harmless at
present because need_reset will never actually be set on these platforms.

Nevertheless having xpcs_switch_interface_mode() on the wrong side of
the reset is an obstacle for future changes where wiping out programmed
configuration with a reset could be harmful.

Reorder xpcs_pre_config() so the reset can happen before we switch
interface mode. To make this work we have to hoist the special case
logic for SGMII into the parent function.

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/net/pcs/pcs-xpcs.c | 59 ++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index b415b93d77c15..49abf5ea6f5cc 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -705,46 +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 ret = 0;
+	/* 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);
 
-	if (xpcs->info.pma == WX_TXGBE_XPCS_PMA_10G_ID) {
-		ret = txgbe_xpcs_switch_mode(xpcs, interface);
-	} else if (xpcs->interface != interface) {
-		if (interface == PHY_INTERFACE_MODE_SGMII)
-			xpcs->need_reset = true;
-		xpcs->interface = interface;
-	}
+	xpcs->interface = interface;
 
-	return ret;
+	return 0;
 }
 
 static void xpcs_pre_config(struct phylink_pcs *pcs, phy_interface_t interface)
 {
 	struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
+	struct device *dev = &xpcs->mdiodev->dev;
 	const struct dw_xpcs_compat *compat;
+	bool force_reset;
 	int ret;
 
+	/*
+	 * According to the XPCS datasheet, a soft reset is required to initiate
+	 * Clause 37 auto-negotiation when the XPCS switches interface modes.
+	 */
+	force_reset = interface == PHY_INTERFACE_MODE_SGMII;
+
+	if (force_reset || xpcs->need_reset) {
+		compat = xpcs_find_compat(xpcs, interface);
+		if (!compat) {
+			dev_err(dev, "unsupported interface %s\n",
+				phy_modes(interface));
+			return;
+		}
+
+		ret = xpcs_soft_reset(xpcs, compat);
+		if (ret) {
+			dev_err(dev, "soft reset failed: %pe\n", ERR_PTR(ret));
+			return;
+		}
+
+		xpcs->need_reset = false;
+	}
+
 	ret = xpcs_switch_interface_mode(xpcs, interface);
 	if (ret)
-		dev_err(&xpcs->mdiodev->dev, "switch interface failed: %pe\n",
-			ERR_PTR(ret));
-
-	if (!xpcs->need_reset)
-		return;
-
-	compat = xpcs_find_compat(xpcs, interface);
-	if (!compat) {
-		dev_err(&xpcs->mdiodev->dev, "unsupported interface %s\n",
-			phy_modes(interface));
-		return;
-	}
-
-	ret = xpcs_soft_reset(xpcs, compat);
-	if (ret)
-		dev_err(&xpcs->mdiodev->dev, "soft reset failed: %pe\n",
-			ERR_PTR(ret));
-
-	xpcs->need_reset = false;
+		dev_err(dev, "switch interface failed: %pe\n", ERR_PTR(ret));
 }
 
 static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
-- 
2.53.0

[PATCH net-next 2/2] net: pcs: xpcs: select operating mode for 10G-baseR capable PCS

[Alex Elder]

From: Daniel Thompson <daniel@riscstar.com>

Currently the XPCS found on Toshiba TC9564 is unable to operate at
2500base-X and slower with a PHY connected using SGMII/2500base-X
(in our case a Qualcomm QCA8081).

The problem arises because this XPCS supports 10Gbase-R. That means
that the reset value of SR_XS_PCS_CTRL2:PCS_TYPE_SEL (0) is valid and
this suppresses the modal switching based on bit 13 of SR_PMA_CTRL1
or SR_XS_PCS_CTRL1.

A fix for this behaviour is implemented by txgbe_xpcs_switch_mode()
as part of the quirks for WangXun devices.

Rather than introduce another quirk for TC9564 let's attempt so solve
this generically by setting SR_XS_PCS_CTRL2:PCS_TYPE_SEL to a reserved
value when we detect the right combination of phy interface and XPCS
feature support.

The generic strategy adopted requires the default value of PCS_TYPE_SEL
to be 0 on devices that support 10Gbase-R. Based on TC9564 documentation
and the logic already implemented for WangXun I believe this is likely
to be the case for currently supported XPCS devices. Sadly I don't have
access to generic XPCS docs to confirm. However I think the benefits
of avoiding a cargo culted quirk outweighs the risk of regression.

Signed-off-by: Daniel Thompson <daniel@riscstar.com>
Co-developed-by: Alex Elder <elder@riscstar.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/net/pcs/pcs-xpcs.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 49abf5ea6f5cc..5316cf9c31f19 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -705,10 +705,47 @@ 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) {
+		switch (interface) {
+		case PHY_INTERFACE_MODE_SGMII:
+		case PHY_INTERFACE_MODE_1000BASEX:
+		case PHY_INTERFACE_MODE_2500BASEX:
+			/*
+			 * Why are we writing MDIO_PCS_CTRL2_TYPE + 1? We want
+			 * the modal behaviour that comes when we pick a
+			 * reserved value. XPCS allocates extra bits to this
+			 * field and allocates values from 15 down so
+			 * MDIO_PCS_CTRL2_TYPE + 1 is the value likely to be
+			 * allocated last (and hopefully never).
+			 */
+			ret = xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL2,
+					 MDIO_PCS_CTRL2_TYPE + 1);
+			break;
+		default:
+			ret = xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL2,
+					 MDIO_PCS_CTRL2_10GBR);
+			break;
+		}
+		if (ret < 0)
+			return ret;
+	}
+
 	xpcs->interface = interface;
 
 	return 0;
-- 
2.53.0

Re: [PATCH net-next 0/2] net: pcs: xpcs: fix operating mode selection

[Andrew Lunn]

On Wed, Oct 14, 2026 at 05:30:57PM -0500, Alex Elder wrote:
> The XPCS found on Toshiba TC9564 is unable to operate at 2500base-X
> and slower with a PHY connected using SGMII/2500base-X.  This XPCS
> supports 10Gbase-R, which is the default operating mode, and this
> causes mode selection to fail for slower interfaces (SGMII, 1000baseX,
> and 2500baseX).  Writing a reserved value to the MDIO_CTRL2 register
> avoids this failure.
> 
> Wangxun already addresses a similar scenario, but includes custom
> quirk handling.
> 
> This series implements handling for slower rates for an XPCS that
> supports 10Gbase-R.  The first patch rearranges code that performs
> soft reset so that it is done *before* the interface mode is
> switched.  The second adds code to xpcs_switch_interface_mode()
> to update the MDIO_CTRL2 register for interfaces that support
> 10Gbase-R so that the desired rate change works as intended.

You Cc: those reported by get_maintainers, but i think a few other
people should be involved, who have made changes in this area:

Coia Prant <coiaprant@gmail.com>
Choong Yong Liang <yong.liang.choong@linux.intel.com>
Vladimir Oltean <vladimir.oltean@nxp.com>
Jiawen Wu <jiawenwu@trustnetic.com>

       Andrew

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

[Andrew Lunn]

I don't know this PCS driver, so maybe dumb questions....

> Currently the XPCS found on Toshiba TC9564 is unable to operate at
> 2500base-X and slower with a PHY connected using SGMII/2500base-X
> (in our case a Qualcomm QCA8081).

Is the PHY actually relevant here? What about SGMII/2500base-X for an
SFP?

Is this purely a PCS problem, or does what the PCS connect to make a
difference?

> The problem arises because this XPCS supports 10Gbase-R. That means
> that the reset value of SR_XS_PCS_CTRL2:PCS_TYPE_SEL (0) is valid and
> this suppresses the modal switching based on bit 13 of SR_PMA_CTRL1
> or SR_XS_PCS_CTRL1.

~/linux/drivers/net/pcs$ grep SR_XS_PCS_CTRL1 *
~/linux/drivers/net/pcs$ grep SR_PMA_CTRL1 *
~/linux/drivers/net/pcs$

This is not helping me understand what is going on here.

> +	if (mdio_stat2 & MDIO_PCS_STAT2_10GBR) {
> +		switch (interface) {
> +		case PHY_INTERFACE_MODE_SGMII:
> +		case PHY_INTERFACE_MODE_1000BASEX:
> +		case PHY_INTERFACE_MODE_2500BASEX:
> +			/*
> +			 * Why are we writing MDIO_PCS_CTRL2_TYPE + 1? We want
> +			 * the modal behaviour that comes when we pick a
> +			 * reserved value. XPCS allocates extra bits to this
> +			 * field and allocates values from 15 down so
> +			 * MDIO_PCS_CTRL2_TYPE + 1 is the value likely to be
> +			 * allocated last (and hopefully never).

MDIO_PCS_CTRL2_TYPE is 3. Looking at 802.3, the value 4 is
40GBASE-R. The 2022 version of the standard has values defined up to
10011b, for speeds up to 400G,

Alex added support for 25G, 50G, 100G.

Is this an issue?

	Andrew

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

[Daniel Thompson]

On Thu, Oct 15, 2026 at 11:09:14PM +0200, Andrew Lunn wrote:
> I don't know this PCS driver, so maybe dumb questions....
>
> > Currently the XPCS found on Toshiba TC9564 is unable to operate at
> > 2500base-X and slower with a PHY connected using SGMII/2500base-X
> > (in our case a Qualcomm QCA8081).
>
> Is the PHY actually relevant here? What about SGMII/2500base-X for an
> SFP?

No, I don't think the phy is relevant here (the bit in brackets just
records the specific hardware combination that we observed the problem
on).


> Is this purely a PCS problem, or does what the PCS connect to make a
> difference?

AFAIK it is purely a PCS problem (but it's all soldered down so I have
no chance to prove I am right by testing it).


> > The problem arises because this XPCS supports 10Gbase-R. That means
> > that the reset value of SR_XS_PCS_CTRL2:PCS_TYPE_SEL (0) is valid and
> > this suppresses the modal switching based on bit 13 of SR_PMA_CTRL1
> > or SR_XS_PCS_CTRL1.
>
> ~/linux/drivers/net/pcs$ grep SR_XS_PCS_CTRL1 *
> ~/linux/drivers/net/pcs$ grep SR_PMA_CTRL1 *
> ~/linux/drivers/net/pcs$
>
> This is not helping me understand what is going on here.

These names are exactly as they appear in the technical documentation
for the TC9564. I assumed these to be inherited from the Synopsis XPCS
docs and would make sense in the context of the XPCS driver.

However, now you mention it I can see that "Speed selection bit (13) of
PCS control 1" would be considerably clearer for reviewers who take a
wider view! Will fix this and will also be a bit clearer about how XPCS
treats PCS_TYPE_SEL.


> > +	if (mdio_stat2 & MDIO_PCS_STAT2_10GBR) {
> > +		switch (interface) {
> > +		case PHY_INTERFACE_MODE_SGMII:
> > +		case PHY_INTERFACE_MODE_1000BASEX:
> > +		case PHY_INTERFACE_MODE_2500BASEX:
> > +			/*
> > +			 * Why are we writing MDIO_PCS_CTRL2_TYPE + 1? We want
> > +			 * the modal behaviour that comes when we pick a
> > +			 * reserved value. XPCS allocates extra bits to this
> > +			 * field and allocates values from 15 down so
> > +			 * MDIO_PCS_CTRL2_TYPE + 1 is the value likely to be
> > +			 * allocated last (and hopefully never).
>
> MDIO_PCS_CTRL2_TYPE is 3. Looking at 802.3, the value 4 is
> 40GBASE-R. The 2022 version of the standard has values defined up to
> 10011b, for speeds up to 400G,
>
> Alex added support for 25G, 50G, 100G.
>
> Is this an issue?

It will probably be an issue when the kernel is first called up on to
support an XPCS that supports 40GBASE-R.

The XPCS (at least in TC956x) handles MDIO_PCS_CTRL2 in a slightly odd way.
It 4 bits wide and accepts 10GBASE-R, 10GBASE-X, 5GBASE-R and 2.5GBASE-X
is defined in the standards up to 2018. All other bit patterns are
reserved and instruct the XPCS to use Speed selection bit (13) of PCS
control 1 to choose between 10GBASE-X or 1000BASE-X.

That means we *must* write a reserved value to this register because the
default value after a reset prevents <=1G modes from working (I think
automatic switch over between 1G and 2.5G also liked this to a reserved
value but I don't fully recall right now).

On that basis the best bit pattern to use one that isn't implemented by
whatever XPCS we happen to be running on. One simple approach might be
keep using 40GBASE-R but to check PCS Status 2 to ensure we fail in a
non-silent manner on hypothesized future XPCS hardware:

	if (mdio_stat2 & MDIO_PCS_STAT2_10GBR) {
		if (mdio_stat2 & MDIO_PCS_STAT2_40GBR) {
			dev_err(dev, "40GBASE-R is *not* a reserved "
			              value on this XPCS");
			return -ENOTSUP;
		}


		switch (interface) {
		case PHY_INTERFACE_MODE_SGMII:
		case PHY_INTERFACE_MODE_1000BASEX:
		case PHY_INTERFACE_MODE_2500BASEX:
			/* better comment goes here */
			ret = xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL2,
					 MDIO_PCS_CTRL2_40GBR);
			...
		}
		...
	}

Given this function would almost certainly need other changes to
support different speeds this shoudn't be too much of a hardship
for whoever ends up working on a 40GBASE-R-capable XPCS.

I assume it would be OK to define the two new values
(MDIO_PCS_STAT2_40GBR and MDIO_PCS_CTRL2_40GBR) in the uapi?


Daniel.