Message-ID: <aMnigwTeMQc0GxaD@shell.armlinux.org.uk>
Date: Tue, 16 Sep 2025 23:19:47 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Mohd Ayaan Anwar <mohd.anwar@....qualcomm.com>
Cc: andrew@...n.ch, Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: phy: qcom: qca808x: Add .get_rate_matching
 support

On Mon, Sep 15, 2025 at 04:11:04PM +0100, Russell King (Oracle) wrote:
> On Mon, Sep 15, 2025 at 08:24:26PM +0530, Mohd Ayaan Anwar wrote:
> > On Mon, Sep 15, 2025 at 01:09:39PM +0100, Russell King (Oracle) wrote:
> > > This shows that the PHY supports SGMII (4) and 2500base-X (23). However,
> > > as we only validate 2500base-X, this suggests stmmac doesn't support
> > > switching between SGMII and 2500base-X.
> > > 
> > > What *exactly* is the setup with stmmac here? Do you have an external
> > > PCS to support 2500base-X, or are you using the stmmac internal PCS?
> > 
> > Internal PCS. But it's not really pure 2500base-X...
> > I found an older thread for this exact MAC core [0], and it looks like
> > we have an overclocked SGMII, i.e., 2500base-X without in-band
> > signalling.
> > 
> > Just wondering if registering a `.get_interfaces` callback in
> > `dwmac-qcom-ethqos.c` and doing something like the following will be
> > helpful?
> > 
> > case PHY_INTERFACE_MODE_2500BASEX:
> > 	__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
> > 	fallthrough;
> > case PHY_INTERFACE_MODE_SGMII:
> > 	__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
> > 	break;
> > ...
> > 
> > This should ensure that both SGMII and 2500base-X are validated,
> > allowing switching between them.
> 
> So, this is something that has never worked with this hardware setup.
> I don't think we should rush to make it work. The stmmac internal
> PCS code is a mess, bypassing phylink. I had a patch series which
> addressed this a while back but it went nowhere, but I guess this is
> an opportunity to say "look, we need to get this sorted properly".
> 
> I suspect this isn't going to be simple - stmmac does _not_ use
> phylink properly (I've been doing lots of cleanups to this driver
> over the last year or so to try and make the code more
> understandable so I can start addressing this deficiency) and
> there's still lots of work to be done. The way the "platform glue"
> drivers work is far from ideal, especially when it comes to
> switching interfaces.
> 
> I'll try to post the stmmac PCS cleanup series in the coming few
> days, and it would be useful if you could give it whatever
> testing you can.

... and it's been delayed because I've had to rework three of the
patch series I recently posted.

I did get some time late last night to read through the documentation
I have for one version of the dwmac which has optional PCS, and I'm
coming to the conclusion that the whole mac_interface vs phy_interface
thing is wrong in the driver.

My comment update which added this a few years ago:

        /* MAC ----- optional PCS ----- SerDes ----- optional PHY ----- Media
         *       ^                               ^
         * mac_interface                   phy_interface
         *
         * mac_interface is the MAC-side interface, which may be the same
         * as phy_interface if there is no intervening PCS. If there is a
         * PCS, then mac_interface describes the interface mode between the
         * MAC and PCS, and phy_interface describes the interface mode
         * between the PCS and PHY.
         */

appears to be incorrect. It was based on just phylink knowledge and a
reasonable guess about what was going on with this driver. It seems
no one had any better ideas on exactly what mac_interface was trying
to describe.

Having looked at the information I now have, and referred back to the
psat code, it appears to me that what is actually going on here is
this:

	MAC --- optional integrated PCS --- SerDes --- world (media or PHY)
                                         ^          ^
                                 mac_interface  phy_interface
				        TBI      1000base-X

It seems that TBI is used on the PCS output when talking to a SerDes
for 1000BASE-X or SGMII. RTBI is used with a PHY that can talk RTBI.

Considering just 2.5G and below, it seems to me that mac_interface
can be determined from phy_interface:

phy_interface		mac_interface
SGMII			TBI
1000BASE-X		TBI
2500BASE-X		TBI
RTBI			RTBI

These are the "official" modes. There is also a seperate block that
is used for SMII and RGMII which the code treats as a PCS (partly
because it uses the same registers) so I'd throw into this:

phy_interface		mac_interface
SMII			SMII
RGMII*			RGMII*

For every other phy_interface <= 2.5G, mac_interface is basically
not applicable, and we should be referring to phy_interface everywhere.

In fact, I can't see that mac_interface actually matters for most of
the driver. The only case I can see it matters is when the core
supports multiple interfaces, and needs to be configured appropriately
(which needs an entire core-wide reset if it changes.)

So, I'm going to propose at the very least selecting whether the driver
uses the PCS not based on mac_interface as the code currently does, but
on phy_interface (actually the interface passed by phylink.) That will
make phylink happier when stmmac is converted to phylink_pcs.

This means I need to spend some time reworking my series... and yay,
more patches to add to my already massive stack of stmmac patches. :/

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