[PATCH v4 0/7] iio: light: vcnl4000: Add features for vncl4040/4200

[PATCH v4 0/7] iio: light: vcnl4000: Add features for vncl4040/4200

[Astrid Rost]

Add a more complete support for vncl4040 and vcnl4200, which allows to
change the distance of proximity detection and interrupt support for the
illuminance sensor.

Proximity functionality:
  - Interrupt support (new on vcnl4200).

Proximity reduce the amount of interrupts:
  - Adaptable integration time (new on vcnl4200) - the sampling rate
    changes according to this value.
  - Period - interrupt is asserted if the value is above or
    below a certain threshold.

Proximity change the activity distance:
  - Oversampling ratio - Amount of LED pulses per measured raw value.
  - Calibration bias - LED current calibration of the sensor.

Illuminance functionality:
  - Interrupt support.

Illuminance reduce the amount of interrupts:
  - Adaptable integration time - the sampling rate and scale changes
    according to this value.
  - Period – interrupt is asserted if the value is above or
    below a certain threshold.

changes v2:
- [PATCH v2 3/7] Fixed calculation of al_scale.
  Fix the value of vcnl4040 according to the data-sheet.
  Use div_u64 for the division.
scription for the branch

changes v3:
- [PATCH v3 1-3/7] Add differences between the chips as variables in
  chip-spec.
- [PATCH v3 4/7] Changed commit message.
- [PATCH v3 5/7] Use period instead of debounce time. This causes some
  calculations as the period is a time and the chip allows to set a certain
  amount of measurements above/below the threshold, before throwing an
  interrupt.
- [PATCH v3 6/7] Changed commit message.

changes v4:
- [PATCH v3 1-3/7] Fix setting correct als_it for vcnl4040.
- [PATCH v3 5/7] Use MICRO macro.
  Fix values greater than 1 s for the proximity period.

Astrid Rost (7):
  [PATCH v4 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200
  [PATCH v4 2/7] iio: light: vcnl4000: Add proximity ps_it for vcnl4200
  [PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200
  [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200
  [PATCH v4 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200
  [PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200
  [PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200

 drivers/iio/light/vcnl4000.c | 721 +++++++++++++++++++++++++++++++----
 1 file changed, 653 insertions(+), 68 deletions(-)

-- 
2.30.2

[PATCH v4 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200

[Astrid Rost]

Add proximity interrupt support for vcnl4200 (similar to vcnl4040).
Add support to configure proximity sensor interrupts and threshold
limits. If an interrupt is detected an event will be pushed to the
event interface.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 56d3963d3d66..565b9c9ea470 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -65,6 +65,7 @@
 #define VCNL4200_PS_DATA	0x08 /* Proximity data */
 #define VCNL4200_AL_DATA	0x09 /* Ambient light data */
 #define VCNL4040_INT_FLAGS	0x0b /* Interrupt register */
+#define VCNL4200_INT_FLAGS	0x0d /* Interrupt register */
 #define VCNL4200_DEV_ID		0x0e /* Device ID, slave address and version */
 
 #define VCNL4040_DEV_ID		0x0c /* Device ID and version */
@@ -164,6 +165,7 @@ struct vcnl4000_chip_spec {
 	int (*set_power_state)(struct vcnl4000_data *data, bool on);
 	irqreturn_t (*irq_thread)(int irq, void *priv);
 	irqreturn_t (*trig_buffer_func)(int irq, void *priv);
+	u8 int_reg;
 };
 
 static const struct i2c_device_id vcnl4000_id[] = {
@@ -1005,7 +1007,7 @@ static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4040_INT_FLAGS);
+	ret = i2c_smbus_read_word_data(data->client, data->chip_spec->int_reg);
 	if (ret < 0)
 		return IRQ_HANDLED;
 
@@ -1314,6 +1316,7 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.num_channels = ARRAY_SIZE(vcnl4040_channels),
 		.info = &vcnl4040_info,
 		.irq_thread = vcnl4040_irq_thread,
+		.int_reg = VCNL4040_INT_FLAGS,
 	},
 	[VCNL4200] = {
 		.prod = "VCNL4200",
@@ -1321,9 +1324,11 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.measure_light = vcnl4200_measure_light,
 		.measure_proximity = vcnl4200_measure_proximity,
 		.set_power_state = vcnl4200_set_power_state,
-		.channels = vcnl4000_channels,
+		.channels = vcnl4040_channels,
 		.num_channels = ARRAY_SIZE(vcnl4000_channels),
-		.info = &vcnl4000_info,
+		.info = &vcnl4040_info,
+		.irq_thread = vcnl4040_irq_thread,
+		.int_reg = VCNL4200_INT_FLAGS,
 	},
 };
 
-- 
2.30.2

[PATCH v4 2/7] iio: light: vcnl4000: Add proximity ps_it for vcnl4200

[Astrid Rost]

Add ps_it attributes for vcnl4200 (similar to vcnl4040).
Add read/write attribute for proximity integration time.
Read attribute for available proximity integration times.
Change sampling rate depending on integration time.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 565b9c9ea470..086a493fb6e5 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -124,6 +124,15 @@ static const int vcnl4040_ps_it_times[][2] = {
 	{0, 800},
 };
 
+static const int vcnl4200_ps_it_times[][2] = {
+	{0, 96},
+	{0, 144},
+	{0, 192},
+	{0, 384},
+	{0, 768},
+	{0, 864},
+};
+
 #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
 
 enum vcnl4000_device_ids {
@@ -166,6 +175,8 @@ struct vcnl4000_chip_spec {
 	irqreturn_t (*irq_thread)(int irq, void *priv);
 	irqreturn_t (*trig_buffer_func)(int irq, void *priv);
 	u8 int_reg;
+	const int(*ps_it_times)[][2];
+	const int num_ps_it_times;
 };
 
 static const struct i2c_device_id vcnl4000_id[] = {
@@ -508,11 +519,11 @@ static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2)
 
 	ret = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret);
 
-	if (ret >= ARRAY_SIZE(vcnl4040_ps_it_times))
+	if (ret >= data->chip_spec->num_ps_it_times)
 		return -EINVAL;
 
-	*val = vcnl4040_ps_it_times[ret][0];
-	*val2 = vcnl4040_ps_it_times[ret][1];
+	*val = (*data->chip_spec->ps_it_times)[ret][0];
+	*val2 = (*data->chip_spec->ps_it_times)[ret][1];
 
 	return 0;
 }
@@ -523,8 +534,8 @@ static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val)
 	int ret, index = -1;
 	u16 regval;
 
-	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_it_times); i++) {
-		if (val == vcnl4040_ps_it_times[i][1]) {
+	for (i = 0; i < data->chip_spec->num_ps_it_times; i++) {
+		if (val == (*data->chip_spec->ps_it_times)[i][1]) {
 			index = i;
 			break;
 		}
@@ -533,6 +544,8 @@ static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val)
 	if (index < 0)
 		return -EINVAL;
 
+	data->vcnl4200_ps.sampling_rate = ktime_set(0, val * 60000);
+
 	mutex_lock(&data->vcnl4000_lock);
 
 	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
@@ -620,11 +633,13 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
 			       const int **vals, int *type, int *length,
 			       long mask)
 {
+	struct vcnl4000_data *data = iio_priv(indio_dev);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_INT_TIME:
-		*vals = (int *)vcnl4040_ps_it_times;
+		*vals = (int *)(*data->chip_spec->ps_it_times);
 		*type = IIO_VAL_INT_PLUS_MICRO;
-		*length = 2 * ARRAY_SIZE(vcnl4040_ps_it_times);
+		*length = 2 * data->chip_spec->num_ps_it_times;
 		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
@@ -1317,6 +1332,8 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.info = &vcnl4040_info,
 		.irq_thread = vcnl4040_irq_thread,
 		.int_reg = VCNL4040_INT_FLAGS,
+		.ps_it_times = &vcnl4040_ps_it_times,
+		.num_ps_it_times = ARRAY_SIZE(vcnl4040_ps_it_times),
 	},
 	[VCNL4200] = {
 		.prod = "VCNL4200",
@@ -1329,6 +1346,8 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.info = &vcnl4040_info,
 		.irq_thread = vcnl4040_irq_thread,
 		.int_reg = VCNL4200_INT_FLAGS,
+		.ps_it_times = &vcnl4200_ps_it_times,
+		.num_ps_it_times = ARRAY_SIZE(vcnl4200_ps_it_times),
 	},
 };
 
-- 
2.30.2

[PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200

[Astrid Rost]

Add illuminance integration time for vcnl4040 and vcnl4200.
Add read/write attribute for illuminance integration time and read
attribute for available integration times.
Set scale and sampling rate according to the integration time.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 121 ++++++++++++++++++++++++++++++++---
 1 file changed, 113 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 086a493fb6e5..285f1dd314d8 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -80,6 +80,7 @@
 #define VCNL4000_SELF_TIMED_EN	BIT(0) /* start self-timed measurement */
 
 #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
+#define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
 #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
 #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
@@ -133,6 +134,20 @@ static const int vcnl4200_ps_it_times[][2] = {
 	{0, 864},
 };
 
+static const int vcnl4040_als_it_times[][2] = {
+	{0, 80000},
+	{0, 160000},
+	{0, 320000},
+	{0, 640000},
+};
+
+static const int vcnl4200_als_it_times[][2] = {
+	{0, 50000},
+	{0, 100000},
+	{0, 200000},
+	{0, 400000},
+};
+
 #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
 
 enum vcnl4000_device_ids {
@@ -177,6 +192,8 @@ struct vcnl4000_chip_spec {
 	u8 int_reg;
 	const int(*ps_it_times)[][2];
 	const int num_ps_it_times;
+	const int(*als_it_times)[][2];
+	const int num_als_it_times;
 };
 
 static const struct i2c_device_id vcnl4000_id[] = {
@@ -337,7 +354,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 		data->vcnl4200_al.sampling_rate = ktime_set(0, 96000 * 1000);
 		/* Default wait time is 5ms, add 20% tolerance. */
 		data->vcnl4200_ps.sampling_rate = ktime_set(0, 6000 * 1000);
-		data->al_scale = 120000;
+		data->al_scale = 100000;
 		break;
 	}
 	mutex_init(&data->vcnl4200_al.lock);
@@ -509,6 +526,65 @@ static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on)
 	return ret;
 }
 
+static int vcnl4040_read_als_it(struct vcnl4000_data *data, int *val, int *val2)
+{
+	int ret;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+	if (ret < 0)
+		return ret;
+
+	ret = FIELD_GET(VCNL4040_ALS_CONF_IT, ret);
+
+	if (ret >= data->chip_spec->num_als_it_times)
+		return -EINVAL;
+
+	*val = (*data->chip_spec->als_it_times)[ret][0];
+	*val2 = (*data->chip_spec->als_it_times)[ret][1];
+
+	return 0;
+}
+
+static ssize_t vcnl4040_write_als_it(struct vcnl4000_data *data, int val)
+{
+	unsigned int i;
+	int ret, index = -1;
+	u16 regval;
+
+	for (i = 0; i < data->chip_spec->num_als_it_times; i++) {
+		if (val == (*data->chip_spec->als_it_times)[i][1]) {
+			index = i;
+			break;
+		}
+	}
+
+	if (index < 0)
+		return -EINVAL;
+
+	data->vcnl4200_al.sampling_rate = ktime_set(0, val * 1200000);
+	if (data->id == VCNL4200)
+		/* 24000 µlux/step * 50000 µs / val  */
+		data->al_scale = div_u64(1200000000, val);
+	else
+		/* 100000 µlux/step * 80000 µs / val */
+		data->al_scale = div_u64(8000000000, val);
+
+	mutex_lock(&data->vcnl4000_lock);
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+	if (ret < 0)
+		goto out;
+
+	regval = (ret & ~VCNL4040_ALS_CONF_IT) |
+	    FIELD_PREP(VCNL4040_ALS_CONF_IT, index);
+	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
+					regval);
+
+out:
+	mutex_unlock(&data->vcnl4000_lock);
+	return ret;
+}
+
 static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2)
 {
 	int ret;
@@ -599,9 +675,16 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 		*val2 = data->al_scale;
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_INT_TIME:
-		if (chan->type != IIO_PROXIMITY)
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ret = vcnl4040_read_als_it(data, val, val2);
+			break;
+		case IIO_PROXIMITY:
+			ret = vcnl4040_read_ps_it(data, val, val2);
+			break;
+		default:
 			return -EINVAL;
-		ret = vcnl4040_read_ps_it(data, val, val2);
+		}
 		if (ret < 0)
 			return ret;
 		return IIO_VAL_INT_PLUS_MICRO;
@@ -620,9 +703,14 @@ static int vcnl4040_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_INT_TIME:
 		if (val != 0)
 			return -EINVAL;
-		if (chan->type != IIO_PROXIMITY)
+		switch (chan->type) {
+		case IIO_LIGHT:
+			return vcnl4040_write_als_it(data, val2);
+		case IIO_PROXIMITY:
+			return vcnl4040_write_ps_it(data, val2);
+		default:
 			return -EINVAL;
-		return vcnl4040_write_ps_it(data, val2);
+		}
 	default:
 		return -EINVAL;
 	}
@@ -637,9 +725,20 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_INT_TIME:
-		*vals = (int *)(*data->chip_spec->ps_it_times);
+
+		switch (chan->type) {
+		case IIO_LIGHT:
+			*vals = (int *)(*data->chip_spec->als_it_times);
+			*length = 2 * data->chip_spec->num_als_it_times;
+			break;
+		case IIO_PROXIMITY:
+			*vals = (int *)(*data->chip_spec->ps_it_times);
+			*length = 2 * data->chip_spec->num_ps_it_times;
+			break;
+		default:
+			return -EINVAL;
+		}
 		*type = IIO_VAL_INT_PLUS_MICRO;
-		*length = 2 * data->chip_spec->num_ps_it_times;
 		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
@@ -1261,7 +1360,9 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 	{
 		.type = IIO_LIGHT,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-			BIT(IIO_CHAN_INFO_SCALE),
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_INT_TIME),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
 	}, {
 		.type = IIO_PROXIMITY,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
@@ -1334,6 +1435,8 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.int_reg = VCNL4040_INT_FLAGS,
 		.ps_it_times = &vcnl4040_ps_it_times,
 		.num_ps_it_times = ARRAY_SIZE(vcnl4040_ps_it_times),
+		.als_it_times = &vcnl4040_als_it_times,
+		.num_als_it_times = ARRAY_SIZE(vcnl4040_als_it_times),
 	},
 	[VCNL4200] = {
 		.prod = "VCNL4200",
@@ -1348,6 +1451,8 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
 		.int_reg = VCNL4200_INT_FLAGS,
 		.ps_it_times = &vcnl4200_ps_it_times,
 		.num_ps_it_times = ARRAY_SIZE(vcnl4200_ps_it_times),
+		.als_it_times = &vcnl4200_als_it_times,
+		.num_als_it_times = ARRAY_SIZE(vcnl4200_als_it_times),
 	},
 };
 
-- 
2.30.2

Re: [PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200

[andy.shevchenko]

Mon, May 22, 2023 at 04:26:17PM +0200, Astrid Rost kirjoitti:
> Add illuminance integration time for vcnl4040 and vcnl4200.
> Add read/write attribute for illuminance integration time and read
> attribute for available integration times.
> Set scale and sampling rate according to the integration time.

...

> +	data->vcnl4200_al.sampling_rate = ktime_set(0, val * 1200000);
> +	if (data->id == VCNL4200)
> +		/* 24000 µlux/step * 50000 µs / val  */
> +		data->al_scale = div_u64(1200000000, val);

Without type these constant are integers. Is this okay with 32-bit builds?

> +	else
> +		/* 100000 µlux/step * 80000 µs / val */
> +		data->al_scale = div_u64(8000000000, val);

Ditto.

Also, can you use constants from units.h (KILO) and time.h (USEC_PER_MSEC)?

...

> +out:

out_unlock:

> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200

[Astrid Rost]

Hello Andy,

Thanks for your comment.


On 5/29/23 00:42, andy.shevchenko@gmail.com wrote:
> Mon, May 22, 2023 at 04:26:17PM +0200, Astrid Rost kirjoitti:
>> Add illuminance integration time for vcnl4040 and vcnl4200.
>> Add read/write attribute for illuminance integration time and read
>> attribute for available integration times.
>> Set scale and sampling rate according to the integration time.
> 
> ...
> 
>> +	data->vcnl4200_al.sampling_rate = ktime_set(0, val * 1200000);
>> +	if (data->id == VCNL4200)
>> +		/* 24000 µlux/step * 50000 µs / val  */
>> +		data->al_scale = div_u64(1200000000, val);
> 
> Without type these constant are integers. Is this okay with 32-bit builds?

No, I got an error on x86 on my first patch. But i did not really 
understand. All variables are within int and not defined as something 
bigger.

> 
>> +	else
>> +		/* 100000 µlux/step * 80000 µs / val */
>> +		data->al_scale = div_u64(8000000000, val);
> 
> Ditto.

8000000000 exceeds at least 32 bit.
> 
> Also, can you use constants from units.h (KILO) and time.h (USEC_PER_MSEC)?
> 
> ...
> 
>> +out:
> 
> out_unlock:
> 
>> +	mutex_unlock(&data->vcnl4000_lock);
>> +	return ret;
> 

[PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

[Astrid Rost]

Add support to configure ambient light sensor interrupts and threshold
limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
will be pushed to the event interface.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 197 ++++++++++++++++++++++++++---------
 1 file changed, 146 insertions(+), 51 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 285f1dd314d8..a84fda5e29d0 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -62,6 +62,8 @@
 #define VCNL4200_PS_CONF1	0x03 /* Proximity configuration */
 #define VCNL4040_PS_THDL_LM	0x06 /* Proximity threshold low */
 #define VCNL4040_PS_THDH_LM	0x07 /* Proximity threshold high */
+#define VCNL4040_ALS_THDL_LM	0x02 /* Ambient light threshold low */
+#define VCNL4040_ALS_THDH_LM	0x01 /* Ambient light threshold high */
 #define VCNL4200_PS_DATA	0x08 /* Proximity data */
 #define VCNL4200_AL_DATA	0x09 /* Ambient light data */
 #define VCNL4040_INT_FLAGS	0x0b /* Interrupt register */
@@ -81,11 +83,14 @@
 
 #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
 #define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
+#define VCNL4040_ALS_CONF_INT_EN	BIT(1) /* Ambient light Interrupt enable */
 #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
 #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
 #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
 #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
+#define VCNL4040_ALS_RISING		BIT(12) /* Ambient Light cross high threshold */
+#define VCNL4040_ALS_FALLING		BIT(13) /* Ambient Light cross low threshold */
 
 /* Bit masks for interrupt registers. */
 #define VCNL4010_INT_THR_SEL	BIT(0) /* Select threshold interrupt source */
@@ -170,6 +175,7 @@ struct vcnl4000_data {
 	int rev;
 	int al_scale;
 	u8 ps_int;		/* proximity interrupt mode */
+	u8 als_int;		/* ambient light interrupt mode*/
 	const struct vcnl4000_chip_spec *chip_spec;
 	struct mutex vcnl4000_lock;
 	struct vcnl4200_channel vcnl4200_al;
@@ -293,7 +299,7 @@ static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
 	int ret;
 
 	/* Do not power down if interrupts are enabled */
-	if (!on && data->ps_int)
+	if (!on && (data->ps_int || data->als_int))
 		return 0;
 
 	ret = vcnl4000_write_als_enable(data, on);
@@ -338,6 +344,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 
 	data->rev = (ret >> 8) & 0xf;
 	data->ps_int = 0;
+	data->als_int = 0;
 
 	data->vcnl4200_al.reg = VCNL4200_AL_DATA;
 	data->vcnl4200_ps.reg = VCNL4200_PS_DATA;
@@ -931,27 +938,45 @@ static int vcnl4040_read_event(struct iio_dev *indio_dev,
 			       enum iio_event_info info,
 			       int *val, int *val2)
 {
-	int ret;
+	int ret = -EINVAL;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		ret = i2c_smbus_read_word_data(data->client,
-					       VCNL4040_PS_THDH_LM);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-		return IIO_VAL_INT;
-	case IIO_EV_DIR_FALLING:
-		ret = i2c_smbus_read_word_data(data->client,
-					       VCNL4040_PS_THDL_LM);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-		return IIO_VAL_INT;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_ALS_THDH_LM);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_ALS_THDL_LM);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_PROXIMITY:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_PS_THDH_LM);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_read_word_data(data->client,
+						       VCNL4040_PS_THDL_LM);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
+	if (ret < 0)
+		return ret;
+	*val = ret;
+	return IIO_VAL_INT;
 }
 
 static int vcnl4040_write_event(struct iio_dev *indio_dev,
@@ -961,25 +986,43 @@ static int vcnl4040_write_event(struct iio_dev *indio_dev,
 				enum iio_event_info info,
 				int val, int val2)
 {
-	int ret;
+	int ret = -EINVAL;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
-
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		ret = i2c_smbus_write_word_data(data->client,
-						VCNL4040_PS_THDH_LM, val);
-		if (ret < 0)
-			return ret;
-		return IIO_VAL_INT;
-	case IIO_EV_DIR_FALLING:
-		ret = i2c_smbus_write_word_data(data->client,
-						VCNL4040_PS_THDL_LM, val);
-		if (ret < 0)
-			return ret;
-		return IIO_VAL_INT;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_ALS_THDH_LM, val);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_ALS_THDL_LM, val);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_PROXIMITY:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_PS_THDH_LM, val);
+			break;
+		case IIO_EV_DIR_FALLING:
+			ret = i2c_smbus_write_word_data(
+				data->client, VCNL4040_PS_THDL_LM, val);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
+	if (ret < 0)
+		return ret;
+	return IIO_VAL_INT;
 }
 
 static bool vcnl4010_is_thr_enabled(struct vcnl4000_data *data)
@@ -1071,16 +1114,28 @@ static int vcnl4040_read_event_config(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
+	switch (chan->type) {
+	case IIO_LIGHT:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+		if (ret < 0)
+			return ret;
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
-	if (ret < 0)
-		return ret;
+		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, ret);
 
-	data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, ret);
+		return data->als_int;
+	case IIO_PROXIMITY:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+		if (ret < 0)
+			return ret;
+
+		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, ret);
 
-	return (dir == IIO_EV_DIR_RISING) ?
-		FIELD_GET(VCNL4040_PS_IF_AWAY, ret) :
-		FIELD_GET(VCNL4040_PS_IF_CLOSE, ret);
+		return (dir == IIO_EV_DIR_RISING) ?
+			FIELD_GET(VCNL4040_PS_IF_AWAY, ret) :
+			FIELD_GET(VCNL4040_PS_IF_CLOSE, ret);
+	default:
+		return -EINVAL;
+	}
 }
 
 static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
@@ -1088,29 +1143,51 @@ static int vcnl4040_write_event_config(struct iio_dev *indio_dev,
 				       enum iio_event_type type,
 				       enum iio_event_direction dir, int state)
 {
-	int ret;
+	int ret = -EINVAL;
 	u16 val, mask;
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 
 	mutex_lock(&data->vcnl4000_lock);
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
-	if (ret < 0)
-		goto out;
+	switch (chan->type) {
+	case IIO_LIGHT:
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+		if (ret < 0)
+			goto out;
 
-	if (dir == IIO_EV_DIR_RISING)
-		mask = VCNL4040_PS_IF_AWAY;
-	else
-		mask = VCNL4040_PS_IF_CLOSE;
+		mask = VCNL4040_ALS_CONF_INT_EN;
 
-	val = state ? (ret | mask) : (ret & ~mask);
+		val = state ? (ret | mask) : (ret & ~mask);
 
-	data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val);
+		data->als_int = FIELD_GET(VCNL4040_ALS_CONF_INT_EN, val);
+		ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
+						val);
+		break;
+	case IIO_PROXIMITY:
+
+		ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+		if (ret < 0)
+			goto out;
+
+		if (dir == IIO_EV_DIR_RISING)
+			mask = VCNL4040_PS_IF_AWAY;
+		else
+			mask = VCNL4040_PS_IF_CLOSE;
+
+		val = state ? (ret | mask) : (ret & ~mask);
+
+		data->ps_int = FIELD_GET(VCNL4040_PS_CONF2_PS_INT, val);
+		ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
+						val);
+		break;
+	default:
+		break;
+	}
 
 out:
 	mutex_unlock(&data->vcnl4000_lock);
-	data->chip_spec->set_power_state(data, data->ps_int != 0);
+	data->chip_spec->set_power_state(data, data->ps_int ||
+						data->als_int);
 
 	return ret;
 }
@@ -1141,6 +1218,22 @@ static irqreturn_t vcnl4040_irq_thread(int irq, void *p)
 			       iio_get_time_ns(indio_dev));
 	}
 
+	if (ret & VCNL4040_ALS_FALLING) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_FALLING),
+			       iio_get_time_ns(indio_dev));
+	}
+
+	if (ret & VCNL4040_ALS_RISING) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+			       iio_get_time_ns(indio_dev));
+	}
+
 	return IRQ_HANDLED;
 }
 
@@ -1363,6 +1456,8 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE) |
 			BIT(IIO_CHAN_INFO_INT_TIME),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
+		.event_spec = vcnl4000_event_spec,
+		.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
 	}, {
 		.type = IIO_PROXIMITY,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-- 
2.30.2

Re: [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

[andy.shevchenko]

Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
> Add support to configure ambient light sensor interrupts and threshold
> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> will be pushed to the event interface.

...

> +		case IIO_EV_DIR_RISING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_ALS_THDH_LM, val);

Strange indentation.

> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_ALS_THDL_LM, val);

Same.

> +			break;

...

> +	case IIO_PROXIMITY:
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_PS_THDH_LM, val);

Same.

> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			ret = i2c_smbus_write_word_data(
> +				data->client, VCNL4040_PS_THDL_LM, val);

Same.

> +			break;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

[Astrid Rost]

Hello Andy,

Thanks for reviewing.
I can change this. But this is how it gets formatted by .clang-format.

Astrid

On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:
> Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
>> Add support to configure ambient light sensor interrupts and threshold
>> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
>> will be pushed to the event interface.
> 
> ...
> 
>> +		case IIO_EV_DIR_RISING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_ALS_THDH_LM, val);
> 
> Strange indentation.
> 
>> +			break;
>> +		case IIO_EV_DIR_FALLING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_ALS_THDL_LM, val);
> 
> Same.
> 
>> +			break;
> 
> ...
> 
>> +	case IIO_PROXIMITY:
>> +		switch (dir) {
>> +		case IIO_EV_DIR_RISING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_PS_THDH_LM, val);
> 
> Same.
> 
>> +			break;
>> +		case IIO_EV_DIR_FALLING:
>> +			ret = i2c_smbus_write_word_data(
>> +				data->client, VCNL4040_PS_THDL_LM, val);
> 
> Same.
> 
>> +			break;
> 

Re: [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

[Andy Shevchenko]

On Mon, May 29, 2023 at 10:41 AM Astrid Rost <astridr@axis.com> wrote:
> Thanks for reviewing.
> I can change this. But this is how it gets formatted by .clang-format.

I would suggest to report the bug (in case it's not configurable) or
configure to avoid such a misindentation.

> On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:
> > Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:
> >> Add support to configure ambient light sensor interrupts and threshold
> >> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> >> will be pushed to the event interface.
> >
> > ...
> >
> >> +            case IIO_EV_DIR_RISING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_ALS_THDH_LM, val);
> >
> > Strange indentation.
> >
> >> +                    break;
> >> +            case IIO_EV_DIR_FALLING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_ALS_THDL_LM, val);
> >
> > Same.
> >
> >> +                    break;
> >
> > ...
> >
> >> +    case IIO_PROXIMITY:
> >> +            switch (dir) {
> >> +            case IIO_EV_DIR_RISING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_PS_THDH_LM, val);
> >
> > Same.
> >
> >> +                    break;
> >> +            case IIO_EV_DIR_FALLING:
> >> +                    ret = i2c_smbus_write_word_data(
> >> +                            data->client, VCNL4040_PS_THDL_LM, val);
> >
> > Same.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200

[Jonathan Cameron]

On Mon, 29 May 2023 12:02:16 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, May 29, 2023 at 10:41 AM Astrid Rost <astridr@axis.com> wrote:
> > Thanks for reviewing.
> > I can change this. But this is how it gets formatted by .clang-format.  
> 
> I would suggest to report the bug (in case it's not configurable) or
> configure to avoid such a misindentation.

I assume it's trading off line length against aligning with opening brackets.
Maybe made the wrong decision, but it is not totally unreasonable to my eyes.
I don't care either way though, so go with whatever you have for next version!

Jonathan

> 
> > On 5/29/23 00:45, andy.shevchenko@gmail.com wrote:  
> > > Mon, May 22, 2023 at 04:26:18PM +0200, Astrid Rost kirjoitti:  
> > >> Add support to configure ambient light sensor interrupts and threshold
> > >> limits for vcnl4040 and vcnl4200. If an interrupt is detected an event
> > >> will be pushed to the event interface.  
> > >
> > > ...
> > >  
> > >> +            case IIO_EV_DIR_RISING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_ALS_THDH_LM, val);  
> > >
> > > Strange indentation.
> > >  
> > >> +                    break;
> > >> +            case IIO_EV_DIR_FALLING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_ALS_THDL_LM, val);  
> > >
> > > Same.
> > >  
> > >> +                    break;  
> > >
> > > ...
> > >  
> > >> +    case IIO_PROXIMITY:
> > >> +            switch (dir) {
> > >> +            case IIO_EV_DIR_RISING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_PS_THDH_LM, val);  
> > >
> > > Same.
> > >  
> > >> +                    break;
> > >> +            case IIO_EV_DIR_FALLING:
> > >> +                    ret = i2c_smbus_write_word_data(
> > >> +                            data->client, VCNL4040_PS_THDL_LM, val);  
> > >
> > > Same.  
> 

[PATCH v4 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200

[Astrid Rost]

Add read/write attribute for proximity and illuminance period.
The period is set in the interrupt persistence flags
(PS_PERS and ALS_PERS). An interrupt will not be asserted if the raw
value is not over (or lower) than the threshold for the set
continued amount of measurements.
The time in seconds is calculated by the number of continued refreshes
multiplied with the integration time.
It will always pick the next lower possible value. The period changes,
if the integration time is changed.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 255 ++++++++++++++++++++++++++++++-----
 1 file changed, 220 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index a84fda5e29d0..4fd4d4ef5990 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -24,6 +24,7 @@
 #include <linux/delay.h>
 #include <linux/pm_runtime.h>
 #include <linux/interrupt.h>
+#include <linux/units.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/events.h>
@@ -84,8 +85,10 @@
 #define VCNL4040_ALS_CONF_ALS_SHUTDOWN	BIT(0)
 #define VCNL4040_ALS_CONF_IT		GENMASK(7, 6) /* Ambient integration time */
 #define VCNL4040_ALS_CONF_INT_EN	BIT(1) /* Ambient light Interrupt enable */
+#define VCNL4040_ALS_CONF_PERS	GENMASK(3, 2) /* Ambient interrupt persistence setting */
 #define VCNL4040_PS_CONF1_PS_SHUTDOWN	BIT(0)
 #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
+#define VCNL4040_CONF1_PS_PERS	GENMASK(5, 4) /* Proximity interrupt persistence setting */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
 #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
 #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
@@ -152,6 +155,8 @@ static const int vcnl4200_als_it_times[][2] = {
 	{0, 200000},
 	{0, 400000},
 };
+static const int vcnl4040_als_persistence[] = {1, 2, 4, 8};
+static const int vcnl4040_ps_persistence[] = {1, 2, 3, 4};
 
 #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
 
@@ -645,6 +650,134 @@ static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val)
 	return ret;
 }
 
+static ssize_t vcnl4040_read_als_period(struct vcnl4000_data *data, int *val, int *val2)
+{
+	int ret, ret_pers, ret_it;
+	int64_t val_c;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+	if (ret < 0)
+		return ret;
+
+	ret_pers = FIELD_GET(VCNL4040_ALS_CONF_PERS, ret);
+
+	if (ret_pers >= ARRAY_SIZE(vcnl4040_als_persistence))
+		return -EINVAL;
+
+	ret_it = FIELD_GET(VCNL4040_ALS_CONF_IT, ret);
+
+	if (ret_it >= data->chip_spec->num_als_it_times)
+		return -EINVAL;
+
+	val_c = mul_u32_u32((*data->chip_spec->als_it_times)[ret_it][1],
+	      vcnl4040_als_persistence[ret_pers]);
+	*val = div_u64_rem(val_c, MICRO, val2);
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static ssize_t vcnl4040_write_als_period(struct vcnl4000_data *data, int val, int val2)
+{
+	unsigned int index;
+	int ret, ret_it;
+	u16 regval;
+	int64_t val_n = mul_u32_u32(val, MICRO) + val2;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+	if (ret < 0)
+		return ret;
+
+	ret_it = FIELD_GET(VCNL4040_ALS_CONF_IT, ret);
+
+	if (ret_it >= data->chip_spec->num_als_it_times)
+		return -EINVAL;
+
+	for (index = 0; index < ARRAY_SIZE(vcnl4040_als_persistence) - 1; index++)
+		if (val_n < mul_u32_u32(vcnl4040_als_persistence[index],
+				(*data->chip_spec->als_it_times)[ret_it][1]))
+			break;
+
+	mutex_lock(&data->vcnl4000_lock);
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF);
+	if (ret < 0)
+		goto out;
+
+	regval = (ret & ~VCNL4040_ALS_CONF_PERS) |
+	    FIELD_PREP(VCNL4040_ALS_CONF_PERS, index);
+	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF,
+					regval);
+
+out:
+	mutex_unlock(&data->vcnl4000_lock);
+	return ret;
+}
+
+static ssize_t vcnl4040_read_ps_period(struct vcnl4000_data *data, int *val, int *val2)
+{
+	int ret, ret_pers, ret_it;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+	if (ret < 0)
+		return ret;
+
+	ret_pers = FIELD_GET(VCNL4040_CONF1_PS_PERS, ret);
+
+	if (ret_pers >= ARRAY_SIZE(vcnl4040_ps_persistence))
+		return -EINVAL;
+
+	ret_it = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret);
+
+	if (ret_it >= data->chip_spec->num_ps_it_times)
+		return -EINVAL;
+
+	*val = (*data->chip_spec->ps_it_times)[ret_it][0];
+	*val2 = (*data->chip_spec->ps_it_times)[ret_it][1]
+		* vcnl4040_ps_persistence[ret_pers];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static ssize_t vcnl4040_write_ps_period(struct vcnl4000_data *data, int val, int val2)
+{
+	int ret, ret_it, index;
+	u16 regval;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+	if (ret < 0)
+		return ret;
+
+	ret_it = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret);
+
+	if (ret_it >= data->chip_spec->num_ps_it_times)
+		return -EINVAL;
+
+	if (val > 0)
+		index = ARRAY_SIZE(vcnl4040_ps_persistence) - 1;
+	else {
+		for (index = 0; index < ARRAY_SIZE(vcnl4040_ps_persistence) - 1; index++) {
+			if (val2 <= vcnl4040_ps_persistence[index]
+					* (*data->chip_spec->ps_it_times)[ret_it][1])
+				break;
+		}
+	}
+
+	mutex_lock(&data->vcnl4000_lock);
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+	if (ret < 0)
+		goto out;
+
+	regval = (ret & ~VCNL4040_CONF1_PS_PERS) |
+	    FIELD_PREP(VCNL4040_CONF1_PS_PERS, index);
+	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
+					regval);
+
+out:
+	mutex_unlock(&data->vcnl4000_lock);
+	return ret;
+}
+
 static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 				struct iio_chan_spec const *chan,
 				int *val, int *val2, long mask)
@@ -943,28 +1076,44 @@ static int vcnl4040_read_event(struct iio_dev *indio_dev,
 
 	switch (chan->type) {
 	case IIO_LIGHT:
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			ret = i2c_smbus_read_word_data(data->client,
-						       VCNL4040_ALS_THDH_LM);
-			break;
-		case IIO_EV_DIR_FALLING:
-			ret = i2c_smbus_read_word_data(data->client,
-						       VCNL4040_ALS_THDL_LM);
+		switch (info) {
+		case IIO_EV_INFO_PERIOD:
+			return vcnl4040_read_als_period(data, val, val2);
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				ret = i2c_smbus_read_word_data(data->client,
+							       VCNL4040_ALS_THDH_LM);
+				break;
+			case IIO_EV_DIR_FALLING:
+				ret = i2c_smbus_read_word_data(data->client,
+							       VCNL4040_ALS_THDL_LM);
+				break;
+			default:
+				return -EINVAL;
+			}
 			break;
 		default:
 			return -EINVAL;
 		}
 		break;
 	case IIO_PROXIMITY:
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			ret = i2c_smbus_read_word_data(data->client,
-						       VCNL4040_PS_THDH_LM);
-			break;
-		case IIO_EV_DIR_FALLING:
-			ret = i2c_smbus_read_word_data(data->client,
-						       VCNL4040_PS_THDL_LM);
+		switch (info) {
+		case IIO_EV_INFO_PERIOD:
+			return vcnl4040_read_ps_period(data, val, val2);
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				ret = i2c_smbus_read_word_data(data->client,
+							       VCNL4040_PS_THDH_LM);
+				break;
+			case IIO_EV_DIR_FALLING:
+				ret = i2c_smbus_read_word_data(data->client,
+							       VCNL4040_PS_THDL_LM);
+				break;
+			default:
+				return -EINVAL;
+			}
 			break;
 		default:
 			return -EINVAL;
@@ -990,28 +1139,44 @@ static int vcnl4040_write_event(struct iio_dev *indio_dev,
 	struct vcnl4000_data *data = iio_priv(indio_dev);
 	switch (chan->type) {
 	case IIO_LIGHT:
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			ret = i2c_smbus_write_word_data(
-				data->client, VCNL4040_ALS_THDH_LM, val);
-			break;
-		case IIO_EV_DIR_FALLING:
-			ret = i2c_smbus_write_word_data(
-				data->client, VCNL4040_ALS_THDL_LM, val);
+		switch (info) {
+		case IIO_EV_INFO_PERIOD:
+			return vcnl4040_write_als_period(data, val, val2);
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				ret = i2c_smbus_write_word_data(
+					data->client, VCNL4040_ALS_THDH_LM, val);
+				break;
+			case IIO_EV_DIR_FALLING:
+				ret = i2c_smbus_write_word_data(
+					data->client, VCNL4040_ALS_THDL_LM, val);
+				break;
+			default:
+				return -EINVAL;
+			}
 			break;
 		default:
 			return -EINVAL;
 		}
 		break;
 	case IIO_PROXIMITY:
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			ret = i2c_smbus_write_word_data(
-				data->client, VCNL4040_PS_THDH_LM, val);
-			break;
-		case IIO_EV_DIR_FALLING:
-			ret = i2c_smbus_write_word_data(
-				data->client, VCNL4040_PS_THDL_LM, val);
+		switch (info) {
+		case IIO_EV_INFO_PERIOD:
+			return vcnl4040_write_ps_period(data, val, val2);
+		case IIO_EV_INFO_VALUE:
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				ret = i2c_smbus_write_word_data(
+					data->client, VCNL4040_PS_THDH_LM, val);
+				break;
+			case IIO_EV_DIR_FALLING:
+				ret = i2c_smbus_write_word_data(
+					data->client, VCNL4040_PS_THDL_LM, val);
+				break;
+			default:
+				return -EINVAL;
+			}
 			break;
 		default:
 			return -EINVAL;
@@ -1400,6 +1565,22 @@ static const struct iio_event_spec vcnl4000_event_spec[] = {
 	}
 };
 
+static const struct iio_event_spec vcnl4040_als_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) |  BIT(IIO_EV_INFO_PERIOD),
+	}
+};
+
 static const struct iio_event_spec vcnl4040_event_spec[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -1409,7 +1590,11 @@ static const struct iio_event_spec vcnl4040_event_spec[] = {
 		.type = IIO_EV_TYPE_THRESH,
 		.dir = IIO_EV_DIR_FALLING,
 		.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
-	},
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
+	}
 };
 
 static const struct iio_chan_spec vcnl4000_channels[] = {
@@ -1456,8 +1641,8 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE) |
 			BIT(IIO_CHAN_INFO_INT_TIME),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
-		.event_spec = vcnl4000_event_spec,
-		.num_event_specs = ARRAY_SIZE(vcnl4000_event_spec),
+		.event_spec = vcnl4040_als_event_spec,
+		.num_event_specs = ARRAY_SIZE(vcnl4040_als_event_spec),
 	}, {
 		.type = IIO_PROXIMITY,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-- 
2.30.2

Re: [PATCH v4 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200

[andy.shevchenko]

Mon, May 22, 2023 at 04:26:19PM +0200, Astrid Rost kirjoitti:
> Add read/write attribute for proximity and illuminance period.
> The period is set in the interrupt persistence flags
> (PS_PERS and ALS_PERS). An interrupt will not be asserted if the raw
> value is not over (or lower) than the threshold for the set
> continued amount of measurements.
> The time in seconds is calculated by the number of continued refreshes
> multiplied with the integration time.
> It will always pick the next lower possible value. The period changes,
> if the integration time is changed.

Something interesting happened to the indentation in the above text.

...

> +out:

out_unlock:

> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;

...

> +	*val2 = (*data->chip_spec->ps_it_times)[ret_it][1]
> +		* vcnl4040_ps_persistence[ret_pers];

' *' can be placed on the previous line.

...

> +			if (val2 <= vcnl4040_ps_persistence[index]
> +					* (*data->chip_spec->ps_it_times)[ret_it][1])

Ditto.

> +				break;

...

> +out:

out_unlock:

> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;

...

> +			case IIO_EV_DIR_RISING:
> +				ret = i2c_smbus_write_word_data(
> +					data->client, VCNL4040_ALS_THDH_LM, val);

Indentation.

> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				ret = i2c_smbus_write_word_data(
> +					data->client, VCNL4040_ALS_THDL_LM, val);

Ditto.

> +				break;

...

> +			case IIO_EV_DIR_RISING:
> +				ret = i2c_smbus_write_word_data(
> +					data->client, VCNL4040_PS_THDH_LM, val);
> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				ret = i2c_smbus_write_word_data(
> +					data->client, VCNL4040_PS_THDL_LM, val);
> +				break;

Ditto.

...

> +	{
> +		.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) |  BIT(IIO_EV_INFO_PERIOD),
> +	}

Can we keep trailing comma here?

...

> -	},

Stray change (and see just above).

> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_PERIOD),
> +	}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200

[Astrid Rost]

Add the proximity multi pulse (PS_MPS) as oversampling_ratio.
Instead of one single pulse per every defined time frame,
one can program 2, 4, or even 8 pulses. This leads to a
longer IRED on-time for each proximity measurement value, which
also results in a higher detection range.

Add read/write attribute for proximity oversampling-ratio and read
attribute for available oversampling-ratio.
This is supported for vcnl4040 and vcnl4200.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 84 ++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 4fd4d4ef5990..67d5d0e14bcd 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -61,6 +61,7 @@
 
 #define VCNL4200_AL_CONF	0x00 /* Ambient light configuration */
 #define VCNL4200_PS_CONF1	0x03 /* Proximity configuration */
+#define VCNL4200_PS_CONF3	0x04 /* Proximity configuration */
 #define VCNL4040_PS_THDL_LM	0x06 /* Proximity threshold low */
 #define VCNL4040_PS_THDH_LM	0x07 /* Proximity threshold high */
 #define VCNL4040_ALS_THDL_LM	0x02 /* Ambient light threshold low */
@@ -90,6 +91,7 @@
 #define VCNL4040_PS_CONF2_PS_IT	GENMASK(3, 1) /* Proximity integration time */
 #define VCNL4040_CONF1_PS_PERS	GENMASK(5, 4) /* Proximity interrupt persistence setting */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
+#define VCNL4040_PS_CONF3_MPS		GENMASK(6, 5) /* Proximity multi pulse number */
 #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
 #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
 #define VCNL4040_ALS_RISING		BIT(12) /* Ambient Light cross high threshold */
@@ -157,6 +159,7 @@ static const int vcnl4200_als_it_times[][2] = {
 };
 static const int vcnl4040_als_persistence[] = {1, 2, 4, 8};
 static const int vcnl4040_ps_persistence[] = {1, 2, 3, 4};
+static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
 
 #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
 
@@ -778,6 +781,56 @@ static ssize_t vcnl4040_write_ps_period(struct vcnl4000_data *data, int val, int
 	return ret;
 }
 
+static ssize_t vcnl4040_read_ps_oversampling_ratio(struct vcnl4000_data *data, int *val)
+{
+	int ret;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
+	if (ret < 0)
+		return ret;
+
+	ret = FIELD_GET(VCNL4040_PS_CONF3_MPS, ret);
+
+	if (ret >= ARRAY_SIZE(vcnl4040_ps_oversampling_ratio))
+		return -EINVAL;
+
+	*val = vcnl4040_ps_oversampling_ratio[ret];
+
+	return ret;
+}
+
+static ssize_t vcnl4040_write_ps_oversampling_ratio(struct vcnl4000_data *data, int val)
+{
+	unsigned int i;
+	int ret, index = -1;
+	u16 regval;
+
+	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_oversampling_ratio); i++) {
+		if (val == vcnl4040_ps_oversampling_ratio[i]) {
+			index = i;
+			break;
+		}
+	}
+
+	if (index < 0)
+		return -EINVAL;
+
+	mutex_lock(&data->vcnl4000_lock);
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
+	if (ret < 0)
+		goto out;
+
+	regval = (ret & ~VCNL4040_PS_CONF3_MPS) |
+	    FIELD_PREP(VCNL4040_PS_CONF3_MPS, index);
+	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
+					regval);
+
+out:
+	mutex_unlock(&data->vcnl4000_lock);
+	return ret;
+}
+
 static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 				struct iio_chan_spec const *chan,
 				int *val, int *val2, long mask)
@@ -828,6 +881,17 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			ret = vcnl4040_read_ps_oversampling_ratio(data, val);
+			if (ret < 0)
+				return ret;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+
 	default:
 		return -EINVAL;
 	}
@@ -851,6 +915,13 @@ static int vcnl4040_write_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			return vcnl4040_write_ps_oversampling_ratio(data, val);
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -880,6 +951,16 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
 		}
 		*type = IIO_VAL_INT_PLUS_MICRO;
 		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			*vals = (int *)vcnl4040_ps_oversampling_ratio;
+			*length = ARRAY_SIZE(vcnl4040_ps_oversampling_ratio);
+			*type = IIO_VAL_INT;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -1648,6 +1729,9 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 			BIT(IIO_CHAN_INFO_INT_TIME),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 		.ext_info = vcnl4000_ext_info,
 		.event_spec = vcnl4040_event_spec,
 		.num_event_specs = ARRAY_SIZE(vcnl4040_event_spec),
-- 
2.30.2

Re: [PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200

[Jonathan Cameron]

On Mon, 22 May 2023 16:26:20 +0200
Astrid Rost <astrid.rost@axis.com> wrote:

> Add the proximity multi pulse (PS_MPS) as oversampling_ratio.
> Instead of one single pulse per every defined time frame,
> one can program 2, 4, or even 8 pulses. This leads to a
> longer IRED on-time for each proximity measurement value, which
> also results in a higher detection range.
> 
> Add read/write attribute for proximity oversampling-ratio and read
> attribute for available oversampling-ratio.
> This is supported for vcnl4040 and vcnl4200.
> 
> Signed-off-by: Astrid Rost <astrid.rost@axis.com>

I applied the series and did a build test which showed me...

> @@ -1648,6 +1729,9 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  			BIT(IIO_CHAN_INFO_INT_TIME),
>  		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
Guessing a rebase went wrong?

I thought about just fixing this up by dropping the duplication, but given
I'm not 100% sure nothing else was affected, I'll leave it to you to fix up
and check for any other related issues for a v5. 

Jonathan

>  		.ext_info = vcnl4000_ext_info,
>  		.event_spec = vcnl4040_event_spec,
>  		.num_event_specs = ARRAY_SIZE(vcnl4040_event_spec),

Re: [PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200

[andy.shevchenko]

Mon, May 22, 2023 at 04:26:20PM +0200, Astrid Rost kirjoitti:
> Add the proximity multi pulse (PS_MPS) as oversampling_ratio.
> Instead of one single pulse per every defined time frame,
> one can program 2, 4, or even 8 pulses. This leads to a
> longer IRED on-time for each proximity measurement value, which
> also results in a higher detection range.
> 
> Add read/write attribute for proximity oversampling-ratio and read
> attribute for available oversampling-ratio.
> This is supported for vcnl4040 and vcnl4200.

...

> +static ssize_t vcnl4040_write_ps_oversampling_ratio(struct vcnl4000_data *data, int val)
> +{
> +	unsigned int i;
> +	int ret, index = -1;

Either i or index is redundant.

> +	u16 regval;
> +
> +	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_oversampling_ratio); i++) {
> +		if (val == vcnl4040_ps_oversampling_ratio[i]) {
> +			index = i;
> +			break;
> +		}
> +	}
> +
> +	if (index < 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&data->vcnl4000_lock);
> +
> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
> +	if (ret < 0)
> +		goto out;
> +
> +	regval = (ret & ~VCNL4040_PS_CONF3_MPS) |
> +	    FIELD_PREP(VCNL4040_PS_CONF3_MPS, index);
> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
> +					regval);
> +
> +out:

out_unlock:

> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;
> +}

...

>  		if (ret < 0)
>  			return ret;
>  		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		switch (chan->type) {
> +		case IIO_PROXIMITY:
> +			ret = vcnl4040_read_ps_oversampling_ratio(data, val);
> +			if (ret < 0)
> +				return ret;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}

> +

This is inconsistent blank line.

>  	default:
>  		return -EINVAL;
>  	}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200

[Astrid Rost]

The calibration bias is setting the LED current to change
the detection distance.
Add read/write attribute for proximity calibration bias and read
attribute for available values.
This is supported for vcnl4040 and vcnl4200.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/iio/light/vcnl4000.c | 98 ++++++++++++++++++++++++++++++++++--
 1 file changed, 95 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 67d5d0e14bcd..27a836cbe85a 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -92,6 +92,7 @@
 #define VCNL4040_CONF1_PS_PERS	GENMASK(5, 4) /* Proximity interrupt persistence setting */
 #define VCNL4040_PS_CONF2_PS_INT	GENMASK(9, 8) /* Proximity interrupt mode */
 #define VCNL4040_PS_CONF3_MPS		GENMASK(6, 5) /* Proximity multi pulse number */
+#define VCNL4040_PS_MS_LED_I		GENMASK(10, 8) /* Proximity current */
 #define VCNL4040_PS_IF_AWAY		BIT(8) /* Proximity event cross low threshold */
 #define VCNL4040_PS_IF_CLOSE		BIT(9) /* Proximity event cross high threshold */
 #define VCNL4040_ALS_RISING		BIT(12) /* Ambient Light cross high threshold */
@@ -157,6 +158,18 @@ static const int vcnl4200_als_it_times[][2] = {
 	{0, 200000},
 	{0, 400000},
 };
+
+static const int vcnl4040_ps_calibbias_ua[][2] = {
+	{0, 50000},
+	{0, 75000},
+	{0, 100000},
+	{0, 120000},
+	{0, 140000},
+	{0, 160000},
+	{0, 180000},
+	{0, 200000},
+};
+
 static const int vcnl4040_als_persistence[] = {1, 2, 4, 8};
 static const int vcnl4040_ps_persistence[] = {1, 2, 3, 4};
 static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
@@ -831,6 +844,57 @@ static ssize_t vcnl4040_write_ps_oversampling_ratio(struct vcnl4000_data *data,
 	return ret;
 }
 
+static ssize_t vcnl4040_read_ps_calibbias(struct vcnl4000_data *data, int *val, int *val2)
+{
+	int ret;
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
+	if (ret < 0)
+		return ret;
+
+	ret = FIELD_GET(VCNL4040_PS_MS_LED_I, ret);
+
+	if (ret >= ARRAY_SIZE(vcnl4040_ps_calibbias_ua))
+		return -EINVAL;
+
+	*val = vcnl4040_ps_calibbias_ua[ret][0];
+	*val2 = vcnl4040_ps_calibbias_ua[ret][1];
+
+	return ret;
+}
+
+static ssize_t vcnl4040_write_ps_calibbias(struct vcnl4000_data *data, int val)
+{
+	unsigned int i;
+	int ret, index = -1;
+	u16 regval;
+
+	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_calibbias_ua); i++) {
+		if (val == vcnl4040_ps_calibbias_ua[i][1]) {
+			index = i;
+			break;
+		}
+	}
+
+	if (index < 0)
+		return -EINVAL;
+
+	mutex_lock(&data->vcnl4000_lock);
+
+	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
+	if (ret < 0)
+		goto out;
+
+	regval = (ret & ~VCNL4040_PS_MS_LED_I) |
+	    FIELD_PREP(VCNL4040_PS_MS_LED_I, index);
+	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
+					regval);
+
+out:
+	mutex_unlock(&data->vcnl4000_lock);
+	return ret;
+}
+
 static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 				struct iio_chan_spec const *chan,
 				int *val, int *val2, long mask)
@@ -891,7 +955,16 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
-
+	case IIO_CHAN_INFO_CALIBBIAS:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			ret = vcnl4040_read_ps_calibbias(data, val, val2);
+			if (ret < 0)
+				return ret;
+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -922,6 +995,13 @@ static int vcnl4040_write_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_CALIBBIAS:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			return vcnl4040_write_ps_calibbias(data, val2);
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -961,6 +1041,16 @@ static int vcnl4040_read_avail(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_CALIBBIAS:
+		switch (chan->type) {
+		case IIO_PROXIMITY:
+			*vals = (int *)vcnl4040_ps_calibbias_ua;
+			*length = 2 * ARRAY_SIZE(vcnl4040_ps_calibbias_ua);
+			*type = IIO_VAL_INT_PLUS_MICRO;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+	}
 	default:
 		return -EINVAL;
 	}
@@ -1729,9 +1819,11 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 			BIT(IIO_CHAN_INFO_INT_TIME),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME),
-			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_CALIBBIAS),
 		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
-			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_CALIBBIAS),
 		.ext_info = vcnl4000_ext_info,
 		.event_spec = vcnl4040_event_spec,
 		.num_event_specs = ARRAY_SIZE(vcnl4040_event_spec),
-- 
2.30.2

Re: [PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200

[andy.shevchenko]

Mon, May 22, 2023 at 04:26:21PM +0200, Astrid Rost kirjoitti:
> The calibration bias is setting the LED current to change
> the detection distance.
> Add read/write attribute for proximity calibration bias and read
> attribute for available values.
> This is supported for vcnl4040 and vcnl4200.

Indentation of the commit message is rather random.

...

> +

This means that one of the previous change missed the blank line here.

> +static const int vcnl4040_ps_calibbias_ua[][2] = {
> +	{0, 50000},
> +	{0, 75000},
> +	{0, 100000},
> +	{0, 120000},
> +	{0, 140000},
> +	{0, 160000},
> +	{0, 180000},
> +	{0, 200000},
> +};
> +
>  static const int vcnl4040_als_persistence[] = {1, 2, 4, 8};
>  static const int vcnl4040_ps_persistence[] = {1, 2, 3, 4};
>  static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};

>  	return ret;
>  }

...

> +static ssize_t vcnl4040_read_ps_calibbias(struct vcnl4000_data *data, int *val, int *val2)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = FIELD_GET(VCNL4040_PS_MS_LED_I, ret);
> +
> +	if (ret >= ARRAY_SIZE(vcnl4040_ps_calibbias_ua))
> +		return -EINVAL;
> +
> +	*val = vcnl4040_ps_calibbias_ua[ret][0];
> +	*val2 = vcnl4040_ps_calibbias_ua[ret][1];

> +	return ret;

Are you sure this is correct?

> +}

...

> +static ssize_t vcnl4040_write_ps_calibbias(struct vcnl4000_data *data, int val)
> +{
> +	unsigned int i;
> +	int ret, index = -1;

i or index is redundant.

> +	u16 regval;
> +
> +	for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_calibbias_ua); i++) {
> +		if (val == vcnl4040_ps_calibbias_ua[i][1]) {
> +			index = i;
> +			break;
> +		}
> +	}
> +
> +	if (index < 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&data->vcnl4000_lock);
> +
> +	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
> +	if (ret < 0)
> +		goto out;
> +
> +	regval = (ret & ~VCNL4040_PS_MS_LED_I) |
> +	    FIELD_PREP(VCNL4040_PS_MS_LED_I, index);
> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
> +					regval);
> +
> +out:

out_unlock:

> +	mutex_unlock(&data->vcnl4000_lock);
> +	return ret;
> +}

...

>  		default:
>  			return -EINVAL;
>  		}
> -

Stray and ping-pong style of change at the same time.

Avoid ping-pong style when you add something in the series which is being
removed aftewards in the same series without need to be added in the first
place.

> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		switch (chan->type) {
> +		case IIO_PROXIMITY:
> +			ret = vcnl4040_read_ps_calibbias(data, val, val2);
> +			if (ret < 0)
> +				return ret;
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		default:
> +			return -EINVAL;
> +		}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/7] iio: light: vcnl4000: Add features for vncl4040/4200

[Jonathan Cameron]

On Mon, 22 May 2023 16:26:14 +0200
Astrid Rost <astrid.rost@axis.com> wrote:

> Add a more complete support for vncl4040 and vcnl4200, which allows to
> change the distance of proximity detection and interrupt support for the
> illuminance sensor.

Something odd going on (superficially looks like a rebase mistake in patch 6)

 CHECK   drivers/iio/amplifiers/ad8366.c                                                                                                         
In file included from ./include/linux/bits.h:6,                                                                                                   
                 from ./include/linux/bitops.h:6,                                                                                                 
                 from ./include/linux/kernel.h:22,                                                                                                
                 from ./arch/x86/include/asm/percpu.h:27,                                                                                         
                 from ./arch/x86/include/asm/current.h:10,                                                                                        
                 from ./arch/x86/include/asm/processor.h:17,                                                                                      
                 from ./arch/x86/include/asm/timex.h:5,                                                                                           
                 from ./include/linux/timex.h:67,                                                                                                 
                 from ./include/linux/time32.h:13,                                                                                                
                 from ./include/linux/time.h:60,                                                                                                  
                 from ./include/linux/stat.h:19,                    
                 from ./include/linux/module.h:13,             
                 from drivers/iio/light/vcnl4000.c:21:             
./include/vdso/bits.h:7:33: warning: initialized field overwritten [-Woverride-init]
    7 | #define BIT(nr)                 (UL(1) << (nr))                                                                                           
      |                                 ^                
drivers/iio/light/vcnl4000.c:1824:49: note: in expansion of macro ‘BIT’                                                                           
 1824 |                 .info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
      |                                                 ^~~
./include/vdso/bits.h:7:33: note: (near initialization for ‘vcnl4040_channels[1].info_mask_separate_available’)
    7 | #define BIT(nr)                 (UL(1) << (nr))        
      |                                 ^         
drivers/iio/light/vcnl4000.c:1824:49: note: in expansion of macro ‘BIT’
 1824 |                 .info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |          
      |                                                 ^~~          


> 
> Proximity functionality:
>   - Interrupt support (new on vcnl4200).
> 
> Proximity reduce the amount of interrupts:
>   - Adaptable integration time (new on vcnl4200) - the sampling rate
>     changes according to this value.
>   - Period - interrupt is asserted if the value is above or
>     below a certain threshold.
> 
> Proximity change the activity distance:
>   - Oversampling ratio - Amount of LED pulses per measured raw value.
>   - Calibration bias - LED current calibration of the sensor.
> 
> Illuminance functionality:
>   - Interrupt support.
> 
> Illuminance reduce the amount of interrupts:
>   - Adaptable integration time - the sampling rate and scale changes
>     according to this value.
>   - Period – interrupt is asserted if the value is above or
>     below a certain threshold.
> 
> changes v2:
> - [PATCH v2 3/7] Fixed calculation of al_scale.
>   Fix the value of vcnl4040 according to the data-sheet.
>   Use div_u64 for the division.
> scription for the branch
> 
> changes v3:
> - [PATCH v3 1-3/7] Add differences between the chips as variables in
>   chip-spec.
> - [PATCH v3 4/7] Changed commit message.
> - [PATCH v3 5/7] Use period instead of debounce time. This causes some
>   calculations as the period is a time and the chip allows to set a certain
>   amount of measurements above/below the threshold, before throwing an
>   interrupt.
> - [PATCH v3 6/7] Changed commit message.
> 
> changes v4:
> - [PATCH v3 1-3/7] Fix setting correct als_it for vcnl4040.
> - [PATCH v3 5/7] Use MICRO macro.
>   Fix values greater than 1 s for the proximity period.
> 
> Astrid Rost (7):
>   [PATCH v4 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200
>   [PATCH v4 2/7] iio: light: vcnl4000: Add proximity ps_it for vcnl4200
>   [PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200
>   [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200
>   [PATCH v4 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200
>   [PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200
>   [PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200
> 
>  drivers/iio/light/vcnl4000.c | 721 +++++++++++++++++++++++++++++++----
>  1 file changed, 653 insertions(+), 68 deletions(-)
>