* [PATCH v4 1/3] iio: Add modifier for white light
@ 2018-08-02 18:27 Parthiban Nallathambi
2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Parthiban Nallathambi @ 2018-08-02 18:27 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, robh+dt
Cc: linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg,
wd, sbabic, hs, Parthiban Nallathambi
Signed-off-by: Parthiban Nallathambi <pn@denx.de>
---
Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
drivers/iio/industrialio-core.c | 1 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 ++
4 files changed, 11 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 731146c3b138..43e481aed5b2 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1312,6 +1312,13 @@ Description:
standardised CIE Erythemal Action Spectrum. UV index values range
from 0 (low) to >=11 (extreme).
+What: /sys/.../iio:deviceX/in_intensityY_white_raw
+KernelVersion: 4.18
+Contact: linux-iio@vger.kernel.org
+Description:
+ Modifier white indicates that measurements contain white LED
+ component.
+
What: /sys/.../iio:deviceX/in_intensity_red_integration_time
What: /sys/.../iio:deviceX/in_intensity_green_integration_time
What: /sys/.../iio:deviceX/in_intensity_blue_integration_time
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 19bdf3d2962a..cb939b9fad16 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -108,6 +108,7 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
[IIO_MOD_LIGHT_UV] = "uv",
+ [IIO_MOD_LIGHT_WHITE] = "white",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",
[IIO_MOD_TEMP_OBJECT] = "object",
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 4213cdf88e3c..de87a6c7e6de 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -84,6 +84,7 @@ enum iio_modifier {
IIO_MOD_CO2,
IIO_MOD_VOC,
IIO_MOD_LIGHT_UV,
+ IIO_MOD_LIGHT_WHITE,
};
enum iio_event_type {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index b61245e1181d..a2f9c62a79dd 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_LIGHT_GREEN] = "green",
[IIO_MOD_LIGHT_BLUE] = "blue",
[IIO_MOD_LIGHT_UV] = "uv",
+ [IIO_MOD_LIGHT_WHITE] = "white",
[IIO_MOD_QUATERNION] = "quaternion",
[IIO_MOD_TEMP_AMBIENT] = "ambient",
[IIO_MOD_TEMP_OBJECT] = "object",
@@ -178,6 +179,7 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_MOD_LIGHT_GREEN:
case IIO_MOD_LIGHT_BLUE:
case IIO_MOD_LIGHT_UV:
+ case IIO_MOD_LIGHT_WHITE:
case IIO_MOD_QUATERNION:
case IIO_MOD_TEMP_AMBIENT:
case IIO_MOD_TEMP_OBJECT:
--
2.14.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 2018-08-02 18:27 [PATCH v4 1/3] iio: Add modifier for white light Parthiban Nallathambi @ 2018-08-02 18:27 ` Parthiban Nallathambi 2018-08-02 19:29 ` Peter Meerwald-Stadler 2018-08-02 19:34 ` Andy Shevchenko 2018-08-02 18:27 ` [PATCH v4 3/3] iio: light: Add device tree binding " Parthiban Nallathambi 2018-08-02 19:30 ` [PATCH v4 1/3] iio: Add modifier for white light Peter Meerwald-Stadler 2 siblings, 2 replies; 10+ messages in thread From: Parthiban Nallathambi @ 2018-08-02 18:27 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw, robh+dt Cc: linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs, Parthiban Nallathambi Add support for VCNL4035, which is capable of Ambient light sensing (ALS) and proximity function. This patch adds support only for ALS function Signed-off-by: Parthiban Nallathambi <pn@denx.de> Changelog since v1: 1. Fixed 0-day warning on le16_to_cpu usage 2. Persistence value is directly mapped to datasheet's value to avoid confusions of usage from sysfs Changelog in v3: - Usage of lock is not needed, removed mutex locking - ALS threshold and persistence configuration available as events and threshold events are notified to userspace - Usage of devm_ is re-ordered and exit handling updated - Complexity in timestamp calculation is removed and used iio_get_time_ns Changelog in v4: - New white light index is introduced for getting data from white spectrum - PM enable/disable is called from read_raw accordingly - Probe exit handling re-ordered --- drivers/iio/light/Kconfig | 12 + drivers/iio/light/Makefile | 1 + drivers/iio/light/vcnl4035.c | 680 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 693 insertions(+) create mode 100644 drivers/iio/light/vcnl4035.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index c7ef8d1862d6..b7069a4c28a2 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -447,6 +447,18 @@ config VCNL4000 To compile this driver as a module, choose M here: the module will be called vcnl4000. +config VCNL4035 + tristate "VCNL4035 combined ALS and proximity sensor" + select REGMAP_I2C + depends on I2C + help + Say Y here if you want to build a driver for the Vishay VCNL4035, + combined ambient light (ALS) and proximity sensor. Currently only ALS + function is available. + + To compile this driver as a module, choose M here: the + module will be called vcnl4035. + config VEML6070 tristate "VEML6070 UV A light sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index 80943af5d627..dce98511a59b 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -44,6 +44,7 @@ obj-$(CONFIG_TSL2772) += tsl2772.o obj-$(CONFIG_TSL4531) += tsl4531.o obj-$(CONFIG_US5182D) += us5182d.o obj-$(CONFIG_VCNL4000) += vcnl4000.o +obj-$(CONFIG_VCNL4035) += vcnl4035.o obj-$(CONFIG_VEML6070) += veml6070.o obj-$(CONFIG_VL6180) += vl6180.o obj-$(CONFIG_ZOPT2201) += zopt2201.o diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c new file mode 100644 index 000000000000..f93004452b4e --- /dev/null +++ b/drivers/iio/light/vcnl4035.c @@ -0,0 +1,680 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * VCNL4035 Ambient Light and Proximity Sensor - 7-bit I2C slave address 0x60 + * + * Copyright (c) 2018, DENX Software Engineering GmbH + * Author: Parthiban Nallathambi <pn@denx.de> + * + * + * TODO: Proximity + */ +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/bitops.h> +#include <linux/regmap.h> +#include <linux/pm_runtime.h> + +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/events.h> +#include <linux/iio/buffer.h> +#include <linux/iio/trigger.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +#define VCNL4035_DRV_NAME "vcnl4035" +#define VCNL4035_IRQ_NAME "vcnl4035_event" +#define VCNL4035_REGMAP_NAME "vcnl4035_regmap" + +/* Device registers */ +#define VCNL4035_ALS_CONF 0x00 +#define VCNL4035_ALS_THDH 0x01 +#define VCNL4035_ALS_THDL 0x02 +#define VCNL4035_ALS_DATA 0x0B +#define VCNL4035_WHITE_DATA 0x0C +#define VCNL4035_INT_FLAG 0x0D +#define VCNL4035_DEV_ID 0x0E + +/* Register masks */ +#define VCNL4035_MODE_ALS_MASK BIT(0) +#define VCNL4035_MODE_ALS_WHITE_CHAN BIT(8) +#define VCNL4035_MODE_ALS_INT_MASK BIT(1) +#define VCNL4035_ALS_IT_MASK GENMASK(7, 5) +#define VCNL4035_ALS_PERS_MASK GENMASK(3, 2) +#define VCNL4035_INT_ALS_IF_H_MASK BIT(12) +#define VCNL4035_INT_ALS_IF_L_MASK BIT(13) + +/* Default values */ +#define VCNL4035_MODE_ALS_ENABLE BIT(0) +#define VCNL4035_MODE_ALS_DISABLE 0x00 +#define VCNL4035_MODE_ALS_INT_ENABLE BIT(1) +#define VCNL4035_MODE_ALS_INT_DISABLE 0 +#define VCNL4035_DEV_ID_VAL 0x80 +#define VCNL4035_ALS_IT_DEFAULT 0x01 +#define VCNL4035_ALS_PERS_DEFAULT 0x00 +#define VCNL4035_ALS_THDH_DEFAULT 5000 +#define VCNL4035_ALS_THDL_DEFAULT 100 +#define VCNL4035_SLEEP_DELAY_MS 2000 + +struct vcnl4035_data { + struct i2c_client *client; + struct regmap *regmap; + unsigned int als_it_val; + unsigned int als_persistence:4; + unsigned int als_thresh_low; + unsigned int als_thresh_high; + struct iio_trigger *drdy_trigger0; +}; + +static inline bool vcnl4035_is_triggered(struct vcnl4035_data *data) +{ + int ret; + int reg; + + ret = regmap_read(data->regmap, VCNL4035_INT_FLAG, ®); + if (ret < 0) + return false; + if (reg & (VCNL4035_INT_ALS_IF_H_MASK | VCNL4035_INT_ALS_IF_L_MASK)) + return true; + else + return false; +} + +static irqreturn_t vcnl4035_drdy_irq_thread(int irq, void *private) +{ + struct iio_dev *indio_dev = private; + struct vcnl4035_data *data = iio_priv(indio_dev); + + if (vcnl4035_is_triggered(data)) { + iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_LIGHT, + 0, + IIO_EV_TYPE_THRESH, + IIO_EV_DIR_EITHER), + iio_get_time_ns(indio_dev)); + iio_trigger_poll_chained(data->drdy_trigger0); + return IRQ_HANDLED; + } + + return IRQ_NONE; +} + +/* Triggered buffer */ +static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct vcnl4035_data *data = iio_priv(indio_dev); + u16 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]; + int ret; + + ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer); + if (!ret) + iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_get_time_ns(indio_dev)); + else + dev_err(&data->client->dev, + "Trigger consumer can't read from sensor.\n"); + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static int vcnl4035_als_drdy_set_state(struct iio_trigger *trigger, + bool enable_drdy) +{ + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trigger); + struct vcnl4035_data *data = iio_priv(indio_dev); + int val = enable_drdy ? VCNL4035_MODE_ALS_INT_ENABLE : + VCNL4035_MODE_ALS_INT_DISABLE; + + return regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_MODE_ALS_INT_MASK, + val); +} + +static const struct iio_trigger_ops vcnl4035_trigger_ops = { + .validate_device = iio_trigger_validate_own_device, + .set_trigger_state = vcnl4035_als_drdy_set_state, +}; + +static int vcnl4035_set_pm_runtime_state(struct vcnl4035_data *data, bool on) +{ + int ret; + struct device *dev = &data->client->dev; + + if (on) { + ret = pm_runtime_get_sync(dev); + if (ret < 0) + pm_runtime_put_noidle(dev); + } else { + pm_runtime_mark_last_busy(dev); + ret = pm_runtime_put_autosuspend(dev); + } + + return ret; +} + +/* + * Device IT INT Time (ms) Scale (lux/step) + * 000 50 0.064 + * 001 100 0.032 + * 010 200 0.016 + * 100 400 0.008 + * 101 - 111 800 0.004 + * Values are proportional, so ALS INT is selected for input due to + * simplicity reason. Integration time value and scaling is + * calculated based on device INT value + * + * Raw value needs to be scaled using ALS steps + * + */ +static int vcnl4035_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct vcnl4035_data *data = iio_priv(indio_dev); + int ret; + int raw_data; + unsigned int reg; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = vcnl4035_set_pm_runtime_state(data, true); + if (ret < 0) + return ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (!ret) { + if (chan->channel) + reg = VCNL4035_ALS_DATA; + else + reg = VCNL4035_WHITE_DATA; + ret = regmap_read(data->regmap, reg, &raw_data); + iio_device_release_direct_mode(indio_dev); + if (!ret) { + *val = raw_data; + ret = IIO_VAL_INT; + } + } + vcnl4035_set_pm_runtime_state(data, false); + return ret; + case IIO_CHAN_INFO_INT_TIME: + *val = 50; + if (!data->als_it_val) + *val = data->als_it_val * 100; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = 64; + if (!data->als_it_val) + *val2 = 1000; + else + *val2 = data->als_it_val * 2 * 1000; + return IIO_VAL_FRACTIONAL; + default: + return -EINVAL; + } +} + +static int vcnl4035_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + int ret; + struct vcnl4035_data *data = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_INT_TIME: + if (val <= 0 || val > 800) + return -EINVAL; + + ret = vcnl4035_set_pm_runtime_state(data, true); + if (ret < 0) + return ret; + + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_ALS_IT_MASK, + val / 100); + if (!ret) + data->als_it_val = val / 100; + + vcnl4035_set_pm_runtime_state(data, false); + return ret; + default: + return -EINVAL; + } +} + +/* No direct ABI for persistence and threshold, so eventing */ +static int vcnl4035_read_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) +{ + struct vcnl4035_data *data = iio_priv(indio_dev); + + switch (info) { + case IIO_EV_INFO_VALUE: + switch (dir) { + case IIO_EV_DIR_RISING: + *val = data->als_thresh_high; + return IIO_VAL_INT; + case IIO_EV_DIR_FALLING: + *val = data->als_thresh_low; + return IIO_VAL_INT; + default: + return -EINVAL; + } + break; + case IIO_EV_INFO_PERIOD: + *val = data->als_persistence; + return IIO_VAL_INT; + default: + return -EINVAL; + } + +} + +static int vcnl4035_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) +{ + struct vcnl4035_data *data = iio_priv(indio_dev); + int ret; + + switch (info) { + case IIO_EV_INFO_VALUE: + /* 16 bit threshold range 0 - 65535 */ + if (val < 0 || val > 65535) + return -EINVAL; + if (dir == IIO_EV_DIR_RISING) { + if (val < data->als_thresh_low) + return -EINVAL; + ret = regmap_write(data->regmap, VCNL4035_ALS_THDH, + val); + if (!ret) + data->als_thresh_high = val; + } else { + if (val > data->als_thresh_high) + return -EINVAL; + ret = regmap_write(data->regmap, VCNL4035_ALS_THDL, + val); + if (!ret) + data->als_thresh_low = val; + } + return ret; + case IIO_EV_INFO_PERIOD: + /* allow only 1 2 4 8 as persistence value */ + if (val < 0 || val > 8 || __sw_hweight8(val) != 1) + return -EINVAL; + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_ALS_PERS_MASK, val); + if (!ret) + data->als_persistence = val; + return ret; + default: + return -EINVAL; + } +} + +static IIO_CONST_ATTR_INT_TIME_AVAIL("50 100 200 400 800"); + +static struct attribute *vcnl4035_attributes[] = { + &iio_const_attr_integration_time_available.dev_attr.attr, + NULL, +}; + +static const struct attribute_group vcnl4035_attribute_group = { + .attrs = vcnl4035_attributes, +}; + +static const struct iio_info vcnl4035_info = { + .read_raw = vcnl4035_read_raw, + .write_raw = vcnl4035_write_raw, + .read_event_value = vcnl4035_read_thresh, + .write_event_value = vcnl4035_write_thresh, + .attrs = &vcnl4035_attribute_group, +}; + +static const struct iio_event_spec vcnl4035_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), + }, +}; + +enum vcnl4035_scan_index_order { + VCNL4035_CHAN_INDEX_LIGHT, + VCNL4035_CHAN_INDEX_WHITE_LED, +}; + +static const unsigned long vcnl4035_available_scan_masks[] = { + BIT(VCNL4035_CHAN_INDEX_LIGHT), + BIT(VCNL4035_CHAN_INDEX_WHITE_LED), + 0 +}; + +static const struct iio_chan_spec vcnl4035_channels[] = { + { + .type = IIO_INTENSITY, + .channel = 0, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_INT_TIME) | + BIT(IIO_CHAN_INFO_SCALE), + .event_spec = vcnl4035_event_spec, + .num_event_specs = ARRAY_SIZE(vcnl4035_event_spec), + .scan_index = VCNL4035_CHAN_INDEX_LIGHT, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_LE, + }, + }, + { + .type = IIO_INTENSITY, + .channel = 1, + .modified = 1, + .channel2 = IIO_MOD_LIGHT_WHITE, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .scan_index = VCNL4035_CHAN_INDEX_WHITE_LED, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_LE, + }, + }, +}; + +static int vcnl4035_set_als_power_state(struct vcnl4035_data *data, u8 status) +{ + return regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_MODE_ALS_MASK, + status); +} + +static int vcnl4035_init(struct vcnl4035_data *data) +{ + int ret; + int id; + + ret = regmap_read(data->regmap, VCNL4035_DEV_ID, &id); + if (ret < 0) { + dev_err(&data->client->dev, "Failed to read DEV_ID register\n"); + return ret; + } + + if (id != VCNL4035_DEV_ID_VAL) { + dev_err(&data->client->dev, "Wrong id, got %x, expected %x\n", + id, VCNL4035_DEV_ID_VAL); + return -ENODEV; + } + +#ifndef CONFIG_PM + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); + if (ret < 0) + return ret; +#endif + /* ALS white channel enable */ + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_MODE_ALS_WHITE_CHAN, + 1); + if (ret) { + pr_err("regmap_update_bits white channel enable %d\n", ret); + return ret; + } + + /* set default integration time - 100 ms for ALS */ + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_ALS_IT_MASK, + VCNL4035_ALS_IT_DEFAULT); + if (ret) { + pr_err("regmap_update_bits default ALS IT returned %d\n", ret); + return ret; + } + data->als_it_val = VCNL4035_ALS_IT_DEFAULT; + + /* set default persistence time - 1 for ALS */ + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, + VCNL4035_ALS_PERS_MASK, + VCNL4035_ALS_PERS_DEFAULT); + if (ret) { + pr_err("regmap_update_bits default PERS returned %d\n", ret); + return ret; + } + data->als_persistence = VCNL4035_ALS_PERS_DEFAULT; + + /* set default HIGH threshold for ALS */ + ret = regmap_write(data->regmap, VCNL4035_ALS_THDH, + VCNL4035_ALS_THDH_DEFAULT); + if (ret) { + pr_err("regmap_write default THDH returned %d\n", ret); + return ret; + } + data->als_thresh_high = VCNL4035_ALS_THDH_DEFAULT; + + /* set default LOW threshold for ALS */ + ret = regmap_write(data->regmap, VCNL4035_ALS_THDL, + VCNL4035_ALS_THDL_DEFAULT); + if (ret) { + pr_err("regmap_write default THDL returned %d\n", ret); + return ret; + } + data->als_thresh_low = VCNL4035_ALS_THDL_DEFAULT; + + return 0; +} + +static bool vcnl4035_is_volatile_reg(struct device *dev, unsigned int reg) +{ + switch (reg) { + case VCNL4035_ALS_CONF: + case VCNL4035_DEV_ID: + return false; + default: + return true; + } +} + +static const struct regmap_config vcnl4035_regmap_config = { + .name = VCNL4035_REGMAP_NAME, + .reg_bits = 8, + .val_bits = 16, + .max_register = VCNL4035_DEV_ID, + .cache_type = REGCACHE_RBTREE, + .volatile_reg = vcnl4035_is_volatile_reg, + .val_format_endian = REGMAP_ENDIAN_LITTLE, +}; + +static int vcnl4035_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct vcnl4035_data *data; + struct iio_dev *indio_dev; + struct regmap *regmap; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + regmap = devm_regmap_init_i2c(client, &vcnl4035_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&client->dev, "regmap_init failed!\n"); + return PTR_ERR(regmap); + } + + 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 = &vcnl4035_info; + indio_dev->name = VCNL4035_DRV_NAME; + indio_dev->channels = vcnl4035_channels; + indio_dev->num_channels = ARRAY_SIZE(vcnl4035_channels); + indio_dev->modes = INDIO_DIRECT_MODE; + + ret = vcnl4035_init(data); + if (ret < 0) { + dev_err(&client->dev, "vcnl4035 chip init failed\n"); + return ret; + } + + ret = pm_runtime_set_active(&client->dev); + if (ret < 0) + goto fail_poweroff; + + pm_runtime_enable(&client->dev); + pm_runtime_set_autosuspend_delay(&client->dev, VCNL4035_SLEEP_DELAY_MS); + pm_runtime_use_autosuspend(&client->dev); + + if (client->irq) { + data->drdy_trigger0 = devm_iio_trigger_alloc( + indio_dev->dev.parent, + "%s-dev%d", indio_dev->name, indio_dev->id); + if (!data->drdy_trigger0) { + ret = -ENOMEM; + goto fail_pm_disable; + } + data->drdy_trigger0->dev.parent = indio_dev->dev.parent; + data->drdy_trigger0->ops = &vcnl4035_trigger_ops; + iio_trigger_set_drvdata(data->drdy_trigger0, indio_dev); + ret = iio_trigger_register(data->drdy_trigger0); + if (ret) { + dev_err(&client->dev, "iio trigger register failed\n"); + goto fail_pm_disable; + } + + /* Trigger setup */ + ret = iio_triggered_buffer_setup(indio_dev, NULL, + vcnl4035_trigger_consumer_handler, + NULL); + if (ret < 0) { + dev_err(&client->dev, "iio triggered buffer setup failed\n"); + goto fail_trigger_unregister; + } + + /* IRQ to trigger mapping */ + ret = devm_request_threaded_irq(&client->dev, client->irq, + NULL, vcnl4035_drdy_irq_thread, + IRQF_TRIGGER_LOW | IRQF_ONESHOT, + VCNL4035_IRQ_NAME, indio_dev); + if (ret < 0) { + dev_err(&client->dev, "request irq %d for trigger0 failed\n", + client->irq); + goto fail_buffer_clean; + } + } + + ret = iio_device_register(indio_dev); + if (ret) + goto fail_buffer_clean; + return 0; + +fail_buffer_clean: + iio_triggered_buffer_cleanup(indio_dev); +fail_trigger_unregister: + iio_trigger_unregister(data->drdy_trigger0); +fail_pm_disable: + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); + pm_runtime_put_noidle(&client->dev); +fail_poweroff: + vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE); + return ret; +} + +static int vcnl4035_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + struct vcnl4035_data *data = iio_priv(indio_dev); + + iio_triggered_buffer_cleanup(indio_dev); + iio_trigger_unregister(data->drdy_trigger0); + iio_device_unregister(indio_dev); + + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); + pm_runtime_put_noidle(&client->dev); + + return vcnl4035_set_als_power_state(iio_priv(indio_dev), + VCNL4035_MODE_ALS_DISABLE); +} + +#ifdef CONFIG_PM +static int vcnl4035_runtime_suspend(struct device *dev) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + struct vcnl4035_data *data = iio_priv(indio_dev); + int ret; + + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE); + regcache_mark_dirty(data->regmap); + + return ret; +} + +static int vcnl4035_runtime_resume(struct device *dev) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + struct vcnl4035_data *data = iio_priv(indio_dev); + int ret; + + regcache_sync(data->regmap); + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); + if (ret < 0) + return ret; + /* wait for 1 ALS integration cycle */ + msleep(data->als_it_val * 100); + + return 0; +} +#endif + +static const struct dev_pm_ops vcnl4035_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) + SET_RUNTIME_PM_OPS(vcnl4035_runtime_suspend, + vcnl4035_runtime_resume, NULL) +}; + +static const struct of_device_id vcnl4035_of_match[] = { + { .compatible = "vishay,vcnl4035", }, + { }, +}; +MODULE_DEVICE_TABLE(of, vcnl4035_of_match); + +static const struct i2c_device_id vcnl4035_id[] = { + { "vcnl4035", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, vcnl4035_id); + +static struct i2c_driver vcnl4035_driver = { + .driver = { + .name = VCNL4035_DRV_NAME, + .pm = &vcnl4035_pm_ops, + .of_match_table = of_match_ptr(vcnl4035_of_match), + }, + .probe = vcnl4035_probe, + .remove = vcnl4035_remove, + .id_table = vcnl4035_id, +}; + +module_i2c_driver(vcnl4035_driver); + +MODULE_AUTHOR("Parthiban Nallathambi <pn@denx.de>"); +MODULE_DESCRIPTION("VCNL4035 Ambient Light Sensor driver"); +MODULE_LICENSE("GPL v2"); -- 2.14.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi @ 2018-08-02 19:29 ` Peter Meerwald-Stadler 2018-08-02 19:34 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Peter Meerwald-Stadler @ 2018-08-02 19:29 UTC (permalink / raw) To: Parthiban Nallathambi; +Cc: jic23, linux-iio, linux-kernel, wd, sbabic, hs On Thu, 2 Aug 2018, Parthiban Nallathambi wrote: > Add support for VCNL4035, which is capable of Ambient light > sensing (ALS) and proximity function. This patch adds support > only for ALS function comment below > Signed-off-by: Parthiban Nallathambi <pn@denx.de> > > Changelog since v1: > > 1. Fixed 0-day warning on le16_to_cpu usage > 2. Persistence value is directly mapped to datasheet's value to > avoid confusions of usage from sysfs > > Changelog in v3: > - Usage of lock is not needed, removed mutex locking > - ALS threshold and persistence configuration available > as events and threshold events are notified to userspace > - Usage of devm_ is re-ordered and exit handling updated > - Complexity in timestamp calculation is removed and used > iio_get_time_ns > > Changelog in v4: > - New white light index is introduced for getting data from > white spectrum > - PM enable/disable is called from read_raw accordingly > - Probe exit handling re-ordered > --- > drivers/iio/light/Kconfig | 12 + > drivers/iio/light/Makefile | 1 + > drivers/iio/light/vcnl4035.c | 680 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 693 insertions(+) > create mode 100644 drivers/iio/light/vcnl4035.c > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index c7ef8d1862d6..b7069a4c28a2 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -447,6 +447,18 @@ config VCNL4000 > To compile this driver as a module, choose M here: the > module will be called vcnl4000. > > +config VCNL4035 > + tristate "VCNL4035 combined ALS and proximity sensor" > + select REGMAP_I2C > + depends on I2C > + help > + Say Y here if you want to build a driver for the Vishay VCNL4035, > + combined ambient light (ALS) and proximity sensor. Currently only ALS > + function is available. > + > + To compile this driver as a module, choose M here: the > + module will be called vcnl4035. > + > config VEML6070 > tristate "VEML6070 UV A light sensor" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index 80943af5d627..dce98511a59b 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -44,6 +44,7 @@ obj-$(CONFIG_TSL2772) += tsl2772.o > obj-$(CONFIG_TSL4531) += tsl4531.o > obj-$(CONFIG_US5182D) += us5182d.o > obj-$(CONFIG_VCNL4000) += vcnl4000.o > +obj-$(CONFIG_VCNL4035) += vcnl4035.o > obj-$(CONFIG_VEML6070) += veml6070.o > obj-$(CONFIG_VL6180) += vl6180.o > obj-$(CONFIG_ZOPT2201) += zopt2201.o > diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c > new file mode 100644 > index 000000000000..f93004452b4e > --- /dev/null > +++ b/drivers/iio/light/vcnl4035.c > @@ -0,0 +1,680 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * VCNL4035 Ambient Light and Proximity Sensor - 7-bit I2C slave address 0x60 > + * > + * Copyright (c) 2018, DENX Software Engineering GmbH > + * Author: Parthiban Nallathambi <pn@denx.de> > + * > + * > + * TODO: Proximity > + */ > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <linux/pm_runtime.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/events.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > + > +#define VCNL4035_DRV_NAME "vcnl4035" > +#define VCNL4035_IRQ_NAME "vcnl4035_event" > +#define VCNL4035_REGMAP_NAME "vcnl4035_regmap" > + > +/* Device registers */ > +#define VCNL4035_ALS_CONF 0x00 > +#define VCNL4035_ALS_THDH 0x01 > +#define VCNL4035_ALS_THDL 0x02 > +#define VCNL4035_ALS_DATA 0x0B > +#define VCNL4035_WHITE_DATA 0x0C > +#define VCNL4035_INT_FLAG 0x0D > +#define VCNL4035_DEV_ID 0x0E > + > +/* Register masks */ > +#define VCNL4035_MODE_ALS_MASK BIT(0) > +#define VCNL4035_MODE_ALS_WHITE_CHAN BIT(8) > +#define VCNL4035_MODE_ALS_INT_MASK BIT(1) > +#define VCNL4035_ALS_IT_MASK GENMASK(7, 5) > +#define VCNL4035_ALS_PERS_MASK GENMASK(3, 2) > +#define VCNL4035_INT_ALS_IF_H_MASK BIT(12) > +#define VCNL4035_INT_ALS_IF_L_MASK BIT(13) > + > +/* Default values */ > +#define VCNL4035_MODE_ALS_ENABLE BIT(0) > +#define VCNL4035_MODE_ALS_DISABLE 0x00 > +#define VCNL4035_MODE_ALS_INT_ENABLE BIT(1) > +#define VCNL4035_MODE_ALS_INT_DISABLE 0 > +#define VCNL4035_DEV_ID_VAL 0x80 > +#define VCNL4035_ALS_IT_DEFAULT 0x01 > +#define VCNL4035_ALS_PERS_DEFAULT 0x00 > +#define VCNL4035_ALS_THDH_DEFAULT 5000 > +#define VCNL4035_ALS_THDL_DEFAULT 100 > +#define VCNL4035_SLEEP_DELAY_MS 2000 > + > +struct vcnl4035_data { > + struct i2c_client *client; > + struct regmap *regmap; > + unsigned int als_it_val; > + unsigned int als_persistence:4; > + unsigned int als_thresh_low; > + unsigned int als_thresh_high; > + struct iio_trigger *drdy_trigger0; > +}; > + > +static inline bool vcnl4035_is_triggered(struct vcnl4035_data *data) > +{ > + int ret; > + int reg; > + > + ret = regmap_read(data->regmap, VCNL4035_INT_FLAG, ®); > + if (ret < 0) > + return false; > + if (reg & (VCNL4035_INT_ALS_IF_H_MASK | VCNL4035_INT_ALS_IF_L_MASK)) > + return true; > + else > + return false; > +} > + > +static irqreturn_t vcnl4035_drdy_irq_thread(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct vcnl4035_data *data = iio_priv(indio_dev); > + > + if (vcnl4035_is_triggered(data)) { > + iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_LIGHT, > + 0, > + IIO_EV_TYPE_THRESH, > + IIO_EV_DIR_EITHER), > + iio_get_time_ns(indio_dev)); > + iio_trigger_poll_chained(data->drdy_trigger0); > + return IRQ_HANDLED; > + } > + > + return IRQ_NONE; > +} > + > +/* Triggered buffer */ > +static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct vcnl4035_data *data = iio_priv(indio_dev); > + u16 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]; > + int ret; > + > + ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer); > + if (!ret) > + iio_push_to_buffers_with_timestamp(indio_dev, buffer, > + iio_get_time_ns(indio_dev)); > + else > + dev_err(&data->client->dev, > + "Trigger consumer can't read from sensor.\n"); > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static int vcnl4035_als_drdy_set_state(struct iio_trigger *trigger, > + bool enable_drdy) > +{ > + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trigger); > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int val = enable_drdy ? VCNL4035_MODE_ALS_INT_ENABLE : > + VCNL4035_MODE_ALS_INT_DISABLE; > + > + return regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_MODE_ALS_INT_MASK, > + val); > +} > + > +static const struct iio_trigger_ops vcnl4035_trigger_ops = { > + .validate_device = iio_trigger_validate_own_device, > + .set_trigger_state = vcnl4035_als_drdy_set_state, > +}; > + > +static int vcnl4035_set_pm_runtime_state(struct vcnl4035_data *data, bool on) > +{ > + int ret; > + struct device *dev = &data->client->dev; > + > + if (on) { > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) > + pm_runtime_put_noidle(dev); > + } else { > + pm_runtime_mark_last_busy(dev); > + ret = pm_runtime_put_autosuspend(dev); > + } > + > + return ret; > +} > + > +/* > + * Device IT INT Time (ms) Scale (lux/step) > + * 000 50 0.064 > + * 001 100 0.032 > + * 010 200 0.016 > + * 100 400 0.008 > + * 101 - 111 800 0.004 > + * Values are proportional, so ALS INT is selected for input due to > + * simplicity reason. Integration time value and scaling is > + * calculated based on device INT value > + * > + * Raw value needs to be scaled using ALS steps > + * > + */ > +static int vcnl4035_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int ret; > + int raw_data; > + unsigned int reg; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = vcnl4035_set_pm_runtime_state(data, true); > + if (ret < 0) > + return ret; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (!ret) { > + if (chan->channel) > + reg = VCNL4035_ALS_DATA; > + else > + reg = VCNL4035_WHITE_DATA; > + ret = regmap_read(data->regmap, reg, &raw_data); > + iio_device_release_direct_mode(indio_dev); > + if (!ret) { > + *val = raw_data; > + ret = IIO_VAL_INT; > + } > + } > + vcnl4035_set_pm_runtime_state(data, false); > + return ret; > + case IIO_CHAN_INFO_INT_TIME: > + *val = 50; > + if (!data->als_it_val) > + *val = data->als_it_val * 100; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + *val = 64; > + if (!data->als_it_val) > + *val2 = 1000; > + else > + *val2 = data->als_it_val * 2 * 1000; > + return IIO_VAL_FRACTIONAL; > + default: > + return -EINVAL; > + } > +} > + > +static int vcnl4035_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > +{ > + int ret; > + struct vcnl4035_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_INT_TIME: > + if (val <= 0 || val > 800) > + return -EINVAL; > + > + ret = vcnl4035_set_pm_runtime_state(data, true); > + if (ret < 0) > + return ret; > + > + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_ALS_IT_MASK, > + val / 100); > + if (!ret) > + data->als_it_val = val / 100; > + > + vcnl4035_set_pm_runtime_state(data, false); > + return ret; > + default: > + return -EINVAL; > + } > +} > + > +/* No direct ABI for persistence and threshold, so eventing */ > +static int vcnl4035_read_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) > +{ > + struct vcnl4035_data *data = iio_priv(indio_dev); > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + switch (dir) { > + case IIO_EV_DIR_RISING: > + *val = data->als_thresh_high; > + return IIO_VAL_INT; > + case IIO_EV_DIR_FALLING: > + *val = data->als_thresh_low; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + break; > + case IIO_EV_INFO_PERIOD: > + *val = data->als_persistence; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + > +} > + > +static int vcnl4035_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) > +{ > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + /* 16 bit threshold range 0 - 65535 */ > + if (val < 0 || val > 65535) > + return -EINVAL; > + if (dir == IIO_EV_DIR_RISING) { > + if (val < data->als_thresh_low) > + return -EINVAL; > + ret = regmap_write(data->regmap, VCNL4035_ALS_THDH, > + val); > + if (!ret) > + data->als_thresh_high = val; > + } else { > + if (val > data->als_thresh_high) > + return -EINVAL; > + ret = regmap_write(data->regmap, VCNL4035_ALS_THDL, > + val); > + if (!ret) > + data->als_thresh_low = val; > + } > + return ret; > + case IIO_EV_INFO_PERIOD: > + /* allow only 1 2 4 8 as persistence value */ > + if (val < 0 || val > 8 || __sw_hweight8(val) != 1) > + return -EINVAL; > + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_ALS_PERS_MASK, val); > + if (!ret) > + data->als_persistence = val; > + return ret; > + default: > + return -EINVAL; > + } > +} > + > +static IIO_CONST_ATTR_INT_TIME_AVAIL("50 100 200 400 800"); > + > +static struct attribute *vcnl4035_attributes[] = { > + &iio_const_attr_integration_time_available.dev_attr.attr, > + NULL, > +}; > + > +static const struct attribute_group vcnl4035_attribute_group = { > + .attrs = vcnl4035_attributes, > +}; > + > +static const struct iio_info vcnl4035_info = { > + .read_raw = vcnl4035_read_raw, > + .write_raw = vcnl4035_write_raw, > + .read_event_value = vcnl4035_read_thresh, > + .write_event_value = vcnl4035_write_thresh, > + .attrs = &vcnl4035_attribute_group, > +}; > + > +static const struct iio_event_spec vcnl4035_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), > + }, > +}; > + > +enum vcnl4035_scan_index_order { > + VCNL4035_CHAN_INDEX_LIGHT, > + VCNL4035_CHAN_INDEX_WHITE_LED, > +}; > + > +static const unsigned long vcnl4035_available_scan_masks[] = { > + BIT(VCNL4035_CHAN_INDEX_LIGHT), > + BIT(VCNL4035_CHAN_INDEX_WHITE_LED), > + 0 > +}; > + > +static const struct iio_chan_spec vcnl4035_channels[] = { > + { > + .type = IIO_INTENSITY, > + .channel = 0, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_INT_TIME) | > + BIT(IIO_CHAN_INFO_SCALE), > + .event_spec = vcnl4035_event_spec, > + .num_event_specs = ARRAY_SIZE(vcnl4035_event_spec), > + .scan_index = VCNL4035_CHAN_INDEX_LIGHT, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_LE, > + }, > + }, > + { > + .type = IIO_INTENSITY, > + .channel = 1, > + .modified = 1, > + .channel2 = IIO_MOD_LIGHT_WHITE, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .scan_index = VCNL4035_CHAN_INDEX_WHITE_LED, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_LE, > + }, > + }, > +}; > + > +static int vcnl4035_set_als_power_state(struct vcnl4035_data *data, u8 status) > +{ > + return regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_MODE_ALS_MASK, > + status); > +} > + > +static int vcnl4035_init(struct vcnl4035_data *data) > +{ > + int ret; > + int id; > + > + ret = regmap_read(data->regmap, VCNL4035_DEV_ID, &id); > + if (ret < 0) { > + dev_err(&data->client->dev, "Failed to read DEV_ID register\n"); > + return ret; > + } > + > + if (id != VCNL4035_DEV_ID_VAL) { > + dev_err(&data->client->dev, "Wrong id, got %x, expected %x\n", > + id, VCNL4035_DEV_ID_VAL); > + return -ENODEV; > + } > + > +#ifndef CONFIG_PM > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > + if (ret < 0) > + return ret; > +#endif > + /* ALS white channel enable */ > + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_MODE_ALS_WHITE_CHAN, > + 1); > + if (ret) { > + pr_err("regmap_update_bits white channel enable %d\n", ret); > + return ret; > + } > + > + /* set default integration time - 100 ms for ALS */ > + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_ALS_IT_MASK, > + VCNL4035_ALS_IT_DEFAULT); > + if (ret) { > + pr_err("regmap_update_bits default ALS IT returned %d\n", ret); > + return ret; > + } > + data->als_it_val = VCNL4035_ALS_IT_DEFAULT; > + > + /* set default persistence time - 1 for ALS */ > + ret = regmap_update_bits(data->regmap, VCNL4035_ALS_CONF, > + VCNL4035_ALS_PERS_MASK, > + VCNL4035_ALS_PERS_DEFAULT); > + if (ret) { > + pr_err("regmap_update_bits default PERS returned %d\n", ret); > + return ret; > + } > + data->als_persistence = VCNL4035_ALS_PERS_DEFAULT; > + > + /* set default HIGH threshold for ALS */ > + ret = regmap_write(data->regmap, VCNL4035_ALS_THDH, > + VCNL4035_ALS_THDH_DEFAULT); > + if (ret) { > + pr_err("regmap_write default THDH returned %d\n", ret); > + return ret; > + } > + data->als_thresh_high = VCNL4035_ALS_THDH_DEFAULT; > + > + /* set default LOW threshold for ALS */ > + ret = regmap_write(data->regmap, VCNL4035_ALS_THDL, > + VCNL4035_ALS_THDL_DEFAULT); > + if (ret) { > + pr_err("regmap_write default THDL returned %d\n", ret); > + return ret; > + } > + data->als_thresh_low = VCNL4035_ALS_THDL_DEFAULT; > + > + return 0; > +} > + > +static bool vcnl4035_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case VCNL4035_ALS_CONF: > + case VCNL4035_DEV_ID: > + return false; > + default: > + return true; > + } > +} > + > +static const struct regmap_config vcnl4035_regmap_config = { > + .name = VCNL4035_REGMAP_NAME, > + .reg_bits = 8, > + .val_bits = 16, > + .max_register = VCNL4035_DEV_ID, > + .cache_type = REGCACHE_RBTREE, > + .volatile_reg = vcnl4035_is_volatile_reg, > + .val_format_endian = REGMAP_ENDIAN_LITTLE, > +}; > + > +static int vcnl4035_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct vcnl4035_data *data; > + struct iio_dev *indio_dev; > + struct regmap *regmap; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + regmap = devm_regmap_init_i2c(client, &vcnl4035_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "regmap_init failed!\n"); > + return PTR_ERR(regmap); > + } > + > + 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 = &vcnl4035_info; > + indio_dev->name = VCNL4035_DRV_NAME; > + indio_dev->channels = vcnl4035_channels; > + indio_dev->num_channels = ARRAY_SIZE(vcnl4035_channels); > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + ret = vcnl4035_init(data); > + if (ret < 0) { > + dev_err(&client->dev, "vcnl4035 chip init failed\n"); > + return ret; > + } > + > + ret = pm_runtime_set_active(&client->dev); > + if (ret < 0) > + goto fail_poweroff; > + > + pm_runtime_enable(&client->dev); > + pm_runtime_set_autosuspend_delay(&client->dev, VCNL4035_SLEEP_DELAY_MS); > + pm_runtime_use_autosuspend(&client->dev); > + > + if (client->irq) { > + data->drdy_trigger0 = devm_iio_trigger_alloc( > + indio_dev->dev.parent, > + "%s-dev%d", indio_dev->name, indio_dev->id); > + if (!data->drdy_trigger0) { > + ret = -ENOMEM; > + goto fail_pm_disable; > + } > + data->drdy_trigger0->dev.parent = indio_dev->dev.parent; > + data->drdy_trigger0->ops = &vcnl4035_trigger_ops; > + iio_trigger_set_drvdata(data->drdy_trigger0, indio_dev); > + ret = iio_trigger_register(data->drdy_trigger0); > + if (ret) { > + dev_err(&client->dev, "iio trigger register failed\n"); > + goto fail_pm_disable; > + } > + > + /* Trigger setup */ > + ret = iio_triggered_buffer_setup(indio_dev, NULL, > + vcnl4035_trigger_consumer_handler, > + NULL); > + if (ret < 0) { > + dev_err(&client->dev, "iio triggered buffer setup failed\n"); > + goto fail_trigger_unregister; > + } > + > + /* IRQ to trigger mapping */ > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + NULL, vcnl4035_drdy_irq_thread, > + IRQF_TRIGGER_LOW | IRQF_ONESHOT, > + VCNL4035_IRQ_NAME, indio_dev); > + if (ret < 0) { > + dev_err(&client->dev, "request irq %d for trigger0 failed\n", > + client->irq); > + goto fail_buffer_clean; > + } the closing bracket seems to be mis-indented > + } > + > + ret = iio_device_register(indio_dev); > + if (ret) > + goto fail_buffer_clean; > + return 0; > + > +fail_buffer_clean: > + iio_triggered_buffer_cleanup(indio_dev); > +fail_trigger_unregister: > + iio_trigger_unregister(data->drdy_trigger0); > +fail_pm_disable: > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + pm_runtime_put_noidle(&client->dev); > +fail_poweroff: > + vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE); > + return ret; > +} > + > +static int vcnl4035_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct vcnl4035_data *data = iio_priv(indio_dev); > + > + iio_triggered_buffer_cleanup(indio_dev); > + iio_trigger_unregister(data->drdy_trigger0); > + iio_device_unregister(indio_dev); > + > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + pm_runtime_put_noidle(&client->dev); > + > + return vcnl4035_set_als_power_state(iio_priv(indio_dev), > + VCNL4035_MODE_ALS_DISABLE); > +} > + > +#ifdef CONFIG_PM > +static int vcnl4035_runtime_suspend(struct device *dev) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int ret; > + > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE); > + regcache_mark_dirty(data->regmap); > + > + return ret; > +} > + > +static int vcnl4035_runtime_resume(struct device *dev) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int ret; > + > + regcache_sync(data->regmap); > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > + if (ret < 0) > + return ret; > + /* wait for 1 ALS integration cycle */ > + msleep(data->als_it_val * 100); > + > + return 0; > +} > +#endif > + > +static const struct dev_pm_ops vcnl4035_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > + SET_RUNTIME_PM_OPS(vcnl4035_runtime_suspend, > + vcnl4035_runtime_resume, NULL) > +}; > + > +static const struct of_device_id vcnl4035_of_match[] = { > + { .compatible = "vishay,vcnl4035", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, vcnl4035_of_match); > + > +static const struct i2c_device_id vcnl4035_id[] = { > + { "vcnl4035", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, vcnl4035_id); > + > +static struct i2c_driver vcnl4035_driver = { > + .driver = { > + .name = VCNL4035_DRV_NAME, > + .pm = &vcnl4035_pm_ops, > + .of_match_table = of_match_ptr(vcnl4035_of_match), > + }, > + .probe = vcnl4035_probe, > + .remove = vcnl4035_remove, > + .id_table = vcnl4035_id, > +}; > + > +module_i2c_driver(vcnl4035_driver); > + > +MODULE_AUTHOR("Parthiban Nallathambi <pn@denx.de>"); > +MODULE_DESCRIPTION("VCNL4035 Ambient Light Sensor driver"); > +MODULE_LICENSE("GPL v2"); > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi 2018-08-02 19:29 ` Peter Meerwald-Stadler @ 2018-08-02 19:34 ` Andy Shevchenko 2018-08-03 20:55 ` Jonathan Cameron 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2018-08-02 19:34 UTC (permalink / raw) To: Parthiban Nallathambi Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Rob Herring, linux-iio, Linux Kernel Mailing List, Mark Rutland, devicetree, Matthias Brugger, wd, sbabic, Heiko Schocher On Thu, Aug 2, 2018 at 9:27 PM, Parthiban Nallathambi <pn@denx.de> wrote: > Add support for VCNL4035, which is capable of Ambient light > sensing (ALS) and proximity function. This patch adds support > only for ALS function > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/bitops.h> > +#include <linux/regmap.h> > +#include <linux/pm_runtime.h> Sort? > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/events.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> Ditto. > + if (reg & (VCNL4035_INT_ALS_IF_H_MASK | VCNL4035_INT_ALS_IF_L_MASK)) > + return true; > + else > + return false; return !!(reg & ...); ? > +static irqreturn_t vcnl4035_drdy_irq_thread(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct vcnl4035_data *data = iio_priv(indio_dev); > + > + if (vcnl4035_is_triggered(data)) { Here is negative conditional suits. > +#ifndef CONFIG_PM Why? > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > + if (ret < 0) > + return ret; > +#endif > + pr_err("regmap_write default THDH returned %d\n", ret); dev_err() > + pr_err("regmap_write default THDL returned %d\n", ret); Ditto. > + ret = pm_runtime_set_active(&client->dev); > + if (ret < 0) > + goto fail_poweroff; > + > + pm_runtime_enable(&client->dev); > + pm_runtime_set_autosuspend_delay(&client->dev, VCNL4035_SLEEP_DELAY_MS); > + pm_runtime_use_autosuspend(&client->dev); Usually we do this after we probed the device. > + if (client->irq) { First of all, it can be negative. Second, care to factor out this rather long branch to a separate function? > + if (ret < 0) { > + dev_err(&client->dev, "request irq %d for trigger0 failed\n", > + client->irq); > + goto fail_buffer_clean; > + } Indentation. > + } So, do I understand correctly that IRQ is optional for this device? > +#ifdef CONFIG_PM > +static int vcnl4035_runtime_suspend(struct device *dev) Perhaps __maybe_unused > +static int vcnl4035_runtime_resume(struct device *dev) Ditto. > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct vcnl4035_data *data = iio_priv(indio_dev); > + int ret; > + > + regcache_sync(data->regmap); > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > + if (ret < 0) > + return ret; + blank line > + /* wait for 1 ALS integration cycle */ > + msleep(data->als_it_val * 100); > + > + return 0; > +} > +#endif > +static const struct of_device_id vcnl4035_of_match[] = { > + { .compatible = "vishay,vcnl4035", }, > + { }, Better w/o comma. > +}; > +MODULE_DEVICE_TABLE(of, vcnl4035_of_match); > + > +static const struct i2c_device_id vcnl4035_id[] = { > + { "vcnl4035", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, vcnl4035_id); Are you expecting legacy code to use this? I would rather switch to ->probe_new() and also remove this. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 2018-08-02 19:34 ` Andy Shevchenko @ 2018-08-03 20:55 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2018-08-03 20:55 UTC (permalink / raw) To: Andy Shevchenko Cc: Parthiban Nallathambi, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Rob Herring, linux-iio, Linux Kernel Mailing List, Mark Rutland, devicetree, Matthias Brugger, wd, sbabic, Heiko Schocher On Thu, 2 Aug 2018 22:34:14 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, Aug 2, 2018 at 9:27 PM, Parthiban Nallathambi <pn@denx.de> wrote: > > Add support for VCNL4035, which is capable of Ambient light > > sensing (ALS) and proximity function. This patch adds support > > only for ALS function > > > +#include <linux/module.h> > > +#include <linux/i2c.h> > > +#include <linux/bitops.h> > > +#include <linux/regmap.h> > > +#include <linux/pm_runtime.h> > > Sort? > > > +#include <linux/iio/iio.h> > > +#include <linux/iio/sysfs.h> > > +#include <linux/iio/events.h> > > +#include <linux/iio/buffer.h> > > +#include <linux/iio/trigger.h> > > +#include <linux/iio/trigger_consumer.h> > > +#include <linux/iio/triggered_buffer.h> > > Ditto. > > > + if (reg & (VCNL4035_INT_ALS_IF_H_MASK | VCNL4035_INT_ALS_IF_L_MASK)) > > + return true; > > + else > > + return false; > > return !!(reg & ...); ? > > > +static irqreturn_t vcnl4035_drdy_irq_thread(int irq, void *private) > > +{ > > + struct iio_dev *indio_dev = private; > > + struct vcnl4035_data *data = iio_priv(indio_dev); > > + > > > + if (vcnl4035_is_triggered(data)) { > > Here is negative conditional suits. > > > +#ifndef CONFIG_PM > > Why? > > > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > > + if (ret < 0) > > + return ret; > > +#endif > > > + pr_err("regmap_write default THDH returned %d\n", ret); > > dev_err() > > > + pr_err("regmap_write default THDL returned %d\n", ret); > > Ditto. > > > > + ret = pm_runtime_set_active(&client->dev); > > + if (ret < 0) > > + goto fail_poweroff; > > + > > + pm_runtime_enable(&client->dev); > > + pm_runtime_set_autosuspend_delay(&client->dev, VCNL4035_SLEEP_DELAY_MS); > > + pm_runtime_use_autosuspend(&client->dev); > > Usually we do this after we probed the device. I'm not sure we should. The docs are fairly clear on this: "In order to use autosuspend, subsystems or drivers must call pm_runtime_use_autosuspend() (preferably before registering the device), and thereafter they should use the various *_autosuspend() helper functions instead of the non-autosuspend counterparts:" (Documentation/runtime_pm.txt) It also makes more logical sense to have removed the userspace interfaces before unwinding this as then we don't have annoying races around the inevitable set_suspended. > > > + if (client->irq) { > > First of all, it can be negative. > Second, care to factor out this rather long branch to a separate function? > > > + if (ret < 0) { > > + dev_err(&client->dev, "request irq %d for trigger0 failed\n", > > + client->irq); > > + goto fail_buffer_clean; > > + } > > Indentation. > > > + } > > So, do I understand correctly that IRQ is optional for this device? > > > > > +#ifdef CONFIG_PM > > +static int vcnl4035_runtime_suspend(struct device *dev) > > Perhaps __maybe_unused > > > +static int vcnl4035_runtime_resume(struct device *dev) > > Ditto. > > > +{ > > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > > + struct vcnl4035_data *data = iio_priv(indio_dev); > > + int ret; > > + > > + regcache_sync(data->regmap); > > + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE); > > + if (ret < 0) > > + return ret; > > + blank line > > > + /* wait for 1 ALS integration cycle */ > > + msleep(data->als_it_val * 100); > > + > > + return 0; > > +} > > +#endif > > > +static const struct of_device_id vcnl4035_of_match[] = { > > + { .compatible = "vishay,vcnl4035", }, > > + { }, > > Better w/o comma. > > > +}; > > +MODULE_DEVICE_TABLE(of, vcnl4035_of_match); > > > + > > +static const struct i2c_device_id vcnl4035_id[] = { > > + { "vcnl4035", 0 }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, vcnl4035_id); > > Are you expecting legacy code to use this? I would rather switch to > ->probe_new() and also remove this. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] iio: light: Add device tree binding for vishay vcnl4035 2018-08-02 18:27 [PATCH v4 1/3] iio: Add modifier for white light Parthiban Nallathambi 2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi @ 2018-08-02 18:27 ` Parthiban Nallathambi 2018-08-02 19:30 ` [PATCH v4 1/3] iio: Add modifier for white light Peter Meerwald-Stadler 2 siblings, 0 replies; 10+ messages in thread From: Parthiban Nallathambi @ 2018-08-02 18:27 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw, robh+dt Cc: linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs, Parthiban Nallathambi Adding device tree binding for vcnl4035 and vendor prefix for Vishay Intertechnology Signed-off-by: Parthiban Nallathambi <pn@denx.de> Changelog in v3: - removed interrupt-parent property reference in documentation - renamed vcnl4035 to light-sensor Changelog in v4: - commit message fix - same indexing/space in binding --- .../devicetree/bindings/iio/light/vcnl4035.txt | 18 ++++++++++++++++++ Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 2 files changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/vcnl4035.txt diff --git a/Documentation/devicetree/bindings/iio/light/vcnl4035.txt b/Documentation/devicetree/bindings/iio/light/vcnl4035.txt new file mode 100644 index 000000000000..c07c7f052556 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/light/vcnl4035.txt @@ -0,0 +1,18 @@ +VISHAY VCNL4035 - Ambient Light and proximity sensor + +Link to datasheet: https://www.vishay.com/docs/84251/vcnl4035x01.pdf + +Required properties: + + -compatible: should be "vishay,vcnl4035" + -reg: I2C address of the sensor, should be 0x60 + -interrupts: interrupt mapping for GPIO IRQ (level active low) + +Example: + +light-sensor@60 { + compatible = "vishay,vcnl4035"; + reg = <0x60>; + interrupt-parent = <&gpio4>; + interrupts = <11 IRQ_TYPE_LEVEL_LOW>; +}; diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 7cad066191ee..3cc46d5735a9 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -395,6 +395,7 @@ v3 V3 Semiconductor variscite Variscite Ltd. via VIA Technologies, Inc. virtio Virtual I/O Device Specification, developed by the OASIS consortium +vishay Vishay Intertechnology, Inc vivante Vivante Corporation vocore VoCore Studio voipac Voipac Technologies s.r.o. -- 2.14.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] iio: Add modifier for white light 2018-08-02 18:27 [PATCH v4 1/3] iio: Add modifier for white light Parthiban Nallathambi 2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi 2018-08-02 18:27 ` [PATCH v4 3/3] iio: light: Add device tree binding " Parthiban Nallathambi @ 2018-08-02 19:30 ` Peter Meerwald-Stadler 2018-08-03 10:44 ` Parthiban Nallathambi 2 siblings, 1 reply; 10+ messages in thread From: Peter Meerwald-Stadler @ 2018-08-02 19:30 UTC (permalink / raw) To: Parthiban Nallathambi Cc: jic23, knaack.h, lars, robh+dt, linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs Hello, it is not clear to me why 'white' is needed; isn't that the default, i.e. unfiltered light? thanks, regards, p. > Signed-off-by: Parthiban Nallathambi <pn@denx.de> > --- > Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ > drivers/iio/industrialio-core.c | 1 + > include/uapi/linux/iio/types.h | 1 + > tools/iio/iio_event_monitor.c | 2 ++ > 4 files changed, 11 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > index 731146c3b138..43e481aed5b2 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -1312,6 +1312,13 @@ Description: > standardised CIE Erythemal Action Spectrum. UV index values range > from 0 (low) to >=11 (extreme). > > +What: /sys/.../iio:deviceX/in_intensityY_white_raw > +KernelVersion: 4.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + Modifier white indicates that measurements contain white LED > + component. > + > What: /sys/.../iio:deviceX/in_intensity_red_integration_time > What: /sys/.../iio:deviceX/in_intensity_green_integration_time > What: /sys/.../iio:deviceX/in_intensity_blue_integration_time > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 19bdf3d2962a..cb939b9fad16 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -108,6 +108,7 @@ static const char * const iio_modifier_names[] = { > [IIO_MOD_LIGHT_GREEN] = "green", > [IIO_MOD_LIGHT_BLUE] = "blue", > [IIO_MOD_LIGHT_UV] = "uv", > + [IIO_MOD_LIGHT_WHITE] = "white", > [IIO_MOD_QUATERNION] = "quaternion", > [IIO_MOD_TEMP_AMBIENT] = "ambient", > [IIO_MOD_TEMP_OBJECT] = "object", > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > index 4213cdf88e3c..de87a6c7e6de 100644 > --- a/include/uapi/linux/iio/types.h > +++ b/include/uapi/linux/iio/types.h > @@ -84,6 +84,7 @@ enum iio_modifier { > IIO_MOD_CO2, > IIO_MOD_VOC, > IIO_MOD_LIGHT_UV, > + IIO_MOD_LIGHT_WHITE, > }; > > enum iio_event_type { > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > index b61245e1181d..a2f9c62a79dd 100644 > --- a/tools/iio/iio_event_monitor.c > +++ b/tools/iio/iio_event_monitor.c > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = { > [IIO_MOD_LIGHT_GREEN] = "green", > [IIO_MOD_LIGHT_BLUE] = "blue", > [IIO_MOD_LIGHT_UV] = "uv", > + [IIO_MOD_LIGHT_WHITE] = "white", > [IIO_MOD_QUATERNION] = "quaternion", > [IIO_MOD_TEMP_AMBIENT] = "ambient", > [IIO_MOD_TEMP_OBJECT] = "object", > @@ -178,6 +179,7 @@ static bool event_is_known(struct iio_event_data *event) > case IIO_MOD_LIGHT_GREEN: > case IIO_MOD_LIGHT_BLUE: > case IIO_MOD_LIGHT_UV: > + case IIO_MOD_LIGHT_WHITE: > case IIO_MOD_QUATERNION: > case IIO_MOD_TEMP_AMBIENT: > case IIO_MOD_TEMP_OBJECT: > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] iio: Add modifier for white light 2018-08-02 19:30 ` [PATCH v4 1/3] iio: Add modifier for white light Peter Meerwald-Stadler @ 2018-08-03 10:44 ` Parthiban Nallathambi 2018-08-03 11:38 ` Peter Meerwald-Stadler 0 siblings, 1 reply; 10+ messages in thread From: Parthiban Nallathambi @ 2018-08-03 10:44 UTC (permalink / raw) To: Peter Meerwald-Stadler Cc: pn, jic23, knaack.h, lars, robh+dt, linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs Hello Peter, On 08/02/2018 09:30 PM, Peter Meerwald-Stadler wrote: > Hello, > > it is not clear to me why 'white' is needed; > isn't that the default, i.e. unfiltered light? Yes, it is. But devices like vcnl4035 veml7700, White LED data one register and all other sources of light (like fluorescent, incandescent ,sunlight) in separate register. So in such cases this WHITE modifier is needed. Should it needs to come under IIO_MOD_LIGHT_CLEAR? > > thanks, regards, p. > >> Signed-off-by: Parthiban Nallathambi <pn@denx.de> >> --- >> Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ >> drivers/iio/industrialio-core.c | 1 + >> include/uapi/linux/iio/types.h | 1 + >> tools/iio/iio_event_monitor.c | 2 ++ >> 4 files changed, 11 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio >> index 731146c3b138..43e481aed5b2 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-iio >> +++ b/Documentation/ABI/testing/sysfs-bus-iio >> @@ -1312,6 +1312,13 @@ Description: >> standardised CIE Erythemal Action Spectrum. UV index values range >> from 0 (low) to >=11 (extreme). >> >> +What: /sys/.../iio:deviceX/in_intensityY_white_raw >> +KernelVersion: 4.18 >> +Contact: linux-iio@vger.kernel.org >> +Description: >> + Modifier white indicates that measurements contain white LED >> + component. >> + >> What: /sys/.../iio:deviceX/in_intensity_red_integration_time >> What: /sys/.../iio:deviceX/in_intensity_green_integration_time >> What: /sys/.../iio:deviceX/in_intensity_blue_integration_time >> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c >> index 19bdf3d2962a..cb939b9fad16 100644 >> --- a/drivers/iio/industrialio-core.c >> +++ b/drivers/iio/industrialio-core.c >> @@ -108,6 +108,7 @@ static const char * const iio_modifier_names[] = { >> [IIO_MOD_LIGHT_GREEN] = "green", >> [IIO_MOD_LIGHT_BLUE] = "blue", >> [IIO_MOD_LIGHT_UV] = "uv", >> + [IIO_MOD_LIGHT_WHITE] = "white", >> [IIO_MOD_QUATERNION] = "quaternion", >> [IIO_MOD_TEMP_AMBIENT] = "ambient", >> [IIO_MOD_TEMP_OBJECT] = "object", >> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h >> index 4213cdf88e3c..de87a6c7e6de 100644 >> --- a/include/uapi/linux/iio/types.h >> +++ b/include/uapi/linux/iio/types.h >> @@ -84,6 +84,7 @@ enum iio_modifier { >> IIO_MOD_CO2, >> IIO_MOD_VOC, >> IIO_MOD_LIGHT_UV, >> + IIO_MOD_LIGHT_WHITE, >> }; >> >> enum iio_event_type { >> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c >> index b61245e1181d..a2f9c62a79dd 100644 >> --- a/tools/iio/iio_event_monitor.c >> +++ b/tools/iio/iio_event_monitor.c >> @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = { >> [IIO_MOD_LIGHT_GREEN] = "green", >> [IIO_MOD_LIGHT_BLUE] = "blue", >> [IIO_MOD_LIGHT_UV] = "uv", >> + [IIO_MOD_LIGHT_WHITE] = "white", >> [IIO_MOD_QUATERNION] = "quaternion", >> [IIO_MOD_TEMP_AMBIENT] = "ambient", >> [IIO_MOD_TEMP_OBJECT] = "object", >> @@ -178,6 +179,7 @@ static bool event_is_known(struct iio_event_data *event) >> case IIO_MOD_LIGHT_GREEN: >> case IIO_MOD_LIGHT_BLUE: >> case IIO_MOD_LIGHT_UV: >> + case IIO_MOD_LIGHT_WHITE: >> case IIO_MOD_QUATERNION: >> case IIO_MOD_TEMP_AMBIENT: >> case IIO_MOD_TEMP_OBJECT: >> > -- Thanks, Parthiban Nallathambi DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] iio: Add modifier for white light 2018-08-03 10:44 ` Parthiban Nallathambi @ 2018-08-03 11:38 ` Peter Meerwald-Stadler 2018-08-03 20:34 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Peter Meerwald-Stadler @ 2018-08-03 11:38 UTC (permalink / raw) To: Parthiban Nallathambi Cc: jic23, knaack.h, lars, robh+dt, linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs Hello, > > it is not clear to me why 'white' is needed; > > isn't that the default, i.e. unfiltered light? > > Yes, it is. But devices like vcnl4035 veml7700, White LED data one > register and all other sources of light (like fluorescent, > incandescent ,sunlight) in separate register. > > So in such cases this WHITE modifier is needed. Should it needs to > come under IIO_MOD_LIGHT_CLEAR? it is a mess already :), there is _intensity _intensity_ir _intensity_both _intensity_uv _intensity_red _intensity_green _intensity_blue _intensity_clear I think that ir, uv, red, green, blue are filters for particular ranges of the spectrum _intensity_clear might be unfiltered and _intensity might relate to the human eye photopic curve, I think the proposed white might be the same as clear? 'both' means ir + _intensity (not clearly specified) regards, p. > > > +KernelVersion: 4.18 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Modifier white indicates that measurements contain white LED > > > + component. > > > + > > > What: /sys/.../iio:deviceX/in_intensity_red_integration_time > > > What: > > > /sys/.../iio:deviceX/in_intensity_green_integration_time > > > What: > > > /sys/.../iio:deviceX/in_intensity_blue_integration_time > > > diff --git a/drivers/iio/industrialio-core.c > > > b/drivers/iio/industrialio-core.c > > > index 19bdf3d2962a..cb939b9fad16 100644 > > > --- a/drivers/iio/industrialio-core.c > > > +++ b/drivers/iio/industrialio-core.c > > > @@ -108,6 +108,7 @@ static const char * const iio_modifier_names[] = { > > > [IIO_MOD_LIGHT_GREEN] = "green", > > > [IIO_MOD_LIGHT_BLUE] = "blue", > > > [IIO_MOD_LIGHT_UV] = "uv", > > > + [IIO_MOD_LIGHT_WHITE] = "white", > > > [IIO_MOD_QUATERNION] = "quaternion", > > > [IIO_MOD_TEMP_AMBIENT] = "ambient", > > > [IIO_MOD_TEMP_OBJECT] = "object", > > > diff --git a/include/uapi/linux/iio/types.h > > > b/include/uapi/linux/iio/types.h > > > index 4213cdf88e3c..de87a6c7e6de 100644 > > > --- a/include/uapi/linux/iio/types.h > > > +++ b/include/uapi/linux/iio/types.h > > > @@ -84,6 +84,7 @@ enum iio_modifier { > > > IIO_MOD_CO2, > > > IIO_MOD_VOC, > > > IIO_MOD_LIGHT_UV, > > > + IIO_MOD_LIGHT_WHITE, > > > }; > > > enum iio_event_type { > > > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > > > index b61245e1181d..a2f9c62a79dd 100644 > > > --- a/tools/iio/iio_event_monitor.c > > > +++ b/tools/iio/iio_event_monitor.c > > > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = { > > > [IIO_MOD_LIGHT_GREEN] = "green", > > > [IIO_MOD_LIGHT_BLUE] = "blue", > > > [IIO_MOD_LIGHT_UV] = "uv", > > > + [IIO_MOD_LIGHT_WHITE] = "white", > > > [IIO_MOD_QUATERNION] = "quaternion", > > > [IIO_MOD_TEMP_AMBIENT] = "ambient", > > > [IIO_MOD_TEMP_OBJECT] = "object", > > > @@ -178,6 +179,7 @@ static bool event_is_known(struct iio_event_data > > > *event) > > > case IIO_MOD_LIGHT_GREEN: > > > case IIO_MOD_LIGHT_BLUE: > > > case IIO_MOD_LIGHT_UV: > > > + case IIO_MOD_LIGHT_WHITE: > > > case IIO_MOD_QUATERNION: > > > case IIO_MOD_TEMP_AMBIENT: > > > case IIO_MOD_TEMP_OBJECT: > > > > > > > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] iio: Add modifier for white light 2018-08-03 11:38 ` Peter Meerwald-Stadler @ 2018-08-03 20:34 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2018-08-03 20:34 UTC (permalink / raw) To: Peter Meerwald-Stadler Cc: Parthiban Nallathambi, knaack.h, lars, robh+dt, linux-iio, linux-kernel, mark.rutland, devicetree, matthias.bgg, wd, sbabic, hs On Fri, 3 Aug 2018 13:38:17 +0200 (CEST) Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote: > Hello, > > > > it is not clear to me why 'white' is needed; > > > isn't that the default, i.e. unfiltered light? > > > > Yes, it is. But devices like vcnl4035 veml7700, White LED data one > > register and all other sources of light (like fluorescent, > > incandescent ,sunlight) in separate register. > > > > So in such cases this WHITE modifier is needed. Should it needs to > > come under IIO_MOD_LIGHT_CLEAR? This wins the aware for odd. The sensor is drawn as only have two sensing elements, one for Ambient light sensing and one for proximity. A particular award should be given for using a different axis for the white channel in which normalized output is decimal vs all the others where it is a percentage ;) Looking closely at those graphs I think this is actually a standard sensor combination job. So 'white' is what we have always called _both in that it reads both visible and infrared. The other sensor is infrared. Ambient is illuminance not intensity (approximate human eye response). The only dubious bit about this is that I can see how they managed to cancel out some of the signal around 700nm as there is nothing much on the infrared and lots on the white. Alternative is that they actually have a filter that really approximates the human eye and the white channel is the sum of the other two? > > it is a mess already :), there is > _intensity > _intensity_ir > _intensity_both > _intensity_uv > _intensity_red > _intensity_green > _intensity_blue > _intensity_clear > > I think that ir, uv, red, green, blue are filters for particular ranges > of the spectrum That is the intent certainly. > > _intensity_clear might be unfiltered and _intensity might relate to the > human eye photopic curve, I think the proposed white might be the same as > clear? > > 'both' means ir + _intensity (not clearly specified) Yup, that's the power of initial assumptions. There was a time when the only light sensors you tended to get were ir and both ir and visible. Visible was obtained by taking one from the other. One of those things we'd change if doing it again but it's a bit late now.. Jonathan > > regards, p. > > > > > +KernelVersion: 4.18 > > > > +Contact: linux-iio@vger.kernel.org > > > > +Description: > > > > + Modifier white indicates that measurements contain white LED > > > > + component. > > > > + > > > > What: /sys/.../iio:deviceX/in_intensity_red_integration_time > > > > What: > > > > /sys/.../iio:deviceX/in_intensity_green_integration_time > > > > What: > > > > /sys/.../iio:deviceX/in_intensity_blue_integration_time > > > > diff --git a/drivers/iio/industrialio-core.c > > > > b/drivers/iio/industrialio-core.c > > > > index 19bdf3d2962a..cb939b9fad16 100644 > > > > --- a/drivers/iio/industrialio-core.c > > > > +++ b/drivers/iio/industrialio-core.c > > > > @@ -108,6 +108,7 @@ static const char * const iio_modifier_names[] = { > > > > [IIO_MOD_LIGHT_GREEN] = "green", > > > > [IIO_MOD_LIGHT_BLUE] = "blue", > > > > [IIO_MOD_LIGHT_UV] = "uv", > > > > + [IIO_MOD_LIGHT_WHITE] = "white", > > > > [IIO_MOD_QUATERNION] = "quaternion", > > > > [IIO_MOD_TEMP_AMBIENT] = "ambient", > > > > [IIO_MOD_TEMP_OBJECT] = "object", > > > > diff --git a/include/uapi/linux/iio/types.h > > > > b/include/uapi/linux/iio/types.h > > > > index 4213cdf88e3c..de87a6c7e6de 100644 > > > > --- a/include/uapi/linux/iio/types.h > > > > +++ b/include/uapi/linux/iio/types.h > > > > @@ -84,6 +84,7 @@ enum iio_modifier { > > > > IIO_MOD_CO2, > > > > IIO_MOD_VOC, > > > > IIO_MOD_LIGHT_UV, > > > > + IIO_MOD_LIGHT_WHITE, > > > > }; > > > > enum iio_event_type { > > > > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > > > > index b61245e1181d..a2f9c62a79dd 100644 > > > > --- a/tools/iio/iio_event_monitor.c > > > > +++ b/tools/iio/iio_event_monitor.c > > > > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = { > > > > [IIO_MOD_LIGHT_GREEN] = "green", > > > > [IIO_MOD_LIGHT_BLUE] = "blue", > > > > [IIO_MOD_LIGHT_UV] = "uv", > > > > + [IIO_MOD_LIGHT_WHITE] = "white", > > > > [IIO_MOD_QUATERNION] = "quaternion", > > > > [IIO_MOD_TEMP_AMBIENT] = "ambient", > > > > [IIO_MOD_TEMP_OBJECT] = "object", > > > > @@ -178,6 +179,7 @@ static bool event_is_known(struct iio_event_data > > > > *event) > > > > case IIO_MOD_LIGHT_GREEN: > > > > case IIO_MOD_LIGHT_BLUE: > > > > case IIO_MOD_LIGHT_UV: > > > > + case IIO_MOD_LIGHT_WHITE: > > > > case IIO_MOD_QUATERNION: > > > > case IIO_MOD_TEMP_AMBIENT: > > > > case IIO_MOD_TEMP_OBJECT: > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-08-03 20:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-02 18:27 [PATCH v4 1/3] iio: Add modifier for white light Parthiban Nallathambi 2018-08-02 18:27 ` [PATCH v4 2/3] iio: light: Add support for vishay vcnl4035 Parthiban Nallathambi 2018-08-02 19:29 ` Peter Meerwald-Stadler 2018-08-02 19:34 ` Andy Shevchenko 2018-08-03 20:55 ` Jonathan Cameron 2018-08-02 18:27 ` [PATCH v4 3/3] iio: light: Add device tree binding " Parthiban Nallathambi 2018-08-02 19:30 ` [PATCH v4 1/3] iio: Add modifier for white light Peter Meerwald-Stadler 2018-08-03 10:44 ` Parthiban Nallathambi 2018-08-03 11:38 ` Peter Meerwald-Stadler 2018-08-03 20:34 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).