net: stmmac: phylink PCS conversion part 3 (dodgy stuff)

Signed-off-by Russell King (Oracle) <[email protected]>

Hello Russell,

Quoted 1 line · Russell King (Oracle)

Signed-off-by Russell King (Oracle) <[email protected]>

Maybe this needs a commit log, even a small one ? :(

Thanks for giving Jakub a reason to mark this "changes required." :D I'm not really expecting this to be merged as-is. So why didn't I post it as RFC? Too many people see "RFC" as a sign to ignore the patch series. Some people claim that "RFC" means it isn't ready and thus isn't worth reviewing/testing/etc. I say to those people... I can learn their game and work around their behaviour.

Yes, it will need a better commit log, but what I'm much much more interested in is having people who are using the integrated PCS (in SGMII mode as that's all we support) to test this, especially dwmac-qcom-ethqos folk.

The 2.5G support was submitted by Sneh Shah, and my attempts to make contact have resulted in no response.

Quoted 5 lines · Maxime Chevallier

Hello Russell,

Quoted 1 line · Russell King (Oracle)

Signed-off-by Russell King (Oracle) <[email protected]>

Quoted 1 line

Maybe this needs a commit log, even a small one ? :(

Thanks for giving Jakub a reason to mark this "changes required." :D I'm not really expecting this to be merged as-is. So why didn't I post it as RFC? Too many people see "RFC" as a sign to ignore the patch series. Some people claim that "RFC" means it isn't ready and thus isn't worth reviewing/testing/etc. I say to those people... I can learn their game and work around their behaviour.

Yes, it will need a better commit log, but what I'm much much more interested in is having people who are using the integrated PCS (in SGMII mode as that's all we support) to test this, especially dwmac-qcom-ethqos folk.

The 2.5G support was submitted by Sneh Shah, and my attempts to make contact have resulted in no response.

Hi,

This series is currently the last of the phylink PCS conversion for stmmac. This series contains changes that will cause potential breakage, so I suggest to netdev maintainers that it is only applied if there is a significant response from testers using the PCS.

Paritcularly, dwmac-qcom-ethqos.c users need to test this, since this platform glue driver manipulates the PCS state. Patch 2 is designed to print a warning to the kernel log if this glue driver calls stmmac_pcs_ctrl_ane() to set the AN state differently to how phylink has set it. If this happens, we need to do some pre-work to prevent these prints.

 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  7 +++++-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  7 +++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c   | 29 +++++++++++++++++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h   |  8 +++++-
 4 files changed, 44 insertions(+), 7 deletions(-)

Hi Russell,

Quoted 14 lines · Russell King (Oracle)

Hi,

This series is currently the last of the phylink PCS conversion for stmmac. This series contains changes that will cause potential breakage, so I suggest to netdev maintainers that it is only applied if there is a significant response from testers using the PCS.

Paritcularly, dwmac-qcom-ethqos.c users need to test this, since this platform glue driver manipulates the PCS state. Patch 2 is designed to print a warning to the kernel log if this glue driver calls stmmac_pcs_ctrl_ane() to set the AN state differently to how phylink has set it. If this happens, we need to do some pre-work to prevent these prints.

Quoted 6 lines

 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  7 +++++-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |  7 +++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c   | 29 +++++++++++++++++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h   |  8 +++++-
 4 files changed, 44 insertions(+), 7 deletions(-)

Thank you for the recent stmmac cleanup patches. I apologize for the late reply. I had limited time to test due to some urgent tasks at work. This is a long email, please bear with me.

I have the following devices on which I try to test whatever I can (both of them have the same GMAC core with an integrated PCS, both use phy-mode="2500base-x"):
- Qualcomm QCS9100 Ride R3 (2xAQR115C PHYs)
- Qualcomm IQ8 EVK (QCA808X PHY) - this is the same board for which I had posted [1] to resolve its issue with advertising only 2.5G

# Patch Series: net: stmmac: phylink PCS conversion
I tested this series soon after it got merged to net-next, probably when I tested out the hwif.c cleanups. A summary:
- QCS9100 Ride R3 - no issues found.
- IQ8 EVK - same behavior as without this patch, i.e. 2.5G was working fine, other speeds aren't advertised.

However, this might have been expected as both boards are using 2500Base-X whereas the integrated PCS changes are limited to SGMII. *Sidenote*: I was able to get 2.5G and lower speeds to work on the IQ8 EVK after adding an additional case for 2500Base-X on top of your patch.

# Patch Series (current): net: stmmac: phylink PCS conversion part 3 (dodgy stuff)
- QCS9100 Ride R3 - functionality seems to be fine (again, probably due to the changes only affecting SGMII mode). However, the warning added in patch 2 comes up whenever there's a speed change (I added an additional WARN_ON to check the sequence):

  	[   61.663685] qcom-ethqos 23000000.ethernet eth0: Link is Down
	[   66.235461] dwmac: PCS configuration changed from phylink by glue, please report: 0x00001000 -> 0x00000000

Quoted 8 lines

# Patch Series (current): net: stmmac: phylink PCS conversion part 3 (dodgy stuff)
- QCS9100 Ride R3 - functionality seems to be fine (again, probably due to the changes only affecting SGMII mode). However, the warning added in patch 2 comes up whenever there's a speed change (I added an additional WARN_ON to check the sequence):

  	[   61.663685] qcom-ethqos 23000000.ethernet eth0: Link is Down
	[   66.235461] dwmac: PCS configuration changed from phylink by glue, please report: 0x00001000 -> 0x00000000

That's clearing ANE, turning off AN. This will be because we're not using the PCS code for 2500base-X.

Can you try:

1. in stmmac_check_pcs_mode(), as a hack, add:

	if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_2500BASEX)
		priv->hw->pcs = STMMAC_PCS_SGMII;

2. with part 3 added, please change dwmac4_pcs_init() to:

	phy_interface_t modes[] = {
		PHY_INTERFACE_MODE_SGMII,
		PHY_INTERFACE_MODE_2500BASEX,
	};
	...
	return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE,
					  GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE,
					  modes, ARRAY_SIZE(modes));

This will cause the integrated PCS to also be used for 2500BASE-X.

3. modify dwmac_integrated_pcs_inband_caps() to return LINK_INBAND_DISABLE for PHY_INTERFACE_MODE_2500BASEX.

This should result in the warning going away for you.

I'm not suggesting that this is a final solution.

Please note, however, that the stmmac driver does not support on-the-fly reconfiguration of the PHY-side interface as it stands (and questionable whether it ever will do.) The hardware samples phy_intf_sel inputs to the core at reset (including, I believe, software reset) which configures the core to use the appropriate PHY interface. Performing any kind of reset is very disruptive to the core - likely even causes the PTP timekeeping block to be reset. In my opinion, PHYs that switch their host-side interface were not considered when this IP was designed.

To get stmmac's driver to a state where it _can_ do this if desired is going to take a massive amount of work due to all these glue drivers.

I do have patches which introduce a new callback into platform drivers to set the phy_intf_sel inputs from the core code... but that's some way off before it can be merged (too many other patches I need to get in first.)

I haven't noticed qcom-ethqos using a register field that corresponds with the phy_intf_sel inputs, so even in that series, this driver doesn't get converted.

Hi Russell,

Quoted 30 lines · Russell King (Oracle)

Quoted 8 lines

# Patch Series (current): net: stmmac: phylink PCS conversion part 3 (dodgy stuff)
- QCS9100 Ride R3 - functionality seems to be fine (again, probably due to the changes only affecting SGMII mode). However, the warning added in patch 2 comes up whenever there's a speed change (I added an additional WARN_ON to check the sequence):

  	[   61.663685] qcom-ethqos 23000000.ethernet eth0: Link is Down
	[   66.235461] dwmac: PCS configuration changed from phylink by glue, please report: 0x00001000 -> 0x00000000

That's clearing ANE, turning off AN. This will be because we're not using the PCS code for 2500base-X.

Can you try:

1. in stmmac_check_pcs_mode(), as a hack, add:

	if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_2500BASEX)
		priv->hw->pcs = STMMAC_PCS_SGMII;

2. with part 3 added, please change dwmac4_pcs_init() to:

	phy_interface_t modes[] = {
		PHY_INTERFACE_MODE_SGMII,
		PHY_INTERFACE_MODE_2500BASEX,
	};
	...
	return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE,
					  GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE,
					  modes, ARRAY_SIZE(modes));

Quoted 8 lines

This will cause the integrated PCS to also be used for 2500BASE-X.

3. modify dwmac_integrated_pcs_inband_caps() to return LINK_INBAND_DISABLE for PHY_INTERFACE_MODE_2500BASEX.

This should result in the warning going away for you.

I'm not suggesting that this is a final solution.

Here are my observations (with phylink logs if it helps):

1. Link up at 2.5G

[    8.429331] qcom-ethqos 23000000.ethernet: User ID: 0x20, Synopsys ID: 0x52
[    8.436610] qcom-ethqos 23000000.ethernet:   DWMAC4/5
[   10.395163] qcom-ethqos 23000000.ethernet eth0: PHY stmmac-0:00 uses interfaces 4,23,27, validating 23
[   10.407759] qcom-ethqos 23000000.ethernet eth0:  interface 23 (2500base-x) rate match pause supports 0-7,9,13-14,47
[   10.418507] qcom-ethqos 23000000.ethernet eth0: PHY [stmmac-0:00] driver [Aquantia AQR115C] (irq=343)
[   10.428003] qcom-ethqos 23000000.ethernet eth0: phy: 2500base-x setting supported 0000000,00000000,00008000,000062ff advertising 0000000,00000000,00008000,000062ff
[   10.461072] qcom-ethqos 23000000.ethernet eth0: Enabling Safety Features
[   10.478201] qcom-ethqos 23000000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[   10.487449] qcom-ethqos 23000000.ethernet eth0: registered PTP clock
[   10.494010] qcom-ethqos 23000000.ethernet eth0: configuring for phy/2500base-x link mode
[   10.494014] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/2500base-x
[   10.494018] qcom-ethqos 23000000.ethernet eth0: interface 2500base-x inband modes: pcs=01 phy=00
[   10.494021] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/2500base-x
[   10.494024] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
[   10.508824] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/Unknown/Unknown/none/off/nolpi
[   15.099693] qcom-ethqos 23000000.ethernet eth0: phy link up 2500base-x/2.5Gbps/Full/none/rx/tx/nolpi
[   15.122160] dwmac: PCS configuration changed from phylink by glue, please report: 0x00041000 -> 0x00040000
[   15.140458] qcom-ethqos 23000000.ethernet eth0: Link is Up - 2.5Gbps/Full - flow control rx/tx
[   15.140939] stmmac_pcs: Link Up

As I understand it, the glue layer disables ANE at 2.5G.

2. Link up at 1G:

[    6.261112] qcom-ethqos 23000000.ethernet: User ID: 0x20, Synopsys ID: 0x52
[    6.261116] qcom-ethqos 23000000.ethernet:   DWMAC4/5
[    9.051693] qcom-ethqos 23000000.ethernet eth0: PHY stmmac-0:00 uses interfaces 4,23,27, validating 23
[    9.061261] qcom-ethqos 23000000.ethernet eth0:  interface 23 (2500base-x) rate match pause supports 0-7,9,13-14,47
[    9.061266] qcom-ethqos 23000000.ethernet eth0: PHY [stmmac-0:00] driver [Aquantia AQR115C] (irq=305)
[    9.061269] qcom-ethqos 23000000.ethernet eth0: phy: 2500base-x setting supported 0000000,00000000,00008000,000062ff advertising 0000000,00000000,00008000,000062ff
[    9.080324] qcom-ethqos 23000000.ethernet eth0: Enabling Safety Features
[    9.114550] qcom-ethqos 23000000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[    9.123870] qcom-ethqos 23000000.ethernet eth0: registered PTP clock
[    9.130412] qcom-ethqos 23000000.ethernet eth0: configuring for phy/2500base-x link mode
[    9.138726] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/2500base-x
[    9.138729] qcom-ethqos 23000000.ethernet eth0: interface 2500base-x inband modes: pcs=01 phy=00
[    9.138731] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/2500base-x
[    9.164930] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
[    9.194764] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/Unknown/Unknown/none/off/nolpi
[   12.542771] qcom-ethqos 23000000.ethernet eth0: phy link up sgmii/1Gbps/Full/none/rx/tx/nolpi
[   12.553890] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/sgmii
[   12.561617] qcom-ethqos 23000000.ethernet eth0: interface sgmii inband modes: pcs=03 phy=03
[   12.570220] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/sgmii
[   12.578367] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/none adv=0000000,00000000,00000000,00000000 pause=03
[   12.599545] stmmac_pcs: ANE process completed
[   12.607910] dwmac: PCS configuration changed from phylink by glue, please report: 0x00041000 -> 0x00041200
[   12.616188] stmmac_pcs: Link Up
[   12.634351] qcom-ethqos 23000000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[   12.639575] stmmac_pcs: ANE process completed
[   12.647498] stmmac_pcs: Link Up

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Quoted 3 lines

I haven't noticed qcom-ethqos using a register field that corresponds with the phy_intf_sel inputs, so even in that series, this driver doesn't get converted.

True, I think qcom-ethqos's behaviour is different than other glue drivers. For both SGMII and 2500Base-X, it uses the same ethqos_configure_sgmii() function which is just changing the SerDes speed and PCS and depending on the current speed.

Quoted 3 lines

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Yes, and bit 9 doesn't need to be set at all. SGMII isn't "negotiation" but the PHY says to the MAC "this is how I'm operating" and the MAC says "okay". Nothing more.

I'm afraid the presence of snps,ps-speed, this disrupts the test.

Quoted 8 lines · Russell King (Oracle)

Quoted 3 lines

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Yes, and bit 9 doesn't need to be set at all. SGMII isn't "negotiation" but the PHY says to the MAC "this is how I'm operating" and the MAC says "okay". Nothing more.

Quoted 1 line

I'm afraid the presence of snps,ps-speed, this disrupts the test.

Note also that testing a 10M link, 100M, 1G and finally 100M again in that order would also be interesting given my question about the RGMII register changes that configure_sgmii does.

Signature · 2 lines

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Quoted 9 lines · Russell King (Oracle)

Quoted 8 lines · Russell King (Oracle)

Quoted 3 lines

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Yes, and bit 9 doesn't need to be set at all. SGMII isn't "negotiation" but the PHY says to the MAC "this is how I'm operating" and the MAC says "okay". Nothing more.

Quoted 6 lines

Quoted 1 line

I'm afraid the presence of snps,ps-speed, this disrupts the test.

Note also that testing a 10M link, 100M, 1G and finally 100M again in that order would also be interesting given my question about the RGMII register changes that configure_sgmii does.

Despite several attempts, I couldn't get 10M to work. There is a link-up but the data path is broken. I checked the net-next tip and it's broken there as well.

Oddly enough, configure_sgmii is called with its speed argument set to 1000:

[   12.305488] qcom-ethqos 23040000.ethernet eth0: phy link up sgmii/10Mbps/Half/pause/off/nolpi
[   12.315233] qcom-ethqos 23040000.ethernet eth0: major config, requested phy/sgmii
[   12.322965] qcom-ethqos 23040000.ethernet eth0: interface sgmii inband modes: pcs=00 phy=03
[   12.331586] qcom-ethqos 23040000.ethernet eth0: major config, active phy/outband/sgmii
[   12.339738] qcom-ethqos 23040000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/pause adv=0000000,00000000,00000000,00000000 pause=00
[   12.355113] qcom-ethqos 23040000.ethernet eth0: ethqos_configure_sgmii : Speed = 1000
[   12.363196] qcom-ethqos 23040000.ethernet eth0: Link is Up - 10Mbps/Half - flow control off

Quoted 23 lines · Mohd Ayaan Anwar

Quoted 9 lines · Russell King (Oracle)

Quoted 8 lines · Russell King (Oracle)

Quoted 3 lines

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Yes, and bit 9 doesn't need to be set at all. SGMII isn't "negotiation" but the PHY says to the MAC "this is how I'm operating" and the MAC says "okay". Nothing more.

Quoted 6 lines

Quoted 1 line

I'm afraid the presence of snps,ps-speed, this disrupts the test.

Note also that testing a 10M link, 100M, 1G and finally 100M again in that order would also be interesting given my question about the RGMII register changes that configure_sgmii does.

Despite several attempts, I couldn't get 10M to work. There is a link-up but the data path is broken. I checked the net-next tip and it's broken there as well.

Oddly enough, configure_sgmii is called with its speed argument set to 1000:

Quoted 7 lines

[   12.305488] qcom-ethqos 23040000.ethernet eth0: phy link up sgmii/10Mbps/Half/pause/off/nolpi
[   12.315233] qcom-ethqos 23040000.ethernet eth0: major config, requested phy/sgmii
[   12.322965] qcom-ethqos 23040000.ethernet eth0: interface sgmii inband modes: pcs=00 phy=03
[   12.331586] qcom-ethqos 23040000.ethernet eth0: major config, active phy/outband/sgmii
[   12.339738] qcom-ethqos 23040000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/pause adv=0000000,00000000,00000000,00000000 pause=00
[   12.355113] qcom-ethqos 23040000.ethernet eth0: ethqos_configure_sgmii : Speed = 1000
[   12.363196] qcom-ethqos 23040000.ethernet eth0: Link is Up - 10Mbps/Half - flow control off

If you have "rate matching" enabled (signified by "pause" in the mode= part of phylink_mac_config), then the MAC gets told the maximum speed for the PHY interface, which for Cisco SGMII is 1G. This is intentional to support PHYs that _really_ do use rate matching. Your PHY isn't using it, and rate matching for SGMII is pointless.

Please re-run testing with phy-mode = "sgmii" which you've tested before without your rate-matching patch to the PHY driver, so the system knows the _correct_ parameters for these speeds.

Signature · 2 lines

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

My rate-matching patch was for IQ8 which has the QCA808X PHY. I am putting its testing on hold until we sort everything out on QCS9100 first.

So, for AQR115C, what should be the way forward? It has support for rate matching. For 10M should I remove its .get_rate_matching callback?

Quoted 38 lines · Russell King (Oracle)

Quoted 23 lines · Mohd Ayaan Anwar

Quoted 9 lines · Russell King (Oracle)

Quoted 8 lines · Russell King (Oracle)

Quoted 3 lines

This is probably fine since Bit(9) is self-clearing and its value just after this is 0x00041000.

Yes, and bit 9 doesn't need to be set at all. SGMII isn't "negotiation" but the PHY says to the MAC "this is how I'm operating" and the MAC says "okay". Nothing more.

Quoted 6 lines

Quoted 1 line

I'm afraid the presence of snps,ps-speed, this disrupts the test.

Note also that testing a 10M link, 100M, 1G and finally 100M again in that order would also be interesting given my question about the RGMII register changes that configure_sgmii does.

Despite several attempts, I couldn't get 10M to work. There is a link-up but the data path is broken. I checked the net-next tip and it's broken there as well.

Oddly enough, configure_sgmii is called with its speed argument set to 1000:

Quoted 7 lines

[   12.305488] qcom-ethqos 23040000.ethernet eth0: phy link up sgmii/10Mbps/Half/pause/off/nolpi
[   12.315233] qcom-ethqos 23040000.ethernet eth0: major config, requested phy/sgmii
[   12.322965] qcom-ethqos 23040000.ethernet eth0: interface sgmii inband modes: pcs=00 phy=03
[   12.331586] qcom-ethqos 23040000.ethernet eth0: major config, active phy/outband/sgmii
[   12.339738] qcom-ethqos 23040000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/pause adv=0000000,00000000,00000000,00000000 pause=00
[   12.355113] qcom-ethqos 23040000.ethernet eth0: ethqos_configure_sgmii : Speed = 1000
[   12.363196] qcom-ethqos 23040000.ethernet eth0: Link is Up - 10Mbps/Half - flow control off

If you have "rate matching" enabled (signified by "pause" in the mode= part of phylink_mac_config), then the MAC gets told the maximum speed for the PHY interface, which for Cisco SGMII is 1G. This is intentional to support PHYs that _really_ do use rate matching. Your PHY isn't using it, and rate matching for SGMII is pointless.

Quoted 4 lines

Please re-run testing with phy-mode = "sgmii" which you've tested before without your rate-matching patch to the PHY driver, so the system knows the _correct_ parameters for these speeds.

Sorry, I forgot to mention that all the recent testing is being done on QCS9100 Ride R3 which has the AQR115C PHY.

My rate-matching patch was for IQ8 which has the QCA808X PHY. I am putting its testing on hold until we sort everything out on QCS9100 first.

So, for AQR115C, what should be the way forward? It has support for rate matching. For 10M should I remove its .get_rate_matching callback?

Ayaan

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Quoted 18 lines · Vladimir Oltean

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

Quoted 7 lines

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Thanks. Looks like rate adaptation is only provisioned for 10M, which matches my observation where phylink passes the exact speeds for 100/1000/2500 but 1000 for 10M.

Quoted 27 lines · Mohd Ayaan Anwar

Quoted 18 lines · Vladimir Oltean

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

Quoted 7 lines

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Quoted 3 lines

Thanks. Looks like rate adaptation is only provisioned for 10M, which matches my observation where phylink passes the exact speeds for 100/1000/2500 but 1000 for 10M.

Hmm, I wonder what the PHY is doing for that then. stmmac will be programmed to read the Cisco SGMII in-band control word, and use that to determine whether symbol replication for slower speeds is being used.

If AQR115C is indicating 10M in the in-band control word, but is actually operating the link at 1G speed, things are not going to work, and I would say the PHY is broken to be doing that. The point of the SGMII in-band control word is to tell the MAC about the required symbol replication on the link for transmitting the slower data rates over the link.

stmmac unfortunately doesn't give access to the raw Cisco SGMII in-band control word. However, reading register 0xf8 bits 31:16 for dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this information. In that bitfield, bits 2:1 give the speed. 2 = 1G, 1 = 100M, 0 = 10M.

Quoted 44 lines · Russell King (Oracle)

Quoted 27 lines · Mohd Ayaan Anwar

Quoted 18 lines · Vladimir Oltean

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

Quoted 7 lines

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Quoted 3 lines

Thanks. Looks like rate adaptation is only provisioned for 10M, which matches my observation where phylink passes the exact speeds for 100/1000/2500 but 1000 for 10M.

Hmm, I wonder what the PHY is doing for that then. stmmac will be programmed to read the Cisco SGMII in-band control word, and use that to determine whether symbol replication for slower speeds is being used.

If AQR115C is indicating 10M in the in-band control word, but is actually operating the link at 1G speed, things are not going to work, and I would say the PHY is broken to be doing that. The point of the SGMII in-band control word is to tell the MAC about the required symbol replication on the link for transmitting the slower data rates over the link.

Quoted 5 lines

stmmac unfortunately doesn't give access to the raw Cisco SGMII in-band control word. However, reading register 0xf8 bits 31:16 for dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this information. In that bitfield, bits 2:1 give the speed. 2 = 1G, 1 = 100M, 0 = 10M.

It might be Linux who is forcing the AQR115C into the nonsensical behaviour of advertising 10M in the SGMII control word while simultanously forcing the PHY MII to operate at 1G with flow control for the 10M media speed.

We don't control the latter, but we do control the former:
aqr_gen2_config_inband(), if given modes == LINK_INBAND_ENABLE, will enable in-band for all media speeds that use PHY_INTERFACE_MODE_SGMII. Regardless of how the PHY was provisioned for each media speed, and especially regardless of rate matching settings, this function will uniformly set the same in-band enabled/disabled setting for all media speeds using the same host interface.

If dwmac_integrated_pcs_inband_caps(), as per Russell's patch 1/3, reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, and if aqr_gen2_inband_caps() also reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, then we're giving phylink_pcs_neg_mode() all the tools it needs to shoot itself in the foot, and select LINK_INBAND_ENABLE.

The judgement call in the Aquantia PHY driver was mine, as documented in commit 5d59109d47c0 ("net: phy: aquantia: report and configure in-band autoneg capabilities"). The idea being that the configuration would have been unsupportable anyway given the question that the framework asks:
"does the PHY use in-band for SGMII, or does it not?"

Assuming the configuration at 10Mbps wasn't always broken, there's only one way to know how it was supposed to work: more dumping of the initial provisioning, prior to our modification in aqr_gen2_config_inband(). Ayaan, please re-print the same info with this new untested patch applied. I am going to assume that in-band autoneg isn't enabled in the unmodified provisioning, at least for 10M.

Russell's request for the integrated PCS status is also a good parallel confirmation that yes, we've entered a mode where the PHY advertises SGMII replication at 10M.

From b91162e5dae8e20b477999c4f2fcdb98c219d663 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <[email protected]>
Date: Mon, 3 Nov 2025 14:03:55 +0200
Subject: [PATCH] net: phy: aquantia: add inband setting to the
aqr_gen2_read_global_syscfg() print

Signed-off-by Vladimir Oltean <[email protected]>

---
 drivers/net/phy/aquantia/aquantia_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/net/phy/aquantia/aquantia_main.c +5 -2

index 41f3676c7f1e..f06b7b51bb7d 100644
@@ -839,6 +839,7 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
 
 	for (i = 0; i < AQR_NUM_GLOBAL_CFG; i++) {
 		struct aqr_global_syscfg *syscfg = &priv->global_cfg[i];
+		bool inband;
 
 		syscfg->speed = aqr_global_cfg_regs[i].speed;
 

@@ -849,6 +850,7 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
 
 		serdes_mode = FIELD_GET(VEND1_GLOBAL_CFG_SERDES_MODE, val);
 		rate_adapt = FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val);
