[lm-sensors] [PATCH v5] hwmon: (max31722) Add threshold alarm support

Discussion:

Tiberiu Breana

2016-04-08 09:12:43 UTC

Add threshold alarm support for the max31722 temperature sensor driver.

Signed-off-by: Tiberiu Breana <***@intel.com>
---
Changes from v4:
- addressed Guenter's comments

As specified in the v3 change log, threshold values, as well as the alarm
attribute, are only visible if interrupts are enabled.
---
drivers/hwmon/max31722.c | 181 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 177 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c
index 30a100e..f557c36 100644
--- a/drivers/hwmon/max31722.c
+++ b/drivers/hwmon/max31722.c
@@ -12,23 +12,38 @@
#include <linux/acpi.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/spi/spi.h>

#define MAX31722_REG_CFG 0x00
#define MAX31722_REG_TEMP_LSB 0x01
+#define MAX31722_REG_THIGH_LSB 0x03
+#define MAX31722_REG_TLOW_LSB 0x05

#define MAX31722_MODE_CONTINUOUS 0x00
#define MAX31722_MODE_STANDBY 0x01
#define MAX31722_MODE_MASK 0xFE
#define MAX31722_RESOLUTION_12BIT 0x06
+#define MAX31722_TM_INTERRUPT 0x08
#define MAX31722_WRITE_MASK 0x80
+/*
+ * Minimum temperature value: -2^(12-1) * 62.5 = -128,000
+ * Maximum temperature value: (2^(12-1) - 1) * 62.5 ~= 127,937
+ */
+#define MAX31722_MIN_TEMP -128000
+#define MAX31722_MAX_TEMP 127937

struct max31722_data {
struct device *hwmon_dev;
struct spi_device *spi_device;
+ struct mutex update_lock;
u8 mode;
+ s32 thigh;
+ s32 tlow;
+ bool irq_enabled;
+ bool alarm_active;
};

static int max31722_set_mode(struct max31722_data *data, u8 mode)
@@ -56,6 +71,12 @@ static ssize_t max31722_show_temp(struct device *dev,
{
ssize_t ret;
struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr = to_sensor_dev_attr(attr);
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ return sprintf(buf, "%d\n", data->thigh);
+ else if (dev_attr->index == MAX31722_REG_TLOW_LSB)
+ return sprintf(buf, "%d\n", data->tlow);

ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
if (ret < 0)
@@ -64,15 +85,145 @@ static ssize_t max31722_show_temp(struct device *dev,
return sprintf(buf, "%d\n", (s16)le16_to_cpu(ret) * 125 / 32);
}

-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
- max31722_show_temp, NULL, 0);
+static ssize_t max31722_set_temp(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ long val;
+ s16 thresh;
+ u8 tx_buf[3];
+ struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr = to_sensor_dev_attr(attr);
+
+ ret = kstrtol(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ val = clamp_val(val, MAX31722_MIN_TEMP, MAX31722_MAX_TEMP);
+ thresh = cpu_to_le16(DIV_ROUND_CLOSEST(val * 32, 125));
+
+ tx_buf[0] = dev_attr->index | MAX31722_WRITE_MASK;
+ tx_buf[1] = thresh & 0xff;
+ tx_buf[2] = thresh >> 8;
+
+ mutex_lock(&data->update_lock);
+
+ ret = spi_write(data->spi_device, &tx_buf, sizeof(tx_buf));
+ if (ret < 0) {
+ mutex_unlock(&data->update_lock);
+ return ret;
+ }
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ data->thigh = thresh * 125 / 32;
+ else
+ data->tlow = thresh * 125 / 32;
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t max31722_show_alarm(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_device *spi_device = to_spi_device(dev);
+ struct max31722_data *data = spi_get_drvdata(spi_device);
+
+ return sprintf(buf, "%d\n", data->alarm_active);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, max31722_show_temp, NULL,
+ MAX31722_REG_TEMP_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, max31722_show_temp,
+ max31722_set_temp, MAX31722_REG_THIGH_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, max31722_show_temp,
+ max31722_set_temp, MAX31722_REG_TLOW_LSB);
+static DEVICE_ATTR(temp1_alarm, S_IRUGO, max31722_show_alarm, NULL);

static struct attribute *max31722_attrs[] = {
&sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
+ &dev_attr_temp1_alarm.attr,
NULL,
};

-ATTRIBUTE_GROUPS(max31722);
+static umode_t max31722_is_visible(struct kobject *kobj, struct attribute *attr,
+ int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct max31722_data *data = dev_get_drvdata(dev);
+
+ /* Hide alarm and threshold attributes if interrupts are disabled. */
+ if (!data->irq_enabled && index > 0)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group max31722_group = {
+ .attrs = max31722_attrs, .is_visible = max31722_is_visible,
+};
+
+__ATTRIBUTE_GROUPS(max31722);
+
+static ssize_t max31722_update_alarm(struct max31722_data *data)
+{
+ s16 temp;
+ ssize_t ret;
+ /*
+ * Do a quick temperature reading and compare it with TLOW/THIGH
+ * so we can tell which threshold has been met.
+ */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
+ if (ret < 0)
+ return ret;
+ temp = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ if (temp > data->thigh || (!data->alarm_active && temp > data->tlow))
+ data->alarm_active = true;
+ else if (temp <= data->tlow ||
+ (data->alarm_active && temp < data->thigh))
+ data->alarm_active = false;
+
+ return 0;
+}
+
+static irqreturn_t max31722_irq_handler(int irq, void *private)
+{
+ struct max31722_data *data = private;
+
+ max31722_update_alarm(data);
+ sysfs_notify(&data->spi_device->dev.kobj, NULL, "temp1_alarm");
+
+ return IRQ_HANDLED;
+}
+
+static ssize_t max31722_init_thresh(struct max31722_data *data)
+{
+ ssize_t ret;
+
+ /* Cache threshold values. */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TLOW_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->tlow = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_THIGH_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->thigh = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ return 0;
+
+exit_err:
+ dev_err(&data->spi_device->dev,
+ "failed to read temperature threshold\n");
+ return ret;
+}

static int max31722_probe(struct spi_device *spi)
{
@@ -83,17 +234,39 @@ static int max31722_probe(struct spi_device *spi)
if (!data)
return -ENOMEM;

+ mutex_init(&data->update_lock);
spi_set_drvdata(spi, data);
data->spi_device = spi;
/*
* Set SD bit to 0 so we can have continuous measurements.
* Set resolution to 12 bits for maximum precision.
*/
- data->mode = MAX31722_MODE_CONTINUOUS | MAX31722_RESOLUTION_12BIT;
+ data->mode = MAX31722_MODE_CONTINUOUS | MAX31722_RESOLUTION_12BIT
+ | MAX31722_TM_INTERRUPT;
ret = max31722_set_mode(data, MAX31722_MODE_CONTINUOUS);
if (ret < 0)
return ret;

+ if (spi->irq > 0) {
+ ret = devm_request_threaded_irq(&spi->dev, spi->irq,
+ NULL,
+ max31722_irq_handler,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ dev_name(&spi->dev), data);
+ if (ret < 0) {
+ dev_warn(&spi->dev, "request irq %d failed\n",
+ spi->irq);
+ } else {
+ data->irq_enabled = true;
+ ret = max31722_init_thresh(data);
+ if (ret < 0)
+ return ret;
+ ret = max31722_update_alarm(data);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
data->hwmon_dev = hwmon_device_register_with_groups(&spi->dev,
spi->modalias,
data,

--
1.9.1

Guenter Roeck

2016-04-08 12:24:04 UTC

Permalink

Post by Tiberiu Breana
Add threshold alarm support for the max31722 temperature sensor driver.
---
- addressed Guenter's comments
As specified in the v3 change log, threshold values, as well as the alarm
attribute, are only visible if interrupts are enabled.

I missed that part. Why ? The thresholds are still supported
by the chip, even without interrupts.

Thanks,
Guenter

Post by Tiberiu Breana
---
drivers/hwmon/max31722.c | 181 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 177 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c
index 30a100e..f557c36 100644
--- a/drivers/hwmon/max31722.c
+++ b/drivers/hwmon/max31722.c
@@ -12,23 +12,38 @@
#include <linux/acpi.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/spi/spi.h>
#define MAX31722_REG_CFG 0x00
#define MAX31722_REG_TEMP_LSB 0x01
+#define MAX31722_REG_THIGH_LSB 0x03
+#define MAX31722_REG_TLOW_LSB 0x05
#define MAX31722_MODE_CONTINUOUS 0x00
#define MAX31722_MODE_STANDBY 0x01
#define MAX31722_MODE_MASK 0xFE
#define MAX31722_RESOLUTION_12BIT 0x06
+#define MAX31722_TM_INTERRUPT 0x08
#define MAX31722_WRITE_MASK 0x80
+/*
+ * Minimum temperature value: -2^(12-1) * 62.5 = -128,000
+ * Maximum temperature value: (2^(12-1) - 1) * 62.5 ~= 127,937
+ */
+#define MAX31722_MIN_TEMP -128000
+#define MAX31722_MAX_TEMP 127937
struct max31722_data {
struct device *hwmon_dev;
struct spi_device *spi_device;
+ struct mutex update_lock;
u8 mode;
+ s32 thigh;
+ s32 tlow;
+ bool irq_enabled;
+ bool alarm_active;
};
static int max31722_set_mode(struct max31722_data *data, u8 mode)
@@ -56,6 +71,12 @@ static ssize_t max31722_show_temp(struct device *dev,
{
ssize_t ret;
struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr = to_sensor_dev_attr(attr);
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ return sprintf(buf, "%d\n", data->thigh);
+ else if (dev_attr->index == MAX31722_REG_TLOW_LSB)
+ return sprintf(buf, "%d\n", data->tlow);
ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
if (ret < 0)
@@ -64,15 +85,145 @@ static ssize_t max31722_show_temp(struct device *dev,
return sprintf(buf, "%d\n", (s16)le16_to_cpu(ret) * 125 / 32);
}
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
- max31722_show_temp, NULL, 0);
+static ssize_t max31722_set_temp(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ long val;
+ s16 thresh;
+ u8 tx_buf[3];
+ struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr = to_sensor_dev_attr(attr);
+
+ ret = kstrtol(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ val = clamp_val(val, MAX31722_MIN_TEMP, MAX31722_MAX_TEMP);
+ thresh = cpu_to_le16(DIV_ROUND_CLOSEST(val * 32, 125));
+
+ tx_buf[0] = dev_attr->index | MAX31722_WRITE_MASK;
+ tx_buf[1] = thresh & 0xff;
+ tx_buf[2] = thresh >> 8;
+
+ mutex_lock(&data->update_lock);
+
+ ret = spi_write(data->spi_device, &tx_buf, sizeof(tx_buf));
+ if (ret < 0) {
+ mutex_unlock(&data->update_lock);
+ return ret;
+ }
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ data->thigh = thresh * 125 / 32;
+ else
+ data->tlow = thresh * 125 / 32;
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t max31722_show_alarm(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_device *spi_device = to_spi_device(dev);
+ struct max31722_data *data = spi_get_drvdata(spi_device);
+
+ return sprintf(buf, "%d\n", data->alarm_active);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, max31722_show_temp, NULL,
+ MAX31722_REG_TEMP_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, max31722_show_temp,
+ max31722_set_temp, MAX31722_REG_THIGH_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, max31722_show_temp,
+ max31722_set_temp, MAX31722_REG_TLOW_LSB);
+static DEVICE_ATTR(temp1_alarm, S_IRUGO, max31722_show_alarm, NULL);
static struct attribute *max31722_attrs[] = {
&sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
+ &dev_attr_temp1_alarm.attr,
NULL,
};
-ATTRIBUTE_GROUPS(max31722);
+static umode_t max31722_is_visible(struct kobject *kobj, struct attribute *attr,
+ int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct max31722_data *data = dev_get_drvdata(dev);
+
+ /* Hide alarm and threshold attributes if interrupts are disabled. */
+ if (!data->irq_enabled && index > 0)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group max31722_group = {
+ .attrs = max31722_attrs, .is_visible = max31722_is_visible,
+};
+
+__ATTRIBUTE_GROUPS(max31722);
+
+static ssize_t max31722_update_alarm(struct max31722_data *data)
+{
+ s16 temp;
+ ssize_t ret;
+ /*
+ * Do a quick temperature reading and compare it with TLOW/THIGH
+ * so we can tell which threshold has been met.
+ */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
+ if (ret < 0)
+ return ret;
+ temp = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ if (temp > data->thigh || (!data->alarm_active && temp > data->tlow))
+ data->alarm_active = true;
+ else if (temp <= data->tlow ||
+ (data->alarm_active && temp < data->thigh))
+ data->alarm_active = false;
+
+ return 0;
+}
+
+static irqreturn_t max31722_irq_handler(int irq, void *private)
+{
+ struct max31722_data *data = private;
+
+ max31722_update_alarm(data);
+ sysfs_notify(&data->spi_device->dev.kobj, NULL, "temp1_alarm");
+
+ return IRQ_HANDLED;
+}
+
+static ssize_t max31722_init_thresh(struct max31722_data *data)
+{
+ ssize_t ret;
+
+ /* Cache threshold values. */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TLOW_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->tlow = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_THIGH_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->thigh = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ return 0;
+
+ dev_err(&data->spi_device->dev,
+ "failed to read temperature threshold\n");
+ return ret;
+}
static int max31722_probe(struct spi_device *spi)
{
@@ -83,17 +234,39 @@ static int max31722_probe(struct spi_device *spi)
if (!data)
return -ENOMEM;
+ mutex_init(&data->update_lock);
spi_set_drvdata(spi, data);
data->spi_device = spi;
/*
* Set SD bit to 0 so we can have continuous measurements.
* Set resolution to 12 bits for maximum precision.
*/
- data->mode = MAX31722_MODE_CONTINUOUS | MAX31722_RESOLUTION_12BIT;
+ data->mode = MAX31722_MODE_CONTINUOUS | MAX31722_RESOLUTION_12BIT
+ | MAX31722_TM_INTERRUPT;
ret = max31722_set_mode(data, MAX31722_MODE_CONTINUOUS);
if (ret < 0)
return ret;
+ if (spi->irq > 0) {
+ ret = devm_request_threaded_irq(&spi->dev, spi->irq,
+ NULL,
+ max31722_irq_handler,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ dev_name(&spi->dev), data);
+ if (ret < 0) {
+ dev_warn(&spi->dev, "request irq %d failed\n",
+ spi->irq);
+ } else {
+ data->irq_enabled = true;
+ ret = max31722_init_thresh(data);
+ if (ret < 0)
+ return ret;
+ ret = max31722_update_alarm(data);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
data->hwmon_dev = hwmon_device_register_with_groups(&spi->dev,
spi->modalias,
data,

Breana, Tiberiu A

2016-04-08 12:54:43 UTC

Permalink

-----Original Message-----
Sent: Friday, April 8, 2016 3:24 PM
sensors.org
Subject: Re: [PATCH v5] hwmon: (max31722) Add threshold alarm support

Post by Tiberiu Breana
Add threshold alarm support for the max31722 temperature sensor driver.
---
- addressed Guenter's comments
As specified in the v3 change log, threshold values, as well as the
alarm attribute, are only visible if interrupts are enabled.

I missed that part. Why ? The thresholds are still supported by the chip, even
without interrupts.
Thanks,
Guenter

I don't see the point of exposing the threshold values if they can't be used
to generate interrupts.
Thank you as well for your feedback.

Tiberiu

Post by Tiberiu Breana
---
drivers/hwmon/max31722.c | 181

+++++++++++++++++++++++++++++++++++++++++++++--

Post by Tiberiu Breana
1 file changed, 177 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c

index

Post by Tiberiu Breana
30a100e..f557c36 100644
--- a/drivers/hwmon/max31722.c
+++ b/drivers/hwmon/max31722.c
@@ -12,23 +12,38 @@
#include <linux/acpi.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/spi/spi.h>
#define MAX31722_REG_CFG 0x00
#define MAX31722_REG_TEMP_LSB 0x01
+#define MAX31722_REG_THIGH_LSB 0x03
+#define MAX31722_REG_TLOW_LSB 0x05
#define MAX31722_MODE_CONTINUOUS 0x00
#define MAX31722_MODE_STANDBY 0x01
#define MAX31722_MODE_MASK 0xFE
#define MAX31722_RESOLUTION_12BIT 0x06
+#define MAX31722_TM_INTERRUPT 0x08
#define MAX31722_WRITE_MASK 0x80
+/*
+ * Minimum temperature value: -2^(12-1) * 62.5 = -128,000
+ * Maximum temperature value: (2^(12-1) - 1) * 62.5 ~= 127,937 */
+#define MAX31722_MIN_TEMP -128000
+#define MAX31722_MAX_TEMP 127937
struct max31722_data {
struct device *hwmon_dev;
struct spi_device *spi_device;
+ struct mutex update_lock;
u8 mode;
+ s32 thigh;
+ s32 tlow;
+ bool irq_enabled;
+ bool alarm_active;
};
static int max31722_set_mode(struct max31722_data *data, u8 mode)

Post by Tiberiu Breana
{
ssize_t ret;
struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr =

to_sensor_dev_attr(attr);

Post by Tiberiu Breana
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ return sprintf(buf, "%d\n", data->thigh);
+ else if (dev_attr->index == MAX31722_REG_TLOW_LSB)
+ return sprintf(buf, "%d\n", data->tlow);
ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
if (ret < 0)
@@ -64,15 +85,145 @@ static ssize_t max31722_show_temp(struct device

*dev,

Post by Tiberiu Breana
return sprintf(buf, "%d\n", (s16)le16_to_cpu(ret) * 125 / 32);
}
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
- max31722_show_temp, NULL, 0);
+static ssize_t max31722_set_temp(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ long val;
+ s16 thresh;
+ u8 tx_buf[3];
+ struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr =

to_sensor_dev_attr(attr);

Post by Tiberiu Breana
+
+ ret = kstrtol(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ val = clamp_val(val, MAX31722_MIN_TEMP,

MAX31722_MAX_TEMP);

Post by Tiberiu Breana
+ thresh = cpu_to_le16(DIV_ROUND_CLOSEST(val * 32, 125));
+
+ tx_buf[0] = dev_attr->index | MAX31722_WRITE_MASK;
+ tx_buf[1] = thresh & 0xff;
+ tx_buf[2] = thresh >> 8;
+
+ mutex_lock(&data->update_lock);
+
+ ret = spi_write(data->spi_device, &tx_buf, sizeof(tx_buf));
+ if (ret < 0) {
+ mutex_unlock(&data->update_lock);
+ return ret;
+ }
+
+ if (dev_attr->index == MAX31722_REG_THIGH_LSB)
+ data->thigh = thresh * 125 / 32;
+ else
+ data->tlow = thresh * 125 / 32;
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t max31722_show_alarm(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_device *spi_device = to_spi_device(dev);
+ struct max31722_data *data = spi_get_drvdata(spi_device);
+
+ return sprintf(buf, "%d\n", data->alarm_active); }
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,

max31722_show_temp, NULL,

Post by Tiberiu Breana
+ MAX31722_REG_TEMP_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,

max31722_show_temp,

Post by Tiberiu Breana
+ max31722_set_temp, MAX31722_REG_THIGH_LSB);

static

Post by Tiberiu Breana
+SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,

max31722_show_temp,

Post by Tiberiu Breana
+ max31722_set_temp, MAX31722_REG_TLOW_LSB);

static

Post by Tiberiu Breana
+DEVICE_ATTR(temp1_alarm, S_IRUGO, max31722_show_alarm, NULL);
static struct attribute *max31722_attrs[] = {
&sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
+ &dev_attr_temp1_alarm.attr,
NULL,
};
-ATTRIBUTE_GROUPS(max31722);
+static umode_t max31722_is_visible(struct kobject *kobj, struct attribute

*attr,

Post by Tiberiu Breana
+ int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct max31722_data *data = dev_get_drvdata(dev);
+
+ /* Hide alarm and threshold attributes if interrupts are disabled. */
+ if (!data->irq_enabled && index > 0)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group max31722_group = {
+ .attrs = max31722_attrs, .is_visible = max31722_is_visible, };
+
+__ATTRIBUTE_GROUPS(max31722);
+
+static ssize_t max31722_update_alarm(struct max31722_data *data) {
+ s16 temp;
+ ssize_t ret;
+ /*
+ * Do a quick temperature reading and compare it with TLOW/THIGH
+ * so we can tell which threshold has been met.
+ */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TEMP_LSB);
+ if (ret < 0)
+ return ret;
+ temp = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ if (temp > data->thigh || (!data->alarm_active && temp > data-
tlow))
+ data->alarm_active = true;
+ else if (temp <= data->tlow ||
+ (data->alarm_active && temp < data->thigh))
+ data->alarm_active = false;
+
+ return 0;
+}
+
+static irqreturn_t max31722_irq_handler(int irq, void *private) {
+ struct max31722_data *data = private;
+
+ max31722_update_alarm(data);
+ sysfs_notify(&data->spi_device->dev.kobj, NULL, "temp1_alarm");
+
+ return IRQ_HANDLED;
+}
+
+static ssize_t max31722_init_thresh(struct max31722_data *data) {
+ ssize_t ret;
+
+ /* Cache threshold values. */
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_TLOW_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->tlow = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ ret = spi_w8r16(data->spi_device, MAX31722_REG_THIGH_LSB);
+ if (ret < 0)
+ goto exit_err;
+ data->thigh = (s16)le16_to_cpu(ret) * 125 / 32;
+
+ return 0;
+
+ dev_err(&data->spi_device->dev,
+ "failed to read temperature threshold\n");
+ return ret;
+}
static int max31722_probe(struct spi_device *spi)
{
@@ -83,17 +234,39 @@ static int max31722_probe(struct spi_device *spi)
if (!data)
return -ENOMEM;
+ mutex_init(&data->update_lock);
spi_set_drvdata(spi, data);
data->spi_device = spi;
/*
* Set SD bit to 0 so we can have continuous measurements.
* Set resolution to 12 bits for maximum precision.
*/
- data->mode = MAX31722_MODE_CONTINUOUS |

MAX31722_RESOLUTION_12BIT;

Post by Tiberiu Breana
+ data->mode = MAX31722_MODE_CONTINUOUS |

MAX31722_RESOLUTION_12BIT

Post by Tiberiu Breana
+ | MAX31722_TM_INTERRUPT;
ret = max31722_set_mode(data, MAX31722_MODE_CONTINUOUS);
if (ret < 0)
return ret;
+ if (spi->irq > 0) {
+ ret = devm_request_threaded_irq(&spi->dev, spi->irq,
+ NULL,
+ max31722_irq_handler,
+ IRQF_TRIGGER_LOW |

IRQF_ONESHOT,

Post by Tiberiu Breana
+ dev_name(&spi->dev), data);
+ if (ret < 0) {
+ dev_warn(&spi->dev, "request irq %d failed\n",
+ spi->irq);
+ } else {
+ data->irq_enabled = true;
+ ret = max31722_init_thresh(data);
+ if (ret < 0)
+ return ret;
+ ret = max31722_update_alarm(data);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
data->hwmon_dev = hwmon_device_register_with_groups(&spi-
dev,
spi->modalias,
data,

Guenter Roeck

2016-04-08 13:30:25 UTC

Permalink

Post by Breana, Tiberiu A

-----Original Message-----
Sent: Friday, April 8, 2016 3:24 PM
sensors.org
Subject: Re: [PATCH v5] hwmon: (max31722) Add threshold alarm support

Post by Tiberiu Breana
Add threshold alarm support for the max31722 temperature sensor driver.
---
- addressed Guenter's comments
As specified in the v3 change log, threshold values, as well as the
alarm attribute, are only visible if interrupts are enabled.

I missed that part. Why ? The thresholds are still supported by the chip, even
without interrupts.
Thanks,
Guenter

I don't see the point of exposing the threshold values if they can't be used
to generate interrupts.

Looking into the datasheet again, there are at least two possible other
use cases, one of which does not require interrupt support.

First, by putting the chip into comparator mode and selecting an edge
triggered interrupt, the interrupt line itself would indicate the
temperature status. This would be more beneficial than the current
implementation, since the interrupt line status would indicate
when/if the chip exceeded temperature limits, and the reading the current
temperature as well as the comparisons would not be necessary (assuming
the interrupt line status can be reported).

Second, also in comparator more, the TOUT line could be used
to directly trigger some action, such as turning on a fan. This
use case does not require any interrupt in the first place,
but still makes use of thigh/tlow.

The second use case makes it questionable if interrupt mode should
be selected automatically. The default operating mode is comparator
mode, and that use case requires it. Given that, and if your use case
requires interrupt mode, I think interrupt mode should only be selected
if interrupt support is enabled.

Thanks,
Guenter

Post by Breana, Tiberiu A
Thank you as well for your feedback.
Tiberiu

Post by Tiberiu Breana
---
drivers/hwmon/max31722.c | 181

+++++++++++++++++++++++++++++++++++++++++++++--

Post by Tiberiu Breana
1 file changed, 177 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max31722.c b/drivers/hwmon/max31722.c

index

Post by Tiberiu Breana
{
ssize_t ret;
struct max31722_data *data = dev_get_drvdata(dev);
+ struct sensor_device_attribute *dev_attr =

to_sensor_dev_attr(attr);

*dev,

to_sensor_dev_attr(attr);

Post by Tiberiu Breana
+
+ ret = kstrtol(buf, 10, &val);
+ if (ret < 0)
+ return ret;
+
+ val = clamp_val(val, MAX31722_MIN_TEMP,

MAX31722_MAX_TEMP);

max31722_show_temp, NULL,

Post by Tiberiu Breana
+ MAX31722_REG_TEMP_LSB);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,

max31722_show_temp,

Post by Tiberiu Breana
+ max31722_set_temp, MAX31722_REG_THIGH_LSB);

static

Post by Tiberiu Breana
+SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,

max31722_show_temp,

Post by Tiberiu Breana
+ max31722_set_temp, MAX31722_REG_TLOW_LSB);

static

*attr,

MAX31722_RESOLUTION_12BIT;

Post by Tiberiu Breana
+ data->mode = MAX31722_MODE_CONTINUOUS |

MAX31722_RESOLUTION_12BIT

IRQF_ONESHOT,