[PATCH 0/3] iio: light: add AMS TCS3400 driver

[PATCH 0/3] iio: light: add AMS TCS3400 driver

[Petr Hodina via B4 Relay]

Hi,

This patch adds an IIO driver for the AMS TCS3400 color light-to-digital
converter.

The TCS3400 is an I2C-connected RGB color sensor supporting RGBC and
RGB-IR measurement modes, programmable integration time, selectable
gain, optional interrupt-driven sampling, and regulator-based power
control.

Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
---
Petr Hodina (3):
      doc: add Device Tree binding for AMS TCS3400 light sensor
      iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver
      sdm845: tama: Add AMS TCS3400 ambient light sensor

 .../devicetree/bindings/iio/light/ams,tcs3400.yaml |  54 +++
 MAINTAINERS                                        |   7 +
 .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     |  36 +-
 drivers/iio/light/Kconfig                          |  11 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/tcs3400.c                        | 505 +++++++++++++++++++++
 6 files changed, 613 insertions(+), 1 deletion(-)
---
base-commit: 46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b
change-id: 20260119-tsc3400-68a91d8c1355

Best regards,
-- 
Petr Hodina <petr.hodina@protonmail.com>

[PATCH 1/3] doc: add Device Tree binding for AMS TCS3400 light sensor

[Petr Hodina via B4 Relay]

From: Petr Hodina <petr.hodina@protonmail.com>

Adds a new YAML binding describing the AMS TCS3400 I2C light sensor,
including compatible string, registers, interrupts, power supply, and an
example node.

Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
---
 .../devicetree/bindings/iio/light/ams,tcs3400.yaml | 54 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
new file mode 100644
index 000000000000..2c5a9295af1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: AMS TCS3400 Color Light-to-Digital Converter
+
+maintainers:
+  - name: Petr Hodina
+    email: petr.hodina@protonmail.com
+
+description: |
+  The AMS TCS3400 is an I2C-connected color light sensor providing
+  RGBC or RGB-IR measurements with a programmable integration time
+  and gain.
+
+properties:
+  compatible:
+    const: ams,tcs3400
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      Interrupt line signaling ALS data ready or threshold events.
+
+  vdd-supply:
+    description:
+      Regulator supplying the main sensor power.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@39 {
+            compatible = "ams,tcs3400";
+            reg = <0x39>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
+            vdd-supply = <&vdd_3v3>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 14a06f856b81..ab5307a34180 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22866,6 +22866,12 @@ S:	Supported
 F:	drivers/iio/adc/rohm-bd79112.c
 F:	drivers/iio/adc/rohm-bd79124.c
 
+AMS TCS3400 AMBIENT LIGHT SENSOR DRIVER
+M:	Petr Hodina
+L:	Petr Hodina <petr.hodina@protonmail.com>
+S:	Petr Hodina <petr.hodina@protonmail.com>
+F:	Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
+
 ROHM BH1745 COLOUR SENSOR
 M:	Mudit Sharma <muditsharma.info@gmail.com>
 L:	linux-iio@vger.kernel.org

-- 
2.52.0

Re: [PATCH 1/3] doc: add Device Tree binding for AMS TCS3400 light sensor

[David Heidelberg]

On 19/01/2026 18:19, Petr Hodina via B4 Relay wrote:
> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Adds a new YAML binding describing the AMS TCS3400 I2C light sensor,
> including compatible string, registers, interrupts, power supply, and an
> example node.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>   .../devicetree/bindings/iio/light/ams,tcs3400.yaml | 54 ++++++++++++++++++++++
>   MAINTAINERS                                        |  6 +++
>   2 files changed, 60 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
> new file mode 100644
> index 000000000000..2c5a9295af1a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: AMS TCS3400 Color Light-to-Digital Converter
> +
> +maintainers:
> +  - name: Petr Hodina
> +    email: petr.hodina@protonmail.com

  - name <email>

> +
> +description: |

no need to use | or > here, please drop

> +  The AMS TCS3400 is an I2C-connected color light sensor providing
> +  RGBC or RGB-IR measurements with a programmable integration time
> +  and gain.
> +
> +properties:
> +  compatible:
> +    const: ams,tcs3400
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Interrupt line signaling ALS data ready or threshold events.
> +
> +  vdd-supply:
> +    description:
> +      Regulator supplying the main sensor power.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@39 {
> +            compatible = "ams,tcs3400";
> +            reg = <0x39>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> +            vdd-supply = <&vdd_3v3>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 14a06f856b81..ab5307a34180 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22866,6 +22866,12 @@ S:	Supported
>   F:	drivers/iio/adc/rohm-bd79112.c
>   F:	drivers/iio/adc/rohm-bd79124.c
>   
> +AMS TCS3400 AMBIENT LIGHT SENSOR DRIVER
> +M:	Petr Hodina
> +L:	Petr Hodina <petr.hodina@protonmail.com>
> +S:	Petr Hodina <petr.hodina@protonmail.com>

M: Petr Hodina <petr.hodina@protonmail.com>
S: Maintained

drop the L: (e.g. read the header of Maintainers file ;) )

> +F:	Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
> +
>   ROHM BH1745 COLOUR SENSOR
>   M:	Mudit Sharma <muditsharma.info@gmail.com>
>   L:	linux-iio@vger.kernel.org
> 

-- 
David Heidelberg

Re: [PATCH 1/3] doc: add Device Tree binding for AMS TCS3400 light sensor

[Krzysztof Kozlowski]

On Mon, Jan 19, 2026 at 06:19:06PM +0100, Petr Hodina wrote:
> Adds a new YAML binding describing the AMS TCS3400 I2C light sensor,
> including compatible string, registers, interrupts, power supply, and an
> example node.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>  .../devicetree/bindings/iio/light/ams,tcs3400.yaml | 54 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
> new file mode 100644
> index 000000000000..2c5a9295af1a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Never tested.

> +
> +title: AMS TCS3400 Color Light-to-Digital Converter
> +
> +maintainers:
> +  - name: Petr Hodina
> +    email: petr.hodina@protonmail.com

Don't send us LLM code. Never. Why do I think you sent us LLM microslop?
Because such syntax does not exist. Nowhere.

I finished the review here, everything futher has more trivial issues
which you would fix easily if you took recent binding as starting point.

Best regards,
Krzysztof

Re: [PATCH 1/3] doc: add Device Tree binding for AMS TCS3400 light sensor

[Krzysztof Kozlowski]

On Mon, Jan 19, 2026 at 06:19:06PM +0100, Petr Hodina wrote:
> Adds a new YAML binding describing the AMS TCS3400 I2C light sensor,
> including compatible string, registers, interrupts, power supply, and an
> example node.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---

Also:

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Best regards,
Krzysztof

Re: [PATCH 1/3] doc: add Device Tree binding for AMS TCS3400 light sensor

[Daniel Baluta]

On Mon, Jan 19, 2026 at 7:19 PM Petr Hodina via B4 Relay
<devnull+petr.hodina.protonmail.com@kernel.org> wrote:
>
> From: Petr Hodina <petr.hodina@protonmail.com>
>
> Adds a new YAML binding describing the AMS TCS3400 I2C light sensor,
> including compatible string, registers, interrupts, power supply, and an
> example node.
>
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>  .../devicetree/bindings/iio/light/ams,tcs3400.yaml | 54 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 60 insertions(+)
>

Please use the correct subject prefix. E.g: dt-bindings: iio: light

If you are not sure what prefix to use just look around:

git log --oneline Documentation/devicetree/bindings/iio/light/

[PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Petr Hodina via B4 Relay]

From: Petr Hodina <petr.hodina@protonmail.com>

Add support for the AMS TCS3400 I2C color light-to-digital converter.
The driver supports RGBC and RGB-IR modes, programmable integration
time, optional interrupt-driven buffered capture, and regulator-based
power control.

Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
---
 MAINTAINERS                 |   1 +
 drivers/iio/light/Kconfig   |  11 +
 drivers/iio/light/Makefile  |   1 +
 drivers/iio/light/tcs3400.c | 505 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 518 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ab5307a34180..3d7d0aa10c55 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22871,6 +22871,7 @@ M:	Petr Hodina
 L:	Petr Hodina <petr.hodina@protonmail.com>
 S:	Petr Hodina <petr.hodina@protonmail.com>
 F:	Documentation/devicetree/bindings/iio/light/ams,tcs3400.yaml
+F:	drivers/iio/light/tcs3400.c
 
 ROHM BH1745 COLOUR SENSOR
 M:	Mudit Sharma <muditsharma.info@gmail.com>
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index ac1408d374c9..73419d80e3a7 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -580,6 +580,17 @@ config ST_UVIS25_SPI
 	depends on ST_UVIS25
 	select REGMAP_SPI
 
+config TCS3400
+	tristate "AMS TCS3400 color light-to-digital converter"
+	depends on I2C
+	default n
+	help
+	  If you say yes here you get support for the AMS TCS3400.
+	  This sensor can detect ambient light and color (RGB) values.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called tcs3400.
+
 config TCS3414
 	tristate "TAOS TCS3414 digital color sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index c0048e0d5ca8..847ef7bf0f57 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_STK3310)          += stk3310.o
 obj-$(CONFIG_ST_UVIS25)		+= st_uvis25_core.o
 obj-$(CONFIG_ST_UVIS25_I2C)	+= st_uvis25_i2c.o
 obj-$(CONFIG_ST_UVIS25_SPI)	+= st_uvis25_spi.o
+obj-$(CONFIG_TCS3400)		+= tcs3400.o
 obj-$(CONFIG_TCS3414)		+= tcs3414.o
 obj-$(CONFIG_TCS3472)		+= tcs3472.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
diff --git a/drivers/iio/light/tcs3400.c b/drivers/iio/light/tcs3400.c
new file mode 100644
index 000000000000..22c8c4e803cf
--- /dev/null
+++ b/drivers/iio/light/tcs3400.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TCS3400 - AMS/TAOS color light sensor with RGBC and RGB-IR channels
+ *
+ * Copyright (c) 2025 Petr Hodina
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/regulator/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define TCS3400_DRV_NAME "tcs3400"
+#define TCS3400_CMD_REG(reg) (0x80 | (reg))
+#define TCS3400_CMD_SPECIAL 0xE0
+#define TCS3400_CMD_ALS_INT_CLR 0xE6
+#define TCS3400_CMD_ALL_INT_CLR 0xE7
+#define TCS3400_ENABLE 0x00
+#define TCS3400_ATIME 0x01
+#define TCS3400_WTIME 0x03
+#define TCS3400_PERSIST 0x0C
+#define TCS3400_CONTROL 0x0F /* Gain */
+#define TCS3400_STATUS 0x13
+#define TCS3400_CDATAL 0x14 /* Clear low */
+#define TCS3400_RDATAL 0x16
+#define TCS3400_GDATAL 0x18
+#define TCS3400_BDATAL 0x1A
+#define TCS3400_ID 0x12
+#define TCS3400_CHSEL 0xC0 /* Access IR channel: 0x00 RGBC, 0x80 RGB-IR */
+#define TCS3400_EN_PON BIT(0)
+#define TCS3400_EN_AEN BIT(1)
+#define TCS3400_EN_AIEN BIT(4)
+#define TCS3400_STATUS_AVALID BIT(0)
+#define TCS3400_STATUS_AINT BIT(4)
+#define TCS3400_GAIN_1X 0x00
+#define TCS3400_GAIN_4X 0x01
+#define TCS3400_GAIN_16X 0x02
+#define TCS3400_GAIN_64X 0x03
+#define TCS3400_MAX_ATIME 256
+
+#define TCS3400_IIO_CHANNEL(_index, _mod) { \
+	.type = IIO_INTENSITY, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_##_mod, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			      BIT(IIO_CHAN_INFO_SCALE), \
+			      BIT(IIO_CHAN_INFO_SCALE) | \
+			      BIT(IIO_CHAN_INFO_INT_TIME), \
+	.indexed = 1, \
+	.channel = _index, \
+	.scan_index = _index, \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 16, \
+		.storagebits = 16, \
+		.endianness = IIO_LE, \
+	}, \
+}
+
+struct tcs3400_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	struct regulator *vdd_supply;
+	u8 atime;
+	u8 gain;
+	u8 channel_mode; /* 0x00 or 0x80 */
+	u16 clear_ir; /* clear when mode=0x00, IR when mode=0x80 */
+	u16 red;
+	u16 green;
+	u16 blue;
+};
+
+static const int tcs3400_gains[] = {1, 4, 16, 64};
+
+static int tcs3400_power_on(struct tcs3400_data *data)
+{
+	int ret;
+
+	ret = regulator_enable(data->vdd_supply);
+	if (ret)
+		return ret;
+
+	msleep(20);
+
+	return 0;
+}
+
+static void tcs3400_power_off(struct tcs3400_data *data)
+{
+	regulator_disable(data->vdd_supply);
+}
+
+static int tcs3400_write_reg(struct tcs3400_data *data, u8 reg, u8 val)
+{
+	return i2c_smbus_write_byte_data(data->client, TCS3400_CMD_REG(reg), val);
+}
+
+static int tcs3400_read_reg(struct tcs3400_data *data, u8 reg, u8 *val)
+{
+	int ret = i2c_smbus_read_byte_data(data->client, TCS3400_CMD_REG(reg));
+
+	if (ret < 0)
+		return ret;
+	*val = ret;
+
+	return 0;
+}
+
+static int tcs3400_read_word(struct tcs3400_data *data, u8 reg, u16 *val)
+{
+
+	__le16 buf;
+	int ret = i2c_smbus_read_i2c_block_data(data->client,
+						TCS3400_CMD_REG(reg), 2, (u8 *)&buf);
+	if (ret < 0)
+		return ret;
+	*val = le16_to_cpu(buf);
+	return 0;
+}
+static int tcs3400_clear_interrupt(struct tcs3400_data *data)
+{
+
+	return i2c_smbus_write_byte(data->client, TCS3400_CMD_ALS_INT_CLR);
+}
+
+static int tcs3400_read_channels(struct tcs3400_data *data)
+{
+
+	int ret, retries = 20;
+	u8 status;
+
+	do {
+		ret = tcs3400_read_reg(data, TCS3400_STATUS, &status);
+		if (ret)
+			return ret;
+		if (status & TCS3400_STATUS_AVALID)
+			break;
+		usleep_range(5000, 6000);
+	} while (--retries);
+	if (!retries) {
+		dev_warn(&data->client->dev, "Timeout waiting for valid data\n");
+		return -ETIMEDOUT;
+	}
+	ret = tcs3400_read_word(data, TCS3400_CDATAL, &data->clear_ir);
+	if (ret)
+		return ret;
+
+	ret = tcs3400_read_word(data, TCS3400_RDATAL, &data->red);
+	if (ret)
+		return ret;
+
+	ret = tcs3400_read_word(data, TCS3400_GDATAL, &data->green);
+	if (ret)
+		return ret;
+
+	ret = tcs3400_read_word(data, TCS3400_BDATAL, &data->blue);
+	if (ret)
+		return ret;
+	return 0;
+}
+
+static irqreturn_t tcs3400_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	u16 buf[4];
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = tcs3400_read_channels(data);
+	if (!ret) {
+		buf[0] = data->clear_ir;
+		buf[1] = data->red;
+		buf[2] = data->green;
+		buf[3] = data->blue;
+		iio_push_to_buffers_with_timestamp(indio_dev, buf,
+						   iio_get_time_ns(indio_dev));
+	}
+	mutex_unlock(&data->lock);
+
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tcs3400_irq_handler(int irq, void *priv)
+{
+	struct iio_dev *indio_dev = priv;
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = tcs3400_read_channels(data);
+	if (!ret)
+		iio_trigger_poll_nested(indio_dev->trig);
+
+	tcs3400_clear_interrupt(data);
+	mutex_unlock(&data->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int tcs3400_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = tcs3400_read_channels(data);
+	if (ret) {
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->channel2) {
+		case IIO_MOD_LIGHT_CLEAR:
+			*val = data->clear_ir;
+			break;
+		case IIO_MOD_LIGHT_RED:
+			*val = data->red;
+			break;
+		case IIO_MOD_LIGHT_GREEN:
+			*val = data->green;
+			break;
+		case IIO_MOD_LIGHT_BLUE:
+			*val = data->blue;
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = (TCS3400_MAX_ATIME - data->atime) * 2780000; /* 2.78 ms per cycle */
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int tcs3400_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	int i, ret = 0;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0)
+			return -EINVAL;
+		i = TCS3400_MAX_ATIME - DIV_ROUND_CLOSEST(val2, 2780000);
+		if (i < 1 || i >= TCS3400_MAX_ATIME)
+			return -EINVAL;
+		mutex_lock(&data->lock);
+		data->atime = i;
+		ret = tcs3400_write_reg(data, TCS3400_ATIME, data->atime);
+		mutex_unlock(&data->lock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_chan_spec tcs3400_channels[] = {
+	TCS3400_IIO_CHANNEL(0, LIGHT_CLEAR),
+	TCS3400_IIO_CHANNEL(1, LIGHT_RED),
+	TCS3400_IIO_CHANNEL(2, LIGHT_GREEN),
+	TCS3400_IIO_CHANNEL(3, LIGHT_BLUE),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static ssize_t tcs3400_enable_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	u8 enable;
+	int ret;
+
+	ret = tcs3400_read_reg(data, TCS3400_ENABLE, &enable);
+	if (ret)
+		return ret;
+	return sprintf(buf, "%d\n", !!(enable & (TCS3400_EN_PON | TCS3400_EN_AEN)));
+}
+
+static ssize_t tcs3400_enable_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	bool enable;
+	int ret;
+	u8 val;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+	mutex_lock(&data->lock);
+	if (enable)
+		val = TCS3400_EN_PON | TCS3400_EN_AEN;
+	else
+		val = 0;
+	ret = tcs3400_write_reg(data, TCS3400_ENABLE, val);
+	mutex_unlock(&data->lock);
+	if (ret)
+		return ret;
+
+	if (enable)
+		msleep(20);
+	return len;
+}
+
+static ssize_t tcs3400_ir_mode_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct tcs3400_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n", !!data->channel_mode);
+}
+
+static ssize_t tcs3400_ir_mode_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct tcs3400_data *data = iio_priv(indio_dev);
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+	mutex_lock(&data->lock);
+	data->channel_mode = enable ? 0x80 : 0x00;
+	ret = tcs3400_write_reg(data, TCS3400_CHSEL, data->channel_mode);
+	mutex_unlock(&data->lock);
+	return ret ? ret : len;
+}
+
+static IIO_DEVICE_ATTR(enable, 0644, tcs3400_enable_show, tcs3400_enable_store, 0);
+static IIO_DEVICE_ATTR(ir_mode, 0644, tcs3400_ir_mode_show, tcs3400_ir_mode_store, 0);
+
+static struct attribute *tcs3400_attributes[] = {
+	&iio_dev_attr_enable.dev_attr.attr,
+	&iio_dev_attr_ir_mode.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tcs3400_attribute_group = {
+	.attrs = tcs3400_attributes,
+};
+
+static const struct iio_info tcs3400_info = {
+	.read_raw = tcs3400_read_raw,
+	.write_raw = tcs3400_write_raw,
+	.attrs = &tcs3400_attribute_group,
+};
+
+static int tcs3400_probe(struct i2c_client *client)
+{
+	struct tcs3400_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+	u8 id;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	mutex_init(&data->lock);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
+	if (IS_ERR(data->vdd_supply))
+		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
+				     "Unable to get VDD regulator\n");
+
+	ret = tcs3400_power_on(data);
+	if (ret)
+		goto err_power_off;
+
+	ret = i2c_smbus_read_byte_data(client, TCS3400_CMD_REG(TCS3400_ID));
+	if (ret < 0) {
+		ret = -ENODEV;
+		goto err_power_off;
+		return ret;
+	}
+
+	id = ret;
+	if (id == 0x90)
+		dev_info(&client->dev, "TCS3401/5 Chip ID: 0x%02x\n", id);
+	if (id == 0x93)
+		dev_info(&client->dev, "TCS3403/7 Chip ID: 0x%02x\n", id);
+	else {
+		dev_err(&client->dev, "Unknown chip ID: 0x%02x\n", id);
+		ret = -ENODEV;
+	}
+
+	data->atime = 0xF6; /* ~27.8 ms integration */
+	data->gain = TCS3400_GAIN_1X;
+	data->channel_mode = 0x00;
+
+	tcs3400_write_reg(data, TCS3400_ATIME, data->atime);
+	tcs3400_write_reg(data, TCS3400_CONTROL, data->gain);
+	tcs3400_write_reg(data, TCS3400_PERSIST, 0x00); /* interrupt every cycle */
+	tcs3400_write_reg(data, TCS3400_CHSEL, data->channel_mode); /* RGBC mode */
+
+	tcs3400_write_reg(data, TCS3400_ENABLE,
+			  TCS3400_EN_PON | TCS3400_EN_AEN | TCS3400_EN_AIEN);
+
+	indio_dev->name = TCS3400_DRV_NAME;
+	indio_dev->channels = tcs3400_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tcs3400_channels);
+	indio_dev->info = &tcs3400_info;
+	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
+
+	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
+					      NULL,
+					      tcs3400_trigger_handler,
+					      NULL);
+	if (ret)
+		goto err_power_off;
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, tcs3400_irq_handler,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						"tcs3400_irq", indio_dev);
+		if (ret)
+			goto err_power_off;
+	}
+
+	ret = devm_iio_device_register(&client->dev, indio_dev);
+	if (ret)
+		goto err_power_off;
+	return 0;
+err_power_off:
+	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
+	tcs3400_power_off(data);
+	return ret;
+}
+
+static void tcs3400_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct tcs3400_data *data = iio_priv(indio_dev);
+
+	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
+	tcs3400_power_off(data);
+}
+
+static const struct of_device_id tcs3400_of_match[] = {
+	{ .compatible = "ams,tcs3400" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, tcs3400_of_match);
+
+static const struct i2c_device_id tcs3400_id[] = {
+	{ "tcs3400", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, tcs3400_id);
+
+static struct i2c_driver tcs3400_driver = {
+	.driver = {
+		.name = TCS3400_DRV_NAME,
+		.of_match_table = tcs3400_of_match,
+	},
+	.probe = tcs3400_probe,
+	.remove = tcs3400_remove,
+	.id_table = tcs3400_id,
+};
+
+module_i2c_driver(tcs3400_driver);
+MODULE_AUTHOR("Petr Hodina <petr.hodina@protonmail.com>");
+MODULE_DESCRIPTION("AMS TCS3400 RGB/IR color sensor IIO driver");
+MODULE_LICENSE("GPL");

-- 
2.52.0

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Andy Shevchenko]

On Mon, Jan 19, 2026 at 06:19:07PM +0100, Petr Hodina via B4 Relay wrote:

> Add support for the AMS TCS3400 I2C color light-to-digital converter.
> The driver supports RGBC and RGB-IR modes, programmable integration
> time, optional interrupt-driven buffered capture, and regulator-based
> power control.

...

> +	tristate "AMS TCS3400 color light-to-digital converter"
> +	depends on I2C

> +	default n

This is already default 'default', remove.

> +	help
> +	  If you say yes here you get support for the AMS TCS3400.
> +	  This sensor can detect ambient light and color (RGB) values.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called tcs3400.

...

> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TCS3400 - AMS/TAOS color light sensor with RGBC and RGB-IR channels
> + *
> + * Copyright (c) 2025 Petr Hodina

> + *

Stray blank line.

> + */

...

> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/pm_runtime.h>

Keep this ordered alphabetically and followed IWYU principle. Many are missing.

+ Blank line.

> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>

Also keep this group ordered.

...

> + +struct tcs3400_data {

Have you run `pahole`?

> +	struct i2c_client *client;


> +	struct mutex lock;
> +	struct regulator *vdd_supply;
> +	u8 atime;
> +	u8 gain;
> +	u8 channel_mode; /* 0x00 or 0x80 */
> +	u16 clear_ir; /* clear when mode=0x00, IR when mode=0x80 */
> +	u16 red;
> +	u16 green;
> +	u16 blue;
> +};

...

> +static const int tcs3400_gains[] = {1, 4, 16, 64};

Put inner spaces into {}.

...

> +static int tcs3400_power_on(struct tcs3400_data *data)
> +{
> +	int ret;
> +
> +	ret = regulator_enable(data->vdd_supply);
> +	if (ret)
> +		return ret;

> +	msleep(20);

So-o long sleeps must be commented. Preferably with a reference
to the respective datasheet section / table / etc.

> +	return 0;
> +}

...

> +static int tcs3400_write_reg(struct tcs3400_data *data, u8 reg, u8 val)
> +{
> +	return i2c_smbus_write_byte_data(data->client, TCS3400_CMD_REG(reg), val);
> +}

Why not a regmap?

...

> +static int tcs3400_read_reg(struct tcs3400_data *data, u8 reg, u8 *val)
> +{

> +	int ret = i2c_smbus_read_byte_data(data->client, TCS3400_CMD_REG(reg));
> +
> +	if (ret < 0)
> +		return ret;

This is unmaintainable way of putting things. Better is to split value
definition and assignment.

	int ret;

	ret = ...
	if (ret ...)
		...

> +	*val = ret;
> +
> +	return 0;
> +}

...

> +static int tcs3400_read_word(struct tcs3400_data *data, u8 reg, u16 *val)
> +{
> +
> +	__le16 buf;
> +	int ret = i2c_smbus_read_i2c_block_data(data->client,
> +						TCS3400_CMD_REG(reg), 2, (u8 *)&buf);
> +	if (ret < 0)
> +		return ret;
> +	*val = le16_to_cpu(buf);
> +	return 0;

A lot of missing blank lines.

> +}
> +static int tcs3400_clear_interrupt(struct tcs3400_data *data)
> +{

> +

And here redundant blank line.

> +	return i2c_smbus_write_byte(data->client, TCS3400_CMD_ALS_INT_CLR);
> +}

...

> +static int tcs3400_read_channels(struct tcs3400_data *data)
> +{
> +
> +	int ret, retries = 20;
> +	u8 status;
> +
> +	do {
> +		ret = tcs3400_read_reg(data, TCS3400_STATUS, &status);
> +		if (ret)
> +			return ret;
> +		if (status & TCS3400_STATUS_AVALID)
> +			break;
> +		usleep_range(5000, 6000);
> +	} while (--retries);
> +	if (!retries) {
> +		dev_warn(&data->client->dev, "Timeout waiting for valid data\n");
> +		return -ETIMEDOUT;
> +	}

This is reinvention of something from iopoll.h.

> +	ret = tcs3400_read_word(data, TCS3400_CDATAL, &data->clear_ir);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_RDATAL, &data->red);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_GDATAL, &data->green);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_BDATAL, &data->blue);
> +	if (ret)
> +		return ret;
> +	return 0;
> +}
> +
> +static irqreturn_t tcs3400_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	u16 buf[4];
> +	int ret;

> +	mutex_lock(&data->lock);

Use scoped_guard() from cleanup.h...

> +	ret = tcs3400_read_channels(data);
> +	if (!ret) {

...and usual pattern instead.

> +		buf[0] = data->clear_ir;
> +		buf[1] = data->red;
> +		buf[2] = data->green;
> +		buf[3] = data->blue;
> +		iio_push_to_buffers_with_timestamp(indio_dev, buf,
> +						   iio_get_time_ns(indio_dev));
> +	}
> +	mutex_unlock(&data->lock);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}

I stopped here, please, read other drivers that came into IIO subsystem lately
(last couple of Linux kernel release cycles) for the style that we want to see
in the drivers here.

...

> +	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
> +	if (IS_ERR(data->vdd_supply))
> +		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),

Use

	struct device *dev = &client->dev;

at the top of the function and reduce the verbosity in all the probe code.

> +				     "Unable to get VDD regulator\n");

...

> +err_power_off:
> +	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
> +	tcs3400_power_off(data);
> +	return ret;

Wrong. This messes up with releasing ordering. How did you test this?

...

> +static void tcs3400_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +
> +	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
> +	tcs3400_power_off(data);

Properly wrapped into devm this entire function will gone.

> +}

...

> +static const struct of_device_id tcs3400_of_match[] = {
> +	{ .compatible = "ams,tcs3400" },
> +	{ }
> +};

> +

Drop this blank line.

> +MODULE_DEVICE_TABLE(of, tcs3400_of_match);
> +
> +static const struct i2c_device_id tcs3400_id[] = {
> +	{ "tcs3400", 0 },

Drop ', 0' part

> +	{ }
> +};
> +

Drop blank line.

> +MODULE_DEVICE_TABLE(i2c, tcs3400_id);
> +
> +static struct i2c_driver tcs3400_driver = {
> +	.driver = {
> +		.name = TCS3400_DRV_NAME,
> +		.of_match_table = tcs3400_of_match,
> +	},
> +	.probe = tcs3400_probe,
> +	.remove = tcs3400_remove,
> +	.id_table = tcs3400_id,
> +};

> +

Misplaced blank line...

> +module_i2c_driver(tcs3400_driver);

...should be here.

> +MODULE_AUTHOR("Petr Hodina <petr.hodina@protonmail.com>");
> +MODULE_DESCRIPTION("AMS TCS3400 RGB/IR color sensor IIO driver");
> +MODULE_LICENSE("GPL");

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[kernel test robot]

Hi Petr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b]

url:    https://github.com/intel-lab-lkp/linux/commits/Petr-Hodina-via-B4-Relay/doc-add-Device-Tree-binding-for-AMS-TCS3400-light-sensor/20260120-012240
base:   46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b
patch link:    https://lore.kernel.org/r/20260119-tsc3400-v1-2-82a65c5417aa%40protonmail.com
patch subject: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20260120/202601201721.SsgdtZmN-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601201721.SsgdtZmN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601201721.SsgdtZmN-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/light/tcs3400.c:82:18: warning: 'tcs3400_gains' defined but not used [-Wunused-const-variable=]
      82 | static const int tcs3400_gains[] = {1, 4, 16, 64};
         |                  ^~~~~~~~~~~~~


vim +/tcs3400_gains +82 drivers/iio/light/tcs3400.c

    81	
  > 82	static const int tcs3400_gains[] = {1, 4, 16, 64};
    83	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Konrad Dybcio]

On 1/19/26 6:19 PM, Petr Hodina via B4 Relay wrote:
> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Add support for the AMS TCS3400 I2C color light-to-digital converter.
> The driver supports RGBC and RGB-IR modes, programmable integration
> time, optional interrupt-driven buffered capture, and regulator-based
> power control.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---

FYI this is the driver that shipped on Sony phones:

https://github.com/LineageOS/android_kernel_sony_sdm845/blob/lineage-23.0/drivers/misc/tcs3490.c

And it seems there's a datasheet available:

https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/897/TCS3490.pdf

Konrad

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Andy Shevchenko]

On Tue, Jan 20, 2026 at 12:03:26PM +0100, Konrad Dybcio wrote:
> On 1/19/26 6:19 PM, Petr Hodina via B4 Relay wrote:

> > Add support for the AMS TCS3400 I2C color light-to-digital converter.
> > The driver supports RGBC and RGB-IR modes, programmable integration
> > time, optional interrupt-driven buffered capture, and regulator-based
> > power control.
> > 
> > Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> > ---
> 
> FYI this is the driver that shipped on Sony phones:
> 
> https://github.com/LineageOS/android_kernel_sony_sdm845/blob/lineage-23.0/drivers/misc/tcs3490.c
> 
> And it seems there's a datasheet available:
> 
> https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/897/TCS3490.pdf

Thanks for finding this! It can be transformed to Datasheet tag
in the commit message.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[phodina]

Thanks Konrad for posting the datasheet.

I used directly the link from the vendor site.

https://look.ams-osram.com/m/595d46c644740603/original/TCS3400-Color-Light-to-Digital-Converter.pdf

Will add the link to the driver commit.

Kind regards
Petr Hodina


On Tuesday, January 20th, 2026 at 2:33 PM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Jan 20, 2026 at 12:03:26PM +0100, Konrad Dybcio wrote:
> 
> > On 1/19/26 6:19 PM, Petr Hodina via B4 Relay wrote:
> 
> > > Add support for the AMS TCS3400 I2C color light-to-digital converter.
> > > The driver supports RGBC and RGB-IR modes, programmable integration
> > > time, optional interrupt-driven buffered capture, and regulator-based
> > > power control.
> > > 
> > > Signed-off-by: Petr Hodina petr.hodina@protonmail.com
> > > ---
> > 
> > FYI this is the driver that shipped on Sony phones:
> > 
> > https://github.com/LineageOS/android_kernel_sony_sdm845/blob/lineage-23.0/drivers/misc/tcs3490.c
> > 
> > And it seems there's a datasheet available:
> > 
> > https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/897/TCS3490.pdf
> 
> 
> Thanks for finding this! It can be transformed to Datasheet tag
> in the commit message.
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Konrad Dybcio]

On 1/20/26 2:51 PM, phodina wrote:
> Thanks Konrad for posting the datasheet.
> 
> I used directly the link from the vendor site.
> 
> https://look.ams-osram.com/m/595d46c644740603/original/TCS3400-Color-Light-to-Digital-Converter.pdf

Note these seem to be two separate (but similar) products

Konrad

> 
> Will add the link to the driver commit.
> 
> Kind regards
> Petr Hodina
> 
> 
> On Tuesday, January 20th, 2026 at 2:33 PM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
>> On Tue, Jan 20, 2026 at 12:03:26PM +0100, Konrad Dybcio wrote:
>>
>>> On 1/19/26 6:19 PM, Petr Hodina via B4 Relay wrote:
>>
>>>> Add support for the AMS TCS3400 I2C color light-to-digital converter.
>>>> The driver supports RGBC and RGB-IR modes, programmable integration
>>>> time, optional interrupt-driven buffered capture, and regulator-based
>>>> power control.
>>>>
>>>> Signed-off-by: Petr Hodina petr.hodina@protonmail.com
>>>> ---
>>>
>>> FYI this is the driver that shipped on Sony phones:
>>>
>>> https://github.com/LineageOS/android_kernel_sony_sdm845/blob/lineage-23.0/drivers/misc/tcs3490.c
>>>
>>> And it seems there's a datasheet available:
>>>
>>> https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/897/TCS3490.pdf
>>
>>
>> Thanks for finding this! It can be transformed to Datasheet tag
>> in the commit message.
>>
>> --
>> With Best Regards,
>> Andy Shevchenko

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[kernel test robot]

Hi Petr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b]

url:    https://github.com/intel-lab-lkp/linux/commits/Petr-Hodina-via-B4-Relay/doc-add-Device-Tree-binding-for-AMS-TCS3400-light-sensor/20260120-012240
base:   46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b
patch link:    https://lore.kernel.org/r/20260119-tsc3400-v1-2-82a65c5417aa%40protonmail.com
patch subject: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20260120/202601201908.GaxFkE6t-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601201908.GaxFkE6t-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601201908.GaxFkE6t-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/light/tcs3400.c:82:18: warning: unused variable 'tcs3400_gains' [-Wunused-const-variable]
      82 | static const int tcs3400_gains[] = {1, 4, 16, 64};
         |                  ^~~~~~~~~~~~~
   1 warning generated.


vim +/tcs3400_gains +82 drivers/iio/light/tcs3400.c

    81	
  > 82	static const int tcs3400_gains[] = {1, 4, 16, 64};
    83	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver

[Jonathan Cameron]

On Mon, 19 Jan 2026 18:19:07 +0100
Petr Hodina via B4 Relay <devnull+petr.hodina.protonmail.com@kernel.org> wrote:

> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Add support for the AMS TCS3400 I2C color light-to-digital converter.
> The driver supports RGBC and RGB-IR modes, programmable integration
> time, optional interrupt-driven buffered capture, and regulator-based
> power control.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
Hi Petr,

Seems you have a lot of reviews already.  I'll take a quick look
as well but may well overlap with others.

Thanks,

Jonathan

>  M:	Mudit Sharma <muditsharma.info@gmail.com>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index ac1408d374c9..73419d80e3a7 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -580,6 +580,17 @@ config ST_UVIS25_SPI
>  	depends on ST_UVIS25
>  	select REGMAP_SPI
>  
> +config TCS3400
> +	tristate "AMS TCS3400 color light-to-digital converter"
> +	depends on I2C
> +	default n
Drop that as it's the default with nothing said.
> +	help
> +	  If you say yes here you get support for the AMS TCS3400.
> +	  This sensor can detect ambient light and color (RGB) values.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called tcs3400.

> diff --git a/drivers/iio/light/tcs3400.c b/drivers/iio/light/tcs3400.c
> new file mode 100644
> index 000000000000..22c8c4e803cf
> --- /dev/null
> +++ b/drivers/iio/light/tcs3400.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TCS3400 - AMS/TAOS color light sensor with RGBC and RGB-IR channels
> + *
> + * Copyright (c) 2025 Petr Hodina
> + *
This trailing blank line adds little other than more scrolling. Drop it.
> + */
> +
> +#include <linux/module.h>
Alphabetical order and sanity check that you are following the
include what you use philosophy for includes (see threads
on list currently about that).

> +#include <linux/i2c.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
See below for comments on this.
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define TCS3400_DRV_NAME "tcs3400"
> +#define TCS3400_CMD_REG(reg) (0x80 | (reg))
> +#define TCS3400_CMD_SPECIAL 0xE0
> +#define TCS3400_CMD_ALS_INT_CLR 0xE6
> +#define TCS3400_CMD_ALL_INT_CLR 0xE7
> +#define TCS3400_ENABLE 0x00

I'd add _REG postfix to the register addresses. Makes it
easier to tell what is address and what is field..

> +#define TCS3400_ATIME 0x01
> +#define TCS3400_WTIME 0x03
> +#define TCS3400_PERSIST 0x0C
> +#define TCS3400_CONTROL 0x0F /* Gain */
> +#define TCS3400_STATUS 0x13
> +#define TCS3400_CDATAL 0x14 /* Clear low */
> +#define TCS3400_RDATAL 0x16
> +#define TCS3400_GDATAL 0x18
> +#define TCS3400_BDATAL 0x1A
> +#define TCS3400_ID 0x12
> +#define TCS3400_CHSEL 0xC0 /* Access IR channel: 0x00 RGBC, 0x80 RGB-IR */
> +#define TCS3400_EN_PON BIT(0)
Fine a way to clearly associate fields with register.
Normally repeat the prefix. So this is maybe
TCS3400_ENABLE_PON

Put the fields with each register address define and it is
worth looking at some of the indentation tweaks we often
do to make it easier to read.


> +#define TCS3400_EN_AEN BIT(1)
> +#define TCS3400_EN_AIEN BIT(4)
> +#define TCS3400_STATUS_AVALID BIT(0)
> +#define TCS3400_STATUS_AINT BIT(4)
> +#define TCS3400_GAIN_1X 0x00
> +#define TCS3400_GAIN_4X 0x01
> +#define TCS3400_GAIN_16X 0x02
> +#define TCS3400_GAIN_64X 0x03
> +#define TCS3400_MAX_ATIME 256
> +
> +#define TCS3400_IIO_CHANNEL(_index, _mod) { \
> +	.type = IIO_INTENSITY, \
> +	.modified = 1, \
> +	.channel2 = IIO_MOD_##_mod, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_SCALE), \
> +			      BIT(IIO_CHAN_INFO_SCALE) | \
> +			      BIT(IIO_CHAN_INFO_INT_TIME), \
> +	.indexed = 1, \
> +	.channel = _index, \
> +	.scan_index = _index, \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.endianness = IIO_LE, \

Looks wrong with a le16_to_cpu in the read path.
Arguably that shouldn't be in the buffered path, but given it's i2c and
the cost of even talking to the device is high I don't mind if that
simplifies your code.


> +	}, \
> +}
> +
> +struct tcs3400_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	struct regulator *vdd_supply;
> +	u8 atime;
> +	u8 gain;
> +	u8 channel_mode; /* 0x00 or 0x80 */
> +	u16 clear_ir; /* clear when mode=0x00, IR when mode=0x80 */

Sounds like you should control your ir_mode based on what channel is requested
not a separate attribute that changes the meaning of the data.

> +	u16 red;
> +	u16 green;
> +	u16 blue;
> +};
> +
> +static const int tcs3400_gains[] = {1, 4, 16, 64};

 = { 1, 4, 16, 64 };

is the preferred formatting in IIO.

> +
> +static int tcs3400_power_on(struct tcs3400_data *data)
> +{
> +	int ret;
> +
> +	ret = regulator_enable(data->vdd_supply);
> +	if (ret)
> +		return ret;
> +
> +	msleep(20);
> +
> +	return 0;
> +}
> +
> +static void tcs3400_power_off(struct tcs3400_data *data)
> +{
> +	regulator_disable(data->vdd_supply);
After changes suggested below, this will not be needed.
> +}
> +
> +static int tcs3400_write_reg(struct tcs3400_data *data, u8 reg, u8 val)
> +{
> +	return i2c_smbus_write_byte_data(data->client, TCS3400_CMD_REG(reg), val);
> +}
> +
> +static int tcs3400_read_reg(struct tcs3400_data *data, u8 reg, u8 *val)
> +{
> +	int ret = i2c_smbus_read_byte_data(data->client, TCS3400_CMD_REG(reg));
> +
> +	if (ret < 0)
> +		return ret;
> +	*val = ret;
> +
> +	return 0;
> +}
> +
> +static int tcs3400_read_word(struct tcs3400_data *data, u8 reg, u16 *val)
> +{
> +
> +	__le16 buf;
> +	int ret = i2c_smbus_read_i2c_block_data(data->client,
> +						TCS3400_CMD_REG(reg), 2, (u8 *)&buf);

sizeof(buf) rather than 2.

> +	if (ret < 0)
> +		return ret;
> +	*val = le16_to_cpu(buf);
> +	return 0;
> +}
> +static int tcs3400_clear_interrupt(struct tcs3400_data *data)
> +{
> +
> +	return i2c_smbus_write_byte(data->client, TCS3400_CMD_ALS_INT_CLR);
> +}
> +
> +static int tcs3400_read_channels(struct tcs3400_data *data)
> +{
> +
> +	int ret, retries = 20;
> +	u8 status;
> +
> +	do {
> +		ret = tcs3400_read_reg(data, TCS3400_STATUS, &status);
> +		if (ret)
> +			return ret;
> +		if (status & TCS3400_STATUS_AVALID)
> +			break;
> +		usleep_range(5000, 6000);

Need a comment on why this sleep.  Mostly likely fsleep will be better choice.

> +	} while (--retries);
> +	if (!retries) {
> +		dev_warn(&data->client->dev, "Timeout waiting for valid data\n");
> +		return -ETIMEDOUT;
> +	}
> +	ret = tcs3400_read_word(data, TCS3400_CDATAL, &data->clear_ir);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_RDATAL, &data->red);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_GDATAL, &data->green);
> +	if (ret)
> +		return ret;
> +
> +	ret = tcs3400_read_word(data, TCS3400_BDATAL, &data->blue);

Datasheet is really vague on how much of the i2c protocol is supported.
Any idea if bulk reads are an option?

If not I'd suggest for read_raw at least you read only what is actually wanted.
I2C busses are pretty slow! Might even be worth doing that for buffered
reads.


> +	if (ret)
> +		return ret;
> +	return 0;
return tcs...

> +}
> +
> +static irqreturn_t tcs3400_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	u16 buf[4];
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +	ret = tcs3400_read_channels(data);
> +	if (!ret) {		
> +		buf[0] = data->clear_ir;
> +		buf[1] = data->red;
> +		buf[2] = data->green;
> +		buf[3] = data->blue;
> +		iio_push_to_buffers_with_timestamp(indio_dev, buf,
> +						   iio_get_time_ns(indio_dev));

Use iio_push_to_buffers_with_ts() and fix the error you'll see as a result.
This function is deprecated because it makes it too easy to write a bug
like you have done where the timestamp is off the end of the buffer.

> +	}
> +	mutex_unlock(&data->lock);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t tcs3400_irq_handler(int irq, void *priv)
> +{
> +	struct iio_dev *indio_dev = priv;
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +	ret = tcs3400_read_channels(data);
> +	if (!ret)
> +		iio_trigger_poll_nested(indio_dev->trig);

I'm not sure what intent of this is... You seem to be poking a
trigger that wasn't yours. That's a hard no as it could
do absolutely anything.

Superficially it looks like the driver should be registering
a trigger of it's own and at least some of this logic
belongs on that side of the trigger / device driver
boundary.

> +
> +	tcs3400_clear_interrupt(data);
> +	mutex_unlock(&data->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tcs3400_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +	ret = tcs3400_read_channels(data);
> +	if (ret) {
> +		mutex_unlock(&data->lock);
Guard will help here as well. See below.

> +		return ret;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->channel2) {
> +		case IIO_MOD_LIGHT_CLEAR:
> +			*val = data->clear_ir;
> +			break;
> +		case IIO_MOD_LIGHT_RED:
> +			*val = data->red;
> +			break;
> +		case IIO_MOD_LIGHT_GREEN:
> +			*val = data->green;
> +			break;
> +		case IIO_MOD_LIGHT_BLUE:
> +			*val = data->blue;
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			break;
> +		}
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = 0;
> +		*val2 = (TCS3400_MAX_ATIME - data->atime) * 2780000; /* 2.78 ms per cycle */
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	mutex_unlock(&data->lock);

Use a guard() and early returns so the code flow is simpler.

> +
> +	return ret;
> +}

> +
> +static const struct iio_chan_spec tcs3400_channels[] = {
> +	TCS3400_IIO_CHANNEL(0, LIGHT_CLEAR),
> +	TCS3400_IIO_CHANNEL(1, LIGHT_RED),
> +	TCS3400_IIO_CHANNEL(2, LIGHT_GREEN),
> +	TCS3400_IIO_CHANNEL(3, LIGHT_BLUE),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static ssize_t tcs3400_enable_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	u8 enable;
> +	int ret;
> +
> +	ret = tcs3400_read_reg(data, TCS3400_ENABLE, &enable);
> +	if (ret)
> +		return ret;
> +	return sprintf(buf, "%d\n", !!(enable & (TCS3400_EN_PON | TCS3400_EN_AEN)));
> +}
> +
> +static ssize_t tcs3400_enable_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	bool enable;
> +	int ret;
> +	u8 val;
> +
> +	ret = kstrtobool(buf, &enable);
> +	if (ret)
> +		return ret;
> +	mutex_lock(&data->lock);
> +	if (enable)
> +		val = TCS3400_EN_PON | TCS3400_EN_AEN;
> +	else
> +		val = 0;

Not clear why you set up val under the lock.  Keep lock
scope minimal where it doesn't hurt readability.


> +	ret = tcs3400_write_reg(data, TCS3400_ENABLE, val);
> +	mutex_unlock(&data->lock);
> +	if (ret)
> +		return ret;
> +
> +	if (enable)
> +		msleep(20);
> +	return len;
> +}
> +
> +static ssize_t tcs3400_ir_mode_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", !!data->channel_mode);
> +}
> +
> +static ssize_t tcs3400_ir_mode_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +	bool enable;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &enable);
> +	if (ret)
> +		return ret;
> +	mutex_lock(&data->lock);
> +	data->channel_mode = enable ? 0x80 : 0x00;

Looks like it's setting 1 bit. Probably want a define for that field's mask.
Is there anything else in that register?

> +	ret = tcs3400_write_reg(data, TCS3400_CHSEL, data->channel_mode);
> +	mutex_unlock(&data->lock);
> +	return ret ? ret : len;
> +}
> +
> +static IIO_DEVICE_ATTR(enable, 0644, tcs3400_enable_show, tcs3400_enable_store, 0);
> +static IIO_DEVICE_ATTR(ir_mode, 0644, tcs3400_ir_mode_show, tcs3400_ir_mode_store, 0);
> +
> +static struct attribute *tcs3400_attributes[] = {
> +	&iio_dev_attr_enable.dev_attr.attr,
Why?  Generally expect a light sensor to be enabled and disabled automatically 
based on other config. E.g. whether we have asked for data, enabled buffered capture
or turned events on.

> +	&iio_dev_attr_ir_mode.dev_attr.attr,

Looks like custom ABI.  This is hard to get through review, but starting point
is an ABI document in Documentation/ABI/testing/sysfs-bus-iio-*
so we can easily see what the interface is.

> +	NULL
> +};
> +
> +static const struct attribute_group tcs3400_attribute_group = {
> +	.attrs = tcs3400_attributes,
> +};
> +
> +static const struct iio_info tcs3400_info = {
> +	.read_raw = tcs3400_read_raw,
> +	.write_raw = tcs3400_write_raw,
> +	.attrs = &tcs3400_attribute_group,
> +};
> +
> +static int tcs3400_probe(struct i2c_client *client)
> +{
> +	struct tcs3400_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +	u8 id;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +	mutex_init(&data->lock);

	ret = devm_mutex_init() in new code.

> +
> +	i2c_set_clientdata(client, indio_dev);

Check if you still need this after the other changes suggested below.

> +
> +	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
> +	if (IS_ERR(data->vdd_supply))
> +		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
> +				     "Unable to get VDD regulator\n");
> +
> +	ret = tcs3400_power_on(data);

Given you only turn it on at probe and off at remove
devm_regulator_get_enable() is something you should be using.  Then just
code the sleep inline here.

> +	if (ret)
> +		goto err_power_off;
> +
> +	ret = i2c_smbus_read_byte_data(client, TCS3400_CMD_REG(TCS3400_ID));
> +	if (ret < 0) {
> +		ret = -ENODEV;
> +		goto err_power_off;
> +		return ret;
> +	}
> +
> +	id = ret;
> +	if (id == 0x90)
> +		dev_info(&client->dev, "TCS3401/5 Chip ID: 0x%02x\n", id);
> +	if (id == 0x93)
> +		dev_info(&client->dev, "TCS3403/7 Chip ID: 0x%02x\n", id);
> +	else {
> +		dev_err(&client->dev, "Unknown chip ID: 0x%02x\n", id);
> +		ret = -ENODEV;

you set ret then do nothing with it.  However, you should only dev_info
on a mismatch anyway as failing probe on an ID miss match breaks the
dt concept of fallback compatibles.   If the ID is miss matched, follow what
the firmware told you it was.

> +	}
> +
> +	data->atime = 0xF6; /* ~27.8 ms integration */
> +	data->gain = TCS3400_GAIN_1X;
> +	data->channel_mode = 0x00;
> +
> +	tcs3400_write_reg(data, TCS3400_ATIME, data->atime);

These look likely to need error checks.

> +	tcs3400_write_reg(data, TCS3400_CONTROL, data->gain);
> +	tcs3400_write_reg(data, TCS3400_PERSIST, 0x00); /* interrupt every cycle */

This seems like you have a data ready interrupt. Look at how we register
triggers for those in other drivers.

> +	tcs3400_write_reg(data, TCS3400_CHSEL, data->channel_mode); /* RGBC mode */
> +
> +	tcs3400_write_reg(data, TCS3400_ENABLE,
> +			  TCS3400_EN_PON | TCS3400_EN_AEN | TCS3400_EN_AIEN);
> +
> +	indio_dev->name = TCS3400_DRV_NAME;

Prefer it hard coded string inline here.  There is no particular reason
it should match the driver name, particularly as you are adding multiple
part support. It should match what you think the part is.

> +	indio_dev->channels = tcs3400_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tcs3400_channels);
> +	indio_dev->info = &tcs3400_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;

Look for whether other drivers do this (and more importantly why they don't).

> +
> +	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> +					      NULL,
> +					      tcs3400_trigger_handler,
> +					      NULL);
> +	if (ret)
> +		goto err_power_off;
> +	if (client->irq > 0) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, tcs3400_irq_handler,
> +						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,

Firmware is responsible for direction of irq in modern drivers.
We have some historical ones we can't risk 'fixing' this in. So should 
be just IRQF_ONESHOT.


> +						"tcs3400_irq", indio_dev);
> +		if (ret)
> +			goto err_power_off;
> +	}
> +
> +	ret = devm_iio_device_register(&client->dev, indio_dev);
> +	if (ret)
> +		goto err_power_off;
> +	return 0;
> +err_power_off:
> +	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
> +	tcs3400_power_off(data);
> +	return ret;
> +}
> +
> +static void tcs3400_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct tcs3400_data *data = iio_priv(indio_dev);
> +
> +	tcs3400_write_reg(data, TCS3400_ENABLE, 0);
> +	tcs3400_power_off(data);

Look at how and when devm managed cleanup happens.  This is turning the
power off before removing the userspace interfaces, never
a recipe for success! :)

Hint, see how we use devm_add_action_or_reset for this sort of driver
specific cleanup -> that ensures correct sequencing and allows
you to pair turn off with turn on.


> +}

[PATCH 3/3] sdm845: tama: Add AMS TCS3400 ambient light sensor

[Petr Hodina via B4 Relay]

From: Petr Hodina <petr.hodina@protonmail.com>

Add device tree node for TCS3400 ambient light sensor with
power supplies, interrupt on GPIO11, and pinctrl states.

Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
---
 .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     | 36 +++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
index 7dc9349eedfd..15d56d6831c5 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
@@ -485,7 +485,23 @@ &i2c14 {
 	clock-frequency = <400000>;
 
 	/* SONY ToF sensor @ 52 */
-	/* AMS TCS3490 RGB+IR color sensor @ 72 */
+
+	tcs3400_sensor: tcs3400_sensor@39 {
+		compatible = "ams,tcs3400";
+		reg = <0x39>;
+
+		interrupts-extended = <&tlmm 11 IRQ_TYPE_EDGE_FALLING>;
+
+		vdd-supply = <&vreg_l22a_2p8>;
+		vio-supply = <&cam_vio_vreg>;
+
+		pinctrl-0 = <&ams_sensor_default>;
+		pinctrl-1 = <&ams_sensor_sleep>;
+
+		ams,rgbcir-vdd-supply = <1>;
+		ams,rgbcir-gpio-vdd = <0>;
+		ams,rgbcir-vio-supply = <1>;
+	};
 };
 
 &ibb {
@@ -751,6 +767,24 @@ int-pins {
 			bias-pull-down;
 		};
 	};
+
+	ams_sensor_default: ams-sensor-default-state {
+		int-pins {
+			pins = "gpio11";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
+	ams_sensor_sleep: ams_sensor-sleep-state {
+		int-pins {
+			pins = "gpio11";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+	};
 };
 
 &uart6 {

-- 
2.52.0

Re: [PATCH 3/3] sdm845: tama: Add AMS TCS3400 ambient light sensor

[David Heidelberg]

On 19/01/2026 18:19, Petr Hodina via B4 Relay wrote:
> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Add device tree node for TCS3400 ambient light sensor with
> power supplies, interrupt on GPIO11, and pinctrl states.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>   .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     | 36 +++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> index 7dc9349eedfd..15d56d6831c5 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> @@ -485,7 +485,23 @@ &i2c14 {
>   	clock-frequency = <400000>;
>   
>   	/* SONY ToF sensor @ 52 */
> -	/* AMS TCS3490 RGB+IR color sensor @ 72 */
> +

Nice! Few comments:

You replacing sensor comment at 0x72 with one 0x39, could be there 
different sensor model with different address? e.g. tcs3400 vs tcs3490?

Also the node should go above ToF sensor as @39 < @52.

> +	tcs3400_sensor: tcs3400_sensor@39 {
> +		compatible = "ams,tcs3400";
> +		reg = <0x39>;
> +
> +		interrupts-extended = <&tlmm 11 IRQ_TYPE_EDGE_FALLING>;
> +
> +		vdd-supply = <&vreg_l22a_2p8>;
> +		vio-supply = <&cam_vio_vreg>;
> +
> +		pinctrl-0 = <&ams_sensor_default>;
> +		pinctrl-1 = <&ams_sensor_sleep>;

pinctrl-names missing

> +
> +		ams,rgbcir-vdd-supply = <1>;
> +		ams,rgbcir-gpio-vdd = <0>;
> +		ams,rgbcir-vio-supply = <1>;
> +	};
>   };
>   
>   &ibb {
> @@ -751,6 +767,24 @@ int-pins {
>   			bias-pull-down;
>   		};
>   	};
> +
> +	ams_sensor_default: ams-sensor-default-state {
> +		int-pins {
> +			pins = "gpio11";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	ams_sensor_sleep: ams_sensor-sleep-state {
> +		int-pins {
> +			pins = "gpio11";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +	};
>   };
>   
>   &uart6 {
> 

-- 
David Heidelberg

Re: [PATCH 3/3] sdm845: tama: Add AMS TCS3400 ambient light sensor

[Konrad Dybcio]

On 1/19/26 6:19 PM, Petr Hodina via B4 Relay wrote:
> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Add device tree node for TCS3400 ambient light sensor with
> power supplies, interrupt on GPIO11, and pinctrl states.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>  .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     | 36 +++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> index 7dc9349eedfd..15d56d6831c5 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> @@ -485,7 +485,23 @@ &i2c14 {
>  	clock-frequency = <400000>;
>  
>  	/* SONY ToF sensor @ 52 */
> -	/* AMS TCS3490 RGB+IR color sensor @ 72 */
> +
> +	tcs3400_sensor: tcs3400_sensor@39 {

node names (between ':' and '@') must not contain underscores,
and should be generic where possible, let's call it sensor@,
rgb-ir@ or perhaps colorimeter@ (Krzysztof, opinions?)

> +		compatible = "ams,tcs3400";
> +		reg = <0x39>;

I can't find supporting evidence for this reg change

> +
> +		interrupts-extended = <&tlmm 11 IRQ_TYPE_EDGE_FALLING>;
> +
> +		vdd-supply = <&vreg_l22a_2p8>;
> +		vio-supply = <&cam_vio_vreg>;
> +
> +		pinctrl-0 = <&ams_sensor_default>;
> +		pinctrl-1 = <&ams_sensor_sleep>;
> +
> +		ams,rgbcir-vdd-supply = <1>;
> +		ams,rgbcir-gpio-vdd = <0>;
> +		ams,rgbcir-vio-supply = <1>;
> +	};
>  };
>  
>  &ibb {
> @@ -751,6 +767,24 @@ int-pins {
>  			bias-pull-down;
>  		};
>  	};
> +
> +	ams_sensor_default: ams-sensor-default-state {
> +		int-pins {

Drop this inner level, define the properties directly under the
-state node, both cases

Konrad

Re: [PATCH 0/3] iio: light: add AMS TCS3400 driver

[Rob Herring]

On Mon, 19 Jan 2026 18:19:05 +0100, Petr Hodina wrote:
> Hi,
> 
> This patch adds an IIO driver for the AMS TCS3400 color light-to-digital
> converter.
> 
> The TCS3400 is an I2C-connected RGB color sensor supporting RGBC and
> RGB-IR measurement modes, programmable integration time, selectable
> gain, optional interrupt-driven sampling, and regulator-based power
> control.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
> Petr Hodina (3):
>       doc: add Device Tree binding for AMS TCS3400 light sensor
>       iio: light: add AMS TCS3400 RGB and RGB-IR color sensor driver
>       sdm845: tama: Add AMS TCS3400 ambient light sensor
> 
>  .../devicetree/bindings/iio/light/ams,tcs3400.yaml |  54 +++
>  MAINTAINERS                                        |   7 +
>  .../boot/dts/qcom/sdm845-sony-xperia-tama.dtsi     |  36 +-
>  drivers/iio/light/Kconfig                          |  11 +
>  drivers/iio/light/Makefile                         |   1 +
>  drivers/iio/light/tcs3400.c                        | 505 +++++++++++++++++++++
>  6 files changed, 613 insertions(+), 1 deletion(-)
> ---
> base-commit: 46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b
> change-id: 20260119-tsc3400-68a91d8c1355
> 
> Best regards,
> --
> Petr Hodina <petr.hodina@protonmail.com>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: 46fe65a2c28ecf5df1a7475aba1f08ccf4c0ac1b (use --merge-base to override)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)


Warnings in base: 229
Warnings after series: 235

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/qcom/' for 20260119-tsc3400-v1-0-82a65c5417aa@protonmail.com:

arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akari.dtb: tcs3400_sensor@39 (ams,tcs3400): 'ams,rgbcir-gpio-vdd', 'ams,rgbcir-vdd-supply', 'ams,rgbcir-vio-supply', 'vio-supply' do not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-apollo.dtb: tcs3400_sensor@39 (ams,tcs3400): 'ams,rgbcir-gpio-vdd', 'ams,rgbcir-vdd-supply', 'ams,rgbcir-vio-supply', 'vio-supply' do not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akatsuki.dtb: tcs3400_sensor@39 (ams,tcs3400): 'ams,rgbcir-gpio-vdd', 'ams,rgbcir-vdd-supply', 'ams,rgbcir-vio-supply', 'vio-supply' do not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/iio/light/ams,tcs3400.yaml