* [lm-sensors] hwmon/f75375s.c: buggy if()
@ 2007-10-17 19:54 ` Adrian Bunk
0 siblings, 0 replies; 28+ messages in thread
From: Adrian Bunk @ 2007-10-17 19:54 UTC (permalink / raw)
To: Riku Voipio, Mark M. Hoffman; +Cc: lm-sensors, linux-kernel
drivers/hwmon/f75375s.c contains the following code:
<-- snip -->
...
static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
...
if (val != 0 || val != 1 || data->kind = f75373)
return -EINVAL;
...
<-- snip -->
I'm not sure what exactly was intended, but it was for sure not intended
to always return -EINVAL...
Spotted by the Coverity checker.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 28+ messages in thread* hwmon/f75375s.c: buggy if() @ 2007-10-17 19:54 ` Adrian Bunk 0 siblings, 0 replies; 28+ messages in thread From: Adrian Bunk @ 2007-10-17 19:54 UTC (permalink / raw) To: Riku Voipio, Mark M. Hoffman; +Cc: lm-sensors, linux-kernel drivers/hwmon/f75375s.c contains the following code: <-- snip --> ... static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { ... if (val != 0 || val != 1 || data->kind == f75373) return -EINVAL; ... <-- snip --> I'm not sure what exactly was intended, but it was for sure not intended to always return -EINVAL... Spotted by the Coverity checker. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 19:54 ` Adrian Bunk @ 2007-10-17 20:45 ` Riku Voipio -1 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-17 20:45 UTC (permalink / raw) To: Adrian Bunk; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel > <-- snip --> > > ... > static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > const char *buf, size_t count) > { > ... > if (val != 0 || val != 1 || data->kind = f75373) > return -EINVAL; > ... > > <-- snip --> > I'm not sure what exactly was intended, but it was for sure not intended > to always return -EINVAL... Aiee. val should be 1 or 0, and kind must not be f75373. Signed-off-by: Riku Voipio <riku.voipio@iki.fi> --- drivers/hwmon/f75375s.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index f1df57a..885bfe9 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -344,7 +344,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind = f75373) + if (!(val = 0 || val = 1 ) || data->kind = f75373) return -EINVAL; mutex_lock(&data->update_lock); -- 1.5.3.1 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: hwmon/f75375s.c: buggy if() @ 2007-10-17 20:45 ` Riku Voipio 0 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-17 20:45 UTC (permalink / raw) To: Adrian Bunk; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel > <-- snip --> > > ... > static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > const char *buf, size_t count) > { > ... > if (val != 0 || val != 1 || data->kind == f75373) > return -EINVAL; > ... > > <-- snip --> > I'm not sure what exactly was intended, but it was for sure not intended > to always return -EINVAL... Aiee. val should be 1 or 0, and kind must not be f75373. Signed-off-by: Riku Voipio <riku.voipio@iki.fi> --- drivers/hwmon/f75375s.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index f1df57a..885bfe9 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -344,7 +344,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind == f75373) + if (!(val == 0 || val == 1 ) || data->kind == f75373) return -EINVAL; mutex_lock(&data->update_lock); -- 1.5.3.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 20:45 ` Riku Voipio @ 2007-10-18 13:37 ` Mark M. Hoffman -1 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-18 13:37 UTC (permalink / raw) To: Riku Voipio; +Cc: Adrian Bunk, lm-sensors, linux-kernel Hi: * Riku Voipio <riku.voipio@iki.fi> [2007-10-17 23:45:08 +0300]: > > <-- snip --> > > > > ... > > static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > > const char *buf, size_t count) > > { > > ... > > if (val != 0 || val != 1 || data->kind = f75373) > > return -EINVAL; > > ... > > > > <-- snip --> > > > I'm not sure what exactly was intended, but it was for sure not intended > > to always return -EINVAL... > > Aiee. val should be 1 or 0, and kind must not be f75373. > > Signed-off-by: Riku Voipio <riku.voipio@iki.fi> > --- > drivers/hwmon/f75375s.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index f1df57a..885bfe9 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -344,7 +344,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind = f75373) > + if (!(val = 0 || val = 1 ) || data->kind = f75373) > return -EINVAL; > > mutex_lock(&data->update_lock); > -- > 1.5.3.1 That patch doesn't apply here, so I applied this: commit 805763cd743f2aed41dc61a55569fa43cf1f240c Author: Riku Voipio <riku.voipio@iki.fi> Date: Thu Oct 18 09:29:53 2007 -0400 hwmon: (f75375s) fix pwm mode setting Spotted by the Coverity checker. (Thanks Adrian Bunk) Signed-off-by: Riku Voipio <riku.voipio@iki.fi> Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com> diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 13a0413..59a3470 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind = f75373) + if (!(val = 0 || val = 1) || data->kind = f75373) return -EINVAL; mutex_lock(&data->update_lock); Thanks & regards, -- Mark M. Hoffman mhoffman@lightlink.com _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: hwmon/f75375s.c: buggy if() @ 2007-10-18 13:37 ` Mark M. Hoffman 0 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-18 13:37 UTC (permalink / raw) To: Riku Voipio; +Cc: Adrian Bunk, lm-sensors, linux-kernel Hi: * Riku Voipio <riku.voipio@iki.fi> [2007-10-17 23:45:08 +0300]: > > <-- snip --> > > > > ... > > static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > > const char *buf, size_t count) > > { > > ... > > if (val != 0 || val != 1 || data->kind == f75373) > > return -EINVAL; > > ... > > > > <-- snip --> > > > I'm not sure what exactly was intended, but it was for sure not intended > > to always return -EINVAL... > > Aiee. val should be 1 or 0, and kind must not be f75373. > > Signed-off-by: Riku Voipio <riku.voipio@iki.fi> > --- > drivers/hwmon/f75375s.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index f1df57a..885bfe9 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -344,7 +344,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind == f75373) > + if (!(val == 0 || val == 1 ) || data->kind == f75373) > return -EINVAL; > > mutex_lock(&data->update_lock); > -- > 1.5.3.1 That patch doesn't apply here, so I applied this: commit 805763cd743f2aed41dc61a55569fa43cf1f240c Author: Riku Voipio <riku.voipio@iki.fi> Date: Thu Oct 18 09:29:53 2007 -0400 hwmon: (f75375s) fix pwm mode setting Spotted by the Coverity checker. (Thanks Adrian Bunk) Signed-off-by: Riku Voipio <riku.voipio@iki.fi> Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com> diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 13a0413..59a3470 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind == f75373) + if (!(val == 0 || val == 1) || data->kind == f75373) return -EINVAL; mutex_lock(&data->update_lock); Thanks & regards, -- Mark M. Hoffman mhoffman@lightlink.com ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-18 13:37 ` Mark M. Hoffman @ 2007-10-19 12:37 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-19 12:37 UTC (permalink / raw) To: Mark M. Hoffman; +Cc: Riku Voipio, Adrian Bunk, linux-kernel, lm-sensors Hi Mark, hi Riku, On Thu, 18 Oct 2007 09:37:44 -0400, Mark M. Hoffman wrote: > That patch doesn't apply here, so I applied this: > > commit 805763cd743f2aed41dc61a55569fa43cf1f240c > Author: Riku Voipio <riku.voipio@iki.fi> > Date: Thu Oct 18 09:29:53 2007 -0400 > > hwmon: (f75375s) fix pwm mode setting > > Spotted by the Coverity checker. (Thanks Adrian Bunk) > > Signed-off-by: Riku Voipio <riku.voipio@iki.fi> > Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com> > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index 13a0413..59a3470 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind = f75373) > + if (!(val = 0 || val = 1) || data->kind = f75373) > return -EINVAL; > > mutex_lock(&data->update_lock); BTW, that's the wrong way to do it. If the F75373S doesn't support changing the PWM mode, then the sysfs attribute in question should be read-only for this chip type. Making it writable and returning an error on write is confusing. Riku, can you please submit a patch fixing this? The attribute should be declared read-only, and then you can use sysfs_chmod_file() to change it to read-write where supported. Take a look at the w83781d driver for an example. Thanks, -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-19 12:37 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-19 12:37 UTC (permalink / raw) To: Mark M. Hoffman; +Cc: Riku Voipio, Adrian Bunk, linux-kernel, lm-sensors Hi Mark, hi Riku, On Thu, 18 Oct 2007 09:37:44 -0400, Mark M. Hoffman wrote: > That patch doesn't apply here, so I applied this: > > commit 805763cd743f2aed41dc61a55569fa43cf1f240c > Author: Riku Voipio <riku.voipio@iki.fi> > Date: Thu Oct 18 09:29:53 2007 -0400 > > hwmon: (f75375s) fix pwm mode setting > > Spotted by the Coverity checker. (Thanks Adrian Bunk) > > Signed-off-by: Riku Voipio <riku.voipio@iki.fi> > Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com> > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index 13a0413..59a3470 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind == f75373) > + if (!(val == 0 || val == 1) || data->kind == f75373) > return -EINVAL; > > mutex_lock(&data->update_lock); BTW, that's the wrong way to do it. If the F75373S doesn't support changing the PWM mode, then the sysfs attribute in question should be read-only for this chip type. Making it writable and returning an error on write is confusing. Riku, can you please submit a patch fixing this? The attribute should be declared read-only, and then you can use sysfs_chmod_file() to change it to read-write where supported. Take a look at the w83781d driver for an example. Thanks, -- Jean Delvare ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-19 12:37 ` Jean Delvare @ 2007-10-24 11:50 ` Riku Voipio -1 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-24 11:50 UTC (permalink / raw) To: Jean Delvare; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > Riku, can you please submit a patch fixing this? The attribute should > be declared read-only, and then you can use sysfs_chmod_file() to > change it to read-write where supported. Thanks, this was good suggestion. Patch attached. > Take a look at the w83781d > driver for an example. Btw, I think your example code has a indentation bug: if (kind != w83781d) err = sysfs_chmod_file(&dev->kobj, &sensor_dev_attr_temp3_alarm.dev_attr.attr, S_IRUGO | S_IWUSR); if (err) return err; -- "rm -rf" only sounds scary if you don't have backups _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-24 11:50 ` Riku Voipio 0 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-24 11:50 UTC (permalink / raw) To: Jean Delvare; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > Riku, can you please submit a patch fixing this? The attribute should > be declared read-only, and then you can use sysfs_chmod_file() to > change it to read-write where supported. Thanks, this was good suggestion. Patch attached. > Take a look at the w83781d > driver for an example. Btw, I think your example code has a indentation bug: if (kind != w83781d) err = sysfs_chmod_file(&dev->kobj, &sensor_dev_attr_temp3_alarm.dev_attr.attr, S_IRUGO | S_IWUSR); if (err) return err; -- "rm -rf" only sounds scary if you don't have backups ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-24 11:50 ` Riku Voipio @ 2007-10-25 2:25 ` Mark M. Hoffman -1 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-25 2:25 UTC (permalink / raw) To: Riku Voipio; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors Hi Riku: * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > Riku, can you please submit a patch fixing this? The attribute should > > be declared read-only, and then you can use sysfs_chmod_file() to > > change it to read-write where supported. > > Thanks, this was good suggestion. Patch attached. No patch? -- Mark M. Hoffman mhoffman@lightlink.com _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-25 2:25 ` Mark M. Hoffman 0 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-25 2:25 UTC (permalink / raw) To: Riku Voipio; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors Hi Riku: * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > Riku, can you please submit a patch fixing this? The attribute should > > be declared read-only, and then you can use sysfs_chmod_file() to > > change it to read-write where supported. > > Thanks, this was good suggestion. Patch attached. No patch? -- Mark M. Hoffman mhoffman@lightlink.com ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-25 2:25 ` Mark M. Hoffman @ 2007-10-25 11:48 ` Riku Voipio -1 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-25 11:48 UTC (permalink / raw) To: Mark M. Hoffman; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors [-- Attachment #1: Type: text/plain, Size: 538 bytes --] On Wed, Oct 24, 2007 at 10:25:29PM -0400, Mark M. Hoffman wrote: > * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > > Riku, can you please submit a patch fixing this? The attribute should > > > be declared read-only, and then you can use sysfs_chmod_file() to > > > change it to read-write where supported. > > Thanks, this was good suggestion. Patch attached. > No patch? Let's try again.. -- "rm -rf" only sounds scary if you don't have backups [-- Attachment #2: 0002-Fix-hwmon-f75375s.c-buggy-if.patch --] [-- Type: text/plain, Size: 2296 bytes --] From 90a98836377541819012dfea8bd1bf748cd39723 Mon Sep 17 00:00:00 2001 From: Riku Voipio <riku.voipio@movial.fi> Date: Mon, 22 Oct 2007 17:42:35 +0300 Subject: [PATCH] Fix hwmon/f75375s.c: buggy if() Fix value check in set_pwm_mode(). Instead of checking for chip variant there, make pwmX_mode sysfs nodes only writable on f75375 variant. Signed-off-by: Riku Voipio <riku.voipio@movial.fi> --- drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 13a0413..d3b7932 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind == f75373) + if (!(val == 0 || val == 1)) return -EINVAL; mutex_lock(&data->update_lock); @@ -529,13 +529,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, show_pwm, set_pwm, 0); static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 0); -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 0); static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 1); static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 1); -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 1); static struct attribute *f75375_attributes[] = { @@ -655,6 +655,19 @@ static int f75375_detect(struct i2c_adapter *adapter, int address, int kind) if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) goto exit_detach; + if (kind == f75375) { + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm1_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_detach; + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm2_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_detach; + } + data->hwmon_dev = hwmon_device_register(&client->dev); if (IS_ERR(data->hwmon_dev)) { err = PTR_ERR(data->hwmon_dev); -- 1.5.3.1 [-- Attachment #3: Type: text/plain, Size: 153 bytes --] _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-25 11:48 ` Riku Voipio 0 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-25 11:48 UTC (permalink / raw) To: Mark M. Hoffman; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors [-- Attachment #1: Type: text/plain, Size: 538 bytes --] On Wed, Oct 24, 2007 at 10:25:29PM -0400, Mark M. Hoffman wrote: > * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > > Riku, can you please submit a patch fixing this? The attribute should > > > be declared read-only, and then you can use sysfs_chmod_file() to > > > change it to read-write where supported. > > Thanks, this was good suggestion. Patch attached. > No patch? Let's try again.. -- "rm -rf" only sounds scary if you don't have backups [-- Attachment #2: 0002-Fix-hwmon-f75375s.c-buggy-if.patch --] [-- Type: text/plain, Size: 2297 bytes --] >From 90a98836377541819012dfea8bd1bf748cd39723 Mon Sep 17 00:00:00 2001 From: Riku Voipio <riku.voipio@movial.fi> Date: Mon, 22 Oct 2007 17:42:35 +0300 Subject: [PATCH] Fix hwmon/f75375s.c: buggy if() Fix value check in set_pwm_mode(). Instead of checking for chip variant there, make pwmX_mode sysfs nodes only writable on f75375 variant. Signed-off-by: Riku Voipio <riku.voipio@movial.fi> --- drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 13a0413..d3b7932 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (val != 0 || val != 1 || data->kind == f75373) + if (!(val == 0 || val == 1)) return -EINVAL; mutex_lock(&data->update_lock); @@ -529,13 +529,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, show_pwm, set_pwm, 0); static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 0); -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 0); static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 1); static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 1); -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 1); static struct attribute *f75375_attributes[] = { @@ -655,6 +655,19 @@ static int f75375_detect(struct i2c_adapter *adapter, int address, int kind) if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) goto exit_detach; + if (kind == f75375) { + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm1_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_detach; + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm2_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_detach; + } + data->hwmon_dev = hwmon_device_register(&client->dev); if (IS_ERR(data->hwmon_dev)) { err = PTR_ERR(data->hwmon_dev); -- 1.5.3.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-25 11:48 ` Riku Voipio @ 2007-10-26 8:36 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-26 8:36 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors Hi Riku, On Thu, 25 Oct 2007 14:48:14 +0300, Riku Voipio wrote: > On Wed, Oct 24, 2007 at 10:25:29PM -0400, Mark M. Hoffman wrote: > > * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > > > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > > > Riku, can you please submit a patch fixing this? The attribute should > > > > be declared read-only, and then you can use sysfs_chmod_file() to > > > > change it to read-write where supported. > > > > Thanks, this was good suggestion. Patch attached. > > > No patch? > > Let's try again.. > Fix value check in set_pwm_mode(). Instead of checking for > chip variant there, make pwmX_mode sysfs nodes only writable > on f75375 variant. > > Signed-off-by: Riku Voipio <riku.voipio@movial.fi> > --- > drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- > 1 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index 13a0413..d3b7932 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind = f75373) > + if (!(val = 0 || val = 1)) > return -EINVAL; > > mutex_lock(&data->update_lock); > @@ -529,13 +529,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, > show_pwm, set_pwm, 0); > static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, > show_pwm_enable, set_pwm_enable, 0); > -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, > +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, > show_pwm_mode, set_pwm_mode, 0); > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, > show_pwm, set_pwm, 1); > static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, > show_pwm_enable, set_pwm_enable, 1); > -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, > +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, > show_pwm_mode, set_pwm_mode, 1); > > static struct attribute *f75375_attributes[] = { > @@ -655,6 +655,19 @@ static int f75375_detect(struct i2c_adapter *adapter, int address, int kind) > if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) > goto exit_detach; > > + if (kind = f75375) { > + err = sysfs_chmod_file(&client->dev.kobj, > + &sensor_dev_attr_pwm1_mode.dev_attr.attr, > + S_IRUGO | S_IWUSR); > + if (err) > + goto exit_detach; > + err = sysfs_chmod_file(&client->dev.kobj, > + &sensor_dev_attr_pwm2_mode.dev_attr.attr, > + S_IRUGO | S_IWUSR); > + if (err) > + goto exit_detach; > + } > + > data->hwmon_dev = hwmon_device_register(&client->dev); > if (IS_ERR(data->hwmon_dev)) { > err = PTR_ERR(data->hwmon_dev); Patch looks correct, however it doesn't apply on top of Mark's tree. I was able to get it to apply by reverting "(f75375s) fix pwm mode setting" first, but then the build fails. Presumably the other f75375s patches interact badly. Can you please respin this patch on top of Mark's tree (i.e. on top the the 4 other f75375s patches you sent since the -rc1 merge)? Thanks. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-26 8:36 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-26 8:36 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors Hi Riku, On Thu, 25 Oct 2007 14:48:14 +0300, Riku Voipio wrote: > On Wed, Oct 24, 2007 at 10:25:29PM -0400, Mark M. Hoffman wrote: > > * Riku Voipio <riku.voipio@iki.fi> [2007-10-24 14:50:34 +0300]: > > > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > > > Riku, can you please submit a patch fixing this? The attribute should > > > > be declared read-only, and then you can use sysfs_chmod_file() to > > > > change it to read-write where supported. > > > > Thanks, this was good suggestion. Patch attached. > > > No patch? > > Let's try again.. > Fix value check in set_pwm_mode(). Instead of checking for > chip variant there, make pwmX_mode sysfs nodes only writable > on f75375 variant. > > Signed-off-by: Riku Voipio <riku.voipio@movial.fi> > --- > drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- > 1 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > index 13a0413..d3b7932 100644 > --- a/drivers/hwmon/f75375s.c > +++ b/drivers/hwmon/f75375s.c > @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, > int val = simple_strtoul(buf, NULL, 10); > u8 conf = 0; > > - if (val != 0 || val != 1 || data->kind == f75373) > + if (!(val == 0 || val == 1)) > return -EINVAL; > > mutex_lock(&data->update_lock); > @@ -529,13 +529,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, > show_pwm, set_pwm, 0); > static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, > show_pwm_enable, set_pwm_enable, 0); > -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, > +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, > show_pwm_mode, set_pwm_mode, 0); > static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, > show_pwm, set_pwm, 1); > static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, > show_pwm_enable, set_pwm_enable, 1); > -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, > +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, > show_pwm_mode, set_pwm_mode, 1); > > static struct attribute *f75375_attributes[] = { > @@ -655,6 +655,19 @@ static int f75375_detect(struct i2c_adapter *adapter, int address, int kind) > if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) > goto exit_detach; > > + if (kind == f75375) { > + err = sysfs_chmod_file(&client->dev.kobj, > + &sensor_dev_attr_pwm1_mode.dev_attr.attr, > + S_IRUGO | S_IWUSR); > + if (err) > + goto exit_detach; > + err = sysfs_chmod_file(&client->dev.kobj, > + &sensor_dev_attr_pwm2_mode.dev_attr.attr, > + S_IRUGO | S_IWUSR); > + if (err) > + goto exit_detach; > + } > + > data->hwmon_dev = hwmon_device_register(&client->dev); > if (IS_ERR(data->hwmon_dev)) { > err = PTR_ERR(data->hwmon_dev); Patch looks correct, however it doesn't apply on top of Mark's tree. I was able to get it to apply by reverting "(f75375s) fix pwm mode setting" first, but then the build fails. Presumably the other f75375s patches interact badly. Can you please respin this patch on top of Mark's tree (i.e. on top the the 4 other f75375s patches you sent since the -rc1 merge)? Thanks. -- Jean Delvare ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-26 8:36 ` Jean Delvare @ 2007-10-26 11:14 ` Riku Voipio -1 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-26 11:14 UTC (permalink / raw) To: Jean Delvare; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors [-- Attachment #1: Type: text/plain, Size: 611 bytes --] On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > Patch looks correct, however it doesn't apply on top of Mark's tree. I > was able to get it to apply by reverting "(f75375s) fix pwm mode > setting" first, but then the build fails. Presumably the other f75375s > patches interact badly. Can you please respin this patch on top of > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > the -rc1 merge)? Thanks. The surrounding code had wandered to another function, so it's suprising it applied at all. Here's respin. -- "rm -rf" only sounds scary if you don't have backups [-- Attachment #2: hwmon-f75375s-fix-buggy-if-properly.patch --] [-- Type: text/plain, Size: 2288 bytes --] From 4de69e3ab5b5833cddb503f0dcb2a3ccc2d5b328 Mon Sep 17 00:00:00 2001 From: Riku Voipio <riku.voipio@movial.fi> Date: Fri, 26 Oct 2007 13:53:50 +0300 Subject: [PATCH] hwmon (f75375s) fix buggy if() properly Fix value check in set_pwm_mode(). Instead of checking for chip variant there, make pwmX_mode sysfs nodes only writable on f75375 variant. Signed-off-by: Riku Voipio <riku.voipio@movial.fi> --- drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 472b052..6892f76 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -343,7 +343,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (!(val == 0 || val == 1) || data->kind == f75373) + if (!(val == 0 || val == 1)) return -EINVAL; mutex_lock(&data->update_lock); @@ -549,13 +549,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, show_pwm, set_pwm, 0); static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 0); -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 0); static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 1); static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 1); -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 1); static struct attribute *f75375_attributes[] = { @@ -656,6 +656,19 @@ static int f75375_probe(struct i2c_client *client) if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) goto exit_free; + if (data->kind == f75375) { + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm1_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_remove; + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm2_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_remove; + } + data->hwmon_dev = hwmon_device_register(&client->dev); if (IS_ERR(data->hwmon_dev)) { err = PTR_ERR(data->hwmon_dev); -- 1.5.3.1 [-- Attachment #3: Type: text/plain, Size: 153 bytes --] _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-26 11:14 ` Riku Voipio 0 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-26 11:14 UTC (permalink / raw) To: Jean Delvare; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors [-- Attachment #1: Type: text/plain, Size: 611 bytes --] On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > Patch looks correct, however it doesn't apply on top of Mark's tree. I > was able to get it to apply by reverting "(f75375s) fix pwm mode > setting" first, but then the build fails. Presumably the other f75375s > patches interact badly. Can you please respin this patch on top of > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > the -rc1 merge)? Thanks. The surrounding code had wandered to another function, so it's suprising it applied at all. Here's respin. -- "rm -rf" only sounds scary if you don't have backups [-- Attachment #2: hwmon-f75375s-fix-buggy-if-properly.patch --] [-- Type: text/plain, Size: 2289 bytes --] >From 4de69e3ab5b5833cddb503f0dcb2a3ccc2d5b328 Mon Sep 17 00:00:00 2001 From: Riku Voipio <riku.voipio@movial.fi> Date: Fri, 26 Oct 2007 13:53:50 +0300 Subject: [PATCH] hwmon (f75375s) fix buggy if() properly Fix value check in set_pwm_mode(). Instead of checking for chip variant there, make pwmX_mode sysfs nodes only writable on f75375 variant. Signed-off-by: Riku Voipio <riku.voipio@movial.fi> --- drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 472b052..6892f76 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -343,7 +343,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr, int val = simple_strtoul(buf, NULL, 10); u8 conf = 0; - if (!(val == 0 || val == 1) || data->kind == f75373) + if (!(val == 0 || val == 1)) return -EINVAL; mutex_lock(&data->update_lock); @@ -549,13 +549,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR, show_pwm, set_pwm, 0); static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 0); -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 0); static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 1); static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable, set_pwm_enable, 1); -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR, +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO, show_pwm_mode, set_pwm_mode, 1); static struct attribute *f75375_attributes[] = { @@ -656,6 +656,19 @@ static int f75375_probe(struct i2c_client *client) if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group))) goto exit_free; + if (data->kind == f75375) { + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm1_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_remove; + err = sysfs_chmod_file(&client->dev.kobj, + &sensor_dev_attr_pwm2_mode.dev_attr.attr, + S_IRUGO | S_IWUSR); + if (err) + goto exit_remove; + } + data->hwmon_dev = hwmon_device_register(&client->dev); if (IS_ERR(data->hwmon_dev)) { err = PTR_ERR(data->hwmon_dev); -- 1.5.3.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-26 11:14 ` Riku Voipio @ 2007-10-26 21:15 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-26 21:15 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors On Fri, 26 Oct 2007 14:14:23 +0300, Riku Voipio wrote: > On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > > Patch looks correct, however it doesn't apply on top of Mark's tree. I > > was able to get it to apply by reverting "(f75375s) fix pwm mode > > setting" first, but then the build fails. Presumably the other f75375s > > patches interact badly. Can you please respin this patch on top of > > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > > the -rc1 merge)? Thanks. > > The surrounding code had wandered to another function, so it's suprising > it applied at all. Here's respin. Patch looks OK, thanks. Acked-by: Jean Delvare <khali@linux-fr.org> Riku, I have 14 patches posted to the lm-sensors list pending for review. Would you mind reviewing one of them? Thanks. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-26 21:15 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-26 21:15 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors On Fri, 26 Oct 2007 14:14:23 +0300, Riku Voipio wrote: > On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > > Patch looks correct, however it doesn't apply on top of Mark's tree. I > > was able to get it to apply by reverting "(f75375s) fix pwm mode > > setting" first, but then the build fails. Presumably the other f75375s > > patches interact badly. Can you please respin this patch on top of > > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > > the -rc1 merge)? Thanks. > > The surrounding code had wandered to another function, so it's suprising > it applied at all. Here's respin. Patch looks OK, thanks. Acked-by: Jean Delvare <khali@linux-fr.org> Riku, I have 14 patches posted to the lm-sensors list pending for review. Would you mind reviewing one of them? Thanks. -- Jean Delvare ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-26 11:14 ` Riku Voipio @ 2007-10-28 17:33 ` Mark M. Hoffman -1 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-28 17:33 UTC (permalink / raw) To: Riku Voipio; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors Hi: * Riku Voipio <riku.voipio@iki.fi> [2007-10-26 14:14:23 +0300]: > On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > > Patch looks correct, however it doesn't apply on top of Mark's tree. I > > was able to get it to apply by reverting "(f75375s) fix pwm mode > > setting" first, but then the build fails. Presumably the other f75375s > > patches interact badly. Can you please respin this patch on top of > > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > > the -rc1 merge)? Thanks. > > The surrounding code had wandered to another function, so it's suprising > it applied at all. Here's respin. > > -- > "rm -rf" only sounds scary if you don't have backups > >From 4de69e3ab5b5833cddb503f0dcb2a3ccc2d5b328 Mon Sep 17 00:00:00 2001 > From: Riku Voipio <riku.voipio@movial.fi> > Date: Fri, 26 Oct 2007 13:53:50 +0300 > Subject: [PATCH] hwmon (f75375s) fix buggy if() properly > > Fix value check in set_pwm_mode(). Instead of checking for > chip variant there, make pwmX_mode sysfs nodes only writable > on f75375 variant. > > Signed-off-by: Riku Voipio <riku.voipio@movial.fi> > --- > drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- > 1 files changed, 16 insertions(+), 3 deletions(-) > Applied to hwmon-2.6.git/testing, thanks. -- Mark M. Hoffman mhoffman@lightlink.com _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-28 17:33 ` Mark M. Hoffman 0 siblings, 0 replies; 28+ messages in thread From: Mark M. Hoffman @ 2007-10-28 17:33 UTC (permalink / raw) To: Riku Voipio; +Cc: Jean Delvare, Adrian Bunk, linux-kernel, lm-sensors Hi: * Riku Voipio <riku.voipio@iki.fi> [2007-10-26 14:14:23 +0300]: > On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > > Patch looks correct, however it doesn't apply on top of Mark's tree. I > > was able to get it to apply by reverting "(f75375s) fix pwm mode > > setting" first, but then the build fails. Presumably the other f75375s > > patches interact badly. Can you please respin this patch on top of > > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > > the -rc1 merge)? Thanks. > > The surrounding code had wandered to another function, so it's suprising > it applied at all. Here's respin. > > -- > "rm -rf" only sounds scary if you don't have backups > >From 4de69e3ab5b5833cddb503f0dcb2a3ccc2d5b328 Mon Sep 17 00:00:00 2001 > From: Riku Voipio <riku.voipio@movial.fi> > Date: Fri, 26 Oct 2007 13:53:50 +0300 > Subject: [PATCH] hwmon (f75375s) fix buggy if() properly > > Fix value check in set_pwm_mode(). Instead of checking for > chip variant there, make pwmX_mode sysfs nodes only writable > on f75375 variant. > > Signed-off-by: Riku Voipio <riku.voipio@movial.fi> > --- > drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- > 1 files changed, 16 insertions(+), 3 deletions(-) > Applied to hwmon-2.6.git/testing, thanks. -- Mark M. Hoffman mhoffman@lightlink.com ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-24 11:50 ` Riku Voipio @ 2007-10-25 11:09 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-25 11:09 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors Hi Riku, On Wed, 24 Oct 2007 14:50:34 +0300, Riku Voipio wrote: > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > Take a look at the w83781d > > driver for an example. > > Btw, I think your example code has a indentation bug: > > if (kind != w83781d) > err = sysfs_chmod_file(&dev->kobj, > &sensor_dev_attr_temp3_alarm.dev_attr.attr, > S_IRUGO | S_IWUSR); > if (err) > return err; Indentation is correct, but curly braces are missing! Nice catch, thanks for reporting. It happens to be harmless in this specific case, but it still needs fixing. I'll submit a patch shortly. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() @ 2007-10-25 11:09 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-25 11:09 UTC (permalink / raw) To: Riku Voipio; +Cc: Mark M. Hoffman, Adrian Bunk, linux-kernel, lm-sensors Hi Riku, On Wed, 24 Oct 2007 14:50:34 +0300, Riku Voipio wrote: > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote: > > Take a look at the w83781d > > driver for an example. > > Btw, I think your example code has a indentation bug: > > if (kind != w83781d) > err = sysfs_chmod_file(&dev->kobj, > &sensor_dev_attr_temp3_alarm.dev_attr.attr, > S_IRUGO | S_IWUSR); > if (err) > return err; Indentation is correct, but curly braces are missing! Nice catch, thanks for reporting. It happens to be harmless in this specific case, but it still needs fixing. I'll submit a patch shortly. -- Jean Delvare ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 19:54 ` Adrian Bunk (?) (?) @ 2007-10-26 22:35 ` Riku Voipio -1 siblings, 0 replies; 28+ messages in thread From: Riku Voipio @ 2007-10-26 22:35 UTC (permalink / raw) To: lm-sensors On Fri, Oct 26, 2007 at 11:15:55PM +0200, Jean Delvare wrote: > > The surrounding code had wandered to another function, so it's suprising > > it applied at all. Here's respin. > Patch looks OK, thanks. > Acked-by: Jean Delvare <khali@linux-fr.org> > Riku, I have 14 patches posted to the lm-sensors list pending for > review. Would you mind reviewing one of them? Thanks. Sure. I reviewed just a obvious one. I'll look at reviewing something more complicated on monday. Apart from the fintek, I have currently it87 and k8temp sensors to test, so any patches agains against those would be preferred. -- "rm -rf" only sounds scary if you don't have backups _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 19:54 ` Adrian Bunk ` (2 preceding siblings ...) (?) @ 2007-10-31 15:19 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-10-31 15:19 UTC (permalink / raw) To: lm-sensors On Sat, 27 Oct 2007 01:35:28 +0300, Riku Voipio wrote: > Sure. I reviewed just a obvious one. I'll look at reviewing something > more complicated on monday. Apart from the fintek, I have currently it87 > and k8temp sensors to test, so any patches agains against those would > be preferred. Well, the it87 driver needs to be modified to create individual alarm files. You could work on this if you want, that would be pretty helpful. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 19:54 ` Adrian Bunk ` (3 preceding siblings ...) (?) @ 2007-11-01 11:27 ` Christian Emmerich -1 siblings, 0 replies; 28+ messages in thread From: Christian Emmerich @ 2007-11-01 11:27 UTC (permalink / raw) To: lm-sensors [-- Attachment #1.1: Type: text/plain, Size: 1969 bytes --] Hi > Hi: > > * Riku Voipio [2007-10-26 14:14:23 +0300]: > > On Fri, Oct 26, 2007 at 10:36:47AM +0200, Jean Delvare wrote: > > > Patch looks correct, however it doesn't apply on top of Mark's tree. I > > > was able to get it to apply by reverting "(f75375s) fix pwm mode > > > setting" first, but then the build fails. Presumably the other f75375s > > > patches interact badly. Can you please respin this patch on top of > > > Mark's tree (i.e. on top the the 4 other f75375s patches you sent since > > > the -rc1 merge)? Thanks. > > > > The surrounding code had wandered to another function, so it's suprising > > it applied at all. Here's respin. > > > > -- > > "rm -rf" only sounds scary if you don't have backups > > > >From 4de69e3ab5b5833cddb503f0dcb2a3ccc2d5b328 Mon Sep 17 00:00:00 > 2001 > > From: Riku Voipio > > Date: Fri, 26 Oct 2007 13:53:50 +0300 > > Subject: [PATCH] hwmon (f75375s) fix buggy if() properly > > > > Fix value check in set_pwm_mode(). Instead of checking for > > chip variant there, make pwmX_mode sysfs nodes only writable > > on f75375 variant. > > > > Signed-off-by: Riku Voipio > > --- > > drivers/hwmon/f75375s.c | 19 ++++++++++++++++--- > > 1 files changed, 16 insertions(+), 3 deletions(-) > > > > Applied to hwmon-2.6.git/testing, thanks. > > -- > Mark M. Hoffman > mhoffman@lightlink.com Don't know if necessary, but i tested the driver using kernel 2.6.22-14, driver from kernel/mhoffman/hwmon-2.6.git - testing (28 Oct) and sensors version 3.0.0-rc3 with libsensors version 3.0.0-rc3. Fan-Speed, temperatures and voltage is displayed correctly. I can also change the fan speed without problem. The only modification i made in lm-sensors, setting the value of driver to "f75375s" in prog/detect/sensors-detect for "Fintek F75373S/SG". best regards christian. --------------------------------- Ihr erstes Fernweh? Wo gibt es den schönsten Strand. [-- Attachment #1.2: Type: text/html, Size: 5560 bytes --] [-- Attachment #2: Type: text/plain, Size: 153 bytes --] _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if() 2007-10-17 19:54 ` Adrian Bunk ` (4 preceding siblings ...) (?) @ 2007-11-02 10:24 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2007-11-02 10:24 UTC (permalink / raw) To: lm-sensors Hi Christian, On Thu, 1 Nov 2007 12:27:53 +0100 (CET), Christian Emmerich wrote: > The only modification i made in lm-sensors, setting the value of > driver to "f75375s" in prog/detect/sensors-detect for "Fintek > F75373S/SG". Good catch, this was somehow overlooked. I just fixed it in SVN, thanks for reporting. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2007-11-02 10:24 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-17 19:54 [lm-sensors] hwmon/f75375s.c: buggy if() Adrian Bunk 2007-10-17 19:54 ` Adrian Bunk 2007-10-17 20:45 ` [lm-sensors] " Riku Voipio 2007-10-17 20:45 ` Riku Voipio 2007-10-18 13:37 ` [lm-sensors] " Mark M. Hoffman 2007-10-18 13:37 ` Mark M. Hoffman 2007-10-19 12:37 ` [lm-sensors] " Jean Delvare 2007-10-19 12:37 ` Jean Delvare 2007-10-24 11:50 ` Riku Voipio 2007-10-24 11:50 ` Riku Voipio 2007-10-25 2:25 ` Mark M. Hoffman 2007-10-25 2:25 ` Mark M. Hoffman 2007-10-25 11:48 ` Riku Voipio 2007-10-25 11:48 ` Riku Voipio 2007-10-26 8:36 ` Jean Delvare 2007-10-26 8:36 ` Jean Delvare 2007-10-26 11:14 ` Riku Voipio 2007-10-26 11:14 ` Riku Voipio 2007-10-26 21:15 ` Jean Delvare 2007-10-26 21:15 ` Jean Delvare 2007-10-28 17:33 ` Mark M. Hoffman 2007-10-28 17:33 ` Mark M. Hoffman 2007-10-25 11:09 ` Jean Delvare 2007-10-25 11:09 ` Jean Delvare 2007-10-26 22:35 ` Riku Voipio 2007-10-31 15:19 ` Jean Delvare 2007-11-01 11:27 ` Christian Emmerich 2007-11-02 10:24 ` Jean Delvare
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.