Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420002

Re: [PATCH] iio: light: tcs3472: use iio helper function to guarantee direct mode

Show all headers | View raw

On 07/06/16 06:23, Peter Meerwald-Stadler wrote:
> On Mon, 6 Jun 2016, Alison Schofield wrote:
> 
>> Replace the code that guarantees the device stays in direct mode
>> with iio_device_claim_direct_mode() which does same.  This allows
>> removal of an unused lock in the device private global data.
> 
> Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
>  
>> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
>> Cc: Daniel Baluta <daniel.baluta@gmail.com>
There is an ever so slightly difference in how these all work after
the change.  They will return -EBUSY rather than holding then returning
a valid value under the circumstances of two reads coming through
sysfs at the same time.  This is a pretty obscure case so
I think we are OK with this.

Actually pending responses to this, I'm going to back out the other
two similar patches.  Just goes to show, that if you show someone
something 3 times they might finally notice the issue!


>> ---
>>  drivers/iio/light/tcs3472.c | 13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
>> index 1b530bf..b29312f 100644
>> --- a/drivers/iio/light/tcs3472.c
>> +++ b/drivers/iio/light/tcs3472.c
>> @@ -52,7 +52,6 @@
>>  
>>  struct tcs3472_data {
>>  	struct i2c_client *client;
>> -	struct mutex lock;
>>  	u8 enable;
>>  	u8 control;
>>  	u8 atime;
>> @@ -117,17 +116,16 @@ static int tcs3472_read_raw(struct iio_dev *indio_dev,
>>  
>>  	switch (mask) {
>>  	case IIO_CHAN_INFO_RAW:
>> -		if (iio_buffer_enabled(indio_dev))
>> -			return -EBUSY;
>> -
>> -		mutex_lock(&data->lock);
>> +		ret = iio_device_claim_direct_mode(indio_dev);
>> +		if (ret)
>> +			return ret;
>>  		ret = tcs3472_req_data(data);
>>  		if (ret < 0) {
>> -			mutex_unlock(&data->lock);
>> +			iio_device_release_direct_mode(indio_dev);
>>  			return ret;
>>  		}
>>  		ret = i2c_smbus_read_word_data(data->client, chan->address);
>> -		mutex_unlock(&data->lock);
>> +		iio_device_release_direct_mode(indio_dev);
>>  		if (ret < 0)
>>  			return ret;
>>  		*val = ret;
>> @@ -263,7 +261,6 @@ static int tcs3472_probe(struct i2c_client *client,
>>  	data = iio_priv(indio_dev);
>>  	i2c_set_clientdata(client, indio_dev);
>>  	data->client = client;
>> -	mutex_init(&data->lock);
>>  
>>  	indio_dev->dev.parent = &client->dev;
>>  	indio_dev->info = &tcs3472_info;
>>
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] iio: light: tcs3472: use iio helper function to guarantee  direct mode Alison Schofield <amsfield22@gmail.com> - 2016-06-07 07:20 +0200
  Re: [PATCH] iio: light: tcs3472: use iio helper function to guarantee  direct mode Peter Meerwald-Stadler <pmeerw@pmeerw.net> - 2016-06-07 07:30 +0200
    Re: [PATCH] iio: light: tcs3472: use iio helper function to guarantee  direct mode Jonathan Cameron <jic23@kernel.org> - 2016-06-11 18:10 +0200
      Re: [PATCH] iio: light: tcs3472: use iio helper function to guarantee  direct mode Jonathan Cameron <jic23@kernel.org> - 2016-06-11 18:20 +0200

csiph-web