All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] iio: mlx90614 enhancements
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  0 siblings, 0 replies; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

This series follows up on the first series sent on February 25.  I have dropped
the filter configuration for now, as I am still trying to figure out how to do
that properly.

This set of patches adds the following additional features to the
mlx90614 temperature sensor driver.

* Support for the dual IR sensor model (patch 3)
* Emissivity setting (patches 4 and 5)
* Power management (patch 6)
* Handle errors in temperature reading (patch 7)

The changes have been split into 7 patches to ease code review.
Patches 1 and 2 contain mandatory symbol definitions, which have already
been applied to the togreg branch of iio.git.
Patches 3, 5-7 implement the additional features and are mostly independent
from each other.
Patch 4 introduces IIO_CHAN_INFO_EMISSIVITY in iio core and is a prerequisite
of patch 5.

Patch 6 introduces device tree bindings and is the only one sent to the
devicetree list.

Patch set history:
v2: * Drop processed temperature output (v1 patch 3)
    * Drop raw IR values for now (v1 patch 7)
    * Drop iir, fir, and gain attributes for now (v1 patch 5)
    * Introduce standard attribute for emissivity (patches 4 and 5)
    * Add patch 7, fixing wrong readings
    * Return -EINTR when wake-up is interrupted during read (patch 6)
    * Make adding vendor prefix explicit in commit message of patch 6

Vianney le Clément de Saint-Marcq (7):
  iio: mlx90614: Refactor register symbols
  iio: mlx90614: Add symbols for accessible registers
  iio: mlx90614: Support devices with dual IR sensor
  iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY
  iio: mlx90614: Add emissivity setting
  iio: mlx90614: Add power management
  iio: mlx90614: Check for errors in read values

 Documentation/ABI/testing/sysfs-bus-iio            |  11 +
 .../bindings/iio/temperature/mlx90614.txt          |  24 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/industrialio-core.c                    |   1 +
 drivers/iio/temperature/mlx90614.c                 | 450 ++++++++++++++++++++-
 include/linux/iio/iio.h                            |   1 +
 6 files changed, 472 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt

-- 
2.3.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v2 0/7] iio: mlx90614 enhancements
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  0 siblings, 0 replies; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

This series follows up on the first series sent on February 25.  I have dropped
the filter configuration for now, as I am still trying to figure out how to do
that properly.

This set of patches adds the following additional features to the
mlx90614 temperature sensor driver.

* Support for the dual IR sensor model (patch 3)
* Emissivity setting (patches 4 and 5)
* Power management (patch 6)
* Handle errors in temperature reading (patch 7)

The changes have been split into 7 patches to ease code review.
Patches 1 and 2 contain mandatory symbol definitions, which have already
been applied to the togreg branch of iio.git.
Patches 3, 5-7 implement the additional features and are mostly independent
from each other.
Patch 4 introduces IIO_CHAN_INFO_EMISSIVITY in iio core and is a prerequisite
of patch 5.

Patch 6 introduces device tree bindings and is the only one sent to the
devicetree list.

Patch set history:
v2: * Drop processed temperature output (v1 patch 3)
    * Drop raw IR values for now (v1 patch 7)
    * Drop iir, fir, and gain attributes for now (v1 patch 5)
    * Introduce standard attribute for emissivity (patches 4 and 5)
    * Add patch 7, fixing wrong readings
    * Return -EINTR when wake-up is interrupted during read (patch 6)
    * Make adding vendor prefix explicit in commit message of patch 6

Vianney le Clément de Saint-Marcq (7):
  iio: mlx90614: Refactor register symbols
  iio: mlx90614: Add symbols for accessible registers
  iio: mlx90614: Support devices with dual IR sensor
  iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY
  iio: mlx90614: Add emissivity setting
  iio: mlx90614: Add power management
  iio: mlx90614: Check for errors in read values

 Documentation/ABI/testing/sysfs-bus-iio            |  11 +
 .../bindings/iio/temperature/mlx90614.txt          |  24 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/industrialio-core.c                    |   1 +
 drivers/iio/temperature/mlx90614.c                 | 450 ++++++++++++++++++++-
 include/linux/iio/iio.h                            |   1 +
 6 files changed, 472 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt

-- 
2.3.3


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

