Re: [PATCH v2 1/2] phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-11 07:51:33
Also in:
dri-devel
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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/20260911074015.9994-1-angelogioacchino.delregno@collabora.com?part=1 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy