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

[Daniel Thompson <daniel@riscstar.com>]

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.