Re: [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 29 Apr 2025 06:39:23 -0300
Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br> wrote:

> Refactor to share logic between buffer enable/disable handlers.
> 
> Signed-off-by: Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br>
> Co-developed-by: Augusto Bernarde <augustomb@usp.br>
> Signed-off-by: Augusto Bernarde <augustomb@usp.br>
> ---
>  drivers/iio/proximity/sx9500.c | 50 ++++++++++++++++------------------

In my view this isn't a significant enough reduction to justify the more complex code.
Particularly in the error paths.

Jonathan


>  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
> index c4e94d0fb163..75459c85116b 100644
> --- a/drivers/iio/proximity/sx9500.c
> +++ b/drivers/iio/proximity/sx9500.c
> @@ -674,52 +674,48 @@ static irqreturn_t sx9500_trigger_handler(int irq, void *private)
>  	return IRQ_HANDLED;
>  }
>  
> -static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
> +static int sx9500_buffer_manage_chan_users(struct iio_dev *indio_dev, bool enable_channels)
>  {
>  	struct sx9500_data *data = iio_priv(indio_dev);
>  	int ret = 0, i;
>  
>  	mutex_lock(&data->mutex);
>  
> -	for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> +	for (i = 0; i < SX9500_NUM_CHANNELS; i++) {
>  		if (test_bit(i, indio_dev->active_scan_mask)) {
> -			ret = sx9500_inc_chan_users(data, i);
> +			if (enable_channels)
> +				ret = sx9500_inc_chan_users(data, i);
> +			else
> +				ret = sx9500_dec_chan_users(data, i);
>  			if (ret)
>  				break;
>  		}
> +	}
>  
> -	if (ret)
> -		for (i = i - 1; i >= 0; i--)
> -			if (test_bit(i, indio_dev->active_scan_mask))
> -				sx9500_dec_chan_users(data, i);
> +	if (ret) {
> +		for (i = i - 1; i >= 0; i--) {
> +			if (test_bit(i, indio_dev->active_scan_mask)) {
> +				if (enable_channels)
> +					sx9500_dec_chan_users(data, i);
> +				else
> +					sx9500_inc_chan_users(data, i);
> +			}
> +		}
> +	}
>  
>  	mutex_unlock(&data->mutex);
>  
>  	return ret;
>  }
>  
> -static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> +static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
>  {
> -	struct sx9500_data *data = iio_priv(indio_dev);
> -	int ret = 0, i;
> -
> -	mutex_lock(&data->mutex);
> -
> -	for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> -		if (test_bit(i, indio_dev->active_scan_mask)) {
> -			ret = sx9500_dec_chan_users(data, i);
> -			if (ret)
> -				break;
> -		}
> -
> -	if (ret)
> -		for (i = i - 1; i >= 0; i--)
> -			if (test_bit(i, indio_dev->active_scan_mask))
> -				sx9500_inc_chan_users(data, i);
> -
> -	mutex_unlock(&data->mutex);
> +	return sx9500_buffer_manage_chan_users(indio_dev, true);
> +}
>  
> -	return ret;
> +static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	return sx9500_buffer_manage_chan_users(indio_dev, false);
>  }
>  
>  static const struct iio_buffer_setup_ops sx9500_buffer_setup_ops = {