[PATCH 00/12] staging cleanups

[PATCH 00/12] staging cleanups

[Brian Masney]

Here is another patch series that inches the driver closer to moving out
of staging. The most interesting changes are the last two in the series.
One gets the proximity sensor working and the other one removes the
unnecessary in_proximity0_calibscale_available sysfs attribute from two
of the supported devices. The rest of the patches are not that
interesting and either reduce duplicate code, add error handling, or
other minor cosmetic changes.

Brian Masney (12):
  staging: iio: tsl2x7x: remove power functions from
    tsl2X7X_platform_data
  staging: iio: tsl2x7x: add common function for clearing interrupts
  staging: iio: tsl2x7x: add common function for reading chip status
  staging: iio: tsl2x7x: add common function for writing to the control
    register
  staging: iio: tsl2x7x: convert mutex_trylock() to mutex_lock()
  staging: iio: tsl2x7x: correctly return errors in tsl2x7x_get_prox()
  staging: iio: tsl2x7x: correct 'Avoid CamelCase' warning from
    checkpatch
  staging: iio: tsl2x7x: add error handling to tsl2x7x_prox_cal()
  staging: iio: tsl2x7x: add missing error checks
  staging: iio: tsl2x7x: make logging consistent and correct newlines
  staging: iio: tsl2x7x: remove unnecessary sysfs attribute
  staging: iio: tsl2x7x: make proximity sensor function correctly

 drivers/staging/iio/light/tsl2x7x.c | 346 ++++++++++++++++++------------------
 drivers/staging/iio/light/tsl2x7x.h |   6 +-
 2 files changed, 179 insertions(+), 173 deletions(-)

-- 
2.14.3

[PATCH 01/12] staging: iio: tsl2x7x: remove power functions from tsl2X7X_platform_data

[Brian Masney]

The tsl2X7X_platform_data structure contains the platform_power,
power_on, and power_off function pointers. These power management
functions should not be in the platform data. These functions were
likely used before the regulator framework was put in place. There are
no users of these functions in the mainline kernel.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 126e11530ce0..b7e3f966c3a6 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -588,9 +588,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	u8 reg_val = 0;
 
-	if (chip->pdata && chip->pdata->power_on)
-		chip->pdata->power_on(indio_dev);
-
 	/* Non calculated parameters */
 	chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
 	chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
@@ -736,9 +733,6 @@ static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
 	ret = i2c_smbus_write_byte_data(chip->client,
 					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
 
-	if (chip->pdata && chip->pdata->power_off)
-		chip->pdata->power_off(chip->client);
-
 	return ret;
 }
 
@@ -1792,12 +1786,6 @@ static int tsl2x7x_suspend(struct device *dev)
 		chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
 	}
 
-	if (chip->pdata && chip->pdata->platform_power) {
-		pm_message_t pmm = {PM_EVENT_SUSPEND};
-
-		chip->pdata->platform_power(dev, pmm);
-	}
-
 	return ret;
 }
 
@@ -1807,12 +1795,6 @@ static int tsl2x7x_resume(struct device *dev)
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int ret = 0;
 
-	if (chip->pdata && chip->pdata->platform_power) {
-		pm_message_t pmm = {PM_EVENT_RESUME};
-
-		chip->pdata->platform_power(dev, pmm);
-	}
-
 	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_SUSPENDED)
 		ret = tsl2x7x_chip_on(indio_dev);
 
diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
index df00f2ec1719..6624cbca7a83 100644
--- a/drivers/staging/iio/light/tsl2x7x.h
+++ b/drivers/staging/iio/light/tsl2x7x.h
@@ -21,7 +21,6 @@
 
 #ifndef __TSL2X7X_H
 #define __TSL2X7X_H
-#include <linux/pm.h>
 
 struct tsl2x7x_lux {
 	unsigned int ratio;
@@ -91,9 +90,6 @@ struct tsl2x7x_settings {
  *
  */
 struct tsl2X7X_platform_data {
-	int (*platform_power)(struct device *dev, pm_message_t);
-	int (*power_on)(struct iio_dev *indio_dev);
-	int (*power_off)(struct i2c_client *dev);
 	struct tsl2x7x_lux platform_lux_table[TSL2X7X_MAX_LUX_TABLE_SIZE];
 	struct tsl2x7x_settings *platform_default_settings;
 };
-- 
2.14.3

[PATCH 02/12] staging: iio: tsl2x7x: add common function for clearing interrupts

[Brian Masney]

There were three places where the same chunk of code was used to clear
interrupts. This patch creates a common function
tsl2x7x_clear_interrupts() to reduce duplicate code.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index b7e3f966c3a6..c02db03ef369 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -279,6 +279,20 @@ static const u8 device_channel_config[] = {
 	ALSPRX2
 };
 
+static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(chip->client,
+				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN | reg);
+	if (ret < 0)
+		dev_err(&chip->client->dev,
+			"%s: failed to clear interrupt status %x: %d\n",
+			__func__, reg, ret);
+
+	return ret;
+}
+
 /**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev:	pointer to IIO device
@@ -346,16 +360,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		buf[i] = ret;
 	}
 
-	/* clear any existing interrupt status */
-	ret = i2c_smbus_write_byte(chip->client,
-				   TSL2X7X_CMD_REG |
-				   TSL2X7X_CMD_SPL_FN |
-				   TSL2X7X_CMD_ALS_INT_CLR);
-	if (ret < 0) {
-		dev_err(&chip->client->dev,
-			"i2c_write_command failed - err = %d\n", ret);
-		goto out_unlock; /* have no data, so return failure */
-	}
+	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]);
@@ -706,17 +713,10 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
 				__func__);
 
-		/* Clear out any initial interrupts  */
-		ret = i2c_smbus_write_byte(chip->client,
-					   TSL2X7X_CMD_REG |
-					   TSL2X7X_CMD_SPL_FN |
-					   TSL2X7X_CMD_PROXALS_INT_CLR);
-		if (ret < 0) {
-			dev_err(&chip->client->dev,
-				"%s: Failed to clear Int status\n",
-				__func__);
-		return ret;
-		}
+		ret = tsl2x7x_clear_interrupts(chip,
+					       TSL2X7X_CMD_PROXALS_INT_CLR);
+		if (ret < 0)
+			return ret;
 	}
 
 	return ret;
@@ -1421,14 +1421,10 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 						    IIO_EV_DIR_EITHER),
 			       timestamp);
 	}
-	/* Clear interrupt now that we have handled it. */
-	ret = i2c_smbus_write_byte(chip->client,
-				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN |
-				   TSL2X7X_CMD_PROXALS_INT_CLR);
+
+	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
 	if (ret < 0)
-		dev_err(&chip->client->dev,
-			"Failed to clear irq from event handler. err = %d\n",
-			ret);
+		return ret;
 
 	return IRQ_HANDLED;
 }
-- 
2.14.3

[PATCH 03/12] staging: iio: tsl2x7x: add common function for reading chip status

[Brian Masney]

There were three places where the same chunk of code was used to read
the chip status. This patch creates a common function
tsl2x7x_read_status() to reduce duplicate code. This patch also corrects
tsl2x7x_event_handler() to properly check for an error after reading the
chip status.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index c02db03ef369..6bb622816660 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -293,6 +293,20 @@ static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
 	return ret;
 }
 
+static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(chip->client,
+				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
+	if (ret < 0)
+		dev_err(&chip->client->dev,
+			"%s: failed to read STATUS register: %d\n", __func__,
+			ret);
+
+	return ret;
+}
+
 /**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev:	pointer to IIO device
@@ -332,13 +346,10 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		goto out_unlock;
 	}
 
-	ret = i2c_smbus_read_byte_data(chip->client,
-				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
-	if (ret < 0) {
-		dev_err(&chip->client->dev,
-			"%s: Failed to read STATUS Reg\n", __func__);
+	ret = tsl2x7x_read_status(chip);
+	if (ret < 0)
 		goto out_unlock;
-	}
+
 	/* is data new & valid */
 	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
 		dev_err(&chip->client->dev,
@@ -458,12 +469,9 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 		return -EBUSY;
 	}
 
-	ret = i2c_smbus_read_byte_data(chip->client,
-				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
-	if (ret < 0) {
-		dev_err(&chip->client->dev, "i2c err=%d\n", ret);
+	ret = tsl2x7x_read_status(chip);
+	if (ret < 0)
 		goto prox_poll_err;
-	}
 
 	switch (chip->id) {
 	case tsl2571:
@@ -1396,13 +1404,13 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
-	u8 value;
 
-	value = i2c_smbus_read_byte_data(chip->client,
-					 TSL2X7X_CMD_REG | TSL2X7X_STATUS);
+	ret = tsl2x7x_read_status(chip);
+	if (ret < 0)
+		return ret;
 
 	/* What type of interrupt do we need to process */
-	if (value & TSL2X7X_STA_PRX_INTR) {
+	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,
@@ -1412,7 +1420,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 						    timestamp);
 	}
 
-	if (value & TSL2X7X_STA_ALS_INTR) {
+	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,
-- 
2.14.3

[PATCH 04/12] staging: iio: tsl2x7x: add common function for writing to the control register

[Brian Masney]

There were four places where the same chunk of code was used to write
to the control register. This patch creates a common function
tsl2x7x_write_control_reg() to reduce duplicate code.

The function tsl2x7x_chip_off() did not correctly return an error
code so this patch also corrects that issue.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 6bb622816660..cf16dd206c0b 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -307,6 +307,21 @@ static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
 	return ret;
 }
 
+static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(chip->client,
+					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, data);
+	if (ret < 0) {
+		dev_err(&chip->client->dev,
+			"%s: failed to write to control register %x: %d\n",
+			__func__, data, ret);
+	}
+
+	return ret;
+}
+
 /**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev:	pointer to IIO device
@@ -597,7 +612,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	int i;
 	int ret = 0;
 	u8 *dev_reg;
-	u8 utmp;
 	int als_count;
 	int als_time;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
@@ -659,14 +673,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	 * TSL2X7X Specific power-on / adc enable sequence
 	 * Power on the device 1st.
 	 */
-	utmp = TSL2X7X_CNTL_PWR_ON;
-	ret = i2c_smbus_write_byte_data(chip->client,
-					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
-	if (ret < 0) {
-		dev_err(&chip->client->dev,
-			"%s: failed on CNTRL reg.\n", __func__);
+	ret = tsl2x7x_write_control_reg(chip, TSL2X7X_CNTL_PWR_ON);
+	if (ret < 0)
 		return ret;
-	}
 
 	/*
 	 * Use the following shadow copy for our delay before enabling ADC.
@@ -691,16 +700,12 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	 * NOW enable the ADC
 	 * initialize the desired mode of operation
 	 */
-	utmp = TSL2X7X_CNTL_PWR_ON |
-			TSL2X7X_CNTL_ADC_ENBL |
-			TSL2X7X_CNTL_PROX_DET_ENBL;
-	ret = i2c_smbus_write_byte_data(chip->client,
-					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
-	if (ret < 0) {
-		dev_err(&chip->client->dev,
-			"%s: failed on 2nd CTRL reg.\n", __func__);
+	ret = tsl2x7x_write_control_reg(chip,
+					TSL2X7X_CNTL_PWR_ON |
+					TSL2X7X_CNTL_ADC_ENBL |
+					TSL2X7X_CNTL_PROX_DET_ENBL);
+	if (ret < 0)
 		return ret;
-	}
 
 	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
 
@@ -713,13 +718,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 			reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
 
 		reg_val |= chip->settings.interrupts_en;
-		ret = i2c_smbus_write_byte_data(chip->client,
-						TSL2X7X_CMD_REG | TSL2X7X_CNTRL,
-						reg_val);
+		ret = tsl2x7x_write_control_reg(chip, reg_val);
 		if (ret < 0)
-			dev_err(&chip->client->dev,
-				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
-				__func__);
+			return ret;
 
 		ret = tsl2x7x_clear_interrupts(chip,
 					       TSL2X7X_CMD_PROXALS_INT_CLR);
@@ -732,16 +733,11 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 
 static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
 {
-	int ret;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 
 	/* turn device off */
 	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
-
-	ret = i2c_smbus_write_byte_data(chip->client,
-					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
-
-	return ret;
+	return tsl2x7x_write_control_reg(chip, 0x00);
 }
 
 /**
-- 
2.14.3

[PATCH 05/12] staging: iio: tsl2x7x: convert mutex_trylock() to mutex_lock()

[Brian Masney]

The driver uses mutex_lock() and mutex_trylock() in several places.
Convert the mutex_trylock() to mutex_lock() for consistency with other
IIO light drivers in mainline.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index cf16dd206c0b..5c611250127f 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -350,8 +350,7 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	u32 ch0lux = 0;
 	u32 ch1lux = 0;
 
-	if (mutex_trylock(&chip->als_mutex) == 0)
-		return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
+	mutex_lock(&chip->als_mutex);
 
 	if (chip->tsl2x7x_chip_status != TSL2X7X_CHIP_WORKING) {
 		/* device is not enabled */
@@ -478,11 +477,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	u8 chdata[2];
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 
-	if (mutex_trylock(&chip->prox_mutex) == 0) {
-		dev_err(&chip->client->dev,
-			"%s: Can't get prox mutex\n", __func__);
-		return -EBUSY;
-	}
+	mutex_lock(&chip->prox_mutex);
 
 	ret = tsl2x7x_read_status(chip);
 	if (ret < 0)
-- 
2.14.3

[PATCH 06/12] staging: iio: tsl2x7x: correctly return errors in tsl2x7x_get_prox()

[Brian Masney]

Not all errors that occurred in tsl2x7x_get_prox() were correctly
reported in the return value. This patch changes the error handling
so that errors are now returned properly.

Note that the ret variable is from the call to tsl2x7x_read_status(),
and it already has the correct error check. The -EINVAL error code is
for an unexpected value in the register.

This patch also corrects an unnecessary word wrap in the call to
le16_to_cpup() while changes are being made to this code.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 5c611250127f..cc209b64ed5a 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -489,16 +489,20 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	case tmd2671:
 	case tsl2771:
 	case tmd2771:
-		if (!(ret & TSL2X7X_STA_ADC_VALID))
+		if (!(ret & TSL2X7X_STA_ADC_VALID)) {
+			ret = -EINVAL;
 			goto prox_poll_err;
+		}
 		break;
 	case tsl2572:
 	case tsl2672:
 	case tmd2672:
 	case tsl2772:
 	case tmd2772:
-		if (!(ret & TSL2X7X_STA_PRX_VALID))
+		if (!(ret & TSL2X7X_STA_PRX_VALID)) {
+			ret = -EINVAL;
 			goto prox_poll_err;
+		}
 		break;
 	}
 
@@ -512,14 +516,13 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 		chdata[i] = ret;
 	}
 
-	chip->prox_data =
-			le16_to_cpup((const __le16 *)&chdata[0]);
+	chip->prox_data = le16_to_cpup((const __le16 *)&chdata[0]);
+	ret = chip->prox_data;
 
 prox_poll_err:
-
 	mutex_unlock(&chip->prox_mutex);
 
-	return chip->prox_data;
+	return ret;
 }
 
 /**
-- 
2.14.3

[PATCH 07/12] staging: iio: tsl2x7x: correct 'Avoid CamelCase' warning from checkpatch

[Brian Masney]

The statP and calP variables triggered an 'Avoid CamelCase' warning
from checkpatch.pl. This patch renames these variables to stat and cal
to fix this warning.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index cc209b64ed5a..380a6c96a69a 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -773,7 +773,7 @@ static int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
 }
 
 static void tsl2x7x_prox_calculate(int *data, int length,
-				   struct tsl2x7x_prox_stat *statP)
+				   struct tsl2x7x_prox_stat *stat)
 {
 	int i;
 	int sample_sum;
@@ -783,21 +783,21 @@ static void tsl2x7x_prox_calculate(int *data, int length,
 		length = 1;
 
 	sample_sum = 0;
-	statP->min = INT_MAX;
-	statP->max = INT_MIN;
+	stat->min = INT_MAX;
+	stat->max = INT_MIN;
 	for (i = 0; i < length; i++) {
 		sample_sum += data[i];
-		statP->min = min(statP->min, data[i]);
-		statP->max = max(statP->max, data[i]);
+		stat->min = min(stat->min, data[i]);
+		stat->max = max(stat->max, data[i]);
 	}
 
-	statP->mean = sample_sum / length;
+	stat->mean = sample_sum / length;
 	sample_sum = 0;
 	for (i = 0; i < length; i++) {
-		tmp = data[i] - statP->mean;
+		tmp = data[i] - stat->mean;
 		sample_sum += tmp * tmp;
 	}
-	statP->stddev = int_sqrt((long)sample_sum / length);
+	stat->stddev = int_sqrt((long)sample_sum / length);
 }
 
 /**
@@ -812,7 +812,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 	int prox_history[MAX_SAMPLES_CAL + 1];
 	int i;
 	struct tsl2x7x_prox_stat prox_stat_data[2];
-	struct tsl2x7x_prox_stat *calP;
+	struct tsl2x7x_prox_stat *cal;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	u8 tmp_irq_settings;
 	u8 current_state = chip->tsl2x7x_chip_status;
@@ -844,13 +844,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 	}
 
 	tsl2x7x_chip_off(indio_dev);
-	calP = &prox_stat_data[PROX_STAT_CAL];
+	cal = &prox_stat_data[PROX_STAT_CAL];
 	tsl2x7x_prox_calculate(prox_history,
-			       chip->settings.prox_max_samples_cal, calP);
-	chip->settings.prox_thres_high = (calP->max << 1) - calP->mean;
+			       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",
-		 calP->min, calP->mean, calP->max);
+		 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);
-- 
2.14.3

[PATCH 08/12] staging: iio: tsl2x7x: add error handling to tsl2x7x_prox_cal()

[Brian Masney]

tsl2x7x_prox_cal() did not have any error checks. This patch adds
the missing error handling and ensures that any errors are reported to
user space via in_proximity0_calibrate_store().

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 380a6c96a69a..8adc6db44790 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -807,10 +807,10 @@ static void tsl2x7x_prox_calculate(int *data, int length,
  * Calculates a standard deviation based on the samples,
  * and sets the threshold accordingly.
  */
-static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
+static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 {
 	int prox_history[MAX_SAMPLES_CAL + 1];
-	int i;
+	int i, ret;
 	struct tsl2x7x_prox_stat prox_stat_data[2];
 	struct tsl2x7x_prox_stat *cal;
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
@@ -825,25 +825,33 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 	}
 
 	/* have to stop to change settings */
-	tsl2x7x_chip_off(indio_dev);
+	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;
 
 	/*turn on device if not already on*/
-	tsl2x7x_chip_on(indio_dev);
+	ret = tsl2x7x_chip_on(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	/*gather the samples*/
 	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
 		usleep_range(15000, 17500);
-		tsl2x7x_get_prox(indio_dev);
+		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);
 	}
 
-	tsl2x7x_chip_off(indio_dev);
+	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);
@@ -857,8 +865,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 
 	/* back to the way they were */
 	chip->settings.interrupts_en = tmp_irq_settings;
-	if (current_state == TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_on(indio_dev);
+	if (current_state == TSL2X7X_CHIP_WORKING) {
+		ret = tsl2x7x_chip_on(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 
 static ssize_t
@@ -1018,8 +1031,11 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
 	if (strtobool(buf, &value))
 		return -EINVAL;
 
-	if (value)
-		tsl2x7x_prox_cal(indio_dev);
+	if (value) {
+		ret = tsl2x7x_prox_cal(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = tsl2x7x_invoke_change(indio_dev);
 	if (ret < 0)
-- 
2.14.3

[PATCH 09/12] staging: iio: tsl2x7x: add missing error checks

[Brian Masney]

The functions in_illuminance0_calibrate_store() and
in_illuminance0_lux_table_store() did not have complete error handling
in place. This patch adds the missing error handling.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 8adc6db44790..da7a4e025083 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -940,8 +940,11 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 	if (strtobool(buf, &value))
 		return -EINVAL;
 
-	if (value)
-		tsl2x7x_als_calibrate(indio_dev);
+	if (value) {
+		ret = tsl2x7x_als_calibrate(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = tsl2x7x_invoke_change(indio_dev);
 	if (ret < 0)
@@ -1006,8 +1009,11 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 		return -EINVAL;
 	}
 
-	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_off(indio_dev);
+	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
+		ret = tsl2x7x_chip_off(indio_dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* Zero out the table */
 	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
-- 
2.14.3

[PATCH 10/12] 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 from tsl2x7x_prox_cal().

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index da7a4e025083..fb91c46c8747 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -378,7 +378,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;
 		}
 
@@ -424,7 +425,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;
 	}
@@ -600,7 +603,8 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
 
 	chip->settings.als_gain_trim = ret;
 	dev_info(&chip->client->dev,
-		 "%s als_calibrate completed\n", chip->client->name);
+		 "%s: %s ALS calibration successfully completed\n",
+		 __func__, chip->client->name);
 
 	return ret;
 }
@@ -644,7 +648,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	/* and make sure we're not already on */
 	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
 		/* if forcing a register update - turn off, then on */
-		dev_info(&chip->client->dev, "device is already enabled\n");
+		dev_info(&chip->client->dev, "%s: device is already enabled\n",
+			 __func__);
 		return -EINVAL;
 	}
 
@@ -681,12 +686,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;
 		}
 	}
@@ -708,7 +715,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
 
 	if (chip->settings.interrupts_en != 0) {
-		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
+		dev_info(&chip->client->dev, "%s: Setting up interrupt(s)\n",
+			 __func__);
 
 		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
 		if (chip->settings.interrupts_en == 0x20 ||
@@ -819,8 +827,8 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 
 	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);
+			"%s: prox_max_samples_cal %d is too big\n",
+			__func__, chip->settings.prox_max_samples_cal);
 		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
 	}
 
@@ -845,8 +853,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *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);
@@ -857,12 +863,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 			       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) {
@@ -1000,12 +1000,14 @@ 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);
+		dev_info(dev, "%s: lux table input error 1. n=%d\n",
+			 __func__, n);
 		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);
+		dev_info(dev, "%s: lux table input error 2. n=%d\n",
+			 __func__, n);
 		return -EINVAL;
 	}
 
@@ -1150,14 +1152,14 @@ 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",
+			dev_info(&chip->client->dev, "%s: ALS persistence = %d\n",
 				 __func__, filter_delay);
 		} else {
 			chip->settings.persistence &= 0x0F;
 			chip->settings.persistence |=
 				((filter_delay << 4) & 0xF0);
 			dev_info(&chip->client->dev,
-				 "%s: Proximity persistence = %d",
+				 "%s: Proximity persistence = %d\n",
 				 __func__, filter_delay);
 		}
 		ret = 0;
@@ -1372,7 +1374,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 		chip->settings.als_time =
 			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
 
-		dev_info(&chip->client->dev, "%s: als time = %d",
+		dev_info(&chip->client->dev, "%s: als time = %d\n",
 			 __func__, chip->settings.als_time);
 		break;
 	default:
@@ -1738,8 +1740,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;
 	}
 
@@ -1773,7 +1776,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;
 		}
 	}
@@ -1790,7 +1793,8 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 		return ret;
 	}
 
-	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
+	dev_info(&clientp->dev, "%s: %s Light sensor found.\n", __func__,
+		 id->name);
 
 	return 0;
 }
-- 
2.14.3

[PATCH 11/12] staging: iio: tsl2x7x: remove unnecessary sysfs attribute

[Brian Masney]

The tsl2771 and tmd2771 devices create the
in_proximity0_calibscale_available sysfs attribute. These two particular
devices do not support changing the proximity gain value on the
chip so this patch removes that sysfs attribute. As expected, these two
devices already did not create the IIO_CHAN_INFO_CALIBSCALE channel and
proximity0_calibrate sysfs attribute.

Page 38 of the tsl2772 data sheet shows that the proximity gain can be
adjusted with bits 2-3 on the control register:
https://ams.com/eng/content/download/291503/1066377/file/TSL2772_DS000181_2-00.pdf

Page 35 of the tsl2771 and tmd2771 data sheets shows that bits 2-3 on
the control register are reserved and changing the proximity gain is
not supported:
https://ams.com/eng/content/download/250264/976045/file/TSL2771_DS000105_3-00.pdf
https://ams.com/eng/content/download/250283/976077/file/TMD2771_DS000177_2-00.pdf

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index fb91c46c8747..8c29a52153c1 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1477,7 +1477,6 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
 	&dev_attr_in_illuminance0_lux_table.attr,
-	&iio_const_attr_in_proximity0_calibscale_available.dev_attr.attr,
 	NULL
 };
 
-- 
2.14.3

[PATCH 12/12] staging: iio: tsl2x7x: make proximity sensor function correctly

[Brian Masney]

The bits for setting up the proximity diode were not setup correctly and
this was causing the proximity sensor to not function correctly. This
patch sets up the correct bit mask in tsl2x7x_chip_on() based on what
the data sheet expects. From page 35 of the TSL2771 data sheet:
https://ams.com/eng/content/download/250264/976045/file/TSL2771_DS000105_3-00.pdf

- Bits 0-1 is the ALS gain control
- Bits 2-3 is reserved (The proximity gain control on other tsl2x7x chips)
- Bits 4-5 is the proximity diode select
- Bits 6-7 is the LED drive strength

tsl2x7x_chip_on() had the power and diode hardcoded, so these are
extracted out into the settings so that these fields can be configured
in the platform data.

The default prox_gain is changed from 1 (2X gain) to 0 (1X gain) since
the proximity gain control on the TSL2771, TMD2771, and other chips have
these fields listed as reserved, and to write 0 into those bits.

Verified that the proximity sensor now works correctly on a TSL2771
hooked up to a Raspberry Pi 2.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 8c29a52153c1..ab518cdec43e 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -109,15 +109,15 @@
 #define TSL2X7X_CNTL_INTPROXPON_ENBL	0x2F
 
 /*Prox diode to use */
-#define TSL2X7X_DIODE0			0x10
-#define TSL2X7X_DIODE1			0x20
-#define TSL2X7X_DIODE_BOTH		0x30
+#define TSL2X7X_DIODE0			0x01
+#define TSL2X7X_DIODE1			0x02
+#define TSL2X7X_DIODE_BOTH		0x03
 
 /* LED Power */
 #define TSL2X7X_100_mA			0x00
-#define TSL2X7X_50_mA			0x40
-#define TSL2X7X_25_mA			0x80
-#define TSL2X7X_13_mA			0xD0
+#define TSL2X7X_50_mA			0x01
+#define TSL2X7X_25_mA			0x02
+#define TSL2X7X_13_mA			0x03
 #define TSL2X7X_MAX_TIMER_CNT		0xFF
 
 #define TSL2X7X_MIN_ITIME		3
@@ -228,7 +228,7 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
 	.als_time = 219, /* 101 ms */
 	.als_gain = 0,
 	.prx_time = 254, /* 5.4 ms */
-	.prox_gain = 1,
+	.prox_gain = 0,
 	.wait_time = 245,
 	.prox_config = 0,
 	.als_gain_trim = 1000,
@@ -240,7 +240,9 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
 	.prox_thres_low  = 0,
 	.prox_thres_high = 512,
 	.prox_max_samples_cal = 30,
-	.prox_pulse_count = 8
+	.prox_pulse_count = 8,
+	.prox_diode = TSL2X7X_DIODE1,
+	.prox_power = TSL2X7X_100_mA
 };
 
 static const s16 tsl2x7x_als_gain[] = {
@@ -664,9 +666,10 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 
 	/* Set the gain based on tsl2x7x_settings struct */
 	chip->tsl2x7x_config[TSL2X7X_GAIN] =
-		chip->settings.als_gain |
-			(TSL2X7X_100_mA | TSL2X7X_DIODE1) |
-			(chip->settings.prox_gain << 2);
+		(chip->settings.als_gain & 0xFF) |
+		((chip->settings.prox_gain & 0xFF) << 2) |
+		(chip->settings.prox_diode << 4) |
+		(chip->settings.prox_power << 6);
 
 	/* set chip struct re scaling and saturation */
 	chip->als_saturation = als_count * 922; /* 90% of full scale */
diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
index 6624cbca7a83..28b0e7fdc9b8 100644
--- a/drivers/staging/iio/light/tsl2x7x.h
+++ b/drivers/staging/iio/light/tsl2x7x.h
@@ -78,6 +78,8 @@ struct tsl2x7x_settings {
 	int prox_thres_high;
 	int prox_pulse_count;
 	int prox_max_samples_cal;
+	int prox_diode;
+	int prox_power;
 };
 
 /**
-- 
2.14.3

Re: [PATCH 01/12] staging: iio: tsl2x7x: remove power functions from tsl2X7X_platform_data

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:31 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The tsl2X7X_platform_data structure contains the platform_power,
> power_on, and power_off function pointers. These power management
> functions should not be in the platform data. These functions were
> likely used before the regulator framework was put in place. There are
> no users of these functions in the mainline kernel.
> 
> 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 | 18 ------------------
>  drivers/staging/iio/light/tsl2x7x.h |  4 ----
>  2 files changed, 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 126e11530ce0..b7e3f966c3a6 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -588,9 +588,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	u8 reg_val = 0;
>  
> -	if (chip->pdata && chip->pdata->power_on)
> -		chip->pdata->power_on(indio_dev);
> -
>  	/* Non calculated parameters */
>  	chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
>  	chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
> @@ -736,9 +733,6 @@ static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
>  	ret = i2c_smbus_write_byte_data(chip->client,
>  					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
>  
> -	if (chip->pdata && chip->pdata->power_off)
> -		chip->pdata->power_off(chip->client);
> -
>  	return ret;
>  }
>  
> @@ -1792,12 +1786,6 @@ static int tsl2x7x_suspend(struct device *dev)
>  		chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
>  	}
>  
> -	if (chip->pdata && chip->pdata->platform_power) {
> -		pm_message_t pmm = {PM_EVENT_SUSPEND};
> -
> -		chip->pdata->platform_power(dev, pmm);
> -	}
> -
>  	return ret;
>  }
>  
> @@ -1807,12 +1795,6 @@ static int tsl2x7x_resume(struct device *dev)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int ret = 0;
>  
> -	if (chip->pdata && chip->pdata->platform_power) {
> -		pm_message_t pmm = {PM_EVENT_RESUME};
> -
> -		chip->pdata->platform_power(dev, pmm);
> -	}
> -
>  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_SUSPENDED)
>  		ret = tsl2x7x_chip_on(indio_dev);
>  
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index df00f2ec1719..6624cbca7a83 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -21,7 +21,6 @@
>  
>  #ifndef __TSL2X7X_H
>  #define __TSL2X7X_H
> -#include <linux/pm.h>
>  
>  struct tsl2x7x_lux {
>  	unsigned int ratio;
> @@ -91,9 +90,6 @@ struct tsl2x7x_settings {
>   *
>   */
>  struct tsl2X7X_platform_data {
> -	int (*platform_power)(struct device *dev, pm_message_t);
> -	int (*power_on)(struct iio_dev *indio_dev);
> -	int (*power_off)(struct i2c_client *dev);
>  	struct tsl2x7x_lux platform_lux_table[TSL2X7X_MAX_LUX_TABLE_SIZE];
>  	struct tsl2x7x_settings *platform_default_settings;
>  };

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

Re: [PATCH 01/12] staging: iio: tsl2x7x: remove power functions from tsl2X7X_platform_data

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:31 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The tsl2X7X_platform_data structure contains the platform_power,
> power_on, and power_off function pointers. These power management
> functions should not be in the platform data. These functions were
> likely used before the regulator framework was put in place. There are
> no users of these functions in the mainline kernel.
> 
> 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 | 18 ------------------
>  drivers/staging/iio/light/tsl2x7x.h |  4 ----
>  2 files changed, 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 126e11530ce0..b7e3f966c3a6 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -588,9 +588,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	u8 reg_val = 0;
>  
> -	if (chip->pdata && chip->pdata->power_on)
> -		chip->pdata->power_on(indio_dev);
> -
>  	/* Non calculated parameters */
>  	chip->tsl2x7x_config[TSL2X7X_PRX_TIME] = chip->settings.prx_time;
>  	chip->tsl2x7x_config[TSL2X7X_WAIT_TIME] = chip->settings.wait_time;
> @@ -736,9 +733,6 @@ static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
>  	ret = i2c_smbus_write_byte_data(chip->client,
>  					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
>  
> -	if (chip->pdata && chip->pdata->power_off)
> -		chip->pdata->power_off(chip->client);
> -
>  	return ret;
>  }
>  
> @@ -1792,12 +1786,6 @@ static int tsl2x7x_suspend(struct device *dev)
>  		chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
>  	}
>  
> -	if (chip->pdata && chip->pdata->platform_power) {
> -		pm_message_t pmm = {PM_EVENT_SUSPEND};
> -
> -		chip->pdata->platform_power(dev, pmm);
> -	}
> -
>  	return ret;
>  }
>  
> @@ -1807,12 +1795,6 @@ static int tsl2x7x_resume(struct device *dev)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int ret = 0;
>  
> -	if (chip->pdata && chip->pdata->platform_power) {
> -		pm_message_t pmm = {PM_EVENT_RESUME};
> -
> -		chip->pdata->platform_power(dev, pmm);
> -	}
> -
>  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_SUSPENDED)
>  		ret = tsl2x7x_chip_on(indio_dev);
>  
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index df00f2ec1719..6624cbca7a83 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -21,7 +21,6 @@
>  
>  #ifndef __TSL2X7X_H
>  #define __TSL2X7X_H
> -#include <linux/pm.h>
>  
>  struct tsl2x7x_lux {
>  	unsigned int ratio;
> @@ -91,9 +90,6 @@ struct tsl2x7x_settings {
>   *
>   */
>  struct tsl2X7X_platform_data {
> -	int (*platform_power)(struct device *dev, pm_message_t);
> -	int (*power_on)(struct iio_dev *indio_dev);
> -	int (*power_off)(struct i2c_client *dev);
>  	struct tsl2x7x_lux platform_lux_table[TSL2X7X_MAX_LUX_TABLE_SIZE];
>  	struct tsl2x7x_settings *platform_default_settings;
>  };

Re: [PATCH 02/12] staging: iio: tsl2x7x: add common function for clearing interrupts

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:32 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were three places where the same chunk of code was used to clear
> interrupts. This patch creates a common function
> tsl2x7x_clear_interrupts() to reduce duplicate code.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. This one wasn't immediately clear to me but having looked at the datasheet
I see the logic in combining these into one function so fair enough.

The odd addresses being set as bits within a register had me confused.

Applied

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 52 +++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index b7e3f966c3a6..c02db03ef369 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -279,6 +279,20 @@ static const u8 device_channel_config[] = {
>  	ALSPRX2
>  };
>  
> +static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(chip->client,
> +				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN | reg);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev,
> +			"%s: failed to clear interrupt status %x: %d\n",
> +			__func__, reg, ret);
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -346,16 +360,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		buf[i] = ret;
>  	}
>  
> -	/* clear any existing interrupt status */
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   TSL2X7X_CMD_REG |
> -				   TSL2X7X_CMD_SPL_FN |
> -				   TSL2X7X_CMD_ALS_INT_CLR);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"i2c_write_command failed - err = %d\n", ret);
> -		goto out_unlock; /* have no data, so return failure */
> -	}
> +	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]);
> @@ -706,17 +713,10 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
>  				__func__);
>  
> -		/* Clear out any initial interrupts  */
> -		ret = i2c_smbus_write_byte(chip->client,
> -					   TSL2X7X_CMD_REG |
> -					   TSL2X7X_CMD_SPL_FN |
> -					   TSL2X7X_CMD_PROXALS_INT_CLR);
> -		if (ret < 0) {
> -			dev_err(&chip->client->dev,
> -				"%s: Failed to clear Int status\n",
> -				__func__);
> -		return ret;
> -		}
> +		ret = tsl2x7x_clear_interrupts(chip,
> +					       TSL2X7X_CMD_PROXALS_INT_CLR);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
>  	return ret;
> @@ -1421,14 +1421,10 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  						    IIO_EV_DIR_EITHER),
>  			       timestamp);
>  	}
> -	/* Clear interrupt now that we have handled it. */
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN |
> -				   TSL2X7X_CMD_PROXALS_INT_CLR);
> +
> +	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
>  	if (ret < 0)
> -		dev_err(&chip->client->dev,
> -			"Failed to clear irq from event handler. err = %d\n",
> -			ret);
> +		return ret;
>  
>  	return IRQ_HANDLED;
>  }

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

Re: [PATCH 02/12] staging: iio: tsl2x7x: add common function for clearing interrupts

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:32 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were three places where the same chunk of code was used to clear
> interrupts. This patch creates a common function
> tsl2x7x_clear_interrupts() to reduce duplicate code.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. This one wasn't immediately clear to me but having looked at the datasheet
I see the logic in combining these into one function so fair enough.

The odd addresses being set as bits within a register had me confused.

Applied

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 52 +++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index b7e3f966c3a6..c02db03ef369 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -279,6 +279,20 @@ static const u8 device_channel_config[] = {
>  	ALSPRX2
>  };
>  
> +static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(chip->client,
> +				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN | reg);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev,
> +			"%s: failed to clear interrupt status %x: %d\n",
> +			__func__, reg, ret);
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -346,16 +360,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		buf[i] = ret;
>  	}
>  
> -	/* clear any existing interrupt status */
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   TSL2X7X_CMD_REG |
> -				   TSL2X7X_CMD_SPL_FN |
> -				   TSL2X7X_CMD_ALS_INT_CLR);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"i2c_write_command failed - err = %d\n", ret);
> -		goto out_unlock; /* have no data, so return failure */
> -	}
> +	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]);
> @@ -706,17 +713,10 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
>  				__func__);
>  
> -		/* Clear out any initial interrupts  */
> -		ret = i2c_smbus_write_byte(chip->client,
> -					   TSL2X7X_CMD_REG |
> -					   TSL2X7X_CMD_SPL_FN |
> -					   TSL2X7X_CMD_PROXALS_INT_CLR);
> -		if (ret < 0) {
> -			dev_err(&chip->client->dev,
> -				"%s: Failed to clear Int status\n",
> -				__func__);
> -		return ret;
> -		}
> +		ret = tsl2x7x_clear_interrupts(chip,
> +					       TSL2X7X_CMD_PROXALS_INT_CLR);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
>  	return ret;
> @@ -1421,14 +1421,10 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  						    IIO_EV_DIR_EITHER),
>  			       timestamp);
>  	}
> -	/* Clear interrupt now that we have handled it. */
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   TSL2X7X_CMD_REG | TSL2X7X_CMD_SPL_FN |
> -				   TSL2X7X_CMD_PROXALS_INT_CLR);
> +
> +	ret = tsl2x7x_clear_interrupts(chip, TSL2X7X_CMD_PROXALS_INT_CLR);
>  	if (ret < 0)
> -		dev_err(&chip->client->dev,
> -			"Failed to clear irq from event handler. err = %d\n",
> -			ret);
> +		return ret;
>  
>  	return IRQ_HANDLED;
>  }

Re: [PATCH 03/12] staging: iio: tsl2x7x: add common function for reading chip status

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:33 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were three places where the same chunk of code was used to read
> the chip status. This patch creates a common function
> tsl2x7x_read_status() to reduce duplicate code. This patch also corrects
> tsl2x7x_event_handler() to properly check for an error after reading the
> chip status.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 40 ++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index c02db03ef369..6bb622816660 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -293,6 +293,20 @@ static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
>  	return ret;
>  }
>  
> +static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev,
> +			"%s: failed to read STATUS register: %d\n", __func__,
> +			ret);
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -332,13 +346,10 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		goto out_unlock;
>  	}
>  
> -	ret = i2c_smbus_read_byte_data(chip->client,
> -				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: Failed to read STATUS Reg\n", __func__);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
>  		goto out_unlock;
> -	}
> +
>  	/* is data new & valid */
>  	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
>  		dev_err(&chip->client->dev,
> @@ -458,12 +469,9 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  		return -EBUSY;
>  	}
>  
> -	ret = i2c_smbus_read_byte_data(chip->client,
> -				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "i2c err=%d\n", ret);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
>  		goto prox_poll_err;
> -	}
>  
>  	switch (chip->id) {
>  	case tsl2571:
> @@ -1396,13 +1404,13 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
> -	u8 value;
>  
> -	value = i2c_smbus_read_byte_data(chip->client,
> -					 TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
> +		return ret;
>  
>  	/* What type of interrupt do we need to process */
> -	if (value & TSL2X7X_STA_PRX_INTR) {
> +	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,
> @@ -1412,7 +1420,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  						    timestamp);
>  	}
>  
> -	if (value & TSL2X7X_STA_ALS_INTR) {
> +	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,

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

Re: [PATCH 03/12] staging: iio: tsl2x7x: add common function for reading chip status

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:33 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were three places where the same chunk of code was used to read
> the chip status. This patch creates a common function
> tsl2x7x_read_status() to reduce duplicate code. This patch also corrects
> tsl2x7x_event_handler() to properly check for an error after reading the
> chip status.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 40 ++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index c02db03ef369..6bb622816660 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -293,6 +293,20 @@ static int tsl2x7x_clear_interrupts(struct tsl2X7X_chip *chip, int reg)
>  	return ret;
>  }
>  
> +static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev,
> +			"%s: failed to read STATUS register: %d\n", __func__,
> +			ret);
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -332,13 +346,10 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		goto out_unlock;
>  	}
>  
> -	ret = i2c_smbus_read_byte_data(chip->client,
> -				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: Failed to read STATUS Reg\n", __func__);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
>  		goto out_unlock;
> -	}
> +
>  	/* is data new & valid */
>  	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
>  		dev_err(&chip->client->dev,
> @@ -458,12 +469,9 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  		return -EBUSY;
>  	}
>  
> -	ret = i2c_smbus_read_byte_data(chip->client,
> -				       TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "i2c err=%d\n", ret);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
>  		goto prox_poll_err;
> -	}
>  
>  	switch (chip->id) {
>  	case tsl2571:
> @@ -1396,13 +1404,13 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
> -	u8 value;
>  
> -	value = i2c_smbus_read_byte_data(chip->client,
> -					 TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> +	ret = tsl2x7x_read_status(chip);
> +	if (ret < 0)
> +		return ret;
>  
>  	/* What type of interrupt do we need to process */
> -	if (value & TSL2X7X_STA_PRX_INTR) {
> +	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,
> @@ -1412,7 +1420,7 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  						    timestamp);
>  	}
>  
> -	if (value & TSL2X7X_STA_ALS_INTR) {
> +	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,

Re: [PATCH 04/12] staging: iio: tsl2x7x: add common function for writing to the control register

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:34 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were four places where the same chunk of code was used to write
> to the control register. This patch creates a common function
> tsl2x7x_write_control_reg() to reduce duplicate code.
> 
> The function tsl2x7x_chip_off() did not correctly return an error
> code so this patch also corrects that issue.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. These are getting a bit marginal in benefit but this one just makes
it over the barrier.

The reduction in complexity is really very small...

Applied.

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 54 +++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 6bb622816660..cf16dd206c0b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -307,6 +307,21 @@ static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
>  	return ret;
>  }
>  
> +static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, data);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev,
> +			"%s: failed to write to control register %x: %d\n",
> +			__func__, data, ret);
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -597,7 +612,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	int i;
>  	int ret = 0;
>  	u8 *dev_reg;
> -	u8 utmp;
>  	int als_count;
>  	int als_time;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> @@ -659,14 +673,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	 * TSL2X7X Specific power-on / adc enable sequence
>  	 * Power on the device 1st.
>  	 */
> -	utmp = TSL2X7X_CNTL_PWR_ON;
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed on CNTRL reg.\n", __func__);
> +	ret = tsl2x7x_write_control_reg(chip, TSL2X7X_CNTL_PWR_ON);
> +	if (ret < 0)
>  		return ret;
> -	}
>  
>  	/*
>  	 * Use the following shadow copy for our delay before enabling ADC.
> @@ -691,16 +700,12 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	 * NOW enable the ADC
>  	 * initialize the desired mode of operation
>  	 */
> -	utmp = TSL2X7X_CNTL_PWR_ON |
> -			TSL2X7X_CNTL_ADC_ENBL |
> -			TSL2X7X_CNTL_PROX_DET_ENBL;
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed on 2nd CTRL reg.\n", __func__);
> +	ret = tsl2x7x_write_control_reg(chip,
> +					TSL2X7X_CNTL_PWR_ON |
> +					TSL2X7X_CNTL_ADC_ENBL |
> +					TSL2X7X_CNTL_PROX_DET_ENBL);
> +	if (ret < 0)
>  		return ret;
> -	}
>  
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
> @@ -713,13 +718,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  			reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
>  
>  		reg_val |= chip->settings.interrupts_en;
> -		ret = i2c_smbus_write_byte_data(chip->client,
> -						TSL2X7X_CMD_REG | TSL2X7X_CNTRL,
> -						reg_val);
> +		ret = tsl2x7x_write_control_reg(chip, reg_val);
>  		if (ret < 0)
> -			dev_err(&chip->client->dev,
> -				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
> -				__func__);
> +			return ret;
>  
>  		ret = tsl2x7x_clear_interrupts(chip,
>  					       TSL2X7X_CMD_PROXALS_INT_CLR);
> @@ -732,16 +733,11 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  
>  static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
>  {
> -	int ret;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  
>  	/* turn device off */
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
> -
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
> -
> -	return ret;
> +	return tsl2x7x_write_control_reg(chip, 0x00);
>  }
>  
>  /**

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

Re: [PATCH 04/12] staging: iio: tsl2x7x: add common function for writing to the control register

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:34 -0500
Brian Masney <masneyb@onstation.org> wrote:

> There were four places where the same chunk of code was used to write
> to the control register. This patch creates a common function
> tsl2x7x_write_control_reg() to reduce duplicate code.
> 
> The function tsl2x7x_chip_off() did not correctly return an error
> code so this patch also corrects that issue.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. These are getting a bit marginal in benefit but this one just makes
it over the barrier.

The reduction in complexity is really very small...

Applied.

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 54 +++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 6bb622816660..cf16dd206c0b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -307,6 +307,21 @@ static int tsl2x7x_read_status(struct tsl2X7X_chip *chip)
>  	return ret;
>  }
>  
> +static int tsl2x7x_write_control_reg(struct tsl2X7X_chip *chip, u8 data)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, data);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev,
> +			"%s: failed to write to control register %x: %d\n",
> +			__func__, data, ret);
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
> @@ -597,7 +612,6 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	int i;
>  	int ret = 0;
>  	u8 *dev_reg;
> -	u8 utmp;
>  	int als_count;
>  	int als_time;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> @@ -659,14 +673,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	 * TSL2X7X Specific power-on / adc enable sequence
>  	 * Power on the device 1st.
>  	 */
> -	utmp = TSL2X7X_CNTL_PWR_ON;
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed on CNTRL reg.\n", __func__);
> +	ret = tsl2x7x_write_control_reg(chip, TSL2X7X_CNTL_PWR_ON);
> +	if (ret < 0)
>  		return ret;
> -	}
>  
>  	/*
>  	 * Use the following shadow copy for our delay before enabling ADC.
> @@ -691,16 +700,12 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	 * NOW enable the ADC
>  	 * initialize the desired mode of operation
>  	 */
> -	utmp = TSL2X7X_CNTL_PWR_ON |
> -			TSL2X7X_CNTL_ADC_ENBL |
> -			TSL2X7X_CNTL_PROX_DET_ENBL;
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, utmp);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed on 2nd CTRL reg.\n", __func__);
> +	ret = tsl2x7x_write_control_reg(chip,
> +					TSL2X7X_CNTL_PWR_ON |
> +					TSL2X7X_CNTL_ADC_ENBL |
> +					TSL2X7X_CNTL_PROX_DET_ENBL);
> +	if (ret < 0)
>  		return ret;
> -	}
>  
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
> @@ -713,13 +718,9 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  			reg_val |= TSL2X7X_CNTL_PROX_DET_ENBL;
>  
>  		reg_val |= chip->settings.interrupts_en;
> -		ret = i2c_smbus_write_byte_data(chip->client,
> -						TSL2X7X_CMD_REG | TSL2X7X_CNTRL,
> -						reg_val);
> +		ret = tsl2x7x_write_control_reg(chip, reg_val);
>  		if (ret < 0)
> -			dev_err(&chip->client->dev,
> -				"%s: failed in tsl2x7x_IOCTL_INT_SET.\n",
> -				__func__);
> +			return ret;
>  
>  		ret = tsl2x7x_clear_interrupts(chip,
>  					       TSL2X7X_CMD_PROXALS_INT_CLR);
> @@ -732,16 +733,11 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  
>  static int tsl2x7x_chip_off(struct iio_dev *indio_dev)
>  {
> -	int ret;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  
>  	/* turn device off */
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_SUSPENDED;
> -
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2X7X_CMD_REG | TSL2X7X_CNTRL, 0x00);
> -
> -	return ret;
> +	return tsl2x7x_write_control_reg(chip, 0x00);
>  }
>  
>  /**

Re: [PATCH 05/12] staging: iio: tsl2x7x: convert mutex_trylock() to mutex_lock()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:35 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The driver uses mutex_lock() and mutex_trylock() in several places.
> Convert the mutex_trylock() to mutex_lock() for consistency with other
> IIO light drivers in mainline.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This was a little odd given we don't lock the device for long and all this
will do is slow down userspace access if it tries several at the same time
(a fairy uncommon usecase).

Applied

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index cf16dd206c0b..5c611250127f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -350,8 +350,7 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	u32 ch0lux = 0;
>  	u32 ch1lux = 0;
>  
> -	if (mutex_trylock(&chip->als_mutex) == 0)
> -		return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
> +	mutex_lock(&chip->als_mutex);
>  
>  	if (chip->tsl2x7x_chip_status != TSL2X7X_CHIP_WORKING) {
>  		/* device is not enabled */
> @@ -478,11 +477,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	u8 chdata[2];
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  
> -	if (mutex_trylock(&chip->prox_mutex) == 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: Can't get prox mutex\n", __func__);
> -		return -EBUSY;
> -	}
> +	mutex_lock(&chip->prox_mutex);
>  
>  	ret = tsl2x7x_read_status(chip);
>  	if (ret < 0)

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

Re: [PATCH 05/12] staging: iio: tsl2x7x: convert mutex_trylock() to mutex_lock()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:35 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The driver uses mutex_lock() and mutex_trylock() in several places.
> Convert the mutex_trylock() to mutex_lock() for consistency with other
> IIO light drivers in mainline.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This was a little odd given we don't lock the device for long and all this
will do is slow down userspace access if it tries several at the same time
(a fairy uncommon usecase).

Applied

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index cf16dd206c0b..5c611250127f 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -350,8 +350,7 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	u32 ch0lux = 0;
>  	u32 ch1lux = 0;
>  
> -	if (mutex_trylock(&chip->als_mutex) == 0)
> -		return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
> +	mutex_lock(&chip->als_mutex);
>  
>  	if (chip->tsl2x7x_chip_status != TSL2X7X_CHIP_WORKING) {
>  		/* device is not enabled */
> @@ -478,11 +477,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	u8 chdata[2];
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  
> -	if (mutex_trylock(&chip->prox_mutex) == 0) {
> -		dev_err(&chip->client->dev,
> -			"%s: Can't get prox mutex\n", __func__);
> -		return -EBUSY;
> -	}
> +	mutex_lock(&chip->prox_mutex);
>  
>  	ret = tsl2x7x_read_status(chip);
>  	if (ret < 0)

Re: [PATCH 06/12] staging: iio: tsl2x7x: correctly return errors in tsl2x7x_get_prox()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:36 -0500
Brian Masney <masneyb@onstation.org> wrote:

> Not all errors that occurred in tsl2x7x_get_prox() were correctly
> reported in the return value. This patch changes the error handling
> so that errors are now returned properly.
> 
> Note that the ret variable is from the call to tsl2x7x_read_status(),
> and it already has the correct error check. The -EINVAL error code is
> for an unexpected value in the register.
> 
> This patch also corrects an unnecessary word wrap in the call to
> le16_to_cpup() while changes are being made to this code.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Good cleanup.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to work their magic.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 5c611250127f..cc209b64ed5a 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -489,16 +489,20 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	case tmd2671:
>  	case tsl2771:
>  	case tmd2771:
> -		if (!(ret & TSL2X7X_STA_ADC_VALID))
> +		if (!(ret & TSL2X7X_STA_ADC_VALID)) {
> +			ret = -EINVAL;
>  			goto prox_poll_err;
> +		}
>  		break;
>  	case tsl2572:
>  	case tsl2672:
>  	case tmd2672:
>  	case tsl2772:
>  	case tmd2772:
> -		if (!(ret & TSL2X7X_STA_PRX_VALID))
> +		if (!(ret & TSL2X7X_STA_PRX_VALID)) {
> +			ret = -EINVAL;
>  			goto prox_poll_err;
> +		}
>  		break;
>  	}
>  
> @@ -512,14 +516,13 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  		chdata[i] = ret;
>  	}
>  
> -	chip->prox_data =
> -			le16_to_cpup((const __le16 *)&chdata[0]);
> +	chip->prox_data = le16_to_cpup((const __le16 *)&chdata[0]);
> +	ret = chip->prox_data;
>  
>  prox_poll_err:
> -
>  	mutex_unlock(&chip->prox_mutex);
>  
> -	return chip->prox_data;
> +	return ret;
>  }
>  
>  /**

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

Re: [PATCH 06/12] staging: iio: tsl2x7x: correctly return errors in tsl2x7x_get_prox()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:36 -0500
Brian Masney <masneyb@onstation.org> wrote:

> Not all errors that occurred in tsl2x7x_get_prox() were correctly
> reported in the return value. This patch changes the error handling
> so that errors are now returned properly.
> 
> Note that the ret variable is from the call to tsl2x7x_read_status(),
> and it already has the correct error check. The -EINVAL error code is
> for an unexpected value in the register.
> 
> This patch also corrects an unnecessary word wrap in the call to
> le16_to_cpup() while changes are being made to this code.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Good cleanup.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to work their magic.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 5c611250127f..cc209b64ed5a 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -489,16 +489,20 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	case tmd2671:
>  	case tsl2771:
>  	case tmd2771:
> -		if (!(ret & TSL2X7X_STA_ADC_VALID))
> +		if (!(ret & TSL2X7X_STA_ADC_VALID)) {
> +			ret = -EINVAL;
>  			goto prox_poll_err;
> +		}
>  		break;
>  	case tsl2572:
>  	case tsl2672:
>  	case tmd2672:
>  	case tsl2772:
>  	case tmd2772:
> -		if (!(ret & TSL2X7X_STA_PRX_VALID))
> +		if (!(ret & TSL2X7X_STA_PRX_VALID)) {
> +			ret = -EINVAL;
>  			goto prox_poll_err;
> +		}
>  		break;
>  	}
>  
> @@ -512,14 +516,13 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  		chdata[i] = ret;
>  	}
>  
> -	chip->prox_data =
> -			le16_to_cpup((const __le16 *)&chdata[0]);
> +	chip->prox_data = le16_to_cpup((const __le16 *)&chdata[0]);
> +	ret = chip->prox_data;
>  
>  prox_poll_err:
> -
>  	mutex_unlock(&chip->prox_mutex);
>  
> -	return chip->prox_data;
> +	return ret;
>  }
>  
>  /**

Re: [PATCH 07/12] staging: iio: tsl2x7x: correct 'Avoid CamelCase' warning from checkpatch

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:37 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The statP and calP variables triggered an 'Avoid CamelCase' warning
> from checkpatch.pl. This patch renames these variables to stat and cal
> to fix this warning.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index cc209b64ed5a..380a6c96a69a 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -773,7 +773,7 @@ static int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
>  }
>  
>  static void tsl2x7x_prox_calculate(int *data, int length,
> -				   struct tsl2x7x_prox_stat *statP)
> +				   struct tsl2x7x_prox_stat *stat)
>  {
>  	int i;
>  	int sample_sum;
> @@ -783,21 +783,21 @@ static void tsl2x7x_prox_calculate(int *data, int length,
>  		length = 1;
>  
>  	sample_sum = 0;
> -	statP->min = INT_MAX;
> -	statP->max = INT_MIN;
> +	stat->min = INT_MAX;
> +	stat->max = INT_MIN;
>  	for (i = 0; i < length; i++) {
>  		sample_sum += data[i];
> -		statP->min = min(statP->min, data[i]);
> -		statP->max = max(statP->max, data[i]);
> +		stat->min = min(stat->min, data[i]);
> +		stat->max = max(stat->max, data[i]);
>  	}
>  
> -	statP->mean = sample_sum / length;
> +	stat->mean = sample_sum / length;
>  	sample_sum = 0;
>  	for (i = 0; i < length; i++) {
> -		tmp = data[i] - statP->mean;
> +		tmp = data[i] - stat->mean;
>  		sample_sum += tmp * tmp;
>  	}
> -	statP->stddev = int_sqrt((long)sample_sum / length);
> +	stat->stddev = int_sqrt((long)sample_sum / length);
>  }
>  
>  /**
> @@ -812,7 +812,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	int prox_history[MAX_SAMPLES_CAL + 1];
>  	int i;
>  	struct tsl2x7x_prox_stat prox_stat_data[2];
> -	struct tsl2x7x_prox_stat *calP;
> +	struct tsl2x7x_prox_stat *cal;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	u8 tmp_irq_settings;
>  	u8 current_state = chip->tsl2x7x_chip_status;
> @@ -844,13 +844,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	}
>  
>  	tsl2x7x_chip_off(indio_dev);
> -	calP = &prox_stat_data[PROX_STAT_CAL];
> +	cal = &prox_stat_data[PROX_STAT_CAL];
>  	tsl2x7x_prox_calculate(prox_history,
> -			       chip->settings.prox_max_samples_cal, calP);
> -	chip->settings.prox_thres_high = (calP->max << 1) - calP->mean;
> +			       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",
> -		 calP->min, calP->mean, calP->max);
> +		 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);

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

Re: [PATCH 07/12] staging: iio: tsl2x7x: correct 'Avoid CamelCase' warning from checkpatch

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:37 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The statP and calP variables triggered an 'Avoid CamelCase' warning
> from checkpatch.pl. This patch renames these variables to stat and cal
> to fix this warning.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index cc209b64ed5a..380a6c96a69a 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -773,7 +773,7 @@ static int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
>  }
>  
>  static void tsl2x7x_prox_calculate(int *data, int length,
> -				   struct tsl2x7x_prox_stat *statP)
> +				   struct tsl2x7x_prox_stat *stat)
>  {
>  	int i;
>  	int sample_sum;
> @@ -783,21 +783,21 @@ static void tsl2x7x_prox_calculate(int *data, int length,
>  		length = 1;
>  
>  	sample_sum = 0;
> -	statP->min = INT_MAX;
> -	statP->max = INT_MIN;
> +	stat->min = INT_MAX;
> +	stat->max = INT_MIN;
>  	for (i = 0; i < length; i++) {
>  		sample_sum += data[i];
> -		statP->min = min(statP->min, data[i]);
> -		statP->max = max(statP->max, data[i]);
> +		stat->min = min(stat->min, data[i]);
> +		stat->max = max(stat->max, data[i]);
>  	}
>  
> -	statP->mean = sample_sum / length;
> +	stat->mean = sample_sum / length;
>  	sample_sum = 0;
>  	for (i = 0; i < length; i++) {
> -		tmp = data[i] - statP->mean;
> +		tmp = data[i] - stat->mean;
>  		sample_sum += tmp * tmp;
>  	}
> -	statP->stddev = int_sqrt((long)sample_sum / length);
> +	stat->stddev = int_sqrt((long)sample_sum / length);
>  }
>  
>  /**
> @@ -812,7 +812,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	int prox_history[MAX_SAMPLES_CAL + 1];
>  	int i;
>  	struct tsl2x7x_prox_stat prox_stat_data[2];
> -	struct tsl2x7x_prox_stat *calP;
> +	struct tsl2x7x_prox_stat *cal;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	u8 tmp_irq_settings;
>  	u8 current_state = chip->tsl2x7x_chip_status;
> @@ -844,13 +844,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	}
>  
>  	tsl2x7x_chip_off(indio_dev);
> -	calP = &prox_stat_data[PROX_STAT_CAL];
> +	cal = &prox_stat_data[PROX_STAT_CAL];
>  	tsl2x7x_prox_calculate(prox_history,
> -			       chip->settings.prox_max_samples_cal, calP);
> -	chip->settings.prox_thres_high = (calP->max << 1) - calP->mean;
> +			       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",
> -		 calP->min, calP->mean, calP->max);
> +		 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);

Re: [PATCH 08/12] staging: iio: tsl2x7x: add error handling to tsl2x7x_prox_cal()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:38 -0500
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_prox_cal() did not have any error checks. This patch adds
> the missing error handling and ensures that any errors are reported to
> user space via in_proximity0_calibrate_store().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 380a6c96a69a..8adc6db44790 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -807,10 +807,10 @@ static void tsl2x7x_prox_calculate(int *data, int length,
>   * Calculates a standard deviation based on the samples,
>   * and sets the threshold accordingly.
>   */
> -static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> +static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  {
>  	int prox_history[MAX_SAMPLES_CAL + 1];
> -	int i;
> +	int i, ret;
>  	struct tsl2x7x_prox_stat prox_stat_data[2];
>  	struct tsl2x7x_prox_stat *cal;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> @@ -825,25 +825,33 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	}
>  
>  	/* have to stop to change settings */
> -	tsl2x7x_chip_off(indio_dev);
> +	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;
>  
>  	/*turn on device if not already on*/
> -	tsl2x7x_chip_on(indio_dev);
> +	ret = tsl2x7x_chip_on(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	/*gather the samples*/
>  	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
>  		usleep_range(15000, 17500);
> -		tsl2x7x_get_prox(indio_dev);
> +		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);
>  	}
>  
> -	tsl2x7x_chip_off(indio_dev);
> +	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);
> @@ -857,8 +865,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  
>  	/* back to the way they were */
>  	chip->settings.interrupts_en = tmp_irq_settings;
> -	if (current_state == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_on(indio_dev);
> +	if (current_state == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_on(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static ssize_t
> @@ -1018,8 +1031,11 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
>  
> -	if (value)
> -		tsl2x7x_prox_cal(indio_dev);
> +	if (value) {
> +		ret = tsl2x7x_prox_cal(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = tsl2x7x_invoke_change(indio_dev);
>  	if (ret < 0)

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

Re: [PATCH 08/12] staging: iio: tsl2x7x: add error handling to tsl2x7x_prox_cal()

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:38 -0500
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_prox_cal() did not have any error checks. This patch adds
> the missing error handling and ensures that any errors are reported to
> user space via in_proximity0_calibrate_store().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 380a6c96a69a..8adc6db44790 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -807,10 +807,10 @@ static void tsl2x7x_prox_calculate(int *data, int length,
>   * Calculates a standard deviation based on the samples,
>   * and sets the threshold accordingly.
>   */
> -static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> +static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  {
>  	int prox_history[MAX_SAMPLES_CAL + 1];
> -	int i;
> +	int i, ret;
>  	struct tsl2x7x_prox_stat prox_stat_data[2];
>  	struct tsl2x7x_prox_stat *cal;
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> @@ -825,25 +825,33 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  	}
>  
>  	/* have to stop to change settings */
> -	tsl2x7x_chip_off(indio_dev);
> +	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;
>  
>  	/*turn on device if not already on*/
> -	tsl2x7x_chip_on(indio_dev);
> +	ret = tsl2x7x_chip_on(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	/*gather the samples*/
>  	for (i = 0; i < chip->settings.prox_max_samples_cal; i++) {
>  		usleep_range(15000, 17500);
> -		tsl2x7x_get_prox(indio_dev);
> +		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);
>  	}
>  
> -	tsl2x7x_chip_off(indio_dev);
> +	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);
> @@ -857,8 +865,13 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  
>  	/* back to the way they were */
>  	chip->settings.interrupts_en = tmp_irq_settings;
> -	if (current_state == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_on(indio_dev);
> +	if (current_state == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_on(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static ssize_t
> @@ -1018,8 +1031,11 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
>  
> -	if (value)
> -		tsl2x7x_prox_cal(indio_dev);
> +	if (value) {
> +		ret = tsl2x7x_prox_cal(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = tsl2x7x_invoke_change(indio_dev);
>  	if (ret < 0)

Re: [PATCH 09/12] staging: iio: tsl2x7x: add missing error checks

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:39 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The functions in_illuminance0_calibrate_store() and
> in_illuminance0_lux_table_store() did not have complete error handling
> in place. This patch adds the missing error handling.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.

Thanks,
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 8adc6db44790..da7a4e025083 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -940,8 +940,11 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
>  
> -	if (value)
> -		tsl2x7x_als_calibrate(indio_dev);
> +	if (value) {
> +		ret = tsl2x7x_als_calibrate(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = tsl2x7x_invoke_change(indio_dev);
>  	if (ret < 0)
> @@ -1006,8 +1009,11 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> -	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> +	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_off(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	/* Zero out the table */
>  	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));

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

Re: [PATCH 09/12] staging: iio: tsl2x7x: add missing error checks

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:39 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The functions in_illuminance0_calibrate_store() and
> in_illuminance0_lux_table_store() did not have complete error handling
> in place. This patch adds the missing error handling.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.

Thanks,
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 8adc6db44790..da7a4e025083 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -940,8 +940,11 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
>  
> -	if (value)
> -		tsl2x7x_als_calibrate(indio_dev);
> +	if (value) {
> +		ret = tsl2x7x_als_calibrate(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = tsl2x7x_invoke_change(indio_dev);
>  	if (ret < 0)
> @@ -1006,8 +1009,11 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> -	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> +	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_off(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	/* Zero out the table */
>  	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));

Re: [PATCH 10/12] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:40 -0500
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 from tsl2x7x_prox_cal().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. I nearly fixed this one up but there are a few too many elements I don't
agree with.

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 58 ++++++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index da7a4e025083..fb91c46c8747 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -378,7 +378,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;
>  		}
>  
> @@ -424,7 +425,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;
>  	}
> @@ -600,7 +603,8 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  
>  	chip->settings.als_gain_trim = ret;
>  	dev_info(&chip->client->dev,
> -		 "%s als_calibrate completed\n", chip->client->name);
> +		 "%s: %s ALS calibration successfully completed\n",
> +		 __func__, chip->client->name);
I'm happy with having function name in errors, but it's just noise in info
prints.

Mind you this dev_info should go or become dev_dbg - it is superflous
detail to put in the kernel logs.

>  
>  	return ret;
>  }
> @@ -644,7 +648,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	/* and make sure we're not already on */
>  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
>  		/* if forcing a register update - turn off, then on */
> -		dev_info(&chip->client->dev, "device is already enabled\n");
> +		dev_info(&chip->client->dev, "%s: device is already enabled\n",
> +			 __func__);
Again noise.  dev_dbg or drop entirely as it doesn't need to be logged.
>  		return -EINVAL;
>  	}
>  
> @@ -681,12 +686,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;
>  		}
>  	}
> @@ -708,7 +715,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
>  	if (chip->settings.interrupts_en != 0) {
> -		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
> +		dev_info(&chip->client->dev, "%s: Setting up interrupt(s)\n",
> +			 __func__);
>  
>  		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
>  		if (chip->settings.interrupts_en == 0x20 ||
> @@ -819,8 +827,8 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  
>  	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);
> +			"%s: prox_max_samples_cal %d is too big\n",
> +			__func__, chip->settings.prox_max_samples_cal);
>  		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
>  	}
>  
> @@ -845,8 +853,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *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);
> @@ -857,12 +863,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  			       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) {
> @@ -1000,12 +1000,14 @@ 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);
> +		dev_info(dev, "%s: lux table input error 1. n=%d\n",
> +			 __func__, n);
>  		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);
> +		dev_info(dev, "%s: lux table input error 2. n=%d\n",
> +			 __func__, n);
dev_dbg or drop it.

>  		return -EINVAL;
>  	}
>  
> @@ -1150,14 +1152,14 @@ 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",
> +			dev_info(&chip->client->dev, "%s: ALS persistence = %d\n",
>  				 __func__, filter_delay);
Again, dev_dbg at the most - or drop it.

>  		} else {
>  			chip->settings.persistence &= 0x0F;
>  			chip->settings.persistence |=
>  				((filter_delay << 4) & 0xF0);
>  			dev_info(&chip->client->dev,
> -				 "%s: Proximity persistence = %d",
> +				 "%s: Proximity persistence = %d\n",
>  				 __func__, filter_delay);
>  		}
>  		ret = 0;
> @@ -1372,7 +1374,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  		chip->settings.als_time =
>  			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
>  
> -		dev_info(&chip->client->dev, "%s: als time = %d",
> +		dev_info(&chip->client->dev, "%s: als time = %d\n",
>  			 __func__, chip->settings.als_time);
Drop it.
>  		break;
>  	default:
> @@ -1738,8 +1740,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;
>  	}
>  
> @@ -1773,7 +1776,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;
>  		}
>  	}
> @@ -1790,7 +1793,8 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  		return ret;
>  	}
>  
> -	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
> +	dev_info(&clientp->dev, "%s: %s Light sensor found.\n", __func__,
> +		 id->name);

Again, unnecessary logging.  Definitely doesn't also want the function name.
People 'might' care about whether the sensor is found, but not which function
it is in.

Jonathan

>  
>  	return 0;
>  }

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

Re: [PATCH 10/12] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:40 -0500
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 from tsl2x7x_prox_cal().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. I nearly fixed this one up but there are a few too many elements I don't
agree with.

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 58 ++++++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index da7a4e025083..fb91c46c8747 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -378,7 +378,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;
>  		}
>  
> @@ -424,7 +425,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;
>  	}
> @@ -600,7 +603,8 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  
>  	chip->settings.als_gain_trim = ret;
>  	dev_info(&chip->client->dev,
> -		 "%s als_calibrate completed\n", chip->client->name);
> +		 "%s: %s ALS calibration successfully completed\n",
> +		 __func__, chip->client->name);
I'm happy with having function name in errors, but it's just noise in info
prints.

Mind you this dev_info should go or become dev_dbg - it is superflous
detail to put in the kernel logs.

>  
>  	return ret;
>  }
> @@ -644,7 +648,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	/* and make sure we're not already on */
>  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
>  		/* if forcing a register update - turn off, then on */
> -		dev_info(&chip->client->dev, "device is already enabled\n");
> +		dev_info(&chip->client->dev, "%s: device is already enabled\n",
> +			 __func__);
Again noise.  dev_dbg or drop entirely as it doesn't need to be logged.
>  		return -EINVAL;
>  	}
>  
> @@ -681,12 +686,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;
>  		}
>  	}
> @@ -708,7 +715,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
>  
>  	if (chip->settings.interrupts_en != 0) {
> -		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
> +		dev_info(&chip->client->dev, "%s: Setting up interrupt(s)\n",
> +			 __func__);
>  
>  		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
>  		if (chip->settings.interrupts_en == 0x20 ||
> @@ -819,8 +827,8 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  
>  	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);
> +			"%s: prox_max_samples_cal %d is too big\n",
> +			__func__, chip->settings.prox_max_samples_cal);
>  		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
>  	}
>  
> @@ -845,8 +853,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *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);
> @@ -857,12 +863,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  			       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) {
> @@ -1000,12 +1000,14 @@ 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);
> +		dev_info(dev, "%s: lux table input error 1. n=%d\n",
> +			 __func__, n);
>  		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);
> +		dev_info(dev, "%s: lux table input error 2. n=%d\n",
> +			 __func__, n);
dev_dbg or drop it.

>  		return -EINVAL;
>  	}
>  
> @@ -1150,14 +1152,14 @@ 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",
> +			dev_info(&chip->client->dev, "%s: ALS persistence = %d\n",
>  				 __func__, filter_delay);
Again, dev_dbg at the most - or drop it.

>  		} else {
>  			chip->settings.persistence &= 0x0F;
>  			chip->settings.persistence |=
>  				((filter_delay << 4) & 0xF0);
>  			dev_info(&chip->client->dev,
> -				 "%s: Proximity persistence = %d",
> +				 "%s: Proximity persistence = %d\n",
>  				 __func__, filter_delay);
>  		}
>  		ret = 0;
> @@ -1372,7 +1374,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  		chip->settings.als_time =
>  			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
>  
> -		dev_info(&chip->client->dev, "%s: als time = %d",
> +		dev_info(&chip->client->dev, "%s: als time = %d\n",
>  			 __func__, chip->settings.als_time);
Drop it.
>  		break;
>  	default:
> @@ -1738,8 +1740,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;
>  	}
>  
> @@ -1773,7 +1776,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;
>  		}
>  	}
> @@ -1790,7 +1793,8 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  		return ret;
>  	}
>  
> -	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
> +	dev_info(&clientp->dev, "%s: %s Light sensor found.\n", __func__,
> +		 id->name);

Again, unnecessary logging.  Definitely doesn't also want the function name.
People 'might' care about whether the sensor is found, but not which function
it is in.

Jonathan

>  
>  	return 0;
>  }

Re: [PATCH 11/12] staging: iio: tsl2x7x: remove unnecessary sysfs attribute

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:41 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The tsl2771 and tmd2771 devices create the
> in_proximity0_calibscale_available sysfs attribute. These two particular
> devices do not support changing the proximity gain value on the
> chip so this patch removes that sysfs attribute. As expected, these two
> devices already did not create the IIO_CHAN_INFO_CALIBSCALE channel and
> proximity0_calibrate sysfs attribute.
> 
> Page 38 of the tsl2772 data sheet shows that the proximity gain can be
> adjusted with bits 2-3 on the control register:
> https://ams.com/eng/content/download/291503/1066377/file/TSL2772_DS000181_2-00.pdf
> 
> Page 35 of the tsl2771 and tmd2771 data sheets shows that bits 2-3 on
> the control register are reserved and changing the proximity gain is
> not supported:
> https://ams.com/eng/content/download/250264/976045/file/TSL2771_DS000105_3-00.pdf
> https://ams.com/eng/content/download/250283/976077/file/TMD2771_DS000177_2-00.pdf
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This one applied without the previous so I have done so.

Applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index fb91c46c8747..8c29a52153c1 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1477,7 +1477,6 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
>  	&dev_attr_in_illuminance0_lux_table.attr,
> -	&iio_const_attr_in_proximity0_calibscale_available.dev_attr.attr,
>  	NULL
>  };
>  

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

Re: [PATCH 11/12] staging: iio: tsl2x7x: remove unnecessary sysfs attribute

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:41 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The tsl2771 and tmd2771 devices create the
> in_proximity0_calibscale_available sysfs attribute. These two particular
> devices do not support changing the proximity gain value on the
> chip so this patch removes that sysfs attribute. As expected, these two
> devices already did not create the IIO_CHAN_INFO_CALIBSCALE channel and
> proximity0_calibrate sysfs attribute.
> 
> Page 38 of the tsl2772 data sheet shows that the proximity gain can be
> adjusted with bits 2-3 on the control register:
> https://ams.com/eng/content/download/291503/1066377/file/TSL2772_DS000181_2-00.pdf
> 
> Page 35 of the tsl2771 and tmd2771 data sheets shows that bits 2-3 on
> the control register are reserved and changing the proximity gain is
> not supported:
> https://ams.com/eng/content/download/250264/976045/file/TSL2771_DS000105_3-00.pdf
> https://ams.com/eng/content/download/250283/976077/file/TMD2771_DS000177_2-00.pdf
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This one applied without the previous so I have done so.

Applied.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index fb91c46c8747..8c29a52153c1 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1477,7 +1477,6 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
>  	&dev_attr_in_illuminance0_lux_table.attr,
> -	&iio_const_attr_in_proximity0_calibscale_available.dev_attr.attr,
>  	NULL
>  };
>  

Re: [PATCH 12/12] staging: iio: tsl2x7x: make proximity sensor function correctly

[Jonathan Cameron]

On Sat,  3 Mar 2018 20:49:42 -0500
Brian Masney <masneyb@onstation.org> wrote:

> The bits for setting up the proximity diode were not setup correctly and
> this was causing the proximity sensor to not function correctly. This
> patch sets up the correct bit mask in tsl2x7x_chip_on() based on what
> the data sheet expects. From page 35 of the TSL2771 data sheet:
> https://ams.com/eng/content/download/250264/976045/file/TSL2771_DS000105_3-00.pdf
> 
> - Bits 0-1 is the ALS gain control
> - Bits 2-3 is reserved (The proximity gain control on other tsl2x7x chips)
> - Bits 4-5 is the proximity diode select
> - Bits 6-7 is the LED drive strength
> 
> tsl2x7x_chip_on() had the power and diode hardcoded, so these are
> extracted out into the settings so that these fields can be configured
> in the platform data.
> 
> The default prox_gain is changed from 1 (2X gain) to 0 (1X gain) since
> the proximity gain control on the TSL2771, TMD2771, and other chips have
> these fields listed as reserved, and to write 0 into those bits.
> 
> Verified that the proximity sensor now works correctly on a TSL2771
> hooked up to a Raspberry Pi 2.
> 
> 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.

So just patch 10 that needs a v2.

Thanks for the set - nearly there ;)

Jonathan

> ---
>  drivers/staging/iio/light/tsl2x7x.c | 25 ++++++++++++++-----------
>  drivers/staging/iio/light/tsl2x7x.h |  2 ++
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 8c29a52153c1..ab518cdec43e 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -109,15 +109,15 @@
>  #define TSL2X7X_CNTL_INTPROXPON_ENBL	0x2F
>  
>  /*Prox diode to use */
> -#define TSL2X7X_DIODE0			0x10
> -#define TSL2X7X_DIODE1			0x20
> -#define TSL2X7X_DIODE_BOTH		0x30
> +#define TSL2X7X_DIODE0			0x01
> +#define TSL2X7X_DIODE1			0x02
> +#define TSL2X7X_DIODE_BOTH		0x03
>  
>  /* LED Power */
>  #define TSL2X7X_100_mA			0x00
> -#define TSL2X7X_50_mA			0x40
> -#define TSL2X7X_25_mA			0x80
> -#define TSL2X7X_13_mA			0xD0
> +#define TSL2X7X_50_mA			0x01
> +#define TSL2X7X_25_mA			0x02
> +#define TSL2X7X_13_mA			0x03
>  #define TSL2X7X_MAX_TIMER_CNT		0xFF
>  
>  #define TSL2X7X_MIN_ITIME		3
> @@ -228,7 +228,7 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
>  	.als_time = 219, /* 101 ms */
>  	.als_gain = 0,
>  	.prx_time = 254, /* 5.4 ms */
> -	.prox_gain = 1,
> +	.prox_gain = 0,
>  	.wait_time = 245,
>  	.prox_config = 0,
>  	.als_gain_trim = 1000,
> @@ -240,7 +240,9 @@ static const struct tsl2x7x_settings tsl2x7x_default_settings = {
>  	.prox_thres_low  = 0,
>  	.prox_thres_high = 512,
>  	.prox_max_samples_cal = 30,
> -	.prox_pulse_count = 8
> +	.prox_pulse_count = 8,
> +	.prox_diode = TSL2X7X_DIODE1,
> +	.prox_power = TSL2X7X_100_mA
>  };
>  
>  static const s16 tsl2x7x_als_gain[] = {
> @@ -664,9 +666,10 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  
>  	/* Set the gain based on tsl2x7x_settings struct */
>  	chip->tsl2x7x_config[TSL2X7X_GAIN] =
> -		chip->settings.als_gain |
> -			(TSL2X7X_100_mA | TSL2X7X_DIODE1) |
> -			(chip->settings.prox_gain << 2);
> +		(chip->settings.als_gain & 0xFF) |
> +		((chip->settings.prox_gain & 0xFF) << 2) |
> +		(chip->settings.prox_diode << 4) |
> +		(chip->settings.prox_power << 6);
>  
>  	/* set chip struct re scaling and saturation */
>  	chip->als_saturation = als_count * 922; /* 90% of full scale */
> diff --git a/drivers/staging/iio/light/tsl2x7x.h b/drivers/staging/iio/light/tsl2x7x.h
> index 6624cbca7a83..28b0e7fdc9b8 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -78,6 +78,8 @@ struct tsl2x7x_settings {
>  	int prox_thres_high;
>  	int prox_pulse_count;
>  	int prox_max_samples_cal;
> +	int prox_diode;
> +	int prox_power;
>  };
>  
>  /**

Re: [PATCH 10/12] staging: iio: tsl2x7x: make logging consistent and correct newlines

[Jonathan Cameron]

On Sat, 10 Mar 2018 14:52:40 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat,  3 Mar 2018 20:49:40 -0500
> 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 from tsl2x7x_prox_cal().
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>  
> Hmm. I nearly fixed this one up but there are a few too many elements I don't
> agree with.

I knew there was something I was forgetting.  There is no point in
putting function names in dynamic debug calls, you can turn that
on if you want it for all of them anyway.

Jonathan
> 
> > ---
> >  drivers/staging/iio/light/tsl2x7x.c | 58 ++++++++++++++++++++-----------------
> >  1 file changed, 31 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> > index da7a4e025083..fb91c46c8747 100644
> > --- a/drivers/staging/iio/light/tsl2x7x.c
> > +++ b/drivers/staging/iio/light/tsl2x7x.c
> > @@ -378,7 +378,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;
> >  		}
> >  
> > @@ -424,7 +425,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;
> >  	}
> > @@ -600,7 +603,8 @@ static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
> >  
> >  	chip->settings.als_gain_trim = ret;
> >  	dev_info(&chip->client->dev,
> > -		 "%s als_calibrate completed\n", chip->client->name);
> > +		 "%s: %s ALS calibration successfully completed\n",
> > +		 __func__, chip->client->name);  
> I'm happy with having function name in errors, but it's just noise in info
> prints.
> 
> Mind you this dev_info should go or become dev_dbg - it is superflous
> detail to put in the kernel logs.
> 
> >  
> >  	return ret;
> >  }
> > @@ -644,7 +648,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> >  	/* and make sure we're not already on */
> >  	if (chip->tsl2x7x_chip_status == TSL2X7X_CHIP_WORKING) {
> >  		/* if forcing a register update - turn off, then on */
> > -		dev_info(&chip->client->dev, "device is already enabled\n");
> > +		dev_info(&chip->client->dev, "%s: device is already enabled\n",
> > +			 __func__);  
> Again noise.  dev_dbg or drop entirely as it doesn't need to be logged.
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -681,12 +686,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;
> >  		}
> >  	}
> > @@ -708,7 +715,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> >  	chip->tsl2x7x_chip_status = TSL2X7X_CHIP_WORKING;
> >  
> >  	if (chip->settings.interrupts_en != 0) {
> > -		dev_info(&chip->client->dev, "Setting Up Interrupt(s)\n");
> > +		dev_info(&chip->client->dev, "%s: Setting up interrupt(s)\n",
> > +			 __func__);
> >  
> >  		reg_val = TSL2X7X_CNTL_PWR_ON | TSL2X7X_CNTL_ADC_ENBL;
> >  		if (chip->settings.interrupts_en == 0x20 ||
> > @@ -819,8 +827,8 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> >  
> >  	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);
> > +			"%s: prox_max_samples_cal %d is too big\n",
> > +			__func__, chip->settings.prox_max_samples_cal);
> >  		chip->settings.prox_max_samples_cal = MAX_SAMPLES_CAL;
> >  	}
> >  
> > @@ -845,8 +853,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *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);
> > @@ -857,12 +863,6 @@ static int tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> >  			       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) {
> > @@ -1000,12 +1000,14 @@ 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);
> > +		dev_info(dev, "%s: lux table input error 1. n=%d\n",
> > +			 __func__, n);
> >  		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);
> > +		dev_info(dev, "%s: lux table input error 2. n=%d\n",
> > +			 __func__, n);  
> dev_dbg or drop it.
> 
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -1150,14 +1152,14 @@ 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",
> > +			dev_info(&chip->client->dev, "%s: ALS persistence = %d\n",
> >  				 __func__, filter_delay);  
> Again, dev_dbg at the most - or drop it.
> 
> >  		} else {
> >  			chip->settings.persistence &= 0x0F;
> >  			chip->settings.persistence |=
> >  				((filter_delay << 4) & 0xF0);
> >  			dev_info(&chip->client->dev,
> > -				 "%s: Proximity persistence = %d",
> > +				 "%s: Proximity persistence = %d\n",
> >  				 __func__, filter_delay);
> >  		}
> >  		ret = 0;
> > @@ -1372,7 +1374,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
> >  		chip->settings.als_time =
> >  			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
> >  
> > -		dev_info(&chip->client->dev, "%s: als time = %d",
> > +		dev_info(&chip->client->dev, "%s: als time = %d\n",
> >  			 __func__, chip->settings.als_time);  
> Drop it.
> >  		break;
> >  	default:
> > @@ -1738,8 +1740,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;
> >  	}
> >  
> > @@ -1773,7 +1776,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;
> >  		}
> >  	}
> > @@ -1790,7 +1793,8 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
> >  		return ret;
> >  	}
> >  
> > -	dev_info(&clientp->dev, "%s Light sensor found.\n", id->name);
> > +	dev_info(&clientp->dev, "%s: %s Light sensor found.\n", __func__,
> > +		 id->name);  
> 
> Again, unnecessary logging.  Definitely doesn't also want the function name.
> People 'might' care about whether the sensor is found, but not which function
> it is in.
> 
> Jonathan
> 
> >  
> >  	return 0;
> >  }  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html