Re: [PATCH 00/10] IIO: Use the new cleanup.h magic

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 30 Jan 2024 15:08:39 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sun, 2024-01-28 at 15:05 +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > The prerequisites are now in place upstream, so this series can now
> > introduce the infrastructure and apply it to a few drivers.
> > 
> > Changes since RFC v2: Thanks to David Lechner for review
> >  - Use unreachable() instead of misleading returns in paths we can't reach.
> >  - Various minor tweaks and local variable scope reduction.
> >  
> > A lot of the advantages of the automated cleanup added for locks and similar
> > are not that useful in IIO unless we also deal with the
> > iio_device_claim_direct_mode() / iio_device_release_direct_mode()
> > calls that prevent IIO device drivers from transitioning into buffered
> > mode whilst calls are in flight + prevent sysfs reads and writes from
> > interfering with buffered capture if it is enabled.
> > 
> > This can now be neatly done using new scoped_cond_guard() to elegantly
> > return if the attempt to claim direct mode fails.
> > 
> > The need to always handle what happens after
> > iio_device_claim_direct_scoped() {} is a little irritating but the
> > compiler will warn if you don't do it and it's not obvious how to
> > let the compiler know the magic loop (hidden in the cleanup.h macros)
> > always runs once.  Example:
> > 
> > 	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> > 		return 42;
> > 	}
> > 	/* Can't get here, but compiler about no return val without this */
> > 	unreachable();
> > }
> > 
> > Jonathan Cameron (10):
> >   iio: locking: introduce __cleanup() based direct mode claiming
> >     infrastructure
> >   iio: dummy: Use automatic lock and direct mode cleanup.
> >   iio: accel: adxl367: Use automated cleanup for locks and iio direct
> >     mode.
> >   iio: imu: bmi323: Use cleanup handling for
> >     iio_device_claim_direct_mode()
> >   iio: adc: max1363: Use automatic cleanup for locks and iio mode
> >     claiming.
> >   iio: proximity: sx9360: Use automated cleanup for locks and IIO mode
> >     claiming.
> >   iio: proximity: sx9324: Use automated cleanup for locks and IIO mode
> >     claiming.
> >   iio: proximity: sx9310: Use automated cleanup for locks and IIO mode
> >     claiming.
> >   iio: adc: ad4130: Use automatic cleanup of locks and direct mode.
> >   iio: adc: ad7091r-base: Use auto cleanup of locks.
> > 
> >  drivers/iio/accel/adxl367.c          | 297 +++++++++++----------------
> >  drivers/iio/adc/ad4130.c             | 131 +++++-------
> >  drivers/iio/adc/ad7091r-base.c       |  25 +--
> >  drivers/iio/adc/max1363.c            | 171 +++++++--------
> >  drivers/iio/dummy/iio_simple_dummy.c | 182 ++++++++--------
> >  drivers/iio/imu/bmi323/bmi323_core.c |  78 +++----
> >  drivers/iio/proximity/sx9310.c       | 114 ++++------
> >  drivers/iio/proximity/sx9324.c       | 109 ++++------
> >  drivers/iio/proximity/sx9360.c       | 115 ++++-------
> >  include/linux/iio/iio.h              |  25 +++
> >  10 files changed, 518 insertions(+), 729 deletions(-)
> >   
> 
> 
> Just one comment that boils down to preference... So, LGTM:
> 
> Reviewed-by: Nuno Sa <nuno.a@analog.com>
> 
Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to poke at it. I tweaked patch 2 as discussed in the thread.

Thanks!

Jonathan