Message-ID: <aQHc6SowbWsIA1A5@shell.armlinux.org.uk>
Date: Wed, 29 Oct 2025 09:22:49 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Mohd Ayaan Anwar <mohd.anwar@....qualcomm.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Alexis Lothoré <alexis.lothore@...tlin.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Boon Khai Ng <boon.khai.ng@...era.com>,
	Daniel Machon <daniel.machon@...rochip.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Furong Xu <0x1207@...il.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Jakub Kicinski <kuba@...nel.org>,
	"Jan Petrous (OSS)" <jan.petrous@....nxp.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
	Vladimir Oltean <olteanv@...il.com>,
	Yu-Chun Lin <eleanor15x@...il.com>
Subject: Re: [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3
 (dodgy stuff)

On Wed, Oct 29, 2025 at 02:42:39AM +0530, Mohd Ayaan Anwar wrote:
> Hi Russell,
> 
> On Sat, Oct 25, 2025 at 09:47:37PM +0100, Russell King (Oracle) wrote:
> > 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(-)
> > 
> 
> 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.

Thanks for the feedback.

> 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.

Great.

> 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

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.

Thanks.

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

Powered by blists - more mailing lists