Re: ROHM ALS, integration time

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 30 Jan 2023 13:42:27 +0000
"Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:

> On 1/30/23 15:02, Jonathan Cameron wrote:
> > On Mon, 30 Jan 2023 14:04:53 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> 
> Hi Jonathan! Thanks for a _very fast_ response! It's nice "chatting" 
> with you again!
> 
> >>
> >> I hope this is all Ok from interface POV.  
> > 
> > This matches what I'd expect to see.  
> 
> Glad to hear :)
> 
> >> Now, finally, my dear persistent readers - the question:
> >> As mentioned, sensor allows setting the sampling time. I thought I'll
> >> map this to the IIO_CHAN_INFO_INT_TIME. This config is not per/channel
> >> in the hardware. Again, my lux-computing algorithm takes the integration
> >> time into account - and changing the time should not be reflected to the
> >> IIO_LIGHT channel values (other than accuracy). However, the values
> >> spilled from raw IIO_INTENSITY channels will change when integration
> >> time is changed. So, should I use the info_mask_shared_by_type =
> >> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?  
> > 
> > Ah. This problem. The mixture of two things that effectively map to scaling
> > of raw channels. As _scale must be applied by userspace to the _raw channel
> > that has to reflect both the result of integration time and a front end amplifier  
> 
> Ouch. I know that this makes perfectly sense. It indeed may not be clear 
> how the integration time impacts the scale. Thus, it is very reasonable 
> that the driver code should know this and not leave the burden to the 
> applications :/ Ouch because now I need to try inventing this logic in 
> driver myself ;)
> 
> > and is the control typical userspace expects to use to vary the sensitivity.  
> 
> Yes. Now that you wrote this it seems obvious.
> 
> > That makes it messy because it's not always totally obvious whether, when
> > trying to increase sensitivity, it is better to increase sample time or gain.
> > Usually you do sample time first as that tends to reduce noise and for light
> > sensors we rarely need particular quick answers.
> > 
> > So in the interests of keeping things easy to understand for userspace code
> > you would need to provide writeable _scale that then attempts to find the
> > best combination of amplifier gain and sampling time.   You can allow read
> > only access to allow a curious user to find out what was chosen:
> > INT_TIME for the integration time.  
> 
> Right. This still makes sense.
> 
> > Not really a good option for the amplifier gain though.  I don't like using
> > HARDWARE_GAIN for this though maybe we could stretch the ABI to cover this
> > as long as it was read only.  
> 
> Well, I don't (yet) have the need for this but it is good to keep on 
> mind :) It shouldn't be a big thing to add reading of (hardware)-gain.
> 
> >> Sorry for the long post. I do appreciate all help/pointers on my journey
> >> to writing my first light sensor drivers ;) And yes, my plan is to send
> >> out the patches - when I first get the sensor hardware at my hands ;)  
> > No problem. Light sensors tend to cause us more ABI headaches than almost
> > anything else  
> 
> Hm. This is the reason why I wanted to ask this right away. I realized 
> that we have a kernel<->user ABI once the sysfs starts spilling the 
> values from the driver. It wouldn't be such much fun 'fixing' this later on.
> 
>   (don't get me started on the ones used for blood oxygen level
> > measurement in which it's a light sensor and a controllable light source).  
> 
> Hmm... I think a colleague of mine did actually one such driver (AFAIR a 
> BH<add some numbers here>) not so many years ago ;) Not sure if it went 
> upstream though.
> 
> For an occasional contributor like me it could be helpful if the defines 
> like IIO_INTENSITY, IIO_LIGHT had documentation in headers explaining 
> for example the units. Maybe also some words about the 
> IIO_CHAN_INFO_INT_TIME and IIO_CHAN_INFO_SCALE as well ;) I guess I can 
> cook some doc - but only for couple of defines which I have discussed 
> with you this far. Do you think such comment docs would be welcome - 
> even if they covered only couple of defines? Maybe others would continue 
> from that.

I'd worry about the Docs disagreeing with the ABI docs
in Documentation/ABI/testing/sysfs-bus-iio
which needs to be the 'one true source' of this stuff.

So might be fine but would need careful consideration of that risk.

Jonathan
> 
> Yours,
> 	-- Matti
>