[PATCH v7 00/10] iio: light: rpr0521

[PATCH v7 00/10] iio: light: rpr0521

[Mikko Koivunen]

Patchset v6->v7 changes:
  0001-0008 not reposted as they went upstream already
  0009 Shared irq support fixed, whitespace fixed

Patchset v5->v6 changes:
  0001-0008 not reposted as they went upstream already
  0009 Shared irq support, default trigger removed, devm_
  0010 patch added: Changed iio_device_register to devm_

Patchset v4->v5 changes:
  cover letter added
  0002 (void) removed

Patchset v3->v4 changes:
  0006 indent fix
  0009 on/off to preenable/postdisable

Patchset v2->v3 changes:
  More commits, numbers change, order change
  Changes listed in individual commits

Mikko Koivunen (10):
  iio: light: rpr0521 disable sensor -bugfix
  iio: light: rpr0521 poweroff for probe fails
  iio: light: rpr0521 on-off sequence change for CONFIG_PM
  iio: light: rpr0521 magic number to sizeof() on value read
  iio: light: rpr0521 whitespace fixes
  iio: light: rpr0521 sample_frequency read/write
  iio: light: rpr0521 proximity offset read/write
  iio: light: rpr0521 channel numbers reordered
  iio: light: rpr0521 triggered buffer
  iio: light: rpr0521 iio_device_register to devm_

 drivers/iio/light/rpr0521.c |  643 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 588 insertions(+), 55 deletions(-)

-- 
1.7.9.5

[PATCH v7 09/10] iio: light: rpr0521 triggered buffer

[Mikko Koivunen]

Set up and use triggered buffer if there is irq defined for device in
device tree. Trigger producer triggers from rpr0521 drdy interrupt line.
Trigger consumer reads rpr0521 data to scan buffer.
Depends on previous commits of _scale and _offset.

Signed-off-by: Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com>
---
Patch v2->v3 changes:
.shift = 0 removed
reordered functions to remove forward declarations
whitespace changes
refactored some update_bits->write, no "err = err || *"-pattern
refactored trigger_consumer_handler
reordered _probe() and _remove()
added int clear on poweroff()
checkpatch.pl OK
Tested on LeMaker HiKey with AOSP7.1 kernel 4.4.
Builds ok with torvalds/linux feb 27.

Patch v3->v4 changes:
Moved sensor on/off commands to buffer preenable and postdisable
 - Since drdy happens only on measurement data ready and register writes
   are cached, the trigger producer doesn't care of suspend/resume state.
available_scan_masks added
indent fix
checkpatch.pl OK
Lightly re-tested on LeMaker HiKey with AOSP7.1 kernel 4.4.
Builds ok with torvalds/linux feb 27.

Patch v5->v6 changes:
Changed base from kernel 4.4 to 4.9
 - Using iio_device_claim_direct_mode()
 - Using iio_trigger_using_own()
 - Changed trigger/buffer to devm_
Added shared irq support
 - divided trigger producer to top/bottom
 - divided trigger consumer to top/bottom
 - Using irq_timestamp when available
Removed default trigger

Patch v6->v7 changes:
trigger_poll to trigger_poll_chained
 + some timestamp saving logic changes due that
2 whitespace fixes

 drivers/iio/light/rpr0521.c |  336 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 329 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index 9d0c2e8..a6efa126 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -9,7 +9,7 @@
  *
  * IIO driver for RPR-0521RS (7-bit I2C slave address 0x38).
  *
- * TODO: illuminance channel, buffer
+ * TODO: illuminance channel
  */
 
 #include <linux/module.h>
@@ -20,6 +20,10 @@
 #include <linux/acpi.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 #include <linux/iio/sysfs.h>
 #include <linux/pm_runtime.h>
 
@@ -30,6 +34,7 @@
 #define RPR0521_REG_PXS_DATA		0x44 /* 16-bit, little endian */
 #define RPR0521_REG_ALS_DATA0		0x46 /* 16-bit, little endian */
 #define RPR0521_REG_ALS_DATA1		0x48 /* 16-bit, little endian */
+#define RPR0521_REG_INTERRUPT		0x4A
 #define RPR0521_REG_PS_OFFSET_LSB	0x53
 #define RPR0521_REG_ID			0x92
 
@@ -42,16 +47,31 @@
 #define RPR0521_ALS_DATA1_GAIN_SHIFT	2
 #define RPR0521_PXS_GAIN_MASK		GENMASK(5, 4)
 #define RPR0521_PXS_GAIN_SHIFT		4
+#define RPR0521_PXS_PERSISTENCE_MASK	GENMASK(3, 0)
+#define RPR0521_INTERRUPT_INT_TRIG_PS_MASK	BIT(0)
+#define RPR0521_INTERRUPT_INT_TRIG_ALS_MASK	BIT(1)
+#define RPR0521_INTERRUPT_INT_REASSERT_MASK	BIT(3)
+#define RPR0521_INTERRUPT_ALS_INT_STATUS_MASK	BIT(6)
+#define RPR0521_INTERRUPT_PS_INT_STATUS_MASK	BIT(7)
 
 #define RPR0521_MODE_ALS_ENABLE		BIT(7)
 #define RPR0521_MODE_ALS_DISABLE	0x00
 #define RPR0521_MODE_PXS_ENABLE		BIT(6)
 #define RPR0521_MODE_PXS_DISABLE	0x00
+#define RPR0521_PXS_PERSISTENCE_DRDY	0x00
+
+#define RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE	BIT(0)
+#define RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE	0x00
+#define RPR0521_INTERRUPT_INT_TRIG_ALS_ENABLE	BIT(1)
+#define RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE	0x00
+#define RPR0521_INTERRUPT_INT_REASSERT_ENABLE	BIT(3)
+#define RPR0521_INTERRUPT_INT_REASSERT_DISABLE	0x00
 
 #define RPR0521_MANUFACT_ID		0xE0
 #define RPR0521_DEFAULT_MEAS_TIME	0x06 /* ALS - 100ms, PXS - 100ms */
 
 #define RPR0521_DRV_NAME		"RPR0521"
+#define RPR0521_IRQ_NAME		"rpr0521_event"
 #define RPR0521_REGMAP_NAME		"rpr0521_regmap"
 
 #define RPR0521_SLEEP_DELAY_MS	2000
@@ -167,6 +187,9 @@ struct rpr0521_data {
 	bool als_dev_en;
 	bool pxs_dev_en;
 
+	struct iio_trigger *drdy_trigger0;
+	s64 irq_timestamp;
+
 	/* optimize runtime pm ops - enable/disable device only if needed */
 	bool als_ps_need_en;
 	bool pxs_ps_need_en;
@@ -196,6 +219,19 @@ static const struct attribute_group rpr0521_attribute_group = {
 	.attrs = rpr0521_attributes,
 };
 
+/* Order of the channel data in buffer */
+enum rpr0521_scan_index_order {
+	RPR0521_CHAN_INDEX_PXS,
+	RPR0521_CHAN_INDEX_BOTH,
+	RPR0521_CHAN_INDEX_IR,
+};
+
+static const unsigned long rpr0521_available_scan_masks[] = {
+	BIT(RPR0521_CHAN_INDEX_PXS) | BIT(RPR0521_CHAN_INDEX_BOTH) |
+	BIT(RPR0521_CHAN_INDEX_IR),
+	0
+};
+
 static const struct iio_chan_spec rpr0521_channels[] = {
 	{
 		.type = IIO_PROXIMITY,
@@ -204,6 +240,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
 			BIT(IIO_CHAN_INFO_OFFSET) |
 			BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.scan_index = RPR0521_CHAN_INDEX_PXS,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
 	},
 	{
 		.type = IIO_INTENSITY,
@@ -213,6 +256,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 			BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.scan_index = RPR0521_CHAN_INDEX_BOTH,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
 	},
 	{
 		.type = IIO_INTENSITY,
@@ -222,6 +272,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 			BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.scan_index = RPR0521_CHAN_INDEX_IR,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
 	},
 };
 
@@ -330,6 +387,198 @@ static int rpr0521_set_power_state(struct rpr0521_data *data, bool on,
 	return 0;
 }
 
+/* Interrupt register tells if this sensor caused the interrupt or not. */
+static inline bool rpr0521_is_triggered(struct rpr0521_data *data)
+{
+	int ret;
+	int reg;
+
+	ret = regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &reg);
+	if (ret < 0)
+		return false;   /* Reg read failed. */
+	if (reg &
+	    (RPR0521_INTERRUPT_ALS_INT_STATUS_MASK |
+	    RPR0521_INTERRUPT_PS_INT_STATUS_MASK))
+		return true;
+	else
+		return false;   /* Int not from this sensor. */
+}
+
+/* IRQ to trigger handler */
+static irqreturn_t rpr0521_drdy_irq_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct rpr0521_data *data = iio_priv(indio_dev);
+
+	data->irq_timestamp = iio_get_time_ns(indio_dev);
+	/*
+	 * We need to wake the thread to read the interrupt reg. It
+	 * is not possible to do that here because regmap_read takes a
+	 * mutex.
+	 */
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t rpr0521_drdy_irq_thread(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct rpr0521_data *data = iio_priv(indio_dev);
+
+	if (rpr0521_is_triggered(data)) {
+		iio_trigger_poll_chained(data->drdy_trigger0);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static irqreturn_t rpr0521_trigger_consumer_store_time(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+
+	/* Other trigger polls store time here. */
+	if (!iio_trigger_using_own(indio_dev))
+		pf->timestamp = iio_get_time_ns(indio_dev);
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct rpr0521_data *data = iio_priv(indio_dev);
+	int err;
+
+	u8 buffer[16]; /* 3 16-bit channels + padding + ts */
+
+	/* Use irq timestamp when reasonable. */
+	if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) {
+		pf->timestamp = data->irq_timestamp;
+		data->irq_timestamp = 0;
+	}
+	/* Other chained trigger polls get timestamp only here. */
+	if (!pf->timestamp)
+		pf->timestamp = iio_get_time_ns(indio_dev);
+
+	err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA,
+		&buffer,
+		(3 * 2) + 1);	/* 3 * 16-bit + (discarded) int clear reg. */
+	if (!err)
+		iio_push_to_buffers_with_timestamp(indio_dev,
+						   buffer, pf->timestamp);
+	else
+		dev_err(&data->client->dev,
+			"Trigger consumer can't read from sensor.\n");
+	pf->timestamp = 0;
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int rpr0521_write_int_enable(struct rpr0521_data *data)
+{
+	int err;
+
+	/* Interrupt after each measurement */
+	err = regmap_update_bits(data->regmap, RPR0521_REG_PXS_CTRL,
+		RPR0521_PXS_PERSISTENCE_MASK,
+		RPR0521_PXS_PERSISTENCE_DRDY);
+	if (err) {
+		dev_err(&data->client->dev, "PS control reg write fail.\n");
+		return -EBUSY;
+		}
+
+	/* Ignore latch and mode because of drdy */
+	err = regmap_write(data->regmap, RPR0521_REG_INTERRUPT,
+		RPR0521_INTERRUPT_INT_REASSERT_DISABLE |
+		RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |
+		RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE
+		);
+	if (err) {
+		dev_err(&data->client->dev, "Interrupt setup write fail.\n");
+		return -EBUSY;
+		}
+
+	return 0;
+}
+
+static int rpr0521_write_int_disable(struct rpr0521_data *data)
+{
+	/* Don't care of clearing mode, assert and latch. */
+	return regmap_write(data->regmap, RPR0521_REG_INTERRUPT,
+				RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |
+				RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE
+				);
+}
+
+/*
+ * Trigger producer enable / disable. Note that there will be trigs only when
+ * measurement data is ready to be read.
+ */
+static int rpr0521_pxs_drdy_set_state(struct iio_trigger *trigger,
+	bool enable_drdy)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trigger);
+	struct rpr0521_data *data = iio_priv(indio_dev);
+	int err;
+
+	if (enable_drdy)
+		err = rpr0521_write_int_enable(data);
+	else
+		err = rpr0521_write_int_disable(data);
+	if (err)
+		dev_err(&data->client->dev, "rpr0521_pxs_drdy_set_state failed\n");
+
+	return err;
+}
+
+static const struct iio_trigger_ops rpr0521_trigger_ops = {
+	.set_trigger_state = rpr0521_pxs_drdy_set_state,
+	.owner = THIS_MODULE,
+	};
+
+
+static int rpr0521_buffer_preenable(struct iio_dev *indio_dev)
+{
+	int err;
+	struct rpr0521_data *data = iio_priv(indio_dev);
+
+	mutex_lock(&data->lock);
+	err = rpr0521_set_power_state(data, true,
+		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));
+	mutex_unlock(&data->lock);
+	if (err)
+		dev_err(&data->client->dev, "_buffer_preenable fail\n");
+
+	return err;
+}
+
+static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	int err;
+	struct rpr0521_data *data = iio_priv(indio_dev);
+
+	mutex_lock(&data->lock);
+	err = rpr0521_set_power_state(data, false,
+		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));
+	mutex_unlock(&data->lock);
+	if (err)
+		dev_err(&data->client->dev, "_buffer_postdisable fail\n");
+
+	return err;
+}
+
+static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops = {
+	.preenable = rpr0521_buffer_preenable,
+	.postenable = iio_triggered_buffer_postenable,
+	.predisable = iio_triggered_buffer_predisable,
+	.postdisable = rpr0521_buffer_postdisable,
+};
+
 static int rpr0521_get_gain(struct rpr0521_data *data, int chan,
 			    int *val, int *val2)
 {
@@ -473,6 +722,7 @@ static int rpr0521_read_raw(struct iio_dev *indio_dev,
 {
 	struct rpr0521_data *data = iio_priv(indio_dev);
 	int ret;
+	int busy;
 	u8 device_mask;
 	__le16 raw_data;
 
@@ -481,26 +731,30 @@ static int rpr0521_read_raw(struct iio_dev *indio_dev,
 		if (chan->type != IIO_INTENSITY && chan->type != IIO_PROXIMITY)
 			return -EINVAL;
 
+		busy = iio_device_claim_direct_mode(indio_dev);
+		if (busy)
+			return -EBUSY;
+
 		device_mask = rpr0521_data_reg[chan->address].device_mask;
 
 		mutex_lock(&data->lock);
 		ret = rpr0521_set_power_state(data, true, device_mask);
-		if (ret < 0) {
-			mutex_unlock(&data->lock);
-			return ret;
-		}
+		if (ret < 0)
+			goto rpr0521_read_raw_out;
 
 		ret = regmap_bulk_read(data->regmap,
 				       rpr0521_data_reg[chan->address].address,
 				       &raw_data, sizeof(raw_data));
 		if (ret < 0) {
 			rpr0521_set_power_state(data, false, device_mask);
-			mutex_unlock(&data->lock);
-			return ret;
+			goto rpr0521_read_raw_out;
 		}
 
 		ret = rpr0521_set_power_state(data, false, device_mask);
+
+rpr0521_read_raw_out:
 		mutex_unlock(&data->lock);
+		iio_device_release_direct_mode(indio_dev);
 		if (ret < 0)
 			return ret;
 
@@ -617,12 +871,15 @@ static int rpr0521_init(struct rpr0521_data *data)
 		return ret;
 #endif
 
+	data->irq_timestamp = 0;
+
 	return 0;
 }
 
 static int rpr0521_poweroff(struct rpr0521_data *data)
 {
 	int ret;
+	int tmp;
 
 	ret = regmap_update_bits(data->regmap, RPR0521_REG_MODE_CTRL,
 				 RPR0521_MODE_ALS_MASK |
@@ -635,6 +892,16 @@ static int rpr0521_poweroff(struct rpr0521_data *data)
 	data->als_dev_en = false;
 	data->pxs_dev_en = false;
 
+	/*
+	 * Int pin keeps state after power off. Set pin to high impedance
+	 * mode to prevent power drain.
+	 */
+	ret = regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &tmp);
+	if (ret) {
+		dev_err(&data->client->dev, "Failed to reset int pin.\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -707,6 +974,61 @@ static int rpr0521_probe(struct i2c_client *client,
 	pm_runtime_set_autosuspend_delay(&client->dev, RPR0521_SLEEP_DELAY_MS);
 	pm_runtime_use_autosuspend(&client->dev);
 
+	/*
+	 * If sensor write/read is needed in _probe after _use_autosuspend,
+	 * sensor needs to be _resumed first using rpr0521_set_power_state().
+	 */
+
+	/* IRQ to trigger setup */
+	if (client->irq) {
+		/* Trigger0 producer setup */
+		data->drdy_trigger0 = devm_iio_trigger_alloc(
+			indio_dev->dev.parent,
+			"%s-dev%d", indio_dev->name, indio_dev->id);
+		if (!data->drdy_trigger0) {
+			ret = -ENOMEM;
+			goto err_pm_disable;
+		}
+		data->drdy_trigger0->dev.parent = indio_dev->dev.parent;
+		data->drdy_trigger0->ops = &rpr0521_trigger_ops;
+		indio_dev->available_scan_masks = rpr0521_available_scan_masks;
+		iio_trigger_set_drvdata(data->drdy_trigger0, indio_dev);
+
+		/* Ties irq to trigger producer handler. */
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+			rpr0521_drdy_irq_handler, rpr0521_drdy_irq_thread,
+			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			RPR0521_IRQ_NAME, indio_dev);
+		if (ret < 0) {
+			dev_err(&client->dev, "request irq %d for trigger0 failed\n",
+				client->irq);
+			goto err_pm_disable;
+			}
+
+		ret = devm_iio_trigger_register(indio_dev->dev.parent,
+						data->drdy_trigger0);
+		if (ret) {
+			dev_err(&client->dev, "iio trigger register failed\n");
+			goto err_pm_disable;
+		}
+
+		/*
+		 * Now whole pipe from physical interrupt (irq defined by
+		 * devicetree to device) to trigger0 output is set up.
+		 */
+
+		/* Trigger consumer setup */
+		ret = devm_iio_triggered_buffer_setup(indio_dev->dev.parent,
+			indio_dev,
+			rpr0521_trigger_consumer_store_time,
+			rpr0521_trigger_consumer_handler,
+			&rpr0521_buffer_setup_ops);
+		if (ret < 0) {
+			dev_err(&client->dev, "iio triggered buffer setup failed\n");
+			goto err_pm_disable;
+		}
+	}
+
 	ret = iio_device_register(indio_dev);
 	if (ret)
 		goto err_pm_disable;
-- 
1.7.9.5

Re: [PATCH v7 09/10] iio: light: rpr0521 triggered buffer

[Jonathan Cameron]

On Mon,  3 Jul 2017 15:50:13 +0300
Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com> wrote:

> Set up and use triggered buffer if there is irq defined for device in
> device tree. Trigger producer triggers from rpr0521 drdy interrupt line.
> Trigger consumer reads rpr0521 data to scan buffer.
> Depends on previous commits of _scale and _offset.
> 
> Signed-off-by: Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com>
one side comment inline, but so tiny I'm not even going to bother changing
it whilst applying.

Very nice.

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

Thanks,

Jonathan
> ---
> Patch v2->v3 changes:
> .shift = 0 removed
> reordered functions to remove forward declarations
> whitespace changes
> refactored some update_bits->write, no "err = err || *"-pattern
> refactored trigger_consumer_handler
> reordered _probe() and _remove()
> added int clear on poweroff()
> checkpatch.pl OK
> Tested on LeMaker HiKey with AOSP7.1 kernel 4.4.
> Builds ok with torvalds/linux feb 27.
> 
> Patch v3->v4 changes:
> Moved sensor on/off commands to buffer preenable and postdisable
>  - Since drdy happens only on measurement data ready and register writes
>    are cached, the trigger producer doesn't care of suspend/resume state.
> available_scan_masks added
> indent fix
> checkpatch.pl OK
> Lightly re-tested on LeMaker HiKey with AOSP7.1 kernel 4.4.
> Builds ok with torvalds/linux feb 27.
> 
> Patch v5->v6 changes:
> Changed base from kernel 4.4 to 4.9
>  - Using iio_device_claim_direct_mode()
>  - Using iio_trigger_using_own()
>  - Changed trigger/buffer to devm_
> Added shared irq support
>  - divided trigger producer to top/bottom
>  - divided trigger consumer to top/bottom
>  - Using irq_timestamp when available
> Removed default trigger
> 
> Patch v6->v7 changes:
> trigger_poll to trigger_poll_chained
>  + some timestamp saving logic changes due that
> 2 whitespace fixes
> 
>  drivers/iio/light/rpr0521.c |  336 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 329 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
> index 9d0c2e8..a6efa126 100644
> --- a/drivers/iio/light/rpr0521.c
> +++ b/drivers/iio/light/rpr0521.c
> @@ -9,7 +9,7 @@
>   *
>   * IIO driver for RPR-0521RS (7-bit I2C slave address 0x38).
>   *
> - * TODO: illuminance channel, buffer
> + * TODO: illuminance channel
>   */
>  
>  #include <linux/module.h>
> @@ -20,6 +20,10 @@
>  #include <linux/acpi.h>
>  
>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/pm_runtime.h>
>  
> @@ -30,6 +34,7 @@
>  #define RPR0521_REG_PXS_DATA		0x44 /* 16-bit, little endian */
>  #define RPR0521_REG_ALS_DATA0		0x46 /* 16-bit, little endian */
>  #define RPR0521_REG_ALS_DATA1		0x48 /* 16-bit, little endian */
> +#define RPR0521_REG_INTERRUPT		0x4A
>  #define RPR0521_REG_PS_OFFSET_LSB	0x53
>  #define RPR0521_REG_ID			0x92
>  
> @@ -42,16 +47,31 @@
>  #define RPR0521_ALS_DATA1_GAIN_SHIFT	2
>  #define RPR0521_PXS_GAIN_MASK		GENMASK(5, 4)
>  #define RPR0521_PXS_GAIN_SHIFT		4
> +#define RPR0521_PXS_PERSISTENCE_MASK	GENMASK(3, 0)
> +#define RPR0521_INTERRUPT_INT_TRIG_PS_MASK	BIT(0)
> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_MASK	BIT(1)
> +#define RPR0521_INTERRUPT_INT_REASSERT_MASK	BIT(3)
> +#define RPR0521_INTERRUPT_ALS_INT_STATUS_MASK	BIT(6)
> +#define RPR0521_INTERRUPT_PS_INT_STATUS_MASK	BIT(7)
>  
>  #define RPR0521_MODE_ALS_ENABLE		BIT(7)
>  #define RPR0521_MODE_ALS_DISABLE	0x00
>  #define RPR0521_MODE_PXS_ENABLE		BIT(6)
>  #define RPR0521_MODE_PXS_DISABLE	0x00
> +#define RPR0521_PXS_PERSISTENCE_DRDY	0x00
> +
> +#define RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE	BIT(0)
> +#define RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE	0x00
> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_ENABLE	BIT(1)
> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE	0x00
> +#define RPR0521_INTERRUPT_INT_REASSERT_ENABLE	BIT(3)
> +#define RPR0521_INTERRUPT_INT_REASSERT_DISABLE	0x00
>  
>  #define RPR0521_MANUFACT_ID		0xE0
>  #define RPR0521_DEFAULT_MEAS_TIME	0x06 /* ALS - 100ms, PXS - 100ms */
>  
>  #define RPR0521_DRV_NAME		"RPR0521"
> +#define RPR0521_IRQ_NAME		"rpr0521_event"
>  #define RPR0521_REGMAP_NAME		"rpr0521_regmap"
>  
>  #define RPR0521_SLEEP_DELAY_MS	2000
> @@ -167,6 +187,9 @@ struct rpr0521_data {
>  	bool als_dev_en;
>  	bool pxs_dev_en;
>  
> +	struct iio_trigger *drdy_trigger0;
> +	s64 irq_timestamp;
> +
>  	/* optimize runtime pm ops - enable/disable device only if needed */
>  	bool als_ps_need_en;
>  	bool pxs_ps_need_en;
> @@ -196,6 +219,19 @@ static const struct attribute_group rpr0521_attribute_group = {
>  	.attrs = rpr0521_attributes,
>  };
>  
> +/* Order of the channel data in buffer */
> +enum rpr0521_scan_index_order {
> +	RPR0521_CHAN_INDEX_PXS,
> +	RPR0521_CHAN_INDEX_BOTH,
> +	RPR0521_CHAN_INDEX_IR,
> +};
> +
> +static const unsigned long rpr0521_available_scan_masks[] = {
> +	BIT(RPR0521_CHAN_INDEX_PXS) | BIT(RPR0521_CHAN_INDEX_BOTH) |
> +	BIT(RPR0521_CHAN_INDEX_IR),
> +	0
> +};
> +
>  static const struct iio_chan_spec rpr0521_channels[] = {
>  	{
>  		.type = IIO_PROXIMITY,
> @@ -204,6 +240,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
>  			BIT(IIO_CHAN_INFO_OFFSET) |
>  			BIT(IIO_CHAN_INFO_SCALE),
>  		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.scan_index = RPR0521_CHAN_INDEX_PXS,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
>  	},
>  	{
>  		.type = IIO_INTENSITY,
> @@ -213,6 +256,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  			BIT(IIO_CHAN_INFO_SCALE),
>  		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.scan_index = RPR0521_CHAN_INDEX_BOTH,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
>  	},
>  	{
>  		.type = IIO_INTENSITY,
> @@ -222,6 +272,13 @@ static const struct iio_chan_spec rpr0521_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  			BIT(IIO_CHAN_INFO_SCALE),
>  		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.scan_index = RPR0521_CHAN_INDEX_IR,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 16,
> +			.storagebits = 16,
> +			.endianness = IIO_LE,
> +		},
>  	},
>  };
>  
> @@ -330,6 +387,198 @@ static int rpr0521_set_power_state(struct rpr0521_data *data, bool on,
>  	return 0;
>  }
>  
> +/* Interrupt register tells if this sensor caused the interrupt or not. */
> +static inline bool rpr0521_is_triggered(struct rpr0521_data *data)
> +{
> +	int ret;
> +	int reg;
> +
> +	ret = regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &reg);
> +	if (ret < 0)
> +		return false;   /* Reg read failed. */
> +	if (reg &
> +	    (RPR0521_INTERRUPT_ALS_INT_STATUS_MASK |
> +	    RPR0521_INTERRUPT_PS_INT_STATUS_MASK))
> +		return true;
> +	else
> +		return false;   /* Int not from this sensor. */
> +}
> +
> +/* IRQ to trigger handler */
> +static irqreturn_t rpr0521_drdy_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +
> +	data->irq_timestamp = iio_get_time_ns(indio_dev);
> +	/*
> +	 * We need to wake the thread to read the interrupt reg. It
> +	 * is not possible to do that here because regmap_read takes a
> +	 * mutex.
> +	 */
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t rpr0521_drdy_irq_thread(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +
> +	if (rpr0521_is_triggered(data)) {
> +		iio_trigger_poll_chained(data->drdy_trigger0);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static irqreturn_t rpr0521_trigger_consumer_store_time(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +
> +	/* Other trigger polls store time here. */
> +	if (!iio_trigger_using_own(indio_dev))
> +		pf->timestamp = iio_get_time_ns(indio_dev);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +	int err;
> +
> +	u8 buffer[16]; /* 3 16-bit channels + padding + ts */
> +
> +	/* Use irq timestamp when reasonable. */
> +	if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) {
> +		pf->timestamp = data->irq_timestamp;
> +		data->irq_timestamp = 0;
> +	}
> +	/* Other chained trigger polls get timestamp only here. */
> +	if (!pf->timestamp)
> +		pf->timestamp = iio_get_time_ns(indio_dev);
> +
> +	err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA,
> +		&buffer,
> +		(3 * 2) + 1);	/* 3 * 16-bit + (discarded) int clear reg. */
> +	if (!err)
> +		iio_push_to_buffers_with_timestamp(indio_dev,
> +						   buffer, pf->timestamp);
> +	else
> +		dev_err(&data->client->dev,
> +			"Trigger consumer can't read from sensor.\n");
> +	pf->timestamp = 0;
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rpr0521_write_int_enable(struct rpr0521_data *data)
> +{
> +	int err;
> +
> +	/* Interrupt after each measurement */
> +	err = regmap_update_bits(data->regmap, RPR0521_REG_PXS_CTRL,
> +		RPR0521_PXS_PERSISTENCE_MASK,
> +		RPR0521_PXS_PERSISTENCE_DRDY);
> +	if (err) {
> +		dev_err(&data->client->dev, "PS control reg write fail.\n");
> +		return -EBUSY;
> +		}
> +
> +	/* Ignore latch and mode because of drdy */
> +	err = regmap_write(data->regmap, RPR0521_REG_INTERRUPT,
> +		RPR0521_INTERRUPT_INT_REASSERT_DISABLE |
> +		RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |
> +		RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE
> +		);
> +	if (err) {
> +		dev_err(&data->client->dev, "Interrupt setup write fail.\n");
> +		return -EBUSY;
> +		}
> +
> +	return 0;
> +}
> +
> +static int rpr0521_write_int_disable(struct rpr0521_data *data)
> +{
> +	/* Don't care of clearing mode, assert and latch. */
> +	return regmap_write(data->regmap, RPR0521_REG_INTERRUPT,
> +				RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |
> +				RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE
> +				);
> +}
> +
> +/*
> + * Trigger producer enable / disable. Note that there will be trigs only when
> + * measurement data is ready to be read.
> + */
> +static int rpr0521_pxs_drdy_set_state(struct iio_trigger *trigger,
> +	bool enable_drdy)
> +{
> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trigger);
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +	int err;
> +
> +	if (enable_drdy)
> +		err = rpr0521_write_int_enable(data);
> +	else
> +		err = rpr0521_write_int_disable(data);
> +	if (err)
> +		dev_err(&data->client->dev, "rpr0521_pxs_drdy_set_state failed\n");
> +
> +	return err;
> +}
> +
> +static const struct iio_trigger_ops rpr0521_trigger_ops = {
> +	.set_trigger_state = rpr0521_pxs_drdy_set_state,
> +	.owner = THIS_MODULE,
> +	};
> +
> +
> +static int rpr0521_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	int err;
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +
> +	mutex_lock(&data->lock);
> +	err = rpr0521_set_power_state(data, true,
> +		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));
> +	mutex_unlock(&data->lock);
> +	if (err)
> +		dev_err(&data->client->dev, "_buffer_preenable fail\n");
> +
> +	return err;
> +}
> +
> +static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	int err;
> +	struct rpr0521_data *data = iio_priv(indio_dev);
> +
> +	mutex_lock(&data->lock);
> +	err = rpr0521_set_power_state(data, false,
> +		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));
> +	mutex_unlock(&data->lock);
> +	if (err)
> +		dev_err(&data->client->dev, "_buffer_postdisable fail\n");
> +
> +	return err;
> +}
> +
> +static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops = {
> +	.preenable = rpr0521_buffer_preenable,
> +	.postenable = iio_triggered_buffer_postenable,
> +	.predisable = iio_triggered_buffer_predisable,
> +	.postdisable = rpr0521_buffer_postdisable,
> +};
> +
>  static int rpr0521_get_gain(struct rpr0521_data *data, int chan,
>  			    int *val, int *val2)
>  {
> @@ -473,6 +722,7 @@ static int rpr0521_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct rpr0521_data *data = iio_priv(indio_dev);
>  	int ret;
> +	int busy;
>  	u8 device_mask;
>  	__le16 raw_data;
>  
> @@ -481,26 +731,30 @@ static int rpr0521_read_raw(struct iio_dev *indio_dev,
>  		if (chan->type != IIO_INTENSITY && chan->type != IIO_PROXIMITY)
>  			return -EINVAL;
>  
> +		busy = iio_device_claim_direct_mode(indio_dev);
> +		if (busy)
> +			return -EBUSY;
> +
>  		device_mask = rpr0521_data_reg[chan->address].device_mask;
>  
>  		mutex_lock(&data->lock);
>  		ret = rpr0521_set_power_state(data, true, device_mask);
> -		if (ret < 0) {
> -			mutex_unlock(&data->lock);
> -			return ret;
> -		}
> +		if (ret < 0)
> +			goto rpr0521_read_raw_out;
>  
>  		ret = regmap_bulk_read(data->regmap,
>  				       rpr0521_data_reg[chan->address].address,
>  				       &raw_data, sizeof(raw_data));
>  		if (ret < 0) {
>  			rpr0521_set_power_state(data, false, device_mask);
> -			mutex_unlock(&data->lock);
> -			return ret;
> +			goto rpr0521_read_raw_out;
>  		}
>  
>  		ret = rpr0521_set_power_state(data, false, device_mask);
> +
> +rpr0521_read_raw_out:
>  		mutex_unlock(&data->lock);
> +		iio_device_release_direct_mode(indio_dev);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -617,12 +871,15 @@ static int rpr0521_init(struct rpr0521_data *data)
>  		return ret;
>  #endif
>  
> +	data->irq_timestamp = 0;
> +
>  	return 0;
>  }
>  
>  static int rpr0521_poweroff(struct rpr0521_data *data)
>  {
>  	int ret;
> +	int tmp;
>  
>  	ret = regmap_update_bits(data->regmap, RPR0521_REG_MODE_CTRL,
>  				 RPR0521_MODE_ALS_MASK |
> @@ -635,6 +892,16 @@ static int rpr0521_poweroff(struct rpr0521_data *data)
>  	data->als_dev_en = false;
>  	data->pxs_dev_en = false;
>  
> +	/*
> +	 * Int pin keeps state after power off. Set pin to high impedance
> +	 * mode to prevent power drain.
> +	 */
> +	ret = regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &tmp);
> +	if (ret) {
> +		dev_err(&data->client->dev, "Failed to reset int pin.\n");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -707,6 +974,61 @@ static int rpr0521_probe(struct i2c_client *client,
>  	pm_runtime_set_autosuspend_delay(&client->dev, RPR0521_SLEEP_DELAY_MS);
>  	pm_runtime_use_autosuspend(&client->dev);
>  
> +	/*
> +	 * If sensor write/read is needed in _probe after _use_autosuspend,
> +	 * sensor needs to be _resumed first using rpr0521_set_power_state().
> +	 */
> +
> +	/* IRQ to trigger setup */
> +	if (client->irq) {
> +		/* Trigger0 producer setup */
> +		data->drdy_trigger0 = devm_iio_trigger_alloc(
> +			indio_dev->dev.parent,
> +			"%s-dev%d", indio_dev->name, indio_dev->id);
> +		if (!data->drdy_trigger0) {
> +			ret = -ENOMEM;
> +			goto err_pm_disable;
> +		}
> +		data->drdy_trigger0->dev.parent = indio_dev->dev.parent;
> +		data->drdy_trigger0->ops = &rpr0521_trigger_ops;
> +		indio_dev->available_scan_masks = rpr0521_available_scan_masks;
If I was being really fussy, this has nothing much to do with the trigger
(semantically anyway) so should probably just be set whatever or at very
least down by where you setup the buffer.


> +		iio_trigger_set_drvdata(data->drdy_trigger0, indio_dev);
> +
> +		/* Ties irq to trigger producer handler. */
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +			rpr0521_drdy_irq_handler, rpr0521_drdy_irq_thread,
> +			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +			RPR0521_IRQ_NAME, indio_dev);
> +		if (ret < 0) {
> +			dev_err(&client->dev, "request irq %d for trigger0 failed\n",
> +				client->irq);
> +			goto err_pm_disable;
> +			}
> +
> +		ret = devm_iio_trigger_register(indio_dev->dev.parent,
> +						data->drdy_trigger0);
> +		if (ret) {
> +			dev_err(&client->dev, "iio trigger register failed\n");
> +			goto err_pm_disable;
> +		}
> +
> +		/*
> +		 * Now whole pipe from physical interrupt (irq defined by
> +		 * devicetree to device) to trigger0 output is set up.
> +		 */
> +
> +		/* Trigger consumer setup */
> +		ret = devm_iio_triggered_buffer_setup(indio_dev->dev.parent,
> +			indio_dev,
> +			rpr0521_trigger_consumer_store_time,
> +			rpr0521_trigger_consumer_handler,
> +			&rpr0521_buffer_setup_ops);
> +		if (ret < 0) {
> +			dev_err(&client->dev, "iio triggered buffer setup failed\n");
> +			goto err_pm_disable;
> +		}
> +	}
> +
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
>  		goto err_pm_disable;

Re: [PATCH v7 09/10] iio: light: rpr0521 triggered buffer

[Koivunen, Mikko]

On 4.7.2017 22:48, Jonathan Cameron wrote:=0A=
> On Mon,  3 Jul 2017 15:50:13 +0300=0A=
> Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com> wrote:=0A=
>=0A=
>> Set up and use triggered buffer if there is irq defined for device in=0A=
>> device tree. Trigger producer triggers from rpr0521 drdy interrupt line.=
=0A=
>> Trigger consumer reads rpr0521 data to scan buffer.=0A=
>> Depends on previous commits of _scale and _offset.=0A=
>>=0A=
>> Signed-off-by: Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com>=0A=
> one side comment inline, but so tiny I'm not even going to bother changin=
g=0A=
> it whilst applying.=0A=
>=0A=
> Very nice.=0A=
>=0A=
> Applied to the togreg branch of iio.git and pushed out as testing=0A=
> for the autobuilders to play with it.=0A=
>=0A=
> Thanks,=0A=
>=0A=
> Jonathan=0A=
=0A=
Ack. And thanks for review, this is much better with all the changes.=0A=
=0A=
>> ---=0A=
>> Patch v2->v3 changes:=0A=
>> .shift =3D 0 removed=0A=
>> reordered functions to remove forward declarations=0A=
>> whitespace changes=0A=
>> refactored some update_bits->write, no "err =3D err || *"-pattern=0A=
>> refactored trigger_consumer_handler=0A=
>> reordered _probe() and _remove()=0A=
>> added int clear on poweroff()=0A=
>> checkpatch.pl OK=0A=
>> Tested on LeMaker HiKey with AOSP7.1 kernel 4.4.=0A=
>> Builds ok with torvalds/linux feb 27.=0A=
>>=0A=
>> Patch v3->v4 changes:=0A=
>> Moved sensor on/off commands to buffer preenable and postdisable=0A=
>>  - Since drdy happens only on measurement data ready and register writes=
=0A=
>>    are cached, the trigger producer doesn't care of suspend/resume state=
.=0A=
>> available_scan_masks added=0A=
>> indent fix=0A=
>> checkpatch.pl OK=0A=
>> Lightly re-tested on LeMaker HiKey with AOSP7.1 kernel 4.4.=0A=
>> Builds ok with torvalds/linux feb 27.=0A=
>>=0A=
>> Patch v5->v6 changes:=0A=
>> Changed base from kernel 4.4 to 4.9=0A=
>>  - Using iio_device_claim_direct_mode()=0A=
>>  - Using iio_trigger_using_own()=0A=
>>  - Changed trigger/buffer to devm_=0A=
>> Added shared irq support=0A=
>>  - divided trigger producer to top/bottom=0A=
>>  - divided trigger consumer to top/bottom=0A=
>>  - Using irq_timestamp when available=0A=
>> Removed default trigger=0A=
>>=0A=
>> Patch v6->v7 changes:=0A=
>> trigger_poll to trigger_poll_chained=0A=
>>  + some timestamp saving logic changes due that=0A=
>> 2 whitespace fixes=0A=
>>=0A=
>>  drivers/iio/light/rpr0521.c |  336 ++++++++++++++++++++++++++++++++++++=
++++++-=0A=
>>  1 file changed, 329 insertions(+), 7 deletions(-)=0A=
>>=0A=
>> diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c=
=0A=
>> index 9d0c2e8..a6efa126 100644=0A=
>> --- a/drivers/iio/light/rpr0521.c=0A=
>> +++ b/drivers/iio/light/rpr0521.c=0A=
>> @@ -9,7 +9,7 @@=0A=
>>   *=0A=
>>   * IIO driver for RPR-0521RS (7-bit I2C slave address 0x38).=0A=
>>   *=0A=
>> - * TODO: illuminance channel, buffer=0A=
>> + * TODO: illuminance channel=0A=
>>   */=0A=
>>  =0A=
>>  #include <linux/module.h>=0A=
>> @@ -20,6 +20,10 @@=0A=
>>  #include <linux/acpi.h>=0A=
>>  =0A=
>>  #include <linux/iio/iio.h>=0A=
>> +#include <linux/iio/buffer.h>=0A=
>> +#include <linux/iio/trigger.h>=0A=
>> +#include <linux/iio/trigger_consumer.h>=0A=
>> +#include <linux/iio/triggered_buffer.h>=0A=
>>  #include <linux/iio/sysfs.h>=0A=
>>  #include <linux/pm_runtime.h>=0A=
>>  =0A=
>> @@ -30,6 +34,7 @@=0A=
>>  #define RPR0521_REG_PXS_DATA		0x44 /* 16-bit, little endian */=0A=
>>  #define RPR0521_REG_ALS_DATA0		0x46 /* 16-bit, little endian */=0A=
>>  #define RPR0521_REG_ALS_DATA1		0x48 /* 16-bit, little endian */=0A=
>> +#define RPR0521_REG_INTERRUPT		0x4A=0A=
>>  #define RPR0521_REG_PS_OFFSET_LSB	0x53=0A=
>>  #define RPR0521_REG_ID			0x92=0A=
>>  =0A=
>> @@ -42,16 +47,31 @@=0A=
>>  #define RPR0521_ALS_DATA1_GAIN_SHIFT	2=0A=
>>  #define RPR0521_PXS_GAIN_MASK		GENMASK(5, 4)=0A=
>>  #define RPR0521_PXS_GAIN_SHIFT		4=0A=
>> +#define RPR0521_PXS_PERSISTENCE_MASK	GENMASK(3, 0)=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_PS_MASK	BIT(0)=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_MASK	BIT(1)=0A=
>> +#define RPR0521_INTERRUPT_INT_REASSERT_MASK	BIT(3)=0A=
>> +#define RPR0521_INTERRUPT_ALS_INT_STATUS_MASK	BIT(6)=0A=
>> +#define RPR0521_INTERRUPT_PS_INT_STATUS_MASK	BIT(7)=0A=
>>  =0A=
>>  #define RPR0521_MODE_ALS_ENABLE		BIT(7)=0A=
>>  #define RPR0521_MODE_ALS_DISABLE	0x00=0A=
>>  #define RPR0521_MODE_PXS_ENABLE		BIT(6)=0A=
>>  #define RPR0521_MODE_PXS_DISABLE	0x00=0A=
>> +#define RPR0521_PXS_PERSISTENCE_DRDY	0x00=0A=
>> +=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE	BIT(0)=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE	0x00=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_ENABLE	BIT(1)=0A=
>> +#define RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE	0x00=0A=
>> +#define RPR0521_INTERRUPT_INT_REASSERT_ENABLE	BIT(3)=0A=
>> +#define RPR0521_INTERRUPT_INT_REASSERT_DISABLE	0x00=0A=
>>  =0A=
>>  #define RPR0521_MANUFACT_ID		0xE0=0A=
>>  #define RPR0521_DEFAULT_MEAS_TIME	0x06 /* ALS - 100ms, PXS - 100ms */=
=0A=
>>  =0A=
>>  #define RPR0521_DRV_NAME		"RPR0521"=0A=
>> +#define RPR0521_IRQ_NAME		"rpr0521_event"=0A=
>>  #define RPR0521_REGMAP_NAME		"rpr0521_regmap"=0A=
>>  =0A=
>>  #define RPR0521_SLEEP_DELAY_MS	2000=0A=
>> @@ -167,6 +187,9 @@ struct rpr0521_data {=0A=
>>  	bool als_dev_en;=0A=
>>  	bool pxs_dev_en;=0A=
>>  =0A=
>> +	struct iio_trigger *drdy_trigger0;=0A=
>> +	s64 irq_timestamp;=0A=
>> +=0A=
>>  	/* optimize runtime pm ops - enable/disable device only if needed */=
=0A=
>>  	bool als_ps_need_en;=0A=
>>  	bool pxs_ps_need_en;=0A=
>> @@ -196,6 +219,19 @@ static const struct attribute_group rpr0521_attribu=
te_group =3D {=0A=
>>  	.attrs =3D rpr0521_attributes,=0A=
>>  };=0A=
>>  =0A=
>> +/* Order of the channel data in buffer */=0A=
>> +enum rpr0521_scan_index_order {=0A=
>> +	RPR0521_CHAN_INDEX_PXS,=0A=
>> +	RPR0521_CHAN_INDEX_BOTH,=0A=
>> +	RPR0521_CHAN_INDEX_IR,=0A=
>> +};=0A=
>> +=0A=
>> +static const unsigned long rpr0521_available_scan_masks[] =3D {=0A=
>> +	BIT(RPR0521_CHAN_INDEX_PXS) | BIT(RPR0521_CHAN_INDEX_BOTH) |=0A=
>> +	BIT(RPR0521_CHAN_INDEX_IR),=0A=
>> +	0=0A=
>> +};=0A=
>> +=0A=
>>  static const struct iio_chan_spec rpr0521_channels[] =3D {=0A=
>>  	{=0A=
>>  		.type =3D IIO_PROXIMITY,=0A=
>> @@ -204,6 +240,13 @@ static const struct iio_chan_spec rpr0521_channels[=
] =3D {=0A=
>>  			BIT(IIO_CHAN_INFO_OFFSET) |=0A=
>>  			BIT(IIO_CHAN_INFO_SCALE),=0A=
>>  		.info_mask_shared_by_all =3D BIT(IIO_CHAN_INFO_SAMP_FREQ),=0A=
>> +		.scan_index =3D RPR0521_CHAN_INDEX_PXS,=0A=
>> +		.scan_type =3D {=0A=
>> +			.sign =3D 'u',=0A=
>> +			.realbits =3D 16,=0A=
>> +			.storagebits =3D 16,=0A=
>> +			.endianness =3D IIO_LE,=0A=
>> +		},=0A=
>>  	},=0A=
>>  	{=0A=
>>  		.type =3D IIO_INTENSITY,=0A=
>> @@ -213,6 +256,13 @@ static const struct iio_chan_spec rpr0521_channels[=
] =3D {=0A=
>>  		.info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW) |=0A=
>>  			BIT(IIO_CHAN_INFO_SCALE),=0A=
>>  		.info_mask_shared_by_all =3D BIT(IIO_CHAN_INFO_SAMP_FREQ),=0A=
>> +		.scan_index =3D RPR0521_CHAN_INDEX_BOTH,=0A=
>> +		.scan_type =3D {=0A=
>> +			.sign =3D 'u',=0A=
>> +			.realbits =3D 16,=0A=
>> +			.storagebits =3D 16,=0A=
>> +			.endianness =3D IIO_LE,=0A=
>> +		},=0A=
>>  	},=0A=
>>  	{=0A=
>>  		.type =3D IIO_INTENSITY,=0A=
>> @@ -222,6 +272,13 @@ static const struct iio_chan_spec rpr0521_channels[=
] =3D {=0A=
>>  		.info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW) |=0A=
>>  			BIT(IIO_CHAN_INFO_SCALE),=0A=
>>  		.info_mask_shared_by_all =3D BIT(IIO_CHAN_INFO_SAMP_FREQ),=0A=
>> +		.scan_index =3D RPR0521_CHAN_INDEX_IR,=0A=
>> +		.scan_type =3D {=0A=
>> +			.sign =3D 'u',=0A=
>> +			.realbits =3D 16,=0A=
>> +			.storagebits =3D 16,=0A=
>> +			.endianness =3D IIO_LE,=0A=
>> +		},=0A=
>>  	},=0A=
>>  };=0A=
>>  =0A=
>> @@ -330,6 +387,198 @@ static int rpr0521_set_power_state(struct rpr0521_=
data *data, bool on,=0A=
>>  	return 0;=0A=
>>  }=0A=
>>  =0A=
>> +/* Interrupt register tells if this sensor caused the interrupt or not.=
 */=0A=
>> +static inline bool rpr0521_is_triggered(struct rpr0521_data *data)=0A=
>> +{=0A=
>> +	int ret;=0A=
>> +	int reg;=0A=
>> +=0A=
>> +	ret =3D regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &reg);=0A=
>> +	if (ret < 0)=0A=
>> +		return false;   /* Reg read failed. */=0A=
>> +	if (reg &=0A=
>> +	    (RPR0521_INTERRUPT_ALS_INT_STATUS_MASK |=0A=
>> +	    RPR0521_INTERRUPT_PS_INT_STATUS_MASK))=0A=
>> +		return true;=0A=
>> +	else=0A=
>> +		return false;   /* Int not from this sensor. */=0A=
>> +}=0A=
>> +=0A=
>> +/* IRQ to trigger handler */=0A=
>> +static irqreturn_t rpr0521_drdy_irq_handler(int irq, void *private)=0A=
>> +{=0A=
>> +	struct iio_dev *indio_dev =3D private;=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +=0A=
>> +	data->irq_timestamp =3D iio_get_time_ns(indio_dev);=0A=
>> +	/*=0A=
>> +	 * We need to wake the thread to read the interrupt reg. It=0A=
>> +	 * is not possible to do that here because regmap_read takes a=0A=
>> +	 * mutex.=0A=
>> +	 */=0A=
>> +=0A=
>> +	return IRQ_WAKE_THREAD;=0A=
>> +}=0A=
>> +=0A=
>> +static irqreturn_t rpr0521_drdy_irq_thread(int irq, void *private)=0A=
>> +{=0A=
>> +	struct iio_dev *indio_dev =3D private;=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +=0A=
>> +	if (rpr0521_is_triggered(data)) {=0A=
>> +		iio_trigger_poll_chained(data->drdy_trigger0);=0A=
>> +		return IRQ_HANDLED;=0A=
>> +	}=0A=
>> +=0A=
>> +	return IRQ_NONE;=0A=
>> +}=0A=
>> +=0A=
>> +static irqreturn_t rpr0521_trigger_consumer_store_time(int irq, void *p=
)=0A=
>> +{=0A=
>> +	struct iio_poll_func *pf =3D p;=0A=
>> +	struct iio_dev *indio_dev =3D pf->indio_dev;=0A=
>> +=0A=
>> +	/* Other trigger polls store time here. */=0A=
>> +	if (!iio_trigger_using_own(indio_dev))=0A=
>> +		pf->timestamp =3D iio_get_time_ns(indio_dev);=0A=
>> +=0A=
>> +	return IRQ_WAKE_THREAD;=0A=
>> +}=0A=
>> +=0A=
>> +static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)=
=0A=
>> +{=0A=
>> +	struct iio_poll_func *pf =3D p;=0A=
>> +	struct iio_dev *indio_dev =3D pf->indio_dev;=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +	int err;=0A=
>> +=0A=
>> +	u8 buffer[16]; /* 3 16-bit channels + padding + ts */=0A=
>> +=0A=
>> +	/* Use irq timestamp when reasonable. */=0A=
>> +	if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) {=0A=
>> +		pf->timestamp =3D data->irq_timestamp;=0A=
>> +		data->irq_timestamp =3D 0;=0A=
>> +	}=0A=
>> +	/* Other chained trigger polls get timestamp only here. */=0A=
>> +	if (!pf->timestamp)=0A=
>> +		pf->timestamp =3D iio_get_time_ns(indio_dev);=0A=
>> +=0A=
>> +	err =3D regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA,=0A=
>> +		&buffer,=0A=
>> +		(3 * 2) + 1);	/* 3 * 16-bit + (discarded) int clear reg. */=0A=
>> +	if (!err)=0A=
>> +		iio_push_to_buffers_with_timestamp(indio_dev,=0A=
>> +						   buffer, pf->timestamp);=0A=
>> +	else=0A=
>> +		dev_err(&data->client->dev,=0A=
>> +			"Trigger consumer can't read from sensor.\n");=0A=
>> +	pf->timestamp =3D 0;=0A=
>> +=0A=
>> +	iio_trigger_notify_done(indio_dev->trig);=0A=
>> +=0A=
>> +	return IRQ_HANDLED;=0A=
>> +}=0A=
>> +=0A=
>> +static int rpr0521_write_int_enable(struct rpr0521_data *data)=0A=
>> +{=0A=
>> +	int err;=0A=
>> +=0A=
>> +	/* Interrupt after each measurement */=0A=
>> +	err =3D regmap_update_bits(data->regmap, RPR0521_REG_PXS_CTRL,=0A=
>> +		RPR0521_PXS_PERSISTENCE_MASK,=0A=
>> +		RPR0521_PXS_PERSISTENCE_DRDY);=0A=
>> +	if (err) {=0A=
>> +		dev_err(&data->client->dev, "PS control reg write fail.\n");=0A=
>> +		return -EBUSY;=0A=
>> +		}=0A=
>> +=0A=
>> +	/* Ignore latch and mode because of drdy */=0A=
>> +	err =3D regmap_write(data->regmap, RPR0521_REG_INTERRUPT,=0A=
>> +		RPR0521_INTERRUPT_INT_REASSERT_DISABLE |=0A=
>> +		RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |=0A=
>> +		RPR0521_INTERRUPT_INT_TRIG_PS_ENABLE=0A=
>> +		);=0A=
>> +	if (err) {=0A=
>> +		dev_err(&data->client->dev, "Interrupt setup write fail.\n");=0A=
>> +		return -EBUSY;=0A=
>> +		}=0A=
>> +=0A=
>> +	return 0;=0A=
>> +}=0A=
>> +=0A=
>> +static int rpr0521_write_int_disable(struct rpr0521_data *data)=0A=
>> +{=0A=
>> +	/* Don't care of clearing mode, assert and latch. */=0A=
>> +	return regmap_write(data->regmap, RPR0521_REG_INTERRUPT,=0A=
>> +				RPR0521_INTERRUPT_INT_TRIG_ALS_DISABLE |=0A=
>> +				RPR0521_INTERRUPT_INT_TRIG_PS_DISABLE=0A=
>> +				);=0A=
>> +}=0A=
>> +=0A=
>> +/*=0A=
>> + * Trigger producer enable / disable. Note that there will be trigs onl=
y when=0A=
>> + * measurement data is ready to be read.=0A=
>> + */=0A=
>> +static int rpr0521_pxs_drdy_set_state(struct iio_trigger *trigger,=0A=
>> +	bool enable_drdy)=0A=
>> +{=0A=
>> +	struct iio_dev *indio_dev =3D iio_trigger_get_drvdata(trigger);=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +	int err;=0A=
>> +=0A=
>> +	if (enable_drdy)=0A=
>> +		err =3D rpr0521_write_int_enable(data);=0A=
>> +	else=0A=
>> +		err =3D rpr0521_write_int_disable(data);=0A=
>> +	if (err)=0A=
>> +		dev_err(&data->client->dev, "rpr0521_pxs_drdy_set_state failed\n");=
=0A=
>> +=0A=
>> +	return err;=0A=
>> +}=0A=
>> +=0A=
>> +static const struct iio_trigger_ops rpr0521_trigger_ops =3D {=0A=
>> +	.set_trigger_state =3D rpr0521_pxs_drdy_set_state,=0A=
>> +	.owner =3D THIS_MODULE,=0A=
>> +	};=0A=
>> +=0A=
>> +=0A=
>> +static int rpr0521_buffer_preenable(struct iio_dev *indio_dev)=0A=
>> +{=0A=
>> +	int err;=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +=0A=
>> +	mutex_lock(&data->lock);=0A=
>> +	err =3D rpr0521_set_power_state(data, true,=0A=
>> +		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));=0A=
>> +	mutex_unlock(&data->lock);=0A=
>> +	if (err)=0A=
>> +		dev_err(&data->client->dev, "_buffer_preenable fail\n");=0A=
>> +=0A=
>> +	return err;=0A=
>> +}=0A=
>> +=0A=
>> +static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)=0A=
>> +{=0A=
>> +	int err;=0A=
>> +	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>> +=0A=
>> +	mutex_lock(&data->lock);=0A=
>> +	err =3D rpr0521_set_power_state(data, false,=0A=
>> +		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));=0A=
>> +	mutex_unlock(&data->lock);=0A=
>> +	if (err)=0A=
>> +		dev_err(&data->client->dev, "_buffer_postdisable fail\n");=0A=
>> +=0A=
>> +	return err;=0A=
>> +}=0A=
>> +=0A=
>> +static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops =3D {=
=0A=
>> +	.preenable =3D rpr0521_buffer_preenable,=0A=
>> +	.postenable =3D iio_triggered_buffer_postenable,=0A=
>> +	.predisable =3D iio_triggered_buffer_predisable,=0A=
>> +	.postdisable =3D rpr0521_buffer_postdisable,=0A=
>> +};=0A=
>> +=0A=
>>  static int rpr0521_get_gain(struct rpr0521_data *data, int chan,=0A=
>>  			    int *val, int *val2)=0A=
>>  {=0A=
>> @@ -473,6 +722,7 @@ static int rpr0521_read_raw(struct iio_dev *indio_de=
v,=0A=
>>  {=0A=
>>  	struct rpr0521_data *data =3D iio_priv(indio_dev);=0A=
>>  	int ret;=0A=
>> +	int busy;=0A=
>>  	u8 device_mask;=0A=
>>  	__le16 raw_data;=0A=
>>  =0A=
>> @@ -481,26 +731,30 @@ static int rpr0521_read_raw(struct iio_dev *indio_=
dev,=0A=
>>  		if (chan->type !=3D IIO_INTENSITY && chan->type !=3D IIO_PROXIMITY)=
=0A=
>>  			return -EINVAL;=0A=
>>  =0A=
>> +		busy =3D iio_device_claim_direct_mode(indio_dev);=0A=
>> +		if (busy)=0A=
>> +			return -EBUSY;=0A=
>> +=0A=
>>  		device_mask =3D rpr0521_data_reg[chan->address].device_mask;=0A=
>>  =0A=
>>  		mutex_lock(&data->lock);=0A=
>>  		ret =3D rpr0521_set_power_state(data, true, device_mask);=0A=
>> -		if (ret < 0) {=0A=
>> -			mutex_unlock(&data->lock);=0A=
>> -			return ret;=0A=
>> -		}=0A=
>> +		if (ret < 0)=0A=
>> +			goto rpr0521_read_raw_out;=0A=
>>  =0A=
>>  		ret =3D regmap_bulk_read(data->regmap,=0A=
>>  				       rpr0521_data_reg[chan->address].address,=0A=
>>  				       &raw_data, sizeof(raw_data));=0A=
>>  		if (ret < 0) {=0A=
>>  			rpr0521_set_power_state(data, false, device_mask);=0A=
>> -			mutex_unlock(&data->lock);=0A=
>> -			return ret;=0A=
>> +			goto rpr0521_read_raw_out;=0A=
>>  		}=0A=
>>  =0A=
>>  		ret =3D rpr0521_set_power_state(data, false, device_mask);=0A=
>> +=0A=
>> +rpr0521_read_raw_out:=0A=
>>  		mutex_unlock(&data->lock);=0A=
>> +		iio_device_release_direct_mode(indio_dev);=0A=
>>  		if (ret < 0)=0A=
>>  			return ret;=0A=
>>  =0A=
>> @@ -617,12 +871,15 @@ static int rpr0521_init(struct rpr0521_data *data)=
=0A=
>>  		return ret;=0A=
>>  #endif=0A=
>>  =0A=
>> +	data->irq_timestamp =3D 0;=0A=
>> +=0A=
>>  	return 0;=0A=
>>  }=0A=
>>  =0A=
>>  static int rpr0521_poweroff(struct rpr0521_data *data)=0A=
>>  {=0A=
>>  	int ret;=0A=
>> +	int tmp;=0A=
>>  =0A=
>>  	ret =3D regmap_update_bits(data->regmap, RPR0521_REG_MODE_CTRL,=0A=
>>  				 RPR0521_MODE_ALS_MASK |=0A=
>> @@ -635,6 +892,16 @@ static int rpr0521_poweroff(struct rpr0521_data *da=
ta)=0A=
>>  	data->als_dev_en =3D false;=0A=
>>  	data->pxs_dev_en =3D false;=0A=
>>  =0A=
>> +	/*=0A=
>> +	 * Int pin keeps state after power off. Set pin to high impedance=0A=
>> +	 * mode to prevent power drain.=0A=
>> +	 */=0A=
>> +	ret =3D regmap_read(data->regmap, RPR0521_REG_INTERRUPT, &tmp);=0A=
>> +	if (ret) {=0A=
>> +		dev_err(&data->client->dev, "Failed to reset int pin.\n");=0A=
>> +		return ret;=0A=
>> +	}=0A=
>> +=0A=
>>  	return 0;=0A=
>>  }=0A=
>>  =0A=
>> @@ -707,6 +974,61 @@ static int rpr0521_probe(struct i2c_client *client,=
=0A=
>>  	pm_runtime_set_autosuspend_delay(&client->dev, RPR0521_SLEEP_DELAY_MS)=
;=0A=
>>  	pm_runtime_use_autosuspend(&client->dev);=0A=
>>  =0A=
>> +	/*=0A=
>> +	 * If sensor write/read is needed in _probe after _use_autosuspend,=0A=
>> +	 * sensor needs to be _resumed first using rpr0521_set_power_state().=
=0A=
>> +	 */=0A=
>> +=0A=
>> +	/* IRQ to trigger setup */=0A=
>> +	if (client->irq) {=0A=
>> +		/* Trigger0 producer setup */=0A=
>> +		data->drdy_trigger0 =3D devm_iio_trigger_alloc(=0A=
>> +			indio_dev->dev.parent,=0A=
>> +			"%s-dev%d", indio_dev->name, indio_dev->id);=0A=
>> +		if (!data->drdy_trigger0) {=0A=
>> +			ret =3D -ENOMEM;=0A=
>> +			goto err_pm_disable;=0A=
>> +		}=0A=
>> +		data->drdy_trigger0->dev.parent =3D indio_dev->dev.parent;=0A=
>> +		data->drdy_trigger0->ops =3D &rpr0521_trigger_ops;=0A=
>> +		indio_dev->available_scan_masks =3D rpr0521_available_scan_masks;=0A=
> If I was being really fussy, this has nothing much to do with the trigger=
=0A=
> (semantically anyway) so should probably just be set whatever or at very=
=0A=
> least down by where you setup the buffer.=0A=
=0A=
True. I was probably thinking (incorrectly) trigger producer/consumer as=0A=
more bound together while writing that.=0A=
=0A=
>=0A=
>> +		iio_trigger_set_drvdata(data->drdy_trigger0, indio_dev);=0A=
>> +=0A=
>> +		/* Ties irq to trigger producer handler. */=0A=
>> +		ret =3D devm_request_threaded_irq(&client->dev, client->irq,=0A=
>> +			rpr0521_drdy_irq_handler, rpr0521_drdy_irq_thread,=0A=
>> +			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,=0A=
>> +			RPR0521_IRQ_NAME, indio_dev);=0A=
>> +		if (ret < 0) {=0A=
>> +			dev_err(&client->dev, "request irq %d for trigger0 failed\n",=0A=
>> +				client->irq);=0A=
>> +			goto err_pm_disable;=0A=
>> +			}=0A=
>> +=0A=
>> +		ret =3D devm_iio_trigger_register(indio_dev->dev.parent,=0A=
>> +						data->drdy_trigger0);=0A=
>> +		if (ret) {=0A=
>> +			dev_err(&client->dev, "iio trigger register failed\n");=0A=
>> +			goto err_pm_disable;=0A=
>> +		}=0A=
>> +=0A=
>> +		/*=0A=
>> +		 * Now whole pipe from physical interrupt (irq defined by=0A=
>> +		 * devicetree to device) to trigger0 output is set up.=0A=
>> +		 */=0A=
>> +=0A=
>> +		/* Trigger consumer setup */=0A=
>> +		ret =3D devm_iio_triggered_buffer_setup(indio_dev->dev.parent,=0A=
>> +			indio_dev,=0A=
>> +			rpr0521_trigger_consumer_store_time,=0A=
>> +			rpr0521_trigger_consumer_handler,=0A=
>> +			&rpr0521_buffer_setup_ops);=0A=
>> +		if (ret < 0) {=0A=
>> +			dev_err(&client->dev, "iio triggered buffer setup failed\n");=0A=
>> +			goto err_pm_disable;=0A=
>> +		}=0A=
>> +	}=0A=
>> +=0A=
>>  	ret =3D iio_device_register(indio_dev);=0A=
>>  	if (ret)=0A=
>>  		goto err_pm_disable;=0A=
>=0A=
=0A=

[PATCH v7 10/10] iio: light: rpr0521 iio_device_register to devm_

[Mikko Koivunen]

Change iio_device_register to devm_

--

 drivers/iio/light/rpr0521.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index a6efa126..4c090d0 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -1029,7 +1029,7 @@ static int rpr0521_probe(struct i2c_client *client,
 		}
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret)
 		goto err_pm_disable;
 
@@ -1049,8 +1049,6 @@ static int rpr0521_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 
-	iio_device_unregister(indio_dev);
-
 	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 	pm_runtime_put_noidle(&client->dev);
-- 
1.7.9.5

Re: [PATCH v7 10/10] iio: light: rpr0521 iio_device_register to devm_

[Jonathan Cameron]

On Mon,  3 Jul 2017 15:50:14 +0300
Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com> wrote:

> Change iio_device_register to devm_
> 
> --
> 
>  drivers/iio/light/rpr0521.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
> index a6efa126..4c090d0 100644
> --- a/drivers/iio/light/rpr0521.c
> +++ b/drivers/iio/light/rpr0521.c
> @@ -1029,7 +1029,7 @@ static int rpr0521_probe(struct i2c_client *client,
>  		}
>  	}
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
>  	if (ret)
>  		goto err_pm_disable;
>  
> @@ -1049,8 +1049,6 @@ static int rpr0521_remove(struct i2c_client *client)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>  
> -	iio_device_unregister(indio_dev);
> -
Given this now means you turn the power off before you remove the userspace
interfaces (and in kernel ones) this is a very bad idea.  

So dropping this patch.

Thanks,

Jonathan
>  	pm_runtime_disable(&client->dev);
>  	pm_runtime_set_suspended(&client->dev);
>  	pm_runtime_put_noidle(&client->dev);