On 7/1/26 14:31, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
to consider:
Pre-existing issues:
- [High] Calculations using `14835 * MEGA`, `5 * GIGA`, and `12 * GIGA`
silently overflow 32-bit `unsigned long` types on 32-bit architectures,
completely breaking clock tiering and boundary checks.
--
commit 8179a32cf1dac8f1d080e57bbce79246661b50eb
Author: AngeloGioacchino Del Regno <[email protected]>
phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow
This commit fixes an issue where calculating a PLL rate for high display
resolutions results in a pixel clock exceeding 32 bits, but the digital
clock divider calculation uses div_u64(), which expects a 32-bit divisor.
It resolves this by using div64_u64() instead to prevent overflow.
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,
[Severity: High]
This is a pre-existing issue, but do the constants in this function silently
overflow on 32-bit architectures?
There is no 32-bit architecture compatible with this driver. This will run only
on AArch64, and it's very unlikely to see a 32-bits kernel running on MT8195.
This can be eventually fixed with a later series, but shall not block the
proposed
fixes from being picked, both because it's unlikely to happen, and because those
are pre-existing issues.
Besides, the series *is* actually fixing some other overflow issues and was
already
tested good.
Earlier in mtk_hdmi_pll_calc(), there are tier threshold calculations like:
else if ((tmds_clk * 100) >= 14835 * MEGA && (tmds_clk * 10) < 2967 * MEGA)
and later boundary checks:
if (ns_hdmipll_ck >= 5 * GIGA &&
ns_hdmipll_ck <= 12 * GIGA)
If MEGA and GIGA evaluate as 32-bit types on 32-bit systems, a multiplication
like 14835 * MEGA evaluates to 14,835,000,000, which wraps around modulo 2^32
to 1,950,098,112 before being promoted to u64 for the comparison against
tmds_clk * 100.
Similarly, 5 * GIGA and 12 * GIGA would overflow, potentially rendering the
valid PLL clock range mathematically unreachable and forcing the function to
always return -EINVAL on 32-bit builds.
Should these constants be explicitly typed or cast to u64 to ensure the
comparisons work correctly on 32-bit kernels?
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);
if (!(digital_div <= 32 && digital_div >= 1))
return -EINVAL;
