Re: [PATCH v4 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup
From: sashiko-bot@kernel.org
Date: 2026-09-10 09:30:34
Also in:
dri-devel, linux-phy
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mtk_dp_phy_disable_all_lanes() ignores the return value of regmap_read() and unconditionally returns 0, leading to uninitialized memory use and dead code. -- commit 96237b9548980d0963b925da5fbae9fec554beb6 Author: AngeloGioacchino Del Regno [off-list ref] Subject: phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup This commit adds support for configuring a specific number of lanes via the .configure() callback. It also introduces functionality to disable all lanes during power-off and reset to ensure proper hardware state cleanup.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c index 4247506ceb996..276391d991aae 100644 --- a/drivers/phy/mediatek/phy-mtk-dp.c +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -254,12 +272,39 @@ 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; + + /* Get mask of currently enabled lane */ + regmap_read(dp_phy->regmap, pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0], &val);
[Severity: High] Does this code handle the possibility of regmap_read() failing? If regmap_read() returns an error (e.g., if the underlying bus or clock is in an invalid state), it will not initialize val. In this scenario, the subsequent operations would use uninitialized stack memory, potentially leading to random lanes being disabled based on garbage data.
+ val = FIELD_GET(PHYD_TX_LN_EN, val);
+ if (val == 0)
+ return 0;
+
+ /* Disable all lanes (needs to be done one by one, from last to first) */
+ do {
+ u32 lane_num = fls(val) - 1;
+ val &= ~BIT(lane_num);
+
+ regmap_clear_bits(dp_phy->regmap,
+ pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0],
+ FIELD_PREP(PHYD_TX_LN_EN, BIT(lane_num)));
+ } while (val);
+
+ return 0;[Severity: High] Should this function return the error state from the regmap operations? It declares a ret variable but never uses it, and unconditionally returns 0.
quoted hunk ↗ jump to hunk
+} + 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; regmap_set_bits(dp_phy->regmap, pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],@@ -269,6 +314,12 @@ static int mtk_dp_phy_power_off(struct phy *phy) regmap_write(dp_phy->regmap, pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0); + ret = mtk_dp_phy_disable_all_lanes(dp_phy); + if (ret) { + dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n"); + return ret; + }
[Severity: High] Since mtk_dp_phy_disable_all_lanes() unconditionally returns 0, this error handling block appears to be dead code, preventing the driver from properly detecting hardware state failures during cleanup. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260910092038.48291-1-angelogioacchino.delregno@collabora.com?part=6