Re: [PATCH 1/2] phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow

[sashiko-bot@kernel.org]

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 <angelogioacchino.delregno@collabora.com>

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?

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;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701121936.19407-1-angelogioacchino.delregno@collabora.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy