Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Multiplications involving `MEGA` and `GIGA` macros overflow on 32-bit architectures, breaking clock calculations. -- commit 83c70bcce00287d7e880c60c53f322581568117d Author: AngeloGioacchino Del Regno <[email protected]> phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow When trying to calculate a PLL rate for target display resolutions above 2560x1440, 24bpp, 30Hz, the pixel clock value will be more than 32-bits long but the division to finally calculate the digital clock divider is being done with div_u64(), which expects a 32bit unsigned divisor. > diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > index 1426a2db984d5..e6ee8e0800222 100644 > --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c > @@ -290,7 +290,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy > *hdmi_phy, struct clk_hw *hw, > posdiv2 = 1; > > /* Digital clk divider, max /32 */ > - digital_div = div_u64(ns_hdmipll_ck, posdiv1 * posdiv2 * pixel_clk); > + digital_div = div64_u64(ns_hdmipll_ck, posdiv1 * posdiv2 * pixel_clk); [Severity: High] This isn't a bug introduced by this patch, but since this patch fixes 32-bit overflows, could the multiplications involving MEGA and GIGA in mtk_hdmi_pll_calc() and mtk_hdmi_pll_drv_setting() also overflow on 32-bit architectures? In drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c:mtk_hdmi_pll_calc(), the bounds checks use MEGA and GIGA: else if (tmds_clk >= 54 * MEGA && (tmds_clk * 100) < 14835 * MEGA) and: if (ns_hdmipll_ck >= 5 * GIGA && ns_hdmipll_ck <= 12 * GIGA) Since MEGA and GIGA are defined with the UL suffix in <linux/units.h>, do values like 14835 * MEGA and 5 * GIGA silently wrap around when evaluated as 32-bit unsigned long values (e.g., 14835 * MEGA becoming ~1.95 billion)? Similarly, in mtk_hdmi_pll_drv_setting(): } else if (((u64)pixel_clk * 1000) >= 74175 * MEGA && pixel_clk <= 300 * MEGA) { Would 74175 * MEGA wrap to ~1.13 billion on 32-bit builds instead of 74.1 billion, potentially intercepting conditions that should fall to a different impedance configuration block? > if (!(digital_div <= 32 && digital_div >= 1)) > return -EINVAL; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
