Re: [PATCH 11/12] phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data - sashiko-bot

From: sashiko-bot@kernel.org
To: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
Cc: devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	olteanv@gmail.com, robh@kernel.org, conor+dt@kernel.org,
	vkoul@kernel.org, linux-phy@lists.infradead.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 11/12] phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data
Date: Wed, 01 Jul 2026 13:07:10 +0000	[thread overview]
Message-ID: <20260701130710.C83071F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260701122008.19509-12-angelogioacchino.delregno@collabora.com>

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

Pre-existing issues:
- [High] mtk_dp_phy_disable_all_lanes() executes undefined behavior if initially zero, and incorrectly clears lanes by shifting the lane index instead of a bitmask.
--

commit d17ede4ab4b1189e133fc8f8e84c59aaf2d2590e
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data

This commit prepares for supporting newer SoCs by moving the PHYD_TX_LN_EN
register mask into the SoC-specific data structure.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index 7c8c04787b78c..f067acef222f7 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c

[ ... ]

> @@ -442,7 +446,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
>  		return ret;
>  
>  	/* Get mask of currently enabled lane */
> -	val = FIELD_GET(PHYD_TX_LN_EN, val);
> +	val = field_get(pdata->mask_dig_tx_ln, val);
>  
>  	/* Disable all lanes (needs to be done one by one, from last to first) */
>  	do {
> @@ -451,7 +455,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
>  
>  		ret = regmap_clear_bits(dp_phy->regmap,
>  					pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0],
> -					FIELD_PREP(PHYD_TX_LN_EN, lane_num));
> +					field_prep(pdata->mask_dig_tx_ln, lane_num));

[Severity: High]
This is a pre-existing issue, but does this loop safely handle the case
where val is initially 0?

Since this is a do-while loop, if no lanes are currently enabled, fls(0)
will evaluate to 0, which leaves lane_num as -1. This would then result
in an out-of-bounds shift during the BIT(lane_num) calculation.

Additionally, should lane_num be passed into field_prep() as a bitmask
(e.g., BIT(lane_num)) rather than the raw integer index? 

Passing the integer index directly (like 2 or 3) instead of a bitmask
appears to generate an incorrect value, which could cause regmap_clear_bits()
to clear the wrong bits in the register and leave lanes incorrectly powered on.

>  		if (ret)
>  			return ret;
>  	} while (val);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701122008.19509-1-angelogioacchino.delregno@collabora.com?part=11

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy