On Wed, 1 May 2024 18:57:24 -0300
Jorge Harrisonn <jorge.harrisonn@xxxxxx> 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@xxxxxx>
> Signed-off-by: Lais Nuto <laisnuto@xxxxxx>
> Signed-off-by: Jorge Harrisonn <jorge.harrisonn@xxxxxx>
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)
