2023-11-19 04:58:25

by Javier Carrasco

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

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 <[email protected]>
---
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 <[email protected]>


2023-11-19 04:58:30

by Javier Carrasco

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

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 <[email protected]>
---
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 <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/iio/light/veml6075.yaml
+F: drivers/iio/light/veml6075.c
+
VISL VIRTUAL STATELESS DECODER DRIVER
M: Daniel Almeida <[email protected]>
L: [email protected]
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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_DESCRIPTION("Vishay VEML6075 UVA and UVB light sensor driver");
+MODULE_LICENSE("GPL");

--
2.39.2

2023-11-19 04:58:32

by Javier Carrasco

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

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 <[email protected]>
---
.../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 <[email protected]>
+
+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

2023-11-19 05:08:37

by Javier Carrasco

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



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 <[email protected]>
> ---
> 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 <[email protected]>
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/light/veml6075.yaml
> +F: drivers/iio/light/veml6075.c
> +
> VISL VIRTUAL STATELESS DECODER DRIVER
> M: Daniel Almeida <[email protected]>
> L: [email protected]
> 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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +MODULE_DESCRIPTION("Vishay VEML6075 UVA and UVB light sensor driver");
> +MODULE_LICENSE("GPL");
>
Best regards,
Javier Carrasco

2023-11-19 14:26:07

by Jonathan Cameron

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

On Sun, 19 Nov 2023 06:08:25 +0100
Javier Carrasco <[email protected]> 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 <[email protected]>
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);
> > +}
> > +

2023-11-19 14:31:32

by Jonathan Cameron

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

On Sun, 19 Nov 2023 05:58:04 +0100
Javier Carrasco <[email protected]> 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 <[email protected]>

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 <[email protected]>
> +
> +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>;
> + };
> + };
> +...
>

2023-11-19 15:03:23

by Jonathan Cameron

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

On Sun, 19 Nov 2023 05:58:03 +0100
Javier Carrasco <[email protected]> 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 <[email protected]>

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 <[email protected]>
> + *
> + * 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


2023-11-19 17:40:27

by Javier Carrasco

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

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

2023-11-19 22:23:20

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-11-20 10:24:25

by Krzysztof Kozlowski

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

On 19/11/2023 15:30, Jonathan Cameron wrote:
> On Sun, 19 Nov 2023 05:58:04 +0100
> Javier Carrasco <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>

---

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