linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion
@ 2019-03-19 11:37 Alexandru Ardelean
  2019-03-24 17:47 ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandru Ardelean @ 2019-03-19 11:37 UTC (permalink / raw)
  To: linux-iio; +Cc: Lars-Peter Clausen, Alexandru Ardelean

From: Lars-Peter Clausen <lars@metafoo.de>

For devices from the SigmaDelta family we need to keep CS low when doing a
conversion, since the device will use the MISO line as a interrupt to
indicate that the conversion is complete.

This is why the driver locks the SPI bus and when the SPI bus is locked
keeps as long as a conversion is going on. The current implementation gets
one small detail wrong though. CS is only de-asserted after the SPI bus is
unlocked. This means it is possible for a different SPI device on the same
bus to send a message which would be wrongfully be addressed to the
SigmaDelta device as well. Make sure that the last SPI transfer that is
done while holding the SPI bus lock de-asserts the CS signal.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <Alexandru.Ardelean@analog.com>
---

Changelog v1 -> v2:
* added my S-o-B line

 drivers/iio/adc/ad_sigma_delta.c       | 16 +++++++++++-----
 include/linux/iio/adc/ad_sigma_delta.h |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index ff5f2da2e1b1..af6cbc683214 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -62,7 +62,7 @@ int ad_sd_write_reg(struct ad_sigma_delta *sigma_delta, unsigned int reg,
 	struct spi_transfer t = {
 		.tx_buf		= data,
 		.len		= size + 1,
-		.cs_change	= sigma_delta->bus_locked,
+		.cs_change	= sigma_delta->keep_cs_asserted,
 	};
 	struct spi_message m;
 	int ret;
@@ -217,6 +217,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
 
 	spi_bus_lock(sigma_delta->spi->master);
 	sigma_delta->bus_locked = true;
+	sigma_delta->keep_cs_asserted = true;
 	reinit_completion(&sigma_delta->completion);
 
 	ret = ad_sigma_delta_set_mode(sigma_delta, mode);
@@ -234,9 +235,10 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
 		ret = 0;
 	}
 out:
+	sigma_delta->keep_cs_asserted = false;
+	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
 	sigma_delta->bus_locked = false;
 	spi_bus_unlock(sigma_delta->spi->master);
-	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
 
 	return ret;
 }
@@ -289,6 +291,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
 
 	spi_bus_lock(sigma_delta->spi->master);
 	sigma_delta->bus_locked = true;
+	sigma_delta->keep_cs_asserted = true;
 	reinit_completion(&sigma_delta->completion);
 
 	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);
@@ -298,9 +301,6 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
 	ret = wait_for_completion_interruptible_timeout(
 			&sigma_delta->completion, HZ);
 
-	sigma_delta->bus_locked = false;
-	spi_bus_unlock(sigma_delta->spi->master);
-
 	if (ret == 0)
 		ret = -EIO;
 	if (ret < 0)
@@ -321,7 +321,10 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
 		sigma_delta->irq_dis = true;
 	}
 
+	sigma_delta->keep_cs_asserted = false;
 	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);
 
 	if (ret)
@@ -358,6 +361,8 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
 
 	spi_bus_lock(sigma_delta->spi->master);
 	sigma_delta->bus_locked = true;
+	sigma_delta->keep_cs_asserted = true;
+
 	ret = ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_CONTINUOUS);
 	if (ret)
 		goto err_unlock;
@@ -386,6 +391,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev *indio_dev)
 		sigma_delta->irq_dis = true;
 	}
 
+	sigma_delta->keep_cs_asserted = false;
 	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
 
 	sigma_delta->bus_locked = false;
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 7e84351fa2c0..6e9fb1932dde 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -69,6 +69,7 @@ struct ad_sigma_delta {
 	bool			irq_dis;
 
 	bool			bus_locked;
+	bool			keep_cs_asserted;
 
 	uint8_t			comm;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion
  2019-03-19 11:37 [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion Alexandru Ardelean
@ 2019-03-24 17:47 ` Jonathan Cameron
  2019-03-26 12:43   ` Ardelean, Alexandru
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2019-03-24 17:47 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, Lars-Peter Clausen

On Tue, 19 Mar 2019 13:37:55 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> For devices from the SigmaDelta family we need to keep CS low when doing a
> conversion, since the device will use the MISO line as a interrupt to
> indicate that the conversion is complete.
> 
> This is why the driver locks the SPI bus and when the SPI bus is locked
> keeps as long as a conversion is going on. The current implementation gets
> one small detail wrong though. CS is only de-asserted after the SPI bus is
> unlocked. This means it is possible for a different SPI device on the same
> bus to send a message which would be wrongfully be addressed to the
> SigmaDelta device as well. Make sure that the last SPI transfer that is
> done while holding the SPI bus lock de-asserts the CS signal.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <Alexandru.Ardelean@analog.com>
Hi Alex,

So, it's a fix for a long existing problem.  Do we have anyone who
has experienced it?  I'm trying to judge whether this is stable material
or not.  If it is stable material, do you want to have a go at identifying
a patch for the Fixes tag?

Patch looks good.

Jonathan

> ---
> 
> Changelog v1 -> v2:
> * added my S-o-B line
> 
>  drivers/iio/adc/ad_sigma_delta.c       | 16 +++++++++++-----
>  include/linux/iio/adc/ad_sigma_delta.h |  1 +
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index ff5f2da2e1b1..af6cbc683214 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -62,7 +62,7 @@ int ad_sd_write_reg(struct ad_sigma_delta *sigma_delta, unsigned int reg,
>  	struct spi_transfer t = {
>  		.tx_buf		= data,
>  		.len		= size + 1,
> -		.cs_change	= sigma_delta->bus_locked,
> +		.cs_change	= sigma_delta->keep_cs_asserted,
>  	};
>  	struct spi_message m;
>  	int ret;
> @@ -217,6 +217,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
>  
>  	spi_bus_lock(sigma_delta->spi->master);
>  	sigma_delta->bus_locked = true;
> +	sigma_delta->keep_cs_asserted = true;
>  	reinit_completion(&sigma_delta->completion);
>  
>  	ret = ad_sigma_delta_set_mode(sigma_delta, mode);
> @@ -234,9 +235,10 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
>  		ret = 0;
>  	}
>  out:
> +	sigma_delta->keep_cs_asserted = false;
> +	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
>  	sigma_delta->bus_locked = false;
>  	spi_bus_unlock(sigma_delta->spi->master);
> -	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
>  
>  	return ret;
>  }
> @@ -289,6 +291,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
>  
>  	spi_bus_lock(sigma_delta->spi->master);
>  	sigma_delta->bus_locked = true;
> +	sigma_delta->keep_cs_asserted = true;
>  	reinit_completion(&sigma_delta->completion);
>  
>  	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);
> @@ -298,9 +301,6 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
>  	ret = wait_for_completion_interruptible_timeout(
>  			&sigma_delta->completion, HZ);
>  
> -	sigma_delta->bus_locked = false;
> -	spi_bus_unlock(sigma_delta->spi->master);
> -
>  	if (ret == 0)
>  		ret = -EIO;
>  	if (ret < 0)
> @@ -321,7 +321,10 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
>  		sigma_delta->irq_dis = true;
>  	}
>  
> +	sigma_delta->keep_cs_asserted = false;
>  	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);
>  
>  	if (ret)
> @@ -358,6 +361,8 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
>  
>  	spi_bus_lock(sigma_delta->spi->master);
>  	sigma_delta->bus_locked = true;
> +	sigma_delta->keep_cs_asserted = true;
> +
>  	ret = ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_CONTINUOUS);
>  	if (ret)
>  		goto err_unlock;
> @@ -386,6 +391,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev *indio_dev)
>  		sigma_delta->irq_dis = true;
>  	}
>  
> +	sigma_delta->keep_cs_asserted = false;
>  	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
>  
>  	sigma_delta->bus_locked = false;
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
> index 7e84351fa2c0..6e9fb1932dde 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -69,6 +69,7 @@ struct ad_sigma_delta {
>  	bool			irq_dis;
>  
>  	bool			bus_locked;
> +	bool			keep_cs_asserted;
>  
>  	uint8_t			comm;
>  


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion
  2019-03-24 17:47 ` Jonathan Cameron
@ 2019-03-26 12:43   ` Ardelean, Alexandru
  2019-03-31 11:07     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Ardelean, Alexandru @ 2019-03-26 12:43 UTC (permalink / raw)
  To: jic23; +Cc: lars, linux-iio

On Sun, 2019-03-24 at 17:47 +0000, Jonathan Cameron wrote:
> [External]
> 
> 
> On Tue, 19 Mar 2019 13:37:55 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > From: Lars-Peter Clausen <lars@metafoo.de>
> > 
> > For devices from the SigmaDelta family we need to keep CS low when
> > doing a
> > conversion, since the device will use the MISO line as a interrupt to
> > indicate that the conversion is complete.
> > 
> > This is why the driver locks the SPI bus and when the SPI bus is locked
> > keeps as long as a conversion is going on. The current implementation
> > gets
> > one small detail wrong though. CS is only de-asserted after the SPI bus
> > is
> > unlocked. This means it is possible for a different SPI device on the
> > same
> > bus to send a message which would be wrongfully be addressed to the
> > SigmaDelta device as well. Make sure that the last SPI transfer that is
> > done while holding the SPI bus lock de-asserts the CS signal.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <Alexandru.Ardelean@analog.com>
> 
> Hi Alex,
> 
> So, it's a fix for a long existing problem.  Do we have anyone who
> has experienced it?  I'm trying to judge whether this is stable material
> or not.  If it is stable material, do you want to have a go at
> identifying
> a patch for the Fixes tag?

So I actually forgot to do the preparation before cherry-picking this one
[i.e. whether it should have a Fixes tag or not].
Sorry abou that.

This change is more in the context of some non-upstream functionality for
the SigmaDelta. i.e. it's a fix, but it's more important for a
functionality that will be added later.

There aren't any people that have experienced issues with the mainline
version of the ad_sigma_delta drivers that I know of.

Bottom line is, we can skip adding this one to stable.

Thanks
Alex


> Patch looks good.
> 
> Jonathan
> 
> > ---
> > 
> > Changelog v1 -> v2:
> > * added my S-o-B line
> > 
> >  drivers/iio/adc/ad_sigma_delta.c       | 16 +++++++++++-----
> >  include/linux/iio/adc/ad_sigma_delta.h |  1 +
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad_sigma_delta.c
> > b/drivers/iio/adc/ad_sigma_delta.c
> > index ff5f2da2e1b1..af6cbc683214 100644
> > --- a/drivers/iio/adc/ad_sigma_delta.c
> > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > @@ -62,7 +62,7 @@ int ad_sd_write_reg(struct ad_sigma_delta
> > *sigma_delta, unsigned int reg,
> >       struct spi_transfer t = {
> >               .tx_buf         = data,
> >               .len            = size + 1,
> > -             .cs_change      = sigma_delta->bus_locked,
> > +             .cs_change      = sigma_delta->keep_cs_asserted,
> >       };
> >       struct spi_message m;
> >       int ret;
> > @@ -217,6 +217,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta
> > *sigma_delta,
> > 
> >       spi_bus_lock(sigma_delta->spi->master);
> >       sigma_delta->bus_locked = true;
> > +     sigma_delta->keep_cs_asserted = true;
> >       reinit_completion(&sigma_delta->completion);
> > 
> >       ret = ad_sigma_delta_set_mode(sigma_delta, mode);
> > @@ -234,9 +235,10 @@ static int ad_sd_calibrate(struct ad_sigma_delta
> > *sigma_delta,
> >               ret = 0;
> >       }
> >  out:
> > +     sigma_delta->keep_cs_asserted = false;
> > +     ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> >       sigma_delta->bus_locked = false;
> >       spi_bus_unlock(sigma_delta->spi->master);
> > -     ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> > 
> >       return ret;
> >  }
> > @@ -289,6 +291,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev
> > *indio_dev,
> > 
> >       spi_bus_lock(sigma_delta->spi->master);
> >       sigma_delta->bus_locked = true;
> > +     sigma_delta->keep_cs_asserted = true;
> >       reinit_completion(&sigma_delta->completion);
> > 
> >       ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);
> > @@ -298,9 +301,6 @@ int ad_sigma_delta_single_conversion(struct iio_dev
> > *indio_dev,
> >       ret = wait_for_completion_interruptible_timeout(
> >                       &sigma_delta->completion, HZ);
> > 
> > -     sigma_delta->bus_locked = false;
> > -     spi_bus_unlock(sigma_delta->spi->master);
> > -
> >       if (ret == 0)
> >               ret = -EIO;
> >       if (ret < 0)
> > @@ -321,7 +321,10 @@ int ad_sigma_delta_single_conversion(struct
> > iio_dev *indio_dev,
> >               sigma_delta->irq_dis = true;
> >       }
> > 
> > +     sigma_delta->keep_cs_asserted = false;
> >       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);
> > 
> >       if (ret)
> > @@ -358,6 +361,8 @@ static int ad_sd_buffer_postenable(struct iio_dev
> > *indio_dev)
> > 
> >       spi_bus_lock(sigma_delta->spi->master);
> >       sigma_delta->bus_locked = true;
> > +     sigma_delta->keep_cs_asserted = true;
> > +
> >       ret = ad_sigma_delta_set_mode(sigma_delta,
> > AD_SD_MODE_CONTINUOUS);
> >       if (ret)
> >               goto err_unlock;
> > @@ -386,6 +391,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev
> > *indio_dev)
> >               sigma_delta->irq_dis = true;
> >       }
> > 
> > +     sigma_delta->keep_cs_asserted = false;
> >       ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> > 
> >       sigma_delta->bus_locked = false;
> > diff --git a/include/linux/iio/adc/ad_sigma_delta.h
> > b/include/linux/iio/adc/ad_sigma_delta.h
> > index 7e84351fa2c0..6e9fb1932dde 100644
> > --- a/include/linux/iio/adc/ad_sigma_delta.h
> > +++ b/include/linux/iio/adc/ad_sigma_delta.h
> > @@ -69,6 +69,7 @@ struct ad_sigma_delta {
> >       bool                    irq_dis;
> > 
> >       bool                    bus_locked;
> > +     bool                    keep_cs_asserted;
> > 
> >       uint8_t                 comm;
> > 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion
  2019-03-26 12:43   ` Ardelean, Alexandru
@ 2019-03-31 11:07     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2019-03-31 11:07 UTC (permalink / raw)
  To: Ardelean, Alexandru; +Cc: lars, linux-iio

On Tue, 26 Mar 2019 12:43:32 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Sun, 2019-03-24 at 17:47 +0000, Jonathan Cameron wrote:
> > [External]
> > 
> > 
> > On Tue, 19 Mar 2019 13:37:55 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >   
> > > From: Lars-Peter Clausen <lars@metafoo.de>
> > > 
> > > For devices from the SigmaDelta family we need to keep CS low when
> > > doing a
> > > conversion, since the device will use the MISO line as a interrupt to
> > > indicate that the conversion is complete.
> > > 
> > > This is why the driver locks the SPI bus and when the SPI bus is locked
> > > keeps as long as a conversion is going on. The current implementation
> > > gets
> > > one small detail wrong though. CS is only de-asserted after the SPI bus
> > > is
> > > unlocked. This means it is possible for a different SPI device on the
> > > same
> > > bus to send a message which would be wrongfully be addressed to the
> > > SigmaDelta device as well. Make sure that the last SPI transfer that is
> > > done while holding the SPI bus lock de-asserts the CS signal.
> > > 
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Alexandru Ardelean <Alexandru.Ardelean@analog.com>  
> > 
> > Hi Alex,
> > 
> > So, it's a fix for a long existing problem.  Do we have anyone who
> > has experienced it?  I'm trying to judge whether this is stable material
> > or not.  If it is stable material, do you want to have a go at
> > identifying
> > a patch for the Fixes tag?  
> 
> So I actually forgot to do the preparation before cherry-picking this one
> [i.e. whether it should have a Fixes tag or not].
> Sorry abou that.
> 
> This change is more in the context of some non-upstream functionality for
> the SigmaDelta. i.e. it's a fix, but it's more important for a
> functionality that will be added later.
> 
> There aren't any people that have experienced issues with the mainline
> version of the ad_sigma_delta drivers that I know of.
> 
> Bottom line is, we can skip adding this one to stable.
> 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> Thanks
> Alex
> 
> 
> > Patch looks good.
> > 
> > Jonathan
> >   
> > > ---
> > > 
> > > Changelog v1 -> v2:
> > > * added my S-o-B line
> > > 
> > >  drivers/iio/adc/ad_sigma_delta.c       | 16 +++++++++++-----
> > >  include/linux/iio/adc/ad_sigma_delta.h |  1 +
> > >  2 files changed, 12 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/ad_sigma_delta.c
> > > b/drivers/iio/adc/ad_sigma_delta.c
> > > index ff5f2da2e1b1..af6cbc683214 100644
> > > --- a/drivers/iio/adc/ad_sigma_delta.c
> > > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > > @@ -62,7 +62,7 @@ int ad_sd_write_reg(struct ad_sigma_delta
> > > *sigma_delta, unsigned int reg,
> > >       struct spi_transfer t = {
> > >               .tx_buf         = data,
> > >               .len            = size + 1,
> > > -             .cs_change      = sigma_delta->bus_locked,
> > > +             .cs_change      = sigma_delta->keep_cs_asserted,
> > >       };
> > >       struct spi_message m;
> > >       int ret;
> > > @@ -217,6 +217,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta
> > > *sigma_delta,
> > > 
> > >       spi_bus_lock(sigma_delta->spi->master);
> > >       sigma_delta->bus_locked = true;
> > > +     sigma_delta->keep_cs_asserted = true;
> > >       reinit_completion(&sigma_delta->completion);
> > > 
> > >       ret = ad_sigma_delta_set_mode(sigma_delta, mode);
> > > @@ -234,9 +235,10 @@ static int ad_sd_calibrate(struct ad_sigma_delta
> > > *sigma_delta,
> > >               ret = 0;
> > >       }
> > >  out:
> > > +     sigma_delta->keep_cs_asserted = false;
> > > +     ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> > >       sigma_delta->bus_locked = false;
> > >       spi_bus_unlock(sigma_delta->spi->master);
> > > -     ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> > > 
> > >       return ret;
> > >  }
> > > @@ -289,6 +291,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev
> > > *indio_dev,
> > > 
> > >       spi_bus_lock(sigma_delta->spi->master);
> > >       sigma_delta->bus_locked = true;
> > > +     sigma_delta->keep_cs_asserted = true;
> > >       reinit_completion(&sigma_delta->completion);
> > > 
> > >       ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);
> > > @@ -298,9 +301,6 @@ int ad_sigma_delta_single_conversion(struct iio_dev
> > > *indio_dev,
> > >       ret = wait_for_completion_interruptible_timeout(
> > >                       &sigma_delta->completion, HZ);
> > > 
> > > -     sigma_delta->bus_locked = false;
> > > -     spi_bus_unlock(sigma_delta->spi->master);
> > > -
> > >       if (ret == 0)
> > >               ret = -EIO;
> > >       if (ret < 0)
> > > @@ -321,7 +321,10 @@ int ad_sigma_delta_single_conversion(struct
> > > iio_dev *indio_dev,
> > >               sigma_delta->irq_dis = true;
> > >       }
> > > 
> > > +     sigma_delta->keep_cs_asserted = false;
> > >       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);
> > > 
> > >       if (ret)
> > > @@ -358,6 +361,8 @@ static int ad_sd_buffer_postenable(struct iio_dev
> > > *indio_dev)
> > > 
> > >       spi_bus_lock(sigma_delta->spi->master);
> > >       sigma_delta->bus_locked = true;
> > > +     sigma_delta->keep_cs_asserted = true;
> > > +
> > >       ret = ad_sigma_delta_set_mode(sigma_delta,
> > > AD_SD_MODE_CONTINUOUS);
> > >       if (ret)
> > >               goto err_unlock;
> > > @@ -386,6 +391,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev
> > > *indio_dev)
> > >               sigma_delta->irq_dis = true;
> > >       }
> > > 
> > > +     sigma_delta->keep_cs_asserted = false;
> > >       ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
> > > 
> > >       sigma_delta->bus_locked = false;
> > > diff --git a/include/linux/iio/adc/ad_sigma_delta.h
> > > b/include/linux/iio/adc/ad_sigma_delta.h
> > > index 7e84351fa2c0..6e9fb1932dde 100644
> > > --- a/include/linux/iio/adc/ad_sigma_delta.h
> > > +++ b/include/linux/iio/adc/ad_sigma_delta.h
> > > @@ -69,6 +69,7 @@ struct ad_sigma_delta {
> > >       bool                    irq_dis;
> > > 
> > >       bool                    bus_locked;
> > > +     bool                    keep_cs_asserted;
> > > 
> > >       uint8_t                 comm;
> > >   
> > 
> >   


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-31 11:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19 11:37 [PATCH V2] iio: ad_sigma_delta: Properly handle SPI bus locking vs CS assertion Alexandru Ardelean
2019-03-24 17:47 ` Jonathan Cameron
2019-03-26 12:43   ` Ardelean, Alexandru
2019-03-31 11:07     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).