Re: [PATCH] iio: gyro: bmg160: reject duplicate event disable

[Jonathan Cameron]

On Sat, 26 Sep 2026 02:01:33 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Fri, 25 Sep 2026 22:33:48 +0800 (CST)
> "jiale yao" <19888972804@xxxxxxx> wrote:
> 
> > At 2026-09-25 22:18:05, "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxx> wrote:  
> > >On Fri, Sep 25, 2026 at 10:09:54PM +0800, Jiale Yao wrote:    
> > >> The IIO core does not filter duplicate writes to the event enable
> > >> attribute.    
> > >
> > >Can it be done there once for all?    
> > 
> > I don't think the core can safely do this generically, since it doesn't own
> > the per-event state and read_event_config() may reflect shared hardware state.
> > 
> > The runtime PM accounting is driver-specific, so handling duplicate writes in
> > the driver seems safer. gp2ap002 does the same in commit 579c049b4cb6,
> > refer https://lore.kernel.org/all/20260720193911.74919-2-nikhilgtr@xxxxxxxxx/  
> 
> Exactly right.
> 
> Event handling in the core would indeed require complex mapping of
> what can actually be enabled on a given device. Some of those we might
> be able to represent with simple approaches similar to what we do
> for avail_scan_masks for buffered channels but it is hard to generalize
> and FWIW that concept still misses some corner cases of channel enabling
> where drivers have to apply constraints in the actual buffer enable.
> Those are rare enough that it isn't too much of a problem in practice.
> Probably fine on 99% of drivers.
> 
> Events have simplre cases like:
> - 1 enable bit turns a whole load of events on (we don't filter in
>   the kernel - so userspace has to cope with surprise events).
> - Always on event signals.
> - Limited numbers of general purpose event engines (the ADI IMUs do
>   this as they tend to have 2 instances). Each engine can do anything
>   but there are only two of them.
> 
> We also have more complex ones like mode bits that flip a subset
> of events from one enable state to another.  There are some really
> odd rules in some devices because some upstream filter that is needed
> for 5 seemingly unrelated events has to be in a particular state for
> subsets of them.
> 
> Anyhow, all in all event control is a mess of 'original solutions'
> from vendors and relies on the (naughty) get out of IIO ABI that
> just occasionally any write to any userspace ABI element can
> change what is read back from any other.
> 
> Not great, but I've never come up with a better solution for events.
> If anyone wants to have a go at architecting a filtering solution
> a bit like the IIO buffer demux maybe we can opt in to it from
> some drivers.
> 
> I am wondering if it is worth registering a driver specific tidy
> up events call though that would disable everything a bit like
> we disable buffers as part of the userspace tear down.  That would
> close out some races sashiko has reported.  That project would
> I think be a lot simpler to implement.
> 
This one needs a fixes tag, as do the other similar ones.
Fine to just reply to each thread with the appropriate tag.

Thanks,

Jonathan

> Jonathan
> 
> >   
> > >    
> > >> bmg160_write_event_config() already ignores repeated enable
> > >> requests, but a repeated disable request still calls
> > >> bmg160_set_power_state(data, false), dropping a runtime PM reference
> > >> that was not acquired for this request. This can underflow the runtime
> > >> PM usage count and trigger a "Runtime PM usage count underflow" warning.
> > >> 
> > >> Return early when the requested state already matches ev_enable_state.    
> > >
> > >-- 
> > >With Best Regards,
> > >Andy Shevchenko
> > >    
> 
>