[PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Tue Jul 7 08:56:23 PDT 2026
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 at 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
More information about the linux-phy
mailing list