2023-11-25 11:57:17

by Javier Carrasco

[permalink] [raw]
Subject: [PATCH v2 0/3] 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]>
---
Changes in v2:
- General: swap patch order (bindings first).
- iio core: add uva and uvb modifiers.
- veml6075.c: use uva and uvb modifiers instead of extend_name
- veml6075.c: remove redundant information from the description.
- veml6075.c: inline device name.
- veml6075.c: use read_avail() for available attributes.
- veml6075.c: use guard(mutex) instead of lock/unlock().
- veml6075.c: use regulator_get_enable() without _optional.
- veml6075.c: register managed iio device and delete remove().
- veml6075.c: remove remaining debug messages.
- veml6075.c: error path cleanup (return type after val assignment).
- veml6075.c: remove zero from i2c_device_id.
- MAINTAINERS: fix bindings name.
- vishay,veml6075.yaml: remove vdd-supply description and mark property
as true.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Javier Carrasco (3):
iio: add modifiers for A and B ultraviolet light
dt-bindings: iio: light: add support for Vishay VEML6075
iio: light: add VEML6075 UVA and UVB light sensor driver

Documentation/ABI/testing/sysfs-bus-iio | 7 +-
.../bindings/iio/light/vishay,veml6075.yaml | 39 ++
MAINTAINERS | 6 +
drivers/iio/industrialio-core.c | 2 +
drivers/iio/light/Kconfig | 11 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/veml6075.c | 486 +++++++++++++++++++++
include/uapi/linux/iio/types.h | 2 +
tools/iio/iio_event_monitor.c | 2 +
9 files changed, 554 insertions(+), 2 deletions(-)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231110-veml6075-321522ceaca9

Best regards,
--
Javier Carrasco <[email protected]>


2023-11-25 11:57:32

by Javier Carrasco

[permalink] [raw]
Subject: [PATCH v2 2/3] 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]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/iio/light/vishay,veml6075.yaml | 39 ++++++++++++++++++++++
1 file changed, 39 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..abee04cd126e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/vishay,veml6075.yaml
@@ -0,0 +1,39 @@
+# 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: true
+
+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-25 11:57:40

by Javier Carrasco

[permalink] [raw]
Subject: [PATCH v2 1/3] iio: add modifiers for A and B ultraviolet light

Currently there are only two modifiers for ultraviolet light: a generic
one for any ultraviolet light (IIO_MOD_LIGHT_UV) and one for deep
ultraviolet (IIO_MOD_LIGHT_DUV), which is also referred as ultraviolet
C (UV-C) band and covers short-wave ultraviolet.

There are still no modifiers for the long-wave and medium-wave
ultraviolet bands. These two bands are the main components used to
obtain the UV index on the Earth's surface.

Add modifiers for the ultraviolet A (UV-A) and ultraviolet B (UV-B)
bands.

Signed-off-by: Javier Carrasco <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio | 7 +++++--
drivers/iio/industrialio-core.c | 2 ++
include/uapi/linux/iio/types.h | 2 ++
tools/iio/iio_event_monitor.c | 2 ++
4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 19cde14f3869..e2a9937be99c 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1574,6 +1574,8 @@ What: /sys/.../iio:deviceX/in_intensityY_raw
What: /sys/.../iio:deviceX/in_intensityY_ir_raw
What: /sys/.../iio:deviceX/in_intensityY_both_raw
What: /sys/.../iio:deviceX/in_intensityY_uv_raw
+What: /sys/.../iio:deviceX/in_intensityY_uva_raw
+What: /sys/.../iio:deviceX/in_intensityY_uvb_raw
What: /sys/.../iio:deviceX/in_intensityY_duv_raw
KernelVersion: 3.4
Contact: [email protected]
@@ -1582,8 +1584,9 @@ Description:
that measurements contain visible and infrared light
components or just infrared light, respectively. Modifier
uv indicates that measurements contain ultraviolet light
- components. Modifier duv indicates that measurements
- contain deep ultraviolet light components.
+ components. Modifiers uva, uvb and duv indicate that
+ measurements contain A, B or deep (C) ultraviolet light
+ components respectively.

