[PATCH v3 0/6] iio:pressure:ms5611: fix and enhancements

[PATCH v3 0/6] iio:pressure:ms5611: fix and enhancements

[Gregor Boirie]

Changes since v2:
* remove non printable characters from commit messages

Gregor Boirie (3):
  iio:pressure:ms5611: fix ms5607 temp compensation
  iio:pressure:ms5611: oversampling rate support
  iio:pressure:ms5611: continuous sampling support

Grégor Boirie (3):
  iio:pressure:ms5611: use probed device name
  iio:pressure:ms5611: power regulator support
  iio:pressure:ms5611: DT bindings documentation

 .../devicetree/bindings/iio/pressure/ms5611.txt    |  19 ++
 drivers/iio/pressure/ms5611.h                      |  31 ++-
 drivers/iio/pressure/ms5611_core.c                 | 252 ++++++++++++++++++---
 drivers/iio/pressure/ms5611_i2c.c                  |  14 +-
 drivers/iio/pressure/ms5611_spi.c                  |  23 +-
 5 files changed, 284 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/ms5611.txt

-- 
2.1.4

[PATCH v3 1/6] iio:pressure:ms5611: fix ms5607 temp compensation

[Gregor Boirie]

Computation of sens2 was wrong and is fixed by this patch, sens2 should be:
    2 * (t - 2000)^2
See page 8 of ms5607 datasheet here:
    http://www.meas-spec.com/product/pressure/MS5607-02BA03.aspx

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
---
 drivers/iio/pressure/ms5611_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index c7885f0c..84ab8d2 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -136,17 +136,17 @@ static int ms5607_temp_and_pressure_compensate(struct ms5611_chip_info *chip_inf
 
 	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
 	if (t < 2000) {
-		s64 off2, sens2, t2;
+		s64 off2, sens2, t2, tmp;
 
 		t2 = (dt * dt) >> 31;
-		off2 = (61 * (t - 2000) * (t - 2000)) >> 4;
-		sens2 = off2 << 1;
+		tmp = (t - 2000) * (t - 2000);
+		off2 = (61 * tmp) >> 4;
+		sens2 = tmp << 1;
 
 		if (t < -1500) {
-			s64 tmp = (t + 1500) * (t + 1500);
-
+			tmp = (t + 1500) * (t + 1500);
 			off2 += 15 * tmp;
-			sens2 += (8 * tmp);
+			sens2 += 8 * tmp;
 		}
 
 		t -= t2;
-- 
2.1.4

[PATCH v3 2/6] iio:pressure:ms5611: use probed device name

[Gregor Boirie]

From: Grégor Boirie <gregor.boirie@parrot.com>

Use name of probed device instead of driver's one when registering device.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/ms5611.h      | 3 ++-
 drivers/iio/pressure/ms5611_core.c | 5 +++--
 drivers/iio/pressure/ms5611_i2c.c  | 2 +-
 drivers/iio/pressure/ms5611_spi.c  | 4 ++--
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
index 2d70dd6..8b08e4b 100644
--- a/drivers/iio/pressure/ms5611.h
+++ b/drivers/iio/pressure/ms5611.h
@@ -51,7 +51,8 @@ struct ms5611_state {
 	struct ms5611_chip_info *chip_info;
 };
 
-int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type);
+int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
+                 const char* name, int type);
 int ms5611_remove(struct iio_dev *indio_dev);
 
 #endif /* _MS5611_H */
diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index 84ab8d2..acd8e37 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -298,7 +298,8 @@ static int ms5611_init(struct iio_dev *indio_dev)
 	return ms5611_read_prom(indio_dev);
 }
 
-int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type)
+int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
+                 const char *name, int type)
 {
 	int ret;
 	struct ms5611_state *st = iio_priv(indio_dev);
@@ -306,7 +307,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type)
 	mutex_init(&st->lock);
 	st->chip_info = &chip_info_tbl[type];
 	indio_dev->dev.parent = dev;
-	indio_dev->name = dev->driver->name;
+	indio_dev->name = name;
 	indio_dev->info = &ms5611_info;
 	indio_dev->channels = ms5611_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ms5611_channels);
diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c
index 42706a8..ae67539 100644
--- a/drivers/iio/pressure/ms5611_i2c.c
+++ b/drivers/iio/pressure/ms5611_i2c.c
@@ -105,7 +105,7 @@ static int ms5611_i2c_probe(struct i2c_client *client,
 	st->read_adc_temp_and_pressure = ms5611_i2c_read_adc_temp_and_pressure;
 	st->client = client;
 
-	return ms5611_probe(indio_dev, &client->dev, id->driver_data);
+	return ms5611_probe(indio_dev, &client->dev, id->name, id->driver_data);
 }
 
 static int ms5611_i2c_remove(struct i2c_client *client)
diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
index c4bf4e8..5cc009e 100644
--- a/drivers/iio/pressure/ms5611_spi.c
+++ b/drivers/iio/pressure/ms5611_spi.c
@@ -105,8 +105,8 @@ static int ms5611_spi_probe(struct spi_device *spi)
 	st->read_adc_temp_and_pressure = ms5611_spi_read_adc_temp_and_pressure;
 	st->client = spi;
 
-	return ms5611_probe(indio_dev, &spi->dev,
-			    spi_get_device_id(spi)->driver_data);
+	return ms5611_probe(indio_dev, &spi->dev, spi_get_device_id(spi)->name,
+	                    spi_get_device_id(spi)->driver_data);
 }
 
 static int ms5611_spi_remove(struct spi_device *spi)
-- 
2.1.4

[PATCH v3 3/6] iio:pressure:ms5611: power regulator support

[Gregor Boirie]

From: Grégor Boirie <gregor.boirie@parrot.com>

Add support for an optional regulator which, if found into device-tree,
will power on device at probing time.
The regulator is declared into ms5611 DTS entry as a "vdd-supply" property.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/ms5611_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index acd8e37..992ad8d 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/iio/iio.h>
 #include <linux/delay.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
