Hi Alexey,
Resending so it hits the ML because I once again triggered the spam
filter by using the "wrong" mail address.
On 7/13/26 8:35 PM, Alexey Charkov wrote:
> Current code uses magical constants when rounding up the magnitude of
> negative fractional PLL component k. This leads to overshooting the
> requested rate when the calculated fractional part has less than 0.3 in
> its decimal part due to failure to round up the fractional part.
>
The choice of magic numbers sure is odd.
> Use a proper rounding up function to avoid overshooting the requested
> rate and make the calculation more readable.
>
Just for posterity, ffrac is guaranteed to not underflow in the if
condition because the caller of rockchip_rk3588_pll_k_get() actually
check that fvco < ffrac (computed with m+1).
ffrac * 65536 won't overflow because ffrac can be roughly max 1024 *
fin_hz, so fin_hz needs to be U64_MAX / 1024 / 65536 which is 256GHz.
Therefore:
Reviewed-by: Quentin Schulz <[email protected]>
Thanks!
Quentin
> Fixes: 6bfb37e70209 ("clk: rockchip: rk3588: fix up the frac pll
calculation")
> Signed-off-by: Alexey Charkov <[email protected]>
> ---
> drivers/clk/rockchip/clk_pll.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk_pll.c
b/drivers/clk/rockchip/clk_pll.c
> index d0df3b8fb49d..69d2d182dcb5 100644
> --- a/drivers/clk/rockchip/clk_pll.c
> +++ b/drivers/clk/rockchip/clk_pll.c
> @@ -11,6 +11,7 @@
> #include <asm/arch-rockchip/hardware.h>
> #include <div64.h>
> #include <linux/delay.h>
> +#include <linux/math64.h>
> static struct rockchip_pll_rate_table rockchip_auto_table;
> @@ -177,7 +178,10 @@ rockchip_rk3588_pll_k_get(u32 m, u32 p, u32 s,
u64 fin_hz, u64 fvco)
> k = ffrac * 65536 / fref;
> if (k > 32767) {
> ffrac = ((m + 1) * fref) - fvco;
> - k = ((ffrac * 65536 * 10 / fref) + 7) / 10;
> + /*
> + * Round up to avoid overshooting requested rate for negative k
> + */
> + k = DIV64_U64_ROUND_UP(ffrac * 65536, fref);
> if (k > 32767)
> k = 0;
> else
>
