| Message ID | 20180117201421.25954-3-jernej.skrabec@siol.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Jan 17, 2018 at 09:14:11PM +0100, Jernej Skrabec wrote: > This commit changes formula from this: > > Freq = (parent_freq * N * K) / (M * P) > > to this: > > Freq = (parent_freq / M) * N * K / P > > This improves situation when N is in the range 1-255. PLL parent clock > is almost always 24 MHz, which means that for N >= 180 original formula > overflows and result becomes useless. Situation can be improved if M is > used as predivider as it can be seen in the second formula. That way at > least M > 1 is considered, but it still leaves small gap for wrong result > when M = 1 and N >= 180. > > Using M as predivider shouldn't cause any issue, because it is in range > 1-4 at most, so there is no or only minimal rounding error. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> I'd really prefer to stick to the formula documented and that we've used so far. NKMP clocks are most notably used for the CPU PLLs and I've debugged way too many cpufreq bugs already :) What about using long long types for the parent * n * k result? Maxime
Hi, Dne četrtek, 18. januar 2018 ob 11:58:41 CET je Maxime Ripard napisal(a): > Hi, > > On Wed, Jan 17, 2018 at 09:14:11PM +0100, Jernej Skrabec wrote: > > This commit changes formula from this: > > > > Freq = (parent_freq * N * K) / (M * P) > > > > to this: > > > > Freq = (parent_freq / M) * N * K / P > > > > This improves situation when N is in the range 1-255. PLL parent clock > > is almost always 24 MHz, which means that for N >= 180 original formula > > overflows and result becomes useless. Situation can be improved if M is > > used as predivider as it can be seen in the second formula. That way at > > least M > 1 is considered, but it still leaves small gap for wrong result > > when M = 1 and N >= 180. > > > > Using M as predivider shouldn't cause any issue, because it is in range > > 1-4 at most, so there is no or only minimal rounding error. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > I'd really prefer to stick to the formula documented and that we've > used so far. NKMP clocks are most notably used for the CPU PLLs and > I've debugged way too many cpufreq bugs already :) > > What about using long long types for the parent * n * k result? Yes, using long long is the best possible solution and covers all cases whereas this patch does not. I thought that do_div() would cause a lot of overhead, but I noticed that it's not big if both numbers fit in 32 bit, which in our case is true most of the time. I will make a helper function for calculating rate, since using long long needs more than one line of code. Best regards, Jernej
diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c b/drivers/clk/sunxi-ng/ccu_nkmp.c index a99068a08315..e6c996ad4483 100644 --- a/drivers/clk/sunxi-ng/ccu_nkmp.c +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c @@ -33,7 +33,7 @@ static void ccu_nkmp_find_best(unsigned long parent, unsigned long rate, for (_p = nkmp->min_p; _p <= nkmp->max_p; _p <<= 1) { unsigned long tmp_rate; - tmp_rate = parent * _n * _k / (_m * _p); + tmp_rate = (parent / _m) * _n * _k / _p; if (tmp_rate > rate) continue; @@ -107,7 +107,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw, p = reg >> nkmp->p.shift; p &= (1 << nkmp->p.width) - 1; - return (parent_rate * n * k >> p) / m; + return (parent_rate / m) * n * k >> p; } static long ccu_nkmp_round_rate(struct clk_hw *hw, unsigned long rate, @@ -127,7 +127,7 @@ static long ccu_nkmp_round_rate(struct clk_hw *hw, unsigned long rate, ccu_nkmp_find_best(*parent_rate, rate, &_nkmp); - return *parent_rate * _nkmp.n * _nkmp.k / (_nkmp.m * _nkmp.p); + return (*parent_rate / _nkmp.m) * _nkmp.n * _nkmp.k / _nkmp.p; } static int ccu_nkmp_set_rate(struct clk_hw *hw, unsigned long rate,
This commit changes formula from this: Freq = (parent_freq * N * K) / (M * P) to this: Freq = (parent_freq / M) * N * K / P This improves situation when N is in the range 1-255. PLL parent clock is almost always 24 MHz, which means that for N >= 180 original formula overflows and result becomes useless. Situation can be improved if M is used as predivider as it can be seen in the second formula. That way at least M > 1 is considered, but it still leaves small gap for wrong result when M = 1 and N >= 180. Using M as predivider shouldn't cause any issue, because it is in range 1-4 at most, so there is no or only minimal rounding error. Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> --- drivers/clk/sunxi-ng/ccu_nkmp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)