| Message ID | 20240128150537.44592-3-jic23@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | IIO: Use the new cleanup.h magic | expand |
On Sun, Jan 28, 2024 at 9:06 AM Jonathan Cameron <jic23@kernel.org> wrote: > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Given we now have iio_device_claim_direct_scoped() to perform automatic > releasing of direct mode at exit from the scope that follows it, this can > be used in conjunction with guard(mutex) etc remove a lot of special case > handling. > > Note that in this particular example code, there is no real reason you can't > read channels via sysfs at the same time as filling the software buffer. > To make it look more like a real driver constrain raw and processed > channel reads from occurring whilst the buffer is in use. > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > Since RFC: > - Dropped a stale comment about a local variable that existed > in an earlier version of this patch before the new scoped_guard_cond() > infrastructure was added. > - Use unreachable() to convince the compiler we can't get to code > at end of the pattern. > > iio_device_claim_direct_scoped(return -EBUSY, iio_dev) { > return 0; > } > unreacahable(); > --- > drivers/iio/dummy/iio_simple_dummy.c | 182 +++++++++++++-------------- > 1 file changed, 88 insertions(+), 94 deletions(-) > > diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c > index c24f609c2ade..d6ef556698fb 100644 > --- a/drivers/iio/dummy/iio_simple_dummy.c > +++ b/drivers/iio/dummy/iio_simple_dummy.c > @@ -283,65 +283,63 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev, > long mask) > { > struct iio_dummy_state *st = iio_priv(indio_dev); > - int ret = -EINVAL; > > - mutex_lock(&st->lock); > switch (mask) { > case IIO_CHAN_INFO_RAW: /* magic value - channel value read */ > - switch (chan->type) { > - case IIO_VOLTAGE: > - if (chan->output) { > - /* Set integer part to cached value */ > - *val = st->dac_val; > - ret = IIO_VAL_INT; > - } else if (chan->differential) { > - if (chan->channel == 1) > - *val = st->differential_adc_val[0]; > - else > - *val = st->differential_adc_val[1]; > - ret = IIO_VAL_INT; > - } else { > - *val = st->single_ended_adc_val; > - ret = IIO_VAL_INT; > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { > + guard(mutex)(&st->lock); > + switch (chan->type) { > + case IIO_VOLTAGE: > + if (chan->output) { > + /* Set integer part to cached value */ > + *val = st->dac_val; > + return IIO_VAL_INT; > + } else if (chan->differential) { > + if (chan->channel == 1) > + *val = st->differential_adc_val[0]; > + else > + *val = st->differential_adc_val[1]; > + return IIO_VAL_INT; > + } else { > + *val = st->single_ended_adc_val; > + return IIO_VAL_INT; > + } > + > + case IIO_ACCEL: > + *val = st->accel_val; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > } > - break; > - case IIO_ACCEL: > - *val = st->accel_val; > - ret = IIO_VAL_INT; > - break; > - default: > - break; > } > - break; > + unreachable(); > case IIO_CHAN_INFO_PROCESSED: > - switch (chan->type) { > - case IIO_STEPS: > - *val = st->steps; > - ret = IIO_VAL_INT; > - break; > - case IIO_ACTIVITY: > - switch (chan->channel2) { > - case IIO_MOD_RUNNING: > - *val = st->activity_running; > - ret = IIO_VAL_INT; > - break; > - case IIO_MOD_WALKING: > - *val = st->activity_walking; > - ret = IIO_VAL_INT; > - break; > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { > + guard(mutex)(&st->lock); > + switch (chan->type) { > + case IIO_STEPS: > + *val = st->steps; > + return IIO_VAL_INT; > + case IIO_ACTIVITY: > + switch (chan->channel2) { > + case IIO_MOD_RUNNING: > + *val = st->activity_running; > + return IIO_VAL_INT; > + case IIO_MOD_WALKING: > + *val = st->activity_walking; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > default: > - break; > + return -EINVAL; > } > - break; > - default: > - break; > } > - break; > + unreachable(); > case IIO_CHAN_INFO_OFFSET: > /* only single ended adc -> 7 */ > *val = 7; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > switch (chan->type) { > case IIO_VOLTAGE: > @@ -350,60 +348,57 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev, > /* only single ended adc -> 0.001333 */ > *val = 0; > *val2 = 1333; > - ret = IIO_VAL_INT_PLUS_MICRO; > - break; > + return IIO_VAL_INT_PLUS_MICRO; > case 1: > /* all differential adc -> 0.000001344 */ > *val = 0; > *val2 = 1344; > - ret = IIO_VAL_INT_PLUS_NANO; > + return IIO_VAL_INT_PLUS_NANO; > + default: > + return -EINVAL; > } > - break; > default: > - break; > + return -EINVAL; > } > - break; > - case IIO_CHAN_INFO_CALIBBIAS: > + case IIO_CHAN_INFO_CALIBBIAS: { > + guard(mutex)(&st->lock); > /* only the acceleration axis - read from cache */ > *val = st->accel_calibbias; > - ret = IIO_VAL_INT; > - break; > - case IIO_CHAN_INFO_CALIBSCALE: > + return IIO_VAL_INT; > + } > + case IIO_CHAN_INFO_CALIBSCALE: { > + guard(mutex)(&st->lock); > *val = st->accel_calibscale->val; > *val2 = st->accel_calibscale->val2; > - ret = IIO_VAL_INT_PLUS_MICRO; > - break; > + return IIO_VAL_INT_PLUS_MICRO; > + } > case IIO_CHAN_INFO_SAMP_FREQ: > *val = 3; > *val2 = 33; > - ret = IIO_VAL_INT_PLUS_NANO; > - break; > - case IIO_CHAN_INFO_ENABLE: > + return IIO_VAL_INT_PLUS_NANO; > + case IIO_CHAN_INFO_ENABLE: { > + guard(mutex)(&st->lock); > switch (chan->type) { > case IIO_STEPS: > *val = st->steps_enabled; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > default: > - break; > + return -EINVAL; > } > - break; > - case IIO_CHAN_INFO_CALIBHEIGHT: > + } > + case IIO_CHAN_INFO_CALIBHEIGHT: { > + guard(mutex)(&st->lock); > switch (chan->type) { > case IIO_STEPS: > *val = st->height; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > default: > - break; > + return -EINVAL; > } > - break; > - > + } > default: > - break; > + return -EINVAL; > } > - mutex_unlock(&st->lock); > - return ret; > } > > /** > @@ -436,10 +431,10 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > if (chan->output == 0) > return -EINVAL; > > - /* Locking not required as writing single value */ > - mutex_lock(&st->lock); > - st->dac_val = val; > - mutex_unlock(&st->lock); > + scoped_guard(mutex, &st->lock) { > + /* Locking not required as writing single value */ > + st->dac_val = val; > + } > return 0; > default: > return -EINVAL; > @@ -447,9 +442,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > case IIO_CHAN_INFO_PROCESSED: > switch (chan->type) { > case IIO_STEPS: > - mutex_lock(&st->lock); > - st->steps = val; > - mutex_unlock(&st->lock); > + scoped_guard(mutex, &st->lock) { > + st->steps = val; > + } > return 0; > case IIO_ACTIVITY: > if (val < 0) > @@ -470,30 +465,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > default: > return -EINVAL; > } > - case IIO_CHAN_INFO_CALIBSCALE: > - mutex_lock(&st->lock); > + case IIO_CHAN_INFO_CALIBSCALE: { > + guard(mutex)(&st->lock); > /* Compare against table - hard matching here */ > for (i = 0; i < ARRAY_SIZE(dummy_scales); i++) > if (val == dummy_scales[i].val && > val2 == dummy_scales[i].val2) > break; > if (i == ARRAY_SIZE(dummy_scales)) > - ret = -EINVAL; > - else > - st->accel_calibscale = &dummy_scales[i]; > - mutex_unlock(&st->lock); > + return -EINVAL; > + st->accel_calibscale = &dummy_scales[i]; > return ret; Can we change this to `return 0;` and get rid of the `ret = 0` initialization at the beginning of the function? > + } > case IIO_CHAN_INFO_CALIBBIAS: > - mutex_lock(&st->lock); > - st->accel_calibbias = val; > - mutex_unlock(&st->lock); > + scoped_guard(mutex, &st->lock) { > + st->accel_calibbias = val; > + } > return 0; > case IIO_CHAN_INFO_ENABLE: > switch (chan->type) { > case IIO_STEPS: > - mutex_lock(&st->lock); > - st->steps_enabled = val; > - mutex_unlock(&st->lock); > + scoped_guard(mutex, &st->lock) { > + st->steps_enabled = val; > + } > return 0; > default: > return -EINVAL; > -- > 2.43.0 >
> > @@ -436,10 +431,10 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > if (chan->output == 0) > > return -EINVAL; > > > > - /* Locking not required as writing single value */ > > - mutex_lock(&st->lock); > > - st->dac_val = val; > > - mutex_unlock(&st->lock); > > + scoped_guard(mutex, &st->lock) { > > + /* Locking not required as writing single value */ > > + st->dac_val = val; > > + } > > return 0; > > default: > > return -EINVAL; > > @@ -447,9 +442,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > case IIO_CHAN_INFO_PROCESSED: > > switch (chan->type) { > > case IIO_STEPS: > > - mutex_lock(&st->lock); > > - st->steps = val; > > - mutex_unlock(&st->lock); > > + scoped_guard(mutex, &st->lock) { > > + st->steps = val; > > + } > > return 0; > > case IIO_ACTIVITY: > > if (val < 0) > > @@ -470,30 +465,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > default: > > return -EINVAL; > > } > > - case IIO_CHAN_INFO_CALIBSCALE: > > - mutex_lock(&st->lock); > > + case IIO_CHAN_INFO_CALIBSCALE: { > > + guard(mutex)(&st->lock); > > /* Compare against table - hard matching here */ > > for (i = 0; i < ARRAY_SIZE(dummy_scales); i++) > > if (val == dummy_scales[i].val && > > val2 == dummy_scales[i].val2) > > break; > > if (i == ARRAY_SIZE(dummy_scales)) > > - ret = -EINVAL; > > - else > > - st->accel_calibscale = &dummy_scales[i]; > > - mutex_unlock(&st->lock); > > + return -EINVAL; > > + st->accel_calibscale = &dummy_scales[i]; > > return ret; > > Can we change this to `return 0;` and get rid of the `ret = 0` > initialization at the beginning of the function? Yes. That would make sense. > > > + }
On Mon, 29 Jan 2024 11:46:22 +0000 Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > > > @@ -436,10 +431,10 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > if (chan->output == 0) > > > return -EINVAL; > > > > > > - /* Locking not required as writing single value */ > > > - mutex_lock(&st->lock); > > > - st->dac_val = val; > > > - mutex_unlock(&st->lock); > > > + scoped_guard(mutex, &st->lock) { > > > + /* Locking not required as writing single value */ > > > + st->dac_val = val; > > > + } > > > return 0; > > > default: > > > return -EINVAL; > > > @@ -447,9 +442,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > case IIO_CHAN_INFO_PROCESSED: > > > switch (chan->type) { > > > case IIO_STEPS: > > > - mutex_lock(&st->lock); > > > - st->steps = val; > > > - mutex_unlock(&st->lock); > > > + scoped_guard(mutex, &st->lock) { > > > + st->steps = val; > > > + } > > > return 0; > > > case IIO_ACTIVITY: > > > if (val < 0) > > > @@ -470,30 +465,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > default: > > > return -EINVAL; > > > } > > > - case IIO_CHAN_INFO_CALIBSCALE: > > > - mutex_lock(&st->lock); > > > + case IIO_CHAN_INFO_CALIBSCALE: { > > > + guard(mutex)(&st->lock); > > > /* Compare against table - hard matching here */ > > > for (i = 0; i < ARRAY_SIZE(dummy_scales); i++) > > > if (val == dummy_scales[i].val && > > > val2 == dummy_scales[i].val2) > > > break; > > > if (i == ARRAY_SIZE(dummy_scales)) > > > - ret = -EINVAL; > > > - else > > > - st->accel_calibscale = &dummy_scales[i]; > > > - mutex_unlock(&st->lock); > > > + return -EINVAL; > > > + st->accel_calibscale = &dummy_scales[i]; > > > return ret; > > > > Can we change this to `return 0;` and get rid of the `ret = 0` > > initialization at the beginning of the function? > > Yes. That would make sense. Given it's fairly trivial, I may not post it again but instead just tidy that up whilst applying. Diff will also git rid of the bonus space in this block. oops. diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c index d6ef556698fb..09efacaf8f78 100644 --- a/drivers/iio/dummy/iio_simple_dummy.c +++ b/drivers/iio/dummy/iio_simple_dummy.c @@ -421,7 +421,6 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, long mask) { int i; - int ret = 0; struct iio_dummy_state *st = iio_priv(indio_dev); switch (mask) { @@ -473,9 +472,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, val2 == dummy_scales[i].val2) break; if (i == ARRAY_SIZE(dummy_scales)) - return -EINVAL; + return -EINVAL; st->accel_calibscale = &dummy_scales[i]; - return ret; + return 0; } case IIO_CHAN_INFO_CALIBBIAS: scoped_guard(mutex, &st->lock) { > > > > > + } >
On Mon, Jan 29, 2024 at 1:58 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Mon, 29 Jan 2024 11:46:22 +0000 > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > > > > > @@ -436,10 +431,10 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > > if (chan->output == 0) > > > > return -EINVAL; > > > > > > > > - /* Locking not required as writing single value */ > > > > - mutex_lock(&st->lock); > > > > - st->dac_val = val; > > > > - mutex_unlock(&st->lock); > > > > + scoped_guard(mutex, &st->lock) { > > > > + /* Locking not required as writing single value */ > > > > + st->dac_val = val; > > > > + } > > > > return 0; > > > > default: > > > > return -EINVAL; > > > > @@ -447,9 +442,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > > case IIO_CHAN_INFO_PROCESSED: > > > > switch (chan->type) { > > > > case IIO_STEPS: > > > > - mutex_lock(&st->lock); > > > > - st->steps = val; > > > > - mutex_unlock(&st->lock); > > > > + scoped_guard(mutex, &st->lock) { > > > > + st->steps = val; > > > > + } > > > > return 0; > > > > case IIO_ACTIVITY: > > > > if (val < 0) > > > > @@ -470,30 +465,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > > > > default: > > > > return -EINVAL; > > > > } > > > > - case IIO_CHAN_INFO_CALIBSCALE: > > > > - mutex_lock(&st->lock); > > > > + case IIO_CHAN_INFO_CALIBSCALE: { > > > > + guard(mutex)(&st->lock); > > > > /* Compare against table - hard matching here */ > > > > for (i = 0; i < ARRAY_SIZE(dummy_scales); i++) > > > > if (val == dummy_scales[i].val && > > > > val2 == dummy_scales[i].val2) > > > > break; > > > > if (i == ARRAY_SIZE(dummy_scales)) > > > > - ret = -EINVAL; > > > > - else > > > > - st->accel_calibscale = &dummy_scales[i]; > > > > - mutex_unlock(&st->lock); > > > > + return -EINVAL; > > > > + st->accel_calibscale = &dummy_scales[i]; > > > > return ret; > > > > > > Can we change this to `return 0;` and get rid of the `ret = 0` > > > initialization at the beginning of the function? > > > > Yes. That would make sense. > > Given it's fairly trivial, I may not post it again but instead just > tidy that up whilst applying. Diff will also git rid of the bonus space > in this block. oops. In that case: Reviewed-by: David Lechner <dlechner@baylibre.com> > > diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c > index d6ef556698fb..09efacaf8f78 100644 > --- a/drivers/iio/dummy/iio_simple_dummy.c > +++ b/drivers/iio/dummy/iio_simple_dummy.c > @@ -421,7 +421,6 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > long mask) > { > int i; > - int ret = 0; > struct iio_dummy_state *st = iio_priv(indio_dev); > > switch (mask) { > @@ -473,9 +472,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, > val2 == dummy_scales[i].val2) > break; > if (i == ARRAY_SIZE(dummy_scales)) > - return -EINVAL; > + return -EINVAL; > st->accel_calibscale = &dummy_scales[i]; > - return ret; > + return 0; > } > case IIO_CHAN_INFO_CALIBBIAS: > scoped_guard(mutex, &st->lock) { > > > > > > > > + } > > >
Sun, Jan 28, 2024 at 03:05:29PM +0000, Jonathan Cameron kirjoitti: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Given we now have iio_device_claim_direct_scoped() to perform automatic > releasing of direct mode at exit from the scope that follows it, this can > be used in conjunction with guard(mutex) etc remove a lot of special case > handling. > > Note that in this particular example code, there is no real reason you can't > read channels via sysfs at the same time as filling the software buffer. > To make it look more like a real driver constrain raw and processed > channel reads from occurring whilst the buffer is in use. ... > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { > + guard(mutex)(&st->lock); > + switch (chan->type) { > + case IIO_VOLTAGE: > + if (chan->output) { > + /* Set integer part to cached value */ > + *val = st->dac_val; > + return IIO_VAL_INT; > + } else if (chan->differential) { > + if (chan->channel == 1) > + *val = st->differential_adc_val[0]; > + else > + *val = st->differential_adc_val[1]; > + return IIO_VAL_INT; > + } else { > + *val = st->single_ended_adc_val; > + return IIO_VAL_INT; > + } Now you may go further and use only single return statement here. > + case IIO_ACCEL: > + *val = st->accel_val; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > } ... > + unreachable(); Hmm... Is it really required? Why? ... P.S> I hope you are using --histogram diff algo when preparing patches.
On Sun, 4 Feb 2024 18:29:25 +0200 andy.shevchenko@gmail.com wrote: > Sun, Jan 28, 2024 at 03:05:29PM +0000, Jonathan Cameron kirjoitti: > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > Given we now have iio_device_claim_direct_scoped() to perform automatic > > releasing of direct mode at exit from the scope that follows it, this can > > be used in conjunction with guard(mutex) etc remove a lot of special case > > handling. > > > > Note that in this particular example code, there is no real reason you can't > > read channels via sysfs at the same time as filling the software buffer. > > To make it look more like a real driver constrain raw and processed > > channel reads from occurring whilst the buffer is in use. > > ... > > > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { > > + guard(mutex)(&st->lock); > > + switch (chan->type) { > > + case IIO_VOLTAGE: > > + if (chan->output) { > > + /* Set integer part to cached value */ > > + *val = st->dac_val; > > + return IIO_VAL_INT; > > + } else if (chan->differential) { > > + if (chan->channel == 1) > > + *val = st->differential_adc_val[0]; > > + else > > + *val = st->differential_adc_val[1]; > > + return IIO_VAL_INT; > > + } else { > > + *val = st->single_ended_adc_val; > > + return IIO_VAL_INT; > > + } > > Now you may go further and use only single return statement here. True though this is an example driver and it's only really coincidence those returns are the same so I'd rather keep it as explicitly matching *val with the return. > > > + case IIO_ACCEL: > > + *val = st->accel_val; > > + return IIO_VAL_INT; > > + default: > > + return -EINVAL; > > } > > ... > > > + unreachable(); > > Hmm... Is it really required? Why? Try compiling without it. (I'm running with C=1 W=1 but think this will show up anyway. Seems it can't tell we never leave the for loop. > In file included from ./include/linux/preempt.h:11, from ./include/linux/spinlock.h:56, from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:7, from ./include/linux/slab.h:16, from drivers/iio/dummy/iio_simple_dummy.c:15: drivers/iio/dummy/iio_simple_dummy.c: In function ‘iio_dummy_read_raw’: ./include/linux/cleanup.h:173:9: warning: this statement may fall through [-Wimplicit-fallthrough=] 173 | for (CLASS(_name, scope)(args), \ | ^~~ ./include/linux/iio/iio.h:667:9: note: in expansion of macro ‘scoped_cond_guard’ 667 | scoped_cond_guard(iio_claim_direct_try, fail, iio_dev) | ^~~~~~~~~~~~~~~~~ drivers/iio/dummy/iio_simple_dummy.c:289:17: note: in expansion of macro ‘iio_device_claim_direct_scoped’ 289 | iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/dummy/iio_simple_dummy.c:316:9: note: here 316 | case IIO_CHAN_INFO_PROCESSED: | ^~~~ > ... > > P.S> I hope you are using --histogram diff algo when preparing patches. >
diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c index c24f609c2ade..d6ef556698fb 100644 --- a/drivers/iio/dummy/iio_simple_dummy.c +++ b/drivers/iio/dummy/iio_simple_dummy.c @@ -283,65 +283,63 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev, long mask) { struct iio_dummy_state *st = iio_priv(indio_dev); - int ret = -EINVAL; - mutex_lock(&st->lock); switch (mask) { case IIO_CHAN_INFO_RAW: /* magic value - channel value read */ - switch (chan->type) { - case IIO_VOLTAGE: - if (chan->output) { - /* Set integer part to cached value */ - *val = st->dac_val; - ret = IIO_VAL_INT; - } else if (chan->differential) { - if (chan->channel == 1) - *val = st->differential_adc_val[0]; - else - *val = st->differential_adc_val[1]; - ret = IIO_VAL_INT; - } else { - *val = st->single_ended_adc_val; - ret = IIO_VAL_INT; + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { + guard(mutex)(&st->lock); + switch (chan->type) { + case IIO_VOLTAGE: + if (chan->output) { + /* Set integer part to cached value */ + *val = st->dac_val; + return IIO_VAL_INT; + } else if (chan->differential) { + if (chan->channel == 1) + *val = st->differential_adc_val[0]; + else + *val = st->differential_adc_val[1]; + return IIO_VAL_INT; + } else { + *val = st->single_ended_adc_val; + return IIO_VAL_INT; + } + + case IIO_ACCEL: + *val = st->accel_val; + return IIO_VAL_INT; + default: + return -EINVAL; } - break; - case IIO_ACCEL: - *val = st->accel_val; - ret = IIO_VAL_INT; - break; - default: - break; } - break; + unreachable(); case IIO_CHAN_INFO_PROCESSED: - switch (chan->type) { - case IIO_STEPS: - *val = st->steps; - ret = IIO_VAL_INT; - break; - case IIO_ACTIVITY: - switch (chan->channel2) { - case IIO_MOD_RUNNING: - *val = st->activity_running; - ret = IIO_VAL_INT; - break; - case IIO_MOD_WALKING: - *val = st->activity_walking; - ret = IIO_VAL_INT; - break; + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { + guard(mutex)(&st->lock); + switch (chan->type) { + case IIO_STEPS: + *val = st->steps; + return IIO_VAL_INT; + case IIO_ACTIVITY: + switch (chan->channel2) { + case IIO_MOD_RUNNING: + *val = st->activity_running; + return IIO_VAL_INT; + case IIO_MOD_WALKING: + *val = st->activity_walking; + return IIO_VAL_INT; + default: + return -EINVAL; + } default: - break; + return -EINVAL; } - break; - default: - break; } - break; + unreachable(); case IIO_CHAN_INFO_OFFSET: /* only single ended adc -> 7 */ *val = 7; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: switch (chan->type) { case IIO_VOLTAGE: @@ -350,60 +348,57 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev, /* only single ended adc -> 0.001333 */ *val = 0; *val2 = 1333; - ret = IIO_VAL_INT_PLUS_MICRO; - break; + return IIO_VAL_INT_PLUS_MICRO; case 1: /* all differential adc -> 0.000001344 */ *val = 0; *val2 = 1344; - ret = IIO_VAL_INT_PLUS_NANO; + return IIO_VAL_INT_PLUS_NANO; + default: + return -EINVAL; } - break; default: - break; + return -EINVAL; } - break; - case IIO_CHAN_INFO_CALIBBIAS: + case IIO_CHAN_INFO_CALIBBIAS: { + guard(mutex)(&st->lock); /* only the acceleration axis - read from cache */ *val = st->accel_calibbias; - ret = IIO_VAL_INT; - break; - case IIO_CHAN_INFO_CALIBSCALE: + return IIO_VAL_INT; + } + case IIO_CHAN_INFO_CALIBSCALE: { + guard(mutex)(&st->lock); *val = st->accel_calibscale->val; *val2 = st->accel_calibscale->val2; - ret = IIO_VAL_INT_PLUS_MICRO; - break; + return IIO_VAL_INT_PLUS_MICRO; + } case IIO_CHAN_INFO_SAMP_FREQ: *val = 3; *val2 = 33; - ret = IIO_VAL_INT_PLUS_NANO; - break; - case IIO_CHAN_INFO_ENABLE: + return IIO_VAL_INT_PLUS_NANO; + case IIO_CHAN_INFO_ENABLE: { + guard(mutex)(&st->lock); switch (chan->type) { case IIO_STEPS: *val = st->steps_enabled; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - break; + return -EINVAL; } - break; - case IIO_CHAN_INFO_CALIBHEIGHT: + } + case IIO_CHAN_INFO_CALIBHEIGHT: { + guard(mutex)(&st->lock); switch (chan->type) { case IIO_STEPS: *val = st->height; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - break; + return -EINVAL; } - break; - + } default: - break; + return -EINVAL; } - mutex_unlock(&st->lock); - return ret; } /** @@ -436,10 +431,10 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, if (chan->output == 0) return -EINVAL; - /* Locking not required as writing single value */ - mutex_lock(&st->lock); - st->dac_val = val; - mutex_unlock(&st->lock); + scoped_guard(mutex, &st->lock) { + /* Locking not required as writing single value */ + st->dac_val = val; + } return 0; default: return -EINVAL; @@ -447,9 +442,9 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_PROCESSED: switch (chan->type) { case IIO_STEPS: - mutex_lock(&st->lock); - st->steps = val; - mutex_unlock(&st->lock); + scoped_guard(mutex, &st->lock) { + st->steps = val; + } return 0; case IIO_ACTIVITY: if (val < 0) @@ -470,30 +465,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev, default: return -EINVAL; } - case IIO_CHAN_INFO_CALIBSCALE: - mutex_lock(&st->lock); + case IIO_CHAN_INFO_CALIBSCALE: { + guard(mutex)(&st->lock); /* Compare against table - hard matching here */ for (i = 0; i < ARRAY_SIZE(dummy_scales); i++) if (val == dummy_scales[i].val && val2 == dummy_scales[i].val2) break; if (i == ARRAY_SIZE(dummy_scales)) - ret = -EINVAL; - else - st->accel_calibscale = &dummy_scales[i]; - mutex_unlock(&st->lock); + return -EINVAL; + st->accel_calibscale = &dummy_scales[i]; return ret; + } case IIO_CHAN_INFO_CALIBBIAS: - mutex_lock(&st->lock); - st->accel_calibbias = val; - mutex_unlock(&st->lock); + scoped_guard(mutex, &st->lock) { + st->accel_calibbias = val; + } return 0; case IIO_CHAN_INFO_ENABLE: switch (chan->type) { case IIO_STEPS: - mutex_lock(&st->lock); - st->steps_enabled = val; - mutex_unlock(&st->lock); + scoped_guard(mutex, &st->lock) { + st->steps_enabled = val; + } return 0; default: return -EINVAL;