Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1605868 > unrolled thread
| Started by | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| First post | 2017-03-21 19:10 +0100 |
| Last post | 2017-03-21 20:00 +0100 |
| Articles | 6 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v3 0/2] Replace mlock with private lock and delete whitespaces simran singhal <singhalsimran0@gmail.com> - 2017-03-21 19:10 +0100
[PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock simran singhal <singhalsimran0@gmail.com> - 2017-03-21 19:10 +0100
Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock Jonathan Cameron <jic23@kernel.org> - 2017-03-22 21:30 +0100
Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2017-03-23 19:20 +0100
Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-03-23 19:20 +0100
Re: [Outreachy kernel] [PATCH v3 0/2] Replace mlock with private lock and delete whitespaces Alison Schofield <amsfield22@gmail.com> - 2017-03-21 20:00 +0100
| From | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-21 19:10 +0100 |
| Subject | [PATCH v3 0/2] Replace mlock with private lock and delete whitespaces |
| Message-ID | <tnwoh-6UG-5@gated-at.bofh.it> |
The patch series replaces mlock with a private lock for driver ad9834 and Fix coding style issues related to white spaces. v3: -Using new private "lock" instead of using "buf_lock" as it can cause deadlock. -Sending it as a series of two patches. v2: -Using the existing buf_lock instead of lock. simran singhal (2): staging: iio: ade7753: Remove trailing whitespaces staging: iio: ade7753: Replace mlock with driver private lock drivers/staging/iio/meter/ade7753.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) -- 2.7.4
[toc] | [next] | [standalone]
| From | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-21 19:10 +0100 |
| Subject | [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock |
| Message-ID | <tnwoj-6UG-31@gated-at.bofh.it> |
| In reply to | #1605868 |
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: simran singhal <singhalsimran0@gmail.com>
---
drivers/staging/iio/meter/ade7753.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
index b71fbd3..9674e05 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -80,11 +80,13 @@
* @us: actual spi_device
* @tx: transmit buffer
* @rx: receive buffer
+ * @lock: protect sensor state
* @buf_lock: mutex to protect tx and rx
**/
struct ade7753_state {
struct spi_device *us;
struct mutex buf_lock;
+ struct mutex lock; /* protect sensor state */
u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
u8 rx[ADE7753_MAX_RX];
};
@@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
t = 27900 / val;
if (t > 0)
@@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
out:
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret ? ret : len;
}
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2017-03-22 21:30 +0100 |
| Subject | Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock |
| Message-ID | <tnV3j-89A-9@gated-at.bofh.it> |
| In reply to | #1605871 |
On 21/03/17 18:03, simran singhal 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: simran singhal <singhalsimran0@gmail.com>
Mutex is not initialized...
> ---
> drivers/staging/iio/meter/ade7753.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
> index b71fbd3..9674e05 100644
> --- a/drivers/staging/iio/meter/ade7753.c
> +++ b/drivers/staging/iio/meter/ade7753.c
> @@ -80,11 +80,13 @@
> * @us: actual spi_device
> * @tx: transmit buffer
> * @rx: receive buffer
> + * @lock: protect sensor state
> * @buf_lock: mutex to protect tx and rx
> **/
> struct ade7753_state {
> struct spi_device *us;
> struct mutex buf_lock;
> + struct mutex lock; /* protect sensor state */
> u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
> u8 rx[ADE7753_MAX_RX];
> };
> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
> if (!val)
> return -EINVAL;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
>
> t = 27900 / val;
> if (t > 0)
> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
> ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>
> out:
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return ret ? ret : len;
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@jic23.retrosnub.co.uk> |
|---|---|
| Date | 2017-03-23 19:20 +0100 |
| Subject | Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock |
| Message-ID | <tofv4-615-3@gated-at.bofh.it> |
| In reply to | #1606987 |
On 23 March 2017 18:12:33 GMT+00:00, SIMRAN SINGHAL <singhalsimran0@gmail.com> wrote:
>On Thu, Mar 23, 2017 at 1:55 AM, Jonathan Cameron <jic23@kernel.org>
>wrote:
>> On 21/03/17 18:03, simran singhal 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: simran singhal <singhalsimran0@gmail.com>
>> Mutex is not initialized...
>
>Mutex is already initialized in ade7753_probe().
Given you introduce a new mutex it seems unlikely that one is. You have to initialise each mutex.
>
>>> ---
>>> drivers/staging/iio/meter/ade7753.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/meter/ade7753.c
>b/drivers/staging/iio/meter/ade7753.c
>>> index b71fbd3..9674e05 100644
>>> --- a/drivers/staging/iio/meter/ade7753.c
>>> +++ b/drivers/staging/iio/meter/ade7753.c
>>> @@ -80,11 +80,13 @@
>>> * @us: actual spi_device
>>> * @tx: transmit buffer
>>> * @rx: receive buffer
>>> + * @lock: protect sensor state
>>> * @buf_lock: mutex to protect tx and rx
>>> **/
>>> struct ade7753_state {
>>> struct spi_device *us;
>>> struct mutex buf_lock;
>>> + struct mutex lock; /* protect sensor state */
>>> u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
>>> u8 rx[ADE7753_MAX_RX];
>>> };
>>> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct
>device *dev,
>>> if (!val)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&indio_dev->mlock);
>>> + mutex_lock(&st->lock);
>>>
>>> t = 27900 / val;
>>> if (t > 0)
>>> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct
>device *dev,
>>> ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>>
>>> out:
>>> - mutex_unlock(&indio_dev->mlock);
>>> + mutex_unlock(&st->lock);
>>>
>>> return ret ? ret : len;
>>> }
>>>
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
[toc] | [prev] | [next] | [standalone]
| From | SIMRAN SINGHAL <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-23 19:20 +0100 |
| Subject | Re: [PATCH v3 2/2] staging: iio: ade7753: Replace mlock with driver private lock |
| Message-ID | <tofv4-615-5@gated-at.bofh.it> |
| In reply to | #1606987 |
On Thu, Mar 23, 2017 at 1:55 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 21/03/17 18:03, simran singhal 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: simran singhal <singhalsimran0@gmail.com>
> Mutex is not initialized...
Mutex is already initialized in ade7753_probe().
>> ---
>> drivers/staging/iio/meter/ade7753.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
>> index b71fbd3..9674e05 100644
>> --- a/drivers/staging/iio/meter/ade7753.c
>> +++ b/drivers/staging/iio/meter/ade7753.c
>> @@ -80,11 +80,13 @@
>> * @us: actual spi_device
>> * @tx: transmit buffer
>> * @rx: receive buffer
>> + * @lock: protect sensor state
>> * @buf_lock: mutex to protect tx and rx
>> **/
>> struct ade7753_state {
>> struct spi_device *us;
>> struct mutex buf_lock;
>> + struct mutex lock; /* protect sensor state */
>> u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
>> u8 rx[ADE7753_MAX_RX];
>> };
>> @@ -484,7 +486,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
>> if (!val)
>> return -EINVAL;
>>
>> - mutex_lock(&indio_dev->mlock);
>> + mutex_lock(&st->lock);
>>
>> t = 27900 / val;
>> if (t > 0)
>> @@ -505,7 +507,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
>> ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>>
>> out:
>> - mutex_unlock(&indio_dev->mlock);
>> + mutex_unlock(&st->lock);
>>
>> return ret ? ret : len;
>> }
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Alison Schofield <amsfield22@gmail.com> |
|---|---|
| Date | 2017-03-21 20:00 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH v3 0/2] Replace mlock with private lock and delete whitespaces |
| Message-ID | <tnxaG-7fa-11@gated-at.bofh.it> |
| In reply to | #1605868 |
On Tue, Mar 21, 2017 at 11:33:54PM +0530, simran singhal wrote: > The patch series replaces mlock with a private lock for driver ad9834 and > Fix coding style issues related to white spaces. Hi Simran, I'm getting lost. Patchset Subject Line needs subsystem and driver. The comment above says ad9834 but the patches below say ade7753. Can we drive adis16060 through ACK and then come back to this one? (ie. applyling lessons learned) thanks, alisons > > v3: > -Using new private "lock" instead of using "buf_lock" > as it can cause deadlock. > -Sending it as a series of two patches. > > v2: > -Using the existing buf_lock instead of lock. > > > simran singhal (2): > staging: iio: ade7753: Remove trailing whitespaces > staging: iio: ade7753: Replace mlock with driver private lock > > drivers/staging/iio/meter/ade7753.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1490119436-20042-1-git-send-email-singhalsimran0%40gmail.com. > For more options, visit https://groups.google.com/d/optout.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web