Re: [PATCH] iio: adc: mcp3422: fix locking scope

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 14 Aug 2020 08:24:46 +0200
Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote:

> Il giorno sab 1 ago 2020 alle ore 18:46 Jonathan Cameron
> <jic23@jic23.retrosnub.co.uk> ha scritto:
> >
> > On Sat,  1 Aug 2020 15:55:11 +0200
> > Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote:
> >  
> > > Locking should be held for the entire reading sequence involving setting
> > > the channel, waiting for the channel switch and reading from the
> > > channel.
> > > If not, reading from a channel can result mixing with the reading from
> > > another channel.
> > >
> > > Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>  
> >
> > Looks like we should be backporting this.  Fixes tag please.  
> 
> I don't know what it fixes, there is no signalled bug for this, I
> simply discovered it doing some testing.

In this case I'm just looking for which patch originally introduced the
bug.  It might be the original driver introduction of course in which
case give me a tag for that (look at SubmittingPatches.rst for info).

That info is needed to let us figure out which stable kernels we should
backport this to.

Thanks

Jonathan

> 
> >
> > Jonathan
> >  
> > > ---
> > >  drivers/iio/adc/mcp3422.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
> > > index d86c0b5d80a3..02a60fb164cd 100644
> > > --- a/drivers/iio/adc/mcp3422.c
> > > +++ b/drivers/iio/adc/mcp3422.c
> > > @@ -96,16 +96,12 @@ static int mcp3422_update_config(struct mcp3422 *adc, u8 newconfig)
> > >  {
> > >       int ret;
> > >
> > > -     mutex_lock(&adc->lock);
> > > -
> > >       ret = i2c_master_send(adc->i2c, &newconfig, 1);
> > >       if (ret > 0) {
> > >               adc->config = newconfig;
> > >               ret = 0;
> > >       }
> > >
> > > -     mutex_unlock(&adc->lock);
> > > -
> > >       return ret;
> > >  }
> > >
> > > @@ -138,6 +134,8 @@ static int mcp3422_read_channel(struct mcp3422 *adc,
> > >       u8 config;
> > >       u8 req_channel = channel->channel;
> > >
> > > +     mutex_lock(&adc->lock);
> > > +
> > >       if (req_channel != MCP3422_CHANNEL(adc->config)) {
> > >               config = adc->config;
> > >               config &= ~MCP3422_CHANNEL_MASK;
> > > @@ -150,7 +148,11 @@ static int mcp3422_read_channel(struct mcp3422 *adc,
> > >               msleep(mcp3422_read_times[MCP3422_SAMPLE_RATE(adc->config)]);
> > >       }
> > >
> > > -     return mcp3422_read(adc, value, &config);
> > > +     ret = mcp3422_read(adc, value, &config);
> > > +
> > > +     mutex_unlock(&adc->lock);
> > > +
> > > +     return ret;
> > >  }
> > >
> > >  static int mcp3422_read_raw(struct iio_dev *iio,  
> >  
> 
>