[lm-sensors] Patch: hwmon-fscher-individual-alarm-files.patch

[lm-sensors] Patch: hwmon-fscher-individual-alarm-files.patch

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 160 bytes --]

Hi all,

As promised a patch adding individual alarm and fault files to the fscher
driver.

Signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>

Regards,

Hans

[-- Attachment #2: hwmon-fscher-individual-alarm-files.patch --]
[-- Type: text/x-patch, Size: 4737 bytes --]

As promised a patch adding individual alarm and fault files to the fscher
driver.

Signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>
diff -up drivers/hwmon/fscher.c.alarms drivers/hwmon/fscher.c
--- drivers/hwmon/fscher.c.alarms	2007-04-26 05:08:32.000000000 +0200
+++ drivers/hwmon/fscher.c	2007-07-03 13:14:07.000000000 +0200
@@ -192,7 +192,7 @@ static DEVICE_ATTR(kind##offset##sub, S_
 
 #define sysfs_ro(kind, sub, reg) \
 sysfs_r(kind, sub, 0, reg) \
-static DEVICE_ATTR(kind, S_IRUGO, show_##kind##0##sub, NULL);
+static DEVICE_ATTR(kind##sub, S_IRUGO, show_##kind##0##sub, NULL);
 
 #define sysfs_fan(offset, reg_status, reg_min, reg_ripple, reg_act) \
 sysfs_rw_n(pwm,        , offset, reg_min) \
@@ -202,6 +202,7 @@ sysfs_ro_n(fan, _input , offset, reg_act
 
 #define sysfs_temp(offset, reg_status, reg_act) \
 sysfs_rw_n(temp, _status, offset, reg_status) \
+sysfs_ro_n(temp, _fault , offset, reg_status) \
 sysfs_ro_n(temp, _input , offset, reg_act)
     
 #define sysfs_in(offset, reg_act) \
@@ -210,7 +211,13 @@ sysfs_ro_n(in, _input, offset, reg_act)
 #define sysfs_revision(reg_revision) \
 sysfs_ro(revision, , reg_revision)
 
+/* Only fan 1 and temp 1 have an alarm, so we add them here and not to
+   sysfs_fan / sysfs_temp */
 #define sysfs_alarms(reg_events) \
+sysfs_ro_n(fan, _alarm , 1, reg_events) \
+sysfs_ro_n(temp, _alarm , 1, reg_events) \
+sysfs_ro(control, _alarm, reg_events) \
+sysfs_ro(watchdog, _alarm, reg_events) \
 sysfs_ro(alarms, , reg_events)
 
 #define sysfs_control(reg_control) \
@@ -245,10 +252,12 @@ static struct attribute *fscher_attribut
 	&dev_attr_revision.attr,
 	&dev_attr_alarms.attr,
 	&dev_attr_control.attr,
+	&dev_attr_control_alarm.attr,
 
 	&dev_attr_watchdog_status.attr,
 	&dev_attr_watchdog_control.attr,
 	&dev_attr_watchdog_preset.attr,
+	&dev_attr_watchdog_alarm.attr,
 
 	&dev_attr_in0_input.attr,
 	&dev_attr_in1_input.attr,
@@ -257,6 +266,7 @@ static struct attribute *fscher_attribut
 	&dev_attr_fan1_status.attr,
 	&dev_attr_fan1_div.attr,
 	&dev_attr_fan1_input.attr,
+	&dev_attr_fan1_alarm.attr,
 	&dev_attr_pwm1.attr,
 	&dev_attr_fan2_status.attr,
 	&dev_attr_fan2_div.attr,
@@ -269,10 +279,14 @@ static struct attribute *fscher_attribut
 
 	&dev_attr_temp1_status.attr,
 	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_fault.attr,
+	&dev_attr_temp1_alarm.attr,
 	&dev_attr_temp2_status.attr,
 	&dev_attr_temp2_input.attr,
+	&dev_attr_temp2_fault.attr,
 	&dev_attr_temp3_status.attr,
 	&dev_attr_temp3_input.attr,
+	&dev_attr_temp3_fault.attr,
 	NULL
 };
 
@@ -531,6 +545,13 @@ static ssize_t show_fan_input (struct fs
 	return sprintf(buf, "%u\n", RPM_FROM_REG(data->fan_act[FAN_INDEX_FROM_NUM(nr)]));
 }
 
+static ssize_t show_fan_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x01)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
 
 
 #define TEMP_INDEX_FROM_NUM(nr)		((nr) - 1)
@@ -554,6 +575,15 @@ static ssize_t show_temp_status(struct f
 	return sprintf(buf, "%u\n", data->temp_status[TEMP_INDEX_FROM_NUM(nr)] & 0x03);
 }
 
+static ssize_t show_temp_fault(struct fscher_data *data, char *buf, int nr)
+{
+	/* bit 0 set means sensor working ok, so no fault! */
+	if (data->temp_status[TEMP_INDEX_FROM_NUM(nr)] & 0x01)
+		return sprintf(buf, "0\n");
+	else
+		return sprintf(buf, "1\n");
+}
+
 #define TEMP_FROM_REG(val)	(((val) - 128) * 1000)
 
 static ssize_t show_temp_input(struct fscher_data *data, char *buf, int nr)
@@ -561,6 +591,14 @@ static ssize_t show_temp_input(struct fs
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_act[TEMP_INDEX_FROM_NUM(nr)]));
 }
 
+static ssize_t show_temp_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x02)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
+
 /*
  * The final conversion is specified in sensors.conf, as it depends on
  * mainboard specific values. We export the registers contents as
@@ -611,6 +649,13 @@ static ssize_t show_control(struct fsche
 	return sprintf(buf, "%u\n", data->global_control & 0x01);
 }
 
+static ssize_t show_control_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x10)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
 
 
 static ssize_t set_watchdog_control(struct i2c_client *client, struct
@@ -670,6 +715,14 @@ static ssize_t show_watchdog_preset(stru
 	return sprintf(buf, "%u\n", data->watchdog[0]);
 }
 
+static ssize_t show_watchdog_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x08)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
+
 static int __init sensors_fscher_init(void)
 {
 	return i2c_add_driver(&fscher_driver);

[-- 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

[lm-sensors] PATCH: hwmon-fscher-individual-alarm-files-v2.patch

[Hans de Goede]

Hi all,

This is version 2 (non incremental) of the patch adding individual alarm and
fault files to the fscher driver. This driver uses the status registers for
the alarms instead of the global event register, allowing seperate alarms
for all sensors. Also it makes the _alarm files r/w, writing 0 to them
resets the alarm flag (this is not done automatically be the hardware once an
alarm condition is cleared).

Signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>

Regards,

Hans


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[lm-sensors] PATCH: hwmon-fscher-individual-alarm-files-v2.patch

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

<oops one more time now with attachment, ehhhh>

Hi all,

This is version 2 (non incremental) of the patch adding individual alarm and
fault files to the fscher driver. This driver uses the status registers for
the alarms instead of the global event register, allowing seperate alarms
for all sensors. Also it makes the _alarm files r/w, writing 0 to them
resets the alarm flag (this is not done automatically be the hardware once an
alarm condition is cleared).

Signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>

Regards,

Hans


[-- Attachment #2: hwmon-fscher-individual-alarm-files-v2.patch --]
[-- Type: text/x-patch, Size: 6272 bytes --]

This is version 2 (non incremental) of the patch adding individual alarm and
fault files to the fscher driver. This driver uses the status registers for
the alarms instead of the global event register, allowing seperate alarms
for all sensors. Also it makes the _alarm files r/w, writing 0 to them
resets the alarm flag (this is not done automatically be the hardware once an
alarm condition is cleared).

Signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>
diff -up linux-2.6.22-rc4/drivers/hwmon/fscher.c.alarms linux-2.6.22-rc4/drivers/hwmon/fscher.c
--- linux-2.6.22-rc4/drivers/hwmon/fscher.c.alarms	2007-04-26 05:08:32.000000000 +0200
+++ linux-2.6.22-rc4/drivers/hwmon/fscher.c	2007-07-04 16:11:33.000000000 +0200
@@ -192,16 +192,19 @@ static DEVICE_ATTR(kind##offset##sub, S_
 
 #define sysfs_ro(kind, sub, reg) \
 sysfs_r(kind, sub, 0, reg) \
-static DEVICE_ATTR(kind, S_IRUGO, show_##kind##0##sub, NULL);
+static DEVICE_ATTR(kind##sub, S_IRUGO, show_##kind##0##sub, NULL);
 
 #define sysfs_fan(offset, reg_status, reg_min, reg_ripple, reg_act) \
 sysfs_rw_n(pwm,        , offset, reg_min) \
 sysfs_rw_n(fan, _status, offset, reg_status) \
+sysfs_rw_n(fan, _alarm , offset, reg_status) \
 sysfs_rw_n(fan, _div   , offset, reg_ripple) \
 sysfs_ro_n(fan, _input , offset, reg_act)
 
 #define sysfs_temp(offset, reg_status, reg_act) \
 sysfs_rw_n(temp, _status, offset, reg_status) \
+sysfs_rw_n(temp, _alarm , offset, reg_status) \
+sysfs_ro_n(temp, _fault , offset, reg_status) \
 sysfs_ro_n(temp, _input , offset, reg_act)
     
 #define sysfs_in(offset, reg_act) \
@@ -211,6 +214,8 @@ sysfs_ro_n(in, _input, offset, reg_act)
 sysfs_ro(revision, , reg_revision)
 
 #define sysfs_alarms(reg_events) \
+sysfs_ro(control, _alarm, reg_events) \
+sysfs_ro(watchdog, _alarm, reg_events) \
 sysfs_ro(alarms, , reg_events)
 
 #define sysfs_control(reg_control) \
@@ -245,10 +250,12 @@ static struct attribute *fscher_attribut
 	&dev_attr_revision.attr,
 	&dev_attr_alarms.attr,
 	&dev_attr_control.attr,
+	&dev_attr_control_alarm.attr,
 
 	&dev_attr_watchdog_status.attr,
 	&dev_attr_watchdog_control.attr,
 	&dev_attr_watchdog_preset.attr,
+	&dev_attr_watchdog_alarm.attr,
 
 	&dev_attr_in0_input.attr,
 	&dev_attr_in1_input.attr,
@@ -257,22 +264,31 @@ static struct attribute *fscher_attribut
 	&dev_attr_fan1_status.attr,
 	&dev_attr_fan1_div.attr,
 	&dev_attr_fan1_input.attr,
+	&dev_attr_fan1_alarm.attr,
 	&dev_attr_pwm1.attr,
 	&dev_attr_fan2_status.attr,
 	&dev_attr_fan2_div.attr,
 	&dev_attr_fan2_input.attr,
+	&dev_attr_fan2_alarm.attr,
 	&dev_attr_pwm2.attr,
 	&dev_attr_fan3_status.attr,
 	&dev_attr_fan3_div.attr,
 	&dev_attr_fan3_input.attr,
+	&dev_attr_fan3_alarm.attr,
 	&dev_attr_pwm3.attr,
 
 	&dev_attr_temp1_status.attr,
 	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_fault.attr,
+	&dev_attr_temp1_alarm.attr,
 	&dev_attr_temp2_status.attr,
 	&dev_attr_temp2_input.attr,
+	&dev_attr_temp2_fault.attr,
+	&dev_attr_temp2_alarm.attr,
 	&dev_attr_temp3_status.attr,
 	&dev_attr_temp3_input.attr,
+	&dev_attr_temp3_fault.attr,
+	&dev_attr_temp3_alarm.attr,
 	NULL
 };
 
@@ -531,6 +547,27 @@ static ssize_t show_fan_input (struct fs
 	return sprintf(buf, "%u\n", RPM_FROM_REG(data->fan_act[FAN_INDEX_FROM_NUM(nr)]));
 }
 
+static ssize_t set_fan_alarm(struct i2c_client *client, struct fscher_data *data,
+			      const char *buf, size_t count, int nr, int reg)
+{
+	unsigned long v = simple_strtoul(buf, NULL, 10);
+
+	if (v == 0) { /* clear alarm? */
+		mutex_lock(&data->update_lock);
+		fscher_write_value(client, reg, 0x04);
+		data->fan_status[FAN_INDEX_FROM_NUM(nr)] &= ~0x04;
+		mutex_unlock(&data->update_lock);
+	}
+	return count;
+}
+
+static ssize_t show_fan_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->fan_status[FAN_INDEX_FROM_NUM(nr)] & 0x04)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
 
 
 #define TEMP_INDEX_FROM_NUM(nr)		((nr) - 1)
