Re: [PATCH v3 2/2] iio: light: Add Capella CM3217 ambient light sensor driver

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 13 Nov 2026 20:59:00 +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. The driver
> takes the second address from the second reg entry and falls back to
> 0x11 when there is no firmware node, as with new_device.
> 
> 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 (drivers/staging/iio/light/cm3217.c in the 3.4 kernel
> rel-tegranote-r8 and the 3.10 kernel
> android-tegra-flounder-3.10-nougat-mr1.1). One value differs: the vendor
> header defines 4 T as 4 << 2, which is outside the two-bit field. On a
> Tegra Note 7 field value 3 gave four times the 1 T count, so the driver
> uses that. 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.
> 
> Tested-by: Svyatoslav Ryhel <clamor95@gmail.com> # Tegratab P1988
> Signed-off-by: Patrick Fischer <admin@tsv-stahl-riesa.de>
Hi Patrick

One small thing about how you are handling the firmware absent case.

Thanks,

Jonathan

> diff --git a/drivers/iio/light/cm3217.c b/drivers/iio/light/cm3217.c
> new file mode 100644
> index 000000000000..0c2c7f9d7d69
> --- /dev/null
> +++ b/drivers/iio/light/cm3217.c
> @@ -0,0 +1,419 @@

> +
> +static int cm3217_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct cm3217_data *data;
> +	struct iio_dev *indio_dev;
> +	u16 addr_lsb = CM3217_ADDR_CMD2_LSB;
> +	u32 reg[2];
> +	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");
> +
> +	/* Without a firmware node (e.g. sysfs new_device) use the fixed one */

I'd rather we handled that explicitly.

	if (dev_fwmode(dev))

That way we can return an error if there is firmware but it's broken.

> +	if (!device_property_read_u32_array(dev, "reg", reg, ARRAY_SIZE(reg)))
> +		addr_lsb = reg[1];