[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9d810e5c-c7a5-41e5-8073-b703717faf3d@gmail.com>
Date: Mon, 2 Dec 2024 10:22:19 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jonathan Cameron <jic23@...nel.org>,
Mikael Gonella-Bolduc <mgonellabolduc@...onoff.com>
Cc: Mikael Gonella-Bolduc via B4 Relay
<devnull+mgonellabolduc.dimonoff.com@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
<morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
Mikael Gonella-Bolduc <m.gonella.bolduc@...il.com>,
linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
Hugo Villeneuve <hvilleneuve@...onoff.com>
Subject: Re: [PATCH 2/2] iio: light: Add APDS9160 ALS & Proximity sensor
driver
Hi Jonathan & Mikael,
On 01/12/2024 15:20, Jonathan Cameron wrote:
>
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, apds9160_of_match);
>>>> +
>>>> +static struct i2c_driver apds9160_driver = {
>>>> + .driver = {
>>>> + .name = APDS9160_DRIVER_NAME,
>>>> + .owner = THIS_MODULE,
>>>> + .of_match_table = apds9160_of_match,
>>>> + },
>>>> + .probe = apds9160_probe,
>>>> + .remove = apds9160_remove,
>>>> + .id_table = apds9160_id,
>>>> +};
>> First, regarding the integration time/gain/scale parameters. I took a look at the datasheet again as there is a table
>> provided to get lux/count (scale?) for the ALS sensor depending on gain and integration time.
>>
>> It looks like the correlation in the table is almost linear but it's not as there is a loss of precision.
>> For example, at 1x gain with integration time 100ms the lux/count is 0.819 but at 3x the table is stating 0.269 instead of exepected 0.273.
>>
>> Is it still possible to use the gts helpers in that case?
>
> Ah. Probably not if it goes non linear. Matti? (+CC)
Disclaimer - I didn't go through the patch and I just respond from the
top of my head :) So, please take my words with a pinch of salt.
AFAIR, it is not required that the impact of integration time is
_linear_ through the range. The "multiplication factor" can be set for
each integration time separately. So, it is perfectly Ok to say:
time 1 => multiply by 1
time 2 => multiply by 2
time 10 => multiply by 9 <= not linear, as linear would be 10.
time 15 => multiply by 15
...
The notable limitation of _current_ implementation is that the
"multiplication factor" needs to be integer. So, this may result loss of
accuracy.
>> Second, regarding the use of the IIO_CHAN_INFO_HARDWAREGAIN channel info.
>> I took a look at a couple of recent drivers, some use the IIO_CHAN_INFO_SCALE to ajust gain type registers.
>>
>> In my use case, it feels like the scale is read-only as it is affected by the gain and integration time and both can be set independently
>> with their respective available values. How should I handle this?
> The general preference is for the scale to be the primary control.
> For a light sensor assuming the device doesn't support very long integration times, the
> trade off is normally set the integration time as high as possible (as that gives lowest
> noise) then tune the gain as necessary.
>
> Another model is to let the integration time be controllable and then try and adjust
> the gain to keep as close as possible to a requested scale. Matti has spent more
> brain power on this than anyone so I'll over to him for more precise suggestions!
This is the very reason why I wrote the gts helpers. The recently
removed rohm-bu27008.c light-sensor driver and the rohm-bu27034.c allow
setting integration time. When time is set the driver will also set the
gain, and does this so that the scale is maintained. (Eg, if gain caused
by time is doubled by the user request, the hw-gain is halved by the
driver).
For scale setting I used logic where the scale setting tries to maintain
the integration time when possible, and only set the hardware gain.
Ideology is that when the driver allows user to set the time, the user
is expected to know whether to prefer longer time (and typically better
accuracy), or faster measurement with more hw-gain.
There are still some corner cases in these drivers where the scale can't
be maintained when time is set, or time can't be maintained when scale
is set. AFAIR, These drivers chose to in any case set the requested
time/scale - which means the userland needs to be prepared to "pick up
the pieces" and deal with the other entity changing. Other option would
be to deny such changes, but it would limit the use of the hardware -
and I hate drivers trying to outsmart the users.
Where this gets really hairy is when there are multiple channels and
some of the controls can be set per channel, and some are common for all
channels. As an example, I've faced sensors where gain can be set
separately for the channels but the time is common for all. Here we may
end up in situations where part of the channels can compensate the
changes while others can't. Furthermore, there were situations where
some of the scales could only be achieved with a specific integration
times. This can get very confusing for users.
In order to make it somehow tolerable the gts helpers support also
advertising only scales which can be supported with currently selected
integration time via the "available_scales". Also, I strongly recommend
having at least a read-only hardwaregain to help understanding which
part of the scale is contributed by time, and which by hardwaregain. I
think this is very helpful when debugging ;)
I know Jonathan (and maybe rest of the world :) ) prefers keeping the
scale as the single main way of changing the gain. Still, my personal
opinion on this is that there are cases, where having both the
hardwaregain and integration time directly changeable would be the
simplest and clearest solution.
Putting my opinions aside (as this is not a battle I feel like fighting
- nor do I feel I am the best expert on this!), with the current 'one
scale to rule them all' approach, my recommendation is to consider if
always using largest integration time and smallest hardwaregain to
achieve the requested scale fits your users. (Also, I think your case
where the impact of integration time is not strictly linear will cause
some scales to be 'almost same' but not quite. I am not sure if it makes
sense to support all of these, or just always round to the scale where
the integration time is longest). If yes, then maybe do it.
If you think the longest times may cause unacceptable wait times for
some users, or that the small scale differences really matter, then you
may want to go the route bu27008 / bu27034 drivers took - but it may get
hairy.
Finally, maybe taking a look if the integration time multiplier in gts
helpers could support fixed point wouldn't be completely futile(?) No
promises on that route though, I haven't considered this when writing
those so it may lead to dead end - but one never knows before taking a
look, right? :)
Ha. Looking all the letters I typed, I feel I helped really little while
being very verbose ... Sorry bout that but I really don't have a great
recommendation :(
Yours,
-- Matti
Powered by blists - more mailing lists