On Wed, Dec 11, 2024 at 08:53:02PM +0000, Jonathan Cameron wrote: > On Mon, 9 Dec 2024 16:49:12 -0500 > Mikael Gonella-Bolduc <mgonellabolduc@xxxxxxxxxxxx> wrote: > > > On Sun, Dec 08, 2024 at 12:20:38PM +0000, Jonathan Cameron wrote: > > > On Fri, 06 Dec 2024 11:09:57 -0500 > > > Mikael Gonella-Bolduc via B4 Relay <devnull+mgonellabolduc.dimonoff.com@xxxxxxxxxx> wrote: > > > > > > > From: Mikael Gonella-Bolduc <mgonellabolduc@xxxxxxxxxxxx> > > > > > > > > APDS9160 is a combination of ALS and proximity sensors. > > > > > > > > This patch add supports for: > > > > - Intensity clear data and illuminance data > > > > - Proximity data > > > > - Gain control, rate control > > > > - Event thresholds > > > > > > > > Signed-off-by: Mikael Gonella-Bolduc <mgonellabolduc@xxxxxxxxxxxx> > > > > > > Hi Mikael, > > > > > > As the bots noted, the maintainers entry has the wrong vendor prefix, > > > or the binding does. > > > > > > Also the issue with missing include of linux/bitfield.h > > > > > > Unused gain table is less obvious. Not sure what intent was on that one. > > > > > > Given the discussion with Matti about how to do the gain control, please add > > > some description here of the outcome. The control scheme is not particularly > > > obvious and is the key bit we should be reviewing. It feels like you've > > > applied the feedback on v1 to the light channel but it is equally applicable > > > to proximity channels when they are just measures of reflected light intensity. > > > > > > Various other minor things inline. > > > > > > Thanks, > > > > > > > > > Jonathan > > > > Hi Jonathan, > > > > I will fix the warnings the bots noted and other inline comments for v3, sorry about that. > > Regarding gain control for ALS, I kept the non-linear table provided in the datasheet. > > The user can adjust the integration time and the available scales will update > > depending on the value. > > For example, at 100ms, you have possible scales of 0.819, 0.269, 0.131, etc. (lux/count). > > The hardware gain and other relevant registers gets adjusted by the driver depending on selected scale. > > The attribute is kept as read-only as Matti suggested. > > > > Now, for proximity, again I'm confused. Please bear with me a little. > > The only "scale" I see in the datasheet is that the proximity sensor is for a short distance of under 70mm. > > That sounds like a design point for sensitivity of sensor vs light source brightness. > > > There's nothing provided in the datasheet to convert the proximity ADC count to a distance or to anything meaningful like standard units. > > I don't feel like this is really precise and the intended use case is probably like mine where you can use this to detect > > if there's something covering the sensor or not. > > > > I took a look at other light/proximity sensors, again, it's not clear for me how to handle this. > > It seems that some drivers just directly control the hardware gain register with the scale even if it's not really a scale. > Typical case is that it is a scale, just not of distance. But rather controls an amplifier on the light sensor, > so same as for the ambient light sensor. > > The ABI docs are a little vague on this Documentation/ABI/testing/sysfs-bus-iio > has > What: /sys/.../iio:deviceX/in_proximity_raw > What: /sys/.../iio:deviceX/in_proximity_input > What: /sys/.../iio:deviceX/in_proximityY_raw > KernelVersion: 3.4 > Contact: linux-iio@xxxxxxxxxxxxxxx > Description: > Proximity measurement indicating that some > object is near the sensor, usually by observing > reflectivity of infrared or ultrasound emitted. > > Often these sensors are unit less and as such conversion > to SI units is not possible. Higher proximity measurements > indicate closer objects, and vice versa. Units after > application of scale and offset are meters. > > So it kind of says we can't relate them to real units, but then we provide > a unit. Hmm, not our finest and clearest documentation. > > Probably best bet is to follow precedence as even if we haven't tightly defined > it that is what any userspace tuning these value will be using. > > Given inverse square law and different characteristics of reflective surfaces > I think it is normally a case of crank the gain up until the signal is good. > > In most cases these proximity sensors aren't much more than fancy switches > though can be used for approaching vs moving away detection. > > Anyhow, I haven't checked all the precedence in existing drivers but from > memory scale is the standard choice. > > Hardware gain as a writable control is just rarely used and only in devices where > it doesn't affect what we are measuring. In proximity that means time of flight > sensors, not ones based on reflected intensity. > > Jonathan > Hi Jonathan, Thank you for the clarifications. Please see v3. Best regards, Mikael
