Re: [PATCH net-next 2/3] net: stmmac: xgmac: Correct supported speed modes

[G Thomas, Rohan]

Hi Serge,

Thanks for the detailed response.

On 7/17/2025 10:52 PM, Serge Semin wrote:
> > > DW XGMAC IP-core of v2.x and older don't support 10/100Mbps modes
> > > neither in the XGMII nor in the GMII interfaces. That's why I dropped
> > > the 10/100Mbps link capabilities retaining 1G, 2.5G and 10G speeds
> > > only (the only speeds supported for DW XGMAC 1.20a/2.11a Tx in the
> > > MAC_Tx_Configuration.SS register field). Although I should have
> > > dropped the MAC_5000FD too since it has been supported since v3.0
> > > IP-core version. My bad.(
> > >
> > > Starting from DW XGMAC v3.00a IP-core the list of the supported speeds
> > > has been extended to: 10/100Mbps (MII), 1G/2.5G (GMII), 2.5G/5G/10G
> > > (XGMII). Thus the more appropriate fix here should take into account
> > > the IP-core version. Like this:
> > > 	if (dma_cap->mbps_1000 && MAC_Version.SNPSVER >= 0x30)
> > > 		dma_cap->mbps_10_100 = 1;
> > >   > Then you can use the mbps_1000 and mbps_10_100 flags to set the proper
> > > MAC-capabilities to hw->link.caps in the dwxgmac2_setup() method. I
> > > would have added the XGMII 2.5G/5G MAC-capabilities setting up to the
> > > dwxgmac2_setup() method too for the v3.x IP-cores and newer.
> >
> > Agreed. Will do in the next version.
> >
> > Will ensure that mbps_10_100 is set only if SNPSVER >= 0x30 and will
> > also conditionally enable 2.5G/5G MAC capabilities for IP versions
> > v3.00a and above.
> >
> > In the stmmac_dvr_probe() the setup() callback is invoked before
> > hw_cap_support is populated. Given that, do you think it is sufficient
> > to add these checks into the dwxgmac2_update_caps() instead?
>
> Arrgh, I was looking at my local repo with a refactored hwif initialization
> procedure which, amongst other things, implies the HW-features detection
> performed in the setup methods.(
> So in case of the vanilla STMMAC driver AFAICS there are three options
> here:
>
> 1. Repeat what I did in my local repo and move the HW-features
> detection procedure (calling the *_get_hw_feature() functions) to the
> *_setup() methods. After that change you can use the retrieved
> dma_cap-data to init the MAC capabilities exactly in sync to the
> detected HW-features. But that must be thoroughly thought through
> since there are Sun8i and Loongson MACs which provide their own HWIF
> setup() methods (by means of plat_stmmacenet_data::setup()). So the
> respective *_get_hw_feature() functions might need to be called in
> these methods too (at least in the Loongson MACs setup() method).
>
> 2. Define new dwxgmac3_setup() method and thus a new entry in
> stmmac_hw[]. Then dwxgmac2_setup() could be left with only 1G, 2.5G
> and 10G MAC-capabilities declared, meanwhile dwxgmac3_setup() would
> add all the DW XGMAC v3.00a MAC-capabilities. In this case you'd need
> the dwxgmac2_update_caps() method defined anyway in order to filter
> out the MAC-capabilities based on the
> dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state.
>
> 3. As you suggest indeed declare all the possible DW XGMAC
> MAC-capabilities in the dwxgmac2_setup() method and then filter them
> out in dwxgmac2_update_caps() based on the
> dma_features::{mbps_1000,mbps_10_100,half_duplex} flags state and
> IP-core version.
>
> The later option seems the least code-invasive but implements a more
> complex logic with declaring all the possible MAC-capabilities and
> then filtering them out. Option two still implies filtering the
> MAC-capabilities out but only from those specific for the particular
> IP-core version. Finally option one is more complex to implement
> implying the HWIF procedure refactoring with higher risks to break
> things, but after it's done the setup() methods will turn to a more
> useful procedures which could be used not only for the more exact
> MAC-capabilities initialization but also for other
> data/fields/callbacks setting up.
>
> It's up to you and the maintainers to decide which solution would be
> more appropriate.
>
> -Serge(y)

Unless there are concerns, I'll proceed with option 3 for now, as it’s
the least invasive and aligns well with the current driver structure.
I’ll declare all possible DW XGMAC MAC-capabilities in dwxgmac2_setup()
and then filter them appropriately in dwxgmac2_update_caps() based on
the dma_features flags and IP-core version.

Let me know if any concerns with this approach.

Best Regards,
Rohan