Re: [PATCH v2 0/2] Support for Texas Instruments OPT4060 RGBW Color sensor.

[Jonathan Cameron]

On Sat, 5 Oct 2024 18:51:17 +0200
Per-Daniel Olsson <perdaniel.olsson@xxxxxxxx> wrote:

> This patch series adds support for Texas Instruments OPT4060 RGBW Color sensor
> using the i2c interface.
> 
> The driver exposes both raw adc values and calculated lux values though sysfs.
> Integration time can be configured though sysfs as well.

Lux is a unit with a particular light curve.  It has no defined meaning for
color channels.  As a result we usually only have colors as intensity channels
(unit free).  If it is possible to compute an estimate of the illuminance then
that can be a separate IIO_LIGHT channel.

> The OPT4060 sensor
> supports both rising and falling threshold interrupts. These interrupts are
> exposed as IIO events. The driver also implements an IIO triggered buffer with
> two triggers, one trigger for conversion ready interrupts and one trigger for
> threshold interrupts. The typical use case for this is to define a threshold and
> listen for the events, and at the same time enable the triggered buffer with the
> threshold trigger. Once the application gets the threshold event, the values
> from the time of the event will be available in the triggered buffer. This
> limits the number of interrupts between sensor and host and also the the usage
> of sysfs for reading values after events.

We have had various discussions of threshold triggers in the past, but I don't
think we ever merged any (maybe there is one somewhere?) The reasons for that are:
1) They are hard for generic userspace to understand.
2) For light sensors the input tends to be slow moving - grabbing one reading
   on a threshold interrupt is rarely that informative (or are you grabbing them
   on dataready once the threshold is breached?)
3) If we want to do threshold triggers we should really add generic infrastructure
   for them based on adding an in kernel events consumer path and a way to
   instantiate a trigger based on any event.  Doing it in a single driver creates
   an ABI we won't want to retain long term.

So how important is this feature to you and why?  Maybe it is time to finally
take a close look at option 3.

Jonathan

> 
> Changes in v2:
> - dt-bindings: Removed incorrect allOf.
> - dt-bindings: Changed to generic node name.
> - Correction in opt4060_trigger_one_shot(...) for continuous mode.
> - Correction in opt4060_power_down(...), wrong register was read.
> - Corrected usage of active_scan_mask in opt4060_trigger_handler(...).
> - Clean-up of various comments.
> - Link to V1: https://lore.kernel.org/lkml/20241003164932.1162049-1-perdaniel.olsson@xxxxxxxx/
> 
> Per-Daniel Olsson (2):
>   dt-bindings: iio: light: Document TI OPT4060 RGBW sensor
>   iio: light: Add support for TI OPT4060 color sensor
> 
>  .../bindings/iio/light/ti,opt4060.yaml        |   51 +
>  drivers/iio/light/Kconfig                     |   13 +
>  drivers/iio/light/Makefile                    |    1 +
>  drivers/iio/light/opt4060.c                   | 1216 +++++++++++++++++
>  4 files changed, 1281 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/ti,opt4060.yaml
>  create mode 100644 drivers/iio/light/opt4060.c
> 
> 
> base-commit: 0c559323bbaabee7346c12e74b497e283aaafef5