[PATCH v3 0/3] iio: adc: ad7266: Use iio_device_claim_direct_scoped() and clean some codestyle error

[PATCH v3 0/3] iio: adc: ad7266: Use iio_device_claim_direct_scoped() and clean some codestyle error

[Fernando Yang]

Checks ret variable after iio_device_release_direct_mode()
Switching _direct_mode() to the _scoped() version
Fix some codestyle errors indicated by checkpatch.pl

changelog v2 -> v3:
- Adding Fixed tag c70df20e3159 ("iio: adc: ad7266: claim direct mode during sensor read")

Fernando Yang (3):
  iio: adc: ad7266: Fix variable checking bug
  iio: adc: ad7266: Use iio_device_claim_direct_scoped()
  iio: adc: ad7266: Fix codestyle error

 drivers/iio/adc/ad7266.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] iio: adc: ad7266: Fix variable checking bug

[Fernando Yang]

The ret variable was not checked after iio_device_release_direct_mode(),
which could possibly cause errors

Fixes: c70df20e3159 ("iio: adc: ad7266: claim direct mode during sensor read")

Signed-off-by: Fernando Yang <hagisf@usp.br>
---
 drivers/iio/adc/ad7266.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index 353a97f9c..13ea8a107 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -157,6 +157,8 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
 		ret = ad7266_read_single(st, val, chan->address);
 		iio_device_release_direct_mode(indio_dev);
 
+		if (ret < 0)
+			return ret;
 		*val = (*val >> 2) & 0xfff;
 		if (chan->scan_type.sign == 's')
 			*val = sign_extend32(*val,
-- 
2.34.1

Re: [PATCH v3 1/3] iio: adc: ad7266: Fix variable checking bug

[Jonathan Cameron]

On Thu, 13 Jun 2024 12:39:18 -0300
Fernando Yang <hagisf@usp.br> wrote:

> The ret variable was not checked after iio_device_release_direct_mode(),
> which could possibly cause errors
> 
> Fixes: c70df20e3159 ("iio: adc: ad7266: claim direct mode during sensor read")
> 
No blank line here as Fixes is very much part of the tag block.

> Signed-off-by: Fernando Yang <hagisf@usp.br>
Also I've just sent a pull request with this included.

I mentioned it was queued in reply to v2.
Once a patch is queued, don't resend it as it just confuses people :(

Jonathan


> ---
>  drivers/iio/adc/ad7266.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
> index 353a97f9c..13ea8a107 100644
> --- a/drivers/iio/adc/ad7266.c
> +++ b/drivers/iio/adc/ad7266.c
> @@ -157,6 +157,8 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
>  		ret = ad7266_read_single(st, val, chan->address);
>  		iio_device_release_direct_mode(indio_dev);
>  
> +		if (ret < 0)
> +			return ret;
>  		*val = (*val >> 2) & 0xfff;
>  		if (chan->scan_type.sign == 's')
>  			*val = sign_extend32(*val,

[PATCH v3 2/3] iio: adc: ad7266: Use iio_device_claim_direct_scoped()

[Fernando Yang]

Switching to the _scoped() version can make the error handling more
natural instead of delayed until direct mode was released.

Signed-off-by: Fernando Yang <hagisf@usp.br>
---
 drivers/iio/adc/ad7266.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index 13ea8a107..356c2fe07 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -151,20 +151,19 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
-		ret = iio_device_claim_direct_mode(indio_dev);
-		if (ret)
-			return ret;
-		ret = ad7266_read_single(st, val, chan->address);
-		iio_device_release_direct_mode(indio_dev);
-
-		if (ret < 0)
-			return ret;
-		*val = (*val >> 2) & 0xfff;
-		if (chan->scan_type.sign == 's')
-			*val = sign_extend32(*val,
-					     chan->scan_type.realbits - 1);
-
-		return IIO_VAL_INT;
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+			ret = ad7266_read_single(st, val, chan->address);
+			
+			if (ret < 0)
+				return ret;
+			*val = (*val >> 2) & 0xfff;
+			if (chan->scan_type.sign == 's')
+				*val = sign_extend32(*val,
+							 chan->scan_type.realbits - 1);
+
+			return IIO_VAL_INT;
+		}
+		unreachable();
 	case IIO_CHAN_INFO_SCALE:
 		scale_mv = st->vref_mv;
 		if (st->mode == AD7266_MODE_DIFF)
-- 
2.34.1

Re: [PATCH v3 2/3] iio: adc: ad7266: Use iio_device_claim_direct_scoped()

[Jonathan Cameron]

On Thu, 13 Jun 2024 12:39:19 -0300
Fernando Yang <hagisf@usp.br> wrote:

> Switching to the _scoped() version can make the error handling more
> natural instead of delayed until direct mode was released.
> 
> Signed-off-by: Fernando Yang <hagisf@usp.br>
> ---
>  drivers/iio/adc/ad7266.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
> index 13ea8a107..356c2fe07 100644
> --- a/drivers/iio/adc/ad7266.c
> +++ b/drivers/iio/adc/ad7266.c
> @@ -151,20 +151,19 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
> -		ret = iio_device_claim_direct_mode(indio_dev);
> -		if (ret)
> -			return ret;
> -		ret = ad7266_read_single(st, val, chan->address);
> -		iio_device_release_direct_mode(indio_dev);
> -
> -		if (ret < 0)
> -			return ret;
> -		*val = (*val >> 2) & 0xfff;
> -		if (chan->scan_type.sign == 's')
> -			*val = sign_extend32(*val,
> -					     chan->scan_type.realbits - 1);
> -
> -		return IIO_VAL_INT;
> +		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +			ret = ad7266_read_single(st, val, chan->address);
> +			
Seems to be some stray white space (tabs) on the line above.
Make sure to run checkpatch.pl.

Also reformat this to bring call and error condition together.
		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
			ret = ad7266_read_single(st, val, chan->address);
			if (ret < 0)
				return ret;

			*val = ...

> +			if (ret < 0)
> +				return ret;
> +			*val = (*val >> 2) & 0xfff;
> +			if (chan->scan_type.sign == 's')
> +				*val = sign_extend32(*val,
> +							 chan->scan_type.realbits - 1);
> +
This doesn't look like a correct indent.  Align c with * as done in the original code
> +			return IIO_VAL_INT;
> +		}
> +		unreachable();
>  	case IIO_CHAN_INFO_SCALE:
>  		scale_mv = st->vref_mv;
>  		if (st->mode == AD7266_MODE_DIFF)

[PATCH v3 3/3] iio: adc: ad7266: Fix codestyle error

[Fernando Yang]

Fix some codestyle errors indicated by checkpatch.pl

Signed-off-by: Fernando Yang <hagisf@usp.br>
---
 drivers/iio/adc/ad7266.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index 356c2fe07..5a2f1bb27 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -63,12 +63,14 @@ static int ad7266_powerdown(struct ad7266_state *st)
 static int ad7266_preenable(struct iio_dev *indio_dev)
 {
 	struct ad7266_state *st = iio_priv(indio_dev);
+
 	return ad7266_wakeup(st);
 }
 
 static int ad7266_postdisable(struct iio_dev *indio_dev)
 {
 	struct ad7266_state *st = iio_priv(indio_dev);
+
 	return ad7266_powerdown(st);
 }
 
@@ -153,7 +155,7 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
 			ret = ad7266_read_single(st, val, chan->address);
-			
+
 			if (ret < 0)
 				return ret;
 			*val = (*val >> 2) & 0xfff;
-- 
2.34.1

Re: [PATCH v3 3/3] iio: adc: ad7266: Fix codestyle error

[Jonathan Cameron]

On Thu, 13 Jun 2024 12:39:20 -0300
Fernando Yang <hagisf@usp.br> wrote:

> Fix some codestyle errors indicated by checkpatch.pl
> 
> Signed-off-by: Fernando Yang <hagisf@usp.br>
> ---
>  drivers/iio/adc/ad7266.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
> index 356c2fe07..5a2f1bb27 100644
> --- a/drivers/iio/adc/ad7266.c
> +++ b/drivers/iio/adc/ad7266.c
> @@ -63,12 +63,14 @@ static int ad7266_powerdown(struct ad7266_state *st)
>  static int ad7266_preenable(struct iio_dev *indio_dev)
>  {
>  	struct ad7266_state *st = iio_priv(indio_dev);
> +
>  	return ad7266_wakeup(st);
>  }
>  
>  static int ad7266_postdisable(struct iio_dev *indio_dev)
>  {
>  	struct ad7266_state *st = iio_priv(indio_dev);
> +
>  	return ad7266_powerdown(st);
>  }
>  
> @@ -153,7 +155,7 @@ static int ad7266_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_RAW:
>  		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
>  			ret = ad7266_read_single(st, val, chan->address);
> -			
> +
This is fixing and error from your previous patch.  Fix it there so this
is never needed!

>  			if (ret < 0)
>  				return ret;
>  			*val = (*val >> 2) & 0xfff;