* [PATCH v2 1/7] iio: mlx90614: Refactor register symbols
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  2015-03-28 12:40   ` Jonathan Cameron
  -1 siblings, 1 reply; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

The defined registers only make sense when used for accessing RAM. Make
MLX90614_OP_RAM part of the symbol definition to avoid accidental access
to the wrong register.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 drivers/iio/temperature/mlx90614.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index c8b6ac8..e2c6f1a 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -23,8 +23,8 @@
 #define MLX90614_OP_RAM 0x00
 
 /* RAM offsets with 16-bit data, MSB first */
-#define MLX90614_TA 0x06 /* ambient temperature */
-#define MLX90614_TOBJ1 0x07 /* object temperature */
+#define MLX90614_TA	(MLX90614_OP_RAM | 0x06) /* ambient temperature */
+#define MLX90614_TOBJ1	(MLX90614_OP_RAM | 0x07) /* object 1 temperature */
 
 struct mlx90614_data {
 	struct i2c_client *client;
@@ -42,13 +42,13 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 		switch (channel->channel2) {
 		case IIO_MOD_TEMP_AMBIENT:
 			ret = i2c_smbus_read_word_data(data->client,
-			    MLX90614_OP_RAM | MLX90614_TA);
+			    MLX90614_TA);
 			if (ret < 0)
 				return ret;
 			break;
 		case IIO_MOD_TEMP_OBJECT:
 			ret = i2c_smbus_read_word_data(data->client,
-			    MLX90614_OP_RAM | MLX90614_TOBJ1);
+			    MLX90614_TOBJ1);
 			if (ret < 0)
 				return ret;
 			break;
-- 
2.3.3


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

* [PATCH v2 2/7] iio: mlx90614: Add symbols for accessible registers
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  (?)
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  -1 siblings, 0 replies; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

Add symbols for all accessible RAM and EEPROM registers, as well as the
sleep command and timings defined in the datasheet.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 drivers/iio/temperature/mlx90614.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index e2c6f1a..a2e3aa6 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -20,11 +20,35 @@
 
 #include <linux/iio/iio.h>
 
-#define MLX90614_OP_RAM 0x00
+#define MLX90614_OP_RAM		0x00
+#define MLX90614_OP_EEPROM	0x20
+#define MLX90614_OP_SLEEP	0xff
 
 /* RAM offsets with 16-bit data, MSB first */
+#define MLX90614_RAW1	(MLX90614_OP_RAM | 0x04) /* raw data IR channel 1 */
+#define MLX90614_RAW2	(MLX90614_OP_RAM | 0x05) /* raw data IR channel 2 */
 #define MLX90614_TA	(MLX90614_OP_RAM | 0x06) /* ambient temperature */
 #define MLX90614_TOBJ1	(MLX90614_OP_RAM | 0x07) /* object 1 temperature */
+#define MLX90614_TOBJ2	(MLX90614_OP_RAM | 0x08) /* object 2 temperature */
+
+/* EEPROM offsets with 16-bit data, MSB first */
+#define MLX90614_EMISSIVITY	(MLX90614_OP_EEPROM | 0x04) /* emissivity correction coefficient */
+#define MLX90614_CONFIG		(MLX90614_OP_EEPROM | 0x05) /* configuration register */
+
+/* Control bits in configuration register */
+#define MLX90614_CONFIG_IIR_SHIFT 0 /* IIR coefficient */
+#define MLX90614_CONFIG_IIR_MASK (0x7 << MLX90614_CONFIG_IIR_SHIFT)
+#define MLX90614_CONFIG_DUAL_SHIFT 6 /* single (0) or dual (1) IR sensor */
+#define MLX90614_CONFIG_DUAL_MASK (1 << MLX90614_CONFIG_DUAL_SHIFT)
+#define MLX90614_CONFIG_FIR_SHIFT 8 /* FIR coefficient */
+#define MLX90614_CONFIG_FIR_MASK (0x7 << MLX90614_CONFIG_FIR_SHIFT)
+#define MLX90614_CONFIG_GAIN_SHIFT 11 /* gain */
+#define MLX90614_CONFIG_GAIN_MASK (0x7 << MLX90614_CONFIG_GAIN_SHIFT)
+
+/* Timings (in ms) */
+#define MLX90614_TIMING_EEPROM 20 /* time for EEPROM write/erase to complete */
+#define MLX90614_TIMING_WAKEUP 34 /* time to hold SDA low for wake-up */
+#define MLX90614_TIMING_STARTUP 250 /* time before first data after wake-up */
 
 struct mlx90614_data {
 	struct i2c_client *client;
-- 
2.3.3


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

* [PATCH v2 3/7] iio: mlx90614: Support devices with dual IR sensor
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
                   ` (2 preceding siblings ...)
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  2015-03-28 12:44   ` Jonathan Cameron
  -1 siblings, 1 reply; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

The model is detected by reading the EEPROM configuration during
probing.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---

v2: * remove symmetrical ambient channel
    * rename _probe_model to _probe_num_ir_sensors
---
 drivers/iio/temperature/mlx90614.c | 67 ++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index a2e3aa6..a112fc9 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -2,6 +2,7 @@
  * mlx90614.c - Support for Melexis MLX90614 contactless IR temperature sensor
  *
  * Copyright (c) 2014 Peter Meerwald <pmeerw@pmeerw.net>
+ * Copyright (c) 2015 Essensium NV
  *
  * 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
@@ -59,26 +60,34 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			    int *val2, long mask)
 {
 	struct mlx90614_data *data = iio_priv(indio_dev);
+	u8 cmd;
 	s32 ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW: /* 0.02K / LSB */
 		switch (channel->channel2) {
 		case IIO_MOD_TEMP_AMBIENT:
-			ret = i2c_smbus_read_word_data(data->client,
-			    MLX90614_TA);
-			if (ret < 0)
-				return ret;
+			cmd = MLX90614_TA;
 			break;
 		case IIO_MOD_TEMP_OBJECT:
-			ret = i2c_smbus_read_word_data(data->client,
-			    MLX90614_TOBJ1);
-			if (ret < 0)
-				return ret;
+			switch (channel->channel) {
+			case 0:
+				cmd = MLX90614_TOBJ1;
+				break;
+			case 1:
+				cmd = MLX90614_TOBJ2;
+				break;
+			default:
+				return -EINVAL;
+			}
 			break;
 		default:
 			return -EINVAL;
 		}
+
+		ret = i2c_smbus_read_word_data(data->client, cmd);
+		if (ret < 0)
+			return ret;
 		*val = ret;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
@@ -110,6 +119,16 @@ static const struct iio_chan_spec mlx90614_channels[] = {
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
 		    BIT(IIO_CHAN_INFO_SCALE),
 	},
+	{
+		.type = IIO_TEMP,
+		.indexed = 1,
+		.modified = 1,
+		.channel = 1,
+		.channel2 = IIO_MOD_TEMP_OBJECT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+		    BIT(IIO_CHAN_INFO_SCALE),
+	},
 };
 
 static const struct iio_info mlx90614_info = {
@@ -117,11 +136,25 @@ static const struct iio_info mlx90614_info = {
 	.driver_module = THIS_MODULE,
 };
 
+/* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
+static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
+{
+	s32 ret;
+
+	ret = i2c_smbus_read_word_data(client, MLX90614_CONFIG);
+
+	if (ret < 0)
+		return ret;
+
+	return (ret & MLX90614_CONFIG_DUAL_MASK) ? 1 : 0;
+}
+
 static int mlx90614_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct iio_dev *indio_dev;
 	struct mlx90614_data *data;
+	int ret;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
 		return -ENODEV;
@@ -139,8 +172,21 @@ static int mlx90614_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &mlx90614_info;
 
-	indio_dev->channels = mlx90614_channels;
-	indio_dev->num_channels = ARRAY_SIZE(mlx90614_channels);
+	ret = mlx90614_probe_num_ir_sensors(client);
+	switch (ret) {
+	case 0:
+		dev_dbg(&client->dev, "Found single sensor");
+		indio_dev->channels = mlx90614_channels;
+		indio_dev->num_channels = 2;
+		break;
+	case 1:
+		dev_dbg(&client->dev, "Found dual sensor");
+		indio_dev->channels = mlx90614_channels;
+		indio_dev->num_channels = 3;
+		break;
+	default:
+		return ret;
+	}
 
 	return iio_device_register(indio_dev);
 }
@@ -170,5 +216,6 @@ static struct i2c_driver mlx90614_driver = {
 module_i2c_driver(mlx90614_driver);
 
 MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_AUTHOR("Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>");
 MODULE_DESCRIPTION("Melexis MLX90614 contactless IR temperature sensor driver");
 MODULE_LICENSE("GPL");
-- 
2.3.3


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

* [PATCH v2 4/7] iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
                   ` (3 preceding siblings ...)
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  2015-03-24 16:02   ` Lars-Peter Clausen
  -1 siblings, 1 reply; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

Contact-less IR temperature sensors measure the temperature of an object
by using its thermal radiation.  Surfaces with different emissivity
ratios emit different amounts of energy at the same temperature.

IIO_CHAN_INFO_EMISSIVITY allows the user to inform the sensor of the
emissivity of the object in front of it, in order to effectively measure
its temperature.

A device providing such setting is Melexis's MLX90614:
http://melexis.com/Assets/IR-sensor-thermometer-MLX90614-Datasheet-5152.aspx.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---

v2: new patch
---
 Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
 drivers/iio/industrialio-core.c         |  1 +
 include/linux/iio/iio.h                 |  1 +
 3 files changed, 13 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 9a70c31..a8b103a 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1249,3 +1249,14 @@ Contact:	linux-iio@vger.kernel.org
 Description:
 		Specifies number of seconds in which we compute the steps
 		that occur in order to decide if the consumer is making steps.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_temp_emissivity
+What:		/sys/bus/iio/devices/iio:deviceX/in_tempX_emissivity
+What:		/sys/bus/iio/devices/iio:deviceX/in_temp_object_emissivity
+What:		/sys/bus/iio/devices/iio:deviceX/in_tempX_object_emissivity
+KernelVersion:	4.0
+Contact:	linux-iio@vger.kernel.org
+Description:
+		The emissivity ratio of the surface in the field of view of the
+		contactless temperature sensor.  Emissivity varies from 0 to 1,
+		with 1 being the emissivity of a black body.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index aaba9d3..902d03b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -128,6 +128,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_CALIBWEIGHT] = "calibweight",
 	[IIO_CHAN_INFO_DEBOUNCE_COUNT] = "debounce_count",
 	[IIO_CHAN_INFO_DEBOUNCE_TIME] = "debounce_time",
+	[IIO_CHAN_INFO_EMISSIVITY] = "emissivity",
 };
 
 /**
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 80d8550..24bd0bdd 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -43,6 +43,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_CALIBWEIGHT,
 	IIO_CHAN_INFO_DEBOUNCE_COUNT,
 	IIO_CHAN_INFO_DEBOUNCE_TIME,
+	IIO_CHAN_INFO_EMISSIVITY,
 };
 
 enum iio_shared_by {
-- 
2.3.3


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

* [PATCH v2 5/7] iio: mlx90614: Add emissivity setting
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
                   ` (4 preceding siblings ...)
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  2015-03-28 12:46   ` Jonathan Cameron
  -1 siblings, 1 reply; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

The mapping from the 16-bit EEPROM value to the decimal 0-1 range is
approximate.  A special case ensures 0xFFFF shows as 1.0 instead of
0.999998565.

Writing to EEPROM requires an explicit erase by writing zero.  In
addition, it takes 20ms for the erase/write to complete.  During this
time no EEPROM register should be accessed.  Therefore, two msleep()s
are added to the write function and a mutex protects against concurrent
access.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---

v2: * remove iir, fir, and gain attributes
    * refactor emissivity to use IIO_CHAN_INFO_EMISSIVITY
---
 drivers/iio/temperature/mlx90614.c | 106 +++++++++++++++++++++++++++++++++++--
 1 file changed, 103 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index a112fc9..b981d1e 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -12,12 +12,13 @@
  *
  * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
  *
- * TODO: sleep mode, configuration EEPROM
+ * TODO: sleep mode, filter configuration
  */
 
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/delay.h>
 
 #include <linux/iio/iio.h>
 
@@ -53,8 +54,47 @@
 
 struct mlx90614_data {
 	struct i2c_client *client;
+	struct mutex lock; /* for EEPROM access only */
 };
 
+/*
+ * Erase an address and write word.
+ * The mutex must be locked before calling.
+ */
+static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
+			       u16 value)
+{
+	/*
+	 * Note: The mlx90614 requires a PEC on writing but does not send us a
+	 * valid PEC on reading.  Hence, we cannot set I2C_CLIENT_PEC in
+	 * i2c_client.flags.  As a workaround, we use i2c_smbus_xfer here.
+	 */
+	union i2c_smbus_data data;
+	s32 ret;
+
+	dev_dbg(&client->dev, "Writing 0x%x to address 0x%x", value, command);
+
+	data.word = 0x0000; /* erase command */
+	ret = i2c_smbus_xfer(client->adapter, client->addr,
+			     client->flags | I2C_CLIENT_PEC,
+			     I2C_SMBUS_WRITE, command,
+			     I2C_SMBUS_WORD_DATA, &data);
+	if (ret < 0)
+		return ret;
+
+	msleep(MLX90614_TIMING_EEPROM);
+
+	data.word = value; /* actual write */
+	ret = i2c_smbus_xfer(client->adapter, client->addr,
+			     client->flags | I2C_CLIENT_PEC,
+			     I2C_SMBUS_WRITE, command,
+			     I2C_SMBUS_WORD_DATA, &data);
+
+	msleep(MLX90614_TIMING_EEPROM);
+
+	return ret;
+}
+
 static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *channel, int *val,
 			    int *val2, long mask)
@@ -97,6 +137,61 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		*val = 20;
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
+		mutex_lock(&data->lock);
+		ret = i2c_smbus_read_word_data(data->client,
+					       MLX90614_EMISSIVITY);
+		mutex_unlock(&data->lock);
+
+		if (ret < 0)
+			return ret;
+
+		if (ret == 65535) {
+			*val = 1;
+			*val2 = 0;
+		} else {
+			*val = 0;
+			*val2 = ret * 15259; /* 1/65535 ~ 0.000015259 */
+		}
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90614_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *channel, int val,
+			     int val2, long mask)
+{
+	struct mlx90614_data *data = iio_priv(indio_dev);
+	s32 ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
+		if (val < 0 || val2 < 0 || val > 1 || (val == 1 && val2 != 0))
+			return -EINVAL;
+		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
+
+		mutex_lock(&data->lock);
+		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
+					  val);
+		mutex_unlock(&data->lock);
+
+		if (ret < 0)
+			return ret;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90614_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec const *channel,
+				     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_EMISSIVITY:
+		return IIO_VAL_INT_PLUS_NANO;
 	default:
 		return -EINVAL;
 	}
@@ -115,7 +210,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
 		.type = IIO_TEMP,
 		.modified = 1,
 		.channel2 = IIO_MOD_TEMP_OBJECT,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		    BIT(IIO_CHAN_INFO_EMISSIVITY),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
 		    BIT(IIO_CHAN_INFO_SCALE),
 	},
@@ -125,7 +221,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
 		.modified = 1,
 		.channel = 1,
 		.channel2 = IIO_MOD_TEMP_OBJECT,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		    BIT(IIO_CHAN_INFO_EMISSIVITY),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
 		    BIT(IIO_CHAN_INFO_SCALE),
 	},
@@ -133,6 +230,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
 
 static const struct iio_info mlx90614_info = {
 	.read_raw = mlx90614_read_raw,
+	.write_raw = mlx90614_write_raw,
+	.write_raw_get_fmt = mlx90614_write_raw_get_fmt,
 	.driver_module = THIS_MODULE,
 };
 
@@ -166,6 +265,7 @@ static int mlx90614_probe(struct i2c_client *client,
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
+	mutex_init(&data->lock);
 
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->name = id->name;
-- 
2.3.3


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

* [PATCH v2 6/7] iio: mlx90614: Add power management
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
@ 2015-03-24 15:54     ` Vianney le Clément de Saint-Marcq
  -1 siblings, 0 replies; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

Add support for system sleep and runtime power management.

To wake up the device, the SDA line should be held low for at least 33ms
while SCL is high.  As this is not possible using the i2c API (and not
supported by all i2c adapters), a GPIO connected to the SDA line is
needed.  The GPIO is named "wakeup" and can be specified in a device
tree with the "wakeup-gpios" binding.

If the wake-up GPIO is not given, disable power management for the
device.  Entering sleep requires an SMBus byte access, hence power
management is also disabled if byte access is not supported by the
adapter.

Introduce "melexis" as a vendor prefix for device tree bindings.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout-agwphqFHkxc@public.gmane.org>

---

The default autosleep delay (5s) is chosen arbitrarily, trying to take
into account the long startup time (250ms).  Feel free to change it to
another value if you think it is saner.

v2: return -EINTR when the startup delay is interrupted
---
 .../bindings/iio/temperature/mlx90614.txt          |  24 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/temperature/mlx90614.c                 | 246 ++++++++++++++++++++-
 3 files changed, 269 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt

diff --git a/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
new file mode 100644
index 0000000..9be57b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
@@ -0,0 +1,24 @@
+* Melexis MLX90614 contactless IR temperature sensor
+
+http://melexis.com/Infrared-Thermometer-Sensors/Infrared-Thermometer-Sensors/MLX90614-615.aspx
+
+Required properties:
+
+  - compatible: should be "melexis,mlx90614"
+  - reg: the I2C address of the sensor
+
+Optional properties:
+
+  - wakeup-gpios: device tree identifier of the GPIO connected to the SDA line
+      to hold low in order to wake up the device.  In normal operation, the
+      GPIO is set as input and will not interfere in I2C communication.  There
+      is no need for a GPIO driving the SCL line.  If no GPIO is given, power
+      management is disabled.
+
+Example:
+
+mlx90614@5a {
+	compatible = "melexis,mlx90614";
+	reg = <0x5a>;
+	wakeup-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index fae26d0..c8db30c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -110,6 +110,7 @@ lltc	Linear Technology Corporation
 marvell	Marvell Technology Group Ltd.
 maxim	Maxim Integrated Products
 mediatek	MediaTek Inc.
+melexis	Melexis N.V.
 merrii	Merrii Technology Co., Ltd.
 micrel	Micrel Inc.
 microchip	Microchip Technology Inc.
diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index b981d1e..6b9bfcd 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -12,13 +12,24 @@
  *
  * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
  *
- * TODO: sleep mode, filter configuration
+ * To wake up from sleep mode, the SDA line must be held low while SCL is high
+ * for at least 33ms.  This is achieved with an extra GPIO that can be connected
+ * directly to the SDA line.  In normal operation, the GPIO is set as input and
+ * will not interfere in I2C communication.  While the GPIO is driven low, the
+ * i2c adapter is locked since it cannot be used by other clients.  The SCL line
+ * always has a pull-up so we do not need an extra GPIO to drive it high.  If
+ * the "wakeup" GPIO is not given, power management will be disabled.
+ *
+ * TODO: filter configuration
  */
 
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/iio/iio.h>
 
@@ -52,9 +63,13 @@
 #define MLX90614_TIMING_WAKEUP 34 /* time to hold SDA low for wake-up */
 #define MLX90614_TIMING_STARTUP 250 /* time before first data after wake-up */
 
+#define MLX90614_AUTOSLEEP_DELAY 5000 /* default autosleep delay */
+
 struct mlx90614_data {
 	struct i2c_client *client;
 	struct mutex lock; /* for EEPROM access only */
+	struct gpio_desc *wakeup_gpio; /* NULL to disable sleep/wake-up */
+	unsigned long ready_timestamp; /* in jiffies */
 };
 
 /*
@@ -95,6 +110,54 @@ static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
 	return ret;
 }
 
+#ifdef CONFIG_PM
+/*
+ * If @startup is true, make sure MLX90614_TIMING_STARTUP ms have elapsed since
+ * the last wake-up.  This is normally only needed to get a valid temperature
+ * reading.  EEPROM access does not need such delay.
+ * Return 0 on success, <0 on error.
+ */
+static int mlx90614_power_get(struct mlx90614_data *data, bool startup)
+{
+	unsigned long now;
+
+	if (!data->wakeup_gpio)
+		return 0;
+
+	pm_runtime_get_sync(&data->client->dev);
+
+	if (startup) {
+		now = jiffies;
+		if (time_before(now, data->ready_timestamp) &&
+		    msleep_interruptible(jiffies_to_msecs(
+				data->ready_timestamp - now)) != 0) {
+			pm_runtime_put_autosuspend(&data->client->dev);
+			return -EINTR;
+		}
+	}
+
+	return 0;
+}
+
+static void mlx90614_power_put(struct mlx90614_data *data)
+{
+	if (!data->wakeup_gpio)
+		return;
+
+	pm_runtime_mark_last_busy(&data->client->dev);
+	pm_runtime_put_autosuspend(&data->client->dev);
+}
+#else
+static inline int mlx90614_power_get(struct mlx90614_data *data, bool startup)
+{
+	return 0;
+}
+
+static inline void mlx90614_power_put(struct mlx90614_data *data)
+{
+}
+#endif
+
 static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *channel, int *val,
 			    int *val2, long mask)
@@ -125,7 +188,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 
+		ret = mlx90614_power_get(data, true);
+		if (ret < 0)
+			return ret;
 		ret = i2c_smbus_read_word_data(data->client, cmd);
+		mlx90614_power_put(data);
+
 		if (ret < 0)
 			return ret;
 		*val = ret;
@@ -138,10 +206,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 		*val = 20;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
+		mlx90614_power_get(data, false);
 		mutex_lock(&data->lock);
 		ret = i2c_smbus_read_word_data(data->client,
 					       MLX90614_EMISSIVITY);
 		mutex_unlock(&data->lock);
+		mlx90614_power_put(data);
 
 		if (ret < 0)
 			return ret;
@@ -172,10 +242,12 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
 
+		mlx90614_power_get(data, false);
 		mutex_lock(&data->lock);
 		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
 					  val);
 		mutex_unlock(&data->lock);
+		mlx90614_power_put(data);
 
 		if (ret < 0)
 			return ret;
@@ -235,6 +307,98 @@ static const struct iio_info mlx90614_info = {
 	.driver_module = THIS_MODULE,
 };
 
+#ifdef CONFIG_PM
+static int mlx90614_sleep(struct mlx90614_data *data)
+{
+	s32 ret;
+
+	if (!data->wakeup_gpio) {
+		dev_dbg(&data->client->dev, "Sleep disabled");
+		return -ENOSYS;
+	}
+
+	dev_dbg(&data->client->dev, "Requesting sleep");
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_xfer(data->client->adapter, data->client->addr,
+			     data->client->flags | I2C_CLIENT_PEC,
+			     I2C_SMBUS_WRITE, MLX90614_OP_SLEEP,
+			     I2C_SMBUS_BYTE, NULL);
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int mlx90614_wakeup(struct mlx90614_data *data)
+{
+	if (!data->wakeup_gpio) {
+		dev_dbg(&data->client->dev, "Wake-up disabled");
+		return -ENOSYS;
+	}
+
+	dev_dbg(&data->client->dev, "Requesting wake-up");
+
+	i2c_lock_adapter(data->client->adapter);
+	gpiod_direction_output(data->wakeup_gpio, 0);
+	msleep(MLX90614_TIMING_WAKEUP);
+	gpiod_direction_input(data->wakeup_gpio);
+	i2c_unlock_adapter(data->client->adapter);
+
+	data->ready_timestamp = jiffies +
+			msecs_to_jiffies(MLX90614_TIMING_STARTUP);
+
+	/*
+	 * Quirk: the i2c controller may get confused right after the
+	 * wake-up signal has been sent.  As a workaround, do a dummy read.
+	 * If the read fails, the controller will probably be reset so that
+	 * further reads will work.
+	 */
+	i2c_smbus_read_word_data(data->client, MLX90614_CONFIG);
+
+	return 0;
+}
+
+/* Return wake-up GPIO or NULL if sleep functionality should be disabled. */
+static struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
+{
+	struct gpio_desc *gpio;
+
+	if (!i2c_check_functionality(client->adapter,
+						I2C_FUNC_SMBUS_WRITE_BYTE)) {
+		dev_info(&client->dev,
+			 "i2c adapter does not support SMBUS_WRITE_BYTE, sleep disabled");
+		return NULL;
+	}
+
+	gpio = devm_gpiod_get_optional(&client->dev, "wakeup", GPIOD_IN);
+
+	if (IS_ERR(gpio)) {
+		dev_warn(&client->dev,
+			 "gpio acquisition failed with error %ld, sleep disabled",
+			 PTR_ERR(gpio));
+		return NULL;
+	} else if (!gpio) {
+		dev_info(&client->dev,
+			 "wakeup-gpio not found, sleep disabled");
+	}
+
+	return gpio;
+}
+#else
+static inline int mlx90614_sleep(struct mlx90614_data *data)
+{
+	return -ENOSYS;
+}
+static inline int mlx90614_wakeup(struct mlx90614_data *data)
+{
+	return -ENOSYS;
+}
+static inline struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
+{
+	return NULL;
+}
+#endif
+
 /* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
 static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
 {
@@ -266,6 +430,9 @@ static int mlx90614_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 	mutex_init(&data->lock);
+	data->wakeup_gpio = mlx90614_probe_wakeup(client);
+
+	mlx90614_wakeup(data);
 
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->name = id->name;
@@ -288,12 +455,30 @@ static int mlx90614_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	if (data->wakeup_gpio) {
+		pm_runtime_set_autosuspend_delay(&client->dev,
+						 MLX90614_AUTOSLEEP_DELAY);
+		pm_runtime_use_autosuspend(&client->dev);
+		pm_runtime_set_active(&client->dev);
+		pm_runtime_enable(&client->dev);
+	}
+
 	return iio_device_register(indio_dev);
 }
 
 static int mlx90614_remove(struct i2c_client *client)
 {
-	iio_device_unregister(i2c_get_clientdata(client));
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	if (data->wakeup_gpio) {
+		pm_runtime_disable(&client->dev);
+		if (!pm_runtime_status_suspended(&client->dev))
+			mlx90614_sleep(data);
+		pm_runtime_set_suspended(&client->dev);
+	}
+
+	iio_device_unregister(indio_dev);
 
 	return 0;
 }
@@ -304,10 +489,67 @@ static const struct i2c_device_id mlx90614_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mlx90614_id);
 
+#ifdef CONFIG_PM_SLEEP
+static int mlx90614_pm_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	if (data->wakeup_gpio && pm_runtime_active(dev))
+		return mlx90614_sleep(data);
+
+	return 0;
+}
+
+static int mlx90614_pm_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+	int err;
+
+	if (data->wakeup_gpio) {
+		err = mlx90614_wakeup(data);
+		if (err < 0)
+			return err;
+
+		pm_runtime_disable(dev);
+		pm_runtime_set_active(dev);
+		pm_runtime_enable(dev);
+	}
+
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM
+static int mlx90614_pm_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	return mlx90614_sleep(data);
+}
+
+static int mlx90614_pm_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	return mlx90614_wakeup(data);
+}
+#endif
+
+static const struct dev_pm_ops mlx90614_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mlx90614_pm_suspend, mlx90614_pm_resume)
+	SET_RUNTIME_PM_OPS(mlx90614_pm_runtime_suspend,
+			   mlx90614_pm_runtime_resume, NULL)
+};
+
 static struct i2c_driver mlx90614_driver = {
 	.driver = {
 		.name	= "mlx90614",
 		.owner	= THIS_MODULE,
+		.pm	= &mlx90614_pm_ops,
 	},
 	.probe = mlx90614_probe,
 	.remove = mlx90614_remove,
-- 
2.3.3

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

* [PATCH v2 6/7] iio: mlx90614: Add power management
@ 2015-03-24 15:54     ` Vianney le Clément de Saint-Marcq
  0 siblings, 0 replies; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

Add support for system sleep and runtime power management.

To wake up the device, the SDA line should be held low for at least 33ms
while SCL is high.  As this is not possible using the i2c API (and not
supported by all i2c adapters), a GPIO connected to the SDA line is
needed.  The GPIO is named "wakeup" and can be specified in a device
tree with the "wakeup-gpios" binding.

If the wake-up GPIO is not given, disable power management for the
device.  Entering sleep requires an SMBus byte access, hence power
management is also disabled if byte access is not supported by the
adapter.

Introduce "melexis" as a vendor prefix for device tree bindings.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---

The default autosleep delay (5s) is chosen arbitrarily, trying to take
into account the long startup time (250ms).  Feel free to change it to
another value if you think it is saner.

v2: return -EINTR when the startup delay is interrupted
---
 .../bindings/iio/temperature/mlx90614.txt          |  24 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/temperature/mlx90614.c                 | 246 ++++++++++++++++++++-
 3 files changed, 269 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt

diff --git a/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
new file mode 100644
index 0000000..9be57b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
@@ -0,0 +1,24 @@
+* Melexis MLX90614 contactless IR temperature sensor
+
+http://melexis.com/Infrared-Thermometer-Sensors/Infrared-Thermometer-Sensors/MLX90614-615.aspx
+
+Required properties:
+
+  - compatible: should be "melexis,mlx90614"
+  - reg: the I2C address of the sensor
+
+Optional properties:
+
+  - wakeup-gpios: device tree identifier of the GPIO connected to the SDA line
+      to hold low in order to wake up the device.  In normal operation, the
+      GPIO is set as input and will not interfere in I2C communication.  There
+      is no need for a GPIO driving the SCL line.  If no GPIO is given, power
+      management is disabled.
+
+Example:
+
+mlx90614@5a {
+	compatible = "melexis,mlx90614";
+	reg = <0x5a>;
+	wakeup-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index fae26d0..c8db30c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -110,6 +110,7 @@ lltc	Linear Technology Corporation
 marvell	Marvell Technology Group Ltd.
 maxim	Maxim Integrated Products
 mediatek	MediaTek Inc.
+melexis	Melexis N.V.
 merrii	Merrii Technology Co., Ltd.
 micrel	Micrel Inc.
 microchip	Microchip Technology Inc.
diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index b981d1e..6b9bfcd 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -12,13 +12,24 @@
  *
  * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
  *
- * TODO: sleep mode, filter configuration
+ * To wake up from sleep mode, the SDA line must be held low while SCL is high
+ * for at least 33ms.  This is achieved with an extra GPIO that can be connected
+ * directly to the SDA line.  In normal operation, the GPIO is set as input and
+ * will not interfere in I2C communication.  While the GPIO is driven low, the
+ * i2c adapter is locked since it cannot be used by other clients.  The SCL line
+ * always has a pull-up so we do not need an extra GPIO to drive it high.  If
+ * the "wakeup" GPIO is not given, power management will be disabled.
+ *
+ * TODO: filter configuration
  */
 
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/iio/iio.h>
 
@@ -52,9 +63,13 @@
 #define MLX90614_TIMING_WAKEUP 34 /* time to hold SDA low for wake-up */
 #define MLX90614_TIMING_STARTUP 250 /* time before first data after wake-up */
 
+#define MLX90614_AUTOSLEEP_DELAY 5000 /* default autosleep delay */
+
 struct mlx90614_data {
 	struct i2c_client *client;
 	struct mutex lock; /* for EEPROM access only */
+	struct gpio_desc *wakeup_gpio; /* NULL to disable sleep/wake-up */
+	unsigned long ready_timestamp; /* in jiffies */
 };
 
 /*
@@ -95,6 +110,54 @@ static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
 	return ret;
 }
 
+#ifdef CONFIG_PM
+/*
+ * If @startup is true, make sure MLX90614_TIMING_STARTUP ms have elapsed since
+ * the last wake-up.  This is normally only needed to get a valid temperature
+ * reading.  EEPROM access does not need such delay.
+ * Return 0 on success, <0 on error.
+ */
+static int mlx90614_power_get(struct mlx90614_data *data, bool startup)
+{
+	unsigned long now;
+
+	if (!data->wakeup_gpio)
+		return 0;
+
+	pm_runtime_get_sync(&data->client->dev);
+
+	if (startup) {
+		now = jiffies;
+		if (time_before(now, data->ready_timestamp) &&
+		    msleep_interruptible(jiffies_to_msecs(
+				data->ready_timestamp - now)) != 0) {
+			pm_runtime_put_autosuspend(&data->client->dev);
+			return -EINTR;
+		}
+	}
+
+	return 0;
+}
+
+static void mlx90614_power_put(struct mlx90614_data *data)
+{
+	if (!data->wakeup_gpio)
+		return;
+
+	pm_runtime_mark_last_busy(&data->client->dev);
+	pm_runtime_put_autosuspend(&data->client->dev);
+}
+#else
+static inline int mlx90614_power_get(struct mlx90614_data *data, bool startup)
+{
+	return 0;
+}
+
+static inline void mlx90614_power_put(struct mlx90614_data *data)
+{
+}
+#endif
+
 static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *channel, int *val,
 			    int *val2, long mask)
@@ -125,7 +188,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 
+		ret = mlx90614_power_get(data, true);
+		if (ret < 0)
+			return ret;
 		ret = i2c_smbus_read_word_data(data->client, cmd);
+		mlx90614_power_put(data);
+
 		if (ret < 0)
 			return ret;
 		*val = ret;
@@ -138,10 +206,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 		*val = 20;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
+		mlx90614_power_get(data, false);
 		mutex_lock(&data->lock);
 		ret = i2c_smbus_read_word_data(data->client,
 					       MLX90614_EMISSIVITY);
 		mutex_unlock(&data->lock);
+		mlx90614_power_put(data);
 
 		if (ret < 0)
 			return ret;
@@ -172,10 +242,12 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
 
+		mlx90614_power_get(data, false);
 		mutex_lock(&data->lock);
 		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
 					  val);
 		mutex_unlock(&data->lock);
+		mlx90614_power_put(data);
 
 		if (ret < 0)
 			return ret;
@@ -235,6 +307,98 @@ static const struct iio_info mlx90614_info = {
 	.driver_module = THIS_MODULE,
 };
 
+#ifdef CONFIG_PM
+static int mlx90614_sleep(struct mlx90614_data *data)
+{
+	s32 ret;
+
+	if (!data->wakeup_gpio) {
+		dev_dbg(&data->client->dev, "Sleep disabled");
+		return -ENOSYS;
+	}
+
+	dev_dbg(&data->client->dev, "Requesting sleep");
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_xfer(data->client->adapter, data->client->addr,
+			     data->client->flags | I2C_CLIENT_PEC,
+			     I2C_SMBUS_WRITE, MLX90614_OP_SLEEP,
+			     I2C_SMBUS_BYTE, NULL);
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int mlx90614_wakeup(struct mlx90614_data *data)
+{
+	if (!data->wakeup_gpio) {
+		dev_dbg(&data->client->dev, "Wake-up disabled");
+		return -ENOSYS;
+	}
+
+	dev_dbg(&data->client->dev, "Requesting wake-up");
+
+	i2c_lock_adapter(data->client->adapter);
+	gpiod_direction_output(data->wakeup_gpio, 0);
+	msleep(MLX90614_TIMING_WAKEUP);
+	gpiod_direction_input(data->wakeup_gpio);
+	i2c_unlock_adapter(data->client->adapter);
+
+	data->ready_timestamp = jiffies +
+			msecs_to_jiffies(MLX90614_TIMING_STARTUP);
+
+	/*
+	 * Quirk: the i2c controller may get confused right after the
+	 * wake-up signal has been sent.  As a workaround, do a dummy read.
+	 * If the read fails, the controller will probably be reset so that
+	 * further reads will work.
+	 */
+	i2c_smbus_read_word_data(data->client, MLX90614_CONFIG);
+
+	return 0;
+}
+
+/* Return wake-up GPIO or NULL if sleep functionality should be disabled. */
+static struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
+{
+	struct gpio_desc *gpio;
+
+	if (!i2c_check_functionality(client->adapter,
+						I2C_FUNC_SMBUS_WRITE_BYTE)) {
+		dev_info(&client->dev,
+			 "i2c adapter does not support SMBUS_WRITE_BYTE, sleep disabled");
+		return NULL;
+	}
+
+	gpio = devm_gpiod_get_optional(&client->dev, "wakeup", GPIOD_IN);
+
+	if (IS_ERR(gpio)) {
+		dev_warn(&client->dev,
+			 "gpio acquisition failed with error %ld, sleep disabled",
+			 PTR_ERR(gpio));
+		return NULL;
+	} else if (!gpio) {
+		dev_info(&client->dev,
+			 "wakeup-gpio not found, sleep disabled");
+	}
+
+	return gpio;
+}
+#else
+static inline int mlx90614_sleep(struct mlx90614_data *data)
+{
+	return -ENOSYS;
+}
+static inline int mlx90614_wakeup(struct mlx90614_data *data)
+{
+	return -ENOSYS;
+}
+static inline struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
+{
+	return NULL;
+}
+#endif
+
 /* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
 static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
 {
@@ -266,6 +430,9 @@ static int mlx90614_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 	mutex_init(&data->lock);
+	data->wakeup_gpio = mlx90614_probe_wakeup(client);
+
+	mlx90614_wakeup(data);
 
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->name = id->name;
@@ -288,12 +455,30 @@ static int mlx90614_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	if (data->wakeup_gpio) {
+		pm_runtime_set_autosuspend_delay(&client->dev,
+						 MLX90614_AUTOSLEEP_DELAY);
+		pm_runtime_use_autosuspend(&client->dev);
+		pm_runtime_set_active(&client->dev);
+		pm_runtime_enable(&client->dev);
+	}
+
 	return iio_device_register(indio_dev);
 }
 
 static int mlx90614_remove(struct i2c_client *client)
 {
-	iio_device_unregister(i2c_get_clientdata(client));
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	if (data->wakeup_gpio) {
+		pm_runtime_disable(&client->dev);
+		if (!pm_runtime_status_suspended(&client->dev))
+			mlx90614_sleep(data);
+		pm_runtime_set_suspended(&client->dev);
+	}
+
+	iio_device_unregister(indio_dev);
 
 	return 0;
 }
@@ -304,10 +489,67 @@ static const struct i2c_device_id mlx90614_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mlx90614_id);
 
+#ifdef CONFIG_PM_SLEEP
+static int mlx90614_pm_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	if (data->wakeup_gpio && pm_runtime_active(dev))
+		return mlx90614_sleep(data);
+
+	return 0;
+}
+
+static int mlx90614_pm_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+	int err;
+
+	if (data->wakeup_gpio) {
+		err = mlx90614_wakeup(data);
+		if (err < 0)
+			return err;
+
+		pm_runtime_disable(dev);
+		pm_runtime_set_active(dev);
+		pm_runtime_enable(dev);
+	}
+
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM
+static int mlx90614_pm_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	return mlx90614_sleep(data);
+}
+
+static int mlx90614_pm_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct mlx90614_data *data = iio_priv(indio_dev);
+
+	return mlx90614_wakeup(data);
+}
+#endif
+
+static const struct dev_pm_ops mlx90614_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mlx90614_pm_suspend, mlx90614_pm_resume)
+	SET_RUNTIME_PM_OPS(mlx90614_pm_runtime_suspend,
+			   mlx90614_pm_runtime_resume, NULL)
+};
+
 static struct i2c_driver mlx90614_driver = {
 	.driver = {
 		.name	= "mlx90614",
 		.owner	= THIS_MODULE,
+		.pm	= &mlx90614_pm_ops,
 	},
 	.probe = mlx90614_probe,
 	.remove = mlx90614_remove,
-- 
2.3.3


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

* [PATCH v2 7/7] iio: mlx90614: Check for errors in read values
  2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
                   ` (6 preceding siblings ...)
  (?)
@ 2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
  2015-03-28 13:43   ` Jonathan Cameron
  -1 siblings, 1 reply; 18+ messages in thread
From: Vianney le Clément de Saint-Marcq @ 2015-03-24 15:54 UTC (permalink / raw)
  To: linux-iio
  Cc: Vianney le Clément de Saint-Marcq,
	Arnout Vandecappelle (Essensium/Mind)

The device uses the MSB of the returned temperature values as an error
flag.  Return a read error when this bit is set.

Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---

v2: new patch
---
 drivers/iio/temperature/mlx90614.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
index 6b9bfcd..6cb1e81 100644
--- a/drivers/iio/temperature/mlx90614.c
+++ b/drivers/iio/temperature/mlx90614.c
@@ -196,6 +196,11 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
 
 		if (ret < 0)
 			return ret;
+
+		/* MSB is an error flag */
+		if (ret & 0x8000)
+			return -EREMOTEIO;
+
 		*val = ret;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
-- 
2.3.3


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

* Re: [PATCH v2 4/7] iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY
  2015-03-24 15:54 ` [PATCH v2 4/7] iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY Vianney le Clément de Saint-Marcq
@ 2015-03-24 16:02   ` Lars-Peter Clausen
  2015-03-28 12:45     ` Jonathan Cameron
  0 siblings, 1 reply; 18+ messages in thread
From: Lars-Peter Clausen @ 2015-03-24 16:02 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 03/24/2015 04:54 PM, Vianney le Clément de Saint-Marcq wrote:
> Contact-less IR temperature sensors measure the temperature of an object
> by using its thermal radiation.  Surfaces with different emissivity
> ratios emit different amounts of energy at the same temperature.
>
> IIO_CHAN_INFO_EMISSIVITY allows the user to inform the sensor of the
> emissivity of the object in front of it, in order to effectively measure
> its temperature.

All the other attributes that need to be setup according to the object being 
measured start with the calib prefix. So maybe call this calibemissivity.

- Lars

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

* Re: [PATCH v2 1/7] iio: mlx90614: Refactor register symbols
  2015-03-24 15:54 ` [PATCH v2 1/7] iio: mlx90614: Refactor register symbols Vianney le Clément de Saint-Marcq
@ 2015-03-28 12:40   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:40 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> The defined registers only make sense when used for accessing RAM. Make
> MLX90614_OP_RAM part of the symbol definition to avoid accidental access
> to the wrong register.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
I've already applied this one - so please drop it from future 
postings of the series.
> ---
>  drivers/iio/temperature/mlx90614.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index c8b6ac8..e2c6f1a 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -23,8 +23,8 @@
>  #define MLX90614_OP_RAM 0x00
>  
>  /* RAM offsets with 16-bit data, MSB first */
> -#define MLX90614_TA 0x06 /* ambient temperature */
> -#define MLX90614_TOBJ1 0x07 /* object temperature */
> +#define MLX90614_TA	(MLX90614_OP_RAM | 0x06) /* ambient temperature */
> +#define MLX90614_TOBJ1	(MLX90614_OP_RAM | 0x07) /* object 1 temperature */
>  
>  struct mlx90614_data {
>  	struct i2c_client *client;
> @@ -42,13 +42,13 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  		switch (channel->channel2) {
>  		case IIO_MOD_TEMP_AMBIENT:
>  			ret = i2c_smbus_read_word_data(data->client,
> -			    MLX90614_OP_RAM | MLX90614_TA);
> +			    MLX90614_TA);
>  			if (ret < 0)
>  				return ret;
>  			break;
>  		case IIO_MOD_TEMP_OBJECT:
>  			ret = i2c_smbus_read_word_data(data->client,
> -			    MLX90614_OP_RAM | MLX90614_TOBJ1);
> +			    MLX90614_TOBJ1);
>  			if (ret < 0)
>  				return ret;
>  			break;
> 


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

* Re: [PATCH v2 3/7] iio: mlx90614: Support devices with dual IR sensor
  2015-03-24 15:54 ` [PATCH v2 3/7] iio: mlx90614: Support devices with dual IR sensor Vianney le Clément de Saint-Marcq
@ 2015-03-28 12:44   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:44 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> The model is detected by reading the EEPROM configuration during
> probing.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play with it.

Thanks,
> ---
> 
> v2: * remove symmetrical ambient channel
>     * rename _probe_model to _probe_num_ir_sensors
> ---
>  drivers/iio/temperature/mlx90614.c | 67 ++++++++++++++++++++++++++++++++------
>  1 file changed, 57 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index a2e3aa6..a112fc9 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -2,6 +2,7 @@
>   * mlx90614.c - Support for Melexis MLX90614 contactless IR temperature sensor
>   *
>   * Copyright (c) 2014 Peter Meerwald <pmeerw@pmeerw.net>
> + * Copyright (c) 2015 Essensium NV
>   *
>   * 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
> @@ -59,26 +60,34 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			    int *val2, long mask)
>  {
>  	struct mlx90614_data *data = iio_priv(indio_dev);
> +	u8 cmd;
>  	s32 ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW: /* 0.02K / LSB */
>  		switch (channel->channel2) {
>  		case IIO_MOD_TEMP_AMBIENT:
> -			ret = i2c_smbus_read_word_data(data->client,
> -			    MLX90614_TA);
> -			if (ret < 0)
> -				return ret;
> +			cmd = MLX90614_TA;
>  			break;
>  		case IIO_MOD_TEMP_OBJECT:
> -			ret = i2c_smbus_read_word_data(data->client,
> -			    MLX90614_TOBJ1);
> -			if (ret < 0)
> -				return ret;
> +			switch (channel->channel) {
> +			case 0:
> +				cmd = MLX90614_TOBJ1;
> +				break;
> +			case 1:
> +				cmd = MLX90614_TOBJ2;
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
>  			break;
>  		default:
>  			return -EINVAL;
>  		}
> +
> +		ret = i2c_smbus_read_word_data(data->client, cmd);
> +		if (ret < 0)
> +			return ret;
>  		*val = ret;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_OFFSET:
> @@ -110,6 +119,16 @@ static const struct iio_chan_spec mlx90614_channels[] = {
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>  		    BIT(IIO_CHAN_INFO_SCALE),
>  	},
> +	{
> +		.type = IIO_TEMP,
> +		.indexed = 1,
> +		.modified = 1,
> +		.channel = 1,
> +		.channel2 = IIO_MOD_TEMP_OBJECT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		    BIT(IIO_CHAN_INFO_SCALE),
> +	},
>  };
>  
>  static const struct iio_info mlx90614_info = {
> @@ -117,11 +136,25 @@ static const struct iio_info mlx90614_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +/* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
> +static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
> +{
> +	s32 ret;
> +
> +	ret = i2c_smbus_read_word_data(client, MLX90614_CONFIG);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret & MLX90614_CONFIG_DUAL_MASK) ? 1 : 0;
> +}
> +
>  static int mlx90614_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct iio_dev *indio_dev;
>  	struct mlx90614_data *data;
> +	int ret;
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>  		return -ENODEV;
> @@ -139,8 +172,21 @@ static int mlx90614_probe(struct i2c_client *client,
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &mlx90614_info;
>  
> -	indio_dev->channels = mlx90614_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(mlx90614_channels);
> +	ret = mlx90614_probe_num_ir_sensors(client);
> +	switch (ret) {
> +	case 0:
> +		dev_dbg(&client->dev, "Found single sensor");
> +		indio_dev->channels = mlx90614_channels;
> +		indio_dev->num_channels = 2;
> +		break;
> +	case 1:
> +		dev_dbg(&client->dev, "Found dual sensor");
> +		indio_dev->channels = mlx90614_channels;
> +		indio_dev->num_channels = 3;
> +		break;
> +	default:
> +		return ret;
> +	}
>  
>  	return iio_device_register(indio_dev);
>  }
> @@ -170,5 +216,6 @@ static struct i2c_driver mlx90614_driver = {
>  module_i2c_driver(mlx90614_driver);
>  
>  MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
> +MODULE_AUTHOR("Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>");
>  MODULE_DESCRIPTION("Melexis MLX90614 contactless IR temperature sensor driver");
>  MODULE_LICENSE("GPL");
> 


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

* Re: [PATCH v2 4/7] iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY
  2015-03-24 16:02   ` Lars-Peter Clausen
@ 2015-03-28 12:45     ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:45 UTC (permalink / raw)
  To: Lars-Peter Clausen, Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 16:02, Lars-Peter Clausen wrote:
> On 03/24/2015 04:54 PM, Vianney le Clément de Saint-Marcq wrote:
>> Contact-less IR temperature sensors measure the temperature of an object
>> by using its thermal radiation.  Surfaces with different emissivity
>> ratios emit different amounts of energy at the same temperature.
>>
>> IIO_CHAN_INFO_EMISSIVITY allows the user to inform the sensor of the
>> emissivity of the object in front of it, in order to effectively measure
>> its temperature.
> 

> All the other attributes that need to be setup according to the
> object being measured start with the calib prefix. So maybe call this
> calibemissivity.
Agreed. It's an internally applied calibration parameter (be it one that
is likely to change more than the average calibration!) so should have
the calib prefix.

Thanks,

Jonathan
> - Lars
> -- 
> 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] 18+ messages in thread

* Re: [PATCH v2 5/7] iio: mlx90614: Add emissivity setting
  2015-03-24 15:54 ` [PATCH v2 5/7] iio: mlx90614: Add emissivity setting Vianney le Clément de Saint-Marcq
@ 2015-03-28 12:46   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:46 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> The mapping from the 16-bit EEPROM value to the decimal 0-1 range is
> approximate.  A special case ensures 0xFFFF shows as 1.0 instead of
> 0.999998565.
> 
> Writing to EEPROM requires an explicit erase by writing zero.  In
> addition, it takes 20ms for the erase/write to complete.  During this
> time no EEPROM register should be accessed.  Therefore, two msleep()s
> are added to the write function and a mutex protects against concurrent
> access.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
Fine other than change to calibemissivity.
> ---
> 
> v2: * remove iir, fir, and gain attributes
>     * refactor emissivity to use IIO_CHAN_INFO_EMISSIVITY
> ---
>  drivers/iio/temperature/mlx90614.c | 106 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index a112fc9..b981d1e 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -12,12 +12,13 @@
>   *
>   * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
>   *
> - * TODO: sleep mode, configuration EEPROM
> + * TODO: sleep mode, filter configuration
>   */
>  
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
> +#include <linux/delay.h>
>  
>  #include <linux/iio/iio.h>
>  
> @@ -53,8 +54,47 @@
>  
>  struct mlx90614_data {
>  	struct i2c_client *client;
> +	struct mutex lock; /* for EEPROM access only */
>  };
>  
> +/*
> + * Erase an address and write word.
> + * The mutex must be locked before calling.
> + */
> +static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
> +			       u16 value)
> +{
> +	/*
> +	 * Note: The mlx90614 requires a PEC on writing but does not send us a
> +	 * valid PEC on reading.  Hence, we cannot set I2C_CLIENT_PEC in
> +	 * i2c_client.flags.  As a workaround, we use i2c_smbus_xfer here.
> +	 */
> +	union i2c_smbus_data data;
> +	s32 ret;
> +
> +	dev_dbg(&client->dev, "Writing 0x%x to address 0x%x", value, command);
> +
> +	data.word = 0x0000; /* erase command */
> +	ret = i2c_smbus_xfer(client->adapter, client->addr,
> +			     client->flags | I2C_CLIENT_PEC,
> +			     I2C_SMBUS_WRITE, command,
> +			     I2C_SMBUS_WORD_DATA, &data);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(MLX90614_TIMING_EEPROM);
> +
> +	data.word = value; /* actual write */
> +	ret = i2c_smbus_xfer(client->adapter, client->addr,
> +			     client->flags | I2C_CLIENT_PEC,
> +			     I2C_SMBUS_WRITE, command,
> +			     I2C_SMBUS_WORD_DATA, &data);
> +
> +	msleep(MLX90614_TIMING_EEPROM);
> +
> +	return ret;
> +}
> +
>  static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *channel, int *val,
>  			    int *val2, long mask)
> @@ -97,6 +137,61 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 20;
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
> +		mutex_lock(&data->lock);
> +		ret = i2c_smbus_read_word_data(data->client,
> +					       MLX90614_EMISSIVITY);
> +		mutex_unlock(&data->lock);
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret == 65535) {
> +			*val = 1;
> +			*val2 = 0;
> +		} else {
> +			*val = 0;
> +			*val2 = ret * 15259; /* 1/65535 ~ 0.000015259 */
> +		}
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mlx90614_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *channel, int val,
> +			     int val2, long mask)
> +{
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +	s32 ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
> +		if (val < 0 || val2 < 0 || val > 1 || (val == 1 && val2 != 0))
> +			return -EINVAL;
> +		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
> +
> +		mutex_lock(&data->lock);
> +		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
> +					  val);
> +		mutex_unlock(&data->lock);
> +
> +		if (ret < 0)
> +			return ret;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mlx90614_write_raw_get_fmt(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec const *channel,
> +				     long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_EMISSIVITY:
> +		return IIO_VAL_INT_PLUS_NANO;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -115,7 +210,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
>  		.type = IIO_TEMP,
>  		.modified = 1,
>  		.channel2 = IIO_MOD_TEMP_OBJECT,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +		    BIT(IIO_CHAN_INFO_EMISSIVITY),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>  		    BIT(IIO_CHAN_INFO_SCALE),
>  	},
> @@ -125,7 +221,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
>  		.modified = 1,
>  		.channel = 1,
>  		.channel2 = IIO_MOD_TEMP_OBJECT,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +		    BIT(IIO_CHAN_INFO_EMISSIVITY),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>  		    BIT(IIO_CHAN_INFO_SCALE),
>  	},
> @@ -133,6 +230,8 @@ static const struct iio_chan_spec mlx90614_channels[] = {
>  
>  static const struct iio_info mlx90614_info = {
>  	.read_raw = mlx90614_read_raw,
> +	.write_raw = mlx90614_write_raw,
> +	.write_raw_get_fmt = mlx90614_write_raw_get_fmt,
>  	.driver_module = THIS_MODULE,
>  };
>  
> @@ -166,6 +265,7 @@ static int mlx90614_probe(struct i2c_client *client,
>  	data = iio_priv(indio_dev);
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
> +	mutex_init(&data->lock);
>  
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->name = id->name;
> 


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

* Re: [PATCH v2 6/7] iio: mlx90614: Add power management
  2015-03-24 15:54     ` Vianney le Clément de Saint-Marcq
@ 2015-03-28 12:54         ` Jonathan Cameron
  -1 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:54 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> Add support for system sleep and runtime power management.
> 
> To wake up the device, the SDA line should be held low for at least 33ms
> while SCL is high.  As this is not possible using the i2c API (and not
> supported by all i2c adapters), a GPIO connected to the SDA line is
> needed.  The GPIO is named "wakeup" and can be specified in a device
> tree with the "wakeup-gpios" binding.
> 
> If the wake-up GPIO is not given, disable power management for the
> device.  Entering sleep requires an SMBus byte access, hence power
> management is also disabled if byte access is not supported by the
> adapter.
> 
> Introduce "melexis" as a vendor prefix for device tree bindings.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement-buIOx9BAs4sybS5Ee8rs3A@public.gmane.org>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout-agwphqFHkxc@public.gmane.org>
> 
A couple of bits inline.  In particularly please break the docs into
a separate patch and only add the new element in this one.

Also, your probe and remove are not mirror images and should be.

Jonathan
> ---
> 
> The default autosleep delay (5s) is chosen arbitrarily, trying to take
> into account the long startup time (250ms).  Feel free to change it to
> another value if you think it is saner.
> 
> v2: return -EINTR when the startup delay is interrupted
> ---
>  .../bindings/iio/temperature/mlx90614.txt          |  24 ++
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  drivers/iio/temperature/mlx90614.c                 | 246 ++++++++++++++++++++-
>  3 files changed, 269 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
> new file mode 100644
> index 0000000..9be57b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
Please split out an initial device tree patch adding the device and docs
then a followup one adding the new stuff.
> @@ -0,0 +1,24 @@
> +* Melexis MLX90614 contactless IR temperature sensor
> +
> +http://melexis.com/Infrared-Thermometer-Sensors/Infrared-Thermometer-Sensors/MLX90614-615.aspx
> +
> +Required properties:
> +
> +  - compatible: should be "melexis,mlx90614"
> +  - reg: the I2C address of the sensor
> +
> +Optional properties:
> +
> +  - wakeup-gpios: device tree identifier of the GPIO connected to the SDA line
> +      to hold low in order to wake up the device.  In normal operation, the
> +      GPIO is set as input and will not interfere in I2C communication.  There
> +      is no need for a GPIO driving the SCL line.  If no GPIO is given, power
> +      management is disabled.
> +
> +Example:
> +
> +mlx90614@5a {
> +	compatible = "melexis,mlx90614";
> +	reg = <0x5a>;
> +	wakeup-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index fae26d0..c8db30c 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -110,6 +110,7 @@ lltc	Linear Technology Corporation
>  marvell	Marvell Technology Group Ltd.
>  maxim	Maxim Integrated Products
>  mediatek	MediaTek Inc.
> +melexis	Melexis N.V.
>  merrii	Merrii Technology Co., Ltd.
>  micrel	Micrel Inc.
>  microchip	Microchip Technology Inc.
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index b981d1e..6b9bfcd 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -12,13 +12,24 @@
>   *
>   * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
>   *
> - * TODO: sleep mode, filter configuration
> + * To wake up from sleep mode, the SDA line must be held low while SCL is high
> + * for at least 33ms.  This is achieved with an extra GPIO that can be connected
> + * directly to the SDA line.  In normal operation, the GPIO is set as input and
> + * will not interfere in I2C communication.  While the GPIO is driven low, the
> + * i2c adapter is locked since it cannot be used by other clients.  The SCL line
> + * always has a pull-up so we do not need an extra GPIO to drive it high.  If
> + * the "wakeup" GPIO is not given, power management will be disabled.
> + *
> + * TODO: filter configuration
>   */
>  
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/delay.h>
> +#include <linux/jiffies.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/iio/iio.h>
>  
> @@ -52,9 +63,13 @@
>  #define MLX90614_TIMING_WAKEUP 34 /* time to hold SDA low for wake-up */
>  #define MLX90614_TIMING_STARTUP 250 /* time before first data after wake-up */
>  
> +#define MLX90614_AUTOSLEEP_DELAY 5000 /* default autosleep delay */
> +
>  struct mlx90614_data {
>  	struct i2c_client *client;
>  	struct mutex lock; /* for EEPROM access only */
> +	struct gpio_desc *wakeup_gpio; /* NULL to disable sleep/wake-up */
> +	unsigned long ready_timestamp; /* in jiffies */
>  };
>  
>  /*
> @@ -95,6 +110,54 @@ static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_PM
> +/*
> + * If @startup is true, make sure MLX90614_TIMING_STARTUP ms have elapsed since
> + * the last wake-up.  This is normally only needed to get a valid temperature
> + * reading.  EEPROM access does not need such delay.
> + * Return 0 on success, <0 on error.
> + */
> +static int mlx90614_power_get(struct mlx90614_data *data, bool startup)
> +{
> +	unsigned long now;
> +
> +	if (!data->wakeup_gpio)
> +		return 0;
> +
> +	pm_runtime_get_sync(&data->client->dev);
> +
> +	if (startup) {
> +		now = jiffies;
> +		if (time_before(now, data->ready_timestamp) &&
> +		    msleep_interruptible(jiffies_to_msecs(
> +				data->ready_timestamp - now)) != 0) {
> +			pm_runtime_put_autosuspend(&data->client->dev);
> +			return -EINTR;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void mlx90614_power_put(struct mlx90614_data *data)
> +{
> +	if (!data->wakeup_gpio)
> +		return;
> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	pm_runtime_put_autosuspend(&data->client->dev);
> +}
> +#else
> +static inline int mlx90614_power_get(struct mlx90614_data *data, bool startup)
> +{
> +	return 0;
> +}
> +
> +static inline void mlx90614_power_put(struct mlx90614_data *data)
> +{
> +}
> +#endif
> +
>  static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *channel, int *val,
>  			    int *val2, long mask)
> @@ -125,7 +188,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  
> +		ret = mlx90614_power_get(data, true);
> +		if (ret < 0)
> +			return ret;
>  		ret = i2c_smbus_read_word_data(data->client, cmd);
> +		mlx90614_power_put(data);
> +
>  		if (ret < 0)
>  			return ret;
>  		*val = ret;
> @@ -138,10 +206,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  		*val = 20;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
> +		mlx90614_power_get(data, false);
>  		mutex_lock(&data->lock);
>  		ret = i2c_smbus_read_word_data(data->client,
>  					       MLX90614_EMISSIVITY);
>  		mutex_unlock(&data->lock);
> +		mlx90614_power_put(data);
>  
>  		if (ret < 0)
>  			return ret;
> @@ -172,10 +242,12 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
>  
> +		mlx90614_power_get(data, false);
>  		mutex_lock(&data->lock);
>  		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
>  					  val);
>  		mutex_unlock(&data->lock);
> +		mlx90614_power_put(data);
>  
>  		if (ret < 0)
>  			return ret;
> @@ -235,6 +307,98 @@ static const struct iio_info mlx90614_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +#ifdef CONFIG_PM
> +static int mlx90614_sleep(struct mlx90614_data *data)
> +{
> +	s32 ret;
> +
> +	if (!data->wakeup_gpio) {
> +		dev_dbg(&data->client->dev, "Sleep disabled");
> +		return -ENOSYS;
> +	}
> +
> +	dev_dbg(&data->client->dev, "Requesting sleep");
> +
> +	mutex_lock(&data->lock);
> +	ret = i2c_smbus_xfer(data->client->adapter, data->client->addr,
> +			     data->client->flags | I2C_CLIENT_PEC,
> +			     I2C_SMBUS_WRITE, MLX90614_OP_SLEEP,
> +			     I2C_SMBUS_BYTE, NULL);
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int mlx90614_wakeup(struct mlx90614_data *data)
> +{
> +	if (!data->wakeup_gpio) {
> +		dev_dbg(&data->client->dev, "Wake-up disabled");
> +		return -ENOSYS;
> +	}
> +
> +	dev_dbg(&data->client->dev, "Requesting wake-up");
> +
> +	i2c_lock_adapter(data->client->adapter);
> +	gpiod_direction_output(data->wakeup_gpio, 0);
> +	msleep(MLX90614_TIMING_WAKEUP);
> +	gpiod_direction_input(data->wakeup_gpio);
> +	i2c_unlock_adapter(data->client->adapter);
> +
> +	data->ready_timestamp = jiffies +
> +			msecs_to_jiffies(MLX90614_TIMING_STARTUP);
> +
> +	/*
> +	 * Quirk: the i2c controller may get confused right after the
> +	 * wake-up signal has been sent.  As a workaround, do a dummy read.
> +	 * If the read fails, the controller will probably be reset so that
> +	 * further reads will work.
> +	 */
> +	i2c_smbus_read_word_data(data->client, MLX90614_CONFIG);
That wins the nasty award for the day!  It'll probably not work
on lots of boards, but there we are...  No obvious alternatives exist.

> +
> +	return 0;
> +}
> +
> +/* Return wake-up GPIO or NULL if sleep functionality should be disabled. */
> +static struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
> +{
> +	struct gpio_desc *gpio;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +						I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_info(&client->dev,
> +			 "i2c adapter does not support SMBUS_WRITE_BYTE, sleep disabled");
> +		return NULL;
> +	}
> +
> +	gpio = devm_gpiod_get_optional(&client->dev, "wakeup", GPIOD_IN);
> +
> +	if (IS_ERR(gpio)) {
> +		dev_warn(&client->dev,
> +			 "gpio acquisition failed with error %ld, sleep disabled",
> +			 PTR_ERR(gpio));
> +		return NULL;
> +	} else if (!gpio) {
> +		dev_info(&client->dev,
> +			 "wakeup-gpio not found, sleep disabled");
> +	}
> +
> +	return gpio;
> +}
> +#else
> +static inline int mlx90614_sleep(struct mlx90614_data *data)
> +{
> +	return -ENOSYS;
> +}
> +static inline int mlx90614_wakeup(struct mlx90614_data *data)
> +{
> +	return -ENOSYS;
> +}
> +static inline struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  /* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
>  static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
>  {
> @@ -266,6 +430,9 @@ static int mlx90614_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  	mutex_init(&data->lock);
> +	data->wakeup_gpio = mlx90614_probe_wakeup(client);
> +
> +	mlx90614_wakeup(data);
>  
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->name = id->name;
> @@ -288,12 +455,30 @@ static int mlx90614_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	if (data->wakeup_gpio) {
> +		pm_runtime_set_autosuspend_delay(&client->dev,
> +						 MLX90614_AUTOSLEEP_DELAY);
> +		pm_runtime_use_autosuspend(&client->dev);
> +		pm_runtime_set_active(&client->dev);
> +		pm_runtime_enable(&client->dev);
> +	}
> +
>  	return iio_device_register(indio_dev);
>  }
>  
>  static int mlx90614_remove(struct i2c_client *client)
>  {
> -	iio_device_unregister(i2c_get_clientdata(client));
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	if (data->wakeup_gpio) {
> +		pm_runtime_disable(&client->dev);
> +		if (!pm_runtime_status_suspended(&client->dev))
> +			mlx90614_sleep(data);
> +		pm_runtime_set_suspended(&client->dev);
> +	}
> +
> +	iio_device_unregister(indio_dev);
Your ordering is not the reverse of probe.  The unregister should
be before the pm stuff.  While it may well not actually matter,
keeping this clean remove -> reverse operations of probe makes
for more obviously correct code so is good to maintain where
possible (and add comments where it actually needs to be different!)

>  
>  	return 0;
>  }
> @@ -304,10 +489,67 @@ static const struct i2c_device_id mlx90614_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, mlx90614_id);
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int mlx90614_pm_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	if (data->wakeup_gpio && pm_runtime_active(dev))
> +		return mlx90614_sleep(data);
> +
> +	return 0;
> +}
> +
> +static int mlx90614_pm_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +	int err;
> +
> +	if (data->wakeup_gpio) {
> +		err = mlx90614_wakeup(data);
> +		if (err < 0)
> +			return err;
> +
> +		pm_runtime_disable(dev);
> +		pm_runtime_set_active(dev);
> +		pm_runtime_enable(dev);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_PM
> +static int mlx90614_pm_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	return mlx90614_sleep(data);
> +}
> +
> +static int mlx90614_pm_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	return mlx90614_wakeup(data);
Could have rolled this into one line, but it doesn't really matter.
        return mlx90614_wakeup(iio_priv(i2c_get_clientdata(to_i2c_client(dev))));
> +}
> +#endif
> +
> +static const struct dev_pm_ops mlx90614_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(mlx90614_pm_suspend, mlx90614_pm_resume)
> +	SET_RUNTIME_PM_OPS(mlx90614_pm_runtime_suspend,
> +			   mlx90614_pm_runtime_resume, NULL)
> +};
> +
>  static struct i2c_driver mlx90614_driver = {
>  	.driver = {
>  		.name	= "mlx90614",
>  		.owner	= THIS_MODULE,
> +		.pm	= &mlx90614_pm_ops,
>  	},
>  	.probe = mlx90614_probe,
>  	.remove = mlx90614_remove,
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 6/7] iio: mlx90614: Add power management
@ 2015-03-28 12:54         ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:54 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio, devicetree
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> Add support for system sleep and runtime power management.
> 
> To wake up the device, the SDA line should be held low for at least 33ms
> while SCL is high.  As this is not possible using the i2c API (and not
> supported by all i2c adapters), a GPIO connected to the SDA line is
> needed.  The GPIO is named "wakeup" and can be specified in a device
> tree with the "wakeup-gpios" binding.
> 
> If the wake-up GPIO is not given, disable power management for the
> device.  Entering sleep requires an SMBus byte access, hence power
> management is also disabled if byte access is not supported by the
> adapter.
> 
> Introduce "melexis" as a vendor prefix for device tree bindings.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
A couple of bits inline.  In particularly please break the docs into
a separate patch and only add the new element in this one.

Also, your probe and remove are not mirror images and should be.

Jonathan
> ---
> 
> The default autosleep delay (5s) is chosen arbitrarily, trying to take
> into account the long startup time (250ms).  Feel free to change it to
> another value if you think it is saner.
> 
> v2: return -EINTR when the startup delay is interrupted
> ---
>  .../bindings/iio/temperature/mlx90614.txt          |  24 ++
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  drivers/iio/temperature/mlx90614.c                 | 246 ++++++++++++++++++++-
>  3 files changed, 269 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
> new file mode 100644
> index 0000000..9be57b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/mlx90614.txt
Please split out an initial device tree patch adding the device and docs
then a followup one adding the new stuff.
> @@ -0,0 +1,24 @@
> +* Melexis MLX90614 contactless IR temperature sensor
> +
> +http://melexis.com/Infrared-Thermometer-Sensors/Infrared-Thermometer-Sensors/MLX90614-615.aspx
> +
> +Required properties:
> +
> +  - compatible: should be "melexis,mlx90614"
> +  - reg: the I2C address of the sensor
> +
> +Optional properties:
> +
> +  - wakeup-gpios: device tree identifier of the GPIO connected to the SDA line
> +      to hold low in order to wake up the device.  In normal operation, the
> +      GPIO is set as input and will not interfere in I2C communication.  There
> +      is no need for a GPIO driving the SCL line.  If no GPIO is given, power
> +      management is disabled.
> +
> +Example:
> +
> +mlx90614@5a {
> +	compatible = "melexis,mlx90614";
> +	reg = <0x5a>;
> +	wakeup-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index fae26d0..c8db30c 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -110,6 +110,7 @@ lltc	Linear Technology Corporation
>  marvell	Marvell Technology Group Ltd.
>  maxim	Maxim Integrated Products
>  mediatek	MediaTek Inc.
> +melexis	Melexis N.V.
>  merrii	Merrii Technology Co., Ltd.
>  micrel	Micrel Inc.
>  microchip	Microchip Technology Inc.
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index b981d1e..6b9bfcd 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -12,13 +12,24 @@
>   *
>   * (7-bit I2C slave address 0x5a, 100KHz bus speed only!)
>   *
> - * TODO: sleep mode, filter configuration
> + * To wake up from sleep mode, the SDA line must be held low while SCL is high
> + * for at least 33ms.  This is achieved with an extra GPIO that can be connected
> + * directly to the SDA line.  In normal operation, the GPIO is set as input and
> + * will not interfere in I2C communication.  While the GPIO is driven low, the
> + * i2c adapter is locked since it cannot be used by other clients.  The SCL line
> + * always has a pull-up so we do not need an extra GPIO to drive it high.  If
> + * the "wakeup" GPIO is not given, power management will be disabled.
> + *
> + * TODO: filter configuration
>   */
>  
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/delay.h>
> +#include <linux/jiffies.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/iio/iio.h>
>  
> @@ -52,9 +63,13 @@
>  #define MLX90614_TIMING_WAKEUP 34 /* time to hold SDA low for wake-up */
>  #define MLX90614_TIMING_STARTUP 250 /* time before first data after wake-up */
>  
> +#define MLX90614_AUTOSLEEP_DELAY 5000 /* default autosleep delay */
> +
>  struct mlx90614_data {
>  	struct i2c_client *client;
>  	struct mutex lock; /* for EEPROM access only */
> +	struct gpio_desc *wakeup_gpio; /* NULL to disable sleep/wake-up */
> +	unsigned long ready_timestamp; /* in jiffies */
>  };
>  
>  /*
> @@ -95,6 +110,54 @@ static s32 mlx90614_write_word(const struct i2c_client *client, u8 command,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_PM
> +/*
> + * If @startup is true, make sure MLX90614_TIMING_STARTUP ms have elapsed since
> + * the last wake-up.  This is normally only needed to get a valid temperature
> + * reading.  EEPROM access does not need such delay.
> + * Return 0 on success, <0 on error.
> + */
> +static int mlx90614_power_get(struct mlx90614_data *data, bool startup)
> +{
> +	unsigned long now;
> +
> +	if (!data->wakeup_gpio)
> +		return 0;
> +
> +	pm_runtime_get_sync(&data->client->dev);
> +
> +	if (startup) {
> +		now = jiffies;
> +		if (time_before(now, data->ready_timestamp) &&
> +		    msleep_interruptible(jiffies_to_msecs(
> +				data->ready_timestamp - now)) != 0) {
> +			pm_runtime_put_autosuspend(&data->client->dev);
> +			return -EINTR;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void mlx90614_power_put(struct mlx90614_data *data)
> +{
> +	if (!data->wakeup_gpio)
> +		return;
> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	pm_runtime_put_autosuspend(&data->client->dev);
> +}
> +#else
> +static inline int mlx90614_power_get(struct mlx90614_data *data, bool startup)
> +{
> +	return 0;
> +}
> +
> +static inline void mlx90614_power_put(struct mlx90614_data *data)
> +{
> +}
> +#endif
> +
>  static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *channel, int *val,
>  			    int *val2, long mask)
> @@ -125,7 +188,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  
> +		ret = mlx90614_power_get(data, true);
> +		if (ret < 0)
> +			return ret;
>  		ret = i2c_smbus_read_word_data(data->client, cmd);
> +		mlx90614_power_put(data);
> +
>  		if (ret < 0)
>  			return ret;
>  		*val = ret;
> @@ -138,10 +206,12 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  		*val = 20;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_EMISSIVITY: /* 1/65535 / LSB */
> +		mlx90614_power_get(data, false);
>  		mutex_lock(&data->lock);
>  		ret = i2c_smbus_read_word_data(data->client,
>  					       MLX90614_EMISSIVITY);
>  		mutex_unlock(&data->lock);
> +		mlx90614_power_put(data);
>  
>  		if (ret < 0)
>  			return ret;
> @@ -172,10 +242,12 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		val = val * 65535 + val2 / 15259; /* 1/65535 ~ 0.000015259 */
>  
> +		mlx90614_power_get(data, false);
>  		mutex_lock(&data->lock);
>  		ret = mlx90614_write_word(data->client, MLX90614_EMISSIVITY,
>  					  val);
>  		mutex_unlock(&data->lock);
> +		mlx90614_power_put(data);
>  
>  		if (ret < 0)
>  			return ret;
> @@ -235,6 +307,98 @@ static const struct iio_info mlx90614_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +#ifdef CONFIG_PM
> +static int mlx90614_sleep(struct mlx90614_data *data)
> +{
> +	s32 ret;
> +
> +	if (!data->wakeup_gpio) {
> +		dev_dbg(&data->client->dev, "Sleep disabled");
> +		return -ENOSYS;
> +	}
> +
> +	dev_dbg(&data->client->dev, "Requesting sleep");
> +
> +	mutex_lock(&data->lock);
> +	ret = i2c_smbus_xfer(data->client->adapter, data->client->addr,
> +			     data->client->flags | I2C_CLIENT_PEC,
> +			     I2C_SMBUS_WRITE, MLX90614_OP_SLEEP,
> +			     I2C_SMBUS_BYTE, NULL);
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int mlx90614_wakeup(struct mlx90614_data *data)
> +{
> +	if (!data->wakeup_gpio) {
> +		dev_dbg(&data->client->dev, "Wake-up disabled");
> +		return -ENOSYS;
> +	}
> +
> +	dev_dbg(&data->client->dev, "Requesting wake-up");
> +
> +	i2c_lock_adapter(data->client->adapter);
> +	gpiod_direction_output(data->wakeup_gpio, 0);
> +	msleep(MLX90614_TIMING_WAKEUP);
> +	gpiod_direction_input(data->wakeup_gpio);
> +	i2c_unlock_adapter(data->client->adapter);
> +
> +	data->ready_timestamp = jiffies +
> +			msecs_to_jiffies(MLX90614_TIMING_STARTUP);
> +
> +	/*
> +	 * Quirk: the i2c controller may get confused right after the
> +	 * wake-up signal has been sent.  As a workaround, do a dummy read.
> +	 * If the read fails, the controller will probably be reset so that
> +	 * further reads will work.
> +	 */
> +	i2c_smbus_read_word_data(data->client, MLX90614_CONFIG);
That wins the nasty award for the day!  It'll probably not work
on lots of boards, but there we are...  No obvious alternatives exist.

> +
> +	return 0;
> +}
> +
> +/* Return wake-up GPIO or NULL if sleep functionality should be disabled. */
> +static struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
> +{
> +	struct gpio_desc *gpio;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +						I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_info(&client->dev,
> +			 "i2c adapter does not support SMBUS_WRITE_BYTE, sleep disabled");
> +		return NULL;
> +	}
> +
> +	gpio = devm_gpiod_get_optional(&client->dev, "wakeup", GPIOD_IN);
> +
> +	if (IS_ERR(gpio)) {
> +		dev_warn(&client->dev,
> +			 "gpio acquisition failed with error %ld, sleep disabled",
> +			 PTR_ERR(gpio));
> +		return NULL;
> +	} else if (!gpio) {
> +		dev_info(&client->dev,
> +			 "wakeup-gpio not found, sleep disabled");
> +	}
> +
> +	return gpio;
> +}
> +#else
> +static inline int mlx90614_sleep(struct mlx90614_data *data)
> +{
> +	return -ENOSYS;
> +}
> +static inline int mlx90614_wakeup(struct mlx90614_data *data)
> +{
> +	return -ENOSYS;
> +}
> +static inline struct gpio_desc *mlx90614_probe_wakeup(struct i2c_client *client)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  /* Return 0 for single sensor, 1 for dual sensor, <0 on error. */
>  static int mlx90614_probe_num_ir_sensors(struct i2c_client *client)
>  {
> @@ -266,6 +430,9 @@ static int mlx90614_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  	mutex_init(&data->lock);
> +	data->wakeup_gpio = mlx90614_probe_wakeup(client);
> +
> +	mlx90614_wakeup(data);
>  
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->name = id->name;
> @@ -288,12 +455,30 @@ static int mlx90614_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	if (data->wakeup_gpio) {
> +		pm_runtime_set_autosuspend_delay(&client->dev,
> +						 MLX90614_AUTOSLEEP_DELAY);
> +		pm_runtime_use_autosuspend(&client->dev);
> +		pm_runtime_set_active(&client->dev);
> +		pm_runtime_enable(&client->dev);
> +	}
> +
>  	return iio_device_register(indio_dev);
>  }
>  
>  static int mlx90614_remove(struct i2c_client *client)
>  {
> -	iio_device_unregister(i2c_get_clientdata(client));
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	if (data->wakeup_gpio) {
> +		pm_runtime_disable(&client->dev);
> +		if (!pm_runtime_status_suspended(&client->dev))
> +			mlx90614_sleep(data);
> +		pm_runtime_set_suspended(&client->dev);
> +	}
> +
> +	iio_device_unregister(indio_dev);
Your ordering is not the reverse of probe.  The unregister should
be before the pm stuff.  While it may well not actually matter,
keeping this clean remove -> reverse operations of probe makes
for more obviously correct code so is good to maintain where
possible (and add comments where it actually needs to be different!)

>  
>  	return 0;
>  }
> @@ -304,10 +489,67 @@ static const struct i2c_device_id mlx90614_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, mlx90614_id);
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int mlx90614_pm_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	if (data->wakeup_gpio && pm_runtime_active(dev))
> +		return mlx90614_sleep(data);
> +
> +	return 0;
> +}
> +
> +static int mlx90614_pm_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +	int err;
> +
> +	if (data->wakeup_gpio) {
> +		err = mlx90614_wakeup(data);
> +		if (err < 0)
> +			return err;
> +
> +		pm_runtime_disable(dev);
> +		pm_runtime_set_active(dev);
> +		pm_runtime_enable(dev);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_PM
> +static int mlx90614_pm_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	return mlx90614_sleep(data);
> +}
> +
> +static int mlx90614_pm_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct mlx90614_data *data = iio_priv(indio_dev);
> +
> +	return mlx90614_wakeup(data);
Could have rolled this into one line, but it doesn't really matter.
        return mlx90614_wakeup(iio_priv(i2c_get_clientdata(to_i2c_client(dev))));
> +}
> +#endif
> +
> +static const struct dev_pm_ops mlx90614_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(mlx90614_pm_suspend, mlx90614_pm_resume)
> +	SET_RUNTIME_PM_OPS(mlx90614_pm_runtime_suspend,
> +			   mlx90614_pm_runtime_resume, NULL)
> +};
> +
>  static struct i2c_driver mlx90614_driver = {
>  	.driver = {
>  		.name	= "mlx90614",
>  		.owner	= THIS_MODULE,
> +		.pm	= &mlx90614_pm_ops,
>  	},
>  	.probe = mlx90614_probe,
>  	.remove = mlx90614_remove,
> 


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

* Re: [PATCH v2 7/7] iio: mlx90614: Check for errors in read values
  2015-03-24 15:54 ` [PATCH v2 7/7] iio: mlx90614: Check for errors in read values Vianney le Clément de Saint-Marcq
@ 2015-03-28 13:43   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 13:43 UTC (permalink / raw)
  To: Vianney le Clément de Saint-Marcq, linux-iio
  Cc: Arnout Vandecappelle (Essensium/Mind)

On 24/03/15 15:54, Vianney le Clément de Saint-Marcq wrote:
> The device uses the MSB of the returned temperature values as an error
> flag.  Return a read error when this bit is set.
> 
> Signed-off-by: Vianney le Clément de Saint-Marcq <vianney.leclement@essensium.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
Hmm. Does it really signify a remote IO issue.  I'd have thought EIO was
more appropriate.

I see one -EREMOTEIO using driver (vybrid ADC) but in other similar
cases we've always gone with EIO.

J
> ---
> 
> v2: new patch
> ---
>  drivers/iio/temperature/mlx90614.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index 6b9bfcd..6cb1e81 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -196,6 +196,11 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  
>  		if (ret < 0)
>  			return ret;
> +
> +		/* MSB is an error flag */
> +		if (ret & 0x8000)
> +			return -EREMOTEIO;
> +
>  		*val = ret;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_OFFSET:
> 


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

end of thread, other threads:[~2015-03-28 13:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 15:54 [PATCH v2 0/7] iio: mlx90614 enhancements Vianney le Clément de Saint-Marcq
2015-03-24 15:54 ` Vianney le Clément de Saint-Marcq
2015-03-24 15:54 ` [PATCH v2 1/7] iio: mlx90614: Refactor register symbols Vianney le Clément de Saint-Marcq
2015-03-28 12:40   ` Jonathan Cameron
2015-03-24 15:54 ` [PATCH v2 2/7] iio: mlx90614: Add symbols for accessible registers Vianney le Clément de Saint-Marcq
2015-03-24 15:54 ` [PATCH v2 3/7] iio: mlx90614: Support devices with dual IR sensor Vianney le Clément de Saint-Marcq
2015-03-28 12:44   ` Jonathan Cameron
2015-03-24 15:54 ` [PATCH v2 4/7] iio: core: Introduce IIO_CHAN_INFO_EMISSIVITY Vianney le Clément de Saint-Marcq
2015-03-24 16:02   ` Lars-Peter Clausen
2015-03-28 12:45     ` Jonathan Cameron
2015-03-24 15:54 ` [PATCH v2 5/7] iio: mlx90614: Add emissivity setting Vianney le Clément de Saint-Marcq
2015-03-28 12:46   ` Jonathan Cameron
     [not found] ` <1427212459-31585-1-git-send-email-vianney.leclement-buIOx9BAs4sybS5Ee8rs3A@public.gmane.org>
2015-03-24 15:54   ` [PATCH v2 6/7] iio: mlx90614: Add power management Vianney le Clément de Saint-Marcq
2015-03-24 15:54     ` Vianney le Clément de Saint-Marcq
     [not found]     ` <1427212459-31585-7-git-send-email-vianney.leclement-buIOx9BAs4sybS5Ee8rs3A@public.gmane.org>
2015-03-28 12:54       ` Jonathan Cameron
2015-03-28 12:54         ` Jonathan Cameron
2015-03-24 15:54 ` [PATCH v2 7/7] iio: mlx90614: Check for errors in read values Vianney le Clément de Saint-Marcq
2015-03-28 13:43   ` Jonathan Cameron

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.