@@ -554,6 +591,15 @@ static ssize_t show_temp_status(struct f
 	return sprintf(buf, "%u\n", data->temp_status[TEMP_INDEX_FROM_NUM(nr)] & 0x03);
 }
 
+static ssize_t show_temp_fault(struct fscher_data *data, char *buf, int nr)
+{
+	/* bit 0 set means sensor working ok, so no fault! */
+	if (data->temp_status[TEMP_INDEX_FROM_NUM(nr)] & 0x01)
+		return sprintf(buf, "0\n");
+	else
+		return sprintf(buf, "1\n");
+}
+
 #define TEMP_FROM_REG(val)	(((val) - 128) * 1000)
 
 static ssize_t show_temp_input(struct fscher_data *data, char *buf, int nr)
@@ -561,6 +607,29 @@ static ssize_t show_temp_input(struct fs
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_act[TEMP_INDEX_FROM_NUM(nr)]));
 }
 
+static ssize_t set_temp_alarm(struct i2c_client *client, struct fscher_data *data,
+			       const char *buf, size_t count, int nr, int reg)
+{
+	unsigned long v = simple_strtoul(buf, NULL, 10);
+
+	if (v == 0) { /* clear alarm? */
+		mutex_lock(&data->update_lock);
+		fscher_write_value(client, reg, 0x02);
+		data->temp_status[TEMP_INDEX_FROM_NUM(nr)] &= ~0x02;
+		mutex_unlock(&data->update_lock);
+	}
+	return count;
+}
+
+static ssize_t show_temp_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	/* only signal an alarm if the sensor is working and alert == 1 */
+	if ((data->temp_status[TEMP_INDEX_FROM_NUM(nr)] & 0x03) == 0x03)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
+
 /*
  * The final conversion is specified in sensors.conf, as it depends on
  * mainboard specific values. We export the registers contents as
@@ -611,6 +680,13 @@ static ssize_t show_control(struct fsche
 	return sprintf(buf, "%u\n", data->global_control & 0x01);
 }
 
+static ssize_t show_control_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x10)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
 
 
 static ssize_t set_watchdog_control(struct i2c_client *client, struct
@@ -670,6 +746,14 @@ static ssize_t show_watchdog_preset(stru
 	return sprintf(buf, "%u\n", data->watchdog[0]);
 }
 
+static ssize_t show_watchdog_alarm (struct fscher_data *data, char *buf, int nr)
+{
+	if (data->global_event & 0x08)
+		return sprintf(buf, "1\n");
+	else
+		return sprintf(buf, "0\n");
+}
+
 static int __init sensors_fscher_init(void)
 {
 	return i2c_add_driver(&fscher_driver);

[-- 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

Re: [lm-sensors] PATCH: hwmon-fscher-individual-alarm-files-v3.patch

[Jean Delvare]

Hi Hans,

On Fri, 06 Jul 2007 17:17:41 +0200, Hans de Goede wrote:
> This is version 3 (non incremental) of the patch adding individual alarm and
> fault files to the fscher driver. This version automatically clears alarms
> when the condition for them is gone, and adds tempX_max sysfs atrributes.

Hmm, your patch actually does 4 different things:
* Add the individual alarm files.
* Add the tempX_max sysfs atrributes.
* Add (disabled) auto fan speed control trip points.
* Use arrays to reference register addresses.

That's a bit too much for a single patch. For easier review and faster
acceptance, you should split it into logical steps, each doing only one
thing. See what I did recently for the w83627ehf and lm85 drivers. 

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] PATCH: hwmon-fscher-individual-alarm-files-v3.patch

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Fri, 06 Jul 2007 17:17:41 +0200, Hans de Goede wrote:
>> This is version 3 (non incremental) of the patch adding individual alarm and
>> fault files to the fscher driver. This version automatically clears alarms
>> when the condition for them is gone, and adds tempX_max sysfs atrributes.
> 
> Hmm, your patch actually does 4 different things:
> * Add the individual alarm files.
> * Add the tempX_max sysfs atrributes.
> * Add (disabled) auto fan speed control trip points.
> * Use arrays to reference register addresses.
> 
> That's a bit too much for a single patch. For easier review and faster
> acceptance, you should split it into logical steps, each doing only one
> thing. See what I did recently for the w83627ehf and lm85 drivers. 
> 

Oh,

Come on, thats a bit over the top isn't it?

[hans@shalem ~]$ diffstat hwmon-fscher-individual-alarm-files-v3.patch
  fscher.c |  209 ++++++++++++++++++++++++++++++++++++++++++++-------------------
  1 file changed, 148 insertions(+), 61 deletions(-)

Thats hardly a big / complex patch. Besides that they are all intertwined:

 > * Add the individual alarm files.
As requested

 > * Add the tempX_max sysfs atrributes.
Reading tempX_max from the chip is needed to reset the alarms, as the chip 
doesn't do this itself. Exporting the userspace once read is only logical.

This is the result of you arguing (and me agreeing) that leaving resetting the 
alarms to userspace is very bad.

 > * Add (disabled) auto fan speed control trip points.
Thats just to document my findings while searching for the undocumented 
tempX_max registers, this is 6 lines in total, thats not worth a separate patch 
at all.

 > * Use arrays to reference register addresses.
Which is needed to implement the resetting of the fan and temp alarms sanely 
and thus is an integral part of the individual alarm files patch.

In the Netherlands we have a word for exercises like this its called 
"werkverschaffing" translated "work-providing". Please review the patch as is, 
I've got much better things todo then splitting up this absolutely reasonable 
sized patch (or arguing about splitting it up).

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors