From: Sergiu Cuciurean <[email protected]>
As part of the general cleanup of indio_dev->mlock, this change replaces
it with a local lock on the device's state structure.
This is part of a bigger cleanup.
Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
Signed-off-by: Sergiu Cuciurean <[email protected]>
Signed-off-by: Mircea Caprioru <[email protected]>
---
drivers/iio/adc/spear_adc.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c
index 1bc986a7009d..d93e580b3dc5 100644
--- a/drivers/iio/adc/spear_adc.c
+++ b/drivers/iio/adc/spear_adc.c
@@ -75,6 +75,15 @@ struct spear_adc_state {
struct adc_regs_spear6xx __iomem *adc_base_spear6xx;
struct clk *clk;
struct completion completion;
+ /*
+ * Lock to protect the device state during a potential concurrent
+ * read access from userspace. Reading a raw value requires a sequence
+ * of register writes, then a wait for a completion callback,
+ * and finally a register read, during which userspace could issue
+ * another read request. This lock protects a read access from
+ * ocurring before another one has finished.
+ */
+ struct mutex lock;
u32 current_clk;
u32 sampling_freq;
u32 avg_samples;
@@ -146,7 +155,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
status = SPEAR_ADC_STATUS_CHANNEL_NUM(chan->channel) |
SPEAR_ADC_STATUS_AVG_SAMPLE(st->avg_samples) |
@@ -159,7 +168,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
wait_for_completion(&st->completion); /* set by ISR */
*val = st->value;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return IIO_VAL_INT;
@@ -187,7 +196,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
if (mask != IIO_CHAN_INFO_SAMP_FREQ)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
if ((val < SPEAR_ADC_CLK_MIN) ||
(val > SPEAR_ADC_CLK_MAX) ||
@@ -199,7 +208,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
spear_adc_set_clk(st, val);
out:
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
}
@@ -271,6 +280,9 @@ static int spear_adc_probe(struct platform_device *pdev)
}
st = iio_priv(indio_dev);
+
+ mutex_init(&st->lock);
+
st->np = np;
/*
--
2.25.1
From: Sergiu Cuciurean <[email protected]>
As part of the general cleanup of indio_dev->mlock, this change replaces
it with a local lock on the device's state structure.
This is part of a bigger cleanup.
Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
Signed-off-by: Sergiu Cuciurean <[email protected]>
Signed-off-by: Mircea Caprioru <[email protected]>
---
drivers/iio/adc/npcm_adc.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
index d9d105920001..f7bc0bb7f112 100644
--- a/drivers/iio/adc/npcm_adc.c
+++ b/drivers/iio/adc/npcm_adc.c
@@ -25,6 +25,15 @@ struct npcm_adc {
wait_queue_head_t wq;
struct regulator *vref;
struct reset_control *reset;
+ /*
+ * Lock to protect the device state during a potential concurrent
+ * read access from userspace. Reading a raw value requires a sequence
+ * of register writes, then a wait for a event and finally a register
+ * read, during which userspace could issue another read request.
+ * This lock protects a read access from ocurring before another one
+ * has finished.
+ */
+ struct mutex lock;
};
/* ADC registers */
@@ -135,9 +144,9 @@ static int npcm_adc_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&info->lock);
ret = npcm_adc_read(info, val, chan->channel);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&info->lock);
if (ret) {
dev_err(info->dev, "NPCM ADC read failed\n");
return ret;
@@ -187,6 +196,8 @@ static int npcm_adc_probe(struct platform_device *pdev)
return -ENOMEM;
info = iio_priv(indio_dev);
+ mutex_init(&info->lock);
+
info->dev = &pdev->dev;
info->regs = devm_platform_ioremap_resource(pdev, 0);
--
2.25.1
From: Sergiu Cuciurean <[email protected]>
As part of the general cleanup of indio_dev->mlock, this change replaces
it with a local lock on the device's state structure.
This is part of a bigger cleanup.
Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
Signed-off-by: Sergiu Cuciurean <[email protected]>
Signed-off-by: Mircea Caprioru <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index 889b88768b63..14874f11614d 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -90,6 +90,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
* 3: 800 uA
* @extended_delay: enable the gpadc extended delay mode
* @auto_conversion_period: define the auto_conversion_period
+ * @lock: Lock to protect the device state during a potential concurrent
+ * read access from userspace. Reading a raw value requires a sequence
+ * of register writes, then a wait for a completion callback,
+ * and finally a register read, during which userspace could issue
+ * another read request. This lock protects a read access from
+ * ocurring before another one has finished.
*
* This is the palmas_gpadc structure to store run-time information
* and pointers for this driver instance.
@@ -110,6 +116,7 @@ struct palmas_gpadc {
bool wakeup1_enable;
bool wakeup2_enable;
int auto_conversion_period;
+ struct mutex lock;
};
/*
@@ -388,7 +395,7 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
if (adc_chan > PALMAS_ADC_CH_MAX)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&adc->lock);
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -414,12 +421,12 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
goto out;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&adc->lock);
return ret;
out:
palmas_gpadc_read_done(adc, adc_chan);
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&adc->lock);
return ret;
}
@@ -516,6 +523,9 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
adc->dev = &pdev->dev;
adc->palmas = dev_get_drvdata(pdev->dev.parent);
adc->adc_info = palmas_gpadc_info;
+
+ mutex_init(&adc->lock);
+
init_completion(&adc->conv_completion);
dev_set_drvdata(&pdev->dev, indio_dev);
--
2.25.1
On Mon, 28 Sep 2020 16:13:29 +0300
Mircea Caprioru <[email protected]> wrote:
> From: Sergiu Cuciurean <[email protected]>
>
> As part of the general cleanup of indio_dev->mlock, this change replaces
> it with a local lock on the device's state structure.
>
> This is part of a bigger cleanup.
> Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
>
> Signed-off-by: Sergiu Cuciurean <[email protected]>
> Signed-off-by: Mircea Caprioru <[email protected]>
I guess I was waiting for a v2 of the series. Seeing as it has been a while
and the first 3 patches are fine on their own, I'll pick them up now.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/spear_adc.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/spear_adc.c b/drivers/iio/adc/spear_adc.c
> index 1bc986a7009d..d93e580b3dc5 100644
> --- a/drivers/iio/adc/spear_adc.c
> +++ b/drivers/iio/adc/spear_adc.c
> @@ -75,6 +75,15 @@ struct spear_adc_state {
> struct adc_regs_spear6xx __iomem *adc_base_spear6xx;
> struct clk *clk;
> struct completion completion;
> + /*
> + * Lock to protect the device state during a potential concurrent
> + * read access from userspace. Reading a raw value requires a sequence
> + * of register writes, then a wait for a completion callback,
> + * and finally a register read, during which userspace could issue
> + * another read request. This lock protects a read access from
> + * ocurring before another one has finished.
> + */
> + struct mutex lock;
> u32 current_clk;
> u32 sampling_freq;
> u32 avg_samples;
> @@ -146,7 +155,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
>
> status = SPEAR_ADC_STATUS_CHANNEL_NUM(chan->channel) |
> SPEAR_ADC_STATUS_AVG_SAMPLE(st->avg_samples) |
> @@ -159,7 +168,7 @@ static int spear_adc_read_raw(struct iio_dev *indio_dev,
> wait_for_completion(&st->completion); /* set by ISR */
> *val = st->value;
>
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return IIO_VAL_INT;
>
> @@ -187,7 +196,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
> if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> return -EINVAL;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
>
> if ((val < SPEAR_ADC_CLK_MIN) ||
> (val > SPEAR_ADC_CLK_MAX) ||
> @@ -199,7 +208,7 @@ static int spear_adc_write_raw(struct iio_dev *indio_dev,
> spear_adc_set_clk(st, val);
>
> out:
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
> return ret;
> }
>
> @@ -271,6 +280,9 @@ static int spear_adc_probe(struct platform_device *pdev)
> }
>
> st = iio_priv(indio_dev);
> +
> + mutex_init(&st->lock);
> +
> st->np = np;
>
> /*
On Mon, 28 Sep 2020 16:13:30 +0300
Mircea Caprioru <[email protected]> wrote:
> From: Sergiu Cuciurean <[email protected]>
>
> As part of the general cleanup of indio_dev->mlock, this change replaces
> it with a local lock on the device's state structure.
>
> This is part of a bigger cleanup.
> Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
>
> Signed-off-by: Sergiu Cuciurean <[email protected]>
> Signed-off-by: Mircea Caprioru <[email protected]>
Applied,
Thanks,
Jonathan
> ---
> drivers/iio/adc/palmas_gpadc.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> index 889b88768b63..14874f11614d 100644
> --- a/drivers/iio/adc/palmas_gpadc.c
> +++ b/drivers/iio/adc/palmas_gpadc.c
> @@ -90,6 +90,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
> * 3: 800 uA
> * @extended_delay: enable the gpadc extended delay mode
> * @auto_conversion_period: define the auto_conversion_period
> + * @lock: Lock to protect the device state during a potential concurrent
> + * read access from userspace. Reading a raw value requires a sequence
> + * of register writes, then a wait for a completion callback,
> + * and finally a register read, during which userspace could issue
> + * another read request. This lock protects a read access from
> + * ocurring before another one has finished.
> *
> * This is the palmas_gpadc structure to store run-time information
> * and pointers for this driver instance.
> @@ -110,6 +116,7 @@ struct palmas_gpadc {
> bool wakeup1_enable;
> bool wakeup2_enable;
> int auto_conversion_period;
> + struct mutex lock;
> };
>
> /*
> @@ -388,7 +395,7 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
> if (adc_chan > PALMAS_ADC_CH_MAX)
> return -EINVAL;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&adc->lock);
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -414,12 +421,12 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
> goto out;
> }
>
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&adc->lock);
> return ret;
>
> out:
> palmas_gpadc_read_done(adc, adc_chan);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&adc->lock);
>
> return ret;
> }
> @@ -516,6 +523,9 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> adc->dev = &pdev->dev;
> adc->palmas = dev_get_drvdata(pdev->dev.parent);
> adc->adc_info = palmas_gpadc_info;
> +
> + mutex_init(&adc->lock);
> +
> init_completion(&adc->conv_completion);
> dev_set_drvdata(&pdev->dev, indio_dev);
>
On Mon, 28 Sep 2020 16:13:31 +0300
Mircea Caprioru <[email protected]> wrote:
> From: Sergiu Cuciurean <[email protected]>
>
> As part of the general cleanup of indio_dev->mlock, this change replaces
> it with a local lock on the device's state structure.
>
> This is part of a bigger cleanup.
> Link: https://lore.kernel.org/linux-iio/CA+U=Dsoo6YABe5ODLp+eFNPGFDjk5ZeQEceGkqjxXcVEhLWubw@mail.gmail.com/
>
> Signed-off-by: Sergiu Cuciurean <[email protected]>
> Signed-off-by: Mircea Caprioru <[email protected]>
Applied,
thanks,
Jonathan
> ---
> drivers/iio/adc/npcm_adc.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> index d9d105920001..f7bc0bb7f112 100644
> --- a/drivers/iio/adc/npcm_adc.c
> +++ b/drivers/iio/adc/npcm_adc.c
> @@ -25,6 +25,15 @@ struct npcm_adc {
> wait_queue_head_t wq;
> struct regulator *vref;
> struct reset_control *reset;
> + /*
> + * Lock to protect the device state during a potential concurrent
> + * read access from userspace. Reading a raw value requires a sequence
> + * of register writes, then a wait for a event and finally a register
> + * read, during which userspace could issue another read request.
> + * This lock protects a read access from ocurring before another one
> + * has finished.
> + */
> + struct mutex lock;
> };
>
> /* ADC registers */
> @@ -135,9 +144,9 @@ static int npcm_adc_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&info->lock);
> ret = npcm_adc_read(info, val, chan->channel);
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&info->lock);
> if (ret) {
> dev_err(info->dev, "NPCM ADC read failed\n");
> return ret;
> @@ -187,6 +196,8 @@ static int npcm_adc_probe(struct platform_device *pdev)
> return -ENOMEM;
> info = iio_priv(indio_dev);
>
> + mutex_init(&info->lock);
> +
> info->dev = &pdev->dev;
>
> info->regs = devm_platform_ioremap_resource(pdev, 0);