Message-ID: <ecb2844c-feb5-47d4-b4db-12171380a9cb@gmail.com>
Date: Mon, 7 Apr 2025 08:52:12 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jonathan Cameron <jic23@...nel.org>, Andreas Klinger <ak@...klinger.de>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 lars@...afoo.de, linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, javier.carrasco.cruz@...il.com,
 subhajit.ghosh@...aklogic.com, muditsharma.info@...il.com,
 arthur.becker@...tec.com, ivan.orlov0322@...il.com
Subject: Re: [PATCH 2/3] iio: light: add support for veml6046x00 RGBIR color
 sensor

On 06/04/2025 14:08, Jonathan Cameron wrote:
> On Sun, 6 Apr 2025 10:43:23 +0200
> Andreas Klinger <ak@...klinger.de> wrote:
> 
>> Hi Jonathan,
>>
>> I need to pick up the meaning of scale once again for clarification.
>>
>> Jonathan Cameron <jic23@...nel.org> schrieb am Mo, 17. Mär 11:50:
>>> On Sun, 16 Mar 2025 12:31:30 +0100
>>> Andreas Klinger <ak@...klinger.de> wrote:
>>>    
>>>> +static int veml6046x00_get_scale(struct veml6046x00_data *data,
>>>> +				 int *val, int *val2)
>>>
>>> How is this related to integration time?  I'd normally expect
>>> to see that read in here somewhere as well as doubling integration
>>> time tends to double scale.
>>
>> In the documentation file "sysfs-bus-iio" it says:
>> "
>> What:           /sys/.../iio:deviceX/in_illuminanceY_raw
>> [...]
>> Description:
>>                  Illuminance measurement, units after application of scale
>>                                  and offset are lux.
>> "
>>
>> This means that the scale should be the real factor and not the gain multiplied
>> by photodiode size (PDDIV) as i implemented it so far.
>>
>> This means also that doubling integration time should halve the scale. The
>> higher raw value should lead to the same lux value.
> 
> Sounds correct.

I was CC'd due to the GTS (gain-time-scale)-helpers. The above is the 
beef of those helpers - which, attempt to aid drivers to convert the 
impact of the hardware gain + integration time into a single scale 
value. This approach has some caveats, but the goal is to fulfill the 
expectations of those user-space apps which expect only scale to change 
the gain, while also need to have the integration time controllable (for 
example to reduce the measurement time for one reason or another).

Problem is that, especially when there are multiple channels with 
separate gain control but common integration time, there will be some 
situations where the integration time change _will_ cause changes to 
"total gain (E.g. scale)" too. There may also be cases where some scale 
values can be met only with certain integration times, or where a scale 
for a channel can't be met maintaining the scale for other channels etc.

All in all, I am not sure if the 'unchangeable hardware gain' approach 
makes things as simple as possible - but as long as we want to have it, 
the GTS helpers may be of use :) There are couple of drivers using them 
- feel free to take a look. "git grep gts_ drivers/iio/light/" should 
point you the current users.

>> The documentation of the sensor (veml6046x00.pdf) provides us the calculation
>> between raw green values and lux.
>> Wouldn't it be better to give the user the real factor to be able to get lux?
> 
> Absolutely.  That's the expectation if we are providing illuminance_raw and
> illuminance_scale.
> 
>>
>> The fact that only the green channel can be used for calculation could be
>> documented in the driver.
> 
> Ah. One of these devices.  Hmm. Why do people pretend they can get from
> Green to illuminance.  That has to assume 'white light'.
> I get grumpy about this, but if it is the best we can do I guess we have
> to live with it (I might not be consistent on this).
> 
>>
>> Then i found the "in_illuminance_hardwaregain" property. It seems that this is
>> exactly what the combination of gain and PDDIV is used for.
>>
>> So what is the scale at the moment could become the hardwaregain and the scale
>> the factor from raw value to lux.
> 
> If it is useful to export it separately that works, however it's not typically
> the control attribute - those tend to be read only because, without access to
> datasheets simple software has no idea how to control them.
> 
> The alternative is the GTS helpers that attempt to figure out the best
> way to meet the user requirements in setting the integration time and amplifier
> gain when a scale is requested.
> 
> +CC Matti who is the expert on those.

Seems it doesn't take much to be an expert XD

I have understood that the simplest way to go is to always use the 
maximum integration time which provides the required scale. This should 
probably result the most accurate readings. If users for some reason 
want to explicitly set a shorter time, then the GTS helpers might be useful.

>> To sum up the suggested interface under /sys/bus/iio/devices/iio\:deviceX would
>> be something like:
>>
>> in_illuminance_hardwaregain --> set and get gain and PDDIV on the sensor
> 
> This is usually the read only one as it reflects things that aren't
> easy for a userspace program / user to tune.  They typically want to control
> integration time because it reflects noise level and scale because they want
> to avoid saturation etc and because we need it to get to the actual value
> in lux.
> 
>>
>> integration_time            --> set and get integration time on the sensor
> driving these directly is fine.
>>
>> integration_time_available  --> show available integration time values
>>
>> scale                       --> (only) get real calculation value, taken from
>>                                  sensor documenation, e.g. 1.3440
> This should remain a main control attribute.
>>
>> scale_available             --> not existing anymore
> This gets tricky but the GTS helpers will calculate it for you I think.

The GTS helpers can (or, at least should be able to) calculate the 
available_scales. AFAIR they offer two approaches for this. First one is 
listing _all_ scales which can be achieved by changing both the 
integration time and the hardware gain. The second one lists only the 
scales which can be set with the currently selected integration time.

I think that all of the current GTS users use this first approach of 
listing all the values, so the 'per time tables' - approach is not very 
thoroughly tested. Please, let me know if you hit to any hiccups.

Yours,
	-- Matti

Powered by blists - more mailing lists