Re: [PATCH v3 4/5] clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Arithmetic on `p.k` and `input_fref` produces incorrect results on 32-bit platforms when `p.k` is negative, due to signed-to-unsigned integer promotion and subsequent zero-extension.
--

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

clk: renesas: rzv2h-cpg: Extract PLL calculation math into a library

This commit extracts the common PLL and divider calculation logic from the
RZ/V2H CPG driver into a standalone library. This restructuring allows sharing
the calculation infrastructure with upcoming platforms like RZ/T2H and RZ/N2H
without duplicating code.

> diff --git a/drivers/clk/renesas/rzv2h-cpg-lib.c b/drivers/clk/renesas/rzv2h-cpg-lib.c
> new file mode 100644
> index 0000000000000..124239c7327e4
> --- /dev/null
> +++ b/drivers/clk/renesas/rzv2h-cpg-lib.c

[ ... ]

> +				/* Compute (Ffvco * 65536) */
> +				fvco = mul_u32_u32(p.m * 65536 + p.k, fref);
> +				if (fvco < mul_u32_u32(limits->fvco.min, 65536) ||
> +				    fvco > mul_u32_u32(limits->fvco.max, 65536))
> +					continue;
> +
> +				/* PLL_M component of (output * 65536 * PLL_P) */
> +				output = mul_u32_u32(p.m * 65536, input_fref);
> +				/* PLL_K component of (output * 65536 * PLL_P) */
> +				output += p.k * input_fref;

[Severity: High]
Will this calculation corrupt the output frequency on 32-bit platforms?

Since input_fref is unsigned long and p.k is s16, if p.k is negative on a
32-bit system, doesn't it get promoted to a 32-bit unsigned integer,
wrapping around to a large positive value?

When added to the u64 output, it would be zero-extended rather than
sign-extended, resulting in a massive positive value being added instead
of subtracting the correct proportional amount.

Should p.k be explicitly cast to s64 before the multiplication to prevent
this issue when used by 32-bit targets?

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

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