Message-ID: <20250727172126.35d0a477@jic23-huawei>
Date: Sun, 27 Jul 2025 17:21:26 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Nuno Sá <noname.nuno@...il.com>, Jean Delvare
 <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
 linux-iio@...r.kernel.org, linux-hwmon@...r.kernel.org, Andy Shevchenko
 <andy@...nel.org>, Nuno Sá <nuno.sa@...log.com>,
 linux-kernel@...r.kernel.org, David Lechner <dlechner@...libre.com>
Subject: Re: [PATCH 3/7] iio: Add in-kernel API for events

On Tue, 15 Jul 2025 12:52:19 -0400
Sean Anderson <sean.anderson@...ux.dev> wrote:

> On 7/15/25 07:09, Nuno Sá wrote:
> > 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.  
> 
> For in-kernel consumers I think it's reasonable to expect them to handle
> events they didn't explicitly enable. I'm not sure about userspace
> consumers.

This already happens because we don't have a demux equivalent (what we do
for buffered data flow) so if a device only has a single enable bit that covers
multiple events (annoyingly common for accelerometers for example) then
userspace will get events it didn't ask for.   We 'could' fix that,
but it's never really been worth the effort.

Events tend to be low data rate so an occasionally extra is rather different
to having to have much larger data buffers to handle a range of channels you
never asked for.

Lets be careful to document this behaviour as 'may enable extra events'
as then if we decide later to do demux type stuff we won't be breaking ABI.
No one will mind getting fewer spurious events due to a core improvement.

> 
> > 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?  
> 
> Right now the HWMON consumer never permanently disable events to avoid this
> issue. It does toggle the enable to determine if an alarm should stay
> enabled:
>              ________
> condition __/        \________
>           _____    ____    ___
> enable         \__/    \__/
> 
> event       |     |
>              __    ____
> alarm     __/  \__/    \_____
> 
> read           1       1    0
> 
> I suppose this could also be done by comparing the raw threshold to the
> channel.

I wonder if we should add the option to do a 'get_exclusive' or similar
to block the IIO user interfaces if something critical is using the device.

If we were for instance to use this to block the IOCTL to get the events
fd then any built in driver etc will almost certainly load before anyone
can call the ioctl so it will fairly cleanly block things.

Jonathan

Powered by blists - more mailing lists