Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/2] iio: light: veml3328: add support for new sensor
@ 2026-05-20 21:15 Joshua Crofts
  2026-05-20 21:15 ` [PATCH v2 1/2] dt-bindings: iio: light: veml6030: add veml3328 Joshua Crofts
  2026-05-20 21:15 ` [PATCH v2 2/2] iio: light: veml3328: add support for new device Joshua Crofts
  0 siblings, 2 replies; 4+ messages in thread
From: Joshua Crofts @ 2026-05-20 21:15 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Javier Carrasco,
	Rishi Gupta
  Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts

This patch series adds support for the Vishay VEML3328 RGBCIR light
sensor. The sensor communicates via I2C (SMBus compatible) and provides
5 types of 16-bit measurements: red, green, blue, clear and infrared.

Reasons for adding a new driver:
- Existing Vishay drivers in the kernel do not cover sensors that
  handle RGBC and IR simultaneously.
- The register map and configuration differ from other Vishay light
  sensors currently supported by IIO.

Testing:
- Tested on a Raspberry Pi 4 using a VEML3328 breakout board.

Datasheet:
https://www.vishay.com/docs/84968/veml3328.pdf

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v2:
- Add additional IIO_LIGHT channel for ambient light sensing
- Remove separate dt binding file and added veml3328 entry to veml6030
  yaml
- Move driver to PM_RUNTIME_ACQUIRE_AUTOSUSPEND() macro
- Add missing headers
- Remov redundant mutex as regmap handles it itself
- Use regmap_set/clear_bits() instead of regmap_update_bits()
- Removed redundant dev pointer
- Edit commit messages
- Various code style cleanups
- Link to v1: https://patch.msgid.link/20260516-veml3328-v1-0-1d4b663e2fe3@gmail.com

To: Javier Carrasco <javier.carrasco.cruz@gmail.com>
To: Joshua Crofts <joshua.crofts1@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Rishi Gupta <gupt21@gmail.com>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Joshua Crofts (2):
      dt-bindings: iio: light: veml6030: add veml3328
      iio: light: veml3328: add support for new device

 .../bindings/iio/light/vishay,veml6030.yaml        |   5 +-
 MAINTAINERS                                        |   5 +
 drivers/iio/light/Kconfig                          |  11 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/veml3328.c                       | 416 +++++++++++++++++++++
 5 files changed, 437 insertions(+), 1 deletion(-)
---
base-commit: 74d173f29572951629d1e0b7456b424006e51b87
change-id: 20260426-veml3328-5bf93f85198e

Best regards,
--  
Joshua Crofts <joshua.crofts1@gmail.com>


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

* [PATCH v2 1/2] dt-bindings: iio: light: veml6030: add veml3328
  2026-05-20 21:15 [PATCH v2 0/2] iio: light: veml3328: add support for new sensor Joshua Crofts
@ 2026-05-20 21:15 ` Joshua Crofts
  2026-05-20 21:15 ` [PATCH v2 2/2] iio: light: veml3328: add support for new device Joshua Crofts
  1 sibling, 0 replies; 4+ messages in thread
From: Joshua Crofts @ 2026-05-20 21:15 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Javier Carrasco,
	Rishi Gupta
  Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts

The Vishay VEML3328 is an RGBCIR light sensor that shares similar
devicetree properties as other existing VEMLxxxx sensors in the
kernel.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
index 4ea69f1fdd63..0041e1db6838 100644
--- a/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
+++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/light/vishay,veml6030.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: VEML3235, VEML6030, VEML6035 and VEML7700 Ambient Light Sensors (ALS)
+title: VEML3235, VEML3328, VEML6030, VEML6035 and VEML7700 Ambient Light Sensors (ALS)
 
 maintainers:
   - Rishi Gupta <gupt21@gmail.com>
@@ -21,6 +21,7 @@ description: |
 
   Specifications about the sensors can be found at:
     https://www.vishay.com/docs/80131/veml3235.pdf
+    https://www.vishay.com/docs/84968/veml3328.pdf
     https://www.vishay.com/docs/84366/veml6030.pdf
     https://www.vishay.com/docs/84889/veml6035.pdf
     https://www.vishay.com/docs/84286/veml7700.pdf
@@ -29,6 +30,7 @@ properties:
   compatible:
     enum:
       - vishay,veml3235
+      - vishay,veml3328
       - vishay,veml6030
       - vishay,veml6035
       - vishay,veml7700
@@ -79,6 +81,7 @@ allOf:
         compatible:
           enum:
             - vishay,veml3235
+            - vishay,veml3328
             - vishay,veml7700
     then:
       properties:

-- 
2.34.1


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

* [PATCH v2 2/2] iio: light: veml3328: add support for new device
  2026-05-20 21:15 [PATCH v2 0/2] iio: light: veml3328: add support for new sensor Joshua Crofts
  2026-05-20 21:15 ` [PATCH v2 1/2] dt-bindings: iio: light: veml6030: add veml3328 Joshua Crofts
@ 2026-05-20 21:15 ` Joshua Crofts
  2026-05-20 22:00   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Joshua Crofts @ 2026-05-20 21:15 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Javier Carrasco,
	Rishi Gupta
  Cc: linux-iio, devicetree, linux-kernel, Joshua Crofts

Add support for the Vishay VEML3328 RGB/IR light sensor communicating
via I2C (SMBus compatible).

Also add a new entry for said driver into Kconfig and Makefile.

Assisted-by: Gemini:3.1-Pro
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 MAINTAINERS                  |   5 +
 drivers/iio/light/Kconfig    |  11 ++
 drivers/iio/light/Makefile   |   1 +
 drivers/iio/light/veml3328.c | 416 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 433 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6c3c7d22403..8236ba0c41ca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28386,6 +28386,11 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml
 F:	drivers/iio/light/veml3235.c
 
+VISHAY VEML3328 RGB IR LIGHT SENSOR DRIVER
+M:	Joshua Crofts <joshua.crofts1@gmail.com>
+S:	Maintained
+F:	drivers/iio/light/veml3328.c
+
 VISHAY VEML6030 AMBIENT LIGHT SENSOR DRIVER
 M:	Javier Carrasco <javier.carrasco.cruz@gmail.com>
 S:	Maintained
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index eff33e456c70..1c00bb0de6c8 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -699,6 +699,17 @@ config VEML3235
 	  To compile this driver as a module, choose M here: the
 	  module will be called veml3235.
 
+config VEML3328
+	tristate "VEML3328 RGBCIR light sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VEML3328
+	  RGB IR light sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called veml3328
+
 config VEML6030
 	tristate "VEML6030 and VEML6035 ambient light sensors"
 	select REGMAP_I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index c0048e0d5ca8..e03c6e23a9a4 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_US5182D)		+= us5182d.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
 obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
 obj-$(CONFIG_VEML3235)		+= veml3235.o
+obj-$(CONFIG_VEML3328)		+= veml3328.o
 obj-$(CONFIG_VEML6030)		+= veml6030.o
 obj-$(CONFIG_VEML6040)		+= veml6040.o
 obj-$(CONFIG_VEML6046X00)	+= veml6046x00.o
