[PATCH 5/7] iio: adc: ad4062: Add IIO Trigger support

Jorge Marques posted 7 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 5/7] iio: adc: ad4062: Add IIO Trigger support
Posted by Jorge Marques 2 months, 1 week ago
Adds support for IIO Trigger. Optionally, gp1 is assigned as Data Ready
signal, if not present, fallback to an I3C IBI with the same role.
The software trigger is allocated by the device, but must be attached by
the user before enabling the buffer. The purpose is to not impede
removing the driver due to the increased reference count when
iio_trigger_set_immutable or iio_trigger_get is used.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 drivers/iio/adc/Kconfig  |   2 +
 drivers/iio/adc/ad4062.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 490c01d701bdd1543809cdefad4ed5573c051c24..c7c17f7e04a2f93664ddad3d37875079a9d5ab24 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -74,6 +74,8 @@ config AD4062
 	tristate "Analog Devices AD4062 Driver"
 	depends on I3C
 	select REGMAP_I3C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Analog Devices AD4062 I3C analog
 	  to digital converters (ADC).
diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
index e55a69c62694a71a4e29f29b9a2bfeec3b16c990..40b7c10b8ce7145b010bb11e8e4698baacb6b3d3 100644
--- a/drivers/iio/adc/ad4062.c
+++ b/drivers/iio/adc/ad4062.c
@@ -12,8 +12,12 @@
 #include <linux/err.h>
 #include <linux/i3c/device.h>
 #include <linux/i3c/master.h>
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 #include <linux/interrupt.h>
 #include <linux/jiffies.h>
 #include <linux/math.h>
@@ -432,7 +436,10 @@ static irqreturn_t ad4062_irq_handler_drdy(int irq, void *private)
 	struct iio_dev *indio_dev = private;
 	struct ad4062_state *st = iio_priv(indio_dev);
 
-	complete(&st->completion);
+	if (iio_buffer_enabled(indio_dev) && iio_trigger_using_own(indio_dev))
+		iio_trigger_poll_nested(st->trigger);
+	else
+		complete(&st->completion);
 
 	return IRQ_HANDLED;
 }
@@ -442,7 +449,49 @@ static void ad4062_ibi_handler(struct i3c_device *i3cdev,
 {
 	struct ad4062_state *st = i3cdev_get_drvdata(i3cdev);
 
-	complete(&st->completion);
+	if (iio_buffer_enabled(st->indio_dev))
+		iio_trigger_poll(st->trigger);
+	else
+		complete(&st->completion);
+}
+
+static irqreturn_t ad4062_poll_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ad4062_state *st = iio_priv(indio_dev);
+	u8 addr = AD4062_REG_CONV_TRIGGER;
+	int ret;
+
+	/* Read current and trigger next conversion */
+	struct i3c_priv_xfer t[2] = {
+		{
+			.data.in = &st->raw,
+			.len = 4,
+			.rnw = true,
+		},
+		{
+			.data.out = &addr,
+			.len = 1,
+			.rnw = false,
+		}
+	};
+
+	/* Separated transfers to not infeer repeated-start */
+	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[0], 1);
+	if (ret)
+		goto out;
+	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[1], 1);
+	if (ret)
+		goto out;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &st->raw,
+					   pf->timestamp);
+
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
 }
 
 static int ad4062_request_ibi(struct i3c_device *i3cdev)
@@ -489,6 +538,27 @@ static int ad4062_request_irq(struct iio_dev *indio_dev)
 	return ret;
 }
 
+static const struct iio_trigger_ops ad4062_trigger_ops = {
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
+static int ad4062_request_trigger(struct iio_dev *indio_dev)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->i3cdev->dev;
+
+	st->trigger = devm_iio_trigger_alloc(dev, "%s-dev%d",
+					     indio_dev->name,
+					     iio_device_id(indio_dev));
+	if (!st->trigger)
+		return -ENOMEM;
+
+	st->trigger->ops = &ad4062_trigger_ops;
+	iio_trigger_set_drvdata(st->trigger, indio_dev);
+
+	return devm_iio_trigger_register(dev, st->trigger);
+}
+
 static const int ad4062_oversampling_avail[] = {
 	1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096,
 };
