linux-kernel - Re: [PATCH 2/3] iio: light: add support for veml6046x00 RGBIR color sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250406120825.41b2575c@jic23-huawei>
Date: Sun, 6 Apr 2025 12:08:25 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: 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,
 mazziesaccount@...il.com, subhajit.ghosh@...aklogic.com,
 muditsharma.info@...il.com, arthur.becker@...tec.com,
 ivan.orlov0322@...il.com, Matti Vaittinen <mazziesaccount@...il.com>
Subject: Re: [PATCH 2/3] iio: light: add support for veml6046x00 RGBIR color
 sensor

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.

> 
> 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.


> 
> 
> 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.

Jonathan

> 
> 
> What do you think?
> 
> Andreas
>