Re: bounds of pdiv in clk-pll14xx.c

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, Jun 21, 2023 at 01:24:12PM +0200, Rasmus Villemoes wrote:
> I'm a bit confused by the range of pdiv used in
> imx_pll14xx_calc_settings(), introduced in commit b09c68dc57c9 (clk:
> imx: pll14xx: Support dynamic rates).
> 
> We have this comment
> 
>         /*
>          * Fractional PLL constrains:
>          *
>          * a) 6MHz <= prate <= 25MHz
>          * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz)
>          * c) 64 <= m <= 1023
>          * d) 0 <= s <= 6
>          * e) -32768 <= k <= 32767
> 
> and those values match what I can find in the reference manuals for the
> imx8mm, imx8mn and imx8mp SOCs. But the code then only loops over 1 <= p
> <= 7. I also don't really understand what the parenthesis
> 
>    (1 <= p <= 4 prate = 24MHz)
> 
> is supposed to mean. Is p restricted to <= 4 when the parent rate is
> 24MHz? That doesn't seem to make any sense, and in any case the loop
> does go up to p==7.

The original patch from Adrian that my patch is based on indeed had 1 <=
p <= 4. I did some digging but I haven't found the reason why this was
changed to 1 <= p <= 7 in the patch I upstreamed. I can imagine that at
some point we realized that we also got suboptimal rates and therefore
changed it, but I don't know.

> 
> It also seems that the built-in entries for 393216000 and 361267200 are
> suboptimal. Using m=655, p=5, s=3, k=23593 would give 393216000 exactly,
> and that set would be found by the loop if it wasn't being preceded by
> the table lookup.

I gave the table precedence in case it contains qualified known good
values. If these values turn out to be not so good after all then I
suggest that we remove them.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |