Re: [PATCH] iio: add bmp280 pressure and temperature driver

[Jonathan Cameron <jic23@kernel.org>]

On 03/10/14 15:44, Peter Meerwald wrote:
> 
>>>> +static int bmp280_read_raw(struct iio_dev *indio_dev,
>>>> +			   struct iio_chan_spec const *chan,
>>>> +			   int *val, int *val2, long mask)
>>>> +{
>>>> +	int ret;
>>>> +	struct bmp280_data *data = iio_priv(indio_dev);
>>>> +
>>>> +	mutex_lock(&data->lock);
>>>
>>> locking is only needed for _RAW
>>>
>>>> +
>>>> +	switch (mask) {
>>>> +	case IIO_CHAN_INFO_RAW:
>>>
>>> I think this should be _PROCESSED, not _RAW;
>>> there is calibration and all kind of scaling done internally
>>
>> Just to make sure here:  If I use _PROCESSED, I need to apply scale in
>> kernel space and remove _SCALE, correct?
> 
> yes
> 
> since pressure is using a temperature read and calibration data, it hardly 
> qualifies as 'raw'
I'd typically be happy with this if there was a significant advantage in
leaving the application of the final scale to userspace.  This would either
be in terms of time, accuracy or simplicity (as floating point math would
be available in userspace).  Hence the channel doesn't have to be completely
'raw'.  Having said that, looks like here it makes sense to just do the *10 in
kernel space ;)
> 
>> I will address the rest of your observations in v2, once I get a chance
>> to test using the device on Monday.
>>
>> Thank you for the review.
> 
> p.
>