Re: [PATCH 02/10] iio: dummy: Use automatic lock and direct mode cleanup.

[Jonathan Cameron <jic23@kernel.org>]

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) {

> >   
> > > +       }  
>