Messages in this thread | | | Subject | Re: [RFC PATCH] clk: fractional-divider: Correct max_{m,n} handed over to rational_best_approximation() | From | Liu Ying <> | Date | Wed, 14 Jul 2021 18:10:46 +0800 |
| |
On Wed, 2021-07-14 at 12:12 +0300, Andy Shevchenko wrote: > On Wed, Jul 14, 2021 at 02:41:29PM +0800, Liu Ying wrote: > > If a fractional divider clock has the flag > > CLK_FRAC_DIVIDER_ZERO_BASED set, the maximum > > numerator and denominator handed over to > > rational_best_approximation(), in this case > > max_m and max_n, should be increased by one > > comparing to those have the flag unset. Without > > this patch, a zero based fractional divider > > with 1-bit mwidth and 3-bit nwidth would wrongly > > generate 96MHz clock rate if the parent clock > > rate is 288MHz, while the expected clock rate > > is 115.2MHz with m = 2 and n = 5. > > Make sure that your editor is configured to allow you to have lines ~70-72 > characters long.
Alright, I'll see if I can improve this in v2 if necessary.
> > ... > > > The patch is RFC, because the rationale behind the below snippet in > > clk_fd_general_approximation() is unclear to Jacky and me and we are > > not sure if there is any room to improve this patch due to the snippet. > > Maybe, Andy may help shed some light here. Thanks. > > > > -----------------------------------8<--------------------------------- > > /* > > * Get rate closer to *parent_rate to guarantee there is no overflow > > * for m and n. In the result it will be the nearest rate left shifted > > * by (scale - fd->nwidth) bits. > > */ > > I don't know how to rephrase above comment better. > > > scale = fls_long(*parent_rate / rate - 1); > > if (scale > fd->nwidth) > > rate <<= scale - fd->nwidth; > > This takes an advantage of the numbers be in a form of > > n = k * 2^m, (1) > > where m will be scale in the snippet above. Thus, if n can be represented by > (1), we opportunistically reduce amount of bits needed for it by shifting right > by m bits. > > Does it make sense?
Thanks for your explaination. But, sorry, Jacky and I still don't understand this.
> > The code looks good to me, btw, although I dunno if you need to call the newly > introduced function before or after the above mentioned snippet.
Assuming that snippet is fully orthogonal to this patch, then it doesn't matter if it's before or after.
So, enlightenment/comments/ideas are welcome.
Thanks, Liu Ying
>
| |