All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-09 13:30 Tiberiu Breana
  2016-03-09 13:30 ` [PATCH 1/2] iio: temperature: " Tiberiu Breana
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Tiberiu Breana @ 2016-03-09 13:30 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

This patch set adds support for the MAX31722/MAX31723 temperature sensors /
thermostats.
Patch 1 adds basic support and power management.
Patch 2 adds threshold interrupt support.

Tiberiu Breana (2):
  iio: temperature: Add support for MAX31722/MAX31723 temperature
    sensors
  iio: temperature: Add threshold interrupt support for max31722

 drivers/iio/temperature/Kconfig    |  12 +
 drivers/iio/temperature/Makefile   |   1 +
 drivers/iio/temperature/max31722.c | 673 +++++++++++++++++++++++++++++++++++++
 3 files changed, 686 insertions(+)
 create mode 100644 drivers/iio/temperature/max31722.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] iio: temperature: Add support for MAX31722/MAX31723 temperature sensors
  2016-03-09 13:30 [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Tiberiu Breana
@ 2016-03-09 13:30 ` Tiberiu Breana
  2016-03-12  9:57   ` Jonathan Cameron
  2016-03-09 13:30 ` [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722 Tiberiu Breana
  2016-03-09 20:31 ` [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Jonathan Cameron
  2 siblings, 1 reply; 22+ messages in thread
From: Tiberiu Breana @ 2016-03-09 13:30 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

Add basic support for the Maxim Integrated MAX31722/MAX31723 SPI
temperature sensors / thermostats.

Includes:
	- ACPI support;
	- raw temperature readings;
	- ability to set measurement resolution;
	- power management

Datasheet:
https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf

Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
---
 drivers/iio/temperature/Kconfig    |  12 ++
 drivers/iio/temperature/Makefile   |   1 +
 drivers/iio/temperature/max31722.c | 374 +++++++++++++++++++++++++++++++++++++
 3 files changed, 387 insertions(+)
 create mode 100644 drivers/iio/temperature/max31722.c

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index c4664e5..7587887 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -3,6 +3,18 @@
 #
 menu "Temperature sensors"
 
+config MAX31722
+	tristate "MAX31722 temperature sensor"
+	depends on SPI
+	select REGMAP_SPI
+	help
+	  If you say yes here you get support for the Maxim Integrated
+	  MAX31722/MAX31723 digital thermometers / thermostats operating
+	  over an SPI interface.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called max31722.
+
 config MLX90614
 	tristate "MLX90614 contact-less infrared sensor"
 	depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 02bc79d..a9484d6 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -2,6 +2,7 @@
 # Makefile for industrial I/O temperature drivers
 #
 
+obj-$(CONFIG_MAX31722) += max31722.o
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
 obj-$(CONFIG_TSYS01) += tsys01.o
diff --git a/drivers/iio/temperature/max31722.c b/drivers/iio/temperature/max31722.c
new file mode 100644
index 0000000..fa833b6
--- /dev/null
+++ b/drivers/iio/temperature/max31722.c
@@ -0,0 +1,374 @@
+/**
+ * MAX31722/MAX31723 digital thermometer and thermostat SPI driver
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define MAX31722_REG_CFG				0x00
+#define MAX31722_REG_TEMP_LSB			0x01
+#define MAX31722_REG_TEMP_MSB			0x02
+#define MAX31722_MAX_REG				0x86
+
+#define MAX31722_MODE_CONTINUOUS		0x00
+#define MAX31722_MODE_STANDBY			0x01
+
+#define MAX31722_REGMAP_NAME			"max31722_regmap"
+
+#define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125 0.0625"
+
+#define MAX31722_REGFIELD(name)						 \
+	do {								 \
+		data->reg_##name =					 \
+			devm_regmap_field_alloc(&spi->dev, regmap,	 \
+				max31722_reg_field_##name);		 \
+		if (IS_ERR(data->reg_##name)) {				 \
+			dev_err(&spi->dev, "reg field alloc failed.\n"); \
+			return PTR_ERR(data->reg_##name);		 \
+		}							 \
+	} while (0)
+
+static const struct reg_field max31722_reg_field_state =
+				REG_FIELD(MAX31722_REG_CFG, 0, 0);
+static const struct reg_field max31722_reg_field_resolution =
+				REG_FIELD(MAX31722_REG_CFG, 1, 2);
+
+struct max31722_data {
+	struct spi_device *spi_device;
+	struct mutex lock;
+	struct regmap *regmap;
+	struct regmap_field *reg_state;
+	struct regmap_field *reg_resolution;
+};
+
+/*
+ * This table can double as a negative exponent value lookup table
+ * (as micro units) for calculating LSB temperature values.
+ */
+static const int max31722_scale_table[4] = {
+	500000, 250000, 125000, 62500
+};
+
+static IIO_CONST_ATTR(in_temp_scale_available, MAX31722_SCALE_AVAILABLE);
+
+static struct attribute *max31722_attributes[] = {
+	&iio_const_attr_in_temp_scale_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group max31722_attribute_group = {
+	.attrs = max31722_attributes
+};
+
+static const struct iio_chan_spec max31722_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static bool max31722_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MAX31722_REG_TEMP_LSB:
+	case MAX31722_REG_TEMP_MSB:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool max31722_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MAX31722_REG_TEMP_LSB:
+	case MAX31722_REG_TEMP_MSB:
+		return false;
+	default:
+		return true;
+	}
+}
+
+static struct regmap_config max31722_regmap_config = {
+	.name = MAX31722_REGMAP_NAME,
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = MAX31722_MAX_REG,
+	.cache_type = REGCACHE_RBTREE,
+
+	.volatile_reg = max31722_is_volatile_reg,
+	.writeable_reg = max31722_is_writeable_reg,
+
+	.read_flag_mask = 0x00,
+	.write_flag_mask = 0x80,
+};
+
+/*
+ * Convert a temperature register value to a floating point value.
+ * Data is represented in 9-12 bits, two's complement.
+ * The MSB contains the integer part of the temperature value
+ * (signed), while the LSB contains the fractional part as
+ * negative powers of 2, e.g.: bit 0 is 2^-1, bit 1 is 2^-2, etc.
+ */
+static void max31722_reg_to_float(u16 reg_val, int *val, int *val2)
+{
+	int i;
+	u8 msb;
+	u8 lsb;
+
+	msb = reg_val >> 8;
+	lsb = (reg_val << 8) >> 8;
+
+	/* Calculate the integer part. */
+	if (msb & 0x80) {
+		/* Negative value. Reverse bits and add 1. */
+		u16 orig = reg_val;
+		u16 rev = 0;
+		int num_bits = sizeof(rev) * 8;
+
+		for (i = 0 ; i < num_bits ; i++) {
+			rev |= !(orig & 0x01) << i;
+			orig >>= 1;
+		}
+		rev += 1;
+		msb = rev >> 8;
+		lsb = (rev << 8) >> 8;
+		*val = (-1) * msb;
+	} else { /* Positive value. */
+		*val = msb;
+	}
+
+	/*
+	 * Calculate the fractional part.
+	 * Only the first four LSB bits contain data.
+	 */
+	lsb >>= 4;
+	i = 0;
+	*val2 = 0;
+	while (i < 4) {
+		if ((lsb >> (4 - i - 1)) & 0x01)
+			*val2 += max31722_scale_table[i];
+		lsb <<= 1;
+		lsb >>= 1;
+		i++;
+	}
+}
+
+static int max31722_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	int ret;
+	int index;
+	u16 buf;
+	struct max31722_data *data = iio_priv(indio_dev);
+	struct spi_device *spi = data->spi_device;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&data->lock);
+		ret = regmap_bulk_read(data->regmap,
+				       MAX31722_REG_TEMP_LSB, &buf, 2);
+		if (ret < 0) {
+			dev_err(&spi->dev, "failed to read temperature register\n");
+			mutex_unlock(&data->lock);
+			return ret;
+		}
+		max31722_reg_to_float(buf, val, val2);
+		mutex_unlock(&data->lock);
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_SCALE:
+		mutex_lock(&data->lock);
+		ret = regmap_field_read(data->reg_resolution, &index);
+		if (ret < 0) {
+			dev_err(&spi->dev, "failed to read configuration register\n");
+			mutex_unlock(&data->lock);
+			return ret;
+		}
+		mutex_unlock(&data->lock);
+		*val = 0;
+		*val2 = max31722_scale_table[index];
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	return -EINVAL;
+}
+
+static int max31722_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	int i;
+	int ret;
+	int index = -1;
+	struct max31722_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(max31722_scale_table); i++) {
+			if (val == 0 && val2 == max31722_scale_table[i]) {
+				index = i;
+				break;
+			}
+		}
+		if (index < 0)
+			return -EINVAL;
+
+		mutex_lock(&data->lock);
+		ret = regmap_field_write(data->reg_resolution, index);
+		mutex_unlock(&data->lock);
+
+		return ret;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info max31722_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= max31722_read_raw,
+	.write_raw		= max31722_write_raw,
+	.attrs			= &max31722_attribute_group,
+};
+
+static int max31722_init(struct max31722_data *data)
+{
+	int ret = 0;
+	struct spi_device *spi = data->spi_device;
+	struct regmap *regmap;
+
+	/* Initialize the regmap */
+	regmap = devm_regmap_init_spi(spi, &max31722_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "regmap initialization failed.\n");
+		return PTR_ERR(regmap);
+	}
+	data->regmap = regmap;
+
+	MAX31722_REGFIELD(state);
+	MAX31722_REGFIELD(resolution);
+
+	/* Set SD bit to 0 so we can have continuous measurements. */
+	ret = regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
+	if (ret < 0)
+		dev_err(&spi->dev, "failed to configure sensor.\n");
+
+	return ret;
+}
+
+static int max31722_probe(struct spi_device *spi)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct max31722_data *data;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
+	if (!indio_dev) {
+		dev_err(&spi->dev, "iio allocation failed!\n");
+		return -ENOMEM;
+	}
+
+	data = iio_priv(indio_dev);
+	data->spi_device = spi;
+	spi_set_drvdata(spi, indio_dev);
+	mutex_init(&data->lock);
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->info = &max31722_info;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = max31722_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max31722_channels);
+
+	ret = max31722_init(data);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "iio_device_register failed\n");
+		regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
+	}
+
+	return ret;
+}
+
+static int max31722_remove(struct spi_device *spi)
+{
+	struct max31722_data *data;
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	data = iio_priv(indio_dev);
+
+	return regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max31722_suspend(struct device *dev)
+{
+	struct max31722_data *data;
+
+	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
+
+	return regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
+}
+
+static int max31722_resume(struct device *dev)
+{
+	struct max31722_data *data;
+
+	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
+
+	return regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
+}
+
+static SIMPLE_DEV_PM_OPS(max31722_pm_ops, max31722_suspend, max31722_resume);
+
+#define MAX31722_PM_OPS (&max31722_pm_ops)
+#else
+#define MAX31722_PM_OPS NULL
+#endif
+
+static const struct spi_device_id max31722_spi_id[] = {
+	{"max31722", 0},
+	{"max31723", 0},
+	{}
+};
+
+static const struct acpi_device_id max31722_acpi_id[] = {
+	{"MAX31722", 0},
+	{"MAX31723", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(spi, max31722_spi_id);
+
+static struct spi_driver max31722_driver = {
+	.driver = {
+		.name = "max31722",
+		.pm = MAX31722_PM_OPS,
+		.acpi_match_table = ACPI_PTR(max31722_acpi_id),
+	},
+	.probe =            max31722_probe,
+	.remove =           max31722_remove,
+	.id_table =         max31722_spi_id,
+};
+
+module_spi_driver(max31722_driver);
+
+MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
+MODULE_DESCRIPTION("MAX31722 digital thermometer and thermostat SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722
  2016-03-09 13:30 [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Tiberiu Breana
  2016-03-09 13:30 ` [PATCH 1/2] iio: temperature: " Tiberiu Breana
@ 2016-03-09 13:30 ` Tiberiu Breana
  2016-03-12 10:17   ` Jonathan Cameron
  2016-03-09 20:31 ` [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Jonathan Cameron
  2 siblings, 1 reply; 22+ messages in thread
From: Tiberiu Breana @ 2016-03-09 13:30 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

Added interrupt support for high/low temperature threshold
events to the max31722 driver.

Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
---
 drivers/iio/temperature/max31722.c | 303 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 301 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/temperature/max31722.c b/drivers/iio/temperature/max31722.c
index fa833b6..a8bfe35 100644
--- a/drivers/iio/temperature/max31722.c
+++ b/drivers/iio/temperature/max31722.c
@@ -10,21 +10,32 @@
 
 #include <linux/kernel.h>
 #include <linux/acpi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/spi/spi.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
 #define MAX31722_REG_CFG				0x00
 #define MAX31722_REG_TEMP_LSB			0x01
 #define MAX31722_REG_TEMP_MSB			0x02
+#define MAX31722_REG_THIGH_LSB         0x03
+#define MAX31722_REG_TLOW_LSB          0x05
 #define MAX31722_MAX_REG				0x86
 
 #define MAX31722_MODE_CONTINUOUS		0x00
 #define MAX31722_MODE_STANDBY			0x01
+#define MAX31722_MODE_THERMOSTAT       0x80
+
+#define MAX31722_MIN_TEMP          -55
+#define MAX31722_MAX_TEMP          125
 
 #define MAX31722_REGMAP_NAME			"max31722_regmap"
+#define MAX31722_EVENT             "max31722_event"
+#define MAX31722_GPIO              "max31722_gpio"
 
 #define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125 0.0625"
 
@@ -43,6 +54,8 @@ static const struct reg_field max31722_reg_field_state =
 				REG_FIELD(MAX31722_REG_CFG, 0, 0);
 static const struct reg_field max31722_reg_field_resolution =
 				REG_FIELD(MAX31722_REG_CFG, 1, 2);
+static const struct reg_field max31722_reg_field_thermostat =
+				REG_FIELD(MAX31722_REG_CFG, 3, 3);
 
 struct max31722_data {
 	struct spi_device *spi_device;
@@ -50,6 +63,8 @@ struct max31722_data {
 	struct regmap *regmap;
 	struct regmap_field *reg_state;
 	struct regmap_field *reg_resolution;
+	struct regmap_field *reg_thermostat;
+	u64 timestamp;
 };
 
 /*
@@ -71,11 +86,30 @@ static const struct attribute_group max31722_attribute_group = {
 	.attrs = max31722_attributes
 };
 
+static const struct iio_event_spec max31722_events[] = {
+	/* High temperature event */
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+	/* Low temperature event */
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
 static const struct iio_chan_spec max31722_channels[] = {
 	{
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE),
+		.event_spec = max31722_events,
+		.num_event_specs = ARRAY_SIZE(max31722_events),
 	},
 };
 
@@ -167,6 +201,172 @@ static void max31722_reg_to_float(u16 reg_val, int *val, int *val2)
 	}
 }
 
+/* Convert a floating point value to a register value. */
+static u16 max31722_float_to_reg(int val, int val2)
+{
+	int i;
+	bool negative_nr;
+	u8 lsb;
+	u16 reg_val;
+
+	negative_nr = (val & 0x80) || val2 < 0;
+	if (val2 < 0)
+		val2 *= -1;
+
+	/*
+	 * The LSB value will be an approximation of val2
+	 * due to its limited 4-bit range.
+	 */
+	lsb = 0;
+	for (i = 0 ; i < ARRAY_SIZE(max31722_scale_table) && val2 > 0; i++)
+		if (val2 - max31722_scale_table[i] >= 0) {
+			val2 -= max31722_scale_table[i];
+			lsb += 1 << (4 - i - 1);
+		}
+	lsb <<= 4;
+
+	if (negative_nr) {
+		/*
+		 * Negative value. Temporarily use the complement of val for
+		 * the MSB, then concatenate the LSB, reverse bits & add 1 for
+		 * the final value.
+		 */
+		u8 msb = (u8)(-1 * val);
+		u16 rev = 0;
+		int num_bits = sizeof(rev) * 8;
+
+		reg_val = (msb << 8) | lsb;
+
+		for (i = 0 ; i < num_bits ; i++) {
+			rev |= !(reg_val & 0x01) << i;
+			reg_val >>= 1;
+		}
+		rev += 1;
+		return rev;
+
+	} else {
+		reg_val = ((u8)val << 8) | lsb;
+	}
+
+	return reg_val;
+}
+
+static int max31722_read_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir)
+{
+	unsigned int event_val;
+	int ret;
+	struct max31722_data *data = iio_priv(indio_dev);
+	struct spi_device *spi = data->spi_device;
+
+	mutex_lock(&data->lock);
+	ret = regmap_field_read(data->reg_thermostat, &event_val);
+	if (ret < 0) {
+		dev_err(&spi->dev, "failed to read thermostat status\n");
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+	mutex_unlock(&data->lock);
+
+	return event_val;
+}
+
+static int max31722_write_event_config(struct iio_dev *indio_dev,
+				       const struct iio_chan_spec *chan,
+				       enum iio_event_type type,
+				       enum iio_event_direction dir,
+				       int state)
+{
+	int ret;
+	struct max31722_data *data = iio_priv(indio_dev);
+	struct spi_device *spi = data->spi_device;
+
+	if (state != 0 && state != 1)
+		return -EINVAL;
+
+	/*
+	 * Set thermostat mode:
+	 * 0 : comparator mode (default)
+	 * 1 : interrupt mode
+	 */
+	mutex_lock(&data->lock);
+	ret = regmap_field_write(data->reg_thermostat, state);
+	if (ret < 0)
+		dev_err(&spi->dev, "failed to set thermostat mode\n");
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int max31722_read_event(struct iio_dev *indio_dev,
+			       const struct iio_chan_spec *chan,
+			       enum iio_event_type type,
+			       enum iio_event_direction dir,
+			       enum iio_event_info info,
+			       int *val, int *val2)
+{
+	int ret;
+	u8 reg;
+	u16 buf;
+	struct max31722_data *data = iio_priv(indio_dev);
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_RISING)
+		reg = MAX31722_REG_THIGH_LSB;
+	else if (dir == IIO_EV_DIR_FALLING)
+		reg = MAX31722_REG_TLOW_LSB;
+	else
+		return -EINVAL;
+
+	mutex_lock(&data->lock);
+	ret = regmap_bulk_read(data->regmap, reg, &buf, 2);
+	mutex_unlock(&data->lock);
+	if (ret < 0) {
+		dev_err(&data->spi_device->dev,
+			"failed to read threshold register\n");
+		return ret;
+	}
+	max31722_reg_to_float(buf, val, val2);
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int max31722_write_event(struct iio_dev *indio_dev,
+				const struct iio_chan_spec *chan,
+				enum iio_event_type type,
+				enum iio_event_direction dir,
+				enum iio_event_info info,
+				int val, int val2)
+{
+	int ret;
+	u8 reg;
+	u16 buf;
+	struct max31722_data *data = iio_priv(indio_dev);
+
+	if (val < MAX31722_MIN_TEMP || val > MAX31722_MAX_TEMP)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_RISING)
+		reg = MAX31722_REG_THIGH_LSB;
+	else if (dir == IIO_EV_DIR_FALLING)
+		reg = MAX31722_REG_TLOW_LSB;
+	else
+		return -EINVAL;
+
+	buf = max31722_float_to_reg(val, val2);
+
+	ret = regmap_bulk_write(data->regmap, reg, &buf, 2);
+	if (ret < 0)
+		dev_err(&data->spi_device->dev,
+			"failed to write threshold register\n");
+
+	return ret;
+}
+
 static int max31722_read_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int *val, int *val2, long mask)
@@ -241,8 +441,63 @@ static const struct iio_info max31722_info = {
 	.read_raw		= max31722_read_raw,
 	.write_raw		= max31722_write_raw,
 	.attrs			= &max31722_attribute_group,
+	.read_event_value	= max31722_read_event,
+	.write_event_value	= max31722_write_event,
+	.read_event_config	= max31722_read_event_config,
+	.write_event_config	= max31722_write_event_config,
 };
 
+static irqreturn_t max31722_irq_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct max31722_data *data = iio_priv(indio_dev);
+
+	data->timestamp = iio_get_time_ns();
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t max31722_irq_event_handler(int irq, void *private)
+{
+	int ret;
+	u64 event;
+	u16 temp;
+	u16 tlow;
+	u16 thigh;
+	unsigned int dir;
+	struct iio_dev *indio_dev = private;
+	struct max31722_data *data = iio_priv(indio_dev);
+
+	/*
+	 * Do a quick temperature reading and compare it with TLOW/THIGH
+	 * so we can tell which threshold has been met.
+	 */
+	mutex_lock(&data->lock);
+	ret = regmap_bulk_read(data->regmap, MAX31722_REG_TEMP_LSB, &temp, 2);
+	if (ret < 0)
+		goto exit_err;
+	ret = regmap_bulk_read(data->regmap, MAX31722_REG_TLOW_LSB, &tlow, 2);
+	if (ret < 0)
+		goto exit_err;
+	ret = regmap_bulk_read(data->regmap, MAX31722_REG_THIGH_LSB, &thigh, 2);
+	if (ret < 0)
+		goto exit_err;
+	mutex_unlock(&data->lock);
+
+	dir = (temp > thigh ? 1 : 0);
+	event = IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0, IIO_EV_TYPE_THRESH,
+				     (dir ? IIO_EV_DIR_RISING :
+					    IIO_EV_DIR_FALLING));
+	iio_push_event(indio_dev, event, data->timestamp);
+
+	return IRQ_HANDLED;
+
+exit_err:
+	dev_err(&data->spi_device->dev, "failed to read temperature register\n");
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
 static int max31722_init(struct max31722_data *data)
 {
 	int ret = 0;
@@ -259,15 +514,40 @@ static int max31722_init(struct max31722_data *data)
 
 	MAX31722_REGFIELD(state);
 	MAX31722_REGFIELD(resolution);
+	MAX31722_REGFIELD(thermostat);
 
 	/* Set SD bit to 0 so we can have continuous measurements. */
 	ret = regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
+	if (ret < 0) {
+		dev_err(&spi->dev, "failed to configure sensor.\n");
+		return ret;
+	}
+
+	/* Set thermostat interrupt mode */
+	ret = regmap_field_write(data->reg_thermostat, 1);
 	if (ret < 0)
 		dev_err(&spi->dev, "failed to configure sensor.\n");
 
 	return ret;
 }
 
+static int max31722_gpio_probe(struct spi_device *spi)
+{
+	struct gpio_desc *gpio;
+
+	if (!spi)
+		return -EINVAL;
+
+	/* gpio interrupt pin */
+	gpio = devm_gpiod_get_index(&spi->dev, MAX31722_GPIO, 0, GPIOD_IN);
+	if (IS_ERR(gpio)) {
+		dev_err(&spi->dev, "gpio request failed.\n");
+		return PTR_ERR(gpio);
+	}
+
+	return gpiod_to_irq(gpio);
+}
+
 static int max31722_probe(struct spi_device *spi)
 {
 	int ret;
@@ -294,15 +574,34 @@ static int max31722_probe(struct spi_device *spi)
 
 	ret = max31722_init(data);
 	if (ret < 0)
-		return ret;
+		goto err_standby;
+
+	ret = max31722_gpio_probe(data->spi_device);
+	if (ret < 0)
+		goto err_standby;
+
+	spi->irq = ret;
+	ret = devm_request_threaded_irq(&spi->dev, spi->irq,
+					max31722_irq_handler,
+					max31722_irq_event_handler,
+					IRQF_TRIGGER_LOW,
+					MAX31722_EVENT, indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "request irq %d failed\n", spi->irq);
+		goto err_standby;
+	}
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
 		dev_err(&spi->dev, "iio_device_register failed\n");
-		regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
+		goto err_standby;
 	}
 
 	return ret;
+
+err_standby:
+	regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
+	return ret;
 }
 
 static int max31722_remove(struct spi_device *spi)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-09 13:30 [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Tiberiu Breana
  2016-03-09 13:30 ` [PATCH 1/2] iio: temperature: " Tiberiu Breana
  2016-03-09 13:30 ` [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722 Tiberiu Breana
@ 2016-03-09 20:31 ` Jonathan Cameron
  2016-03-15 12:58   ` Breana, Tiberiu A
  2 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-09 20:31 UTC (permalink / raw)
  To: Tiberiu Breana, linux-iio

On 09/03/16 13:30, Tiberiu Breana wrote:
> This patch set adds support for the MAX31722/MAX31723 temperature sensors /
> thermostats.
> Patch 1 adds basic support and power management.
> Patch 2 adds threshold interrupt support.
> 
> Tiberiu Breana (2):
>   iio: temperature: Add support for MAX31722/MAX31723 temperature
>     sensors
>   iio: temperature: Add threshold interrupt support for max31722
> 
>  drivers/iio/temperature/Kconfig    |  12 +
>  drivers/iio/temperature/Makefile   |   1 +
>  drivers/iio/temperature/max31722.c | 673 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 686 insertions(+)
>  create mode 100644 drivers/iio/temperature/max31722.c
> 
Hi Tiberiu,

When proposing a temperature sensor driver, we need some additional justification
for why it is suited for IIO rather than the more focused (and in someways simpler)
hwmon.

So far we have had thermophiles that don't fit in as devices typically used for
hardware monitoring. Also in there are a few drivers which are for parts where
they are a cut down version of a more complex sensor (the measurement specialties
parts are either pressure sensors or humidty sensors at least in terms of what they
share interfaces with)

Also, please cc the hwmon maintainers (and probably list) as we want their agreement.

Jonathan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] iio: temperature: Add support for MAX31722/MAX31723 temperature sensors
  2016-03-09 13:30 ` [PATCH 1/2] iio: temperature: " Tiberiu Breana
@ 2016-03-12  9:57   ` Jonathan Cameron
  2016-03-14 13:45     ` Breana, Tiberiu A
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-12  9:57 UTC (permalink / raw)
  To: Tiberiu Breana, linux-iio

On 09/03/16 13:30, Tiberiu Breana wrote:
> Add basic support for the Maxim Integrated MAX31722/MAX31723 SPI
> temperature sensors / thermostats.
> 
> Includes:
> 	- ACPI support;
> 	- raw temperature readings;
> 	- ability to set measurement resolution;
> 	- power management
> 
> Datasheet:
> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
> 
> Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
I thought I'd give this a quick review, putting aside the question of
whether it should be in IIO at all.

A few really trivial suggestions, but otherwise a nice clean driver.

Jonathan
> ---
>  drivers/iio/temperature/Kconfig    |  12 ++
>  drivers/iio/temperature/Makefile   |   1 +
>  drivers/iio/temperature/max31722.c | 374 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 387 insertions(+)
>  create mode 100644 drivers/iio/temperature/max31722.c
> 
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index c4664e5..7587887 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,18 @@
>  #
>  menu "Temperature sensors"
>  
> +config MAX31722
> +	tristate "MAX31722 temperature sensor"
> +	depends on SPI
> +	select REGMAP_SPI
> +	help
> +	  If you say yes here you get support for the Maxim Integrated
> +	  MAX31722/MAX31723 digital thermometers / thermostats operating
> +	  over an SPI interface.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called max31722.
> +
>  config MLX90614
>  	tristate "MLX90614 contact-less infrared sensor"
>  	depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 02bc79d..a9484d6 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,6 +2,7 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>  
> +obj-$(CONFIG_MAX31722) += max31722.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
> diff --git a/drivers/iio/temperature/max31722.c b/drivers/iio/temperature/max31722.c
> new file mode 100644
> index 0000000..fa833b6
> --- /dev/null
> +++ b/drivers/iio/temperature/max31722.c
> @@ -0,0 +1,374 @@
> +/**
> + * MAX31722/MAX31723 digital thermometer and thermostat SPI driver
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define MAX31722_REG_CFG				0x00
> +#define MAX31722_REG_TEMP_LSB			0x01
> +#define MAX31722_REG_TEMP_MSB			0x02
> +#define MAX31722_MAX_REG				0x86
> +
> +#define MAX31722_MODE_CONTINUOUS		0x00
> +#define MAX31722_MODE_STANDBY			0x01
> +
> +#define MAX31722_REGMAP_NAME			"max31722_regmap"
> +
> +#define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125 0.0625"
> +
> +#define MAX31722_REGFIELD(name)						 \
> +	do {								 \
> +		data->reg_##name =					 \
> +			devm_regmap_field_alloc(&spi->dev, regmap,	 \
> +				max31722_reg_field_##name);		 \
> +		if (IS_ERR(data->reg_##name)) {				 \
> +			dev_err(&spi->dev, "reg field alloc failed.\n"); \
> +			return PTR_ERR(data->reg_##name);		 \
> +		}							 \
> +	} while (0)
> +
> +static const struct reg_field max31722_reg_field_state =
> +				REG_FIELD(MAX31722_REG_CFG, 0, 0);
> +static const struct reg_field max31722_reg_field_resolution =
> +				REG_FIELD(MAX31722_REG_CFG, 1, 2);
> +
> +struct max31722_data {
> +	struct spi_device *spi_device;
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	struct regmap_field *reg_state;
> +	struct regmap_field *reg_resolution;
> +};
> +
> +/*
> + * This table can double as a negative exponent value lookup table
> + * (as micro units) for calculating LSB temperature values.
> + */
> +static const int max31722_scale_table[4] = {
> +	500000, 250000, 125000, 62500
> +};
> +
> +static IIO_CONST_ATTR(in_temp_scale_available, MAX31722_SCALE_AVAILABLE);
> +
> +static struct attribute *max31722_attributes[] = {
> +	&iio_const_attr_in_temp_scale_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group max31722_attribute_group = {
> +	.attrs = max31722_attributes
> +};
> +
> +static const struct iio_chan_spec max31722_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static bool max31722_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case MAX31722_REG_TEMP_LSB:
> +	case MAX31722_REG_TEMP_MSB:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool max31722_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case MAX31722_REG_TEMP_LSB:
> +	case MAX31722_REG_TEMP_MSB:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +static struct regmap_config max31722_regmap_config = {
> +	.name = MAX31722_REGMAP_NAME,
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = MAX31722_MAX_REG,
> +	.cache_type = REGCACHE_RBTREE,
> +
> +	.volatile_reg = max31722_is_volatile_reg,
> +	.writeable_reg = max31722_is_writeable_reg,
> +
> +	.read_flag_mask = 0x00,
> +	.write_flag_mask = 0x80,
> +};
> +
> +/*
> + * Convert a temperature register value to a floating point value.
> + * Data is represented in 9-12 bits, two's complement.
> + * The MSB contains the integer part of the temperature value
> + * (signed), while the LSB contains the fractional part as
> + * negative powers of 2, e.g.: bit 0 is 2^-1, bit 1 is 2^-2, etc.
> + */
> +static void max31722_reg_to_float(u16 reg_val, int *val, int *val2)
> +{
> +	int i;
> +	u8 msb;
> +	u8 lsb;
> +
> +	msb = reg_val >> 8;
> +	lsb = (reg_val << 8) >> 8;
> +
> +	/* Calculate the integer part. */
> +	if (msb & 0x80) {
> +		/* Negative value. Reverse bits and add 1. */
> +		u16 orig = reg_val;
> +		u16 rev = 0;
> +		int num_bits = sizeof(rev) * 8;
> +
> +		for (i = 0 ; i < num_bits ; i++) {
> +			rev |= !(orig & 0x01) << i;
> +			orig >>= 1;
> +		}
> +		rev += 1;
> +		msb = rev >> 8;
> +		lsb = (rev << 8) >> 8;
> +		*val = (-1) * msb;
> +	} else { /* Positive value. */
> +		*val = msb;
> +	}
> +
> +	/*
> +	 * Calculate the fractional part.
> +	 * Only the first four LSB bits contain data.
> +	 */
> +	lsb >>= 4;
> +	i = 0;
> +	*val2 = 0;
> +	while (i < 4) {
> +		if ((lsb >> (4 - i - 1)) & 0x01)
> +			*val2 += max31722_scale_table[i];
> +		lsb <<= 1;
> +		lsb >>= 1;
> +		i++;
> +	}
> +}
> +
> +static int max31722_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	int ret;
> +	int index;
> +	u16 buf;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +	struct spi_device *spi = data->spi_device;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = regmap_bulk_read(data->regmap,
> +				       MAX31722_REG_TEMP_LSB, &buf, 2);
> +		if (ret < 0) {
> +			dev_err(&spi->dev, "failed to read temperature register\n");
> +			mutex_unlock(&data->lock);
> +			return ret;
> +		}
> +		max31722_reg_to_float(buf, val, val2);
> +		mutex_unlock(&data->lock);
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_SCALE:
> +		mutex_lock(&data->lock);
> +		ret = regmap_field_read(data->reg_resolution, &index);
> +		if (ret < 0) {
> +			dev_err(&spi->dev, "failed to read configuration register\n");
> +			mutex_unlock(&data->lock);
> +			return ret;
> +		}
> +		mutex_unlock(&data->lock);
> +		*val = 0;
> +		*val2 = max31722_scale_table[index];
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int max31722_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	int i;
> +	int ret;
> +	int index = -1;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		for (i = 0; i < ARRAY_SIZE(max31722_scale_table); i++) {
> +			if (val == 0 && val2 == max31722_scale_table[i]) {
> +				index = i;
> +				break;
> +			}
> +		}
> +		if (index < 0)
> +			return -EINVAL;
> +
> +		mutex_lock(&data->lock);
> +		ret = regmap_field_write(data->reg_resolution, index);
> +		mutex_unlock(&data->lock);
> +
> +		return ret;
> +	}
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info max31722_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= max31722_read_raw,
> +	.write_raw		= max31722_write_raw,
> +	.attrs			= &max31722_attribute_group,
> +};
> +
> +static int max31722_init(struct max31722_data *data)
> +{
> +	int ret = 0;
> +	struct spi_device *spi = data->spi_device;
> +	struct regmap *regmap;
> +
> +	/* Initialize the regmap */
> +	regmap = devm_regmap_init_spi(spi, &max31722_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "regmap initialization failed.\n");
> +		return PTR_ERR(regmap);
> +	}
> +	data->regmap = regmap;
> +
> +	MAX31722_REGFIELD(state);
> +	MAX31722_REGFIELD(resolution);
> +
> +	/* Set SD bit to 0 so we can have continuous measurements. */
> +	ret = regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
> +	if (ret < 0)
> +		dev_err(&spi->dev, "failed to configure sensor.\n");
> +
> +	return ret;
> +}
> +
> +static int max31722_probe(struct spi_device *spi)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct max31722_data *data;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
> +	if (!indio_dev) {
> +		dev_err(&spi->dev, "iio allocation failed!\n");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->spi_device = spi;
> +	spi_set_drvdata(spi, indio_dev);
> +	mutex_init(&data->lock);
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->info = &max31722_info;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = max31722_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max31722_channels);
> +
> +	ret = max31722_init(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "iio_device_register failed\n");
Given this statement is effectively unwinding max31722_init, would have
a max31722_exit function as well with this in.  That makes for
'obviously correct' code which is always good for reviewers!

Alternative would be to pull the mode setting out of the init function
and have it inline in this function making the pairing obvious as well.

> +		regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
> +	}
> +
> +	return ret;
> +}
> +
> +static int max31722_remove(struct spi_device *spi)
> +{
> +	struct max31722_data *data;
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
> +	data = iio_priv(indio_dev);

I would have put this inline with the declaration of data above.
It is pretty much the same level of lookup as spi_get_drvdata.

> +
> +	return regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int max31722_suspend(struct device *dev)
> +{
> +	struct max31722_data *data;
> +
> +	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
> +
> +	return regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
> +}
> +
> +static int max31722_resume(struct device *dev)
> +{
> +	struct max31722_data *data;
> +
> +	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
> +
> +	return regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(max31722_pm_ops, max31722_suspend, max31722_resume);
> +
> +#define MAX31722_PM_OPS (&max31722_pm_ops)
> +#else
> +#define MAX31722_PM_OPS NULL
> +#endif
> +
> +static const struct spi_device_id max31722_spi_id[] = {
> +	{"max31722", 0},
> +	{"max31723", 0},
> +	{}
> +};
> +
> +static const struct acpi_device_id max31722_acpi_id[] = {
> +	{"MAX31722", 0},
> +	{"MAX31723", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(spi, max31722_spi_id);
> +
> +static struct spi_driver max31722_driver = {
> +	.driver = {
> +		.name = "max31722",
> +		.pm = MAX31722_PM_OPS,
> +		.acpi_match_table = ACPI_PTR(max31722_acpi_id),
> +	},
> +	.probe =            max31722_probe,
> +	.remove =           max31722_remove,
> +	.id_table =         max31722_spi_id,
> +};
> +
> +module_spi_driver(max31722_driver);
> +
> +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
> +MODULE_DESCRIPTION("MAX31722 digital thermometer and thermostat SPI driver");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722
  2016-03-09 13:30 ` [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722 Tiberiu Breana
@ 2016-03-12 10:17   ` Jonathan Cameron
  2016-03-14 12:25     ` Daniel Baluta
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-12 10:17 UTC (permalink / raw)
  To: Tiberiu Breana, linux-iio

On 09/03/16 13:30, Tiberiu Breana wrote:
> Added interrupt support for high/low temperature threshold
> events to the max31722 driver.
> 
> Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
Only comment in here is that you might be better off reporting the
event direction as IIO_EVENT_DIR_EITHER and leaving the figuring out
which case occured to userspace.

This case is iritatingly common!

Jonathan
> ---
>  drivers/iio/temperature/max31722.c | 303 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 301 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/temperature/max31722.c b/drivers/iio/temperature/max31722.c
> index fa833b6..a8bfe35 100644
> --- a/drivers/iio/temperature/max31722.c
> +++ b/drivers/iio/temperature/max31722.c
> @@ -10,21 +10,32 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/acpi.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  #include <linux/spi/spi.h>
> +#include <linux/iio/events.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
>  #define MAX31722_REG_CFG				0x00
>  #define MAX31722_REG_TEMP_LSB			0x01
>  #define MAX31722_REG_TEMP_MSB			0x02
> +#define MAX31722_REG_THIGH_LSB         0x03
> +#define MAX31722_REG_TLOW_LSB          0x05
>  #define MAX31722_MAX_REG				0x86
>  
>  #define MAX31722_MODE_CONTINUOUS		0x00
>  #define MAX31722_MODE_STANDBY			0x01
> +#define MAX31722_MODE_THERMOSTAT       0x80
> +
> +#define MAX31722_MIN_TEMP          -55
> +#define MAX31722_MAX_TEMP          125
>  
>  #define MAX31722_REGMAP_NAME			"max31722_regmap"
> +#define MAX31722_EVENT             "max31722_event"
> +#define MAX31722_GPIO              "max31722_gpio"
>  
>  #define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125 0.0625"
>  
> @@ -43,6 +54,8 @@ static const struct reg_field max31722_reg_field_state =
>  				REG_FIELD(MAX31722_REG_CFG, 0, 0);
>  static const struct reg_field max31722_reg_field_resolution =
>  				REG_FIELD(MAX31722_REG_CFG, 1, 2);
> +static const struct reg_field max31722_reg_field_thermostat =
> +				REG_FIELD(MAX31722_REG_CFG, 3, 3);
>  
>  struct max31722_data {
>  	struct spi_device *spi_device;
> @@ -50,6 +63,8 @@ struct max31722_data {
>  	struct regmap *regmap;
>  	struct regmap_field *reg_state;
>  	struct regmap_field *reg_resolution;
> +	struct regmap_field *reg_thermostat;
> +	u64 timestamp;
>  };
>  
>  /*
> @@ -71,11 +86,30 @@ static const struct attribute_group max31722_attribute_group = {
>  	.attrs = max31722_attributes
>  };
>  
> +static const struct iio_event_spec max31722_events[] = {
> +	/* High temperature event */
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +	/* Low temperature event */
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
>  static const struct iio_chan_spec max31722_channels[] = {
>  	{
>  		.type = IIO_TEMP,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  				      BIT(IIO_CHAN_INFO_SCALE),
> +		.event_spec = max31722_events,
> +		.num_event_specs = ARRAY_SIZE(max31722_events),
>  	},
>  };
>  
> @@ -167,6 +201,172 @@ static void max31722_reg_to_float(u16 reg_val, int *val, int *val2)
>  	}
>  }
>  
> +/* Convert a floating point value to a register value. */
> +static u16 max31722_float_to_reg(int val, int val2)
> +{
> +	int i;
> +	bool negative_nr;
> +	u8 lsb;
> +	u16 reg_val;
> +
> +	negative_nr = (val & 0x80) || val2 < 0;
> +	if (val2 < 0)
> +		val2 *= -1;
> +
> +	/*
> +	 * The LSB value will be an approximation of val2
> +	 * due to its limited 4-bit range.
> +	 */
> +	lsb = 0;
> +	for (i = 0 ; i < ARRAY_SIZE(max31722_scale_table) && val2 > 0; i++)
> +		if (val2 - max31722_scale_table[i] >= 0) {
> +			val2 -= max31722_scale_table[i];
> +			lsb += 1 << (4 - i - 1);
> +		}
> +	lsb <<= 4;
> +
> +	if (negative_nr) {
> +		/*
> +		 * Negative value. Temporarily use the complement of val for
> +		 * the MSB, then concatenate the LSB, reverse bits & add 1 for
> +		 * the final value.
> +		 */
> +		u8 msb = (u8)(-1 * val);
> +		u16 rev = 0;
> +		int num_bits = sizeof(rev) * 8;
> +
> +		reg_val = (msb << 8) | lsb;
> +
> +		for (i = 0 ; i < num_bits ; i++) {
> +			rev |= !(reg_val & 0x01) << i;
> +			reg_val >>= 1;
> +		}
> +		rev += 1;
> +		return rev;
> +
> +	} else {
> +		reg_val = ((u8)val << 8) | lsb;
> +	}
> +
> +	return reg_val;
> +}
> +
> +static int max31722_read_event_config(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan,
> +				      enum iio_event_type type,
> +				      enum iio_event_direction dir)
> +{
> +	unsigned int event_val;
> +	int ret;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +	struct spi_device *spi = data->spi_device;
> +
> +	mutex_lock(&data->lock);
> +	ret = regmap_field_read(data->reg_thermostat, &event_val);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "failed to read thermostat status\n");
> +		mutex_unlock(&data->lock);
> +		return ret;
> +	}
> +	mutex_unlock(&data->lock);
> +
> +	return event_val;
> +}
> +
> +static int max31722_write_event_config(struct iio_dev *indio_dev,
> +				       const struct iio_chan_spec *chan,
> +				       enum iio_event_type type,
> +				       enum iio_event_direction dir,
> +				       int state)
> +{
> +	int ret;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +	struct spi_device *spi = data->spi_device;
> +
> +	if (state != 0 && state != 1)
> +		return -EINVAL;
> +
> +	/*
> +	 * Set thermostat mode:
> +	 * 0 : comparator mode (default)
> +	 * 1 : interrupt mode
> +	 */
> +	mutex_lock(&data->lock);
> +	ret = regmap_field_write(data->reg_thermostat, state);
> +	if (ret < 0)
> +		dev_err(&spi->dev, "failed to set thermostat mode\n");
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int max31722_read_event(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       enum iio_event_type type,
> +			       enum iio_event_direction dir,
> +			       enum iio_event_info info,
> +			       int *val, int *val2)
> +{
> +	int ret;
> +	u8 reg;
> +	u16 buf;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +
> +	if (info != IIO_EV_INFO_VALUE)
> +		return -EINVAL;
> +
> +	if (dir == IIO_EV_DIR_RISING)
> +		reg = MAX31722_REG_THIGH_LSB;
> +	else if (dir == IIO_EV_DIR_FALLING)
> +		reg = MAX31722_REG_TLOW_LSB;
> +	else
> +		return -EINVAL;
> +
> +	mutex_lock(&data->lock);
> +	ret = regmap_bulk_read(data->regmap, reg, &buf, 2);
> +	mutex_unlock(&data->lock);
> +	if (ret < 0) {
> +		dev_err(&data->spi_device->dev,
> +			"failed to read threshold register\n");
> +		return ret;
> +	}
> +	max31722_reg_to_float(buf, val, val2);
> +
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int max31722_write_event(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info,
> +				int val, int val2)
> +{
> +	int ret;
> +	u8 reg;
> +	u16 buf;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +
> +	if (val < MAX31722_MIN_TEMP || val > MAX31722_MAX_TEMP)
> +		return -EINVAL;
> +
> +	if (dir == IIO_EV_DIR_RISING)
> +		reg = MAX31722_REG_THIGH_LSB;
> +	else if (dir == IIO_EV_DIR_FALLING)
> +		reg = MAX31722_REG_TLOW_LSB;
> +	else
> +		return -EINVAL;
> +
> +	buf = max31722_float_to_reg(val, val2);
> +
> +	ret = regmap_bulk_write(data->regmap, reg, &buf, 2);
> +	if (ret < 0)
> +		dev_err(&data->spi_device->dev,
> +			"failed to write threshold register\n");
> +
> +	return ret;
> +}
> +
>  static int max31722_read_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan,
>  			     int *val, int *val2, long mask)
> @@ -241,8 +441,63 @@ static const struct iio_info max31722_info = {
>  	.read_raw		= max31722_read_raw,
>  	.write_raw		= max31722_write_raw,
>  	.attrs			= &max31722_attribute_group,
> +	.read_event_value	= max31722_read_event,
> +	.write_event_value	= max31722_write_event,
> +	.read_event_config	= max31722_read_event_config,
> +	.write_event_config	= max31722_write_event_config,
>  };
>  
> +static irqreturn_t max31722_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +
> +	data->timestamp = iio_get_time_ns();
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t max31722_irq_event_handler(int irq, void *private)
> +{
> +	int ret;
> +	u64 event;
> +	u16 temp;
> +	u16 tlow;
> +	u16 thigh;
> +	unsigned int dir;
> +	struct iio_dev *indio_dev = private;
> +	struct max31722_data *data = iio_priv(indio_dev);
> +
> +	/*
> +	 * Do a quick temperature reading and compare it with TLOW/THIGH
> +	 * so we can tell which threshold has been met.
Hmm. Might end up detecting neither occured if the condition is no longer
met.  Iritating hardware!

We do have the 'magic' option of IIO_EV_DIR_EITHER to specify that we
don't know which one occured.  The idea of that has always been that
userspace can elect to do what you have here if it cares rather than
pushing this code into drivers.
> +	 */
> +	mutex_lock(&data->lock);
> +	ret = regmap_bulk_read(data->regmap, MAX31722_REG_TEMP_LSB, &temp, 2);
> +	if (ret < 0)
> +		goto exit_err;
> +	ret = regmap_bulk_read(data->regmap, MAX31722_REG_TLOW_LSB, &tlow, 2);
> +	if (ret < 0)
> +		goto exit_err;
> +	ret = regmap_bulk_read(data->regmap, MAX31722_REG_THIGH_LSB, &thigh, 2);
> +	if (ret < 0)
> +		goto exit_err;
> +	mutex_unlock(&data->lock);
> +
> +	dir = (temp > thigh ? 1 : 0);
> +	event = IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0, IIO_EV_TYPE_THRESH,
> +				     (dir ? IIO_EV_DIR_RISING :
> +					    IIO_EV_DIR_FALLING));
> +	iio_push_event(indio_dev, event, data->timestamp);
> +
> +	return IRQ_HANDLED;
> +
> +exit_err:
> +	dev_err(&data->spi_device->dev, "failed to read temperature register\n");
> +	mutex_unlock(&data->lock);
> +	return ret;
> +}
> +
>  static int max31722_init(struct max31722_data *data)
>  {
>  	int ret = 0;
> @@ -259,15 +514,40 @@ static int max31722_init(struct max31722_data *data)
>  
>  	MAX31722_REGFIELD(state);
>  	MAX31722_REGFIELD(resolution);
> +	MAX31722_REGFIELD(thermostat);
>  
>  	/* Set SD bit to 0 so we can have continuous measurements. */
>  	ret = regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "failed to configure sensor.\n");
> +		return ret;
> +	}
> +
> +	/* Set thermostat interrupt mode */
> +	ret = regmap_field_write(data->reg_thermostat, 1);
>  	if (ret < 0)
>  		dev_err(&spi->dev, "failed to configure sensor.\n");
>  
>  	return ret;
>  }
>  
> +static int max31722_gpio_probe(struct spi_device *spi)
> +{
> +	struct gpio_desc *gpio;
> +
> +	if (!spi)
> +		return -EINVAL;
> +
> +	/* gpio interrupt pin */
> +	gpio = devm_gpiod_get_index(&spi->dev, MAX31722_GPIO, 0, GPIOD_IN);
> +	if (IS_ERR(gpio)) {
> +		dev_err(&spi->dev, "gpio request failed.\n");
> +		return PTR_ERR(gpio);
> +	}
If this is an interrupt, why are we specifying it as a gpio?
> +
> +	return gpiod_to_irq(gpio);
> +}
> +
>  static int max31722_probe(struct spi_device *spi)
>  {
>  	int ret;
> @@ -294,15 +574,34 @@ static int max31722_probe(struct spi_device *spi)
>  
>  	ret = max31722_init(data);
>  	if (ret < 0)
> -		return ret;
> +		goto err_standby;
> +
> +	ret = max31722_gpio_probe(data->spi_device);
> +	if (ret < 0)
> +		goto err_standby;
> +
> +	spi->irq = ret;
> +	ret = devm_request_threaded_irq(&spi->dev, spi->irq,
> +					max31722_irq_handler,
> +					max31722_irq_event_handler,
> +					IRQF_TRIGGER_LOW,
> +					MAX31722_EVENT, indio_dev);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "request irq %d failed\n", spi->irq);
> +		goto err_standby;
> +	}
I would suggest you want to make the irq optional, though that could happen
at a later date when someone has a board where it isn't wired.
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0) {
>  		dev_err(&spi->dev, "iio_device_register failed\n");
> -		regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
> +		goto err_standby;
>  	}
>  
>  	return ret;
> +
> +err_standby:
> +	regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
> +	return ret;
>  }
>  
>  static int max31722_remove(struct spi_device *spi)
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722
  2016-03-12 10:17   ` Jonathan Cameron
@ 2016-03-14 12:25     ` Daniel Baluta
  2016-03-14 15:06       ` Breana, Tiberiu A
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Baluta @ 2016-03-14 12:25 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Tiberiu Breana, linux-iio

On Sat, Mar 12, 2016 at 12:17 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 09/03/16 13:30, Tiberiu Breana wrote:
>> Added interrupt support for high/low temperature threshold
>> events to the max31722 driver.
>>
>> Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
> Only comment in here is that you might be better off reporting the
> event direction as IIO_EVENT_DIR_EITHER and leaving the figuring out
> which case occured to userspace.
>
> This case is iritatingly common!
>
> Jonathan
>> ---
>>  drivers/iio/temperature/max31722.c | 303 ++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 301 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/temperature/max31722.c b/drivers/iio/temperature/max31722.c
>> index fa833b6..a8bfe35 100644
>> --- a/drivers/iio/temperature/max31722.c
>> +++ b/drivers/iio/temperature/max31722.c
>> @@ -10,21 +10,32 @@
>>
>>  #include <linux/kernel.h>
>>  #include <linux/acpi.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/interrupt.h>
>>  #include <linux/module.h>
>>  #include <linux/regmap.h>
>>  #include <linux/spi/spi.h>
>> +#include <linux/iio/events.h>
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>>
>>  #define MAX31722_REG_CFG                             0x00
>>  #define MAX31722_REG_TEMP_LSB                        0x01
>>  #define MAX31722_REG_TEMP_MSB                        0x02
>> +#define MAX31722_REG_THIGH_LSB         0x03
>> +#define MAX31722_REG_TLOW_LSB          0x05
>>  #define MAX31722_MAX_REG                             0x86
>>
>>  #define MAX31722_MODE_CONTINUOUS             0x00
>>  #define MAX31722_MODE_STANDBY                        0x01
>> +#define MAX31722_MODE_THERMOSTAT       0x80
>> +
>> +#define MAX31722_MIN_TEMP          -55
>> +#define MAX31722_MAX_TEMP          125
>>
>>  #define MAX31722_REGMAP_NAME                 "max31722_regmap"
>> +#define MAX31722_EVENT             "max31722_event"
>> +#define MAX31722_GPIO              "max31722_gpio"
>>
>>  #define MAX31722_SCALE_AVAILABLE             "0.5 0.25 0.125 0.0625"
>>
>> @@ -43,6 +54,8 @@ static const struct reg_field max31722_reg_field_state =
>>                               REG_FIELD(MAX31722_REG_CFG, 0, 0);
>>  static const struct reg_field max31722_reg_field_resolution =
>>                               REG_FIELD(MAX31722_REG_CFG, 1, 2);
>> +static const struct reg_field max31722_reg_field_thermostat =
>> +                             REG_FIELD(MAX31722_REG_CFG, 3, 3);
>>
>>  struct max31722_data {
>>       struct spi_device *spi_device;
>> @@ -50,6 +63,8 @@ struct max31722_data {
>>       struct regmap *regmap;
>>       struct regmap_field *reg_state;
>>       struct regmap_field *reg_resolution;
>> +     struct regmap_field *reg_thermostat;
>> +     u64 timestamp;
>>  };
>>
>>  /*
>> @@ -71,11 +86,30 @@ static const struct attribute_group max31722_attribute_group = {
>>       .attrs = max31722_attributes
>>  };
>>
>> +static const struct iio_event_spec max31722_events[] = {
>> +     /* High temperature event */
>> +     {
>> +             .type = IIO_EV_TYPE_THRESH,
>> +             .dir = IIO_EV_DIR_RISING,
>> +             .mask_separate = BIT(IIO_EV_INFO_VALUE) |
>> +                              BIT(IIO_EV_INFO_ENABLE),
>> +     },
>> +     /* Low temperature event */
>> +     {
>> +             .type = IIO_EV_TYPE_THRESH,
>> +             .dir = IIO_EV_DIR_FALLING,
>> +             .mask_separate = BIT(IIO_EV_INFO_VALUE) |
>> +                              BIT(IIO_EV_INFO_ENABLE),
>> +     },
>> +};
>> +
>>  static const struct iio_chan_spec max31722_channels[] = {
>>       {
>>               .type = IIO_TEMP,
>>               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>                                     BIT(IIO_CHAN_INFO_SCALE),
>> +             .event_spec = max31722_events,
>> +             .num_event_specs = ARRAY_SIZE(max31722_events),
>>       },
>>  };
>>
>> @@ -167,6 +201,172 @@ static void max31722_reg_to_float(u16 reg_val, int *val, int *val2)
>>       }
>>  }
>>
>> +/* Convert a floating point value to a register value. */
>> +static u16 max31722_float_to_reg(int val, int val2)
>> +{
>> +     int i;
>> +     bool negative_nr;
>> +     u8 lsb;
>> +     u16 reg_val;
>> +
>> +     negative_nr = (val & 0x80) || val2 < 0;
>> +     if (val2 < 0)
>> +             val2 *= -1;
>> +
>> +     /*
>> +      * The LSB value will be an approximation of val2
>> +      * due to its limited 4-bit range.
>> +      */
>> +     lsb = 0;
>> +     for (i = 0 ; i < ARRAY_SIZE(max31722_scale_table) && val2 > 0; i++)
>> +             if (val2 - max31722_scale_table[i] >= 0) {
>> +                     val2 -= max31722_scale_table[i];
>> +                     lsb += 1 << (4 - i - 1);
>> +             }
>> +     lsb <<= 4;
>> +
>> +     if (negative_nr) {
>> +             /*
>> +              * Negative value. Temporarily use the complement of val for
>> +              * the MSB, then concatenate the LSB, reverse bits & add 1 for
>> +              * the final value.
>> +              */
>> +             u8 msb = (u8)(-1 * val);
>> +             u16 rev = 0;
>> +             int num_bits = sizeof(rev) * 8;
>> +
>> +             reg_val = (msb << 8) | lsb;
>> +
>> +             for (i = 0 ; i < num_bits ; i++) {
>> +                     rev |= !(reg_val & 0x01) << i;
>> +                     reg_val >>= 1;
>> +             }
>> +             rev += 1;
>> +             return rev;
>> +
>> +     } else {
>> +             reg_val = ((u8)val << 8) | lsb;
>> +     }
>> +
>> +     return reg_val;
>> +}
>> +
>> +static int max31722_read_event_config(struct iio_dev *indio_dev,
>> +                                   const struct iio_chan_spec *chan,
>> +                                   enum iio_event_type type,
>> +                                   enum iio_event_direction dir)
>> +{
>> +     unsigned int event_val;
>> +     int ret;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +     struct spi_device *spi = data->spi_device;
>> +
>> +     mutex_lock(&data->lock);
>> +     ret = regmap_field_read(data->reg_thermostat, &event_val);
>> +     if (ret < 0) {
>> +             dev_err(&spi->dev, "failed to read thermostat status\n");
>> +             mutex_unlock(&data->lock);
>> +             return ret;
>> +     }
>> +     mutex_unlock(&data->lock);
>> +
>> +     return event_val;
>> +}
>> +
>> +static int max31722_write_event_config(struct iio_dev *indio_dev,
>> +                                    const struct iio_chan_spec *chan,
>> +                                    enum iio_event_type type,
>> +                                    enum iio_event_direction dir,
>> +                                    int state)
>> +{
>> +     int ret;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +     struct spi_device *spi = data->spi_device;
>> +
>> +     if (state != 0 && state != 1)
>> +             return -EINVAL;
>> +
>> +     /*
>> +      * Set thermostat mode:
>> +      * 0 : comparator mode (default)
>> +      * 1 : interrupt mode
>> +      */
>> +     mutex_lock(&data->lock);
>> +     ret = regmap_field_write(data->reg_thermostat, state);
>> +     if (ret < 0)
>> +             dev_err(&spi->dev, "failed to set thermostat mode\n");
>> +     mutex_unlock(&data->lock);
>> +
>> +     return ret;
>> +}
>> +
>> +static int max31722_read_event(struct iio_dev *indio_dev,
>> +                            const struct iio_chan_spec *chan,
>> +                            enum iio_event_type type,
>> +                            enum iio_event_direction dir,
>> +                            enum iio_event_info info,
>> +                            int *val, int *val2)
>> +{
>> +     int ret;
>> +     u8 reg;
>> +     u16 buf;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +
>> +     if (info != IIO_EV_INFO_VALUE)
>> +             return -EINVAL;
>> +
>> +     if (dir == IIO_EV_DIR_RISING)
>> +             reg = MAX31722_REG_THIGH_LSB;
>> +     else if (dir == IIO_EV_DIR_FALLING)
>> +             reg = MAX31722_REG_TLOW_LSB;
>> +     else
>> +             return -EINVAL;
>> +
>> +     mutex_lock(&data->lock);
>> +     ret = regmap_bulk_read(data->regmap, reg, &buf, 2);
>> +     mutex_unlock(&data->lock);
>> +     if (ret < 0) {
>> +             dev_err(&data->spi_device->dev,
>> +                     "failed to read threshold register\n");
>> +             return ret;
>> +     }
>> +     max31722_reg_to_float(buf, val, val2);
>> +
>> +     return IIO_VAL_INT_PLUS_MICRO;
>> +}
>> +
>> +static int max31722_write_event(struct iio_dev *indio_dev,
>> +                             const struct iio_chan_spec *chan,
>> +                             enum iio_event_type type,
>> +                             enum iio_event_direction dir,
>> +                             enum iio_event_info info,
>> +                             int val, int val2)
>> +{
>> +     int ret;
>> +     u8 reg;
>> +     u16 buf;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +
>> +     if (val < MAX31722_MIN_TEMP || val > MAX31722_MAX_TEMP)
>> +             return -EINVAL;
>> +
>> +     if (dir == IIO_EV_DIR_RISING)
>> +             reg = MAX31722_REG_THIGH_LSB;
>> +     else if (dir == IIO_EV_DIR_FALLING)
>> +             reg = MAX31722_REG_TLOW_LSB;
>> +     else
>> +             return -EINVAL;
>> +
>> +     buf = max31722_float_to_reg(val, val2);
>> +
>> +     ret = regmap_bulk_write(data->regmap, reg, &buf, 2);
>> +     if (ret < 0)
>> +             dev_err(&data->spi_device->dev,
>> +                     "failed to write threshold register\n");
>> +
>> +     return ret;
>> +}
>> +
>>  static int max31722_read_raw(struct iio_dev *indio_dev,
>>                            struct iio_chan_spec const *chan,
>>                            int *val, int *val2, long mask)
>> @@ -241,8 +441,63 @@ static const struct iio_info max31722_info = {
>>       .read_raw               = max31722_read_raw,
>>       .write_raw              = max31722_write_raw,
>>       .attrs                  = &max31722_attribute_group,
>> +     .read_event_value       = max31722_read_event,
>> +     .write_event_value      = max31722_write_event,
>> +     .read_event_config      = max31722_read_event_config,
>> +     .write_event_config     = max31722_write_event_config,
>>  };
>>
>> +static irqreturn_t max31722_irq_handler(int irq, void *private)
>> +{
>> +     struct iio_dev *indio_dev = private;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +
>> +     data->timestamp = iio_get_time_ns();
>> +
>> +     return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t max31722_irq_event_handler(int irq, void *private)
>> +{
>> +     int ret;
>> +     u64 event;
>> +     u16 temp;
>> +     u16 tlow;
>> +     u16 thigh;
>> +     unsigned int dir;
>> +     struct iio_dev *indio_dev = private;
>> +     struct max31722_data *data = iio_priv(indio_dev);
>> +
>> +     /*
>> +      * Do a quick temperature reading and compare it with TLOW/THIGH
>> +      * so we can tell which threshold has been met.
> Hmm. Might end up detecting neither occured if the condition is no longer
> met.  Iritating hardware!
>
> We do have the 'magic' option of IIO_EV_DIR_EITHER to specify that we
> don't know which one occured.  The idea of that has always been that
> userspace can elect to do what you have here if it cares rather than
> pushing this code into drivers.
>> +      */
>> +     mutex_lock(&data->lock);
>> +     ret = regmap_bulk_read(data->regmap, MAX31722_REG_TEMP_LSB, &temp, 2);
>> +     if (ret < 0)
>> +             goto exit_err;
>> +     ret = regmap_bulk_read(data->regmap, MAX31722_REG_TLOW_LSB, &tlow, 2);
>> +     if (ret < 0)
>> +             goto exit_err;
>> +     ret = regmap_bulk_read(data->regmap, MAX31722_REG_THIGH_LSB, &thigh, 2);
>> +     if (ret < 0)
>> +             goto exit_err;
>> +     mutex_unlock(&data->lock);
>> +
>> +     dir = (temp > thigh ? 1 : 0);
>> +     event = IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0, IIO_EV_TYPE_THRESH,
>> +                                  (dir ? IIO_EV_DIR_RISING :
>> +                                         IIO_EV_DIR_FALLING));
>> +     iio_push_event(indio_dev, event, data->timestamp);
>> +
>> +     return IRQ_HANDLED;
>> +
>> +exit_err:
>> +     dev_err(&data->spi_device->dev, "failed to read temperature register\n");
>> +     mutex_unlock(&data->lock);
>> +     return ret;
>> +}
>> +
>>  static int max31722_init(struct max31722_data *data)
>>  {
>>       int ret = 0;
>> @@ -259,15 +514,40 @@ static int max31722_init(struct max31722_data *data)
>>
>>       MAX31722_REGFIELD(state);
>>       MAX31722_REGFIELD(resolution);
>> +     MAX31722_REGFIELD(thermostat);
>>
>>       /* Set SD bit to 0 so we can have continuous measurements. */
>>       ret = regmap_field_write(data->reg_state, MAX31722_MODE_CONTINUOUS);
>> +     if (ret < 0) {
>> +             dev_err(&spi->dev, "failed to configure sensor.\n");
>> +             return ret;
>> +     }
>> +
>> +     /* Set thermostat interrupt mode */
>> +     ret = regmap_field_write(data->reg_thermostat, 1);
>>       if (ret < 0)
>>               dev_err(&spi->dev, "failed to configure sensor.\n");
>>
>>       return ret;
>>  }
>>
>> +static int max31722_gpio_probe(struct spi_device *spi)
>> +{
>> +     struct gpio_desc *gpio;
>> +
>> +     if (!spi)
>> +             return -EINVAL;
>> +
>> +     /* gpio interrupt pin */
>> +     gpio = devm_gpiod_get_index(&spi->dev, MAX31722_GPIO, 0, GPIOD_IN);
>> +     if (IS_ERR(gpio)) {
>> +             dev_err(&spi->dev, "gpio request failed.\n");
>> +             return PTR_ERR(gpio);
>> +     }
> If this is an interrupt, why are we specifying it as a gpio?

It is an interrupt requested on a GPIO pin. Isn't this the standard
way of mapping
a GPIO pin to an IRQ? Anyhow, as explained below this is no longer needed.

>> +
>> +     return gpiod_to_irq(gpio);
>> +}
>> +
>>  static int max31722_probe(struct spi_device *spi)
>>  {
>>       int ret;
>> @@ -294,15 +574,34 @@ static int max31722_probe(struct spi_device *spi)
>>
>>       ret = max31722_init(data);
>>       if (ret < 0)
>> -             return ret;
>> +             goto err_standby;
>> +
>> +     ret = max31722_gpio_probe(data->spi_device);
>> +     if (ret < 0)
>> +             goto err_standby;
>> +
>> +     spi->irq = ret;

If ACPI or DT is properly configured spi->irq should already contain the correct
irq number. We used the gpio_probe approach when ACPI didn't save the interrupt
number. Newer kernel version shouldn't have this problem.

>> +     ret = devm_request_threaded_irq(&spi->dev, spi->irq,
>> +                                     max31722_irq_handler,
>> +                                     max31722_irq_event_handler,
>> +                                     IRQF_TRIGGER_LOW,
>> +                                     MAX31722_EVENT, indio_dev);
>> +     if (ret < 0) {
>> +             dev_err(&spi->dev, "request irq %d failed\n", spi->irq);
>> +             goto err_standby;
>> +     }
> I would suggest you want to make the irq optional, though that could happen
> at a later date when someone has a board where it isn't wired.

Agree. We had previous experience with unwired interrupts pins.

>>
>>       ret = iio_device_register(indio_dev);
>>       if (ret < 0) {
>>               dev_err(&spi->dev, "iio_device_register failed\n");
>> -             regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
>> +             goto err_standby;
>>       }
>>
>>       return ret;
>> +
>> +err_standby:
>> +     regmap_field_write(data->reg_state, MAX31722_MODE_STANDBY);
>> +     return ret;
>>  }
>>
>>  static int max31722_remove(struct spi_device *spi)
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] iio: temperature: Add support for MAX31722/MAX31723 temperature sensors
  2016-03-12  9:57   ` Jonathan Cameron
@ 2016-03-14 13:45     ` Breana, Tiberiu A
  2016-03-14 17:31       ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-14 13:45 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio; +Cc: Baluta, Daniel

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Saturday, March 12, 2016 11:57 AM
> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> iio@vger.kernel.org
> Subject: Re: [PATCH 1/2] iio: temperature: Add support for
> MAX31722/MAX31723 temperature sensors
>=20
> On 09/03/16 13:30, Tiberiu Breana wrote:
> > Add basic support for the Maxim Integrated MAX31722/MAX31723 SPI
> > temperature sensors / thermostats.
> >
> > Includes:
> > 	- ACPI support;
> > 	- raw temperature readings;
> > 	- ability to set measurement resolution;
> > 	- power management
> >
> > Datasheet:
> > https://datasheets.maximintegrated.com/en/ds/MAX31722-
> MAX31723.pdf
> >
> > Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
> I thought I'd give this a quick review, putting aside the question of whe=
ther it
> should be in IIO at all.

What are the criteria for a temperature driver to be in IIO vs hwmon? I see=
 there's another temperature sensor in IIO - TSYS01 (http://www.meas-spec.c=
om/product/temperature/TSYS01.aspx) which is used for "Industrial Control, =
Replacement of Thermistors and NTCs, Heating / Cooling Systems, HVAC".

>=20
> A few really trivial suggestions, but otherwise a nice clean driver.
>=20
> Jonathan
> > ---
> >  drivers/iio/temperature/Kconfig    |  12 ++
> >  drivers/iio/temperature/Makefile   |   1 +
> >  drivers/iio/temperature/max31722.c | 374
> > +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 387 insertions(+)
> >  create mode 100644 drivers/iio/temperature/max31722.c
> >
> > diff --git a/drivers/iio/temperature/Kconfig
> > b/drivers/iio/temperature/Kconfig index c4664e5..7587887 100644
> > --- a/drivers/iio/temperature/Kconfig
> > +++ b/drivers/iio/temperature/Kconfig
> > @@ -3,6 +3,18 @@
> >  #
> >  menu "Temperature sensors"
> >
> > +config MAX31722
> > +	tristate "MAX31722 temperature sensor"
> > +	depends on SPI
> > +	select REGMAP_SPI
> > +	help
> > +	  If you say yes here you get support for the Maxim Integrated
> > +	  MAX31722/MAX31723 digital thermometers / thermostats
> operating
> > +	  over an SPI interface.
> > +
> > +	  This driver can also be built as a module. If so, the module will
> > +	  be called max31722.
> > +
> >  config MLX90614
> >  	tristate "MLX90614 contact-less infrared sensor"
> >  	depends on I2C
> > diff --git a/drivers/iio/temperature/Makefile
> > b/drivers/iio/temperature/Makefile
> > index 02bc79d..a9484d6 100644
> > --- a/drivers/iio/temperature/Makefile
> > +++ b/drivers/iio/temperature/Makefile
> > @@ -2,6 +2,7 @@
> >  # Makefile for industrial I/O temperature drivers  #
> >
> > +obj-$(CONFIG_MAX31722) +=3D max31722.o
> >  obj-$(CONFIG_MLX90614) +=3D mlx90614.o
> >  obj-$(CONFIG_TMP006) +=3D tmp006.o
> >  obj-$(CONFIG_TSYS01) +=3D tsys01.o
> > diff --git a/drivers/iio/temperature/max31722.c
> > b/drivers/iio/temperature/max31722.c
> > new file mode 100644
> > index 0000000..fa833b6
> > --- /dev/null
> > +++ b/drivers/iio/temperature/max31722.c
> > @@ -0,0 +1,374 @@
> > +/**
> > + * MAX31722/MAX31723 digital thermometer and thermostat SPI driver
> > + *
> > + * Copyright (c) 2016, Intel Corporation.
> > + *
> > + * This file is subject to the terms and conditions of version 2 of
> > + * the GNU General Public License. See the file COPYING in the main
> > + * directory of this archive for more details.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/acpi.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +
> > +#define MAX31722_REG_CFG				0x00
> > +#define MAX31722_REG_TEMP_LSB			0x01
> > +#define MAX31722_REG_TEMP_MSB			0x02
> > +#define MAX31722_MAX_REG				0x86
> > +
> > +#define MAX31722_MODE_CONTINUOUS		0x00
> > +#define MAX31722_MODE_STANDBY			0x01
> > +
> > +#define MAX31722_REGMAP_NAME
> 	"max31722_regmap"
> > +
> > +#define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125
> 0.0625"
> > +
> > +#define MAX31722_REGFIELD(name)
> 		 \
> > +	do {								 \
> > +		data->reg_##name =3D					 \
> > +			devm_regmap_field_alloc(&spi->dev, regmap,	 \
> > +				max31722_reg_field_##name);
> 	 \
> > +		if (IS_ERR(data->reg_##name)) {
> 	 \
> > +			dev_err(&spi->dev, "reg field alloc failed.\n"); \
> > +			return PTR_ERR(data->reg_##name);		 \
> > +		}							 \
> > +	} while (0)
> > +
> > +static const struct reg_field max31722_reg_field_state =3D
> > +				REG_FIELD(MAX31722_REG_CFG, 0, 0); static
> const struct reg_field
> > +max31722_reg_field_resolution =3D
> > +				REG_FIELD(MAX31722_REG_CFG, 1, 2);
> > +
> > +struct max31722_data {
> > +	struct spi_device *spi_device;
> > +	struct mutex lock;
> > +	struct regmap *regmap;
> > +	struct regmap_field *reg_state;
> > +	struct regmap_field *reg_resolution; };
> > +
> > +/*
> > + * This table can double as a negative exponent value lookup table
> > + * (as micro units) for calculating LSB temperature values.
> > + */
> > +static const int max31722_scale_table[4] =3D {
> > +	500000, 250000, 125000, 62500
> > +};
> > +
> > +static IIO_CONST_ATTR(in_temp_scale_available,
> > +MAX31722_SCALE_AVAILABLE);
> > +
> > +static struct attribute *max31722_attributes[] =3D {
> > +	&iio_const_attr_in_temp_scale_available.dev_attr.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group max31722_attribute_group =3D {
> > +	.attrs =3D max31722_attributes
> > +};
> > +
> > +static const struct iio_chan_spec max31722_channels[] =3D {
> > +	{
> > +		.type =3D IIO_TEMP,
> > +		.info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW) |
> > +				      BIT(IIO_CHAN_INFO_SCALE),
> > +	},
> > +};
> > +
> > +static bool max31722_is_volatile_reg(struct device *dev, unsigned int
> > +reg) {
> > +	switch (reg) {
> > +	case MAX31722_REG_TEMP_LSB:
> > +	case MAX31722_REG_TEMP_MSB:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> > +static bool max31722_is_writeable_reg(struct device *dev, unsigned
> > +int reg) {
> > +	switch (reg) {
> > +	case MAX31722_REG_TEMP_LSB:
> > +	case MAX31722_REG_TEMP_MSB:
> > +		return false;
> > +	default:
> > +		return true;
> > +	}
> > +}
> > +
> > +static struct regmap_config max31722_regmap_config =3D {
> > +	.name =3D MAX31722_REGMAP_NAME,
> > +	.reg_bits =3D 8,
> > +	.val_bits =3D 8,
> > +
> > +	.max_register =3D MAX31722_MAX_REG,
> > +	.cache_type =3D REGCACHE_RBTREE,
> > +
> > +	.volatile_reg =3D max31722_is_volatile_reg,
> > +	.writeable_reg =3D max31722_is_writeable_reg,
> > +
> > +	.read_flag_mask =3D 0x00,
> > +	.write_flag_mask =3D 0x80,
> > +};
> > +
> > +/*
> > + * Convert a temperature register value to a floating point value.
> > + * Data is represented in 9-12 bits, two's complement.
> > + * The MSB contains the integer part of the temperature value
> > + * (signed), while the LSB contains the fractional part as
> > + * negative powers of 2, e.g.: bit 0 is 2^-1, bit 1 is 2^-2, etc.
> > + */
> > +static void max31722_reg_to_float(u16 reg_val, int *val, int *val2) {
> > +	int i;
> > +	u8 msb;
> > +	u8 lsb;
> > +
> > +	msb =3D reg_val >> 8;
> > +	lsb =3D (reg_val << 8) >> 8;
> > +
> > +	/* Calculate the integer part. */
> > +	if (msb & 0x80) {
> > +		/* Negative value. Reverse bits and add 1. */
> > +		u16 orig =3D reg_val;
> > +		u16 rev =3D 0;
> > +		int num_bits =3D sizeof(rev) * 8;
> > +
> > +		for (i =3D 0 ; i < num_bits ; i++) {
> > +			rev |=3D !(orig & 0x01) << i;
> > +			orig >>=3D 1;
> > +		}
> > +		rev +=3D 1;
> > +		msb =3D rev >> 8;
> > +		lsb =3D (rev << 8) >> 8;
> > +		*val =3D (-1) * msb;
> > +	} else { /* Positive value. */
> > +		*val =3D msb;
> > +	}
> > +
> > +	/*
> > +	 * Calculate the fractional part.
> > +	 * Only the first four LSB bits contain data.
> > +	 */
> > +	lsb >>=3D 4;
> > +	i =3D 0;
> > +	*val2 =3D 0;
> > +	while (i < 4) {
> > +		if ((lsb >> (4 - i - 1)) & 0x01)
> > +			*val2 +=3D max31722_scale_table[i];
> > +		lsb <<=3D 1;
> > +		lsb >>=3D 1;
> > +		i++;
> > +	}
> > +}
> > +
> > +static int max31722_read_raw(struct iio_dev *indio_dev,
> > +			     struct iio_chan_spec const *chan,
> > +			     int *val, int *val2, long mask) {
> > +	int ret;
> > +	int index;
> > +	u16 buf;
> > +	struct max31722_data *data =3D iio_priv(indio_dev);
> > +	struct spi_device *spi =3D data->spi_device;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		mutex_lock(&data->lock);
> > +		ret =3D regmap_bulk_read(data->regmap,
> > +				       MAX31722_REG_TEMP_LSB, &buf, 2);
> > +		if (ret < 0) {
> > +			dev_err(&spi->dev, "failed to read temperature
> register\n");
> > +			mutex_unlock(&data->lock);
> > +			return ret;
> > +		}
> > +		max31722_reg_to_float(buf, val, val2);
> > +		mutex_unlock(&data->lock);
> > +		return IIO_VAL_INT_PLUS_MICRO;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		mutex_lock(&data->lock);
> > +		ret =3D regmap_field_read(data->reg_resolution, &index);
> > +		if (ret < 0) {
> > +			dev_err(&spi->dev, "failed to read configuration
> register\n");
> > +			mutex_unlock(&data->lock);
> > +			return ret;
> > +		}
> > +		mutex_unlock(&data->lock);
> > +		*val =3D 0;
> > +		*val2 =3D max31722_scale_table[index];
> > +		return IIO_VAL_INT_PLUS_MICRO;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int max31722_write_raw(struct iio_dev *indio_dev,
> > +			      struct iio_chan_spec const *chan,
> > +			      int val, int val2, long mask) {
> > +	int i;
> > +	int ret;
> > +	int index =3D -1;
> > +	struct max31722_data *data =3D iio_priv(indio_dev);
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_SCALE:
> > +		for (i =3D 0; i < ARRAY_SIZE(max31722_scale_table); i++) {
> > +			if (val =3D=3D 0 && val2 =3D=3D max31722_scale_table[i]) {
> > +				index =3D i;
> > +				break;
> > +			}
> > +		}
> > +		if (index < 0)
> > +			return -EINVAL;
> > +
> > +		mutex_lock(&data->lock);
> > +		ret =3D regmap_field_write(data->reg_resolution, index);
> > +		mutex_unlock(&data->lock);
> > +
> > +		return ret;
> > +	}
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct iio_info max31722_info =3D {
> > +	.driver_module		=3D THIS_MODULE,
> > +	.read_raw		=3D max31722_read_raw,
> > +	.write_raw		=3D max31722_write_raw,
> > +	.attrs			=3D &max31722_attribute_group,
> > +};
> > +
> > +static int max31722_init(struct max31722_data *data) {
> > +	int ret =3D 0;
> > +	struct spi_device *spi =3D data->spi_device;
> > +	struct regmap *regmap;
> > +
> > +	/* Initialize the regmap */
> > +	regmap =3D devm_regmap_init_spi(spi, &max31722_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		dev_err(&spi->dev, "regmap initialization failed.\n");
> > +		return PTR_ERR(regmap);
> > +	}
> > +	data->regmap =3D regmap;
> > +
> > +	MAX31722_REGFIELD(state);
> > +	MAX31722_REGFIELD(resolution);
> > +
> > +	/* Set SD bit to 0 so we can have continuous measurements. */
> > +	ret =3D regmap_field_write(data->reg_state,
> MAX31722_MODE_CONTINUOUS);
> > +	if (ret < 0)
> > +		dev_err(&spi->dev, "failed to configure sensor.\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int max31722_probe(struct spi_device *spi) {
> > +	int ret;
> > +	struct iio_dev *indio_dev;
> > +	struct max31722_data *data;
> > +
> > +	indio_dev =3D devm_iio_device_alloc(&spi->dev, sizeof(*data));
> > +	if (!indio_dev) {
> > +		dev_err(&spi->dev, "iio allocation failed!\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	data =3D iio_priv(indio_dev);
> > +	data->spi_device =3D spi;
> > +	spi_set_drvdata(spi, indio_dev);
> > +	mutex_init(&data->lock);
> > +
> > +	indio_dev->dev.parent =3D &spi->dev;
> > +	indio_dev->info =3D &max31722_info;
> > +	indio_dev->name =3D spi_get_device_id(spi)->name;
> > +	indio_dev->modes =3D INDIO_DIRECT_MODE;
> > +	indio_dev->channels =3D max31722_channels;
> > +	indio_dev->num_channels =3D ARRAY_SIZE(max31722_channels);
> > +
> > +	ret =3D max31722_init(data);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret =3D iio_device_register(indio_dev);
> > +	if (ret < 0) {
> > +		dev_err(&spi->dev, "iio_device_register failed\n");
> Given this statement is effectively unwinding max31722_init, would have a
> max31722_exit function as well with this in.  That makes for 'obviously
> correct' code which is always good for reviewers!
>=20
> Alternative would be to pull the mode setting out of the init function an=
d
> have it inline in this function making the pairing obvious as well.
>=20
> > +		regmap_field_write(data->reg_state,
> MAX31722_MODE_STANDBY);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int max31722_remove(struct spi_device *spi) {
> > +	struct max31722_data *data;
> > +	struct iio_dev *indio_dev =3D spi_get_drvdata(spi);
> > +
> > +	iio_device_unregister(indio_dev);
> > +	data =3D iio_priv(indio_dev);
>=20
> I would have put this inline with the declaration of data above.
> It is pretty much the same level of lookup as spi_get_drvdata.
>=20
> > +
> > +	return regmap_field_write(data->reg_state,
> MAX31722_MODE_STANDBY); }
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int max31722_suspend(struct device *dev) {
> > +	struct max31722_data *data;
> > +
> > +	data =3D iio_priv(spi_get_drvdata(to_spi_device(dev)));
> > +
> > +	return regmap_field_write(data->reg_state,
> MAX31722_MODE_STANDBY); }
> > +
> > +static int max31722_resume(struct device *dev) {
> > +	struct max31722_data *data;
> > +
> > +	data =3D iio_priv(spi_get_drvdata(to_spi_device(dev)));
> > +
> > +	return regmap_field_write(data->reg_state,
> > +MAX31722_MODE_CONTINUOUS); }
> > +
> > +static SIMPLE_DEV_PM_OPS(max31722_pm_ops, max31722_suspend,
> > +max31722_resume);
> > +
> > +#define MAX31722_PM_OPS (&max31722_pm_ops) #else #define
> > +MAX31722_PM_OPS NULL #endif
> > +
> > +static const struct spi_device_id max31722_spi_id[] =3D {
> > +	{"max31722", 0},
> > +	{"max31723", 0},
> > +	{}
> > +};
> > +
> > +static const struct acpi_device_id max31722_acpi_id[] =3D {
> > +	{"MAX31722", 0},
> > +	{"MAX31723", 0},
> > +	{}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(spi, max31722_spi_id);
> > +
> > +static struct spi_driver max31722_driver =3D {
> > +	.driver =3D {
> > +		.name =3D "max31722",
> > +		.pm =3D MAX31722_PM_OPS,
> > +		.acpi_match_table =3D ACPI_PTR(max31722_acpi_id),
> > +	},
> > +	.probe =3D            max31722_probe,
> > +	.remove =3D           max31722_remove,
> > +	.id_table =3D         max31722_spi_id,
> > +};
> > +
> > +module_spi_driver(max31722_driver);
> > +
> > +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
> > +MODULE_DESCRIPTION("MAX31722 digital thermometer and thermostat
> SPI
> > +driver"); MODULE_LICENSE("GPL v2");
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722
  2016-03-14 12:25     ` Daniel Baluta
@ 2016-03-14 15:06       ` Breana, Tiberiu A
  0 siblings, 0 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-14 15:06 UTC (permalink / raw)
  To: Daniel Baluta, Jonathan Cameron; +Cc: linux-iio

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBEYW5pZWwgQmFsdXRhIFttYWls
dG86ZGFuaWVsLmJhbHV0YUBnbWFpbC5jb21dDQo+IFNlbnQ6IE1vbmRheSwgTWFyY2ggMTQsIDIw
MTYgMjoyNiBQTQ0KPiBUbzogSm9uYXRoYW4gQ2FtZXJvbiA8amljMjNAa2VybmVsLm9yZz4NCj4g
Q2M6IEJyZWFuYSwgVGliZXJpdSBBIDx0aWJlcml1LmEuYnJlYW5hQGludGVsLmNvbT47IGxpbnV4
LQ0KPiBpaW9Admdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMi8yXSBpaW86
IHRlbXBlcmF0dXJlOiBBZGQgdGhyZXNob2xkIGludGVycnVwdCBzdXBwb3J0DQo+IGZvciBtYXgz
MTcyMg0KPiANCj4gT24gU2F0LCBNYXIgMTIsIDIwMTYgYXQgMTI6MTcgUE0sIEpvbmF0aGFuIENh
bWVyb24gPGppYzIzQGtlcm5lbC5vcmc+DQo+IHdyb3RlOg0KPiA+IE9uIDA5LzAzLzE2IDEzOjMw
LCBUaWJlcml1IEJyZWFuYSB3cm90ZToNCj4gPj4gQWRkZWQgaW50ZXJydXB0IHN1cHBvcnQgZm9y
IGhpZ2gvbG93IHRlbXBlcmF0dXJlIHRocmVzaG9sZCBldmVudHMgdG8NCj4gPj4gdGhlIG1heDMx
NzIyIGRyaXZlci4NCj4gPj4NCj4gPj4gU2lnbmVkLW9mZi1ieTogVGliZXJpdSBCcmVhbmEgPHRp
YmVyaXUuYS5icmVhbmFAaW50ZWwuY29tPg0KPiA+IE9ubHkgY29tbWVudCBpbiBoZXJlIGlzIHRo
YXQgeW91IG1pZ2h0IGJlIGJldHRlciBvZmYgcmVwb3J0aW5nIHRoZQ0KPiA+IGV2ZW50IGRpcmVj
dGlvbiBhcyBJSU9fRVZFTlRfRElSX0VJVEhFUiBhbmQgbGVhdmluZyB0aGUgZmlndXJpbmcgb3V0
DQo+ID4gd2hpY2ggY2FzZSBvY2N1cmVkIHRvIHVzZXJzcGFjZS4NCj4gPg0KPiA+IFRoaXMgY2Fz
ZSBpcyBpcml0YXRpbmdseSBjb21tb24hDQo+ID4NCj4gPiBKb25hdGhhbg0KPiA+PiAtLS0NCj4g
Pj4gIGRyaXZlcnMvaWlvL3RlbXBlcmF0dXJlL21heDMxNzIyLmMgfCAzMDMNCj4gPj4gKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrLQ0KPiA+PiAgMSBmaWxlIGNoYW5nZWQsIDMw
MSBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPiA+Pg0KPiA+PiBkaWZmIC0tZ2l0IGEv
ZHJpdmVycy9paW8vdGVtcGVyYXR1cmUvbWF4MzE3MjIuYw0KPiA+PiBiL2RyaXZlcnMvaWlvL3Rl
bXBlcmF0dXJlL21heDMxNzIyLmMNCj4gPj4gaW5kZXggZmE4MzNiNi4uYThiZmUzNSAxMDA2NDQN
Cj4gPj4gLS0tIGEvZHJpdmVycy9paW8vdGVtcGVyYXR1cmUvbWF4MzE3MjIuYw0KPiA+PiArKysg
Yi9kcml2ZXJzL2lpby90ZW1wZXJhdHVyZS9tYXgzMTcyMi5jDQo+ID4+IEBAIC0xMCwyMSArMTAs
MzIgQEANCj4gPj4NCj4gPj4gICNpbmNsdWRlIDxsaW51eC9rZXJuZWwuaD4NCj4gPj4gICNpbmNs
dWRlIDxsaW51eC9hY3BpLmg+DQo+ID4+ICsjaW5jbHVkZSA8bGludXgvZ3Bpby9jb25zdW1lci5o
Pg0KPiA+PiArI2luY2x1ZGUgPGxpbnV4L2ludGVycnVwdC5oPg0KPiA+PiAgI2luY2x1ZGUgPGxp
bnV4L21vZHVsZS5oPg0KPiA+PiAgI2luY2x1ZGUgPGxpbnV4L3JlZ21hcC5oPg0KPiA+PiAgI2lu
Y2x1ZGUgPGxpbnV4L3NwaS9zcGkuaD4NCj4gPj4gKyNpbmNsdWRlIDxsaW51eC9paW8vZXZlbnRz
Lmg+DQo+ID4+ICAjaW5jbHVkZSA8bGludXgvaWlvL2lpby5oPg0KPiA+PiAgI2luY2x1ZGUgPGxp
bnV4L2lpby9zeXNmcy5oPg0KPiA+Pg0KPiA+PiAgI2RlZmluZSBNQVgzMTcyMl9SRUdfQ0ZHICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAweDAwDQo+ID4+ICAjZGVmaW5lIE1BWDMxNzIyX1JF
R19URU1QX0xTQiAgICAgICAgICAgICAgICAgICAgICAgIDB4MDENCj4gPj4gICNkZWZpbmUgTUFY
MzE3MjJfUkVHX1RFTVBfTVNCICAgICAgICAgICAgICAgICAgICAgICAgMHgwMg0KPiA+PiArI2Rl
ZmluZSBNQVgzMTcyMl9SRUdfVEhJR0hfTFNCICAgICAgICAgMHgwMw0KPiA+PiArI2RlZmluZSBN
QVgzMTcyMl9SRUdfVExPV19MU0IgICAgICAgICAgMHgwNQ0KPiA+PiAgI2RlZmluZSBNQVgzMTcy
Ml9NQVhfUkVHICAgICAgICAgICAgICAgICAgICAgICAgICAgICAweDg2DQo+ID4+DQo+ID4+ICAj
ZGVmaW5lIE1BWDMxNzIyX01PREVfQ09OVElOVU9VUyAgICAgICAgICAgICAweDAwDQo+ID4+ICAj
ZGVmaW5lIE1BWDMxNzIyX01PREVfU1RBTkRCWSAgICAgICAgICAgICAgICAgICAgICAgIDB4MDEN
Cj4gPj4gKyNkZWZpbmUgTUFYMzE3MjJfTU9ERV9USEVSTU9TVEFUICAgICAgIDB4ODANCj4gPj4g
Kw0KPiA+PiArI2RlZmluZSBNQVgzMTcyMl9NSU5fVEVNUCAgICAgICAgICAtNTUNCj4gPj4gKyNk
ZWZpbmUgTUFYMzE3MjJfTUFYX1RFTVAgICAgICAgICAgMTI1DQo+ID4+DQo+ID4+ICAjZGVmaW5l
IE1BWDMxNzIyX1JFR01BUF9OQU1FICAgICAgICAgICAgICAgICAibWF4MzE3MjJfcmVnbWFwIg0K
PiA+PiArI2RlZmluZSBNQVgzMTcyMl9FVkVOVCAgICAgICAgICAgICAibWF4MzE3MjJfZXZlbnQi
DQo+ID4+ICsjZGVmaW5lIE1BWDMxNzIyX0dQSU8gICAgICAgICAgICAgICJtYXgzMTcyMl9ncGlv
Ig0KPiA+Pg0KPiA+PiAgI2RlZmluZSBNQVgzMTcyMl9TQ0FMRV9BVkFJTEFCTEUgICAgICAgICAg
ICAgIjAuNSAwLjI1IDAuMTI1IDAuMDYyNSINCj4gPj4NCj4gPj4gQEAgLTQzLDYgKzU0LDggQEAg
c3RhdGljIGNvbnN0IHN0cnVjdCByZWdfZmllbGQNCj4gbWF4MzE3MjJfcmVnX2ZpZWxkX3N0YXRl
ID0NCj4gPj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUkVHX0ZJRUxEKE1BWDMxNzIy
X1JFR19DRkcsIDAsIDApOw0KPiA+PiBzdGF0aWMgY29uc3Qgc3RydWN0IHJlZ19maWVsZCBtYXgz
MTcyMl9yZWdfZmllbGRfcmVzb2x1dGlvbiA9DQo+ID4+ICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIFJFR19GSUVMRChNQVgzMTcyMl9SRUdfQ0ZHLCAxLCAyKTsNCj4gPj4gK3N0YXRpYyBj
b25zdCBzdHJ1Y3QgcmVnX2ZpZWxkIG1heDMxNzIyX3JlZ19maWVsZF90aGVybW9zdGF0ID0NCj4g
Pj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUkVHX0ZJRUxEKE1BWDMxNzIyX1JFR19D
RkcsIDMsIDMpOw0KPiA+Pg0KPiA+PiAgc3RydWN0IG1heDMxNzIyX2RhdGEgew0KPiA+PiAgICAg
ICBzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpX2RldmljZTsNCj4gPj4gQEAgLTUwLDYgKzYzLDggQEAg
c3RydWN0IG1heDMxNzIyX2RhdGEgew0KPiA+PiAgICAgICBzdHJ1Y3QgcmVnbWFwICpyZWdtYXA7
DQo+ID4+ICAgICAgIHN0cnVjdCByZWdtYXBfZmllbGQgKnJlZ19zdGF0ZTsNCj4gPj4gICAgICAg
c3RydWN0IHJlZ21hcF9maWVsZCAqcmVnX3Jlc29sdXRpb247DQo+ID4+ICsgICAgIHN0cnVjdCBy
ZWdtYXBfZmllbGQgKnJlZ190aGVybW9zdGF0Ow0KPiA+PiArICAgICB1NjQgdGltZXN0YW1wOw0K
PiA+PiAgfTsNCj4gPj4NCj4gPj4gIC8qDQo+ID4+IEBAIC03MSwxMSArODYsMzAgQEAgc3RhdGlj
IGNvbnN0IHN0cnVjdCBhdHRyaWJ1dGVfZ3JvdXANCj4gbWF4MzE3MjJfYXR0cmlidXRlX2dyb3Vw
ID0gew0KPiA+PiAgICAgICAuYXR0cnMgPSBtYXgzMTcyMl9hdHRyaWJ1dGVzDQo+ID4+ICB9Ow0K
PiA+Pg0KPiA+PiArc3RhdGljIGNvbnN0IHN0cnVjdCBpaW9fZXZlbnRfc3BlYyBtYXgzMTcyMl9l
dmVudHNbXSA9IHsNCj4gPj4gKyAgICAgLyogSGlnaCB0ZW1wZXJhdHVyZSBldmVudCAqLw0KPiA+
PiArICAgICB7DQo+ID4+ICsgICAgICAgICAgICAgLnR5cGUgPSBJSU9fRVZfVFlQRV9USFJFU0gs
DQo+ID4+ICsgICAgICAgICAgICAgLmRpciA9IElJT19FVl9ESVJfUklTSU5HLA0KPiA+PiArICAg
ICAgICAgICAgIC5tYXNrX3NlcGFyYXRlID0gQklUKElJT19FVl9JTkZPX1ZBTFVFKSB8DQo+ID4+
ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBCSVQoSUlPX0VWX0lORk9fRU5BQkxFKSwN
Cj4gPj4gKyAgICAgfSwNCj4gPj4gKyAgICAgLyogTG93IHRlbXBlcmF0dXJlIGV2ZW50ICovDQo+
ID4+ICsgICAgIHsNCj4gPj4gKyAgICAgICAgICAgICAudHlwZSA9IElJT19FVl9UWVBFX1RIUkVT
SCwNCj4gPj4gKyAgICAgICAgICAgICAuZGlyID0gSUlPX0VWX0RJUl9GQUxMSU5HLA0KPiA+PiAr
ICAgICAgICAgICAgIC5tYXNrX3NlcGFyYXRlID0gQklUKElJT19FVl9JTkZPX1ZBTFVFKSB8DQo+
ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBCSVQoSUlPX0VWX0lORk9fRU5BQkxF
KSwNCj4gPj4gKyAgICAgfSwNCj4gPj4gK307DQo+ID4+ICsNCj4gPj4gIHN0YXRpYyBjb25zdCBz
dHJ1Y3QgaWlvX2NoYW5fc3BlYyBtYXgzMTcyMl9jaGFubmVsc1tdID0gew0KPiA+PiAgICAgICB7
DQo+ID4+ICAgICAgICAgICAgICAgLnR5cGUgPSBJSU9fVEVNUCwNCj4gPj4gICAgICAgICAgICAg
ICAuaW5mb19tYXNrX3NlcGFyYXRlID0gQklUKElJT19DSEFOX0lORk9fUkFXKSB8DQo+ID4+ICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIEJJVChJSU9fQ0hBTl9JTkZPX1NDQUxF
KSwNCj4gPj4gKyAgICAgICAgICAgICAuZXZlbnRfc3BlYyA9IG1heDMxNzIyX2V2ZW50cywNCj4g
Pj4gKyAgICAgICAgICAgICAubnVtX2V2ZW50X3NwZWNzID0gQVJSQVlfU0laRShtYXgzMTcyMl9l
dmVudHMpLA0KPiA+PiAgICAgICB9LA0KPiA+PiAgfTsNCj4gPj4NCj4gPj4gQEAgLTE2Nyw2ICsy
MDEsMTcyIEBAIHN0YXRpYyB2b2lkIG1heDMxNzIyX3JlZ190b19mbG9hdCh1MTYgcmVnX3ZhbCwN
Cj4gaW50ICp2YWwsIGludCAqdmFsMikNCj4gPj4gICAgICAgfQ0KPiA+PiAgfQ0KPiA+Pg0KPiA+
PiArLyogQ29udmVydCBhIGZsb2F0aW5nIHBvaW50IHZhbHVlIHRvIGEgcmVnaXN0ZXIgdmFsdWUu
ICovIHN0YXRpYyB1MTYNCj4gPj4gK21heDMxNzIyX2Zsb2F0X3RvX3JlZyhpbnQgdmFsLCBpbnQg
dmFsMikgew0KPiA+PiArICAgICBpbnQgaTsNCj4gPj4gKyAgICAgYm9vbCBuZWdhdGl2ZV9ucjsN
Cj4gPj4gKyAgICAgdTggbHNiOw0KPiA+PiArICAgICB1MTYgcmVnX3ZhbDsNCj4gPj4gKw0KPiA+
PiArICAgICBuZWdhdGl2ZV9uciA9ICh2YWwgJiAweDgwKSB8fCB2YWwyIDwgMDsNCj4gPj4gKyAg
ICAgaWYgKHZhbDIgPCAwKQ0KPiA+PiArICAgICAgICAgICAgIHZhbDIgKj0gLTE7DQo+ID4+ICsN
Cj4gPj4gKyAgICAgLyoNCj4gPj4gKyAgICAgICogVGhlIExTQiB2YWx1ZSB3aWxsIGJlIGFuIGFw
cHJveGltYXRpb24gb2YgdmFsMg0KPiA+PiArICAgICAgKiBkdWUgdG8gaXRzIGxpbWl0ZWQgNC1i
aXQgcmFuZ2UuDQo+ID4+ICsgICAgICAqLw0KPiA+PiArICAgICBsc2IgPSAwOw0KPiA+PiArICAg
ICBmb3IgKGkgPSAwIDsgaSA8IEFSUkFZX1NJWkUobWF4MzE3MjJfc2NhbGVfdGFibGUpICYmIHZh
bDIgPiAwOyBpKyspDQo+ID4+ICsgICAgICAgICAgICAgaWYgKHZhbDIgLSBtYXgzMTcyMl9zY2Fs
ZV90YWJsZVtpXSA+PSAwKSB7DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICB2YWwyIC09IG1h
eDMxNzIyX3NjYWxlX3RhYmxlW2ldOw0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgbHNiICs9
IDEgPDwgKDQgLSBpIC0gMSk7DQo+ID4+ICsgICAgICAgICAgICAgfQ0KPiA+PiArICAgICBsc2Ig
PDw9IDQ7DQo+ID4+ICsNCj4gPj4gKyAgICAgaWYgKG5lZ2F0aXZlX25yKSB7DQo+ID4+ICsgICAg
ICAgICAgICAgLyoNCj4gPj4gKyAgICAgICAgICAgICAgKiBOZWdhdGl2ZSB2YWx1ZS4gVGVtcG9y
YXJpbHkgdXNlIHRoZSBjb21wbGVtZW50IG9mIHZhbCBmb3INCj4gPj4gKyAgICAgICAgICAgICAg
KiB0aGUgTVNCLCB0aGVuIGNvbmNhdGVuYXRlIHRoZSBMU0IsIHJldmVyc2UgYml0cyAmIGFkZCAx
IGZvcg0KPiA+PiArICAgICAgICAgICAgICAqIHRoZSBmaW5hbCB2YWx1ZS4NCj4gPj4gKyAgICAg
ICAgICAgICAgKi8NCj4gPj4gKyAgICAgICAgICAgICB1OCBtc2IgPSAodTgpKC0xICogdmFsKTsN
Cj4gPj4gKyAgICAgICAgICAgICB1MTYgcmV2ID0gMDsNCj4gPj4gKyAgICAgICAgICAgICBpbnQg
bnVtX2JpdHMgPSBzaXplb2YocmV2KSAqIDg7DQo+ID4+ICsNCj4gPj4gKyAgICAgICAgICAgICBy
ZWdfdmFsID0gKG1zYiA8PCA4KSB8IGxzYjsNCj4gPj4gKw0KPiA+PiArICAgICAgICAgICAgIGZv
ciAoaSA9IDAgOyBpIDwgbnVtX2JpdHMgOyBpKyspIHsNCj4gPj4gKyAgICAgICAgICAgICAgICAg
ICAgIHJldiB8PSAhKHJlZ192YWwgJiAweDAxKSA8PCBpOw0KPiA+PiArICAgICAgICAgICAgICAg
ICAgICAgcmVnX3ZhbCA+Pj0gMTsNCj4gPj4gKyAgICAgICAgICAgICB9DQo+ID4+ICsgICAgICAg
ICAgICAgcmV2ICs9IDE7DQo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIHJldjsNCj4gPj4gKw0K
PiA+PiArICAgICB9IGVsc2Ugew0KPiA+PiArICAgICAgICAgICAgIHJlZ192YWwgPSAoKHU4KXZh
bCA8PCA4KSB8IGxzYjsNCj4gPj4gKyAgICAgfQ0KPiA+PiArDQo+ID4+ICsgICAgIHJldHVybiBy
ZWdfdmFsOw0KPiA+PiArfQ0KPiA+PiArDQo+ID4+ICtzdGF0aWMgaW50IG1heDMxNzIyX3JlYWRf
ZXZlbnRfY29uZmlnKHN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYsDQo+ID4+ICsgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIGNvbnN0IHN0cnVjdCBpaW9fY2hhbl9zcGVjICpjaGFu
LA0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIGlpb19ldmVu
dF90eXBlIHR5cGUsDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGVu
dW0gaWlvX2V2ZW50X2RpcmVjdGlvbiBkaXIpIHsNCj4gPj4gKyAgICAgdW5zaWduZWQgaW50IGV2
ZW50X3ZhbDsNCj4gPj4gKyAgICAgaW50IHJldDsNCj4gPj4gKyAgICAgc3RydWN0IG1heDMxNzIy
X2RhdGEgKmRhdGEgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiA+PiArICAgICBzdHJ1Y3Qgc3Bp
X2RldmljZSAqc3BpID0gZGF0YS0+c3BpX2RldmljZTsNCj4gPj4gKw0KPiA+PiArICAgICBtdXRl
eF9sb2NrKCZkYXRhLT5sb2NrKTsNCj4gPj4gKyAgICAgcmV0ID0gcmVnbWFwX2ZpZWxkX3JlYWQo
ZGF0YS0+cmVnX3RoZXJtb3N0YXQsICZldmVudF92YWwpOw0KPiA+PiArICAgICBpZiAocmV0IDwg
MCkgew0KPiA+PiArICAgICAgICAgICAgIGRldl9lcnIoJnNwaS0+ZGV2LCAiZmFpbGVkIHRvIHJl
YWQgdGhlcm1vc3RhdCBzdGF0dXNcbiIpOw0KPiA+PiArICAgICAgICAgICAgIG11dGV4X3VubG9j
aygmZGF0YS0+bG9jayk7DQo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIHJldDsNCj4gPj4gKyAg
ICAgfQ0KPiA+PiArICAgICBtdXRleF91bmxvY2soJmRhdGEtPmxvY2spOw0KPiA+PiArDQo+ID4+
ICsgICAgIHJldHVybiBldmVudF92YWw7DQo+ID4+ICt9DQo+ID4+ICsNCj4gPj4gK3N0YXRpYyBp
bnQgbWF4MzE3MjJfd3JpdGVfZXZlbnRfY29uZmlnKHN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYs
DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBzdHJ1Y3Qg
aWlvX2NoYW5fc3BlYyAqY2hhbiwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIGVudW0gaWlvX2V2ZW50X3R5cGUgdHlwZSwNCj4gPj4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIGVudW0gaWlvX2V2ZW50X2RpcmVjdGlvbiBkaXIsDQo+ID4+ICsg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpbnQgc3RhdGUpIHsNCj4gPj4gKyAg
ICAgaW50IHJldDsNCj4gPj4gKyAgICAgc3RydWN0IG1heDMxNzIyX2RhdGEgKmRhdGEgPSBpaW9f
cHJpdihpbmRpb19kZXYpOw0KPiA+PiArICAgICBzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpID0gZGF0
YS0+c3BpX2RldmljZTsNCj4gPj4gKw0KPiA+PiArICAgICBpZiAoc3RhdGUgIT0gMCAmJiBzdGF0
ZSAhPSAxKQ0KPiA+PiArICAgICAgICAgICAgIHJldHVybiAtRUlOVkFMOw0KPiA+PiArDQo+ID4+
ICsgICAgIC8qDQo+ID4+ICsgICAgICAqIFNldCB0aGVybW9zdGF0IG1vZGU6DQo+ID4+ICsgICAg
ICAqIDAgOiBjb21wYXJhdG9yIG1vZGUgKGRlZmF1bHQpDQo+ID4+ICsgICAgICAqIDEgOiBpbnRl
cnJ1cHQgbW9kZQ0KPiA+PiArICAgICAgKi8NCj4gPj4gKyAgICAgbXV0ZXhfbG9jaygmZGF0YS0+
bG9jayk7DQo+ID4+ICsgICAgIHJldCA9IHJlZ21hcF9maWVsZF93cml0ZShkYXRhLT5yZWdfdGhl
cm1vc3RhdCwgc3RhdGUpOw0KPiA+PiArICAgICBpZiAocmV0IDwgMCkNCj4gPj4gKyAgICAgICAg
ICAgICBkZXZfZXJyKCZzcGktPmRldiwgImZhaWxlZCB0byBzZXQgdGhlcm1vc3RhdCBtb2RlXG4i
KTsNCj4gPj4gKyAgICAgbXV0ZXhfdW5sb2NrKCZkYXRhLT5sb2NrKTsNCj4gPj4gKw0KPiA+PiAr
ICAgICByZXR1cm4gcmV0Ow0KPiA+PiArfQ0KPiA+PiArDQo+ID4+ICtzdGF0aWMgaW50IG1heDMx
NzIyX3JlYWRfZXZlbnQoc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiwNCj4gPj4gKyAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBjb25zdCBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyAqY2hhbiwNCj4g
Pj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIGlpb19ldmVudF90eXBlIHR5cGUs
DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgZW51bSBpaW9fZXZlbnRfZGlyZWN0
aW9uIGRpciwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIGlpb19ldmVu
dF9pbmZvIGluZm8sDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW50ICp2YWws
IGludCAqdmFsMikgew0KPiA+PiArICAgICBpbnQgcmV0Ow0KPiA+PiArICAgICB1OCByZWc7DQo+
ID4+ICsgICAgIHUxNiBidWY7DQo+ID4+ICsgICAgIHN0cnVjdCBtYXgzMTcyMl9kYXRhICpkYXRh
ID0gaWlvX3ByaXYoaW5kaW9fZGV2KTsNCj4gPj4gKw0KPiA+PiArICAgICBpZiAoaW5mbyAhPSBJ
SU9fRVZfSU5GT19WQUxVRSkNCj4gPj4gKyAgICAgICAgICAgICByZXR1cm4gLUVJTlZBTDsNCj4g
Pj4gKw0KPiA+PiArICAgICBpZiAoZGlyID09IElJT19FVl9ESVJfUklTSU5HKQ0KPiA+PiArICAg
ICAgICAgICAgIHJlZyA9IE1BWDMxNzIyX1JFR19USElHSF9MU0I7DQo+ID4+ICsgICAgIGVsc2Ug
aWYgKGRpciA9PSBJSU9fRVZfRElSX0ZBTExJTkcpDQo+ID4+ICsgICAgICAgICAgICAgcmVnID0g
TUFYMzE3MjJfUkVHX1RMT1dfTFNCOw0KPiA+PiArICAgICBlbHNlDQo+ID4+ICsgICAgICAgICAg
ICAgcmV0dXJuIC1FSU5WQUw7DQo+ID4+ICsNCj4gPj4gKyAgICAgbXV0ZXhfbG9jaygmZGF0YS0+
bG9jayk7DQo+ID4+ICsgICAgIHJldCA9IHJlZ21hcF9idWxrX3JlYWQoZGF0YS0+cmVnbWFwLCBy
ZWcsICZidWYsIDIpOw0KPiA+PiArICAgICBtdXRleF91bmxvY2soJmRhdGEtPmxvY2spOw0KPiA+
PiArICAgICBpZiAocmV0IDwgMCkgew0KPiA+PiArICAgICAgICAgICAgIGRldl9lcnIoJmRhdGEt
PnNwaV9kZXZpY2UtPmRldiwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICJmYWlsZWQgdG8g
cmVhZCB0aHJlc2hvbGQgcmVnaXN0ZXJcbiIpOw0KPiA+PiArICAgICAgICAgICAgIHJldHVybiBy
ZXQ7DQo+ID4+ICsgICAgIH0NCj4gPj4gKyAgICAgbWF4MzE3MjJfcmVnX3RvX2Zsb2F0KGJ1Ziwg
dmFsLCB2YWwyKTsNCj4gPj4gKw0KPiA+PiArICAgICByZXR1cm4gSUlPX1ZBTF9JTlRfUExVU19N
SUNSTzsNCj4gPj4gK30NCj4gPj4gKw0KPiA+PiArc3RhdGljIGludCBtYXgzMTcyMl93cml0ZV9l
dmVudChzdHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2LA0KPiA+PiArICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBjb25zdCBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyAqY2hhbiwNCj4gPj4gKyAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgZW51bSBpaW9fZXZlbnRfdHlwZSB0eXBlLA0KPiA+PiAr
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIGlpb19ldmVudF9kaXJlY3Rpb24gZGly
LA0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIGlpb19ldmVudF9pbmZv
IGluZm8sDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCB2YWwsIGludCB2
YWwyKSB7DQo+ID4+ICsgICAgIGludCByZXQ7DQo+ID4+ICsgICAgIHU4IHJlZzsNCj4gPj4gKyAg
ICAgdTE2IGJ1ZjsNCj4gPj4gKyAgICAgc3RydWN0IG1heDMxNzIyX2RhdGEgKmRhdGEgPSBpaW9f
cHJpdihpbmRpb19kZXYpOw0KPiA+PiArDQo+ID4+ICsgICAgIGlmICh2YWwgPCBNQVgzMTcyMl9N
SU5fVEVNUCB8fCB2YWwgPiBNQVgzMTcyMl9NQVhfVEVNUCkNCj4gPj4gKyAgICAgICAgICAgICBy
ZXR1cm4gLUVJTlZBTDsNCj4gPj4gKw0KPiA+PiArICAgICBpZiAoZGlyID09IElJT19FVl9ESVJf
UklTSU5HKQ0KPiA+PiArICAgICAgICAgICAgIHJlZyA9IE1BWDMxNzIyX1JFR19USElHSF9MU0I7
DQo+ID4+ICsgICAgIGVsc2UgaWYgKGRpciA9PSBJSU9fRVZfRElSX0ZBTExJTkcpDQo+ID4+ICsg
ICAgICAgICAgICAgcmVnID0gTUFYMzE3MjJfUkVHX1RMT1dfTFNCOw0KPiA+PiArICAgICBlbHNl
DQo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIC1FSU5WQUw7DQo+ID4+ICsNCj4gPj4gKyAgICAg
YnVmID0gbWF4MzE3MjJfZmxvYXRfdG9fcmVnKHZhbCwgdmFsMik7DQo+ID4+ICsNCj4gPj4gKyAg
ICAgcmV0ID0gcmVnbWFwX2J1bGtfd3JpdGUoZGF0YS0+cmVnbWFwLCByZWcsICZidWYsIDIpOw0K
PiA+PiArICAgICBpZiAocmV0IDwgMCkNCj4gPj4gKyAgICAgICAgICAgICBkZXZfZXJyKCZkYXRh
LT5zcGlfZGV2aWNlLT5kZXYsDQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAiZmFpbGVkIHRv
IHdyaXRlIHRocmVzaG9sZCByZWdpc3RlclxuIik7DQo+ID4+ICsNCj4gPj4gKyAgICAgcmV0dXJu
IHJldDsNCj4gPj4gK30NCj4gPj4gKw0KPiA+PiAgc3RhdGljIGludCBtYXgzMTcyMl9yZWFkX3Jh
dyhzdHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2LA0KPiA+PiAgICAgICAgICAgICAgICAgICAgICAg
ICAgICBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyBjb25zdCAqY2hhbiwNCj4gPj4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgaW50ICp2YWwsIGludCAqdmFsMiwgbG9uZyBtYXNrKSBAQCAtMjQxLDgN
Cj4gPj4gKzQ0MSw2MyBAQCBzdGF0aWMgY29uc3Qgc3RydWN0IGlpb19pbmZvIG1heDMxNzIyX2lu
Zm8gPSB7DQo+ID4+ICAgICAgIC5yZWFkX3JhdyAgICAgICAgICAgICAgID0gbWF4MzE3MjJfcmVh
ZF9yYXcsDQo+ID4+ICAgICAgIC53cml0ZV9yYXcgICAgICAgICAgICAgID0gbWF4MzE3MjJfd3Jp
dGVfcmF3LA0KPiA+PiAgICAgICAuYXR0cnMgICAgICAgICAgICAgICAgICA9ICZtYXgzMTcyMl9h
dHRyaWJ1dGVfZ3JvdXAsDQo+ID4+ICsgICAgIC5yZWFkX2V2ZW50X3ZhbHVlICAgICAgID0gbWF4
MzE3MjJfcmVhZF9ldmVudCwNCj4gPj4gKyAgICAgLndyaXRlX2V2ZW50X3ZhbHVlICAgICAgPSBt
YXgzMTcyMl93cml0ZV9ldmVudCwNCj4gPj4gKyAgICAgLnJlYWRfZXZlbnRfY29uZmlnICAgICAg
PSBtYXgzMTcyMl9yZWFkX2V2ZW50X2NvbmZpZywNCj4gPj4gKyAgICAgLndyaXRlX2V2ZW50X2Nv
bmZpZyAgICAgPSBtYXgzMTcyMl93cml0ZV9ldmVudF9jb25maWcsDQo+ID4+ICB9Ow0KPiA+Pg0K
PiA+PiArc3RhdGljIGlycXJldHVybl90IG1heDMxNzIyX2lycV9oYW5kbGVyKGludCBpcnEsIHZv
aWQgKnByaXZhdGUpIHsNCj4gPj4gKyAgICAgc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiA9IHBy
aXZhdGU7DQo+ID4+ICsgICAgIHN0cnVjdCBtYXgzMTcyMl9kYXRhICpkYXRhID0gaWlvX3ByaXYo
aW5kaW9fZGV2KTsNCj4gPj4gKw0KPiA+PiArICAgICBkYXRhLT50aW1lc3RhbXAgPSBpaW9fZ2V0
X3RpbWVfbnMoKTsNCj4gPj4gKw0KPiA+PiArICAgICByZXR1cm4gSVJRX1dBS0VfVEhSRUFEOw0K
PiA+PiArfQ0KPiA+PiArDQo+ID4+ICtzdGF0aWMgaXJxcmV0dXJuX3QgbWF4MzE3MjJfaXJxX2V2
ZW50X2hhbmRsZXIoaW50IGlycSwgdm9pZA0KPiA+PiArKnByaXZhdGUpIHsNCj4gPj4gKyAgICAg
aW50IHJldDsNCj4gPj4gKyAgICAgdTY0IGV2ZW50Ow0KPiA+PiArICAgICB1MTYgdGVtcDsNCj4g
Pj4gKyAgICAgdTE2IHRsb3c7DQo+ID4+ICsgICAgIHUxNiB0aGlnaDsNCj4gPj4gKyAgICAgdW5z
aWduZWQgaW50IGRpcjsNCj4gPj4gKyAgICAgc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiA9IHBy
aXZhdGU7DQo+ID4+ICsgICAgIHN0cnVjdCBtYXgzMTcyMl9kYXRhICpkYXRhID0gaWlvX3ByaXYo
aW5kaW9fZGV2KTsNCj4gPj4gKw0KPiA+PiArICAgICAvKg0KPiA+PiArICAgICAgKiBEbyBhIHF1
aWNrIHRlbXBlcmF0dXJlIHJlYWRpbmcgYW5kIGNvbXBhcmUgaXQgd2l0aCBUTE9XL1RISUdIDQo+
ID4+ICsgICAgICAqIHNvIHdlIGNhbiB0ZWxsIHdoaWNoIHRocmVzaG9sZCBoYXMgYmVlbiBtZXQu
DQo+ID4gSG1tLiBNaWdodCBlbmQgdXAgZGV0ZWN0aW5nIG5laXRoZXIgb2NjdXJlZCBpZiB0aGUg
Y29uZGl0aW9uIGlzIG5vDQo+ID4gbG9uZ2VyIG1ldC4gIElyaXRhdGluZyBoYXJkd2FyZSENCj4g
Pg0KPiA+IFdlIGRvIGhhdmUgdGhlICdtYWdpYycgb3B0aW9uIG9mIElJT19FVl9ESVJfRUlUSEVS
IHRvIHNwZWNpZnkgdGhhdCB3ZQ0KPiA+IGRvbid0IGtub3cgd2hpY2ggb25lIG9jY3VyZWQuICBU
aGUgaWRlYSBvZiB0aGF0IGhhcyBhbHdheXMgYmVlbiB0aGF0DQo+ID4gdXNlcnNwYWNlIGNhbiBl
bGVjdCB0byBkbyB3aGF0IHlvdSBoYXZlIGhlcmUgaWYgaXQgY2FyZXMgcmF0aGVyIHRoYW4NCj4g
PiBwdXNoaW5nIHRoaXMgY29kZSBpbnRvIGRyaXZlcnMuDQo+ID4+ICsgICAgICAqLw0KPiA+PiAr
ICAgICBtdXRleF9sb2NrKCZkYXRhLT5sb2NrKTsNCj4gPj4gKyAgICAgcmV0ID0gcmVnbWFwX2J1
bGtfcmVhZChkYXRhLT5yZWdtYXAsIE1BWDMxNzIyX1JFR19URU1QX0xTQiwNCj4gJnRlbXAsIDIp
Ow0KPiA+PiArICAgICBpZiAocmV0IDwgMCkNCj4gPj4gKyAgICAgICAgICAgICBnb3RvIGV4aXRf
ZXJyOw0KPiA+PiArICAgICByZXQgPSByZWdtYXBfYnVsa19yZWFkKGRhdGEtPnJlZ21hcCwgTUFY
MzE3MjJfUkVHX1RMT1dfTFNCLA0KPiAmdGxvdywgMik7DQo+ID4+ICsgICAgIGlmIChyZXQgPCAw
KQ0KPiA+PiArICAgICAgICAgICAgIGdvdG8gZXhpdF9lcnI7DQo+ID4+ICsgICAgIHJldCA9IHJl
Z21hcF9idWxrX3JlYWQoZGF0YS0+cmVnbWFwLCBNQVgzMTcyMl9SRUdfVEhJR0hfTFNCLA0KPiAm
dGhpZ2gsIDIpOw0KPiA+PiArICAgICBpZiAocmV0IDwgMCkNCj4gPj4gKyAgICAgICAgICAgICBn
b3RvIGV4aXRfZXJyOw0KPiA+PiArICAgICBtdXRleF91bmxvY2soJmRhdGEtPmxvY2spOw0KPiA+
PiArDQo+ID4+ICsgICAgIGRpciA9ICh0ZW1wID4gdGhpZ2ggPyAxIDogMCk7DQo+ID4+ICsgICAg
IGV2ZW50ID0gSUlPX1VOTU9EX0VWRU5UX0NPREUoSUlPX1RFTVAsIDAsDQo+IElJT19FVl9UWVBF
X1RIUkVTSCwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAoZGlyID8g
SUlPX0VWX0RJUl9SSVNJTkcgOg0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBJSU9fRVZfRElSX0ZBTExJTkcpKTsNCj4gPj4gKyAgICAgaWlvX3B1c2hfZXZl
bnQoaW5kaW9fZGV2LCBldmVudCwgZGF0YS0+dGltZXN0YW1wKTsNCj4gPj4gKw0KPiA+PiArICAg
ICByZXR1cm4gSVJRX0hBTkRMRUQ7DQo+ID4+ICsNCj4gPj4gK2V4aXRfZXJyOg0KPiA+PiArICAg
ICBkZXZfZXJyKCZkYXRhLT5zcGlfZGV2aWNlLT5kZXYsICJmYWlsZWQgdG8gcmVhZCB0ZW1wZXJh
dHVyZQ0KPiByZWdpc3RlclxuIik7DQo+ID4+ICsgICAgIG11dGV4X3VubG9jaygmZGF0YS0+bG9j
ayk7DQo+ID4+ICsgICAgIHJldHVybiByZXQ7DQo+ID4+ICt9DQo+ID4+ICsNCj4gPj4gIHN0YXRp
YyBpbnQgbWF4MzE3MjJfaW5pdChzdHJ1Y3QgbWF4MzE3MjJfZGF0YSAqZGF0YSkgIHsNCj4gPj4g
ICAgICAgaW50IHJldCA9IDA7DQo+ID4+IEBAIC0yNTksMTUgKzUxNCw0MCBAQCBzdGF0aWMgaW50
IG1heDMxNzIyX2luaXQoc3RydWN0IG1heDMxNzIyX2RhdGENCj4gPj4gKmRhdGEpDQo+ID4+DQo+
ID4+ICAgICAgIE1BWDMxNzIyX1JFR0ZJRUxEKHN0YXRlKTsNCj4gPj4gICAgICAgTUFYMzE3MjJf
UkVHRklFTEQocmVzb2x1dGlvbik7DQo+ID4+ICsgICAgIE1BWDMxNzIyX1JFR0ZJRUxEKHRoZXJt
b3N0YXQpOw0KPiA+Pg0KPiA+PiAgICAgICAvKiBTZXQgU0QgYml0IHRvIDAgc28gd2UgY2FuIGhh
dmUgY29udGludW91cyBtZWFzdXJlbWVudHMuICovDQo+ID4+ICAgICAgIHJldCA9IHJlZ21hcF9m
aWVsZF93cml0ZShkYXRhLT5yZWdfc3RhdGUsDQo+ID4+IE1BWDMxNzIyX01PREVfQ09OVElOVU9V
Uyk7DQo+ID4+ICsgICAgIGlmIChyZXQgPCAwKSB7DQo+ID4+ICsgICAgICAgICAgICAgZGV2X2Vy
cigmc3BpLT5kZXYsICJmYWlsZWQgdG8gY29uZmlndXJlIHNlbnNvci5cbiIpOw0KPiA+PiArICAg
ICAgICAgICAgIHJldHVybiByZXQ7DQo+ID4+ICsgICAgIH0NCj4gPj4gKw0KPiA+PiArICAgICAv
KiBTZXQgdGhlcm1vc3RhdCBpbnRlcnJ1cHQgbW9kZSAqLw0KPiA+PiArICAgICByZXQgPSByZWdt
YXBfZmllbGRfd3JpdGUoZGF0YS0+cmVnX3RoZXJtb3N0YXQsIDEpOw0KPiA+PiAgICAgICBpZiAo
cmV0IDwgMCkNCj4gPj4gICAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwgImZhaWxlZCB0
byBjb25maWd1cmUgc2Vuc29yLlxuIik7DQo+ID4+DQo+ID4+ICAgICAgIHJldHVybiByZXQ7DQo+
ID4+ICB9DQo+ID4+DQo+ID4+ICtzdGF0aWMgaW50IG1heDMxNzIyX2dwaW9fcHJvYmUoc3RydWN0
IHNwaV9kZXZpY2UgKnNwaSkgew0KPiA+PiArICAgICBzdHJ1Y3QgZ3Bpb19kZXNjICpncGlvOw0K
PiA+PiArDQo+ID4+ICsgICAgIGlmICghc3BpKQ0KPiA+PiArICAgICAgICAgICAgIHJldHVybiAt
RUlOVkFMOw0KPiA+PiArDQo+ID4+ICsgICAgIC8qIGdwaW8gaW50ZXJydXB0IHBpbiAqLw0KPiA+
PiArICAgICBncGlvID0gZGV2bV9ncGlvZF9nZXRfaW5kZXgoJnNwaS0+ZGV2LCBNQVgzMTcyMl9H
UElPLCAwLA0KPiBHUElPRF9JTik7DQo+ID4+ICsgICAgIGlmIChJU19FUlIoZ3BpbykpIHsNCj4g
Pj4gKyAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwgImdwaW8gcmVxdWVzdCBmYWlsZWQu
XG4iKTsNCj4gPj4gKyAgICAgICAgICAgICByZXR1cm4gUFRSX0VSUihncGlvKTsNCj4gPj4gKyAg
ICAgfQ0KPiA+IElmIHRoaXMgaXMgYW4gaW50ZXJydXB0LCB3aHkgYXJlIHdlIHNwZWNpZnlpbmcg
aXQgYXMgYSBncGlvPw0KPiANCj4gSXQgaXMgYW4gaW50ZXJydXB0IHJlcXVlc3RlZCBvbiBhIEdQ
SU8gcGluLiBJc24ndCB0aGlzIHRoZSBzdGFuZGFyZCB3YXkgb2YNCj4gbWFwcGluZyBhIEdQSU8g
cGluIHRvIGFuIElSUT8gQW55aG93LCBhcyBleHBsYWluZWQgYmVsb3cgdGhpcyBpcyBubyBsb25n
ZXINCj4gbmVlZGVkLg0KDQpBY3R1YWxseSwgd2l0aG91dCBjYWxsaW5nIGRldm1fZ3Bpb2RfZ2V0
X2luZGV4LCB3aGljaCBldmVudHVhbGx5IGNhbGxzIGdwaW9kX3JlcXVlc3QsDQpJJ20gdW5hYmxl
IHRvIHJlY2VpdmUgaW50ZXJydXB0cy4NCg0KPiANCj4gPj4gKw0KPiA+PiArICAgICByZXR1cm4g
Z3Bpb2RfdG9faXJxKGdwaW8pOw0KPiA+PiArfQ0KPiA+PiArDQo+ID4+ICBzdGF0aWMgaW50IG1h
eDMxNzIyX3Byb2JlKHN0cnVjdCBzcGlfZGV2aWNlICpzcGkpICB7DQo+ID4+ICAgICAgIGludCBy
ZXQ7DQo+ID4+IEBAIC0yOTQsMTUgKzU3NCwzNCBAQCBzdGF0aWMgaW50IG1heDMxNzIyX3Byb2Jl
KHN0cnVjdCBzcGlfZGV2aWNlDQo+ID4+ICpzcGkpDQo+ID4+DQo+ID4+ICAgICAgIHJldCA9IG1h
eDMxNzIyX2luaXQoZGF0YSk7DQo+ID4+ICAgICAgIGlmIChyZXQgPCAwKQ0KPiA+PiAtICAgICAg
ICAgICAgIHJldHVybiByZXQ7DQo+ID4+ICsgICAgICAgICAgICAgZ290byBlcnJfc3RhbmRieTsN
Cj4gPj4gKw0KPiA+PiArICAgICByZXQgPSBtYXgzMTcyMl9ncGlvX3Byb2JlKGRhdGEtPnNwaV9k
ZXZpY2UpOw0KPiA+PiArICAgICBpZiAocmV0IDwgMCkNCj4gPj4gKyAgICAgICAgICAgICBnb3Rv
IGVycl9zdGFuZGJ5Ow0KPiA+PiArDQo+ID4+ICsgICAgIHNwaS0+aXJxID0gcmV0Ow0KPiANCj4g
SWYgQUNQSSBvciBEVCBpcyBwcm9wZXJseSBjb25maWd1cmVkIHNwaS0+aXJxIHNob3VsZCBhbHJl
YWR5IGNvbnRhaW4gdGhlDQo+IGNvcnJlY3QgaXJxIG51bWJlci4gV2UgdXNlZCB0aGUgZ3Bpb19w
cm9iZSBhcHByb2FjaCB3aGVuIEFDUEkgZGlkbid0DQo+IHNhdmUgdGhlIGludGVycnVwdCBudW1i
ZXIuIE5ld2VyIGtlcm5lbCB2ZXJzaW9uIHNob3VsZG4ndCBoYXZlIHRoaXMNCj4gcHJvYmxlbS4N
Cj4NCg0KVGhlIEFDUEkgY29yZSBpcyBpbmRlZWQgYWxsb2NhdGluZyBhIHZhbGlkIHNwaS0+aXJx
IG51bWJlciwgaG93ZXZlciwgZ3Bpbw0KSW50ZXJydXB0cyBkb24ndCB3b3JrIHVubGVzcyBncGlv
ZF9yZXF1ZXN0IGlzIGNhbGxlZC4gUGVyaGFwcyB0aGlzIGlzIGR1ZSB0bw0KYW4gaXNzdWUgaW4g
dGhlIFNQSSBjb3JlPw0KDQo+ID4+ICsgICAgIHJldCA9IGRldm1fcmVxdWVzdF90aHJlYWRlZF9p
cnEoJnNwaS0+ZGV2LCBzcGktPmlycSwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBtYXgzMTcyMl9pcnFfaGFuZGxlciwNCj4gPj4gKyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBtYXgzMTcyMl9pcnFfZXZlbnRfaGFuZGxlciwNCj4gPj4gKyAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBJUlFGX1RSSUdHRVJfTE9XLA0KPiA+
PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIE1BWDMxNzIyX0VWRU5ULCBp
bmRpb19kZXYpOw0KPiA+PiArICAgICBpZiAocmV0IDwgMCkgew0KPiA+PiArICAgICAgICAgICAg
IGRldl9lcnIoJnNwaS0+ZGV2LCAicmVxdWVzdCBpcnEgJWQgZmFpbGVkXG4iLCBzcGktPmlycSk7
DQo+ID4+ICsgICAgICAgICAgICAgZ290byBlcnJfc3RhbmRieTsNCj4gPj4gKyAgICAgfQ0KPiA+
IEkgd291bGQgc3VnZ2VzdCB5b3Ugd2FudCB0byBtYWtlIHRoZSBpcnEgb3B0aW9uYWwsIHRob3Vn
aCB0aGF0IGNvdWxkDQo+ID4gaGFwcGVuIGF0IGEgbGF0ZXIgZGF0ZSB3aGVuIHNvbWVvbmUgaGFz
IGEgYm9hcmQgd2hlcmUgaXQgaXNuJ3Qgd2lyZWQuDQo+IA0KPiBBZ3JlZS4gV2UgaGFkIHByZXZp
b3VzIGV4cGVyaWVuY2Ugd2l0aCB1bndpcmVkIGludGVycnVwdHMgcGlucy4NCj4gDQo+ID4+DQo+
ID4+ICAgICAgIHJldCA9IGlpb19kZXZpY2VfcmVnaXN0ZXIoaW5kaW9fZGV2KTsNCj4gPj4gICAg
ICAgaWYgKHJldCA8IDApIHsNCj4gPj4gICAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwg
Imlpb19kZXZpY2VfcmVnaXN0ZXIgZmFpbGVkXG4iKTsNCj4gPj4gLSAgICAgICAgICAgICByZWdt
YXBfZmllbGRfd3JpdGUoZGF0YS0+cmVnX3N0YXRlLA0KPiBNQVgzMTcyMl9NT0RFX1NUQU5EQlkp
Ow0KPiA+PiArICAgICAgICAgICAgIGdvdG8gZXJyX3N0YW5kYnk7DQo+ID4+ICAgICAgIH0NCj4g
Pj4NCj4gPj4gICAgICAgcmV0dXJuIHJldDsNCj4gPj4gKw0KPiA+PiArZXJyX3N0YW5kYnk6DQo+
ID4+ICsgICAgIHJlZ21hcF9maWVsZF93cml0ZShkYXRhLT5yZWdfc3RhdGUsIE1BWDMxNzIyX01P
REVfU1RBTkRCWSk7DQo+ID4+ICsgICAgIHJldHVybiByZXQ7DQo+ID4+ICB9DQo+ID4+DQo+ID4+
ICBzdGF0aWMgaW50IG1heDMxNzIyX3JlbW92ZShzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpKQ0KPiA+
Pg0KPiA+DQo+ID4gLS0NCj4gPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0
aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtaWlvIg0KPiA+IGluIHRoZSBib2R5IG9mIGEgbWVz
c2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnIE1vcmUNCj4gbWFqb3Jkb21vDQo+ID4g
aW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo=

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 1/2] iio: temperature: Add support for MAX31722/MAX31723 temperature sensors
  2016-03-14 13:45     ` Breana, Tiberiu A
@ 2016-03-14 17:31       ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-14 17:31 UTC (permalink / raw)
  To: Breana, Tiberiu A, linux-iio; +Cc: Baluta, Daniel



On 14 March 2016 13:45:29 GMT+00:00, "Breana, Tiberiu A" <tiberiu.a.breana@intel.com> wrote:
>> -----Original Message-----
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> Sent: Saturday, March 12, 2016 11:57 AM
>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>> iio@vger.kernel.org
>> Subject: Re: [PATCH 1/2] iio: temperature: Add support for
>> MAX31722/MAX31723 temperature sensors
>> 
>> On 09/03/16 13:30, Tiberiu Breana wrote:
>> > Add basic support for the Maxim Integrated MAX31722/MAX31723 SPI
>> > temperature sensors / thermostats.
>> >
>> > Includes:
>> > 	- ACPI support;
>> > 	- raw temperature readings;
>> > 	- ability to set measurement resolution;
>> > 	- power management
>> >
>> > Datasheet:
>> > https://datasheets.maximintegrated.com/en/ds/MAX31722-
>> MAX31723.pdf
>> >
>> > Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
>> I thought I'd give this a quick review, putting aside the question of
>whether it
>> should be in IIO at all.
>
>What are the criteria for a temperature driver to be in IIO vs hwmon? I
>see there's another temperature sensor in IIO - TSYS01
>(http://www.meas-spec.com/product/temperature/TSYS01.aspx) which is
>used for "Industrial Control, Replacement of Thermistors and NTCs,
>Heating / Cooling Systems, HVAC".
There unfortunately isn't a clear divide and we do have the iio-hwmon bridge driver
to paper over the cracks.  The measspec drivers are an oddity as they are actually
interface matching with pressure and humidity devices (probably the same silicon?) Decision there was made
with the hwmon guys was that we were better off not duplicating drivers and
having the temp drivers as IIO drivers despite the irritation of using the bridge
 driver when hwmon type usage was desired.

Rule of thumb (beyond persuading hwmon maintainers) I go by is that there
needs to be a usecase or code related reason that hwmon is unsuitable.

Jonathan
>
>> 
>> A few really trivial suggestions, but otherwise a nice clean driver.
>> 
>> Jonathan
>> > ---
>> >  drivers/iio/temperature/Kconfig    |  12 ++
>> >  drivers/iio/temperature/Makefile   |   1 +
>> >  drivers/iio/temperature/max31722.c | 374
>> > +++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 387 insertions(+)
>> >  create mode 100644 drivers/iio/temperature/max31722.c
>> >
>> > diff --git a/drivers/iio/temperature/Kconfig
>> > b/drivers/iio/temperature/Kconfig index c4664e5..7587887 100644
>> > --- a/drivers/iio/temperature/Kconfig
>> > +++ b/drivers/iio/temperature/Kconfig
>> > @@ -3,6 +3,18 @@
>> >  #
>> >  menu "Temperature sensors"
>> >
>> > +config MAX31722
>> > +	tristate "MAX31722 temperature sensor"
>> > +	depends on SPI
>> > +	select REGMAP_SPI
>> > +	help
>> > +	  If you say yes here you get support for the Maxim Integrated
>> > +	  MAX31722/MAX31723 digital thermometers / thermostats
>> operating
>> > +	  over an SPI interface.
>> > +
>> > +	  This driver can also be built as a module. If so, the module
>will
>> > +	  be called max31722.
>> > +
>> >  config MLX90614
>> >  	tristate "MLX90614 contact-less infrared sensor"
>> >  	depends on I2C
>> > diff --git a/drivers/iio/temperature/Makefile
>> > b/drivers/iio/temperature/Makefile
>> > index 02bc79d..a9484d6 100644
>> > --- a/drivers/iio/temperature/Makefile
>> > +++ b/drivers/iio/temperature/Makefile
>> > @@ -2,6 +2,7 @@
>> >  # Makefile for industrial I/O temperature drivers  #
>> >
>> > +obj-$(CONFIG_MAX31722) += max31722.o
>> >  obj-$(CONFIG_MLX90614) += mlx90614.o
>> >  obj-$(CONFIG_TMP006) += tmp006.o
>> >  obj-$(CONFIG_TSYS01) += tsys01.o
>> > diff --git a/drivers/iio/temperature/max31722.c
>> > b/drivers/iio/temperature/max31722.c
>> > new file mode 100644
>> > index 0000000..fa833b6
>> > --- /dev/null
>> > +++ b/drivers/iio/temperature/max31722.c
>> > @@ -0,0 +1,374 @@
>> > +/**
>> > + * MAX31722/MAX31723 digital thermometer and thermostat SPI driver
>> > + *
>> > + * Copyright (c) 2016, Intel Corporation.
>> > + *
>> > + * This file is subject to the terms and conditions of version 2
>of
>> > + * the GNU General Public License. See the file COPYING in the
>main
>> > + * directory of this archive for more details.
>> > + */
>> > +
>> > +#include <linux/kernel.h>
>> > +#include <linux/acpi.h>
>> > +#include <linux/module.h>
>> > +#include <linux/regmap.h>
>> > +#include <linux/spi/spi.h>
>> > +#include <linux/iio/iio.h>
>> > +#include <linux/iio/sysfs.h>
>> > +
>> > +#define MAX31722_REG_CFG				0x00
>> > +#define MAX31722_REG_TEMP_LSB			0x01
>> > +#define MAX31722_REG_TEMP_MSB			0x02
>> > +#define MAX31722_MAX_REG				0x86
>> > +
>> > +#define MAX31722_MODE_CONTINUOUS		0x00
>> > +#define MAX31722_MODE_STANDBY			0x01
>> > +
>> > +#define MAX31722_REGMAP_NAME
>> 	"max31722_regmap"
>> > +
>> > +#define MAX31722_SCALE_AVAILABLE		"0.5 0.25 0.125
>> 0.0625"
>> > +
>> > +#define MAX31722_REGFIELD(name)
>> 		 \
>> > +	do {								 \
>> > +		data->reg_##name =					 \
>> > +			devm_regmap_field_alloc(&spi->dev, regmap,	 \
>> > +				max31722_reg_field_##name);
>> 	 \
>> > +		if (IS_ERR(data->reg_##name)) {
>> 	 \
>> > +			dev_err(&spi->dev, "reg field alloc failed.\n"); \
>> > +			return PTR_ERR(data->reg_##name);		 \
>> > +		}							 \
>> > +	} while (0)
>> > +
>> > +static const struct reg_field max31722_reg_field_state =
>> > +				REG_FIELD(MAX31722_REG_CFG, 0, 0); static
>> const struct reg_field
>> > +max31722_reg_field_resolution =
>> > +				REG_FIELD(MAX31722_REG_CFG, 1, 2);
>> > +
>> > +struct max31722_data {
>> > +	struct spi_device *spi_device;
>> > +	struct mutex lock;
>> > +	struct regmap *regmap;
>> > +	struct regmap_field *reg_state;
>> > +	struct regmap_field *reg_resolution; };
>> > +
>> > +/*
>> > + * This table can double as a negative exponent value lookup table
>> > + * (as micro units) for calculating LSB temperature values.
>> > + */
>> > +static const int max31722_scale_table[4] = {
>> > +	500000, 250000, 125000, 62500
>> > +};
>> > +
>> > +static IIO_CONST_ATTR(in_temp_scale_available,
>> > +MAX31722_SCALE_AVAILABLE);
>> > +
>> > +static struct attribute *max31722_attributes[] = {
>> > +	&iio_const_attr_in_temp_scale_available.dev_attr.attr,
>> > +	NULL
>> > +};
>> > +
>> > +static const struct attribute_group max31722_attribute_group = {
>> > +	.attrs = max31722_attributes
>> > +};
>> > +
>> > +static const struct iio_chan_spec max31722_channels[] = {
>> > +	{
>> > +		.type = IIO_TEMP,
>> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> > +				      BIT(IIO_CHAN_INFO_SCALE),
>> > +	},
>> > +};
>> > +
>> > +static bool max31722_is_volatile_reg(struct device *dev, unsigned
>int
>> > +reg) {
>> > +	switch (reg) {
>> > +	case MAX31722_REG_TEMP_LSB:
>> > +	case MAX31722_REG_TEMP_MSB:
>> > +		return true;
>> > +	default:
>> > +		return false;
>> > +	}
>> > +}
>> > +
>> > +static bool max31722_is_writeable_reg(struct device *dev, unsigned
>> > +int reg) {
>> > +	switch (reg) {
>> > +	case MAX31722_REG_TEMP_LSB:
>> > +	case MAX31722_REG_TEMP_MSB:
>> > +		return false;
>> > +	default:
>> > +		return true;
>> > +	}
>> > +}
>> > +
>> > +static struct regmap_config max31722_regmap_config = {
>> > +	.name = MAX31722_REGMAP_NAME,
>> > +	.reg_bits = 8,
>> > +	.val_bits = 8,
>> > +
>> > +	.max_register = MAX31722_MAX_REG,
>> > +	.cache_type = REGCACHE_RBTREE,
>> > +
>> > +	.volatile_reg = max31722_is_volatile_reg,
>> > +	.writeable_reg = max31722_is_writeable_reg,
>> > +
>> > +	.read_flag_mask = 0x00,
>> > +	.write_flag_mask = 0x80,
>> > +};
>> > +
>> > +/*
>> > + * Convert a temperature register value to a floating point value.
>> > + * Data is represented in 9-12 bits, two's complement.
>> > + * The MSB contains the integer part of the temperature value
>> > + * (signed), while the LSB contains the fractional part as
>> > + * negative powers of 2, e.g.: bit 0 is 2^-1, bit 1 is 2^-2, etc.
>> > + */
>> > +static void max31722_reg_to_float(u16 reg_val, int *val, int
>*val2) {
>> > +	int i;
>> > +	u8 msb;
>> > +	u8 lsb;
>> > +
>> > +	msb = reg_val >> 8;
>> > +	lsb = (reg_val << 8) >> 8;
>> > +
>> > +	/* Calculate the integer part. */
>> > +	if (msb & 0x80) {
>> > +		/* Negative value. Reverse bits and add 1. */
>> > +		u16 orig = reg_val;
>> > +		u16 rev = 0;
>> > +		int num_bits = sizeof(rev) * 8;
>> > +
>> > +		for (i = 0 ; i < num_bits ; i++) {
>> > +			rev |= !(orig & 0x01) << i;
>> > +			orig >>= 1;
>> > +		}
>> > +		rev += 1;
>> > +		msb = rev >> 8;
>> > +		lsb = (rev << 8) >> 8;
>> > +		*val = (-1) * msb;
>> > +	} else { /* Positive value. */
>> > +		*val = msb;
>> > +	}
>> > +
>> > +	/*
>> > +	 * Calculate the fractional part.
>> > +	 * Only the first four LSB bits contain data.
>> > +	 */
>> > +	lsb >>= 4;
>> > +	i = 0;
>> > +	*val2 = 0;
>> > +	while (i < 4) {
>> > +		if ((lsb >> (4 - i - 1)) & 0x01)
>> > +			*val2 += max31722_scale_table[i];
>> > +		lsb <<= 1;
>> > +		lsb >>= 1;
>> > +		i++;
>> > +	}
>> > +}
>> > +
>> > +static int max31722_read_raw(struct iio_dev *indio_dev,
>> > +			     struct iio_chan_spec const *chan,
>> > +			     int *val, int *val2, long mask) {
>> > +	int ret;
>> > +	int index;
>> > +	u16 buf;
>> > +	struct max31722_data *data = iio_priv(indio_dev);
>> > +	struct spi_device *spi = data->spi_device;
>> > +
>> > +	switch (mask) {
>> > +	case IIO_CHAN_INFO_RAW:
>> > +		mutex_lock(&data->lock);
>> > +		ret = regmap_bulk_read(data->regmap,
>> > +				       MAX31722_REG_TEMP_LSB, &buf, 2);
>> > +		if (ret < 0) {
>> > +			dev_err(&spi->dev, "failed to read temperature
>> register\n");
>> > +			mutex_unlock(&data->lock);
>> > +			return ret;
>> > +		}
>> > +		max31722_reg_to_float(buf, val, val2);
>> > +		mutex_unlock(&data->lock);
>> > +		return IIO_VAL_INT_PLUS_MICRO;
>> > +	case IIO_CHAN_INFO_SCALE:
>> > +		mutex_lock(&data->lock);
>> > +		ret = regmap_field_read(data->reg_resolution, &index);
>> > +		if (ret < 0) {
>> > +			dev_err(&spi->dev, "failed to read configuration
>> register\n");
>> > +			mutex_unlock(&data->lock);
>> > +			return ret;
>> > +		}
>> > +		mutex_unlock(&data->lock);
>> > +		*val = 0;
>> > +		*val2 = max31722_scale_table[index];
>> > +		return IIO_VAL_INT_PLUS_MICRO;
>> > +	}
>> > +
>> > +	return -EINVAL;
>> > +}
>> > +
>> > +static int max31722_write_raw(struct iio_dev *indio_dev,
>> > +			      struct iio_chan_spec const *chan,
>> > +			      int val, int val2, long mask) {
>> > +	int i;
>> > +	int ret;
>> > +	int index = -1;
>> > +	struct max31722_data *data = iio_priv(indio_dev);
>> > +
>> > +	switch (mask) {
>> > +	case IIO_CHAN_INFO_SCALE:
>> > +		for (i = 0; i < ARRAY_SIZE(max31722_scale_table); i++) {
>> > +			if (val == 0 && val2 == max31722_scale_table[i]) {
>> > +				index = i;
>> > +				break;
>> > +			}
>> > +		}
>> > +		if (index < 0)
>> > +			return -EINVAL;
>> > +
>> > +		mutex_lock(&data->lock);
>> > +		ret = regmap_field_write(data->reg_resolution, index);
>> > +		mutex_unlock(&data->lock);
>> > +
>> > +		return ret;
>> > +	}
>> > +	return -EINVAL;
>> > +}
>> > +
>> > +static const struct iio_info max31722_info = {
>> > +	.driver_module		= THIS_MODULE,
>> > +	.read_raw		= max31722_read_raw,
>> > +	.write_raw		= max31722_write_raw,
>> > +	.attrs			= &max31722_attribute_group,
>> > +};
>> > +
>> > +static int max31722_init(struct max31722_data *data) {
>> > +	int ret = 0;
>> > +	struct spi_device *spi = data->spi_device;
>> > +	struct regmap *regmap;
>> > +
>> > +	/* Initialize the regmap */
>> > +	regmap = devm_regmap_init_spi(spi, &max31722_regmap_config);
>> > +	if (IS_ERR(regmap)) {
>> > +		dev_err(&spi->dev, "regmap initialization failed.\n");
>> > +		return PTR_ERR(regmap);
>> > +	}
>> > +	data->regmap = regmap;
>> > +
>> > +	MAX31722_REGFIELD(state);
>> > +	MAX31722_REGFIELD(resolution);
>> > +
>> > +	/* Set SD bit to 0 so we can have continuous measurements. */
>> > +	ret = regmap_field_write(data->reg_state,
>> MAX31722_MODE_CONTINUOUS);
>> > +	if (ret < 0)
>> > +		dev_err(&spi->dev, "failed to configure sensor.\n");
>> > +
>> > +	return ret;
>> > +}
>> > +
>> > +static int max31722_probe(struct spi_device *spi) {
>> > +	int ret;
>> > +	struct iio_dev *indio_dev;
>> > +	struct max31722_data *data;
>> > +
>> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>> > +	if (!indio_dev) {
>> > +		dev_err(&spi->dev, "iio allocation failed!\n");
>> > +		return -ENOMEM;
>> > +	}
>> > +
>> > +	data = iio_priv(indio_dev);
>> > +	data->spi_device = spi;
>> > +	spi_set_drvdata(spi, indio_dev);
>> > +	mutex_init(&data->lock);
>> > +
>> > +	indio_dev->dev.parent = &spi->dev;
>> > +	indio_dev->info = &max31722_info;
>> > +	indio_dev->name = spi_get_device_id(spi)->name;
>> > +	indio_dev->modes = INDIO_DIRECT_MODE;
>> > +	indio_dev->channels = max31722_channels;
>> > +	indio_dev->num_channels = ARRAY_SIZE(max31722_channels);
>> > +
>> > +	ret = max31722_init(data);
>> > +	if (ret < 0)
>> > +		return ret;
>> > +
>> > +	ret = iio_device_register(indio_dev);
>> > +	if (ret < 0) {
>> > +		dev_err(&spi->dev, "iio_device_register failed\n");
>> Given this statement is effectively unwinding max31722_init, would
>have a
>> max31722_exit function as well with this in.  That makes for
>'obviously
>> correct' code which is always good for reviewers!
>> 
>> Alternative would be to pull the mode setting out of the init
>function and
>> have it inline in this function making the pairing obvious as well.
>> 
>> > +		regmap_field_write(data->reg_state,
>> MAX31722_MODE_STANDBY);
>> > +	}
>> > +
>> > +	return ret;
>> > +}
>> > +
>> > +static int max31722_remove(struct spi_device *spi) {
>> > +	struct max31722_data *data;
>> > +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> > +
>> > +	iio_device_unregister(indio_dev);
>> > +	data = iio_priv(indio_dev);
>> 
>> I would have put this inline with the declaration of data above.
>> It is pretty much the same level of lookup as spi_get_drvdata.
>> 
>> > +
>> > +	return regmap_field_write(data->reg_state,
>> MAX31722_MODE_STANDBY); }
>> > +
>> > +#ifdef CONFIG_PM_SLEEP
>> > +static int max31722_suspend(struct device *dev) {
>> > +	struct max31722_data *data;
>> > +
>> > +	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
>> > +
>> > +	return regmap_field_write(data->reg_state,
>> MAX31722_MODE_STANDBY); }
>> > +
>> > +static int max31722_resume(struct device *dev) {
>> > +	struct max31722_data *data;
>> > +
>> > +	data = iio_priv(spi_get_drvdata(to_spi_device(dev)));
>> > +
>> > +	return regmap_field_write(data->reg_state,
>> > +MAX31722_MODE_CONTINUOUS); }
>> > +
>> > +static SIMPLE_DEV_PM_OPS(max31722_pm_ops, max31722_suspend,
>> > +max31722_resume);
>> > +
>> > +#define MAX31722_PM_OPS (&max31722_pm_ops) #else #define
>> > +MAX31722_PM_OPS NULL #endif
>> > +
>> > +static const struct spi_device_id max31722_spi_id[] = {
>> > +	{"max31722", 0},
>> > +	{"max31723", 0},
>> > +	{}
>> > +};
>> > +
>> > +static const struct acpi_device_id max31722_acpi_id[] = {
>> > +	{"MAX31722", 0},
>> > +	{"MAX31723", 0},
>> > +	{}
>> > +};
>> > +
>> > +MODULE_DEVICE_TABLE(spi, max31722_spi_id);
>> > +
>> > +static struct spi_driver max31722_driver = {
>> > +	.driver = {
>> > +		.name = "max31722",
>> > +		.pm = MAX31722_PM_OPS,
>> > +		.acpi_match_table = ACPI_PTR(max31722_acpi_id),
>> > +	},
>> > +	.probe =            max31722_probe,
>> > +	.remove =           max31722_remove,
>> > +	.id_table =         max31722_spi_id,
>> > +};
>> > +
>> > +module_spi_driver(max31722_driver);
>> > +
>> > +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
>> > +MODULE_DESCRIPTION("MAX31722 digital thermometer and thermostat
>> SPI
>> > +driver"); MODULE_LICENSE("GPL v2");
>> >
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-09 20:31 ` [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Jonathan Cameron
@ 2016-03-15 12:58   ` Breana, Tiberiu A
  2016-03-15 13:16       ` [lm-sensors] " Breana, Tiberiu A
  2016-03-15 13:34       ` [lm-sensors] " Guenter Roeck
  0 siblings, 2 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-15 12:58 UTC (permalink / raw)
  To: linux, lm-sensors; +Cc: Jonathan Cameron, linux-iio, Baluta, Daniel

Hello Guenter, hwmon,

We have a dilemma as to wether this device driver belongs in IIO or hwmon:
https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf

Could you please give us your opinion on this?
Thanks,

Tiberiu

> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> Sent: Wednesday, March 9, 2016 10:32 PM
> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> iio@vger.kernel.org
> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> temperature sensors
>=20
> On 09/03/16 13:30, Tiberiu Breana wrote:
> > This patch set adds support for the MAX31722/MAX31723 temperature
> > sensors / thermostats.
> > Patch 1 adds basic support and power management.
> > Patch 2 adds threshold interrupt support.
> >
> > Tiberiu Breana (2):
> >   iio: temperature: Add support for MAX31722/MAX31723 temperature
> >     sensors
> >   iio: temperature: Add threshold interrupt support for max31722
> >
> >  drivers/iio/temperature/Kconfig    |  12 +
> >  drivers/iio/temperature/Makefile   |   1 +
> >  drivers/iio/temperature/max31722.c | 673
> > +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 686 insertions(+)
> >  create mode 100644 drivers/iio/temperature/max31722.c
> >
> Hi Tiberiu,
>=20
> When proposing a temperature sensor driver, we need some additional
> justification for why it is suited for IIO rather than the more focused (=
and in
> someways simpler) hwmon.
>=20
> So far we have had thermophiles that don't fit in as devices typically us=
ed for
> hardware monitoring. Also in there are a few drivers which are for parts
> where they are a cut down version of a more complex sensor (the
> measurement specialties parts are either pressure sensors or humidty
> sensors at least in terms of what they share interfaces with)
>=20
> Also, please cc the hwmon maintainers (and probably list) as we want thei=
r
> agreement.
>=20
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in t=
he body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 12:58   ` Breana, Tiberiu A
@ 2016-03-15 13:16       ` Breana, Tiberiu A
  2016-03-15 13:34       ` [lm-sensors] " Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-15 13:16 UTC (permalink / raw)
  To: linux, lm-sensors; +Cc: Jonathan Cameron, linux-iio, Baluta, Daniel

Forwarding to the proper list address.=0A=
________________________________________=0A=
From: linux-iio-owner@vger.kernel.org [linux-iio-owner@vger.kernel.org] on =
behalf of Breana, Tiberiu A [tiberiu.a.breana@intel.com]=0A=
Sent: Tuesday, March 15, 2016 2:58 PM=0A=
To: linux@roeck-us.net; lm-sensors@vger.kernel.org=0A=
Cc: Jonathan Cameron; linux-iio@vger.kernel.org; Baluta, Daniel=0A=
Subject: RE: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sens=
ors=0A=
=0A=
Hello Guenter, hwmon,=0A=
=0A=
We have a dilemma as to wether this device driver belongs in IIO or hwmon:=
=0A=
https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf=0A=
=0A=
Could you please give us your opinion on this?=0A=
Thanks,=0A=
=0A=
Tiberiu=0A=
=0A=
> -----Original Message-----=0A=
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-=0A=
> owner@vger.kernel.org] On Behalf Of Jonathan Cameron=0A=
> Sent: Wednesday, March 9, 2016 10:32 PM=0A=
> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-=0A=
> iio@vger.kernel.org=0A=
> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723=0A=
> temperature sensors=0A=
>=0A=
> On 09/03/16 13:30, Tiberiu Breana wrote:=0A=
> > This patch set adds support for the MAX31722/MAX31723 temperature=0A=
> > sensors / thermostats.=0A=
> > Patch 1 adds basic support and power management.=0A=
> > Patch 2 adds threshold interrupt support.=0A=
> >=0A=
> > Tiberiu Breana (2):=0A=
> >   iio: temperature: Add support for MAX31722/MAX31723 temperature=0A=
> >     sensors=0A=
> >   iio: temperature: Add threshold interrupt support for max31722=0A=
> >=0A=
> >  drivers/iio/temperature/Kconfig    |  12 +=0A=
> >  drivers/iio/temperature/Makefile   |   1 +=0A=
> >  drivers/iio/temperature/max31722.c | 673=0A=
> > +++++++++++++++++++++++++++++++++++++=0A=
> >  3 files changed, 686 insertions(+)=0A=
> >  create mode 100644 drivers/iio/temperature/max31722.c=0A=
> >=0A=
> Hi Tiberiu,=0A=
>=0A=
> When proposing a temperature sensor driver, we need some additional=0A=
> justification for why it is suited for IIO rather than the more focused (=
and in=0A=
> someways simpler) hwmon.=0A=
>=0A=
> So far we have had thermophiles that don't fit in as devices typically us=
ed for=0A=
> hardware monitoring. Also in there are a few drivers which are for parts=
=0A=
> where they are a cut down version of a more complex sensor (the=0A=
> measurement specialties parts are either pressure sensors or humidty=0A=
> sensors at least in terms of what they share interfaces with)=0A=
>=0A=
> Also, please cc the hwmon maintainers (and probably list) as we want thei=
r=0A=
> agreement.=0A=
>=0A=
> Jonathan=0A=
> --=0A=
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in t=
he body=0A=
> of a message to majordomo@vger.kernel.org More majordomo info at=0A=
> http://vger.kernel.org/majordomo-info.html=0A=
--=0A=
To unsubscribe from this list: send the line "unsubscribe linux-iio" in=0A=
the body of a message to majordomo@vger.kernel.org=0A=
More majordomo info at  http://vger.kernel.org/majordomo-info.html=0A=

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-15 13:16       ` Breana, Tiberiu A
  0 siblings, 0 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-15 13:16 UTC (permalink / raw)
  To: linux, lm-sensors; +Cc: Jonathan Cameron, linux-iio, Baluta, Daniel

Forwarding to the proper list address.
________________________________________
From: linux-iio-owner@vger.kernel.org [linux-iio-owner@vger.kernel.org] on behalf of Breana, Tiberiu A [tiberiu.a.breana@intel.com]
Sent: Tuesday, March 15, 2016 2:58 PM
To: linux@roeck-us.net; lm-sensors@vger.kernel.org
Cc: Jonathan Cameron; linux-iio@vger.kernel.org; Baluta, Daniel
Subject: RE: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors

Hello Guenter, hwmon,

We have a dilemma as to wether this device driver belongs in IIO or hwmon:
https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf

Could you please give us your opinion on this?
Thanks,

Tiberiu

> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> Sent: Wednesday, March 9, 2016 10:32 PM
> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> iio@vger.kernel.org
> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> temperature sensors
>
> On 09/03/16 13:30, Tiberiu Breana wrote:
> > This patch set adds support for the MAX31722/MAX31723 temperature
> > sensors / thermostats.
> > Patch 1 adds basic support and power management.
> > Patch 2 adds threshold interrupt support.
> >
> > Tiberiu Breana (2):
> >   iio: temperature: Add support for MAX31722/MAX31723 temperature
> >     sensors
> >   iio: temperature: Add threshold interrupt support for max31722
> >
> >  drivers/iio/temperature/Kconfig    |  12 +
> >  drivers/iio/temperature/Makefile   |   1 +
> >  drivers/iio/temperature/max31722.c | 673
> > +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 686 insertions(+)
> >  create mode 100644 drivers/iio/temperature/max31722.c
> >
> Hi Tiberiu,
>
> When proposing a temperature sensor driver, we need some additional
> justification for why it is suited for IIO rather than the more focused (and in
> someways simpler) hwmon.
>
> So far we have had thermophiles that don't fit in as devices typically used for
> hardware monitoring. Also in there are a few drivers which are for parts
> where they are a cut down version of a more complex sensor (the
> measurement specialties parts are either pressure sensors or humidty
> sensors at least in terms of what they share interfaces with)
>
> Also, please cc the hwmon maintainers (and probably list) as we want their
> agreement.
>
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 12:58   ` Breana, Tiberiu A
@ 2016-03-15 13:34       ` Guenter Roeck
  2016-03-15 13:34       ` [lm-sensors] " Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2016-03-15 13:34 UTC (permalink / raw)
  To: Breana, Tiberiu A, LM Sensors; +Cc: Jonathan Cameron, linux-iio, Baluta, Daniel

On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> Hello Guenter, hwmon,
>
> We have a dilemma as to wether this device driver belongs in IIO or hwmon:
> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
>
> Could you please give us your opinion on this?

The chip has limit registers and an alarm signal output, suggesting that
its intended use is for hardware monitoring purposes.

The conversion time is between 50ms and 200ms, which suggests that the
faster ABI provided by iio isn't really beneficial.

Given all that, I think that hwmon would be a better place.

If you want to keep the driver in iio, I would suggest to consider enhancing
the iio-hwmon bridge with threshold/limit support if that is possible.
This would ensure that the chip can be used for its intended purpose.

Thanks,
Guenter

> Thanks,
>
> Tiberiu
>
>> -----Original Message-----
>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
>> Sent: Wednesday, March 9, 2016 10:32 PM
>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>> iio@vger.kernel.org
>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
>> temperature sensors
>>
>> On 09/03/16 13:30, Tiberiu Breana wrote:
>>> This patch set adds support for the MAX31722/MAX31723 temperature
>>> sensors / thermostats.
>>> Patch 1 adds basic support and power management.
>>> Patch 2 adds threshold interrupt support.
>>>
>>> Tiberiu Breana (2):
>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
>>>      sensors
>>>    iio: temperature: Add threshold interrupt support for max31722
>>>
>>>   drivers/iio/temperature/Kconfig    |  12 +
>>>   drivers/iio/temperature/Makefile   |   1 +
>>>   drivers/iio/temperature/max31722.c | 673
>>> +++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 686 insertions(+)
>>>   create mode 100644 drivers/iio/temperature/max31722.c
>>>
>> Hi Tiberiu,
>>
>> When proposing a temperature sensor driver, we need some additional
>> justification for why it is suited for IIO rather than the more focused (and in
>> someways simpler) hwmon.
>>
>> So far we have had thermophiles that don't fit in as devices typically used for
>> hardware monitoring. Also in there are a few drivers which are for parts
>> where they are a cut down version of a more complex sensor (the
>> measurement specialties parts are either pressure sensors or humidty
>> sensors at least in terms of what they share interfaces with)
>>
>> Also, please cc the hwmon maintainers (and probably list) as we want their
>> agreement.
>>
>> Jonathan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body
>> of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-15 13:34       ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2016-03-15 13:34 UTC (permalink / raw)
  To: Breana, Tiberiu A, LM Sensors; +Cc: Jonathan Cameron, linux-iio, Baluta, Daniel

On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> Hello Guenter, hwmon,
>
> We have a dilemma as to wether this device driver belongs in IIO or hwmon:
> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
>
> Could you please give us your opinion on this?

The chip has limit registers and an alarm signal output, suggesting that
its intended use is for hardware monitoring purposes.

The conversion time is between 50ms and 200ms, which suggests that the
faster ABI provided by iio isn't really beneficial.

Given all that, I think that hwmon would be a better place.

If you want to keep the driver in iio, I would suggest to consider enhancing
the iio-hwmon bridge with threshold/limit support if that is possible.
This would ensure that the chip can be used for its intended purpose.

Thanks,
Guenter

> Thanks,
>
> Tiberiu
>
>> -----Original Message-----
>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
>> Sent: Wednesday, March 9, 2016 10:32 PM
>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>> iio@vger.kernel.org
>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
>> temperature sensors
>>
>> On 09/03/16 13:30, Tiberiu Breana wrote:
>>> This patch set adds support for the MAX31722/MAX31723 temperature
>>> sensors / thermostats.
>>> Patch 1 adds basic support and power management.
>>> Patch 2 adds threshold interrupt support.
>>>
>>> Tiberiu Breana (2):
>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
>>>      sensors
>>>    iio: temperature: Add threshold interrupt support for max31722
>>>
>>>   drivers/iio/temperature/Kconfig    |  12 +
>>>   drivers/iio/temperature/Makefile   |   1 +
>>>   drivers/iio/temperature/max31722.c | 673
>>> +++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 686 insertions(+)
>>>   create mode 100644 drivers/iio/temperature/max31722.c
>>>
>> Hi Tiberiu,
>>
>> When proposing a temperature sensor driver, we need some additional
>> justification for why it is suited for IIO rather than the more focused (and in
>> someways simpler) hwmon.
>>
>> So far we have had thermophiles that don't fit in as devices typically used for
>> hardware monitoring. Also in there are a few drivers which are for parts
>> where they are a cut down version of a more complex sensor (the
>> measurement specialties parts are either pressure sensors or humidty
>> sensors at least in terms of what they share interfaces with)
>>
>> Also, please cc the hwmon maintainers (and probably list) as we want their
>> agreement.
>>
>> Jonathan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body
>> of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 13:34       ` [lm-sensors] " Guenter Roeck
@ 2016-03-15 17:28         ` Jonathan Cameron
  -1 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-15 17:28 UTC (permalink / raw)
  To: Guenter Roeck, Breana, Tiberiu A, LM Sensors
  Cc: Jonathan Cameron, linux-iio, Baluta, Daniel



On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-us.net> wrote:
>On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
>> Hello Guenter, hwmon,
>>
>> We have a dilemma as to wether this device driver belongs in IIO or
>hwmon:
>> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
>>
>> Could you please give us your opinion on this?
>
>The chip has limit registers and an alarm signal output, suggesting
>that
>its intended use is for hardware monitoring purposes.
>
>The conversion time is between 50ms and 200ms, which suggests that the
>faster ABI provided by iio isn't really beneficial.
>
>Given all that, I think that hwmon would be a better place.
Agreed.
>
>If you want to keep the driver in iio, I would suggest to consider
>enhancing
>the iio-hwmon bridge with threshold/limit support if that is possible.

Possible but non trivial as no one has yet implemented the core support for allowing
in kernel consumers to get events. It has been on the to-do list as a blocker of iio
 on IIO (which is really about reaching the point where the IIO userspace front end is
an optional nonspecial case consumer of
 IIO backends). Would be excellent to have this but may not be that straight forward.

Jonathan
>This would ensure that the chip can be used for its intended purpose.
>
>Thanks,
>Guenter
>
>> Thanks,
>>
>> Tiberiu
>>
>>> -----Original Message-----
>>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
>>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
>>> Sent: Wednesday, March 9, 2016 10:32 PM
>>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>>> iio@vger.kernel.org
>>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
>>> temperature sensors
>>>
>>> On 09/03/16 13:30, Tiberiu Breana wrote:
>>>> This patch set adds support for the MAX31722/MAX31723 temperature
>>>> sensors / thermostats.
>>>> Patch 1 adds basic support and power management.
>>>> Patch 2 adds threshold interrupt support.
>>>>
>>>> Tiberiu Breana (2):
>>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
>>>>      sensors
>>>>    iio: temperature: Add threshold interrupt support for max31722
>>>>
>>>>   drivers/iio/temperature/Kconfig    |  12 +
>>>>   drivers/iio/temperature/Makefile   |   1 +
>>>>   drivers/iio/temperature/max31722.c | 673
>>>> +++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 686 insertions(+)
>>>>   create mode 100644 drivers/iio/temperature/max31722.c
>>>>
>>> Hi Tiberiu,
>>>
>>> When proposing a temperature sensor driver, we need some additional
>>> justification for why it is suited for IIO rather than the more
>focused (and in
>>> someways simpler) hwmon.
>>>
>>> So far we have had thermophiles that don't fit in as devices
>typically used for
>>> hardware monitoring. Also in there are a few drivers which are for
>parts
>>> where they are a cut down version of a more complex sensor (the
>>> measurement specialties parts are either pressure sensors or humidty
>>> sensors at least in terms of what they share interfaces with)
>>>
>>> Also, please cc the hwmon maintainers (and probably list) as we want
>their
>>> agreement.
>>>
>>> Jonathan
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
>in the body
>>> of a message to majordomo@vger.kernel.org More majordomo info at
>>> http://vger.kernel.org/majordomo-info.html
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-15 17:28         ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2016-03-15 17:28 UTC (permalink / raw)
  To: Guenter Roeck, Breana, Tiberiu A, LM Sensors
  Cc: Jonathan Cameron, linux-iio, Baluta, Daniel



On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-us.net> wrote:
>On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
>> Hello Guenter, hwmon,
>>
>> We have a dilemma as to wether this device driver belongs in IIO or
>hwmon:
>> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
>>
>> Could you please give us your opinion on this?
>
>The chip has limit registers and an alarm signal output, suggesting
>that
>its intended use is for hardware monitoring purposes.
>
>The conversion time is between 50ms and 200ms, which suggests that the
>faster ABI provided by iio isn't really beneficial.
>
>Given all that, I think that hwmon would be a better place.
Agreed.
>
>If you want to keep the driver in iio, I would suggest to consider
>enhancing
>the iio-hwmon bridge with threshold/limit support if that is possible.

Possible but non trivial as no one has yet implemented the core support for allowing
in kernel consumers to get events. It has been on the to-do list as a blocker of iio
 on IIO (which is really about reaching the point where the IIO userspace front end is
an optional nonspecial case consumer of
 IIO backends). Would be excellent to have this but may not be that straight forward.

Jonathan
>This would ensure that the chip can be used for its intended purpose.
>
>Thanks,
>Guenter
>
>> Thanks,
>>
>> Tiberiu
>>
>>> -----Original Message-----
>>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
>>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
>>> Sent: Wednesday, March 9, 2016 10:32 PM
>>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>>> iio@vger.kernel.org
>>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
>>> temperature sensors
>>>
>>> On 09/03/16 13:30, Tiberiu Breana wrote:
>>>> This patch set adds support for the MAX31722/MAX31723 temperature
>>>> sensors / thermostats.
>>>> Patch 1 adds basic support and power management.
>>>> Patch 2 adds threshold interrupt support.
>>>>
>>>> Tiberiu Breana (2):
>>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
>>>>      sensors
>>>>    iio: temperature: Add threshold interrupt support for max31722
>>>>
>>>>   drivers/iio/temperature/Kconfig    |  12 +
>>>>   drivers/iio/temperature/Makefile   |   1 +
>>>>   drivers/iio/temperature/max31722.c | 673
>>>> +++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 686 insertions(+)
>>>>   create mode 100644 drivers/iio/temperature/max31722.c
>>>>
>>> Hi Tiberiu,
>>>
>>> When proposing a temperature sensor driver, we need some additional
>>> justification for why it is suited for IIO rather than the more
>focused (and in
>>> someways simpler) hwmon.
>>>
>>> So far we have had thermophiles that don't fit in as devices
>typically used for
>>> hardware monitoring. Also in there are a few drivers which are for
>parts
>>> where they are a cut down version of a more complex sensor (the
>>> measurement specialties parts are either pressure sensors or humidty
>>> sensors at least in terms of what they share interfaces with)
>>>
>>> Also, please cc the hwmon maintainers (and probably list) as we want
>their
>>> agreement.
>>>
>>> Jonathan
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
>in the body
>>> of a message to majordomo@vger.kernel.org More majordomo info at
>>> http://vger.kernel.org/majordomo-info.html
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 17:28         ` [lm-sensors] " Jonathan Cameron
@ 2016-03-15 18:30           ` Guenter Roeck
  -1 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2016-03-15 18:30 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Breana, Tiberiu A, LM Sensors, Jonathan Cameron, linux-iio,
	Baluta, Daniel

On Tue, Mar 15, 2016 at 05:28:58PM +0000, Jonathan Cameron wrote:
> 
> 
> On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-us.net> wrote:
> >On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> >> Hello Guenter, hwmon,
> >>
> >> We have a dilemma as to wether this device driver belongs in IIO or
> >hwmon:
> >> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
> >>
> >> Could you please give us your opinion on this?
> >
> >The chip has limit registers and an alarm signal output, suggesting
> >that
> >its intended use is for hardware monitoring purposes.
> >
> >The conversion time is between 50ms and 200ms, which suggests that the
> >faster ABI provided by iio isn't really beneficial.
> >
> >Given all that, I think that hwmon would be a better place.
> Agreed.
> >
> >If you want to keep the driver in iio, I would suggest to consider
> >enhancing
> >the iio-hwmon bridge with threshold/limit support if that is possible.
> 
> Possible but non trivial as no one has yet implemented the core support for allowing
> in kernel consumers to get events. It has been on the to-do list as a blocker of iio
>  on IIO (which is really about reaching the point where the IIO userspace front end is
> an optional nonspecial case consumer of
>  IIO backends). Would be excellent to have this but may not be that straight forward.
> 
I concluded that much as well after looking into it a bit more. Not just event
handling, but the ability to set and get thresholds is missing from the kernel API
as well. Yes, this will need some work. It would be useful, though, for other
drivers such as max1363. If it ever gets done, please let me know, and I'll see
if I can help testing it.

Guenter

> Jonathan
> >This would ensure that the chip can be used for its intended purpose.
> >
> >Thanks,
> >Guenter
> >
> >> Thanks,
> >>
> >> Tiberiu
> >>
> >>> -----Original Message-----
> >>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> >>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> >>> Sent: Wednesday, March 9, 2016 10:32 PM
> >>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> >>> iio@vger.kernel.org
> >>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> >>> temperature sensors
> >>>
> >>> On 09/03/16 13:30, Tiberiu Breana wrote:
> >>>> This patch set adds support for the MAX31722/MAX31723 temperature
> >>>> sensors / thermostats.
> >>>> Patch 1 adds basic support and power management.
> >>>> Patch 2 adds threshold interrupt support.
> >>>>
> >>>> Tiberiu Breana (2):
> >>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
> >>>>      sensors
> >>>>    iio: temperature: Add threshold interrupt support for max31722
> >>>>
> >>>>   drivers/iio/temperature/Kconfig    |  12 +
> >>>>   drivers/iio/temperature/Makefile   |   1 +
> >>>>   drivers/iio/temperature/max31722.c | 673
> >>>> +++++++++++++++++++++++++++++++++++++
> >>>>   3 files changed, 686 insertions(+)
> >>>>   create mode 100644 drivers/iio/temperature/max31722.c
> >>>>
> >>> Hi Tiberiu,
> >>>
> >>> When proposing a temperature sensor driver, we need some additional
> >>> justification for why it is suited for IIO rather than the more
> >focused (and in
> >>> someways simpler) hwmon.
> >>>
> >>> So far we have had thermophiles that don't fit in as devices
> >typically used for
> >>> hardware monitoring. Also in there are a few drivers which are for
> >parts
> >>> where they are a cut down version of a more complex sensor (the
> >>> measurement specialties parts are either pressure sensors or humidty
> >>> sensors at least in terms of what they share interfaces with)
> >>>
> >>> Also, please cc the hwmon maintainers (and probably list) as we want
> >their
> >>> agreement.
> >>>
> >>> Jonathan
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
> >in the body
> >>> of a message to majordomo@vger.kernel.org More majordomo info at
> >>> http://vger.kernel.org/majordomo-info.html
> >>
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-15 18:30           ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2016-03-15 18:30 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Breana, Tiberiu A, LM Sensors, Jonathan Cameron, linux-iio,
	Baluta, Daniel

On Tue, Mar 15, 2016 at 05:28:58PM +0000, Jonathan Cameron wrote:
> 
> 
> On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-us.net> wrote:
> >On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> >> Hello Guenter, hwmon,
> >>
> >> We have a dilemma as to wether this device driver belongs in IIO or
> >hwmon:
> >> https://datasheets.maximintegrated.com/en/ds/MAX31722-MAX31723.pdf
> >>
> >> Could you please give us your opinion on this?
> >
> >The chip has limit registers and an alarm signal output, suggesting
> >that
> >its intended use is for hardware monitoring purposes.
> >
> >The conversion time is between 50ms and 200ms, which suggests that the
> >faster ABI provided by iio isn't really beneficial.
> >
> >Given all that, I think that hwmon would be a better place.
> Agreed.
> >
> >If you want to keep the driver in iio, I would suggest to consider
> >enhancing
> >the iio-hwmon bridge with threshold/limit support if that is possible.
> 
> Possible but non trivial as no one has yet implemented the core support for allowing
> in kernel consumers to get events. It has been on the to-do list as a blocker of iio
>  on IIO (which is really about reaching the point where the IIO userspace front end is
> an optional nonspecial case consumer of
>  IIO backends). Would be excellent to have this but may not be that straight forward.
> 
I concluded that much as well after looking into it a bit more. Not just event
handling, but the ability to set and get thresholds is missing from the kernel API
as well. Yes, this will need some work. It would be useful, though, for other
drivers such as max1363. If it ever gets done, please let me know, and I'll see
if I can help testing it.

Guenter

> Jonathan
> >This would ensure that the chip can be used for its intended purpose.
> >
> >Thanks,
> >Guenter
> >
> >> Thanks,
> >>
> >> Tiberiu
> >>
> >>> -----Original Message-----
> >>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> >>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> >>> Sent: Wednesday, March 9, 2016 10:32 PM
> >>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> >>> iio@vger.kernel.org
> >>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> >>> temperature sensors
> >>>
> >>> On 09/03/16 13:30, Tiberiu Breana wrote:
> >>>> This patch set adds support for the MAX31722/MAX31723 temperature
> >>>> sensors / thermostats.
> >>>> Patch 1 adds basic support and power management.
> >>>> Patch 2 adds threshold interrupt support.
> >>>>
> >>>> Tiberiu Breana (2):
> >>>>    iio: temperature: Add support for MAX31722/MAX31723 temperature
> >>>>      sensors
> >>>>    iio: temperature: Add threshold interrupt support for max31722
> >>>>
> >>>>   drivers/iio/temperature/Kconfig    |  12 +
> >>>>   drivers/iio/temperature/Makefile   |   1 +
> >>>>   drivers/iio/temperature/max31722.c | 673
> >>>> +++++++++++++++++++++++++++++++++++++
> >>>>   3 files changed, 686 insertions(+)
> >>>>   create mode 100644 drivers/iio/temperature/max31722.c
> >>>>
> >>> Hi Tiberiu,
> >>>
> >>> When proposing a temperature sensor driver, we need some additional
> >>> justification for why it is suited for IIO rather than the more
> >focused (and in
> >>> someways simpler) hwmon.
> >>>
> >>> So far we have had thermophiles that don't fit in as devices
> >typically used for
> >>> hardware monitoring. Also in there are a few drivers which are for
> >parts
> >>> where they are a cut down version of a more complex sensor (the
> >>> measurement specialties parts are either pressure sensors or humidty
> >>> sensors at least in terms of what they share interfaces with)
> >>>
> >>> Also, please cc the hwmon maintainers (and probably list) as we want
> >their
> >>> agreement.
> >>>
> >>> Jonathan
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
> >in the body
> >>> of a message to majordomo@vger.kernel.org More majordomo info at
> >>> http://vger.kernel.org/majordomo-info.html
> >>
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 18:30           ` [lm-sensors] " Guenter Roeck
@ 2016-03-16 15:16             ` Breana, Tiberiu A
  -1 siblings, 0 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-16 15:16 UTC (permalink / raw)
  To: Guenter Roeck, Jonathan Cameron
  Cc: LM Sensors, Jonathan Cameron, linux-iio, Baluta, Daniel

> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> Sent: Tuesday, March 15, 2016 8:30 PM
> To: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
> Cc: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; LM Sensors <lm-
> sensors@lm-sensors.org>; Jonathan Cameron <jic23@kernel.org>; linux-
> iio@vger.kernel.org; Baluta, Daniel <daniel.baluta@intel.com>
> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> temperature sensors
>=20
> On Tue, Mar 15, 2016 at 05:28:58PM +0000, Jonathan Cameron wrote:
> >
> >
> > On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-
> us.net> wrote:
> > >On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> > >> Hello Guenter, hwmon,
> > >>
> > >> We have a dilemma as to wether this device driver belongs in IIO or
> > >hwmon:
> > >> https://datasheets.maximintegrated.com/en/ds/MAX31722-
> MAX31723.pdf
> > >>
> > >> Could you please give us your opinion on this?
> > >
> > >The chip has limit registers and an alarm signal output, suggesting
> > >that its intended use is for hardware monitoring purposes.
> > >
> > >The conversion time is between 50ms and 200ms, which suggests that
> > >the faster ABI provided by iio isn't really beneficial.
> > >
> > >Given all that, I think that hwmon would be a better place.
> > Agreed.
> > >
> > >If you want to keep the driver in iio, I would suggest to consider
> > >enhancing the iio-hwmon bridge with threshold/limit support if that
> > >is possible.
> >
> > Possible but non trivial as no one has yet implemented the core
> > support for allowing in kernel consumers to get events. It has been on
> > the to-do list as a blocker of iio  on IIO (which is really about
> > reaching the point where the IIO userspace front end is an optional
> > nonspecial case consumer of  IIO backends). Would be excellent to have
> this but may not be that straight forward.
> >
> I concluded that much as well after looking into it a bit more. Not just =
event
> handling, but the ability to set and get thresholds is missing from the k=
ernel
> API as well. Yes, this will need some work. It would be useful, though, f=
or
> other drivers such as max1363. If it ever gets done, please let me know, =
and
> I'll see if I can help testing it.
>=20
> Guenter

Thanks for your insight on this. I'll rewrite the driver for hwmon.

Tiberiu

>=20
> > Jonathan
> > >This would ensure that the chip can be used for its intended purpose.
> > >
> > >Thanks,
> > >Guenter
> > >
> > >> Thanks,
> > >>
> > >> Tiberiu
> > >>
> > >>> -----Original Message-----
> > >>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> > >>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> > >>> Sent: Wednesday, March 9, 2016 10:32 PM
> > >>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> > >>> iio@vger.kernel.org
> > >>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> > >>> temperature sensors
> > >>>
> > >>> On 09/03/16 13:30, Tiberiu Breana wrote:
> > >>>> This patch set adds support for the MAX31722/MAX31723
> temperature
> > >>>> sensors / thermostats.
> > >>>> Patch 1 adds basic support and power management.
> > >>>> Patch 2 adds threshold interrupt support.
> > >>>>
> > >>>> Tiberiu Breana (2):
> > >>>>    iio: temperature: Add support for MAX31722/MAX31723
> temperature
> > >>>>      sensors
> > >>>>    iio: temperature: Add threshold interrupt support for max31722
> > >>>>
> > >>>>   drivers/iio/temperature/Kconfig    |  12 +
> > >>>>   drivers/iio/temperature/Makefile   |   1 +
> > >>>>   drivers/iio/temperature/max31722.c | 673
> > >>>> +++++++++++++++++++++++++++++++++++++
> > >>>>   3 files changed, 686 insertions(+)
> > >>>>   create mode 100644 drivers/iio/temperature/max31722.c
> > >>>>
> > >>> Hi Tiberiu,
> > >>>
> > >>> When proposing a temperature sensor driver, we need some
> > >>> additional justification for why it is suited for IIO rather than
> > >>> the more
> > >focused (and in
> > >>> someways simpler) hwmon.
> > >>>
> > >>> So far we have had thermophiles that don't fit in as devices
> > >typically used for
> > >>> hardware monitoring. Also in there are a few drivers which are for
> > >parts
> > >>> where they are a cut down version of a more complex sensor (the
> > >>> measurement specialties parts are either pressure sensors or
> > >>> humidty sensors at least in terms of what they share interfaces
> > >>> with)
> > >>>
> > >>> Also, please cc the hwmon maintainers (and probably list) as we
> > >>> want
> > >their
> > >>> agreement.
> > >>>
> > >>> Jonathan
> > >>> --
> > >>> To unsubscribe from this list: send the line "unsubscribe linux-iio=
"
> > >in the body
> > >>> of a message to majordomo@vger.kernel.org More majordomo info at
> > >>> http://vger.kernel.org/majordomo-info.html
> > >>
> > >
> > >--
> > >To unsubscribe from this list: send the line "unsubscribe linux-iio"
> > >in the body of a message to majordomo@vger.kernel.org More
> majordomo
> > >info at  http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Sent from my Android device with K-9 Mail. Please excuse my brevity.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in t=
he body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
@ 2016-03-16 15:16             ` Breana, Tiberiu A
  0 siblings, 0 replies; 22+ messages in thread
From: Breana, Tiberiu A @ 2016-03-16 15:16 UTC (permalink / raw)
  To: Guenter Roeck, Jonathan Cameron
  Cc: LM Sensors, Jonathan Cameron, linux-iio, Baluta, Daniel

> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> Sent: Tuesday, March 15, 2016 8:30 PM
> To: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
> Cc: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; LM Sensors <lm-
> sensors@lm-sensors.org>; Jonathan Cameron <jic23@kernel.org>; linux-
> iio@vger.kernel.org; Baluta, Daniel <daniel.baluta@intel.com>
> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> temperature sensors
> 
> On Tue, Mar 15, 2016 at 05:28:58PM +0000, Jonathan Cameron wrote:
> >
> >
> > On 15 March 2016 13:34:25 GMT+00:00, Guenter Roeck <linux@roeck-
> us.net> wrote:
> > >On 03/15/2016 05:58 AM, Breana, Tiberiu A wrote:
> > >> Hello Guenter, hwmon,
> > >>
> > >> We have a dilemma as to wether this device driver belongs in IIO or
> > >hwmon:
> > >> https://datasheets.maximintegrated.com/en/ds/MAX31722-
> MAX31723.pdf
> > >>
> > >> Could you please give us your opinion on this?
> > >
> > >The chip has limit registers and an alarm signal output, suggesting
> > >that its intended use is for hardware monitoring purposes.
> > >
> > >The conversion time is between 50ms and 200ms, which suggests that
> > >the faster ABI provided by iio isn't really beneficial.
> > >
> > >Given all that, I think that hwmon would be a better place.
> > Agreed.
> > >
> > >If you want to keep the driver in iio, I would suggest to consider
> > >enhancing the iio-hwmon bridge with threshold/limit support if that
> > >is possible.
> >
> > Possible but non trivial as no one has yet implemented the core
> > support for allowing in kernel consumers to get events. It has been on
> > the to-do list as a blocker of iio  on IIO (which is really about
> > reaching the point where the IIO userspace front end is an optional
> > nonspecial case consumer of  IIO backends). Would be excellent to have
> this but may not be that straight forward.
> >
> I concluded that much as well after looking into it a bit more. Not just event
> handling, but the ability to set and get thresholds is missing from the kernel
> API as well. Yes, this will need some work. It would be useful, though, for
> other drivers such as max1363. If it ever gets done, please let me know, and
> I'll see if I can help testing it.
> 
> Guenter

Thanks for your insight on this. I'll rewrite the driver for hwmon.

Tiberiu

> 
> > Jonathan
> > >This would ensure that the chip can be used for its intended purpose.
> > >
> > >Thanks,
> > >Guenter
> > >
> > >> Thanks,
> > >>
> > >> Tiberiu
> > >>
> > >>> -----Original Message-----
> > >>> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-
> > >>> owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> > >>> Sent: Wednesday, March 9, 2016 10:32 PM
> > >>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> > >>> iio@vger.kernel.org
> > >>> Subject: Re: [PATCH 0/2] Add support for MAX31722/MAX31723
> > >>> temperature sensors
> > >>>
> > >>> On 09/03/16 13:30, Tiberiu Breana wrote:
> > >>>> This patch set adds support for the MAX31722/MAX31723
> temperature
> > >>>> sensors / thermostats.
> > >>>> Patch 1 adds basic support and power management.
> > >>>> Patch 2 adds threshold interrupt support.
> > >>>>
> > >>>> Tiberiu Breana (2):
> > >>>>    iio: temperature: Add support for MAX31722/MAX31723
> temperature
> > >>>>      sensors
> > >>>>    iio: temperature: Add threshold interrupt support for max31722
> > >>>>
> > >>>>   drivers/iio/temperature/Kconfig    |  12 +
> > >>>>   drivers/iio/temperature/Makefile   |   1 +
> > >>>>   drivers/iio/temperature/max31722.c | 673
> > >>>> +++++++++++++++++++++++++++++++++++++
> > >>>>   3 files changed, 686 insertions(+)
> > >>>>   create mode 100644 drivers/iio/temperature/max31722.c
> > >>>>
> > >>> Hi Tiberiu,
> > >>>
> > >>> When proposing a temperature sensor driver, we need some
> > >>> additional justification for why it is suited for IIO rather than
> > >>> the more
> > >focused (and in
> > >>> someways simpler) hwmon.
> > >>>
> > >>> So far we have had thermophiles that don't fit in as devices
> > >typically used for
> > >>> hardware monitoring. Also in there are a few drivers which are for
> > >parts
> > >>> where they are a cut down version of a more complex sensor (the
> > >>> measurement specialties parts are either pressure sensors or
> > >>> humidty sensors at least in terms of what they share interfaces
> > >>> with)
> > >>>
> > >>> Also, please cc the hwmon maintainers (and probably list) as we
> > >>> want
> > >their
> > >>> agreement.
> > >>>
> > >>> Jonathan
> > >>> --
> > >>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
> > >in the body
> > >>> of a message to majordomo@vger.kernel.org More majordomo info at
> > >>> http://vger.kernel.org/majordomo-info.html
> > >>
> > >
> > >--
> > >To unsubscribe from this list: send the line "unsubscribe linux-iio"
> > >in the body of a message to majordomo@vger.kernel.org More
> majordomo
> > >info at  http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Sent from my Android device with K-9 Mail. Please excuse my brevity.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [lm-sensors] [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors
  2016-03-15 13:16       ` [lm-sensors] " Breana, Tiberiu A
  (?)
@ 2016-03-22 11:41       ` Tiberiu Breana
  -1 siblings, 0 replies; 22+ messages in thread
From: Tiberiu Breana @ 2016-03-22 11:41 UTC (permalink / raw)
  To: lm-sensors

This patch set adds support for the MAX31722/MAX31723 temperature sensors /
thermostats.
Patch 1 adds basic support and power management.
Patch 2 adds alarm support.

A discussion of whether this belongs in hwmon vs iio can be found here:
http://www.spinics.net/lists/linux-iio/msg23463.html

Tiberiu Breana (2):
  hwmon: (max31722) Add support for MAX31722/MAX31723 temperature
    sensors
  hwmon: (max31722) Add alarm support

 Documentation/hwmon/max31722 |  41 +++++
 drivers/hwmon/Kconfig        |  11 ++
 drivers/hwmon/Makefile       |   1 +
 drivers/hwmon/max31722.c     | 422 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 475 insertions(+)
 create mode 100644 Documentation/hwmon/max31722
 create mode 100644 drivers/hwmon/max31722.c

-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-03-22 11:41 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 13:30 [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Tiberiu Breana
2016-03-09 13:30 ` [PATCH 1/2] iio: temperature: " Tiberiu Breana
2016-03-12  9:57   ` Jonathan Cameron
2016-03-14 13:45     ` Breana, Tiberiu A
2016-03-14 17:31       ` Jonathan Cameron
2016-03-09 13:30 ` [PATCH 2/2] iio: temperature: Add threshold interrupt support for max31722 Tiberiu Breana
2016-03-12 10:17   ` Jonathan Cameron
2016-03-14 12:25     ` Daniel Baluta
2016-03-14 15:06       ` Breana, Tiberiu A
2016-03-09 20:31 ` [PATCH 0/2] Add support for MAX31722/MAX31723 temperature sensors Jonathan Cameron
2016-03-15 12:58   ` Breana, Tiberiu A
2016-03-15 13:16     ` Breana, Tiberiu A
2016-03-15 13:16       ` [lm-sensors] " Breana, Tiberiu A
2016-03-22 11:41       ` Tiberiu Breana
2016-03-15 13:34     ` Guenter Roeck
2016-03-15 13:34       ` [lm-sensors] " Guenter Roeck
2016-03-15 17:28       ` Jonathan Cameron
2016-03-15 17:28         ` [lm-sensors] " Jonathan Cameron
2016-03-15 18:30         ` Guenter Roeck
2016-03-15 18:30           ` [lm-sensors] " Guenter Roeck
2016-03-16 15:16           ` Breana, Tiberiu A
2016-03-16 15:16             ` [lm-sensors] " Breana, Tiberiu A

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.