Re: [PATCH v2 24/27] iio: dummy: Stop using iio_device_claim_direct_scoped()

["Nuno Sá" <noname.nuno@gmail.com>]

On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This complex cleanup.h use case of conditional guards has proved
> to be more trouble that it is worth in terms of false positive compiler
> warnings and hard to read code.
> 
> Move directly to the new claim/release_direct() that allow sparse
> to check for unbalanced context. Introduce two new utility functions
> to allow for direct returns with claim and release of direct mode
> in the caller.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dummy/iio_simple_dummy.c | 119 ++++++++++++++++-----------
>  1 file changed, 70 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/iio/dummy/iio_simple_dummy.c
> b/drivers/iio/dummy/iio_simple_dummy.c
> index 09efacaf8f78..8575d4a08963 100644
> --- a/drivers/iio/dummy/iio_simple_dummy.c
> +++ b/drivers/iio/dummy/iio_simple_dummy.c
> @@ -267,6 +267,65 @@ static const struct iio_chan_spec iio_dummy_channels[] =
> {
>  	},
>  };
>  
> +static int __iio_dummy_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val)
> +{
> +	struct iio_dummy_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		if (chan->output) {
> +			/* Set integer part to cached value */
> +			*val = st->dac_val;
> +			return IIO_VAL_INT;
> +		} else if (chan->differential) {
> +			if (chan->channel == 1)
> +				*val = st->differential_adc_val[0];
> +			else
> +				*val = st->differential_adc_val[1];
> +			return IIO_VAL_INT;
> +		} else {
> +			*val = st->single_ended_adc_val;
> +			return IIO_VAL_INT;
> +		}
> +
> +	case IIO_ACCEL:
> +		*val = st->accel_val;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int __iio_dummy_read_processed(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      int *val)
> +{
> +	struct iio_dummy_state *st = iio_priv(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +	switch (chan->type) {
> +	case IIO_STEPS:
> +		*val = st->steps;
> +		return IIO_VAL_INT;
> +	case IIO_ACTIVITY:
> +		switch (chan->channel2) {
> +		case IIO_MOD_RUNNING:
> +			*val = st->activity_running;
> +			return IIO_VAL_INT;
> +		case IIO_MOD_WALKING:
> +			*val = st->activity_walking;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  /**
>   * iio_dummy_read_raw() - data read function.
>   * @indio_dev:	the struct iio_dev associated with this device instance
> @@ -283,59 +342,21 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
>  			      long mask)
>  {
>  	struct iio_dummy_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW: /* magic value - channel value read */
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			guard(mutex)(&st->lock);
> -			switch (chan->type) {
> -			case IIO_VOLTAGE:
> -				if (chan->output) {
> -					/* Set integer part to cached value
> */
> -					*val = st->dac_val;
> -					return IIO_VAL_INT;
> -				} else if (chan->differential) {
> -					if (chan->channel == 1)
> -						*val = st-
> >differential_adc_val[0];
> -					else
> -						*val = st-
> >differential_adc_val[1];
> -					return IIO_VAL_INT;
> -				} else {
> -					*val = st->single_ended_adc_val;
> -					return IIO_VAL_INT;
> -				}
> -
> -			case IIO_ACCEL:
> -				*val = st->accel_val;
> -				return IIO_VAL_INT;
> -			default:
> -				return -EINVAL;
> -			}
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __iio_dummy_read_raw(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_PROCESSED:
> -		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -			guard(mutex)(&st->lock);
> -			switch (chan->type) {
> -			case IIO_STEPS:
> -				*val = st->steps;
> -				return IIO_VAL_INT;
> -			case IIO_ACTIVITY:
> -				switch (chan->channel2) {
> -				case IIO_MOD_RUNNING:
> -					*val = st->activity_running;
> -					return IIO_VAL_INT;
> -				case IIO_MOD_WALKING:
> -					*val = st->activity_walking;
> -					return IIO_VAL_INT;
> -				default:
> -					return -EINVAL;
> -				}
> -			default:
> -				return -EINVAL;
> -			}
> -		}
> -		unreachable();
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = __iio_dummy_read_processed(indio_dev, chan, val);
> +		iio_device_release_direct(indio_dev);
> +		return ret;
>  	case IIO_CHAN_INFO_OFFSET:
>  		/* only single ended adc -> 7 */
>  		*val = 7;