* [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr
@ 2009-10-22 10:08 Hans de Goede
2009-10-28 9:06 ` Jean Delvare
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Hans de Goede @ 2009-10-22 10:08 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
hwmon-f71882fg: Cleanup sysfs attr creation 2/2
This patch merges the f71882fg_auto_pwm_attr array into the
fxxxx_fan_attr resp. fxxxx_auto_pwm_attr array, as the f71882fg_auto_pwm_attr
array was merely extending these 2 with entries for a 4th fan, it also makes
these 2 arrays 2 dimensional so that the rest of the code can choose to
add attr for 3 or 4 fans without needing to know the nr of attr per fan.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[-- Attachment #2: hwmon-f71882fg-sysfs-create-cleanup2.patch --]
[-- Type: text/plain, Size: 9281 bytes --]
hwmon-f71882fg: Cleanup sysfs attr creation 2/2
This patch merges the f71882fg_auto_pwm_attr array into the
fxxxx_fan_attr resp. fxxxx_auto_pwm_attr array, as the f71882fg_auto_pwm_attr
array was merely extending these 2 with entries for a 4th fan, it also makes
these 2 arrays 2 dimensional so that the rest of the code can choose to
add attr for 3 or 4 fans without needing to know the nr of attr per fan.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
--- linux/drivers/hwmon/f71882fg.c 2009-10-21 11:59:06.000000000 +0200
+++ linux/drivers/hwmon/f71882fg.c 2009-10-21 13:25:13.000000000 +0200
@@ -416,41 +416,51 @@
};
/* Fan / PWM attr common to all models */
-static struct sensor_device_attribute_2 fxxxx_fan_attr[] = {
+static struct sensor_device_attribute_2 fxxxx_fan_attr[4][6] = { {
SENSOR_ATTR_2(fan1_input, S_IRUGO, show_fan, NULL, 0, 0),
SENSOR_ATTR_2(fan1_full_speed, S_IRUGO|S_IWUSR,
show_fan_full_speed,
store_fan_full_speed, 0, 0),
SENSOR_ATTR_2(fan1_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 0),
- SENSOR_ATTR_2(fan2_input, S_IRUGO, show_fan, NULL, 0, 1),
- SENSOR_ATTR_2(fan2_full_speed, S_IRUGO|S_IWUSR,
- show_fan_full_speed,
- store_fan_full_speed, 0, 1),
- SENSOR_ATTR_2(fan2_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 1),
- SENSOR_ATTR_2(fan3_input, S_IRUGO, show_fan, NULL, 0, 2),
- SENSOR_ATTR_2(fan3_full_speed, S_IRUGO|S_IWUSR,
- show_fan_full_speed,
- store_fan_full_speed, 0, 2),
- SENSOR_ATTR_2(fan3_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 2),
-
SENSOR_ATTR_2(pwm1, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 0),
SENSOR_ATTR_2(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
store_pwm_enable, 0, 0),
SENSOR_ATTR_2(pwm1_interpolate, S_IRUGO|S_IWUSR,
show_pwm_interpolate, store_pwm_interpolate, 0, 0),
-
+}, {
+ SENSOR_ATTR_2(fan2_input, S_IRUGO, show_fan, NULL, 0, 1),
+ SENSOR_ATTR_2(fan2_full_speed, S_IRUGO|S_IWUSR,
+ show_fan_full_speed,
+ store_fan_full_speed, 0, 1),
+ SENSOR_ATTR_2(fan2_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 1),
SENSOR_ATTR_2(pwm2, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 1),
SENSOR_ATTR_2(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
store_pwm_enable, 0, 1),
SENSOR_ATTR_2(pwm2_interpolate, S_IRUGO|S_IWUSR,
show_pwm_interpolate, store_pwm_interpolate, 0, 1),
-
+}, {
+ SENSOR_ATTR_2(fan3_input, S_IRUGO, show_fan, NULL, 0, 2),
+ SENSOR_ATTR_2(fan3_full_speed, S_IRUGO|S_IWUSR,
+ show_fan_full_speed,
+ store_fan_full_speed, 0, 2),
+ SENSOR_ATTR_2(fan3_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 2),
SENSOR_ATTR_2(pwm3, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 2),
SENSOR_ATTR_2(pwm3_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
store_pwm_enable, 0, 2),
SENSOR_ATTR_2(pwm3_interpolate, S_IRUGO|S_IWUSR,
show_pwm_interpolate, store_pwm_interpolate, 0, 2),
-};
+}, {
+ SENSOR_ATTR_2(fan4_input, S_IRUGO, show_fan, NULL, 0, 3),
+ SENSOR_ATTR_2(fan4_full_speed, S_IRUGO|S_IWUSR,
+ show_fan_full_speed,
+ store_fan_full_speed, 0, 3),
+ SENSOR_ATTR_2(fan4_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 3),
+ SENSOR_ATTR_2(pwm4, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 3),
+ SENSOR_ATTR_2(pwm4_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
+ store_pwm_enable, 0, 3),
+ SENSOR_ATTR_2(pwm4_interpolate, S_IRUGO|S_IWUSR,
+ show_pwm_interpolate, store_pwm_interpolate, 0, 3),
+} };
/* Attr for models which can beep on Fan alarm */
static struct sensor_device_attribute_2 fxxxx_fan_beep_attr[] = {
@@ -460,10 +470,12 @@
store_fan_beep, 0, 1),
SENSOR_ATTR_2(fan3_beep, S_IRUGO|S_IWUSR, show_fan_beep,
store_fan_beep, 0, 2),
+ SENSOR_ATTR_2(fan4_beep, S_IRUGO|S_IWUSR, show_fan_beep,
+ store_fan_beep, 0, 3),
};
/* PWM attr for the f71862fg, less pwms and less zones per pwm than the
- f71882fg / f71889fg */
+ f71858fg / f71882fg / f71889fg */
static struct sensor_device_attribute_2 f71862fg_auto_pwm_attr[] = {
SENSOR_ATTR_2(pwm1_auto_channels_temp, S_IRUGO|S_IWUSR,
show_pwm_auto_point_channel,
@@ -533,7 +545,7 @@
};
/* PWM attr common to both the f71858fg, f71882fg and f71889fg */
-static struct sensor_device_attribute_2 fxxxx_auto_pwm_attr[] = {
+static struct sensor_device_attribute_2 fxxxx_auto_pwm_attr[4][14] = { {
SENSOR_ATTR_2(pwm1_auto_channels_temp, S_IRUGO|S_IWUSR,
show_pwm_auto_point_channel,
store_pwm_auto_point_channel, 0, 0),
@@ -574,7 +586,7 @@
show_pwm_auto_point_temp_hyst, NULL, 2, 0),
SENSOR_ATTR_2(pwm1_auto_point4_temp_hyst, S_IRUGO,
show_pwm_auto_point_temp_hyst, NULL, 3, 0),
-
+}, {
SENSOR_ATTR_2(pwm2_auto_channels_temp, S_IRUGO|S_IWUSR,
show_pwm_auto_point_channel,
store_pwm_auto_point_channel, 0, 1),
@@ -615,7 +627,7 @@
show_pwm_auto_point_temp_hyst, NULL, 2, 1),
SENSOR_ATTR_2(pwm2_auto_point4_temp_hyst, S_IRUGO,
show_pwm_auto_point_temp_hyst, NULL, 3, 1),
-
+}, {
SENSOR_ATTR_2(pwm3_auto_channels_temp, S_IRUGO|S_IWUSR,
show_pwm_auto_point_channel,
store_pwm_auto_point_channel, 0, 2),
@@ -656,23 +668,7 @@
show_pwm_auto_point_temp_hyst, NULL, 2, 2),
SENSOR_ATTR_2(pwm3_auto_point4_temp_hyst, S_IRUGO,
show_pwm_auto_point_temp_hyst, NULL, 3, 2),
-};
-
-/* Fan / PWM attr found on the f71882fg but not on the f71858fg */
-static struct sensor_device_attribute_2 f71882fg_auto_pwm_attr[] = {
- SENSOR_ATTR_2(fan4_input, S_IRUGO, show_fan, NULL, 0, 3),
- SENSOR_ATTR_2(fan4_full_speed, S_IRUGO|S_IWUSR,
- show_fan_full_speed,
- store_fan_full_speed, 0, 3),
- SENSOR_ATTR_2(fan4_beep, S_IRUGO|S_IWUSR, show_fan_beep,
- store_fan_beep, 0, 3),
- SENSOR_ATTR_2(fan4_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 3),
-
- SENSOR_ATTR_2(pwm4, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 3),
- SENSOR_ATTR_2(pwm4_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
- store_pwm_enable, 0, 3),
- SENSOR_ATTR_2(pwm4_interpolate, S_IRUGO|S_IWUSR,
- show_pwm_interpolate, store_pwm_interpolate, 0, 3),
+}, {
SENSOR_ATTR_2(pwm4_auto_channels_temp, S_IRUGO|S_IWUSR,
show_pwm_auto_point_channel,
store_pwm_auto_point_channel, 0, 3),
@@ -713,7 +709,7 @@
show_pwm_auto_point_temp_hyst, NULL, 2, 3),
SENSOR_ATTR_2(pwm4_auto_point4_temp_hyst, S_IRUGO,
show_pwm_auto_point_temp_hyst, NULL, 3, 3),
-};
+} };
/* Fan attr specific to the f8000 (4th fan input can only measure speed) */
static struct sensor_device_attribute_2 f8000_fan_attr[] = {
@@ -1917,39 +1913,24 @@
goto exit_unregister_sysfs;
}
- err = f71882fg_create_sysfs_files(pdev, fxxxx_fan_attr,
- ARRAY_SIZE(fxxxx_fan_attr));
+ err = f71882fg_create_sysfs_files(pdev, &fxxxx_fan_attr[0][0],
+ ARRAY_SIZE(fxxxx_fan_attr[0]) * nr_fans);
if (err)
goto exit_unregister_sysfs;
- switch (data->type) {
- case f71862fg:
+ if (data->type == f71862fg || data->type == f71882fg) {
err = f71882fg_create_sysfs_files(pdev,
- fxxxx_fan_beep_attr,
- ARRAY_SIZE(fxxxx_fan_beep_attr));
+ fxxxx_fan_beep_attr, nr_fans);
if (err)
goto exit_unregister_sysfs;
+ }
+
+ switch (data->type) {
+ case f71862fg:
err = f71882fg_create_sysfs_files(pdev,
f71862fg_auto_pwm_attr,
ARRAY_SIZE(f71862fg_auto_pwm_attr));
break;
- case f71882fg:
- err = f71882fg_create_sysfs_files(pdev,
- fxxxx_fan_beep_attr,
- ARRAY_SIZE(fxxxx_fan_beep_attr));
- if (err)
- goto exit_unregister_sysfs;
- err = f71882fg_create_sysfs_files(pdev,
- f71882fg_auto_pwm_attr,
- ARRAY_SIZE(f71882fg_auto_pwm_attr));
- if (err)
- goto exit_unregister_sysfs;
- /* fall through! */
- case f71858fg:
- err = f71882fg_create_sysfs_files(pdev,
- fxxxx_auto_pwm_attr,
- ARRAY_SIZE(fxxxx_auto_pwm_attr));
- break;
case f8000:
err = f71882fg_create_sysfs_files(pdev,
f8000_fan_attr,
@@ -1960,6 +1941,10 @@
f8000_auto_pwm_attr,
ARRAY_SIZE(f8000_auto_pwm_attr));
break;
+ default: /* f71858fg / f71882fg / f71889fg */
+ err = f71882fg_create_sysfs_files(pdev,
+ &fxxxx_auto_pwm_attr[0][0],
+ ARRAY_SIZE(fxxxx_auto_pwm_attr[0]) * nr_fans);
}
if (err)
goto exit_unregister_sysfs;
@@ -1989,7 +1974,7 @@
static int f71882fg_remove(struct platform_device *pdev)
{
- int i;
+ int i, j;
struct f71882fg_data *data = platform_get_drvdata(pdev);
platform_set_drvdata(pdev, NULL);
@@ -2009,15 +1994,18 @@
&fxxxx_in1_alarm_attr[i].dev_attr);
for (i = 0; i < ARRAY_SIZE(fxxxx_fan_attr); i++)
- device_remove_file(&pdev->dev, &fxxxx_fan_attr[i].dev_attr);
+ for (j = 0; j < ARRAY_SIZE(fxxxx_fan_attr[0]); j++)
+ device_remove_file(&pdev->dev,
+ &fxxxx_fan_attr[i][j].dev_attr);
for (i = 0; i < ARRAY_SIZE(fxxxx_fan_beep_attr); i++)
device_remove_file(&pdev->dev,
&fxxxx_fan_beep_attr[i].dev_attr);
- for (i = 0; i < ARRAY_SIZE(f71882fg_auto_pwm_attr); i++)
- device_remove_file(&pdev->dev,
- &f71882fg_auto_pwm_attr[i].dev_attr);
+ for (i = 0; i < ARRAY_SIZE(fxxxx_auto_pwm_attr); i++)
+ for (j = 0; j < ARRAY_SIZE(fxxxx_auto_pwm_attr[0]); j++)
+ device_remove_file(&pdev->dev,
+ &fxxxx_auto_pwm_attr[i][j].dev_attr);
for (i = 0; i < ARRAY_SIZE(f8000_auto_pwm_attr); i++)
device_remove_file(&pdev->dev,
[-- 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 [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr
2009-10-22 10:08 [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr Hans de Goede
@ 2009-10-28 9:06 ` Jean Delvare
2009-10-28 9:26 ` Hans de Goede
2009-10-28 9:32 ` Jean Delvare
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-10-28 9:06 UTC (permalink / raw)
To: lm-sensors
On Thu, 22 Oct 2009 12:08:58 +0200, Hans de Goede wrote:
> hwmon-f71882fg: Cleanup sysfs attr creation 2/2
>
> This patch merges the f71882fg_auto_pwm_attr array into the
> fxxxx_fan_attr resp. fxxxx_auto_pwm_attr array, as the f71882fg_auto_pwm_attr
> array was merely extending these 2 with entries for a 4th fan, it also makes
> these 2 arrays 2 dimensional so that the rest of the code can choose to
> add attr for 3 or 4 fans without needing to know the nr of attr per fan.
Again only minor comments, which I can fix myself:
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> --- linux/drivers/hwmon/f71882fg.c 2009-10-21 11:59:06.000000000 +0200
> +++ linux/drivers/hwmon/f71882fg.c 2009-10-21 13:25:13.000000000 +0200
> @@ -416,41 +416,51 @@
> };
>
> /* Fan / PWM attr common to all models */
> -static struct sensor_device_attribute_2 fxxxx_fan_attr[] = {
> +static struct sensor_device_attribute_2 fxxxx_fan_attr[4][6] = { {
> SENSOR_ATTR_2(fan1_input, S_IRUGO, show_fan, NULL, 0, 0),
> SENSOR_ATTR_2(fan1_full_speed, S_IRUGO|S_IWUSR,
> show_fan_full_speed,
> store_fan_full_speed, 0, 0),
> SENSOR_ATTR_2(fan1_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 0),
> - SENSOR_ATTR_2(fan2_input, S_IRUGO, show_fan, NULL, 0, 1),
> - SENSOR_ATTR_2(fan2_full_speed, S_IRUGO|S_IWUSR,
> - show_fan_full_speed,
> - store_fan_full_speed, 0, 1),
> - SENSOR_ATTR_2(fan2_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 1),
> - SENSOR_ATTR_2(fan3_input, S_IRUGO, show_fan, NULL, 0, 2),
> - SENSOR_ATTR_2(fan3_full_speed, S_IRUGO|S_IWUSR,
> - show_fan_full_speed,
> - store_fan_full_speed, 0, 2),
> - SENSOR_ATTR_2(fan3_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 2),
> -
> SENSOR_ATTR_2(pwm1, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 0),
> SENSOR_ATTR_2(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
> store_pwm_enable, 0, 0),
> SENSOR_ATTR_2(pwm1_interpolate, S_IRUGO|S_IWUSR,
> show_pwm_interpolate, store_pwm_interpolate, 0, 0),
> -
> +}, {
> + SENSOR_ATTR_2(fan2_input, S_IRUGO, show_fan, NULL, 0, 1),
> + SENSOR_ATTR_2(fan2_full_speed, S_IRUGO|S_IWUSR,
> + show_fan_full_speed,
> + store_fan_full_speed, 0, 1),
> + SENSOR_ATTR_2(fan2_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 1),
> SENSOR_ATTR_2(pwm2, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 1),
> SENSOR_ATTR_2(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
> store_pwm_enable, 0, 1),
> SENSOR_ATTR_2(pwm2_interpolate, S_IRUGO|S_IWUSR,
> show_pwm_interpolate, store_pwm_interpolate, 0, 1),
> -
> +}, {
> + SENSOR_ATTR_2(fan3_input, S_IRUGO, show_fan, NULL, 0, 2),
> + SENSOR_ATTR_2(fan3_full_speed, S_IRUGO|S_IWUSR,
> + show_fan_full_speed,
> + store_fan_full_speed, 0, 2),
> + SENSOR_ATTR_2(fan3_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 2),
> SENSOR_ATTR_2(pwm3, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 2),
> SENSOR_ATTR_2(pwm3_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
> store_pwm_enable, 0, 2),
> SENSOR_ATTR_2(pwm3_interpolate, S_IRUGO|S_IWUSR,
> show_pwm_interpolate, store_pwm_interpolate, 0, 2),
> -};
> +}, {
> + SENSOR_ATTR_2(fan4_input, S_IRUGO, show_fan, NULL, 0, 3),
> + SENSOR_ATTR_2(fan4_full_speed, S_IRUGO|S_IWUSR,
> + show_fan_full_speed,
> + store_fan_full_speed, 0, 3),
> + SENSOR_ATTR_2(fan4_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 3),
> + SENSOR_ATTR_2(pwm4, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 3),
> + SENSOR_ATTR_2(pwm4_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
> + store_pwm_enable, 0, 3),
> + SENSOR_ATTR_2(pwm4_interpolate, S_IRUGO|S_IWUSR,
> + show_pwm_interpolate, store_pwm_interpolate, 0, 3),
> +} };
>
> /* Attr for models which can beep on Fan alarm */
> static struct sensor_device_attribute_2 fxxxx_fan_beep_attr[] = {
> @@ -460,10 +470,12 @@
> store_fan_beep, 0, 1),
> SENSOR_ATTR_2(fan3_beep, S_IRUGO|S_IWUSR, show_fan_beep,
> store_fan_beep, 0, 2),
> + SENSOR_ATTR_2(fan4_beep, S_IRUGO|S_IWUSR, show_fan_beep,
> + store_fan_beep, 0, 3),
> };
>
> /* PWM attr for the f71862fg, less pwms and less zones per pwm than the
> - f71882fg / f71889fg */
> + f71858fg / f71882fg / f71889fg */
This comment change should be merged into patch 1.
> static struct sensor_device_attribute_2 f71862fg_auto_pwm_attr[] = {
> SENSOR_ATTR_2(pwm1_auto_channels_temp, S_IRUGO|S_IWUSR,
> show_pwm_auto_point_channel,
> @@ -533,7 +545,7 @@
> };
>
> /* PWM attr common to both the f71858fg, f71882fg and f71889fg */
> -static struct sensor_device_attribute_2 fxxxx_auto_pwm_attr[] = {
> +static struct sensor_device_attribute_2 fxxxx_auto_pwm_attr[4][14] = { {
> SENSOR_ATTR_2(pwm1_auto_channels_temp, S_IRUGO|S_IWUSR,
> show_pwm_auto_point_channel,
> store_pwm_auto_point_channel, 0, 0),
> @@ -574,7 +586,7 @@
> show_pwm_auto_point_temp_hyst, NULL, 2, 0),
> SENSOR_ATTR_2(pwm1_auto_point4_temp_hyst, S_IRUGO,
> show_pwm_auto_point_temp_hyst, NULL, 3, 0),
> -
> +}, {
> SENSOR_ATTR_2(pwm2_auto_channels_temp, S_IRUGO|S_IWUSR,
> show_pwm_auto_point_channel,
> store_pwm_auto_point_channel, 0, 1),
> @@ -615,7 +627,7 @@
> show_pwm_auto_point_temp_hyst, NULL, 2, 1),
> SENSOR_ATTR_2(pwm2_auto_point4_temp_hyst, S_IRUGO,
> show_pwm_auto_point_temp_hyst, NULL, 3, 1),
> -
> +}, {
> SENSOR_ATTR_2(pwm3_auto_channels_temp, S_IRUGO|S_IWUSR,
> show_pwm_auto_point_channel,
> store_pwm_auto_point_channel, 0, 2),
> @@ -656,23 +668,7 @@
> show_pwm_auto_point_temp_hyst, NULL, 2, 2),
> SENSOR_ATTR_2(pwm3_auto_point4_temp_hyst, S_IRUGO,
> show_pwm_auto_point_temp_hyst, NULL, 3, 2),
> -};
> -
> -/* Fan / PWM attr found on the f71882fg but not on the f71858fg */
> -static struct sensor_device_attribute_2 f71882fg_auto_pwm_attr[] = {
> - SENSOR_ATTR_2(fan4_input, S_IRUGO, show_fan, NULL, 0, 3),
> - SENSOR_ATTR_2(fan4_full_speed, S_IRUGO|S_IWUSR,
> - show_fan_full_speed,
> - store_fan_full_speed, 0, 3),
> - SENSOR_ATTR_2(fan4_beep, S_IRUGO|S_IWUSR, show_fan_beep,
> - store_fan_beep, 0, 3),
> - SENSOR_ATTR_2(fan4_alarm, S_IRUGO, show_fan_alarm, NULL, 0, 3),
> -
> - SENSOR_ATTR_2(pwm4, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 0, 3),
> - SENSOR_ATTR_2(pwm4_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
> - store_pwm_enable, 0, 3),
> - SENSOR_ATTR_2(pwm4_interpolate, S_IRUGO|S_IWUSR,
> - show_pwm_interpolate, store_pwm_interpolate, 0, 3),
> +}, {
> SENSOR_ATTR_2(pwm4_auto_channels_temp, S_IRUGO|S_IWUSR,
> show_pwm_auto_point_channel,
> store_pwm_auto_point_channel, 0, 3),
> @@ -713,7 +709,7 @@
> show_pwm_auto_point_temp_hyst, NULL, 2, 3),
> SENSOR_ATTR_2(pwm4_auto_point4_temp_hyst, S_IRUGO,
> show_pwm_auto_point_temp_hyst, NULL, 3, 3),
> -};
> +} };
>
> /* Fan attr specific to the f8000 (4th fan input can only measure speed) */
> static struct sensor_device_attribute_2 f8000_fan_attr[] = {
> @@ -1917,39 +1913,24 @@
> goto exit_unregister_sysfs;
> }
>
> - err = f71882fg_create_sysfs_files(pdev, fxxxx_fan_attr,
> - ARRAY_SIZE(fxxxx_fan_attr));
> + err = f71882fg_create_sysfs_files(pdev, &fxxxx_fan_attr[0][0],
> + ARRAY_SIZE(fxxxx_fan_attr[0]) * nr_fans);
I have to admit I am a little worried by this. Looping over individual
sub-arrays would look better (less hackish) to me. But maybe it's just
me. Another problem is that any hole in the array will cause a bug, and
unfortunately (my version of) gcc doesn't warn about this (and looping
over individual sub-arrays doesn't help.) But well, if you know what
you're doing it should be OK.
As a side note, I don't think "fxxxx_fan_attr" or
"&fxxxx_fan_attr[0][0]" make any difference.
> if (err)
> goto exit_unregister_sysfs;
>
> - switch (data->type) {
> - case f71862fg:
> + if (data->type = f71862fg || data->type = f71882fg) {
> err = f71882fg_create_sysfs_files(pdev,
> - fxxxx_fan_beep_attr,
> - ARRAY_SIZE(fxxxx_fan_beep_attr));
> + fxxxx_fan_beep_attr, nr_fans);
> if (err)
> goto exit_unregister_sysfs;
> + }
> +
> + switch (data->type) {
> + case f71862fg:
> err = f71882fg_create_sysfs_files(pdev,
> f71862fg_auto_pwm_attr,
> ARRAY_SIZE(f71862fg_auto_pwm_attr));
> break;
> - case f71882fg:
> - err = f71882fg_create_sysfs_files(pdev,
> - fxxxx_fan_beep_attr,
> - ARRAY_SIZE(fxxxx_fan_beep_attr));
> - if (err)
> - goto exit_unregister_sysfs;
> - err = f71882fg_create_sysfs_files(pdev,
> - f71882fg_auto_pwm_attr,
> - ARRAY_SIZE(f71882fg_auto_pwm_attr));
> - if (err)
> - goto exit_unregister_sysfs;
> - /* fall through! */
> - case f71858fg:
> - err = f71882fg_create_sysfs_files(pdev,
> - fxxxx_auto_pwm_attr,
> - ARRAY_SIZE(fxxxx_auto_pwm_attr));
> - break;
> case f8000:
> err = f71882fg_create_sysfs_files(pdev,
> f8000_fan_attr,
> @@ -1960,6 +1941,10 @@
> f8000_auto_pwm_attr,
> ARRAY_SIZE(f8000_auto_pwm_attr));
> break;
> + default: /* f71858fg / f71882fg / f71889fg */
> + err = f71882fg_create_sysfs_files(pdev,
> + &fxxxx_auto_pwm_attr[0][0],
> + ARRAY_SIZE(fxxxx_auto_pwm_attr[0]) * nr_fans);
> }
> if (err)
> goto exit_unregister_sysfs;
> @@ -1989,7 +1974,7 @@
>
> static int f71882fg_remove(struct platform_device *pdev)
> {
> - int i;
> + int i, j;
> struct f71882fg_data *data = platform_get_drvdata(pdev);
>
> platform_set_drvdata(pdev, NULL);
> @@ -2009,15 +1994,18 @@
> &fxxxx_in1_alarm_attr[i].dev_attr);
>
> for (i = 0; i < ARRAY_SIZE(fxxxx_fan_attr); i++)
> - device_remove_file(&pdev->dev, &fxxxx_fan_attr[i].dev_attr);
> + for (j = 0; j < ARRAY_SIZE(fxxxx_fan_attr[0]); j++)
> + device_remove_file(&pdev->dev,
> + &fxxxx_fan_attr[i][j].dev_attr);
>
> for (i = 0; i < ARRAY_SIZE(fxxxx_fan_beep_attr); i++)
> device_remove_file(&pdev->dev,
> &fxxxx_fan_beep_attr[i].dev_attr);
>
> - for (i = 0; i < ARRAY_SIZE(f71882fg_auto_pwm_attr); i++)
> - device_remove_file(&pdev->dev,
> - &f71882fg_auto_pwm_attr[i].dev_attr);
> + for (i = 0; i < ARRAY_SIZE(fxxxx_auto_pwm_attr); i++)
> + for (j = 0; j < ARRAY_SIZE(fxxxx_auto_pwm_attr[0]); j++)
> + device_remove_file(&pdev->dev,
> + &fxxxx_auto_pwm_attr[i][j].dev_attr);
>
> for (i = 0; i < ARRAY_SIZE(f8000_auto_pwm_attr); i++)
> device_remove_file(&pdev->dev,
--
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] 4+ messages in thread
* Re: [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr
2009-10-22 10:08 [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr Hans de Goede
2009-10-28 9:06 ` Jean Delvare
@ 2009-10-28 9:26 ` Hans de Goede
2009-10-28 9:32 ` Jean Delvare
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2009-10-28 9:26 UTC (permalink / raw)
To: lm-sensors
Hi,
On 10/28/2009 10:06 AM, Jean Delvare wrote:
> On Thu, 22 Oct 2009 12:08:58 +0200, Hans de Goede wrote:
>> hwmon-f71882fg: Cleanup sysfs attr creation 2/2
>>
>> This patch merges the f71882fg_auto_pwm_attr array into the
>> fxxxx_fan_attr resp. fxxxx_auto_pwm_attr array, as the f71882fg_auto_pwm_attr
>> array was merely extending these 2 with entries for a 4th fan, it also makes
>> these 2 arrays 2 dimensional so that the rest of the code can choose to
>> add attr for 3 or 4 fans without needing to know the nr of attr per fan.
>
> Again only minor comments, which I can fix myself:
>
And again thanks and feel free to fix them!
Some responses to your comments below:
<snip>
>>
>> /* Fan attr specific to the f8000 (4th fan input can only measure speed) */
>> static struct sensor_device_attribute_2 f8000_fan_attr[] = {
>> @@ -1917,39 +1913,24 @@
>> goto exit_unregister_sysfs;
>> }
>>
>> - err = f71882fg_create_sysfs_files(pdev, fxxxx_fan_attr,
>> - ARRAY_SIZE(fxxxx_fan_attr));
>> + err = f71882fg_create_sysfs_files(pdev,&fxxxx_fan_attr[0][0],
>> + ARRAY_SIZE(fxxxx_fan_attr[0]) * nr_fans);
>
> I have to admit I am a little worried by this. Looping over individual
> sub-arrays would look better (less hackish) to me. But maybe it's just
> me. Another problem is that any hole in the array will cause a bug, and
> unfortunately (my version of) gcc doesn't warn about this (and looping
> over individual sub-arrays doesn't help.) But well, if you know what
> you're doing it should be OK.
>
I agree this is a bit tricky, at first I wanted to add a
f71882fg_create_sysfs_files2 function, which would do a double loop, but that
then would need to get passed in the array dimensions and would still
need to get passed in a normal pointer to the first element, as I can
not code any of the dimensions inside the parameter list as they are not
constant, so that function would end up doing exactly the same as I'm
doing now, so I opted to just use the existing function for this.
> As a side note, I don't think "fxxxx_fan_attr" or
> "&fxxxx_fan_attr[0][0]" make any difference.
>
I would expect the type of fxxxx_fan_attr to be a *foo[] and not a *foo,
and thus to get a compiler warning, but you might be right, I didn't try.
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr
2009-10-22 10:08 [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr Hans de Goede
2009-10-28 9:06 ` Jean Delvare
2009-10-28 9:26 ` Hans de Goede
@ 2009-10-28 9:32 ` Jean Delvare
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-10-28 9:32 UTC (permalink / raw)
To: lm-sensors
On Wed, 28 Oct 2009 10:26:00 +0100, Hans de Goede wrote:
> > As a side note, I don't think "fxxxx_fan_attr" or
> > "&fxxxx_fan_attr[0][0]" make any difference.
> >
>
> I would expect the type of fxxxx_fan_attr to be a *foo[] and not a *foo,
> and thus to get a compiler warning, but you might be right, I didn't try.
Hmm, you're right, I get a warning with fxxxx_fan_attr. Apparently I
lost an opportunity to keep my mouth closed ;)
--
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] 4+ messages in thread
end of thread, other threads:[~2009-10-28 9:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-22 10:08 [lm-sensors] [PATCH 2/4] hwmon-f71882fg: Cleanup sysfs attr Hans de Goede
2009-10-28 9:06 ` Jean Delvare
2009-10-28 9:26 ` Hans de Goede
2009-10-28 9:32 ` 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.