What: /sys/.../iio:deviceX/in_uvindex_input
KernelVersion: 4.6
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index c77745b594bd..65f567e4ff2a 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -117,6 +117,8 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
[IIO_MOD_LIGHT_UV] = "uv",
+ [IIO_MOD_LIGHT_UVA] = "uva",
+ [IIO_MOD_LIGHT_UVB] = "uvb",
[IIO_MOD_LIGHT_DUV] = "duv",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 9c2ffdcd6623..5060963707b1 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -91,6 +91,8 @@ enum iio_modifier {
IIO_MOD_CO2,
IIO_MOD_VOC,
IIO_MOD_LIGHT_UV,
+ IIO_MOD_LIGHT_UVA,
+ IIO_MOD_LIGHT_UVB,
IIO_MOD_LIGHT_DUV,
IIO_MOD_PM1,
IIO_MOD_PM2P5,
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 2eaaa7123b04..8073c9e4fe46 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -105,6 +105,8 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
[IIO_MOD_LIGHT_UV] = "uv",
+ [IIO_MOD_LIGHT_UVA] = "uva",
+ [IIO_MOD_LIGHT_UVB] = "uvb",
[IIO_MOD_LIGHT_DUV] = "duv",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",

--
2.39.2

2023-11-25 11:57:45

by Javier Carrasco

[permalink] [raw]
Subject: [PATCH v2 3/3] 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 signal measured in counts per
integration period, where the integration time is configurable.

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 | 486 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 504 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..4ebf66036f6a 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/vishay,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..a33488884076
--- /dev/null
+++ b/drivers/iio/light/veml6075.c
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Vishay VEML6075 UVA and UVB light sensor
+ *
+ * Copyright 2023 Javier Carrasco <[email protected]>
+ *
+ * 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>
+
+#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_IT_50_MS 0x00
+#define VEML6075_IT_100_MS 0x01
+#define VEML6075_IT_200_MS 0x02
+#define VEML6075_IT_400_MS 0x03
+#define VEML6075_IT_800_MS 0x04
+
+#define VEML6075_AF_DISABLE 0x00
+#define VEML6075_AF_ENABLE 0x01
+
+#define VEML6075_SD_DISABLE 0x00
+#define VEML6075_SD_ENABLE 0x01
+
+/* 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 };
+
+struct veml6075_data {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct mutex lock; /* integration time/measurement trigger 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_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),
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
+ },
+ {
+ .type = IIO_INTENSITY,
+ .channel = CH_UVB,
+ .modified = 1,
+ .channel2 = IIO_MOD_LIGHT_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),
+ .info_mask_shared_by_all_available = 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),
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME),
+ },
+};
+
+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;
+
+ 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;
+
+ 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_uv_direct(struct veml6075_data *data, int chan,
+ int *val)
+{
+ int c1, c2, ret;
+
+ guard(mutex)(&data->lock);
+
+ 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 = regmap_read(data->regmap, VEML6075_CMD_UVA, val);
+ if (ret < 0)
+ return ret;
+
+ *val = veml6075_uva_comp(*val, c1, c2);
+ return IIO_VAL_INT;
+ case CH_UVB:
+ ret = regmap_read(data->regmap, VEML6075_CMD_UVB, val);
+ if (ret < 0)
+ return ret;
+
+ *val = veml6075_uvb_comp(*val, c1, c2);
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+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;
+
+ guard(mutex)(&data->lock);
+ 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_IT_50_MS:
+ return uvia_micro + uvib_micro;
+ case VEML6075_IT_100_MS:
+ case VEML6075_IT_200_MS:
+ case VEML6075_IT_400_MS:
+ case VEML6075_IT_800_MS:
+ 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;
+
+ guard(mutex)(&data->lock);
+
+ ret = veml6075_request_measurement(data);
+ if (ret < 0)
+ return ret;
+
+ ret = veml6075_read_comp(data, &c1, &c2);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(data->regmap, VEML6075_CMD_UVA, &uva);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(data->regmap, VEML6075_CMD_UVB, &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;
+ return IIO_VAL_INT_PLUS_NANO;
+ case CH_UVB:
+ /* resp = 2.1 c/uW/cm2: scale = 0.476190476 */
+ *val = 0;
+ *val2 = 476190476;
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int veml6075_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_INT_TIME:
+ *length = ARRAY_SIZE(veml6075_it_ms);
+ *vals = veml6075_it_ms;
+ *type = IIO_VAL_INT;
+ return IIO_AVAIL_LIST;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+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:
+ ret = veml6075_read_uv_direct(data, chan->channel, val);
+ break;
+ case IIO_CHAN_INFO_PROCESSED:
+ ret = veml6075_read_uvi(data, val, val2);
+ break;
+ case IIO_CHAN_INFO_INT_TIME:
+ 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;
+ }
+
+ return ret;
+}
+
+static int veml6075_write_int_time_ms(struct veml6075_data *data, int val)
+{
+ int i = ARRAY_SIZE(veml6075_it_ms);
+
+ guard(mutex)(&data->lock);
+
+ while (i-- > 0) {
+ if (val == veml6075_it_ms[i])
+ break;
+ }
+ if (i < 0)
+ return -EINVAL;
+
+ return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
+ VEML6075_CONF_IT,
+ FIELD_PREP(VEML6075_CONF_IT, i));
+}
+
+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:
+ ret = veml6075_write_int_time_ms(data, val);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static const struct iio_info veml6075_info = {
+ .read_avail = veml6075_read_avail,
+ .read_raw = veml6075_read_raw,
+ .write_raw = veml6075_write_raw,
+};
+
+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",
+ .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";
+ 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(&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_IT_100_MS) |
+ FIELD_PREP(VEML6075_CONF_AF, VEML6075_AF_ENABLE) |
+ FIELD_PREP(VEML6075_CONF_SD, VEML6075_SD_ENABLE);
+ ret = regmap_write(data->regmap, VEML6075_CMD_CONF, config);
+ if (ret < 0)
+ return ret;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id veml6075_id[] = {
+ { "veml6075" },
+ { }
+};
+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",
+ .of_match_table = veml6075_of_match,
+ },
+ .probe = veml6075_probe,
+ .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-25 15:12:54

by Jonathan Cameron

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

On Sat, 25 Nov 2023 12:56:57 +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 signal measured in counts per
> integration period, where the integration time is configurable.
>
> 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,

A few more minor things. Looks good in general.

Jonathan

> diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c
> new file mode 100644
> index 000000000000..a33488884076
> --- /dev/null
> +++ b/drivers/iio/light/veml6075.c
> @@ -0,0 +1,486 @@
...


> +struct veml6075_data {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct mutex lock; /* integration time/measurement trigger lock */

Could perhaps be clearer. Maybe something like
/* Prevent integration time changing during a measurement */

> +};

> +
> +static int veml6075_shutdown(struct veml6075_data *data)

Only used in one place. Maybe just do the regmap bit directly there?

> +{
> + return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
> + VEML6075_CONF_SD, VEML6075_CONF_SD);
> +}

> +
> +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;
> +
> + 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;

Any of units.h appropriate here? I'm not sure if the / 1000U is a units
thing or not.

> + comp2b_c = (comp2 * VEML6075_D_COEF) / 1000U;
> + uvb_comp = raw_uvb - comp1b_c - comp2b_c;
> +
> + return clamp_val(uvb_comp, 0, U16_MAX);
> +}

> +
> +static int veml6075_read_uvi(struct veml6075_data *data, int *val, int *val2)
> +{
> + int ret, c1, c2, uva, uvb, uvi_micro;
> +
> + guard(mutex)(&data->lock);
> +
> + ret = veml6075_request_measurement(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = veml6075_read_comp(data, &c1, &c2);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(data->regmap, VEML6075_CMD_UVA, &uva);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(data->regmap, VEML6075_CMD_UVB, &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;

MICRO for divisor.
https://elixir.bootlin.com/linux/latest/source/include/linux/units.h#L18


> + *val2 = uvi_micro % 1000000LL;
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}

...

> +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:
> + ret = veml6075_read_uv_direct(data, chan->channel, val);
> + break;
> + case IIO_CHAN_INFO_PROCESSED:
> + ret = veml6075_read_uvi(data, val, val2);
> + break;
> + case IIO_CHAN_INFO_INT_TIME:
> + 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;
> + }
> +
> + return ret;
return in remaining case statements above. No point in break unless
there is shared stuff to do after the switch.
> +}
> +
> +static int veml6075_write_int_time_ms(struct veml6075_data *data, int val)
> +{
> + int i = ARRAY_SIZE(veml6075_it_ms);
> +
> + guard(mutex)(&data->lock);
> +
> + while (i-- > 0) {
> + if (val == veml6075_it_ms[i])
> + break;
> + }
> + if (i < 0)
> + return -EINVAL;
> +
> + return regmap_update_bits(data->regmap, VEML6075_CMD_CONF,
> + VEML6075_CONF_IT,
> + FIELD_PREP(VEML6075_CONF_IT, i));
> +}
> +
> +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:
> + ret = veml6075_write_int_time_ms(data, val);
return here.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
return above and safe a few lines + local variable ret.

> +}
> +
> +static const struct iio_info veml6075_info = {
> + .read_avail = veml6075_read_avail,
> + .read_raw = veml6075_read_raw,
> + .write_raw = veml6075_write_raw,
> +};

...

> +static const struct regmap_config veml6075_regmap_config = {
> + .name = "veml6075",
> + .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,
> +
stray blank line here that should go.
> +};
> +
> +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);

why? Doesn't look like you get it back anywhere in the driver.


> + data->client = client;
> + data->regmap = regmap;
> +
> + mutex_init(&data->lock);
> +
> + indio_dev->name = "veml6075";
> + 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(&client->dev, "vdd");
> + if (ret < 0 && ret != -ENODEV)

I'm lost here. devm_regulator_get_enable() shouldn't return -ENODEV
unless you have specified that an incomplete set of regs are provided.
If you've specified that you should provide a fixed reg.
https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L2194
The paths to not having that provided shouldn't effect normal use (IIRC).

So I'd not expect special handling for -ENODEV.
That would be appropriate if you were using the optional variant, but
this regulator isn't optional (we might just not have described it!)


> + return ret;
> +
> + /* default: 100ms integration time, active force enable, shutdown */
> + config = FIELD_PREP(VEML6075_CONF_IT, VEML6075_IT_100_MS) |
> + FIELD_PREP(VEML6075_CONF_AF, VEML6075_AF_ENABLE) |
> + FIELD_PREP(VEML6075_CONF_SD, VEML6075_SD_ENABLE);
> + ret = regmap_write(data->regmap, VEML6075_CMD_CONF, config);
> + if (ret < 0)
> + return ret;
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}

2023-11-25 16:17:17

by Javier Carrasco

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

On 25.11.23 16:11, Jonathan Cameron wrote:
> On Sat, 25 Nov 2023 12:56:57 +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 signal measured in counts per
>> integration period, where the integration time is configurable.
>>
>> 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,
>
> A few more minor things. Looks good in general.
>
> Jonathan
>
>> +struct veml6075_data {
>> + struct i2c_client *client;
>> + struct regmap *regmap;
>> + struct mutex lock; /* integration time/measurement trigger lock */
>
> Could perhaps be clearer. Maybe something like
> /* Prevent integration time changing during a measurement */
>
It prevents integration time changing as well as measurement triggers
while a measurement is underway. I just wanted to cover both usages with
a short comment in the same line.
>> +
>> +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;
>> +
>> + 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;
>
> Any of units.h appropriate here? I'm not sure if the / 1000U is a units
> thing or not.
>
These divisions are used to scale the coefficients down, as they are
defined as entire numbers. These coefficients have no units and the
resulting value is a count.

I have nothing to add to the rest of your comments. I will start working
on v3.

Thanks again for your thorough review.

Best regards,
Javier Carrasco

2023-11-25 16:47:03

by Jonathan Cameron

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

On Sat, 25 Nov 2023 17:17:06 +0100
Javier Carrasco <[email protected]> wrote:

> On 25.11.23 16:11, Jonathan Cameron wrote:
> > On Sat, 25 Nov 2023 12:56:57 +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 signal measured in counts per
> >> integration period, where the integration time is configurable.
> >>
> >> 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,
> >
> > A few more minor things. Looks good in general.
> >
> > Jonathan
> >
> >> +struct veml6075_data {
> >> + struct i2c_client *client;
> >> + struct regmap *regmap;
> >> + struct mutex lock; /* integration time/measurement trigger lock */
> >
> > Could perhaps be clearer. Maybe something like
> > /* Prevent integration time changing during a measurement */
> >
> It prevents integration time changing as well as measurement triggers
> while a measurement is underway. I just wanted to cover both usages with
> a short comment in the same line.

Ah. Well I misunderstood it, so burn a few more lines :)

> >> +
> >> +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;
> >> +
> >> + 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;
> >
> > Any of units.h appropriate here? I'm not sure if the / 1000U is a units
> > thing or not.
> >
> These divisions are used to scale the coefficients down, as they are
> defined as entire numbers. These coefficients have no units and the
> resulting value is a count.
Fair enough.
>
> I have nothing to add to the rest of your comments. I will start working
> on v3.
>
> Thanks again for your thorough review.
>
> Best regards,
> Javier Carrasco