[PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Martin Schlemmer]

[-- Attachment #1.1: Type: text/plain, Size: 158 bytes --]

Hi

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


Regards,

-- 

Martin Schlemmer



[-- Attachment #1.2: w83781d-in_milli-volt.patch --]
[-- Type: text/plain, Size: 678 bytes --]

--- drivers/i2c/chips/w83781d.c.orig	2003-04-07 22:53:37.000000000 +0200
+++ drivers/i2c/chips/w83781d.c	2003-04-07 22:53:34.000000000 +0200
@@ -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; \

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Greg KH]

On Mon, Apr 07, 2003 at 11:16:03PM +0200, Martin Schlemmer wrote:
> --- drivers/i2c/chips/w83781d.c.orig	2003-04-07 22:53:37.000000000 +0200
> +++ drivers/i2c/chips/w83781d.c	2003-04-07 22:53:34.000000000 +0200
> @@ -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

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Martin Schlemmer]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On Mon, 2003-04-07 at 23:54, Greg KH wrote:
> On Mon, Apr 07, 2003 at 11:16:03PM +0200, Martin Schlemmer wrote:
> > --- drivers/i2c/chips/w83781d.c.orig	2003-04-07 22:53:37.000000000 +0200
> > +++ drivers/i2c/chips/w83781d.c	2003-04-07 22:53:34.000000000 +0200
> > @@ -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 :)
> 

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


-- 

Martin Schlemmer



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Greg KH]

On Tue, Apr 08, 2003 at 06:11:18AM +0200, Martin Schlemmer wrote:
> On Mon, 2003-04-07 at 23:54, Greg KH wrote:
> > On Mon, Apr 07, 2003 at 11:16:03PM +0200, Martin Schlemmer wrote:
> > > --- drivers/i2c/chips/w83781d.c.orig	2003-04-07 22:53:37.000000000 +0200
> > > +++ drivers/i2c/chips/w83781d.c	2003-04-07 22:53:34.000000000 +0200
> > > @@ -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 :)
> > 
> 
> 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

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Martin Schlemmer]

[-- Attachment #1.1: Type: text/plain, Size: 497 bytes --]

On Mon, 2003-04-07 at 23:54, Greg KH wrote:

> 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,

-- 

Martin Schlemmer




[-- Attachment #1.2: w83781d-in_milli-volt.patch --]
[-- Type: text/plain, Size: 677 bytes --]

--- 1/drivers/i2c/chips/w83781d.c	2003-04-07 22:53:37.000000000 +0200
+++ 2/drivers/i2c/chips/w83781d.c	2003-04-07 22:53:34.000000000 +0200
@@ -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; \

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Martin Schlemmer]

On Wed, 2003-04-09 at 00:04, Greg KH wrote:

> 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:


--- 1/drivers/i2c/chips/w83781d.c	2003-04-09 08:16:08.000000000 +0200
+++ 2/drivers/i2c/chips/w83781d.c	2003-04-09 08:16:22.000000000 +0200
@@ -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,

-- 
Martin Schlemmer

Re: [PATCH-2.5] Fix w83781d sensor to use Milli-Volt for in_* in sysfs

[Greg KH]

On Wed, Apr 09, 2003 at 08:18:21AM +0200, Martin Schlemmer wrote:
> On Wed, 2003-04-09 at 00:04, Greg KH wrote:
> 
> > 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:

Thanks, I've applied this.

greg k-h