Re: [PATCH v4 2/2] iio: temperature: add support for Analog Devices MAX30210

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 15 Oct 2026 09:21:59 +0800
John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com> wrote:

> Add support for the Analog Devices MAX30210 I2C temperature
> sensor.
> 
> The driver uses regmap for register access and integrates with
> the IIO framework. It supports:
> 
> - Direct mode temperature conversion
> - Configurable sampling frequency
> - Threshold events
> - FIFO operation with IIO kfifo buffer support
> - Optional interrupt-driven data ready signaling
> 
> The device provides 16-bit signed temperature data and a
> 64-sample FIFO.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>

Josh already pointed out the sashiko feedback so I'll just
assume you'll either fix that or reply with why you disagree.

This is showing up just how bad our event handling infrastructure
is wrt to lifetimes.  Anyhow, I took a look at that and think
the right solution is to just not worry so much about a bit of
expansion in struct iio_dev_opaque and just always embed the
iio_event_interface.

That way we need much simpler ordering guarantee that interrupts
are disable before the iio_dev is freed.  That should always be
the case.

Jonathan

> diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
> new file mode 100644
> index 0000000000000..04123d51c6da4
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,788 @@

> +
> +/* Keeps the device sampling autonomously while buffer and/or events need it. */
> +static int max30210_autoconv_get(struct max30210_state *st)
> +{
> +	int ret = 0;
> +
> +	guard(mutex)(&st->lock);
> +
> +	if (st->autoconv_users == 0)
> +		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +				   MAX30210_TEMPCONV_AUTO | MAX30210_TEMPCONV_CONV_T);
Errors out of line where ever possible.
	if (ret)
		return ret;

	st->autconv_users++;

	return 0;

Though note sashiko raised some concerns around this. I haven't looked
closely at whether it is right that you can increment this for ever.

> +	if (!ret)
> +		st->autoconv_users++;
> +
> +	return ret;
> +}

> +
> +static irqreturn_t max30210_irq_handler(int irq, void *dev_id)
> +{
> +	struct iio_dev *indio_dev = dev_id;
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int status;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Status read failed\n");
> +		return IRQ_NONE;
> +	}
> +
> +	if (status & MAX30210_STATUS_A_FULL_MASK)
> +		max30210_fifo_read(indio_dev);
> +
> +	if (status & MAX30210_STATUS_TEMP_HI_MASK)
> +		iio_push_event(indio_dev,

So it's this call I assume is the root of the issue with events due
to interrupts in the tear down path.

I think we need to fix the core lifetime issue rather than making
it a driver problem to paper over it.  Note however that drivers
should still be doing what ever is necessary to disable interrupts
at source.  Sometimes that happens naturally due to power down. Other
times you have to register specific cleanup routines to do that.

I'm not sure which is true here.

> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +			       iio_get_time_ns(indio_dev));
> +
> +	if (status & MAX30210_STATUS_TEMP_LO_MASK)
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_FALLING),
> +			       iio_get_time_ns(indio_dev));
> +
> +	return IRQ_HANDLED;
> +}
>

