mbox series

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

Message ID 20240128150537.44592-1-jic23@kernel.org (mailing list archive)
Headers show
Series IIO: Use the new cleanup.h magic | expand

Message

Jonathan Cameron Jan. 28, 2024, 3:05 p.m. UTC
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(-)

Comments

Nuno Sá Jan. 30, 2024, 2:08 p.m. UTC | #1
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>
Jonathan Cameron Feb. 4, 2024, 4:06 p.m. UTC | #2
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