All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup
       [not found] <20170924215920.7465-1-stefan.bruens@rwth-aachen.de>
@ 2017-09-24 21:59 ` Stefan Brüns
  2017-09-30 20:21   ` Jonathan Cameron
  2017-09-24 21:59 ` [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register Stefan Brüns
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Stefan Brüns @ 2017-09-24 21:59 UTC (permalink / raw)
  To: linux-iio
  Cc: Rob Herring, Peter Meerwald-Stadler, Stefan Brüns,
	linux-kernel, Manivannan Sadhasivam, Lars-Peter Clausen,
	Jonathan Cameron, Hartmut Knaack

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>

---

Changes in v3:
- Drop whitespace change

Changes in v2:
- Removed redundant parenthesis

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

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 6e25b724d941..3b6351b89ce7 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,10 +416,8 @@ 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;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register
       [not found] <20170924215920.7465-1-stefan.bruens@rwth-aachen.de>
  2017-09-24 21:59 ` [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup Stefan Brüns
@ 2017-09-24 21:59 ` Stefan Brüns
  2017-09-30 20:23   ` Jonathan Cameron
  2017-09-24 21:59 ` [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback Stefan Brüns
  2017-09-24 21:59 ` [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time Stefan Brüns
  3 siblings, 1 reply; 8+ messages in thread
From: Stefan Brüns @ 2017-09-24 21:59 UTC (permalink / raw)
  To: linux-iio
  Cc: Rob Herring, Peter Meerwald-Stadler, Stefan Brüns,
	linux-kernel, Manivannan Sadhasivam, Lars-Peter Clausen,
	Jonathan Cameron, Hartmut Knaack

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>

---

Changes in v3:
- Use IIO_VAL_FRACTIONAL for integration time return value

Changes in v2: None

 drivers/iio/light/vl6180.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index 3b6351b89ce7..9b056c83a90a 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 = data->als_it_ms;
+		*val2 = 1000;
+
+		return IIO_VAL_FRACTIONAL;
 
-		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);
@@ -471,6 +473,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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback
       [not found] <20170924215920.7465-1-stefan.bruens@rwth-aachen.de>
  2017-09-24 21:59 ` [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup Stefan Brüns
  2017-09-24 21:59 ` [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register Stefan Brüns
@ 2017-09-24 21:59 ` Stefan Brüns
  2017-09-30 20:29   ` Jonathan Cameron
  2017-09-24 21:59 ` [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time Stefan Brüns
  3 siblings, 1 reply; 8+ messages in thread
From: Stefan Brüns @ 2017-09-24 21:59 UTC (permalink / raw)
  To: linux-iio
  Cc: Rob Herring, Peter Meerwald-Stadler, Stefan Brüns,
	linux-kernel, Manivannan Sadhasivam, Lars-Peter Clausen,
	Jonathan Cameron, Hartmut Knaack

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>

---

Changes in v3: None
Changes in 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 9b056c83a90a..49e9f92cd116 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;
 
-	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;
+
+	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)
@@ -479,6 +481,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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time
       [not found] <20170924215920.7465-1-stefan.bruens@rwth-aachen.de>
                   ` (2 preceding siblings ...)
  2017-09-24 21:59 ` [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback Stefan Brüns
@ 2017-09-24 21:59 ` Stefan Brüns
  2017-09-30 20:31   ` Jonathan Cameron
  3 siblings, 1 reply; 8+ messages in thread
From: Stefan Brüns @ 2017-09-24 21:59 UTC (permalink / raw)
  To: linux-iio
  Cc: Rob Herring, Peter Meerwald-Stadler, Stefan Brüns,
	linux-kernel, Manivannan Sadhasivam, Lars-Peter Clausen,
	Jonathan Cameron, Hartmut Knaack

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>

---

Changes in v3: None
Changes in v2: None

 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 49e9f92cd116..67f8beb84fc3 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup
  2017-09-24 21:59 ` [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup Stefan Brüns
@ 2017-09-30 20:21   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2017-09-30 20:21 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Rob Herring, Peter Meerwald-Stadler, linux-kernel,
	Manivannan Sadhasivam, Lars-Peter Clausen, Hartmut Knaack

On Sun, 24 Sep 2017 23:59:17 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> 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>

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3:
> - Drop whitespace change
> 
> Changes in v2:
> - Removed redundant parenthesis
> 
>  drivers/iio/light/vl6180.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 6e25b724d941..3b6351b89ce7 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,10 +416,8 @@ 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;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register
  2017-09-24 21:59 ` [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register Stefan Brüns
@ 2017-09-30 20:23   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2017-09-30 20:23 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Rob Herring, Peter Meerwald-Stadler, linux-kernel,
	Manivannan Sadhasivam, Lars-Peter Clausen, Hartmut Knaack

On Sun, 24 Sep 2017 23:59:18 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> 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>

Applied to the togreg branch of iio.git and pushed out as testing.

Jonathan
> 
> ---
> 
> Changes in v3:
> - Use IIO_VAL_FRACTIONAL for integration time return value
> 
> Changes in v2: None
> 
>  drivers/iio/light/vl6180.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index 3b6351b89ce7..9b056c83a90a 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 = data->als_it_ms;
> +		*val2 = 1000;
> +
> +		return IIO_VAL_FRACTIONAL;
>  
> -		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);
> @@ -471,6 +473,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;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback
  2017-09-24 21:59 ` [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback Stefan Brüns
@ 2017-09-30 20:29   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2017-09-30 20:29 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Rob Herring, Peter Meerwald-Stadler, linux-kernel,
	Manivannan Sadhasivam, Lars-Peter Clausen, Hartmut Knaack

On Sun, 24 Sep 2017 23:59:19 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> 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>
> 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changes in v3: None
> Changes in 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 9b056c83a90a..49e9f92cd116 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;
>  
> -	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;
> +
> +	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)
> @@ -479,6 +481,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;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time
  2017-09-24 21:59 ` [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time Stefan Brüns
@ 2017-09-30 20:31   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2017-09-30 20:31 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-iio, Rob Herring, Peter Meerwald-Stadler, linux-kernel,
	Manivannan Sadhasivam, Lars-Peter Clausen, Hartmut Knaack

On Sun, 24 Sep 2017 23:59:20 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> 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>

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Whilst this is clearly a fix, it isn't a regression.  As such the extent
of the changes mean I've queued this up for the next merge window rather
than going in quicker as a fix.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  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 49e9f92cd116..67f8beb84fc3 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;

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-09-30 20:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20170924215920.7465-1-stefan.bruens@rwth-aachen.de>
2017-09-24 21:59 ` [PATCH v3 1/4] iio: light: vl6180: Move range check to integration time setter, cleanup Stefan Brüns
2017-09-30 20:21   ` Jonathan Cameron
2017-09-24 21:59 ` [PATCH v3 2/4] iio: light: vl6180: Avoid readback of integration time register Stefan Brüns
2017-09-30 20:23   ` Jonathan Cameron
2017-09-24 21:59 ` [PATCH v3 3/4] iio: light: vl6180: Cleanup als_gain lookup, avoid register readback Stefan Brüns
2017-09-30 20:29   ` Jonathan Cameron
2017-09-24 21:59 ` [PATCH v3 4/4] iio: light: vl6180: Correct ALS scale for non-default gain/integration time Stefan Brüns
2017-09-30 20:31   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.