Re: [PATCH 01/15] clk: rockchip: pll: Fix double use of postdiv1

Hi Jonas,

On 8/7/26 12:38 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 8/7/2026 11:36 AM, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 7/30/26 4:12 PM, Jonas Karlman wrote:
>>> The possible frac_rate part of rk3036_pll plls is divided two times by
>>> postdiv1 instead of both postdiv1 and postdiv2 as the integer part. Fix
>>> this by using postdiv2 in the second do_div() call.
>>>
>>
>> I... cannot find which SoC(s) actually makes use of that function. It's
>> only called when pll->type = rk3036 (which is 0, so anything that
>> doesn't explicitly set ->type will have its type be rk3036), but all the
>> drivers declaring a rockchip_pll_clock array specify something that
>> isn't rk3036. So... is this dead code or what did I miss :)?
> 
> It is also called for the pll_rk3328 type, i.e. rk3308, rk3506, rk3528,
> rk3588 and rv1126, strangely not for rk3328 (in U-Boot).
> 

s/rk3588/rk3568/ above.

Thanks for the pointer, I missed the second call to 
rk3036_pll_[gs]et_rate() when pll->type = pll_rk3328.

As for rk3328, it isn't using the common PLL core from 
drivers/clk/rockchip/clk-pll.c but it is indeed weird to have it named 
following the name of an SoC that doesn't actually make use of it.

> I do think there is a very low chance we actually use the fractal pll
> rates in U-Boot but the double use of postdiv1 seems (and should be)
> wrong.
> 

Alexey started to look into supporting them for RK3576 and found a few 
issues, so I'm guessing we are going to start seeing more users.

>>
>> Where did you get the formula also, I couldn't quickly find a publicly
>> leaked TRM for RK3036 (and since I couldn't figure out which SoC(s) use
>> the same formula...).
> 
> I think they are very similar as most fractional plls used by RK.
> 
> Following is a snippet from Rockchip RK3036 TRM V1.0 20150907-Part1
> (that used to exist in a GitHub repo at Poco-Ye/rk-datasheet)
> 
>    The Fractional PLL output frequency can be calculated using some
>    simple formulas.
>    
>    If DSMPD = 1 (DSM is disabled, "integer mode")
>     FOUTVCO = FREF / REFDIV * FBDIV
>     FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
>    
>    If DSMPD = 0 (DSM is enabled, "fractional mode")
>     FOUTVCO = FREF / REFDIV * (FBDIV + FRAC / 224)

This one's funny because I think it's supposed to be 2^24 and not 224. 
They made the same mistake in the RK3308 TRM. It's fine in other TRMs.

>     FOUTPOSTDIV = FOUTVCO / POSTDIV1 / POSTDIV2
>    
>    Where:
>     FOUTVCO = Fractional PLL non-divided output frequency
>     FOUTPOSTDIV = Fractional PLL divided output frequency (output of
>                   second post divider)
>     FREF = Fractional PLL input reference frequency
>     REFDIV = Fractional PLL input reference clock divider
>     FVCO = Frequency of internal VCO
>     FBDIV = Integer value programmed into feedback divide
>     FRAC = Fractional value programmed into DSM
> 
> And Linux use following in rockchip_rk3036_pll_recalc_rate():
> 
> 	rate64 *= cur.fbdiv;
> 	do_div(rate64, cur.refdiv);
> 
> 	if (cur.dsmpd == 0) {
> 		/* fractional mode */
> 		u64 frac_rate64 = prate * cur.frac;
> 
> 		do_div(frac_rate64, cur.refdiv);
> 		rate64 += frac_rate64 >> 24;
> 	}
> 
> 	do_div(rate64, cur.postdiv1);
> 	do_div(rate64, cur.postdiv2);
> 
> In U-Boot the postdiv part seem to be handled for the integer and
> fractal parts separately. Maybe we should just try to adopt something
> similar/closer to Linux?
> 

Casey has posted a series[1] porting some parts of the Linux's CCF to 
U-Boot which should allow us to import the Linux kernel driver(s) with 
minimal adaptation[2]. We would still need a minimal clock driver for 
xPL (and currently also proper pre-relocation) if I understood 
correctly, but we hopefully would need so little that it wouldn't cost 
us much to effectively have two separate clock drivers depending on the 
boot stage. I believe she's looking into making it work in proper pre-reloc.

I'm honestly not sure if I'll have the time to give this a try any time 
soon but maybe someone else would like to see if this can work out for 
us. I would also feel much better if we had CI with Rockchip boards so 
it feels less like a coin toss to do such big changes.

[1] 
https://lore.kernel.org/u-boot/[email protected]/
[2] 
https://lore.kernel.org/u-boot/[email protected]/

Cheers,
Quentin