Re: [PATCH net-next 5/6] net: stmmac: mediatek: add support for TX deallocation adjustment feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The fine stage delay field MT8189_CTRL0_DLY_GTXC_STAGE_FINE is defined as a multi-bit mask but is incorrectly used as a value, unconditionally setting all 6 bits to 1. Additionally, reciprocal delay functions are not updated to account for fine stages.
--

--- Patch [5]: [PATCH net-next 5/6] net: stmmac: mediatek: add support for TX deallocation adjustment feature ---
commit 848888a93a986153c930951bd429a61b8a876923
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

net: stmmac: mediatek: add support for TX deallocation adjustment feature

The MT8189 SoC has in the Ethernet control 0 register from the
peripheral configuration (pericfg) additional bits to adjust the TX
deallocation.

In preparation of MT8189 SoC support, add its definition, use in the
set_delay_v2 callback, and a support flag in the platform data.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index bcc0baef3f718..6b0a42b5839f2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -37,7 +37,8 @@
>  #define ETH_FINE_DLY_RXC	BIT(0)
>  
>  /* Peri Configuration register for mt8189 */
> -#define MT8189_CTRL0_TXC_OUT_OP		BIT(20)
> +#define MT8189_CTRL0_TXC_OUT_OP			BIT(20)
> +#define MT8189_CTRL0_DLY_GTXC_STAGE_FINE	GENMASK(11, 6)

[ ... ]

> @@ -402,6 +407,9 @@ static int set_delay_v2(struct mediatek_dwmac_plat_data *plat)
>  		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_STAGES, mac_delay->tx_delay);
>  		gtxc_delay_val |= FIELD_PREP(MT8195_DLY_GTXC_INV, mac_delay->tx_inv);
>  
> +		if (plat->variant->use_stage_fine)
> +			gtxc_delay_val |= MT8189_CTRL0_DLY_GTXC_STAGE_FINE;

[Severity: Medium]
Does this unintentionally set all 6 bits of the fine stage delay to 1?

MT8189_CTRL0_DLY_GTXC_STAGE_FINE is defined as GENMASK(11, 6), making it a
multi-bit mask. Other delay configurations in this function use FIELD_PREP()
with a specific value, like FIELD_PREP(MT8195_DLY_GTXC_STAGES,
mac_delay->tx_delay).

If this is intended to hold a programmable fine delay value, should it be
using FIELD_PREP() instead of ORing the mask directly into gtxc_delay_val?

Additionally, does delay_stage2ps_v2() need to be updated to account for
these fine stages when reconstructing the picosecond delay?

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