All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: light: Add Capella CM3217 ambient light sensor
@ 2026-11-01 14:46 ` Patrick Fischer via B4 Relay
  0 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

The Capella CM3217 is a small ambient light sensor without registers.
NVIDIA used it in the Tegra Note 7 and the Shield Tablet. Mainline has
no driver for it: cm32181 and cm3232 expect registers and an ID, the
CM3217 has neither.

Patch 1 adds the binding, patch 2 the driver with raw value, scale and
integration time. The datasheet is not public, so the bit fields come
from NVIDIA's downstream drivers and the scale from Capella's published
detection range. Patch 2 explains both.

Tested on a Tegra Note 7 (P1640) with v7.3 and the driver built as a
module, the device created through new_device. In room light all 32
combinations of integration time and scale gave the same illuminance
within the count resolution (6.0 to 7.3 lux), and the first read after
each change matched the second one. The raw counts scale linearly with
both fields. Svyatoslav Ryhel also tried it on the P1988 variant of the
Tegra Note 7.

Probing through the device tree works with the real supply (vdd_dev,
Palmas LDO6), and unloading and reloading the module twice brought the
device back each time. Next to a phone light sensor the readings were
about 7 to 20 times lower. NVIDIA's Android HAL for this tablet
multiplies the count by 2.1505 at the same 2 T/100 ms setting, which is
7.2 times the scale here, so most of that is the dark cover window. With
that factor the tablet was within 15 % of the phone in bright light but
a factor of three lower in dim room light; I could not tell whether that
was my setup.

The device tree node for the Tegra Note 7 will follow with Svyatoslav's
tegratab series.

Tool use (see Documentation/process/generated-content.rst): this series
was prepared with an LLM coding assistant (Claude Code). It read the
NVIDIA downstream drivers and Capella's product data, wrote the driver,
the binding and the texts from my instructions, and ran the build and
binding checks. I reviewed the code and ran the tests on the device.

---
Patrick Fischer (2):
      dt-bindings: iio: light: Add Capella CM3217
      iio: light: Add Capella CM3217 ambient light sensor driver

 .../bindings/iio/light/capella,cm3217.yaml         |  45 +++
 drivers/iio/light/Kconfig                          |  10 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/cm3217.c                         | 383 +++++++++++++++++++++
 4 files changed, 439 insertions(+)
---
base-commit: 137ca6e5de3bd4e1056877d0b82214e82143efa7
change-id: 20261101-upstream-iio-cm3217-e3f14c4760e5

Best regards,
--  
Patrick Fischer <admin@tsv-stahl-riesa.de>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/2] iio: light: Add Capella CM3217 ambient light sensor
@ 2026-11-01 14:46 ` Patrick Fischer via B4 Relay
  0 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer via B4 Relay @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

The Capella CM3217 is a small ambient light sensor without registers.
NVIDIA used it in the Tegra Note 7 and the Shield Tablet. Mainline has
no driver for it: cm32181 and cm3232 expect registers and an ID, the
CM3217 has neither.

Patch 1 adds the binding, patch 2 the driver with raw value, scale and
integration time. The datasheet is not public, so the bit fields come
from NVIDIA's downstream drivers and the scale from Capella's published
detection range. Patch 2 explains both.

Tested on a Tegra Note 7 (P1640) with v7.3 and the driver built as a
module, the device created through new_device. In room light all 32
combinations of integration time and scale gave the same illuminance
within the count resolution (6.0 to 7.3 lux), and the first read after
each change matched the second one. The raw counts scale linearly with
both fields. Svyatoslav Ryhel also tried it on the P1988 variant of the
Tegra Note 7.

Probing through the device tree works with the real supply (vdd_dev,
Palmas LDO6), and unloading and reloading the module twice brought the
device back each time. Next to a phone light sensor the readings were
about 7 to 20 times lower. NVIDIA's Android HAL for this tablet
multiplies the count by 2.1505 at the same 2 T/100 ms setting, which is
7.2 times the scale here, so most of that is the dark cover window. With
that factor the tablet was within 15 % of the phone in bright light but
a factor of three lower in dim room light; I could not tell whether that
was my setup.

The device tree node for the Tegra Note 7 will follow with Svyatoslav's
tegratab series.

Tool use (see Documentation/process/generated-content.rst): this series
was prepared with an LLM coding assistant (Claude Code). It read the
NVIDIA downstream drivers and Capella's product data, wrote the driver,
the binding and the texts from my instructions, and ran the build and
binding checks. I reviewed the code and ran the tests on the device.

---
Patrick Fischer (2):
      dt-bindings: iio: light: Add Capella CM3217
      iio: light: Add Capella CM3217 ambient light sensor driver

 .../bindings/iio/light/capella,cm3217.yaml         |  45 +++
 drivers/iio/light/Kconfig                          |  10 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/cm3217.c                         | 383 +++++++++++++++++++++
 4 files changed, 439 insertions(+)
---
base-commit: 137ca6e5de3bd4e1056877d0b82214e82143efa7
change-id: 20261101-upstream-iio-cm3217-e3f14c4760e5

Best regards,
--  
Patrick Fischer <admin@tsv-stahl-riesa.de>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] dt-bindings: iio: light: Add Capella CM3217
  2026-11-01 14:46 ` Patrick Fischer via B4 Relay
@ 2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  -1 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

The Capella CM3217 is an ambient light sensor with an I2C interface but
without registers and without an interrupt line. It always answers on
the two addresses 0x10 and 0x11. The node describes the first address;
the second one is fixed and claimed by the driver.

The sensor is used in the NVIDIA Tegra Note 7 and the NVIDIA Shield
Tablet.

Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
---
 .../bindings/iio/light/capella,cm3217.yaml         | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
new file mode 100644
index 000000000000..b2ddb14d657b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/capella,cm3217.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Capella CM3217 ambient light sensor
+
+maintainers:
+  - Patrick Fischer <admin@tsv-stahl-riesa.de>
+
+description:
+  The CM3217 is an I2C ambient light sensor without registers or interrupt
+  line. It always occupies the two I2C addresses 0x10 and 0x11; the node
+  describes the first one.
+
+properties:
+  compatible:
+    const: capella,cm3217
+
+  reg:
+    const: 0x10
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@10 {
+            compatible = "capella,cm3217";
+            reg = <0x10>;
+            vdd-supply = <&vdd_2v85_sen>;
+        };
+    };
+...

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/2] dt-bindings: iio: light: Add Capella CM3217
@ 2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  0 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer via B4 Relay @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

From: Patrick Fischer <admin@tsv-stahl-riesa.de>

The Capella CM3217 is an ambient light sensor with an I2C interface but
without registers and without an interrupt line. It always answers on
the two addresses 0x10 and 0x11. The node describes the first address;
the second one is fixed and claimed by the driver.

The sensor is used in the NVIDIA Tegra Note 7 and the NVIDIA Shield
Tablet.

Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
---
 .../bindings/iio/light/capella,cm3217.yaml         | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
new file mode 100644
index 000000000000..b2ddb14d657b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/capella,cm3217.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Capella CM3217 ambient light sensor
+
+maintainers:
+  - Patrick Fischer <admin@tsv-stahl-riesa.de>
+
+description:
+  The CM3217 is an I2C ambient light sensor without registers or interrupt
+  line. It always occupies the two I2C addresses 0x10 and 0x11; the node
+  describes the first one.
+
+properties:
+  compatible:
+    const: capella,cm3217
+
+  reg:
+    const: 0x10
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@10 {
+            compatible = "capella,cm3217";
+            reg = <0x10>;
+            vdd-supply = <&vdd_2v85_sen>;
+        };
+    };
+...

-- 
2.56.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver
  2026-11-01 14:46 ` Patrick Fischer via B4 Relay
@ 2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  -1 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

The CM3217 has no registers and no ID. Writing a byte to address 0x10
or 0x11 sets command byte 1 or 2, reading a byte from 0x10 or 0x11
returns the high or low byte of the 16-bit light count.

Command byte 1 holds the shutdown bit and the IT_T field (1/2 T, 1 T,
2 T or 4 T), command byte 2 the FD_IT field (800 ms down to 66 ms). The
count scales linearly with both. The driver exposes FD_IT as the
integration time and IT_T through the scale, so the scale list offers
four ranges for each integration time.

The datasheet is not public. The bit fields, the reserved bits that are
written as 1 and the startup setting of 2 T and 100 ms come from NVIDIA's
downstream drivers for the Tegra Note 7 (drivers/staging/iio/light/
cm3217.c in the 3.4 and 3.10 kernels). For the resolution, Capella
gives a range of 0.01875 to 119156 lux. The driver takes 0.01875 lux
per count at 4 T and 800 ms. This matches both the upper end (65535
counts at 1/2 T and 66 ms) and the 0.3 lux per count that the NVIDIA
3.4 driver uses at 2 T and 100 ms.

The sensor has no data-ready flag, and a new setting only takes effect
when the conversion in progress ends. A conversion takes IT_T times
FD_IT, up to 3.2 s. Leaving shutdown starts a new one at once, so the
driver passes through shutdown on every change and waits one conversion
period plus a margin before it reads the count. The timing was measured
on a Tegra Note 7.

Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
---
 drivers/iio/light/Kconfig  |  10 ++
 drivers/iio/light/Makefile |   1 +
 drivers/iio/light/cm3217.c | 383 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 394 insertions(+)

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index d0dea4f3113b..451d26e6e97d 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -196,6 +196,16 @@ config BH1780
 	  To compile this driver as a module, choose M here: the module will
 	  be called bh1780.
 
+config CM3217
+	tristate "Capella CM3217 ambient light sensor"
+	depends on I2C
+	help
+	  Say Y here to build support for the Capella Microsystems CM3217
+	  ambient light sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called cm3217.
+
 config CM32181
 	depends on I2C
 	tristate "CM32181 driver"
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 9fc046e636a2..5daff8ef0daa 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_AS7343)		+= as7343.o
 obj-$(CONFIG_BH1745)		+= bh1745.o
 obj-$(CONFIG_BH1750)		+= bh1750.o
 obj-$(CONFIG_BH1780)		+= bh1780.o
+obj-$(CONFIG_CM3217)		+= cm3217.o
 obj-$(CONFIG_CM32181)		+= cm32181.o
 obj-$(CONFIG_CM3232)		+= cm3232.o
 obj-$(CONFIG_CM3323)		+= cm3323.o
diff --git a/drivers/iio/light/cm3217.c b/drivers/iio/light/cm3217.c
new file mode 100644
index 000000000000..9e0e9afee092
--- /dev/null
+++ b/drivers/iio/light/cm3217.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Capella CM3217 ambient light sensor driver
+ *
+ * The CM3217 has no registers. It answers on two fixed I2C addresses:
+ * writing a byte to 0x10 or 0x11 sets command byte 1 or 2, reading a byte
+ * from 0x10 or 0x11 returns the MSB or LSB of the 16-bit light count.
+ *
+ * Copyright (C) 2026 Patrick Fischer <admin@tsv-stahl-riesa.de>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/ktime.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#define CM3217_ADDR_CMD2_LSB	0x11
+
+#define CM3217_CMD1_RSVD	BIT(5)		/* reserved, set to 1 */
+#define CM3217_CMD1_IT_T	GENMASK(3, 2)	/* 1/2 T, 1 T, 2 T, 4 T */
+#define CM3217_CMD1_WDM		BIT(1)		/* set to 1 like the vendor driver */
+#define CM3217_CMD1_SD		BIT(0)		/* shutdown when set */
+
+#define CM3217_CMD2_FD_IT	GENMASK(7, 5)
+
+#define CM3217_IT_T_2T		2
+#define CM3217_FD_IT_100MS	5
+
+/* 0.01875 lux per count at 4 T and 800 ms, the finest resolution */
+#define CM3217_NLUX_MS		15000000000ULL
+
+#define CM3217_MARGIN_MS	10
+
+/* FD_IT settings in milliseconds, indexed by the field value */
+static const unsigned int cm3217_fd_it_ms[] = {
+	800, 400, 266, 200, 130, 100, 80, 66,
+};
+
+static const int cm3217_int_time_avail[][2] = {
+	{ 0, 800000 }, { 0, 400000 }, { 0, 266000 }, { 0, 200000 },
+	{ 0, 130000 }, { 0, 100000 }, { 0, 80000 }, { 0, 66000 },
+};
+
+struct cm3217_data {
+	struct i2c_client *client;
+	struct i2c_client *client_lsb;
+	/* protects the command bytes, the scale table and ready */
+	struct mutex lock;
+	u8 cmd1;
+	u8 cmd2;
+	ktime_t ready;
+	int scale_avail[4][2];
+};
+
+static unsigned int cm3217_conv_ms(struct cm3217_data *data)
+	__must_hold(&data->lock)
+{
+	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+	unsigned int it_t = FIELD_GET(CM3217_CMD1_IT_T, data->cmd1);
+
+	return (cm3217_fd_it_ms[fd_it] << it_t) / 2;
+}
+
+static void cm3217_update_scale_avail(struct cm3217_data *data)
+	__must_hold(&data->lock)
+{
+	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+	unsigned int i;
+	u32 rem;
+	u64 nlux;
+
+	for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
+		nlux = div_u64(CM3217_NLUX_MS << (3 - i), cm3217_fd_it_ms[fd_it]);
+		data->scale_avail[i][0] = div_u64_rem(nlux, NANO, &rem);
+		data->scale_avail[i][1] = rem;
+	}
+}
+
+/*
+ * A new setting only takes effect when the conversion in progress ends,
+ * which can take up to 3.2 s. Leaving shutdown starts a new conversion
+ * right away, so pass through shutdown on every change.
+ */
+static int cm3217_write_cmd(struct cm3217_data *data, u8 cmd1, u8 cmd2)
+	__must_hold(&data->lock)
+{
+	unsigned int conv_ms;
+	int ret;
+
+	ret = i2c_smbus_write_byte(data->client, cmd1 | CM3217_CMD1_SD);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_write_byte(data->client_lsb, cmd2);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_write_byte(data->client, cmd1);
+	if (ret)
+		return ret;
+
+	data->cmd1 = cmd1;
+	data->cmd2 = cmd2;
+	cm3217_update_scale_avail(data);
+
+	/* the first count is ready after one conversion */
+	conv_ms = cm3217_conv_ms(data);
+	data->ready = ktime_add_ms(ktime_get(),
+				   conv_ms + conv_ms / 10 + CM3217_MARGIN_MS);
+
+	return 0;
+}
+
+static int cm3217_read_count(struct cm3217_data *data, int *val)
+	__must_hold(&data->lock)
+{
+	s64 wait_ms;
+	int lsb, msb;
+
+	wait_ms = ktime_ms_delta(data->ready, ktime_get());
+	if (wait_ms > 0)
+		msleep(wait_ms);
+
+	lsb = i2c_smbus_read_byte(data->client_lsb);
+	if (lsb < 0)
+		return lsb;
+
+	msb = i2c_smbus_read_byte(data->client);
+	if (msb < 0)
+		return msb;
+
+	*val = msb << 8 | lsb;
+
+	return 0;
+}
+
+static const struct iio_chan_spec cm3217_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+};
+
+static int cm3217_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+	unsigned int idx;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = cm3217_read_count(data, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		idx = FIELD_GET(CM3217_CMD1_IT_T, data->cmd1);
+		*val = data->scale_avail[idx][0];
+		*val2 = data->scale_avail[idx][1];
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		idx = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+		*val = cm3217_int_time_avail[idx][0];
+		*val2 = cm3217_int_time_avail[idx][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)data->scale_avail;
+		*length = 2 * ARRAY_SIZE(data->scale_avail);
+		*type = IIO_VAL_INT_PLUS_NANO;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_INT_TIME:
+		*vals = (const int *)cm3217_int_time_avail;
+		*length = 2 * ARRAY_SIZE(cm3217_int_time_avail);
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+	unsigned int i;
+	u8 cmd1, cmd2;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
+			if (data->scale_avail[i][0] == val &&
+			    data->scale_avail[i][1] == val2)
+				break;
+		}
+		if (i == ARRAY_SIZE(data->scale_avail))
+			return -EINVAL;
+
+		cmd1 = data->cmd1 & ~CM3217_CMD1_IT_T;
+		cmd1 |= FIELD_PREP(CM3217_CMD1_IT_T, i);
+
+		return cm3217_write_cmd(data, cmd1, data->cmd2);
+	case IIO_CHAN_INFO_INT_TIME:
+		for (i = 0; i < ARRAY_SIZE(cm3217_int_time_avail); i++) {
+			if (cm3217_int_time_avail[i][0] == val &&
+			    cm3217_int_time_avail[i][1] == val2)
+				break;
+		}
+		if (i == ARRAY_SIZE(cm3217_int_time_avail))
+			return -EINVAL;
+
+		cmd2 = FIELD_PREP(CM3217_CMD2_FD_IT, i);
+
+		return cm3217_write_cmd(data, data->cmd1, cmd2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_write_raw_get_fmt(struct iio_dev *indio_dev,
+				    struct iio_chan_spec const *chan,
+				    long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info cm3217_info = {
+	.read_raw = cm3217_read_raw,
+	.read_avail = cm3217_read_avail,
+	.write_raw = cm3217_write_raw,
+	.write_raw_get_fmt = cm3217_write_raw_get_fmt,
+};
+
+static void cm3217_power_off(void *_data)
+{
+	struct cm3217_data *data = _data;
+
+	i2c_smbus_write_byte(data->client, data->cmd1 | CM3217_CMD1_SD);
+}
+
+static int cm3217_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct cm3217_data *data;
+	struct iio_dev *indio_dev;
+	u8 cmd1, cmd2;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable vdd supply\n");
+
+	data->client_lsb = devm_i2c_new_dummy_device(dev, client->adapter,
+						     CM3217_ADDR_CMD2_LSB);
+	if (IS_ERR(data->client_lsb))
+		return dev_err_probe(dev, PTR_ERR(data->client_lsb),
+				     "failed to register second address\n");
+
+	/* Start with the setting of the vendor driver: 2 T, 100 ms */
+	cmd1 = CM3217_CMD1_RSVD | CM3217_CMD1_WDM |
+	       FIELD_PREP(CM3217_CMD1_IT_T, CM3217_IT_T_2T);
+	cmd2 = FIELD_PREP(CM3217_CMD2_FD_IT, CM3217_FD_IT_100MS);
+
+	scoped_guard(mutex, &data->lock)
+		ret = cm3217_write_cmd(data, cmd1, cmd2);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to power on\n");
+
+	ret = devm_add_action_or_reset(dev, cm3217_power_off, data);
+	if (ret)
+		return ret;
+
+	indio_dev->name = "cm3217";
+	indio_dev->info = &cm3217_info;
+	indio_dev->channels = cm3217_channels;
+	indio_dev->num_channels = ARRAY_SIZE(cm3217_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int cm3217_suspend(struct device *dev)
+{
+	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
+
+	guard(mutex)(&data->lock);
+
+	return i2c_smbus_write_byte(data->client, data->cmd1 | CM3217_CMD1_SD);
+}
+
+static int cm3217_resume(struct device *dev)
+{
+	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
+
+	guard(mutex)(&data->lock);
+
+	return cm3217_write_cmd(data, data->cmd1, data->cmd2);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(cm3217_pm_ops, cm3217_suspend, cm3217_resume);
+
+static const struct i2c_device_id cm3217_id[] = {
+	{ .name = "cm3217" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, cm3217_id);
+
+static const struct of_device_id cm3217_of_match[] = {
+	{ .compatible = "capella,cm3217" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cm3217_of_match);
+
+static struct i2c_driver cm3217_driver = {
+	.driver = {
+		.name = "cm3217",
+		.of_match_table = cm3217_of_match,
+		.pm = pm_sleep_ptr(&cm3217_pm_ops),
+	},
+	.probe = cm3217_probe,
+	.id_table = cm3217_id,
+};
+module_i2c_driver(cm3217_driver);
+
+MODULE_AUTHOR("Patrick Fischer <admin@tsv-stahl-riesa.de>");
+MODULE_DESCRIPTION("Capella CM3217 ambient light sensor driver");
+MODULE_LICENSE("GPL");

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver
@ 2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  0 siblings, 0 replies; 10+ messages in thread
From: Patrick Fischer via B4 Relay @ 2026-11-01 14:46 UTC (permalink / raw)
  To: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel, Patrick Fischer

From: Patrick Fischer <admin@tsv-stahl-riesa.de>

The CM3217 has no registers and no ID. Writing a byte to address 0x10
or 0x11 sets command byte 1 or 2, reading a byte from 0x10 or 0x11
returns the high or low byte of the 16-bit light count.

Command byte 1 holds the shutdown bit and the IT_T field (1/2 T, 1 T,
2 T or 4 T), command byte 2 the FD_IT field (800 ms down to 66 ms). The
count scales linearly with both. The driver exposes FD_IT as the
integration time and IT_T through the scale, so the scale list offers
four ranges for each integration time.

The datasheet is not public. The bit fields, the reserved bits that are
written as 1 and the startup setting of 2 T and 100 ms come from NVIDIA's
downstream drivers for the Tegra Note 7 (drivers/staging/iio/light/
cm3217.c in the 3.4 and 3.10 kernels). For the resolution, Capella
gives a range of 0.01875 to 119156 lux. The driver takes 0.01875 lux
per count at 4 T and 800 ms. This matches both the upper end (65535
counts at 1/2 T and 66 ms) and the 0.3 lux per count that the NVIDIA
3.4 driver uses at 2 T and 100 ms.

The sensor has no data-ready flag, and a new setting only takes effect
when the conversion in progress ends. A conversion takes IT_T times
FD_IT, up to 3.2 s. Leaving shutdown starts a new one at once, so the
driver passes through shutdown on every change and waits one conversion
period plus a margin before it reads the count. The timing was measured
on a Tegra Note 7.

Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
---
 drivers/iio/light/Kconfig  |  10 ++
 drivers/iio/light/Makefile |   1 +
 drivers/iio/light/cm3217.c | 383 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 394 insertions(+)

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index d0dea4f3113b..451d26e6e97d 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -196,6 +196,16 @@ config BH1780
 	  To compile this driver as a module, choose M here: the module will
 	  be called bh1780.
 
+config CM3217
+	tristate "Capella CM3217 ambient light sensor"
+	depends on I2C
+	help
+	  Say Y here to build support for the Capella Microsystems CM3217
+	  ambient light sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called cm3217.
+
 config CM32181
 	depends on I2C
 	tristate "CM32181 driver"
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 9fc046e636a2..5daff8ef0daa 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_AS7343)		+= as7343.o
 obj-$(CONFIG_BH1745)		+= bh1745.o
 obj-$(CONFIG_BH1750)		+= bh1750.o
 obj-$(CONFIG_BH1780)		+= bh1780.o
+obj-$(CONFIG_CM3217)		+= cm3217.o
 obj-$(CONFIG_CM32181)		+= cm32181.o
 obj-$(CONFIG_CM3232)		+= cm3232.o
 obj-$(CONFIG_CM3323)		+= cm3323.o
diff --git a/drivers/iio/light/cm3217.c b/drivers/iio/light/cm3217.c
new file mode 100644
index 000000000000..9e0e9afee092
--- /dev/null
+++ b/drivers/iio/light/cm3217.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Capella CM3217 ambient light sensor driver
+ *
+ * The CM3217 has no registers. It answers on two fixed I2C addresses:
+ * writing a byte to 0x10 or 0x11 sets command byte 1 or 2, reading a byte
+ * from 0x10 or 0x11 returns the MSB or LSB of the 16-bit light count.
+ *
+ * Copyright (C) 2026 Patrick Fischer <admin@tsv-stahl-riesa.de>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/ktime.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#define CM3217_ADDR_CMD2_LSB	0x11
+
+#define CM3217_CMD1_RSVD	BIT(5)		/* reserved, set to 1 */
+#define CM3217_CMD1_IT_T	GENMASK(3, 2)	/* 1/2 T, 1 T, 2 T, 4 T */
+#define CM3217_CMD1_WDM		BIT(1)		/* set to 1 like the vendor driver */
+#define CM3217_CMD1_SD		BIT(0)		/* shutdown when set */
+
+#define CM3217_CMD2_FD_IT	GENMASK(7, 5)
+
+#define CM3217_IT_T_2T		2
+#define CM3217_FD_IT_100MS	5
+
+/* 0.01875 lux per count at 4 T and 800 ms, the finest resolution */
+#define CM3217_NLUX_MS		15000000000ULL
+
+#define CM3217_MARGIN_MS	10
+
+/* FD_IT settings in milliseconds, indexed by the field value */
+static const unsigned int cm3217_fd_it_ms[] = {
+	800, 400, 266, 200, 130, 100, 80, 66,
+};
+
+static const int cm3217_int_time_avail[][2] = {
+	{ 0, 800000 }, { 0, 400000 }, { 0, 266000 }, { 0, 200000 },
+	{ 0, 130000 }, { 0, 100000 }, { 0, 80000 }, { 0, 66000 },
+};
+
+struct cm3217_data {
+	struct i2c_client *client;
+	struct i2c_client *client_lsb;
+	/* protects the command bytes, the scale table and ready */
+	struct mutex lock;
+	u8 cmd1;
+	u8 cmd2;
+	ktime_t ready;
+	int scale_avail[4][2];
+};
+
+static unsigned int cm3217_conv_ms(struct cm3217_data *data)
+	__must_hold(&data->lock)
+{
+	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+	unsigned int it_t = FIELD_GET(CM3217_CMD1_IT_T, data->cmd1);
+
+	return (cm3217_fd_it_ms[fd_it] << it_t) / 2;
+}
+
+static void cm3217_update_scale_avail(struct cm3217_data *data)
+	__must_hold(&data->lock)
+{
+	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+	unsigned int i;
+	u32 rem;
+	u64 nlux;
+
+	for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
+		nlux = div_u64(CM3217_NLUX_MS << (3 - i), cm3217_fd_it_ms[fd_it]);
+		data->scale_avail[i][0] = div_u64_rem(nlux, NANO, &rem);
+		data->scale_avail[i][1] = rem;
+	}
+}
+
+/*
+ * A new setting only takes effect when the conversion in progress ends,
+ * which can take up to 3.2 s. Leaving shutdown starts a new conversion
+ * right away, so pass through shutdown on every change.
+ */
+static int cm3217_write_cmd(struct cm3217_data *data, u8 cmd1, u8 cmd2)
+	__must_hold(&data->lock)
+{
+	unsigned int conv_ms;
+	int ret;
+
+	ret = i2c_smbus_write_byte(data->client, cmd1 | CM3217_CMD1_SD);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_write_byte(data->client_lsb, cmd2);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_write_byte(data->client, cmd1);
+	if (ret)
+		return ret;
+
+	data->cmd1 = cmd1;
+	data->cmd2 = cmd2;
+	cm3217_update_scale_avail(data);
+
+	/* the first count is ready after one conversion */
+	conv_ms = cm3217_conv_ms(data);
+	data->ready = ktime_add_ms(ktime_get(),
+				   conv_ms + conv_ms / 10 + CM3217_MARGIN_MS);
+
+	return 0;
+}
+
+static int cm3217_read_count(struct cm3217_data *data, int *val)
+	__must_hold(&data->lock)
+{
+	s64 wait_ms;
+	int lsb, msb;
+
+	wait_ms = ktime_ms_delta(data->ready, ktime_get());
+	if (wait_ms > 0)
+		msleep(wait_ms);
+
+	lsb = i2c_smbus_read_byte(data->client_lsb);
+	if (lsb < 0)
+		return lsb;
+
+	msb = i2c_smbus_read_byte(data->client);
+	if (msb < 0)
+		return msb;
+
+	*val = msb << 8 | lsb;
+
+	return 0;
+}
+
+static const struct iio_chan_spec cm3217_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+};
+
+static int cm3217_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+	unsigned int idx;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = cm3217_read_count(data, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		idx = FIELD_GET(CM3217_CMD1_IT_T, data->cmd1);
+		*val = data->scale_avail[idx][0];
+		*val2 = data->scale_avail[idx][1];
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		idx = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
+		*val = cm3217_int_time_avail[idx][0];
+		*val2 = cm3217_int_time_avail[idx][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)data->scale_avail;
+		*length = 2 * ARRAY_SIZE(data->scale_avail);
+		*type = IIO_VAL_INT_PLUS_NANO;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_INT_TIME:
+		*vals = (const int *)cm3217_int_time_avail;
+		*length = 2 * ARRAY_SIZE(cm3217_int_time_avail);
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct cm3217_data *data = iio_priv(indio_dev);
+	unsigned int i;
+	u8 cmd1, cmd2;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
+			if (data->scale_avail[i][0] == val &&
+			    data->scale_avail[i][1] == val2)
+				break;
+		}
+		if (i == ARRAY_SIZE(data->scale_avail))
+			return -EINVAL;
+
+		cmd1 = data->cmd1 & ~CM3217_CMD1_IT_T;
+		cmd1 |= FIELD_PREP(CM3217_CMD1_IT_T, i);
+
+		return cm3217_write_cmd(data, cmd1, data->cmd2);
+	case IIO_CHAN_INFO_INT_TIME:
+		for (i = 0; i < ARRAY_SIZE(cm3217_int_time_avail); i++) {
+			if (cm3217_int_time_avail[i][0] == val &&
+			    cm3217_int_time_avail[i][1] == val2)
+				break;
+		}
+		if (i == ARRAY_SIZE(cm3217_int_time_avail))
+			return -EINVAL;
+
+		cmd2 = FIELD_PREP(CM3217_CMD2_FD_IT, i);
+
+		return cm3217_write_cmd(data, data->cmd1, cmd2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cm3217_write_raw_get_fmt(struct iio_dev *indio_dev,
+				    struct iio_chan_spec const *chan,
+				    long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info cm3217_info = {
+	.read_raw = cm3217_read_raw,
+	.read_avail = cm3217_read_avail,
+	.write_raw = cm3217_write_raw,
+	.write_raw_get_fmt = cm3217_write_raw_get_fmt,
+};
+
+static void cm3217_power_off(void *_data)
+{
+	struct cm3217_data *data = _data;
+
+	i2c_smbus_write_byte(data->client, data->cmd1 | CM3217_CMD1_SD);
+}
+
+static int cm3217_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct cm3217_data *data;
+	struct iio_dev *indio_dev;
+	u8 cmd1, cmd2;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable vdd supply\n");
+
+	data->client_lsb = devm_i2c_new_dummy_device(dev, client->adapter,
+						     CM3217_ADDR_CMD2_LSB);
+	if (IS_ERR(data->client_lsb))
+		return dev_err_probe(dev, PTR_ERR(data->client_lsb),
+				     "failed to register second address\n");
+
+	/* Start with the setting of the vendor driver: 2 T, 100 ms */
+	cmd1 = CM3217_CMD1_RSVD | CM3217_CMD1_WDM |
+	       FIELD_PREP(CM3217_CMD1_IT_T, CM3217_IT_T_2T);
+	cmd2 = FIELD_PREP(CM3217_CMD2_FD_IT, CM3217_FD_IT_100MS);
+
+	scoped_guard(mutex, &data->lock)
+		ret = cm3217_write_cmd(data, cmd1, cmd2);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to power on\n");
+
+	ret = devm_add_action_or_reset(dev, cm3217_power_off, data);
+	if (ret)
+		return ret;
+
+	indio_dev->name = "cm3217";
+	indio_dev->info = &cm3217_info;
+	indio_dev->channels = cm3217_channels;
+	indio_dev->num_channels = ARRAY_SIZE(cm3217_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int cm3217_suspend(struct device *dev)
+{
+	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
+
+	guard(mutex)(&data->lock);
+
+	return i2c_smbus_write_byte(data->client, data->cmd1 | CM3217_CMD1_SD);
+}
+
+static int cm3217_resume(struct device *dev)
+{
+	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
+
+	guard(mutex)(&data->lock);
+
+	return cm3217_write_cmd(data, data->cmd1, data->cmd2);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(cm3217_pm_ops, cm3217_suspend, cm3217_resume);
+
+static const struct i2c_device_id cm3217_id[] = {
+	{ .name = "cm3217" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, cm3217_id);
+
+static const struct of_device_id cm3217_of_match[] = {
+	{ .compatible = "capella,cm3217" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cm3217_of_match);
+
+static struct i2c_driver cm3217_driver = {
+	.driver = {
+		.name = "cm3217",
+		.of_match_table = cm3217_of_match,
+		.pm = pm_sleep_ptr(&cm3217_pm_ops),
+	},
+	.probe = cm3217_probe,
+	.id_table = cm3217_id,
+};
+module_i2c_driver(cm3217_driver);
+
+MODULE_AUTHOR("Patrick Fischer <admin@tsv-stahl-riesa.de>");
+MODULE_DESCRIPTION("Capella CM3217 ambient light sensor driver");
+MODULE_LICENSE("GPL");

-- 
2.56.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver
  2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  (?)
@ 2026-11-01 14:56   ` sashiko-bot
  -1 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-01 14:56 UTC (permalink / raw)
  To: Patrick Fischer; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iio: light: cm3217: data race on scale_avail in cm3217_read_avail()
--

commit 3a1806ec170fd01b8d751c110690623e96b608ae
Author: Patrick Fischer <admin@tsv-stahl-riesa.de>

iio: light: Add Capella CM3217 ambient light sensor driver

This commit adds an IIO ambient light sensor driver for the Capella
CM3217, communicating over two fixed I2C addresses (0x10 and 0x11) to
configure integration time and range and read the 16-bit light count.

> diff --git a/drivers/iio/light/cm3217.c b/drivers/iio/light/cm3217.c
> new file mode 100644
> index 0000000000000..9e0e9afee0925
> --- /dev/null
> +++ b/drivers/iio/light/cm3217.c
> @@ -0,0 +1,383 @@

[ ... ]

> +struct cm3217_data {
> +	struct i2c_client *client;
> +	struct i2c_client *client_lsb;
> +	/* protects the command bytes, the scale table and ready */
> +	struct mutex lock;
> +	u8 cmd1;
> +	u8 cmd2;
> +	ktime_t ready;
> +	int scale_avail[4][2];
> +};

[ ... ]

> +static void cm3217_update_scale_avail(struct cm3217_data *data)
> +	__must_hold(&data->lock)
> +{
> +	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
> +	unsigned int i;
> +	u32 rem;
> +	u64 nlux;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
> +		nlux = div_u64(CM3217_NLUX_MS << (3 - i), cm3217_fd_it_ms[fd_it]);
> +		data->scale_avail[i][0] = div_u64_rem(nlux, NANO, &rem);
> +		data->scale_avail[i][1] = rem;
> +	}
> +}

[ ... ]

> +static int cm3217_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long mask)
> +{
> +	struct cm3217_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (const int *)data->scale_avail;
> +		*length = 2 * ARRAY_SIZE(data->scale_avail);
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		return IIO_AVAIL_LIST;

[Severity: Medium]
Can concurrent sysfs reads of in_illuminance_scale_available and writes to
in_illuminance_integration_time race on data->scale_avail?

In cm3217_read_avail(), a raw pointer to data->scale_avail is returned to the
caller without holding data->lock, and the caller in
drivers/iio/industrialio-core.c reads and formats the array elements after
cm3217_read_avail() returns:

iio_read_channel_info_avail()
  indio_dev->info->read_avail() -> cm3217_read_avail()
  iio_format_avail_list(buf, vals, type, length)

At the same time, cm3217_write_raw() or cm3217_resume() can call
cm3217_write_cmd() -> cm3217_update_scale_avail(), overwriting
data->scale_avail[i][0] and data->scale_avail[i][1] in place while
iio_format_avail_list() is iterating over vals. That can produce torn integer
and fractional scale pairs or a list mixing scales from two integration
times.

Since there are only 8 fixed integration times in cm3217_fd_it_ms[] and 4
ranges per integration time, could the scale tables be stored in a static
const table indexed by the integration time selector so cm3217_read_avail()
can return a pointer to an immutable row under data->lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-upstream-iio-cm3217-v1-0-5e1724cae7b7@tsv-stahl-riesa.de?part=2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver
  2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  (?)
  (?)
@ 2026-11-01 15:59   ` Svyatoslav Ryhel
  -1 siblings, 0 replies; 10+ messages in thread
From: Svyatoslav Ryhel @ 2026-11-01 15:59 UTC (permalink / raw)
  To: admin
  Cc: Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	linux-iio, devicetree, linux-tegra, linux-kernel

нд, 1 лист. 2026 р. о 16:46 Patrick Fischer via B4 Relay
<devnull+admin.tsv-stahl-riesa.de@kernel.org> пише:
>
> From: Patrick Fischer <admin@tsv-stahl-riesa.de>
>
> The CM3217 has no registers and no ID. Writing a byte to address 0x10
> or 0x11 sets command byte 1 or 2, reading a byte from 0x10 or 0x11
> returns the high or low byte of the 16-bit light count.
>
> Command byte 1 holds the shutdown bit and the IT_T field (1/2 T, 1 T,
> 2 T or 4 T), command byte 2 the FD_IT field (800 ms down to 66 ms). The
> count scales linearly with both. The driver exposes FD_IT as the
> integration time and IT_T through the scale, so the scale list offers
> four ranges for each integration time.
>
> The datasheet is not public. The bit fields, the reserved bits that are
> written as 1 and the startup setting of 2 T and 100 ms come from NVIDIA's
> downstream drivers for the Tegra Note 7 (drivers/staging/iio/light/
> cm3217.c in the 3.4 and 3.10 kernels). For the resolution, Capella
> gives a range of 0.01875 to 119156 lux. The driver takes 0.01875 lux
> per count at 4 T and 800 ms. This matches both the upper end (65535
> counts at 1/2 T and 66 ms) and the 0.3 lux per count that the NVIDIA
> 3.4 driver uses at 2 T and 100 ms.
>
> The sensor has no data-ready flag, and a new setting only takes effect
> when the conversion in progress ends. A conversion takes IT_T times
> FD_IT, up to 3.2 s. Leaving shutdown starts a new one at once, so the
> driver passes through shutdown on every change and waits one conversion
> period plus a margin before it reads the count. The timing was measured
> on a Tegra Note 7.
>
> Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
> ---
>  drivers/iio/light/Kconfig  |  10 ++
>  drivers/iio/light/Makefile |   1 +
>  drivers/iio/light/cm3217.c | 383 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 394 insertions(+)
>

Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Tegratab P1988

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: light: Add Capella CM3217
  2026-11-01 14:46   ` Patrick Fischer via B4 Relay
  (?)
@ 2026-11-01 17:48   ` Jonathan Cameron
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2026-11-01 17:48 UTC (permalink / raw)
  To: Patrick Fischer via B4 Relay
  Cc: admin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel

On Sun, 01 Nov 2026 15:46:06 +0100
Patrick Fischer via B4 Relay <devnull+admin.tsv-stahl-riesa.de@kernel.org> wrote:

> From: Patrick Fischer <admin@tsv-stahl-riesa.de>
> 
> The Capella CM3217 is an ambient light sensor with an I2C interface but
> without registers and without an interrupt line. It always answers on
> the two addresses 0x10 and 0x11. The node describes the first address;
> the second one is fixed and claimed by the driver.
> 
> The sensor is used in the NVIDIA Tegra Note 7 and the NVIDIA Shield
> Tablet.
> 
> Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>

Hi Patrick

One small thing inline. 

> ---
>  .../bindings/iio/light/capella,cm3217.yaml         | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
> new file mode 100644
> index 000000000000..b2ddb14d657b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/capella,cm3217.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/capella,cm3217.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Capella CM3217 ambient light sensor
> +
> +maintainers:
> +  - Patrick Fischer <admin@tsv-stahl-riesa.de>
> +
> +description:
> +  The CM3217 is an I2C ambient light sensor without registers or interrupt
> +  line. It always occupies the two I2C addresses 0x10 and 0x11; the node
> +  describes the first one.
> +
> +properties:
> +  compatible:
> +    const: capella,cm3217
> +
> +  reg:
> +    const: 0x10

Put the documentation about using who I2C bus addresses here rather than
in the introduction. This is where anyone wondering about allowed reg for
a device that sits on two addresses would look.  Maybe add a little
on what goes to each address.

> +
> +  vdd-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@10 {
> +            compatible = "capella,cm3217";
> +            reg = <0x10>;
> +            vdd-supply = <&vdd_2v85_sen>;
> +        };
> +    };
> +...
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver
  2026-11-01 14:46   ` Patrick Fischer via B4 Relay
                     ` (2 preceding siblings ...)
  (?)
@ 2026-11-01 18:06   ` Jonathan Cameron
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2026-11-01 18:06 UTC (permalink / raw)
  To: Patrick Fischer via B4 Relay
  Cc: admin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	David Lechner, Nuno Sá, Andy Shevchenko, Kevin Tsai,
	Svyatoslav Ryhel, linux-iio, devicetree, linux-tegra,
	linux-kernel

On Sun, 01 Nov 2026 15:46:07 +0100
Patrick Fischer via B4 Relay <devnull+admin.tsv-stahl-riesa.de@kernel.org> wrote:

> From: Patrick Fischer <admin@tsv-stahl-riesa.de>
> 
> The CM3217 has no registers and no ID. Writing a byte to address 0x10
> or 0x11 sets command byte 1 or 2, reading a byte from 0x10 or 0x11
> returns the high or low byte of the 16-bit light count.
> 
> Command byte 1 holds the shutdown bit and the IT_T field (1/2 T, 1 T,
> 2 T or 4 T), command byte 2 the FD_IT field (800 ms down to 66 ms). The
> count scales linearly with both. The driver exposes FD_IT as the
> integration time and IT_T through the scale, so the scale list offers
> four ranges for each integration time.
> 
> The datasheet is not public. The bit fields, the reserved bits that are
> written as 1 and the startup setting of 2 T and 100 ms come from NVIDIA's
> downstream drivers for the Tegra Note 7 (drivers/staging/iio/light/
> cm3217.c in the 3.4 and 3.10 kernels). For the resolution, Capella
> gives a range of 0.01875 to 119156 lux. The driver takes 0.01875 lux
> per count at 4 T and 800 ms. This matches both the upper end (65535
> counts at 1/2 T and 66 ms) and the 0.3 lux per count that the NVIDIA
> 3.4 driver uses at 2 T and 100 ms.
> 
> The sensor has no data-ready flag, and a new setting only takes effect
> when the conversion in progress ends. A conversion takes IT_T times
> FD_IT, up to 3.2 s. Leaving shutdown starts a new one at once, so the
> driver passes through shutdown on every change and waits one conversion
> period plus a margin before it reads the count. The timing was measured
> on a Tegra Note 7.

I don't think we have a current active Capella Micro developer on the
list as it's been a while since I heard from Kevin Tsai who contributed
drivers for them, so your approach of using the vendor driver as a
guide is probably the best we can do.

> 
> Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
A few comments inline.

If it is really hard to precompute all the available values, then
we can probably get away with not doing so.  I don't think it is
though and picking between precomputed sets is both cleaner and avoids
a race if we ever get light driver inkernel consumers.

Thanks

Jonathan

> diff --git a/drivers/iio/light/cm3217.c b/drivers/iio/light/cm3217.c
> new file mode 100644
> index 000000000000..9e0e9afee092
> --- /dev/null
> +++ b/drivers/iio/light/cm3217.c
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Capella CM3217 ambient light sensor driver
> + *
> + * The CM3217 has no registers. It answers on two fixed I2C addresses:
> + * writing a byte to 0x10 or 0x11 sets command byte 1 or 2, reading a byte
> + * from 0x10 or 0x11 returns the MSB or LSB of the 16-bit light count.
> + *
> + * Copyright (C) 2026 Patrick Fischer <admin@tsv-stahl-riesa.de>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/ktime.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define CM3217_ADDR_CMD2_LSB	0x11
> +
> +#define CM3217_CMD1_RSVD	BIT(5)		/* reserved, set to 1 */
> +#define CM3217_CMD1_IT_T	GENMASK(3, 2)	/* 1/2 T, 1 T, 2 T, 4 T */
> +#define CM3217_CMD1_WDM		BIT(1)		/* set to 1 like the vendor driver */
> +#define CM3217_CMD1_SD		BIT(0)		/* shutdown when set */
> +
> +#define CM3217_CMD2_FD_IT	GENMASK(7, 5)
> +
> +#define CM3217_IT_T_2T		2
> +#define CM3217_FD_IT_100MS	5
> +
> +/* 0.01875 lux per count at 4 T and 800 ms, the finest resolution */
> +#define CM3217_NLUX_MS		15000000000ULL

Use a muiltiplier for that definition. Various ones in units.h.
I haven't figured out what makes sense, but that is too many 
zeros to eyeball!

> +
> +#define CM3217_MARGIN_MS	10


> +
> +static unsigned int cm3217_conv_ms(struct cm3217_data *data)
> +	__must_hold(&data->lock)
> +{
> +	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
> +	unsigned int it_t = FIELD_GET(CM3217_CMD1_IT_T, data->cmd1);
> +
> +	return (cm3217_fd_it_ms[fd_it] << it_t) / 2;
> +}
> +
> +static void cm3217_update_scale_avail(struct cm3217_data *data)
There are some unfortunately complex race conditions around
read_avail where a consumer driver (iio/inkern.c) can hold a pointer to the
array and there is no visibility in the consumer of the fact the
data might be changing under it. The concern isn't so much getting
wrong values as getting torn ones as the writes are occuring. 
There have been a few goes at solving the race generically but none
have landed yet.  Generally avoiding that sort of handoff of
dynamic data is a nice to have and often easy enough to do.

I'm not that concerned for a light sensor as we have no consumers
in upstream anyway, but how bad would it be to precompute all possible
tables at probe time or even at compile time, and then just pick between them?

> +	__must_hold(&data->lock)
> +{
> +	unsigned int fd_it = FIELD_GET(CM3217_CMD2_FD_IT, data->cmd2);
> +	unsigned int i;
> +	u32 rem;
> +	u64 nlux;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->scale_avail); i++) {
Might as well do

	for (unsigned int i = 0; .. {
	

> +		nlux = div_u64(CM3217_NLUX_MS << (3 - i), cm3217_fd_it_ms[fd_it]);
> +		data->scale_avail[i][0] = div_u64_rem(nlux, NANO, &rem);
> +		data->scale_avail[i][1] = rem;
> +	}
> +}



> +
> +static int cm3217_suspend(struct device *dev)
> +{
> +	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
> +
> +	guard(mutex)(&data->lock);
> +
> +	return i2c_smbus_write_byte(data->client, data->cmd1 | CM3217_CMD1_SD);

Given this lines up with the power off callback above and the start of
the write_cmd() sequence, maybe have a little helper to put this write
command in just one place?

> +}
> +
> +static int cm3217_resume(struct device *dev)
> +{
> +	struct cm3217_data *data = iio_priv(dev_get_drvdata(dev));
> +
> +	guard(mutex)(&data->lock);
> +
> +	return cm3217_write_cmd(data, data->cmd1, data->cmd2);
> +}

> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-11-01 18:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-01 14:46 [PATCH 0/2] iio: light: Add Capella CM3217 ambient light sensor Patrick Fischer
2026-11-01 14:46 ` Patrick Fischer via B4 Relay
2026-11-01 14:46 ` [PATCH 1/2] dt-bindings: iio: light: Add Capella CM3217 Patrick Fischer
2026-11-01 14:46   ` Patrick Fischer via B4 Relay
2026-11-01 17:48   ` Jonathan Cameron
2026-11-01 14:46 ` [PATCH 2/2] iio: light: Add Capella CM3217 ambient light sensor driver Patrick Fischer
2026-11-01 14:46   ` Patrick Fischer via B4 Relay
2026-11-01 14:56   ` sashiko-bot
2026-11-01 15:59   ` Svyatoslav Ryhel
2026-11-01 18:06   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.