Re: [PATCH v2 4/4] iio: light: add support for veml6031x00 ALS series

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 13/05/2026 07:49, Javier Carrasco wrote:
> These sensors provide two light channels (ALS and IR), I2C communication
> and a multiplexed interrupt line to signal data ready and configurable
> threshold alarms.

Thanks for the patch! I did only a very quick review, sorry for that. 
Overall this looks nice! I think Jonathan already asked about the 
atomics - I agree it looks like locking might be needed.

Anyways, the GTS usage (at a glance) looked nice! :) Just very minor 
questions below.

Yours,
	-- Matti

> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>   MAINTAINERS                     |    6 +
>   drivers/iio/light/Kconfig       |   14 +
>   drivers/iio/light/Makefile      |    1 +
>   drivers/iio/light/veml6031x00.c | 1193 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 1214 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2fb1c75afd16..47da46717c16 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28381,6 +28381,12 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/iio/light/vishay,veml6046x00.yaml
>   F:	drivers/iio/light/veml6046x00.c
>   
> +VISHAY VEML6031X00 AMBIENT LIGHT SENSOR DRIVER
> +M:	Javier Carrasco <javier.carrasco.cruz@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/light/vishay,veml6030.yaml

Is this needed? I was (once upon a time) asked to drop the binding doc 
file from the MAINTAINERS because the binding doc contains the 
maintainer information. Not sure if it is used by the get_maintainer.pl 
though, so maybe this is Ok?

