[PATCH 00/11] staging: iio: tsl2x7x: staging cleanups

[PATCH 00/11] staging: iio: tsl2x7x: staging cleanups

[Brian Masney]

Here is another set of staging cleanups for the tsl2x7x driver. The
driver is about ready to move out of staging. I want to test a few other
things first before submitting a patch to move the driver out of
staging.

High-level changes in this series:
- Interrupts and IIO events now work properly
. Split out interrupt and persistence variables into separate ALS and
  proximity variables to improve code readability.
- Simplify code, remove unnecessary code, and improve code readability.

# How to test the interrupts

cd /sys/bus/iio/devices/iio:device0

echo 0 > events/in_intensity0_thresh_falling_value
echo 512 > events/in_intensity0_thresh_rising_value
echo 1 > events/in_intensity0_thresh_rising_en

# proximity0 already has threshold range at [0,512]
echo 1 > events/in_proximity0_thresh_rising_en

Run iio_event_monitor tsl2772 and verify that events are sent properly.

Question: Should I use in_intensity0_thresh_either_en instead of
in_intensity0_thresh_{rising,falling}_en? The same goes for the
proximity0. This is the current contents of the events/ directory:

in_intensity0_thresh_either_period
in_intensity0_thresh_falling_en
in_intensity0_thresh_falling_value
in_intensity0_thresh_rising_en
in_intensity0_thresh_rising_value
in_proximity0_thresh_either_period
in_proximity0_thresh_falling_en
in_proximity0_thresh_falling_value
in_proximity0_thresh_rising_en
in_proximity0_thresh_rising_value

Brian Masney (11):
  staging: iio: tsl2x7x: remove unnecessary code
  staging: iio: tsl2x7x: correct interrupt handler trigger
  staging: iio: tsl2x7x: no need to clear interrupt flag when getting
    lux
  staging: iio: tsl2x7x: simplify tsl2x7x_prox_cal()
  staging: iio: tsl2x7x: split out als and prox interrupt settings
  staging: iio: tsl2x7x: make logging consistent and correct newlines
  staging: iio: tsl2x7x: split out als and prox persistence settings
  staging: iio: tsl2x7x: remove unused variables from tsl2x7x_get_lux()
  staging: iio: tsl2x7x: remove ch0 and ch1 variables from
    tsl2x7x_get_lux()
  staging: iio: tsl2x7x: put local variables in reverse Christmas tree
    order
  staging: iio: tsl2x7x: add copyright

 drivers/staging/iio/light/tsl2x7x.c | 300 ++++++++++--------------------------
 drivers/staging/iio/light/tsl2x7x.h |  16 +-
 2 files changed, 91 insertions(+), 225 deletions(-)

-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/11] staging: iio: tsl2x7x: remove unnecessary code

[Brian Masney]

As a follow up to the work in commit a0722d05a195 ("staging: iio:
tsl2x7x: convert mutex_trylock() to mutex_lock()"), this patch removes
the unnecessary calls to tsl2x7x_get_prox() and tsl2x7x_get_lux() in
tsl2x7x_event_handler(). Previously, these functions were locked with
mutex_trylock(), but that is no longer the case. This patch also removes
a comment that is no longer relevant about returning the last sample.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 82681300e106..82cf9d853b18 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1430,7 +1430,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 
 	/* What type of interrupt do we need to process */
 	if (ret & TSL2X7X_STA_PRX_INTR) {
-		tsl2x7x_get_prox(indio_dev); /* freshen data for ABI */
 		iio_push_event(indio_dev,
 			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY,
 						    0,
@@ -1440,7 +1439,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 	}
 
 	if (ret & TSL2X7X_STA_ALS_INTR) {
-		tsl2x7x_get_lux(indio_dev); /* freshen data for ABI */
 		iio_push_event(indio_dev,
 			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT,
 						    0,
@@ -1745,10 +1743,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 		return ret;
 	}
 
-	/*
-	 * ALS and PROX functions can be invoked via user space poll
-	 * or H/W interrupt. If busy return last sample.
-	 */
 	mutex_init(&chip->als_mutex);
 	mutex_init(&chip->prox_mutex);
 
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/11] staging: iio: tsl2x7x: remove unnecessary code

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:02 -0400
Brian Masney <masneyb@onstation.org> wrote:

> As a follow up to the work in commit a0722d05a195 ("staging: iio:
> tsl2x7x: convert mutex_trylock() to mutex_lock()"), this patch removes
> the unnecessary calls to tsl2x7x_get_prox() and tsl2x7x_get_lux() in
> tsl2x7x_event_handler(). Previously, these functions were locked with
> mutex_trylock(), but that is no longer the case. This patch also removes
> a comment that is no longer relevant about returning the last sample.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 82681300e106..82cf9d853b18 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1430,7 +1430,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  
>  	/* What type of interrupt do we need to process */
>  	if (ret & TSL2X7X_STA_PRX_INTR) {
> -		tsl2x7x_get_prox(indio_dev); /* freshen data for ABI */
>  		iio_push_event(indio_dev,
>  			       IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY,
>  						    0,
> @@ -1440,7 +1439,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  	}
>  
>  	if (ret & TSL2X7X_STA_ALS_INTR) {
> -		tsl2x7x_get_lux(indio_dev); /* freshen data for ABI */
>  		iio_push_event(indio_dev,
>  			       IIO_UNMOD_EVENT_CODE(IIO_LIGHT,
>  						    0,
> @@ -1745,10 +1743,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  		return ret;
>  	}
>  
> -	/*
> -	 * ALS and PROX functions can be invoked via user space poll
> -	 * or H/W interrupt. If busy return last sample.
> -	 */
>  	mutex_init(&chip->als_mutex);
>  	mutex_init(&chip->prox_mutex);
>  

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/11] staging: iio: tsl2x7x: correct interrupt handler trigger

[Brian Masney]

tsl2x7x_event_handler() was not called as expected when the device was
asserting a hardware interrupt. This patch changes the interrupt line
trigger from rising to falling.

The driver was tested on a TSL2772 hooked up to a Raspberry Pi 2. The
interrupt pin also had a 10K pull-up resistor per the requirements from
the datasheet. The relevant device tree binding:

&i2c1 {
	tsl2772@39 {
		compatible = "amstaos,tsl2772";
		reg = <0x39>;
		interrupt-parent = <&gpio>;
		interrupts = <22 0x2>;
	};
};

With this patch, iio_event_monitor now shows the events when the
channels are outside the defined interrupt thresholds.

$ sudo ./iio_event_monitor tsl2772
Found IIO device with name tsl2772 with device number 0
Event: time: 1478193460053760446, type: proximity, channel: 0, evtype:
thresh, direction: either
...
Event: time: 1478193463020270185, type: illuminance, channel: 0, evtype:
thresh, direction: either
...

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 82cf9d853b18..59921850a226 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1763,7 +1763,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 		ret = devm_request_threaded_irq(&clientp->dev, clientp->irq,
 						NULL,
 						&tsl2x7x_event_handler,
-						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
 						IRQF_ONESHOT,
 						"TSL2X7X_event",
 						indio_dev);
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/11] staging: iio: tsl2x7x: correct interrupt handler trigger

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:03 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_event_handler() was not called as expected when the device was
> asserting a hardware interrupt. This patch changes the interrupt line
> trigger from rising to falling.
I guess the original test board used for driver development must have
inverted this for some reason and hence was miss configured.

Anyhow, good catch.
Applied to the togreg branch of iio.git.

Thanks,

Jonathan

> 
> The driver was tested on a TSL2772 hooked up to a Raspberry Pi 2. The
> interrupt pin also had a 10K pull-up resistor per the requirements from
> the datasheet. The relevant device tree binding:
> 
> &i2c1 {
> 	tsl2772@39 {
> 		compatible = "amstaos,tsl2772";
> 		reg = <0x39>;
> 		interrupt-parent = <&gpio>;
> 		interrupts = <22 0x2>;
> 	};
> };
> 
> With this patch, iio_event_monitor now shows the events when the
> channels are outside the defined interrupt thresholds.
> 
> $ sudo ./iio_event_monitor tsl2772
> Found IIO device with name tsl2772 with device number 0
> Event: time: 1478193460053760446, type: proximity, channel: 0, evtype:
> thresh, direction: either
> ...
> Event: time: 1478193463020270185, type: illuminance, channel: 0, evtype:
> thresh, direction: either
> ...
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 82cf9d853b18..59921850a226 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1763,7 +1763,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  		ret = devm_request_threaded_irq(&clientp->dev, clientp->irq,
>  						NULL,
>  						&tsl2x7x_event_handler,
> -						IRQF_TRIGGER_RISING |
> +						IRQF_TRIGGER_FALLING |
>  						IRQF_ONESHOT,
>  						"TSL2X7X_event",
>  						indio_dev);

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/11] staging: iio: tsl2x7x: no need to clear interrupt flag when getting lux

[Brian Masney]

tsl2x7x_get_lux() does not need to clear the interrupt flag when
querying the ALS. The interrupt flag is cleared in
tsl2x7x_event_handler(). This patches removes the unnecessary code.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 59921850a226..9c929e273135 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -387,10 +387,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		buf[i] = ret;
 	}
 
-	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_ALS_INT_CLR);
-	if (ret < 0)
-		goto out_unlock;
-
 	/* extract ALS/lux data */
 	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
 	ch1 = le16_to_cpup((const __le16 *)&buf[2]);
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/11] staging: iio: tsl2x7x: no need to clear interrupt flag when getting lux

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:04 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_get_lux() does not need to clear the interrupt flag when
> querying the ALS. The interrupt flag is cleared in
> tsl2x7x_event_handler(). This patches removes the unnecessary code.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 59921850a226..9c929e273135 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -387,10 +387,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		buf[i] = ret;
>  	}
>  
> -	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_ALS_INT_CLR);
> -	if (ret < 0)
> -		goto out_unlock;
> -
>  	/* extract ALS/lux data */
>  	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
>  	ch1 = le16_to_cpup((const __le16 *)&buf[2]);

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/11] staging: iio: tsl2x7x: simplify tsl2x7x_prox_cal()

[Brian Masney]

tsl2x7x_prox_cal() would set the interrupt flag, and reset the device to
start doing the calibration routine. However, this did not actually
affect the readings since they are polled. This patch drops the interrupt
code.

This patch also drops the function tsl2x7x_prox_calculate() and removes
support for the standard deviation and min sample since those values
were not used.

Driver was tested using a TSL2772 hooked up to a Raspberry Pi 2. I
performed the following testing at various distances:

- Put hand in front of sensor and keep the sensor and hand stationary.
- Perform calibration routine.
- Run iio_event_monitor.
- Verify that a proximity event is triggered when my hand comes
  anywhere between the sensor and where I performed the calibration
  routine.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 107 +++++-------------------------------
 1 file changed, 15 insertions(+), 92 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 9c929e273135..99230d9313e1 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -149,13 +149,6 @@ struct tsl2x7x_als_info {
 	u16 lux;
 };
 
-struct tsl2x7x_prox_stat {
-	int min;
-	int max;
-	int mean;
-	unsigned long stddev;
-};
-
 struct tsl2x7x_chip_info {
 	int chan_table_elements;
 	struct iio_chan_spec		channel[4];
@@ -771,106 +764,36 @@ static int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
 	return ret;
 }
 
-static void tsl2x7x_prox_calculate(int *data, int length,
-				   struct tsl2x7x_prox_stat *stat)
-{
-	int i;
-	int sample_sum;
-	int tmp;
-
-	if (!length)
-		length = 1;
-
-	sample_sum = 0;
-	stat->min = INT_MAX;
-	stat->max = INT_MIN;
-	for (i = 0; i < length; i++) {
-		sample_sum += data[i];
-		stat->min = min(stat->min, data[i]);
-		stat->max = max(stat->max, data[i]);
-	}
-
-	stat->mean = sample_sum / length;
-	sample_sum = 0;
-	for (i = 0; i < length; i++) {
-		tmp = data[i] - stat->mean;
-		sample_sum += tmp * tmp;
-	}
-	stat->stddev = int_sqrt((long)sample_sum / length);
-}
-
-/**
- * tsl2x7x_prox_cal() - Calculates std. and sets thresholds.
- * @indio_dev:	pointer to IIO device
- *
- * Calculates a standard deviation based on the samples,
- * and sets the threshold accordingly.
- */
 static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 {
-	int prox_history[MAX_SAMPLES_CAL + 1];
-	int i, ret;
-	struct tsl2x7x_prox_stat prox_stat_data[2];
-	struct tsl2x7x_prox_stat *cal;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-	u8 tmp_irq_settings;
-	u8 current_state = chip->tsl2x7x_chip_status;
-
-	if (chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL) {
-		dev_err(&chip->client->dev,
-			"max prox samples cal is too big: %d\n",
-			chip->settings.prox_max_samples_cal);
-		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
-	}
-
-	/* have to stop to change settings */
-	ret = tsl2x7x_chip_off(indio_dev);
-	if (ret < 0)
-		return ret;
-
-	/* Enable proximity detection save just in case prox not wanted yet*/
-	tmp_irq_settings = chip->settings.interrupts_en;
-	chip->settings.interrupts_en |= TSL2X7X_CNTL_PROX_INT_ENBL;
+	int prox_history[MAX_SAMPLES_CAL + 1];
+	int i, ret, mean, max, sample_sum;
 
-	/*turn on device if not already on*/
-	ret = tsl2x7x_chip_on(indio_dev);
-	if (ret < 0)
-		return ret;
+	if (chip->settings.prox_max_samples_cal < 1 ||
+	    chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL)
+		return -EINVAL;
 
-	/*gather the samples*/
 	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
 		usleep_range(15000, 17500);
 		ret = tsl2x7x_get_prox(indio_dev);
 		if (ret < 0)
 			return ret;
+
 		prox_history[i] = chip->prox_data;
-		dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",
-			 i, chip->prox_data);
 	}
 
-	ret = tsl2x7x_chip_off(indio_dev);
-	if (ret < 0)
-		return ret;
-	cal = &prox_stat_data[PROX_STAT_CAL];
-	tsl2x7x_prox_calculate(prox_history,
-			       chip->settings.prox_max_samples_cal, cal);
-	chip->settings.prox_thres_high = (cal->max << 1) - cal->mean;
-
-	dev_info(&chip->client->dev, " cal min=%d mean=%d max=%d\n",
-		 cal->min, cal->mean, cal->max);
-	dev_info(&chip->client->dev,
-		 "%s proximity threshold set to %d\n",
-		 chip->client->name, chip->settings.prox_thres_high);
-
-	/* back to the way they were */
-	chip->settings.interrupts_en = tmp_irq_settings;
-	if (current_state == TSL2X7X_CHIP_WORKING) {
-		ret = tsl2x7x_chip_on(indio_dev);
-		if (ret < 0)
-			return ret;
+	sample_sum = 0;
+	max = INT_MIN;
+	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
+		sample_sum += prox_history[i];
+		max = max(max, prox_history[i]);
 	}
+	mean = sample_sum / chip->settings.prox_max_samples_cal;
 
