Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] mtk_dp_phy_disable_all_lanes() executes undefined behavior if initially zero, and incorrectly clears lanes by shifting the lane index instead of a bitmask. -- commit d17ede4ab4b1189e133fc8f8e84c59aaf2d2590e Author: AngeloGioacchino Del Regno <[email protected]> phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data This commit prepares for supporting newer SoCs by moving the PHYD_TX_LN_EN register mask into the SoC-specific data structure. > diff --git a/drivers/phy/mediatek/phy-mtk-dp.c > b/drivers/phy/mediatek/phy-mtk-dp.c > index 7c8c04787b78c..f067acef222f7 100644 > --- a/drivers/phy/mediatek/phy-mtk-dp.c > +++ b/drivers/phy/mediatek/phy-mtk-dp.c [ ... ] > @@ -442,7 +446,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy > *dp_phy) > return ret; > > /* Get mask of currently enabled lane */ > - val = FIELD_GET(PHYD_TX_LN_EN, val); > + val = field_get(pdata->mask_dig_tx_ln, val); > > /* Disable all lanes (needs to be done one by one, from last to first) > */ > do { > @@ -451,7 +455,7 @@ static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy > *dp_phy) > > 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)); > + field_prep(pdata->mask_dig_tx_ln, > lane_num)); [Severity: High] This is a pre-existing issue, but does this loop safely handle the case where val is initially 0? Since this is a do-while loop, if no lanes are currently enabled, fls(0) will evaluate to 0, which leaves lane_num as -1. This would then result in an out-of-bounds shift during the BIT(lane_num) calculation. Additionally, should lane_num be passed into field_prep() as a bitmask (e.g., BIT(lane_num)) rather than the raw integer index? Passing the integer index directly (like 2 or 3) instead of a bitmask appears to generate an incorrect value, which could cause regmap_clear_bits() to clear the wrong bits in the register and leave lanes incorrectly powered on. > if (ret) > return ret; > } while (val); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
