On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <[email protected]> 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?
Daniel.
On 06/03/2015 09:56 AM, Daniel Baluta wrote:
> On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <[email protected]> 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.
>
On Sun, Jun 7, 2015 at 12:08 AM, Jonathan Cameron <[email protected]> wrote:
>
>
> On 06/03/2015 09:56 AM, Daniel Baluta wrote:
>>
>> On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <[email protected]>
>> 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.
+ Matt.
Ok, now I see where the ambiguity comes from. The ABI for proximity also
covers AS3935 Franklin Lightning Sensor IC, where sensor's output provides
an estimation on the distance to the head of a storm :).
But, I don't think this is the type of proximity most of people think of :).
I am not sure how to modify the ABI without breaking the AS3935, but I will
think of a solution and send a RFC as soon as possible.
thanks,
Daniel.
Daniel Baluta writes:
> On Sun, Jun 7, 2015 at 12:08 AM, Jonathan Cameron <[email protected]> wrote:
>>
>>
>> On 06/03/2015 09:56 AM, Daniel Baluta wrote:
>>>
>>> On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <[email protected]>
>>> 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.
>
> + Matt.
>
> Ok, now I see where the ambiguity comes from. The ABI for proximity also
> covers AS3935 Franklin Lightning Sensor IC, where sensor's output provides
> an estimation on the distance to the head of a storm :).
>
> But, I don't think this is the type of proximity most of people think of :).
>
> I am not sure how to modify the ABI without breaking the AS3935, but I will
> think of a solution and send a RFC as soon as possible.
>
> thanks,
> Daniel.
Hmm. We could define new ABI and deprecate the old one I suppose. Or
take the view that ABI breakage fixes are aloud as long as we have both
options in the tree currently and hence are defining one of them to be
wrong.