[PATCH] sh: remove bogus highest / lowest logic from clock rate

[PATCH] sh: remove bogus highest / lowest logic from clock rate

[Guennadi Liakhovetski]

The use of highest and lowest in clk_rate_table_round() is completely bogus
and superfluous. Remove it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/sh/clk.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/drivers/sh/clk.c b/drivers/sh/clk.c
index 5d84ada..691f829 100644
--- a/drivers/sh/clk.c
+++ b/drivers/sh/clk.c
@@ -73,22 +73,14 @@ long clk_rate_table_round(struct clk *clk,
 {
 	unsigned long rate_error, rate_error_prev = ~0UL;
 	unsigned long rate_best_fit = rate;
-	unsigned long highest, lowest;
 	int i;
 
-	highest = lowest = 0;
-
 	for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
 		unsigned long freq = freq_table[i].frequency;
 
 		if (freq = CPUFREQ_ENTRY_INVALID)
 			continue;
 
-		if (freq > highest)
-			highest = freq;
-		if (freq < lowest)
-			lowest = freq;
-
 		rate_error = abs(freq - rate);
 		if (rate_error < rate_error_prev) {
 			rate_best_fit = freq;
@@ -99,11 +91,6 @@ long clk_rate_table_round(struct clk *clk,
 			break;
 	}
 
-	if (rate >= highest)
-		rate_best_fit = highest;
-	if (rate <= lowest)
-		rate_best_fit = lowest;
-
 	return rate_best_fit;
 }
 
-- 
1.6.2.4

Re: [PATCH] sh: remove bogus highest / lowest logic from clock rate rounding

[Paul Mundt]

On Fri, Jun 25, 2010 at 09:19:39AM +0200, Guennadi Liakhovetski wrote:
> The use of highest and lowest in clk_rate_table_round() is completely bogus
> and superfluous. Remove it.
> 
In what way is it bogus or superfluous? They are there to provide
high/low rate bounding when we have no idea what sort of frequencies are
available for an underlying clock.

Re: [PATCH] sh: remove bogus highest / lowest logic from clock rate

[Guennadi Liakhovetski]

On Fri, 25 Jun 2010, Paul Mundt wrote:

> On Fri, Jun 25, 2010 at 09:19:39AM +0200, Guennadi Liakhovetski wrote:
> > The use of highest and lowest in clk_rate_table_round() is completely bogus
> > and superfluous. Remove it.
> > 
> In what way is it bogus or superfluous? They are there to provide
> high/low rate bounding when we have no idea what sort of frequencies are
> available for an underlying clock.

Well, firstly initialising lowest to 0 doesn't make sense - there are no 
unsigned integers below it;) It should have been ~0UL. "No idea" you mean 
when the frequency table is empty? But then the highest / lowest checks 
make even less sense - the function would just return 0 ATM. Otherwise 
what they look like they'd have to do, is: in the loop they find max and 
min clock values. If the requested frequency is below min or above max, 
one of them would be used respectively. But that's already done by the 
minimum error check, unless there's a wrap... Am I missing anything?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] sh: remove bogus highest / lowest logic from clock rate rounding

[Paul Mundt]

On Fri, Jun 25, 2010 at 10:27:23AM +0200, Guennadi Liakhovetski wrote:
> On Fri, 25 Jun 2010, Paul Mundt wrote:
> 
> > On Fri, Jun 25, 2010 at 09:19:39AM +0200, Guennadi Liakhovetski wrote:
> > > The use of highest and lowest in clk_rate_table_round() is completely bogus
> > > and superfluous. Remove it.
> > > 
> > In what way is it bogus or superfluous? They are there to provide
> > high/low rate bounding when we have no idea what sort of frequencies are
> > available for an underlying clock.
> 
> Well, firstly initialising lowest to 0 doesn't make sense - there are no 
> unsigned integers below it;) It should have been ~0UL. "No idea" you mean 
> when the frequency table is empty? But then the highest / lowest checks 
> make even less sense - the function would just return 0 ATM. Otherwise 
> what they look like they'd have to do, is: in the loop they find max and 
> min clock values. If the requested frequency is below min or above max, 
> one of them would be used respectively. But that's already done by the 
> minimum error check, unless there's a wrap... Am I missing anything?
> 
Yes, the lowest initializer is wrong. I suppose we've just been getting
lucky there. The highest/lowest things are used for bounding when probing
the table for the high/low pairs (ie, rounding down from 0xffffffff and
up from 0). I only have a vague memory of writing the code in question
but seem to recall that the rate error calculation itself was not
sufficient for this. Looking at it again though I don't specifically
remember why, so I'll poke at it with your changes and see how it goes.
Given that the lowest thing is already broken it doesn't seem like much
of a stretch to assume that the rate error calculation is already saving
us.