Re: [PATCH v2] iio: light: Add support for ROHM RPR0521 sensor

[Jonathan Cameron <jic23@kernel.org>]

On 06/03/2015 09:56 AM, Daniel Baluta wrote:
> On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <daniel.baluta@intel.com> wrote:
>> <snip>
>>
>>>>>> +static const struct iio_chan_spec rpr0521_channels[] = {
>>>>>> +     {
>>>>>> +             .type = IIO_INTENSITY,
>>>>>> +             .modified = 1,
>>>>>> +             .address = RPR0521_CHAN_ALS_DATA0,
>>>>>> +             .channel2 = IIO_MOD_LIGHT_BOTH,
>>>>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>>>>> +                     BIT(IIO_CHAN_INFO_CALIBSCALE),
>>>>>
>>>>> why CALIBSCALE and not SCALE?
>>>>
>>>> Because this is used to set/get gain, which is used by the hardware
>>>> to do proper scaling.
>>>>
>>>> AFAIK this should be calibscale.
>>>
>>> in sysfs-bus-iiof on CALIBSCALE: Hardware applied calibration scale factor
>>> (assumed to fix production inaccuracies).
>>>
>>> this doesn't seem applicable here, it is a gain factor controlling
>>> measurement resolution
>>
>> Ok, I see now and it makes sense :).
>>
>> # echo 1 > in_intensity_ir_calibscale
>> # cat in_intensity_ir_raw
>> 79
>> # echo 64 > in_intensity_ir_calibscale
>> # cat in_intensity_ir_raw
>> 5084
>>
>> The user should get the same value regardless of the gain :), and in the
>> above example for x64 gain it should have a 1/64 scale.
>>
>>
>> <snip>
>>
>>>> Or we can consider that the chan->type is always valid?
>>>
>>> I'd think so; you also assume that chan->address is valid
>>>
>>> I suggest to use chan->address to point to a table containing the
>>> address and the mask
>>
>> <snip>
>>
>>>> Which sensors? It means they do not agree with the ABI:
>>>>
>>>> http://lxr.free-electrons.com/source/Documentation/ABI/testing/sysfs-bus-iio#L1131
>>>
>>> that 'clarification' was added recently,
>>> 614e8842ddf5502f0e781f91695bfbc1e1e1d9b6 (with 3.18)
>>> "Proximity measurement .. by observing reflectivity"
>>>
>>> high proximity <-> high reflectivity -- this is the reality of what most
>>> sensors output (including yours)
>>>
>>> proximity and distance are opposite concepts;
>>> high proximity <-> low distance, and vice versa
>>>
>>> the distance part doesn't make sense in the ABI description
>>
>> At least sx9500 uses this convention and userspace applications rely on this.
>
> OK, so wee need to agree on this part and to add a proper descriptor to the ABI.
>
> Jonathan, what do you say?
>
I agree that we need to agree one way or the other.  Proximity being 
higher as you get closer seems slightly more logical to me
(I wish now that I'd argued in favour of just doing distance, but such
is hindsight).  Still I'm happy with whatever consensus forms.

>