Re: [PATCH v6 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously

[Sean Nyekjaer <sean@geanix.com>]

On 09/09/2019 13.55, Lorenzo Bianconi wrote:
>> When events and buffered reads is enabled simultaneously, and the first
>> event accours the interrupt pin stays high.
>>
>> This can be reverted when we find a solution to allow events and
>> buffered reads simultaneously.
>>
>> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
>> ---
>> Changes since v4:
>>   * Use fifo configuration mutex to prevent a race in hw->enable_event check.
>>
>> Changes since v5:
>>   * Updated do not return without unlocking mutexes
>>   * Lock mutex before unlock
>>   * Runtime tested ;-)
>>
> 
> [...]
> 
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -1340,8 +1340,12 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>>   	if (type != IIO_EV_TYPE_THRESH)
>>   		return -EINVAL;
>>   
>> -	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
>> -		return -EBUSY;
>> +	mutex_lock(&hw->conf_lock);
>> +
>> +	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
>> +		err = -EBUSY;
>> +		goto out;
>> +	}
> 
> This patch is still broken!!! Returning in case of error you need to relase the
> lock.
> 

Hmm,

please read below this:

-	return 0;
+out:
+	mutex_unlock(&hw->conf_lock);
+
+	return err;


>>   
>>   	/* do not enable events if they are already enabled */
>>   	if (state && hw->enable_event)
>> @@ -1357,7 +1361,10 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>>   
>>   	hw->enable_event = state;
>>   
>> -	return 0;
>> +out:
>> +	mutex_unlock(&hw->conf_lock);
>> +
>> +	return err;
>>   }
>>   
>>   int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
>> -- 
>> 2.23.0
>>