Re: [PATCH net-next v9 1/4] net: stmmac: Add DW25GMAC support in stmmac core driver

[Russell King (Oracle)]

Not withstanding my comment about the other Synopsys xlgmac driver that
we have in the kernel...

On Thu, Apr 02, 2026 at 02:36:26PM -0700, Jitendra Vegiraju wrote:
> From: Jitendra Vegiraju <jitendra.vegiraju@xxxxxxxxxxxx>
> 
> The DW25GMAC introduced a new DMA architecture called Hyper-DMA (HDMA) for
> virtualization scalability. This is realized by decoupling physical DMA
> channels(PDMA) from potentially large number of virtual DMA channels(VDMA).
> The VDMAs provide software abstraction to driver that map to PDMAs for
> frame transmission and reception.
> Since 25GMAC is a derivative of XGMAC, majority of IP is common to both.
> 
> To add support for the HDMA in 25GMAC, a new instance of dma_ops,
> dw25gmac400_dma_ops is introduced.
> To support the current needs, a simple one-to-one mapping of dw25gmac's
> logical VDMA (channel) to TC to PDMAs is used. Most of the other dma
> operation functions in existing dwxgamc2_dma.c file are reused whereever

Typo: dwxgmac2_dma.c

> applicable.
> Added setup function for DW25GMAC's stmmac_hwif_entry in stmmac core.

In a previous review, I questioned the use of DWMAC_CORE_25GMAC and
asked about its version numberspace. I believe you indicated that the
version numberspace is the same as the existing XGMAC core.

I'm going to question the value of adding DWMAC_CORE_25GMAC.

1. What is the value of splitting DWMAC_CORE_25GMAC from
   DWMAC_CORE_XGMAC given that it's in the same versioning numberspace
   as XGMAC, and most tests (via dwmac_is_xgmac()) test for XGMAC or
   25GMAC?

2. Have you reviewed all the places that explicitly test for
   DWMAC_CORE_XGMAC, looking at their "false" paths (for non-XGMAC
   cores) to determine whether they are suitable? For example:

        if (priv->plat->core_type == DWMAC_CORE_XGMAC)
                ndev->max_mtu = XGMAC_JUMBO_LEN;
        else if (priv->plat->enh_desc || priv->synopsys_id >= DWMAC_CORE_4_00)
                ndev->max_mtu = JUMBO_LEN;
        else
                ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);

   XGMAC can handle a max MTU of 16368, but with your code using
   DWMAC_CORE_25GMAC, we fall back to the next test, which tests the
   IP version against 0x40, and uses a max MTU of 9000. Given that
   DWMAC_CORE_4_00 is a different "version number space" this seems
   wrong.

3. Looking at the MDIO code, this looks very wrong if you're
   introducing DWMAC_CORE_25GMAC. Have you tested MDIO accesses?

   dwxgmac2_setup() is called for DWMAC_CORE_XGMAC core-type. In
   stmmac_mdio_register(), DWMAC_CORE_XGMAC uses different functions
   for MDIO bus access for C22 and C45 from other cores - it uses the
   stmmac_xgmac2_mdio_* functions.

   These use stmmac_xgmac2_c45_format() and stmmac_xgmac2_c22_format()
   to format the register values which do not depend on mii.*_mask, but
   do use mii.address and mii.data for the register offsets. Thus, is
   there any point to setting mii.addr_mask and mii.reg_mask ?

   For non-DWMAC_CORE_XGMAC cores, we fall back to the stmmac_mdio_*()
   functions, which for non-DWMAC_CORE_GMAC4 will only support Clause
   22 access, not Clause 45 - which would be very strange for a 25G
   core.

4. What about the feature printing in
   stmmac_main.c::stmmac_dma_cap_show() ?

5. What about similar tests in stmmac_est.c and stmmac_ethtool.c ?



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