Re: [PATCH 10/12] phy: phy-mtk-dp: Add bitrate register val definitions to SoC data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtk_dp_phy_disable_all_lanes loop executes undefined behavior when val is 0, and incorrectly clears lane bits using the lane index instead of a bitmask.
--

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

phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

Add support for enabling a specifically requested number of lanes in the
configure callback, and disable all lanes in power off and reset
callbacks for proper hardware cleanup.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> index 586e727956332..a2cd22b9da066 100644
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
> @@ -261,6 +267,35 @@ static int mtk_dp_phy_power_on(struct phy *phy)
>  	return 0;
>  }
>  
> +static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
> +{
> +	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
> +	const u8 *regs = pdata->regs_dig_glb;
> +	int ret;
> +	u32 val;
> +
> +	ret = regmap_read(dp_phy->regmap, pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0], &val);
> +	if (ret)
> +		return ret;
> +
> +	/* Get mask of currently enabled lane */
> +	val = FIELD_GET(PHYD_TX_LN_EN, val);
> +
> +	/* Disable all lanes (needs to be done one by one, from last to first) */
> +	do {
> +		u32 lane_num = fls(val) - 1;

[Severity: High]
If no lanes are currently enabled, val will be 0. Does this cause fls(0) - 1
to underflow, leading to undefined behavior when computing BIT(lane_num)?

> +		val &= ~BIT(lane_num);
> +
> +		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));

[Severity: High]
Does this incorrectly clear bits using the lane index instead of a bitmask?

For example, if lane 3 is being disabled (lane_num == 3), this passes 3 to
FIELD_PREP instead of BIT(3). This would clear lanes 0 and 1 (3 << 4) while
leaving lane 3 enabled. Should this use BIT(lane_num) instead?

> +		if (ret)
> +			return ret;
> +	} while (val);
> +
> +	return 0;
> +}
[ ... ]

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