* hwmon/f75375s.c: buggy if()
@ 2007-10-17 19:54 Adrian Bunk
2007-10-17 20:45 ` Riku Voipio
0 siblings, 1 reply; 12+ 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] 12+ messages in thread
* Re: hwmon/f75375s.c: buggy if()
2007-10-17 19:54 hwmon/f75375s.c: buggy if() Adrian Bunk
@ 2007-10-17 20:45 ` Riku Voipio
2007-10-18 13:37 ` Mark M. Hoffman
0 siblings, 1 reply; 12+ 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] 12+ messages in thread
* Re: hwmon/f75375s.c: buggy if()
2007-10-17 20:45 ` Riku Voipio
@ 2007-10-18 13:37 ` Mark M. Hoffman
2007-10-19 12:37 ` [lm-sensors] " Jean Delvare
0 siblings, 1 reply; 12+ 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] 12+ 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
2007-10-24 11:50 ` Riku Voipio
0 siblings, 1 reply; 12+ 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] 12+ messages in thread
* Re: [lm-sensors] hwmon/f75375s.c: buggy if()
2007-10-19 12:37 ` [lm-sensors] " Jean Delvare
@ 2007-10-24 11:50 ` Riku Voipio
2007-10-25 2:25 ` Mark M. Hoffman
2007-10-25 11:09 ` Jean Delvare
0 siblings, 2 replies; 12+ 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] 12+ 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
2007-10-25 11:48 ` Riku Voipio
2007-10-25 11:09 ` Jean Delvare
1 sibling, 1 reply; 12+ 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] 12+ 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
@ 2007-10-25 11:09 ` Jean Delvare
1 sibling, 0 replies; 12+ 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] 12+ 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
2007-10-26 8:36 ` Jean Delvare
0 siblings, 1 reply; 12+ 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] 12+ 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
2007-10-26 11:14 ` Riku Voipio
0 siblings, 1 reply; 12+ 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] 12+ 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
2007-10-26 21:15 ` Jean Delvare
2007-10-28 17:33 ` Mark M. Hoffman
0 siblings, 2 replies; 12+ 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] 12+ 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
2007-10-28 17:33 ` Mark M. Hoffman
1 sibling, 0 replies; 12+ 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] 12+ 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
@ 2007-10-28 17:33 ` Mark M. Hoffman
1 sibling, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2007-10-28 17:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-17 19:54 hwmon/f75375s.c: buggy if() Adrian Bunk
2007-10-17 20:45 ` Riku Voipio
2007-10-18 13:37 ` Mark M. Hoffman
2007-10-19 12:37 ` [lm-sensors] " Jean Delvare
2007-10-24 11:50 ` Riku Voipio
2007-10-25 2:25 ` Mark M. Hoffman
2007-10-25 11:48 ` Riku Voipio
2007-10-26 8:36 ` Jean Delvare
2007-10-26 11:14 ` Riku Voipio
2007-10-26 21:15 ` Jean Delvare
2007-10-28 17:33 ` Mark M. Hoffman
2007-10-25 11:09 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox