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
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 <[email protected]>
Signed-off-by: Lais Nuto <[email protected]>
Signed-off-by: Jorge Harrisonn <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Lais Nuto <[email protected]>
Signed-off-by: Jorge Harrisonn <[email protected]>
---
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
On Wed, 1 May 2024 18:57:23 -0300
Jorge Harrisonn <[email protected]> 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 <[email protected]>
> Signed-off-by: Lais Nuto <[email protected]>
> Signed-off-by: Jorge Harrisonn <[email protected]>
> ---
> 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;
On Wed, 1 May 2024 18:57:24 -0300
Jorge Harrisonn <[email protected]> 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 <[email protected]>
> Signed-off-by: Lais Nuto <[email protected]>
> Signed-off-by: Jorge Harrisonn <[email protected]>
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)
…
> _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