Re: [PATCH v2 1/2] clk: rockchip: pll: Fix the fractional part denominator on RK3588/RK3576

Hello Alexey,

On 7/23/2026 6:21 PM, Alexey Charkov wrote:
> According to the TRM, the fractional PLL coefficient should be divided by
> 65536 rather than 65535 to obtain the output rate.
> 
> Fix the denominator and add a comment with the TRM provided clock formulae
> for future reference.
> 
> See RK3576 TRM Part 1 V1.2 section 2.13.1.4 Setting Guide on P, M, S and K
> or equivalently RK3588 TRM part 1 V1.0 section 2.17.1.4 Setting Guide on P,
> M, S and K.
> 
> Fractional PLL rates don't seem to be used by any current mainline
> consumers, so this is purely a correctness fix. It will also be important
> to properly support DisplayPort output going forward, as the video output
> controller derives its pixel clock from system PLLs with no dedicated PHY
> PLL option for DP unlike HDMI, and some display modes are only achievable
> with fractional PLL rates.
> 
> Fixes: 8f6594494b1c ("clk: rockchip: add pll type for RK3588")
> Signed-off-by: Alexey Charkov <[email protected]>
> ---
>  drivers/clk/rockchip/clk-pll.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
> index 6b853800cb6b..bf8acf7cee0d 100644
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -900,6 +900,13 @@ static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll,
>  	rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK);
>  }
>  
> +/*
> + * 2250 MHz <= Fvco <= 4500 MHz
> + * For Fvco > 3 GHz: period jitter +-1% frac PLL, +-0.75% int PLL
> + * For Fvco < 3 GHz: period jitter +-2% frac PLL, +-1.50% int PLL
> + * Fvco = ((m + k / 65536) * Fin) / p
> + * Fout = ((m + k / 65536) * Fin) / (p * 2^s)
> + */
>  static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
>  {
>  	struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
> @@ -915,7 +922,7 @@ static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned
>  		/* fractional mode */
>  		u64 frac_rate64 = prate * cur.k;
>  
> -		postdiv = cur.p * 65535;
> +		postdiv = cur.p * 65536;


How about using 65536ULL? Will the multiplication here overflow?

>  		do_div(frac_rate64, postdiv);
>  		rate64 += frac_rate64;
>  	}
> 

-- 
Best, 
Chaoyi