Re: [PATCH v2] iio: light: gp2ap020a00f: fix iio_triggered_buffer_{predisable,postenable} positions

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 6 Mar 2020 13:11:37 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> the poll functions.
> 
> For the predisable hook, the disable code should occur before detaching
> the poll func, and for the postenable hook, the poll func should be
> attached before the enable code.
> 
> This change moves the postenable/predisable hooks into the correct
> positions.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Seems unlikely anyone who knows this driver well will respond, so
applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to poke at it - maybe...

There is an odd warning I'm getting in here on x86 builds but it
has nothing to do with your patch so hopefully autobuilders won't
moan about.

thanks,

Jonathan

> ---
> 
> Changelog v1 -> v2:
> * Fix omitted compiler warnings: 'warning: unused label 'error_unlock' [-Wunused-label]'
>   Reported-by: kbuild test robot <lkp@intel.com>
> 
>  drivers/iio/light/gp2ap020a00f.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index 4d70c5bf35da..7fbbce0d4bc7 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -1390,6 +1390,12 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
>  
>  	mutex_lock(&data->lock);
>  
> +	err = iio_triggered_buffer_postenable(indio_dev);
> +	if (err < 0) {
> +		mutex_unlock(&data->lock);
> +		return err;
> +	}
> +
>  	/*
>  	 * Enable triggers according to the scan_mask. Enabling either
>  	 * LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
> @@ -1420,14 +1426,12 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
>  		goto error_unlock;
>  
>  	data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> -	if (!data->buffer) {
> +	if (!data->buffer)
>  		err = -ENOMEM;
> -		goto error_unlock;
> -	}
> -
> -	err = iio_triggered_buffer_postenable(indio_dev);
>  
>  error_unlock:
> +	if (err < 0)
> +		iio_triggered_buffer_predisable(indio_dev);
>  	mutex_unlock(&data->lock);
>  
>  	return err;
> @@ -1436,14 +1440,10 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
>  static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
>  {
>  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
> -	int i, err;
> +	int i, err = 0;
>  
>  	mutex_lock(&data->lock);
>  
> -	err = iio_triggered_buffer_predisable(indio_dev);
> -	if (err < 0)
> -		goto error_unlock;
> -
>  	for_each_set_bit(i, indio_dev->active_scan_mask,
>  		indio_dev->masklength) {
>  		switch (i) {
> @@ -1465,7 +1465,8 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
>  	if (err == 0)
>  		kfree(data->buffer);
>  
> -error_unlock:
> +	iio_triggered_buffer_predisable(indio_dev);
> +
>  	mutex_unlock(&data->lock);
>  
>  	return err;