Fix w83781d sensor to use Milli-Volt for in_* in sysfs

PATCH-2.5 Martin Schlemmer 2 participants 7 messages Apr 7, 2003 – Apr 9, 2003

lore

Share & tools

lore

Martin Schlemmer

author

2003-04-07 21:07 UTC #0 lore raw

I did the w83781d sysfs update as per the old spec, which was not milli-volt. This patch should fix it.

Regards,

Signature · 1 line

Martin Schlemmer

i2c/chips/w83781d.c renamed from i2c/chips/w83781d.c.orig +2 -2

@@ -364,7 +364,7 @@
 	 \
 	w83781d_update_client(client); \
 	 \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr] * 10)); \
+	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
 }
 show_in_reg(in);
 show_in_reg(in_min);

@@ -378,7 +378,7 @@
 	u32 val; \
 	 \
 	val = simple_strtoul(buf, NULL, 10); \
-	data->in_##reg[nr] = (IN_TO_REG(val) / 10); \
+	data->in_##reg[nr] = IN_TO_REG(val); \
 	w83781d_write_value(client, W83781D_REG_IN_##REG(nr), data->in_##reg[nr]); \
 	 \
 	return count; \

Greg KH

2003-04-07 21:40 UTC #1 lore raw

i2c/chips/w83781d.c line 367

Quoted 8 lines · Martin Schlemmer

i2c/chips/w83781d.c renamed from i2c/chips/w83781d.c.orig +1 -1

@@ -364,7 +364,7 @@
 	 \
 	w83781d_update_client(client); \
 	 \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr] * 10)); \
+	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \

Hm, this patch looks backwards, is it?

Also, just as a side note, can you make your patches so that they can be applied with "patch -p1" instead of "patch -p0" which your current ones are? My tools treat -p1 patches much better :)

thanks,

greg k-h

Martin Schlemmer

author

2003-04-08 04:03 UTC #2 lore raw

Quoted 12 lines · Greg KH

i2c/chips/w83781d.c line 367
Quoted 8 lines · Martin Schlemmer
i2c/chips/w83781d.c renamed from i2c/chips/w83781d.c.orig +1 -1
@@ -364,7 +364,7 @@
 	 \
 	w83781d_update_client(client); \
 	 \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr] * 10)); \
+	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
Hm, this patch looks backwards, is it?

Quoted 4 lines

Also, just as a side note, can you make your patches so that they can be applied with "patch -p1" instead of "patch -p0" which your current ones are? My tools treat -p1 patches much better :)

Yes, sorry, that is reverse. I have to go to work in a bit, so if you need me to resend tonight, just ask.

Signature · 1 line

Martin Schlemmer

Greg KH

2003-04-08 21:50 UTC #3 lore raw

Quoted 13 lines · Martin Schlemmer

Quoted 12 lines · Greg KH
i2c/chips/w83781d.c line 367
Quoted 8 lines · Martin Schlemmer
i2c/chips/w83781d.c renamed from i2c/chips/w83781d.c.orig +1 -1
@@ -364,7 +364,7 @@
 	 \
 	w83781d_update_client(client); \
 	 \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr] * 10)); \
+	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
Hm, this patch looks backwards, is it?

Quoted 7 lines

Quoted 4 lines
Also, just as a side note, can you make your patches so that they can be applied with "patch -p1" instead of "patch -p0" which your current ones are? My tools treat -p1 patches much better :)
Yes, sorry, that is reverse. I have to go to work in a bit, so if you need me to resend tonight, just ask.

Thanks, I've applied this to my trees.

Oh, I'm getting the following warning when building the driver, want to look into this?

drivers/i2c/chips/w83781d.c: In function `store_fan_div_reg':
drivers/i2c/chips/w83781d.c:715: warning: `old3' might be used uninitialized in this function

thanks,

greg k-h

Martin Schlemmer

author

2003-04-09 06:10 UTC #4 lore raw

Quoted 6 lines · Greg KH

Oh, I'm getting the following warning when building the driver, want to look into this?
drivers/i2c/chips/w83781d.c: In function `store_fan_div_reg':
drivers/i2c/chips/w83781d.c:715: warning: `old3' might be used uninitialized in this function

It is because old3 is only referenced if:

((data->type != w83781d) && data->type != as99127f)

as those two chips don't have extended divisor bits ...

It is however set in the first occurrence:

---------------------------------------------------------------------

       /* w83781d and as99127f don't have extended divisor bits */
       if ((data->type != w83781d) && data->type != as99127f) {
               old3 = w83781d_read_value(client, W83781D_REG_VBAT);
       }

---------------------------------------------------------------------

and thus is rather gcc being brain dead for not being able to figure old3 is only used within a if block like that.

I was not sure about style policy in a case like this, so I left it as is, it should however be possible to 'fix' it with:

drivers/i2c/chips/w83781d.c +1 -1

@@ -712,7 +712,7 @@
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct w83781d_data *data = i2c_get_clientdata(client);
-	u32 val, old, old2, old3;
+	u32 val, old, old2, old3 = 0;
 
 	val = simple_strtoul(buf, NULL, 10);
 	old = w83781d_read_value(client, W83781D_REG_VID_FANDIV);

Regards,

Signature · 1 line

Martin Schlemmer

Greg KH

2003-04-09 07:27 UTC #5 lore raw

Quoted 22 lines · Martin Schlemmer

Quoted 6 lines · Greg KH
Oh, I'm getting the following warning when building the driver, want to look into this?
drivers/i2c/chips/w83781d.c: In function `store_fan_div_reg':
drivers/i2c/chips/w83781d.c:715: warning: `old3' might be used uninitialized in this function
It is because old3 is only referenced if:
((data->type != w83781d) && data->type != as99127f)
as those two chips don't have extended divisor bits ...
It is however set in the first occurrence:
---------------------------------------------------------------------
       /* w83781d and as99127f don't have extended divisor bits */
       if ((data->type != w83781d) && data->type != as99127f) {
               old3 = w83781d_read_value(client, W83781D_REG_VBAT);
       }

Quoted 7 lines

---------------------------------------------------------------------
and thus is rather gcc being brain dead for not being able to figure old3 is only used within a if block like that.
I was not sure about style policy in a case like this, so I left it as is, it should however be possible to 'fix' it with:

Thanks, I've applied this.

greg k-h

Martin Schlemmer

author

2003-04-08 22:03 UTC #6 lore raw

In reply to Greg KH

Quoted 6 lines · Greg KH

Hm, this patch looks backwards, is it?
Also, just as a side note, can you make your patches so that they can be applied with "patch -p1" instead of "patch -p0" which your current ones are? My tools treat -p1 patches much better :)

Ok, here is the proper one. To recap:

I did the w83781d sysfs update as per the old spec, which was not milli-volt. This patch should fix it.

Regards,

Signature · 1 line

Martin Schlemmer

drivers/i2c/chips/w83781d.c +2 -2

@@ -364,7 +364,7 @@
 	 \
 	w83781d_update_client(client); \
 	 \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
+	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr] * 10)); \
 }
 show_in_reg(in);
 show_in_reg(in_min);

@@ -378,7 +378,7 @@
 	u32 val; \
 	 \
 	val = simple_strtoul(buf, NULL, 10); \
-	data->in_##reg[nr] = IN_TO_REG(val); \
+	data->in_##reg[nr] = (IN_TO_REG(val) / 10); \
 	w83781d_write_value(client, W83781D_REG_IN_##REG(nr), data->in_##reg[nr]); \
 	 \
 	return count; \