You know, after seeing that patch, that it just makes sense to add some of those cleanups to the lm80 driver.
Shawn.
lm80-i2c-0-28
Adapter: SMBus PIIX4 adapter at fe00
+5V: +4.90 V (min = +4.74 V, max = +5.25 V)
VTT: +1.72 V (min = +1.70 V, max = +2.10 V)
+3.3V: +3.35 V (min = +3.14 V, max = +3.47 V)
+Vcore: +1.98 V (min = +1.90 V, max = +2.10 V)
+12V: +11.30 V (min = +11.18 V, max = +12.63 V)
-12V: -11.88 V (min = -12.60 V, max = -11.39 V)
-5V: -4.92 V (min = -5.25 V, max = -4.76 V)
fan1: 1679 RPM (min = 1328 RPM, div = 4)
temp: +32.38?C (hot: limit = +60?C, hyst = +65?C)
: (os: limit = +54?C, hyst = +56?C)
Driver works with changes applied.
Signed-off-by: Aurelien Jarno <[email protected]>
Signed-off-by: Shawn Starr <[email protected]>
--- linux-2.6.11-rc2/drivers/i2c/chips/lm80.c 2005-01-26 02:04:38.000000000 -0500
+++ linux-2.6.11-rc2-fixes/drivers/i2c/chips/lm80.c 2005-01-26 02:41:00.000000000 -0500
@@ -72,7 +72,7 @@ SENSORS_INSMOD_1(lm80);
static inline unsigned char FAN_TO_REG(unsigned rpm, unsigned div)
{
- if (rpm == 0)
+ if (rpm <= 0)
return 255;
rpm = SENSORS_LIMIT(rpm, 1, 1000000);
return SENSORS_LIMIT((1350000 + rpm*div / 2) / (rpm*div), 1, 254);
@@ -99,10 +99,7 @@ static inline long TEMP_FROM_REG(u16 tem
#define TEMP_LIMIT_TO_REG(val) SENSORS_LIMIT((val)<0?\
((val)-500)/1000:((val)+500)/1000,0,255)
-#define ALARMS_FROM_REG(val) (val)
-
#define DIV_FROM_REG(val) (1 << (val))
-#define DIV_TO_REG(val) ((val)==8?3:(val)==4?2:(val)==1?0:1)
/*
* Client data (each client gets its own)
@@ -269,7 +266,17 @@ static ssize_t set_fan_div(struct device
DIV_FROM_REG(data->fan_div[nr]));
val = simple_strtoul(buf, NULL, 10);
- data->fan_div[nr] = DIV_TO_REG(val);
+
+ switch (val) {
+ case 1: data->fan_div[nr] = 0; break;
+ case 2: data->fan_div[nr] = 1; break;
+ case 4: data->fan_div[nr] = 2; break;
+ case 8: data->fan_div[nr] = 3; break;
+ default:
+ dev_err(&client->dev, "fan_div value %ld not "
+ "supported. Choose one of 1, 2, 4 or 8!\n", val);
+ return -EINVAL;
+ }
reg = (lm80_read_value(client, LM80_REG_FANDIV) & ~(3 << (2 * (nr + 1))))
| (data->fan_div[nr] << (2 * (nr + 1)));
@@ -327,7 +334,7 @@ set_temp(os_hyst, temp_os_hyst, LM80_REG
static ssize_t show_alarms(struct device *dev, char *buf)
{
struct lm80_data *data = lm80_update_device(dev);
- return sprintf(buf, "%d\n", ALARMS_FROM_REG(data->alarms));
+ return sprintf(buf, "%u\n", data->alarms);
}
static DEVICE_ATTR(in0_min, S_IWUSR | S_IRUGO, show_in_min0, set_in_min0);
You know, after seeing that patch, that it just makes sense to add some of
those cleanups to the lm80 driver.
Shawn.
lm80-i2c-0-28
Adapter: SMBus PIIX4 adapter at fe00
+5V: +4.90 V (min = +4.74 V, max = +5.25 V)
VTT: +1.72 V (min = +1.70 V, max = +2.10 V)
+3.3V: +3.35 V (min = +3.14 V, max = +3.47 V)
+Vcore: +1.98 V (min = +1.90 V, max = +2.10 V)
+12V: +11.30 V (min = +11.18 V, max = +12.63 V)
-12V: -11.88 V (min = -12.60 V, max = -11.39 V)
-5V: -4.92 V (min = -5.25 V, max = -4.76 V)
fan1: 1679 RPM (min = 1328 RPM, div = 4)
temp: +32.38?C (hot: limit = +60?C, hyst = +65?C)
: (os: limit = +54?C, hyst = +56?C)
Driver works with changes applied.
Signed-off-by: Aurelien Jarno <[email protected]>
Signed-off-by: Shawn Starr <[email protected]>
* Note to self, dont use Kmail for raw diff files
On Wed, Jan 26, 2005 at 02:49:23AM -0500, Shawn Starr wrote:
> static inline unsigned char FAN_TO_REG(unsigned rpm, unsigned div)
> {
> - if (rpm == 0)
> + if (rpm <= 0)
The rpm parameter is unsigned so this change is useless. The rest makes sense
to me.
BTW, can anyone tell me why the uints in this parameter list are declared as
'unsigned' and not as 'unsigned int'?
Sytse
On Wed, Jan 26, 2005 at 03:05:09PM +0100, I wrote:
> BTW, can anyone tell me why the uints in this parameter list are declared as
> 'unsigned' and not as 'unsigned int'?
$ find /usr/src/linux/ -name \*.c |xargs grep unsigned\ [^icsl] |wc -l
3151
- Gives himself a good smack on the head -
Sorry about that. I suppose it's just used a lot as a shorthand for 'unsigned
int', which is used a lot more in the kernel (the above command finds 22656
occurrences). On a sidenote, however, wouldn't it be nice for things like this
to be consistent throughout the kernel? Don't think I have an opinion on this,
because I don't, but I'd like to see those of the ones who do.
Sytse
On Wed, Jan 26, 2005 at 02:57:35AM -0500, Shawn Starr wrote:
> static inline unsigned char FAN_TO_REG(unsigned rpm, unsigned div)
> {
> - if (rpm == 0)
> + if (rpm <= 0)
As was pointed out, this doesn't make any sense.
Care to redo the patch?
thanks,
greg k-h
Greg KH wrote:
> On Wed, Jan 26, 2005 at 02:57:35AM -0500, Shawn Starr wrote:
>
>> static inline unsigned char FAN_TO_REG(unsigned rpm, unsigned div)
>> {
>>- if (rpm == 0)
>>+ if (rpm <= 0)
>
>
> As was pointed out, this doesn't make any sense.
>
> Care to redo the patch?
Please note that the problem is not present in the lm78 patch as the
argument is of type int.
Aurelien
--
.''`. Aurelien Jarno GPG: 1024D/F1BCDB73
: :' : Debian GNU/Linux developer | Electrical Engineer
`. `' [email protected] | [email protected]
`- people.debian.org/~aurel32 | http://www.aurel32.net
Here is the corrected fix, yeah that didn't make
sense.
3AM isn't a good time to send patches I guess :-)
--- Greg KH <[email protected]> wrote:
> On Wed, Jan 26, 2005 at 02:57:35AM -0500, Shawn
> Starr wrote:
> > static inline unsigned char FAN_TO_REG(unsigned
> rpm, unsigned div)
> > {
> > - if (rpm == 0)
> > + if (rpm <= 0)
>
> As was pointed out, this doesn't make any sense.
>
> Care to redo the patch?
>
> thanks,
>
> greg k-h
>
On Wed, Jan 26, 2005 at 12:31:30PM -0500, Shawn Starr wrote:
> Here is the corrected fix, yeah that didn't make
> sense.
> 3AM isn't a good time to send patches I guess :-)
Care to resend it, with a full description and a Signed-off-by: line so
I can apply it?
thanks,
greg k-h
Description: Cleanup some cluttered macros, add error
checking for fan divisor value set.
Approved-by: Greg KH <[email protected]>
Signed-off-by: Sytse Wielinga
<[email protected]>
Signed-off-by: Aurelien Jarno <[email protected]>
Signed-off-by: Shawn Starr <[email protected]>
--- Greg KH <[email protected]> wrote:
> On Wed, Jan 26, 2005 at 12:31:30PM -0500, Shawn
> Starr wrote:
> > Here is the corrected fix, yeah that didn't make
> > sense.
> > 3AM isn't a good time to send patches I guess :-)
>
> Care to resend it, with a full description and a
> Signed-off-by: line so
> I can apply it?
>
> thanks,
>
> greg k-h
>
On Thu, Jan 27, 2005 at 11:56:46AM -0500, Shawn Starr wrote:
> Description: Cleanup some cluttered macros, add error
> checking for fan divisor value set.
Hm, we'll get there yet. Your patch was not in a plain text form, so
that I could apply it directly from the email.
> Approved-by: Greg KH <[email protected]>
I have not "approved it" yet at all.
> Signed-off-by: Sytse Wielinga
> <[email protected]>
> Signed-off-by: Aurelien Jarno <[email protected]>
> Signed-off-by: Shawn Starr <[email protected]>
You have line wrap here too :(
> --- Greg KH <[email protected]> wrote:
> > On Wed, Jan 26, 2005 at 12:31:30PM -0500, Shawn
> > Starr wrote:
> > > Here is the corrected fix, yeah that didn't make
> > > sense.
> > > 3AM isn't a good time to send patches I guess :-)
> >
> > Care to resend it, with a full description and a
> > Signed-off-by: line so
> > I can apply it?
> >
> > thanks,
> >
> > greg k-h
> >
What's this "bottom post" stuff in here?
Also, please CC: the sensors mailing list so the developers there can
review the changes.
Can you try it again from scratch?
thanks,
greg k-h
Description: Cleanup some cluttered macros, add error checking for fan divisor value set.
Signed-off-by: Sytse Wielinga <[email protected]>
Signed-off-by: Aurelien Jarno <[email protected]>
Signed-off-by: Shawn Starr <[email protected]>
On Sat, Jan 29, 2005 at 09:08:55PM -0500, Shawn Starr wrote:
> Description: Cleanup some cluttered macros, add error checking for fan divisor value set.
>
> Signed-off-by: Sytse Wielinga <[email protected]>
> Signed-off-by: Aurelien Jarno <[email protected]>
> Signed-off-by: Shawn Starr <[email protected]>
Applied, thanks.
greg k-h