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
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 <[email protected]>
---
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
This patch removes trailing whitespaces in order to follow the Linux
coding style.
Signed-off-by: simran singhal <[email protected]>
---
drivers/staging/iio/meter/ade7753.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
index dfd8b71..b71fbd3 100644
--- a/drivers/staging/iio/meter/ade7753.c
+++ b/drivers/staging/iio/meter/ade7753.c
@@ -83,10 +83,10 @@
* @buf_lock: mutex to protect tx and rx
**/
struct ade7753_state {
- struct spi_device *us;
- struct mutex buf_lock;
- u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
- u8 rx[ADE7753_MAX_RX];
+ struct spi_device *us;
+ struct mutex buf_lock;
+ u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
+ u8 rx[ADE7753_MAX_RX];
};
static int ade7753_spi_write_reg_8(struct device *dev,
--
2.7.4
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 [email protected].
> To post to this group, send email to [email protected].
> 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.
On 21/03/17 18:03, simran singhal wrote:
> This patch removes trailing whitespaces in order to follow the Linux
> coding style.
>
> Signed-off-by: simran singhal <[email protected]>
Applied and pushed out as testing...
Jonathan
> ---
> drivers/staging/iio/meter/ade7753.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7753.c b/drivers/staging/iio/meter/ade7753.c
> index dfd8b71..b71fbd3 100644
> --- a/drivers/staging/iio/meter/ade7753.c
> +++ b/drivers/staging/iio/meter/ade7753.c
> @@ -83,10 +83,10 @@
> * @buf_lock: mutex to protect tx and rx
> **/
> struct ade7753_state {
> - struct spi_device *us;
> - struct mutex buf_lock;
> - u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
> - u8 rx[ADE7753_MAX_RX];
> + struct spi_device *us;
> + struct mutex buf_lock;
> + u8 tx[ADE7753_MAX_TX] ____cacheline_aligned;
> + u8 rx[ADE7753_MAX_RX];
> };
>
> static int ade7753_spi_write_reg_8(struct device *dev,
>
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 <[email protected]>
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;
> }
>
On Thu, Mar 23, 2017 at 1:55 AM, Jonathan Cameron <[email protected]> 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 <[email protected]>
> 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;
>> }
>>
>
On 23 March 2017 18:12:33 GMT+00:00, SIMRAN SINGHAL <[email protected]> wrote:
>On Thu, Mar 23, 2017 at 1:55 AM, Jonathan Cameron <[email protected]>
>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 <[email protected]>
>> 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.