[RESEND PATCH v2 0/3] Add driver for veml6030 ambient light sensor

[RESEND PATCH v2 0/3] Add driver for veml6030 ambient light sensor

[Rishi Gupta]

The veml6030 is an ambient light sensor from vishay and
is a different hardware from an existing hardware for which
driver currently exist, therefore this driver submission.

* All features; ALS, white channel & power management is
  supported.

* All configurable parameters are supported through standard
  iio sysfs entries. User space can get valid values of any
  parameter (xx_available) and then can write to appropriate
  sysfs entry.

* User space can get ALS & White channel readings through RAW
  IIO interface.

* IIO events are used to notify application whenever threshold
  is crossed. This uses IRQ pin of veml6030.

* Some registers in veml6030 are read only. For these registers
  read callback returns error to user space.

There are 3 patches for this submission:
[PATCH 1/3] iio: light: add driver for veml6030 ambient light sensor
[PATCH 2/3] dt-bindings: iio: light: add veml6030 ALS bindings
[PATCH 3/3] iio: documentation: light: Add veml6030 sysfs documentation

Rishi Gupta (3):
  iio: light: add driver for veml6030 ambient light sensor
  dt-bindings: iio: light: add veml6030 ALS bindings
  iio: documentation: light: Add veml6030 sysfs documentation

 .../ABI/testing/sysfs-bus-iio-light-veml6030       |  49 ++
 .../devicetree/bindings/iio/light/veml6030.yaml    |  62 ++
 drivers/iio/light/Kconfig                          |  11 +
 drivers/iio/light/Makefile                         |   1 +
 drivers/iio/light/veml6030.c                       | 633 +++++++++++++++++++++
 5 files changed, 756 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
 create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml
 create mode 100644 drivers/iio/light/veml6030.c

-- 
2.7.4

[RESEND PATCH v2 1/3] iio: light: add driver for veml6030 ambient light sensor

[Rishi Gupta]

veml6030 is an ambient light sensor from Vishay semiconductors.
It has 16-bit resolution, supports both ambient light measurement
and white channel which is more responsive to wider wavelength
spectrum. It has flexible power saving, integration time and
gain options. Communication with host is over I2C.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v2:
* Added comma after CH_WHITE in enum veml6030_chan so it can be extended
* Removed .scan_type as driver doesn't use buffered mode
* Removed iio_device_unregister() as kernel will take care of cleaning

 drivers/iio/light/Kconfig    |  11 +
 drivers/iio/light/Makefile   |   1 +
 drivers/iio/light/veml6030.c | 633 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 645 insertions(+)
 create mode 100644 drivers/iio/light/veml6030.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 08d7e1e..e455cec 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -495,6 +495,17 @@ config VCNL4035
 	  To compile this driver as a module, choose M here: the
 	  module will be called vcnl4035.
 
+config VEML6030
+	tristate "VEML6030 ambient light sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VEML6030
+	  ambient light sensor (ALS).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called veml6030.
+
 config VEML6070
 	tristate "VEML6070 UV A light sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 00d1f9b..5e0c40b 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_US5182D)		+= us5182d.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
 obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
+obj-$(CONFIG_VEML6030)		+= veml6030.o
 obj-$(CONFIG_VEML6070)		+= veml6070.o
 obj-$(CONFIG_VL6180)		+= vl6180.o
 obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
new file mode 100644
index 0000000..3f6b2bd
--- /dev/null
+++ b/drivers/iio/light/veml6030.c
@@ -0,0 +1,633 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VEML6030 Ambient Light Sensor
+ *
+ * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
+ *
+ * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/regmap.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+/* Device registers */
+#define VEML6030_REG_ALS_CONF    0x00
+#define VEML6030_REG_ALS_WH      0x01
+#define VEML6030_REG_ALS_WL      0x02
+#define VEML6030_REG_ALS_PSM     0x03
+#define VEML6030_REG_ALS_DATA    0x04
+#define VEML6030_REG_WHITE_DATA  0x05
+#define VEML6030_REG_ALS_INT     0x06
+
+/* Bit masks for specific functionality */
+#define VEML6030_ALS_IT       GENMASK(9, 6)
+#define VEML6030_PSM          GENMASK(2, 1)
+#define VEML6030_ALS_PERS     GENMASK(5, 4)
+#define VEML6030_ALS_GAIN     GENMASK(12, 11)
+#define VEML6030_PSM_EN       BIT(0)
+#define VEML6030_INT_TH_LOW   BIT(15)
+#define VEML6030_INT_TH_HIGH  BIT(14)
+#define VEML6030_ALS_INT_EN   BIT(1)
+#define VEML6030_ALS_SD       BIT(0)
+
+struct veml6030_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+};
+
+static IIO_CONST_ATTR_INT_TIME_AVAIL("0.025 0.05 0.1 0.2 0.4 0.8");
+
+static struct attribute *veml6030_attributes[] = {
+	&iio_const_attr_integration_time_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group veml6030_attr_group = {
+	.attrs = veml6030_attributes,
+};
+
+/*
+ * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
+ * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
+ */
+static ssize_t veml6030_scale_available(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+	return sprintf(buf, "%s\n", "0.125 0.25 1.0 2.0");
+}
+
+/*
+ * Persistence = 1/2/4/8 x integration time
+ * Minimum time for which light readings must stay above configured
+ * threshold to assert interrupt.
+ */
+static ssize_t veml6030_persistence_available(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+	return sprintf(buf, "%s\n", "1 2 4 8");
+}
+
+/*
+ * Power saving modes supported.
+ * 1/2/3/4 corresponds to PSM modes 1/2/3/4 respectively.
+ */
+static ssize_t veml6030_psm_available(struct iio_dev *indio_dev,
+	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+	return sprintf(buf, "%s\n", "1 2 3 4");
+}
+
+ssize_t veml6030_set_psm(struct iio_dev *indio_dev, uintptr_t priv,
+			const struct iio_chan_spec *chan, const char *buf,
+			size_t len)
+{
+	int ret;
+	unsigned int val;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = kstrtouint(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val < 1 || val > 4)
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+				 VEML6030_PSM | VEML6030_PSM_EN, val - 1);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update psm value %d\n", ret);
+		return ret;
+	}
+
+	return len;
+}
+
+#define VEML6030_AVAIL(_name, _read) \
+{ \
+	.name = (_name "_available"), \
+	.read = _read, \
+	.shared = IIO_SEPARATE, \
+}
+
+static const struct iio_chan_spec_ext_info veml6030_ext_info[] = {
+	{
+		.name = "psm",
+		.write = veml6030_set_psm,
+		.shared = IIO_SEPARATE,
+	},
+	VEML6030_AVAIL("psm", veml6030_psm_available),
+	VEML6030_AVAIL("scale", veml6030_scale_available),
+	VEML6030_AVAIL("period", veml6030_persistence_available),
+	{ },
+};
+
+static const struct iio_event_spec veml6030_event_spec[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+/*
+ * This is used to specify both scan index and numerical
+ * channel number in channel specifications.
+ */
+enum veml6030_chan {
+	CH_ALS,
+	CH_WHITE,
+};
+
+static const struct iio_chan_spec veml6030_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = CH_ALS,
+		.scan_index = CH_ALS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_INT_TIME) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.event_spec = veml6030_event_spec,
+		.num_event_specs = ARRAY_SIZE(veml6030_event_spec),
+		.ext_info = veml6030_ext_info,
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_WHITE,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.scan_index = CH_WHITE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+	},
+};
+
+static const struct regmap_config veml6030_regmap_config = {
+	.name = "veml6030_regmap",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = VEML6030_REG_ALS_INT,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static int veml6030_set_als_gain(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, gain;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val == 0 && val2 == 125000)
+		gain = 0x02;
+	else if (val == 0 && val2 == 250000)
+		gain = 0x03;
+	else if (val == 1 && val2 == 0)
+		gain = 0x00;
+	else if (val == 2 && val2 == 0)
+		gain = 0x01;
+	else
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_GAIN, gain);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't set ALS gain %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int veml6030_set_als_persistence(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_PERS, val);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't set persistence value %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int veml6030_set_als_threshold(struct iio_dev *indio_dev,
+						int val, int val2, int dir)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val > 0xFFFF || val < 0 || val2)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_RISING) {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set high threshold %d\n", ret);
+	} else {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set low threshold %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int veml6030_set_integration_time(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, int_time;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val)
+		return -EINVAL;
+
+	switch (val2) {
+	case 25000:
+		int_time = 12;
+		break;
+	case 50000:
+		int_time = 8;
+		break;
+	case 100000:
+		int_time = 0;
+		break;
+	case 200000:
+		int_time = 1;
+		break;
+	case 400000:
+		int_time = 2;
+		break;
+	case 800000:
+		int_time = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_IT, int_time);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update integration time %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+/*
+ * Light reading in lux is obtained by multiplying a constant
+ * specified in appnote 84367 in the lux calculation table
+ * to the raw reading.
+ */
+static int veml6030_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	int ret, rdval;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct regmap *regmap = data->regmap;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &rdval);
+			if (ret < 0) {
+				dev_err(&data->client->dev, "can't read ALS data\n");
+				return ret;
+			}
+			*val = rdval;
+			return IIO_VAL_INT;
+		case IIO_INTENSITY:
+			ret = regmap_read(regmap, VEML6030_REG_WHITE_DATA, &rdval);
+			if (ret < 0) {
+				dev_err(&data->client->dev, "can't read white data\n");
+				return ret;
+			}
+			*val = rdval;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_integration_time(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_als_gain(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_thresh(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, enum iio_event_info info,
+		int val, int val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		return veml6030_set_als_threshold(indio_dev, val, val2, dir);
+	case IIO_EV_INFO_PERIOD:
+		return veml6030_set_als_persistence(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, int state)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (!data->client->irq || state < 0 || state > 1)
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_INT_EN, state);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't enable threshold interrupt %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct iio_info veml6030_info = {
+	.read_raw  = veml6030_read_raw,
+	.write_raw = veml6030_write_raw,
+	.write_event_value	= veml6030_write_thresh,
+	.write_event_config	= veml6030_write_interrupt_config,
+	.attrs = &veml6030_attr_group,
+};
+
+static irqreturn_t veml6030_event_handler(int irq, void *private)
+{
+	int ret, reg, evtdir;
+	struct iio_dev *indio_dev = private;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't enable interrupt status %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	/* Spurious interrupt handling */
+	if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))
+		return IRQ_HANDLED;
+
+	if (reg & VEML6030_INT_TH_HIGH)
+		evtdir = IIO_EV_DIR_RISING;
+	else
+		evtdir = IIO_EV_DIR_FALLING;
+
+	iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
+					0, IIO_EV_TYPE_THRESH, evtdir),
+					iio_get_time_ns(indio_dev));
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Corresponding to 1200 ms as refresh time, set ALS gain to
+ * ALS gain x 2, integration time to 200 ms and PSM to mode 2.
+ * Set persistence to 1 x integration time, and the threashold
+ * interrupt disabled by default.
+ */
+static int veml6030_hw_init(struct iio_dev *indio_dev)
+{
+	int ret, val;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x850);
+	if (ret) {
+		dev_err(&client->dev, "can't setup ALS configs %d\n", ret);
+		return ret;
+	}
+
+	/* Wait 4 ms to let processor & oscillator start correctly */
+	usleep_range(3990, 4000);
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
+	if (ret) {
+		dev_err(&client->dev, "can't setup defaults for PSM %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
+	if (ret) {
+		dev_err(&client->dev, "can't read high threshold %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
+	if (ret) {
+		dev_err(&client->dev, "can't read low threshold %d\n", ret);
+		return ret;
+	}
+
+	/* Clear stale interrupt status bits if any during start */
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"can't read ALS interrupt status %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int veml6030_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	int ret;
+	struct veml6030_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
+		return -EOPNOTSUPP;
+	}
+
+	regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "can't setup regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	data->regmap = regmap;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &veml6030_info;
+	indio_dev->name = "veml6030";
+	indio_dev->channels = veml6030_channels;
+	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, veml6030_event_handler,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"veml6030", indio_dev);
+		if (ret < 0) {
+			dev_err(&client->dev,
+					"irq %d request failed\n", client->irq);
+			return ret;
+		}
+	}
+
+	ret = veml6030_hw_init(indio_dev);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static int veml6030_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 0x01);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int veml6030_runtime_suspend(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 0x01);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+				"can't suspend veml6030 light sensor\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int veml6030_runtime_resume(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 0x00);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+				"can't resume veml6030 light sensor\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops veml6030_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
+				veml6030_runtime_resume, NULL)
+};
+#endif
+
+static const struct of_device_id veml6030_of_match[] = {
+	{ .compatible = "vishay,veml6030" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, veml6030_of_match);
+
+static const struct i2c_device_id veml6030_id[] = {
+	{ "veml6030", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, veml6030_id);
+
+static struct i2c_driver veml6030_driver = {
+	.driver = {
+		.name = "veml6030",
+		.of_match_table = of_match_ptr(veml6030_of_match),
+#ifdef CONFIG_PM
+		.pm = &veml6030_pm_ops,
+#endif
+	},
+	.probe = veml6030_probe,
+	.remove = veml6030_remove,
+	.id_table = veml6030_id,
+};
+module_i2c_driver(veml6030_driver);
+
+MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
+MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [RESEND PATCH v2 1/3] iio: light: add driver for veml6030 ambient light sensor

[Jonathan Cameron]

On Tue, 24 Sep 2019 16:21:56 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

> veml6030 is an ambient light sensor from Vishay semiconductors.
> It has 16-bit resolution, supports both ambient light measurement
> and white channel which is more responsive to wider wavelength
> spectrum. It has flexible power saving, integration time and
> gain options. Communication with host is over I2C.
> 
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>

Generally looks good. Various comments inline.

Jonathan

> ---
> Changes in v2:
> * Added comma after CH_WHITE in enum veml6030_chan so it can be extended
> * Removed .scan_type as driver doesn't use buffered mode
> * Removed iio_device_unregister() as kernel will take care of cleaning
> 
>  drivers/iio/light/Kconfig    |  11 +
>  drivers/iio/light/Makefile   |   1 +
>  drivers/iio/light/veml6030.c | 633 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 645 insertions(+)
>  create mode 100644 drivers/iio/light/veml6030.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 08d7e1e..e455cec 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -495,6 +495,17 @@ config VCNL4035
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called vcnl4035.
>  
> +config VEML6030
> +	tristate "VEML6030 ambient light sensor"
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Say Y here if you want to build a driver for the Vishay VEML6030
> +	  ambient light sensor (ALS).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called veml6030.
> +
>  config VEML6070
>  	tristate "VEML6070 UV A light sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 00d1f9b..5e0c40b 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)		+= tsl4531.o
>  obj-$(CONFIG_US5182D)		+= us5182d.o
>  obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
>  obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
> +obj-$(CONFIG_VEML6030)		+= veml6030.o
>  obj-$(CONFIG_VEML6070)		+= veml6070.o
>  obj-$(CONFIG_VL6180)		+= vl6180.o
>  obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> new file mode 100644
> index 0000000..3f6b2bd
> --- /dev/null
> +++ b/drivers/iio/light/veml6030.c
> @@ -0,0 +1,633 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VEML6030 Ambient Light Sensor
> + *
> + * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
> + *
> + * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/regmap.h>
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +
> +/* Device registers */
> +#define VEML6030_REG_ALS_CONF    0x00
> +#define VEML6030_REG_ALS_WH      0x01
> +#define VEML6030_REG_ALS_WL      0x02
> +#define VEML6030_REG_ALS_PSM     0x03
> +#define VEML6030_REG_ALS_DATA    0x04
> +#define VEML6030_REG_WHITE_DATA  0x05
> +#define VEML6030_REG_ALS_INT     0x06
> +
> +/* Bit masks for specific functionality */
> +#define VEML6030_ALS_IT       GENMASK(9, 6)
> +#define VEML6030_PSM          GENMASK(2, 1)
> +#define VEML6030_ALS_PERS     GENMASK(5, 4)
> +#define VEML6030_ALS_GAIN     GENMASK(12, 11)
> +#define VEML6030_PSM_EN       BIT(0)
> +#define VEML6030_INT_TH_LOW   BIT(15)
> +#define VEML6030_INT_TH_HIGH  BIT(14)
> +#define VEML6030_ALS_INT_EN   BIT(1)
> +#define VEML6030_ALS_SD       BIT(0)
> +
> +struct veml6030_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +};
> +
> +static IIO_CONST_ATTR_INT_TIME_AVAIL("0.025 0.05 0.1 0.2 0.4 0.8");
> +
> +static struct attribute *veml6030_attributes[] = {
> +	&iio_const_attr_integration_time_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group veml6030_attr_group = {
> +	.attrs = veml6030_attributes,
> +};
> +
> +/*
> + * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
> + * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
> + */
> +static ssize_t veml6030_scale_available(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	return sprintf(buf, "%s\n", "0.125 0.25 1.0 2.0");

These can be more easily done with const attributes as well.
For this one we also have the option of using the callbacks in conjunction
with the _available masks in the chan_spec to generate this automatically.

> +}
> +
> +/*
> + * Persistence = 1/2/4/8 x integration time
> + * Minimum time for which light readings must stay above configured
> + * threshold to assert interrupt.
> + */
> +static ssize_t veml6030_persistence_available(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	return sprintf(buf, "%s\n", "1 2 4 8");
> +}
> +
> +/*
> + * Power saving modes supported.
> + * 1/2/3/4 corresponds to PSM modes 1/2/3/4 respectively.
> + */
> +static ssize_t veml6030_psm_available(struct iio_dev *indio_dev,
> +	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> +{
> +	return sprintf(buf, "%s\n", "1 2 3 4");
> +}
> +
> +ssize_t veml6030_set_psm(struct iio_dev *indio_dev, uintptr_t priv,
> +			const struct iio_chan_spec *chan, const char *buf,
> +			size_t len)
> +{
> +	int ret;
> +	unsigned int val;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = kstrtouint(buf, 0, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (val < 1 || val > 4)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> +				 VEML6030_PSM | VEML6030_PSM_EN, val - 1);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't update psm value %d\n", ret);
> +		return ret;
> +	}
> +
> +	return len;
> +}
> +
> +#define VEML6030_AVAIL(_name, _read) \
> +{ \
> +	.name = (_name "_available"), \
> +	.read = _read, \
> +	.shared = IIO_SEPARATE, \
> +}
> +
> +static const struct iio_chan_spec_ext_info veml6030_ext_info[] = {
> +	{
> +		.name = "psm",
> +		.write = veml6030_set_psm,
> +		.shared = IIO_SEPARATE,

Hmm. Power saving modes.  These normally correspond to something very
specific such as reduced accuracy of slow sampling rates.  A user
will have no idea how to set a magic mode 1 or mode 2 variable so
that should be avoided if possible.

Here it seems fairly directly connected to the fastest a reading can
be taken, which corresponds to samping_frequency (kind of anyway)
> +	},
> +	VEML6030_AVAIL("psm", veml6030_psm_available),
> +	VEML6030_AVAIL("scale", veml6030_scale_available),

The use of scale currently is a bit odd. It's the value that should be
applied to the raw sensor reading to get a value of illuminance in lux.
As before, name it fully to match the channel.  Side effect is that
will then be covered by generic docs so no need to add your own.

> +	VEML6030_AVAIL("period", veml6030_persistence_available),

As this events only exist for one type of channel, I'd give
them the full name to just put _period on the end of the one
generated by the events.  This is also only relevant to events
so should be in the events directory.

 
> +	{ },
> +};
> +
> +static const struct iio_event_spec veml6030_event_spec[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
> +/*
> + * This is used to specify both scan index and numerical
> + * channel number in channel specifications.
> + */
> +enum veml6030_chan {
> +	CH_ALS,
> +	CH_WHITE,
> +};
> +
> +static const struct iio_chan_spec veml6030_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.channel = CH_ALS,
> +		.scan_index = CH_ALS,

You didn't get a reply on the previous version to your question on this.
scan_index is only used in buffered mode so you don't need it here.

> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				BIT(IIO_CHAN_INFO_INT_TIME) |
> +				BIT(IIO_CHAN_INFO_SCALE),
> +		.event_spec = veml6030_event_spec,
> +		.num_event_specs = ARRAY_SIZE(veml6030_event_spec),

As mentioned below... With optional interrupts you want two versions
of this and pick one dependent on whether the interrupts are available
or not.  That way we don't expose interfaces that can't work.

> +		.ext_info = veml6030_ext_info,
> +	},
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_WHITE,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_BOTH,
> +		.scan_index = CH_WHITE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
> +	},
> +};
> +
> +static const struct regmap_config veml6030_regmap_config = {
> +	.name = "veml6030_regmap",
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = VEML6030_REG_ALS_INT,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static int veml6030_set_als_gain(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret, gain;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val == 0 && val2 == 125000)
> +		gain = 0x02;
> +	else if (val == 0 && val2 == 250000)
> +		gain = 0x03;
> +	else if (val == 1 && val2 == 0)
> +		gain = 0x00;
> +	else if (val == 2 && val2 == 0)
> +		gain = 0x01;
> +	else
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_GAIN, gain);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't set ALS gain %d\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_set_als_persistence(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_PERS, val);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't set persistence value %d\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_set_als_threshold(struct iio_dev *indio_dev,
> +						int val, int val2, int dir)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val > 0xFFFF || val < 0 || val2)
> +		return -EINVAL;
> +
> +	if (dir == IIO_EV_DIR_RISING) {
> +		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
> +		if (ret)
> +			dev_err(&data->client->dev,
> +					"can't set high threshold %d\n", ret);
> +	} else {
> +		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
> +		if (ret)
> +			dev_err(&data->client->dev,
> +					"can't set low threshold %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_set_integration_time(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret, int_time;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val)
> +		return -EINVAL;
> +
> +	switch (val2) {
> +	case 25000:
> +		int_time = 12;
> +		break;
> +	case 50000:
> +		int_time = 8;
> +		break;
> +	case 100000:
> +		int_time = 0;
> +		break;
> +	case 200000:
> +		int_time = 1;
> +		break;
> +	case 400000:
> +		int_time = 2;
> +		break;
> +	case 800000:
> +		int_time = 3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_IT, int_time);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't update integration time %d\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Light reading in lux is obtained by multiplying a constant
> + * specified in appnote 84367 in the lux calculation table
> + * to the raw reading.

If it's an actual constant then we should provide SCALE to allow
userspace to apply it.

> + */
> +static int veml6030_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	int ret, rdval;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +	struct regmap *regmap = data->regmap;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &rdval);
> +			if (ret < 0) {
> +				dev_err(&data->client->dev, "can't read ALS data\n");
> +				return ret;
> +			}
> +			*val = rdval;
> +			return IIO_VAL_INT;
> +		case IIO_INTENSITY:
> +			ret = regmap_read(regmap, VEML6030_REG_WHITE_DATA, &rdval);
> +			if (ret < 0) {
> +				dev_err(&data->client->dev, "can't read white data\n");
> +				return ret;
> +			}
> +			*val = rdval;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6030_write_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val, int val2, long mask)
> +{
> +	switch (mask) {

You should be able to read either of these back as well so would expect
to see the code for that in read_raw.

> +	case IIO_CHAN_INFO_INT_TIME:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			return veml6030_set_integration_time(indio_dev, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			return veml6030_set_als_gain(indio_dev, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6030_write_thresh(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir, enum iio_event_info info,
> +		int val, int val2)
> +{
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		return veml6030_set_als_threshold(indio_dev, val, val2, dir);
> +	case IIO_EV_INFO_PERIOD:
> +		return veml6030_set_als_persistence(indio_dev, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir, int state)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (!data->client->irq || state < 0 || state > 1)

So shouldn't be possible to get here if you don't have an irq.
Also, fairly sure it can't pass you anything other than 0 or 1.

See below for what you should change so you can't get here if
no interrupt.

> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_INT_EN, state);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't enable threshold interrupt %d\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info veml6030_info = {
> +	.read_raw  = veml6030_read_raw,
> +	.write_raw = veml6030_write_raw,
> +	.write_event_value	= veml6030_write_thresh,
> +	.write_event_config	= veml6030_write_interrupt_config,
> +	.attrs = &veml6030_attr_group,
> +};
> +
> +static irqreturn_t veml6030_event_handler(int irq, void *private)
> +{
> +	int ret, reg, evtdir;
> +	struct iio_dev *indio_dev = private;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't enable interrupt status %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	/* Spurious interrupt handling */
> +	if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))

If it's spurious, return IRQ_NONE;  That will allow the core kernel
spurious interrupt handling to know it's going on and take measures
as appropriate to block what is probably broken hardware.

> +		return IRQ_HANDLED;
> +
> +	if (reg & VEML6030_INT_TH_HIGH)
> +		evtdir = IIO_EV_DIR_RISING;
> +	else
> +		evtdir = IIO_EV_DIR_FALLING;
> +
> +	iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
> +					0, IIO_EV_TYPE_THRESH, evtdir),
> +					iio_get_time_ns(indio_dev));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * Corresponding to 1200 ms as refresh time, set ALS gain to
> + * ALS gain x 2, integration time to 200 ms and PSM to mode 2.
> + * Set persistence to 1 x integration time, and the threashold
> + * interrupt disabled by default.
> + */
> +static int veml6030_hw_init(struct iio_dev *indio_dev)
> +{
> +	int ret, val;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +	struct i2c_client *client = data->client;
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x850);

I assume this is the call we 'unwind' when we call the stuff in remove.
So add the devm_add_action_or_reset after this so that we will have
correct handling for errors later in this function.

> +	if (ret) {
> +		dev_err(&client->dev, "can't setup ALS configs %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Wait 4 ms to let processor & oscillator start correctly */
> +	usleep_range(3990, 4000);
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> +				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
> +	if (ret) {
> +		dev_err(&client->dev, "can't setup defaults for PSM %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
> +	if (ret) {
> +		dev_err(&client->dev, "can't read high threshold %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
> +	if (ret) {
> +		dev_err(&client->dev, "can't read low threshold %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Clear stale interrupt status bits if any during start */
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
> +	if (ret < 0) {
> +		dev_err(&client->dev,
> +			"can't read ALS interrupt status %d\n", ret);
> +		return ret;

Drop the return ret.. See below. I'm lazy so won't comment on any more of htese.

> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_probe(struct i2c_client *client,
> +			  const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct veml6030_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "can't setup regmap\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +	data->regmap = regmap;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &veml6030_info;
> +	indio_dev->name = "veml6030";
> +	indio_dev->channels = veml6030_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	if (client->irq) {

If interrupts are optional, you need to provide a separate chan_spec array
that doesn't define the events.  We simply don't want the interface to appear
if we can't support it.

> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, veml6030_event_handler,
> +						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +						"veml6030", indio_dev);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +					"irq %d request failed\n", client->irq);
> +			return ret;
> +		}
> +	}
> +
> +	ret = veml6030_hw_init(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);

If this fails I think we should be doing the same stuff as in remove
to turn the device off?  Note that the devm_add_action_or_reset
method suggested below will avoid the need to do that manually.

> +}
> +
> +static int veml6030_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +				 VEML6030_ALS_SD, 0x01);

This gets run before the userspace interfaces are removed when
devm management clears up the devm_iio_device_register.

This we have a nasty race where the device is off by userspace
calls can still come in.

Probably a case of calling devm_add_action_or_reset at appropriate
place in probe to ensure that everything occurs in the right order.
Then you can just git rid of the remove function entirely.

> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
There is a general move away from using ifdef CONFIG_PM in
favour of marking these functions
__maybe_unused.

Ends up simpler and tends to reduce the number of esoteric
build issues.

> +static int veml6030_runtime_suspend(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +				 VEML6030_ALS_SD, 0x01);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +				"can't suspend veml6030 light sensor\n");
> +		return ret;

Drop this return ret, the one below does the same thing and one
of the static analysers will fire on this and we'll get a 'fix'
fairly quickly if we don't do it now ;)

> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_runtime_resume(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +				 VEML6030_ALS_SD, 0x00);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +				"can't resume veml6030 light sensor\n");
> +		return ret;

Drop the return ret here. The one below will do the same thing.

> +	}
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops veml6030_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
> +				veml6030_runtime_resume, NULL)
> +};
> +#endif
> +
> +static const struct of_device_id veml6030_of_match[] = {
> +	{ .compatible = "vishay,veml6030" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, veml6030_of_match);
> +
> +static const struct i2c_device_id veml6030_id[] = {
> +	{ "veml6030", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, veml6030_id);
> +
> +static struct i2c_driver veml6030_driver = {
> +	.driver = {
> +		.name = "veml6030",
> +		.of_match_table = of_match_ptr(veml6030_of_match),

Don't use of_match_ptr.  Since we now have the magic PRP001
ACPI device type which uses the device tree match, we should
ensure it's always visible.

> +#ifdef CONFIG_PM
> +		.pm = &veml6030_pm_ops,

If you use the __maybe_unused stuff above and assign it without
the ifdef it should be fine (and cleaner code).

> +#endif
> +	},
> +	.probe = veml6030_probe,
> +	.remove = veml6030_remove,
> +	.id_table = veml6030_id,
> +};
> +module_i2c_driver(veml6030_driver);
> +
> +MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
> +MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
> +MODULE_LICENSE("GPL v2");

[PATCH v3 1/3] iio: light: add driver for veml6030 ambient light sensor

[Rishi Gupta]

veml6030 is an ambient light sensor from Vishay semiconductors.
It has 16-bit resolution, supports both ambient light measurement
and white channel which is more responsive to wider wavelength
spectrum. It has flexible power saving, integration time and
gain options. Communication with host is over I2C.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v3:
* Added appnote link in topmost comments section
* Dropped 'return ret' statements wherever not needed
* Removed .scan_index from channel specifications, not needed
* If irq is not enabled, events interfaces are not exposed now
* Return IRQ_NONE for spurious interrupt
* Removed CONFIG_PM, added __maybe_unused in power routines
* Removed of_match_ptr when specifying DT device id to match
* Corrected & documented sequence in veml6030_write_interrupt_config()
* Removed veml6030_remove() & added devm_add_action_or_reset functionality
* Added support to read integration time, gain, thresholds, period
* Removed including mutex.h not needed
* Set 100 ms integration & 1/8 gain during probe for better accuracy
* Used IIO_CONST_ATTR to create sysfs entries "_available"
* Minor cosmetics like everything in lower case

Changes in v2:
* Added comma after CH_WHITE in enum veml6030_chan so it can be extended
* Removed .scan_type as driver doesn't use buffered mode
* Removed iio_device_unregister() as kernel will take care of cleaning

 drivers/iio/light/Kconfig    |  11 +
 drivers/iio/light/Makefile   |   1 +
 drivers/iio/light/veml6030.c | 903 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 915 insertions(+)
 create mode 100644 drivers/iio/light/veml6030.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 4a1a883..f9d27ef 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -496,6 +496,17 @@ config VCNL4035
 	  To compile this driver as a module, choose M here: the
 	  module will be called vcnl4035.

+config VEML6030
+	tristate "VEML6030 ambient light sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VEML6030
+	  ambient light sensor (ALS).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called veml6030.
+
 config VEML6070
 	tristate "VEML6070 UV A light sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 00d1f9b..5e0c40b 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_US5182D)		+= us5182d.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
 obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
+obj-$(CONFIG_VEML6030)		+= veml6030.o
 obj-$(CONFIG_VEML6070)		+= veml6070.o
 obj-$(CONFIG_VL6180)		+= vl6180.o
 obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
new file mode 100644
index 0000000..d0d4e42
--- /dev/null
+++ b/drivers/iio/light/veml6030.c
@@ -0,0 +1,903 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VEML6030 Ambient Light Sensor
+ *
+ * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
+ *
+ * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
+ * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/regmap.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+/* Device registers */
+#define VEML6030_REG_ALS_CONF   0x00
+#define VEML6030_REG_ALS_WH     0x01
+#define VEML6030_REG_ALS_WL     0x02
+#define VEML6030_REG_ALS_PSM    0x03
+#define VEML6030_REG_ALS_DATA   0x04
+#define VEML6030_REG_WH_DATA    0x05
+#define VEML6030_REG_ALS_INT    0x06
+
+/* Bit masks for specific functionality */
+#define VEML6030_ALS_IT       GENMASK(9, 6)
+#define VEML6030_PSM          GENMASK(2, 1)
+#define VEML6030_ALS_PERS     GENMASK(5, 4)
+#define VEML6030_ALS_GAIN     GENMASK(12, 11)
+#define VEML6030_PSM_EN       BIT(0)
+#define VEML6030_INT_TH_LOW   BIT(15)
+#define VEML6030_INT_TH_HIGH  BIT(14)
+#define VEML6030_ALS_INT_EN   BIT(1)
+#define VEML6030_ALS_SD       BIT(0)
+
+/*
+ * The resolution depends on both gain and integration time. The
+ * cur_resolution stores one of the resolution mentioned in the
+ * table during startup and gets updated whenever integration time
+ * or gain is changed.
+ *
+ * Table 'resolution and maximum detection range' in appnote 84367
+ * is visualized as a 2D array. The cur_gain stores index of gain
+ * in this table (0-3) while the cur_integration_time holds index
+ * of integration time (0-5).
+ */
+struct veml6030_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	int cur_resolution;
+	int cur_gain;
+	int cur_integration_time;
+};
+
+/* Integration time available in seconds */
+static IIO_CONST_ATTR(in_illuminance_integration_time_available,
+				"0.025 0.05 0.1 0.2 0.4 0.8");
+
+/*
+ * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
+ * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
+ */
+static IIO_CONST_ATTR(in_illuminance_scale_available,
+				"0.125 0.25 1.0 2.0");
+
+/*
+ * Power saving modes 1/2/3/4.
+ * Products can achieve a trade-off between power savings and
+ * frequency of als latest readings available through psm.
+ */
+static IIO_CONST_ATTR(in_illuminance_psm_available,
+				"1 2 3 4");
+
+static ssize_t in_illuminance_psm_store(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf, size_t len)
+{
+	int ret;
+	unsigned int val;
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (kstrtouint(buf, 0, &val))
+		return -EINVAL;
+
+	if (val < 1 || val > 4)
+		return -EINVAL;
+
+	/* update bits 1-2 */
+	val = ((val - 1) << 1);
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+					VEML6030_PSM, val);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update psm value %d\n", ret);
+		return ret;
+	}
+
+	return len;
+}
+
+static ssize_t in_illuminance_psm_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret, reg;
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_PSM, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read psm register %d\n", ret);
+		return ret;
+	}
+
+	return sprintf(buf, "%d\n", (((reg >> 1) & 0x03) + 1));
+}
+
+static IIO_DEVICE_ATTR_RW(in_illuminance_psm, 0);
+
+static struct attribute *veml6030_attributes[] = {
+	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance_psm_available.dev_attr.attr,
+	&iio_dev_attr_in_illuminance_psm.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group veml6030_attr_group = {
+	.attrs = veml6030_attributes,
+};
+
+/*
+ * Persistence = 1/2/4/8 x integration time
+ * Minimum time for which light readings must stay above configured
+ * threshold to assert interrupt.
+ */
+static IIO_CONST_ATTR(in_illuminance_period_available,
+				"1 2 4 8");
+
+static struct attribute *veml6030_event_attributes[] = {
+	&iio_const_attr_in_illuminance_period_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group veml6030_event_attr_group = {
+	.attrs = veml6030_event_attributes,
+};
+
+static int veml6030_als_pwr_on(struct veml6030_data *data)
+{
+	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 0);
+}
+
+static int veml6030_als_shut_down(struct veml6030_data *data)
+{
+	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 1);
+}
+
+static void veml6030_als_shut_down_action(void *data)
+{
+	veml6030_als_shut_down(data);
+}
+
+static const struct iio_event_spec veml6030_event_spec[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+/* Channel number */
+enum veml6030_chan {
+	CH_ALS,
+	CH_WHITE,
+};
+
+static const struct iio_chan_spec veml6030_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = CH_ALS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_PROCESSED) |
+				BIT(IIO_CHAN_INFO_INT_TIME) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.event_spec = veml6030_event_spec,
+		.num_event_specs = ARRAY_SIZE(veml6030_event_spec),
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_WHITE,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_PROCESSED),
+	},
+};
+
+static const struct regmap_config veml6030_regmap_config = {
+	.name = "veml6030_regmap",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = VEML6030_REG_ALS_INT,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static int veml6030_read_persistence(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	*val = 1 << ((reg >> 3) & 0x03);
+	*val2 = 0;
+	return IIO_VAL_INT;
+}
+
+static int veml6030_write_persistence(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_PERS, val << 4);
+	if (ret)
+		dev_err(&data->client->dev,
+				"can't set persistence value %d\n", ret);
+
+	return ret;
+}
+
+static int veml6030_set_als_gain(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, new_gain, gain_idx;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val == 0 && val2 == 125000) {
+		new_gain = 0x1000; /* 0x02 << 11 */
+		gain_idx = 3;
+	} else if (val == 0 && val2 == 250000) {
+		new_gain = 0x1800;
+		gain_idx = 2;
+	} else if (val == 1 && val2 == 0) {
+		new_gain = 0x00;
+		gain_idx = 1;
+	} else if (val == 2 && val2 == 0) {
+		new_gain = 0x800;
+		gain_idx = 0;
+	} else {
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_GAIN, new_gain);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't set als gain %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Cache currently set gain & update resolution. For every
+	 * increase in the gain to next level, resolution is halved
+	 * and vice-versa.
+	 */
+	if (data->cur_gain < gain_idx)
+		data->cur_resolution <<= gain_idx - data->cur_gain;
+	else if (data->cur_gain > gain_idx)
+		data->cur_resolution >>= data->cur_gain - gain_idx;
+
+	data->cur_gain = gain_idx;
+
+	return ret;
+}
+
+static int veml6030_get_als_gain(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	switch ((reg >> 11) & 0x03) {
+	case 0:
+		*val = 1;
+		*val2 = 0;
+		break;
+	case 1:
+		*val = 2;
+		*val2 = 0;
+		break;
+	case 2:
+		*val = 0;
+		*val2 = 125000;
+		break;
+	case 3:
+		*val = 0;
+		*val2 = 250000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int veml6030_read_thresh(struct iio_dev *indio_dev,
+						int *val, int *val2, int dir)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (dir == IIO_EV_DIR_RISING)
+		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WH, &reg);
+	else
+		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WL, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als threshold value %d\n", ret);
+		return ret;
+	}
+
+	*val = reg & 0xffff;
+	*val2 = 0;
+	return IIO_VAL_INT;
+}
+
+static int veml6030_write_thresh(struct iio_dev *indio_dev,
+						int val, int val2, int dir)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val > 0xFFFF || val < 0 || val2)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_RISING) {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set high threshold %d\n", ret);
+	} else {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set low threshold %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int veml6030_get_intgrn_tm(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	switch ((reg >> 6) & 0xF) {
+	case 0:
+		*val2 = 100000;
+		break;
+	case 1:
+		*val2 = 200000;
+		break;
+	case 2:
+		*val2 = 400000;
+		break;
+	case 3:
+		*val2 = 800000;
+		break;
+	case 8:
+		*val2 = 50000;
+		break;
+	case 12:
+		*val2 = 25000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*val = 0;
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int veml6030_set_intgrn_tm(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, new_int_time, int_idx;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val)
+		return -EINVAL;
+
+	switch (val2) {
+	case 25000:
+		new_int_time = 0x300;
+		int_idx = 5;
+		break;
+	case 50000:
+		new_int_time = 0x200;
+		int_idx = 4;
+		break;
+	case 100000:
+		new_int_time = 0x00;
+		int_idx = 3;
+		break;
+	case 200000:
+		new_int_time = 0x40;
+		int_idx = 2;
+		break;
+	case 400000:
+		new_int_time = 0x80;
+		int_idx = 1;
+		break;
+	case 800000:
+		new_int_time = 0xC0;
+		int_idx = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_IT, new_int_time);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update als integration time %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Cache current integration time and update resolution. For every
+	 * increase in integration time to next level, resolution is halved
+	 * and vice-versa.
+	 */
+	if (data->cur_integration_time < int_idx)
+		data->cur_resolution <<= int_idx - data->cur_integration_time;
+	else if (data->cur_integration_time > int_idx)
+		data->cur_resolution >>= data->cur_integration_time - int_idx;
+
+	data->cur_integration_time = int_idx;
+
+	return ret;
+}
+
+/*
+ * Provide both raw as well as light reading in lux.
+ * light (in lux) = resolution * raw reading
+ */
+static int veml6030_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct regmap *regmap = data->regmap;
+	struct device *dev = &data->client->dev;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &reg);
+			if (ret < 0) {
+				dev_err(dev, "can't read als data %d\n", ret);
+				return ret;
+			}
+			if (mask == IIO_CHAN_INFO_PROCESSED) {
+				*val = (reg * data->cur_resolution) / 10000;
+				*val2 = (reg * data->cur_resolution) % 10000;
+				return IIO_VAL_INT_PLUS_MICRO;
+			}
+			*val = reg;
+			*val2 = 0;
+			return IIO_VAL_INT;
+		case IIO_INTENSITY:
+			ret = regmap_read(regmap, VEML6030_REG_WH_DATA, &reg);
+			if (ret < 0) {
+				dev_err(dev, "can't read white data %d\n", ret);
+				return ret;
+			}
+			if (mask == IIO_CHAN_INFO_PROCESSED) {
+				*val = (reg * data->cur_resolution) / 10000;
+				*val2 = (reg * data->cur_resolution) % 10000;
+				return IIO_VAL_INT_PLUS_MICRO;
+			}
+			*val = reg;
+			*val2 = 0;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT)
+			return veml6030_get_intgrn_tm(indio_dev, val, val2);
+		return -EINVAL;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_LIGHT)
+			return veml6030_get_als_gain(indio_dev, val, val2);
+		return -EINVAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_intgrn_tm(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_als_gain(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_read_event_val(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, enum iio_event_info info,
+		int *val, int *val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+		case IIO_EV_DIR_FALLING:
+			return veml6030_read_thresh(indio_dev, val, val2, dir);
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_INFO_PERIOD:
+		return veml6030_read_persistence(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_event_val(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, enum iio_event_info info,
+		int val, int val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		return veml6030_write_thresh(indio_dev, val, val2, dir);
+	case IIO_EV_INFO_PERIOD:
+		return veml6030_write_persistence(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int veml6030_read_interrupt_config(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als conf register %d\n", ret);
+		return ret;
+	}
+
+	if (reg & VEML6030_ALS_INT_EN)
+		return 1;
+	else
+		return 0;
+}
+
+/*
+ * Sensor should not be measuring light when interrupt is configured.
+ * Therefore correct sequence to configure interrupt functionality is:
+ * shut down -> enable/disable interrupt -> power on
+ *
+ * state = 1 enables interrupt, state = 0 disables interrupt
+ */
+static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, int state)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (state < 0 || state > 1)
+		return -EINVAL;
+
+	ret = veml6030_als_shut_down(data);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"can't disable als to configure interrupt %d\n", ret);
+		return ret;
+	}
+
+	/* enable interrupt + power on */
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+			VEML6030_ALS_INT_EN | VEML6030_ALS_SD, state << 1);
+	if (ret)
+		dev_err(&data->client->dev,
+			"can't enable interrupt & poweron als %d\n", ret);
+
+	return ret;
+}
+
+static const struct iio_info veml6030_info = {
+	.read_raw  = veml6030_read_raw,
+	.write_raw = veml6030_write_raw,
+	.read_event_value = veml6030_read_event_val,
+	.write_event_value	= veml6030_write_event_val,
+	.read_event_config = veml6030_read_interrupt_config,
+	.write_event_config	= veml6030_write_interrupt_config,
+	.attrs = &veml6030_attr_group,
+	.event_attrs = &veml6030_event_attr_group,
+};
+
+static const struct iio_info veml6030_info_no_irq = {
+	.read_raw  = veml6030_read_raw,
+	.write_raw = veml6030_write_raw,
+	.attrs = &veml6030_attr_group,
+};
+
+static irqreturn_t veml6030_event_handler(int irq, void *private)
+{
+	int ret, reg, evtdir;
+	struct iio_dev *indio_dev = private;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als interrupt register %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	/* Spurious interrupt handling */
+	if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))
+		return IRQ_NONE;
+
+	if (reg & VEML6030_INT_TH_HIGH)
+		evtdir = IIO_EV_DIR_RISING;
+	else
+		evtdir = IIO_EV_DIR_FALLING;
+
+	iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
+					0, IIO_EV_TYPE_THRESH, evtdir),
+					iio_get_time_ns(indio_dev));
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
+ * persistence to 1 x integration time and the threshold
+ * interrupt disabled by default. First shutdown the sensor,
+ * update registers and then power on the sensor.
+ */
+static int veml6030_hw_init(struct iio_dev *indio_dev)
+{
+	int ret, val;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+
+	ret = veml6030_als_shut_down(data);
+	if (ret) {
+		dev_err(&client->dev, "can't shutdown als %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x1001);
+	if (ret) {
+		dev_err(&client->dev, "can't setup als configs %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
+	if (ret) {
+		dev_err(&client->dev, "can't setup default PSM %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
+	if (ret) {
+		dev_err(&client->dev, "can't setup high threshold %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
+	if (ret) {
+		dev_err(&client->dev, "can't setup low threshold %d\n", ret);
+		return ret;
+	}
+
+	ret = veml6030_als_pwr_on(data);
+	if (ret) {
+		dev_err(&client->dev, "can't poweron als %d\n", ret);
+		return ret;
+	}
+
+	/* Wait 4 ms to let processor & oscillator start correctly */
+	usleep_range(3990, 4000);
+
+	/* Clear stale interrupt status bits if any during start */
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"can't clear als interrupt status %d\n", ret);
+		return ret;
+	}
+
+	/* Cache currently active measurement parameters */
+	data->cur_gain = 3;
+	data->cur_resolution = 4608;
+	data->cur_integration_time = 3;
+
+	return ret;
+}
+
+static int veml6030_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	int ret;
+	struct veml6030_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
+		return -EOPNOTSUPP;
+	}
+
+	regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "can't setup regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	data->regmap = regmap;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->name = "veml6030";
+	indio_dev->channels = veml6030_channels;
+	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, veml6030_event_handler,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"veml6030", indio_dev);
+		if (ret < 0) {
+			dev_err(&client->dev,
+					"irq %d request failed\n", client->irq);
+			return ret;
+		}
+		indio_dev->info = &veml6030_info;
+	} else {
+		indio_dev->info = &veml6030_info_no_irq;
+	}
+
+	ret = devm_add_action_or_reset(&client->dev,
+					veml6030_als_shut_down_action, data);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6030_hw_init(indio_dev);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static int __maybe_unused veml6030_runtime_suspend(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = veml6030_als_shut_down(data);
+	if (ret < 0)
+		dev_err(&data->client->dev, "can't suspend als %d\n", ret);
+
+	return ret;
+}
+
+static int __maybe_unused veml6030_runtime_resume(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = veml6030_als_pwr_on(data);
+	if (ret < 0)
+		dev_err(&data->client->dev, "can't resume als %d\n", ret);
+
+	return ret;
+}
+
+static const struct dev_pm_ops veml6030_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
+				veml6030_runtime_resume, NULL)
+};
+
+static const struct of_device_id veml6030_of_match[] = {
+	{ .compatible = "vishay,veml6030" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, veml6030_of_match);
+
+static const struct i2c_device_id veml6030_id[] = {
+	{ "veml6030", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, veml6030_id);
+
+static struct i2c_driver veml6030_driver = {
+	.driver = {
+		.name = "veml6030",
+		.of_match_table = veml6030_of_match,
+		.pm = &veml6030_pm_ops,
+	},
+	.probe = veml6030_probe,
+	.id_table = veml6030_id,
+};
+module_i2c_driver(veml6030_driver);
+
+MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
+MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v4 1/3] iio: light: add driver for veml6030 ambient light sensor

[Rishi Gupta]

veml6030 is an ambient light sensor from Vishay semiconductors.
It has 16-bit resolution, supports both ambient light measurement
and white channel which is more responsive to wider wavelength
spectrum. It has flexible power saving, integration time and
gain options. Communication with host is over I2C.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v4:
* None

Changes in v3:
* Added appnote link in topmost comments section
* Dropped 'return ret' statements wherever not needed
* Removed .scan_index from channel specifications, not needed
* If irq is not enabled, events interfaces are not exposed now
* Return IRQ_NONE for spurious interrupt
* Removed CONFIG_PM, added __maybe_unused in power routines
* Removed of_match_ptr when specifying DT device id to match
* Corrected & documented sequence in veml6030_write_interrupt_config()
* Removed veml6030_remove() & added devm_add_action_or_reset functionality
* Added support to read integration time, gain, thresholds, period
* Removed including mutex.h not needed
* Set 100 ms integration & 1/8 gain during probe for better accuracy
* Used IIO_CONST_ATTR to create sysfs entries "_available"
* Minor cosmetics like everything in lower case

Changes in v2:
* Added comma after CH_WHITE in enum veml6030_chan so it can be extended
* Removed .scan_type as driver doesn't use buffered mode
* Removed iio_device_unregister() as kernel will take care of cleaning

 drivers/iio/light/Kconfig    |  11 +
 drivers/iio/light/Makefile   |   1 +
 drivers/iio/light/veml6030.c | 903 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 915 insertions(+)
 create mode 100644 drivers/iio/light/veml6030.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 4a1a883..f9d27ef 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -496,6 +496,17 @@ config VCNL4035
 	  To compile this driver as a module, choose M here: the
 	  module will be called vcnl4035.

+config VEML6030
+	tristate "VEML6030 ambient light sensor"
+	select REGMAP_I2C
+	depends on I2C
+	help
+	  Say Y here if you want to build a driver for the Vishay VEML6030
+	  ambient light sensor (ALS).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called veml6030.
+
 config VEML6070
 	tristate "VEML6070 UV A light sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 00d1f9b..5e0c40b 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_US5182D)		+= us5182d.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
 obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
+obj-$(CONFIG_VEML6030)		+= veml6030.o
 obj-$(CONFIG_VEML6070)		+= veml6070.o
 obj-$(CONFIG_VL6180)		+= vl6180.o
 obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
new file mode 100644
index 0000000..d0d4e42
--- /dev/null
+++ b/drivers/iio/light/veml6030.c
@@ -0,0 +1,903 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VEML6030 Ambient Light Sensor
+ *
+ * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
+ *
+ * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
+ * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/regmap.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+/* Device registers */
+#define VEML6030_REG_ALS_CONF   0x00
+#define VEML6030_REG_ALS_WH     0x01
+#define VEML6030_REG_ALS_WL     0x02
+#define VEML6030_REG_ALS_PSM    0x03
+#define VEML6030_REG_ALS_DATA   0x04
+#define VEML6030_REG_WH_DATA    0x05
+#define VEML6030_REG_ALS_INT    0x06
+
+/* Bit masks for specific functionality */
+#define VEML6030_ALS_IT       GENMASK(9, 6)
+#define VEML6030_PSM          GENMASK(2, 1)
+#define VEML6030_ALS_PERS     GENMASK(5, 4)
+#define VEML6030_ALS_GAIN     GENMASK(12, 11)
+#define VEML6030_PSM_EN       BIT(0)
+#define VEML6030_INT_TH_LOW   BIT(15)
+#define VEML6030_INT_TH_HIGH  BIT(14)
+#define VEML6030_ALS_INT_EN   BIT(1)
+#define VEML6030_ALS_SD       BIT(0)
+
+/*
+ * The resolution depends on both gain and integration time. The
+ * cur_resolution stores one of the resolution mentioned in the
+ * table during startup and gets updated whenever integration time
+ * or gain is changed.
+ *
+ * Table 'resolution and maximum detection range' in appnote 84367
+ * is visualized as a 2D array. The cur_gain stores index of gain
+ * in this table (0-3) while the cur_integration_time holds index
+ * of integration time (0-5).
+ */
+struct veml6030_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	int cur_resolution;
+	int cur_gain;
+	int cur_integration_time;
+};
+
+/* Integration time available in seconds */
+static IIO_CONST_ATTR(in_illuminance_integration_time_available,
+				"0.025 0.05 0.1 0.2 0.4 0.8");
+
+/*
+ * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
+ * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
+ */
+static IIO_CONST_ATTR(in_illuminance_scale_available,
+				"0.125 0.25 1.0 2.0");
+
+/*
+ * Power saving modes 1/2/3/4.
+ * Products can achieve a trade-off between power savings and
+ * frequency of als latest readings available through psm.
+ */
+static IIO_CONST_ATTR(in_illuminance_psm_available,
+				"1 2 3 4");
+
+static ssize_t in_illuminance_psm_store(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf, size_t len)
+{
+	int ret;
+	unsigned int val;
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (kstrtouint(buf, 0, &val))
+		return -EINVAL;
+
+	if (val < 1 || val > 4)
+		return -EINVAL;
+
+	/* update bits 1-2 */
+	val = ((val - 1) << 1);
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+					VEML6030_PSM, val);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update psm value %d\n", ret);
+		return ret;
+	}
+
+	return len;
+}
+
+static ssize_t in_illuminance_psm_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret, reg;
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_PSM, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read psm register %d\n", ret);
+		return ret;
+	}
+
+	return sprintf(buf, "%d\n", (((reg >> 1) & 0x03) + 1));
+}
+
+static IIO_DEVICE_ATTR_RW(in_illuminance_psm, 0);
+
+static struct attribute *veml6030_attributes[] = {
+	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance_psm_available.dev_attr.attr,
+	&iio_dev_attr_in_illuminance_psm.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group veml6030_attr_group = {
+	.attrs = veml6030_attributes,
+};
+
+/*
+ * Persistence = 1/2/4/8 x integration time
+ * Minimum time for which light readings must stay above configured
+ * threshold to assert interrupt.
+ */
+static IIO_CONST_ATTR(in_illuminance_period_available,
+				"1 2 4 8");
+
+static struct attribute *veml6030_event_attributes[] = {
+	&iio_const_attr_in_illuminance_period_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group veml6030_event_attr_group = {
+	.attrs = veml6030_event_attributes,
+};
+
+static int veml6030_als_pwr_on(struct veml6030_data *data)
+{
+	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 0);
+}
+
+static int veml6030_als_shut_down(struct veml6030_data *data)
+{
+	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+				 VEML6030_ALS_SD, 1);
+}
+
+static void veml6030_als_shut_down_action(void *data)
+{
+	veml6030_als_shut_down(data);
+}
+
+static const struct iio_event_spec veml6030_event_spec[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+/* Channel number */
+enum veml6030_chan {
+	CH_ALS,
+	CH_WHITE,
+};
+
+static const struct iio_chan_spec veml6030_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = CH_ALS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_PROCESSED) |
+				BIT(IIO_CHAN_INFO_INT_TIME) |
+				BIT(IIO_CHAN_INFO_SCALE),
+		.event_spec = veml6030_event_spec,
+		.num_event_specs = ARRAY_SIZE(veml6030_event_spec),
+	},
+	{
+		.type = IIO_INTENSITY,
+		.channel = CH_WHITE,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_PROCESSED),
+	},
+};
+
+static const struct regmap_config veml6030_regmap_config = {
+	.name = "veml6030_regmap",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = VEML6030_REG_ALS_INT,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static int veml6030_read_persistence(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	*val = 1 << ((reg >> 3) & 0x03);
+	*val2 = 0;
+	return IIO_VAL_INT;
+}
+
+static int veml6030_write_persistence(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
+		return -EINVAL;
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_PERS, val << 4);
+	if (ret)
+		dev_err(&data->client->dev,
+				"can't set persistence value %d\n", ret);
+
+	return ret;
+}
+
+static int veml6030_set_als_gain(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, new_gain, gain_idx;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val == 0 && val2 == 125000) {
+		new_gain = 0x1000; /* 0x02 << 11 */
+		gain_idx = 3;
+	} else if (val == 0 && val2 == 250000) {
+		new_gain = 0x1800;
+		gain_idx = 2;
+	} else if (val == 1 && val2 == 0) {
+		new_gain = 0x00;
+		gain_idx = 1;
+	} else if (val == 2 && val2 == 0) {
+		new_gain = 0x800;
+		gain_idx = 0;
+	} else {
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_GAIN, new_gain);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't set als gain %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Cache currently set gain & update resolution. For every
+	 * increase in the gain to next level, resolution is halved
+	 * and vice-versa.
+	 */
+	if (data->cur_gain < gain_idx)
+		data->cur_resolution <<= gain_idx - data->cur_gain;
+	else if (data->cur_gain > gain_idx)
+		data->cur_resolution >>= data->cur_gain - gain_idx;
+
+	data->cur_gain = gain_idx;
+
+	return ret;
+}
+
+static int veml6030_get_als_gain(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	switch ((reg >> 11) & 0x03) {
+	case 0:
+		*val = 1;
+		*val2 = 0;
+		break;
+	case 1:
+		*val = 2;
+		*val2 = 0;
+		break;
+	case 2:
+		*val = 0;
+		*val2 = 125000;
+		break;
+	case 3:
+		*val = 0;
+		*val2 = 250000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int veml6030_read_thresh(struct iio_dev *indio_dev,
+						int *val, int *val2, int dir)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (dir == IIO_EV_DIR_RISING)
+		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WH, &reg);
+	else
+		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WL, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als threshold value %d\n", ret);
+		return ret;
+	}
+
+	*val = reg & 0xffff;
+	*val2 = 0;
+	return IIO_VAL_INT;
+}
+
+static int veml6030_write_thresh(struct iio_dev *indio_dev,
+						int val, int val2, int dir)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val > 0xFFFF || val < 0 || val2)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_RISING) {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set high threshold %d\n", ret);
+	} else {
+		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
+		if (ret)
+			dev_err(&data->client->dev,
+					"can't set low threshold %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int veml6030_get_intgrn_tm(struct iio_dev *indio_dev,
+						int *val, int *val2)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't als configuration register %d\n", ret);
+		return ret;
+	}
+
+	switch ((reg >> 6) & 0xF) {
+	case 0:
+		*val2 = 100000;
+		break;
+	case 1:
+		*val2 = 200000;
+		break;
+	case 2:
+		*val2 = 400000;
+		break;
+	case 3:
+		*val2 = 800000;
+		break;
+	case 8:
+		*val2 = 50000;
+		break;
+	case 12:
+		*val2 = 25000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*val = 0;
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int veml6030_set_intgrn_tm(struct iio_dev *indio_dev,
+						int val, int val2)
+{
+	int ret, new_int_time, int_idx;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (val)
+		return -EINVAL;
+
+	switch (val2) {
+	case 25000:
+		new_int_time = 0x300;
+		int_idx = 5;
+		break;
+	case 50000:
+		new_int_time = 0x200;
+		int_idx = 4;
+		break;
+	case 100000:
+		new_int_time = 0x00;
+		int_idx = 3;
+		break;
+	case 200000:
+		new_int_time = 0x40;
+		int_idx = 2;
+		break;
+	case 400000:
+		new_int_time = 0x80;
+		int_idx = 1;
+		break;
+	case 800000:
+		new_int_time = 0xC0;
+		int_idx = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+					VEML6030_ALS_IT, new_int_time);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't update als integration time %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Cache current integration time and update resolution. For every
+	 * increase in integration time to next level, resolution is halved
+	 * and vice-versa.
+	 */
+	if (data->cur_integration_time < int_idx)
+		data->cur_resolution <<= int_idx - data->cur_integration_time;
+	else if (data->cur_integration_time > int_idx)
+		data->cur_resolution >>= data->cur_integration_time - int_idx;
+
+	data->cur_integration_time = int_idx;
+
+	return ret;
+}
+
+/*
+ * Provide both raw as well as light reading in lux.
+ * light (in lux) = resolution * raw reading
+ */
+static int veml6030_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct regmap *regmap = data->regmap;
+	struct device *dev = &data->client->dev;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &reg);
+			if (ret < 0) {
+				dev_err(dev, "can't read als data %d\n", ret);
+				return ret;
+			}
+			if (mask == IIO_CHAN_INFO_PROCESSED) {
+				*val = (reg * data->cur_resolution) / 10000;
+				*val2 = (reg * data->cur_resolution) % 10000;
+				return IIO_VAL_INT_PLUS_MICRO;
+			}
+			*val = reg;
+			*val2 = 0;
+			return IIO_VAL_INT;
+		case IIO_INTENSITY:
+			ret = regmap_read(regmap, VEML6030_REG_WH_DATA, &reg);
+			if (ret < 0) {
+				dev_err(dev, "can't read white data %d\n", ret);
+				return ret;
+			}
+			if (mask == IIO_CHAN_INFO_PROCESSED) {
+				*val = (reg * data->cur_resolution) / 10000;
+				*val2 = (reg * data->cur_resolution) % 10000;
+				return IIO_VAL_INT_PLUS_MICRO;
+			}
+			*val = reg;
+			*val2 = 0;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT)
+			return veml6030_get_intgrn_tm(indio_dev, val, val2);
+		return -EINVAL;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_LIGHT)
+			return veml6030_get_als_gain(indio_dev, val, val2);
+		return -EINVAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int val, int val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_intgrn_tm(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return veml6030_set_als_gain(indio_dev, val, val2);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_read_event_val(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, enum iio_event_info info,
+		int *val, int *val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+		case IIO_EV_DIR_FALLING:
+			return veml6030_read_thresh(indio_dev, val, val2, dir);
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_INFO_PERIOD:
+		return veml6030_read_persistence(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int veml6030_write_event_val(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, enum iio_event_info info,
+		int val, int val2)
+{
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		return veml6030_write_thresh(indio_dev, val, val2, dir);
+	case IIO_EV_INFO_PERIOD:
+		return veml6030_write_persistence(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int veml6030_read_interrupt_config(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir)
+{
+	int ret, reg;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als conf register %d\n", ret);
+		return ret;
+	}
+
+	if (reg & VEML6030_ALS_INT_EN)
+		return 1;
+	else
+		return 0;
+}
+
+/*
+ * Sensor should not be measuring light when interrupt is configured.
+ * Therefore correct sequence to configure interrupt functionality is:
+ * shut down -> enable/disable interrupt -> power on
+ *
+ * state = 1 enables interrupt, state = 0 disables interrupt
+ */
+static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, enum iio_event_type type,
+		enum iio_event_direction dir, int state)
+{
+	int ret;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	if (state < 0 || state > 1)
+		return -EINVAL;
+
+	ret = veml6030_als_shut_down(data);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"can't disable als to configure interrupt %d\n", ret);
+		return ret;
+	}
+
+	/* enable interrupt + power on */
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
+			VEML6030_ALS_INT_EN | VEML6030_ALS_SD, state << 1);
+	if (ret)
+		dev_err(&data->client->dev,
+			"can't enable interrupt & poweron als %d\n", ret);
+
+	return ret;
+}
+
+static const struct iio_info veml6030_info = {
+	.read_raw  = veml6030_read_raw,
+	.write_raw = veml6030_write_raw,
+	.read_event_value = veml6030_read_event_val,
+	.write_event_value	= veml6030_write_event_val,
+	.read_event_config = veml6030_read_interrupt_config,
+	.write_event_config	= veml6030_write_interrupt_config,
+	.attrs = &veml6030_attr_group,
+	.event_attrs = &veml6030_event_attr_group,
+};
+
+static const struct iio_info veml6030_info_no_irq = {
+	.read_raw  = veml6030_read_raw,
+	.write_raw = veml6030_write_raw,
+	.attrs = &veml6030_attr_group,
+};
+
+static irqreturn_t veml6030_event_handler(int irq, void *private)
+{
+	int ret, reg, evtdir;
+	struct iio_dev *indio_dev = private;
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
+	if (ret) {
+		dev_err(&data->client->dev,
+				"can't read als interrupt register %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	/* Spurious interrupt handling */
+	if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))
+		return IRQ_NONE;
+
+	if (reg & VEML6030_INT_TH_HIGH)
+		evtdir = IIO_EV_DIR_RISING;
+	else
+		evtdir = IIO_EV_DIR_FALLING;
+
+	iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
+					0, IIO_EV_TYPE_THRESH, evtdir),
+					iio_get_time_ns(indio_dev));
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
+ * persistence to 1 x integration time and the threshold
+ * interrupt disabled by default. First shutdown the sensor,
+ * update registers and then power on the sensor.
+ */
+static int veml6030_hw_init(struct iio_dev *indio_dev)
+{
+	int ret, val;
+	struct veml6030_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+
+	ret = veml6030_als_shut_down(data);
+	if (ret) {
+		dev_err(&client->dev, "can't shutdown als %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x1001);
+	if (ret) {
+		dev_err(&client->dev, "can't setup als configs %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
+				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
+	if (ret) {
+		dev_err(&client->dev, "can't setup default PSM %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
+	if (ret) {
+		dev_err(&client->dev, "can't setup high threshold %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
+	if (ret) {
+		dev_err(&client->dev, "can't setup low threshold %d\n", ret);
+		return ret;
+	}
+
+	ret = veml6030_als_pwr_on(data);
+	if (ret) {
+		dev_err(&client->dev, "can't poweron als %d\n", ret);
+		return ret;
+	}
+
+	/* Wait 4 ms to let processor & oscillator start correctly */
+	usleep_range(3990, 4000);
+
+	/* Clear stale interrupt status bits if any during start */
+	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"can't clear als interrupt status %d\n", ret);
+		return ret;
+	}
+
+	/* Cache currently active measurement parameters */
+	data->cur_gain = 3;
+	data->cur_resolution = 4608;
+	data->cur_integration_time = 3;
+
+	return ret;
+}
+
+static int veml6030_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	int ret;
+	struct veml6030_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
+		return -EOPNOTSUPP;
+	}
+
+	regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "can't setup regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	data->regmap = regmap;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->name = "veml6030";
+	indio_dev->channels = veml6030_channels;
+	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, veml6030_event_handler,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"veml6030", indio_dev);
+		if (ret < 0) {
+			dev_err(&client->dev,
+					"irq %d request failed\n", client->irq);
+			return ret;
+		}
+		indio_dev->info = &veml6030_info;
+	} else {
+		indio_dev->info = &veml6030_info_no_irq;
+	}
+
+	ret = devm_add_action_or_reset(&client->dev,
+					veml6030_als_shut_down_action, data);
+	if (ret < 0)
+		return ret;
+
+	ret = veml6030_hw_init(indio_dev);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static int __maybe_unused veml6030_runtime_suspend(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = veml6030_als_shut_down(data);
+	if (ret < 0)
+		dev_err(&data->client->dev, "can't suspend als %d\n", ret);
+
+	return ret;
+}
+
+static int __maybe_unused veml6030_runtime_resume(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct veml6030_data *data = iio_priv(indio_dev);
+
+	ret = veml6030_als_pwr_on(data);
+	if (ret < 0)
+		dev_err(&data->client->dev, "can't resume als %d\n", ret);
+
+	return ret;
+}
+
+static const struct dev_pm_ops veml6030_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
+				veml6030_runtime_resume, NULL)
+};
+
+static const struct of_device_id veml6030_of_match[] = {
+	{ .compatible = "vishay,veml6030" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, veml6030_of_match);
+
+static const struct i2c_device_id veml6030_id[] = {
+	{ "veml6030", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, veml6030_id);
+
+static struct i2c_driver veml6030_driver = {
+	.driver = {
+		.name = "veml6030",
+		.of_match_table = veml6030_of_match,
+		.pm = &veml6030_pm_ops,
+	},
+	.probe = veml6030_probe,
+	.id_table = veml6030_id,
+};
+module_i2c_driver(veml6030_driver);
+
+MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
+MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v4 1/3] iio: light: add driver for veml6030 ambient light sensor

[Jonathan Cameron]

On Tue, 22 Oct 2019 09:27:15 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

> veml6030 is an ambient light sensor from Vishay semiconductors.
> It has 16-bit resolution, supports both ambient light measurement
> and white channel which is more responsive to wider wavelength
> spectrum. It has flexible power saving, integration time and
> gain options. Communication with host is over I2C.
> 
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>

Please avoid this complex nested replies to old patches.
New version of a series should be a new thread.

It's much easier for myself or others to be sure they have the latest version
than when they are interleaved like this.

I've raised ABI concerns in patch 3, so won't repeat that here

Various minor other things inline.

Jonathan

> ---
> Changes in v4:
> * None
> 
> Changes in v3:
> * Added appnote link in topmost comments section
> * Dropped 'return ret' statements wherever not needed
> * Removed .scan_index from channel specifications, not needed
> * If irq is not enabled, events interfaces are not exposed now
> * Return IRQ_NONE for spurious interrupt
> * Removed CONFIG_PM, added __maybe_unused in power routines
> * Removed of_match_ptr when specifying DT device id to match
> * Corrected & documented sequence in veml6030_write_interrupt_config()
> * Removed veml6030_remove() & added devm_add_action_or_reset functionality
> * Added support to read integration time, gain, thresholds, period
> * Removed including mutex.h not needed
> * Set 100 ms integration & 1/8 gain during probe for better accuracy
> * Used IIO_CONST_ATTR to create sysfs entries "_available"
> * Minor cosmetics like everything in lower case
> 
> Changes in v2:
> * Added comma after CH_WHITE in enum veml6030_chan so it can be extended
> * Removed .scan_type as driver doesn't use buffered mode
> * Removed iio_device_unregister() as kernel will take care of cleaning
> 
>  drivers/iio/light/Kconfig    |  11 +
>  drivers/iio/light/Makefile   |   1 +
>  drivers/iio/light/veml6030.c | 903 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 915 insertions(+)
>  create mode 100644 drivers/iio/light/veml6030.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 4a1a883..f9d27ef 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -496,6 +496,17 @@ config VCNL4035
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called vcnl4035.
> 
> +config VEML6030
> +	tristate "VEML6030 ambient light sensor"
> +	select REGMAP_I2C
> +	depends on I2C
> +	help
> +	  Say Y here if you want to build a driver for the Vishay VEML6030
> +	  ambient light sensor (ALS).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called veml6030.
> +
>  config VEML6070
>  	tristate "VEML6070 UV A light sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 00d1f9b..5e0c40b 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)		+= tsl4531.o
>  obj-$(CONFIG_US5182D)		+= us5182d.o
>  obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
>  obj-$(CONFIG_VCNL4035)		+= vcnl4035.o
> +obj-$(CONFIG_VEML6030)		+= veml6030.o
>  obj-$(CONFIG_VEML6070)		+= veml6070.o
>  obj-$(CONFIG_VL6180)		+= vl6180.o
>  obj-$(CONFIG_ZOPT2201)		+= zopt2201.o
> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> new file mode 100644
> index 0000000..d0d4e42
> --- /dev/null
> +++ b/drivers/iio/light/veml6030.c
> @@ -0,0 +1,903 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VEML6030 Ambient Light Sensor
> + *
> + * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
> + *
> + * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
> + * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/regmap.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +
> +/* Device registers */
> +#define VEML6030_REG_ALS_CONF   0x00
> +#define VEML6030_REG_ALS_WH     0x01
> +#define VEML6030_REG_ALS_WL     0x02
> +#define VEML6030_REG_ALS_PSM    0x03
> +#define VEML6030_REG_ALS_DATA   0x04
> +#define VEML6030_REG_WH_DATA    0x05
> +#define VEML6030_REG_ALS_INT    0x06
> +
> +/* Bit masks for specific functionality */
> +#define VEML6030_ALS_IT       GENMASK(9, 6)
> +#define VEML6030_PSM          GENMASK(2, 1)
> +#define VEML6030_ALS_PERS     GENMASK(5, 4)
> +#define VEML6030_ALS_GAIN     GENMASK(12, 11)
> +#define VEML6030_PSM_EN       BIT(0)
> +#define VEML6030_INT_TH_LOW   BIT(15)
> +#define VEML6030_INT_TH_HIGH  BIT(14)
> +#define VEML6030_ALS_INT_EN   BIT(1)
> +#define VEML6030_ALS_SD       BIT(0)
> +
> +/*
> + * The resolution depends on both gain and integration time. The
> + * cur_resolution stores one of the resolution mentioned in the
> + * table during startup and gets updated whenever integration time
> + * or gain is changed.
> + *
> + * Table 'resolution and maximum detection range' in appnote 84367
> + * is visualized as a 2D array. The cur_gain stores index of gain
> + * in this table (0-3) while the cur_integration_time holds index
> + * of integration time (0-5).
> + */
> +struct veml6030_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	int cur_resolution;
> +	int cur_gain;
> +	int cur_integration_time;
> +};
> +
> +/* Integration time available in seconds */
> +static IIO_CONST_ATTR(in_illuminance_integration_time_available,
> +				"0.025 0.05 0.1 0.2 0.4 0.8");
> +
> +/*
> + * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
> + * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
> + */
> +static IIO_CONST_ATTR(in_illuminance_scale_available,
> +				"0.125 0.25 1.0 2.0");
> +
> +/*
> + * Power saving modes 1/2/3/4.
> + * Products can achieve a trade-off between power savings and
> + * frequency of als latest readings available through psm.
> + */
> +static IIO_CONST_ATTR(in_illuminance_psm_available,
> +				"1 2 3 4");
> +
> +static ssize_t in_illuminance_psm_store(struct device *dev,
> +						struct device_attribute *attr,
> +						const char *buf, size_t len)
> +{
> +	int ret;
> +	unsigned int val;
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (kstrtouint(buf, 0, &val))
> +		return -EINVAL;
> +
> +	if (val < 1 || val > 4)
> +		return -EINVAL;
> +
> +	/* update bits 1-2 */
> +	val = ((val - 1) << 1);
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> +					VEML6030_PSM, val);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't update psm value %d\n", ret);
> +		return ret;
> +	}
> +
> +	return len;
> +}
> +
> +static ssize_t in_illuminance_psm_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int ret, reg;
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_PSM, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't read psm register %d\n", ret);
> +		return ret;
> +	}
> +
> +	return sprintf(buf, "%d\n", (((reg >> 1) & 0x03) + 1));
> +}
> +
> +static IIO_DEVICE_ATTR_RW(in_illuminance_psm, 0);
> +
> +static struct attribute *veml6030_attributes[] = {
> +	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance_psm_available.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_psm.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group veml6030_attr_group = {
> +	.attrs = veml6030_attributes,
> +};
> +
> +/*
> + * Persistence = 1/2/4/8 x integration time
> + * Minimum time for which light readings must stay above configured
> + * threshold to assert interrupt.
> + */
> +static IIO_CONST_ATTR(in_illuminance_period_available,
> +				"1 2 4 8");
> +
> +static struct attribute *veml6030_event_attributes[] = {
> +	&iio_const_attr_in_illuminance_period_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group veml6030_event_attr_group = {
> +	.attrs = veml6030_event_attributes,
> +};
> +
> +static int veml6030_als_pwr_on(struct veml6030_data *data)
> +{
> +	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +				 VEML6030_ALS_SD, 0);
> +}
> +
> +static int veml6030_als_shut_down(struct veml6030_data *data)
> +{
> +	return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +				 VEML6030_ALS_SD, 1);
> +}
> +
> +static void veml6030_als_shut_down_action(void *data)
> +{
> +	veml6030_als_shut_down(data);
> +}
> +
> +static const struct iio_event_spec veml6030_event_spec[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
> +/* Channel number */
> +enum veml6030_chan {
> +	CH_ALS,
> +	CH_WHITE,
> +};
> +
> +static const struct iio_chan_spec veml6030_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.channel = CH_ALS,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				BIT(IIO_CHAN_INFO_PROCESSED) |
> +				BIT(IIO_CHAN_INFO_INT_TIME) |
> +				BIT(IIO_CHAN_INFO_SCALE),
> +		.event_spec = veml6030_event_spec,
> +		.num_event_specs = ARRAY_SIZE(veml6030_event_spec),
> +	},
> +	{
> +		.type = IIO_INTENSITY,
> +		.channel = CH_WHITE,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_BOTH,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				BIT(IIO_CHAN_INFO_PROCESSED),
> +	},
> +};
> +
> +static const struct regmap_config veml6030_regmap_config = {
> +	.name = "veml6030_regmap",
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = VEML6030_REG_ALS_INT,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static int veml6030_read_persistence(struct iio_dev *indio_dev,
> +						int *val, int *val2)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't als configuration register %d\n", ret);
> +		return ret;
> +	}
> +
> +	*val = 1 << ((reg >> 3) & 0x03);
> +	*val2 = 0;
> +	return IIO_VAL_INT;
> +}
> +
> +static int veml6030_write_persistence(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_PERS, val << 4);
> +	if (ret)
> +		dev_err(&data->client->dev,
> +				"can't set persistence value %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int veml6030_set_als_gain(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret, new_gain, gain_idx;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val == 0 && val2 == 125000) {
> +		new_gain = 0x1000; /* 0x02 << 11 */
> +		gain_idx = 3;
> +	} else if (val == 0 && val2 == 250000) {
> +		new_gain = 0x1800;
> +		gain_idx = 2;
> +	} else if (val == 1 && val2 == 0) {
> +		new_gain = 0x00;
> +		gain_idx = 1;
> +	} else if (val == 2 && val2 == 0) {
> +		new_gain = 0x800;
> +		gain_idx = 0;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_GAIN, new_gain);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't set als gain %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Cache currently set gain & update resolution. For every
> +	 * increase in the gain to next level, resolution is halved
> +	 * and vice-versa.
> +	 */
> +	if (data->cur_gain < gain_idx)
> +		data->cur_resolution <<= gain_idx - data->cur_gain;
> +	else if (data->cur_gain > gain_idx)
> +		data->cur_resolution >>= data->cur_gain - gain_idx;
> +
> +	data->cur_gain = gain_idx;
> +
> +	return ret;
> +}
> +
> +static int veml6030_get_als_gain(struct iio_dev *indio_dev,
> +						int *val, int *val2)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't als configuration register %d\n", ret);
> +		return ret;
> +	}
> +
> +	switch ((reg >> 11) & 0x03) {
> +	case 0:
> +		*val = 1;
> +		*val2 = 0;
> +		break;
> +	case 1:
> +		*val = 2;
> +		*val2 = 0;
> +		break;
> +	case 2:
> +		*val = 0;
> +		*val2 = 125000;
> +		break;
> +	case 3:
> +		*val = 0;
> +		*val2 = 250000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int veml6030_read_thresh(struct iio_dev *indio_dev,
> +						int *val, int *val2, int dir)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (dir == IIO_EV_DIR_RISING)
> +		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WH, &reg);
> +	else
> +		ret = regmap_read(data->regmap, VEML6030_REG_ALS_WL, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't read als threshold value %d\n", ret);
> +		return ret;
> +	}
> +
> +	*val = reg & 0xffff;
> +	*val2 = 0;
> +	return IIO_VAL_INT;
> +}
> +
> +static int veml6030_write_thresh(struct iio_dev *indio_dev,
> +						int val, int val2, int dir)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val > 0xFFFF || val < 0 || val2)
> +		return -EINVAL;
> +
> +	if (dir == IIO_EV_DIR_RISING) {
> +		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
> +		if (ret)
> +			dev_err(&data->client->dev,
> +					"can't set high threshold %d\n", ret);
> +	} else {
> +		ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
> +		if (ret)
> +			dev_err(&data->client->dev,
> +					"can't set low threshold %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static int veml6030_get_intgrn_tm(struct iio_dev *indio_dev,
> +						int *val, int *val2)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't als configuration register %d\n", ret);
> +		return ret;
> +	}
> +
> +	switch ((reg >> 6) & 0xF) {
> +	case 0:
> +		*val2 = 100000;
> +		break;
> +	case 1:
> +		*val2 = 200000;
> +		break;
> +	case 2:
> +		*val2 = 400000;
> +		break;
> +	case 3:
> +		*val2 = 800000;
> +		break;
> +	case 8:
> +		*val2 = 50000;
> +		break;
> +	case 12:
> +		*val2 = 25000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*val = 0;
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int veml6030_set_intgrn_tm(struct iio_dev *indio_dev,
> +						int val, int val2)
> +{
> +	int ret, new_int_time, int_idx;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (val)
> +		return -EINVAL;
> +
> +	switch (val2) {
> +	case 25000:
> +		new_int_time = 0x300;
> +		int_idx = 5;
> +		break;
> +	case 50000:
> +		new_int_time = 0x200;
> +		int_idx = 4;
> +		break;
> +	case 100000:
> +		new_int_time = 0x00;
> +		int_idx = 3;
> +		break;
> +	case 200000:
> +		new_int_time = 0x40;
> +		int_idx = 2;
> +		break;
> +	case 400000:
> +		new_int_time = 0x80;
> +		int_idx = 1;
> +		break;
> +	case 800000:
> +		new_int_time = 0xC0;
> +		int_idx = 0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +					VEML6030_ALS_IT, new_int_time);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't update als integration time %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Cache current integration time and update resolution. For every
> +	 * increase in integration time to next level, resolution is halved
> +	 * and vice-versa.
> +	 */
> +	if (data->cur_integration_time < int_idx)
> +		data->cur_resolution <<= int_idx - data->cur_integration_time;
> +	else if (data->cur_integration_time > int_idx)
> +		data->cur_resolution >>= data->cur_integration_time - int_idx;
> +
> +	data->cur_integration_time = int_idx;
> +
> +	return ret;
> +}
> +
> +/*
> + * Provide both raw as well as light reading in lux.
> + * light (in lux) = resolution * raw reading
> + */
> +static int veml6030_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +	struct regmap *regmap = data->regmap;
> +	struct device *dev = &data->client->dev;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &reg);
> +			if (ret < 0) {
> +				dev_err(dev, "can't read als data %d\n", ret);
> +				return ret;
> +			}
> +			if (mask == IIO_CHAN_INFO_PROCESSED) {
> +				*val = (reg * data->cur_resolution) / 10000;
> +				*val2 = (reg * data->cur_resolution) % 10000;
> +				return IIO_VAL_INT_PLUS_MICRO;
> +			}
> +			*val = reg;
> +			*val2 = 0;
> +			return IIO_VAL_INT;
> +		case IIO_INTENSITY:
> +			ret = regmap_read(regmap, VEML6030_REG_WH_DATA, &reg);
> +			if (ret < 0) {
> +				dev_err(dev, "can't read white data %d\n", ret);
> +				return ret;
> +			}
> +			if (mask == IIO_CHAN_INFO_PROCESSED) {
> +				*val = (reg * data->cur_resolution) / 10000;
> +				*val2 = (reg * data->cur_resolution) % 10000;
> +				return IIO_VAL_INT_PLUS_MICRO;
> +			}
> +			*val = reg;
> +			*val2 = 0;

No need to set val2 if returning IIO_VAL_INT, it's not read.

> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT)
> +			return veml6030_get_intgrn_tm(indio_dev, val, val2);
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_LIGHT)
> +			return veml6030_get_als_gain(indio_dev, val, val2);
> +		return -EINVAL;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6030_write_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val, int val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_INT_TIME:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			return veml6030_set_intgrn_tm(indio_dev, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			return veml6030_set_als_gain(indio_dev, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6030_read_event_val(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir, enum iio_event_info info,
> +		int *val, int *val2)
> +{
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +		case IIO_EV_DIR_FALLING:
> +			return veml6030_read_thresh(indio_dev, val, val2, dir);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_EV_INFO_PERIOD:
> +		return veml6030_read_persistence(indio_dev, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int veml6030_write_event_val(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir, enum iio_event_info info,
> +		int val, int val2)
> +{
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		return veml6030_write_thresh(indio_dev, val, val2, dir);
> +	case IIO_EV_INFO_PERIOD:
> +		return veml6030_write_persistence(indio_dev, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +
You can't get here, so loose this final return.
> +	return -EINVAL;
> +}
> +
> +static int veml6030_read_interrupt_config(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir)
> +{
> +	int ret, reg;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't read als conf register %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (reg & VEML6030_ALS_INT_EN)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +/*
> + * Sensor should not be measuring light when interrupt is configured.
> + * Therefore correct sequence to configure interrupt functionality is:
> + * shut down -> enable/disable interrupt -> power on
> + *
> + * state = 1 enables interrupt, state = 0 disables interrupt
> + */
> +static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, enum iio_event_type type,
> +		enum iio_event_direction dir, int state)
> +{
> +	int ret;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	if (state < 0 || state > 1)
> +		return -EINVAL;
> +
> +	ret = veml6030_als_shut_down(data);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"can't disable als to configure interrupt %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* enable interrupt + power on */
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> +			VEML6030_ALS_INT_EN | VEML6030_ALS_SD, state << 1);
> +	if (ret)
> +		dev_err(&data->client->dev,
> +			"can't enable interrupt & poweron als %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info veml6030_info = {
> +	.read_raw  = veml6030_read_raw,
> +	.write_raw = veml6030_write_raw,
> +	.read_event_value = veml6030_read_event_val,
> +	.write_event_value	= veml6030_write_event_val,
> +	.read_event_config = veml6030_read_interrupt_config,
> +	.write_event_config	= veml6030_write_interrupt_config,
> +	.attrs = &veml6030_attr_group,
> +	.event_attrs = &veml6030_event_attr_group,
> +};
> +
> +static const struct iio_info veml6030_info_no_irq = {
> +	.read_raw  = veml6030_read_raw,
> +	.write_raw = veml6030_write_raw,
> +	.attrs = &veml6030_attr_group,
> +};
> +
> +static irqreturn_t veml6030_event_handler(int irq, void *private)
> +{
> +	int ret, reg, evtdir;
> +	struct iio_dev *indio_dev = private;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
> +	if (ret) {
> +		dev_err(&data->client->dev,
> +				"can't read als interrupt register %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	/* Spurious interrupt handling */
> +	if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))
> +		return IRQ_NONE;
> +
> +	if (reg & VEML6030_INT_TH_HIGH)
> +		evtdir = IIO_EV_DIR_RISING;
> +	else
> +		evtdir = IIO_EV_DIR_FALLING;
> +
> +	iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
> +					0, IIO_EV_TYPE_THRESH, evtdir),
> +					iio_get_time_ns(indio_dev));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
> + * persistence to 1 x integration time and the threshold
> + * interrupt disabled by default. First shutdown the sensor,
> + * update registers and then power on the sensor.
> + */
> +static int veml6030_hw_init(struct iio_dev *indio_dev)
> +{
> +	int ret, val;
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +	struct i2c_client *client = data->client;
> +
> +	ret = veml6030_als_shut_down(data);
> +	if (ret) {
> +		dev_err(&client->dev, "can't shutdown als %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x1001);
> +	if (ret) {
> +		dev_err(&client->dev, "can't setup als configs %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> +				 VEML6030_PSM | VEML6030_PSM_EN, 0x03);
> +	if (ret) {
> +		dev_err(&client->dev, "can't setup default PSM %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
> +	if (ret) {
> +		dev_err(&client->dev, "can't setup high threshold %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
> +	if (ret) {
> +		dev_err(&client->dev, "can't setup low threshold %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = veml6030_als_pwr_on(data);
> +	if (ret) {
> +		dev_err(&client->dev, "can't poweron als %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Wait 4 ms to let processor & oscillator start correctly */
> +	usleep_range(3990, 4000);

I you need to sleep 4msecs then the smallest value needs to be 4msecs.

> +
> +	/* Clear stale interrupt status bits if any during start */
> +	ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
> +	if (ret < 0) {
> +		dev_err(&client->dev,
> +			"can't clear als interrupt status %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Cache currently active measurement parameters */
> +	data->cur_gain = 3;
> +	data->cur_resolution = 4608;
> +	data->cur_integration_time = 3;
> +
> +	return ret;
> +}
> +
> +static int veml6030_probe(struct i2c_client *client,
> +			  const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct veml6030_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "can't setup regmap\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +	data->regmap = regmap;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->name = "veml6030";
> +	indio_dev->channels = veml6030_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	if (client->irq) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, veml6030_event_handler,
> +						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +						"veml6030", indio_dev);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +					"irq %d request failed\n", client->irq);
> +			return ret;
> +		}
> +		indio_dev->info = &veml6030_info;
> +	} else {
> +		indio_dev->info = &veml6030_info_no_irq;
> +	}
> +
> +	ret = devm_add_action_or_reset(&client->dev,
> +					veml6030_als_shut_down_action, data);

What is this reversing?  It should be immediately after whatever that is, thus
ensuring we only undo whatever we need to on failure and the ordering is correct
for remove.  I am guessing it should be after hw_init.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = veml6030_hw_init(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static int __maybe_unused veml6030_runtime_suspend(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = veml6030_als_shut_down(data);
> +	if (ret < 0)
> +		dev_err(&data->client->dev, "can't suspend als %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused veml6030_runtime_resume(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct veml6030_data *data = iio_priv(indio_dev);
> +
> +	ret = veml6030_als_pwr_on(data);
> +	if (ret < 0)
> +		dev_err(&data->client->dev, "can't resume als %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops veml6030_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
> +				veml6030_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id veml6030_of_match[] = {
> +	{ .compatible = "vishay,veml6030" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, veml6030_of_match);
> +
> +static const struct i2c_device_id veml6030_id[] = {
> +	{ "veml6030", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, veml6030_id);
> +
> +static struct i2c_driver veml6030_driver = {
> +	.driver = {
> +		.name = "veml6030",
> +		.of_match_table = veml6030_of_match,
> +		.pm = &veml6030_pm_ops,
> +	},
> +	.probe = veml6030_probe,
> +	.id_table = veml6030_id,
> +};
> +module_i2c_driver(veml6030_driver);
> +
> +MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
> +MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH v4 1/3] iio: light: add driver for veml6030 ambient light sensor

[rishi gupta]

Thanks Jonathan, sorry for deep thread, learnt will keep in mind.

All suggested changes done except re-ordering devm_add_action_or_reset.
Please see inline and suggest if I missed something.

On Tue, Oct 22, 2019 at 3:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 22 Oct 2019 09:27:15 +0530
> Rishi Gupta <gupt21@gmail.com> wrote:
>
> > veml6030 is an ambient light sensor from Vishay semiconductors.
> > It has 16-bit resolution, supports both ambient light measurement
> > and white channel which is more responsive to wider wavelength
> > spectrum. It has flexible power saving, integration time and
> > gain options. Communication with host is over I2C.
> >
> > Signed-off-by: Rishi Gupta <gupt21@gmail.com>
>
> Please avoid this complex nested replies to old patches.
> New version of a series should be a new thread.
>
> It's much easier for myself or others to be sure they have the latest version
> than when they are interleaved like this.
>
> I've raised ABI concerns in patch 3, so won't repeat that here
>
> Various minor other things inline.
>
> Jonathan
>
> > ---
> > Changes in v4:
> > * None
> >
> > Changes in v3:
> > * Added appnote link in topmost comments section
> > * Dropped 'return ret' statements wherever not needed
> > * Removed .scan_index from channel specifications, not needed
> > * If irq is not enabled, events interfaces are not exposed now
> > * Return IRQ_NONE for spurious interrupt
> > * Removed CONFIG_PM, added __maybe_unused in power routines
> > * Removed of_match_ptr when specifying DT device id to match
> > * Corrected & documented sequence in veml6030_write_interrupt_config()
> > * Removed veml6030_remove() & added devm_add_action_or_reset functionality
> > * Added support to read integration time, gain, thresholds, period
> > * Removed including mutex.h not needed
> > * Set 100 ms integration & 1/8 gain during probe for better accuracy
> > * Used IIO_CONST_ATTR to create sysfs entries "_available"
> > * Minor cosmetics like everything in lower case
> >
> > Changes in v2:
> > * Added comma after CH_WHITE in enum veml6030_chan so it can be extended
> > * Removed .scan_type as driver doesn't use buffered mode
> > * Removed iio_device_unregister() as kernel will take care of cleaning
> >
> >  drivers/iio/light/Kconfig    |  11 +
> >  drivers/iio/light/Makefile   |   1 +
> >  drivers/iio/light/veml6030.c | 903 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 915 insertions(+)
> >  create mode 100644 drivers/iio/light/veml6030.c
> >
> > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> > index 4a1a883..f9d27ef 100644
> > --- a/drivers/iio/light/Kconfig
> > +++ b/drivers/iio/light/Kconfig
> > @@ -496,6 +496,17 @@ config VCNL4035
> >         To compile this driver as a module, choose M here: the
> >         module will be called vcnl4035.
> >
> > +config VEML6030
> > +     tristate "VEML6030 ambient light sensor"
> > +     select REGMAP_I2C
> > +     depends on I2C
> > +     help
> > +       Say Y here if you want to build a driver for the Vishay VEML6030
> > +       ambient light sensor (ALS).
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called veml6030.
> > +
> >  config VEML6070
> >       tristate "VEML6070 UV A light sensor"
> >       depends on I2C
> > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> > index 00d1f9b..5e0c40b 100644
> > --- a/drivers/iio/light/Makefile
> > +++ b/drivers/iio/light/Makefile
> > @@ -48,6 +48,7 @@ obj-$(CONFIG_TSL4531)               += tsl4531.o
> >  obj-$(CONFIG_US5182D)                += us5182d.o
> >  obj-$(CONFIG_VCNL4000)               += vcnl4000.o
> >  obj-$(CONFIG_VCNL4035)               += vcnl4035.o
> > +obj-$(CONFIG_VEML6030)               += veml6030.o
> >  obj-$(CONFIG_VEML6070)               += veml6070.o
> >  obj-$(CONFIG_VL6180)         += vl6180.o
> >  obj-$(CONFIG_ZOPT2201)               += zopt2201.o
> > diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> > new file mode 100644
> > index 0000000..d0d4e42
> > --- /dev/null
> > +++ b/drivers/iio/light/veml6030.c
> > @@ -0,0 +1,903 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * VEML6030 Ambient Light Sensor
> > + *
> > + * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com>
> > + *
> > + * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf
> > + * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/err.h>
> > +#include <linux/regmap.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/events.h>
> > +
> > +/* Device registers */
> > +#define VEML6030_REG_ALS_CONF   0x00
> > +#define VEML6030_REG_ALS_WH     0x01
> > +#define VEML6030_REG_ALS_WL     0x02
> > +#define VEML6030_REG_ALS_PSM    0x03
> > +#define VEML6030_REG_ALS_DATA   0x04
> > +#define VEML6030_REG_WH_DATA    0x05
> > +#define VEML6030_REG_ALS_INT    0x06
> > +
> > +/* Bit masks for specific functionality */
> > +#define VEML6030_ALS_IT       GENMASK(9, 6)
> > +#define VEML6030_PSM          GENMASK(2, 1)
> > +#define VEML6030_ALS_PERS     GENMASK(5, 4)
> > +#define VEML6030_ALS_GAIN     GENMASK(12, 11)
> > +#define VEML6030_PSM_EN       BIT(0)
> > +#define VEML6030_INT_TH_LOW   BIT(15)
> > +#define VEML6030_INT_TH_HIGH  BIT(14)
> > +#define VEML6030_ALS_INT_EN   BIT(1)
> > +#define VEML6030_ALS_SD       BIT(0)
> > +
> > +/*
> > + * The resolution depends on both gain and integration time. The
> > + * cur_resolution stores one of the resolution mentioned in the
> > + * table during startup and gets updated whenever integration time
> > + * or gain is changed.
> > + *
> > + * Table 'resolution and maximum detection range' in appnote 84367
> > + * is visualized as a 2D array. The cur_gain stores index of gain
> > + * in this table (0-3) while the cur_integration_time holds index
> > + * of integration time (0-5).
> > + */
> > +struct veml6030_data {
> > +     struct i2c_client *client;
> > +     struct regmap *regmap;
> > +     int cur_resolution;
> > +     int cur_gain;
> > +     int cur_integration_time;
> > +};
> > +
> > +/* Integration time available in seconds */
> > +static IIO_CONST_ATTR(in_illuminance_integration_time_available,
> > +                             "0.025 0.05 0.1 0.2 0.4 0.8");
> > +
> > +/*
> > + * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is
> > + * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2.
> > + */
> > +static IIO_CONST_ATTR(in_illuminance_scale_available,
> > +                             "0.125 0.25 1.0 2.0");
> > +
> > +/*
> > + * Power saving modes 1/2/3/4.
> > + * Products can achieve a trade-off between power savings and
> > + * frequency of als latest readings available through psm.
> > + */
> > +static IIO_CONST_ATTR(in_illuminance_psm_available,
> > +                             "1 2 3 4");
> > +
> > +static ssize_t in_illuminance_psm_store(struct device *dev,
> > +                                             struct device_attribute *attr,
> > +                                             const char *buf, size_t len)
> > +{
> > +     int ret;
> > +     unsigned int val;
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (kstrtouint(buf, 0, &val))
> > +             return -EINVAL;
> > +
> > +     if (val < 1 || val > 4)
> > +             return -EINVAL;
> > +
> > +     /* update bits 1-2 */
> > +     val = ((val - 1) << 1);
> > +
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> > +                                     VEML6030_PSM, val);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't update psm value %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     return len;
> > +}
> > +
> > +static ssize_t in_illuminance_psm_show(struct device *dev,
> > +                             struct device_attribute *attr, char *buf)
> > +{
> > +     int ret, reg;
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_PSM, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't read psm register %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     return sprintf(buf, "%d\n", (((reg >> 1) & 0x03) + 1));
> > +}
> > +
> > +static IIO_DEVICE_ATTR_RW(in_illuminance_psm, 0);
> > +
> > +static struct attribute *veml6030_attributes[] = {
> > +     &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
> > +     &iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
> > +     &iio_const_attr_in_illuminance_psm_available.dev_attr.attr,
> > +     &iio_dev_attr_in_illuminance_psm.dev_attr.attr,
> > +     NULL
> > +};
> > +
> > +static const struct attribute_group veml6030_attr_group = {
> > +     .attrs = veml6030_attributes,
> > +};
> > +
> > +/*
> > + * Persistence = 1/2/4/8 x integration time
> > + * Minimum time for which light readings must stay above configured
> > + * threshold to assert interrupt.
> > + */
> > +static IIO_CONST_ATTR(in_illuminance_period_available,
> > +                             "1 2 4 8");
> > +
> > +static struct attribute *veml6030_event_attributes[] = {
> > +     &iio_const_attr_in_illuminance_period_available.dev_attr.attr,
> > +     NULL
> > +};
> > +
> > +static const struct attribute_group veml6030_event_attr_group = {
> > +     .attrs = veml6030_event_attributes,
> > +};
> > +
> > +static int veml6030_als_pwr_on(struct veml6030_data *data)
> > +{
> > +     return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                              VEML6030_ALS_SD, 0);
> > +}
> > +
> > +static int veml6030_als_shut_down(struct veml6030_data *data)
> > +{
> > +     return regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                              VEML6030_ALS_SD, 1);
> > +}
> > +
> > +static void veml6030_als_shut_down_action(void *data)
> > +{
> > +     veml6030_als_shut_down(data);
> > +}
> > +
> > +static const struct iio_event_spec veml6030_event_spec[] = {
> > +     {
> > +             .type = IIO_EV_TYPE_THRESH,
> > +             .dir = IIO_EV_DIR_RISING,
> > +             .mask_separate = BIT(IIO_EV_INFO_VALUE),
> > +     }, {
> > +             .type = IIO_EV_TYPE_THRESH,
> > +             .dir = IIO_EV_DIR_FALLING,
> > +             .mask_separate = BIT(IIO_EV_INFO_VALUE),
> > +     }, {
> > +             .type = IIO_EV_TYPE_THRESH,
> > +             .dir = IIO_EV_DIR_EITHER,
> > +             .mask_separate = BIT(IIO_EV_INFO_PERIOD),
> > +     }, {
> > +             .type = IIO_EV_TYPE_THRESH,
> > +             .dir = IIO_EV_DIR_EITHER,
> > +             .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > +     },
> > +};
> > +
> > +/* Channel number */
> > +enum veml6030_chan {
> > +     CH_ALS,
> > +     CH_WHITE,
> > +};
> > +
> > +static const struct iio_chan_spec veml6030_channels[] = {
> > +     {
> > +             .type = IIO_LIGHT,
> > +             .channel = CH_ALS,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                             BIT(IIO_CHAN_INFO_PROCESSED) |
> > +                             BIT(IIO_CHAN_INFO_INT_TIME) |
> > +                             BIT(IIO_CHAN_INFO_SCALE),
> > +             .event_spec = veml6030_event_spec,
> > +             .num_event_specs = ARRAY_SIZE(veml6030_event_spec),
> > +     },
> > +     {
> > +             .type = IIO_INTENSITY,
> > +             .channel = CH_WHITE,
> > +             .modified = 1,
> > +             .channel2 = IIO_MOD_LIGHT_BOTH,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                             BIT(IIO_CHAN_INFO_PROCESSED),
> > +     },
> > +};
> > +
> > +static const struct regmap_config veml6030_regmap_config = {
> > +     .name = "veml6030_regmap",
> > +     .reg_bits = 8,
> > +     .val_bits = 16,
> > +     .max_register = VEML6030_REG_ALS_INT,
> > +     .val_format_endian = REGMAP_ENDIAN_LITTLE,
> > +};
> > +
> > +static int veml6030_read_persistence(struct iio_dev *indio_dev,
> > +                                             int *val, int *val2)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't als configuration register %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     *val = 1 << ((reg >> 3) & 0x03);
> > +     *val2 = 0;
> > +     return IIO_VAL_INT;
> > +}
> > +
> > +static int veml6030_write_persistence(struct iio_dev *indio_dev,
> > +                                             int val, int val2)
> > +{
> > +     int ret;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (val < 0 || val > 8 || hweight8(val) != 1 || val2)
> > +             return -EINVAL;
> > +
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                                     VEML6030_ALS_PERS, val << 4);
> > +     if (ret)
> > +             dev_err(&data->client->dev,
> > +                             "can't set persistence value %d\n", ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static int veml6030_set_als_gain(struct iio_dev *indio_dev,
> > +                                             int val, int val2)
> > +{
> > +     int ret, new_gain, gain_idx;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (val == 0 && val2 == 125000) {
> > +             new_gain = 0x1000; /* 0x02 << 11 */
> > +             gain_idx = 3;
> > +     } else if (val == 0 && val2 == 250000) {
> > +             new_gain = 0x1800;
> > +             gain_idx = 2;
> > +     } else if (val == 1 && val2 == 0) {
> > +             new_gain = 0x00;
> > +             gain_idx = 1;
> > +     } else if (val == 2 && val2 == 0) {
> > +             new_gain = 0x800;
> > +             gain_idx = 0;
> > +     } else {
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                                     VEML6030_ALS_GAIN, new_gain);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't set als gain %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /*
> > +      * Cache currently set gain & update resolution. For every
> > +      * increase in the gain to next level, resolution is halved
> > +      * and vice-versa.
> > +      */
> > +     if (data->cur_gain < gain_idx)
> > +             data->cur_resolution <<= gain_idx - data->cur_gain;
> > +     else if (data->cur_gain > gain_idx)
> > +             data->cur_resolution >>= data->cur_gain - gain_idx;
> > +
> > +     data->cur_gain = gain_idx;
> > +
> > +     return ret;
> > +}
> > +
> > +static int veml6030_get_als_gain(struct iio_dev *indio_dev,
> > +                                             int *val, int *val2)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't als configuration register %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     switch ((reg >> 11) & 0x03) {
> > +     case 0:
> > +             *val = 1;
> > +             *val2 = 0;
> > +             break;
> > +     case 1:
> > +             *val = 2;
> > +             *val2 = 0;
> > +             break;
> > +     case 2:
> > +             *val = 0;
> > +             *val2 = 125000;
> > +             break;
> > +     case 3:
> > +             *val = 0;
> > +             *val2 = 250000;
> > +             break;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int veml6030_read_thresh(struct iio_dev *indio_dev,
> > +                                             int *val, int *val2, int dir)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (dir == IIO_EV_DIR_RISING)
> > +             ret = regmap_read(data->regmap, VEML6030_REG_ALS_WH, &reg);
> > +     else
> > +             ret = regmap_read(data->regmap, VEML6030_REG_ALS_WL, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't read als threshold value %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     *val = reg & 0xffff;
> > +     *val2 = 0;
> > +     return IIO_VAL_INT;
> > +}
> > +
> > +static int veml6030_write_thresh(struct iio_dev *indio_dev,
> > +                                             int val, int val2, int dir)
> > +{
> > +     int ret;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (val > 0xFFFF || val < 0 || val2)
> > +             return -EINVAL;
> > +
> > +     if (dir == IIO_EV_DIR_RISING) {
> > +             ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, val);
> > +             if (ret)
> > +                     dev_err(&data->client->dev,
> > +                                     "can't set high threshold %d\n", ret);
> > +     } else {
> > +             ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, val);
> > +             if (ret)
> > +                     dev_err(&data->client->dev,
> > +                                     "can't set low threshold %d\n", ret);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int veml6030_get_intgrn_tm(struct iio_dev *indio_dev,
> > +                                             int *val, int *val2)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't als configuration register %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     switch ((reg >> 6) & 0xF) {
> > +     case 0:
> > +             *val2 = 100000;
> > +             break;
> > +     case 1:
> > +             *val2 = 200000;
> > +             break;
> > +     case 2:
> > +             *val2 = 400000;
> > +             break;
> > +     case 3:
> > +             *val2 = 800000;
> > +             break;
> > +     case 8:
> > +             *val2 = 50000;
> > +             break;
> > +     case 12:
> > +             *val2 = 25000;
> > +             break;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     *val = 0;
> > +     return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int veml6030_set_intgrn_tm(struct iio_dev *indio_dev,
> > +                                             int val, int val2)
> > +{
> > +     int ret, new_int_time, int_idx;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (val)
> > +             return -EINVAL;
> > +
> > +     switch (val2) {
> > +     case 25000:
> > +             new_int_time = 0x300;
> > +             int_idx = 5;
> > +             break;
> > +     case 50000:
> > +             new_int_time = 0x200;
> > +             int_idx = 4;
> > +             break;
> > +     case 100000:
> > +             new_int_time = 0x00;
> > +             int_idx = 3;
> > +             break;
> > +     case 200000:
> > +             new_int_time = 0x40;
> > +             int_idx = 2;
> > +             break;
> > +     case 400000:
> > +             new_int_time = 0x80;
> > +             int_idx = 1;
> > +             break;
> > +     case 800000:
> > +             new_int_time = 0xC0;
> > +             int_idx = 0;
> > +             break;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                                     VEML6030_ALS_IT, new_int_time);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't update als integration time %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /*
> > +      * Cache current integration time and update resolution. For every
> > +      * increase in integration time to next level, resolution is halved
> > +      * and vice-versa.
> > +      */
> > +     if (data->cur_integration_time < int_idx)
> > +             data->cur_resolution <<= int_idx - data->cur_integration_time;
> > +     else if (data->cur_integration_time > int_idx)
> > +             data->cur_resolution >>= data->cur_integration_time - int_idx;
> > +
> > +     data->cur_integration_time = int_idx;
> > +
> > +     return ret;
> > +}
> > +
> > +/*
> > + * Provide both raw as well as light reading in lux.
> > + * light (in lux) = resolution * raw reading
> > + */
> > +static int veml6030_read_raw(struct iio_dev *indio_dev,
> > +                         struct iio_chan_spec const *chan, int *val,
> > +                         int *val2, long mask)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +     struct regmap *regmap = data->regmap;
> > +     struct device *dev = &data->client->dev;
> > +
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_RAW:
> > +     case IIO_CHAN_INFO_PROCESSED:
> > +             switch (chan->type) {
> > +             case IIO_LIGHT:
> > +                     ret = regmap_read(regmap, VEML6030_REG_ALS_DATA, &reg);
> > +                     if (ret < 0) {
> > +                             dev_err(dev, "can't read als data %d\n", ret);
> > +                             return ret;
> > +                     }
> > +                     if (mask == IIO_CHAN_INFO_PROCESSED) {
> > +                             *val = (reg * data->cur_resolution) / 10000;
> > +                             *val2 = (reg * data->cur_resolution) % 10000;
> > +                             return IIO_VAL_INT_PLUS_MICRO;
> > +                     }
> > +                     *val = reg;
> > +                     *val2 = 0;
> > +                     return IIO_VAL_INT;
> > +             case IIO_INTENSITY:
> > +                     ret = regmap_read(regmap, VEML6030_REG_WH_DATA, &reg);
> > +                     if (ret < 0) {
> > +                             dev_err(dev, "can't read white data %d\n", ret);
> > +                             return ret;
> > +                     }
> > +                     if (mask == IIO_CHAN_INFO_PROCESSED) {
> > +                             *val = (reg * data->cur_resolution) / 10000;
> > +                             *val2 = (reg * data->cur_resolution) % 10000;
> > +                             return IIO_VAL_INT_PLUS_MICRO;
> > +                     }
> > +                     *val = reg;
> > +                     *val2 = 0;
>
> No need to set val2 if returning IIO_VAL_INT, it's not read.
>
> > +                     return IIO_VAL_INT;
> > +             default:
> > +                     return -EINVAL;
> > +             }
> > +     case IIO_CHAN_INFO_INT_TIME:
> > +             if (chan->type == IIO_LIGHT)
> > +                     return veml6030_get_intgrn_tm(indio_dev, val, val2);
> > +             return -EINVAL;
> > +     case IIO_CHAN_INFO_SCALE:
> > +             if (chan->type == IIO_LIGHT)
> > +                     return veml6030_get_als_gain(indio_dev, val, val2);
> > +             return -EINVAL;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static int veml6030_write_raw(struct iio_dev *indio_dev,
> > +                             struct iio_chan_spec const *chan,
> > +                             int val, int val2, long mask)
> > +{
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_INT_TIME:
> > +             switch (chan->type) {
> > +             case IIO_LIGHT:
> > +                     return veml6030_set_intgrn_tm(indio_dev, val, val2);
> > +             default:
> > +                     return -EINVAL;
> > +             }
> > +     case IIO_CHAN_INFO_SCALE:
> > +             switch (chan->type) {
> > +             case IIO_LIGHT:
> > +                     return veml6030_set_als_gain(indio_dev, val, val2);
> > +             default:
> > +                     return -EINVAL;
> > +             }
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static int veml6030_read_event_val(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, enum iio_event_type type,
> > +             enum iio_event_direction dir, enum iio_event_info info,
> > +             int *val, int *val2)
> > +{
> > +     switch (info) {
> > +     case IIO_EV_INFO_VALUE:
> > +             switch (dir) {
> > +             case IIO_EV_DIR_RISING:
> > +             case IIO_EV_DIR_FALLING:
> > +                     return veml6030_read_thresh(indio_dev, val, val2, dir);
> > +             default:
> > +                     return -EINVAL;
> > +             }
> > +             break;
> > +     case IIO_EV_INFO_PERIOD:
> > +             return veml6030_read_persistence(indio_dev, val, val2);
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
> > +
> > +static int veml6030_write_event_val(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, enum iio_event_type type,
> > +             enum iio_event_direction dir, enum iio_event_info info,
> > +             int val, int val2)
> > +{
> > +     switch (info) {
> > +     case IIO_EV_INFO_VALUE:
> > +             return veml6030_write_thresh(indio_dev, val, val2, dir);
> > +     case IIO_EV_INFO_PERIOD:
> > +             return veml6030_write_persistence(indio_dev, val, val2);
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> You can't get here, so loose this final return.
> > +     return -EINVAL;
> > +}
> > +
> > +static int veml6030_read_interrupt_config(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, enum iio_event_type type,
> > +             enum iio_event_direction dir)
> > +{
> > +     int ret, reg;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't read als conf register %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     if (reg & VEML6030_ALS_INT_EN)
> > +             return 1;
> > +     else
> > +             return 0;
> > +}
> > +
> > +/*
> > + * Sensor should not be measuring light when interrupt is configured.
> > + * Therefore correct sequence to configure interrupt functionality is:
> > + * shut down -> enable/disable interrupt -> power on
> > + *
> > + * state = 1 enables interrupt, state = 0 disables interrupt
> > + */
> > +static int veml6030_write_interrupt_config(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, enum iio_event_type type,
> > +             enum iio_event_direction dir, int state)
> > +{
> > +     int ret;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     if (state < 0 || state > 1)
> > +             return -EINVAL;
> > +
> > +     ret = veml6030_als_shut_down(data);
> > +     if (ret < 0) {
> > +             dev_err(&data->client->dev,
> > +                     "can't disable als to configure interrupt %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /* enable interrupt + power on */
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF,
> > +                     VEML6030_ALS_INT_EN | VEML6030_ALS_SD, state << 1);
> > +     if (ret)
> > +             dev_err(&data->client->dev,
> > +                     "can't enable interrupt & poweron als %d\n", ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct iio_info veml6030_info = {
> > +     .read_raw  = veml6030_read_raw,
> > +     .write_raw = veml6030_write_raw,
> > +     .read_event_value = veml6030_read_event_val,
> > +     .write_event_value      = veml6030_write_event_val,
> > +     .read_event_config = veml6030_read_interrupt_config,
> > +     .write_event_config     = veml6030_write_interrupt_config,
> > +     .attrs = &veml6030_attr_group,
> > +     .event_attrs = &veml6030_event_attr_group,
> > +};
> > +
> > +static const struct iio_info veml6030_info_no_irq = {
> > +     .read_raw  = veml6030_read_raw,
> > +     .write_raw = veml6030_write_raw,
> > +     .attrs = &veml6030_attr_group,
> > +};
> > +
> > +static irqreturn_t veml6030_event_handler(int irq, void *private)
> > +{
> > +     int ret, reg, evtdir;
> > +     struct iio_dev *indio_dev = private;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &reg);
> > +     if (ret) {
> > +             dev_err(&data->client->dev,
> > +                             "can't read als interrupt register %d\n", ret);
> > +             return IRQ_HANDLED;
> > +     }
> > +
> > +     /* Spurious interrupt handling */
> > +     if (!(reg & (VEML6030_INT_TH_HIGH | VEML6030_INT_TH_LOW)))
> > +             return IRQ_NONE;
> > +
> > +     if (reg & VEML6030_INT_TH_HIGH)
> > +             evtdir = IIO_EV_DIR_RISING;
> > +     else
> > +             evtdir = IIO_EV_DIR_FALLING;
> > +
> > +     iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_INTENSITY,
> > +                                     0, IIO_EV_TYPE_THRESH, evtdir),
> > +                                     iio_get_time_ns(indio_dev));
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +/*
> > + * Set ALS gain to 1/8, integration time to 100 ms, PSM to mode 2,
> > + * persistence to 1 x integration time and the threshold
> > + * interrupt disabled by default. First shutdown the sensor,
> > + * update registers and then power on the sensor.
> > + */
> > +static int veml6030_hw_init(struct iio_dev *indio_dev)
> > +{
> > +     int ret, val;
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +     struct i2c_client *client = data->client;
> > +
> > +     ret = veml6030_als_shut_down(data);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't shutdown als %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, 0x1001);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't setup als configs %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM,
> > +                              VEML6030_PSM | VEML6030_PSM_EN, 0x03);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't setup default PSM %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't setup high threshold %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't setup low threshold %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = veml6030_als_pwr_on(data);
> > +     if (ret) {
> > +             dev_err(&client->dev, "can't poweron als %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /* Wait 4 ms to let processor & oscillator start correctly */
> > +     usleep_range(3990, 4000);
>
> I you need to sleep 4msecs then the smallest value needs to be 4msecs.
>
> > +
> > +     /* Clear stale interrupt status bits if any during start */
> > +     ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val);
> > +     if (ret < 0) {
> > +             dev_err(&client->dev,
> > +                     "can't clear als interrupt status %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     /* Cache currently active measurement parameters */
> > +     data->cur_gain = 3;
> > +     data->cur_resolution = 4608;
> > +     data->cur_integration_time = 3;
> > +
> > +     return ret;
> > +}
> > +
> > +static int veml6030_probe(struct i2c_client *client,
> > +                       const struct i2c_device_id *id)
> > +{
> > +     int ret;
> > +     struct veml6030_data *data;
> > +     struct iio_dev *indio_dev;
> > +     struct regmap *regmap;
> > +
> > +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > +             dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
> > +     if (IS_ERR(regmap)) {
> > +             dev_err(&client->dev, "can't setup regmap\n");
> > +             return PTR_ERR(regmap);
> > +     }
> > +
> > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +
> > +     data = iio_priv(indio_dev);
> > +     i2c_set_clientdata(client, indio_dev);
> > +     data->client = client;
> > +     data->regmap = regmap;
> > +
> > +     indio_dev->dev.parent = &client->dev;
> > +     indio_dev->name = "veml6030";
> > +     indio_dev->channels = veml6030_channels;
> > +     indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
> > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +     if (client->irq) {
> > +             ret = devm_request_threaded_irq(&client->dev, client->irq,
> > +                                             NULL, veml6030_event_handler,
> > +                                             IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > +                                             "veml6030", indio_dev);
> > +             if (ret < 0) {
> > +                     dev_err(&client->dev,
> > +                                     "irq %d request failed\n", client->irq);
> > +                     return ret;
> > +             }
> > +             indio_dev->info = &veml6030_info;
> > +     } else {
> > +             indio_dev->info = &veml6030_info_no_irq;
> > +     }
> > +
> > +     ret = devm_add_action_or_reset(&client->dev,
> > +                                     veml6030_als_shut_down_action, data);
>
> What is this reversing?  It should be immediately after whatever that is, thus
> ensuring we only undo whatever we need to on failure and the ordering is correct
> for remove.  I am guessing it should be after hw_init.
>
This just disables active measurements (this is the only thing we need
to do when failure happens).

Suppose hw initialisation succeeds but call to
devm_add_action_or_reset() fails. In this case sensor will be left
turned on as veml6030_als_shut_down_action() will never be executed.
Therefore I kept it before veml6030_hw_init().
Does this sounds correct to you ?

> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = veml6030_hw_init(indio_dev);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     return devm_iio_device_register(&client->dev, indio_dev);
> > +}
> > +
> > +static int __maybe_unused veml6030_runtime_suspend(struct device *dev)
> > +{
> > +     int ret;
> > +     struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = veml6030_als_shut_down(data);
> > +     if (ret < 0)
> > +             dev_err(&data->client->dev, "can't suspend als %d\n", ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static int __maybe_unused veml6030_runtime_resume(struct device *dev)
> > +{
> > +     int ret;
> > +     struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> > +     struct veml6030_data *data = iio_priv(indio_dev);
> > +
> > +     ret = veml6030_als_pwr_on(data);
> > +     if (ret < 0)
> > +             dev_err(&data->client->dev, "can't resume als %d\n", ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct dev_pm_ops veml6030_pm_ops = {
> > +     SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +                             pm_runtime_force_resume)
> > +     SET_RUNTIME_PM_OPS(veml6030_runtime_suspend,
> > +                             veml6030_runtime_resume, NULL)
> > +};
> > +
> > +static const struct of_device_id veml6030_of_match[] = {
> > +     { .compatible = "vishay,veml6030" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, veml6030_of_match);
> > +
> > +static const struct i2c_device_id veml6030_id[] = {
> > +     { "veml6030", 0 },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, veml6030_id);
> > +
> > +static struct i2c_driver veml6030_driver = {
> > +     .driver = {
> > +             .name = "veml6030",
> > +             .of_match_table = veml6030_of_match,
> > +             .pm = &veml6030_pm_ops,
> > +     },
> > +     .probe = veml6030_probe,
> > +     .id_table = veml6030_id,
> > +};
> > +module_i2c_driver(veml6030_driver);
> > +
> > +MODULE_AUTHOR("Rishi Gupta <gupt21@gmail.com>");
> > +MODULE_DESCRIPTION("VEML6030 Ambient Light Sensor");
> > +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v4 1/3] iio: light: add driver for veml6030 ambient light sensor

[Jonathan Cameron]

On Tue, 22 Oct 2019 18:02:51 +0530
rishi gupta <gupt21@gmail.com> wrote:

> Thanks Jonathan, sorry for deep thread, learnt will keep in mind.
> 
> All suggested changes done except re-ordering devm_add_action_or_reset.
> Please see inline and suggest if I missed something.
> 
...
> > > +static int veml6030_probe(struct i2c_client *client,
> > > +                       const struct i2c_device_id *id)
> > > +{
> > > +     int ret;
> > > +     struct veml6030_data *data;
> > > +     struct iio_dev *indio_dev;
> > > +     struct regmap *regmap;
> > > +
> > > +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > > +             dev_err(&client->dev, "i2c adapter doesn't support plain i2c\n");
> > > +             return -EOPNOTSUPP;
> > > +     }
> > > +
> > > +     regmap = devm_regmap_init_i2c(client, &veml6030_regmap_config);
> > > +     if (IS_ERR(regmap)) {
> > > +             dev_err(&client->dev, "can't setup regmap\n");
> > > +             return PTR_ERR(regmap);
> > > +     }
> > > +
> > > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > > +     if (!indio_dev)
> > > +             return -ENOMEM;
> > > +
> > > +     data = iio_priv(indio_dev);
> > > +     i2c_set_clientdata(client, indio_dev);
> > > +     data->client = client;
> > > +     data->regmap = regmap;
> > > +
> > > +     indio_dev->dev.parent = &client->dev;
> > > +     indio_dev->name = "veml6030";
> > > +     indio_dev->channels = veml6030_channels;
> > > +     indio_dev->num_channels = ARRAY_SIZE(veml6030_channels);
> > > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > > +
> > > +     if (client->irq) {
> > > +             ret = devm_request_threaded_irq(&client->dev, client->irq,
> > > +                                             NULL, veml6030_event_handler,
> > > +                                             IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > > +                                             "veml6030", indio_dev);
> > > +             if (ret < 0) {
> > > +                     dev_err(&client->dev,
> > > +                                     "irq %d request failed\n", client->irq);
> > > +                     return ret;
> > > +             }
> > > +             indio_dev->info = &veml6030_info;
> > > +     } else {
> > > +             indio_dev->info = &veml6030_info_no_irq;
> > > +     }
> > > +
> > > +     ret = devm_add_action_or_reset(&client->dev,
> > > +                                     veml6030_als_shut_down_action, data);  
> >
> > What is this reversing?  It should be immediately after whatever that is, thus
> > ensuring we only undo whatever we need to on failure and the ordering is correct
> > for remove.  I am guessing it should be after hw_init.
> >  
> This just disables active measurements (this is the only thing we need
> to do when failure happens).
> 
> Suppose hw initialisation succeeds but call to
> devm_add_action_or_reset() fails. In this case sensor will be left
> turned on as veml6030_als_shut_down_action() will never be executed.
> Therefore I kept it before veml6030_hw_init().
> Does this sounds correct to you ?

Nope, that's the point of the _or_reset part of that call. Note that we used
to manually handle the result of devm_add_action, but this little wrapper
does that for us.

In all failure cases it will run the callback provided to it.
https://elixir.bootlin.com/linux/v4.8/source/include/linux/device.h#L688

So it should always be called 'after' the thing it is setting up the
unwinding function for.

Jonathan

> 
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     ret = veml6030_hw_init(indio_dev);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     return devm_iio_device_register(&client->dev, indio_dev);
> > > +}

[RESEND PATCH v2 2/3] dt-bindings: iio: light: add veml6030 ALS bindings

[Rishi Gupta]

This commit adds device tree bindings for veml6030 ambient
light sensor.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v2:
* Corrected grammatical mistake from 'is' to 'are' in description of bindings

 .../devicetree/bindings/iio/light/veml6030.yaml    | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/veml6030.yaml b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
new file mode 100644
index 0000000..969b314
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/veml6030.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VEML6030 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Rishi Gupta <gupt21@gmail.com>
+
+description: |
+  Bindings for the ambient light sensor veml6030 from Vishay
+  Semiconductors over an i2c interface.
+
+  Irrespective of whether interrupt is used or not, application
+  can get the ALS and White channel reading from IIO raw interface.
+
+  If the interrupts are used, application will receive an IIO event
+  whenever configured threshold is crossed.
+
+  Specifications about the sensor can be found at:
+    https://www.vishay.com/docs/84366/veml6030.pdf
+
+properties:
+  compatible:
+    enum:
+      - vishay,veml6030
+
+  reg:
+    description:
+      I2C address of the device. If the ADDR pin on veml6030
+      is pulled up, this address is 0x48. If the ADDR pin is
+      pulled down, this address is 0x10.
+    maxItems: 1
+
+  interrupts:
+    description:
+      interrupt mapping for IRQ. Configure with IRQ_TYPE_LEVEL_LOW.
+      Refer to interrupt-controller/interrupts.txt for generic
+      interrupt client node bindings.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@10 {
+                compatible = "vishay,veml6030";
+                reg = <0x10>;
+                interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
+...
-- 
2.7.4

Re: [RESEND PATCH v2 2/3] dt-bindings: iio: light: add veml6030 ALS bindings

[Jonathan Cameron]

On Tue, 24 Sep 2019 16:21:57 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

> This commit adds device tree bindings for veml6030 ambient
> light sensor.
> 
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>
looks fine to me, but...

DT bindings should always be sent to the DT-bindings maintainers and mailing list
for review.

+CC Rob, Mark and devicetree list.

Jonathan

> ---
> Changes in v2:
> * Corrected grammatical mistake from 'is' to 'are' in description of bindings
> 
>  .../devicetree/bindings/iio/light/veml6030.yaml    | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/veml6030.yaml b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
> new file mode 100644
> index 0000000..969b314
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/veml6030.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: VEML6030 Ambient Light Sensor (ALS)
> +
> +maintainers:
> +  - Rishi Gupta <gupt21@gmail.com>
> +
> +description: |
> +  Bindings for the ambient light sensor veml6030 from Vishay
> +  Semiconductors over an i2c interface.
> +
> +  Irrespective of whether interrupt is used or not, application
> +  can get the ALS and White channel reading from IIO raw interface.
> +
> +  If the interrupts are used, application will receive an IIO event
> +  whenever configured threshold is crossed.
> +
> +  Specifications about the sensor can be found at:
> +    https://www.vishay.com/docs/84366/veml6030.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - vishay,veml6030
> +
> +  reg:
> +    description:
> +      I2C address of the device. If the ADDR pin on veml6030
> +      is pulled up, this address is 0x48. If the ADDR pin is
> +      pulled down, this address is 0x10.
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      interrupt mapping for IRQ. Configure with IRQ_TYPE_LEVEL_LOW.
> +      Refer to interrupt-controller/interrupts.txt for generic
> +      interrupt client node bindings.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@10 {
> +                compatible = "vishay,veml6030";
> +                reg = <0x10>;
> +                interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> +...

[PATCH v3 2/3] dt-bindings: iio: light: add veml6030 ALS bindings

[Rishi Gupta]

This commit adds device tree bindings for veml6030 ambient
light sensor.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v3:
* None

Changes in v2:
* Corrected grammatical mistake from 'is' to 'are' in description of bindings

 .../devicetree/bindings/iio/light/veml6030.yaml    | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/veml6030.yaml b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
new file mode 100644
index 0000000..969b314
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/veml6030.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VEML6030 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Rishi Gupta <gupt21@gmail.com>
+
+description: |
+  Bindings for the ambient light sensor veml6030 from Vishay
+  Semiconductors over an i2c interface.
+
+  Irrespective of whether interrupt is used or not, application
+  can get the ALS and White channel reading from IIO raw interface.
+
+  If the interrupts are used, application will receive an IIO event
+  whenever configured threshold is crossed.
+
+  Specifications about the sensor can be found at:
+    https://www.vishay.com/docs/84366/veml6030.pdf
+
+properties:
+  compatible:
+    enum:
+      - vishay,veml6030
+
+  reg:
+    description:
+      I2C address of the device. If the ADDR pin on veml6030
+      is pulled up, this address is 0x48. If the ADDR pin is
+      pulled down, this address is 0x10.
+    maxItems: 1
+
+  interrupts:
+    description:
+      interrupt mapping for IRQ. Configure with IRQ_TYPE_LEVEL_LOW.
+      Refer to interrupt-controller/interrupts.txt for generic
+      interrupt client node bindings.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@10 {
+                compatible = "vishay,veml6030";
+                reg = <0x10>;
+                interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
+...
-- 
2.7.4

Re: [PATCH v3 2/3] dt-bindings: iio: light: add veml6030 ALS bindings

[Rob Herring]

On Mon, Oct 21, 2019 at 8:31 AM Rishi Gupta <gupt21@gmail.com> wrote:
>
> This commit adds device tree bindings for veml6030 ambient
> light sensor.
>
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>
> ---
> Changes in v3:
> * None
>
> Changes in v2:
> * Corrected grammatical mistake from 'is' to 'are' in description of bindings
>
>  .../devicetree/bindings/iio/light/veml6030.yaml    | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/light/veml6030.yaml b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
> new file mode 100644
> index 0000000..969b314
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0+

(GPL-2.0-only OR BSD-2-Clause) for new bindings please.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/veml6030.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: VEML6030 Ambient Light Sensor (ALS)
> +
> +maintainers:
> +  - Rishi Gupta <gupt21@gmail.com>
> +
> +description: |
> +  Bindings for the ambient light sensor veml6030 from Vishay
> +  Semiconductors over an i2c interface.
> +
> +  Irrespective of whether interrupt is used or not, application
> +  can get the ALS and White channel reading from IIO raw interface.
> +
> +  If the interrupts are used, application will receive an IIO event
> +  whenever configured threshold is crossed.
> +
> +  Specifications about the sensor can be found at:
> +    https://www.vishay.com/docs/84366/veml6030.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - vishay,veml6030
> +
> +  reg:
> +    description:
> +      I2C address of the device. If the ADDR pin on veml6030
> +      is pulled up, this address is 0x48. If the ADDR pin is
> +      pulled down, this address is 0x10.

If you want to define the addresses, then you do:

enum:
  - 0x10 # ADDR pin pulled down
  - 0x48 # ADDR pin pulled up

> +    maxItems: 1

And drop this.

> +
> +  interrupts:
> +    description:
> +      interrupt mapping for IRQ. Configure with IRQ_TYPE_LEVEL_LOW.
> +      Refer to interrupt-controller/interrupts.txt for generic
> +      interrupt client node bindings.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@10 {
> +                compatible = "vishay,veml6030";
> +                reg = <0x10>;
> +                interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> +...
> --
> 2.7.4
>

[PATCH v4 2/3] dt-bindings: iio: light: add veml6030 ALS bindings

[Rishi Gupta]

This commit adds device tree bindings for veml6030 ambient
light sensor.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v4:
* Added enum in reg property
* Removed maxItems from reg property

Changes in v3:
* None

Changes in v2:
* Corrected grammatical mistake from 'is' to 'are' in description of bindings

 .../devicetree/bindings/iio/light/veml6030.yaml    | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/veml6030.yaml b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
new file mode 100644
index 0000000..0ff9b11
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/veml6030.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/veml6030.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VEML6030 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Rishi Gupta <gupt21@gmail.com>
+
+description: |
+  Bindings for the ambient light sensor veml6030 from Vishay
+  Semiconductors over an i2c interface.
+
+  Irrespective of whether interrupt is used or not, application
+  can get the ALS and White channel reading from IIO raw interface.
+
+  If the interrupts are used, application will receive an IIO event
+  whenever configured threshold is crossed.
+
+  Specifications about the sensor can be found at:
+    https://www.vishay.com/docs/84366/veml6030.pdf
+
+properties:
+  compatible:
+    enum:
+      - vishay,veml6030
+
+  reg:
+    description:
+      I2C address of the device.
+    enum:
+      - 0x10 # ADDR pin pulled down
+      - 0x48 # ADDR pin pulled up
+
+  interrupts:
+    description:
+      interrupt mapping for IRQ. Configure with IRQ_TYPE_LEVEL_LOW.
+      Refer to interrupt-controller/interrupts.txt for generic
+      interrupt client node bindings.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        light-sensor@10 {
+                compatible = "vishay,veml6030";
+                reg = <0x10>;
+                interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+        };
+    };
+...
-- 
2.7.4

[RESEND PATCH v2 3/3] iio: documentation: light: Add veml6030 sysfs documentation

[Rishi Gupta]

The driver for veml6030 light sensor provides custom sysfs entries
used to know parameters supported by the driver and to configure
sensor like setting power saving mode and persistence etc. This
commit document them.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v2:
* Nothing has changed in this file

 .../ABI/testing/sysfs-bus-iio-light-veml6030       | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030 b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
new file mode 100644
index 0000000..13589c9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
@@ -0,0 +1,49 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm_available
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Provides list of valid values that can be used to activate a
+		particular power saving mode in sensor. For ex; 1 means psm
+		mode 1 and 2 means psm mode 2.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Writing '1' will activate power saving mode 1 in sensor.
+		Similarly, 2 is to activate psm mode 2 and so on.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_period_available
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		List of valid values available in multiples of integration time
+		for which the light intensity must be above the cutoff level
+		before interrupt is asserted. This refers to persistence values.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_either_period
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Value in multiple of integration time for which the light intensity must
+		be above the cutoff level before interrupt is asserted.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_rising_value
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is above this value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_falling_value
+Date:		September 2019
+KernelVersion:	5.3.1
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is below this value.
-- 
2.7.4

Re: [RESEND PATCH v2 3/3] iio: documentation: light: Add veml6030 sysfs documentation

[Jonathan Cameron]

On Tue, 24 Sep 2019 16:21:58 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

> The driver for veml6030 light sensor provides custom sysfs entries
> used to know parameters supported by the driver and to configure
> sensor like setting power saving mode and persistence etc. This
> commit document them.
> 
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>
> ---
> Changes in v2:
> * Nothing has changed in this file
> 
>  .../ABI/testing/sysfs-bus-iio-light-veml6030       | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030 b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> new file mode 100644
> index 0000000..13589c9
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> @@ -0,0 +1,49 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm_available
> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Provides list of valid values that can be used to activate a
> +		particular power saving mode in sensor. For ex; 1 means psm
> +		mode 1 and 2 means psm mode 2.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm

Peter was a bit confusing on this. Much of the interface is standard ABI
(or falls within the ABI that is constructed so should be in the main docs -
a particularly combination may be missing and should be added).

psm is the one exception I can see so if we decide to keep it, will indeed
need to be documented here.

However, custom ABI is always problematic as generic userspace code will never
know what to do with it.  So if it is possible to map to standard ABI that
is always a better idea.

> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Writing '1' will activate power saving mode 1 in sensor.
> +		Similarly, 2 is to activate psm mode 2 and so on.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_period_available

As I mentioned in the review of the driver, this is a characteristic of the event
so should be in the events directory.

> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		List of valid values available in multiples of integration time
> +		for which the light intensity must be above the cutoff level
> +		before interrupt is asserted. This refers to persistence values.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_either_period
> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Value in multiple of integration time for which the light intensity must
> +		be above the cutoff level before interrupt is asserted.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_rising_value
> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
> +		light intensity is above this value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_falling_value
> +Date:		September 2019
> +KernelVersion:	5.3.1
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
> +		light intensity is below this value.

[PATCH v3 3/3] iio: documentation: light: Add veml6030 sysfs documentation

[Rishi Gupta]

The driver for veml6030 light sensor provides custom sysfs entries
used to know parameters supported by the driver and to configure
sensor like setting power saving mode and persistence etc. This
commit document them.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v3:
* Updated Date from September to October
* Updated KernelVersion from 5.3.1 to 5.4
* in_illuminance_period_available is now in events directory

Changes in v2:
* None

 .../ABI/testing/sysfs-bus-iio-light-veml6030       | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030 b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
new file mode 100644
index 0000000..13cd321
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
@@ -0,0 +1,49 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm_available
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Provides list of valid values that can be used to activate a
+		particular power saving mode in sensor. For ex; 1 means psm
+		mode 1 and 2 means psm mode 2 and so on.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Writing '1' will activate power saving mode 1 in sensor.
+		Similarly, 2 is to activate psm mode 2 and so on.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_period_available
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		List of valid values available in multiples of integration time
+		for which the light intensity must be above the cutoff level
+		before interrupt is asserted. This refers to persistence values.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_either_period
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Value in multiple of integration time for which the light intensity must
+		be above the cutoff level before interrupt is asserted.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_rising_value
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is above this value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_falling_value
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is below this value.
-- 
2.7.4

[PATCH v4 3/3] iio: documentation: light: Add veml6030 sysfs documentation

[Rishi Gupta]

The driver for veml6030 light sensor provides custom sysfs entries
used to know parameters supported by the driver and to configure
sensor like setting power saving mode and persistence etc. This
commit document them.

Signed-off-by: Rishi Gupta <gupt21@gmail.com>
---
Changes in v4:
* None

Changes in v3:
* Updated Date from September to October
* Updated KernelVersion from 5.3.1 to 5.4
* in_illuminance_period_available is now in events directory

Changes in v2:
* None

 .../ABI/testing/sysfs-bus-iio-light-veml6030       | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030 b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
new file mode 100644
index 0000000..13cd321
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
@@ -0,0 +1,49 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm_available
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Provides list of valid values that can be used to activate a
+		particular power saving mode in sensor. For ex; 1 means psm
+		mode 1 and 2 means psm mode 2 and so on.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Writing '1' will activate power saving mode 1 in sensor.
+		Similarly, 2 is to activate psm mode 2 and so on.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_period_available
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		List of valid values available in multiples of integration time
+		for which the light intensity must be above the cutoff level
+		before interrupt is asserted. This refers to persistence values.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_either_period
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Value in multiple of integration time for which the light intensity must
+		be above the cutoff level before interrupt is asserted.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_rising_value
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is above this value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_falling_value
+Date:		October 2019
+KernelVersion:	5.4
+Contact:	Rishi Gupta <gupt21@gmail.com>
+Description:
+		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
+		light intensity is below this value.
-- 
2.7.4

Re: [PATCH v4 3/3] iio: documentation: light: Add veml6030 sysfs documentation

[Jonathan Cameron]

On Tue, 22 Oct 2019 09:28:25 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

> The driver for veml6030 light sensor provides custom sysfs entries
> used to know parameters supported by the driver and to configure
> sensor like setting power saving mode and persistence etc. This
> commit document them.
> 
> Signed-off-by: Rishi Gupta <gupt21@gmail.com>
Hi Rishi,

Main issue here is that a lot of this is standard ABI.  Only ABI that
is non standard should be documented in a per device file.  If we don't
do this ever driver drifts off in it's own direction!

For PSM you run into the normal problem of custom ABI.  Reality is that
generic userspace will have no idea what to do with it.  Hence try
if at all possible to avoid custom ABI.  The PSM control appears to
be a control on the 'refresh rate' which corresponds to the maximum
possible sampling_frequency.  Hence use the standard ABI for sampling
frequency.  More info inline.

> ---
> Changes in v4:
> * None
> 
> Changes in v3:
> * Updated Date from September to October
> * Updated KernelVersion from 5.3.1 to 5.4
> * in_illuminance_period_available is now in events directory
> 
> Changes in v2:
> * None
> 
>  .../ABI/testing/sysfs-bus-iio-light-veml6030       | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030 b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> new file mode 100644
> index 0000000..13cd321
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> @@ -0,0 +1,49 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm_available
> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Provides list of valid values that can be used to activate a
> +		particular power saving mode in sensor. For ex; 1 means psm
> +		mode 1 and 2 means psm mode 2 and so on.
To a user of this device these modes are meaningless.  A user should not
need to open the datasheet to find out.

One thing to note is we very rarely let in power mode stuff in userspace interfaces
because it's very hard for userspace to know what to do with the them.

If there is a reason to switch modes it should be wrapped up in the driver.
We have things like runtime power management with timing based suspend etc
to magically deal with this stuff for us.

Superficially this effect of these seems to be on the 'refresh time', suggesting
that these are actually trading off potential sampling frequency for power
saving?  If that is the case, please handle this as a sampling_frequency control.
Userspace will set that as low as possible to meet it's requirements, as that
saves power in almost any device.  Clearly this value is interlinked with the
integration time.   That's fine. Integration time takes priority as that is what
a user actually 'needs' to control (to avoid saturation).  So when integration
time changes, the available and current sampling_frequency will change.

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_illuminance_psm
> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Writing '1' will activate power saving mode 1 in sensor.
> +		Similarly, 2 is to activate psm mode 2 and so on.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_period_available

This is standard ABI so should either already be documented, or should be added to the
main ABI file
ABI/testing/sysfs-bus-iio

I think this is true for all the others below as well.


> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		List of valid values available in multiples of integration time
> +		for which the light intensity must be above the cutoff level
> +		before interrupt is asserted. This refers to persistence values.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_either_period
> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Value in multiple of integration time for which the light intensity must
> +		be above the cutoff level before interrupt is asserted.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_rising_value
> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
> +		light intensity is above this value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/events/in_illuminance_thresh_falling_value


> +Date:		October 2019
> +KernelVersion:	5.4
> +Contact:	Rishi Gupta <gupt21@gmail.com>
> +Description:
> +		Raw threshold value from 0 to 0xffffffff. An interrupt will be asserted whenever
> +		light intensity is below this value.

Re: [RESEND PATCH v2 0/3] Add driver for veml6030 ambient light sensor

[Jonathan Cameron]

On Tue, 24 Sep 2019 16:21:55 +0530
Rishi Gupta <gupt21@gmail.com> wrote:

When doing a RESEND as opposed to a new version, please say
why.

> The veml6030 is an ambient light sensor from vishay and
> is a different hardware from an existing hardware for which
> driver currently exist, therefore this driver submission.
> 
> * All features; ALS, white channel & power management is
>   supported.
> 
> * All configurable parameters are supported through standard
>   iio sysfs entries. User space can get valid values of any
>   parameter (xx_available) and then can write to appropriate
>   sysfs entry.
> 
> * User space can get ALS & White channel readings through RAW
>   IIO interface.
> 
> * IIO events are used to notify application whenever threshold
>   is crossed. This uses IRQ pin of veml6030.
> 
> * Some registers in veml6030 are read only. For these registers
>   read callback returns error to user space.
> 
> There are 3 patches for this submission:
> [PATCH 1/3] iio: light: add driver for veml6030 ambient light sensor
> [PATCH 2/3] dt-bindings: iio: light: add veml6030 ALS bindings
> [PATCH 3/3] iio: documentation: light: Add veml6030 sysfs documentation
> 
> Rishi Gupta (3):
>   iio: light: add driver for veml6030 ambient light sensor
>   dt-bindings: iio: light: add veml6030 ALS bindings
>   iio: documentation: light: Add veml6030 sysfs documentation
> 
>  .../ABI/testing/sysfs-bus-iio-light-veml6030       |  49 ++
>  .../devicetree/bindings/iio/light/veml6030.yaml    |  62 ++
>  drivers/iio/light/Kconfig                          |  11 +
>  drivers/iio/light/Makefile                         |   1 +
>  drivers/iio/light/veml6030.c                       | 633 +++++++++++++++++++++
>  5 files changed, 756 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
>  create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml
>  create mode 100644 drivers/iio/light/veml6030.c
> 

Re: [RESEND PATCH v2 0/3] Add driver for veml6030 ambient light sensor

[rishi gupta]

My email server returned error so I was confused whether all patches
reached to mailing list intact or not, that is why I resent.
Regards,
Rishi

On Sat, Oct 5, 2019 at 7:38 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 24 Sep 2019 16:21:55 +0530
> Rishi Gupta <gupt21@gmail.com> wrote:
>
> When doing a RESEND as opposed to a new version, please say
> why.
>
> > The veml6030 is an ambient light sensor from vishay and
> > is a different hardware from an existing hardware for which
> > driver currently exist, therefore this driver submission.
> >
> > * All features; ALS, white channel & power management is
> >   supported.
> >
> > * All configurable parameters are supported through standard
> >   iio sysfs entries. User space can get valid values of any
> >   parameter (xx_available) and then can write to appropriate
> >   sysfs entry.
> >
> > * User space can get ALS & White channel readings through RAW
> >   IIO interface.
> >
> > * IIO events are used to notify application whenever threshold
> >   is crossed. This uses IRQ pin of veml6030.
> >
> > * Some registers in veml6030 are read only. For these registers
> >   read callback returns error to user space.
> >
> > There are 3 patches for this submission:
> > [PATCH 1/3] iio: light: add driver for veml6030 ambient light sensor
> > [PATCH 2/3] dt-bindings: iio: light: add veml6030 ALS bindings
> > [PATCH 3/3] iio: documentation: light: Add veml6030 sysfs documentation
> >
> > Rishi Gupta (3):
> >   iio: light: add driver for veml6030 ambient light sensor
> >   dt-bindings: iio: light: add veml6030 ALS bindings
> >   iio: documentation: light: Add veml6030 sysfs documentation
> >
> >  .../ABI/testing/sysfs-bus-iio-light-veml6030       |  49 ++
> >  .../devicetree/bindings/iio/light/veml6030.yaml    |  62 ++
> >  drivers/iio/light/Kconfig                          |  11 +
> >  drivers/iio/light/Makefile                         |   1 +
> >  drivers/iio/light/veml6030.c                       | 633 +++++++++++++++++++++
> >  5 files changed, 756 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-veml6030
> >  create mode 100644 Documentation/devicetree/bindings/iio/light/veml6030.yaml
> >  create mode 100644 drivers/iio/light/veml6030.c
> >
>