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

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 28 Sep 2026 17:22:09 +0800 (CST)
"jiale yao" <19888972804@163.com> wrote:

> At 2026-09-28 01:16:32, "Jonathan Cameron" <jic23@kernel.org> wrote:
> >On Sat, 26 Sep 2026 02:01:33 +0100
> >Jonathan Cameron <jic23@kernel.org> wrote:
> >  
> >> On Fri, 25 Sep 2026 22:33:48 +0800 (CST)
> >> "jiale yao" <19888972804@163.com> wrote:
> >>   
> >> > At 2026-09-25 22:18:05, "Andy Shevchenko" <andriy.shevchenko@intel.com> 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@gmail.com/    
> >> 
> >> 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.  
> 
> Fixes: 22b46c45fb9b ("iio:gyro:bmg160 Gyro Sensor driver")
> 
Applied to the fixes-togreg branch of iio.git and marked for stable.
Note that I'll be rebasing that on rc1 (once available) before sending
out a pull request.

Thanks,

Jonathan

> >
> >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
> >> > >      
> >> 
> >>