-	return 0;
+	chip->settings.prox_thres_high = (max << 1) - mean;
+
+	return tsl2x7x_invoke_change(indio_dev);
 }
 
 static ssize_t
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/11] staging: iio: tsl2x7x: simplify tsl2x7x_prox_cal()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:05 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_prox_cal() would set the interrupt flag, and reset the device to
> start doing the calibration routine. However, this did not actually
> affect the readings since they are polled. This patch drops the interrupt
> code.
> 
> This patch also drops the function tsl2x7x_prox_calculate() and removes
> support for the standard deviation and min sample since those values
> were not used.
> 
> Driver was tested using a TSL2772 hooked up to a Raspberry Pi 2. I
> performed the following testing at various distances:
> 
> - Put hand in front of sensor and keep the sensor and hand stationary.
> - Perform calibration routine.
> - Run iio_event_monitor.
> - Verify that a proximity event is triggered when my hand comes
>   anywhere between the sensor and where I performed the calibration
>   routine.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.  Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 107 +++++-------------------------------
>  1 file changed, 15 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 9c929e273135..99230d9313e1 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -149,13 +149,6 @@ struct tsl2x7x_als_info {
>  	u16 lux;
>  };
>  
> -struct tsl2x7x_prox_stat {
> -	int min;
> -	int max;
> -	int mean;
> -	unsigned long stddev;
> -};
> -
>  struct tsl2x7x_chip_info {
>  	int chan_table_elements;
>  	struct iio_chan_spec		channel[4];
> @@ -771,106 +764,36 @@ static int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
>  	return ret;
>  }
>  
> -static void tsl2x7x_prox_calculate(int *data, int length,
> -				   struct tsl2x7x_prox_stat *stat)
> -{
> -	int i;
> -	int sample_sum;
> -	int tmp;
> -
> -	if (!length)
> -		length = 1;
> -
> -	sample_sum = 0;
> -	stat->min = INT_MAX;
> -	stat->max = INT_MIN;
> -	for (i = 0; i < length; i++) {
> -		sample_sum += data[i];
> -		stat->min = min(stat->min, data[i]);
> -		stat->max = max(stat->max, data[i]);
> -	}
> -
> -	stat->mean = sample_sum / length;
> -	sample_sum = 0;
> -	for (i = 0; i < length; i++) {
> -		tmp = data[i] - stat->mean;
> -		sample_sum += tmp * tmp;
> -	}
> -	stat->stddev = int_sqrt((long)sample_sum / length);
> -}
> -
> -/**
> - * tsl2x7x_prox_cal() - Calculates std. and sets thresholds.
> - * @indio_dev:	pointer to IIO device
> - *
> - * Calculates a standard deviation based on the samples,
> - * and sets the threshold accordingly.
> - */
>  static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  {
> -	int prox_history[MAX_SAMPLES_CAL + 1];
> -	int i, ret;
> -	struct tsl2x7x_prox_stat prox_stat_data[2];
> -	struct tsl2x7x_prox_stat *cal;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	u8 tmp_irq_settings;
> -	u8 current_state = chip->tsl2x7x_chip_status;
> -
> -	if (chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL) {
> -		dev_err(&chip->client->dev,
> -			"max prox samples cal is too big: %d\n",
> -			chip->settings.prox_max_samples_cal);
> -		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
> -	}
> -
> -	/* have to stop to change settings */
> -	ret = tsl2x7x_chip_off(indio_dev);
> -	if (ret < 0)
> -		return ret;
> -
> -	/* Enable proximity detection save just in case prox not wanted yet*/
> -	tmp_irq_settings = chip->settings.interrupts_en;
> -	chip->settings.interrupts_en |= TSL2X7X_CNTL_PROX_INT_ENBL;
> +	int prox_history[MAX_SAMPLES_CAL + 1];
> +	int i, ret, mean, max, sample_sum;
>  
> -	/*turn on device if not already on*/
> -	ret = tsl2x7x_chip_on(indio_dev);
> -	if (ret < 0)
> -		return ret;
> +	if (chip->settings.prox_max_samples_cal < 1 ||
> +	    chip->settings.prox_max_samples_cal > MAX_SAMPLES_CAL)
> +		return -EINVAL;
>  
> -	/*gather the samples*/
>  	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
>  		usleep_range(15000, 17500);
>  		ret = tsl2x7x_get_prox(indio_dev);
>  		if (ret < 0)
>  			return ret;
> +
>  		prox_history[i] = chip->prox_data;
> -		dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",
> -			 i, chip->prox_data);
>  	}
>  
> -	ret = tsl2x7x_chip_off(indio_dev);
> -	if (ret < 0)
> -		return ret;
> -	cal = &prox_stat_data[PROX_STAT_CAL];
> -	tsl2x7x_prox_calculate(prox_history,
> -			       chip->settings.prox_max_samples_cal, cal);
> -	chip->settings.prox_thres_high = (cal->max << 1) - cal->mean;
> -
> -	dev_info(&chip->client->dev, " cal min=%d mean=%d max=%d\n",
> -		 cal->min, cal->mean, cal->max);
> -	dev_info(&chip->client->dev,
> -		 "%s proximity threshold set to %d\n",
> -		 chip->client->name, chip->settings.prox_thres_high);
> -
> -	/* back to the way they were */
> -	chip->settings.interrupts_en = tmp_irq_settings;
> -	if (current_state == TSL2X7X_CHIP_WORKING) {
> -		ret = tsl2x7x_chip_on(indio_dev);
> -		if (ret < 0)
> -			return ret;
> +	sample_sum = 0;
> +	max = INT_MIN;
> +	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
> +		sample_sum += prox_history[i];
> +		max = max(max, prox_history[i]);
>  	}
> +	mean = sample_sum / chip->settings.prox_max_samples_cal;
>  
> -	return 0;
> +	chip->settings.prox_thres_high = (max << 1) - mean;
> +
> +	return tsl2x7x_invoke_change(indio_dev);
>  }
>  
>  static ssize_t

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/11] staging: iio: tsl2x7x: split out als and prox interrupt settings

[Brian Masney]

The struct tsl2x7x_settings contained an interrupts_en member that was
a bitmask for which interrupts are enabled. This required having
bitmasks in several parts of the code. This patch splits this field
out into two booleans to remove most of the bitmasks in the code.

This patch also fixes a bug where if an interrupt pin was configured,
but proximity interrupts were disabled, then the proximity value could
not be polled.

This patch also removes an unnecessary second call to writing the
control register in tsl2x7x_chip_on().

Driver tested using a TSL2772 hooked up to a Raspberry Pi 2.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 64 ++++++++++++-------------------------
 drivers/staging/iio/light/tsl2x7x.h |  7 ++--
 2 files changed, 24 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 99230d9313e1..f7e7fcc17059 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -226,10 +226,11 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
 	.prox_config = 0,
 	.als_gain_trim = 1000,
 	.als_cal_target = 150,
+	.als_interrupt_en = false,
 	.als_thresh_low = 200,
 	.als_thresh_high = 256,
 	.persistence = 255,
-	.interrupts_en = 0,
+	.prox_interrupt_en = false,
 	.prox_thres_low  = 0,
 	.prox_thres_high = 512,
 	.prox_max_samples_cal = 30,
@@ -686,37 +687,22 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	/* Power-on settling time */
 	usleep_range(3000, 3500);
 
-	/*
-	 * NOW enable the ADC
-	 * initialize the desired mode of operation
-	 */
-	ret = tsl2x7x_write_control_reg(chip,
-					TSL2X7X_CNTL_PWR_ON |
-					TSL2X7X_CNTL_ADC_ENBL |
-					TSL2X7X_CNTL_PROX_DET_ENBL);
+	reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL |
+		  TSL2X7X_CNTL_PROX_DET_ENBL;
+	if (chip->settings.als_interrupt_en)
+		reg_val |= TSL2X7X_CNTL_ALS_INT_ENBL;
+	if (chip->settings.prox_interrupt_en)
+		reg_val |= TSL2X7X_CNTL_PROX_INT_ENBL;
+
+	ret = tsl2x7x_write_control_reg(chip, reg_val);
 	if (ret < 0)
 		return ret;
 
-	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
-
-	if (chip->settings.interrupts_en != 0) {
-		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
-
-		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
-		if (chip->settings.interrupts_en == 0x20 ||
-		    chip->settings.interrupts_en == 0x30)
-			reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
-
-		reg_val |= chip->settings.interrupts_en;
-		ret = tsl2x7x_write_control_reg(chip, reg_val);
-		if (ret < 0)
-			return ret;
+	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
+	if (ret < 0)
+		return ret;
 
-		ret = tsl2x7x_clear_interrupts(chip,
-					       TSL2X7X_CMD_PROXALS_INT_CLR);
-		if (ret < 0)
-			return ret;
-	}
+	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
 
 	return ret;
 }
@@ -978,14 +964,11 @@ static int tsl2x7x_read_interrupt_config(struct iio_dev *indio_dev,
 					 enum iio_event_direction dir)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-	int ret;
 
 	if (chan->type == IIO_INTENSITY)
-		ret = !!(chip->settings.interrupts_en & 0x10);
+		return chip->settings.als_interrupt_en;
 	else
-		ret = !!(chip->settings.interrupts_en & 0x20);
-
-	return ret;
+		return chip->settings.prox_interrupt_en;
 }
 
 static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
@@ -997,17 +980,10 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int ret;
 
-	if (chan->type == IIO_INTENSITY) {
-		if (val)
-			chip->settings.interrupts_en |= 0x10;
-		else
-			chip->settings.interrupts_en &= 0x20;
-	} else {
-		if (val)
-			chip->settings.interrupts_en |= 0x20;
-		else
-			chip->settings.interrupts_en &= 0x10;
-	}
+	if (chan->type == IIO_INTENSITY)
+		chip->settings.als_interrupt_en = val ? true : false;
+	else
+		chip->settings.prox_interrupt_en = val ? true : false;
 
 	ret = tsl2x7x_invoke_change(indio_dev);
 	if (ret < 0)
diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
index 28b0e7fdc9b8..b2aa642299b3 100644
--- a/drivers/staging/iio/light/tsl2x7x.h
+++ b/drivers/staging/iio/light/tsl2x7x.h
@@ -50,12 +50,12 @@ struct tsl2x7x_lux {
  *  @prox_config:           Prox configuration filters.
  *  @als_cal_target:        Known external ALS reading for
  *                          calibration.
- *  @interrupts_en:         Enable/Disable - 0x00 = none, 0x10 = als,
- *                                           0x20 = prx,  0x30 = bth
  *  @persistence:           H/W Filters, Number of 'out of limits'
  *                          ADC readings PRX/ALS.
+ *  @als_interrupt_en:      Enable/Disable ALS interrupts
  *  @als_thresh_low:        CH0 'low' count to trigger interrupt.
  *  @als_thresh_high:       CH0 'high' count to trigger interrupt.
+ *  @prox_interrupt_en:     Enable/Disable proximity interrupts
  *  @prox_thres_low:        Low threshold proximity detection.
  *  @prox_thres_high:       High threshold proximity detection
  *  @prox_pulse_count:      Number if proximity emitter pulses
@@ -70,10 +70,11 @@ struct tsl2x7x_settings {
 	int prox_gain;
 	int prox_config;
 	int als_cal_target;
-	u8  interrupts_en;
 	u8  persistence;
+	bool als_interrupt_en;
 	int als_thresh_low;
 	int als_thresh_high;
+	bool prox_interrupt_en;
 	int prox_thres_low;
 	int prox_thres_high;
 	int prox_pulse_count;
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 05/11] staging: iio: tsl2x7x: split out als and prox interrupt settings

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:06 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The struct tsl2x7x_settings contained an interrupts_en member that was
> a bitmask for which interrupts are enabled. This required having
> bitmasks in several parts of the code. This patch splits this field
> out into two booleans to remove most of the bitmasks in the code.
> 
> This patch also fixes a bug where if an interrupt pin was configured,
> but proximity interrupts were disabled, then the proximity value could
> not be polled.
> 
> This patch also removes an unnecessary second call to writing the
> control register in tsl2x7x_chip_on().
> 
> Driver tested using a TSL2772 hooked up to a Raspberry Pi 2.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 64 ++++++++++++-------------------------
>  drivers/staging/iio/light/tsl2x7x.h |  7 ++--
>  2 files changed, 24 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 99230d9313e1..f7e7fcc17059 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -226,10 +226,11 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
>  	.prox_config = 0,
>  	.als_gain_trim = 1000,
>  	.als_cal_target = 150,
> +	.als_interrupt_en = false,
>  	.als_thresh_low = 200,
>  	.als_thresh_high = 256,
>  	.persistence = 255,
> -	.interrupts_en = 0,
> +	.prox_interrupt_en = false,
>  	.prox_thres_low  = 0,
>  	.prox_thres_high = 512,
>  	.prox_max_samples_cal = 30,
> @@ -686,37 +687,22 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	/* Power-on settling time */
>  	usleep_range(3000, 3500);
>  
> -	/*
> -	 * NOW enable the ADC
> -	 * initialize the desired mode of operation
> -	 */
> -	ret = tsl2x7x_write_control_reg(chip,
> -					TSL2X7X_CNTL_PWR_ON |
> -					TSL2X7X_CNTL_ADC_ENBL |
> -					TSL2X7X_CNTL_PROX_DET_ENBL);
> +	reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL |
> +		  TSL2X7X_CNTL_PROX_DET_ENBL;
> +	if (chip->settings.als_interrupt_en)
> +		reg_val |= TSL2X7X_CNTL_ALS_INT_ENBL;
> +	if (chip->settings.prox_interrupt_en)
> +		reg_val |= TSL2X7X_CNTL_PROX_INT_ENBL;
> +
> +	ret = tsl2x7x_write_control_reg(chip, reg_val);
>  	if (ret < 0)
>  		return ret;
>  
> -	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
> -
> -	if (chip->settings.interrupts_en != 0) {
> -		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
> -
> -		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
> -		if (chip->settings.interrupts_en == 0x20 ||
> -		    chip->settings.interrupts_en == 0x30)
> -			reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
> -
> -		reg_val |= chip->settings.interrupts_en;
> -		ret = tsl2x7x_write_control_reg(chip, reg_val);
> -		if (ret < 0)
> -			return ret;
> +	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
> +	if (ret < 0)
> +		return ret;
>  
> -		ret = tsl2x7x_clear_interrupts(chip,
> -					       TSL2X7X_CMD_PROXALS_INT_CLR);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
>  	return ret;
>  }
> @@ -978,14 +964,11 @@ static int tsl2x7x_read_interrupt_config(struct iio_dev *indio_dev,
>  					 enum iio_event_direction dir)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	int ret;
>  
>  	if (chan->type == IIO_INTENSITY)
> -		ret = !!(chip->settings.interrupts_en & 0x10);
> +		return chip->settings.als_interrupt_en;
>  	else
> -		ret = !!(chip->settings.interrupts_en & 0x20);
> -
> -	return ret;
> +		return chip->settings.prox_interrupt_en;
>  }
>  
>  static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
> @@ -997,17 +980,10 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int ret;
>  
> -	if (chan->type == IIO_INTENSITY) {
> -		if (val)
> -			chip->settings.interrupts_en |= 0x10;
> -		else
> -			chip->settings.interrupts_en &= 0x20;
> -	} else {
> -		if (val)
> -			chip->settings.interrupts_en |= 0x20;
> -		else
> -			chip->settings.interrupts_en &= 0x10;
> -	}
> +	if (chan->type == IIO_INTENSITY)
> +		chip->settings.als_interrupt_en = val ? true : false;
> +	else
> +		chip->settings.prox_interrupt_en = val ? true : false;
>  
>  	ret = tsl2x7x_invoke_change(indio_dev);
>  	if (ret < 0)
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index 28b0e7fdc9b8..b2aa642299b3 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -50,12 +50,12 @@ struct tsl2x7x_lux {
>   *  @prox_config:           Prox configuration filters.
>   *  @als_cal_target:        Known external ALS reading for
>   *                          calibration.
> - *  @interrupts_en:         Enable/Disable - 0x00 = none, 0x10 = als,
> - *                                           0x20 = prx,  0x30 = bth
>   *  @persistence:           H/W Filters, Number of 'out of limits'
>   *                          ADC readings PRX/ALS.
> + *  @als_interrupt_en:      Enable/Disable ALS interrupts
>   *  @als_thresh_low:        CH0 'low' count to trigger interrupt.
>   *  @als_thresh_high:       CH0 'high' count to trigger interrupt.
> + *  @prox_interrupt_en:     Enable/Disable proximity interrupts
>   *  @prox_thres_low:        Low threshold proximity detection.
>   *  @prox_thres_high:       High threshold proximity detection
>   *  @prox_pulse_count:      Number if proximity emitter pulses
> @@ -70,10 +70,11 @@ struct tsl2x7x_settings {
>  	int prox_gain;
>  	int prox_config;
>  	int als_cal_target;
> -	u8  interrupts_en;
>  	u8  persistence;
> +	bool als_interrupt_en;
>  	int als_thresh_low;
>  	int als_thresh_high;
> +	bool prox_interrupt_en;
>  	int prox_thres_low;
>  	int prox_thres_high;
>  	int prox_pulse_count;

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/11] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Brian Masney]

This patch updates all of the logging commands so that they are
consistent with the other messages, includes __func__ in the message,
and all of the messages include newlines. This patch also removes some
debug log messages.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Changes since v1:
- Remove some debug messages and every log message doesn't have to have
  the function name.
- Earlier patches in this current series dropped some of the debug
  messages that were in v1.

 drivers/staging/iio/light/tsl2x7x.c | 42 ++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index f7e7fcc17059..07ce3076a05d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -374,7 +374,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		if (ret < 0) {
 			dev_err(&chip->client->dev,
-				"failed to read. err=%x\n", ret);
+				"%s: failed to read from register %x: %d\n",
+				__func__, reg, ret);
 			goto out_unlock;
 		}
 
@@ -416,7 +417,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 
 	/* note: lux is 31 bit max at this point */
 	if (ch1lux > ch0lux) {
-		dev_dbg(&chip->client->dev, "ch1lux > ch0lux-return last value\n");
+		dev_dbg(&chip->client->dev,
+			"%s: ch1lux > ch0lux; returning last value\n",
+			__func__);
 		ret = chip->als_cur_info.lux;
 		goto out_unlock;
 	}
@@ -591,8 +594,6 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
 		return -ERANGE;
 
 	chip->settings.als_gain_trim = ret;
-	dev_info(&chip->client->dev,
-		 "%s als_calibrate completed\n", chip->client->name);
 
 	return ret;
 }
@@ -674,12 +675,14 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	 */
 	for (i = 0, dev_reg = chip->tsl2x7x_config;
 			i < TSL2X7X_MAX_CONFIG_REG; i++) {
-		ret = i2c_smbus_write_byte_data(chip->client,
-						TSL2X7X_CMD_REG + i,
+		int reg = TSL2X7X_CMD_REG + i;
+
+		ret = i2c_smbus_write_byte_data(chip->client, reg,
 						*dev_reg++);
 		if (ret < 0) {
 			dev_err(&chip->client->dev,
-				"failed on write to reg %d.\n", i);
+				"%s: failed to write to register %x: %d\n",
+				__func__, reg, ret);
 			return ret;
 		}
 	}
@@ -907,15 +910,11 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 	 */
 	n = value[0];
 	if ((n % 3) || n < 6 ||
-	    n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3)) {
-		dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
+	    n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3))
 		return -EINVAL;
-	}
 
-	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
-		dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
+	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0)
 		return -EINVAL;
-	}
 
 	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
 		ret = tsl2x7x_chip_off(indio_dev);
@@ -1048,15 +1047,10 @@ static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
 			chip->settings.persistence &= 0xF0;
 			chip->settings.persistence |=
 				(filter_delay & 0x0F);
-			dev_info(&chip->client->dev, "%s: ALS persistence = %d",
-				 __func__, filter_delay);
 		} else {
 			chip->settings.persistence &= 0x0F;
 			chip->settings.persistence |=
 				((filter_delay << 4) & 0xF0);
-			dev_info(&chip->client->dev,
-				 "%s: Proximity persistence = %d",
-				 __func__, filter_delay);
 		}
 		ret = 0;
 		break;
@@ -1269,9 +1263,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_INT_TIME:
 		chip->settings.als_time =
 			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
-
-		dev_info(&chip->client->dev, "%s: als time = %d",
-			 __func__, chip->settings.als_time);
 		break;
 	default:
 		return -EINVAL;
@@ -1633,8 +1624,9 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 
 	ret = i2c_smbus_write_byte(clientp, TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
 	if (ret < 0) {
-		dev_err(&clientp->dev, "write to cmd reg failed. err = %d\n",
-			ret);
+		dev_err(&clientp->dev,
+			"%s: Failed to write to CMD register: %d\n",
+			__func__, ret);
 		return ret;
 	}
 
@@ -1664,7 +1656,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 						indio_dev);
 		if (ret) {
 			dev_err(&clientp->dev,
-				"%s: irq request failed", __func__);
+				"%s: irq request failed\n", __func__);
 			return ret;
 		}
 	}
@@ -1681,8 +1673,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 		return ret;
 	}
 
-	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
-
 	return 0;
 }
 
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/11] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:07 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch updates all of the logging commands so that they are
> consistent with the other messages, includes __func__ in the message,
> and all of the messages include newlines. This patch also removes some
> debug log messages.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. This is definitely an improvement, but at somepoint we may want
to drop all of the __func__ usage as the dynamic debug system can
provide back traces which obviously include this info.

So might need a follow up, though perhaps not pre moving out of staging
as this really isn't important.

Hence, applied.

Thanks,

Jonathan

> ---
> Changes since v1:
> - Remove some debug messages and every log message doesn't have to have
>   the function name.
> - Earlier patches in this current series dropped some of the debug
>   messages that were in v1.
> 
>  drivers/staging/iio/light/tsl2x7x.c | 42 ++++++++++++++-----------------------
>  1 file changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index f7e7fcc17059..07ce3076a05d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -374,7 +374,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		ret = i2c_smbus_read_byte_data(chip->client, reg);
>  		if (ret < 0) {
>  			dev_err(&chip->client->dev,
> -				"failed to read. err=%x\n", ret);
> +				"%s: failed to read from register %x: %d\n",
> +				__func__, reg, ret);
>  			goto out_unlock;
>  		}
>  
> @@ -416,7 +417,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  
>  	/* note: lux is 31 bit max at this point */
>  	if (ch1lux > ch0lux) {
> -		dev_dbg(&chip->client->dev, "ch1lux > ch0lux-return last value\n");
> +		dev_dbg(&chip->client->dev,
> +			"%s: ch1lux > ch0lux; returning last value\n",
> +			__func__);
>  		ret = chip->als_cur_info.lux;
>  		goto out_unlock;
>  	}
> @@ -591,8 +594,6 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  		return -ERANGE;
>  
>  	chip->settings.als_gain_trim = ret;
> -	dev_info(&chip->client->dev,
> -		 "%s als_calibrate completed\n", chip->client->name);
>  
>  	return ret;
>  }
> @@ -674,12 +675,14 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	 */
>  	for (i = 0, dev_reg = chip->tsl2x7x_config;
>  			i < TSL2X7X_MAX_CONFIG_REG; i++) {
> -		ret = i2c_smbus_write_byte_data(chip->client,
> -						TSL2X7X_CMD_REG + i,
> +		int reg = TSL2X7X_CMD_REG + i;
> +
> +		ret = i2c_smbus_write_byte_data(chip->client, reg,
>  						*dev_reg++);
>  		if (ret < 0) {
>  			dev_err(&chip->client->dev,
> -				"failed on write to reg %d.\n", i);
> +				"%s: failed to write to register %x: %d\n",
> +				__func__, reg, ret);
>  			return ret;
>  		}
>  	}
> @@ -907,15 +910,11 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  	 */
>  	n = value[0];
>  	if ((n % 3) || n < 6 ||
> -	    n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3)) {
> -		dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
> +	    n > ((ARRAY_SIZE(chip->tsl2x7x_device_lux) - 1) * 3))
>  		return -EINVAL;
> -	}
>  
> -	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
> -		dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
> +	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0)
>  		return -EINVAL;
> -	}
>  
>  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
>  		ret = tsl2x7x_chip_off(indio_dev);
> @@ -1048,15 +1047,10 @@ static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
>  			chip->settings.persistence &= 0xF0;
>  			chip->settings.persistence |=
>  				(filter_delay & 0x0F);
> -			dev_info(&chip->client->dev, "%s: ALS persistence = %d",
> -				 __func__, filter_delay);
>  		} else {
>  			chip->settings.persistence &= 0x0F;
>  			chip->settings.persistence |=
>  				((filter_delay << 4) & 0xF0);
> -			dev_info(&chip->client->dev,
> -				 "%s: Proximity persistence = %d",
> -				 __func__, filter_delay);
>  		}
>  		ret = 0;
>  		break;
> @@ -1269,9 +1263,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_INT_TIME:
>  		chip->settings.als_time =
>  			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
> -
> -		dev_info(&chip->client->dev, "%s: als time = %d",
> -			 __func__, chip->settings.als_time);
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -1633,8 +1624,9 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  
>  	ret = i2c_smbus_write_byte(clientp, TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
>  	if (ret < 0) {
> -		dev_err(&clientp->dev, "write to cmd reg failed. err = %d\n",
> -			ret);
> +		dev_err(&clientp->dev,
> +			"%s: Failed to write to CMD register: %d\n",
> +			__func__, ret);
>  		return ret;
>  	}
>  
> @@ -1664,7 +1656,7 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  						indio_dev);
>  		if (ret) {
>  			dev_err(&clientp->dev,
> -				"%s: irq request failed", __func__);
> +				"%s: irq request failed\n", __func__);
>  			return ret;
>  		}
>  	}
> @@ -1681,8 +1673,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  		return ret;
>  	}
>  
> -	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
> -
>  	return 0;
>  }
>  

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/11] staging: iio: tsl2x7x: split out als and prox persistence settings

[Brian Masney]

The struct tsl2x7x_settings contained a persistence member that
contained both the ALS and proximity persistence fields. This patch
splits this out into two separate fields so that the bitmasks in
several parts of the code are no longer necessary.

The default persistence settings are also changed by this patch from:

- Proximity: 0 (Every proximity cycle generates an interrupt)
- ALS: 255 (60 consecutive values out of range)

to something a little more reasonable based on my testing:

- Proximity: 1 (1 proximity value out of range)
- ALS: 1 (1 value outside of threshold range)

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 24 +++++++++++-------------
 drivers/staging/iio/light/tsl2x7x.h |  9 ++++++---
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 07ce3076a05d..c1e441857226 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -226,10 +226,11 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
 	.prox_config = 0,
 	.als_gain_trim = 1000,
 	.als_cal_target = 150,
+	.als_persistence = 1,
 	.als_interrupt_en = false,
 	.als_thresh_low = 200,
 	.als_thresh_high = 256,
-	.persistence = 255,
+	.prox_persistence = 1,
 	.prox_interrupt_en = false,
 	.prox_thres_low  = 0,
 	.prox_thres_high = 512,
@@ -621,7 +622,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 		(chip->settings.als_thresh_high) & 0xFF;
 	chip->tsl2x7x_config[TSL2X7X_ALS_MAXTHRESHHI] =
 		(chip->settings.als_thresh_high >> 8) & 0xFF;
-	chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] = chip->settings.persistence;
+	chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] =
+		(chip->settings.prox_persistence & 0xFF) << 4 |
+		(chip->settings.als_persistence & 0xFF);
 
 	chip->tsl2x7x_config[TSL2X7X_PRX_COUNT] =
 			chip->settings.prox_pulse_count;
@@ -1043,15 +1046,10 @@ static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
 
 		filter_delay = DIV_ROUND_UP((val * 1000) + val2, z);
 
-		if (chan->type == IIO_INTENSITY) {
-			chip->settings.persistence &= 0xF0;
-			chip->settings.persistence |=
-				(filter_delay & 0x0F);
-		} else {
-			chip->settings.persistence &= 0x0F;
-			chip->settings.persistence |=
-				((filter_delay << 4) & 0xF0);
-		}
+		if (chan->type == IIO_INTENSITY)
+			chip->settings.als_persistence = filter_delay;
+		else
+			chip->settings.prox_persistence = filter_delay;
 		ret = 0;
 		break;
 	default:
@@ -1108,10 +1106,10 @@ static int tsl2x7x_read_event_value(struct iio_dev *indio_dev,
 	case IIO_EV_INFO_PERIOD:
 		if (chan->type == IIO_INTENSITY) {
 			time = chip->settings.als_time;
-			mult = chip->settings.persistence & 0x0F;
+			mult = chip->settings.als_persistence;
 		} else {
 			time = chip->settings.prx_time;
-			mult = (chip->settings.persistence & 0xF0) >> 4;
+			mult = chip->settings.prox_persistence;
 		}
 
 		/* Determine integration time */
diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
index b2aa642299b3..d382cdbb976e 100644
--- a/drivers/staging/iio/light/tsl2x7x.h
+++ b/drivers/staging/iio/light/tsl2x7x.h
@@ -50,11 +50,13 @@ struct tsl2x7x_lux {
  *  @prox_config:           Prox configuration filters.
  *  @als_cal_target:        Known external ALS reading for
  *                          calibration.
- *  @persistence:           H/W Filters, Number of 'out of limits'
- *                          ADC readings PRX/ALS.
+ *  @als_persistence:       H/W Filters, Number of 'out of limits'
+ *                          ALS readings.
  *  @als_interrupt_en:      Enable/Disable ALS interrupts
  *  @als_thresh_low:        CH0 'low' count to trigger interrupt.
  *  @als_thresh_high:       CH0 'high' count to trigger interrupt.
+ *  @prox_persistence:      H/W Filters, Number of 'out of limits'
+ *                          proximity readings.
  *  @prox_interrupt_en:     Enable/Disable proximity interrupts
  *  @prox_thres_low:        Low threshold proximity detection.
  *  @prox_thres_high:       High threshold proximity detection
@@ -70,10 +72,11 @@ struct tsl2x7x_settings {
 	int prox_gain;
 	int prox_config;
 	int als_cal_target;
-	u8  persistence;
+	u8 als_persistence;
 	bool als_interrupt_en;
 	int als_thresh_low;
 	int als_thresh_high;
+	u8 prox_persistence;
 	bool prox_interrupt_en;
 	int prox_thres_low;
 	int prox_thres_high;
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 07/11] staging: iio: tsl2x7x: split out als and prox persistence settings

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:08 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The struct tsl2x7x_settings contained a persistence member that
> contained both the ALS and proximity persistence fields. This patch
> splits this out into two separate fields so that the bitmasks in
> several parts of the code are no longer necessary.
> 
> The default persistence settings are also changed by this patch from:
> 
> - Proximity: 0 (Every proximity cycle generates an interrupt)
> - ALS: 255 (60 consecutive values out of range)
> 
> to something a little more reasonable based on my testing:
> 
> - Proximity: 1 (1 proximity value out of range)
> - ALS: 1 (1 value outside of threshold range)
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied, thanks.

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 24 +++++++++++-------------
>  drivers/staging/iio/light/tsl2x7x.h |  9 ++++++---
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 07ce3076a05d..c1e441857226 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -226,10 +226,11 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
>  	.prox_config = 0,
>  	.als_gain_trim = 1000,
>  	.als_cal_target = 150,
> +	.als_persistence = 1,
>  	.als_interrupt_en = false,
>  	.als_thresh_low = 200,
>  	.als_thresh_high = 256,
> -	.persistence = 255,
> +	.prox_persistence = 1,
>  	.prox_interrupt_en = false,
>  	.prox_thres_low  = 0,
>  	.prox_thres_high = 512,
> @@ -621,7 +622,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  		(chip->settings.als_thresh_high) & 0xFF;
>  	chip->tsl2x7x_config[TSL2X7X_ALS_MAXTHRESHHI] =
>  		(chip->settings.als_thresh_high >> 8) & 0xFF;
> -	chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] = chip->settings.persistence;
> +	chip->tsl2x7x_config[TSL2X7X_PERSISTENCE] =
> +		(chip->settings.prox_persistence & 0xFF) << 4 |
> +		(chip->settings.als_persistence & 0xFF);
>  
>  	chip->tsl2x7x_config[TSL2X7X_PRX_COUNT] =
>  			chip->settings.prox_pulse_count;
> @@ -1043,15 +1046,10 @@ static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
>  
>  		filter_delay = DIV_ROUND_UP((val * 1000) + val2, z);
>  
> -		if (chan->type == IIO_INTENSITY) {
> -			chip->settings.persistence &= 0xF0;
> -			chip->settings.persistence |=
> -				(filter_delay & 0x0F);
> -		} else {
> -			chip->settings.persistence &= 0x0F;
> -			chip->settings.persistence |=
> -				((filter_delay << 4) & 0xF0);
> -		}
> +		if (chan->type == IIO_INTENSITY)
> +			chip->settings.als_persistence = filter_delay;
> +		else
> +			chip->settings.prox_persistence = filter_delay;
>  		ret = 0;
>  		break;
>  	default:
> @@ -1108,10 +1106,10 @@ static int tsl2x7x_read_event_value(struct iio_dev *indio_dev,
>  	case IIO_EV_INFO_PERIOD:
>  		if (chan->type == IIO_INTENSITY) {
>  			time = chip->settings.als_time;
> -			mult = chip->settings.persistence & 0x0F;
> +			mult = chip->settings.als_persistence;
>  		} else {
>  			time = chip->settings.prx_time;
> -			mult = (chip->settings.persistence & 0xF0) >> 4;
> +			mult = chip->settings.prox_persistence;
>  		}
>  
>  		/* Determine integration time */
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index b2aa642299b3..d382cdbb976e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -50,11 +50,13 @@ struct tsl2x7x_lux {
>   *  @prox_config:           Prox configuration filters.
>   *  @als_cal_target:        Known external ALS reading for
>   *                          calibration.
> - *  @persistence:           H/W Filters, Number of 'out of limits'
> - *                          ADC readings PRX/ALS.
> + *  @als_persistence:       H/W Filters, Number of 'out of limits'
> + *                          ALS readings.
>   *  @als_interrupt_en:      Enable/Disable ALS interrupts
>   *  @als_thresh_low:        CH0 'low' count to trigger interrupt.
>   *  @als_thresh_high:       CH0 'high' count to trigger interrupt.
> + *  @prox_persistence:      H/W Filters, Number of 'out of limits'
> + *                          proximity readings.
>   *  @prox_interrupt_en:     Enable/Disable proximity interrupts
>   *  @prox_thres_low:        Low threshold proximity detection.
>   *  @prox_thres_high:       High threshold proximity detection
> @@ -70,10 +72,11 @@ struct tsl2x7x_settings {
>  	int prox_gain;
>  	int prox_config;
>  	int als_cal_target;
> -	u8  persistence;
> +	u8 als_persistence;
>  	bool als_interrupt_en;
>  	int als_thresh_low;
>  	int als_thresh_high;
> +	u8 prox_persistence;
>  	bool prox_interrupt_en;
>  	int prox_thres_low;
>  	int prox_thres_high;

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/11] staging: iio: tsl2x7x: remove unused variables from tsl2x7x_get_lux()

[Brian Masney]

tsl2x7x_get_lux() has a ch0lux and ch1lux variables that are not used
so this patch removes them.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index c1e441857226..1e38e8449f9e 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -344,8 +344,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	struct tsl2x7x_lux *p;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int i, ret;
-	u32 ch0lux = 0;
-	u32 ch1lux = 0;
 
 	mutex_lock(&chip->als_mutex);
 
@@ -416,15 +414,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 				   tsl2x7x_als_gain[chip->settings.als_gain]);
 	}
 
-	/* note: lux is 31 bit max at this point */
-	if (ch1lux > ch0lux) {
-		dev_dbg(&chip->client->dev,
-			"%s: ch1lux > ch0lux; returning last value\n",
-			__func__);
-		ret = chip->als_cur_info.lux;
-		goto out_unlock;
-	}
-
 	/* adjust for active time scale */
 	if (chip->als_time_scale == 0)
 		lux = 0;
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 08/11] staging: iio: tsl2x7x: remove unused variables from tsl2x7x_get_lux()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:09 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_get_lux() has a ch0lux and ch1lux variables that are not used
> so this patch removes them.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Had to argue with this one ;)

Applied,

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index c1e441857226..1e38e8449f9e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -344,8 +344,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	struct tsl2x7x_lux *p;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int i, ret;
> -	u32 ch0lux = 0;
> -	u32 ch1lux = 0;
>  
>  	mutex_lock(&chip->als_mutex);
>  
> @@ -416,15 +414,6 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  				   tsl2x7x_als_gain[chip->settings.als_gain]);
>  	}
>  
> -	/* note: lux is 31 bit max at this point */
> -	if (ch1lux > ch0lux) {
> -		dev_dbg(&chip->client->dev,
> -			"%s: ch1lux > ch0lux; returning last value\n",
> -			__func__);
> -		ret = chip->als_cur_info.lux;
> -		goto out_unlock;
> -	}
> -
>  	/* adjust for active time scale */
>  	if (chip->als_time_scale == 0)
>  		lux = 0;

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/11] staging: iio: tsl2x7x: remove ch0 and ch1 variables from tsl2x7x_get_lux()

[Brian Masney]

Remove the ch0 and ch1 variables from tsl2x7x_get_lux() and
write those values directly into the chip->als_cur_info.als_ch0
and chip->als_cur_info.als_ch01 variables.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 1e38e8449f9e..65b7fbca8c5d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -336,7 +336,6 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
  */
 static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 {
-	u16 ch0, ch1; /* separated ch0/ch1 data from device */
 	u32 lux; /* raw lux calculated from device data */
 	u64 lux64;
 	u32 ratio;
@@ -382,24 +381,24 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	}
 
 	/* extract ALS/lux data */
-	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
-	ch1 = le16_to_cpup((const __le16 *)&buf[2]);
+	chip->als_cur_info.als_ch0 = le16_to_cpup((const __le16 *)&buf[0]);
+	chip->als_cur_info.als_ch1 = le16_to_cpup((const __le16 *)&buf[2]);
 
-	chip->als_cur_info.als_ch0 = ch0;
-	chip->als_cur_info.als_ch1 = ch1;
-
-	if (ch0 >= chip->als_saturation || ch1 >= chip->als_saturation) {
+	if (chip->als_cur_info.als_ch0 >= chip->als_saturation ||
+	    chip->als_cur_info.als_ch1 >= chip->als_saturation) {
 		lux = TSL2X7X_LUX_CALC_OVER_FLOW;
 		goto return_max;
 	}
 
-	if (!ch0) {
+	if (!chip->als_cur_info.als_ch0) {
 		/* have no data, so return LAST VALUE */
 		ret = chip->als_cur_info.lux;
 		goto out_unlock;
 	}
+
 	/* calculate ratio */
-	ratio = (ch1 << 15) / ch0;
+	ratio = (chip->als_cur_info.als_ch1 << 15) / chip->als_cur_info.als_ch0;
+
 	/* convert to unscaled lux using the pointer to the table */
 	p = (struct tsl2x7x_lux *)chip->tsl2x7x_device_lux;
 	while (p->ratio != 0 && p->ratio < ratio)
@@ -408,9 +407,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	if (p->ratio == 0) {
 		lux = 0;
 	} else {
-		lux = DIV_ROUND_UP(ch0 * p->ch0,
+		lux = DIV_ROUND_UP(chip->als_cur_info.als_ch0 * p->ch0,
 				   tsl2x7x_als_gain[chip->settings.als_gain]) -
-		      DIV_ROUND_UP(ch1 * p->ch1,
+		      DIV_ROUND_UP(chip->als_cur_info.als_ch1 * p->ch1,
 				   tsl2x7x_als_gain[chip->settings.als_gain]);
 	}
 
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/11] staging: iio: tsl2x7x: remove ch0 and ch1 variables from tsl2x7x_get_lux()

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:10 -0400
Brian Masney <masneyb@onstation.org> wrote:

> Remove the ch0 and ch1 variables from tsl2x7x_get_lux() and
> write those values directly into the chip->als_cur_info.als_ch0
> and chip->als_cur_info.als_ch01 variables.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. A marginal improvement in readability I suppose, but not a big one.
This I wouldn't have bothered changing but fair enough.

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

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 1e38e8449f9e..65b7fbca8c5d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -336,7 +336,6 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
>   */
>  static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  {
> -	u16 ch0, ch1; /* separated ch0/ch1 data from device */
>  	u32 lux; /* raw lux calculated from device data */
>  	u64 lux64;
>  	u32 ratio;
> @@ -382,24 +381,24 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	}
>  
>  	/* extract ALS/lux data */
> -	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
> -	ch1 = le16_to_cpup((const __le16 *)&buf[2]);
> +	chip->als_cur_info.als_ch0 = le16_to_cpup((const __le16 *)&buf[0]);
> +	chip->als_cur_info.als_ch1 = le16_to_cpup((const __le16 *)&buf[2]);
>  
> -	chip->als_cur_info.als_ch0 = ch0;
> -	chip->als_cur_info.als_ch1 = ch1;
> -
> -	if (ch0 >= chip->als_saturation || ch1 >= chip->als_saturation) {
> +	if (chip->als_cur_info.als_ch0 >= chip->als_saturation ||
> +	    chip->als_cur_info.als_ch1 >= chip->als_saturation) {
>  		lux = TSL2X7X_LUX_CALC_OVER_FLOW;
>  		goto return_max;
>  	}
>  
> -	if (!ch0) {
> +	if (!chip->als_cur_info.als_ch0) {
>  		/* have no data, so return LAST VALUE */
>  		ret = chip->als_cur_info.lux;
>  		goto out_unlock;
>  	}
> +
>  	/* calculate ratio */
> -	ratio = (ch1 << 15) / ch0;
> +	ratio = (chip->als_cur_info.als_ch1 << 15) / chip->als_cur_info.als_ch0;
> +
>  	/* convert to unscaled lux using the pointer to the table */
>  	p = (struct tsl2x7x_lux *)chip->tsl2x7x_device_lux;
>  	while (p->ratio != 0 && p->ratio < ratio)
> @@ -408,9 +407,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	if (p->ratio == 0) {
>  		lux = 0;
>  	} else {
> -		lux = DIV_ROUND_UP(ch0 * p->ch0,
> +		lux = DIV_ROUND_UP(chip->als_cur_info.als_ch0 * p->ch0,
>  				   tsl2x7x_als_gain[chip->settings.als_gain]) -
> -		      DIV_ROUND_UP(ch1 * p->ch1,
> +		      DIV_ROUND_UP(chip->als_cur_info.als_ch1 * p->ch1,
>  				   tsl2x7x_als_gain[chip->settings.als_gain]);
>  	}
>  

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/11] staging: iio: tsl2x7x: put local variables in reverse Christmas tree order

[Brian Masney]

This patch ensures that all of the local variable declarations are in
reverse Christmas tree order where possible to increase code
readability.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 65b7fbca8c5d..5f78664d2b0e 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -336,13 +336,12 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
  */
 static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 {
-	u32 lux; /* raw lux calculated from device data */
-	u64 lux64;
-	u32 ratio;
-	u8 buf[4];
-	struct tsl2x7x_lux *p;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	struct tsl2x7x_lux *p;
+	u32 lux, ratio;
 	int i, ret;
+	u64 lux64;
+	u8 buf[4];
 
 	mutex_lock(&chip->als_mutex);
 
@@ -589,13 +588,9 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
 
 static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 {
-	int i;
-	int ret = 0;
-	u8 *dev_reg;
-	int als_count;
-	int als_time;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-	u8 reg_val = 0;
+	int ret, i, als_count, als_time;
+	u8 *dev_reg, reg_val;
 
 	/* Non calculated parameters */
 	chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
@@ -1121,8 +1116,8 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
 			    int *val2,
 			    long mask)
 {
-	int ret = -EINVAL;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	int ret = -EINVAL;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
@@ -1583,9 +1578,9 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
 static int tsl2x7x_probe(struct i2c_client *clientp,
 			 const struct i2c_device_id *id)
 {
-	int ret;
 	struct iio_dev *indio_dev;
 	struct tsl2X7X_chip *chip;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(&clientp->dev, sizeof(*chip));
 	if (!indio_dev)
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/11] staging: iio: tsl2x7x: put local variables in reverse Christmas tree order

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:11 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch ensures that all of the local variable declarations are in
> reverse Christmas tree order where possible to increase code
> readability.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied. As ever it's a minor improvement but I suppose worth doing
whilst we are there!

Thanks,

Jonathan.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 65b7fbca8c5d..5f78664d2b0e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -336,13 +336,12 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
>   */
>  static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  {
> -	u32 lux; /* raw lux calculated from device data */
> -	u64 lux64;
> -	u32 ratio;
> -	u8 buf[4];
> -	struct tsl2x7x_lux *p;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	struct tsl2x7x_lux *p;
> +	u32 lux, ratio;
>  	int i, ret;
> +	u64 lux64;
> +	u8 buf[4];
>  
>  	mutex_lock(&chip->als_mutex);
>  
> @@ -589,13 +588,9 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  
>  static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  {
> -	int i;
> -	int ret = 0;
> -	u8 *dev_reg;
> -	int als_count;
> -	int als_time;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	u8 reg_val = 0;
> +	int ret, i, als_count, als_time;
> +	u8 *dev_reg, reg_val;
>  
>  	/* Non calculated parameters */
>  	chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
> @@ -1121,8 +1116,8 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
>  			    int *val2,
>  			    long mask)
>  {
> -	int ret = -EINVAL;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_PROCESSED:
> @@ -1583,9 +1578,9 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  static int tsl2x7x_probe(struct i2c_client *clientp,
>  			 const struct i2c_device_id *id)
>  {
> -	int ret;
>  	struct iio_dev *indio_dev;
>  	struct tsl2X7X_chip *chip;
> +	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&clientp->dev, sizeof(*chip));
>  	if (!indio_dev)

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/11] staging: iio: tsl2x7x: add copyright

[Brian Masney]

Add Brian Masney's copyright and to the list of module authors for all
of the staging cleanups. This patch also update's Jon Brenner's current
work email address since AMS now owns TAOS.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/staging/iio/light/tsl2x7x.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 5f78664d2b0e..77a81d75af4f 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -3,6 +3,7 @@
  * and proximity detection (prox) within the TAOS TSL2X7X family of devices.
  *
  * Copyright (c) 2012, TAOS Corporation.
+ * Copyright (c) 2017-2018 Brian Masney <masneyb@onstation.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -1744,6 +1745,7 @@ static struct i2c_driver tsl2x7x_driver = {
 
 module_i2c_driver(tsl2x7x_driver);
 
-MODULE_AUTHOR("J. August Brenner<jbrenner@taosinc.com>");
+MODULE_AUTHOR("J. August Brenner <Jon.Brenner@ams.com>");
+MODULE_AUTHOR("Brian Masney <masneyb@onstation.org>");
 MODULE_DESCRIPTION("TAOS tsl2x7x ambient and proximity light sensor driver");
 MODULE_LICENSE("GPL");
-- 
2.14.3

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 11/11] staging: iio: tsl2x7x: add copyright

[Jonathan Cameron]

On Wed, 21 Mar 2018 06:29:12 -0400
Brian Masney <masneyb@onstation.org> wrote:

> Add Brian Masney's copyright and to the list of module authors for all
> of the staging cleanups. This patch also update's Jon Brenner's current
> work email address since AMS now owns TAOS.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as testing.
Likely these will be going upstream after the coming merge window.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 5f78664d2b0e..77a81d75af4f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -3,6 +3,7 @@
>   * and proximity detection (prox) within the TAOS TSL2X7X family of devices.
>   *
>   * Copyright (c) 2012, TAOS Corporation.
> + * Copyright (c) 2017-2018 Brian Masney <masneyb@onstation.org>
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -1744,6 +1745,7 @@ static struct i2c_driver tsl2x7x_driver = {
>  
>  module_i2c_driver(tsl2x7x_driver);
>  
> -MODULE_AUTHOR("J. August Brenner<jbrenner@taosinc.com>");
> +MODULE_AUTHOR("J. August Brenner <Jon.Brenner@ams.com>");
> +MODULE_AUTHOR("Brian Masney <masneyb@onstation.org>");
>  MODULE_DESCRIPTION("TAOS tsl2x7x ambient and proximity light sensor driver");
>  MODULE_LICENSE("GPL");

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel