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

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

[Alexandru Ardelean]

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>
---
 drivers/iio/light/gp2ap020a00f.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 4d70c5bf35da..24339098e707 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;
@@ -1440,10 +1444,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
 
 	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) {
@@ -1466,6 +1466,8 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
 		kfree(data->buffer);
 
 error_unlock:
+	iio_triggered_buffer_predisable(indio_dev);
+
 	mutex_unlock(&data->lock);
 
 	return err;
-- 
2.20.1

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4896 bytes --]

Hi Alexandru,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on linux/master linus/master v5.6-rc4 next-20200304]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/iio-light-gp2ap020a00f-fix-iio_triggered_buffer_-predisable-postenable-positions/20200305-022018
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/iio/light/gp2ap020a00f.c: In function 'gp2ap020a00f_buffer_predisable':
   drivers/iio/light/gp2ap020a00f.c:1468:1: warning: label 'error_unlock' defined but not used [-Wunused-label]
    error_unlock:
    ^~~~~~~~~~~~
>> drivers/iio/light/gp2ap020a00f.c:1473:9: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return err;
            ^~~

vim +/err +1473 drivers/iio/light/gp2ap020a00f.c

bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1439  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1440  static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1441  {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1442  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1443  	int i, err;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1444  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1445  	mutex_lock(&data->lock);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1446  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1447  	for_each_set_bit(i, indio_dev->active_scan_mask,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1448  		indio_dev->masklength) {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1449  		switch (i) {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1450  		case GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1451  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1452  					GP2AP020A00F_CMD_TRIGGER_CLEAR_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1453  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1454  		case GP2AP020A00F_SCAN_MODE_LIGHT_IR:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1455  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1456  					GP2AP020A00F_CMD_TRIGGER_IR_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1457  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1458  		case GP2AP020A00F_SCAN_MODE_PROXIMITY:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1459  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1460  					GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1461  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1462  		}
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1463  	}
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1464  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1465  	if (err == 0)
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1466  		kfree(data->buffer);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1467  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1468  error_unlock:
29f67b610cb7f3 Alexandru Ardelean 2020-03-04  1469  	iio_triggered_buffer_predisable(indio_dev);
29f67b610cb7f3 Alexandru Ardelean 2020-03-04  1470  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1471  	mutex_unlock(&data->lock);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1472  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09 @1473  	return err;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1474  }
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1475  

:::::: The code at line 1473 was first introduced by commit
:::::: bf29fbeaa13d3350ca71df70c705d8b883c45692 iio: gp2ap020a00f: Add a driver for the device

:::::: TO: Jacek Anaszewski <j.anaszewski@samsung.com>
:::::: CC: Jonathan Cameron <jic23@kernel.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 52793 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4418 bytes --]

Hi Alexandru,

I love your patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on linux/master linus/master v5.6-rc4 next-20200304]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/iio-light-gp2ap020a00f-fix-iio_triggered_buffer_-predisable-postenable-positions/20200305-022018
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (git://gitmirror/llvm_project f70e7dc17d78a83265a89158ee549f1147d7d500)
reproduce:
        # FIXME the reproduce steps for clang is not ready yet

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/iio/light/gp2ap020a00f.c:1468:1: warning: unused label 'error_unlock' [-Wunused-label]
   error_unlock:
   ^~~~~~~~~~~~~
   1 warning generated.

vim +/error_unlock +1468 drivers/iio/light/gp2ap020a00f.c

bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1439  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1440  static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1441  {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1442  	struct gp2ap020a00f_data *data = iio_priv(indio_dev);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1443  	int i, err;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1444  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1445  	mutex_lock(&data->lock);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1446  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1447  	for_each_set_bit(i, indio_dev->active_scan_mask,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1448  		indio_dev->masklength) {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1449  		switch (i) {
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1450  		case GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1451  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1452  					GP2AP020A00F_CMD_TRIGGER_CLEAR_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1453  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1454  		case GP2AP020A00F_SCAN_MODE_LIGHT_IR:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1455  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1456  					GP2AP020A00F_CMD_TRIGGER_IR_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1457  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1458  		case GP2AP020A00F_SCAN_MODE_PROXIMITY:
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1459  			err = gp2ap020a00f_exec_cmd(data,
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1460  					GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1461  			break;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1462  		}
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1463  	}
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1464  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1465  	if (err == 0)
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1466  		kfree(data->buffer);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1467  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09 @1468  error_unlock:
29f67b610cb7f3 Alexandru Ardelean 2020-03-04  1469  	iio_triggered_buffer_predisable(indio_dev);
29f67b610cb7f3 Alexandru Ardelean 2020-03-04  1470  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1471  	mutex_unlock(&data->lock);
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1472  
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1473  	return err;
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1474  }
bf29fbeaa13d33 Jacek Anaszewski   2013-09-09  1475  

:::::: The code at line 1468 was first introduced by commit
:::::: bf29fbeaa13d3350ca71df70c705d8b883c45692 iio: gp2ap020a00f: Add a driver for the device

:::::: TO: Jacek Anaszewski <j.anaszewski@samsung.com>
:::::: CC: Jonathan Cameron <jic23@kernel.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 72216 bytes --]

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

[Alexandru Ardelean]

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>
---

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;
-- 
2.20.1

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

[Jonathan Cameron]

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;