* [PATCH] adt7470: Support per-sensor alarm files
@ 2007-12-19 4:01 Darrick J. Wong
2007-12-19 14:40 ` Jean Delvare
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2007-12-19 4:01 UTC (permalink / raw)
To: lm-sensors; +Cc: Mark M. Hoffman, linux-kernel
Remove the old alarms sysfs hack and replace it with per-sensor alarm files.
Also don't read the second alarm register if it's not needed.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
drivers/hwmon/adt7470.c | 97 ++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 86 insertions(+), 11 deletions(-)
diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index a215560..59ba7e5 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -48,7 +48,22 @@ I2C_CLIENT_INSMOD_1(adt7470);
#define ADT7470_REG_CFG 0x40
#define ADT7470_FSPD_MASK 0x04
#define ADT7470_REG_ALARM1 0x41
+#define ADT7470_R1T_ALARM 0x01
+#define ADT7470_R2T_ALARM 0x02
+#define ADT7470_R3T_ALARM 0x04
+#define ADT7470_R4T_ALARM 0x08
+#define ADT7470_R5T_ALARM 0x10
+#define ADT7470_R6T_ALARM 0x20
+#define ADT7470_R7T_ALARM 0x40
+#define ADT7470_OOL_ALARM 0x80
#define ADT7470_REG_ALARM2 0x42
+#define ADT7470_R8T_ALARM 0x01
+#define ADT7470_R9T_ALARM 0x02
+#define ADT7470_R10T_ALARM 0x04
+#define ADT7470_FAN1_ALARM 0x10
+#define ADT7470_FAN2_ALARM 0x20
+#define ADT7470_FAN3_ALARM 0x40
+#define ADT7470_FAN4_ALARM 0x80
#define ADT7470_REG_TEMP_LIMITS_BASE_ADDR 0x44
#define ADT7470_REG_TEMP_LIMITS_MAX_ADDR 0x57
#define ADT7470_REG_FAN_MIN_BASE_ADDR 0x58
@@ -97,6 +112,8 @@ I2C_CLIENT_INSMOD_1(adt7470);
#define ADT7470_REG_PWM_AUTO_TEMP(x) (ADT7470_REG_PWM_AUTO_TEMP_BASE_ADDR + \
((x) / 2))
+#define ALARM2(x) ((x) << 8)
+
#define ADT7470_VENDOR 0x41
#define ADT7470_DEVICE 0x70
/* datasheet only mentions a revision 2 */
@@ -136,7 +153,8 @@ struct adt7470_data {
u16 fan[ADT7470_FAN_COUNT];
u16 fan_min[ADT7470_FAN_COUNT];
u16 fan_max[ADT7470_FAN_COUNT];
- u16 alarms, alarms_mask;
+ u8 alarm1, alarm2;
+ u16 alarms_mask;
u8 force_pwm_max;
u8 pwm[ADT7470_PWM_COUNT];
u8 pwm_max[ADT7470_PWM_COUNT];
@@ -260,7 +278,12 @@ static struct adt7470_data *adt7470_update_device(struct device *dev)
else
data->force_pwm_max = 0;
- data->alarms = adt7470_read_word_data(client, ADT7470_REG_ALARM1);
+ data->alarm1 = i2c_smbus_read_byte_data(client, ADT7470_REG_ALARM1);
+ if (data->alarm2 & ADT7470_OOL_ALARM)
+ data->alarm2 = i2c_smbus_read_byte_data(client,
+ ADT7470_REG_ALARM2);
+ else
+ data->alarm2 = 0;
data->alarms_mask = adt7470_read_word_data(client,
ADT7470_REG_ALARM1_MASK);
@@ -368,17 +391,13 @@ static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
return sprintf(buf, "%d\n", 1000 * data->temp[attr->index]);
}
-static ssize_t show_alarms(struct device *dev,
+static ssize_t show_alarm_mask(struct device *dev,
struct device_attribute *devattr,
char *buf)
{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
struct adt7470_data *data = adt7470_update_device(dev);
- if (attr->index)
- return sprintf(buf, "%x\n", data->alarms);
- else
- return sprintf(buf, "%x\n", data->alarms_mask);
+ return sprintf(buf, "%x\n", data->alarms_mask);
}
static ssize_t show_fan_max(struct device *dev,
@@ -713,8 +732,21 @@ static ssize_t set_pwm_auto_temp(struct device *dev,
return count;
}
-static SENSOR_DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL, 0);
-static SENSOR_DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarms, NULL, 1);
+static ssize_t show_alarm(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ struct adt7470_data *data = adt7470_update_device(dev);
+
+ if (data->alarm1 & (attr->index & 0xFF) ||
+ data->alarm2 & (attr->index >> 8))
+ return sprintf(buf, "1\n");
+ else
+ return sprintf(buf, "0\n");
+}
+
+static SENSOR_DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarm_mask, NULL, 0);
static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
set_temp_max, 0);
@@ -769,6 +801,27 @@ static SENSOR_DEVICE_ATTR(temp8_input, S_IRUGO, show_temp, NULL, 7);
static SENSOR_DEVICE_ATTR(temp9_input, S_IRUGO, show_temp, NULL, 8);
static SENSOR_DEVICE_ATTR(temp10_input, S_IRUGO, show_temp, NULL, 9);
+static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R1T_ALARM);
+static SENSOR_DEVICE_ATTR(temp2_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R2T_ALARM);
+static SENSOR_DEVICE_ATTR(temp3_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R3T_ALARM);
+static SENSOR_DEVICE_ATTR(temp4_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R4T_ALARM);
+static SENSOR_DEVICE_ATTR(temp5_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R5T_ALARM);
+static SENSOR_DEVICE_ATTR(temp6_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R6T_ALARM);
+static SENSOR_DEVICE_ATTR(temp7_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R7T_ALARM);
+static SENSOR_DEVICE_ATTR(temp8_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R8T_ALARM);
+static SENSOR_DEVICE_ATTR(temp9_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R9T_ALARM);
+static SENSOR_DEVICE_ATTR(temp10_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R10T_ALARM);
+
static SENSOR_DEVICE_ATTR(fan1_max, S_IWUSR | S_IRUGO, show_fan_max,
set_fan_max, 0);
static SENSOR_DEVICE_ATTR(fan2_max, S_IWUSR | S_IRUGO, show_fan_max,
@@ -792,6 +845,15 @@ static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
static SENSOR_DEVICE_ATTR(fan4_input, S_IRUGO, show_fan, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN1_ALARM));
+static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN2_ALARM));
+static SENSOR_DEVICE_ATTR(fan3_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN3_ALARM));
+static SENSOR_DEVICE_ATTR(fan4_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN4_ALARM));
+
static SENSOR_DEVICE_ATTR(force_pwm_max, S_IWUSR | S_IRUGO,
show_force_pwm_max, set_force_pwm_max, 0);
@@ -856,7 +918,6 @@ static SENSOR_DEVICE_ATTR(pwm4_auto_channels_temp, S_IWUSR | S_IRUGO,
static struct attribute *adt7470_attr[] =
{
- &sensor_dev_attr_alarms.dev_attr.attr,
&sensor_dev_attr_alarm_mask.dev_attr.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -888,6 +949,16 @@ static struct attribute *adt7470_attr[] =
&sensor_dev_attr_temp8_input.dev_attr.attr,
&sensor_dev_attr_temp9_input.dev_attr.attr,
&sensor_dev_attr_temp10_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp2_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp3_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp4_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp5_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp6_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp7_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp8_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp9_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp10_alarm.dev_attr.attr,
&sensor_dev_attr_fan1_max.dev_attr.attr,
&sensor_dev_attr_fan2_max.dev_attr.attr,
&sensor_dev_attr_fan3_max.dev_attr.attr,
@@ -900,6 +971,10 @@ static struct attribute *adt7470_attr[] =
&sensor_dev_attr_fan2_input.dev_attr.attr,
&sensor_dev_attr_fan3_input.dev_attr.attr,
&sensor_dev_attr_fan4_input.dev_attr.attr,
+ &sensor_dev_attr_fan1_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan2_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan3_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan4_alarm.dev_attr.attr,
&sensor_dev_attr_force_pwm_max.dev_attr.attr,
&sensor_dev_attr_pwm1.dev_attr.attr,
&sensor_dev_attr_pwm2.dev_attr.attr,
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] adt7470: Support per-sensor alarm files
2007-12-19 4:01 [PATCH] adt7470: Support per-sensor alarm files Darrick J. Wong
@ 2007-12-19 14:40 ` Jean Delvare
2007-12-19 22:11 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Jean Delvare @ 2007-12-19 14:40 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: lm-sensors, Mark M. Hoffman, linux-kernel
Hi Darrick,
On Tue, 18 Dec 2007 20:01:23 -0800, Darrick J. Wong wrote:
> Remove the old alarms sysfs hack and replace it with per-sensor alarm files.
> Also don't read the second alarm register if it's not needed.
Thanks for doing this, I appreciate it. There are so many drivers left
to convert...
In general we keep the all-in-one alarms file for compatibility, but
given that this driver is fairly new and libsensors never had specific
support for it anyway, it's probably OK to drop it this time.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
>
> drivers/hwmon/adt7470.c | 97 ++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 86 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index a215560..59ba7e5 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -48,7 +48,22 @@ I2C_CLIENT_INSMOD_1(adt7470);
> #define ADT7470_REG_CFG 0x40
> #define ADT7470_FSPD_MASK 0x04
> #define ADT7470_REG_ALARM1 0x41
> +#define ADT7470_R1T_ALARM 0x01
> +#define ADT7470_R2T_ALARM 0x02
> +#define ADT7470_R3T_ALARM 0x04
> +#define ADT7470_R4T_ALARM 0x08
> +#define ADT7470_R5T_ALARM 0x10
> +#define ADT7470_R6T_ALARM 0x20
> +#define ADT7470_R7T_ALARM 0x40
> +#define ADT7470_OOL_ALARM 0x80
> #define ADT7470_REG_ALARM2 0x42
> +#define ADT7470_R8T_ALARM 0x01
> +#define ADT7470_R9T_ALARM 0x02
> +#define ADT7470_R10T_ALARM 0x04
> +#define ADT7470_FAN1_ALARM 0x10
> +#define ADT7470_FAN2_ALARM 0x20
> +#define ADT7470_FAN3_ALARM 0x40
> +#define ADT7470_FAN4_ALARM 0x80
> #define ADT7470_REG_TEMP_LIMITS_BASE_ADDR 0x44
> #define ADT7470_REG_TEMP_LIMITS_MAX_ADDR 0x57
> #define ADT7470_REG_FAN_MIN_BASE_ADDR 0x58
> @@ -97,6 +112,8 @@ I2C_CLIENT_INSMOD_1(adt7470);
> #define ADT7470_REG_PWM_AUTO_TEMP(x) (ADT7470_REG_PWM_AUTO_TEMP_BASE_ADDR + \
> ((x) / 2))
>
> +#define ALARM2(x) ((x) << 8)
> +
> #define ADT7470_VENDOR 0x41
> #define ADT7470_DEVICE 0x70
> /* datasheet only mentions a revision 2 */
> @@ -136,7 +153,8 @@ struct adt7470_data {
> u16 fan[ADT7470_FAN_COUNT];
> u16 fan_min[ADT7470_FAN_COUNT];
> u16 fan_max[ADT7470_FAN_COUNT];
> - u16 alarms, alarms_mask;
> + u8 alarm1, alarm2;
> + u16 alarms_mask;
I don't think that you win anything by splitting the alarm field. In
fact it makes your code more complex than it needs to be. When
converting the other drivers, I've always kept a single field for the
alarms.
> u8 force_pwm_max;
> u8 pwm[ADT7470_PWM_COUNT];
> u8 pwm_max[ADT7470_PWM_COUNT];
> @@ -260,7 +278,12 @@ static struct adt7470_data *adt7470_update_device(struct device *dev)
> else
> data->force_pwm_max = 0;
>
> - data->alarms = adt7470_read_word_data(client, ADT7470_REG_ALARM1);
> + data->alarm1 = i2c_smbus_read_byte_data(client, ADT7470_REG_ALARM1);
> + if (data->alarm2 & ADT7470_OOL_ALARM)
You certainly mean data->alarm1.
> + data->alarm2 = i2c_smbus_read_byte_data(client,
> + ADT7470_REG_ALARM2);
> + else
> + data->alarm2 = 0;
> data->alarms_mask = adt7470_read_word_data(client,
> ADT7470_REG_ALARM1_MASK);
>
> @@ -368,17 +391,13 @@ static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
> return sprintf(buf, "%d\n", 1000 * data->temp[attr->index]);
> }
>
> -static ssize_t show_alarms(struct device *dev,
> +static ssize_t show_alarm_mask(struct device *dev,
> struct device_attribute *devattr,
> char *buf)
> {
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> struct adt7470_data *data = adt7470_update_device(dev);
>
> - if (attr->index)
> - return sprintf(buf, "%x\n", data->alarms);
> - else
> - return sprintf(buf, "%x\n", data->alarms_mask);
> + return sprintf(buf, "%x\n", data->alarms_mask);
> }
>
> static ssize_t show_fan_max(struct device *dev,
> @@ -713,8 +732,21 @@ static ssize_t set_pwm_auto_temp(struct device *dev,
> return count;
> }
>
> -static SENSOR_DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL, 0);
> -static SENSOR_DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarms, NULL, 1);
> +static ssize_t show_alarm(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + struct adt7470_data *data = adt7470_update_device(dev);
> +
> + if (data->alarm1 & (attr->index & 0xFF) ||
> + data->alarm2 & (attr->index >> 8))
That's a complex way to write it... It would be more simple with a
16-bit data->alarm field. Please see the lm83 driver for an example.
> + return sprintf(buf, "1\n");
> + else
> + return sprintf(buf, "0\n");
> +}
> +
> +static SENSOR_DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarm_mask, NULL, 0);
Note that this could become a simple DEVICE_ATTR now.
>
> static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
> set_temp_max, 0);
> @@ -769,6 +801,27 @@ static SENSOR_DEVICE_ATTR(temp8_input, S_IRUGO, show_temp, NULL, 7);
> static SENSOR_DEVICE_ATTR(temp9_input, S_IRUGO, show_temp, NULL, 8);
> static SENSOR_DEVICE_ATTR(temp10_input, S_IRUGO, show_temp, NULL, 9);
>
> +static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R1T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp2_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R2T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp3_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R3T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp4_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R4T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp5_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R5T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp6_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R6T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp7_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R7T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp8_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R8T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp9_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R9T_ALARM);
> +static SENSOR_DEVICE_ATTR(temp10_alarm, S_IRUGO, show_alarm, NULL,
> + ADT7470_R10T_ALARM);
Missing ALARM2() for temp8, temp9 and temp10.
> +
> static SENSOR_DEVICE_ATTR(fan1_max, S_IWUSR | S_IRUGO, show_fan_max,
> set_fan_max, 0);
> static SENSOR_DEVICE_ATTR(fan2_max, S_IWUSR | S_IRUGO, show_fan_max,
> @@ -792,6 +845,15 @@ static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
> static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
> static SENSOR_DEVICE_ATTR(fan4_input, S_IRUGO, show_fan, NULL, 3);
>
> +static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL,
> + ALARM2(ADT7470_FAN1_ALARM));
> +static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL,
> + ALARM2(ADT7470_FAN2_ALARM));
> +static SENSOR_DEVICE_ATTR(fan3_alarm, S_IRUGO, show_alarm, NULL,
> + ALARM2(ADT7470_FAN3_ALARM));
> +static SENSOR_DEVICE_ATTR(fan4_alarm, S_IRUGO, show_alarm, NULL,
> + ALARM2(ADT7470_FAN4_ALARM));
> +
> static SENSOR_DEVICE_ATTR(force_pwm_max, S_IWUSR | S_IRUGO,
> show_force_pwm_max, set_force_pwm_max, 0);
>
> @@ -856,7 +918,6 @@ static SENSOR_DEVICE_ATTR(pwm4_auto_channels_temp, S_IWUSR | S_IRUGO,
>
> static struct attribute *adt7470_attr[] =
> {
> - &sensor_dev_attr_alarms.dev_attr.attr,
> &sensor_dev_attr_alarm_mask.dev_attr.attr,
> &sensor_dev_attr_temp1_max.dev_attr.attr,
> &sensor_dev_attr_temp2_max.dev_attr.attr,
> @@ -888,6 +949,16 @@ static struct attribute *adt7470_attr[] =
> &sensor_dev_attr_temp8_input.dev_attr.attr,
> &sensor_dev_attr_temp9_input.dev_attr.attr,
> &sensor_dev_attr_temp10_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp4_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp5_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp6_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp7_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp8_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp9_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp10_alarm.dev_attr.attr,
> &sensor_dev_attr_fan1_max.dev_attr.attr,
> &sensor_dev_attr_fan2_max.dev_attr.attr,
> &sensor_dev_attr_fan3_max.dev_attr.attr,
> @@ -900,6 +971,10 @@ static struct attribute *adt7470_attr[] =
> &sensor_dev_attr_fan2_input.dev_attr.attr,
> &sensor_dev_attr_fan3_input.dev_attr.attr,
> &sensor_dev_attr_fan4_input.dev_attr.attr,
> + &sensor_dev_attr_fan1_alarm.dev_attr.attr,
> + &sensor_dev_attr_fan2_alarm.dev_attr.attr,
> + &sensor_dev_attr_fan3_alarm.dev_attr.attr,
> + &sensor_dev_attr_fan4_alarm.dev_attr.attr,
> &sensor_dev_attr_force_pwm_max.dev_attr.attr,
> &sensor_dev_attr_pwm1.dev_attr.attr,
> &sensor_dev_attr_pwm2.dev_attr.attr,
--
Jean Delvare
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] adt7470: Support per-sensor alarm files
2007-12-19 14:40 ` Jean Delvare
@ 2007-12-19 22:11 ` Darrick J. Wong
2007-12-20 9:58 ` Jean Delvare
2008-01-27 14:39 ` [lm-sensors] " Mark M. Hoffman
0 siblings, 2 replies; 7+ messages in thread
From: Darrick J. Wong @ 2007-12-19 22:11 UTC (permalink / raw)
To: Jean Delvare; +Cc: lm-sensors, Mark M. Hoffman, linux-kernel
On Wed, Dec 19, 2007 at 03:40:12PM +0100, Jean Delvare wrote:
> In general we keep the all-in-one alarms file for compatibility, but
> given that this driver is fairly new and libsensors never had specific
> support for it anyway, it's probably OK to drop it this time.
Thanks for the code review. I've made the changes you asked for and
here's a new patch to supersede yesterday's.
---
Remove the old alarms hack and replace it with per-sensor alarm files.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
drivers/hwmon/adt7470.c | 96 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index a215560..747693a 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -48,7 +48,22 @@ I2C_CLIENT_INSMOD_1(adt7470);
#define ADT7470_REG_CFG 0x40
#define ADT7470_FSPD_MASK 0x04
#define ADT7470_REG_ALARM1 0x41
+#define ADT7470_R1T_ALARM 0x01
+#define ADT7470_R2T_ALARM 0x02
+#define ADT7470_R3T_ALARM 0x04
+#define ADT7470_R4T_ALARM 0x08
+#define ADT7470_R5T_ALARM 0x10
+#define ADT7470_R6T_ALARM 0x20
+#define ADT7470_R7T_ALARM 0x40
+#define ADT7470_OOL_ALARM 0x80
#define ADT7470_REG_ALARM2 0x42
+#define ADT7470_R8T_ALARM 0x01
+#define ADT7470_R9T_ALARM 0x02
+#define ADT7470_R10T_ALARM 0x04
+#define ADT7470_FAN1_ALARM 0x10
+#define ADT7470_FAN2_ALARM 0x20
+#define ADT7470_FAN3_ALARM 0x40
+#define ADT7470_FAN4_ALARM 0x80
#define ADT7470_REG_TEMP_LIMITS_BASE_ADDR 0x44
#define ADT7470_REG_TEMP_LIMITS_MAX_ADDR 0x57
#define ADT7470_REG_FAN_MIN_BASE_ADDR 0x58
@@ -97,6 +112,8 @@ I2C_CLIENT_INSMOD_1(adt7470);
#define ADT7470_REG_PWM_AUTO_TEMP(x) (ADT7470_REG_PWM_AUTO_TEMP_BASE_ADDR + \
((x) / 2))
+#define ALARM2(x) ((x) << 8)
+
#define ADT7470_VENDOR 0x41
#define ADT7470_DEVICE 0x70
/* datasheet only mentions a revision 2 */
@@ -136,7 +153,8 @@ struct adt7470_data {
u16 fan[ADT7470_FAN_COUNT];
u16 fan_min[ADT7470_FAN_COUNT];
u16 fan_max[ADT7470_FAN_COUNT];
- u16 alarms, alarms_mask;
+ u16 alarm;
+ u16 alarms_mask;
u8 force_pwm_max;
u8 pwm[ADT7470_PWM_COUNT];
u8 pwm_max[ADT7470_PWM_COUNT];
@@ -260,7 +278,10 @@ static struct adt7470_data *adt7470_update_device(struct device *dev)
else
data->force_pwm_max = 0;
- data->alarms = adt7470_read_word_data(client, ADT7470_REG_ALARM1);
+ data->alarm = i2c_smbus_read_byte_data(client, ADT7470_REG_ALARM1);
+ if (data->alarm & ADT7470_OOL_ALARM)
+ data->alarm |= ALARM2(i2c_smbus_read_byte_data(client,
+ ADT7470_REG_ALARM2));
data->alarms_mask = adt7470_read_word_data(client,
ADT7470_REG_ALARM1_MASK);
@@ -368,17 +389,13 @@ static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
return sprintf(buf, "%d\n", 1000 * data->temp[attr->index]);
}
-static ssize_t show_alarms(struct device *dev,
+static ssize_t show_alarm_mask(struct device *dev,
struct device_attribute *devattr,
char *buf)
{
- struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
struct adt7470_data *data = adt7470_update_device(dev);
- if (attr->index)
- return sprintf(buf, "%x\n", data->alarms);
- else
- return sprintf(buf, "%x\n", data->alarms_mask);
+ return sprintf(buf, "%x\n", data->alarms_mask);
}
static ssize_t show_fan_max(struct device *dev,
@@ -713,8 +730,20 @@ static ssize_t set_pwm_auto_temp(struct device *dev,
return count;
}
-static SENSOR_DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL, 0);
-static SENSOR_DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarms, NULL, 1);
+static ssize_t show_alarm(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ struct adt7470_data *data = adt7470_update_device(dev);
+
+ if (data->alarm & attr->index)
+ return sprintf(buf, "1\n");
+ else
+ return sprintf(buf, "0\n");
+}
+
+static DEVICE_ATTR(alarm_mask, S_IRUGO, show_alarm_mask, NULL);
static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
set_temp_max, 0);
@@ -769,6 +798,27 @@ static SENSOR_DEVICE_ATTR(temp8_input, S_IRUGO, show_temp, NULL, 7);
static SENSOR_DEVICE_ATTR(temp9_input, S_IRUGO, show_temp, NULL, 8);
static SENSOR_DEVICE_ATTR(temp10_input, S_IRUGO, show_temp, NULL, 9);
+static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R1T_ALARM);
+static SENSOR_DEVICE_ATTR(temp2_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R2T_ALARM);
+static SENSOR_DEVICE_ATTR(temp3_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R3T_ALARM);
+static SENSOR_DEVICE_ATTR(temp4_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R4T_ALARM);
+static SENSOR_DEVICE_ATTR(temp5_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R5T_ALARM);
+static SENSOR_DEVICE_ATTR(temp6_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R6T_ALARM);
+static SENSOR_DEVICE_ATTR(temp7_alarm, S_IRUGO, show_alarm, NULL,
+ ADT7470_R7T_ALARM);
+static SENSOR_DEVICE_ATTR(temp8_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_R8T_ALARM));
+static SENSOR_DEVICE_ATTR(temp9_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_R9T_ALARM));
+static SENSOR_DEVICE_ATTR(temp10_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_R10T_ALARM));
+
static SENSOR_DEVICE_ATTR(fan1_max, S_IWUSR | S_IRUGO, show_fan_max,
set_fan_max, 0);
static SENSOR_DEVICE_ATTR(fan2_max, S_IWUSR | S_IRUGO, show_fan_max,
@@ -792,6 +842,15 @@ static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan, NULL, 1);
static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, show_fan, NULL, 2);
static SENSOR_DEVICE_ATTR(fan4_input, S_IRUGO, show_fan, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan1_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN1_ALARM));
+static SENSOR_DEVICE_ATTR(fan2_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN2_ALARM));
+static SENSOR_DEVICE_ATTR(fan3_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN3_ALARM));
+static SENSOR_DEVICE_ATTR(fan4_alarm, S_IRUGO, show_alarm, NULL,
+ ALARM2(ADT7470_FAN4_ALARM));
+
static SENSOR_DEVICE_ATTR(force_pwm_max, S_IWUSR | S_IRUGO,
show_force_pwm_max, set_force_pwm_max, 0);
@@ -856,8 +915,7 @@ static SENSOR_DEVICE_ATTR(pwm4_auto_channels_temp, S_IWUSR | S_IRUGO,
static struct attribute *adt7470_attr[] =
{
- &sensor_dev_attr_alarms.dev_attr.attr,
- &sensor_dev_attr_alarm_mask.dev_attr.attr,
+ &dev_attr_alarm_mask.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
&sensor_dev_attr_temp3_max.dev_attr.attr,
@@ -888,6 +946,16 @@ static struct attribute *adt7470_attr[] =
&sensor_dev_attr_temp8_input.dev_attr.attr,
&sensor_dev_attr_temp9_input.dev_attr.attr,
&sensor_dev_attr_temp10_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp2_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp3_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp4_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp5_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp6_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp7_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp8_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp9_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp10_alarm.dev_attr.attr,
&sensor_dev_attr_fan1_max.dev_attr.attr,
&sensor_dev_attr_fan2_max.dev_attr.attr,
&sensor_dev_attr_fan3_max.dev_attr.attr,
@@ -900,6 +968,10 @@ static struct attribute *adt7470_attr[] =
&sensor_dev_attr_fan2_input.dev_attr.attr,
&sensor_dev_attr_fan3_input.dev_attr.attr,
&sensor_dev_attr_fan4_input.dev_attr.attr,
+ &sensor_dev_attr_fan1_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan2_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan3_alarm.dev_attr.attr,
+ &sensor_dev_attr_fan4_alarm.dev_attr.attr,
&sensor_dev_attr_force_pwm_max.dev_attr.attr,
&sensor_dev_attr_pwm1.dev_attr.attr,
&sensor_dev_attr_pwm2.dev_attr.attr,
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] adt7470: Support per-sensor alarm files
2007-12-19 22:11 ` Darrick J. Wong
@ 2007-12-20 9:58 ` Jean Delvare
2007-12-20 18:57 ` Darrick J. Wong
2008-01-27 14:39 ` [lm-sensors] " Mark M. Hoffman
1 sibling, 1 reply; 7+ messages in thread
From: Jean Delvare @ 2007-12-20 9:58 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: lm-sensors, Mark M. Hoffman, linux-kernel
Hi Darrick,
On Wed, 19 Dec 2007 14:11:25 -0800, Darrick J. Wong wrote:
> On Wed, Dec 19, 2007 at 03:40:12PM +0100, Jean Delvare wrote:
> > In general we keep the all-in-one alarms file for compatibility, but
> > given that this driver is fairly new and libsensors never had specific
> > support for it anyway, it's probably OK to drop it this time.
>
> Thanks for the code review. I've made the changes you asked for and
> here's a new patch to supersede yesterday's.
> ---
> Remove the old alarms hack and replace it with per-sensor alarm files.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
Looks alright to me this time.
Acked-by: Jean Delvare <khali@linux-fr.org>
BTW, did you try your driver with lm-sensors 3.0.0?
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] adt7470: Support per-sensor alarm files
2007-12-20 9:58 ` Jean Delvare
@ 2007-12-20 18:57 ` Darrick J. Wong
2007-12-22 11:34 ` Jean Delvare
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2007-12-20 18:57 UTC (permalink / raw)
To: Jean Delvare; +Cc: lm-sensors, Mark M. Hoffman, linux-kernel
On Thu, Dec 20, 2007 at 10:58:08AM +0100, Jean Delvare wrote:
> BTW, did you try your driver with lm-sensors 3.0.0?
Yes I did, and it looked ok to me. Did you find something wrong?
--D
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] adt7470: Support per-sensor alarm files
2007-12-20 18:57 ` Darrick J. Wong
@ 2007-12-22 11:34 ` Jean Delvare
0 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2007-12-22 11:34 UTC (permalink / raw)
To: djwong; +Cc: lm-sensors, Mark M. Hoffman, linux-kernel
Hi Darrick,
Le 20/12/2007, Darrick J. Wong écrit:
>On Thu, Dec 20, 2007 at 10:58:08AM +0100, Jean Delvare wrote:
>> BTW, did you try your driver with lm-sensors 3.0.0?
>
>Yes I did, and it looked ok to me. Did you find something wrong?
No, I just wanted to make sure that you had tested it. Now that
libsensors relies on drivers implementing the standard sysfs interface,
it is a convenient way to verify that you implemented it properly.
--
Jean Delvare
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [lm-sensors] [PATCH] adt7470: Support per-sensor alarm files
2007-12-19 22:11 ` Darrick J. Wong
2007-12-20 9:58 ` Jean Delvare
@ 2008-01-27 14:39 ` Mark M. Hoffman
1 sibling, 0 replies; 7+ messages in thread
From: Mark M. Hoffman @ 2008-01-27 14:39 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Jean Delvare, linux-kernel, lm-sensors
Hi Darrick:
* Darrick J. Wong <djwong@us.ibm.com> [2007-12-19 14:11:25 -0800]:
> On Wed, Dec 19, 2007 at 03:40:12PM +0100, Jean Delvare wrote:
> > In general we keep the all-in-one alarms file for compatibility, but
> > given that this driver is fairly new and libsensors never had specific
> > support for it anyway, it's probably OK to drop it this time.
>
> Thanks for the code review. I've made the changes you asked for and
> here's a new patch to supersede yesterday's.
> ---
> Remove the old alarms hack and replace it with per-sensor alarm files.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
>
> drivers/hwmon/adt7470.c | 96 +++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 84 insertions(+), 12 deletions(-)
Applied to hwmon-2.6.git/testing, thanks.
--
Mark M. Hoffman
mhoffman@lightlink.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-01-27 14:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-19 4:01 [PATCH] adt7470: Support per-sensor alarm files Darrick J. Wong
2007-12-19 14:40 ` Jean Delvare
2007-12-19 22:11 ` Darrick J. Wong
2007-12-20 9:58 ` Jean Delvare
2007-12-20 18:57 ` Darrick J. Wong
2007-12-22 11:34 ` Jean Delvare
2008-01-27 14:39 ` [lm-sensors] " Mark M. Hoffman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).