[PATCH 0/2] Use of `claim_direct_scoped` for improved error handling

[PATCH 0/2] Use of `claim_direct_scoped` for improved error handling

[Jorge Harrisonn]

Make use of `claim_direct_scoped` in two modules, in order to make error handling
more natural and simple than the former call to `claim_direct_mode` and
`release_direct_mode`

Jorge Harrisonn (2):
  iio: adc: ad7606: using claim_direct_scoped for code simplification
  iio: adc: ad7923: using claim_direct_scoped for code simplification

 drivers/iio/adc/ad7606.c | 19 ++++++++-----------
 drivers/iio/adc/ad7923.c | 30 ++++++++++++++----------------
 2 files changed, 22 insertions(+), 27 deletions(-)

-- 
2.34.1

[PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification

[Jorge Harrisonn]

Using iio_device_claim_direct_scoped instead of calling `iio_device
_claim_direct_modeand later callingiio_device_release_direct_mode`

This should make code cleaner and error handling easier

Co-authored-by: Lais Nuto <laisnuto@usp.br>
Signed-off-by: Lais Nuto <laisnuto@usp.br>
Signed-off-by: Jorge Harrisonn <jorge.harrisonn@usp.br>
---
 drivers/iio/adc/ad7606.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 1928d9ae5bcf..fa989e0d7e70 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -174,17 +174,14 @@ static int ad7606_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 = ad7606_scan_direct(indio_dev, chan->address);
-		iio_device_release_direct_mode(indio_dev);
-
-		if (ret < 0)
-			return ret;
-		*val = (short)ret;
-		return IIO_VAL_INT;
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+			ret = ad7606_scan_direct(indio_dev, chan->address);
+			if (ret < 0) 
+				return ret;
+			*val = (short) ret;
+			return IIO_VAL_INT;
+		}
+		unreachable();
 	case IIO_CHAN_INFO_SCALE:
 		if (st->sw_mode_en)
 			ch = chan->address;
-- 
2.34.1

Re: [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification

[Jonathan Cameron]

On Wed,  1 May 2024 18:57:23 -0300
Jorge Harrisonn <jorge.harrisonn@usp.br> wrote:

> Using iio_device_claim_direct_scoped instead of calling `iio_device
> _claim_direct_modeand later callingiio_device_release_direct_mode`
> 
> This should make code cleaner and error handling easier
> 
> Co-authored-by: Lais Nuto <laisnuto@usp.br>
> Signed-off-by: Lais Nuto <laisnuto@usp.br>
> Signed-off-by: Jorge Harrisonn <jorge.harrisonn@usp.br>
> ---
>  drivers/iio/adc/ad7606.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 1928d9ae5bcf..fa989e0d7e70 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -174,17 +174,14 @@ static int ad7606_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 = ad7606_scan_direct(indio_dev, chan->address);
> -		iio_device_release_direct_mode(indio_dev);
> -
> -		if (ret < 0)
> -			return ret;
> -		*val = (short)ret;
> -		return IIO_VAL_INT;
> +		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> +			ret = ad7606_scan_direct(indio_dev, chan->address);
> +			if (ret < 0) 

There is a stray space on the line above after the )

Make sure to run checkpatch.pl which should have caught this

> +				return ret;
> +			*val = (short) ret;

Not really part of your patch so I'll leave it alone, but that cast seems rather
pointless.

Applied to the togreg branch of iio.git and pushed out as testing with the
white space above fixed.   Note I'll be rebasing that tree after the
merge window so until then this will only be visible on the testing
branch.

Thanks,

Jonathan

> +			return IIO_VAL_INT;
> +		}
> +		unreachable();
>  	case IIO_CHAN_INFO_SCALE:
>  		if (st->sw_mode_en)
>  			ch = chan->address;

Re: [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification

[Markus Elfring]

…
> _claim_direct_modeand later callingiio_device_release_direct_mode`
>
> This should make code cleaner and error handling easier

* Please avoid typos in such a change description.

* Can imperative wordings be also more desirable here?


Regards,
Markus

[PATCH 2/2] iio: adc: ad7923: using claim_direct_scoped for code simplification

[Jorge Harrisonn]

Using iio_device_claim_direct_scoped instead of calling `iio_device
_claim_direct_modeand later callingiio_device_release_direct_mode`

This should make code cleaner and error handling easier

Co-authored-by: Lais Nuto <laisnuto@usp.br>
Signed-off-by: Lais Nuto <laisnuto@usp.br>
Signed-off-by: Jorge Harrisonn <jorge.harrisonn@usp.br>
---
 drivers/iio/adc/ad7923.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 9d6bf6d0927a..a646521b2ef3 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -260,22 +260,20 @@ static int ad7923_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 = ad7923_scan_direct(st, chan->address);
-		iio_device_release_direct_mode(indio_dev);
-
-		if (ret < 0)
-			return ret;
-
-		if (chan->address == EXTRACT(ret, 12, 4))
-			*val = EXTRACT(ret, chan->scan_type.shift,
-				       chan->scan_type.realbits);
-		else
-			return -EIO;
-
-		return IIO_VAL_INT;
+		    iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {         	
+			ret = ad7923_scan_direct(st, chan->address); 
+				
+			if (ret < 0)
+				return ret;
+
+			if (chan->address == EXTRACT(ret, 12, 4))
+				*val = EXTRACT(ret, 0, 12);
+			else
+				return -EIO;
+
+			return IIO_VAL_INT;
+		}
+    	unreachable();
 	case IIO_CHAN_INFO_SCALE:
 		ret = ad7923_get_range(st);
 		if (ret < 0)
-- 
2.34.1

Re: [PATCH 2/2] iio: adc: ad7923: using claim_direct_scoped for code simplification

[Jonathan Cameron]

On Wed,  1 May 2024 18:57:24 -0300
Jorge Harrisonn <jorge.harrisonn@usp.br> wrote:

> Using iio_device_claim_direct_scoped instead of calling `iio_device
> _claim_direct_modeand later callingiio_device_release_direct_mode`
> 
> This should make code cleaner and error handling easier
> 
> Co-authored-by: Lais Nuto <laisnuto@usp.br>
> Signed-off-by: Lais Nuto <laisnuto@usp.br>
> Signed-off-by: Jorge Harrisonn <jorge.harrisonn@usp.br>
Hi Jorge, Lais,

Comments inline.

> ---
>  drivers/iio/adc/ad7923.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 9d6bf6d0927a..a646521b2ef3 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -260,22 +260,20 @@ static int ad7923_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 = ad7923_scan_direct(st, chan->address);
> -		iio_device_release_direct_mode(indio_dev);
> -
> -		if (ret < 0)
> -			return ret;
> -
> -		if (chan->address == EXTRACT(ret, 12, 4))
> -			*val = EXTRACT(ret, chan->scan_type.shift,
> -				       chan->scan_type.realbits);
> -		else
> -			return -EIO;
> -
> -		return IIO_VAL_INT;
> +		    iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {         	

Lots of stray white space + why this indent for the line above?

> +			ret = ad7923_scan_direct(st, chan->address); 
> +				
No blank line here as the check should be clearly associated with
the line above.
> +			if (ret < 0)
> +				return ret;
> +
> +			if (chan->address == EXTRACT(ret, 12, 4))
> +				*val = EXTRACT(ret, 0, 12);
> +			else
> +				return -EIO;
Flip this logic
			if (chan->address != EXTRACT(ret, 12, 4))
				return -EIO;

			*val = EXTRACT(ret, 0, 12);

However, better to also change the cases where the masks is fixed at
compile time to use the standard FIELD_GET() and provide appropriate
mask defines.

That change would be a separate patch, so up to you whether you want
to do that as an addition for v2.



> +
> +			return IIO_VAL_INT;
> +		}
> +    	unreachable();
>  	case IIO_CHAN_INFO_SCALE:
>  		ret = ad7923_get_range(st);
>  		if (ret < 0)