iio: light: add driver for Broadcom APDS9999

Add Device Tree binding documentation for the Broadcom APDS9999 ambient light sensor.

Proximity and RGB features are not yet implemented in the driver and therefore not included in this binding.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

---
 .../bindings/iio/light/brcm,apds9999.yaml     | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml

Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml new file +5 -0

new file mode 100644
index 000000000000..b20ef3677670
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/brcm,apds9999.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#

Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml new file +11 -0

+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        light-sensor@52 {
+            compatible = "brcm,apds9999";
+            reg = <0x52>;
+            vdd-supply = <&vdd_reg>;
+        };
+    };

On Mon, 11 May 2026 12:14:44 +0200
"Jose A. Perez de Azpillaga" <[email protected]> wrote:

Title doesn't need to mention binding twice.

dt-bindings: iio: light: add Broadcomm ADPS9999

Quoted 6 lines

Add Device Tree binding documentation for the Broadcom APDS9999 ambient light sensor.

Proximity and RGB features are not yet implemented in the driver and therefore not included in this binding.

Quoted 1 line

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

Can you combine this with the existing avago,apds9300.yaml binding? Other than the missing interrupt in this file they look identical.

Note that a separate driver doesn't necessarily need a separate binding doc. May not be worth doing given the extra supply this part takes (which you are currently missing by the look of it)

Power supply should be required

Also you need to fully describe the part where possible. Here there is an interrupt and a vcsel-supply by the look of it.

Add IIO driver for Broadcom APDS9999 ambient light sensor.

The APDS9999 is a digital proximity and RGB sensor with ALS capability. This driver implements the ALS/Lux functionality using the green channel.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

---
 drivers/iio/light/apds9999.c | 325 +++++++++++++++++++++++++++++++++++
 1 file changed, 325 insertions(+)
 create mode 100644 drivers/iio/light/apds9999.c

drivers/iio/light/apds9999.c new file +0 -0

new file mode 100644
index 000000000000..cf44c3003415

drivers/iio/light/apds9999.c new file +8 -0

+ *
+ * Copyright (C) 2026
+ * Author: Jose A. Perez de Azpillaga <[email protected]>
+ *
+ * TODO: proximity and color sensor
+ */
+
+#include <linux/bitops.h>

drivers/iio/light/apds9999.c new file +6 -0

+};
+
+/*
+ * ALS resolution / integration time
+ * Resolution bits [6:4] set both the ADC bit-width and the
+ * integration time used in the lux conversion formula

drivers/iio/light/apds9999.c new file +4 -0

+};
+
+static const int apds9999_itimes_us[APDS9999_RES_NUM] = {
+	[APDS9999_RES_20BIT] =   400000,

drivers/iio/light/apds9999.c new file +5 -0

+	int als_gain_idx;	/* index into apds9999_gains[] */
+	int als_res;		/* index into apds9999_itimes_us[] */
+	int als_rate;		/* LS_MEAS_RATE[2:0] register value */
+};
+

drivers/iio/light/apds9999.c new file +7 -0

+
+out:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+/*

drivers/iio/light/apds9999.c new file +34 -0

+	ret = 0;
+
+out:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int apds9999_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct apds9999_data *data = iio_priv(indio_dev);
+	int gain, itime_us;
+	u64 scale_nano;
+	u32 counts;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = apds9999_als_read(data, &counts);
+		if (ret < 0)
+			return ret;
+		*val = (int)counts;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Scale (lux per count) = 54 / (gain * integration_time_ms)
+		 *
+		 * The constant 54 is derived from the datasheet table:
+		 *   at gain = 3x, itime = 100 ms → 0.180 lux/count
+		 *   → C = 0.180 * 3 * 100 = 54
+		 *
+		 * Expressed as IIO_VAL_INT_PLUS_NANO.

drivers/iio/light/apds9999.c new file +9 -0

+		*val2 = (int)(scale_nano % 1000000000);
+		return IIO_VAL_INT_PLUS_NANO;
+
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = apds9999_itimes_us[data->als_res];
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	default:

drivers/iio/light/apds9999.c new file +11 -0

+	},
+};
+
+static const struct iio_info apds9999_info = {
+	.read_raw = apds9999_read_raw,
+};
+
+static int apds9999_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct apds9999_data *data;

drivers/iio/light/apds9999.c new file +10 -0

+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, apds9999_id);
+
+static const struct of_device_id apds9999_of_match[] = {
+	{ .compatible = "brcm,apds9999" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, apds9999_of_match);
+

drivers/iio/light/apds9999.c new file +5 -0

+module_i2c_driver(apds9999_driver);
+
+MODULE_AUTHOR("Jose A. Perez de Azpillaga <[email protected]>");
+MODULE_DESCRIPTION("APDS-9999 Lux Light Sensor IIO Driver");
+MODULE_LICENSE("GPL");

Quoted 7 lines · On Mon, 11 May 2026 12:14:47 +0200 "Jose A. Perez de Azpillaga"…

Add IIO driver for Broadcom APDS9999 ambient light sensor.

The APDS9999 is a digital proximity and RGB sensor with ALS capability. This driver implements the ALS/Lux functionality using the green channel.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

Hi Jose,

For an RFC I'd normally expect to see a question list or other statement of why it's an RFC rather than a formal submission. One thing to note is many reviewers won't even look at an RFC that doesn't have such a list of questions.

Various comments inline but so far looks like a fairly standard light sensor driver so all minor stuff.

Thanks,

Jonathan

Quoted 5 lines

---
 drivers/iio/light/apds9999.c | 325 +++++++++++++++++++++++++++++++++++
 1 file changed, 325 insertions(+)
 create mode 100644 drivers/iio/light/apds9999.c

drivers/iio/light/apds9999.c line 7

Quoted 13 lines

drivers/iio/light/apds9999.c new file +7 -0

new file mode 100644
index 000000000000..cf44c3003415
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IIO driver for Broadcom APDS9999 Lux Light Sensor
+ *
+ * Uses the green channel (ALS) which approximates human eye spectral
+ * response for ambient light sensing (lux). RGB and proximity (PS)
+ * functionality not yet implemented.

I'd not mention what is todo twice. That just makes churn in patches adding them!

Quoted 8 lines

drivers/iio/light/apds9999.c new file +8 -0

+ *
+ * Copyright (C) 2026
+ * Author: Jose A. Perez de Azpillaga <[email protected]>
+ *
+ * TODO: proximity and color sensor
+ */
+
+#include <linux/bitops.h>

drivers/iio/light/apds9999.c line 25

Quoted 10 lines

drivers/iio/light/apds9999.c new file +10 -0

+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+/* Registers */

Ideally a set of defines should be self describing enough to not need 'section' comments. These comments have a nasty habbit of becoming wrong over time!

drivers/iio/light/apds9999.c line 36

Quoted 11 lines

drivers/iio/light/apds9999.c new file +11 -0

+#define APDS9999_REG_MAIN_CTRL		0x00
+#define APDS9999_REG_LS_MEAS_RATE	0x04
+#define APDS9999_REG_LS_GAIN		0x05
+#define APDS9999_REG_PART_ID		0x06
+#define APDS9999_REG_MAIN_STATUS	0x07
+#define APDS9999_REG_LS_DATA_GREEN_0	0x0D
+
+#define APDS9999_PART_ID		0xC2
+
+/* MAIN_CTRL */
+#define APDS9999_MAIN_CTRL_LS_EN	BIT(1)

I would keep these with the registers addresses above. A nice trick to make them look visually different is to use some additional indenting. e.g.

#define APDS9999_REG_MAIN_CTRL 0x00
#define APDS9999_MAIN_CTRL_LS_EN BIT(1)

#define APDS9999_REG_LS_MEAS_RATE 0x04
#define APDS9999_REG_LS_GAIN 0x05
#define APDS9999_REG_PART_ID 0x06
#define APDS9999_REG_MAIN_STATUS 0x07
#define APDS9999_MAIN_STATUS_LS_DATA BIT(3)
#define APDS9999_REG_LS_DATA_GREEN_0 0x0D

drivers/iio/light/apds9999.c line 42

Quoted 6 lines

drivers/iio/light/apds9999.c new file +6 -0

+
+/* MAIN_STATUS */
+#define APDS9999_MAIN_STATUS_LS_DATA	BIT(3)
+
+/* LS_MEAS_RATE */
+#define APDS9999_LS_RES_SHIFT		4

Define a mask instead then use FIELD_PREP() / FIELD_GET() That combines ensuring no overflow with a single representation of the register field.

drivers/iio/light/apds9999.c line 46

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+
+/*
+ * ALS gain options (register value -> factor)
+ */

Where it fits use single line comments
/* ALS gain options (register value -> factor) */

Given they are specific register values, it may make sense to use defines or an enum then explicit initialization

drivers/iio/light/apds9999.c line 48

Quoted 2 lines

drivers/iio/light/apds9999.c new file +2 -0

+static const int apds9999_gains[] = {
+	1, 3, 6, 9, 18,

	[ADPDS9999_GAIN_X1] = 1,
	[ADPDS9999_GAIN_X3] = 3,

Then if you are initializing defaults etc you can use the register value define directly. Similar to you've done for RES.

Quoted 6 lines

drivers/iio/light/apds9999.c new file +6 -0

+};
+
+/*
+ * ALS resolution / integration time
+ * Resolution bits [6:4] set both the ADC bit-width and the
+ * integration time used in the lux conversion formula

drivers/iio/light/apds9999.c line 64

Quoted 10 lines

drivers/iio/light/apds9999.c new file +10 -0

+ * Value: integration time in microseconds
+ */
+enum apds9999_resolution {
+	APDS9999_RES_20BIT,	/* 400 ms */
+	APDS9999_RES_19BIT,	/* 200 ms */
+	APDS9999_RES_18BIT,	/* 100 ms (default) */
+	APDS9999_RES_17BIT,	/*  50 ms */
+	APDS9999_RES_16BIT,	/*  25 ms */
+	APDS9999_RES_13BIT,	/*  3.125 ms */
+	APDS9999_RES_NUM,

No trailing comma as that has to be last one.
For the others I'd use explicit assignment e.g.

	APDS9999_RES_20BIT = 0,

etc so it is clear they are actual register field values not just an enum where any unique values are enough.

Quoted 9 lines

drivers/iio/light/apds9999.c new file +9 -0

+};
+
+static const int apds9999_itimes_us[APDS9999_RES_NUM] = {
+	[APDS9999_RES_20BIT] =   400000,
+	[APDS9999_RES_19BIT] =   200000,
+	[APDS9999_RES_18BIT] =   100000,
+	[APDS9999_RES_17BIT] =    50000,
+	[APDS9999_RES_16BIT] =    25000,
+	[APDS9999_RES_13BIT] =     3125,

drivers/iio/light/apds9999.c line 78

Quoted 5 lines

drivers/iio/light/apds9999.c new file +5 -0

+};
+
+#define APDS9999_DEFAULT_RES		APDS9999_RES_18BIT
+#define APDS9999_DEFAULT_GAIN_IDX	1	/* 3x */
+#define APDS9999_DEFAULT_RATE		2	/* 100 ms */

Give defines for the actual register values. Then use them in init() rather than 'DEFAULT magic values.

drivers/iio/light/apds9999.c line 82

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+
+struct apds9999_data {
+	struct i2c_client *client;
+	struct mutex lock;

Locks must have a comment saying what data they are protecting.

Quoted 8 lines

drivers/iio/light/apds9999.c new file +8 -0

+	int als_gain_idx;	/* index into apds9999_gains[] */
+	int als_res;		/* index into apds9999_itimes_us[] */
+	int als_rate;		/* LS_MEAS_RATE[2:0] register value */
+};
+
+/*
+ * Hardware init, write default register values and enable ALS
+ */

drivers/iio/light/apds9999.c line 97

Quoted 7 lines

drivers/iio/light/apds9999.c new file +7 -0

+static int apds9999_init(struct apds9999_data *data)
+{
+	struct i2c_client *client = data->client;
+	u8 reg;
+	int ret;
+
+	mutex_lock(&data->lock);

As below - guard() helps in simple cases like this.

drivers/iio/light/apds9999.c line 100

Quoted 3 lines

drivers/iio/light/apds9999.c new file +3 -0

+
+	reg = (APDS9999_DEFAULT_RES << APDS9999_LS_RES_SHIFT) |
+	      APDS9999_DEFAULT_RATE;

FIELD_PREP() for both of them so we won't need to figure out if they are shifted or not.

drivers/iio/light/apds9999.c line 106

Quoted 6 lines

drivers/iio/light/apds9999.c new file +6 -0

+	ret = i2c_smbus_write_byte_data(client, APDS9999_REG_LS_MEAS_RATE, reg);
+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_byte_data(client, APDS9999_REG_LS_GAIN,
+					APDS9999_DEFAULT_GAIN_IDX);

As mentioned above, actual define for that index rather than a default one. The only place I expect to see what chosen defaults actually are is what is passed to calls in this function.

drivers/iio/light/apds9999.c line 117

Quoted 11 lines

drivers/iio/light/apds9999.c new file +11 -0

+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_byte_data(client, APDS9999_REG_MAIN_CTRL,
+					APDS9999_MAIN_CTRL_LS_EN);
+	if (ret < 0)
+		goto out;
+
+	data->als_gain_idx = APDS9999_DEFAULT_GAIN_IDX;
+	data->als_res = APDS9999_DEFAULT_RES;
+	data->als_rate = APDS9999_DEFAULT_RATE;

Given you don't unwind any of these written before an error (which is fine) it makes more logical sense to assign these caches are each write is known to succeed - that is spread them out in the function above.

Quoted 7 lines

drivers/iio/light/apds9999.c new file +7 -0

+
+out:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+/*

drivers/iio/light/apds9999.c line 134

Quoted 10 lines

drivers/iio/light/apds9999.c new file +10 -0

+ * Read the 3-byte green-channel ADC value from 0x0D..0x0F
+ * Waits for LS_DATA_STATUS to be asserted (up to ~2x integration time)
+ */
+static int apds9999_als_read(struct apds9999_data *data, u32 *counts)
+{
+	struct i2c_client *client = data->client;
+	u8 buf[3];
+	int ret, tries;
+
+	mutex_lock(&data->lock);

Use guard(mutex)(&data->lock); and direct returns in error paths given no need to get to the unlock once it is happening on leaving the scope of the function.

Make sure to also included cleanup.h for that.

drivers/iio/light/apds9999.c line 138

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+
+	/*
+	 * Poll MAIN_STATUS for new data.  Timeout: ~2 integration periods
+	 * plus margin.  Each try sleeps 20 ms

Do we have no way of having a better idea of how long we might need to sleep?

drivers/iio/light/apds9999.c line 142

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+	 */
+	tries = (apds9999_itimes_us[data->als_res] * 2) / 20000;
+	if (tries < 2)
+		tries = 2;

	tries = max(2, tries);

Maybe combine with line above.

drivers/iio/light/apds9999.c line 151

Quoted 9 lines

drivers/iio/light/apds9999.c new file +9 -0

+
+	while (tries--) {
+		ret = i2c_smbus_read_byte_data(client,
+						APDS9999_REG_MAIN_STATUS);
+		if (ret < 0)
+			goto out;
+		if (ret & APDS9999_MAIN_STATUS_LS_DATA)
+			break;
+		msleep(20);

fsleep() probably appropriate.

drivers/iio/light/apds9999.c line 162

Quoted 11 lines

drivers/iio/light/apds9999.c new file +11 -0

+	}
+
+	if (tries < 0) {
+		ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	/* Block-read all 3 bytes of LS_DATA_GREEN */
+	ret = i2c_smbus_read_i2c_block_data(client,
+					     APDS9999_REG_LS_DATA_GREEN_0,
+					     3, buf);

sizeof(buf)

drivers/iio/light/apds9999.c line 165

Quoted 3 lines

drivers/iio/light/apds9999.c new file +3 -0

+	if (ret < 0)
+		goto out;
+	if (ret != 3) {

sizeof(buf)

drivers/iio/light/apds9999.c line 170

Quoted 5 lines

drivers/iio/light/apds9999.c new file +5 -0

+		ret = -EIO;
+		goto out;
+	}
+
+	*counts = buf[0] | ((u32)buf[1] << 8) | ((u32)(buf[2] & 0x0F) << 16);

Smells like get_unaligned_le24() & GENMASK(20, 0)
or something like that.

Quoted 38 lines

drivers/iio/light/apds9999.c new file +38 -0

+	ret = 0;
+
+out:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
+static int apds9999_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct apds9999_data *data = iio_priv(indio_dev);
+	int gain, itime_us;
+	u64 scale_nano;
+	u32 counts;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = apds9999_als_read(data, &counts);
+		if (ret < 0)
+			return ret;
+		*val = (int)counts;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Scale (lux per count) = 54 / (gain * integration_time_ms)
+		 *
+		 * The constant 54 is derived from the datasheet table:
+		 *   at gain = 3x, itime = 100 ms → 0.180 lux/count
+		 *   → C = 0.180 * 3 * 100 = 54
+		 *
+		 * Expressed as IIO_VAL_INT_PLUS_NANO.
+		 * Uses itime_us directly to avoid truncation for
+		 * sub-millisecond integration times (e.g. 3.125 ms).
+		 */
+		gain = apds9999_gains[data->als_gain_idx];

drivers/iio/light/apds9999.c line 214

Quoted 6 lines

drivers/iio/light/apds9999.c new file +6 -0

+		itime_us = apds9999_itimes_us[data->als_res];
+
+		/* scale_nano = 54e12 / (gain * itime_us) nano-lux/count */
+		scale_nano = div_u64(54000000000000ULL,
+				     (u32)(gain * itime_us));
+		*val = (int)(scale_nano / 1000000000);

NANO rather than making a reviewer count 0s ;)

Quoted 15 lines

drivers/iio/light/apds9999.c new file +15 -0

+		*val2 = (int)(scale_nano % 1000000000);
+		return IIO_VAL_INT_PLUS_NANO;
+
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = apds9999_itimes_us[data->als_res];
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_chan_spec apds9999_channels[] = {
+	{

drivers/iio/light/apds9999.c line 233

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),

Given this is mainly an RGB sensor I'd want to see the IIO_INTENSITY channels there as well for an initial submission. Understood that you've sent this RFC for early feedback.

Quoted 11 lines

drivers/iio/light/apds9999.c new file +11 -0

+	},
+};
+
+static const struct iio_info apds9999_info = {
+	.read_raw = apds9999_read_raw,
+};
+
+static int apds9999_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct apds9999_data *data;

drivers/iio/light/apds9999.c line 254

Quoted 10 lines

drivers/iio/light/apds9999.c new file +10 -0

+	struct iio_dev *indio_dev;
+	int ret, part_id;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	mutex_init(&data->lock);

	ret = devm_mutex_init(&data->lock);
	if (ret)
		return ret;

The advantage this brings for lock debugging is pretty minor but it's also cheap so for new code always use it.

Quoted 8 lines

drivers/iio/light/apds9999.c new file +8 -0

+
+	part_id = i2c_smbus_read_byte_data(client, APDS9999_REG_PART_ID);
+	if (part_id < 0) {
+		dev_err(dev, "failed to read PART_ID: %d\n", part_id);
+		return part_id;
+	}
+	if (part_id != APDS9999_PART_ID) {
+		dev_err(dev, "unexpected PART_ID 0x%02x (expected 0x%02x)\n",

drivers/iio/light/apds9999.c line 263

Quoted 1 line

drivers/iio/light/apds9999.c new file +1 -0

+			part_id, APDS9999_PART_ID);

We don't fail on missmatched IDs - it's fine to bring an info message but not more than that. The reason is fallback compatibles in device tree. If a future device is fully compatible with this (it may have more features and typically has a different part ID) we want the driver from an old kernel to support it. That only works if we don't error out on failure to match IDs.

drivers/iio/light/apds9999.c line 272

Quoted 9 lines

drivers/iio/light/apds9999.c new file +9 -0

+		return -ENODEV;
+	}
+
+	dev_dbg(dev, "APDS-9999 detected, PART_ID=0x%02x\n", part_id);
+
+	ret = apds9999_init(data);
+	if (ret < 0) {
+		dev_err(dev, "failed to initialize device: %d\n", ret);
+		return ret;

As below - return dev_err_probe()

drivers/iio/light/apds9999.c line 284

Quoted 12 lines

drivers/iio/light/apds9999.c new file +12 -0

+	}
+
+	indio_dev->name = "apds9999";
+	indio_dev->info = &apds9999_info;
+	indio_dev->channels = apds9999_channels;
+	indio_dev->num_channels = ARRAY_SIZE(apds9999_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to register IIO device: %d\n", ret);
+		return ret;

For all error messages in probe() or functions called from probe() use

		return dev_err_probe(dev, ret, "failed to register IIO device\n");

which handles deferred probing, drop unnecessary prints such as those on memory failure, pretty prints the return value and usefully is more compact.

drivers/iio/light/apds9999.c line 296

Quoted 12 lines

drivers/iio/light/apds9999.c new file +12 -0

+	}
+
+	return 0;
+}
+
+/*
+ * Disable the light sensor on driver removal
+ */
+static void apds9999_remove(struct i2c_client *client)
+{
+	/* Set MAIN_CTRL to 0 → LS_EN cleared, sensor enters standby */
+	i2c_smbus_write_byte_data(client, APDS9999_REG_MAIN_CTRL, 0);

This looks to me like you will be doing tear down in a different order to setup. For example the userspace interfaces will not be removeed until after you've put the sensor in stand by.

There are two solutions to this (1st preferred)
1. Use devm_add_action_or_reset() to register the standby write here in a driver specific callback that is cleaned up in the right place. That might be registered in adps9999_init() rather than directly in probe. Once you've done that drop the remove() callback as no longer needed.
2. Stop using devm_ registration calls from whereever in probe this matches up with.

Basically you mustn't mix and match devm and non devm based cleanup.

drivers/iio/light/apds9999.c line 300

Quoted 4 lines

drivers/iio/light/apds9999.c new file +4 -0

+}
+
+static const struct i2c_device_id apds9999_id[] = {
+	{ "apds9999", 0 },

The , 0 doesn't add anything so drop it

	{ "apds9999" },

Quoted 14 lines

drivers/iio/light/apds9999.c new file +14 -0

+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, apds9999_id);
+
+static const struct of_device_id apds9999_of_match[] = {
+	{ .compatible = "brcm,apds9999" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, apds9999_of_match);
+
+static struct i2c_driver apds9999_driver = {
+	.driver = {
+		.name = "apds9999",
+		.of_match_table = apds9999_of_match,

drivers/iio/light/apds9999.c line 320

Quoted 6 lines

drivers/iio/light/apds9999.c new file +6 -0

+	},
+	.probe = apds9999_probe,
+	.remove = apds9999_remove,
+	.id_table = apds9999_id,
+};
+

Common convention to tightly compule this macro with the struct i2c_driver by not having a blank line here.

drivers/iio/light/apds9999.c line 325

Quoted 5 lines

drivers/iio/light/apds9999.c new file +5 -0

+module_i2c_driver(apds9999_driver);
+
+MODULE_AUTHOR("Jose A. Perez de Azpillaga <[email protected]>");
+MODULE_DESCRIPTION("APDS-9999 Lux Light Sensor IIO Driver");
+MODULE_LICENSE("GPL");

Add Kconfig entry and Makefile rule for the APDS9999 driver.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

---
 drivers/iio/light/Kconfig  | 10 ++++++++++
 drivers/iio/light/Makefile |  1 +
 2 files changed, 11 insertions(+)

drivers/iio/light/Kconfig +10 -0

index eff33e456c70..c7256ffb972d 100644
@@ -119,6 +119,16 @@ config APDS9960
 	  To compile this driver as a module, choose M here: the
 	  module will be called apds9960
 
+config APDS9999
+	tristate "APDS9999 ambient light sensor"
+	depends on I2C
+	help
+	  Say Y here if you want to build support for the Broadcom APDS9999
+	  ambient light sensor (ALS/Lux).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called apds9999.
+
 config AS73211
 	tristate "AMS AS73211 XYZ color sensor and AMS AS7331 UV sensor"
 	depends on I2C

drivers/iio/light/Makefile +1 -0

index c0048e0d5ca8..39e62dfc10c7 100644
@@ -14,6 +14,7 @@ obj-$(CONFIG_APDS9160)		+= apds9160.o
 obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_APDS9306)		+= apds9306.o
 obj-$(CONFIG_APDS9960)		+= apds9960.o
+obj-$(CONFIG_APDS9999)		+= apds9999.o
 obj-$(CONFIG_AS73211)		+= as73211.o
 obj-$(CONFIG_BH1745)		+= bh1745.o
 obj-$(CONFIG_BH1750)		+= bh1750.o

Quoted 3 lines · On Mon, 11 May 2026 12:14:50 +0200 "Jose A. Perez de Azpillaga"…

Add Kconfig entry and Makefile rule for the APDS9999 driver.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

Squash with previous patch. There are some corners of the kernel
where they have a pattern of building up particularly complex drivers in multiple patches that won't compile then adding build support.

We never do that in IIO each patch must build whatever was added (slightly exception for complex multi module drivers where it can
be tricky to do).

Anyhow, short story is merge with previous.

Add maintainer entry for the APDS9999 driver.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

MAINTAINERS +0 -0

index 1aa9c989973f..cdb703f37f06 100644

MAINTAINERS +1 -0

+
 AVIA HX711 ANALOG DIGITAL CONVERTER IIO DRIVER
 M:	Andreas Klinger <[email protected]>
 L:	[email protected]

Quoted 12 lines · On Mon, 11 May 2026 12:14:52 +0200 "Jose A. Perez de Azpillaga"…

Add maintainer entry for the APDS9999 driver.

Signed-off-by Jose A. Perez de Azpillaga <[email protected]>

---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

MAINTAINERS +0 -0

index 1aa9c989973f..cdb703f37f06 100644

MAINTAINERS S: Maintained · line 4310

Quoted 9 lines

MAINTAINERS +6 -0

@@ -4302,6 +4302,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
 F:	drivers/iio/light/apds9306.c
 
+BROADCOM APDS9999 AMBIENT LIGHT SENSOR DRIVER
+M:	Jose A. Perez de Azpillaga <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml
+F:	drivers/iio/light/apds9999.c

Build this up as part of the earlier patches.

So add the initial entry in the dt-binding patch (if that's a separate file) then add th driver in the follow on patch.

There is no advantage in having it separate and it annoys some tooling that checks for valid coverage for each file added on a patch by patch basis.

This series adds support for the Broadcom APDS9999 ambient light sensor. The APDS9999 is a digital proximity and RGB sensor with ALS capability, featuring I2C interface, programmable gain and integration time.

This chip is considered by Broadcom to be the 'successor' to the APDS-9960 which has technically been discontinued.

The initial driver implements the ALS/Lux functionality using the green channel, which approximates the human eye spectral response.

Sent as RFC to gather feedback on the driver structure and DT binding.

Jose A. Perez de Azpillaga (4):
dt-bindings: iio: light: add DT binding for Broadcom APDS9999
iio: light: add support for APDS9999 sensor
iio: light: add build support for APDS9999
MAINTAINERS: add myself as APDS9999 maintainer

 .../bindings/iio/light/brcm,apds9999.yaml     |  33 ++
 MAINTAINERS                                   |   7 +
 drivers/iio/light/Kconfig                     |  10 +
 drivers/iio/light/Makefile                    |   1 +
 drivers/iio/light/apds9999.c                  | 325 ++++++++++++++++++
 5 files changed, 376 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml
 create mode 100644 drivers/iio/light/apds9999.c

Quoted 10 lines · On Mon, 11 May 2026 12:14:42 +0200 "Jose A. Perez de Azpillaga"…

This series adds support for the Broadcom APDS9999 ambient light sensor. The APDS9999 is a digital proximity and RGB sensor with ALS capability, featuring I2C interface, programmable gain and integration time.

This chip is considered by Broadcom to be the 'successor' to the APDS-9960 which has technically been discontinued.

The initial driver implements the ALS/Lux functionality using the green channel, which approximates the human eye spectral response.

That's not normally a good approximation unless it is also has very bad colour curves for green! Better than any other channel on it's own but still not good. If we had calibrated sRGB it would be:

Something like: 0.2126R+0.7152G+0.0722B
for example.

The lux vs white light and lux vs incandecent light are fairly linear so 'maybe' it's good enough but it's a bit of a stretch to describe it as an illuminance measure which is what we'd need for it to be measured in lux.

So I'm on the fence at the moment. We might be better off teaching userspace code to do it's own approximation if it has an RGB sensor.

Quoted 21 lines · Jonathan Cameron

Quoted 10 lines · On Mon, 11 May 2026 12:14:42 +0200 "Jose A. Perez de Azpillaga"…

This series adds support for the Broadcom APDS9999 ambient light sensor. The APDS9999 is a digital proximity and RGB sensor with ALS capability, featuring I2C interface, programmable gain and integration time.

This chip is considered by Broadcom to be the 'successor' to the APDS-9960 which has technically been discontinued.

The initial driver implements the ALS/Lux functionality using the green channel, which approximates the human eye spectral response.

That's not normally a good approximation unless it is also has very bad colour curves for green! Better than any other channel on it's own but still not good. If we had calibrated sRGB it would be:

Something like: 0.2126R+0.7152G+0.0722B
for example.

Quoted 4 lines

The lux vs white light and lux vs incandecent light are fairly linear so 'maybe' it's good enough but it's a bit of a stretch to describe it as an illuminance measure which is what we'd need for it to be measured in lux.

the datasheet states: "Approximates human eye response with green channel" and "Uses optical coating technology to emulate human eye spectral response."

that's the reason why I chose to use green channel.

Quoted 2 lines

So I'm on the fence at the moment. We might be better off teaching userspace code to do it's own approximation if it has an RGB sensor.

I'll also add IIO_INTENSITY channels for R, G, B in v2 so userspace can comapute its own weighted lux if it prefers.

--
jose a. p-a

Quoted 27 lines · On Mon, 11 May 2026 17:19:54 +0200 "Jose A. Perez de Azpillaga"…

Quoted 25 lines · Jonathan Cameron

Quoted 10 lines · On Mon, 11 May 2026 12:14:42 +0200 "Jose A. Perez de Azpillaga"…

This series adds support for the Broadcom APDS9999 ambient light sensor. The APDS9999 is a digital proximity and RGB sensor with ALS capability, featuring I2C interface, programmable gain and integration time.

This chip is considered by Broadcom to be the 'successor' to the APDS-9960 which has technically been discontinued.

The initial driver implements the ALS/Lux functionality using the green channel, which approximates the human eye spectral response.

That's not normally a good approximation unless it is also has very bad colour curves for green! Better than any other channel on it's own but still not good. If we had calibrated sRGB it would be:

Something like: 0.2126R+0.7152G+0.0722B
for example.

The lux vs white light and lux vs incandecent light are fairly linear so 'maybe' it's good enough but it's a bit of a stretch to describe it as an illuminance measure which is what we'd need for it to be measured in lux.

Quoted 3 lines

the datasheet states: "Approximates human eye response with green channel" and "Uses optical coating technology to emulate human eye spectral response."

Hehe. Marketing. They provide the light curves that show it approximates very well the green part of human eye spectral response and very badly the rest of it! :)