[PATCH v2 0/4] iio: light: vl6180: Several fixes and enhancements

[PATCH v2 0/4] iio: light: vl6180: Several fixes and enhancements

[Stefan Brüns]

Currently, the vl6180 driver reports bogus in_illuminance_scale values
when the integration time or hardware gain are changed from its default
value, i.e. it always reports a fixed value.

To avoid readback of the register values in case integration time, gain
or scale is queried, save the register values. Use the saved values
to report the correct scale value.

v2: removed redundant parenthesis, add missing spaces

Stefan Brüns (4):
  iio: light: vl6180: Move range check to integration time setter,
    cleanup
  iio: light: vl6180: Avoid readback of integration time register
  iio: light: vl6180: Cleanup als_gain lookup, avoid register readback
  iio: light: vl6180: Correct ALS scale for non-default gain/integration
    time

 drivers/iio/light/vl6180.c | 125 +++++++++++++++++++++++++--------------------
 1 file changed, 69 insertions(+), 56 deletions(-)

-- 
2.14.1

[PATCH v2 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup

[Stefan Brüns]

This improves code uniformity (range checks for als_gain are also done
in the setter). Also unmangle rounding and calculation of register value.

The calculated integration time it_ms is required in the next patch of
the series.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
v2: removed redundant parenthesis

 drivers/iio/light/vl6180.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 6e25b724d941..68b9babfd298 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -386,16 +386,21 @@ static int vl6180_set_als_gain(struct vl6180_data *data, int val, int val2)
 	return -EINVAL;
 }
 
-static int vl6180_set_it(struct vl6180_data *data, int val2)
+static int vl6180_set_it(struct vl6180_data *data, int val, int val2)
 {
-	int ret;
+	int ret, it_ms;
+
+	it_ms = (val2 + 500) / 1000; /* round to ms */
+	if (val != 0 || it_ms < 1 || it_ms > 512)
+		return -EINVAL;
 
 	mutex_lock(&data->lock);
 	ret = vl6180_hold(data, true);
 	if (ret < 0)
 		goto fail;
-	ret = vl6180_write_word(data->client, VL6180_ALS_IT,
-		(val2 - 500) / 1000); /* write value in ms */
+
+	ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
+
 fail:
 	vl6180_hold(data, false);
 	mutex_unlock(&data->lock);
@@ -411,15 +416,14 @@ static int vl6180_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_INT_TIME:
-		if (val != 0 || val2 < 500 || val2 >= 512500)
-			return -EINVAL;
+		return vl6180_set_it(data, val, val2);
 
-		return vl6180_set_it(data, val2);
 	case IIO_CHAN_INFO_HARDWAREGAIN:
 		if (chan->type != IIO_LIGHT)
 			return -EINVAL;
 
 		return vl6180_set_als_gain(data, val, val2);
+
 	default:
 		return -EINVAL;
 	}
-- 
2.14.1

[PATCH v2 2/4] iio: light: vl6180: Avoid readback of integration time register

[Stefan Brüns]

Instead of reading the value from the register on each query, store the
set value.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/iio/light/vl6180.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 68b9babfd298..e6466cc81766 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -86,6 +86,7 @@
 struct vl6180_data {
 	struct i2c_client *client;
 	struct mutex lock;
+	unsigned int als_it_ms;
 };
 
 enum { VL6180_ALS, VL6180_RANGE, VL6180_PROX };
@@ -306,13 +307,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_INT_TIME:
-		ret = vl6180_read_word(data->client, VL6180_ALS_IT);
-		if (ret < 0)
-			return ret;
-		*val = 0; /* 1 count = 1ms (0 = 1ms) */
-		*val2 = (ret + 1) * 1000; /* convert to seconds */
+		*val = 0;
+		*val2 = data->als_it_ms * 1000;
 
 		return IIO_VAL_INT_PLUS_MICRO;
+
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_LIGHT:
@@ -401,6 +400,9 @@ static int vl6180_set_it(struct vl6180_data *data, int val, int val2)
 
 	ret = vl6180_write_word(data->client, VL6180_ALS_IT, it_ms - 1);
 
+	if (ret >= 0)
+		data->als_it_ms = it_ms;
+
 fail:
 	vl6180_hold(data, false);
 	mutex_unlock(&data->lock);
@@ -472,6 +474,7 @@ static int vl6180_init(struct vl6180_data *data)
 		return ret;
 
 	/* ALS integration time: 100ms */
+	data->als_it_ms = 100;
 	ret = vl6180_write_word(client, VL6180_ALS_IT, VL6180_ALS_IT_100);
 	if (ret < 0)
 		return ret;
-- 
2.14.1

[PATCH v2 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback

[Stefan Brüns]

Instead of manually iterating the array of allowed gain values, use
find_closest. Storing the current gain setting avoids accessing the
hardware on each query.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
v2: add missing spaces

 drivers/iio/light/vl6180.c | 85 ++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 41 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index e6466cc81766..5de64bce305c 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -24,6 +24,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/delay.h>
+#include <linux/util_macros.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -86,6 +87,7 @@
 struct vl6180_data {
 	struct i2c_client *client;
 	struct mutex lock;
+	unsigned int als_gain_milli;
 	unsigned int als_it_ms;
 };
 
@@ -276,19 +278,17 @@ static const struct iio_chan_spec vl6180_channels[] = {
 };
 
 /*
- * Columns 3 & 4 represent the same value in decimal and hex notations.
- * Kept in order to avoid the datatype conversion while reading the
- * hardware_gain.
+ * Available Ambient Light Sensor gain settings, 1/1000th, and
+ * corresponding setting for the VL6180_ALS_GAIN register
  */
-static const int vl6180_als_gain[8][4] = {
-	{ 1,	0,	70,	VL6180_ALS_GAIN_1 },
-	{ 1,    250000, 69,	VL6180_ALS_GAIN_1_25 },
-	{ 1,    670000, 68,	VL6180_ALS_GAIN_1_67 },
-	{ 2,    500000, 67,	VL6180_ALS_GAIN_2_5 },
-	{ 5,    0,      66,	VL6180_ALS_GAIN_5 },
-	{ 10,   0,      65,	VL6180_ALS_GAIN_10 },
-	{ 20,   0,      64,	VL6180_ALS_GAIN_20 },
-	{ 40,   0,      71,	VL6180_ALS_GAIN_40 }
+static const int vl6180_als_gain_tab[8] = {
+	1000, 1250, 1670, 2500, 5000, 10000, 20000, 40000
+};
+static const u8 vl6180_als_gain_tab_bits[8] = {
+	VL6180_ALS_GAIN_1,    VL6180_ALS_GAIN_1_25,
+	VL6180_ALS_GAIN_1_67, VL6180_ALS_GAIN_2_5,
+	VL6180_ALS_GAIN_5,    VL6180_ALS_GAIN_10,
+	VL6180_ALS_GAIN_20,   VL6180_ALS_GAIN_40
 };
 
 static int vl6180_read_raw(struct iio_dev *indio_dev,
@@ -296,7 +296,7 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
 				int *val, int *val2, long mask)
 {
 	struct vl6180_data *data = iio_priv(indio_dev);
-	int ret, i;
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -328,17 +328,11 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_HARDWAREGAIN:
-		ret = vl6180_read_byte(data->client, VL6180_ALS_GAIN);
-		if (ret < 0)
-			return -EINVAL;
-		for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
-			if (ret == vl6180_als_gain[i][2]) {
-				*val = vl6180_als_gain[i][0];
-				*val2 = vl6180_als_gain[i][1];
-			}
-		}
+		*val = data->als_gain_milli;
+		*val2 = 1000;
+
+		return IIO_VAL_FRACTIONAL;
 
-		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -364,25 +358,33 @@ static int vl6180_hold(struct vl6180_data *data, bool hold)
 
 static int vl6180_set_als_gain(struct vl6180_data *data, int val, int val2)
 {
-	int i, ret;
-
-	for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
-		if (val == vl6180_als_gain[i][0] &&
-			val2 == vl6180_als_gain[i][1]) {
-			mutex_lock(&data->lock);
-			ret = vl6180_hold(data, true);
-			if (ret < 0)
-				goto fail;
-			ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
-				vl6180_als_gain[i][3]);
-fail:
-			vl6180_hold(data, false);
-			mutex_unlock(&data->lock);
-			return ret;
-		}
-	}
+	int i, ret, gain;
+
+	if (val < 1 || val > 40)
+		return -EINVAL;
+
+	gain = (val * 1000000 + val2) / 1000;
+	if (gain < 1 || gain > 40000)
+		return -EINVAL;
+
+	i = find_closest(gain, vl6180_als_gain_tab,
+			 ARRAY_SIZE(vl6180_als_gain_tab));
+
+	mutex_lock(&data->lock);
+	ret = vl6180_hold(data, true);
+	if (ret < 0)
+		goto fail;
 
-	return -EINVAL;
+	ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
+				vl6180_als_gain_tab_bits[i]);
+
+	if (ret >= 0)
+		data->als_gain_milli = vl6180_als_gain_tab[i];
+
+fail:
+	vl6180_hold(data, false);
+	mutex_unlock(&data->lock);
+	return ret;
 }
 
 static int vl6180_set_it(struct vl6180_data *data, int val, int val2)
@@ -480,6 +482,7 @@ static int vl6180_init(struct vl6180_data *data)
 		return ret;
 
 	/* ALS gain: 1 */
+	data->als_gain_milli = 1000;
 	ret = vl6180_write_byte(client, VL6180_ALS_GAIN, VL6180_ALS_GAIN_1);
 	if (ret < 0)
 		return ret;
-- 
2.14.1

[PATCH v2 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time

[Stefan Brüns]

The reported scale was only correct for the default settings of 100 ms
integration time and gain 1.

This aligns the reported scale with the behaviour of any other IIO driver
and the documented ABI, but may require userspace changes if someone uses
non-default settings.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/iio/light/vl6180.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 5de64bce305c..52dfa2de88f7 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -315,9 +315,12 @@ static int vl6180_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_LIGHT:
-			*val = 0; /* one ALS count is 0.32 Lux */
-			*val2 = 320000;
-			break;
+			/* one ALS count is 0.32 Lux @ gain 1, IT 100 ms */
+			*val = 32000; /* 0.32 * 1000 * 100 */
+			*val2 = data->als_gain_milli * data->als_it_ms;
+
+			return IIO_VAL_FRACTIONAL;
+
 		case IIO_DISTANCE:
 			*val = 0; /* sensor reports mm, scale to meter */
 			*val2 = 1000;
-- 
2.14.1