* [RFC] staging: iio: update locking method during frequency writes
@ 2017-03-25 19:55 Gargi Sharma
2017-03-29 21:23 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Gargi Sharma @ 2017-03-25 19:55 UTC (permalink / raw)
To: outreachy-kernel
Cc: linux-iio, lars, Michael.Hennerich, jic23, knaack.h, pmeerw,
gregkh, Gargi Sharma
The driver needs to insure atomicity during frequency
changes of bus and device. The iiodev->mlock as used
was not doing that. Replace it with the drivers existing
buffer lock and execute spi_write directly.
Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
ade7754_set_irq() has a similar read then write without any locking
mechanism in place. Do we want to introduce something similar there?
---
drivers/staging/iio/meter/ade7754.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
index c8d2d4c..48339e9a 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -513,7 +513,7 @@ static ssize_t ade7754_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->buf_lock);
t = 26000 / val;
if (t > 0)
@@ -531,10 +531,13 @@ static ssize_t ade7754_write_frequency(struct device *dev,
reg &= ~(3 << 3);
reg |= t << 3;
- ret = ade7754_spi_write_reg_8(dev, ADE7754_WAVMODE, reg);
+ st->tx[0] = ADE7754_WRITE_REG(ADE7754_WAVMODE);
+ st->tx[1] = reg;
+
+ ret = spi_write(st->us, st->tx, 2);
out:
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->buf_lock);
return ret ? ret : len;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC] staging: iio: update locking method during frequency writes
2017-03-25 19:55 [RFC] staging: iio: update locking method during frequency writes Gargi Sharma
@ 2017-03-29 21:23 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2017-03-29 21:23 UTC (permalink / raw)
To: Gargi Sharma, outreachy-kernel
Cc: linux-iio, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh
On 25/03/17 19:55, Gargi Sharma wrote:
> The driver needs to insure atomicity during frequency
> changes of bus and device. The iiodev->mlock as used
> was not doing that. Replace it with the drivers existing
> buffer lock and execute spi_write directly.
>
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> ---
> ade7754_set_irq() has a similar read then write without any locking
> mechanism in place. Do we want to introduce something similar there?
Hmm. Tricky to construct a sequence where lack of locking would be a
problem. Right now it is definitely not possible to call it twice
concurrently. If it became possible in future it would perhaps
need revisiting.
> ---
> drivers/staging/iio/meter/ade7754.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
> index c8d2d4c..48339e9a 100644
> --- a/drivers/staging/iio/meter/ade7754.c
> +++ b/drivers/staging/iio/meter/ade7754.c
> @@ -513,7 +513,7 @@ static ssize_t ade7754_write_frequency(struct device *dev,
> if (!val)
> return -EINVAL;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->buf_lock);
>
> t = 26000 / val;
> if (t > 0)
> @@ -531,10 +531,13 @@ static ssize_t ade7754_write_frequency(struct device *dev,
> reg &= ~(3 << 3);
> reg |= t << 3;
>
> - ret = ade7754_spi_write_reg_8(dev, ADE7754_WAVMODE, reg);
> + st->tx[0] = ADE7754_WRITE_REG(ADE7754_WAVMODE);
> + st->tx[1] = reg;
> +
> + ret = spi_write(st->us, st->tx, 2);
hmm. Approach is fairly sound, though might be marginally neater to define a
__ade7754_spi_write_reg_8 or similar with a comment saying it is the unlocked
version of the normally named function. Use that in ade7754_spi_write_reg_8 and
here.
More importantly, buf_lock is now protecting rather more than before so the
comment should be updated when documenting the structure.
>
> out:
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->buf_lock);
>
> return ret ? ret : len;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-29 21:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25 19:55 [RFC] staging: iio: update locking method during frequency writes Gargi Sharma
2017-03-29 21:23 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.