linux-kernel - Re: [PATCH v3 2/2] iio: light: Add APDS9160 ALS & Proximity sensor driver

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] [day] [month] [year] [list]

Message-ID: <Z3xZ9NOsoGeMD99u@uva.nl>
Date: Mon, 6 Jan 2025 17:32:20 -0500
From: Mikael Gonella-Bolduc <mgonellabolduc@...onoff.com>
To: Jonathan Cameron <jic23@...nel.org>
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>,
	Matti Vaittinen <mazziesaccount@...il.com>
Subject: Re: [PATCH v3 2/2] iio: light: Add APDS9160 ALS & Proximity sensor
 driver

On Fri, Dec 20, 2024 at 07:06:08PM +0000, Jonathan Cameron wrote:
> On Thu, 19 Dec 2024 14:22:28 -0500
> Mikael Gonella-Bolduc <mgonellabolduc@...onoff.com> wrote:
> 
> > Hi Jonathan,
> > 
> > Regarding calibration attributes, while I did include them and made sure that what's exposed by the driver 
> > is actually being correctly set in the registers and that it has an impact on the input data, 
> > I do not have a real life use case for them right now and that's probably the case for almost everyone using the same chip.
> > It's there as a provision in case something comes up and I end up needing them.
> > 
> > I'm also not sure on how it should be used and in which scenario.
> > From what I understand, depending on the type of material in front of the sensor (tempered glass in my case),
> > it's there to cancel out unwanted light reflection of what you don't want to detect.
> > It does so by emitting another, very short, light pulse and takes the reflected light ADC count from that and substracts it.
> 
> I guess they apply a very short exposure time on that measurement to ensure it only picks up on light
> received very soon after the pulse and hence in theory very near to the sensor.
> 
> > There's also another digital substraction parameter to always substract a value if you know what's the value to cancel out.
> > 
> > The 3 parameters in question in the datasheet:
> > - PS_CAN_DIG : This is just a digital substraction
> that one is fine as in_proximity_calibbias.
> 
> > - PS_CAN_ANA_DURATION: The duration of the short cancellation light pulse
> > - PS_CAN_ANA_CURRENT: The light pulse current used
> 
> These two are new concepts so I think we may need some new ABI
> 
> It is kind of like a differential channel with separate controls
> on the current and integration time (as a proxy for the pulse length).
> 
> For the current I think it is cleaner to use a second
> out_currentX_* channel with labels provided to associate which current
> channel is which. Provide the read_label attribute and label all channels.
> 
> Taking the duration, that's a bit of an oddity as it's really a characteristic
> of the the output current channel, not the measurement itself. So
> how about out_current1_pulse_time ?  Would need to be in seconds though
> to match with integration_time ABI.
> 
> Having said all this. If there is a right setting (or a calibration procedure
> to get something that works) for a given device incorporating this sensor
> then the stuff perhaps belongs in DT.  If you go that way makes sure to cleanly
> document why these are device characteristics rather than corrections for
> calibration differences between different phones of the same model for instance.
> 
> 
> 
> 
> > 
> > I used a standard calibration attribute name for all of those, respectively:
> > - in_proximity_calibscale
> > - in_proxmiity_calibbias
> > - out_current_calibbias
> > 
> > I don't know if this is a correct use or not.
> 
> Thanks for all the details.  That helps a lot!
> 
> Gut feeling from me is that we are looking at something best defined in firmware
> / DT given it's all about the glass in front of the sensor.

Hi Jonathan,

Indeed, it makes sense to me to define all these in the DT.
Please see v4 for update.

Best regards & happy new year,
Mikael