Message-ID: <sashiko-outbox-208947@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH net-next v5 06/10] net: stmmac: mediatek: use standard
 internal delay properties
Reply-To: sashiko-reviews@lists.linux.dev
To: "Louis-Alexis Eyraud" <louisalexis.eyraud@collabora.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
In-Reply-To: <20261110-dwmac-mediatek-mt8189-v5-6-7d99d6292962@collabora.com>
References: <20261110-dwmac-mediatek-mt8189-v5-0-7d99d6292962@collabora.com>
 <20261110-dwmac-mediatek-mt8189-v5-6-7d99d6292962@collabora.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Wed, 11 Nov 2026 12:09:15 +0000
Precedence: list

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: stmmac: mediatek: off-by-one RGMII cap in
  mediatek_dwmac_config_dt()
--

commit 67e9a961803ad0d99ebf09bcf04f114dc90c181a
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

net: stmmac: mediatek: use standard internal delay properties

This commit adds support for the standard rx-internal-delay-ps and
tx-internal-delay-ps Device Tree properties for RGMII internal clock
delays, falling back to the vendor delay properties when the standard ones
are absent.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drive=
rs/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index cda3f3edde39f..ee43d694099bd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

[ ... ]

> @@ -427,24 +432,58 @@ static int mediatek_dwmac_config_dt(struct mediatek=
_dwmac_plat_data *plat)
>  		return PTR_ERR(plat->peri_regmap);
>  	}
> =20
> -	if (!of_property_read_u32(plat->np, "mediatek,tx-delay-ps", &tx_delay_p=
s)) {
> -		if (tx_delay_ps < plat->variant->tx_delay_max) {
> -			mac_delay->tx_delay =3D tx_delay_ps;
> -		} else {
> -			dev_err(plat->dev, "Invalid TX clock delay: %dps\n", tx_delay_ps);
> +	of_property_read_u32(plat->np, "mediatek,tx-delay-ps", &tx_delay_ps);
> +	if (tx_delay_ps >=3D plat->variant->mii_delay_max) {
> +		dev_err(plat->dev, "Invalid TX clock delay: %dps\n",
> +			tx_delay_ps);
> +		return -EINVAL;
> +	}
> +
> +	/* Read RGMII TX internal delay standard property */
> +	if (!of_property_read_u32(plat->np, "tx-internal-delay-ps",
> +				  &rgmii_tx_delay_ps)) {
> +		if (rgmii_tx_delay_ps >=3D plat->variant->rgmii_delay_max) {
> +			dev_err(plat->dev, "Invalid RGMII TX clock delay: %dps\n",
> +				rgmii_tx_delay_ps);
>  			return -EINVAL;
>  		}
> +	} else {
> +		/*
> +		 * standard property value not available, fallback on vendor
> +		 * value instead and cap it to max RGMII delay value
> +		 */
> +		rgmii_tx_delay_ps =3D min(tx_delay_ps,
> +					plat->variant->rgmii_delay_max);
>  	}
> =20
> -	if (!of_property_read_u32(plat->np, "mediatek,rx-delay-ps", &rx_delay_p=
s)) {
> -		if (rx_delay_ps < plat->variant->rx_delay_max) {
> -			mac_delay->rx_delay =3D rx_delay_ps;
> -		} else {
> -			dev_err(plat->dev, "Invalid RX clock delay: %dps\n", rx_delay_ps);
> +	of_property_read_u32(plat->np, "mediatek,rx-delay-ps", &rx_delay_ps);
> +	if (rx_delay_ps >=3D plat->variant->mii_delay_max) {
> +		dev_err(plat->dev, "Invalid RX clock delay: %dps\n",
> +			rx_delay_ps);
> +		return -EINVAL;
> +	}
> +
> +	/* Read RGMII RX internal delay standard property */
> +	if (!of_property_read_u32(plat->np, "rx-internal-delay-ps",
> +				  &rgmii_rx_delay_ps)) {
> +		if (rgmii_rx_delay_ps >=3D plat->variant->rgmii_delay_max) {
> +			dev_err(plat->dev, "Invalid RGMII RX clock delay: %dps\n",
> +				rgmii_rx_delay_ps);
>  			return -EINVAL;
>  		}
> +	} else {
> +		/*
> +		 * standard property value not available, fallback on vendor
> +		 * value instead and cap it to max RGMII delay value
> +		 */
> +		rgmii_rx_delay_ps =3D min(rx_delay_ps,
> +					plat->variant->rgmii_delay_max);
>  	}

[Severity: Medium]
Can capping rgmii_tx_delay_ps and rgmii_rx_delay_ps to
plat->variant->rgmii_delay_max overflow the 5-bit stage fields in
mt2712_set_delay()?

rgmii_delay_max is an exclusive upper bound (5440 on MT2712, or 32 * 170ps).
When a fallback vendor delay in [5440, 17599] ps is clamped to 5440,
dividing by MT2712_RGMII_DLY_STAGE_DIV yields stage 32 (0x20), which
FIELD_PREP() truncates to stage 0 in ETH_DLY_GTXC_STAGES and
ETH_DLY_RXC_STAGES instead of clamping to stage 31 (for example via
plat->variant->rgmii_delay_max - 1).

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20261110-dwmac-medi=
atek-mt8189-v5-0-7d99d6292962@collabora.com?part=3D6