Re: [PATCH] Re: [2.5.39] (3/5) CPUfreq i386 drivers

[Gerald Britton <gbritton@alum.mit.edu>]

> -		if (!speedstep_low_freq || !speedstep_high_freq || 
> +		if (!low || !high || 
>  		    (speedstep_low_freq == speedstep_high_freq))
>  			return -EIO;

This is still obviously broken.

First time through the loop, high and low are 0, one of the two of them gets
set and the other is still 0.  This !low || !high test is still within the loop
so it will drop out with -EIO.  I'd been using cpufreq under 2.4 for a while
and with the recent updates (profile api), speedstep hasn't started up because
of this, or because it was sending bad data into the notifier chains.  Your
patch here passes bogus data into the notifier chains, which could be bad imho.

If I fix the init by moving the !low || !high test below the loop, and prevent
bad data from being passed into the notifier chains, I start getting memory
corruption being detected by slab poisioning.  This causes an oops during boot
and also shortly after load if speedstep is modular.

Also, the irq locking in speedstep.c is rather screwy and redundant in places:

/* Disable IRQs */
local_irq_save(flags);
local_irq_disable();
  . . .
/* Enable IRQs */
local_irq_enable();
local_irq_restore(flags);

				-- Gerald