Hi Jonas, On Thu, Jul 30, 2026 at 4:16 PM Jonas Karlman <[email protected]> wrote: > > Hi Alexey, > > On 7/23/2026 5:04 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. > > > > Use a proper rounding up function to avoid overshooting the requested > > rate and make the calculation more readable. > > > > Fixes: 6bfb37e70209 ("clk: rockchip: rk3588: fix up the frac pll > > calculation") > > Reviewed-by: Quentin Schulz <[email protected]> > > 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); > > Use of DIV64_U64_ROUND_UP() seem to cause build errors for Rockchip 32-bit > tagets, see CI job at [1]. > > [1] > https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/jobs/54629#L4016
Weirdly, the macro is only defined for 64bit [2], even though the function it wraps around also exists for other word sizes. This calls for a separate fix to take the macro definition to the bottom of the header, like Linux does [3]. [2] https://elixir.bootlin.com/u-boot/v2026.07/source/include/linux/math64.h#L51-L53 [3] https://elixir.bootlin.com/linux/v7.2-rc3/source/include/linux/math64.h#L344-L355 Thanks for spotting this! Best regards, Alexey