> +
> +static int max30210_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int uval;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 5;
> +		*val2 = MILLI;
> +
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_SAMP_FREQ: {
> +		unsigned int period;
> +
> +		ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
> +		if (ret)
> +			return ret;
> +
> +		period = FIELD_GET(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, uval);
> +
> +		/*
> +		 * TEMP_PERIOD is an index into the sampling frequency lookup
> +		 * table. Clamp in case the register holds a reserved value.
> +		 */
> +		if (period >= ARRAY_SIZE(max30210_samp_freq_avail))
> +			period = ARRAY_SIZE(max30210_samp_freq_avail) - 1;

		period = min(period, ARRAY_SIZE(max30210_samp_freq_avail) - 1);

> +
> +		*val = max30210_samp_freq_avail[period][0];
> +		*val2 = max30210_samp_freq_avail[period][1];
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	case IIO_CHAN_INFO_RAW: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		guard(mutex)(&st->lock);
> +
> +		/*
> +		 * TEMP_DATA_REG is only refreshed by a single-shot
> +		 * conversion; the buffer/FIFO path is the only thing that
> +		 * gets updated during AUTO mode. If AUTO is already running
> +		 * for the buffer and/or events, triggering a one-off
> +		 * conversion here would stomp the AUTO bit out from under
> +		 * whoever enabled it, and there is no way to get a fresh
> +		 * single-shot value without doing so -- refuse instead of
> +		 * returning stale data.
> +		 */
> +		if (st->autoconv_users > 0)
> +			return -EBUSY;
> +
> +		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +				   MAX30210_TEMPCONV_CONV_T);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Wait until CONVERT_T auto-clears.
> +		 * Datasheet:
> +		 *   tBIAS_WU = 260 µs
> +		 *   tINT     = 8 ms
> +		 *
> +		 * Worst-case conversion ≈ 8.26 ms.
> +		 * Use 10 ms timeout for margin.
> +		 */
> +		ret = regmap_read_poll_timeout(st->regmap, MAX30210_TEMP_CONV_REG, uval,
> +					       !(uval & MAX30210_TEMPCONV_CONV_T),
> +					       500, 10 * USEC_PER_MSEC);
> +		if (ret)
> +			return ret;
> +
> +		return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static int max30210_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct max30210_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);
For a modern driver we pretty much always do
	ret = devm_mutex_init(dev, &st->lock);
	if (ret)
		return ret;

It only brings a bit of extra lock debug but it's simple so why not.

> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to enable vdd regulator.\n");
> +
> +	st->regmap = devm_regmap_init_i2c(client, &max30210_regmap);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to allocate regmap.\n");
> +
> +	st->watermark = MAX30210_WATERMARK_DEFAULT;
> +
> +	ret = max30210_setup(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &max30210_channel;
> +	indio_dev->num_channels = 1;
> +	indio_dev->name = "max30210";
> +	indio_dev->info = &max30210_info;
> +
> +	/* Without an interrupt there is no way to drain the FIFO into the buffer */

Maybe stick "currently" in that statement given you can just poll.

> +	if (client->irq) {
> +		ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev, &max30210_buffer_ops,
> +						      max30210_fifo_attributes);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	if (!client->irq)
> +		return 0;
> +
> +	/*
> +	 * Request the IRQ after registering the IIO device so devm tears it
> +	 * down first on removal, before the event interface is freed.

Ok. I can see the logic in this, but I'm not particularly keen on the
position where we have a device up with exposed interfaces, some of which
would expect to generate interrupts, but we don't have interrupt support.

I think it is worth looking at whether we can solve this cleanly in the IIO
core.  As you observe, the issue is that event_interface is allocated
and freed as part of the device_register() on basis we don't need it
if there are no events. That smells to me like a design error (mine
long ago!).   Maybe we should just make it a part of the iio_dev_opaque?
Small amount of bloat as a result but it isn't huge.

That way, it's lifetime matches the device and it will
be around much longer.  Once that is true we can add a flag that indicates
the device is being torn down and short cut dropping events (as by
definition they aren't going anywhere) but not worry if we race
because it should be harmless to carry on using the event interface.

We can't use the info_exist_lock here as the events might be pushed
from an interrupt so fixing this up in similar fashion to the buffer
state management doesn't work.

Note drivers should still be disabling their interrupts at source
even if becomes safer to not do so.

I hacked something up to get an idea of how bad this would be to actually do.
The header dependencies will require yet another header which is a bit annoying
but such is life.  This is build tested only.  If you want to
give it a spin that would be great.  The new header naming needs some more
thought.  At somepoint I should also fix up that iio_opaque is currently
extremely ordering dependent as it doesn't include headers needed
for the stuff it defines.  That was kind of intentional but now perhaps
we need to rethink.

From 6c3ac6b288db7d5840298649d34bf289be2a5a74 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Date: Sat, 17 Oct 2026 22:52:57 +0100
Subject: [PATCH] TEMP HACK

Stop allocating the event_interface independently of the device as
it makes for some annoying lifetime issues.

Note that until someone actually attaches to the kfifo no events
get stored anyway.

---
 drivers/iio/iio_core.h           |  8 ++---
 drivers/iio/industrialio-core.c  | 11 +++---
 drivers/iio/industrialio-event.c | 57 ++++++++------------------------
 drivers/iio/inkern.c             |  2 ++
 include/linux/iio/iio-opaque.h   | 28 +++++++++++++++-
 include/linux/iio/iio-types.h    | 14 ++++++++
 6 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 59fa1bd9924b..92fa47f74393 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -12,6 +12,8 @@
 #include <linux/kernel.h>
 #include <linux/device.h>
 
+#include <linux/iio/iio-types.h>
+
 struct iio_buffer;
 struct iio_chan_spec;
 struct iio_dev;
@@ -24,11 +26,6 @@ struct iio_dev_buffer_pair {
 };
 
 #define IIO_IOCTL_UNHANDLED	1
-struct iio_ioctl_handler {
-	struct list_head entry;
-	long (*ioctl)(struct iio_dev *indio_dev, struct file *filp,
-		      unsigned int cmd, unsigned long arg);
-};
 
 void iio_device_ioctl_handler_register(struct iio_dev *indio_dev,
 				       struct iio_ioctl_handler *h);
@@ -61,6 +58,7 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
 
 /* Event interface flags */
 #define IIO_BUSY_BIT_POS 1
+#define IIO_EVENTS_PRESENT 2
 
 #ifdef CONFIG_IIO_BUFFER
 struct poll_table_struct;
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d73f2541375f..e109bc190716 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -345,13 +345,12 @@ int iio_device_set_clock(struct iio_dev *indio_dev, clockid_t clock_id)
 {
 	int ret;
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	const struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	const struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 
 	ret = mutex_lock_interruptible(&iio_dev_opaque->mlock);
 	if (ret)
 		return ret;
-	if ((ev_int && iio_event_enabled(ev_int)) ||
-	    iio_buffer_enabled(indio_dev)) {
+	if (iio_event_enabled(ev_int) || iio_buffer_enabled(indio_dev)) {
 		mutex_unlock(&iio_dev_opaque->mlock);
 		return -EBUSY;
 	}
@@ -1640,7 +1639,7 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
 			attrcount += ret;
 		}
 
-	if (iio_dev_opaque->event_interface)
+	if (test_bit(IIO_EVENTS_PRESENT, &iio_dev_opaque->event_interface.flags))
 		clk = &dev_attr_current_timestamp_clock.attr;
 
 	if (indio_dev->name)
@@ -2165,10 +2164,10 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 
 	if (iio_dev_opaque->attached_buffers_cnt)
 		cdev_init(&iio_dev_opaque->chrdev, &iio_buffer_fileops);
-	else if (iio_dev_opaque->event_interface)
+	else if (test_bit(IIO_EVENTS_PRESENT, &iio_dev_opaque->event_interface.flags))
 		cdev_init(&iio_dev_opaque->chrdev, &iio_event_fileops);
 
-	if (iio_dev_opaque->attached_buffers_cnt || iio_dev_opaque->event_interface) {
+	if (iio_dev_opaque->attached_buffers_cnt || test_bit(IIO_EVENTS_PRESENT, &iio_dev_opaque->event_interface.flags)) {
 		indio_dev->dev.devt = MKDEV(MAJOR(iio_devt), iio_dev_opaque->id);
 		iio_dev_opaque->chrdev.owner = this_mod;
 	}
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index e6730f52262a..5e37af966002 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -23,27 +23,6 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
 
-/**
- * struct iio_event_interface - chrdev interface for an event line
- * @wait:		wait queue to allow blocking reads of events
- * @det_events:		list of detected events
- * @dev_attr_list:	list of event interface sysfs attribute
- * @flags:		file operations related flags including busy flag.
- * @group:		event interface sysfs attribute group
- * @read_lock:		lock to protect kfifo read operations
- * @ioctl_handler:	handler for event ioctl() calls
- */
-struct iio_event_interface {
-	wait_queue_head_t	wait;
-	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
-
-	struct list_head	dev_attr_list;
-	unsigned long		flags;
-	struct attribute_group	group;
-	struct mutex		read_lock;
-	struct iio_ioctl_handler	ioctl_handler;
-};
-
 bool iio_event_enabled(const struct iio_event_interface *ev_int)
 {
 	return !!test_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
@@ -66,7 +45,7 @@ bool iio_event_enabled(const struct iio_event_interface *ev_int)
 int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 	struct iio_event_data ev;
 	int copied;
 
@@ -101,7 +80,7 @@ static __poll_t iio_event_poll(struct file *filep,
 {
 	struct iio_dev *indio_dev = filep->private_data;
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 	__poll_t events = 0;
 
 	if (!indio_dev->info)
@@ -122,7 +101,7 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
 {
 	struct iio_dev *indio_dev = filep->private_data;
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 	unsigned int copied;
 	int ret;
 
@@ -172,7 +151,7 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
 {
 	struct iio_dev *indio_dev = filep->private_data;
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 
 	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
 
@@ -192,7 +171,7 @@ static const struct file_operations iio_event_chrdev_fileops = {
 static int iio_event_getfd(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 	int fd;
 
 	if (ev_int == NULL)
@@ -422,7 +401,7 @@ static int iio_device_add_event(struct iio_dev *indio_dev,
 		ret = __iio_add_chan_devattr(postfix, chan, show, store,
 			 (i << 16) | spec_index, shared_by, &indio_dev->dev,
 			 NULL,
-			&iio_dev_opaque->event_interface->dev_attr_list);
+			&iio_dev_opaque->event_interface.dev_attr_list);
 		kfree(postfix);
 
 		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
@@ -462,7 +441,7 @@ static int iio_device_add_event_label(struct iio_dev *indio_dev,
 
 	ret = __iio_add_chan_devattr(postfix, chan, &iio_ev_label_show, NULL,
 				spec_index, IIO_SEPARATE, &indio_dev->dev, NULL,
-				&iio_dev_opaque->event_interface->dev_attr_list);
+				&iio_dev_opaque->event_interface.dev_attr_list);
 
 	kfree(postfix);
 
@@ -574,7 +553,7 @@ static const char *iio_event_group_name = "events";
 int iio_device_register_eventset(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 	struct iio_dev_attr *p;
 	int ret = 0, attrcount_orig = 0, attrcount, attrn;
 	struct attribute **attr;
@@ -583,12 +562,6 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
 	      iio_check_for_dynamic_events(indio_dev)))
 		return 0;
 
-	ev_int = kzalloc_obj(*ev_int);
-	if (!ev_int)
-		return -ENOMEM;
-
-	iio_dev_opaque->event_interface = ev_int;
-
 	INIT_LIST_HEAD(&ev_int->dev_attr_list);
 
 	iio_setup_ev_int(ev_int);
@@ -625,6 +598,7 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
 	if (ret)
 		goto error_free_group_attrs;
 
+	__set_bit(IIO_EVENTS_PRESENT, &ev_int->flags);
 	ev_int->ioctl_handler.ioctl = iio_event_ioctl;
 	iio_device_ioctl_handler_register(&iio_dev_opaque->indio_dev,
 					  &ev_int->ioctl_handler);
@@ -635,8 +609,6 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
 	kfree(ev_int->group.attrs);
 error_free_setup_event_lines:
 	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
-	kfree(ev_int);
-	iio_dev_opaque->event_interface = NULL;
 	return ret;
 }
 
@@ -650,23 +622,22 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
 void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 
-	if (iio_dev_opaque->event_interface == NULL)
+	if (!test_bit(IIO_EVENTS_PRESENT, &ev_int->flags))
 		return;
-	wake_up(&iio_dev_opaque->event_interface->wait);
+	wake_up(&ev_int->wait);
 }
 
 void iio_device_unregister_eventset(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
-	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
+	struct iio_event_interface *ev_int = &iio_dev_opaque->event_interface;
 
-	if (ev_int == NULL)
+	if (!test_bit(IIO_EVENTS_PRESENT, &ev_int->flags))
 		return;
 
 	iio_device_ioctl_handler_unregister(&ev_int->ioctl_handler);
 	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
 	kfree(ev_int->group.attrs);
-	kfree(ev_int);
-	iio_dev_opaque->event_interface = NULL;
 }
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 1f22275de55c..a97fb9aded8e 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -14,6 +14,8 @@
 #include <linux/units.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/events.h>
+/* ordering matters */
 #include <linux/iio/iio-opaque.h>
 #include "iio_core.h"
 #include <linux/iio/machine.h>
diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio-opaque.h
index 98330385e08d..91934621326b 100644
--- a/include/linux/iio/iio-opaque.h
+++ b/include/linux/iio/iio-opaque.h
@@ -3,6 +3,32 @@
 #ifndef _INDUSTRIAL_IO_OPAQUE_H_
 #define _INDUSTRIAL_IO_OPAQUE_H_
 
+
+#include <linux/kfifo.h>
+#include <linux/iio/iio-types.h>
+#include <linux/iio/events.h>
+
+/**
+ * struct iio_event_interface - chrdev interface for an event line
+ * @wait:		wait queue to allow blocking reads of events
+ * @det_events:		list of detected events
+ * @dev_attr_list:	list of event interface sysfs attribute
+ * @flags:		file operations related flags including busy flag.
+ * @group:		event interface sysfs attribute group
+ * @read_lock:		lock to protect kfifo read operations
+ * @ioctl_handler:	handler for event ioctl() calls
+ */
+struct iio_event_interface {
+	wait_queue_head_t	wait;
+	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
+
+	struct list_head	dev_attr_list;
+	unsigned long		flags;
+	struct attribute_group	group;
+	struct mutex		read_lock;
+	struct iio_ioctl_handler	ioctl_handler;
+};
+
 /**
  * struct iio_dev_opaque - industrial I/O device opaque information
  * @indio_dev:			public industrial I/O device information
@@ -42,6 +68,7 @@
  */
 struct iio_dev_opaque {
 	struct iio_dev			indio_dev;
+	struct iio_event_interface	event_interface;
 	int				currentmode;
 	int				id;
 	struct module			*driver_module;
@@ -50,7 +77,6 @@ struct iio_dev_opaque {
 	struct mutex			info_exist_lock;
 	struct lock_class_key		info_exist_key;
 	bool				trig_readonly;
-	struct iio_event_interface	*event_interface;
 	struct iio_buffer		**attached_buffers;
 	unsigned int			attached_buffers_cnt;
 	struct iio_ioctl_handler	*buffer_ioctl_handler;
diff --git a/include/linux/iio/iio-types.h b/include/linux/iio/iio-types.h
new file mode 100644
index 000000000000..b18a1f29a936
--- /dev/null
+++ b/include/linux/iio/iio-types.h
@@ -0,0 +1,14 @@
+#ifndef _IIO_INTERNAL_TYPES_H_
+#define  _IIO_INTERNAL_TYPES_H_
+#include <linux/list.h>
+
+struct iio_dev;
+struct file;
+
+struct iio_ioctl_handler {
+	struct list_head entry;
+	long (*ioctl)(struct iio_dev *indio_dev, struct file *filp,
+		      unsigned int cmd, unsigned long arg);
+};
+
+#endif /* _IIO_INTERNAL_TYPES_H_ */
-- 
2.56.0




> +	 */
> +	return devm_request_threaded_irq(dev, client->irq, NULL,
> +					 max30210_irq_handler, IRQF_ONESHOT,
> +					 indio_dev->name, indio_dev);
> +}