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@xxxxxxxxxx> wrote:
>
> > From: Lars-Peter Clausen <lars@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Signed-off-by: Alexandru Ardelean <Alexandru.Ardelean@xxxxxxxxxx>
>
> 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;
> >
>
>
