Re: [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces

[Thomas Karpiniec]

On 16/09/2026 10:58 pm, Maxime Chevallier wrote:
> On 9/16/26 14:17, Mohd Ayaan Anwar wrote:
> > On Wed, Sep 16, 2026 at 08:39:50PM +1000, Thomas Karpiniec wrote:
> > > +static void ethqos_get_interfaces_serdes(struct stmmac_priv *priv, void *bsp_priv,
> > > +					 unsigned long *interfaces)
> > > +{
> > > +	struct qcom_ethqos *ethqos = bsp_priv;
> > > +
> > > +	if (!ethqos->serdes_phy)
> > > +		return;
> > > +
> > > +	/* PHYs such as QCA8081 switch between SGMII and 2500BASE-X with
> > > +	 * the negotiated copper speed. mac_finish reconfigures the SerDes
> > > +	 * accordingly; let phylink validate all modes that path supports.
> > > +	 */
> > > +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
> > > +			  PHY_INTERFACE_MODE_SGMII, NULL))
> > > +		__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
> > > +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
> > > +			  PHY_INTERFACE_MODE_2500BASEX, NULL))
> > > +		__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
> > > +}
> > > +
> >
> > So, I had proposed this exact change a while back:
> > https://lore.kernel.org/netdev/aMgootkPQ%2FGcdiXX@xxxxxxxxxxxxxxxx/
> >
> > As Russell noted, this behaviour is due to the messed-up integrated
> > PCS support in STMMAC/QCOM-ETHQOS.
> >
> > I would defer to him for this if he's around.
>
> Indeed, but we haven't heard from him in a while, let's see if we can
> figure this out.
>
> Does this IP have an integrated PCS ? (i.e. dma_cap.pcs is set)
> you can check that in debugfs :
>
> mount -t debugfs none /sys/kernel/debug
> cat /sys/kernel/debug/stmmaceth/eth0/dma_cap | grep PCS
>
> if it's Y, then you have the integrated one.
>
> Russell worked hard on that, it looks like a lot of the plumbing went
> through.
>
> I think the right approach here is to have the PCS itself report the
> list of supported interfaces, instead of relying on the glue. looking
> at stmmac_pcs.c :
>
> 220 int stmmac_integrated_pcs_init(struct stmmac_priv *priv,
> 221                                const struct stmmac_pcs_info *pcs_info)
> 222 {
> 223         struct stmmac_pcs *spcs;
> 224
> 225         spcs = devm_kzalloc(priv->device, sizeof(*spcs), GFP_KERNEL);
> 226         if (!spcs)
> 227                 return -ENOMEM;
>
> [...]
>                                                               
> 246         /* Only allow 2500BASE-X if the SerDes has support. */
> 247         if (priv->plat->flags & STMMAC_FLAG_SERDES_SUPPORTS_2500M)
> 248                 __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> 249                           spcs->pcs.supported_interfaces);
> 250
> 251         priv->integrated_pcs = spcs;
> 252
> 253         return 0;
> 254 }
>
> Can you test settung the STMMAC_FLAG_SERDES_SUPPORTS_2500M flag in
> dwmac-qcom-ethqos ?

I can confirm that dma_cap.pcs = Y.

I tested setting STMMAC_FLAG_SERDES_SUPPORTS_2500M on hardware and 
unfortunately the flag alone doesn't solve the problem.

I think there are a couple of things missing. stmmac_phylink_setup 
doesn't use the integrated PCS bitmap to populate 
config->supported_interfaces, so SGMII is still not advertised to 
phylink as a usable host interface. Also stmmac_integrated_pcs_init is 
adding 1000BASE-X unconditionally, which wouldn't be appropriate here.

I did a proof-of-concept fixing those things and the end result looks a 
little messy: config->supported_interfaces could come from either 
get_interfaces _or_ the integrated PCS with unclear (to me) responsibility.

This POC did show that STMMAC_FLAG_SERDES_SUPPORTS_2500M enabled phylink 
to configure the integrated PCS for 2500BASE-X and a 2.5 Gbps link came 
up with no apparent issues.

Do you have any thoughts what the clean end-state should be?

Tom