On Mon, Aug 8, 2016 at 4:05 AM, Vignesh R <vigneshr@xxxxxx> wrote:
> It is possible that two or more ADC channels can be simultaneously
> requested for raw samples, in which case there can be race in access to
> FIFO data resulting in loss of samples.
> If am335x_tsc_se_set_once() is called again from tiadc_read_raw(), when
> ADC is still acquired to sample one of the channels, the second process
> might be put into uninterruptible sleep state. Fix these issues, by
> protecting FIFO access and channel configurations with a mutex. Since
> tiadc_read_raw() might take anywhere between few microseconds to few
> milliseconds to finish execution (depending on averaging and delay
> values supplied via DT), its better to use mutex instead of spinlock.
>
> Signed-off-by: Vignesh R <vigneshr@xxxxxx>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 8a368756881b..bed9977a1863 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -32,6 +32,7 @@
>
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct mutex fifo1_lock; /* to protect fifo access */
Are there more than one FIFOs, or even possible? Just wondering the
number indexing here..
> int channels;
> u8 channel_line[8];
> u8 channel_step[8];
> @@ -359,6 +360,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long mask)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + int ret = IIO_VAL_INT;
> int i, map_val;
> unsigned int fifo1count, read, stepid;
> bool found = false;
> @@ -372,6 +374,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> if (!step_en)
> return -EINVAL;
>
> + mutex_lock(&adc_dev->fifo1_lock);
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> while (fifo1count--)
> tiadc_readl(adc_dev, REG_FIFO1);
> @@ -388,7 +391,8 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
>
> if (time_after(jiffies, timeout)) {
> am335x_tsc_se_adc_done(adc_dev->mfd_tscadc);
> - return -EAGAIN;
> + ret = -EAGAIN;
> + goto err_unlock;
> }
> }
> map_val = adc_dev->channel_step[chan->scan_index];
> @@ -414,8 +418,11 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> am335x_tsc_se_adc_done(adc_dev->mfd_tscadc);
>
> if (found == false)
> - return -EBUSY;
> - return IIO_VAL_INT;
> + ret = -EBUSY;
> +
> +err_unlock:
> + mutex_unlock(&adc_dev->fifo1_lock);
> + return ret;
> }
>
> static const struct iio_info tiadc_info = {
> @@ -483,6 +490,7 @@ static int tiadc_probe(struct platform_device *pdev)
>
> tiadc_step_config(indio_dev);
> tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
> + mutex_init(&adc_dev->fifo1_lock);
>
> err = tiadc_channel_init(indio_dev, adc_dev->channels);
> if (err < 0)
> --
> 2.9.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
