iio:pressure:ms5611: fix and enhancements

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 <[email protected]>

Acked-by Lucas De Marchi <[email protected]>

Acked-by Peter Meerwald-Stadler <[email protected]>

---
 drivers/iio/pressure/ms5611_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/iio/pressure/ms5611_core.c +6 -6

index c7885f0c..84ab8d2 100644
@@ -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;

From: Grégor Boirie <[email protected]>

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

Signed-off-by Gregor Boirie <[email protected]>

---
 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(-)

drivers/iio/pressure/ms5611.h +2 -1

index 2d70dd6..8b08e4b 100644
@@ -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 */

drivers/iio/pressure/ms5611_core.c +3 -2

index 84ab8d2..acd8e37 100644
@@ -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);

drivers/iio/pressure/ms5611_i2c.c +1 -1

index 42706a8..ae67539 100644
@@ -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)

drivers/iio/pressure/ms5611_spi.c +2 -2

index c4bf4e8..5cc009e 100644
@@ -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)

From: Grégor Boirie <[email protected]>

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 <[email protected]>

---
 drivers/iio/pressure/ms5611_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/iio/pressure/ms5611_core.c +13 -0

index acd8e37..992ad8d 100644
@@ -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)

From: Grégor Boirie <[email protected]>

Signed-off-by Gregor Boirie <[email protected]>

---
 .../devicetree/bindings/iio/pressure/ms5611.txt       | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/ms5611.txt

Documentation/devicetree/bindings/iio/pressure/ms5611.txt new file +0 -0

new file mode 100644
index 0000000..986415a

Documentation/devicetree/bindings/iio/pressure/ms5611.txt new file +12 -0

+
+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>;
+};

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 <[email protected]>

---
 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(-)

drivers/iio/pressure/ms5611.h +14 -6

index 8b08e4b..b07897f 100644
@@ -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,

drivers/iio/pressure/ms5611_core.c +103 -2

index 992ad8d..06d453c 100644
@@ -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;

drivers/iio/pressure/ms5611_i2c.c +6 -6

index ae67539..b482ca4 100644
@@ -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);
 }
 

drivers/iio/pressure/ms5611_spi.c +10 -9

index 5cc009e..aa6cf2d 100644
@@ -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);
 }
 

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 <[email protected]>

---
 drivers/iio/pressure/ms5611.h      |   8 +++
 drivers/iio/pressure/ms5611_core.c | 117 ++++++++++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 20 deletions(-)

drivers/iio/pressure/ms5611.h +8 -0

index b07897f..27edd384 100644
@@ -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,

drivers/iio/pressure/ms5611_core.c +97 -20

index 06d453c..443a0ce 100644
@@ -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;

Quoted 6 lines · 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.

Quoted 1 line

Signed-off-by Gregor Boirie <[email protected]>

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

Quoted 219 lines

---
  drivers/iio/pressure/ms5611.h      |   8 +++
  drivers/iio/pressure/ms5611_core.c | 117 ++++++++++++++++++++++++++++++-------
  2 files changed, 105 insertions(+), 20 deletions(-)

drivers/iio/pressure/ms5611.h +8 -0

index b07897f..27edd384 100644
@@ -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,

drivers/iio/pressure/ms5611_core.c +97 -20

index 06d453c..443a0ce 100644
@@ -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;