@@ -290,6 +291,18 @@ static const struct iio_info ms5611_info = {
 static int ms5611_init(struct iio_dev *indio_dev)
 {
 	int ret;
+	struct regulator *vdd = devm_regulator_get(indio_dev->dev.parent,
+	                                           "vdd");
+
+	/* Enable attached regulator if any. */
+	if (!IS_ERR(vdd)) {
+		ret = regulator_enable(vdd);
+		if (ret) {
+			dev_err(indio_dev->dev.parent,
+			        "failed to enable Vdd supply: %d\n", ret);
+			return ret;
+		}
+	}
 
 	ret = ms5611_reset(indio_dev);
 	if (ret < 0)
-- 
2.1.4

[PATCH v3 4/6] iio:pressure:ms5611: DT bindings documentation

[Gregor Boirie]

From: Grégor Boirie <gregor.boirie@parrot.com>

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 .../devicetree/bindings/iio/pressure/ms5611.txt       | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/ms5611.txt

diff --git a/Documentation/devicetree/bindings/iio/pressure/ms5611.txt b/Documentation/devicetree/bindings/iio/pressure/ms5611.txt
new file mode 100644
index 0000000..986415a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/pressure/ms5611.txt
@@ -0,0 +1,19 @@
+MEAS ms5611 family pressure sensors
+
+Pressure sensors from MEAS Switzerland with SPI and I2C bus interfaces.
+
+Required properties:
+- compatible: "ms5611" or "ms5607"
+- reg: the I2C or SPI address the device will respond to
+
+Optional properties:
+- vdd-supply: an optional regulator that needs to be on to provide VDD
+  power to the sensor.
+
+Example:
+
+ms5607@77 {
+	compatible = "ms5607";
+	reg = <0x77>;
+	vdd-supply = <&ldo_3v3_gnss>;
+};
-- 
2.1.4

[PATCH v3 5/6] iio:pressure:ms5611: oversampling rate support

[Gregor Boirie]

Add support for setting and retrieving OverSampling Rate independently for
each of the temperature and pressure channels. This allows userspace to
fine tune hardware sampling process according to the following tradeoffs :
* the higher the OSR, the finer the resolution ;
* the higher the OSR, the lower the noise ;
BUT:
* the higher the OSR, the larger the drift ;
* the higher the OSR, the longer the response time, i.e. less samples per
  unit of time.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/ms5611.h      |  20 ++++---
 drivers/iio/pressure/ms5611_core.c | 105 ++++++++++++++++++++++++++++++++++++-
 drivers/iio/pressure/ms5611_i2c.c  |  12 ++---
 drivers/iio/pressure/ms5611_spi.c  |  19 +++----
 4 files changed, 133 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
index 8b08e4b..b07897f 100644
--- a/drivers/iio/pressure/ms5611.h
+++ b/drivers/iio/pressure/ms5611.h
@@ -19,12 +19,6 @@
 #define MS5611_RESET			0x1e
 #define MS5611_READ_ADC			0x00
 #define MS5611_READ_PROM_WORD		0xA0
-#define MS5611_START_TEMP_CONV		0x58
-#define MS5611_START_PRESSURE_CONV	0x48
-
-#define MS5611_CONV_TIME_MIN		9040
-#define MS5611_CONV_TIME_MAX		10000
-
 #define MS5611_PROM_WORDS_NB		8
 
 enum {
@@ -39,10 +33,24 @@ struct ms5611_chip_info {
 					    s32 *temp, s32 *pressure);
 };
 
+/*
+ * OverSampling Rate descriptor.
+ * Warning: cmd MUST be kept aligned on a word boundary (see
+ * m5611_spi_read_adc_temp_and_pressure in ms5611_spi.c).
+ */
+struct ms5611_osr {
+	unsigned long conv_usec;
+	uint8_t cmd;
+	unsigned short rate;
+};
+
 struct ms5611_state {
 	void *client;
 	struct mutex lock;
 
+	const struct ms5611_osr *pressure_osr;
+	const struct ms5611_osr *temp_osr;
+
 	int (*reset)(struct device *dev);
 	int (*read_prom_word)(struct device *dev, int index, u16 *word);
 	int (*read_adc_temp_and_pressure)(struct device *dev,
diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index 992ad8d..06d453c 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -18,11 +18,44 @@
 #include <linux/delay.h>
 #include <linux/regulator/consumer.h>
 
+#include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 #include "ms5611.h"
 
+#define MS5611_INIT_OSR(_cmd, _conv_usec, _rate) \
+	{ .cmd = _cmd, .conv_usec = _conv_usec, .rate = _rate }
+
+static const struct ms5611_osr ms5611_avail_pressure_osr[] = {
+	MS5611_INIT_OSR(0x40, 600,  256),
+	MS5611_INIT_OSR(0x42, 1170, 512),
+	MS5611_INIT_OSR(0x44, 2280, 1024),
+	MS5611_INIT_OSR(0x46, 4540, 2048),
+	MS5611_INIT_OSR(0x48, 9040, 4096)
+};
+
+static const struct ms5611_osr ms5611_avail_temp_osr[] = {
+	MS5611_INIT_OSR(0x50, 600,  256),
+	MS5611_INIT_OSR(0x52, 1170, 512),
+	MS5611_INIT_OSR(0x54, 2280, 1024),
+	MS5611_INIT_OSR(0x56, 4540, 2048),
+	MS5611_INIT_OSR(0x58, 9040, 4096)
+};
+
+static const char ms5611_show_osr[] = "256 512 1024 2048 4096";
+
+static IIO_CONST_ATTR(oversampling_ratio_available, ms5611_show_osr);
+
+static struct attribute *ms5611_attributes[] = {
+	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ms5611_attribute_group = {
+	.attrs = ms5611_attributes,
+};
+
 static bool ms5611_prom_is_valid(u16 *prom, size_t len)
 {
 	int i, j;
@@ -239,11 +272,70 @@ static int ms5611_read_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		if (chan->type != IIO_TEMP && chan->type != IIO_PRESSURE)
+			break;
+		mutex_lock(&st->lock);
+		if (chan->type == IIO_TEMP)
+			*val = (int) st->temp_osr->rate;
+		else
+			*val = (int) st->pressure_osr->rate;
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
 	}
 
 	return -EINVAL;
 }
 
+static const struct ms5611_osr* ms5611_find_osr(int rate,
+                                                const struct ms5611_osr *osr,
+                                                size_t count)
+{
+	unsigned int r;
+
+	for (r = 0; r < count; r++)
+		if ((unsigned short) rate == osr[r].rate)
+			break;
+	if (r >= count)
+		return NULL;
+	return &osr[r];
+}
+
+static int ms5611_write_raw(struct iio_dev *indio_dev,
+                            struct iio_chan_spec const *chan,
+                            int val, int val2, long mask)
+{
+	struct ms5611_state *st = iio_priv(indio_dev);
+	const struct ms5611_osr *osr = NULL;
+
+	if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
+		return -EINVAL;
+
+	if (chan->type == IIO_TEMP)
+		osr = ms5611_find_osr(val, ms5611_avail_temp_osr,
+		                      ARRAY_SIZE(ms5611_avail_temp_osr));
+	else if (chan->type == IIO_PRESSURE)
+		osr = ms5611_find_osr(val, ms5611_avail_pressure_osr,
+		                      ARRAY_SIZE(ms5611_avail_pressure_osr));
+	if (!osr)
+		return -EINVAL;
+
+	mutex_lock(&st->lock);
+
+	if (iio_buffer_enabled(indio_dev)) {
+		mutex_unlock(&st->lock);
+		return -EBUSY;
+	}
+
+	if (chan->type == IIO_TEMP)
+		st->temp_osr = osr;
+	else
+		st->pressure_osr = osr;
+
+	mutex_unlock(&st->lock);
+	return 0;
+}
+
 static const unsigned long ms5611_scan_masks[] = {0x3, 0};
 
 static struct ms5611_chip_info chip_info_tbl[] = {
@@ -259,7 +351,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
 	{
 		.type = IIO_PRESSURE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
-			BIT(IIO_CHAN_INFO_SCALE),
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 		.scan_index = 0,
 		.scan_type = {
 			.sign = 's',
@@ -271,7 +364,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
 	{
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
-			BIT(IIO_CHAN_INFO_SCALE),
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 		.scan_index = 1,
 		.scan_type = {
 			.sign = 's',
@@ -285,6 +379,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
 
 static const struct iio_info ms5611_info = {
 	.read_raw = &ms5611_read_raw,
+	.write_raw = &ms5611_write_raw,
+	.attrs = &ms5611_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
@@ -319,6 +415,11 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 
 	mutex_init(&st->lock);
 	st->chip_info = &chip_info_tbl[type];
+	st->temp_osr =
+		&ms5611_avail_temp_osr[ARRAY_SIZE(ms5611_avail_temp_osr) - 1];
+	st->pressure_osr =
+		&ms5611_avail_pressure_osr[ARRAY_SIZE(ms5611_avail_pressure_osr)
+		                           - 1];
 	indio_dev->dev.parent = dev;
 	indio_dev->name = name;
 	indio_dev->info = &ms5611_info;
diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c
index ae67539..b482ca4 100644
--- a/drivers/iio/pressure/ms5611_i2c.c
+++ b/drivers/iio/pressure/ms5611_i2c.c
@@ -62,23 +62,23 @@ static int ms5611_i2c_read_adc_temp_and_pressure(struct device *dev,
 {
 	int ret;
 	struct ms5611_state *st = iio_priv(dev_to_iio_dev(dev));
+	const struct ms5611_osr *osr = st->temp_osr;
 
-	ret = i2c_smbus_write_byte(st->client, MS5611_START_TEMP_CONV);
+	ret = i2c_smbus_write_byte(st->client, osr->cmd);
 	if (ret < 0)
 		return ret;
 
-	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
-
+	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
 	ret = ms5611_i2c_read_adc(st, temp);
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_write_byte(st->client, MS5611_START_PRESSURE_CONV);
+	osr = st->pressure_osr;
+	ret = i2c_smbus_write_byte(st->client, osr->cmd);
 	if (ret < 0)
 		return ret;
 
-	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
-
+	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
 	return ms5611_i2c_read_adc(st, pressure);
 }
 
diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
index 5cc009e..aa6cf2d 100644
--- a/drivers/iio/pressure/ms5611_spi.c
+++ b/drivers/iio/pressure/ms5611_spi.c
@@ -55,28 +55,29 @@ static int ms5611_spi_read_adc(struct device *dev, s32 *val)
 static int ms5611_spi_read_adc_temp_and_pressure(struct device *dev,
 						 s32 *temp, s32 *pressure)
 {
-	u8 cmd;
 	int ret;
 	struct ms5611_state *st = iio_priv(dev_to_iio_dev(dev));
+	const struct ms5611_osr *osr = st->temp_osr;
 
-	cmd = MS5611_START_TEMP_CONV;
-	ret = spi_write_then_read(st->client, &cmd, 1, NULL, 0);
+	/*
+	 * Warning: &osr->cmd MUST be aligned on a word boundary since used as
+	 * 2nd argument (void*) of spi_write_then_read.
+	 */
+	ret = spi_write_then_read(st->client, &osr->cmd, 1, NULL, 0);
 	if (ret < 0)
 		return ret;
 
-	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
-
+	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
 	ret = ms5611_spi_read_adc(dev, temp);
 	if (ret < 0)
 		return ret;
 
-	cmd = MS5611_START_PRESSURE_CONV;
-	ret = spi_write_then_read(st->client, &cmd, 1, NULL, 0);
+	osr = st->pressure_osr;
+	ret = spi_write_then_read(st->client, &osr->cmd, 1, NULL, 0);
 	if (ret < 0)
 		return ret;
 
-	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
-
+	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
 	return ms5611_spi_read_adc(dev, pressure);
 }
 
-- 
2.1.4

[PATCH v3 6/6] iio:pressure:ms5611: continuous sampling support

[Gregor Boirie]

Implement INDIO_BUFFER_SOFTWARE mode to allow continuous sampling without
relying upon external trigger.
Sampling is carried out by a kthread which life cycle is bound to samples
buffering: it is created at postenable time and destroyed at predisable
time.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/ms5611.h      |   8 +++
 drivers/iio/pressure/ms5611_core.c | 117 ++++++++++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
index b07897f..27edd384 100644
--- a/drivers/iio/pressure/ms5611.h
+++ b/drivers/iio/pressure/ms5611.h
@@ -44,10 +44,17 @@ struct ms5611_osr {
 	unsigned short rate;
 };
 
+/*
+ * Number of s32 for a complete sample, i.e. s32 (pressure) + s32 (temp) +
+ * 2 * s32 (timestamp).
+ */
+#define MS5611_BUFFER_NR (4U)
+
 struct ms5611_state {
 	void *client;
 	struct mutex lock;
 
+	s32 buf[MS5611_BUFFER_NR];
 	const struct ms5611_osr *pressure_osr;
 	const struct ms5611_osr *temp_osr;
 
@@ -57,6 +64,7 @@ struct ms5611_state {
 					  s32 *temp, s32 *pressure);
 
 	struct ms5611_chip_info *chip_info;
+	struct task_struct *task;
 };
 
 int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index 06d453c..443a0ce 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -14,16 +14,22 @@
  */
 
 #include <linux/module.h>
-#include <linux/iio/iio.h>
 #include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/freezer.h>
 #include <linux/regulator/consumer.h>
-
+#include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 #include "ms5611.h"
 
+enum {
+	MS5611_PRESSURE = 0,
+	MS5611_TEMP = 1
+};
+
 #define MS5611_INIT_OSR(_cmd, _conv_usec, _rate) \
 	{ .cmd = _cmd, .conv_usec = _conv_usec, .rate = _rate }
 
@@ -210,25 +216,28 @@ static int ms5611_reset(struct iio_dev *indio_dev)
 	return 0;
 }
 
-static irqreturn_t ms5611_trigger_handler(int irq, void *p)
+static void ms5611_push_data(struct iio_dev *indio_dev,
+                             struct ms5611_state *state)
 {
-	struct iio_poll_func *pf = p;
-	struct iio_dev *indio_dev = pf->indio_dev;
-	struct ms5611_state *st = iio_priv(indio_dev);
-	s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */
 	int ret;
 
-	mutex_lock(&st->lock);
-	ret = ms5611_read_temp_and_pressure(indio_dev, &buf[1], &buf[0]);
-	mutex_unlock(&st->lock);
-	if (ret < 0)
-		goto err;
+	mutex_lock(&state->lock);
+	ret = ms5611_read_temp_and_pressure(indio_dev, &state->buf[MS5611_TEMP],
+	                                    &state->buf[MS5611_PRESSURE]);
+	mutex_unlock(&state->lock);
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+	if (!ret)
+		iio_push_to_buffers_with_timestamp(indio_dev, state->buf,
+		                                   iio_get_time_ns());
+}
 
-err:
-	iio_trigger_notify_done(indio_dev->trig);
+static irqreturn_t ms5611_trigger_handler(int irq, void *p)
+{
+	const struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
 
+	ms5611_push_data(indio_dev, iio_priv(indio_dev));
+	iio_trigger_notify_done(indio_dev->trig);
 	return IRQ_HANDLED;
 }
 
@@ -236,15 +245,24 @@ static int ms5611_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
 {
-	int ret;
+	int ret = 0;
 	s32 temp, pressure;
 	struct ms5611_state *st = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
 		mutex_lock(&st->lock);
-		ret = ms5611_read_temp_and_pressure(indio_dev,
-						    &temp, &pressure);
+		if (iio_buffer_enabled(indio_dev)) {
+			/*
+			 * Return "cached" data since sampling is ongoing in the
+			 * background.
+			 */
+			pressure = st->buf[MS5611_PRESSURE];
+			temp = st->buf[MS5611_TEMP];
+		}
+		else
+			ret = ms5611_read_temp_and_pressure(indio_dev, &temp,
+			                                    &pressure);
 		mutex_unlock(&st->lock);
 		if (ret < 0)
 			return ret;
@@ -384,6 +402,64 @@ static const struct iio_info ms5611_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static int ms5611d(void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct ms5611_state *st = iio_priv(indio_dev);
+
+	set_freezable();
+
+	do {
+		ms5611_push_data(indio_dev, st);
+	} while (likely(!kthread_freezable_should_stop(NULL)));
+
+	return 0;
+}
+
+static int ms5611_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ms5611_state *st = iio_priv(indio_dev);
+	int ret;
+
+	/*
+	 * Initialize temperature and pressure so that a client getting samples
+	 * through read_raw retrieves valid data when buffering has just been
+	 * enabled but first sampling round is not yet completed.
+	 */
+	mutex_lock(&st->lock);
+	ret = ms5611_read_temp_and_pressure(indio_dev, &st->buf[MS5611_TEMP],
+	                                    &st->buf[MS5611_PRESSURE]);
+	mutex_unlock(&st->lock);
+	if (ret < 0)
+		return ret;
+
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
+		return iio_triggered_buffer_postenable(indio_dev);
+
+	st->task = kthread_run(ms5611d, indio_dev, indio_dev->name);
+	if (unlikely(IS_ERR(st->task))) {
+		dev_err(&indio_dev->dev, "failed to create buffering task\n");
+		return PTR_ERR(st->task);
+	}
+
+	return 0;
+}
+
+static int ms5611_buffer_predisable(struct iio_dev *indio_dev)
+{
+
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
+		return iio_triggered_buffer_predisable(indio_dev);
+
+	kthread_stop(((struct ms5611_state*) iio_priv(indio_dev))->task);
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops ms5611_buffer_ops = {
+	.postenable = ms5611_buffer_postenable,
+	.predisable = ms5611_buffer_predisable,
+};
+
 static int ms5611_init(struct iio_dev *indio_dev)
 {
 	int ret;
@@ -425,7 +501,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 	indio_dev->info = &ms5611_info;
 	indio_dev->channels = ms5611_channels;
 	indio_dev->num_channels = ARRAY_SIZE(ms5611_channels);
-	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
 	indio_dev->available_scan_masks = ms5611_scan_masks;
 
 	ret = ms5611_init(indio_dev);
@@ -433,7 +509,8 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 		return ret;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 ms5611_trigger_handler, NULL);
+	                                 ms5611_trigger_handler,
+	                                 &ms5611_buffer_ops);
 	if (ret < 0) {
 		dev_err(dev, "iio triggered buffer setup failed\n");
 		return ret;
-- 
2.1.4

Re: [PATCH v3 6/6] iio:pressure:ms5611: continuous sampling support

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> Implement INDIO_BUFFER_SOFTWARE mode to allow continuous sampling without
> relying upon external trigger.
> Sampling is carried out by a kthread which life cycle is bound to samples
> buffering: it is created at postenable time and destroyed at predisable
> time.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
Hi Gregor,

I'm having a few issues getting my head around what is happening here,
and in particular if there is a more generic way of doing it.

What is the benefit of this approach over using a high resolution timer trigger?
(I think I understand this - but an explanation in the patch description that
makes this clear would make for a stronger patch submission).

As I read this we are effectively spinning as fast as the device can sample.

I'd prefer a trigger that does the same thing and can then be used for
all devices.  It would fire, then the notification that all devices hanging
off it were done would be enough to make it refire.  Might have to limit it
to chained interrupt handlers however... (this part is slow, but some aren't
- doing this on a ADC on a SoC could be amusing for starters).

Rest of the series looks good (and less controversial :) but I would like
to let them sit for a few more days for possible reviews.

Jonathan

> ---
>  drivers/iio/pressure/ms5611.h      |   8 +++
>  drivers/iio/pressure/ms5611_core.c | 117 ++++++++++++++++++++++++++++++-------
>  2 files changed, 105 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
> index b07897f..27edd384 100644
> --- a/drivers/iio/pressure/ms5611.h
> +++ b/drivers/iio/pressure/ms5611.h
> @@ -44,10 +44,17 @@ struct ms5611_osr {
>  	unsigned short rate;
>  };
>  
> +/*
> + * Number of s32 for a complete sample, i.e. s32 (pressure) + s32 (temp) +
> + * 2 * s32 (timestamp).
> + */
> +#define MS5611_BUFFER_NR (4U)
> +
>  struct ms5611_state {
>  	void *client;
>  	struct mutex lock;
>  
> +	s32 buf[MS5611_BUFFER_NR];
>  	const struct ms5611_osr *pressure_osr;
>  	const struct ms5611_osr *temp_osr;
>  
> @@ -57,6 +64,7 @@ struct ms5611_state {
>  					  s32 *temp, s32 *pressure);
>  
>  	struct ms5611_chip_info *chip_info;
> +	struct task_struct *task;
>  };
>  
>  int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index 06d453c..443a0ce 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -14,16 +14,22 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/iio/iio.h>
>  #include <linux/delay.h>
> +#include <linux/kthread.h>
> +#include <linux/freezer.h>
>  #include <linux/regulator/consumer.h>
> -
> +#include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include "ms5611.h"
>  
> +enum {
> +	MS5611_PRESSURE = 0,
> +	MS5611_TEMP = 1
> +};
> +
>  #define MS5611_INIT_OSR(_cmd, _conv_usec, _rate) \
>  	{ .cmd = _cmd, .conv_usec = _conv_usec, .rate = _rate }
>  
> @@ -210,25 +216,28 @@ static int ms5611_reset(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> -static irqreturn_t ms5611_trigger_handler(int irq, void *p)
> +static void ms5611_push_data(struct iio_dev *indio_dev,
> +                             struct ms5611_state *state)
>  {
> -	struct iio_poll_func *pf = p;
> -	struct iio_dev *indio_dev = pf->indio_dev;
> -	struct ms5611_state *st = iio_priv(indio_dev);
> -	s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */
>  	int ret;
>  
> -	mutex_lock(&st->lock);
> -	ret = ms5611_read_temp_and_pressure(indio_dev, &buf[1], &buf[0]);
> -	mutex_unlock(&st->lock);
> -	if (ret < 0)
> -		goto err;
> +	mutex_lock(&state->lock);
> +	ret = ms5611_read_temp_and_pressure(indio_dev, &state->buf[MS5611_TEMP],
> +	                                    &state->buf[MS5611_PRESSURE]);
> +	mutex_unlock(&state->lock);
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +	if (!ret)
> +		iio_push_to_buffers_with_timestamp(indio_dev, state->buf,
> +		                                   iio_get_time_ns());
> +}
>  
> -err:
> -	iio_trigger_notify_done(indio_dev->trig);
> +static irqreturn_t ms5611_trigger_handler(int irq, void *p)
> +{
> +	const struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
>  
> +	ms5611_push_data(indio_dev, iio_priv(indio_dev));
> +	iio_trigger_notify_done(indio_dev->trig);
>  	return IRQ_HANDLED;
>  }
>  
> @@ -236,15 +245,24 @@ static int ms5611_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
>  {
> -	int ret;
> +	int ret = 0;
>  	s32 temp, pressure;
>  	struct ms5611_state *st = iio_priv(indio_dev);
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_PROCESSED:
>  		mutex_lock(&st->lock);
> -		ret = ms5611_read_temp_and_pressure(indio_dev,
> -						    &temp, &pressure);
> +		if (iio_buffer_enabled(indio_dev)) {
> +			/*
> +			 * Return "cached" data since sampling is ongoing in the
> +			 * background.
> +			 */
> +			pressure = st->buf[MS5611_PRESSURE];
> +			temp = st->buf[MS5611_TEMP];
> +		}
> +		else
> +			ret = ms5611_read_temp_and_pressure(indio_dev, &temp,
> +			                                    &pressure);
>  		mutex_unlock(&st->lock);
>  		if (ret < 0)
>  			return ret;
> @@ -384,6 +402,64 @@ static const struct iio_info ms5611_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static int ms5611d(void *data)
> +{
> +	struct iio_dev *indio_dev = data;
> +	struct ms5611_state *st = iio_priv(indio_dev);
> +
> +	set_freezable();
> +
> +	do {
> +		ms5611_push_data(indio_dev, st);
> +	} while (likely(!kthread_freezable_should_stop(NULL)));
> +
> +	return 0;
> +}
> +
> +static int ms5611_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ms5611_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	/*
> +	 * Initialize temperature and pressure so that a client getting samples
> +	 * through read_raw retrieves valid data when buffering has just been
> +	 * enabled but first sampling round is not yet completed.
> +	 */
> +	mutex_lock(&st->lock);
> +	ret = ms5611_read_temp_and_pressure(indio_dev, &st->buf[MS5611_TEMP],
> +	                                    &st->buf[MS5611_PRESSURE]);
> +	mutex_unlock(&st->lock);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> +		return iio_triggered_buffer_postenable(indio_dev);
> +
> +	st->task = kthread_run(ms5611d, indio_dev, indio_dev->name);
> +	if (unlikely(IS_ERR(st->task))) {
> +		dev_err(&indio_dev->dev, "failed to create buffering task\n");
> +		return PTR_ERR(st->task);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ms5611_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +
> +	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> +		return iio_triggered_buffer_predisable(indio_dev);
> +
> +	kthread_stop(((struct ms5611_state*) iio_priv(indio_dev))->task);
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops ms5611_buffer_ops = {
> +	.postenable = ms5611_buffer_postenable,
> +	.predisable = ms5611_buffer_predisable,
> +};
> +
>  static int ms5611_init(struct iio_dev *indio_dev)
>  {
>  	int ret;
> @@ -425,7 +501,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>  	indio_dev->info = &ms5611_info;
>  	indio_dev->channels = ms5611_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ms5611_channels);
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
>  	indio_dev->available_scan_masks = ms5611_scan_masks;
>  
>  	ret = ms5611_init(indio_dev);
> @@ -433,7 +509,8 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>  		return ret;
>  
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> -					 ms5611_trigger_handler, NULL);
> +	                                 ms5611_trigger_handler,
> +	                                 &ms5611_buffer_ops);
>  	if (ret < 0) {
>  		dev_err(dev, "iio triggered buffer setup failed\n");
>  		return ret;
> 

Re: [PATCH v3 6/6] iio:pressure:ms5611: continuous sampling support

[Gregor Boirie]

On 02/17/2016 10:09 PM, Jonathan Cameron wrote:
> On 17/02/16 17:52, Gregor Boirie wrote:
>> Implement INDIO_BUFFER_SOFTWARE mode to allow continuous sampling without
>> relying upon external trigger.
>> Sampling is carried out by a kthread which life cycle is bound to samples
>> buffering: it is created at postenable time and destroyed at predisable
>> time.
>>
>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> Hi Gregor,
>
> I'm having a few issues getting my head around what is happening here,
> and in particular if there is a more generic way of doing it.
I'm sure there is. It is a good candidate for this but I thought 
applying this
to a single driver would make a first proof of concept.
>
> What is the benefit of this approach over using a high resolution timer trigger?
> (I think I understand this - but an explanation in the patch description that
> makes this clear would make for a stronger patch submission).
>
> As I read this we are effectively spinning as fast as the device can sample.
This is it.
>
> I'd prefer a trigger that does the same thing and can then be used for
> all devices.  It would fire, then the notification that all devices hanging
> off it were done would be enough to make it refire.  Might have to limit it
> to chained interrupt handlers however... (this part is slow, but some aren't
> - doing this on a ADC on a SoC could be amusing for starters).
The whole thing relates to our particular use case which I could sum up 
as following:
* poll multiple IIO devices sitting on I2C buses every 5 ms ;
* for each 5 ms time slot, get every samples available for each device ;
* assign each samples the right time slot ;
* each device must deliver as much samples as possible according to their
   respective HW sampling settings (oversampling rate, frequency, 
resolution,
   acceptable noise...).

The thing is, in our HW setup :
1. many devices compete for bus access ;
2. once serialized, all bus transactions for 1 time slot take much 
longer than 5 ms ;
3. most of the time, a transaction spends a lot of time waiting for 
device conversion
    results (see calls to usleep_range).

Hence, we need to interleave transactions to devices sitting on the same 
bus instance
to fit in a single 5 ms time slot... This patch is a naive way to 
implement this using
kthread since neither Linux I2C, neither IIO APIs allow asynchronous 
processing.

A single IIO trigger would not be of much help because it would 
serialize transactions.
We would need almost as many "refiring" triggers as the number of devices.
I thought a kthread would be much simpler than the burden of a virtual 
irq chip
machinery.

I hope this is clear enough. I would be glad to read your suggestions if 
you think of
something more appropriate.

> Rest of the series looks good (and less controversial :) but I would like
> to let them sit for a few more days for possible reviews.
There is no urge : I prefer ensuring nothing important is missing or 
ill-designed.

And by the way, many thanks for your valuable time. Regards,
Grégor.
> Jonathan
>
>> ---
>>   drivers/iio/pressure/ms5611.h      |   8 +++
>>   drivers/iio/pressure/ms5611_core.c | 117 ++++++++++++++++++++++++++++++-------
>>   2 files changed, 105 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
>> index b07897f..27edd384 100644
>> --- a/drivers/iio/pressure/ms5611.h
>> +++ b/drivers/iio/pressure/ms5611.h
>> @@ -44,10 +44,17 @@ struct ms5611_osr {
>>   	unsigned short rate;
>>   };
>>   
>> +/*
>> + * Number of s32 for a complete sample, i.e. s32 (pressure) + s32 (temp) +
>> + * 2 * s32 (timestamp).
>> + */
>> +#define MS5611_BUFFER_NR (4U)
>> +
>>   struct ms5611_state {
>>   	void *client;
>>   	struct mutex lock;
>>   
>> +	s32 buf[MS5611_BUFFER_NR];
>>   	const struct ms5611_osr *pressure_osr;
>>   	const struct ms5611_osr *temp_osr;
>>   
>> @@ -57,6 +64,7 @@ struct ms5611_state {
>>   					  s32 *temp, s32 *pressure);
>>   
>>   	struct ms5611_chip_info *chip_info;
>> +	struct task_struct *task;
>>   };
>>   
>>   int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
>> index 06d453c..443a0ce 100644
>> --- a/drivers/iio/pressure/ms5611_core.c
>> +++ b/drivers/iio/pressure/ms5611_core.c
>> @@ -14,16 +14,22 @@
>>    */
>>   
>>   #include <linux/module.h>
>> -#include <linux/iio/iio.h>
>>   #include <linux/delay.h>
>> +#include <linux/kthread.h>
>> +#include <linux/freezer.h>
>>   #include <linux/regulator/consumer.h>
>> -
>> +#include <linux/iio/iio.h>
>>   #include <linux/iio/sysfs.h>
>>   #include <linux/iio/buffer.h>
>>   #include <linux/iio/triggered_buffer.h>
>>   #include <linux/iio/trigger_consumer.h>
>>   #include "ms5611.h"
>>   
>> +enum {
>> +	MS5611_PRESSURE = 0,
>> +	MS5611_TEMP = 1
>> +};
>> +
>>   #define MS5611_INIT_OSR(_cmd, _conv_usec, _rate) \
>>   	{ .cmd = _cmd, .conv_usec = _conv_usec, .rate = _rate }
>>   
>> @@ -210,25 +216,28 @@ static int ms5611_reset(struct iio_dev *indio_dev)
>>   	return 0;
>>   }
>>   
>> -static irqreturn_t ms5611_trigger_handler(int irq, void *p)
>> +static void ms5611_push_data(struct iio_dev *indio_dev,
>> +                             struct ms5611_state *state)
>>   {
>> -	struct iio_poll_func *pf = p;
>> -	struct iio_dev *indio_dev = pf->indio_dev;
>> -	struct ms5611_state *st = iio_priv(indio_dev);
>> -	s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */
>>   	int ret;
>>   
>> -	mutex_lock(&st->lock);
>> -	ret = ms5611_read_temp_and_pressure(indio_dev, &buf[1], &buf[0]);
>> -	mutex_unlock(&st->lock);
>> -	if (ret < 0)
>> -		goto err;
>> +	mutex_lock(&state->lock);
>> +	ret = ms5611_read_temp_and_pressure(indio_dev, &state->buf[MS5611_TEMP],
>> +	                                    &state->buf[MS5611_PRESSURE]);
>> +	mutex_unlock(&state->lock);
>>   
>> -	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>> +	if (!ret)
>> +		iio_push_to_buffers_with_timestamp(indio_dev, state->buf,
>> +		                                   iio_get_time_ns());
>> +}
>>   
>> -err:
>> -	iio_trigger_notify_done(indio_dev->trig);
>> +static irqreturn_t ms5611_trigger_handler(int irq, void *p)
>> +{
>> +	const struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>>   
>> +	ms5611_push_data(indio_dev, iio_priv(indio_dev));
>> +	iio_trigger_notify_done(indio_dev->trig);
>>   	return IRQ_HANDLED;
>>   }
>>   
>> @@ -236,15 +245,24 @@ static int ms5611_read_raw(struct iio_dev *indio_dev,
>>   			   struct iio_chan_spec const *chan,
>>   			   int *val, int *val2, long mask)
>>   {
>> -	int ret;
>> +	int ret = 0;
>>   	s32 temp, pressure;
>>   	struct ms5611_state *st = iio_priv(indio_dev);
>>   
>>   	switch (mask) {
>>   	case IIO_CHAN_INFO_PROCESSED:
>>   		mutex_lock(&st->lock);
>> -		ret = ms5611_read_temp_and_pressure(indio_dev,
>> -						    &temp, &pressure);
>> +		if (iio_buffer_enabled(indio_dev)) {
>> +			/*
>> +			 * Return "cached" data since sampling is ongoing in the
>> +			 * background.
>> +			 */
>> +			pressure = st->buf[MS5611_PRESSURE];
>> +			temp = st->buf[MS5611_TEMP];
>> +		}
>> +		else
>> +			ret = ms5611_read_temp_and_pressure(indio_dev, &temp,
>> +			                                    &pressure);
>>   		mutex_unlock(&st->lock);
>>   		if (ret < 0)
>>   			return ret;
>> @@ -384,6 +402,64 @@ static const struct iio_info ms5611_info = {
>>   	.driver_module = THIS_MODULE,
>>   };
>>   
>> +static int ms5611d(void *data)
>> +{
>> +	struct iio_dev *indio_dev = data;
>> +	struct ms5611_state *st = iio_priv(indio_dev);
>> +
>> +	set_freezable();
>> +
>> +	do {
>> +		ms5611_push_data(indio_dev, st);
>> +	} while (likely(!kthread_freezable_should_stop(NULL)));
>> +
>> +	return 0;
>> +}
>> +
>> +static int ms5611_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> +	struct ms5611_state *st = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	/*
>> +	 * Initialize temperature and pressure so that a client getting samples
>> +	 * through read_raw retrieves valid data when buffering has just been
>> +	 * enabled but first sampling round is not yet completed.
>> +	 */
>> +	mutex_lock(&st->lock);
>> +	ret = ms5611_read_temp_and_pressure(indio_dev, &st->buf[MS5611_TEMP],
>> +	                                    &st->buf[MS5611_PRESSURE]);
>> +	mutex_unlock(&st->lock);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
>> +		return iio_triggered_buffer_postenable(indio_dev);
>> +
>> +	st->task = kthread_run(ms5611d, indio_dev, indio_dev->name);
>> +	if (unlikely(IS_ERR(st->task))) {
>> +		dev_err(&indio_dev->dev, "failed to create buffering task\n");
>> +		return PTR_ERR(st->task);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int ms5611_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> +
>> +	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
>> +		return iio_triggered_buffer_predisable(indio_dev);
>> +
>> +	kthread_stop(((struct ms5611_state*) iio_priv(indio_dev))->task);
>> +	return 0;
>> +}
>> +
>> +static const struct iio_buffer_setup_ops ms5611_buffer_ops = {
>> +	.postenable = ms5611_buffer_postenable,
>> +	.predisable = ms5611_buffer_predisable,
>> +};
>> +
>>   static int ms5611_init(struct iio_dev *indio_dev)
>>   {
>>   	int ret;
>> @@ -425,7 +501,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>>   	indio_dev->info = &ms5611_info;
>>   	indio_dev->channels = ms5611_channels;
>>   	indio_dev->num_channels = ARRAY_SIZE(ms5611_channels);
>> -	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
>>   	indio_dev->available_scan_masks = ms5611_scan_masks;
>>   
>>   	ret = ms5611_init(indio_dev);
>> @@ -433,7 +509,8 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>>   		return ret;
>>   
>>   	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> -					 ms5611_trigger_handler, NULL);
>> +	                                 ms5611_trigger_handler,
>> +	                                 &ms5611_buffer_ops);
>>   	if (ret < 0) {
>>   		dev_err(dev, "iio triggered buffer setup failed\n");
>>   		return ret;
>>

Re: [PATCH v3 1/6] iio:pressure:ms5611: fix ms5607 temp compensation

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> Computation of sens2 was wrong and is fixed by this patch, sens2 should be:
>     2 * (t - 2000)^2
> See page 8 of ms5607 datasheet here:
>     http://www.meas-spec.com/product/pressure/MS5607-02BA03.aspx
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Hmm. We are late enough in the cycle now that I'm going to take this through
the togreg branch rather than fixes-togreg.  Partly this is to make my life
easier wrt to noise vs the rest of this series.

Applied to the togreg branch of iio.git - initially pushed out as testing
for the auto builders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/ms5611_core.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index c7885f0c..84ab8d2 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -136,17 +136,17 @@ static int ms5607_temp_and_pressure_compensate(struct ms5611_chip_info *chip_inf
>  
>  	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
>  	if (t < 2000) {
> -		s64 off2, sens2, t2;
> +		s64 off2, sens2, t2, tmp;
>  
>  		t2 = (dt * dt) >> 31;
> -		off2 = (61 * (t - 2000) * (t - 2000)) >> 4;
> -		sens2 = off2 << 1;
> +		tmp = (t - 2000) * (t - 2000);
> +		off2 = (61 * tmp) >> 4;
> +		sens2 = tmp << 1;
>  
>  		if (t < -1500) {
> -			s64 tmp = (t + 1500) * (t + 1500);
> -
> +			tmp = (t + 1500) * (t + 1500);
>  			off2 += 15 * tmp;
> -			sens2 += (8 * tmp);
> +			sens2 += 8 * tmp;
>  		}
>  
>  		t -= t2;
> 

Re: [PATCH v3 2/6] iio:pressure:ms5611: use probed device name

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> From: Grégor Boirie <gregor.boirie@parrot.com>
> 
> Use name of probed device instead of driver's one when registering device.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
*fingers crossed no one is relying on this bit of ABI*
Should definitely have always been as you have it now.

Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/ms5611.h      | 3 ++-
>  drivers/iio/pressure/ms5611_core.c | 5 +++--
>  drivers/iio/pressure/ms5611_i2c.c  | 2 +-
>  drivers/iio/pressure/ms5611_spi.c  | 4 ++--
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
> index 2d70dd6..8b08e4b 100644
> --- a/drivers/iio/pressure/ms5611.h
> +++ b/drivers/iio/pressure/ms5611.h
> @@ -51,7 +51,8 @@ struct ms5611_state {
>  	struct ms5611_chip_info *chip_info;
>  };
>  
> -int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type);
> +int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
> +                 const char* name, int type);
>  int ms5611_remove(struct iio_dev *indio_dev);
>  
>  #endif /* _MS5611_H */
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index 84ab8d2..acd8e37 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -298,7 +298,8 @@ static int ms5611_init(struct iio_dev *indio_dev)
>  	return ms5611_read_prom(indio_dev);
>  }
>  
> -int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type)
> +int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
> +                 const char *name, int type)
>  {
>  	int ret;
>  	struct ms5611_state *st = iio_priv(indio_dev);
> @@ -306,7 +307,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type)
>  	mutex_init(&st->lock);
>  	st->chip_info = &chip_info_tbl[type];
>  	indio_dev->dev.parent = dev;
> -	indio_dev->name = dev->driver->name;
> +	indio_dev->name = name;
>  	indio_dev->info = &ms5611_info;
>  	indio_dev->channels = ms5611_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(ms5611_channels);
> diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c
> index 42706a8..ae67539 100644
> --- a/drivers/iio/pressure/ms5611_i2c.c
> +++ b/drivers/iio/pressure/ms5611_i2c.c
> @@ -105,7 +105,7 @@ static int ms5611_i2c_probe(struct i2c_client *client,
>  	st->read_adc_temp_and_pressure = ms5611_i2c_read_adc_temp_and_pressure;
>  	st->client = client;
>  
> -	return ms5611_probe(indio_dev, &client->dev, id->driver_data);
> +	return ms5611_probe(indio_dev, &client->dev, id->name, id->driver_data);
>  }
>  
>  static int ms5611_i2c_remove(struct i2c_client *client)
> diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> index c4bf4e8..5cc009e 100644
> --- a/drivers/iio/pressure/ms5611_spi.c
> +++ b/drivers/iio/pressure/ms5611_spi.c
> @@ -105,8 +105,8 @@ static int ms5611_spi_probe(struct spi_device *spi)
>  	st->read_adc_temp_and_pressure = ms5611_spi_read_adc_temp_and_pressure;
>  	st->client = spi;
>  
> -	return ms5611_probe(indio_dev, &spi->dev,
> -			    spi_get_device_id(spi)->driver_data);
> +	return ms5611_probe(indio_dev, &spi->dev, spi_get_device_id(spi)->name,
> +	                    spi_get_device_id(spi)->driver_data);
>  }
>  
>  static int ms5611_spi_remove(struct spi_device *spi)
> 

Re: [PATCH v3 3/6] iio:pressure:ms5611: power regulator support

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> From: Grégor Boirie <gregor.boirie@parrot.com>
> 
> Add support for an optional regulator which, if found into device-tree,
> will power on device at probing time.
> The regulator is declared into ms5611 DTS entry as a "vdd-supply" property.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
This usually only gets done when it first matters to someone. Guessing it
matters to you!

Applied to the togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/ms5611_core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index acd8e37..992ad8d 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -16,6 +16,7 @@
>  #include <linux/module.h>
>  #include <linux/iio/iio.h>
>  #include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
> @@ -290,6 +291,18 @@ static const struct iio_info ms5611_info = {
>  static int ms5611_init(struct iio_dev *indio_dev)
>  {
>  	int ret;
> +	struct regulator *vdd = devm_regulator_get(indio_dev->dev.parent,
> +	                                           "vdd");
> +
> +	/* Enable attached regulator if any. */
> +	if (!IS_ERR(vdd)) {
> +		ret = regulator_enable(vdd);
> +		if (ret) {
> +			dev_err(indio_dev->dev.parent,
> +			        "failed to enable Vdd supply: %d\n", ret);
> +			return ret;
> +		}
> +	}
>  
>  	ret = ms5611_reset(indio_dev);
>  	if (ret < 0)
> 

Re: [PATCH v3 4/6] iio:pressure:ms5611: DT bindings documentation

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> From: Grégor Boirie <gregor.boirie@parrot.com>
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
One comment inline - though a quick look shows this idiom is common
which isn't great.

Also ideal would be to add the device_tree id tables and have the
documented binding include the manufacturer prefix.
> ---
>  .../devicetree/bindings/iio/pressure/ms5611.txt       | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/ms5611.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/ms5611.txt b/Documentation/devicetree/bindings/iio/pressure/ms5611.txt
> new file mode 100644
> index 0000000..986415a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/pressure/ms5611.txt
> @@ -0,0 +1,19 @@
> +MEAS ms5611 family pressure sensors
> +
> +Pressure sensors from MEAS Switzerland with SPI and I2C bus interfaces.
> +
> +Required properties:
> +- compatible: "ms5611" or "ms5607"
Preferred to be prefixed with the manufacturer though that requires adding
tables to the various drivers.  Ideally please do so and update this to include those
values as the preferred choices.


> +- reg: the I2C or SPI address the device will respond to
SPI buses don't have an address (they do have a chip select though).

> +
> +Optional properties:
> +- vdd-supply: an optional regulator that needs to be on to provide VDD
> +  power to the sensor.
> +
> +Example:
> +
> +ms5607@77 {
> +	compatible = "ms5607";
> +	reg = <0x77>;
> +	vdd-supply = <&ldo_3v3_gnss>;
> +};
> 

Re: [PATCH v3 5/6] iio:pressure:ms5611: oversampling rate support

[Jonathan Cameron]

On 17/02/16 17:52, Gregor Boirie wrote:
> Add support for setting and retrieving OverSampling Rate independently for
> each of the temperature and pressure channels. This allows userspace to
> fine tune hardware sampling process according to the following tradeoffs :
> * the higher the OSR, the finer the resolution ;
> * the higher the OSR, the lower the noise ;
> BUT:
> * the higher the OSR, the larger the drift ;
> * the higher the OSR, the longer the response time, i.e. less samples per
>   unit of time.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
Whilst this looks fine to me - it's complex enough that I'd like Tomasz to take
a look at it before I apply (ideally anyway!)
> ---
>  drivers/iio/pressure/ms5611.h      |  20 ++++---
>  drivers/iio/pressure/ms5611_core.c | 105 ++++++++++++++++++++++++++++++++++++-
>  drivers/iio/pressure/ms5611_i2c.c  |  12 ++---
>  drivers/iio/pressure/ms5611_spi.c  |  19 +++----
>  4 files changed, 133 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
> index 8b08e4b..b07897f 100644
> --- a/drivers/iio/pressure/ms5611.h
> +++ b/drivers/iio/pressure/ms5611.h
> @@ -19,12 +19,6 @@
>  #define MS5611_RESET			0x1e
>  #define MS5611_READ_ADC			0x00
>  #define MS5611_READ_PROM_WORD		0xA0
> -#define MS5611_START_TEMP_CONV		0x58
> -#define MS5611_START_PRESSURE_CONV	0x48
> -
> -#define MS5611_CONV_TIME_MIN		9040
> -#define MS5611_CONV_TIME_MAX		10000
> -
>  #define MS5611_PROM_WORDS_NB		8
>  
>  enum {
> @@ -39,10 +33,24 @@ struct ms5611_chip_info {
>  					    s32 *temp, s32 *pressure);
>  };
>  
> +/*
> + * OverSampling Rate descriptor.
> + * Warning: cmd MUST be kept aligned on a word boundary (see
> + * m5611_spi_read_adc_temp_and_pressure in ms5611_spi.c).
> + */
> +struct ms5611_osr {
> +	unsigned long conv_usec;
> +	uint8_t cmd;
> +	unsigned short rate;
> +};
> +
>  struct ms5611_state {
>  	void *client;
>  	struct mutex lock;
>  
> +	const struct ms5611_osr *pressure_osr;
> +	const struct ms5611_osr *temp_osr;
> +
>  	int (*reset)(struct device *dev);
>  	int (*read_prom_word)(struct device *dev, int index, u16 *word);
>  	int (*read_adc_temp_and_pressure)(struct device *dev,
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index 992ad8d..06d453c 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -18,11 +18,44 @@
>  #include <linux/delay.h>
>  #include <linux/regulator/consumer.h>
>  
> +#include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include "ms5611.h"
>  
> +#define MS5611_INIT_OSR(_cmd, _conv_usec, _rate) \
> +	{ .cmd = _cmd, .conv_usec = _conv_usec, .rate = _rate }
> +
> +static const struct ms5611_osr ms5611_avail_pressure_osr[] = {
> +	MS5611_INIT_OSR(0x40, 600,  256),
> +	MS5611_INIT_OSR(0x42, 1170, 512),
> +	MS5611_INIT_OSR(0x44, 2280, 1024),
> +	MS5611_INIT_OSR(0x46, 4540, 2048),
> +	MS5611_INIT_OSR(0x48, 9040, 4096)
> +};
> +
> +static const struct ms5611_osr ms5611_avail_temp_osr[] = {
> +	MS5611_INIT_OSR(0x50, 600,  256),
> +	MS5611_INIT_OSR(0x52, 1170, 512),
> +	MS5611_INIT_OSR(0x54, 2280, 1024),
> +	MS5611_INIT_OSR(0x56, 4540, 2048),
> +	MS5611_INIT_OSR(0x58, 9040, 4096)
> +};
> +
> +static const char ms5611_show_osr[] = "256 512 1024 2048 4096";
> +
> +static IIO_CONST_ATTR(oversampling_ratio_available, ms5611_show_osr);
> +
> +static struct attribute *ms5611_attributes[] = {
> +	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group ms5611_attribute_group = {
> +	.attrs = ms5611_attributes,
> +};
> +
>  static bool ms5611_prom_is_valid(u16 *prom, size_t len)
>  {
>  	int i, j;
> @@ -239,11 +272,70 @@ static int ms5611_read_raw(struct iio_dev *indio_dev,
>  		default:
>  			return -EINVAL;
>  		}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		if (chan->type != IIO_TEMP && chan->type != IIO_PRESSURE)
> +			break;
> +		mutex_lock(&st->lock);
> +		if (chan->type == IIO_TEMP)
> +			*val = (int) st->temp_osr->rate;
> +		else
> +			*val = (int) st->pressure_osr->rate;
> +		mutex_unlock(&st->lock);
> +		return IIO_VAL_INT;
>  	}
>  
>  	return -EINVAL;
>  }
>  
> +static const struct ms5611_osr* ms5611_find_osr(int rate,
> +                                                const struct ms5611_osr *osr,
> +                                                size_t count)
> +{
> +	unsigned int r;
> +
> +	for (r = 0; r < count; r++)
> +		if ((unsigned short) rate == osr[r].rate)
> +			break;
> +	if (r >= count)
> +		return NULL;
> +	return &osr[r];
> +}
> +
> +static int ms5611_write_raw(struct iio_dev *indio_dev,
> +                            struct iio_chan_spec const *chan,
> +                            int val, int val2, long mask)
> +{
> +	struct ms5611_state *st = iio_priv(indio_dev);
> +	const struct ms5611_osr *osr = NULL;
> +
> +	if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
> +		return -EINVAL;
> +
> +	if (chan->type == IIO_TEMP)
> +		osr = ms5611_find_osr(val, ms5611_avail_temp_osr,
> +		                      ARRAY_SIZE(ms5611_avail_temp_osr));
> +	else if (chan->type == IIO_PRESSURE)
> +		osr = ms5611_find_osr(val, ms5611_avail_pressure_osr,
> +		                      ARRAY_SIZE(ms5611_avail_pressure_osr));
> +	if (!osr)
> +		return -EINVAL;
> +
> +	mutex_lock(&st->lock);
> +
> +	if (iio_buffer_enabled(indio_dev)) {
> +		mutex_unlock(&st->lock);
> +		return -EBUSY;
> +	}
> +
> +	if (chan->type == IIO_TEMP)
> +		st->temp_osr = osr;
> +	else
> +		st->pressure_osr = osr;
> +
> +	mutex_unlock(&st->lock);
> +	return 0;
> +}
> +
>  static const unsigned long ms5611_scan_masks[] = {0x3, 0};
>  
>  static struct ms5611_chip_info chip_info_tbl[] = {
> @@ -259,7 +351,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
>  	{
>  		.type = IIO_PRESSURE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -			BIT(IIO_CHAN_INFO_SCALE),
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
>  		.scan_index = 0,
>  		.scan_type = {
>  			.sign = 's',
> @@ -271,7 +364,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
>  	{
>  		.type = IIO_TEMP,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -			BIT(IIO_CHAN_INFO_SCALE),
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
>  		.scan_index = 1,
>  		.scan_type = {
>  			.sign = 's',
> @@ -285,6 +379,8 @@ static const struct iio_chan_spec ms5611_channels[] = {
>  
>  static const struct iio_info ms5611_info = {
>  	.read_raw = &ms5611_read_raw,
> +	.write_raw = &ms5611_write_raw,
> +	.attrs = &ms5611_attribute_group,
>  	.driver_module = THIS_MODULE,
>  };
>  
> @@ -319,6 +415,11 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>  
>  	mutex_init(&st->lock);
>  	st->chip_info = &chip_info_tbl[type];
> +	st->temp_osr =
> +		&ms5611_avail_temp_osr[ARRAY_SIZE(ms5611_avail_temp_osr) - 1];
> +	st->pressure_osr =
> +		&ms5611_avail_pressure_osr[ARRAY_SIZE(ms5611_avail_pressure_osr)
> +		                           - 1];
>  	indio_dev->dev.parent = dev;
>  	indio_dev->name = name;
>  	indio_dev->info = &ms5611_info;
> diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c
> index ae67539..b482ca4 100644
> --- a/drivers/iio/pressure/ms5611_i2c.c
> +++ b/drivers/iio/pressure/ms5611_i2c.c
> @@ -62,23 +62,23 @@ static int ms5611_i2c_read_adc_temp_and_pressure(struct device *dev,
>  {
>  	int ret;
>  	struct ms5611_state *st = iio_priv(dev_to_iio_dev(dev));
> +	const struct ms5611_osr *osr = st->temp_osr;
>  
> -	ret = i2c_smbus_write_byte(st->client, MS5611_START_TEMP_CONV);
> +	ret = i2c_smbus_write_byte(st->client, osr->cmd);
>  	if (ret < 0)
>  		return ret;
>  
> -	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
> -
> +	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
>  	ret = ms5611_i2c_read_adc(st, temp);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = i2c_smbus_write_byte(st->client, MS5611_START_PRESSURE_CONV);
> +	osr = st->pressure_osr;
> +	ret = i2c_smbus_write_byte(st->client, osr->cmd);
>  	if (ret < 0)
>  		return ret;
>  
> -	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
> -
> +	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
>  	return ms5611_i2c_read_adc(st, pressure);
>  }
>  
> diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> index 5cc009e..aa6cf2d 100644
> --- a/drivers/iio/pressure/ms5611_spi.c
> +++ b/drivers/iio/pressure/ms5611_spi.c
> @@ -55,28 +55,29 @@ static int ms5611_spi_read_adc(struct device *dev, s32 *val)
>  static int ms5611_spi_read_adc_temp_and_pressure(struct device *dev,
>  						 s32 *temp, s32 *pressure)
>  {
> -	u8 cmd;
>  	int ret;
>  	struct ms5611_state *st = iio_priv(dev_to_iio_dev(dev));
> +	const struct ms5611_osr *osr = st->temp_osr;
>  
> -	cmd = MS5611_START_TEMP_CONV;
> -	ret = spi_write_then_read(st->client, &cmd, 1, NULL, 0);
> +	/*
> +	 * Warning: &osr->cmd MUST be aligned on a word boundary since used as
> +	 * 2nd argument (void*) of spi_write_then_read.
> +	 */
> +	ret = spi_write_then_read(st->client, &osr->cmd, 1, NULL, 0);
>  	if (ret < 0)
>  		return ret;
>  
> -	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
> -
> +	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
>  	ret = ms5611_spi_read_adc(dev, temp);
>  	if (ret < 0)
>  		return ret;
>  
> -	cmd = MS5611_START_PRESSURE_CONV;
> -	ret = spi_write_then_read(st->client, &cmd, 1, NULL, 0);
> +	osr = st->pressure_osr;
> +	ret = spi_write_then_read(st->client, &osr->cmd, 1, NULL, 0);
>  	if (ret < 0)
>  		return ret;
>  
> -	usleep_range(MS5611_CONV_TIME_MIN, MS5611_CONV_TIME_MAX);
> -
> +	usleep_range(osr->conv_usec, osr->conv_usec + (osr->conv_usec / 10UL));
>  	return ms5611_spi_read_adc(dev, pressure);
>  }
>  
>