Add support for AS7343 multi-spectral sensor

Add binding for AMS AS7343 which is a 14-channel multi-spectral sensor with i2c address of 0x39.

The GPIO pin is not described because the datasheet does not document how to use it for hardware synchronization at all. The GPIO register also has no mode-select bit to isolate the pin from the chip's internal control logic. Exposing it as a generic GPIO could result in kernel writes being overwritten by internal logic.

Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf

Signed-off-by Chang Yu <[email protected]>

Add a driver for the AMS AS7343 14-channel multi-spectral sensor.

The AS7343 is a 14-channel spectral sensor featuring 11 visible channels, 1 near-infrared channel, 1 clear channel (VIS), and 1 flicker detection channel.

The driver exposes 12 spectral channels (11 visible light and 1 near-infrared) via sysfs. Runtime PM is implemented to stop measurements when the device is suspended or torn down. Power is never cut (PON=1 always) to preserve register values.

Future patches will add configurable gain and integration time, interrupt support, buffered reads, VIS channel, and flicker detection.

Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf

Signed-off-by Chang Yu <[email protected]>

MAINTAINERS +1 -0

index f0c08e634bf4..9496280903d9 100644
@@ -1460,6 +1460,7 @@ M:	Chang Yu <[email protected]>
 L:	[email protected]
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
+F:	drivers/iio/light/as7343.c
 
 AMT (Automatic Multicast Tunneling)
 M:	Taehee Yoo <[email protected]>

drivers/iio/light/Kconfig +11 -0

index 462fff1ddef2..d0dea4f3113b 100644
@@ -152,6 +152,17 @@ config AS73211
 	 This driver can also be built as a module.  If so, the module
 	 will be called as73211.
 
+config AS7343
+	tristate "AMS AS7343 14-Channel Multi-Spectral Sensor"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	 Say Y here to build support for the AMS AS7343 14-channel
+	 multi-spectral sensor.
+
+	 To compile this driver as a module, choose M here: the module will
+	 be called as7343.
+
 config BH1745
 	tristate "ROHM BH1745 colour sensor"
 	depends on I2C

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

new file mode 100644
index 000000000000..c079e0c6aa70
 

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

+	return n - 1;
+}
+

drivers/iio/light/as7343.c new file +45 -0

+	if (ret)
+		return ret;
+
+	/* Need to set REG_BANK to 1 before we can access ID */
+	ret = regmap_update_bits(map, AS7343_CFG0_REG, AS7343_CFG0_REG_BANK,
+				 FIELD_PREP(AS7343_CFG0_REG_BANK, 1));
+	if (ret)
+		return ret;
+
+	ret = regmap_read(map, AS7343_ID, &val);
+	if (ret)
+		return ret;
+
+	if (val != 0x81)
+		dev_info(dev, "Unknown device ID: %x\n", val);
+
+	ret = regmap_update_bits(map, AS7343_CFG0_REG, AS7343_CFG0_REG_BANK,
+				 FIELD_PREP(AS7343_CFG0_REG_BANK, 0));
+	if (ret)
+		return ret;
+
+	/* Configure the SMUX to readout all channels */
+	ret = regmap_update_bits(map, AS7343_CFG20_REG, AS7343_CFG20_AUTO_SMUX,
+				 FIELD_PREP(AS7343_CFG20_AUTO_SMUX,
+					    AS7343_CFG20_AUTO_SMUX_READOUT_ALL));
+	if (ret)
+		return ret;
+
+	/*
+	 * Setting a 600 * 2.78us * 30 = 50ms integration time as the default
+	 * for now.
+	 */
+	step = cpu_to_le16(as7343_astep_x2780ns(600));
+	ret = regmap_bulk_write(map, AS7343_ASTEP_REG, &step, sizeof(step));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(map, AS7343_ATIME_REG, as7343_atime_steps(30));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(map, AS7343_CFG1_REG, AS7343_CFG1_AGAIN,
+				  FIELD_PREP(AS7343_CFG1_AGAIN,
+					     AS7343_CFG1_AGAIN_X256));
+}

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

+static int as7343_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct iio_dev *indio_dev;
+	struct as7343_data *data;
+	struct regmap *regmap;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, indio_dev);
+
+	regmap = devm_regmap_init_i2c(client, &as7343_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	data = iio_priv(indio_dev);
+	data->regmap = regmap;
+
+	ret = devm_mutex_init(dev, &data->mutex);
+	if (ret)
+		return ret;
+
+	indio_dev->name = "as7343";
+	indio_dev->info = &as7343_info;
+	indio_dev->channels = as7343_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as7343_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return ret;
+
+	ret = as7343_setup_device(dev, data);
+	if (ret)
+		return ret;

Quoted 15 lines

Add a driver for the AMS AS7343 14-channel multi-spectral sensor.

The AS7343 is a 14-channel spectral sensor featuring 11 visible channels, 1 near-infrared channel, 1 clear channel (VIS), and 1 flicker detection channel.

The driver exposes 12 spectral channels (11 visible light and 1 near-infrared) via sysfs. Runtime PM is implemented to stop measurements when the device is suspended or torn down. Power is never cut (PON=1 always) to preserve register values.

Future patches will add configurable gain and integration time, interrupt support, buffered reads, VIS channel, and flicker detection.

Quoted 2 lines

Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf

Signed-off-by Chang Yu <[email protected]>

FWIW I believe the sashiko feedback autosuspend is incorrect, but probably good to sanity check that by proving device and just checking the calls in dd.c are sufficient to put it to sleep after the auto suspend time.

Add a driver for the AMS AS7343 14-channel multi-spectral sensor.

The AS7343 is a 14-channel spectral sensor featuring 11 visible channels, 1 near-infrared channel, 1 clear channel (VIS), and 1 flicker detection channel.

The driver exposes 12 spectral channels (11 visible light and 1 near-infrared) via sysfs. Runtime PM is implemented to stop measurements when the device is suspended or torn down. Power is never cut (PON=1 always) to preserve register values.

Future patches will add configurable gain and integration time, interrupt support, buffered reads, VIS channel, and flicker detection.

+#include <linux/build_bug.h>

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

+	if (channel < 0 || channel >= ARRAY_SIZE(as7343_channel_labels))
+		return -EINVAL;
+
+	return sysfs_emit(label, "%s\n", as7343_channel_labels[channel]);
+}

Quoted 14 lines · Chang Yu

Add a driver for the AMS AS7343 14-channel multi-spectral sensor.

The AS7343 is a 14-channel spectral sensor featuring 11 visible channels, 1 near-infrared channel, 1 clear channel (VIS), and 1 flicker detection channel.

The driver exposes 12 spectral channels (11 visible light and 1 near-infrared) via sysfs. Runtime PM is implemented to stop measurements when the device is suspended or torn down. Power is never cut (PON=1 always) to preserve register values.

Future patches will add configurable gain and integration time, interrupt support, buffered reads, VIS channel, and flicker detection.

...

Quoted 1 line

+#include <linux/build_bug.h>

This is unusual to meet...

...

drivers/iio/light/as7343.c line 139

Quoted 3 lines

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

+static inline u8 as7343_atime_steps(unsigned int step)
+{
+   BUILD_BUG_ON(step < 1 || step > 256);

How is this any useful? The step == 1 gives the result 0, the step 256, 255 respectively. This covers the whole range of the returned variable (u8).

drivers/iio/light/as7343.c line 147

Quoted 8 lines

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

+   return step - 1;
+}
+
+static inline u16 as7343_astep_x2780ns(unsigned int n)
+{
+   BUILD_BUG_ON(n < 1 || n > 65535);
+   return n - 1;
+}

Ditto.

...

TL;DR: I do not see anything meaningful from these BUILD_BUG_ON():s. It's not a problem if the result gives just wrapped-up value. Or is it?

drivers/iio/light/as7343.c line 237

Quoted 4 lines · Chang Yu

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

+static int as7343_read_label(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, char *label)
+{
+	int channel = chan->channel;

Why signed?
Also split assignment, it will make code more maintainable.

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

+static int as7343_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct as7343_data *data = iio_priv(indio_dev);
+	struct regmap *map = data->regmap;
+	struct device *dev = regmap_get_device(map);
+	unsigned int unused;
+	__le16 result;
+	int ret;

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

+
+		/*
+		 * Reading ASTATUS latches all data registers to this read.
+		 * We don't care about the returned saturation/gain status for
+		 * now.
+		 */
+		guard(mutex)(&data->mutex);
+
+		ret = regmap_read(map, AS7343_ASTATUS_REG, &unused);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(map, chan->address,
+				       &result, sizeof(result));
+		if (ret)

Quoted 19 lines · Chang Yu

drivers/iio/light/as7343.c new file +19 -0

+static int as7343_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct as7343_data *data = iio_priv(indio_dev);
+	struct regmap *map = data->regmap;
+	struct device *dev = regmap_get_device(map);
+	unsigned int unused;
+	__le16 result;
+	int ret;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		/* Wait until integration time passes for all 3 cycles. */

drivers/iio/light/as7343.c line 174

Quoted 1 line

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

+		msleep(160);

three integration periods hardcoded next to a hardcoded integration time, so nothing keeps them in sync. make ATIME/ASTEP writable and the fixed wait no longer covers a readout. as73211, which this is based on, computes the timeout and polls NDATA. STATUS2 (0x90) bit 6 is AVALID is defined and never read, does it cover all three cycles? the datasheet does not say.