2020-04-06 14:54:46

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v2 0/4] iio: vcnl: Add interrupts support for VCNL4010/20.

Hello,

Thanks for reviewing Jonathan! I did split the original patch into four
parts. I should have done it in the first place, sorry about that.

I did drop the ALS data from the buffer, as they are not really useful
anyway. I also used iio_device_claim_direct_mode as requested. But making sure
buffer and event capture are not done simultaneously is quite tricky as you
mentionned.

Thanks,

Mathieu

Changes from v1:
* Split into four different patches.
* Use iio_device_claim_direct_mode to protect
raw access from buffer capture.
* Requesting a sampling frequency above the limit is no longer possible.
* Inline read_isr and write_isr functions.
* Remove IIO_LIGHT data from buffer capture.
* Make sure postenable and predisable functions respect the common form.
* Do not set the trigger by default.
* Remove the devm_iio_triggered_buffer_setup top half.

Mathieu Othacehe (4):
iio: vcnl4000: Factorize data reading and writing.
iio: vcnl4000: Add event support for VCNL4010/20.
iio: vcnl4000: Add sampling frequency support for VCNL4010/20.
iio: vcnl4000: Add buffer support for VCNL4010/20.

drivers/iio/light/Kconfig | 2 +
drivers/iio/light/vcnl4000.c | 856 +++++++++++++++++++++++++++++++++--
2 files changed, 813 insertions(+), 45 deletions(-)

--
2.26.0


2020-04-06 14:54:48

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v2 1/4] iio: vcnl4000: Factorize data reading and writing.

Factorize data reading in vcnl4000_measure into a vcnl4000_read_block_data
function. Use it to provide a vcnl4000_read_data function that is able to
read sensor data under lock. Also add a vcnl4000_write_data function.

Signed-off-by: Mathieu Othacehe <[email protected]>
---
drivers/iio/light/vcnl4000.c | 54 +++++++++++++++++++++++++++++++++---
1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index e5b00a6611ac..c95b89740dfa 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -189,11 +189,59 @@ static int vcnl4200_init(struct vcnl4000_data *data)
return 0;
};

+static int vcnl4000_read_block_data(struct vcnl4000_data *data, u8 data_reg,
+ int *val)
+{
+ __be16 buf;
+ int ret;
+
+ ret = i2c_smbus_read_i2c_block_data(data->client,
+ data_reg, sizeof(buf), (u8 *) &buf);
+ if (ret < 0)
+ goto end;
+
+ *val = be16_to_cpu(buf);
+end:
+ return ret;
+}
+
+static int vcnl4000_read_data(struct vcnl4000_data *data, u8 data_reg,
+ int *val)
+{
+ int ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+ ret = vcnl4000_read_block_data(data, data_reg, val);
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
+static int vcnl4000_write_data(struct vcnl4000_data *data, u8 data_reg,
+ u16 val)
+{
+ int ret;
+
+ if (val > U16_MAX)
+ return -ERANGE;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_write_byte_data(data->client, data_reg, val >> 8);
+ if (ret < 0)
+ goto end;
+
+ ret = i2c_smbus_write_byte_data(data->client, data_reg + 1, val);
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
u8 rdy_mask, u8 data_reg, int *val)
{
int tries = 20;
- __be16 buf;
int ret;

mutex_lock(&data->vcnl4000_lock);
@@ -220,13 +268,11 @@ static int vcnl4000_measure(struct vcnl4000_data *data, u8 req_mask,
goto fail;
}

- ret = i2c_smbus_read_i2c_block_data(data->client,
- data_reg, sizeof(buf), (u8 *) &buf);
+ ret = vcnl4000_read_block_data(data, data_reg, val);
if (ret < 0)
goto fail;

mutex_unlock(&data->vcnl4000_lock);
- *val = be16_to_cpu(buf);

return 0;

--
2.26.0

2020-04-06 14:55:31

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v2 3/4] iio: vcnl4000: Add sampling frequency support for VCNL4010/20.

Add sampling frequency support for ambient light and proximity data on
VCNL4010 and VCNL4020 chips.

Signed-off-by: Mathieu Othacehe <[email protected]>
---
drivers/iio/light/vcnl4000.c | 191 ++++++++++++++++++++++++++++++++++-
1 file changed, 190 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index b0aaa4dd7a50..899abef137a2 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -87,6 +87,24 @@
#define VCNL4010_INT_DRDY \
(BIT(VCNL4010_INT_PROXIMITY) | BIT(VCNL4010_INT_ALS))

+#define VCNL4010_ALS_SAMPLING_FREQUENCY_AVAILABLE \
+ "1 2 3 4 5 6 8 10"
+#define VCNL4010_PROXIMITY_SAMPLING_FREQUENCY_AVAILABLE \
+ "1.95 3.90 7.81 16.62 31.25 62.5 125 250"
+
+static const int vcnl4010_als_sampling_frequency[] = {
+ 1, 2, 3, 4, 5, 6, 8, 10};
+
+static const int vcnl4010_prox_sampling_frequency[][2] = {
+ {1, 950000},
+ {3, 906250},
+ {7, 812500},
+ {16, 625000},
+ {31, 250000},
+ {62, 500000},
+ {125, 0},
+ {250, 0}
+};

enum vcnl4000_device_ids {
VCNL4000,
@@ -367,6 +385,50 @@ static int vcnl4200_measure_proximity(struct vcnl4000_data *data, int *val)
return vcnl4200_measure(data, &data->vcnl4200_ps, val);
}

+static int vcnl4010_read_proxy_samp_freq(struct vcnl4000_data *data, int *val,
+ int *val2)
+{
+ int ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_PROX_RATE);
+ if (ret < 0)
+ goto end;
+
+ if (ret >= ARRAY_SIZE(vcnl4010_prox_sampling_frequency))
+ return -EINVAL;
+
+ *val = vcnl4010_prox_sampling_frequency[ret][0];
+ *val2 = vcnl4010_prox_sampling_frequency[ret][1];
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
+static int vcnl4010_read_als_samp_freq(struct vcnl4000_data *data, int *val)
+{
+ int ret;
+ int index, mask = 0x70;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ALS_PARAM);
+ if (ret < 0)
+ goto end;
+
+ index = (ret & mask) >> 4;
+ if (index < 0 || index >= ARRAY_SIZE(vcnl4010_als_sampling_frequency))
+ return -EINVAL;
+
+ *val = vcnl4010_als_sampling_frequency[index];
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
static int vcnl4010_in_periodic_mode(struct vcnl4000_data *data)
{
int ret;
@@ -444,11 +506,119 @@ static int vcnl4010_read_raw(struct iio_dev *indio_dev,

iio_device_release_direct_mode(indio_dev);
return ret;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ switch (chan->type) {
+ case IIO_PROXIMITY:
+ ret = vcnl4010_read_proxy_samp_freq(data, val, val2);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_LIGHT:
+ ret = vcnl4010_read_als_samp_freq(data, val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
default:
return -EINVAL;
}
}

+static int vcnl4010_write_proxy_samp_freq(struct vcnl4000_data *data, int val,
+ int val2)
+{
+ int ret;
+ int i;
+ int len = ARRAY_SIZE(vcnl4010_prox_sampling_frequency);
+
+ for (i = 0; i < len; i++) {
+ if (val <= vcnl4010_prox_sampling_frequency[i][0])
+ break;
+ }
+
+ if (i == len)
+ return -EINVAL;
+
+ mutex_lock(&data->vcnl4000_lock);
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_PROX_RATE, i);
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
+static int vcnl4010_write_als_samp_freq(struct vcnl4000_data *data, int val)
+{
+ int ret;
+ int i;
+ int param;
+ int mask = 0x70;
+ int len = ARRAY_SIZE(vcnl4010_als_sampling_frequency);
+
+ for (i = 0; i < len; i++) {
+ if (val <= vcnl4010_als_sampling_frequency[i])
+ break;
+ }
+
+ if (i == len)
+ return -EINVAL;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ALS_PARAM);
+ if (ret < 0)
+ goto end;
+
+ param = (ret & ~mask) | (i << 4);
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_ALS_PARAM,
+ param);
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
+static int vcnl4010_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ int ret;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ /* Protect against event capture. */
+ if (vcnl4010_in_periodic_mode(data)) {
+ ret = -EBUSY;
+ goto end;
+ }
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ switch (chan->type) {
+ case IIO_PROXIMITY:
+ ret = vcnl4010_write_proxy_samp_freq(data, val, val2);
+ goto end;
+ case IIO_LIGHT:
+ ret = vcnl4010_write_als_samp_freq(data, val);
+ goto end;
+ default:
+ ret = -EINVAL;
+ goto end;
+ }
+ default:
+ ret = -EINVAL;
+ goto end;
+ }
+
+end:
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+}
+
static int vcnl4010_read_event(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
@@ -641,26 +811,45 @@ static const struct iio_chan_spec vcnl4010_channels[] = {
.type = IIO_LIGHT,
.scan_index = -1,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_SCALE),
}, {
.type = IIO_PROXIMITY,
.scan_index = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
.event_spec = vcnl4000_event_spec,
.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
},
};

+static IIO_CONST_ATTR(in_illuminance_sampling_frequency_available,
+ VCNL4010_ALS_SAMPLING_FREQUENCY_AVAILABLE);
+static IIO_CONST_ATTR(in_proximity_sampling_frequency_available,
+ VCNL4010_PROXIMITY_SAMPLING_FREQUENCY_AVAILABLE);
+
+static struct attribute *vcnl4010_attributes[] = {
+ &iio_const_attr_in_illuminance_sampling_frequency_available.dev_attr.attr,
+ &iio_const_attr_in_proximity_sampling_frequency_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group vcnl4010_attribute_group = {
+ .attrs = vcnl4010_attributes
+};
+
static const struct iio_info vcnl4000_info = {
.read_raw = vcnl4000_read_raw,
};

static const struct iio_info vcnl4010_info = {
.read_raw = vcnl4010_read_raw,
+ .write_raw = vcnl4010_write_raw,
.read_event_value = vcnl4010_read_event,
.write_event_value = vcnl4010_write_event,
.read_event_config = vcnl4010_read_event_config,
.write_event_config = vcnl4010_write_event_config,
+ .attrs = &vcnl4010_attribute_group,
};

static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
--
2.26.0

2020-04-06 14:55:41

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v2 2/4] iio: vcnl4000: Add event support for VCNL4010/20.

The VCNL4010 and VCNL4020 chips are able to raise interrupts on proximity
threshold events. Add support for threshold rising and falling events for
those two chips.

Signed-off-by: Mathieu Othacehe <[email protected]>
---
drivers/iio/light/vcnl4000.c | 435 +++++++++++++++++++++++++++++++----
1 file changed, 396 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index c95b89740dfa..b0aaa4dd7a50 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -22,7 +22,9 @@
#include <linux/i2c.h>
#include <linux/err.h>
#include <linux/delay.h>
+#include <linux/interrupt.h>

+#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>

@@ -43,6 +45,15 @@
#define VCNL4000_PS_MEAS_FREQ 0x89 /* Proximity test signal frequency */
#define VCNL4000_PS_MOD_ADJ 0x8a /* Proximity modulator timing adjustment */

+#define VCNL4010_PROX_RATE 0x82 /* Proximity rate */
+#define VCNL4010_ALS_PARAM 0x84 /* ALS rate */
+#define VCNL4010_INT_CTRL 0x89 /* Interrupt control */
+#define VCNL4010_LOW_THR_HI 0x8a /* Low threshold, MSB */
+#define VCNL4010_LOW_THR_LO 0x8b /* Low threshold, LSB */
+#define VCNL4010_HIGH_THR_HI 0x8c /* High threshold, MSB */
+#define VCNL4010_HIGH_THR_LO 0x8d /* High threshold, LSB */
+#define VCNL4010_ISR 0x8e /* Interrupt status */
+
#define VCNL4200_AL_CONF 0x00 /* Ambient light configuration */
#define VCNL4200_PS_CONF1 0x03 /* Proximity configuration */
#define VCNL4200_PS_DATA 0x08 /* Proximity data */
@@ -56,6 +67,26 @@
#define VCNL4000_PS_RDY BIT(5) /* proximity data ready? */
#define VCNL4000_AL_OD BIT(4) /* start on-demand ALS measurement */
#define VCNL4000_PS_OD BIT(3) /* start on-demand proximity measurement */
+#define VCNL4000_ALS_EN BIT(2) /* start ALS measurement */
+#define VCNL4000_PROX_EN BIT(1) /* start proximity measurement */
+#define VCNL4000_SELF_TIMED_EN BIT(0) /* start self-timed measurement */
+
+/* Bit masks for interrupt registers. */
+#define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */
+#define VCNL4010_INT_THR_EN BIT(1) /* Threshold interrupt type */
+#define VCNL4010_INT_ALS_EN BIT(2) /* Enable on ALS data ready */
+#define VCNL4010_INT_PROX_EN BIT(3) /* Enable on proximity data ready */
+
+#define VCNL4010_INT_THR_HIGH 0 /* High threshold exceeded */
+#define VCNL4010_INT_THR_LOW 1 /* Low threshold exceeded */
+#define VCNL4010_INT_ALS 2 /* ALS data ready */
+#define VCNL4010_INT_PROXIMITY 3 /* Proximity data ready */
+
+#define VCNL4010_INT_THR \
+ (BIT(VCNL4010_INT_THR_LOW) | BIT(VCNL4010_INT_THR_HIGH))
+#define VCNL4010_INT_DRDY \
+ (BIT(VCNL4010_INT_PROXIMITY) | BIT(VCNL4010_INT_ALS))
+

enum vcnl4000_device_ids {
VCNL4000,
@@ -84,6 +115,10 @@ struct vcnl4000_data {

struct vcnl4000_chip_spec {
const char *prod;
+ struct iio_chan_spec const *channels;
+ const int num_channels;
+ const struct iio_info *info;
+ bool irq_support;
int (*init)(struct vcnl4000_data *data);
int (*measure_light)(struct vcnl4000_data *data, int *val);
int (*measure_proximity)(struct vcnl4000_data *data, int *val);
@@ -332,43 +367,23 @@ static int vcnl4200_measure_proximity(struct vcnl4000_data *data, int *val)
return vcnl4200_measure(data, &data->vcnl4200_ps, val);
}

-static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
- [VCNL4000] = {
- .prod = "VCNL4000",
- .init = vcnl4000_init,
- .measure_light = vcnl4000_measure_light,
- .measure_proximity = vcnl4000_measure_proximity,
- },
- [VCNL4010] = {
- .prod = "VCNL4010/4020",
- .init = vcnl4000_init,
- .measure_light = vcnl4000_measure_light,
- .measure_proximity = vcnl4000_measure_proximity,
- },
- [VCNL4040] = {
- .prod = "VCNL4040",
- .init = vcnl4200_init,
- .measure_light = vcnl4200_measure_light,
- .measure_proximity = vcnl4200_measure_proximity,
- },
- [VCNL4200] = {
- .prod = "VCNL4200",
- .init = vcnl4200_init,
- .measure_light = vcnl4200_measure_light,
- .measure_proximity = vcnl4200_measure_proximity,
- },
-};
+static int vcnl4010_in_periodic_mode(struct vcnl4000_data *data)
+{
+ int ret;

-static const struct iio_chan_spec vcnl4000_channels[] = {
- {
- .type = IIO_LIGHT,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
- BIT(IIO_CHAN_INFO_SCALE),
- }, {
- .type = IIO_PROXIMITY,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
- }
-};
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
+ if (ret < 0)
+ goto end;
+
+ ret = (ret & VCNL4000_SELF_TIMED_EN) > 0;
+
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}

static int vcnl4000_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -405,10 +420,339 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
}
}

+static int vcnl4010_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ int ret;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_SCALE:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ /* Protect against event capture. */
+ if (vcnl4010_in_periodic_mode(data)) {
+ ret = -EBUSY;
+ } else {
+ ret = vcnl4000_read_raw(indio_dev, chan, val, val2,
+ mask);
+ }
+
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int vcnl4010_read_event(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)
+{
+ int ret;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ ret = vcnl4000_read_data(data, VCNL4010_HIGH_THR_HI,
+ val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_EV_DIR_FALLING:
+ ret = vcnl4000_read_data(data, VCNL4010_LOW_THR_HI,
+ val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int vcnl4010_write_event(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)
+{
+ int ret;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ ret = vcnl4000_write_data(data, VCNL4010_HIGH_THR_HI,
+ val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_EV_DIR_FALLING:
+ ret = vcnl4000_write_data(data, VCNL4010_LOW_THR_HI,
+ val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int vcnl4010_thr_enabled(struct vcnl4000_data *data)
+{
+ int ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_INT_CTRL);
+ if (ret < 0)
+ goto end;
+
+ ret = (ret & VCNL4010_INT_THR_EN) > 0;
+
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ return ret;
+}
+
+static int vcnl4010_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+
+ switch (chan->type) {
+ case IIO_PROXIMITY:
+ return vcnl4010_thr_enabled(data);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int vcnl4010_prox_threshold(struct iio_dev *indio_dev, int state)
+{
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ int ret;
+ int icr;
+ int command;
+
+ if (state) {
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ /* Enable periodic measurement of proximity data. */
+ command = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
+
+ /*
+ * Enable interrupts on threshold, for proximity data by
+ * default.
+ */
+ icr = VCNL4010_INT_THR_EN;
+ } else {
+ if (!vcnl4010_thr_enabled(data))
+ return 0;
+
+ command = 0;
+ icr = 0;
+ }
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND,
+ command);
+ if (ret < 0)
+ goto end;
+
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, icr);
+
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ if (state)
+ iio_device_release_direct_mode(indio_dev);
+
+ return ret;
+}
+
+static int vcnl4010_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
+{
+ switch (chan->type) {
+ case IIO_PROXIMITY:
+ return vcnl4010_prox_threshold(indio_dev, state);
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static const struct iio_event_spec vcnl4000_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_ENABLE),
+ }
+};
+
+static const struct iio_chan_spec vcnl4000_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ }, {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ }
+};
+
+static const struct iio_chan_spec vcnl4010_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .scan_index = -1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ }, {
+ .type = IIO_PROXIMITY,
+ .scan_index = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .event_spec = vcnl4000_event_spec,
+ .num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
+ },
+};
+
static const struct iio_info vcnl4000_info = {
.read_raw = vcnl4000_read_raw,
};

+static const struct iio_info vcnl4010_info = {
+ .read_raw = vcnl4010_read_raw,
+ .read_event_value = vcnl4010_read_event,
+ .write_event_value = vcnl4010_write_event,
+ .read_event_config = vcnl4010_read_event_config,
+ .write_event_config = vcnl4010_write_event_config,
+};
+
+static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
+ [VCNL4000] = {
+ .prod = "VCNL4000",
+ .init = vcnl4000_init,
+ .measure_light = vcnl4000_measure_light,
+ .measure_proximity = vcnl4000_measure_proximity,
+ .channels = vcnl4000_channels,
+ .num_channels = ARRAY_SIZE(vcnl4000_channels),
+ .info = &vcnl4000_info,
+ .irq_support = false,
+ },
+ [VCNL4010] = {
+ .prod = "VCNL4010/4020",
+ .init = vcnl4000_init,
+ .measure_light = vcnl4000_measure_light,
+ .measure_proximity = vcnl4000_measure_proximity,
+ .channels = vcnl4010_channels,
+ .num_channels = ARRAY_SIZE(vcnl4010_channels),
+ .info = &vcnl4010_info,
+ .irq_support = true,
+ },
+ [VCNL4040] = {
+ .prod = "VCNL4040",
+ .init = vcnl4200_init,
+ .measure_light = vcnl4200_measure_light,
+ .measure_proximity = vcnl4200_measure_proximity,
+ .channels = vcnl4000_channels,
+ .num_channels = ARRAY_SIZE(vcnl4000_channels),
+ .info = &vcnl4000_info,
+ .irq_support = false,
+ },
+ [VCNL4200] = {
+ .prod = "VCNL4200",
+ .init = vcnl4200_init,
+ .measure_light = vcnl4200_measure_light,
+ .measure_proximity = vcnl4200_measure_proximity,
+ .channels = vcnl4000_channels,
+ .num_channels = ARRAY_SIZE(vcnl4000_channels),
+ .info = &vcnl4000_info,
+ .irq_support = false,
+ },
+};
+
+static irqreturn_t vcnl4010_irq_thread(int irq, void *p)
+{
+ struct iio_dev *indio_dev = p;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ unsigned long isr;
+ int ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ISR);
+ mutex_unlock(&data->vcnl4000_lock);
+
+ if (ret < 0)
+ goto end;
+
+ isr = ret;
+
+ if (isr & VCNL4010_INT_THR) {
+ if (test_bit(VCNL4010_INT_THR_LOW, &isr)) {
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(
+ IIO_PROXIMITY,
+ 1,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ iio_get_time_ns(indio_dev));
+ }
+
+ if (test_bit(VCNL4010_INT_THR_HIGH, &isr)) {
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(
+ IIO_PROXIMITY,
+ 1,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ iio_get_time_ns(indio_dev));
+ }
+
+ mutex_lock(&data->vcnl4000_lock);
+ i2c_smbus_write_byte_data(data->client, VCNL4010_ISR,
+ isr & VCNL4010_INT_THR);
+ mutex_unlock(&data->vcnl4000_lock);
+ }
+
+end:
+ return IRQ_HANDLED;
+}
+
static int vcnl4000_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -434,12 +778,25 @@ static int vcnl4000_probe(struct i2c_client *client,
data->chip_spec->prod, data->rev);

indio_dev->dev.parent = &client->dev;
- indio_dev->info = &vcnl4000_info;
- indio_dev->channels = vcnl4000_channels;
- indio_dev->num_channels = ARRAY_SIZE(vcnl4000_channels);
+ indio_dev->info = data->chip_spec->info;
+ indio_dev->channels = data->chip_spec->channels;
+ indio_dev->num_channels = data->chip_spec->num_channels;
indio_dev->name = VCNL4000_DRV_NAME;
indio_dev->modes = INDIO_DIRECT_MODE;

+ if (client->irq && data->chip_spec->irq_support) {
+ ret = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, vcnl4010_irq_thread,
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "vncl4010_irq",
+ indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "irq request failed\n");
+ return ret;
+ }
+ }
+
return devm_iio_device_register(&client->dev, indio_dev);
}

--
2.26.0

2020-04-06 14:56:25

by Mathieu Othacehe

[permalink] [raw]
Subject: [PATCH v2 4/4] iio: vcnl4000: Add buffer support for VCNL4010/20.

The VCNL4010 and VCNL4020 chips are able to raise interrupts on data ready.
Use it to provide triggered buffer support for proximity data.

Those two chips also provide ambient light data. However, they are sampled
at different rate than proximity data. As this is not handled by the IIO
framework for now, and the sample frequencies of ambient light data are
very low, do add buffer support for them.

Signed-off-by: Mathieu Othacehe <[email protected]>
---
drivers/iio/light/Kconfig | 2 +
drivers/iio/light/vcnl4000.c | 178 ++++++++++++++++++++++++++++++++++-
2 files changed, 178 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 9968f982fbc7..0c066b800743 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -485,6 +485,8 @@ config US5182D

config VCNL4000
tristate "VCNL4000/4010/4020/4200 combined ALS and proximity sensor"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
depends on I2C
help
Say Y here if you want to build a driver for the Vishay VCNL4000,
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 899abef137a2..625d1a8114e5 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -5,6 +5,7 @@
*
* Copyright 2012 Peter Meerwald <[email protected]>
* Copyright 2019 Pursim SPC
+ * Copyright 2020 Mathieu Othacehe <[email protected]>
*
* IIO driver for:
* VCNL4000/10/20 (7-bit I2C slave address 0x13)
@@ -14,8 +15,7 @@
* TODO:
* allow to adjust IR current
* proximity threshold and event handling
- * periodic ALS/proximity measurement (VCNL4010/20)
- * interrupts (VCNL4010/20/40, VCNL4200)
+ * interrupts (VCNL4040, VCNL4200)
*/

#include <linux/module.h>
@@ -24,9 +24,13 @@
#include <linux/delay.h>
#include <linux/interrupt.h>

+#include <linux/iio/buffer.h>
#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>

#define VCNL4000_DRV_NAME "vcnl4000"
#define VCNL4000_PROD_ID 0x01
@@ -820,7 +824,14 @@ static const struct iio_chan_spec vcnl4010_channels[] = {
BIT(IIO_CHAN_INFO_SAMP_FREQ),
.event_spec = vcnl4000_event_spec,
.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(1),
};

static IIO_CONST_ATTR(in_illuminance_sampling_frequency_available,
@@ -938,10 +949,158 @@ static irqreturn_t vcnl4010_irq_thread(int irq, void *p)
mutex_unlock(&data->vcnl4000_lock);
}

+ if (isr & VCNL4010_INT_DRDY && iio_buffer_enabled(indio_dev))
+ iio_trigger_poll_chained(indio_dev->trig);
+
+end:
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ const unsigned long *active_scan_mask = indio_dev->active_scan_mask;
+ u16 buffer[8] = {0}; /* 1x16-bit + ts */
+ bool data_read = false;
+ unsigned long isr;
+ int val = 0;
+ int ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ISR);
+ mutex_unlock(&data->vcnl4000_lock);
+
+ if (ret < 0)
+ goto end;
+
+ isr = ret;
+
+ if (test_bit(0, active_scan_mask)) {
+ if (test_bit(VCNL4010_INT_PROXIMITY, &isr)) {
+ ret = vcnl4000_read_data(data,
+ VCNL4000_PS_RESULT_HI,
+ &val);
+ if (ret < 0)
+ goto end;
+
+ buffer[0] = val;
+ data_read = true;
+ }
+ }
+
+ mutex_lock(&data->vcnl4000_lock);
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_ISR,
+ isr & VCNL4010_INT_DRDY);
+ mutex_unlock(&data->vcnl4000_lock);
+
+ if (ret < 0 || !data_read)
+ goto end;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+ iio_get_time_ns(indio_dev));
+
end:
+ iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED;
}

+static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ int ret;
+ int cmd;
+
+ ret = iio_triggered_buffer_postenable(indio_dev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
+ if (ret < 0)
+ goto end;
+
+ /* Do not enable the buffer if we are already capturing events. */
+ if ((ret & VCNL4000_SELF_TIMED_EN) > 0) {
+ ret = -EBUSY;
+ goto end;
+ }
+
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
+ VCNL4010_INT_PROX_EN);
+ if (ret < 0)
+ goto end;
+
+ cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
+ if (ret < 0)
+ goto end;
+
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+ if (ret < 0)
+ iio_triggered_buffer_predisable(indio_dev);
+
+ return ret;
+}
+
+static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ int ret, ret_disable;
+
+ mutex_lock(&data->vcnl4000_lock);
+
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
+ if (ret < 0)
+ goto end;
+
+ ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
+
+end:
+ mutex_unlock(&data->vcnl4000_lock);
+
+ ret_disable = iio_triggered_buffer_predisable(indio_dev);
+ if (ret == 0)
+ ret = ret_disable;
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops vcnl4010_buffer_ops = {
+ .postenable = &vcnl4010_buffer_postenable,
+ .predisable = &vcnl4010_buffer_predisable,
+};
+
+static const struct iio_trigger_ops vcnl4010_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static int vcnl4010_probe_trigger(struct iio_dev *indio_dev)
+{
+ struct vcnl4000_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = data->client;
+ struct iio_trigger *trigger;
+ int ret;
+
+ trigger = devm_iio_trigger_alloc(&client->dev, "%s-dev%d",
+ indio_dev->name, indio_dev->id);
+ if (!trigger)
+ return -ENOMEM;
+
+ trigger->dev.parent = &client->dev;
+ trigger->ops = &vcnl4010_trigger_ops;
+ iio_trigger_set_drvdata(trigger, indio_dev);
+
+ ret = devm_iio_trigger_register(&client->dev, trigger);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int vcnl4000_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -974,6 +1133,16 @@ static int vcnl4000_probe(struct i2c_client *client,
indio_dev->modes = INDIO_DIRECT_MODE;

if (client->irq && data->chip_spec->irq_support) {
+ ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
+ NULL,
+ vcnl4010_trigger_handler,
+ &vcnl4010_buffer_ops);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "unable to setup iio triggered buffer\n");
+ return ret;
+ }
+
ret = devm_request_threaded_irq(&client->dev, client->irq,
NULL, vcnl4010_irq_thread,
IRQF_TRIGGER_FALLING |
@@ -984,6 +1153,10 @@ static int vcnl4000_probe(struct i2c_client *client,
dev_err(&client->dev, "irq request failed\n");
return ret;
}
+
+ ret = vcnl4010_probe_trigger(indio_dev);
+ if (ret < 0)
+ return ret;
}

return devm_iio_device_register(&client->dev, indio_dev);
@@ -1026,5 +1199,6 @@ static struct i2c_driver vcnl4000_driver = {
module_i2c_driver(vcnl4000_driver);

MODULE_AUTHOR("Peter Meerwald <[email protected]>");
+MODULE_AUTHOR("Mathieu Othacehe <[email protected]>");
MODULE_DESCRIPTION("Vishay VCNL4000 proximity/ambient light sensor driver");
MODULE_LICENSE("GPL");
--
2.26.0

2020-04-12 13:49:22

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: vcnl4000: Add buffer support for VCNL4010/20.

On Mon, 6 Apr 2020 16:53:56 +0200
Mathieu Othacehe <[email protected]> wrote:

> The VCNL4010 and VCNL4020 chips are able to raise interrupts on data ready.
> Use it to provide triggered buffer support for proximity data.
>
> Those two chips also provide ambient light data. However, they are sampled
> at different rate than proximity data. As this is not handled by the IIO
> framework for now, and the sample frequencies of ambient light data are
> very low, do add buffer support for them.
>
> Signed-off-by: Mathieu Othacehe <[email protected]>

Hi Mathieu,

One nitpick in here which I can fix whilst applying. The series as
a whole looks good to me.

On a more general note we probably have a merge conflict between this
series and Guido's one adding the near level property to the driver.

Based on a bit of arbitrary timing I'd like to apply Guido's first
then try and take yours on top. If there are any problems I'll
probably ask you to do the rebase at that point.

Guido's series is waiting for some other reviews to come in so
it may be a week or so.

Thanks,

Jonathan


> ---
> drivers/iio/light/Kconfig | 2 +
> drivers/iio/light/vcnl4000.c | 178 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 178 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 9968f982fbc7..0c066b800743 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -485,6 +485,8 @@ config US5182D
>
> config VCNL4000
> tristate "VCNL4000/4010/4020/4200 combined ALS and proximity sensor"
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> depends on I2C
> help
> Say Y here if you want to build a driver for the Vishay VCNL4000,
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 899abef137a2..625d1a8114e5 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -5,6 +5,7 @@
> *
> * Copyright 2012 Peter Meerwald <[email protected]>
> * Copyright 2019 Pursim SPC
> + * Copyright 2020 Mathieu Othacehe <[email protected]>
> *
> * IIO driver for:
> * VCNL4000/10/20 (7-bit I2C slave address 0x13)
> @@ -14,8 +15,7 @@
> * TODO:
> * allow to adjust IR current
> * proximity threshold and event handling
> - * periodic ALS/proximity measurement (VCNL4010/20)
> - * interrupts (VCNL4010/20/40, VCNL4200)
> + * interrupts (VCNL4040, VCNL4200)
> */
>
> #include <linux/module.h>
> @@ -24,9 +24,13 @@
> #include <linux/delay.h>
> #include <linux/interrupt.h>
>
> +#include <linux/iio/buffer.h>
> #include <linux/iio/events.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>
> #define VCNL4000_DRV_NAME "vcnl4000"
> #define VCNL4000_PROD_ID 0x01
> @@ -820,7 +824,14 @@ static const struct iio_chan_spec vcnl4010_channels[] = {
> BIT(IIO_CHAN_INFO_SAMP_FREQ),
> .event_spec = vcnl4000_event_spec,
> .num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_CPU,
> + },
> },
> + IIO_CHAN_SOFT_TIMESTAMP(1),
> };
>
> static IIO_CONST_ATTR(in_illuminance_sampling_frequency_available,
> @@ -938,10 +949,158 @@ static irqreturn_t vcnl4010_irq_thread(int irq, void *p)
> mutex_unlock(&data->vcnl4000_lock);
> }
>
> + if (isr & VCNL4010_INT_DRDY && iio_buffer_enabled(indio_dev))
> + iio_trigger_poll_chained(indio_dev->trig);
> +
> +end:
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t vcnl4010_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct vcnl4000_data *data = iio_priv(indio_dev);
> + const unsigned long *active_scan_mask = indio_dev->active_scan_mask;
> + u16 buffer[8] = {0}; /* 1x16-bit + ts */
> + bool data_read = false;
> + unsigned long isr;
> + int val = 0;
> + int ret;
> +
> + mutex_lock(&data->vcnl4000_lock);
> + ret = i2c_smbus_read_byte_data(data->client, VCNL4010_ISR);
> + mutex_unlock(&data->vcnl4000_lock);
> +
> + if (ret < 0)
> + goto end;
> +
> + isr = ret;
> +
> + if (test_bit(0, active_scan_mask)) {
> + if (test_bit(VCNL4010_INT_PROXIMITY, &isr)) {
> + ret = vcnl4000_read_data(data,
> + VCNL4000_PS_RESULT_HI,
> + &val);
> + if (ret < 0)
> + goto end;
> +
> + buffer[0] = val;
> + data_read = true;
> + }
> + }
> +
> + mutex_lock(&data->vcnl4000_lock);
> + ret = i2c_smbus_write_byte_data(data->client, VCNL4010_ISR,
> + isr & VCNL4010_INT_DRDY);
> + mutex_unlock(&data->vcnl4000_lock);
> +
> + if (ret < 0 || !data_read)
> + goto end;
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> + iio_get_time_ns(indio_dev));
> +
> end:
> + iio_trigger_notify_done(indio_dev->trig);
> return IRQ_HANDLED;
> }
>
> +static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct vcnl4000_data *data = iio_priv(indio_dev);
> + int ret;
> + int cmd;
> +
> + ret = iio_triggered_buffer_postenable(indio_dev);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&data->vcnl4000_lock);
> +
> + ret = i2c_smbus_read_byte_data(data->client, VCNL4000_COMMAND);
> + if (ret < 0)
> + goto end;
> +
> + /* Do not enable the buffer if we are already capturing events. */
> + if ((ret & VCNL4000_SELF_TIMED_EN) > 0) {
> + ret = -EBUSY;
> + goto end;
> + }
> +
> + ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
> + VCNL4010_INT_PROX_EN);
> + if (ret < 0)
> + goto end;
> +
> + cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
> + ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
> + if (ret < 0)
> + goto end;
> +
> +end:
> + mutex_unlock(&data->vcnl4000_lock);
> + if (ret < 0)
> + iio_triggered_buffer_predisable(indio_dev);
> +
> + return ret;
> +}
> +
> +static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct vcnl4000_data *data = iio_priv(indio_dev);
> + int ret, ret_disable;
> +
> + mutex_lock(&data->vcnl4000_lock);
> +
> + ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
> + if (ret < 0)
> + goto end;
> +
> + ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
> +
> +end:
> + mutex_unlock(&data->vcnl4000_lock);
> +
> + ret_disable = iio_triggered_buffer_predisable(indio_dev);
> + if (ret == 0)
> + ret = ret_disable;
> +
> + return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops vcnl4010_buffer_ops = {
> + .postenable = &vcnl4010_buffer_postenable,
> + .predisable = &vcnl4010_buffer_predisable,
> +};
> +
> +static const struct iio_trigger_ops vcnl4010_trigger_ops = {
> + .validate_device = iio_trigger_validate_own_device,
> +};
> +
> +static int vcnl4010_probe_trigger(struct iio_dev *indio_dev)
> +{
> + struct vcnl4000_data *data = iio_priv(indio_dev);
> + struct i2c_client *client = data->client;
> + struct iio_trigger *trigger;
> + int ret;
> +
> + trigger = devm_iio_trigger_alloc(&client->dev, "%s-dev%d",
> + indio_dev->name, indio_dev->id);
> + if (!trigger)
> + return -ENOMEM;
> +
> + trigger->dev.parent = &client->dev;
> + trigger->ops = &vcnl4010_trigger_ops;
> + iio_trigger_set_drvdata(trigger, indio_dev);
> +
> + ret = devm_iio_trigger_register(&client->dev, trigger);
> + if (ret)
> + return ret;

nitpick:
return devm_iio_trigger_register
does the same thing.

> +
> + return 0;
> +}
> +
> static int vcnl4000_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -974,6 +1133,16 @@ static int vcnl4000_probe(struct i2c_client *client,
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> if (client->irq && data->chip_spec->irq_support) {
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> + NULL,
> + vcnl4010_trigger_handler,
> + &vcnl4010_buffer_ops);
> + if (ret < 0) {
> + dev_err(&client->dev,
> + "unable to setup iio triggered buffer\n");
> + return ret;
> + }
> +
> ret = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, vcnl4010_irq_thread,
> IRQF_TRIGGER_FALLING |
> @@ -984,6 +1153,10 @@ static int vcnl4000_probe(struct i2c_client *client,
> dev_err(&client->dev, "irq request failed\n");
> return ret;
> }
> +
> + ret = vcnl4010_probe_trigger(indio_dev);
> + if (ret < 0)
> + return ret;
> }
>
> return devm_iio_device_register(&client->dev, indio_dev);
> @@ -1026,5 +1199,6 @@ static struct i2c_driver vcnl4000_driver = {
> module_i2c_driver(vcnl4000_driver);
>
> MODULE_AUTHOR("Peter Meerwald <[email protected]>");
> +MODULE_AUTHOR("Mathieu Othacehe <[email protected]>");
> MODULE_DESCRIPTION("Vishay VCNL4000 proximity/ambient light sensor driver");
> MODULE_LICENSE("GPL");

2020-04-14 16:23:31

by Mathieu Othacehe

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio: vcnl4000: Add buffer support for VCNL4010/20.


Hello Jonathan,

> One nitpick in here which I can fix whilst applying. The series as
> a whole looks good to me.
>
> On a more general note we probably have a merge conflict between this
> series and Guido's one adding the near level property to the driver.
>
> Based on a bit of arbitrary timing I'd like to apply Guido's first
> then try and take yours on top. If there are any problems I'll
> probably ask you to do the rebase at that point.

No problem :) There are indeed some conflicts to solve, I'll send an
updated revision when Guido's patch is integrated.

Thanks,

Mathieu