diff --git a/drivers/iio/light/veml3328.c b/drivers/iio/light/veml3328.c
new file mode 100644
index 000000000000..fdfa8fd1666b
--- /dev/null
+++ b/drivers/iio/light/veml3328.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Vishay VEML3328 RGBCIR light sensor driver
+ *
+ * Copyright (c) 2026 Joshua Crofts <joshua.crofts1@gmail.com>
+ *
+ * Datasheet: https://www.vishay.com/docs/84968/veml3328.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/time.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+#define VEML3328_REG_CONF		0x00
+#define VEML3328_REG_ID			0x0c
+#define VEML3328_REG_DATA_C		0x04
+#define VEML3328_REG_DATA_R		0x05
+#define VEML3328_REG_DATA_G		0x06
+#define VEML3328_REG_DATA_B		0x07
+#define VEML3328_REG_DATA_IR		0x08
+
+#define VEML3328_CONF_IT_MASK		GENMASK(5, 4)
+#define VEML3328_CONF_GAIN_MASK		GENMASK(11, 10)
+
+#define VEML3328_MAX_IT_TIME		(400 * USEC_PER_MSEC)
+
+#define VEML3328_ID_VAL			0x28
+
+#define VEML3328_SHUTDOWN		(BIT(0) | BIT(15))
+
+struct veml3328_data {
+	struct regmap *regmap;
+};
+
+static const struct regmap_config veml3328_regmap_config = {
+	.name = "veml3328",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = VEML3328_REG_ID,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+#define VEML3328_CHAN_SPEC(_color, _addr) { \
+	.type = IIO_INTENSITY, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_LIGHT_##_color, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) | \
+				   BIT(IIO_CHAN_INFO_SCALE), \
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) | \
+					     BIT(IIO_CHAN_INFO_SCALE), \
+	.address = _addr, \
+}
+
+static const struct iio_chan_spec veml3328_channels[] = {
+	{
+		.type = IIO_LIGHT,
+
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+		.address = VEML3328_REG_DATA_G,
+	},
+	VEML3328_CHAN_SPEC(CLEAR, VEML3328_REG_DATA_C),
+	VEML3328_CHAN_SPEC(RED, VEML3328_REG_DATA_R),
+	VEML3328_CHAN_SPEC(GREEN, VEML3328_REG_DATA_G),
+	VEML3328_CHAN_SPEC(BLUE, VEML3328_REG_DATA_B),
+	VEML3328_CHAN_SPEC(IR, VEML3328_REG_DATA_IR),
+};
+
+/* scale values per datasheet */
+static const int veml3328_scale_vals[][2] = {
+	{ 0, 500000 },
+	{ 1, 0 },
+	{ 2, 0 },
+	{ 4, 0 },
+};
+
+/* integration times in microseconds */
+static const int veml3328_it_times[][2] = {
+	{ 0, 50 * USEC_PER_MSEC },
+	{ 0, 100 * USEC_PER_MSEC },
+	{ 0, 200 * USEC_PER_MSEC },
+	{ 0, 400 * USEC_PER_MSEC },
+};
+
+static int veml3328_power_down(struct veml3328_data *data)
+{
+	return regmap_set_bits(data->regmap, VEML3328_REG_CONF,
+			       VEML3328_SHUTDOWN);
+}
+
+static int veml3328_power_up(struct veml3328_data *data)
+{
+	int ret;
+
+	ret = regmap_clear_bits(data->regmap, VEML3328_REG_CONF,
+				VEML3328_SHUTDOWN);
+	if (ret)
+		return ret;
+
+	/*
+	 * Sleep for maximum integration time to ensure sensor is powered on
+	 * correctly.
+	 */
+	fsleep(VEML3328_MAX_IT_TIME);
+
+	return 0;
+}
+
+static void veml3328_power_down_action(void *data)
+{
+	veml3328_power_down(data);
+}
+
+static int veml3328_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct veml3328_data *data = iio_priv(indio_dev);
+	struct regmap *regmap = data->regmap;
+	struct device *dev = regmap_get_device(regmap);
+	unsigned int reg_val;
+	int ret;
+	int reg;
+
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(regmap, chan->address, &reg_val);
+		if (ret)
+			return ret;
+
+		*val = reg_val;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_INT_TIME:
+		ret = regmap_read(regmap, VEML3328_REG_CONF, &reg_val);
+		if (ret)
+			return ret;
+
+		reg = FIELD_GET(VEML3328_CONF_IT_MASK, reg_val);
+		if (reg >= ARRAY_SIZE(veml3328_it_times))
+			return -EINVAL;
+
+		*val = veml3328_it_times[reg][0];
+		*val2 = veml3328_it_times[reg][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regmap_read(regmap, VEML3328_REG_CONF, &reg_val);
+		if (ret)
+			return ret;
+
+		if (chan->type == IIO_LIGHT) {
+			int it_inx, gain_inx;
+			int it_us, scale_u;
+
+			it_inx = FIELD_GET(VEML3328_CONF_IT_MASK, reg_val);
+			gain_inx = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
+
+			if (it_inx >= ARRAY_SIZE(veml3328_it_times) ||
+			    gain_inx >= ARRAY_SIZE(veml3328_scale_vals))
+				return -EINVAL;
+
+			it_us = veml3328_it_times[it_inx][1];
+
+			/*
+			 * Equation for calculating ambient light scale:
+			 * Scale = 0.384 * (50000 / Current_IT) * (1x / Current_Gain)
+			 */
+			scale_u = 384 * USEC_PER_MSEC;
+			scale_u = scale_u  * 50000 / it_us;
+
+			switch (gain_inx) {
+			case 0:
+				scale_u *= 2;
+				break;
+			case 1:
+				break;
+			case 2:
+				scale_u /= 2;
+				break;
+			case 3:
+				scale_u /= 4;
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			*val = 0;
+			*val2 = scale_u;
+
+			return IIO_VAL_INT_PLUS_MICRO;
+		}
+
+		reg = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
+		if (reg >= ARRAY_SIZE(veml3328_scale_vals))
+			return -EINVAL;
+
+		*val = veml3328_scale_vals[reg][0];
+		*val2 = veml3328_scale_vals[reg][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml3328_read_avail(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       const int **vals, int *type, int *length,
+			       long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		*length = ARRAY_SIZE(veml3328_it_times) * 2;
+		*vals = (const int *)veml3328_it_times;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_SCALE:
+		*length = ARRAY_SIZE(veml3328_scale_vals) * 2;
+		*vals = (const int *)veml3328_scale_vals;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml3328_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct veml3328_data *data = iio_priv(indio_dev);
+	struct regmap *regmap = data->regmap;
+	struct device *dev = regmap_get_device(regmap);
+	int ret;
+	int i;
+
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0)
+			return -EINVAL;
+
+		for (i = 0; i < ARRAY_SIZE(veml3328_it_times); i++) {
+			if (veml3328_it_times[i][1] == val2)
+				break;
+		}
+
+		if (i == ARRAY_SIZE(veml3328_it_times))
+			return -EINVAL;
+
+		return regmap_update_bits(regmap, VEML3328_REG_CONF,
+					  VEML3328_CONF_IT_MASK,
+					  FIELD_PREP(VEML3328_CONF_IT_MASK, i));
+
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < ARRAY_SIZE(veml3328_scale_vals); i++) {
+			if (val == veml3328_scale_vals[i][0] &&
+			    val2 == veml3328_scale_vals[i][1])
+				break;
+		}
+
+		if (i == ARRAY_SIZE(veml3328_scale_vals))
+			return -EINVAL;
+
+		return regmap_update_bits(regmap, VEML3328_REG_CONF,
+					  VEML3328_CONF_GAIN_MASK,
+					  FIELD_PREP(VEML3328_CONF_GAIN_MASK, i));
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info veml3328_info = {
+	.read_raw = veml3328_read_raw,
+	.write_raw = veml3328_write_raw,
+	.read_avail = veml3328_read_avail,
+};
+
+static int veml3328_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct veml3328_data *data;
+	struct iio_dev *indio_dev;
+	unsigned int reg_val;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+
+	data->regmap = devm_regmap_init_i2c(client, &veml3328_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "Failed to initialize regmap\n");
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulator\n");
+
+	ret = regmap_read(data->regmap, VEML3328_REG_ID, &reg_val);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read ID register\n");
+
+	if ((reg_val & 0xff) != VEML3328_ID_VAL)
+		dev_warn(dev, "Unknown device ID: 0x%02x\n", reg_val & 0xff);
+
+	ret = veml3328_power_up(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to power on sensor\n");
+
+	ret = devm_add_action_or_reset(dev, veml3328_power_down_action, data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register teardown\n");
+
+	indio_dev->name = "veml3328";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &veml3328_info;
+	indio_dev->channels = veml3328_channels;
+	indio_dev->num_channels = ARRAY_SIZE(veml3328_channels);
+
+	pm_runtime_set_active(dev);
+	pm_runtime_set_autosuspend_delay(dev, 2000);
+	pm_runtime_use_autosuspend(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int veml3328_runtime_suspend(struct device *dev)
+{
+	struct veml3328_data *data = iio_priv(dev_get_drvdata(dev));
+	int ret;
+
+	ret = veml3328_power_down(data);
+	if (ret)
+		dev_err(dev, "Failed to suspend: %d\n", ret);
+
+	return ret;
+}
+
+static int veml3328_runtime_resume(struct device *dev)
+{
+	struct veml3328_data *data = iio_priv(dev_get_drvdata(dev));
+	int ret;
+
+	ret = veml3328_power_up(data);
+	if (ret)
+		dev_err(dev, "Failed to resume: %d\n", ret);
+
+	return ret;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(veml3328_pm_ops,
+				 veml3328_runtime_suspend,
+				 veml3328_runtime_resume,
+				 NULL);
+
+static const struct of_device_id veml3328_of_match[] = {
+	{ .compatible = "vishay,veml3328" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, veml3328_of_match);
+
+static const struct i2c_device_id veml3328_id[] = {
+	{ .name = "veml3328" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, veml3328_id);
+
+static struct i2c_driver veml3328_driver = {
+	.driver = {
+		.name = "veml3328",
+		.of_match_table = veml3328_of_match,
+		.pm = pm_ptr(&veml3328_pm_ops),
+	},
+	.probe = veml3328_probe,
+	.id_table = veml3328_id,
+};
+module_i2c_driver(veml3328_driver);
+
+MODULE_AUTHOR("Joshua Crofts <joshua.crofts1@gmail.com>");
+MODULE_DESCRIPTION("VEML3328 RGBCIR Light Sensor");
+MODULE_LICENSE("GPL");

-- 
2.34.1


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

* Re: [PATCH v2 2/2] iio: light: veml3328: add support for new device
  2026-05-20 21:15 ` [PATCH v2 2/2] iio: light: veml3328: add support for new device Joshua Crofts
@ 2026-05-20 22:00   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-05-20 22:00 UTC (permalink / raw)
  To: Joshua Crofts; +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Signed 32-bit integer overflow in the lux scale calculation due to large multiplication.
- [Medium] Inconsistent handling of the IIO_LIGHT channel scale across read, write, and available attributes breaks the IIO ABI.
- [Medium] The scale attribute for IIO_INTENSITY channels incorrectly returns the hardware gain multiplier instead of the true IIO scale factor.
--

commit 9edb449d4e1325159429ae7e39210b37d222a175
Author: Joshua Crofts <joshua.crofts1@gmail.com>

iio: light: veml3328: add support for new device

Add support for the Vishay VEML3328 RGB/IR light sensor communicating
via I2C (SMBus compatible).

> diff --git a/drivers/iio/light/veml3328.c b/drivers/iio/light/veml3328.c
> new file mode 100644
> index 0000000000000..fdfa8fd1666b6
> --- /dev/null
> +++ b/drivers/iio/light/veml3328.c

[ ... ]

> +static const struct iio_chan_spec veml3328_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +		.address = VEML3328_REG_DATA_G,
> +	},

[Severity: Medium]
Is the in_illuminance_scale_available sysfs attribute missing here because
info_mask_shared_by_all_available does not include the scale bit for the
IIO_LIGHT channel?

[ ... ]

> +static int veml3328_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{

[ ... ]

> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regmap_read(regmap, VEML3328_REG_CONF, &reg_val);
> +		if (ret)
> +			return ret;
> +
> +		if (chan->type == IIO_LIGHT) {
> +			int it_inx, gain_inx;
> +			int it_us, scale_u;
> +
> +			it_inx = FIELD_GET(VEML3328_CONF_IT_MASK, reg_val);
> +			gain_inx = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
> +
> +			if (it_inx >= ARRAY_SIZE(veml3328_it_times) ||
> +			    gain_inx >= ARRAY_SIZE(veml3328_scale_vals))
> +				return -EINVAL;
> +
> +			it_us = veml3328_it_times[it_inx][1];
> +
> +			/*
> +			 * Equation for calculating ambient light scale:
> +			 * Scale = 0.384 * (50000 / Current_IT) * (1x / Current_Gain)
> +			 */
> +			scale_u = 384 * USEC_PER_MSEC;
> +			scale_u = scale_u  * 50000 / it_us;

[Severity: High]
Does this code overflow scale_u?

Since scale_u is initially set to 384,000 (384 * 1000) and is a 32-bit signed
integer, the multiplication by 50000 evaluates to 19,200,000,000. This exceeds
the maximum positive value of 2,147,483,647 for a signed 32-bit int.

> +			switch (gain_inx) {

[ ... ]

> +		}
> +
> +		reg = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
> +		if (reg >= ARRAY_SIZE(veml3328_scale_vals))
> +			return -EINVAL;
> +
> +		*val = veml3328_scale_vals[reg][0];
> +		*val2 = veml3328_scale_vals[reg][1];
> +		return IIO_VAL_INT_PLUS_MICRO;

[Severity: Medium]
For IIO_INTENSITY channels, this returns the hardware gain multiplier directly
from veml3328_scale_vals. Will this instruct userspace to multiply an already
increased raw value by the hardware gain again?

Should the scale instead return the inverse (1 / gain) to correctly normalize
the readings?

[ ... ]

> +static int veml3328_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{

[ ... ]

> +	case IIO_CHAN_INFO_SCALE:
> +		for (i = 0; i < ARRAY_SIZE(veml3328_scale_vals); i++) {
> +			if (val == veml3328_scale_vals[i][0] &&
> +			    val2 == veml3328_scale_vals[i][1])
> +				break;
> +		}

[Severity: Medium]
For the IIO_LIGHT channel, veml3328_read_raw() dynamically calculates a lux
scale, but this write function compares the input against the raw hardware
gain multipliers in veml3328_scale_vals without verifying the channel type.

Does this mean writing the read scale value back will fail with -EINVAL?

Writing a valid gain value here seems to succeed and modify the hardware
gain, which would change the lux scale to something entirely different.
Could this inconsistent behavior break the expected IIO ABI principle of
being able to write back the values that were read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-veml3328-v2-0-a57f0d9e28d4@gmail.com?part=2

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

end of thread, other threads:[~2026-05-20 22:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-20 21:15 [PATCH v2 0/2] iio: light: veml3328: add support for new sensor Joshua Crofts
2026-05-20 21:15 ` [PATCH v2 1/2] dt-bindings: iio: light: veml6030: add veml3328 Joshua Crofts
2026-05-20 21:15 ` [PATCH v2 2/2] iio: light: veml3328: add support for new device Joshua Crofts
2026-05-20 22:00   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox