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

[jic23]

Daniel Baluta writes: 

> On Sun, Jun 7, 2015 at 12:08 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > On 06/03/2015 09:56 AM, Daniel Baluta wrote:
> > > On Thu, May 28, 2015 at 5:17 PM, Daniel Baluta <daniel.baluta@xxxxxxxxx>
> > > 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/