Re: [PATCH net-next v4 08/10] net: stmmac: mediatek: add PERI_ETH_CTRLx register offset in platform data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net: stmmac: mediatek: mediatek_dwmac_config_dt() rejects valid MII delays
- [Low] net: stmmac: mediatek: wrong variable logged in mediatek_dwmac_config_dt()
--

commit 05d42c75b36c4f80f5abd461a78c55c1f2aba21e
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

net: stmmac: mediatek: add PERI_ETH_CTRLx register offset in platform data

This commit adds a peri_eth_ctrl_offset field to struct
mediatek_dwmac_variant and updates mt8195_set_interface() and
mt8195_set_delay() to access the PERI_ETH_CTRLx syscon registers relative
to that base offset.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 61bc1ff08cc44..0426a08769569 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

[ ... ]

> @@ -432,6 +443,7 @@ static const struct mediatek_dwmac_variant mt8195_gmac_variant = {
>  	.mii_delay_stage_div = MT8195_MII_DLY_STAGE_DIV,
>  	.rgmii_delay_stage_div = MT8195_RGMII_DLY_STAGE_DIV,
>  	.dma_bit_mask = 35,
> +	.peri_eth_ctrl_offset = MT8195_PERI_ETH_CTRL_OFFSET,
>  };
>  
>  static int mediatek_dwmac_config_dt(struct mediatek_dwmac_plat_data *plat)

[Severity: High]
In mediatek_dwmac_probe() -> mediatek_dwmac_config_dt(), can valid MII or
RMII clock delays on MT2712 be rejected with -EINVAL?

In drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c, mt2712_gmac_variant
defines a larger maximum delay for MII/RMII (17600 ps) than for RGMII
(5440 ps):

static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
	...
	.mii_delay_max = MT2712_MII_DELAY_MAX,
	.rgmii_delay_max = MT2712_RGMII_DELAY_MAX,
	...
};

However, when the RGMII-specific tx-internal-delay-ps or rx-internal-delay-ps
properties are absent in devicetree (as is normal for MII or RMII modes),
mediatek_dwmac_config_dt() falls back rgmii_tx_delay_ps to tx_delay_ps and
rgmii_rx_delay_ps to rx_delay_ps, and then unconditionally checks both
against plat->variant->rgmii_delay_max regardless of plat->phy_mode:

	if (of_property_read_u32(plat->np, "tx-internal-delay-ps",
				 &rgmii_tx_delay_ps)) {
		rgmii_tx_delay_ps = tx_delay_ps;
	}

	if (rgmii_tx_delay_ps >= plat->variant->rgmii_delay_max) {
		dev_err(plat->dev, "Invalid RGMII TX clock delay: %dps\n",
			rgmii_tx_delay_ps);
		return -EINVAL;
	}

Wouldn't any MT2712 board in MII or RMII mode with mediatek,tx-delay-ps or
mediatek,rx-delay-ps in the valid range [5440, 17600) ps fail the
rgmii_delay_max check and abort probe?

[Severity: Low]
Also in mediatek_dwmac_config_dt() in
drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c, when the RGMII RX
clock delay check fails:

	if (rgmii_rx_delay_ps >= plat->variant->rgmii_delay_max) {
		dev_err(plat->dev, "Invalid RGMII RX clock delay: %dps\n",
			rgmii_tx_delay_ps);
		return -EINVAL;
	}

Should dev_err() log rgmii_rx_delay_ps instead of rgmii_tx_delay_ps here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-dwmac-mediatek-mt8189-v4-0-c86c14fc50f4@collabora.com?part=8