Re: [PATCH net-next v5 06/10] net: stmmac: mediatek: use standard internal delay properties

[sashiko-bot@kernel.org]

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/drivers/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);
>  	}
>  
> -	if (!of_property_read_u32(plat->np, "mediatek,tx-delay-ps", &tx_delay_ps)) {
> -		if (tx_delay_ps < plat->variant->tx_delay_max) {
> -			mac_delay->tx_delay = 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 >= 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 >= 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 = min(tx_delay_ps,
> +					plat->variant->rgmii_delay_max);
>  	}
>  
> -	if (!of_property_read_u32(plat->np, "mediatek,rx-delay-ps", &rx_delay_ps)) {
> -		if (rx_delay_ps < plat->variant->rx_delay_max) {
> -			mac_delay->rx_delay = 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 >= 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 >= 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 = 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).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-dwmac-mediatek-mt8189-v5-0-7d99d6292962@collabora.com?part=6