> +F:	drivers/iio/light/veml6031x00.c
> +
>   VISHAY VEML6075 UVA AND UVB 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..b99f4e8d9a70 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -713,6 +713,20 @@ config VEML6030
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called veml6030.
>   
> +config VEML6031X00
> +	tristate "VEML6031X00 ambient light sensor series"
> +	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select IIO_GTS_HELPER
> +	depends on I2C
> +	help
> +	  Say Y here if you want to build a driver for the Vishay VEML6031X00
> +	  ambient light sensor series.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called veml6031x00.
> +
>   config VEML6040
>   	tristate "VEML6040 RGBW light sensor"
>   	select REGMAP_I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index c0048e0d5ca8..a8cc03cfb6c2 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
>   obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
>   obj-$(CONFIG_VEML3235)		+= veml3235.o
>   obj-$(CONFIG_VEML6030)		+= veml6030.o
> +obj-$(CONFIG_VEML6031X00)	+= veml6031x00.o
>   obj-$(CONFIG_VEML6040)		+= veml6040.o
>   obj-$(CONFIG_VEML6046X00)	+= veml6046x00.o
>   obj-$(CONFIG_VEML6070)		+= veml6070.o
> diff --git a/drivers/iio/light/veml6031x00.c b/drivers/iio/light/veml6031x00.c
> new file mode 100644
> index 000000000000..c7808768f45a
> --- /dev/null
> +++ b/drivers/iio/light/veml6031x00.c
> @@ -0,0 +1,1193 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VEML6031X00 Ambient Light Sensor
> + *
> + * Copyright (c) 2026, Javier Carrasco <javier.carrasco.cruz@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/units.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/iio-gts-helper.h>
> +
> +/* Device registers */
> +#define VEML6031X00_REG_CONF0       0x00
> +#define VEML6031X00_REG_CONF1       0x01
> +#define VEML6031X00_REG_WH_L        0x04
> +#define VEML6031X00_REG_WH_H        0x05
> +#define VEML6031X00_REG_WL_L        0x06
> +#define VEML6031X00_REG_WL_H        0x07
> +#define VEML6031X00_REG_ALS_L       0x10
> +#define VEML6031X00_REG_ALS_H       0x11
> +#define VEML6031X00_REG_IR_L        0x12
> +#define VEML6031X00_REG_IR_H        0x13
> +#define VEML6031X00_REG_ID_L        0x14
> +#define VEML6031X00_REG_ID_H        0x15
> +#define VEML6031X00_REG_INT         0x17
> +
> +/* Bit masks for specific functionality */
> +#define VEML6031X00_ALL_CH_MASK     GENMASK(1, 0)
> +#define VEML6031X00_CONF0_SD        BIT(0)
> +#define VEML6031X00_CONF0_AF_TRIG   BIT(2)
> +#define VEML6031X00_CONF0_AF        BIT(3)
> +#define VEML6031X00_CONF1_IR_SD     BIT(7)
> +#define VEML6031X00_INT_TH_H        BIT(1)
> +#define VEML6031X00_INT_TH_L        BIT(2)
> +#define VEML6031X00_INT_DRDY        BIT(3)
> +#define VEML6031X00_INT_MASK        (VEML6031X00_INT_TH_L | \
> +				     VEML6031X00_INT_TH_H | \
> +				     VEML6031X00_INT_DRDY)
> +
> +/* Autosuspend delay */
> +#define VEML6031X00_AUTOSUSPEND_MS  2000
> +
> +enum veml6031x00_scan {
> +	VEML6031X00_SCAN_ALS,
> +	VEML6031X00_SCAN_IR,
> +	VEML6031X00_SCAN_TIMESTAMP,
> +};
> +
> +struct veml6031x00_rf {
> +	struct regmap_field *gain;
> +	struct regmap_field *int_en;
> +	struct regmap_field *it;
> +	struct regmap_field *pd_div4;
> +	struct regmap_field *pers;
> +};
> +
> +struct veml6031x00_chip {
> +	const char *name;
> +	const int part_id;
> +};
> +
> +struct veml6031x00_data {
> +	struct device *dev;
> +	struct iio_gts gts;
> +	struct regmap *regmap;
> +	struct iio_trigger *trig;
> +	struct veml6031x00_rf rf;
> +	const struct veml6031x00_chip *chip;
> +	/* serialize access to irq enable/disable by events and trigger */
> +	struct mutex lock;
> +	atomic_t int_users;
> +	bool ev_en;
> +	bool trig_en;
> +};
> +
> +static const struct iio_itime_sel_mul veml6031x00_it_sel[] = {
> +	GAIN_SCALE_ITIME_US(3125, 0, 1),
> +	GAIN_SCALE_ITIME_US(6250, 1, 2),
> +	GAIN_SCALE_ITIME_US(12500, 2, 4),
> +	GAIN_SCALE_ITIME_US(25000, 3, 8),
> +	GAIN_SCALE_ITIME_US(50000, 4, 16),
> +	GAIN_SCALE_ITIME_US(100000, 5, 32),
> +	GAIN_SCALE_ITIME_US(200000, 6, 64),
> +	GAIN_SCALE_ITIME_US(400000, 7, 128),
> +};
> +
> +/*
> + * The gain selector encodes (PD_D4 << 2) | GAIN to identify each gain setting.
> + * Gains are multiplied by 8 to work with integers. The values in the iio-gts
> + * tables don't need corrections because the maximum value of the scale refers
> + * to GAIN = x1, and the rest of the values are obtained from the resulting
> + * linear function.
> + * TODO: add support for MILLI_GAIN_X165 and MILLI_GAIN_X660
> + */
> +#define VEML6031X00_SEL_MILLI_GAIN_X125  0x07
> +#define VEML6031X00_SEL_MILLI_GAIN_X250  0x04
> +#define VEML6031X00_SEL_MILLI_GAIN_X500  0x03
> +#define VEML6031X00_SEL_MILLI_GAIN_X1000 0x00
> +#define VEML6031X00_SEL_MILLI_GAIN_X2000 0x01
> +static const struct iio_gain_sel_pair veml6031x00_gain_sel[] = {
> +	GAIN_SCALE_GAIN(1,  VEML6031X00_SEL_MILLI_GAIN_X125),
> +	GAIN_SCALE_GAIN(2,  VEML6031X00_SEL_MILLI_GAIN_X250),
> +	GAIN_SCALE_GAIN(4, VEML6031X00_SEL_MILLI_GAIN_X500),
> +	GAIN_SCALE_GAIN(8, VEML6031X00_SEL_MILLI_GAIN_X1000),
> +	GAIN_SCALE_GAIN(16, VEML6031X00_SEL_MILLI_GAIN_X2000),
> +};
> +
> +static IIO_CONST_ATTR(in_illuminance_thresh_either_period_available, "1 2 4 8");
> +
> +static struct attribute *veml6031x00_event_attributes[] = {
> +	&iio_const_attr_in_illuminance_thresh_either_period_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group veml6031x00_event_attr_group = {
> +	.attrs = veml6031x00_event_attributes,
> +};
> +
> +/*
> + * Two shutdown bits (SD and ALS_IR_SD) must be cleared to power on
> + * the device.
> + */
> +static int veml6031x00_als_power_on(struct veml6031x00_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_clear_bits(data->regmap, VEML6031X00_REG_CONF0,
> +				VEML6031X00_CONF0_SD);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_clear_bits(data->regmap, VEML6031X00_REG_CONF1,
> +				 VEML6031X00_CONF1_IR_SD);
> +}
> +
> +/*
> + * Two shutdown bits (SD and ALS_IR_SD) must be set to power off
> + * the device.
> + */
> +static int veml6031x00_als_shutdown(struct veml6031x00_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_set_bits(data->regmap, VEML6031X00_REG_CONF0,
> +			      VEML6031X00_CONF0_SD);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_set_bits(data->regmap, VEML6031X00_REG_CONF1,
> +			       VEML6031X00_CONF1_IR_SD);
> +}
> +
> +static void veml6031x00_als_shutdown_action(void *data)
> +{
> +	veml6031x00_als_shutdown(data);
> +}

I wonder if the register values are maintained when the device is 
powered down? If not, then the regmap cache should be invalidated.

Yours,
	-- Matti

---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~