Re: [PATCH 3/7] iio: Add in-kernel API for events

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2025-07-14 at 21:20 -0400, Sean Anderson wrote:
> Add an API to notify consumers about events. Events still need to be
> enabled using the iio_read_event/iio_write_event functions. Of course,
> userspace can also manipulate the enabled events. I don't think this is
> too much of an issue, since userspace can also manipulate the event
> thresholds. But enabling events may cause existing programs to be
> surprised when they get something unexpected. Maybe we should set the
> interface as busy when there are any in-kernel listeners?
> 

Sensible question. I'm not that familiar with events but I suspect is not
trivial (if doable) to do a similar approach as with buffers? With buffers, an
inkernal consumer get's it's own buffer object (that goes into a list of active
buffers in the iio device) with all channels enabled and then we demux the
appropriate channels for each consumer.

Independent of the above, we can argue that having both inkernel and userspace
changing thresholds is ok (I mean, there's nothing stopping two userspace apps
doing that) but we should likely be careful with enabling/disabling. If multiple
consumers enable the same event, one of them disabling it should not disable it
for all the consumers, right?

- Nuno Sá

> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
>  drivers/iio/industrialio-event.c | 34 +++++++++++++++++++++++++++-----
>  include/linux/iio/consumer.h     | 30 ++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-
> event.c
> index 06295cfc2da8..b9e3ff1cd5c9 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -12,11 +12,13 @@
>  #include <linux/kernel.h>
>  #include <linux/kfifo.h>
>  #include <linux/module.h>
> +#include <linux/notifier.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/wait.h>
> +#include <linux/iio/consumer.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/iio-opaque.h>
>  #include "iio_core.h"
> @@ -26,6 +28,7 @@
>  /**
>   * struct iio_event_interface - chrdev interface for an event line
>   * @wait:		wait queue to allow blocking reads of events
> + * @notifier:		notifier head for in-kernel event listeners
>   * @det_events:		list of detected events
>   * @dev_attr_list:	list of event interface sysfs attribute
>   * @flags:		file operations related flags including busy flag.
> @@ -35,6 +38,7 @@
>   */
>  struct iio_event_interface {
>  	wait_queue_head_t	wait;
> +	struct atomic_notifier_head notifier;
>  	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>  
>  	struct list_head	dev_attr_list;
> @@ -67,18 +71,19 @@ 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_data ev;
> +	struct iio_event_data ev = {
> +		.id = ev_code,
> +		.timestamp = timestamp,
> +	};
>  	int copied;
>  
>  	if (!ev_int)
>  		return 0;
>  
> +	atomic_notifier_call_chain(&ev_int->notifier, IIO_NOTIFY_EVENT, &ev);
> +
>  	/* Does anyone care? */
>  	if (iio_event_enabled(ev_int)) {
> -
> -		ev.id = ev_code;
> -		ev.timestamp = timestamp;
> -
>  		copied = kfifo_put(&ev_int->det_events, ev);
>  		if (copied != 0)
>  			wake_up_poll(&ev_int->wait, EPOLLIN);
> @@ -223,6 +228,25 @@ static int iio_event_getfd(struct iio_dev *indio_dev)
>  	return fd;
>  }
>  
> +int iio_event_register(struct iio_dev *indio_dev, struct notifier_block
> *block)
> +{
> +	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
> +
> +	return atomic_notifier_chain_register(&ev_int->notifier, block);
> +}
> +EXPORT_SYMBOL_GPL(iio_event_register);
> +
> +void iio_event_unregister(struct iio_dev *indio_dev,
> +			  struct notifier_block *block)
> +{
> +	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
> +
> +	WARN_ON(atomic_notifier_chain_unregister(&ev_int->notifier, block));
> +}
> +EXPORT_SYMBOL_GPL(iio_event_unregister);
> +
>  static const char * const iio_ev_type_text[] = {
>  	[IIO_EV_TYPE_THRESH] = "thresh",
>  	[IIO_EV_TYPE_MAG] = "mag",
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 16e7682474f3..9918e3f7af3d 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -507,4 +507,34 @@ int iio_write_event_processed_scale(struct iio_channel
> *chan,
>  				    enum iio_event_info info, int processed,
>  				    unsigned int scale);
>  
> +struct notifier_block;
> +enum iio_notifier_val {
> +	/** IIO_NOTIFY_EVENT: v is a pointer to &struct iio_event_data */
> +	IIO_NOTIFY_EVENT,
> +};
> +
> +/**
> + * iio_event_register() - Register a notifier for events
> + * @indio_dev: Device to be notified of events on
> + * @block: Notifier block to register
> + *
> + * Register a notifier for events on @indio_dev. @v will be a member of &enum
> + * iio_notifier_val. Notifiers will be called in atomic context. @indio_dev
> + * must stay valid until you call iio_event_unregister().
> + *
> + * Return: 0 on success, or -EEXIST if @block has already been registered
> + */
> +int iio_event_register(struct iio_dev *indio_dev,
> +		       struct notifier_block *block);
> +
> +/**
> + * iio_event_unregister() - Remove a previously-added notifier
> + * @indio_dev: Device to be notified of events on
> + * @block: Notifier previously-registered with iio_event_register()
> + *
> + * Remove a previously-added notifier.
> + */
> +void iio_event_unregister(struct iio_dev *indio_dev,
> +			  struct notifier_block *block);
> +
>  #endif