| Message ID | 20240501215724.26655-3-jorge.harrisonn@usp.br (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Use of `claim_direct_scoped` for improved error handling | expand |
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)
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)