+		inband = FIELD_GET(VEND1_GLOBAL_CFG_AUTONEG_ENA, val);
 
 		switch (serdes_mode) {
 		case VEND1_GLOBAL_CFG_SERDES_MODE_XFI:

@@ -896,12 +898,13 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
 		}
 
 		phydev_dbg(phydev,
-			   "Media speed %d uses host interface %s with %s\n",
+			   "Media speed %d uses host interface %s with %s, inband %s\n",
 			   syscfg->speed, phy_modes(syscfg->interface),
 			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
 			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
 			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
-			   "unrecognized rate adaptation type");
+			   "unrecognized rate adaptation type",
+			   str_enabled_disabled(inband));
 	}
 
 	return 0;

Signature · 1 line

2.43.0

Here are the logs when I boot up with a 10M link:

Media speed 10 uses host interface sgmii with rate adaptation through flow control, inband enabled

Quoted 5 lines

stmmac unfortunately doesn't give access to the raw Cisco SGMII in-band control word. However, reading register 0xf8 bits 31:16 for dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this information. In that bitfield, bits 2:1 give the speed. 2 = 1G, 1 = 100M, 0 = 10M.

It might be Linux who is forcing the AQR115C into the nonsensical behaviour of advertising 10M in the SGMII control word while simultanously forcing the PHY MII to operate at 1G with flow control for the 10M media speed.

We don't control the latter, but we do control the former:
aqr_gen2_config_inband(), if given modes == LINK_INBAND_ENABLE, will enable in-band for all media speeds that use PHY_INTERFACE_MODE_SGMII. Regardless of how the PHY was provisioned for each media speed, and especially regardless of rate matching settings, this function will uniformly set the same in-band enabled/disabled setting for all media speeds using the same host interface.

If dwmac_integrated_pcs_inband_caps(), as per Russell's patch 1/3, reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, and if aqr_gen2_inband_caps() also reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, then we're giving phylink_pcs_neg_mode() all the tools it needs to shoot itself in the foot, and select LINK_INBAND_ENABLE.

The judgement call in the Aquantia PHY driver was mine, as documented in commit 5d59109d47c0 ("net: phy: aquantia: report and configure in-band autoneg capabilities"). The idea being that the configuration would have been unsupportable anyway given the question that the framework asks:
"does the PHY use in-band for SGMII, or does it not?"

Quoted 27 lines · Mohd Ayaan Anwar

Quoted 18 lines · Vladimir Oltean

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

Quoted 7 lines

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Quoted 3 lines

Thanks. Looks like rate adaptation is only provisioned for 10M, which matches my observation where phylink passes the exact speeds for 100/1000/2500 but 1000 for 10M.

Hmm, I wonder what the PHY is doing for that then. stmmac will be programmed to read the Cisco SGMII in-band control word, and use that to determine whether symbol replication for slower speeds is being used.

If AQR115C is indicating 10M in the in-band control word, but is actually operating the link at 1G speed, things are not going to work, and I would say the PHY is broken to be doing that. The point of the SGMII in-band control word is to tell the MAC about the required symbol replication on the link for transmitting the slower data rates over the link.

stmmac unfortunately doesn't give access to the raw Cisco SGMII in-band control word. However, reading register 0xf8 bits 31:16 for dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this information. In that bitfield, bits 2:1 give the speed. 2 = 1G, 1 = 100M, 0 = 10M.

Signature · 2 lines

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Quoted 44 lines · Russell King (Oracle)

Quoted 27 lines · Mohd Ayaan Anwar

Quoted 18 lines · Vladimir Oltean

As Russell partially pointed out, there are several assumptions in the Aquantia PHY driver and in phylink, three of them being that:
- rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
- if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface, pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using that interface
- if rate matching is used, the PHY is configured to use it for all media speeds <= phylink_interface_max_speed(link_state.interface)

Those assumptions are not validated very well against the ground truth from the PHY provisioning, so the next step would be for us to see that directly.

Please turn this print from aqr_gen2_read_global_syscfg() into something visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():

Quoted 7 lines

		phydev_dbg(phydev,
			   "Media speed %d uses host interface %s with %s\n",
			   syscfg->speed, phy_modes(syscfg->interface),
			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
			   "unrecognized rate adaptation type");

Quoted 3 lines

Thanks. Looks like rate adaptation is only provisioned for 10M, which matches my observation where phylink passes the exact speeds for 100/1000/2500 but 1000 for 10M.

Hmm, I wonder what the PHY is doing for that then. stmmac will be programmed to read the Cisco SGMII in-band control word, and use that to determine whether symbol replication for slower speeds is being used.

If AQR115C is indicating 10M in the in-band control word, but is actually operating the link at 1G speed, things are not going to work, and I would say the PHY is broken to be doing that. The point of the SGMII in-band control word is to tell the MAC about the required symbol replication on the link for transmitting the slower data rates over the link.

Quoted 6 lines

stmmac unfortunately doesn't give access to the raw Cisco SGMII in-band control word. However, reading register 0xf8 bits 31:16 for dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this information. In that bitfield, bits 2:1 give the speed. 2 = 1G, 1 = 100M, 0 = 10M.

This is dwmac4 and I got the following values with devmem at different link speeds:

1. 10M:		0x00080000  => Bit 2:1 = 0
2. 100M:	0x000A0000  => Bit 2:1 = 1
3. 1G: 		0x000D0000  => Bit 2:1 = 2

Quoted 18 lines

Quoted 18 lines

Can you try:

1. in stmmac_check_pcs_mode(), as a hack, add:

	if (priv->dma_cap.pcs && interface == PHY_INTERFACE_MODE_2500BASEX)
		priv->hw->pcs = STMMAC_PCS_SGMII;

2. with part 3 added, please change dwmac4_pcs_init() to:

	phy_interface_t modes[] = {
		PHY_INTERFACE_MODE_SGMII,
		PHY_INTERFACE_MODE_2500BASEX,
	};
	...
	return stmmac_integrated_pcs_init(priv, GMAC_PCS_BASE,
					  GMAC_INT_PCS_LINK | GMAC_INT_PCS_ANE,
					  modes, ARRAY_SIZE(modes));

Quoted 8 lines

Quoted 8 lines

This will cause the integrated PCS to also be used for 2500BASE-X.

3. modify dwmac_integrated_pcs_inband_caps() to return LINK_INBAND_DISABLE for PHY_INTERFACE_MODE_2500BASEX.

This should result in the warning going away for you.

I'm not suggesting that this is a final solution.

<snip>
Please try again, this time with snps,ps-speed removed from the DT description for the interface. This property was a buggy attempt at reverse-SGMII, and incorrectly produced a warning if not specified when the integrated PCS was being used. The "bug" in the attempt with this was a typo in each MAC core driver, where specifying this set the TE (transmit enable) bit rather than the TC (transmit configuration) bit in the MAC control register. All the rest of the setup for reverse-SGMII mode was in place, but this bug made the entire thing useless.

The "invalid port speed" warning that results if this property is not set to 10, 100 or 1000 is another bug - only if this warning is printed will the "normal" mode be selected.

Since the PCS series 1 and 2 have been merged into net-next, it will be safe to submit patches removing these properties from your DT files, without fear of this warning appearing.

Hello Russell,

I finally got some time to test out 100M/1G/2.5G with all these changes on the QCS9100 Ride R3 board (AQR115C PHY, qcom-ethqos).

Apart from this patch series, I incorporated the following changes:
1. Hacks suggested to have the PCS code work for 2500Base-X.
2. Removed snps,ps-speed from DT.
3. Picked [PATCH net-next v2] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification.

Apologies for the lengthy email- I’ve included phylink logs for all five points for completeness.

*Observations:*

1. 2.5G Link up - no warning about the PCS configuration getting changed by glue. Data path works fine.

	[   10.342908] qcom-ethqos 23000000.ethernet eth0: PHY stmmac-0:00 uses interfaces 4,23,27, validating 23
	[   10.352486] qcom-ethqos 23000000.ethernet eth0:  interface 23 (2500base-x) rate match pause supports 0-7,9,13-14,47
	[   10.363215] qcom-ethqos 23000000.ethernet eth0: PHY [stmmac-0:00] driver [Aquantia AQR115C] (irq=365)
	[   10.372690] qcom-ethqos 23000000.ethernet eth0: phy: 2500base-x setting supported 0000000,00000000,00008000,000062ff advertising 0000000,00000000,00008000,000062ff
	[   10.428389] qcom-ethqos 23000000.ethernet eth0: configuring for phy/2500base-x link mode
	[   10.436717] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/2500base-x
	[   10.444870] qcom-ethqos 23000000.ethernet eth0: interface 2500base-x inband modes: pcs=01 phy=00
	[   10.453913] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/2500base-x
	[   10.462506] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
	[   10.485700] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/Unknown/Unknown/none/off/nolpi
	[   15.131941] qcom-ethqos 23000000.ethernet eth0: phy link up 2500base-x/2.5Gbps/Full/none/rx/tx/nolpi
	[   15.143632] qcom-ethqos 23000000.ethernet eth0: ethqos_configure_sgmii : Speed = 2500
	[   15.153226] stmmac_pcs: Link Up
	[   15.153274] qcom-ethqos 23000000.ethernet eth0: Link is Up - 2.5Gbps/Full - flow control rx/tx

2. 1G Link up - Warning (PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200). Data path works fine.

	[   10.532328] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/2500base-x
	[   10.540919] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
	[   10.563437] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/Unknown/Unknown/none/off/nolpi
	[   13.912074] qcom-ethqos 23000000.ethernet eth0: phy link up sgmii/1Gbps/Full/none/rx/tx/nolpi
	[   13.919074] stmmac_pcs: Link Up
	< a *bunch* of "stmmac_pcs: Link Down" prints, more details in 4.>
	[   14.948996] stmmac_pcs: Link Up
	[   14.949149] stmmac_pcs: Link Down
	[   14.949169] stmmac_pcs: Link Up

	[   14.949301] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/sgmii
	[   14.949317] stmmac_pcs: Link Down
	[   14.949326] qcom-ethqos 23000000.ethernet eth0: interface sgmii inband modes: pcs=03 phy=03
	[   14.949331] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/sgmii
	[   14.949335] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/none adv=0000000,00000000,00000000,00000000 pause=03
	[   14.952026] qcom-ethqos 23000000.ethernet eth0: ethqos_configure_sgmii : Speed = 1000
	[   14.952033] dwmac: PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200
	[   14.952057] qcom-ethqos 23000000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
	[   15.035977] stmmac_pcs: ANE process completed
	[   15.035978] stmmac_pcs: Link Down
	[   15.036004] stmmac_pcs: Link Up

3. 100M Link up - Warning (PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200). Data path works fine.

	[   20.390144] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/2500base-x
	[   20.398720] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
	[   20.418477] stmmac_pcs: Link Down
	[   20.421912] stmmac_pcs: Link Down
	[   20.426255] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/100Mbps/Full/none/rx/tx/nolpi
	[   20.440795] stmmac_pcs: Link Down
	[   23.095229] qcom-ethqos 23000000.ethernet eth0: phy link up sgmii/100Mbps/Full/none/rx/tx/nolpi
	[   23.101362] stmmac_pcs: Link Down
	[   23.106527] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/sgmii
	[   23.107624] stmmac_pcs: Link Down
	[   23.118707] stmmac_pcs: Link Down
	[   23.124703] qcom-ethqos 23000000.ethernet eth0: interface sgmii inband modes: pcs=03 phy=03
	[   23.128141] stmmac_pcs: Link Down
	[   23.136699] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/sgmii
	[   23.140126] stmmac_pcs: Link Down
	[   23.148232] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/none adv=0000000,00000000,00000000,00000000 pause=03
	[   23.151657] stmmac_pcs: Link Down

	[   23.166924] qcom-ethqos 23000000.ethernet eth0: ethqos_configure_sgmii : Speed = 100
	[   23.167584] stmmac_pcs: Link Down
	[   23.175511] dwmac: PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200
	[   23.178944] stmmac_pcs: Link Up
	[   23.188862] qcom-ethqos 23000000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
	[   23.192097] stmmac_pcs: Link Down
	[   23.204346] stmmac_pcs: ANE process completed
	[   23.208828] stmmac_pcs: Link Up

4. Sometimes after toggling the interface or even during boot-up, the console gets flooded with "stmmac_pcs: Link Down" prints. For e.g.,

	// Interface toggled
	< a bunch of "stmmac_pcs: Link Down" prints>
	[  549.898750] stmmac_pcs: Link Down
	[  549.902186] stmmac_pcs: Link Down
	[  549.905628] stmmac_pcs: Link Down
	[  549.909069] stmmac_pcs: Link Down
	[  549.912509] stmmac_pcs: Link Down
	[  549.915948] stmmac_pcs: Link Down
	[  549.919391] stmmac_pcs: Link Down
	[  549.922858] stmmac_pcs: Link Down
	[  549.924140] qcom-ethqos 23000000.ethernet eth0: ethqos_configure_sgmii : Speed = 2500
	[  549.926304] stmmac_pcs: Link Down
	[  549.934349] qcom-ethqos 23000000.ethernet eth0: Link is Up - 2.5Gbps/Full - flow control rx/tx
	[  549.937746] stmmac_pcs: Link Up

ethtool stats reveal an unusually high number of interrupts (I have seen this number go as high as about 16000 when booting up with a 1G link)

	     irq_pcs_ane_n: 0
	     irq_pcs_link_n: 1998	

5. Switching between 100M/1G/2.5G link is a bit of a mixed bag. Sometimes it works, sometimes the data path breaks and needs an interface toggle to be functional again. I don't necessarily think that it's due to the speed specific configurations done by configure_sgmii as that shouldn't impact switching between 1G and 2.5G, or even the switch from 1G/2.5G to 100M.

While this is *broken* on net-next as well, the current patch series allowed me to notice a peculiar behavior - it looks like sometimes the PCS link doesn't come up:

	[   55.491996] qcom-ethqos 23000000.ethernet eth0: phy link down 2500base-x/2.5Gbps/Full/none/rx/tx/nolpi
	[   55.501622] qcom-ethqos 23000000.ethernet eth0: Link is Down
	[   58.907705] qcom-ethqos 23000000.ethernet eth0: phy link up sgmii/1Gbps/Full/none/rx/tx/nolpi
	[   58.913724] stmmac_pcs: Link Down
	[   58.919947] qcom-ethqos 23000000.ethernet eth0: major config, requested phy/sgmii
	[   58.927656] qcom-ethqos 23000000.ethernet eth0: interface sgmii inband modes: pcs=03 phy=03
	[   58.936256] qcom-ethqos 23000000.ethernet eth0: major config, active phy/outband/sgmii
	[   58.944409] qcom-ethqos 23000000.ethernet eth0: phylink_mac_config: mode=phy/sgmii/none adv=0000000,00000000,00000000,00000000 pause=03
	[   58.958298] qcom-ethqos 23000000.ethernet eth0: ethqos_configure_sgmii : Speed = 1000
	[   58.967448] dwmac: PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200
	[   58.977392] qcom-ethqos 23000000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx

Since 5 is unrelated to this series (I'll try to debug it separately), let me know if you'd like me to run any other experiments for 2, 3, and 4.

Quoted 4 lines

The "invalid port speed" warning that results if this property is not set to 10, 100 or 1000 is another bug - only if this warning is printed will the "normal" mode be selected.

Quoted 4 lines

Since the PCS series 1 and 2 have been merged into net-next, it will be safe to submit patches removing these properties from your DT files, without fear of this warning appearing.

Thanks for the explanation. I see the incorrect use of snps,ps-speed in the DT of a couple of more boards that use the same MAC core. Would it be okay to add your Suggested-by when submitting the fix patches?