[PATCH v2 0/2] Add support for AS7343 multi-spectral sensor

[PATCH v2 0/2] Add support for AS7343 multi-spectral sensor

[Chang Yu]

This series adds initial support for the AMS AS7343 14-channel
multi-spectral sensor.

The sensor has 14 optical channels (11 visible + NIR + clear +
flicker) in total. This initial driver supports reading raw counts
from all 12 spectral channels (11 visible + NIR) via sysfs and basic
power management.

Not yet supported:
- Configurable integration time and gain
- Interrupt support
- Clear (VIS) channel and flicker detection

Patch 1 adds device tree bindings.
Patch 2 adds the driver.

Testing was performed on a Raspberry Pi 4.

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

Changes in v2:
  - Add the LDR, the interrupt pin, and the GPIO pin to the dt bindings.
  - Adjust the default gain to x256 and integration time to 50.1ms as
  recommended by the datasheet.
  - Use read_label instead of .extend
  - Remove all unused scan_index related constants.

Chang Yu (2):
  dt-bindings: iio: light: add as7343
  iio: light: add AS7343 multi-spectral sensor driver

 .../bindings/iio/light/ams,as7343.yaml        |  69 ++++
 MAINTAINERS                                   |   7 +
 drivers/iio/light/Kconfig                     |  11 +
 drivers/iio/light/Makefile                    |   1 +
 drivers/iio/light/as7343.c                    | 372 ++++++++++++++++++
 5 files changed, 460 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
 create mode 100644 drivers/iio/light/as7343.c

-- 
2.55.0

[PATCH v2 1/2] dt-bindings: iio: light: add as7343

[Chang Yu]

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

The GPIO pin is described as a generic GPIO for now. Binding design for the
more advanced measurement/LED synchronization use cases are deferred to
future patches.

Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>
---
Changes in v2:
  - Add the LDR, the interrupt pin, and the GPIO pin to the bindings.
  - Fix node name and unit address mismatch.
  - Include MAINTAINERS changes.

 .../bindings/iio/light/ams,as7343.yaml        | 69 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/ams,as7343.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
new file mode 100644
index 000000000000..b06d445b92b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/ams,as7343.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS AS7343 14-Channel Multi-Spectral Sensor
+
+maintainers:
+  - Chang Yu <marcus.yu.56@gmail.com>
+
+description: |
+  The AMS AS7343 is a 14-channel multi-spectral sensor with i2c address of 0x39.
+  https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
+
+properties:
+  compatible:
+    enum:
+      - ams,as7343
+
+  reg:
+    description:
+      I2C address of the device (0x39).
+    maxItems: 1
+
+  interrupts:
+    description:
+      Open drain output active low interrupt pin.
+    maxItems: 1
+
+  vdd-supply: true
+
+  ams,led-current-microamp:
+    description:
+      The driver current for the external LED connected to the LDR pin.
+    minimum: 4000
+    maximum: 258000
+    multipleOf: 2000
+    default: 12000
+
+  gpios:
+    description:
+      Optional GPIO pin for general I/O.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        multispectral-sensor@39 {
+            compatible = "ams,as7343";
+            reg = <0x39>;
+            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+            vdd-supply = <&vdd_regulator>;
+            ams,led-current-microamp = <14000>;
+            gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 04fa5322d9f7..8cf4e1635053 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1430,6 +1430,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
 F:	drivers/iio/light/as73211.c
 
+AMS AS7343 DRIVER
+M:	Chang Yu <marcus.yu.56@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
+
 AMT (Automatic Multicast Tunneling)
 M:	Taehee Yoo <ap420073@gmail.com>
 L:	netdev@vger.kernel.org
-- 
2.55.0

[PATCH v2 2/2] iio: light: add AS7343 multi-spectral sensor driver

[Chang Yu]

This patch adds a driver for the AMS AS7343 14-channel multi-spectral
sensor with I2C interface.

The driver exposes 12 spectral channels (11 visible + 1 near-infrared)
via the IIO sysfs interface. Each channel's raw data is provided as a
16-bit little-endian unsigned integer.

Basic power management (suspend/resume) is supported. More complex
features such as auto-suspend, interrupts, and configurable
gain/integration time will be added in future patches.

Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>
---
Changes in v2:
  - Adjust the default gain to x256 and integration time to 50.1ms as
  recommended by the datasheet.
  - Use read_label instead of .extend
  - Remove all unused scan_index related constants.
  - Add missing calls to pm_runtime_* methods in probe.
  - Fix various code style issues

 MAINTAINERS                |   1 +
 drivers/iio/light/Kconfig  |  11 ++
 drivers/iio/light/Makefile |   1 +
 drivers/iio/light/as7343.c | 372 +++++++++++++++++++++++++++++++++++++
 4 files changed, 385 insertions(+)
 create mode 100644 drivers/iio/light/as7343.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8cf4e1635053..236102b3c28e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1435,6 +1435,7 @@ M:	Chang Yu <marcus.yu.56@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
+F:	drivers/iio/light/as7343.c
 
 AMT (Automatic Multicast Tunneling)
 M:	Taehee Yoo <ap420073@gmail.com>
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index ef36824f312f..edbeeba9d873 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -149,6 +149,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
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 64e354c49ed8..5d0d33802519 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_APDS9306)		+= apds9306.o
 obj-$(CONFIG_APDS9960)		+= apds9960.o
 obj-$(CONFIG_APDS9999)		+= apds9999.o
 obj-$(CONFIG_AS73211)		+= as73211.o
+obj-$(CONFIG_AS7343)		+= as7343.o
 obj-$(CONFIG_BH1745)		+= bh1745.o
 obj-$(CONFIG_BH1750)		+= bh1750.o
 obj-$(CONFIG_BH1780)		+= bh1780.o
diff --git a/drivers/iio/light/as7343.c b/drivers/iio/light/as7343.c
new file mode 100644
index 000000000000..f8ee0f2e7392
--- /dev/null
+++ b/drivers/iio/light/as7343.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Support for AMS AS7343 14-channel multi-spectral sensor.
+ * (7-bit I2C slave address 0x39)
+ *
+ * Based on the work of:
+ *   Christian Eggers <ceggers@arri.de> (AS73211 driver)
+ *
+ * Copyright (c) 2026 Chang Yu <marcus.yu.56@gmail.com>
+ *
+ * Datasheets:
+ *   https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
+ *
+ * TODO:
+ *   - Autosuspend
+ *   - Support for configurable gain and integration time
+ *   - Interrupt support
+ *   - Add support for reading the VIS channel
+ *   - Flicker detection
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/iio.h>
+
+/* AS7343 registers */
+#define AS7343_ID	    0x5a
+
+#define AS7343_ENABLE	    0x80
+#define AS7343_ENABLE_PON   BIT(0)
+#define AS7343_ENABLE_SP_EN BIT(1)
+
+/*
+ * Integration time is calculated as (ATIME + 1) * ((ASTEP + 1) * 2.78us).
+ * Setting a 30 * 1.67ms = 50.1ms integration test as the default for now.
+ */
+#define AS7343_ATIME			   0x81
+#define AS7343_ATIME_VAL		   29 /* (29 + 1) = 30 steps */
+#define AS7343_ASTEP			   0xd4
+#define AS7343_ASTEP_VAL		   599 /* 1.67ms step size */
+
+#define AS7343_CFG0			   0xbf
+#define AS7343_CFG0_REG_BANK		   BIT(4)
+
+#define AS7343_CFG1			   0xc6
+#define AS7343_CFG1_AGAIN		   GENMASK(4, 0)
+#define AS7343_CFG1_AGAIN_X0_5		   0
+#define AS7343_CFG1_AGAIN_X1		   1
+#define AS7343_CFG1_AGAIN_X2		   2
+#define AS7343_CFG1_AGAIN_X4		   3
+#define AS7343_CFG1_AGAIN_X8		   4
+#define AS7343_CFG1_AGAIN_X16		   5
+#define AS7343_CFG1_AGAIN_X32		   6
+#define AS7343_CFG1_AGAIN_X64		   7
+#define AS7343_CFG1_AGAIN_X128		   8
+#define AS7343_CFG1_AGAIN_X256		   9
+#define AS7343_CFG1_AGAIN_X512		   10
+#define AS7343_CFG1_AGAIN_X1024		   11
+#define AS7343_CFG1_AGAIN_X2048		   12
+
+#define AS7343_CFG20			   0xd6
+#define AS7343_CFG20_AUTO_SMUX		   GENMASK(6, 5)
+#define AS7343_CFG20_AUTO_SMUX_READOUT_ALL 3 /* all-channel readout */
+
+#define AS7343_ASTATUS			   0x94
+
+/* AS7343 data registers */
+#define AS7343_DATA_FZ	0x95
+#define AS7343_DATA_FY	0x97
+#define AS7343_DATA_FXL 0x99
+#define AS7343_DATA_NIR 0x9b
+#define AS7343_DATA_F2	0xa1
+#define AS7343_DATA_F3	0xa3
+#define AS7343_DATA_F4	0xa5
+#define AS7343_DATA_F6	0xa7
+#define AS7343_DATA_F1	0xad
+#define AS7343_DATA_F7	0xaf
+#define AS7343_DATA_F8	0xb1
+#define AS7343_DATA_F5	0xb3
+#define AS7343_MAX	0xff
+
+/* AS7343 channel indices. MUST match data register order above. */
+#define AS7343_CHAN_IDX_FZ  0
+#define AS7343_CHAN_IDX_FY  1
+#define AS7343_CHAN_IDX_FXL 2
+#define AS7343_CHAN_IDX_NIR 3
+#define AS7343_CHAN_IDX_F2  4
+#define AS7343_CHAN_IDX_F3  5
+#define AS7343_CHAN_IDX_F4  6
+#define AS7343_CHAN_IDX_F6  7
+#define AS7343_CHAN_IDX_F1  8
+#define AS7343_CHAN_IDX_F7  9
+#define AS7343_CHAN_IDX_F8  10
+#define AS7343_CHAN_IDX_F5  11
+
+#define AS7343_CHAN(_chan)                                    \
+	{                                                     \
+		.type = IIO_INTENSITY,                        \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+		.address = AS7343_DATA_##_chan,               \
+		.indexed = 1,                                 \
+		.channel = AS7343_CHAN_IDX_##_chan,           \
+	}
+
+static const struct iio_chan_spec as7343_channels[] = {
+	AS7343_CHAN(FZ), AS7343_CHAN(FY), AS7343_CHAN(FXL), AS7343_CHAN(NIR),
+	AS7343_CHAN(F2), AS7343_CHAN(F3), AS7343_CHAN(F4),  AS7343_CHAN(F6),
+	AS7343_CHAN(F1), AS7343_CHAN(F7), AS7343_CHAN(F8),  AS7343_CHAN(F5),
+};
+
+struct as7343_data {
+	struct regmap *regmap;
+};
+
+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);
+	int ret;
+	unsigned int unused;
+	u16 result;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		/*
+		 * Reading ASTATUS latches all data registers to this read.
+		 * We don't care about the returned saturation/gain status for
+		 * now.
+		 */
+		ret = regmap_read(data->regmap, AS7343_ASTATUS, &unused);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(data->regmap, chan->address, &result, 2);
+		if (ret)
+			return ret;
+
+		*val = le16_to_cpu(result);
+		return IIO_VAL_INT;
+	}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const char *as7343_channel_label(struct iio_chan_spec const *chan)
+{
+	switch (chan->channel) {
+	case AS7343_CHAN_IDX_FZ:
+		return "FZ";
+	case AS7343_CHAN_IDX_FY:
+		return "FY";
+	case AS7343_CHAN_IDX_FXL:
+		return "FXL";
+	case AS7343_CHAN_IDX_NIR:
+		return "NIR";
+	case AS7343_CHAN_IDX_F2:
+		return "F2";
+	case AS7343_CHAN_IDX_F3:
+		return "F3";
+	case AS7343_CHAN_IDX_F4:
+		return "F4";
+	case AS7343_CHAN_IDX_F6:
+		return "F6";
+	case AS7343_CHAN_IDX_F1:
+		return "F1";
+	case AS7343_CHAN_IDX_F7:
+		return "F7";
+	case AS7343_CHAN_IDX_F8:
+		return "F8";
+	case AS7343_CHAN_IDX_F5:
+		return "F5";
+	default:
+		return NULL;
+	}
+}
+
+static int as7343_read_label(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, char *label)
+{
+	const char *name;
+
+	name = as7343_channel_label(chan);
+	if (!name)
+		return -EINVAL;
+	return sysfs_emit(label, "%s\n", name);
+}
+
+static const struct iio_info as7343_info = {
+	.read_raw = as7343_read_raw,
+	.read_label = as7343_read_label,
+};
+
+static const struct regmap_config as7343_regmap_config = {
+	.name = "as7343",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AS7343_MAX,
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.cache_type = REGCACHE_NONE,
+};
+
+static int as7343_setup_device(struct device *dev, struct as7343_data *data)
+{
+	unsigned int val;
+	u16 step;
+	int ret;
+
+	/* Power on */
+	ret = regmap_set_bits(data->regmap, AS7343_ENABLE, AS7343_ENABLE_PON);
+	if (ret)
+		return ret;
+
+	/* Need to set REG_BANK to 1 before we can access ID */
+	ret = regmap_set_bits(data->regmap, AS7343_CFG0, AS7343_CFG0_REG_BANK);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, AS7343_ID, &val);
+	if (val != 0x81)
+		dev_info(dev, "Unknown device ID: %x\n", val);
+
+	ret = regmap_clear_bits(data->regmap, AS7343_CFG0,
+				AS7343_CFG0_REG_BANK);
+	if (ret)
+		return ret;
+
+	/* Configure the SMUX to readout all channels */
+	ret = regmap_update_bits(
+		data->regmap, AS7343_CFG20, AS7343_CFG20_AUTO_SMUX,
+		FIELD_PREP(AS7343_CFG20_AUTO_SMUX,
+			   AS7343_CFG20_AUTO_SMUX_READOUT_ALL));
+	if (ret)
+		return ret;
+
+	/* Set 50.1ms integration time and x256 gain for now */
+	step = cpu_to_le16(AS7343_ASTEP_VAL);
+	ret = regmap_bulk_write(data->regmap, AS7343_ASTEP, &step, 2);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, AS7343_ATIME, AS7343_ATIME_VAL);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, AS7343_CFG1, AS7343_CFG1_AGAIN,
+				 FIELD_PREP(AS7343_CFG1_AGAIN,
+					    AS7343_CFG1_AGAIN_X256));
+	if (ret)
+		return ret;
+
+	/* Start measurements */
+	return regmap_set_bits(data->regmap, AS7343_ENABLE,
+			       AS7343_ENABLE_SP_EN);
+}
+
+static int as7343_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct as7343_data *data = iio_priv(indio_dev);
+
+	return regmap_clear_bits(data->regmap, AS7343_ENABLE,
+				 AS7343_ENABLE_SP_EN);
+}
+
+static int as7343_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct as7343_data *data = iio_priv(indio_dev);
+
+	return regmap_set_bits(data->regmap, AS7343_ENABLE,
+			       AS7343_ENABLE_SP_EN);
+}
+
+static void as7343_suspend_action(void *data)
+{
+	as7343_suspend(data);
+}
+
+static int as7343_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct as7343_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_i2c(client, &as7343_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->regmap = regmap;
+
+	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(&client->dev, "vdd");
+	if (ret)
+		return ret;
+
+	ret = as7343_setup_device(dev, data);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, as7343_suspend_action, dev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to add suspend action\n");
+
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to activate PM runtime\n");
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable PM runtime\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(as7343_pm_ops, as7343_suspend, as7343_resume,
+				 NULL);
+
+static const struct of_device_id as7343_of_match[] = {
+	{ .compatible = "ams,as7343" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, as7343_of_match);
+
+static const struct i2c_device_id as7343_id[] = {
+	{ .name = "as7343" },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, as7343_id);
+
+static struct i2c_driver as7343_driver = {
+	.driver = {
+		.name           = "as7343",
+		.of_match_table = as7343_of_match,
+		.pm             = pm_sleep_ptr(&as7343_pm_ops),
+	},
+	.probe      = as7343_probe,
+	.id_table   = as7343_id,
+};
+module_i2c_driver(as7343_driver);
+
+MODULE_AUTHOR("Chang Yu <marcus.yu.56@gmail.com>");
+MODULE_DESCRIPTION("AS7343 14 Channel Multi-Spectral Sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.55.0

Re: [PATCH v2 1/2] dt-bindings: iio: light: add as7343

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#include <dt-bindings/gpio/gpio.h>` for `GPIO_ACTIVE_LOW` in the dt-binding example.
--

commit 6fee3afaa3c4f47b1efbffef07588793c6bc1b7e
Author: Chang Yu <marcus.yu.56@gmail.com>

dt-bindings: iio: light: add as7343

This commit adds a new device tree binding for the AMS AS7343 14-channel
multi-spectral sensor.

> diff --git a/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> new file mode 100644
> index 0000000000000..b06d445b92b3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        multispectral-sensor@39 {
> +            compatible = "ams,as7343";
> +            reg = <0x39>;
> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +            vdd-supply = <&vdd_regulator>;
> +            ams,led-current-microamp = <14000>;
> +            gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;

[Severity: Low]
Does this examples block need to include <dt-bindings/gpio/gpio.h>?

This gpios assignment relies on the GPIO_ACTIVE_LOW macro. Without the
corresponding header included at the top of the examples block, the dtc
compiler will fail with a syntax error during make dt_binding_check
because the unexpanded macro will be treated as an invalid token inside
the cell array.

> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907210042.32552-1-marcus.yu.56@gmail.com?part=1

Re: [PATCH v2 2/2] iio: light: add AS7343 multi-spectral sensor driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Device configuration is not restored on system resume, breaking the sensor if power is lost.
- [Medium] The PM operations structure pointer is incorrectly wrapped with pm_sleep_ptr() instead of pm_ptr().
- [Medium] Missing PM runtime get/put in as7343_read_raw() allows reads while the device is runtime-suspended.
- [Medium] The variable 'val' is evaluated uninitialized when regmap_read() fails.
- [Medium] The device is left powered on during driver unbind and error paths.
- [Low] Incorrect base types used for endianness conversions cause Sparse warnings.
- [Low] Concurrent sysfs reads cause a race condition in as7343_read_raw() when latching and reading sensor data.
--

commit b11536303f5757f091de7d446bfca2f105210812
Author: Chang Yu <marcus.yu.56@gmail.com>

iio: light: add AS7343 multi-spectral sensor driver

This patch adds a driver for the AMS AS7343 14-channel multi-spectral
sensor with I2C interface. It exposes 12 spectral channels via IIO sysfs
and adds basic power management support.

> diff --git a/drivers/iio/light/as7343.c b/drivers/iio/light/as7343.c
> new file mode 100644
> index 0000000000000..f8ee0f2e7392d
> --- /dev/null
> +++ b/drivers/iio/light/as7343.c
[ ... ]
> +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);
> +	int ret;
> +	unsigned int unused;
> +	u16 result;

[Severity: Low]
Do these assignments cause static analysis warnings with Sparse? 'result'
is declared as u16 but assigned a __le16 value via regmap_bulk_read() and
passed to le16_to_cpu() below.

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		/*
> +		 * Reading ASTATUS latches all data registers to this read.
> +		 * We don't care about the returned saturation/gain status for
> +		 * now.
> +		 */
> +		ret = regmap_read(data->regmap, AS7343_ASTATUS, &unused);

[Severity: Medium]
Can this execute I2C reads against a suspended sensor?

The driver enables runtime PM which allows the device to be suspended
(clearing the measurement enable bit). Reading raw data without waking the
device using pm_runtime_resume_and_get() first could return stale or zero
data.

[Severity: Low]
Can concurrent sysfs reads from different channels cause a race condition
here?

The ASTATUS read latches all channels before the channel data is read.
Without a lock, could interleaved reads result in one thread receiving data
latched by another thread's ASTATUS read?

> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(data->regmap, chan->address, &result, 2);
> +		if (ret)
> +			return ret;
> +
> +		*val = le16_to_cpu(result);
> +		return IIO_VAL_INT;
> +	}
[ ... ]
> +static int as7343_setup_device(struct device *dev, struct as7343_data *data)
> +{
> +	unsigned int val;
> +	u16 step;

[Severity: Low]
Does this cause a strict typing warning with Sparse? 'step' is declared as
u16 but receives a __le16 value via cpu_to_le16() below.

> +	int ret;
> +
> +	/* Power on */
> +	ret = regmap_set_bits(data->regmap, AS7343_ENABLE, AS7343_ENABLE_PON);
> +	if (ret)
> +		return ret;
> +
> +	/* Need to set REG_BANK to 1 before we can access ID */
> +	ret = regmap_set_bits(data->regmap, AS7343_CFG0, AS7343_CFG0_REG_BANK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, AS7343_ID, &val);
> +	if (val != 0x81)
> +		dev_info(dev, "Unknown device ID: %x\n", val);

[Severity: Medium]
If regmap_read() fails, it returns an error code and leaves the stack
variable 'val' uninitialized. Could we be evaluating and printing
uninitialized memory here since the error is ignored?

> +
> +	ret = regmap_clear_bits(data->regmap, AS7343_CFG0,
> +				AS7343_CFG0_REG_BANK);
[ ... ]
> +static int as7343_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct as7343_data *data = iio_priv(indio_dev);
> +
> +	return regmap_clear_bits(data->regmap, AS7343_ENABLE,
> +				 AS7343_ENABLE_SP_EN);

[Severity: Medium]
Does this leave the device powered on indefinitely?

as7343_setup_device() sets the PON bit during probe, but this suspend
action only clears SP_EN, meaning PON remains set when the device is
unbound or suspended.

> +}
> +
> +static int as7343_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct as7343_data *data = iio_priv(indio_dev);
> +
> +	return regmap_set_bits(data->regmap, AS7343_ENABLE,
> +			       AS7343_ENABLE_SP_EN);

[Severity: High]
Does this leave the sensor in a broken state after resuming from S3 sleep?

The driver uses REGCACHE_NONE, so all volatile configurations (gain,
integration time, multiplexer) are lost when power to the sensor is lost.
This resume function only sets the SP_EN bit without reinitializing the
sensor's configuration.

> +}
[ ... ]
> +static int as7343_probe(struct i2c_client *client)
> +{
[ ... ]
> +	ret = as7343_setup_device(dev, data);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, as7343_suspend_action, dev);

[Severity: Medium]
Does this leave the device powered on during error paths?

as7343_setup_device() powers on the device, but if a subsequent I2C
operation in that function fails, this cleanup action is never registered,
leaving the hardware powered on.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to add suspend action\n");
[ ... ]
> +static struct i2c_driver as7343_driver = {
> +	.driver = {
> +		.name           = "as7343",
> +		.of_match_table = as7343_of_match,
> +		.pm             = pm_sleep_ptr(&as7343_pm_ops),

[Severity: Medium]
Does this inadvertently drop runtime PM support when CONFIG_PM_SLEEP is
disabled?

The dev_pm_ops structure defines runtime PM callbacks, but using
pm_sleep_ptr() evaluates to NULL when CONFIG_PM_SLEEP=n, even if
CONFIG_PM=y is set. Should this be wrapped with pm_ptr() instead?

> +	},
> +	.probe      = as7343_probe,
> +	.id_table   = as7343_id,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907210042.32552-1-marcus.yu.56@gmail.com?part=2

Re: [PATCH v2 2/2] iio: light: add AS7343 multi-spectral sensor driver

[Joshua Crofts]

Hi Chang,

Please check out Sashiko's review, there are some PM runtime things that
should be resolved + some comments inline.

https://sashiko.dev/#/patchset/20260907210042.32552-1-marcus.yu.56%40gmail.com

Thanks!

Josh

On Mon,  7 Sep 2026 14:00:42 -0700
Chang Yu <marcus.yu.56@gmail.com> wrote:

...

> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sysfs.h>

+ bits.h, types.h, <asm/byteorder.h> (asm headers go separately, as IIO headers).

> +
> +#include <linux/iio/iio.h>
> +

...

> +struct as7343_data {
> +	struct regmap *regmap;
> +};
> +
> +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);
> +	int ret;
> +	unsigned int unused;
> +	u16 result;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {

The device should resume here, otherwise you'll be reading while suspended.

> +		/*
> +		 * Reading ASTATUS latches all data registers to this read.
> +		 * We don't care about the returned saturation/gain status for
> +		 * now.
> +		 */

Sashiko points out that a mutex here would come in handy. If you're going to add
a mutex, use the guard(mutex) macro for automatic unlocking on scope exit.

> +		ret = regmap_read(data->regmap, AS7343_ASTATUS, &unused);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(data->regmap, chan->address, &result, 2);
> +		if (ret)
> +			return ret;
> +
> +		*val = le16_to_cpu(result);
> +		return IIO_VAL_INT;
> +	}
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const char *as7343_channel_label(struct iio_chan_spec const *chan)
> +{
> +	switch (chan->channel) {
> +	case AS7343_CHAN_IDX_FZ:
> +		return "FZ";
> +	case AS7343_CHAN_IDX_FY:
> +		return "FY";
> +	case AS7343_CHAN_IDX_FXL:
> +		return "FXL";
> +	case AS7343_CHAN_IDX_NIR:
> +		return "NIR";
> +	case AS7343_CHAN_IDX_F2:
> +		return "F2";
> +	case AS7343_CHAN_IDX_F3:
> +		return "F3";
> +	case AS7343_CHAN_IDX_F4:
> +		return "F4";
> +	case AS7343_CHAN_IDX_F6:
> +		return "F6";
> +	case AS7343_CHAN_IDX_F1:
> +		return "F1";
> +	case AS7343_CHAN_IDX_F7:
> +		return "F7";
> +	case AS7343_CHAN_IDX_F8:
> +		return "F8";
> +	case AS7343_CHAN_IDX_F5:
> +		return "F5";
> +	default:
> +		return NULL;
> +	}
> +}
> +
> +static int as7343_read_label(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, char *label)
> +{
> +	const char *name;
> +
> +	name = as7343_channel_label(chan);
> +	if (!name)
> +		return -EINVAL;

A blank line here would be better.

> +	return sysfs_emit(label, "%s\n", name);
> +}
> +
> +static const struct iio_info as7343_info = {
> +	.read_raw = as7343_read_raw,
> +	.read_label = as7343_read_label,
> +};
> +
> +static const struct regmap_config as7343_regmap_config = {
> +	.name = "as7343",
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = AS7343_MAX,
> +	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +static int as7343_setup_device(struct device *dev, struct as7343_data *data)
> +{
> +	unsigned int val;
> +	u16 step;

__le16 instead of u16.

> +	int ret;
> +
> +	/* Power on */
> +	ret = regmap_set_bits(data->regmap, AS7343_ENABLE, AS7343_ENABLE_PON);
> +	if (ret)
> +		return ret;
> +
> +	/* Need to set REG_BANK to 1 before we can access ID */
> +	ret = regmap_set_bits(data->regmap, AS7343_CFG0, AS7343_CFG0_REG_BANK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, AS7343_ID, &val);

You should check the value of ret as well in case of a regmap failure.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2 1/2] dt-bindings: iio: light: add as7343

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4446 bytes --]

On Mon, Sep 07, 2026 at 02:00:41PM -0700, Chang Yu wrote:
> Add binding for AMS AS7343 which is a 14-channel multi-spectral sensor
> with i2c address of 0x39.
> 
> The GPIO pin is described as a generic GPIO for now. Binding design for the
> more advanced measurement/LED synchronization use cases are deferred to
> future patches.

Unfortunately, you can't change what you document, so picking something
correct now is needed - even if the driver doesn't use it yet.
> 
> Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>
> ---
> Changes in v2:
>   - Add the LDR, the interrupt pin, and the GPIO pin to the bindings.
>   - Fix node name and unit address mismatch.
>   - Include MAINTAINERS changes.
> 
>  .../bindings/iio/light/ams,as7343.yaml        | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> new file mode 100644
> index 000000000000..b06d445b92b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/ams,as7343.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMS AS7343 14-Channel Multi-Spectral Sensor
> +
> +maintainers:
> +  - Chang Yu <marcus.yu.56@gmail.com>
> +
> +description: |
> +  The AMS AS7343 is a 14-channel multi-spectral sensor with i2c address of 0x39.
> +  https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ams,as7343
> +
> +  reg:
> +    description:
> +      I2C address of the device (0x39).
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Open drain output active low interrupt pin.
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +  ams,led-current-microamp:
> +    description:
> +      The driver current for the external LED connected to the LDR pin.
> +    minimum: 4000
> +    maximum: 258000
> +    multipleOf: 2000
> +    default: 12000

Rather than a custom property, the tsl2772 uses led-max-microamp:
tsl2772.yaml
46:  led-max-microamp:
81:                led-max-microamp = <100000>;

I wonder if the same should be done here, or if there should be an leds
subnode? Perhaps the IIO folks can comment on that.

> +
> +  gpios:
> +    description:
> +      Optional GPIO pin for general I/O.

Please expand on "general IO". Docs appear to suggest this is used for
synchronisation.

Additionally, I think this name will make the gpio somewhat difficult to
request. Calling it "sync-gpios" or w/e will permit the usual gpiod
functions being used.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        multispectral-sensor@39 {
> +            compatible = "ams,as7343";
> +            reg = <0x39>;
> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +            vdd-supply = <&vdd_regulator>;
> +            ams,led-current-microamp = <14000>;
> +            gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;

Valid Sashiko comment here I believe.
pw-bot: changes-requested

Thanks,
Conor.

> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04fa5322d9f7..8cf4e1635053 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1430,6 +1430,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
>  F:	drivers/iio/light/as73211.c
>  
> +AMS AS7343 DRIVER
> +M:	Chang Yu <marcus.yu.56@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> +
>  AMT (Automatic Multicast Tunneling)
>  M:	Taehee Yoo <ap420073@gmail.com>
>  L:	netdev@vger.kernel.org
> -- 
> 2.55.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/2] dt-bindings: iio: light: add as7343

[Jonathan Cameron]

On Tue, 8 Sep 2026 19:13:55 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Mon, Sep 07, 2026 at 02:00:41PM -0700, Chang Yu wrote:
> > Add binding for AMS AS7343 which is a 14-channel multi-spectral sensor
> > with i2c address of 0x39.
> > 
> > The GPIO pin is described as a generic GPIO for now. Binding design for the
> > more advanced measurement/LED synchronization use cases are deferred to
> > future patches.  
> 
> Unfortunately, you can't change what you document, so picking something
> correct now is needed - even if the driver doesn't use it yet.

Definitely needs an outline of how it would be backwards compatible and
an explanation of why not now.  Sometimes a portion of the binding is
so uncertain that we do kick it back from initial version but we 'must'
be sure we can extend the binding to new configurations.  Normally this
is one of those we are fairly sure, but not entirely sure cases - or
picking between two options where consensus isn't being reached.

Chang Yu: This sort of things needs discussion and is one of the reasons to
go slowly.

> > 
> > Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> > Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>
> > ---
> > Changes in v2:
> >   - Add the LDR, the interrupt pin, and the GPIO pin to the bindings.
> >   - Fix node name and unit address mismatch.
> >   - Include MAINTAINERS changes.
> > 
> >  .../bindings/iio/light/ams,as7343.yaml        | 69 +++++++++++++++++++
> >  MAINTAINERS                                   |  6 ++
> >  2 files changed, 75 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > new file mode 100644
> > index 000000000000..b06d445b92b3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/light/ams,as7343.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMS AS7343 14-Channel Multi-Spectral Sensor
> > +
> > +maintainers:
> > +  - Chang Yu <marcus.yu.56@gmail.com>
> > +
> > +description: |
> > +  The AMS AS7343 is a 14-channel multi-spectral sensor with i2c address of 0x39.
> > +  https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ams,as7343
> > +
> > +  reg:
> > +    description:
> > +      I2C address of the device (0x39).
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    description:
> > +      Open drain output active low interrupt pin.
> > +    maxItems: 1
> > +
> > +  vdd-supply: true
> > +
> > +  ams,led-current-microamp:
> > +    description:
> > +      The driver current for the external LED connected to the LDR pin.
> > +    minimum: 4000
> > +    maximum: 258000
> > +    multipleOf: 2000
> > +    default: 12000  
> 
> Rather than a custom property, the tsl2772 uses led-max-microamp:
> tsl2772.yaml
> 46:  led-max-microamp:
> 81:                led-max-microamp = <100000>;
> 
> I wonder if the same should be done here, or if there should be an leds
> subnode? Perhaps the IIO folks can comment on that.

I don't think we've ever bothered with a subnode as there only tends
to be one of them.  Given the enabling etc is all hardware controlled
I'm not sure a more generic LED binding makes sense. I don't know that
much about the led bindings though so maybe it is worth doing a subnode
just to use the leds/common.yaml definition of led-max-microamp?


Thanks,
Jonathan