Re: [PATCH v4 1/5] clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent 32-bit unsigned integer overflow and zero-extension corrupts PLL calculations when 'p.k' is negative on 32-bit architectures.
--

commit 68e2983ee9a6e8d870b293872442cdc2bc7b3e74
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations

Introduce a per-SoC PLL reference input frequency parameter to avoid
relying on a hardcoded 24MHz constant during PLL configuration math.

> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
> index e271c04cee34f..fff89f2bdc0b2 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c

[ ... ]

> @@ -335,9 +335,9 @@ bool rzv2h_get_pll_pars(const struct rzv2h_pll_limits *limits,
>  					continue;
>  
>  				/* PLL_M component of (output * 65536 * PLL_P) */
> -				output = mul_u32_u32(p.m * 65536, RZ_V2H_OSC_CLK_IN_MEGA);
> +				output = mul_u32_u32(p.m * 65536, input_fref);
>  				/* PLL_K component of (output * 65536 * PLL_P) */
> -				output += p.k * RZ_V2H_OSC_CLK_IN_MEGA;
> +				output += p.k * input_fref;
[Severity: High]
This is a pre-existing issue, but does this multiplication safely handle
negative values of p.k on 32-bit architectures?

Since p.k is a signed 16-bit integer (with limits->k.min allowed to be -32768)
and input_fref is a 32-bit unsigned long, C promotion rules on 32-bit builds
will convert p.k to a 32-bit unsigned integer before the multiplication.

If p.k is negative, the resulting 32-bit unsigned modulo value will be
zero-extended when added to the 64-bit output variable. This would incorrectly
add over 4 billion to the output frequency rather than subtracting the
intended fraction, which could severely break the PLL parameter search.

Could p.k be cast to s64 before the multiplication to prevent this unsigned
overflow and ensure proper sign-extension into the 64-bit output?

>  				/* Make it in mHz */
>  				output *= MILLI;
>  				output = DIV_U64_ROUND_CLOSEST(output, 65536 * p.p * divider);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618181949.3036280-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1