hwmon/sis5595: Add individual alarm files
PATCH 1/1 hwmon/sis5595: Add individual alarm files
Add individual alarm files needed by the new libsensors.
--- drivers/hwmon/sis5595.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)
--- linux-2.6.22.9.orig/drivers/hwmon/sis5595.c 2007-10-11
19:38:11.000000000 +0200
+++ linux-2.6.22.9/drivers/hwmon/sis5595.c 2007-10-11
19:41:56.000000000 +0200
@@ -62,6 +62,7 @@
#include <linux/jiffies.h>
#include <linux/mutex.h>
#include <linux/sysfs.h>
+#include <linux/hwmon-sysfs.h>
#include <asm/io.h>
@@ -473,26 +474,47 @@
}
static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+static ssize_t show_alarm(struct device *dev, struct device_attribute *da, char *buf)
+{
+ struct sis5595_data *data = sis5595_update_device(dev);
+ int nr = to_sensor_dev_attr(da)->index;
+ return sprintf(buf, "%u\n", (data->alarms >> nr) & 1);
+}
+ static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0);
+ static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1);
+ static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2);
+ static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3);
+ static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15);
+ static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6);
+ static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7);
+ static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15);
+
static struct attribute *sis5595_attributes[] = {
&dev_attr_in0_input.attr,
&dev_attr_in0_min.attr,
&dev_attr_in0_max.attr,
+ &sensor_dev_attr_in0_alarm.dev_attr.attr,
&dev_attr_in1_input.attr,
&dev_attr_in1_min.attr,
&dev_attr_in1_max.attr,
+ &sensor_dev_attr_in1_alarm.dev_attr.attr,
&dev_attr_in2_input.attr,
&dev_attr_in2_min.attr,
&dev_attr_in2_max.attr,
+ &sensor_dev_attr_in2_alarm.dev_attr.attr,
&dev_attr_in3_input.attr,
&dev_attr_in3_min.attr,
&dev_attr_in3_max.attr,+ &sensor_dev_attr_in3_alarm.dev_attr.attr,
&dev_attr_fan1_input.attr,
&dev_attr_fan1_min.attr,
&dev_attr_fan1_div.attr,
+ &sensor_dev_attr_fan1_alarm.dev_attr.attr,
&dev_attr_fan2_input.attr,
&dev_attr_fan2_min.attr,
&dev_attr_fan2_div.attr,+ &sensor_dev_attr_fan2_alarm.dev_attr.attr,
&dev_attr_alarms.attr,
NULL
@@ -506,10 +528,12 @@
&dev_attr_in4_input.attr,
&dev_attr_in4_min.attr,
&dev_attr_in4_max.attr,+ &sensor_dev_attr_in4_alarm.dev_attr.attr,
&dev_attr_temp1_input.attr,
&dev_attr_temp1_max.attr,
&dev_attr_temp1_max_hyst.attr,+ &sensor_dev_attr_temp1_alarm.dev_attr.attr,
NULL
};
@@ -617,7 +641,9 @@
|| (err = device_create_file(&new_client->dev,
&dev_attr_in4_min))
|| (err = device_create_file(&new_client->dev,- &dev_attr_in4_max)))
+ &dev_attr_in4_max))
+ || (err = device_create_file(&new_client->dev,
+ &sensor_dev_attr_in4_alarm.dev_attr)))
goto exit_remove_files;
} else {
if ((err = device_create_file(&new_client->dev,
@@ -625,7 +651,9 @@
|| (err = device_create_file(&new_client->dev,
&dev_attr_temp1_max))
|| (err = device_create_file(&new_client->dev,- &dev_attr_temp1_max_hyst)))
+ &dev_attr_temp1_max_hyst))
+ || (err = device_create_file(&new_client->dev,
+ &sensor_dev_attr_temp1_alarm.dev_attr)))
goto exit_remove_files;
}_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Ivo,
Quoted 29 lines · Pinkel
Add individual alarm files needed by the new libsensors.
Signed-off-by Ivo Manca <[email protected]>--- drivers/hwmon/sis5595.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)---
linux-2.6.22.9.orig/drivers/hwmon/sis5595.c2007-10-11
19:38:11.000000000 +0200
+++linux-2.6.22.9/drivers/hwmon/sis5595.c2007-10-11
19:41:56.000000000 +0200(quoted diff) +5
-0@@ -62,6 +62,7 @@ #include <linux/jiffies.h> #include <linux/mutex.h> #include <linux/sysfs.h> +#include <linux/hwmon-sysfs.h> #include <asm/io.h>@@ -473,26 +474,47 @@ } static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +static ssize_t show_alarm(struct device *dev, struct device_attribute *da, char *buf) +{ + struct sis5595_data *data = sis5595_update_device(dev); + int nr = to_sensor_dev_attr(da)->index;
Review line 491
Quoted 10 lines
(quoted diff) +10
-0+ return sprintf(buf, "%u\n", (data->alarms >> nr) & 1); +} + static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0); + static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1); + static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2); + static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3); + static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15); + static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6); + static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7); + static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15);
Your mailer destroyed the patch formatting :( Can you please fix it or resend the patch as an attachment?
Thanks,
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 48 lines · Jean Delvare
Hi Ivo,
Quoted 30 lines · Pinkel
Add individual alarm files needed by the new libsensors.
Signed-off-by Ivo Manca <[email protected]>--- drivers/hwmon/sis5595.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)---
linux-2.6.22.9.orig/drivers/hwmon/sis5595.c2007-10-11
19:38:11.000000000 +0200
+++linux-2.6.22.9/drivers/hwmon/sis5595.c2007-10-11
19:41:56.000000000 +0200(quoted diff) +6
-0@@ -62,6 +62,7 @@ #include <linux/jiffies.h> #include <linux/mutex.h> #include <linux/sysfs.h> +#include <linux/hwmon-sysfs.h> #include <asm/io.h>@@ -473,26 +474,47 @@ } static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +static ssize_t show_alarm(struct device *dev, struct device_attribute *da, char *buf) +{ + struct sis5595_data *data = sis5595_update_device(dev); + int nr = to_sensor_dev_attr(da)->index; + return sprintf(buf, "%u\n", (data->alarms >> nr) & 1);Review line 492
Quoted 10 lines
(quoted diff) +9
-0+} + static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0); + static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1); + static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2); + static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3); + static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15); + static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6); + static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7); + static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15);Your mailer destroyed the patch formatting :( Can you please fix it or resend the patch as an attachment?
Quoted 2 lines
Thanks,
Sure. It's attached now, hope that works better.
Ivo
drivers/hwmon/sis5595.c +30 -2
@@ -62,6 +62,7 @@#include <linux/jiffies.h> #include <linux/mutex.h> #include <linux/sysfs.h> +#include <linux/hwmon-sysfs.h> #include <asm/io.h>
@@ -473,26 +474,47 @@ } static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +static ssize_t show_alarm (struct device *dev,struct device_attribute *da,char *buf) +{ +struct sis5595_data *data =sis5595_update_device (dev); +int nr =to_sensor_dev_attr (da)->index; +return sprintf (buf,"%u\n" , (data->alarms >> nr) &1 ); +} + static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0); + static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1); + static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2); + static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3); + static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15); + static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6); + static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7); + static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15); +static struct attribute *sis5595_attributes[] = { &dev_attr_in0_input.attr, &dev_attr_in0_min.attr, &dev_attr_in0_max.attr, + &sensor_dev_attr_in0_alarm.dev_attr.attr, &dev_attr_in1_input.attr, &dev_attr_in1_min.attr, &dev_attr_in1_max.attr, + &sensor_dev_attr_in1_alarm.dev_attr.attr, &dev_attr_in2_input.attr, &dev_attr_in2_min.attr, &dev_attr_in2_max.attr, + &sensor_dev_attr_in2_alarm.dev_attr.attr, &dev_attr_in3_input.attr, &dev_attr_in3_min.attr, &dev_attr_in3_max.attr, + &sensor_dev_attr_in3_alarm.dev_attr.attr, &dev_attr_fan1_input.attr, &dev_attr_fan1_min.attr, &dev_attr_fan1_div.attr, + &sensor_dev_attr_fan1_alarm.dev_attr.attr, &dev_attr_fan2_input.attr, &dev_attr_fan2_min.attr, &dev_attr_fan2_div.attr, + &sensor_dev_attr_fan2_alarm.dev_attr.attr, &dev_attr_alarms.attr,NULL
@@ -506,10 +528,12 @@ &dev_attr_in4_input.attr, &dev_attr_in4_min.attr, &dev_attr_in4_max.attr, + &sensor_dev_attr_in4_alarm.dev_attr.attr, &dev_attr_temp1_input.attr, &dev_attr_temp1_max.attr, &dev_attr_temp1_max_hyst.attr, + &sensor_dev_attr_temp1_alarm.dev_attr.attr,NULL };
@@ -617,7 +641,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_in4_min)) || (err = device_create_file(&new_client->dev, - &dev_attr_in4_max))) + &dev_attr_in4_max)) + || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_in4_alarm.dev_attr))) goto exit_remove_files; } else { if ((err = device_create_file(&new_client->dev,
@@ -625,7 +651,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_temp1_max)) || (err = device_create_file(&new_client->dev, - &dev_attr_temp1_max_hyst))) + &dev_attr_temp1_max_hyst)) + || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_temp1_alarm.dev_attr))) goto exit_remove_files; }
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Ivo,
Quoted 7 lines · Ivo Manca
Quoted 4 lines · Jean Delvare
Your mailer destroyed the patch formatting :( Can you please fix it or resend the patch as an attachment?
Thanks,
Sure. It's attached now, hope that works better.
Yes, it's better but now there's another problem: you based your patch on 2.6.22.9, but the sis5595 driver changed a lot in 2.6.23, so I still can't apply it. Would you be so kind and send a new patch that would apply on top of 2.6.23?
Still, I can review the patch:
Quoted 12 lines
diff -ubrN
linux-2.6.22.9.orig/drivers/hwmon/sis5595.clinux-2.6.22.9/drivers/hwmon/sis5595.cdrivers/hwmon/sis5595.c +1
-0@@ -62,6 +62,7 @@#include <linux/jiffies.h> #include <linux/mutex.h> #include <linux/sysfs.h> +#include <linux/hwmon-sysfs.h> #include <asm/io.h>
drivers/hwmon/sis5595.c line 477
Quoted 4 lines
drivers/hwmon/sis5595.c +1
-0@@ -473,26 +474,47 @@ } static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +static ssize_t show_alarm (struct device *dev,struct device_attribute *da,char *buf)
Remember that line length should never exceed 80 columns.
drivers/hwmon/sis5595.c line 490
Quoted 13 lines
drivers/hwmon/sis5595.c +13
-0+{ +struct sis5595_data *data =sis5595_update_device (dev); +int nr =to_sensor_dev_attr (da)->index; +return sprintf (buf,"%u\n" , (data->alarms >> nr) &1 ); +} + static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0); + static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1); + static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2); + static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3); + static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15); + static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6); + static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7); + static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15);
Bad indentation, all these "static SENSOR_DEVICE_ATTR" lines shouldn't be indented.
Quoted 55 lines
drivers/hwmon/sis5595.c +12
-1+static struct attribute *sis5595_attributes[] = { &dev_attr_in0_input.attr, &dev_attr_in0_min.attr, &dev_attr_in0_max.attr, + &sensor_dev_attr_in0_alarm.dev_attr.attr, &dev_attr_in1_input.attr, &dev_attr_in1_min.attr, &dev_attr_in1_max.attr, + &sensor_dev_attr_in1_alarm.dev_attr.attr, &dev_attr_in2_input.attr, &dev_attr_in2_min.attr, &dev_attr_in2_max.attr, + &sensor_dev_attr_in2_alarm.dev_attr.attr, &dev_attr_in3_input.attr, &dev_attr_in3_min.attr, &dev_attr_in3_max.attr, + &sensor_dev_attr_in3_alarm.dev_attr.attr, &dev_attr_fan1_input.attr, &dev_attr_fan1_min.attr, &dev_attr_fan1_div.attr, + &sensor_dev_attr_fan1_alarm.dev_attr.attr, &dev_attr_fan2_input.attr, &dev_attr_fan2_min.attr, &dev_attr_fan2_div.attr, + &sensor_dev_attr_fan2_alarm.dev_attr.attr, &dev_attr_alarms.attr,NULL @@ -506,10 +528,12 @@ &dev_attr_in4_input.attr, &dev_attr_in4_min.attr, &dev_attr_in4_max.attr, + &sensor_dev_attr_in4_alarm.dev_attr.attr, &dev_attr_temp1_input.attr, &dev_attr_temp1_max.attr, &dev_attr_temp1_max_hyst.attr, + &sensor_dev_attr_temp1_alarm.dev_attr.attr,NULL };@@ -617,7 +641,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_in4_min)) || (err = device_create_file(&new_client->dev, - &dev_attr_in4_max))) + &dev_attr_in4_max)) + || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_in4_alarm.dev_attr))) goto exit_remove_files; } else { if ((err = device_create_file(&new_client->dev,
drivers/hwmon/sis5595.c line 659
Quoted 10 lines
drivers/hwmon/sis5595.c +3
-1@@ -625,7 +651,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_temp1_max)) || (err = device_create_file(&new_client->dev, - &dev_attr_temp1_max_hyst))) + &dev_attr_temp1_max_hyst)) + || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_temp1_alarm.dev_attr))) goto exit_remove_files; }
Other than that, your patch looks good.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hey Jean,
Thanks for the quick review. It's starting to show I'm new to this, isn't it? ;)
Anyway, one question while making the patch for 2.6.23.1:
@@ -553,7 +578,9 @@
|| (err = device_create_file(&pdev->dev,
&dev_attr_temp1_max))
|| (err = device_create_file(&pdev->dev,- &dev_attr_temp1_max_hyst)))
+ &dev_attr_temp1_max_hyst))
+ || (err = device_create_file(&pdev->dev,
+ &sensor_dev_attr_temp1_alarm.dev_attr)))
goto exit_remove_files;
(E-mail client will most likely mess this up, however, it's also in the original patch you just reviewed...).
The line "&sensor_dev_attr_temp1_alarm.dev_attr)))" can not fit within 80 characters using the same indentation as the lines above it; it takes 87 characters. However, removing the spaces before it, will still make it use 81 chars... I can't see any hint about it in the CodingStyles and removing some tabs will make it look just plain ugly. Any hint how to format it? Patch against 2.6.23.1 is ready, just waiting to be sure how to do this correct :p
Anyway,
Quoted 14 lines · Jean Delvare
Hi Ivo,
Quoted 9 lines · Ivo Manca
Quoted 5 lines · Jean Delvare
Your mailer destroyed the patch formatting :( Can you please fix it or resend the patch as an attachment?
Thanks,
Sure. It's attached now, hope that works better.
Quoted 7 lines
Yes, it's better but now there's another problem: you based your patch on 2.6.22.9, but the sis5595 driver changed a lot in 2.6.23, so I still can't apply it. Would you be so kind and send a new patch that would apply on top of 2.6.23?
Still, I can review the patch:
Ugh. I should have known that, silly me. Anyway, I'll apply my changes to it. Patch on its way.
Quoted 11 lines
Quoted 11 lines
diff -ubrN
linux-2.6.22.9.orig/drivers/hwmon/sis5595.clinux-2.6.22.9/drivers/hwmon/sis5595.cdrivers/hwmon/sis5595.c +1
-0@@ -62,6 +62,7 @@#include <linux/jiffies.h> #include <linux/mutex.h> #include <linux/sysfs.h> +#include <linux/hwmon-sysfs.h> #include <asm/io.h>
Quoted 9 lines
drivers/hwmon/sis5595.c line 478
Quoted 6 lines
drivers/hwmon/sis5595.c +1
-0@@ -473,26 +474,47 @@ } static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL); +static ssize_t show_alarm (struct device *dev,struct device_attribute *da,char *buf)Remember that line length should never exceed 80 columns.
Yup, fixed
Quoted 9 lines
Quoted 9 lines
drivers/hwmon/sis5595.c +9
-0+{ +struct sis5595_data *data =sis5595_update_device (dev); +int nr =to_sensor_dev_attr (da)->index; +return sprintf (buf,"%u\n" , (data->alarms >> nr) &1 ); +} + static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, show_alarm, NULL, 0); + static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, show_alarm, NULL, 1); + static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, show_alarm, NULL, 2); + static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, show_alarm, NULL, 3);
Quoted 9 lines
drivers/hwmon/sis5595.c line 492
Quoted 5 lines
drivers/hwmon/sis5595.c +3
-0+ static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, show_alarm, NULL, 15); + static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 6); + static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 7); + static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 15);Bad indentation, all these "static
SENSOR_DEVICE_ATTR" lines shouldn't be indented.
Doh, fixed
Quoted 60 lines
Quoted 60 lines
drivers/hwmon/sis5595.c +13
-2+static struct attribute *sis5595_attributes[] = { &dev_attr_in0_input.attr, &dev_attr_in0_min.attr, &dev_attr_in0_max.attr, + &sensor_dev_attr_in0_alarm.dev_attr.attr, &dev_attr_in1_input.attr, &dev_attr_in1_min.attr, &dev_attr_in1_max.attr, + &sensor_dev_attr_in1_alarm.dev_attr.attr, &dev_attr_in2_input.attr, &dev_attr_in2_min.attr, &dev_attr_in2_max.attr, + &sensor_dev_attr_in2_alarm.dev_attr.attr, &dev_attr_in3_input.attr, &dev_attr_in3_min.attr, &dev_attr_in3_max.attr, + &sensor_dev_attr_in3_alarm.dev_attr.attr, &dev_attr_fan1_input.attr, &dev_attr_fan1_min.attr, &dev_attr_fan1_div.attr, + &sensor_dev_attr_fan1_alarm.dev_attr.attr, &dev_attr_fan2_input.attr, &dev_attr_fan2_min.attr, &dev_attr_fan2_div.attr, + &sensor_dev_attr_fan2_alarm.dev_attr.attr, &dev_attr_alarms.attr,NULL @@ -506,10 +528,12 @@ &dev_attr_in4_input.attr, &dev_attr_in4_min.attr, &dev_attr_in4_max.attr, + &sensor_dev_attr_in4_alarm.dev_attr.attr, &dev_attr_temp1_input.attr, &dev_attr_temp1_max.attr, &dev_attr_temp1_max_hyst.attr, + &sensor_dev_attr_temp1_alarm.dev_attr.attr,NULL };@@ -617,7 +641,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_in4_min)) || (err = device_create_file(&new_client->dev, - &dev_attr_in4_max))) + &dev_attr_in4_max)) + || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_in4_alarm.dev_attr))) goto exit_remove_files; } else { if ((err = device_create_file(&new_client->dev,@@ -625,7 +651,9 @@ || (err = device_create_file(&new_client->dev, &dev_attr_temp1_max)) || (err =device_create_file (&new_client->dev, - &dev_attr_temp1_max_hyst))) + &dev_attr_temp1_max_hyst))
Quoted 10 lines
drivers/hwmon/sis5595.c line 659
Quoted 6 lines
drivers/hwmon/sis5595.c +2
-0+ || (err = device_create_file(&new_client->dev, + &sensor_dev_attr_temp1_alarm.dev_attr))) goto exit_remove_files; }Other than that, your patch looks good.
Thanks,
Ivo
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Ivo,
Quoted 4 lines · Ivo Manca
Hey Jean,
Thanks for the quick review. It's starting to show I'm new to this, isn't it? ;)
New contributors are always welcome :)
Quoted 12 lines
Anyway, one question while making the patch for 2.6.23.1:
(quoted diff) +3
-1@@ -553,7 +578,9 @@ || (err = device_create_file(&pdev->dev, &dev_attr_temp1_max)) || (err = device_create_file(&pdev->dev, - &dev_attr_temp1_max_hyst))) + &dev_attr_temp1_max_hyst)) + || (err = device_create_file(&pdev->dev, + &sensor_dev_attr_temp1_alarm.dev_attr))) goto exit_remove_files;(E-mail client will most likely mess this up, however, it's also in the
Quoted 8 lines
original patch you just reviewed...).
The line "&sensor_dev_attr_temp1_alarm.dev_attr)))" can not fit within 80 characters using the same indentation as the lines above it; it takes 87 characters. However, removing the spaces before it, will still make it use 81 chars... I can't see any hint about it in the CodingStyles and removing some tabs will make it look just plain ugly. Any hint how to format it? Patch against 2.6.23.1 is ready, just waiting to be sure how to do this correct :p
The usual way to workaround this is indeed to reduce the level of indentation. Not necessarily very nice, but the easiest. In this particular case, if you don't want to do that, I see two alternatives (probably for a different patch though):
1* Move the creation of these individual files to a separate function. This saves one level of indentation, so it should fit.
2* Split sis5595_attributes_opt into sis5595_attributes_in4 and sis5595_attributes_temp1, and use sysfs_create_group on these instead of individual calls to device_create_file.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors