Re: [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Inconsistent use of device pointers for error logging in `mtk_dp_phy_power_off` and `mtk_dp_phy_reset`.
--

--- Patch [6]: [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup ---
commit cab309557ac4fcb462c5b8978a8cd660acab1ec1
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

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

This commit adds support for enabling a specifically requested number of
lanes in the .configure() callback. It also updates the power off and reset
callbacks to disable all lanes for proper hardware cleanup.

> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
> @@ -263,31 +281,68 @@ static int mtk_dp_phy_power_on(struct phy *phy)
[ ... ]
>  static int mtk_dp_phy_power_off(struct phy *phy)
>  {
>  	struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
>  	const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
>  	const u8 *regs_dig = pdata->regs_dig_glb;
>  	const u8 *regs_ana = pdata->regs_ana_glb;
> -	int ret_cktx, ret_aux;
> +	int ret_cktx, ret_aux, ret;
>  
>  	ret_cktx = regmap_set_bits(dp_phy->regmap,
>  				   pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
>  				   CKM_CKTX0_EN_FORCE_MODE);
> +	if (ret_cktx)
> +		dev_err(&phy->dev, "Could not disable CKTX0: %d\n", ret_cktx);
[ ... ]
> +	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
> +	if (ret) {
> +		dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n");

[Severity: Low]
Should this use &phy->dev instead of dp_phy->dev for error logging?

The adjacent error handlers for CKTX and AUX in this same function log 
errors using &phy->dev. Mixing device pointers might lead to inconsistent 
device names in the kernel log.

> +		return ret;
>  	}
[ ... ]
> @@ -307,6 +363,11 @@ static int mtk_dp_phy_reset(struct phy *phy)
[ ... ]
>  	/* Setting bits means go out of reset */
>  	regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
>  
> +	/* Disable all lanes and continue reset even if this fails, but notify */
> +	ret = mtk_dp_phy_disable_all_lanes(dp_phy);
> +	if (ret)
> +		dev_err(dp_phy->dev, "Could not disable lanes during reset!\n");

[Severity: Low]
Similarly here, would it be better to use &phy->dev for consistency with 
the rest of the API implementation?

> +
>  	return 0;
>  }

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

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