Re: [PATCH net-next v7 2/5] net: stmmac: Add DW25GMAC support in stmmac core driver

[Jitendra Vegiraju]

Hi Russell,
Thank you for reviewing the patches.


On Fri, Mar 13, 2026 at 4:14 PM Russell King (Oracle)
<linux@xxxxxxxxxxxxxxx> wrote:
>
> On Fri, Mar 13, 2026 at 03:22:03PM -0700, \Jitendra Vegiraju wrote:
> > @@ -42,13 +42,18 @@
> >  #define DWXGMAC_CORE_2_20    0x22
> >  #define DWXLGMAC_CORE_2_00   0x20
> >
> > +#define DW25GMAC_CORE_3_20   0x32
> > +#define DW25GMAC_CORE_4_00   0x40
> > +
>
> Do these live in the same versioning numberspace as DWXGMAC and
> DWXLGMAC?
>
> > +int dw25gmac_setup(struct stmmac_priv *priv)
> > +{
> > +     struct mac_device_info *mac = priv->hw;
> > +
> > +     dev_info(priv->device, "\tDW25GMAC\n");
> > +
> > +     priv->dev->priv_flags |= IFF_UNICAST_FLT;
> > +     mac->pcsr = priv->ioaddr;
> > +     mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> > +     mac->unicast_filter_entries = priv->plat->unicast_filter_entries;
> > +     mac->mcast_bits_log2 = 0;
> > +
> > +     if (mac->multicast_filter_bins)
> > +             mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
>
> The above seems to be common amongst the other two setup functions.
> Maybe there should be a helper for this?
>
Ack. Will address this in v8.
> > +
> > +     mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> > +                      MAC_1000FD | MAC_2500FD | MAC_5000FD |
> > +                      MAC_10000FD | MAC_25000FD;
> > +     mac->link.duplex = 0;
> > +     mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
> > +     mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
> > +     mac->link.speed1000 = XGMAC_CONFIG_SS_1000_GMII;
> > +     mac->link.speed2500 = XGMAC_CONFIG_SS_2500_GMII;
> > +     mac->link.xgmii.speed2500 = XGMAC_CONFIG_SS_2500;
> > +     mac->link.xgmii.speed5000 = XGMAC_CONFIG_SS_5000;
> > +     mac->link.xgmii.speed10000 = XGMAC_CONFIG_SS_10000;
> > +     mac->link.xgmii.speed25000 = XGMAC_CONFIG_SS_25000;
> > +     mac->link.speed_mask = XGMAC_CONFIG_SS_MASK;
>
> Is there any point populating link.speed10 and link.speed100 ?
> You're telling phylink that 10/100M speeds aren't supported in
> link.caps, so these speeds won't ever be used.
>
Thanks, I missed this. Will remove 10/100M entries.
> > +
> > +     mac->mii.addr = XGMAC_MDIO_ADDR;
> > +     mac->mii.data = XGMAC_MDIO_DATA;
> > +     mac->mii.addr_mask = GENMASK(20, 16);
> > +     mac->mii.reg_mask = GENMASK(15, 0);
> > +     mac->mii.clk_csr_mask = GENMASK(21, 19);
>
> This will be the third instance of this particular mii data setup.
> Please consider adding a function to do this setup.
>
I will move this code segment to common helper function.
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature