Re: [PATCH v2 16/27] iio: adc: max1363: 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.
> 
> 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/adc/max1363.c | 165 ++++++++++++++++++++------------------
>  1 file changed, 89 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index e8d731bc34e0..35717ec082ce 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -364,55 +364,52 @@ static int max1363_read_single_chan(struct iio_dev
> *indio_dev,
>  				    int *val,
>  				    long m)
>  {
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		s32 data;
> -		u8 rxbuf[2];
> -		struct max1363_state *st = iio_priv(indio_dev);
> -		struct i2c_client *client = st->client;
> -
> -		guard(mutex)(&st->lock);
> -
> -		/*
> -		 * If monitor mode is enabled, the method for reading a
> single
> -		 * channel will have to be rather different and has not yet
> -		 * been implemented.
> -		 *
> -		 * Also, cannot read directly if buffered capture enabled.
> -		 */
> -		if (st->monitor_on)
> -			return -EBUSY;
> +	s32 data;
> +	u8 rxbuf[2];
> +	struct max1363_state *st = iio_priv(indio_dev);
> +	struct i2c_client *client = st->client;
>  
> -		/* Check to see if current scan mode is correct */
> -		if (st->current_mode != &max1363_mode_table[chan->address]) {
> -			int ret;
> +	guard(mutex)(&st->lock);
>  
> -			/* Update scan mode if needed */
> -			st->current_mode = &max1363_mode_table[chan-
> >address];
> -			ret = max1363_set_scan_mode(st);
> -			if (ret < 0)
> -				return ret;
> -		}
> -		if (st->chip_info->bits != 8) {
> -			/* Get reading */
> -			data = st->recv(client, rxbuf, 2);
> -			if (data < 0)
> -				return data;
> -
> -			data = get_unaligned_be16(rxbuf) &
> -				((1 << st->chip_info->bits) - 1);
> -		} else {
> -			/* Get reading */
> -			data = st->recv(client, rxbuf, 1);
> -			if (data < 0)
> -				return data;
> -
> -			data = rxbuf[0];
> -		}
> -		*val = data;
> +	/*
> +	 * If monitor mode is enabled, the method for reading a single
> +	 * channel will have to be rather different and has not yet
> +	 * been implemented.
> +	 *
> +	 * Also, cannot read directly if buffered capture enabled.
> +	 */
> +	if (st->monitor_on)
> +		return -EBUSY;
> +
> +	/* Check to see if current scan mode is correct */
> +	if (st->current_mode != &max1363_mode_table[chan->address]) {
> +		int ret;
> +
> +		/* Update scan mode if needed */
> +		st->current_mode = &max1363_mode_table[chan->address];
> +		ret = max1363_set_scan_mode(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +	if (st->chip_info->bits != 8) {
> +		/* Get reading */
> +		data = st->recv(client, rxbuf, 2);
> +		if (data < 0)
> +			return data;
> +
> +		data = get_unaligned_be16(rxbuf) &
> +			((1 << st->chip_info->bits) - 1);
> +	} else {
> +		/* Get reading */
> +		data = st->recv(client, rxbuf, 1);
> +		if (data < 0)
> +			return data;
>  
> -		return 0;
> +		data = rxbuf[0];
>  	}
> -	unreachable();
> +	*val = data;
> +
> +	return 0;
>  }
>  
>  static int max1363_read_raw(struct iio_dev *indio_dev,
> @@ -426,7 +423,11 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +
>  		ret = max1363_read_single_chan(indio_dev, chan, val, m);
> +		iio_device_release_direct(indio_dev);
>  		if (ret < 0)
>  			return ret;
>  		return IIO_VAL_INT;
> @@ -947,46 +948,58 @@ static inline int __max1363_check_event_mask(int
> thismask, int checkmask)
>  	return ret;
>  }
>  
> -static int max1363_write_event_config(struct iio_dev *indio_dev,
> -	const struct iio_chan_spec *chan, enum iio_event_type type,
> +static int __max1363_write_event_config(struct max1363_state *st,
> +	const struct iio_chan_spec *chan,
>  	enum iio_event_direction dir, bool state)
>  {
> -	struct max1363_state *st = iio_priv(indio_dev);
> -
> -	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> -		int number = chan->channel;
> -		u16 unifiedmask;
> -		int ret;
> +	int number = chan->channel;
> +	u16 unifiedmask;
> +	int ret;
>  
> -		guard(mutex)(&st->lock);
> +	guard(mutex)(&st->lock);
>  
> -		unifiedmask = st->mask_low | st->mask_high;
> -		if (dir == IIO_EV_DIR_FALLING) {
> +	unifiedmask = st->mask_low | st->mask_high;
> +	if (dir == IIO_EV_DIR_FALLING) {
>  
> -			if (state == 0)
> -				st->mask_low &= ~(1 << number);
> -			else {
> -				ret = __max1363_check_event_mask((1 <<
> number),
> -								
> unifiedmask);
> -				if (ret)
> -					return ret;
> -				st->mask_low |= (1 << number);
> -			}
> -		} else {
> -			if (state == 0)
> -				st->mask_high &= ~(1 << number);
> -			else {
> -				ret = __max1363_check_event_mask((1 <<
> number),
> -								
> unifiedmask);
> -				if (ret)
> -					return ret;
> -				st->mask_high |= (1 << number);
> -			}
> +		if (state == 0)
> +			st->mask_low &= ~(1 << number);
> +		else {
> +			ret = __max1363_check_event_mask((1 << number),
> +							 unifiedmask);
> +			if (ret)
> +				return ret;
> +			st->mask_low |= (1 << number);
> +		}
> +	} else {
> +		if (state == 0)
> +			st->mask_high &= ~(1 << number);
> +		else {
> +			ret = __max1363_check_event_mask((1 << number),
> +							 unifiedmask);
> +			if (ret)
> +				return ret;
> +			st->mask_high |= (1 << number);
>  		}
>  	}
> -	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
>  
>  	return 0;
> +
> +}
> +static int max1363_write_event_config(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir, bool state)
> +{
> +	struct max1363_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = __max1363_write_event_config(st, chan,  dir, state);
> +	iio_device_release_direct(indio_dev);
> +	max1363_monitor_mode_update(st, !!(st->mask_high | st->mask_low));
> +
> +	return ret;
>  }
>  
>  /*