[PATCH 0/2] iio: light: add support for VEML6075 UVA and UVB light sensor

[PATCH 0/2] iio: light: add support for VEML6075 UVA and UVB light sensor

[Javier Carrasco]

This series adds support for the Vishay VEML6075 ultraviolet sensor,
which offers UVA and UVB measurement channels and I2C communication.

The device bindings and a simple example are also provided.

This driver has been tested with a Gravity VEML6075 UV Sensor Module in
open air conditions.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (2):
      iio: light: add VEML6075 UVA and UVB light sensor driver
      dt-bindings: iio: light: add support for Vishay VEML6075

 .../bindings/iio/light/vishay,veml6075.yaml        |  40 ++
 MAINTAINERS                                        |   6 +
 drivers/iio/light/Kconfig                          |  11 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/veml6075.c                       | 503 +++++++++++++++++++++
 5 files changed, 561 insertions(+)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231110-veml6075-321522ceaca9

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Javier Carrasco]

The Vishay VEMl6075 is a low power, 16-bit resolution UVA and UVB
light sensor with I2C interface and noise compensation (visible and
infrarred).

Every UV channel generates an output measured in counts per integration
period. Available integration times are 50 ms, 100 ms, 200 ms, 400 ms
and 800 ms,

This driver adds support for both UV channels and the ultraviolet
index (UVI) inferred from them according to the device application note
with open-air (no teflon) coefficients.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 MAINTAINERS                  |   6 +
 drivers/iio/light/Kconfig    |  11 +
 drivers/iio/light/Makefile   |   1 +
 drivers/iio/light/veml6075.c | 503 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 521 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..2f13a5088d41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23184,6 +23184,12 @@ S:	Maintained
 F:	drivers/input/serio/userio.c
 F:	include/uapi/linux/userio.h
 
+VISHAY VEML6075 UVA AND UVB LIGHT SENSOR DRIVER
+M:	Javier Carrasco <javier.carrasco.cruz@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/light/veml6075.yaml
+F:	drivers/iio/light/veml6075.c
+
 VISL VIRTUAL STATELESS DECODER DRIVER
 M:	Daniel Almeida <daniel.almeida@collabora.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 45edba797e4c..f68e62196bc2 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -637,6 +637,17 @@ config VEML6070
 	  To compile this driver as a module, choose M here: the
 	  module will be called veml6070.
 
+config VEML6075
+	tristate "VEML6075 UVA and UVB light sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VEML6075 UVA
+	  and UVB light sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called veml6075.
+
 config VL6180
 	tristate "VL6180 ALS, range and proximity sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index c0db4c4c36ec..c8289e24e3f6 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -60,5 +60,6 @@ obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
 obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
 obj-$(CONFIG_VEML6030)		+= veml6030.o
 obj-$(CONFIG_VEML6070)		+= veml6070.o
+obj-$(CONFIG_VEML6075)		+= veml6075.o
 obj-$(CONFIG_VL6180)		+= vl6180.o
 obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c
new file mode 100644
index 000000000000..b7d9319c3906
--- /dev/null
+++ b/drivers/iio/light/veml6075.c
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * veml6075.c - Support for Vishay VEML6075 UVA and UVB light sensor
+ *
+ * Copyright 2023 Javier Carrasco <javier.carrasco.cruz@gmail.com>
+ *
+ * IIO driver for VEML6075 (7-bit I2C slave address 0x10)
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define VEML6075_DRIVER_NAME "veml6075"
+
+#define VEML6075_CMD_CONF	0x00 /* configuration register */
+#define VEML6075_CMD_UVA	0x07 /* UVA channel */
+#define VEML6075_CMD_UVB	0x09 /* UVB channel */
+#define VEML6075_CMD_COMP1	0x0A /* visible light compensation */
+#define VEML6075_CMD_COMP2	0x0B /* infrarred light compensation */
+#define VEML6075_CMD_ID		0x0C /* device ID */
+
+#define VEML6075_CONF_IT	GENMASK(6, 4) /* intregration time */
+#define VEML6075_CONF_HD	BIT(3) /* dynamic setting */
+#define VEML6075_CONF_TRIG	BIT(2) /* trigger */
+#define VEML6075_CONF_AF	BIT(1) /* active force enable */
+#define VEML6075_CONF_SD	BIT(0) /* shutdown */
+
+#define VEML6075_CONF_IT_50	0x00 /* integration time 50 ms */
+#define VEML6075_CONF_IT_100	0x01 /* integration time 100 ms */
+#define VEML6075_CONF_IT_200	0x02 /* integration time 200 ms */
+#define VEML6075_CONF_IT_400	0x03 /* integration time 400 ms */
+#define VEML6075_CONF_IT_800	0x04 /* integration time 800 ms */
+
+/* Open-air coefficients and responsivity */
+#define VEML6075_A_COEF		2220
+#define VEML6075_B_COEF		1330
+#define VEML6075_C_COEF		2950
+#define VEML6075_D_COEF		1740
+#define VEML6075_UVA_RESP	1461
+#define VEML6075_UVB_RESP	2591
+
+static const int veml6075_it_ms[] = { 50, 100, 200, 400, 800 };
+static const char veml6075_it_ms_avail[] = "50 100 200 400 800";
+
+struct veml6075_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct mutex lock; /* register access lock */
+};
+
+/* channel number */
+enum veml6075_chan {
+	CH_UVA,
+	CH_UVB,
+};
+
+static const struct iio_chan_spec veml6075_channels[] = {
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_UVA,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_UV,
+		.extend_name = "UVA",
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_UVB,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_UV,
+		.extend_name = "UVB",
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+	{
+		.type = IIO_UVINDEX,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+};
+
+static IIO_CONST_ATTR_INT_TIME_AVAIL(veml6075_it_ms_avail);
+
+static struct attribute *veml6075_attributes[] = {
+	&iio_const_attr_integration_time_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group veml6075_attribute_group = {
+	.attrs = veml6075_attributes,
+};
+
+static int veml6075_shutdown(struct veml6075_data *data)
+{
+	return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
+				  VEML6075_CONF_SD, VEML6075_CONF_SD);
+}
+
+static int veml6075_request_measurement(struct veml6075_data *data)
+{
+	int ret, conf, int_time;
+
+	ret = regmap_read(data->regmap, VEML6075_CMD_CONF, &conf);
+	if (ret < 0)
+		return ret;
+
+	/* disable shutdown and trigger measurement */
+	ret = regmap_write(data->regmap, VEML6075_CMD_CONF,
+			   (conf | VEML6075_CONF_TRIG) & ~VEML6075_CONF_SD);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * A measurement requires between 1.30 and 1.40 times the integration
+	 * time for all possible configurations. Using a 1.50 factor simplifies
+	 * operations and ensures reliability under all circumstances.
+	 */
+	int_time = veml6075_it_ms[FIELD_GET(VEML6075_CONF_IT, conf)];
+	msleep(int_time + (int_time / 2));
+
+	/* shutdown again, data registers are still accessible */
+	return veml6075_shutdown(data);
+}
+
+static int veml6075_uva_comp(int raw_uva, int comp1, int comp2)
+{
+	int comp1a_c, comp2a_c, uva_comp;
+
+	comp1a_c = (comp1 * VEML6075_A_COEF) / 1000U;
+	comp2a_c = (comp2 * VEML6075_B_COEF) / 1000U;
+	uva_comp = raw_uva - comp1a_c - comp2a_c;
+	pr_err("JCC: uva=%d, c1=%d, c2=%d, c1ca=%d, c2ca=%d, uvac=%d\n",
+	       raw_uva, comp1, comp2, comp1a_c, comp2a_c, uva_comp);
+
+	return clamp_val(uva_comp, 0, U16_MAX);
+}
+
+static int veml6075_uvb_comp(int raw_uvb, int comp1, int comp2)
+{
+	int comp1b_c, comp2b_c, uvb_comp;
+
+	comp1b_c = (comp1 * VEML6075_C_COEF) / 1000U;
+	comp2b_c = (comp2 * VEML6075_D_COEF) / 1000U;
+	uvb_comp = raw_uvb - comp1b_c - comp2b_c;
+	pr_err("JCC: uvb=%d, c1=%d, c2=%d, c1cb=%d, c2cb=%d, uvbc=%d\n",
+	       raw_uvb, comp1, comp2, comp1b_c, comp2b_c, uvb_comp);
+
+	return clamp_val(uvb_comp, 0, U16_MAX);
+}
+
+static int veml6075_read_comp(struct veml6075_data *data, int *c1, int *c2)
+{
+	int ret;
+
+	ret = regmap_read(data->regmap, VEML6075_CMD_COMP1, c1);
+	if (ret < 0)
+		return ret;
+
+	return regmap_read(data->regmap, VEML6075_CMD_COMP2, c2);
+}
+
+static int veml6075_read_uva_count(struct veml6075_data *data, int *uva)
+{
+	return regmap_read(data->regmap, VEML6075_CMD_UVA, uva);
+}
+
+static int veml6075_read_uvb_count(struct veml6075_data *data, int *uvb)
+{
+	return regmap_read(data->regmap, VEML6075_CMD_UVB, uvb);
+}
+
+static int veml6075_read_uv_direct(struct veml6075_data *data, int chan,
+				   int *val)
+{
+	int c1, c2, ret;
+
+	ret = veml6075_request_measurement(data);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6075_read_comp(data, &c1, &c2);
+	if (ret < 0)
+		return ret;
+
+	switch (chan) {
+	case CH_UVA:
+		ret = veml6075_read_uva_count(data, val);
+		if (ret < 0)
+			return ret;
+
+		*val = veml6075_uva_comp(*val, c1, c2);
+		break;
+	case CH_UVB:
+		ret = veml6075_read_uvb_count(data, val);
+		if (ret < 0)
+			return ret;
+
+		*val = veml6075_uvb_comp(*val, c1, c2);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT;
+}
+
+static int veml6075_read_int_time_index(struct veml6075_data *data)
+{
+	int ret, conf;
+
+	ret = regmap_read(data->regmap, VEML6075_CMD_CONF, &conf);
+	if (ret < 0)
+		return ret;
+
+	return FIELD_GET(VEML6075_CONF_IT, conf);
+}
+
+static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)
+{
+	int int_index;
+
+	int_index = veml6075_read_int_time_index(data);
+	if (int_index < 0)
+		return int_index;
+
+	*val = veml6075_it_ms[int_index];
+
+	return IIO_VAL_INT;
+}
+
+static int veml6075_get_uvi_micro(struct veml6075_data *data, int uva_comp,
+				  int uvb_comp)
+{
+	int uvia_micro = uva_comp * VEML6075_UVA_RESP;
+	int uvib_micro = uvb_comp * VEML6075_UVB_RESP;
+	int int_index;
+
+	int_index = veml6075_read_int_time_index(data);
+	if (int_index < 0)
+		return int_index;
+
+	switch (int_index) {
+	case VEML6075_CONF_IT_50:
+		return uvia_micro + uvib_micro;
+	case VEML6075_CONF_IT_100:
+	case VEML6075_CONF_IT_200:
+	case VEML6075_CONF_IT_400:
+	case VEML6075_CONF_IT_800:
+		return (uvia_micro + uvib_micro) / (2 << int_index);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6075_read_uvi(struct veml6075_data *data, int *val, int *val2)
+{
+	int ret, c1, c2, uva, uvb, uvi_micro;
+
+	ret = veml6075_request_measurement(data);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6075_read_comp(data, &c1, &c2);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6075_read_uva_count(data, &uva);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6075_read_uvb_count(data, &uvb);
+	if (ret < 0)
+		return ret;
+
+	uvi_micro = veml6075_get_uvi_micro(data, veml6075_uva_comp(uva, c1, c2),
+					   veml6075_uvb_comp(uvb, c1, c2));
+	if (uvi_micro < 0)
+		return uvi_micro;
+
+	*val = uvi_micro / 1000000LL;
+	*val2 = uvi_micro % 1000000LL;
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int veml6075_read_responsivity(int chan, int *val, int *val2)
+{
+	/* scale = 1 / resp */
+	switch (chan) {
+	case CH_UVA:
+		/* resp = 0.93 c/uW/cm2: scale = 1.75268817 */
+		*val = 1;
+		*val2 = 75268817;
+		break;
+	case CH_UVB:
+		/* resp = 2.1 c/uW/cm2: scale = 0.476190476 */
+		*val = 0;
+		*val2 = 476190476;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int veml6075_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct veml6075_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&data->lock);
+		ret = veml6075_read_uv_direct(data, chan->channel, val);
+		break;
+	case IIO_CHAN_INFO_PROCESSED:
+		mutex_lock(&data->lock);
+		ret = veml6075_read_uvi(data, val, val2);
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		mutex_lock(&data->lock);
+		ret = veml6075_read_int_time_ms(data, val);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		return veml6075_read_responsivity(chan->channel, val, val2);
+	default:
+		return -EINVAL;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int veml6075_write_int_time_ms(struct veml6075_data *data, int val)
+{
+	int conf, i = ARRAY_SIZE(veml6075_it_ms);
+
+	while (i-- > 0) {
+		if (val == veml6075_it_ms[i])
+			break;
+	}
+	if (i < 0)
+		return -EINVAL;
+
+	conf = FIELD_PREP(VEML6075_CONF_IT, i);
+
+	return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
+				  VEML6075_CONF_IT, conf);
+}
+
+static int veml6075_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct veml6075_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		mutex_lock(&data->lock);
+		ret = veml6075_write_int_time_ms(data, val);
+		mutex_unlock(&data->lock);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct iio_info veml6075_info = {
+	.read_raw = veml6075_read_raw,
+	.write_raw = veml6075_write_raw,
+	.attrs = &veml6075_attribute_group,
+};
+
+static bool veml6075_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case VEML6075_CMD_CONF:
+	case VEML6075_CMD_UVA:
+	case VEML6075_CMD_UVB:
+	case VEML6075_CMD_COMP1:
+	case VEML6075_CMD_COMP2:
+	case VEML6075_CMD_ID:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool veml6075_writable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case VEML6075_CMD_CONF:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config veml6075_regmap_config = {
+	.name = VEML6075_DRIVER_NAME,
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = VEML6075_CMD_ID,
+	.readable_reg = veml6075_readable_reg,
+	.writeable_reg = veml6075_writable_reg,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+
+};
+
+static int veml6075_probe(struct i2c_client *client)
+{
+	struct veml6075_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	int config, ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_i2c(client, &veml6075_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	data->regmap = regmap;
+
+	mutex_init(&data->lock);
+
+	indio_dev->name = VEML6075_DRIVER_NAME;
+	indio_dev->info = &veml6075_info;
+	indio_dev->channels = veml6075_channels;
+	indio_dev->num_channels = ARRAY_SIZE(veml6075_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
+	if (ret < 0 && ret != -ENODEV)
+		return ret;
+
+	/* default: 100ms integration time, active force enable, shutdown */
+	config = FIELD_PREP(VEML6075_CONF_IT, VEML6075_CONF_IT_100) |
+		VEML6075_CONF_AF | VEML6075_CONF_SD;
+	ret = regmap_write(data->regmap, VEML6075_CMD_CONF, config);
+	if (ret < 0)
+		return ret;
+
+	return iio_device_register(indio_dev);
+}
+
+static void veml6075_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+}
+
+static const struct i2c_device_id veml6075_id[] = {
+	{ "veml6075", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, veml6075_id);
+
+static const struct of_device_id veml6075_of_match[] = {
+	{ .compatible = "vishay,veml6075" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, veml6075_of_match);
+
+static struct i2c_driver veml6075_driver = {
+	.driver = {
+		.name   = VEML6075_DRIVER_NAME,
+		.of_match_table = veml6075_of_match,
+	},
+	.probe = veml6075_probe,
+	.remove  = veml6075_remove,
+	.id_table = veml6075_id,
+};
+
+module_i2c_driver(veml6075_driver);
+
+MODULE_AUTHOR("Javier Carrasco <javier.carrasco.cruz@gmail.com>");
+MODULE_DESCRIPTION("Vishay VEML6075 UVA and UVB light sensor driver");
+MODULE_LICENSE("GPL");

-- 
2.39.2

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Javier Carrasco]

On 19.11.23 05:58, Javier Carrasco wrote:
> The Vishay VEMl6075 is a low power, 16-bit resolution UVA and UVB
> light sensor with I2C interface and noise compensation (visible and
> infrarred).
> 
> Every UV channel generates an output measured in counts per integration
> period. Available integration times are 50 ms, 100 ms, 200 ms, 400 ms
> and 800 ms,
> 
> This driver adds support for both UV channels and the ultraviolet
> index (UVI) inferred from them according to the device application note
> with open-air (no teflon) coefficients.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  MAINTAINERS                  |   6 +
>  drivers/iio/light/Kconfig    |  11 +
>  drivers/iio/light/Makefile   |   1 +
>  drivers/iio/light/veml6075.c | 503 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 521 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 97f51d5ec1cf..2f13a5088d41 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23184,6 +23184,12 @@ S:	Maintained
>  F:	drivers/input/serio/userio.c
>  F:	include/uapi/linux/userio.h
>  
> +VISHAY VEML6075 UVA AND UVB LIGHT SENSOR DRIVER
> +M:	Javier Carrasco <javier.carrasco.cruz@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/light/veml6075.yaml
> +F:	drivers/iio/light/veml6075.c
> +
>  VISL VIRTUAL STATELESS DECODER DRIVER
>  M:	Daniel Almeida <daniel.almeida@collabora.com>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 45edba797e4c..f68e62196bc2 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -637,6 +637,17 @@ config VEML6070
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called veml6070.
>  
> +config VEML6075
> +	tristate "VEML6075 UVA and UVB light sensor"
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Say Y here if you want to build a driver for the Vishay VEML6075 UVA
> +	  and UVB light sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called veml6075.
> +
>  config VL6180
>  	tristate "VL6180 ALS, range and proximity sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index c0db4c4c36ec..c8289e24e3f6 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -60,5 +60,6 @@ obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
>  obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
>  obj-$(CONFIG_VEML6030)		+= veml6030.o
>  obj-$(CONFIG_VEML6070)		+= veml6070.o
> +obj-$(CONFIG_VEML6075)		+= veml6075.o
>  obj-$(CONFIG_VL6180)		+= vl6180.o
>  obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
> diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c
> new file mode 100644
> index 000000000000..b7d9319c3906
> --- /dev/null
> +++ b/drivers/iio/light/veml6075.c
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * veml6075.c - Support for Vishay VEML6075 UVA and UVB light sensor
> + *
> + * Copyright 2023 Javier Carrasco <javier.carrasco.cruz@gmail.com>
> + *
> + * IIO driver for VEML6075 (7-bit I2C slave address 0x10)
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VEML6075_DRIVER_NAME "veml6075"
> +
> +#define VEML6075_CMD_CONF	0x00 /* configuration register */
> +#define VEML6075_CMD_UVA	0x07 /* UVA channel */
> +#define VEML6075_CMD_UVB	0x09 /* UVB channel */
> +#define VEML6075_CMD_COMP1	0x0A /* visible light compensation */
> +#define VEML6075_CMD_COMP2	0x0B /* infrarred light compensation */
> +#define VEML6075_CMD_ID		0x0C /* device ID */
> +
> +#define VEML6075_CONF_IT	GENMASK(6, 4) /* intregration time */
> +#define VEML6075_CONF_HD	BIT(3) /* dynamic setting */
> +#define VEML6075_CONF_TRIG	BIT(2) /* trigger */
> +#define VEML6075_CONF_AF	BIT(1) /* active force enable */
> +#define VEML6075_CONF_SD	BIT(0) /* shutdown */
> +
> +#define VEML6075_CONF_IT_50	0x00 /* integration time 50 ms */
> +#define VEML6075_CONF_IT_100	0x01 /* integration time 100 ms */
> +#define VEML6075_CONF_IT_200	0x02 /* integration time 200 ms */
> +#define VEML6075_CONF_IT_400	0x03 /* integration time 400 ms */
> +#define VEML6075_CONF_IT_800	0x04 /* integration time 800 ms */
> +
> +/* Open-air coefficients and responsivity */
> +#define VEML6075_A_COEF		2220
> +#define VEML6075_B_COEF		1330
> +#define VEML6075_C_COEF		2950
> +#define VEML6075_D_COEF		1740
> +#define VEML6075_UVA_RESP	1461
> +#define VEML6075_UVB_RESP	2591
> +
> +static const int veml6075_it_ms[] = { 50, 100, 200, 400, 800 };
> +static const char veml6075_it_ms_avail[] = "50 100 200 400 800";
> +
> +struct veml6075_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct mutex lock; /* register access lock */
> +};
> +
> +/* channel number */
> +enum veml6075_chan {
> +	CH_UVA,
> +	CH_UVB,
> +};
> +
> +static const struct iio_chan_spec veml6075_channels[] = {
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_UVA,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_UV,
> +		.extend_name = "UVA",
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_UVB,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_UV,
> +		.extend_name = "UVB",
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +	{
> +		.type = IIO_UVINDEX,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +};
> +
> +static IIO_CONST_ATTR_INT_TIME_AVAIL(veml6075_it_ms_avail);
> +
> +static struct attribute *veml6075_attributes[] = {
> +	&iio_const_attr_integration_time_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group veml6075_attribute_group = {
> +	.attrs = veml6075_attributes,
> +};
> +
> +static int veml6075_shutdown(struct veml6075_data *data)
> +{
> +	return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
> +				  VEML6075_CONF_SD, VEML6075_CONF_SD);
> +}
> +
> +static int veml6075_request_measurement(struct veml6075_data *data)
> +{
> +	int ret, conf, int_time;
> +
> +	ret = regmap_read(data->regmap, VEML6075_CMD_CONF, &conf);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* disable shutdown and trigger measurement */
> +	ret = regmap_write(data->regmap, VEML6075_CMD_CONF,
> +			   (conf | VEML6075_CONF_TRIG) & ~VEML6075_CONF_SD);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * A measurement requires between 1.30 and 1.40 times the integration
> +	 * time for all possible configurations. Using a 1.50 factor simplifies
> +	 * operations and ensures reliability under all circumstances.
> +	 */
> +	int_time = veml6075_it_ms[FIELD_GET(VEML6075_CONF_IT, conf)];
> +	msleep(int_time + (int_time / 2));
> +
> +	/* shutdown again, data registers are still accessible */
> +	return veml6075_shutdown(data);
> +}
> +
> +static int veml6075_uva_comp(int raw_uva, int comp1, int comp2)
> +{
> +	int comp1a_c, comp2a_c, uva_comp;
> +
> +	comp1a_c = (comp1 * VEML6075_A_COEF) / 1000U;
> +	comp2a_c = (comp2 * VEML6075_B_COEF) / 1000U;
> +	uva_comp = raw_uva - comp1a_c - comp2a_c;
> +	pr_err("JCC: uva=%d, c1=%d, c2=%d, c1ca=%d, c2ca=%d, uvac=%d\n",
> +	       raw_uva, comp1, comp2, comp1a_c, comp2a_c, uva_comp);
Obviously this debug message should be gone and it will be removed for v2.
> +
> +	return clamp_val(uva_comp, 0, U16_MAX);
> +}
> +
> +static int veml6075_uvb_comp(int raw_uvb, int comp1, int comp2)
> +{
> +	int comp1b_c, comp2b_c, uvb_comp;
> +
> +	comp1b_c = (comp1 * VEML6075_C_COEF) / 1000U;
> +	comp2b_c = (comp2 * VEML6075_D_COEF) / 1000U;
> +	uvb_comp = raw_uvb - comp1b_c - comp2b_c;
> +	pr_err("JCC: uvb=%d, c1=%d, c2=%d, c1cb=%d, c2cb=%d, uvbc=%d\n",
> +	       raw_uvb, comp1, comp2, comp1b_c, comp2b_c, uvb_comp);
Same here.
> +
> +	return clamp_val(uvb_comp, 0, U16_MAX);
> +}
> +
> +static int veml6075_read_comp(struct veml6075_data *data, int *c1, int *c2)
> +{
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, VEML6075_CMD_COMP1, c1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_read(data->regmap, VEML6075_CMD_COMP2, c2);
> +}
> +
> +static int veml6075_read_uva_count(struct veml6075_data *data, int *uva)
> +{
> +	return regmap_read(data->regmap, VEML6075_CMD_UVA, uva);
> +}
> +
> +static int veml6075_read_uvb_count(struct veml6075_data *data, int *uvb)
> +{
> +	return regmap_read(data->regmap, VEML6075_CMD_UVB, uvb);
> +}
> +
> +static int veml6075_read_uv_direct(struct veml6075_data *data, int chan,
> +				   int *val)
> +{
> +	int c1, c2, ret;
> +
> +	ret = veml6075_request_measurement(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6075_read_comp(data, &c1, &c2);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (chan) {
> +	case CH_UVA:
> +		ret = veml6075_read_uva_count(data, val);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = veml6075_uva_comp(*val, c1, c2);
> +		break;
> +	case CH_UVB:
> +		ret = veml6075_read_uvb_count(data, val);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = veml6075_uvb_comp(*val, c1, c2);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int veml6075_read_int_time_index(struct veml6075_data *data)
> +{
> +	int ret, conf;
> +
> +	ret = regmap_read(data->regmap, VEML6075_CMD_CONF, &conf);
> +	if (ret < 0)
> +		return ret;
> +
> +	return FIELD_GET(VEML6075_CONF_IT, conf);
> +}
> +
> +static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val)
> +{
> +	int int_index;
> +
> +	int_index = veml6075_read_int_time_index(data);
> +	if (int_index < 0)
> +		return int_index;
> +
> +	*val = veml6075_it_ms[int_index];
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int veml6075_get_uvi_micro(struct veml6075_data *data, int uva_comp,
> +				  int uvb_comp)
> +{
> +	int uvia_micro = uva_comp * VEML6075_UVA_RESP;
> +	int uvib_micro = uvb_comp * VEML6075_UVB_RESP;
> +	int int_index;
> +
> +	int_index = veml6075_read_int_time_index(data);
> +	if (int_index < 0)
> +		return int_index;
> +
> +	switch (int_index) {
> +	case VEML6075_CONF_IT_50:
> +		return uvia_micro + uvib_micro;
> +	case VEML6075_CONF_IT_100:
> +	case VEML6075_CONF_IT_200:
> +	case VEML6075_CONF_IT_400:
> +	case VEML6075_CONF_IT_800:
> +		return (uvia_micro + uvib_micro) / (2 << int_index);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6075_read_uvi(struct veml6075_data *data, int *val, int *val2)
> +{
> +	int ret, c1, c2, uva, uvb, uvi_micro;
> +
> +	ret = veml6075_request_measurement(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6075_read_comp(data, &c1, &c2);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6075_read_uva_count(data, &uva);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6075_read_uvb_count(data, &uvb);
> +	if (ret < 0)
> +		return ret;
> +
> +	uvi_micro = veml6075_get_uvi_micro(data, veml6075_uva_comp(uva, c1, c2),
> +					   veml6075_uvb_comp(uvb, c1, c2));
> +	if (uvi_micro < 0)
> +		return uvi_micro;
> +
> +	*val = uvi_micro / 1000000LL;
> +	*val2 = uvi_micro % 1000000LL;
> +
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int veml6075_read_responsivity(int chan, int *val, int *val2)
> +{
> +	/* scale = 1 / resp */
> +	switch (chan) {
> +	case CH_UVA:
> +		/* resp = 0.93 c/uW/cm2: scale = 1.75268817 */
> +		*val = 1;
> +		*val2 = 75268817;
> +		break;
> +	case CH_UVB:
> +		/* resp = 2.1 c/uW/cm2: scale = 0.476190476 */
> +		*val = 0;
> +		*val2 = 476190476;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT_PLUS_NANO;
> +}
> +
> +static int veml6075_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct veml6075_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_read_uv_direct(data, chan->channel, val);
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_read_uvi(data, val, val2);
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_read_int_time_ms(data, val);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		return veml6075_read_responsivity(chan->channel, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int veml6075_write_int_time_ms(struct veml6075_data *data, int val)
> +{
> +	int conf, i = ARRAY_SIZE(veml6075_it_ms);
> +
> +	while (i-- > 0) {
> +		if (val == veml6075_it_ms[i])
> +			break;
> +	}
> +	if (i < 0)
> +		return -EINVAL;
> +
> +	conf = FIELD_PREP(VEML6075_CONF_IT, i);
> +
> +	return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
> +				  VEML6075_CONF_IT, conf);
> +}
> +
> +static int veml6075_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	struct veml6075_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_INT_TIME:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_write_int_time_ms(data, val);
> +		mutex_unlock(&data->lock);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info veml6075_info = {
> +	.read_raw = veml6075_read_raw,
> +	.write_raw = veml6075_write_raw,
> +	.attrs = &veml6075_attribute_group,
> +};
> +
> +static bool veml6075_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case VEML6075_CMD_CONF:
> +	case VEML6075_CMD_UVA:
> +	case VEML6075_CMD_UVB:
> +	case VEML6075_CMD_COMP1:
> +	case VEML6075_CMD_COMP2:
> +	case VEML6075_CMD_ID:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool veml6075_writable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case VEML6075_CMD_CONF:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config veml6075_regmap_config = {
> +	.name = VEML6075_DRIVER_NAME,
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = VEML6075_CMD_ID,
> +	.readable_reg = veml6075_readable_reg,
> +	.writeable_reg = veml6075_writable_reg,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +
> +};
> +
> +static int veml6075_probe(struct i2c_client *client)
> +{
> +	struct veml6075_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	int config, ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &veml6075_regmap_config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +	data->regmap = regmap;
> +
> +	mutex_init(&data->lock);
> +
> +	indio_dev->name = VEML6075_DRIVER_NAME;
> +	indio_dev->info = &veml6075_info;
> +	indio_dev->channels = veml6075_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(veml6075_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
> +	if (ret < 0 && ret != -ENODEV)
> +		return ret;
> +
> +	/* default: 100ms integration time, active force enable, shutdown */
> +	config = FIELD_PREP(VEML6075_CONF_IT, VEML6075_CONF_IT_100) |
> +		VEML6075_CONF_AF | VEML6075_CONF_SD;
> +	ret = regmap_write(data->regmap, VEML6075_CMD_CONF, config);
> +	if (ret < 0)
> +		return ret;
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static void veml6075_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +}
> +
> +static const struct i2c_device_id veml6075_id[] = {
> +	{ "veml6075", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, veml6075_id);
> +
> +static const struct of_device_id veml6075_of_match[] = {
> +	{ .compatible = "vishay,veml6075" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, veml6075_of_match);
> +
> +static struct i2c_driver veml6075_driver = {
> +	.driver = {
> +		.name   = VEML6075_DRIVER_NAME,
> +		.of_match_table = veml6075_of_match,
> +	},
> +	.probe = veml6075_probe,
> +	.remove  = veml6075_remove,
> +	.id_table = veml6075_id,
> +};
> +
> +module_i2c_driver(veml6075_driver);
> +
> +MODULE_AUTHOR("Javier Carrasco <javier.carrasco.cruz@gmail.com>");
> +MODULE_DESCRIPTION("Vishay VEML6075 UVA and UVB light sensor driver");
> +MODULE_LICENSE("GPL");
> 
Best regards,
Javier Carrasco

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Jonathan Cameron]

On Sun, 19 Nov 2023 06:08:25 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> On 19.11.23 05:58, Javier Carrasco wrote:
> > The Vishay VEMl6075 is a low power, 16-bit resolution UVA and UVB
> > light sensor with I2C interface and noise compensation (visible and
> > infrarred).
> > 
> > Every UV channel generates an output measured in counts per integration
> > period. Available integration times are 50 ms, 100 ms, 200 ms, 400 ms
> > and 800 ms,
> > 
> > This driver adds support for both UV channels and the ultraviolet
> > index (UVI) inferred from them according to the device application note
> > with open-air (no teflon) coefficients.
> > 
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Hi Javier,

One process note highlighted by this email.
Please crop to only relevant text when replying.  Scrolling takes time!
+ makes it hard to be sure we didn't miss anything buried in hundreds
of lines of unnecessary context.

I'm jet lagged so may be more grumpy than average today :(

Thanks,

Jonathan


> > +static int veml6075_uva_comp(int raw_uva, int comp1, int comp2)
> > +{
> > +	int comp1a_c, comp2a_c, uva_comp;
> > +
> > +	comp1a_c = (comp1 * VEML6075_A_COEF) / 1000U;
> > +	comp2a_c = (comp2 * VEML6075_B_COEF) / 1000U;
> > +	uva_comp = raw_uva - comp1a_c - comp2a_c;
> > +	pr_err("JCC: uva=%d, c1=%d, c2=%d, c1ca=%d, c2ca=%d, uvac=%d\n",
> > +	       raw_uva, comp1, comp2, comp1a_c, comp2a_c, uva_comp);  
> Obviously this debug message should be gone and it will be removed for v2.
> > +
> > +	return clamp_val(uva_comp, 0, U16_MAX);
> > +}
> > +
> > +static int veml6075_uvb_comp(int raw_uvb, int comp1, int comp2)
> > +{
> > +	int comp1b_c, comp2b_c, uvb_comp;
> > +
> > +	comp1b_c = (comp1 * VEML6075_C_COEF) / 1000U;
> > +	comp2b_c = (comp2 * VEML6075_D_COEF) / 1000U;
> > +	uvb_comp = raw_uvb - comp1b_c - comp2b_c;
> > +	pr_err("JCC: uvb=%d, c1=%d, c2=%d, c1cb=%d, c2cb=%d, uvbc=%d\n",
> > +	       raw_uvb, comp1, comp2, comp1b_c, comp2b_c, uvb_comp);  
> Same here.
> > +
> > +	return clamp_val(uvb_comp, 0, U16_MAX);
> > +}
> > +

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Jonathan Cameron]

On Sun, 19 Nov 2023 05:58:03 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> The Vishay VEMl6075 is a low power, 16-bit resolution UVA and UVB
> light sensor with I2C interface and noise compensation (visible and
> infrarred).
> 
> Every UV channel generates an output measured in counts per integration
> period. Available integration times are 50 ms, 100 ms, 200 ms, 400 ms
> and 800 ms,
> 
> This driver adds support for both UV channels and the ultraviolet
> index (UVI) inferred from them according to the device application note
> with open-air (no teflon) coefficients.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Hi Javier,

Various comments inline, but all minor stuff. Looks good in general to me.

Jonathan

...

> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index c0db4c4c36ec..c8289e24e3f6 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -60,5 +60,6 @@ obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
>  obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
>  obj-$(CONFIG_VEML6030)		+= veml6030.o
>  obj-$(CONFIG_VEML6070)		+= veml6070.o
> +obj-$(CONFIG_VEML6075)		+= veml6075.o
>  obj-$(CONFIG_VL6180)		+= vl6180.o
>  obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
> diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c
> new file mode 100644
> index 000000000000..b7d9319c3906
> --- /dev/null
> +++ b/drivers/iio/light/veml6075.c
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * veml6075.c - Support for Vishay VEML6075 UVA and UVB light sensor

Little point in having the filename inside the file comments. Just makes it
a pain to move if we ever do and adds little of use.
Arguably "Support for" also a bit pointless.
Vishay VEML... 
is what Id' keep.

> + *
> + * Copyright 2023 Javier Carrasco <javier.carrasco.cruz@gmail.com>
> + *
> + * IIO driver for VEML6075 (7-bit I2C slave address 0x10)

The IIO and driver bit is obvious from the file.  Fine to keep the address
if you like.

> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

You won't need sysfs.h after the changes suggested inline.
Every time it is included, it's a signal to reviewers to take a close
look at the ABI.  There are still reasons to use it but they normally
mean there should be additional documentation as well in the patch.

> +
> +#define VEML6075_DRIVER_NAME "veml6075"

As mentioned below, even though there are several uses of this I'd
rather see the string inline than hidden behind a define.

> +
> +#define VEML6075_CMD_CONF	0x00 /* configuration register */
> +#define VEML6075_CMD_UVA	0x07 /* UVA channel */
> +#define VEML6075_CMD_UVB	0x09 /* UVB channel */
> +#define VEML6075_CMD_COMP1	0x0A /* visible light compensation */
> +#define VEML6075_CMD_COMP2	0x0B /* infrarred light compensation */
> +#define VEML6075_CMD_ID		0x0C /* device ID */
> +
> +#define VEML6075_CONF_IT	GENMASK(6, 4) /* intregration time */
> +#define VEML6075_CONF_HD	BIT(3) /* dynamic setting */
> +#define VEML6075_CONF_TRIG	BIT(2) /* trigger */
> +#define VEML6075_CONF_AF	BIT(1) /* active force enable */
> +#define VEML6075_CONF_SD	BIT(0) /* shutdown */
> +
> +#define VEML6075_CONF_IT_50	0x00 /* integration time 50 ms */
Rename them to _IT_50_MS etc and drop the comments as they only really
tell us the unit.  The other part is fairly obvious.
> +#define VEML6075_CONF_IT_100	0x01 /* integration time 100 ms */
> +#define VEML6075_CONF_IT_200	0x02 /* integration time 200 ms */
> +#define VEML6075_CONF_IT_400	0x03 /* integration time 400 ms */
> +#define VEML6075_CONF_IT_800	0x04 /* integration time 800 ms */
> +
> +/* Open-air coefficients and responsivity */
> +#define VEML6075_A_COEF		2220
> +#define VEML6075_B_COEF		1330
> +#define VEML6075_C_COEF		2950
> +#define VEML6075_D_COEF		1740
> +#define VEML6075_UVA_RESP	1461
> +#define VEML6075_UVB_RESP	2591
> +
> +static const int veml6075_it_ms[] = { 50, 100, 200, 400, 800 };
> +static const char veml6075_it_ms_avail[] = "50 100 200 400 800";

A strong reason for using read_avail() is that you can just use the
array of numbers. See below for more on this.

> +
> +struct veml6075_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct mutex lock; /* register access lock */

regmap provides register locking as typically does the bus lock, so good to
say exactly what you mean here.  Is there a Read Modify Write cycle you need
to protect for instance, or consistency across multiple register accesses?

> +};

> +
> +static const struct iio_chan_spec veml6075_channels[] = {
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_UVA,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_UV,
> +		.extend_name = "UVA",
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_UVB,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_UV,
> +		.extend_name = "UVB",

Extent name is very rarely used any more.  It's a horrible userspace interface
and an old design mistake. 
Instead we use the channel label infrastructure.  Provide the read_label()
callback to use that instead.

I'm not sure this is a great solution here though.  For some similar cases
such as visible light colours we've just added additional modifiers, but that
doesn't really scale to lots of sensitive ranges.

One thing we have talked about in the past, but I don't think we have done in
a driver yet, is to provide actual characteristics of the sensitivity graph.
Perhaps just a wavelength of maximum sensitivity?

Visible light sensors often have hideous sensitivity curves, including sometimes
have multiple peaks, but in this case they look pretty good.
Do you think such an ABI would be more useful than A, B labelling?



> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +	{
> +		.type = IIO_UVINDEX,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +};
> +
> +static IIO_CONST_ATTR_INT_TIME_AVAIL(veml6075_it_ms_avail);
> +
> +static struct attribute *veml6075_attributes[] = {
> +	&iio_const_attr_integration_time_available.dev_attr.attr,
Use the core support for handling available callbacks.
See read_avail() and the matching bit masks.

That makes this info accessible to in kernel consumers + is generally
cleaner than hand rolling an attribute.  We have a lot of drivers
that predate that core code existing and haven't yet converted them
all over so you'll find plenty of code that looks like yours in the tree.
It's just out of date style wise.

Note you won't need iio/sysfs.h after that change.

> +	NULL,
> +};

...

> +
> +static int veml6075_read_uvb_count(struct veml6075_data *data, int *uvb)
> +{
> +	return regmap_read(data->regmap, VEML6075_CMD_UVB, uvb);

Up to you, but to my mind wrappers that basically do nothing beyond calling
regmap_read() don't aid code readability - so you might as well call the regmap_read
inline.

> +}
> +
> +static int veml6075_read_uv_direct(struct veml6075_data *data, int chan,
> +				   int *val)
> +{
> +	int c1, c2, ret;
> +
> +	ret = veml6075_request_measurement(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6075_read_comp(data, &c1, &c2);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (chan) {
> +	case CH_UVA:
> +		ret = veml6075_read_uva_count(data, val);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = veml6075_uva_comp(*val, c1, c2);
> +		break;
> +	case CH_UVB:
> +		ret = veml6075_read_uvb_count(data, val);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = veml6075_uvb_comp(*val, c1, c2);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT;
return directly above. Lets us see the type right next to the functions
filling in val.

> +}


> +
> +static int veml6075_read_responsivity(int chan, int *val, int *val2)
> +{
> +	/* scale = 1 / resp */
> +	switch (chan) {
> +	case CH_UVA:
> +		/* resp = 0.93 c/uW/cm2: scale = 1.75268817 */
> +		*val = 1;
> +		*val2 = 75268817;
> +		break;
> +	case CH_UVB:
> +		/* resp = 2.1 c/uW/cm2: scale = 0.476190476 */
> +		*val = 0;
> +		*val2 = 476190476;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT_PLUS_NANO;
return instead of break above, so the formatting is right next to the values.

> +}
> +
> +static int veml6075_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct veml6075_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_read_uv_direct(data, chan->channel, val);
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		mutex_lock(&data->lock);

Use guard(mutex)(&data->lock); and appropriate scope (add {})
so you can return directly from each of these.
Current scheme of unlocking is nasty.

Note that if we didn't have those, just unlocking before break;
in each of these would be better than what you currently have where
the lock scope is hard to follow.

> +		ret = veml6075_read_uvi(data, val, val2);
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		mutex_lock(&data->lock);
> +		ret = veml6075_read_int_time_ms(data, val);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		return veml6075_read_responsivity(chan->channel, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int veml6075_write_int_time_ms(struct veml6075_data *data, int val)
> +{

> +	int conf, i = ARRAY_SIZE(veml6075_it_ms);

Don't combine variable declarations that set values with ones that don't.
It's just a little bit hard to read, so better to use multiple lines if
you are setting values.

> +
> +	while (i-- > 0) {
> +		if (val == veml6075_it_ms[i])
> +			break;
> +	}
> +	if (i < 0)
> +		return -EINVAL;
> +
> +	conf = FIELD_PREP(VEML6075_CONF_IT, i);

Put this conf inline in the call below.  Saves us a few lines and
an unnecessary local variable.

> +
> +	return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
> +				  VEML6075_CONF_IT, conf);
> +}
> +
> +static int veml6075_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	struct veml6075_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_INT_TIME:
> +		mutex_lock(&data->lock);

A nice place to use the new automated guard stuff.
Cleanest here is probably scoped_guard.

	case IIO_CHAN_INFO_INT_TIME: {
		guard(mutex)(&data->lock);
		return veml6075_write_int_time_ms(data, val);
	}
> +		ret = veml6075_write_int_time_ms(data, val);
> +		mutex_unlock(&data->lock);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +

Then drop this return ret; Which incidentally can return uninitialized values
in current code.  Also worth noting this would still be needed if you did a
scoped_guard() above in order to squash a compiler warning.

> +	return ret;
> +}
> +
> +static const struct iio_info veml6075_info = {
> +	.read_raw = veml6075_read_raw,
> +	.write_raw = veml6075_write_raw,
> +	.attrs = &veml6075_attribute_group,
> +};
...

> +
> +static int veml6075_probe(struct i2c_client *client)
> +{
> +	struct veml6075_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	int config, ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &veml6075_regmap_config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +	data->regmap = regmap;
> +
> +	mutex_init(&data->lock);
> +
> +	indio_dev->name = VEML6075_DRIVER_NAME;

As below, I'd rather see the string here.

> +	indio_dev->info = &veml6075_info;
> +	indio_dev->channels = veml6075_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(veml6075_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_regulator_get_enable_optional(&client->dev, "vdd");

Main power supplies tend not to be optional... They may not have been specified
if they are always on, but in that case the regulator framework will provide us
with a fake regulator anyway so that's safe.  Hence just
devm_regulator_get_enable()



> +	if (ret < 0 && ret != -ENODEV)
> +		return ret;
> +
> +	/* default: 100ms integration time, active force enable, shutdown */
> +	config = FIELD_PREP(VEML6075_CONF_IT, VEML6075_CONF_IT_100) |
> +		VEML6075_CONF_AF | VEML6075_CONF_SD;

For consistency, use FIELD_PREP for all fields, even the single bit ones.

> +	ret = regmap_write(data->regmap, VEML6075_CMD_CONF, config);
> +	if (ret < 0)
> +		return ret;
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static void veml6075_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);

Finding just a device unregister in here is odd...
If you need to handle this manually as opposed to
devm_iio_device_register() and automated cleanup, then there would need
to be something else to do first.

> +}
> +
> +static const struct i2c_device_id veml6075_id[] = {
> +	{ "veml6075", 0 },

The 0 isn't used so I wouldn't set it explicitly - it will be zeroed anyway
due to how C handles this sort of initializer.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, veml6075_id);
> +
> +static const struct of_device_id veml6075_of_match[] = {
> +	{ .compatible = "vishay,veml6075" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, veml6075_of_match);
> +
> +static struct i2c_driver veml6075_driver = {
> +	.driver = {
> +		.name   = VEML6075_DRIVER_NAME,
I'm not a fan of putting a string used as a device indentifier behind
a define as I like to be able to quickly check what .name is set to.
So I'd rather just see the string inline in all the places this is used.

Jonathan

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Javier Carrasco]

On 19.11.23 16:02, Jonathan Cameron wrote:
>> +
>> +struct veml6075_data {
>> +	struct i2c_client *client;
>> +	struct regmap *regmap;
>> +	struct mutex lock; /* register access lock */
> 
> regmap provides register locking as typically does the bus lock, so good to
> say exactly what you mean here.  Is there a Read Modify Write cycle you need
> to protect for instance, or consistency across multiple register accesses?
> 
What I want to avoid with this lock is an access to the measurement
trigger or an integration time modification from different places while
there is a measurement reading going on. "register access lock" is
probably not the best name I could have chosen though.

I was not aware of that guard(mutex) mechanism. I guess it is new
because only one driver uses it in the iio subsystem (up to v6.7-rc1).
I will have a look at it.
>> +};
> 
>> +
>> +static const struct iio_chan_spec veml6075_channels[] = {
>> +	{
>> +		.type = IIO_INTENSITY,
>> +		.channel = CH_UVA,
>> +		.modified = 1,
>> +		.channel2 = IIO_MOD_LIGHT_UV,
>> +		.extend_name = "UVA",
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> +			BIT(IIO_CHAN_INFO_SCALE),
>> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
>> +	},
>> +	{
>> +		.type = IIO_INTENSITY,
>> +		.channel = CH_UVB,
>> +		.modified = 1,
>> +		.channel2 = IIO_MOD_LIGHT_UV,
>> +		.extend_name = "UVB",
> 
> Extent name is very rarely used any more.  It's a horrible userspace interface
> and an old design mistake. 
> Instead we use the channel label infrastructure.  Provide the read_label()
> callback to use that instead.
> 
> I'm not sure this is a great solution here though.  For some similar cases
> such as visible light colours we've just added additional modifiers, but that
> doesn't really scale to lots of sensitive ranges.
> 
> One thing we have talked about in the past, but I don't think we have done in
> a driver yet, is to provide actual characteristics of the sensitivity graph.
> Perhaps just a wavelength of maximum sensitivity?
> 
> Visible light sensors often have hideous sensitivity curves, including sometimes
> have multiple peaks, but in this case they look pretty good.
> Do you think such an ABI would be more useful than A, B labelling?
> 
My first idea was adding new modifiers because I saw that
IIO_MOD_LIGHT_UV and IIO_MOD_LIGHT_DUV coexist, but then I thought _UVA
and _UVB might not be used very often (wrong assumption?) and opted for
a local solution with extended names. But any cleaner solution would be
welcome because the label attributes are redundant.

Maybe adding UV-A, UV-B and UV-C modifiers is not a big deal as these
are fairly common bands. Actually DUV is pretty much UV-C and could be
left as it is.

This sensor has a single peak per channel, but I do not know how I would
provide that information to the core if that is better than adding UVA
and UVB bands. Would that add attributes to sysfs for the wavelengths or
extend the channel name? In that case two new modifiers might be a
better  and more obvious solution.
> Jonathan
> 
> 
I will work on the other issues you pointed out. Thanks a lot for your
review.

Best regards,
Javier Carrasco

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[Jonathan Cameron]

On Sun, 19 Nov 2023 18:40:16 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> On 19.11.23 16:02, Jonathan Cameron wrote:
> >> +
> >> +struct veml6075_data {
> >> +	struct i2c_client *client;
> >> +	struct regmap *regmap;
> >> +	struct mutex lock; /* register access lock */  
> > 
> > regmap provides register locking as typically does the bus lock, so good to
> > say exactly what you mean here.  Is there a Read Modify Write cycle you need
> > to protect for instance, or consistency across multiple register accesses?
> >   
> What I want to avoid with this lock is an access to the measurement
> trigger or an integration time modification from different places while
> there is a measurement reading going on. "register access lock" is
> probably not the best name I could have chosen though.
> 
> I was not aware of that guard(mutex) mechanism. I guess it is new
> because only one driver uses it in the iio subsystem (up to v6.7-rc1).
> I will have a look at it.

Yup. It is very new.

> >> +};  
> >   
> >> +
> >> +static const struct iio_chan_spec veml6075_channels[] = {
> >> +	{
> >> +		.type = IIO_INTENSITY,
> >> +		.channel = CH_UVA,
> >> +		.modified = 1,
> >> +		.channel2 = IIO_MOD_LIGHT_UV,
> >> +		.extend_name = "UVA",
> >> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> >> +			BIT(IIO_CHAN_INFO_SCALE),
> >> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
> >> +	},
> >> +	{
> >> +		.type = IIO_INTENSITY,
> >> +		.channel = CH_UVB,
> >> +		.modified = 1,
> >> +		.channel2 = IIO_MOD_LIGHT_UV,
> >> +		.extend_name = "UVB",  
> > 
> > Extent name is very rarely used any more.  It's a horrible userspace interface
> > and an old design mistake. 
> > Instead we use the channel label infrastructure.  Provide the read_label()
> > callback to use that instead.
> > 
> > I'm not sure this is a great solution here though.  For some similar cases
> > such as visible light colours we've just added additional modifiers, but that
> > doesn't really scale to lots of sensitive ranges.
> > 
> > One thing we have talked about in the past, but I don't think we have done in
> > a driver yet, is to provide actual characteristics of the sensitivity graph.
> > Perhaps just a wavelength of maximum sensitivity?
> > 
> > Visible light sensors often have hideous sensitivity curves, including sometimes
> > have multiple peaks, but in this case they look pretty good.
> > Do you think such an ABI would be more useful than A, B labelling?
> >   
> My first idea was adding new modifiers because I saw that
> IIO_MOD_LIGHT_UV and IIO_MOD_LIGHT_DUV coexist, but then I thought _UVA
> and _UVB might not be used very often (wrong assumption?) and opted for
> a local solution with extended names. But any cleaner solution would be
> welcome because the label attributes are redundant.
> 
> Maybe adding UV-A, UV-B and UV-C modifiers is not a big deal as these
> are fairly common bands. Actually DUV is pretty much UV-C and could be
> left as it is.

Ok. Add UV-A and UV-B as that's inline with other cases.
Always a guessing game for how often a modifier will turn up.  We have
space and the list isn't growing that fast so should be fine.

> 
> This sensor has a single peak per channel, but I do not know how I would
> provide that information to the core if that is better than adding UVA
> and UVB bands. Would that add attributes to sysfs for the wavelengths or
> extend the channel name? In that case two new modifiers might be a
> better  and more obvious solution.

Would be attributes if we did add max sensitivity wavelengths.
Might be worth a revisit at somepoint, but not feeling like it's necessary
for this driver.

> > Jonathan
> > 
> >   
> I will work on the other issues you pointed out. Thanks a lot for your
> review.

> 
> Best regards,
> Javier Carrasco
> 
> 

Re: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver

[kernel test robot]

Hi Javier,

kernel test robot noticed the following build warnings:

[auto build test WARNING on b85ea95d086471afb4ad062012a4d73cd328fa86]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/iio-light-add-VEML6075-UVA-and-UVB-light-sensor-driver/20231119-130003
base:   b85ea95d086471afb4ad062012a4d73cd328fa86
patch link:    https://lore.kernel.org/r/20231110-veml6075-v1-1-354b3245e14a%40gmail.com
patch subject: [PATCH 1/2] iio: light: add VEML6075 UVA and UVB light sensor driver
reproduce: (https://download.01.org/0day-ci/archive/20231120/202311200534.wKn5Bfu1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311200534.wKn5Bfu1-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/iio/light/veml6075.yaml

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/2] dt-bindings: iio: light: add support for Vishay VEML6075

[Javier Carrasco]

The Vishay VEML6075 is a 16-bit digital UVA and UVB sensor with I2C
interface.

Add bindings and an example for the Vishay VEML6075.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 .../bindings/iio/light/vishay,veml6075.yaml        | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
new file mode 100644
index 000000000000..f8e2db29af42
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/vishay,veml6075.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Vishay VEML6075 UVA and UVB sensor
+
+maintainers:
+  - Javier Carrasco <javier.carrasco.cruz@gmail.com>
+
+properties:
+  compatible:
+    const: vishay,veml6075
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        uv-sensor@10 {
+            compatible = "vishay,veml6075";
+            reg = <0x10>;
+            vdd-supply = <&vdd_reg>;
+        };
+    };
+...

-- 
2.39.2

Re: [PATCH 2/2] dt-bindings: iio: light: add support for Vishay VEML6075

[Jonathan Cameron]

On Sun, 19 Nov 2023 05:58:04 +0100
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> The Vishay VEML6075 is a 16-bit digital UVA and UVB sensor with I2C
> interface.
> 
> Add bindings and an example for the Vishay VEML6075.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Hmm. This is a very simple device and we have a bunch of similar vishay light
sensor bindings. One option here would be to combine all the binding
docs and add an explicit check for no interrupts being specified for this
compatible.

Perhaps that should be a follow up patch though given how simple this is
and a desire to not slow down merging the driver.

One comment inline,

Jonathan


> ---
>  .../bindings/iio/light/vishay,veml6075.yaml        | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml b/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
> new file mode 100644
> index 000000000000..f8e2db29af42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/vishay,veml6075.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Vishay VEML6075 UVA and UVB sensor
> +
> +maintainers:
> +  - Javier Carrasco <javier.carrasco.cruz@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: vishay,veml6075
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor

The description doesn't really add anything.
  vdd-supply: true

is sufficient.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        uv-sensor@10 {
> +            compatible = "vishay,veml6075";
> +            reg = <0x10>;
> +            vdd-supply = <&vdd_reg>;
> +        };
> +    };
> +...
> 

Re: [PATCH 2/2] dt-bindings: iio: light: add support for Vishay VEML6075

[Krzysztof Kozlowski]

On 19/11/2023 15:30, Jonathan Cameron wrote:
> On Sun, 19 Nov 2023 05:58:04 +0100
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> 
>> The Vishay VEML6075 is a 16-bit digital UVA and UVB sensor with I2C
>> interface.
>>
>> Add bindings and an example for the Vishay VEML6075.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> 
> Hmm. This is a very simple device and we have a bunch of similar vishay light
> sensor bindings. One option here would be to combine all the binding
> docs and add an explicit check for no interrupts being specified for this
> compatible.
> 
> Perhaps that should be a follow up patch though given how simple this is
> and a desire to not slow down merging the driver.

I am fine with both approaches.

> 
> One comment inline,

With this fixed:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof