Re: [PATCH v2 1/3] clk: meson: Support PLL with fixed fractional denominators

[Chuan Liu <chuan.liu@amlogic.com>]

Hi, Jerome:

         Thank you for your meticulous explanation.


On 2024/9/9 15:40, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon 09 Sep 2024 at 09:55, Chuan Liu <chuan.liu@amlogic.com> wrote:
>
>> Sorry, I don't quite understand this one. Is it because you suggest keeping
>>
>> "(1 << pll->frac_max)" here, followed by "if" to determine whether to assign
>>
>> "pll->frac_max"?
>>
>>
>> "unlikely" is used here. My idea is that it will be possible to determine
>> the value
>>
>> of "frac_max" at compile time, which will result in one less "if" judgment
>> and
>>
>> slightly improve drive performance.
> I'll rephrase.
>
> Please drop the 'unlikely()' call.
>
> You may add that :
>   * in a separate change
>   * if you really really wish to
>   * if you provide profiling numbers for the different supported
>     platforms and PLLs, not just the one targeted by this patchset.


Okay, Understood. So you suggest like this?

static unsigned long __pll_params_to_rate(unsigned long parent_rate,
                                           struct meson_clk_pll_data *pll)
  {
         u64 rate = (u64)parent_rate * m;
+       unsigned int frac_max = (1 << pll->frac.width);

         if (frac && MESON_PARM_APPLICABLE(&pll->frac)) {
                 u64 frac_rate = (u64)parent_rate * frac;

-               rate += DIV_ROUND_UP_ULL(frac_rate,
-                                        (1 << pll->frac.width));
+               if (pll->frac_max)
+                       frac_max = pll->frac_max;
+
+               rate += DIV_ROUND_UP_ULL(frac_rate, frac_max);


In my opinion, this change seems more logical, but the amount of

change is larger?😮