[patch] hwmon-sysfs.h: add _RO and _RW versions of SENSOR*_ATTR macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jean Delvare wrote:
> Hi Jim,
>
>   
>> 1st attachment adds _RO and _RW versions to the following macros:
>> SENSOR_ATTR, SENSOR_DEVICE_ATTR,
>> SENSOR_ATTR_2, SENSOR_DEVICE_ATTR_2,
>>
>> with these defined, the script (2nd attachment) will convert ~800 uses
>> (in 17 files) to the correct _RO/_RW version, and remove the linewraps.
>> The results compile cleanly (all modules in hwmon).
>>     
>
> What are you trying to achieve?
>
>   
cosmetics and abstraction.
- the 800 uses effectively have 2 modes; 0644 and 0444, but have 
multiple ways of saying it
- these permissions approach 'canonical' use, so if we hide them, 
exceptions become more distinct.
- lines are shorter, much less line-wrap fussing.

> May you please post a sample patch as obtained by using the script
> above on one arbitrary driver? So that we can see what it looks like.
>
>   
ok.  Ive in-lined the patch to hwmon-sysfs.h, and the script's 
conversion of f71805f.c

>> Is this something you'd consider applying ? If so, when ?
>>     
>
> It merely depends on what our purpose is. As for "when", remember that
> we have two high priority tasks for hwmon drivers now: individual alarm
> files, and i2c-isa removal. I would like the first one to be done for
> 2.6.20, and the second in 2.6.21 but individual drivers can (should) be
> converted to platform drivers before that. These will be large and
> intrusive patches and I am unlikely to take other large and intrudive
> patches until it's done.
>
>   
>> Theres obvious timing issues - since this patch touches many drivers, 
>> which may have work in-queue (yours or other hackers').  Its also why
>> I sent the script - the patch it yields is 94KB, so the script is far
>> more inspectable, and with the script, you can chose the optimal
>> time(s) w/o synchronization hassles.
>>     
>
> Yes, sounds like a good idea, even though in this case your script is
> rather frightening. I can't get why you didn't write a real perl
> script? It would perform better and would certainly be somewhat more
> readable too.
>
>   
its an assembly of 1-liners.
regexs always look like line-noise, its their nature.
runtime on hwmon/*.c was <5 sec, hardly prohibitive for a 1-time script.

That said, Ive rewritten it as a pure perl script, attached.
use it like so:
perl chg-ro drivers/hwmon/*.c

diff -ruNp -X dontdiff -X exclude-diffs 19rc1-0/include/linux/hwmon-sysfs.h ro-rw/include/linux/hwmon-sysfs.h
--- 19rc1-0/include/linux/hwmon-sysfs.h	2006-06-17 19:49:35.000000000 -0600
+++ ro-rw/include/linux/hwmon-sysfs.h	2006-10-08 09:33:48.000000000 -0600
@@ -31,10 +31,24 @@ struct sensor_device_attribute{
 	{ .dev_attr = __ATTR(_name, _mode, _show, _store),	\
 	  .index = _index }
 
+#define SENSOR_ATTR_RO(_name, _show, _index)	\
+	SENSOR_ATTR(_name, S_IRUGO, _show, NULL, _index)
+
+#define SENSOR_ATTR_RW(_name, _show, _store, _index)	\
+	SENSOR_ATTR(_name, S_IRUGO | S_IWUSR, _show, _store, _index)
+
 #define SENSOR_DEVICE_ATTR(_name, _mode, _show, _store, _index)	\
 struct sensor_device_attribute sensor_dev_attr_##_name		\
 	= SENSOR_ATTR(_name, _mode, _show, _store, _index)
 
+#define SENSOR_DEVICE_ATTR_RO(_name, _show, _index)	\
+struct sensor_device_attribute sensor_dev_attr_##_name	\
+	= SENSOR_ATTR_RO(_name, _show, _index)
+
+#define SENSOR_DEVICE_ATTR_RW(_name, _show, _store, _index)	\
+struct sensor_device_attribute sensor_dev_attr_##_name		\
+	= SENSOR_ATTR_RW(_name, _show, _store, _index)
+
 struct sensor_device_attribute_2 {
 	struct device_attribute dev_attr;
 	u8 index;
@@ -48,8 +62,22 @@ struct sensor_device_attribute_2 {
 	  .index = _index,					\
 	  .nr = _nr }
 
+#define SENSOR_ATTR_2RO(_name, _show, _nr, _index)		\
+	SENSOR_ATTR_2(_name, S_IRUGO, _show, NULL, _nr, _index)
+
+#define SENSOR_ATTR_2RW(_name, _show, _store, _nr, _index)	\
+	SENSOR_ATTR_2(_name, S_IRUGO | S_IWUSR, _show, _store, _nr, _index)
+
 #define SENSOR_DEVICE_ATTR_2(_name,_mode,_show,_store,_nr,_index)	\
 struct sensor_device_attribute_2 sensor_dev_attr_##_name		\
 	= SENSOR_ATTR_2(_name, _mode, _show, _store, _nr, _index)
 
+#define SENSOR_DEVICE_ATTR_2RO(_name,_show,_nr,_index)		\
+struct sensor_device_attribute_2 sensor_dev_attr_##_name	\
+	= SENSOR_ATTR_2RO(_name, _show, _nr, _index)
+
+#define SENSOR_DEVICE_ATTR_2RW(_name,_show,_store,_nr,_index)	\
+struct sensor_device_attribute_2 sensor_dev_attr_##_name	\
+	= SENSOR_ATTR_2RW(_name, _show, _store, _nr, _index)
+
 #endif /* _LINUX_HWMON_SYSFS_H */


diff -ruNp -X dontdiff -X exclude-diffs 19rc1-3/drivers/hwmon/f71805f.c 19rc1-4/drivers/hwmon/f71805f.c
--- 19rc1-3/drivers/hwmon/f71805f.c	2006-10-05 20:17:49.000000000 -0600
+++ 19rc1-4/drivers/hwmon/f71805f.c	2006-10-08 22:01:58.000000000 -0600
@@ -597,91 +597,66 @@ static ssize_t show_name(struct device *
 static DEVICE_ATTR(in0_input, S_IRUGO, show_in0, NULL);
 static DEVICE_ATTR(in0_max, S_IRUGO| S_IWUSR, show_in0_max, set_in0_max);
 static DEVICE_ATTR(in0_min, S_IRUGO| S_IWUSR, show_in0_min, set_in0_min);
-static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, show_in, NULL, 1);
-static SENSOR_DEVICE_ATTR(in1_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 1);
-static SENSOR_DEVICE_ATTR(in1_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 1);
-static SENSOR_DEVICE_ATTR(in2_input, S_IRUGO, show_in, NULL, 2);
-static SENSOR_DEVICE_ATTR(in2_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 2);
-static SENSOR_DEVICE_ATTR(in2_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 2);
-static SENSOR_DEVICE_ATTR(in3_input, S_IRUGO, show_in, NULL, 3);
-static SENSOR_DEVICE_ATTR(in3_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 3);
-static SENSOR_DEVICE_ATTR(in3_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 3);
-static SENSOR_DEVICE_ATTR(in4_input, S_IRUGO, show_in, NULL, 4);
-static SENSOR_DEVICE_ATTR(in4_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 4);
-static SENSOR_DEVICE_ATTR(in4_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 4);
-static SENSOR_DEVICE_ATTR(in5_input, S_IRUGO, show_in, NULL, 5);
-static SENSOR_DEVICE_ATTR(in5_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 5);
-static SENSOR_DEVICE_ATTR(in5_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 5);
-static SENSOR_DEVICE_ATTR(in6_input, S_IRUGO, show_in, NULL, 6);
-static SENSOR_DEVICE_ATTR(in6_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 6);
-static SENSOR_DEVICE_ATTR(in6_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 6);
-static SENSOR_DEVICE_ATTR(in7_input, S_IRUGO, show_in, NULL, 7);
-static SENSOR_DEVICE_ATTR(in7_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 7);
-static SENSOR_DEVICE_ATTR(in7_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 7);
-static SENSOR_DEVICE_ATTR(in8_input, S_IRUGO, show_in, NULL, 8);
-static SENSOR_DEVICE_ATTR(in8_max, S_IRUGO | S_IWUSR,
-			  show_in_max, set_in_max, 8);
-static SENSOR_DEVICE_ATTR(in8_min, S_IRUGO | S_IWUSR,
-			  show_in_min, set_in_min, 8);
-
-static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
-static SENSOR_DEVICE_ATTR(fan1_min, S_IRUGO | S_IWUSR,
-			  show_fan_min, set_fan_min, 0);
-static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
-static SENSOR_DEVICE_ATTR(fan2_min, S_IRUGO | S_IWUSR,
-			  show_fan_min, set_fan_min, 1);
-static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
-static SENSOR_DEVICE_ATTR(fan3_min, S_IRUGO | S_IWUSR,
-			  show_fan_min, set_fan_min, 2);
-
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO | S_IWUSR,
-		    show_temp_max, set_temp_max, 0);
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IRUGO | S_IWUSR,
-		    show_temp_hyst, set_temp_hyst, 0);
-static SENSOR_DEVICE_ATTR(temp1_type, S_IRUGO, show_temp_type, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp2_max, S_IRUGO | S_IWUSR,
-		    show_temp_max, set_temp_max, 1);
-static SENSOR_DEVICE_ATTR(temp2_max_hyst, S_IRUGO | S_IWUSR,
-		    show_temp_hyst, set_temp_hyst, 1);
-static SENSOR_DEVICE_ATTR(temp2_type, S_IRUGO, show_temp_type, NULL, 1);
-static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 2);
-static SENSOR_DEVICE_ATTR(temp3_max, S_IRUGO | S_IWUSR,
-		    show_temp_max, set_temp_max, 2);
-static SENSOR_DEVICE_ATTR(temp3_max_hyst, S_IRUGO | S_IWUSR,
-		    show_temp_hyst, set_temp_hyst, 2);
-static SENSOR_DEVICE_ATTR(temp3_type, S_IRUGO, show_temp_type, NULL, 2);
-
-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, 4);
-static SENSOR_DEVICE_ATTR(in5_alarm, S_IRUGO, show_alarm, NULL, 5);
-static SENSOR_DEVICE_ATTR(in6_alarm, S_IRUGO, show_alarm, NULL, 6);
-static SENSOR_DEVICE_ATTR(in7_alarm, S_IRUGO, show_alarm, NULL, 7);
-static SENSOR_DEVICE_ATTR(in8_alarm, S_IRUGO, show_alarm, NULL, 8);
-static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 11);
-static SENSOR_DEVICE_ATTR(temp2_alarm, S_IRUGO, show_alarm, NULL, 12);
-static SENSOR_DEVICE_ATTR(temp3_alarm, S_IRUGO, show_alarm, NULL, 13);
-static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL, 16);
-static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL, 17);
-static SENSOR_DEVICE_ATTR(fan3_alarm, S_IRUGO, show_alarm, NULL, 18);
+static SENSOR_DEVICE_ATTR_RO(in1_input, show_in, 1);
+static SENSOR_DEVICE_ATTR_RW(in1_max, show_in_max, set_in_max, 1);
+static SENSOR_DEVICE_ATTR_RW(in1_min, show_in_min, set_in_min, 1);
+static SENSOR_DEVICE_ATTR_RO(in2_input, show_in, 2);
+static SENSOR_DEVICE_ATTR_RW(in2_max, show_in_max, set_in_max, 2);
+static SENSOR_DEVICE_ATTR_RW(in2_min, show_in_min, set_in_min, 2);
+static SENSOR_DEVICE_ATTR_RO(in3_input, show_in, 3);
+static SENSOR_DEVICE_ATTR_RW(in3_max, show_in_max, set_in_max, 3);
+static SENSOR_DEVICE_ATTR_RW(in3_min, show_in_min, set_in_min, 3);
+static SENSOR_DEVICE_ATTR_RO(in4_input, show_in, 4);
+static SENSOR_DEVICE_ATTR_RW(in4_max, show_in_max, set_in_max, 4);
+static SENSOR_DEVICE_ATTR_RW(in4_min, show_in_min, set_in_min, 4);
+static SENSOR_DEVICE_ATTR_RO(in5_input, show_in, 5);
+static SENSOR_DEVICE_ATTR_RW(in5_max, show_in_max, set_in_max, 5);
+static SENSOR_DEVICE_ATTR_RW(in5_min, show_in_min, set_in_min, 5);
+static SENSOR_DEVICE_ATTR_RO(in6_input, show_in, 6);
+static SENSOR_DEVICE_ATTR_RW(in6_max, show_in_max, set_in_max, 6);
+static SENSOR_DEVICE_ATTR_RW(in6_min, show_in_min, set_in_min, 6);
+static SENSOR_DEVICE_ATTR_RO(in7_input, show_in, 7);
+static SENSOR_DEVICE_ATTR_RW(in7_max, show_in_max, set_in_max, 7);
+static SENSOR_DEVICE_ATTR_RW(in7_min, show_in_min, set_in_min, 7);
+static SENSOR_DEVICE_ATTR_RO(in8_input, show_in, 8);
+static SENSOR_DEVICE_ATTR_RW(in8_max, show_in_max, set_in_max, 8);
+static SENSOR_DEVICE_ATTR_RW(in8_min, show_in_min, set_in_min, 8);
+
+static SENSOR_DEVICE_ATTR_RO(fan1_input, show_fan, 0);
+static SENSOR_DEVICE_ATTR_RW(fan1_min, show_fan_min, set_fan_min, 0);
+static SENSOR_DEVICE_ATTR_RO(fan2_input, show_fan, 1);
+static SENSOR_DEVICE_ATTR_RW(fan2_min, show_fan_min, set_fan_min, 1);
+static SENSOR_DEVICE_ATTR_RO(fan3_input, show_fan, 2);
+static SENSOR_DEVICE_ATTR_RW(fan3_min, show_fan_min, set_fan_min, 2);
+
+static SENSOR_DEVICE_ATTR_RO(temp1_input, show_temp, 0);
+static SENSOR_DEVICE_ATTR_RW(temp1_max, show_temp_max, set_temp_max, 0);
+static SENSOR_DEVICE_ATTR_RW(temp1_max_hyst, show_temp_hyst, set_temp_hyst, 0);
+static SENSOR_DEVICE_ATTR_RO(temp1_type, show_temp_type, 0);
+static SENSOR_DEVICE_ATTR_RO(temp2_input, show_temp, 1);
+static SENSOR_DEVICE_ATTR_RW(temp2_max, show_temp_max, set_temp_max, 1);
+static SENSOR_DEVICE_ATTR_RW(temp2_max_hyst, show_temp_hyst, set_temp_hyst, 1);
+static SENSOR_DEVICE_ATTR_RO(temp2_type, show_temp_type, 1);
+static SENSOR_DEVICE_ATTR_RO(temp3_input, show_temp, 2);
+static SENSOR_DEVICE_ATTR_RW(temp3_max, show_temp_max, set_temp_max, 2);
+static SENSOR_DEVICE_ATTR_RW(temp3_max_hyst, show_temp_hyst, set_temp_hyst, 2);
+static SENSOR_DEVICE_ATTR_RO(temp3_type, show_temp_type, 2);
+
+static SENSOR_DEVICE_ATTR_RO(in0_alarm, show_alarm, 0);
+static SENSOR_DEVICE_ATTR_RO(in1_alarm, show_alarm, 1);
+static SENSOR_DEVICE_ATTR_RO(in2_alarm, show_alarm, 2);
+static SENSOR_DEVICE_ATTR_RO(in3_alarm, show_alarm, 3);
+static SENSOR_DEVICE_ATTR_RO(in4_alarm, show_alarm, 4);
+static SENSOR_DEVICE_ATTR_RO(in5_alarm, show_alarm, 5);
+static SENSOR_DEVICE_ATTR_RO(in6_alarm, show_alarm, 6);
+static SENSOR_DEVICE_ATTR_RO(in7_alarm, show_alarm, 7);
+static SENSOR_DEVICE_ATTR_RO(in8_alarm, show_alarm, 8);
+static SENSOR_DEVICE_ATTR_RO(temp1_alarm, show_alarm, 11);
+static SENSOR_DEVICE_ATTR_RO(temp2_alarm, show_alarm, 12);
+static SENSOR_DEVICE_ATTR_RO(temp3_alarm, show_alarm, 13);
+static SENSOR_DEVICE_ATTR_RO(fan1_alarm, show_alarm, 16);
+static SENSOR_DEVICE_ATTR_RO(fan2_alarm, show_alarm, 17);
+static SENSOR_DEVICE_ATTR_RO(fan3_alarm, show_alarm, 18);
 static DEVICE_ATTR(alarms_in, S_IRUGO, show_alarms_in, NULL);
 static DEVICE_ATTR(alarms_fan, S_IRUGO, show_alarms_fan, NULL);
 static DEVICE_ATTR(alarms_temp, S_IRUGO, show_alarms_temp, NULL);



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: chg-ro
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20061010/3d42e697/attachment.pl 


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux