On Mon, Jan 29, 2024 at 1:58 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Mon, 29 Jan 2024 11:46:22 +0000
> Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> 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@xxxxxxxxxxxx>
>
> 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) {
>
> > >
> > > > + }
> >
>
