All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Use of `claim_direct_scoped` for improved error handling
@ 2024-05-01 21:57 Jorge Harrisonn
  2024-05-01 21:57 ` [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification Jorge Harrisonn
  2024-05-01 21:57 ` [PATCH 2/2] iio: adc: ad7923: " Jorge Harrisonn
  0 siblings, 2 replies; 6+ messages in thread
From: Jorge Harrisonn @ 2024-05-01 21:57 UTC (permalink / raw
  To: jic23, lars; +Cc: Jorge Harrisonn, linux-iio, linux-kernel, laisnuto

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification
  2024-05-01 21:57 [PATCH 0/2] Use of `claim_direct_scoped` for improved error handling Jorge Harrisonn
@ 2024-05-01 21:57 ` Jorge Harrisonn
  2024-05-04 11:34   ` Jonathan Cameron
  2024-05-04 16:08   ` Markus Elfring
  2024-05-01 21:57 ` [PATCH 2/2] iio: adc: ad7923: " Jorge Harrisonn
  1 sibling, 2 replies; 6+ messages in thread
From: Jorge Harrisonn @ 2024-05-01 21:57 UTC (permalink / raw
  To: jic23, lars; +Cc: Jorge Harrisonn, linux-iio, linux-kernel, laisnuto

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] iio: adc: ad7923: using claim_direct_scoped for code simplification
  2024-05-01 21:57 [PATCH 0/2] Use of `claim_direct_scoped` for improved error handling Jorge Harrisonn
  2024-05-01 21:57 ` [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification Jorge Harrisonn
@ 2024-05-01 21:57 ` Jorge Harrisonn
  2024-05-04 11:38   ` Jonathan Cameron
  1 sibling, 1 reply; 6+ messages in thread
From: Jorge Harrisonn @ 2024-05-01 21:57 UTC (permalink / raw
  To: jic23, lars; +Cc: Jorge Harrisonn, linux-iio, linux-kernel, laisnuto

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification
  2024-05-01 21:57 ` [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification Jorge Harrisonn
@ 2024-05-04 11:34   ` Jonathan Cameron
  2024-05-04 16:08   ` Markus Elfring
  1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2024-05-04 11:34 UTC (permalink / raw
  To: Jorge Harrisonn; +Cc: lars, linux-iio, linux-kernel, laisnuto

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;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] iio: adc: ad7923: using claim_direct_scoped for code simplification
  2024-05-01 21:57 ` [PATCH 2/2] iio: adc: ad7923: " Jorge Harrisonn
@ 2024-05-04 11:38   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2024-05-04 11:38 UTC (permalink / raw
  To: Jorge Harrisonn; +Cc: lars, linux-iio, linux-kernel, laisnuto

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)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification
  2024-05-01 21:57 ` [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification Jorge Harrisonn
  2024-05-04 11:34   ` Jonathan Cameron
@ 2024-05-04 16:08   ` Markus Elfring
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2024-05-04 16:08 UTC (permalink / raw
  To: Jorge Harrisonn, Lais Nuto, linux-iio, kernel-janitors,
	Jonathan Cameron, Lars-Peter Clausen
  Cc: LKML> _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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-04 16:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-01 21:57 [PATCH 0/2] Use of `claim_direct_scoped` for improved error handling Jorge Harrisonn
2024-05-01 21:57 ` [PATCH 1/2] iio: adc: ad7606: using claim_direct_scoped for code simplification Jorge Harrisonn
2024-05-04 11:34   ` Jonathan Cameron
2024-05-04 16:08   ` Markus Elfring
2024-05-01 21:57 ` [PATCH 2/2] iio: adc: ad7923: " Jorge Harrisonn
2024-05-04 11:38   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.