Re: [PATCH 01/15] iio: adc: ad_sigma_delta: do not use internal iio_dev lock

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 20 Sep 2022 13:54:10 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Nuno,
> 
> nuno.sa@analog.com wrote on Tue, 20 Sep 2022 13:28:07 +0200:
> 
> > Drop 'mlock' usage by making use of iio_device_claim_direct_mode().
> > This change actually makes sure we cannot do a single conversion while
> > buffering is enable. Note there was a potential race in the previous
> > code since we were only acquiring the lock after checking if the bus is
> > enabled.
> > 
> > Fixes: af3008485ea0 ("iio:adc: Add common code for ADI Sigma Deltadevices")  
> 
> To answer your question in the cover letter, I feel like this Fixes:
> tag is deserved.
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Agreed to the fixes tag being valid.
There is the slightly fun aspect that the issue predates
the introduction of iio_device_claim_direct_mode() so when this gets
backported we could get failures.  However, I think the oldest kernel
anyone is bothering with in the stable series is 4.9 and that function
was introduced earlier the same year as that kernel so we are probably fine :)

I'm going to pick the easy ones up in this series, though note that these
are 'probably' now being queued up for next cycle.  Linus made some noises
about a possible rc8 though so I might do an extra pull request if that looks
likely - then these might make it in rather sooner.

Jonathan



> 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/iio/adc/ad_sigma_delta.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> > index 261a9a6b45e1..d8570f620785 100644
> > --- a/drivers/iio/adc/ad_sigma_delta.c
> > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > @@ -281,10 +281,10 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
> >  	unsigned int data_reg;
> >  	int ret = 0;
> >  
> > -	if (iio_buffer_enabled(indio_dev))
> > -		return -EBUSY;
> > +	ret = iio_device_claim_direct_mode(indio_dev);
> > +	if (ret)
> > +		return ret;
> >  
> > -	mutex_lock(&indio_dev->mlock);
> >  	ad_sigma_delta_set_channel(sigma_delta, chan->address);
> >  
> >  	spi_bus_lock(sigma_delta->spi->master);
> > @@ -323,7 +323,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
> >  	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> >  	sigma_delta->bus_locked = false;
> >  	spi_bus_unlock(sigma_delta->spi->master);
> > -	mutex_unlock(&indio_dev->mlock);
> > +	iio_device_release_direct_mode(indio_dev);
> >  
> >  	if (ret)
> >  		return ret;  
> 
> 
> Thanks,
> Miquèl


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic