Re: [RFC PATCH v2 0/2] iio: light: add support for Broadcom APDS9999

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 13 May 2026 10:09:02 +0200
"Jose A. Perez de Azpillaga" <azpijr@gmail.com> wrote:

> Add IIO driver and DT binding for the Broadcom APDS9999 ambient light
> and proximity sensor. The green channel uses optical coating technology
> to approximate the human eye spectral response for ALS/lux measurements.
> IIO_INTENSITY channels are also provided for red, green, blue, and
> clear/IR so userspace can compute its own weighted lux.
> 
> Questions for reviewers:
>   1. Is the green channel as an ALS/lux approximation acceptable given
>      the datasheet's claim of optical coating to emulate V(lambda)?

It's the best we can do and the datasheet does say to do this. So whilst
I'll express some doubts, it's fine. I think you shout about it enough that
we shouldn't get that copied into other places.

>   2. Is the IIO_INTENSITY channel layout correct? (red, green, blue,
>      clear with modifiers, no scale)

They sometimes still have scale if there is a variable gain amplifier
we need to control / express the value of.  Looks like that might apply here.

However, it's the one channel where the ABI docs make it clear there is not
meaningful unit from _raw * _scale.  Just that if raw doubles and scale
halves we should get the same value.

>   3. Should the binding be merged with avago,apds9300.yaml or kept
>      separate given the extra vled-supply?

I'm fine with this either way. Marginal case.

With those answered, you can drop the RFC for v3 ;)

Jonathan

> 
> v2:
>   - Added IIO_INTENSITY channels for R, G, B, clear
>   - Switched to guard(mutex)(), devm_mutex_init(), dev_err_probe()
>   - Replaced remove() with devm_add_action_or_reset()
>   - Don't fail probe on PART_ID mismatch (fallback compatibles)
>   - Replaced manual shifts with FIELD_PREP() / GENMASK
>   - Used get_unaligned_le24(), sizeof(buf), NSEC_PER_SEC, fsleep()
>   - Named gain register values, explicit resolution enum values
>   - Removed section comments, grouped bit defines under registers
>   - Squashed Kconfig/Makefile into driver patch
>   - Split MAINTAINERS entry across patches
>   - DT binding: added vled-supply, interrupts, required vdd-supply
>   - DT binding: fixed title, added blank lines, full description
>   - Dropped "DT binding" duplication from binding commit subject
> 
> Link to v1: https://lore.kernel.org/linux-iio/cover.1778491503.git.azpijr@gmail.com/
> 
> Jose A. Perez de Azpillaga (2):
>   dt-bindings: iio: light: add Broadcom APDS9999
>   iio: light: add support for APDS9999 sensor
> 
>  .../bindings/iio/light/brcm,apds9999.yaml     |  53 +++
>  MAINTAINERS                                   |   7 +
>  drivers/iio/light/Kconfig                     |  10 +
>  drivers/iio/light/Makefile                    |   1 +
>  drivers/iio/light/apds9999.c                  | 344 ++++++++++++++++++
>  5 files changed, 415 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml
>  create mode 100644 drivers/iio/light/apds9999.c
> 
>