* [PATCH v2] staging: iio: adc: Replace mlock with driver private lock
@ 2017-03-15 13:28 Gargi Sharma
2017-03-15 19:38 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Gargi Sharma @ 2017-03-15 13:28 UTC (permalink / raw)
To: outreachy-kernel
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, linux-iio,
Gargi Sharma
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
Changes in v2:
- Made the commit log clearer.
---
drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index ee679ac..b482606 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -134,6 +134,7 @@ struct ad7280_state {
unsigned char aux_threshhigh;
unsigned char aux_threshlow;
unsigned char cb_mask[AD7280A_MAX_CHAIN];
+ struct mutex lock; /* protect sensor state */
__be32 buf[2] ____cacheline_aligned;
};
@@ -410,7 +411,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
devaddr = this_attr->address >> 8;
ch = this_attr->address & 0xFF;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
if (readin)
st->cb_mask[devaddr] |= 1 << (ch + 2);
else
@@ -418,7 +419,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
ret = ad7280_write(st, devaddr, AD7280A_CELL_BALANCE,
0, st->cb_mask[devaddr]);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret ? ret : len;
}
@@ -433,10 +434,10 @@ static ssize_t ad7280_show_balance_timer(struct device *dev,
int ret;
unsigned int msecs;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
ret = ad7280_read(st, this_attr->address >> 8,
this_attr->address & 0xFF);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
if (ret < 0)
return ret;
@@ -466,11 +467,11 @@ static ssize_t ad7280_store_balance_timer(struct device *dev,
if (val > 31)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
ret = ad7280_write(st, this_attr->address >> 8,
this_attr->address & 0xFF,
0, (val & 0x1F) << 3);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret ? ret : len;
}
@@ -655,7 +656,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
val = clamp(val, 0L, 0xFFL);
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
switch ((u32)this_attr->address) {
case AD7280A_CELL_OVERVOLTAGE:
st->cell_threshhigh = val;
@@ -674,7 +675,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
this_attr->address, 1, val);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret ? ret : len;
}
@@ -792,13 +793,13 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
switch (m) {
case IIO_CHAN_INFO_RAW:
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
if (chan->address == AD7280A_ALL_CELLS)
ret = ad7280_read_all_channels(st, st->scan_cnt, NULL);
else
ret = ad7280_read_channel(st, chan->address >> 8,
chan->address & 0xFF);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
if (ret < 0)
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] staging: iio: adc: Replace mlock with driver private lock
2017-03-15 13:28 [PATCH v2] staging: iio: adc: Replace mlock with driver private lock Gargi Sharma
@ 2017-03-15 19:38 ` Jonathan Cameron
2017-03-15 22:00 ` Alison Schofield
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2017-03-15 19:38 UTC (permalink / raw)
To: Gargi Sharma, outreachy-kernel
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, linux-iio
On 15/03/17 13:28, Gargi Sharma wrote:
> The IIO subsystem is redefining iio_dev->mlock to be used by
> the IIO core only for protecting device operating mode changes.
> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>
> In this driver, mlock was being used to protect hardware state
> changes. Replace it with a lock in the devices global data.
>
> Signed-off-by: Gargi Sharma <gs051095@gmail.com>
Was missing from Alison's original patch (since fixed), but
we need to call mutex_init on the mutex somewhere suitable in probe.
Thanks,
Jonathan
> ---
> Changes in v2:
> - Made the commit log clearer.
> ---
> drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index ee679ac..b482606 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -134,6 +134,7 @@ struct ad7280_state {
> unsigned char aux_threshhigh;
> unsigned char aux_threshlow;
> unsigned char cb_mask[AD7280A_MAX_CHAIN];
> + struct mutex lock; /* protect sensor state */
>
> __be32 buf[2] ____cacheline_aligned;
> };
> @@ -410,7 +411,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
> devaddr = this_attr->address >> 8;
> ch = this_attr->address & 0xFF;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> if (readin)
> st->cb_mask[devaddr] |= 1 << (ch + 2);
> else
> @@ -418,7 +419,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
>
> ret = ad7280_write(st, devaddr, AD7280A_CELL_BALANCE,
> 0, st->cb_mask[devaddr]);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return ret ? ret : len;
> }
> @@ -433,10 +434,10 @@ static ssize_t ad7280_show_balance_timer(struct device *dev,
> int ret;
> unsigned int msecs;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> ret = ad7280_read(st, this_attr->address >> 8,
> this_attr->address & 0xFF);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> if (ret < 0)
> return ret;
> @@ -466,11 +467,11 @@ static ssize_t ad7280_store_balance_timer(struct device *dev,
> if (val > 31)
> return -EINVAL;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> ret = ad7280_write(st, this_attr->address >> 8,
> this_attr->address & 0xFF,
> 0, (val & 0x1F) << 3);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return ret ? ret : len;
> }
> @@ -655,7 +656,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
>
> val = clamp(val, 0L, 0xFFL);
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> switch ((u32)this_attr->address) {
> case AD7280A_CELL_OVERVOLTAGE:
> st->cell_threshhigh = val;
> @@ -674,7 +675,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
> ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
> this_attr->address, 1, val);
>
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return ret ? ret : len;
> }
> @@ -792,13 +793,13 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
>
> switch (m) {
> case IIO_CHAN_INFO_RAW:
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> if (chan->address == AD7280A_ALL_CELLS)
> ret = ad7280_read_all_channels(st, st->scan_cnt, NULL);
> else
> ret = ad7280_read_channel(st, chan->address >> 8,
> chan->address & 0xFF);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> if (ret < 0)
> return ret;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] staging: iio: adc: Replace mlock with driver private lock
2017-03-15 19:38 ` Jonathan Cameron
@ 2017-03-15 22:00 ` Alison Schofield
0 siblings, 0 replies; 3+ messages in thread
From: Alison Schofield @ 2017-03-15 22:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Gargi Sharma, outreachy-kernel, lars, Michael.Hennerich, knaack.h,
pmeerw, linux-iio
On Wed, Mar 15, 2017 at 07:38:33PM +0000, Jonathan Cameron wrote:
> On 15/03/17 13:28, Gargi Sharma wrote:
> > The IIO subsystem is redefining iio_dev->mlock to be used by
> > the IIO core only for protecting device operating mode changes.
> > ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
> >
> > In this driver, mlock was being used to protect hardware state
> > changes. Replace it with a lock in the devices global data.
> >
> > Signed-off-by: Gargi Sharma <gs051095@gmail.com>
> Was missing from Alison's original patch (since fixed), but
> we need to call mutex_init on the mutex somewhere suitable in probe.
>
> Thanks,
>
> Jonathan
Hi Gargi - please include the driver name in subject line when you rev it.
alisons
> > ---
> > Changes in v2:
> > - Made the commit log clearer.
> > ---
> > drivers/staging/iio/adc/ad7280a.c | 21 +++++++++++----------
> > 1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > index ee679ac..b482606 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -134,6 +134,7 @@ struct ad7280_state {
> > unsigned char aux_threshhigh;
> > unsigned char aux_threshlow;
> > unsigned char cb_mask[AD7280A_MAX_CHAIN];
> > + struct mutex lock; /* protect sensor state */
> >
> > __be32 buf[2] ____cacheline_aligned;
> > };
> > @@ -410,7 +411,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
> > devaddr = this_attr->address >> 8;
> > ch = this_attr->address & 0xFF;
> >
> > - mutex_lock(&indio_dev->mlock);
> > + mutex_lock(&st->lock);
> > if (readin)
> > st->cb_mask[devaddr] |= 1 << (ch + 2);
> > else
> > @@ -418,7 +419,7 @@ static ssize_t ad7280_store_balance_sw(struct device *dev,
> >
> > ret = ad7280_write(st, devaddr, AD7280A_CELL_BALANCE,
> > 0, st->cb_mask[devaddr]);
> > - mutex_unlock(&indio_dev->mlock);
> > + mutex_unlock(&st->lock);
> >
> > return ret ? ret : len;
> > }
> > @@ -433,10 +434,10 @@ static ssize_t ad7280_show_balance_timer(struct device *dev,
> > int ret;
> > unsigned int msecs;
> >
> > - mutex_lock(&indio_dev->mlock);
> > + mutex_lock(&st->lock);
> > ret = ad7280_read(st, this_attr->address >> 8,
> > this_attr->address & 0xFF);
> > - mutex_unlock(&indio_dev->mlock);
> > + mutex_unlock(&st->lock);
> >
> > if (ret < 0)
> > return ret;
> > @@ -466,11 +467,11 @@ static ssize_t ad7280_store_balance_timer(struct device *dev,
> > if (val > 31)
> > return -EINVAL;
> >
> > - mutex_lock(&indio_dev->mlock);
> > + mutex_lock(&st->lock);
> > ret = ad7280_write(st, this_attr->address >> 8,
> > this_attr->address & 0xFF,
> > 0, (val & 0x1F) << 3);
> > - mutex_unlock(&indio_dev->mlock);
> > + mutex_unlock(&st->lock);
> >
> > return ret ? ret : len;
> > }
> > @@ -655,7 +656,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
> >
> > val = clamp(val, 0L, 0xFFL);
> >
> > - mutex_lock(&indio_dev->mlock);
> > + mutex_lock(&st->lock);
> > switch ((u32)this_attr->address) {
> > case AD7280A_CELL_OVERVOLTAGE:
> > st->cell_threshhigh = val;
> > @@ -674,7 +675,7 @@ static ssize_t ad7280_write_channel_config(struct device *dev,
> > ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
> > this_attr->address, 1, val);
> >
> > - mutex_unlock(&indio_dev->mlock);
> > + mutex_unlock(&st->lock);
> >
> > return ret ? ret : len;
> > }
> > @@ -792,13 +793,13 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
> >
> > switch (m) {
> > case IIO_CHAN_INFO_RAW:
> > - mutex_lock(&indio_dev->mlock);
> > + mutex_lock(&st->lock);
> > if (chan->address == AD7280A_ALL_CELLS)
> > ret = ad7280_read_all_channels(st, st->scan_cnt, NULL);
> > else
> > ret = ad7280_read_channel(st, chan->address >> 8,
> > chan->address & 0xFF);
> > - mutex_unlock(&indio_dev->mlock);
> > + mutex_unlock(&st->lock);
> >
> > if (ret < 0)
> > return ret;
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-15 22:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-15 13:28 [PATCH v2] staging: iio: adc: Replace mlock with driver private lock Gargi Sharma
2017-03-15 19:38 ` Jonathan Cameron
2017-03-15 22:00 ` Alison Schofield
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox