Re: [PATCH net-next 3/6] net: stmmac: mediatek: rename MT2712 and MT8195 variant methods

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of `mediatek_dwmac_init()` is ignored during driver probe, allowing the driver to silently bind to unconfigured or unsupported hardware.
- [High] Early error return in `set_delay_v2()` causes state corruption of `mac_delay` values on unsupported PHY modes.
--

--- Patch [3]: [PATCH net-next 3/6] net: stmmac: mediatek: rename MT2712 and MT8195 variant methods ---
commit e972fc035e44817992b0ce16715cf1cbcf9432c0
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

    net: stmmac: mediatek: rename MT2712 and MT8195 variant methods
    
    In preparation of newer SoC support, rename MT2712 and MT8195 variant
    methods and sub functions to more generic names.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 0cabab4fd89a3..28e87990b0a10 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
[ ... ]
> @@ -317,13 +317,13 @@ static void mt8195_delay_stage2ps(struct mediatek_dwmac_plat_data *plat)
>  	mac_delay->rx_delay *= 290;
>  }
>  
> -static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
> +static int set_delay_v2(struct mediatek_dwmac_plat_data *plat)
>  {
>  	u32 gtxc_delay_val = 0, delay_val = 0, rmii_delay_val = 0;
>  	struct mac_delay_struct *mac_delay = &plat->mac_delay;
>  	u32 reg_offset = plat->variant->peri_eth_ctrl_offset;
>  
> -	mt8195_delay_ps2stage(plat);
> +	delay_ps2stage_v2(plat);
>  
>  	switch (plat->phy_mode) {
>  	case PHY_INTERFACE_MODE_MII:
> @@ -419,14 +419,14 @@ static int mt8195_set_delay(struct mediatek_dwmac_plat_data *plat)
>  		     reg_offset + MT8195_PERI_ETH_CTRL2,
>  		     rmii_delay_val);
>  
> -	mt8195_delay_stage2ps(plat);
> +	delay_stage2ps_v2(plat);

[Severity: High]
This isn't a bug introduced by this patch, but does the early return on
unsupported PHY modes in set_delay_v2() bypass this call?

If phy_mode is an unsupported mode, the switch statement falls through to the
default case which returns -EINVAL. This skips the call to
delay_stage2ps_v2() that restores the mac_delay values.

Since the values are unconditionally divided at the start of the function:

    delay_ps2stage_v2(plat);
        mac_delay->tx_delay /= 290;
        mac_delay->rx_delay /= 290;

If this happens during suspend/resume cycles, won't the delays be cumulatively
truncated to zero?

>  
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but is the return value of mediatek_dwmac_init()
intentionally ignored during driver probe?

Looking at mediatek_dwmac_probe() in this file:

    mediatek_dwmac_common_data(pdev, plat_dat, priv_plat);
    mediatek_dwmac_init(&pdev->dev, priv_plat);

    ret = mediatek_dwmac_clks_config(priv_plat, true);

If hardware initialization fails (for example, due to an unsupported PHY
interface returning -EINVAL), mediatek_dwmac_probe() ignores the return value
and proceeds with registration.

Could this result in the driver binding to unconfigured hardware, leaving a
phantom interface that fails to pass traffic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-dwmac-mediatek-mt8189-v1-0-17f345eaaca3@collabora.com?part=3