@@ -709,6 +779,52 @@ static int ad4062_write_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static int ad4062_triggered_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	u8 addr = AD4062_REG_CONV_TRIGGER;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(&st->i3cdev->dev);
+	if (ret)
+		return ret;
+
+	ret = ad4062_set_operation_mode(st, st->mode);
+	if (ret)
+		goto out_mode_error;
+
+	/* Trigger first conversion */
+	struct i3c_priv_xfer t = {
+		.data.out = &addr,
+		.len = 1,
+		.rnw = false,
+	};
+
+	ret = i3c_device_do_priv_xfers(st->i3cdev, &t, 1);
+	if (ret)
+		goto out_mode_error;
+	return 0;
+
+out_mode_error:
+	pm_runtime_put_autosuspend(&st->i3cdev->dev);
+
+	return ret;
+}
+
+static int ad4062_triggered_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+
+	pm_runtime_mark_last_busy(&st->i3cdev->dev);
+	pm_runtime_put_autosuspend(&st->i3cdev->dev);
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops ad4062_triggered_buffer_setup_ops = {
+	.postenable = &ad4062_triggered_buffer_postenable,
+	.predisable = &ad4062_triggered_buffer_predisable,
+};
+
 static int ad4062_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 				     unsigned int writeval, unsigned int *readval)
 {
@@ -843,6 +959,17 @@ static int ad4062_probe(struct i3c_device *i3cdev)
 	if (ret)
 		return ret;
 
+	ret = ad4062_request_trigger(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_triggered_buffer_setup(&i3cdev->dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      ad4062_poll_handler,
+					      &ad4062_triggered_buffer_setup_ops);
+	if (ret)
+		return ret;
+
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)

-- 
2.49.0
Re: [PATCH 5/7] iio: adc: ad4062: Add IIO Trigger support
Posted by Jonathan Cameron 2 months ago
On Mon, 13 Oct 2025 09:28:03 +0200
Jorge Marques <jorge.marques@analog.com> wrote:

> Adds support for IIO Trigger. Optionally, gp1 is assigned as Data Ready
> signal, if not present, fallback to an I3C IBI with the same role.
> The software trigger is allocated by the device, but must be attached by
> the user before enabling the buffer. The purpose is to not impede
> removing the driver due to the increased reference count when
> iio_trigger_set_immutable or iio_trigger_get is used.
> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>

A few things inline.
Thanks,

> diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> index e55a69c62694a71a4e29f29b9a2bfeec3b16c990..40b7c10b8ce7145b010bb11e8e4698baacb6b3d3 100644
> --- a/drivers/iio/adc/ad4062.c
> +++ b/drivers/iio/adc/ad4062.c

> +static irqreturn_t ad4062_poll_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ad4062_state *st = iio_priv(indio_dev);
> +	u8 addr = AD4062_REG_CONV_TRIGGER;
> +	int ret;
> +
> +	/* Read current and trigger next conversion */
> +	struct i3c_priv_xfer t[2] = {
> +		{
> +			.data.in = &st->raw,

If it is safe to use addr on the stack, do we need to have
a dma safe buffer for raw?  I'm not sure for i3c!

> +			.len = 4,
> +			.rnw = true,
> +		},
> +		{
> +			.data.out = &addr,
> +			.len = 1,
> +			.rnw = false,
> +		}
> +	};
> +
> +	/* Separated transfers to not infeer repeated-start */
> +	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[0], 1);
> +	if (ret)
> +		goto out;
> +	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[1], 1);

Add a comment on this. I assume it's setting things up for the next
scan?

> +	if (ret)
> +		goto out;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, &st->raw,
> +					   pf->timestamp);
> +
> +out:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
>  }

> +
> +static int ad4062_triggered_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4062_state *st = iio_priv(indio_dev);
> +
> +	pm_runtime_mark_last_busy(&st->i3cdev->dev);

Take a look at the changes across the tree recently.
Now pm_runtime_put_autosuspend() calls pm_runtime_mark_last_busy()
internally to avoid the need for this pair.

> +	pm_runtime_put_autosuspend(&st->i3cdev->dev);
> +	return 0;
> +}
Re: [PATCH 5/7] iio: adc: ad4062: Add IIO Trigger support
Posted by Jorge Marques 3 weeks, 4 days ago
On Sat, Oct 18, 2025 at 05:14:25PM +0100, Jonathan Cameron wrote:
> On Mon, 13 Oct 2025 09:28:03 +0200
> Jorge Marques <jorge.marques@analog.com> wrote:
> 
Hi Jonathan,

> > Adds support for IIO Trigger. Optionally, gp1 is assigned as Data Ready
> > signal, if not present, fallback to an I3C IBI with the same role.
> > The software trigger is allocated by the device, but must be attached by
> > the user before enabling the buffer. The purpose is to not impede
> > removing the driver due to the increased reference count when
> > iio_trigger_set_immutable or iio_trigger_get is used.
> > 
> > Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> 
> A few things inline.
> Thanks,
> 
> > diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> > index e55a69c62694a71a4e29f29b9a2bfeec3b16c990..40b7c10b8ce7145b010bb11e8e4698baacb6b3d3 100644
> > --- a/drivers/iio/adc/ad4062.c
> > +++ b/drivers/iio/adc/ad4062.c
> 
> > +static irqreturn_t ad4062_poll_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *indio_dev = pf->indio_dev;
> > +	struct ad4062_state *st = iio_priv(indio_dev);
> > +	u8 addr = AD4062_REG_CONV_TRIGGER;
> > +	int ret;
> > +
> > +	/* Read current and trigger next conversion */
> > +	struct i3c_priv_xfer t[2] = {
> > +		{
> > +			.data.in = &st->raw,
> 
> If it is safe to use addr on the stack, do we need to have
> a dma safe buffer for raw?  I'm not sure for i3c!
> 
All buffer should be dma aligned, I will use the heap.
I will use a separate buffer that is written once (CONV_READ or
CONV_TRIGGER, depending if gp1 is routed or using the IBI fallback).
> > +			.len = 4,
> > +			.rnw = true,
> > +		},
> > +		{
> > +			.data.out = &addr,
> > +			.len = 1,
> > +			.rnw = false,
> > +		}
> > +	};
> > +
> > +	/* Separated transfers to not infeer repeated-start */
> > +	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[0], 1);
> > +	if (ret)
> > +		goto out;
> > +	ret = i3c_device_do_priv_xfers(st->i3cdev, &t[1], 1);
> 
> Add a comment on this. I assume it's setting things up for the next
> scan?
> 
yes, the next scan is triggered after the reading of the current scan.
There are 2 registers that can be used for scans, CONV_READ and
CONV_TRIGGER:
* CONV_READ: Stop bit of the previous read (without write address),
  triggers the next scan. Allows roughly twice the sample rate, since
  does not requires writing the address every transfer.
* CONV_TRIGGER: The conversion is triggered by writing the address, so
  every new conversion requires writing the address again. Only this
  registers will issue an IBI data ready.

That means, if GPIO is not routed as the IRQ, in the fallback using IBI,
CONV_TRIGGER needs to be used. v2 will also use schedule_work to avoid
the IRQF_ONESHOT being triggered (next conversion data ready), before
the current irq_handler returns.
> > +	if (ret)
> > +		goto out;
> > +
> > +	iio_push_to_buffers_with_timestamp(indio_dev, &st->raw,
> > +					   pf->timestamp);
> > +
> > +out:
> > +	iio_trigger_notify_done(indio_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> >  }
> 
> > +
> > +static int ad4062_triggered_buffer_predisable(struct iio_dev *indio_dev)
> > +{
> > +	struct ad4062_state *st = iio_priv(indio_dev);
> > +
> > +	pm_runtime_mark_last_busy(&st->i3cdev->dev);
> 
> Take a look at the changes across the tree recently.
> Now pm_runtime_put_autosuspend() calls pm_runtime_mark_last_busy()
> internally to avoid the need for this pair.
> 
Ack.
> > +	pm_runtime_put_autosuspend(&st->i3cdev->dev);
> > +	return 0;
> > +}
> 
> 
Best regards,
Jorge