Re: Aquantia PHY in OCSGMII mode?

[Vladimir Oltean <vladimir.oltean@nxp.com>]

[-- Attachment #1: Type: text/plain, Size: 10010 bytes --]

On Wed, Oct 08, 2025 at 09:47:28AM +0200, Alexander Wilhelm wrote:
> On Tue, Oct 07, 2025 at 05:08:19PM +0300, Vladimir Oltean wrote:
> > Hi Alexander,
> [...]
> > Sorry for the delay. What you have found are undoubtebly two major bugs,
> > causing the Lynx PCS to operate in undefined behaviour territory.
> > Nonetheless, while your finding has helped me discover many previously
> > unknown facts about the hardware IP, I still cannot replicate exactly
> > your reported behaviour. In order to fully process things, I would like
> > to ask a few more clarification questions.
> 
> Sure.
> 
> > Is your U-Boot implementation based on NXP's dtsec_configure_serdes()?
> > https://github.com/u-boot/u-boot/blob/master/drivers/net/fm/eth.c#L57
> 
> Unfortunately, I am working with an older U-Boot version v2016.07. However,
> the bug I fixed was not part of the official U-Boot codebase, it was
> introduced by our team:
> 
>     value = PHY_SGMII_IF_MODE_SGMII;
>     value |= PHY_SGMII_IF_MODE_AN;
> 
> I added the missing `if` condition as follows:
> 
>     if (!sgmii_2500) {
>         value = PHY_SGMII_IF_MODE_SGMII;
>         value |= PHY_SGMII_IF_MODE_AN;
>     }
> 
> With the official U-Boot codebase I don't have a ping at none of the
> speeds:
> 
>     value = PHY_SGMII_IF_MODE_SGMII;
>     if (!sgmii_2500)
>         value |= PHY_SGMII_IF_MODE_AN;
> 
> > Why would U-Boot set IF_MODE_SGMII_EN | IF_MODE_USE_SGMII_AN only when
> > the AQR115 resolves only to 100M, but not in the other cases (which do
> > not have this problem)? Or does it do it irrespective of resolved media
> > side link speed? Simply put: what did the code that you fixed up look like?
> 
> In our implementation, the SGMII flags were always set in U-Boot,
> regardless of the negotiated link speed. My assumption is that the SGMII
> mode configuration results in a behavior where only a 100M link applies the
> 10x symbol replication, while 1G does not. For a 2.5G link, the behavior
> ends up being the same as 1G, since there is no actual SGMII mode for 2.5G.

Yes, this assumption seems to hold water thus far, but I have to
validate it by seeing the debugging print for 1G/2.5G, once we figure
out the debug printing aspect.

> > With the U-Boot fix reverted, could you please replicate the broken
> > setup with AQR115 linking at 100Mbps, and add the following function in
> > Linux drivers/pcs-lynx.c?
> > 
> > static void lynx_pcs_debug(struct mdio_device *pcs)
> > {
> > 	int bmsr = mdiodev_read(pcs, MII_BMSR);
> > 	int bmcr = mdiodev_read(pcs, MII_BMCR);
> > 	int adv = mdiodev_read(pcs, MII_ADVERTISE);
> > 	int lpa = mdiodev_read(pcs, MII_LPA);
> > 	int if_mode = mdiodev_read(pcs, IF_MODE);
> > 
> > 	dev_info(&pcs->dev, "BMSR 0x%x, BMCR 0x%x, ADV 0x%x, LPA 0x%x, IF_MODE 0x%x\n", bmsr, bmcr, adv, lpa, if_mode);
> > }
> > 
> > and call it from:
> > 
> > static void lynx_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
> > 			       struct phylink_link_state *state)
> > {
> > 	struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
> > 
> > 	lynx_pcs_debug(lynx->mdio); // <- here
> > 
> > 	switch (state->interface) {
> > 	...
> > 
> > With this, I would like to know:
> > (a) what is the IF_MODE register content outside of the IF_MODE_SGMII_EN
> >     and IF_MODE_USE_SGMII_AN bits.
> > (b) what is the SGMII code word advertised by the AQR115 in OCSGMII mode.
> > 
> > Then if you could replicate this test for 1Gbps medium link speed, it
> > would be great.
> 
> For now, I have reverted both the U-Boot and kernel fixes and added debug
> outputs for further analysis. Unfortunately the function
> `lynx_pcs_get_state` is never called in my kernel code. Therefore I put the
> debug function into `lynx_pcs_config`. Here is the output:
> 
>     mdio_bus 0x0000000ffe4e5000:00: BMSR 0x29, BMCR 0x1140, ADV 0x4001, LPA 0xdc01, IF_MODE 0x3
> 
> I hope it'll help to analyze the problem further.

Correct. lynx_pcs_get_state() is only called for MLO_AN_INBAND (managed = "in-band-status"),
which the Lynx PCS driver does not currently support for 2500base-x.

However, I don't fully trust the positioning of the debug print into lynx_pcs_config().
The BMCR, ADV and IF_MODE registers look plausible, as if lynx_pcs_config() did what it
was supposed to do, but LPA (link config code word coming from AQR115) looks strange.
Field 11:10 (COP_SPD) is 0b11, which is a reserved value, neither 1G nor 100M nor 10M.
Maybe this is the mythical "SGMII 2500" auto-negotiation? Anyway, I don't think there is
any standard for it, and even if there was, the Lynx PCS doesn't implement it.

I'm surprised your AQR115 would transmit in-band code words for OCSGMII. None of the Aquantia
PHYs I've tested on were able to do that, and I'm not sure what register controls that.
If we look at your previous debugging output of the global system configuration registers:
https://lore.kernel.org/lkml/aJH8n0zheqB8tWzb@FUE-ALEWI-WINX/
we see that for 100M line side, the PHY uses "SerDes mode 4 autoneg 0". I also tried modifying
aqr_gen2_config_inband() to set VEND1_GLOBAL_CFG_AUTONEG_ENA for OCSGMII, but it didn't appear
to change anything, so that's probably not the setting. I'll have to ask somebody at Marvell.

In any case, contrary to my previous beliefs and according to your finding plus my parallel
testing, the Lynx PCS actually supports in-band auto-negotiation at 2500 data rate - both
2500base-x auto-negotiation and SGMII auto-negotiation (to the extent that this is a thing
that actually makes sense - it doesn't).

With IF_MODE=3 (SGMII_EN | USE_SGMII_AN), the PCS will automatically reconfigure the data path
for the speed decoded in hardware from the LPA_SGMII_SPD_MASK bits. Apparently it does this
for the lane data rate of 2500 just the same as it does it for 1000, just that the
LPA_SGMII_SPD_MASK bits need to be 0b00 (gigabit) for traffic to pass. Otherwise, it tries to
perform symbol replication (as per your hw engineer's claim), and that didn't work in my testing
either(* details at the end).

I'm not saying that IF_MODE=3 is a valid configuration when the lane data rate is 2500.
It absolutely isn't, and your patch which changes IF_MODE to 0 seems ok. I'm just trying to
understand and then re-explain what the PCS does when configured in this mode, based on the
evidence.

Specifically when LPA_SGMII_SPD_MASK/COP_SPD is 0b11, it isn't documented how it would behave,
I don't have a protocol analyzer to count replicated symbols, and I'm unable to obtain a
functional data path to measure bandwidth with iperf3. Your hardware engineer's claim remains
the most trustworthy source of information we have.

Regarding lynx_pcs_get_state(): I actually was working on the patch attached, which I had in my
tree and I didn't realize it would impact your testing. I would kindly ask you to apply as well.
Applying it alone would be enough to fix the IF_MODE=3 problem, but fixing the problem is not
what we want, instead we want to see the MII_LPA register value at lynx_pcs_get_state() time,
and for multiple link speeds.

For that, please break the link again, by making the following changes on top:

1. Configure IF_MODE=3 (SGMII autoneg format) for 2500base-x:

diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index a88cbe67cc9d..ea42b8d813f3 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -152,11 +152,10 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs,
 		mdiodev_write(pcs, LINK_TIMER_HI, link_timer >> 16);
 	}

-	if (interface == PHY_INTERFACE_MODE_1000BASEX ||
-	    interface == PHY_INTERFACE_MODE_2500BASEX) {
+	if (interface == PHY_INTERFACE_MODE_1000BASEX) {
 		if_mode = 0;
 	} else {
-		/* SGMII and QSGMII */
+		/* SGMII, QSGMII and (incorrectly) 2500base-x */
 		if_mode = IF_MODE_SGMII_EN;
 		if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 			if_mode |= IF_MODE_USE_SGMII_AN;

2. Edit your MAC OF node in the device tree and add:

&mac {
	managed = "in-band-status";  // this
	phy-mode = "2500base-x";
};

This will reliably cause the same behaviour as before, but with no dependency on U-Boot.

Because aqr_gen2_inband_caps() (code added recently to net-next) returns 0 (unknown)
for 2500base-x, phylink doesn't know whether the PHY will send or not in-band code words.
It will have to trust the firmware description with managed = "in-band-status", which will
lead neg_mode to be PHYLINK_PCS_NEG_INBAND_ENABLED, which will cause IF_MODE_USE_SGMII_AN
to be set.

Note: this is something else we'll have to look at later too. What bits control "OCSGMII"
link codeword transmission in Aquantia PHYs?

Your earlier assumption about why 100M is broken but 1G / 2.5G are not
only holds water if, as a result of testing at these other link speeds,
you find the MII_LPA register to contain 0b10 (Gigabit) in bits 11:10
(COP_SPD / LPA_SGMII_SPD_MASK). Aka it worked, but it was purely accidental,
because phylink thought 2500base-x would not use autoneg, yet it did,
and by some miracle the SGMII format coincided and resulted in no change
in the link characteristics.

Regarding my patch vs yours, my thoughts on this topic are: the bug is
old, the PCS driver never worked if the registers were not as expected
(this is not a regression), and your patch is incomplete if MII_BMCR
also contains significant differences. I would recommend submitting
mine, as a new feature to net-next, when it reopens for patches for 6.19.
I've credited you with Co-developed-by due to the significance of your
findings. Thanks.

(*) When testing forced 10x SGMII symbol replication on a 3.125 Gbps
lane over a pair of optical SFP+ modules connected between two LS1028A Lynx PCS
blocks, I can see packets being transmitted, but on the receiver, the
RFRG mEMAC counter increases (rx_fragments). The documentation says for this:
"Incremented for each packet which is shorter than 64 octets received with
a wrong CRC. (Fragments are not delivered to the FIFO interface.)"
Without dedicated equipment, I'm unsure how to push this further.

[-- Attachment #2: 0001-net-pcs-lynx-accept-in-band-autoneg-for-2500base-x.patch --]
[-- Type: text/x-diff, Size: 4689 bytes --]

From 19a375a81f3dae953410f8da607e89c34095a21c Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Fri, 3 Oct 2025 17:20:20 +0300
Subject: [PATCH] net: pcs: lynx: accept in-band autoneg for 2500base-x

Testing in two circumstances:

1. back to back optical SFP+ connection between two LS1028A-QDS ports
   with the SCH-26908 riser card
2. T1042 with on-board AQR115 PHY using "OCSGMII", as per
   https://lore.kernel.org/lkml/aIuEvaSCIQdJWcZx@FUE-ALEWI-WINX/

strongly suggests that enabling in-band auto-negotiation is actually
possible when the lane baud rate is 3.125 Gbps.

It was previously thought that this would not be the case, because it
was only tested on 2500base-x links with on-board Aquantia PHYs, where
it was noticed that MII_LPA is always reported as zero, and it was
thought that this is because of the PCS.

Test case #1 above shows it is not, and the configured MII_ADVERTISE on
system A ends up in the MII_LPA on system B, when in 2500base-x mode
(IF_MODE=0).

Test case #2, which uses "SGMII" auto-negotiation (IF_MODE=3) for the
3.125 Gbps lane, is actually a misconfiguration, but it is what led to
the discovery.

There is actually an old bug in the Lynx PCS driver - it expects all
register values to contain their default out-of-reset values, as if the
PCS were initialized by the Reset Configuration Word (RCW) settings.
There are 2 cases in which this is problematic:
- if the bootloader (or previous kexec-enabled Linux) wrote a different
  IF_MODE value
- if dynamically changing the SerDes protocol from 1000base-x to
  2500base-x, e.g. by replacing the optical SFP module.

Specifically in test case #2, an accidental alignment between the
bootloader configuring the PCS to expect SGMII in-band code words, and
the AQR115 PHY actually transmitting SGMII in-band code words when
operating in the "OCSGMII" system interface protocol, led to the PCS
transmitting replicated symbols at 3.125 Gbps baud rate. This could only
have happened if the PCS saw and reacted to the SGMII code words in the
first place.

Since test #2 is invalid from a protocol perspective (there seems to be
no standard way of negotiating the data rate of 2500 Mbps with SGMII,
and the lower data rates should remain 10/100/1000), in-band auto-negotiation
for 2500base-x effectively means Clause 37 (i.e. IF_MODE=0).

Make 2500base-x be treated like 1000base-x in this regard, by removing
all prior limitations and calling lynx_pcs_config_giga().

This adds a new feature: LINK_INBAND_ENABLE and at the same time fixes
the Lynx PCS's long standing problem that the registers (specifically
IF_MODE, but others could be misconfigured as well) are not written by
the driver to the known valid values for 2500base-x.

Co-developed-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/pcs/pcs-lynx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index 677f92883976..a88cbe67cc9d 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -40,12 +40,12 @@ static unsigned int lynx_pcs_inband_caps(struct phylink_pcs *pcs,
 {
 	switch (interface) {
 	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
 		return LINK_INBAND_DISABLE | LINK_INBAND_ENABLE;
 
 	case PHY_INTERFACE_MODE_10GBASER:
-	case PHY_INTERFACE_MODE_2500BASEX:
 		return LINK_INBAND_DISABLE;
 
 	case PHY_INTERFACE_MODE_USXGMII:
@@ -152,7 +152,8 @@ static int lynx_pcs_config_giga(struct mdio_device *pcs,
 		mdiodev_write(pcs, LINK_TIMER_HI, link_timer >> 16);
 	}
 
-	if (interface == PHY_INTERFACE_MODE_1000BASEX) {
+	if (interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    interface == PHY_INTERFACE_MODE_2500BASEX) {
 		if_mode = 0;
 	} else {
 		/* SGMII and QSGMII */
@@ -202,15 +203,9 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 	case PHY_INTERFACE_MODE_1000BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_QSGMII:
+	case PHY_INTERFACE_MODE_2500BASEX:
 		return lynx_pcs_config_giga(lynx->mdio, ifmode, advertising,
 					    neg_mode);
-	case PHY_INTERFACE_MODE_2500BASEX:
-		if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {
-			dev_err(&lynx->mdio->dev,
-				"AN not supported on 3.125GHz SerDes lane\n");
-			return -EOPNOTSUPP;
-		}
-		break;
 	case PHY_INTERFACE_MODE_USXGMII:
 	case PHY_INTERFACE_MODE_10G_QXGMII:
 		return lynx_pcs_config_usxgmii(lynx->mdio, ifmode, advertising,
-- 
2.34.1