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 <[email protected]> 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/[email protected]?part=10
