[PATCH v2 0/9] staging: iio: tsl2x7x: staging cleanups

[PATCH v2 0/9] staging: iio: tsl2x7x: staging cleanups

[Brian Masney]

This begins my work to clean this driver up and eventually move it out
of staging. Driver changes were tested using a TSL2771 hooked up to a
Raspberry Pi 2.

Thanks to Jon Brenner at AMS/TAOS for loaning me some hardware samples
to test my driver changes.

Changes since v1:
- checkpatch cleanups (oops)
- Split out the device tree documentation and driver change into
  two separate commits so that the changes can be accepted in separate
  trees to avoid merge conflicts later.
- New patch: migrate in_illuminance0_integration_time sysfs
  attribute to iio_chan_spec
- New patch: check return value from tsl2x7x_invoke_change()

Brian Masney (9):
  staging: iio: tsl2x7x: add of_match table for device tree support
  staging: iio: tsl2x7x: add device tree documentation
  staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
  staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
  staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
  staging: iio: tsl2x7x: refactor {read,write}_event_value to allow
    handling multiple iio_event_infos
  staging: iio: tsl2x7x: use usleep_range() instead of mdelay()
  staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs
    attribute to iio_chan_spec
  staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

 .../devicetree/bindings/trivial-devices.txt        |  10 +
 drivers/staging/iio/light/tsl2x7x.c                | 440 ++++++++++-----------
 2 files changed, 209 insertions(+), 241 deletions(-)

-- 
2.9.4

[PATCH v2 1/9] staging: iio: tsl2x7x: add of_match table for device tree support

[Brian Masney]

Add device tree support for the tsl2x7x IIO driver with no custom
properties. The device tree documentation is in a separate commit so
that the changes to trivial-devices.txt can go in via the device
tree subsystem.

Signed-off-by: Brian Masney <masneyb@onstation.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 drivers/staging/iio/light/tsl2x7x.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 1467199..8148986 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -2026,6 +2026,21 @@ static struct i2c_device_id tsl2x7x_idtable[] = {
 
 MODULE_DEVICE_TABLE(i2c, tsl2x7x_idtable);
 
+static const struct of_device_id tsl2x7x_of_match[] = {
+	{ .compatible = "amstaos,tsl2571" },
+	{ .compatible = "amstaos,tsl2671" },
+	{ .compatible = "amstaos,tmd2671" },
+	{ .compatible = "amstaos,tsl2771" },
+	{ .compatible = "amstaos,tmd2771" },
+	{ .compatible = "amstaos,tsl2572" },
+	{ .compatible = "amstaos,tsl2672" },
+	{ .compatible = "amstaos,tmd2672" },
+	{ .compatible = "amstaos,tsl2772" },
+	{ .compatible = "amstaos,tmd2772" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tsl2x7x_of_match);
+
 static const struct dev_pm_ops tsl2x7x_pm_ops = {
 	.suspend = tsl2x7x_suspend,
 	.resume  = tsl2x7x_resume,
@@ -2035,6 +2050,7 @@ static const struct dev_pm_ops tsl2x7x_pm_ops = {
 static struct i2c_driver tsl2x7x_driver = {
 	.driver = {
 		.name = "tsl2x7x",
+		.of_match_table = tsl2x7x_of_match,
 		.pm = &tsl2x7x_pm_ops,
 	},
 	.id_table = tsl2x7x_idtable,
-- 
2.9.4

Re: [PATCH v2 1/9] staging: iio: tsl2x7x: add of_match table for device tree support

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:18 -0400
Brian Masney <masneyb@onstation.org> wrote:

> Add device tree support for the tsl2x7x IIO driver with no custom
> properties. The device tree documentation is in a separate commit so
> that the changes to trivial-devices.txt can go in via the device
> tree subsystem.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.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 | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 1467199..8148986 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -2026,6 +2026,21 @@ static struct i2c_device_id tsl2x7x_idtable[] = {
>  
>  MODULE_DEVICE_TABLE(i2c, tsl2x7x_idtable);
>  
> +static const struct of_device_id tsl2x7x_of_match[] = {
> +	{ .compatible = "amstaos,tsl2571" },
> +	{ .compatible = "amstaos,tsl2671" },
> +	{ .compatible = "amstaos,tmd2671" },
> +	{ .compatible = "amstaos,tsl2771" },
> +	{ .compatible = "amstaos,tmd2771" },
> +	{ .compatible = "amstaos,tsl2572" },
> +	{ .compatible = "amstaos,tsl2672" },
> +	{ .compatible = "amstaos,tmd2672" },
> +	{ .compatible = "amstaos,tsl2772" },
> +	{ .compatible = "amstaos,tmd2772" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, tsl2x7x_of_match);
> +
>  static const struct dev_pm_ops tsl2x7x_pm_ops = {
>  	.suspend = tsl2x7x_suspend,
>  	.resume  = tsl2x7x_resume,
> @@ -2035,6 +2050,7 @@ static const struct dev_pm_ops tsl2x7x_pm_ops = {
>  static struct i2c_driver tsl2x7x_driver = {
>  	.driver = {
>  		.name = "tsl2x7x",
> +		.of_match_table = tsl2x7x_of_match,
>  		.pm = &tsl2x7x_pm_ops,
>  	},
>  	.id_table = tsl2x7x_idtable,

[PATCH v2 2/9] staging: iio: tsl2x7x: add device tree documentation

[Brian Masney]

Add device tree documentation for the tsl2x7x IIO driver.

Signed-off-by: Brian Masney <masneyb@onstation.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/trivial-devices.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
index 35f406d..0e6e953 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -21,6 +21,16 @@ adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
 adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 ams,iaq-core		AMS iAQ-Core VOC Sensor
+amstaos,tsl2571		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2671		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2671		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2771		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2771		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2572		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2672		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2672		AMS/TAOS ALS and proximity sensor
+amstaos,tsl2772		AMS/TAOS ALS and proximity sensor
+amstaos,tmd2772		AMS/TAOS ALS and proximity sensor
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,at97sc3204t	i2c trusted platform module (TPM)
 capella,cm32181		CM32181: Ambient Light Sensor
-- 
2.9.4

Re: [PATCH v2 2/9] staging: iio: tsl2x7x: add device tree documentation

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:19 -0400
Brian Masney <masneyb@onstation.org> wrote:

> Add device tree documentation for the tsl2x7x IIO driver.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
For what it's worth given the simplicity of the patch

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  Documentation/devicetree/bindings/trivial-devices.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
> index 35f406d..0e6e953 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/trivial-devices.txt
> @@ -21,6 +21,16 @@ adi,adt7490		+/-1C TDM Extended Temp Range I.C
>  adi,adxl345		Three-Axis Digital Accelerometer
>  adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
>  ams,iaq-core		AMS iAQ-Core VOC Sensor
> +amstaos,tsl2571		AMS/TAOS ALS and proximity sensor
> +amstaos,tsl2671		AMS/TAOS ALS and proximity sensor
> +amstaos,tmd2671		AMS/TAOS ALS and proximity sensor
> +amstaos,tsl2771		AMS/TAOS ALS and proximity sensor
> +amstaos,tmd2771		AMS/TAOS ALS and proximity sensor
> +amstaos,tsl2572		AMS/TAOS ALS and proximity sensor
> +amstaos,tsl2672		AMS/TAOS ALS and proximity sensor
> +amstaos,tmd2672		AMS/TAOS ALS and proximity sensor
> +amstaos,tsl2772		AMS/TAOS ALS and proximity sensor
> +amstaos,tmd2772		AMS/TAOS ALS and proximity sensor
>  at,24c08		i2c serial eeprom  (24cxx)
>  atmel,at97sc3204t	i2c trusted platform module (TPM)
>  capella,cm32181		CM32181: Ambient Light Sensor

Re: [PATCH v2 2/9] staging: iio: tsl2x7x: add device tree documentation

[Rob Herring]

On Thu, Jul 06, 2017 at 06:56:19PM -0400, Brian Masney wrote:
> Add device tree documentation for the tsl2x7x IIO driver.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/trivial-devices.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Applied.

Rob

[PATCH v2 3/9] staging: iio: tsl2x7x: remove redundant power_state sysfs attribute

[Brian Masney]

The TSL2X7X driver has a custom power_state sysfs attribute. Remove this
attribute since the runtime power management code provides a sysfs
attribute to control the power state of the device.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 8148986..95ed7cc6 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -915,33 +915,6 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 		tsl2x7x_chip_on(indio_dev);
 }
 
-static ssize_t power_state_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-
-	return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
-}
-
-static ssize_t power_state_store(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	bool value;
-
-	if (strtobool(buf, &value))
-		return -EINVAL;
-
-	if (value)
-		tsl2x7x_chip_on(indio_dev);
-	else
-		tsl2x7x_chip_off(indio_dev);
-
-	return len;
-}
-
 static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
@@ -1494,8 +1467,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static DEVICE_ATTR_RW(power_state);
-
 static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
 
 static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
@@ -1580,7 +1551,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 }
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
@@ -1591,13 +1561,11 @@ static struct attribute *tsl2x7x_ALS_device_attrs[] = {
 };
 
 static struct attribute *tsl2x7x_PRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_proximity0_calibrate.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
@@ -1609,14 +1577,12 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
 };
 
 static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_proximity0_calibrate.attr,
 	&dev_attr_in_proximity0_calibscale_available.attr,
 	NULL
 };
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_in_illuminance0_calibscale_available.attr,
 	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
-- 
2.9.4

Re: [PATCH v2 3/9] staging: iio: tsl2x7x: remove redundant power_state sysfs attribute

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:20 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The TSL2X7X driver has a custom power_state sysfs attribute. Remove this
> attribute since the runtime power management code provides a sysfs
> attribute to control the power state of the device.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 34 ----------------------------------
>  1 file changed, 34 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 8148986..95ed7cc6 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -915,33 +915,6 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  		tsl2x7x_chip_on(indio_dev);
>  }
>  
> -static ssize_t power_state_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> -{
> -	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
> -
> -	return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
> -}
> -
> -static ssize_t power_state_store(struct device *dev,
> -					 struct device_attribute *attr,
> -					 const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	bool value;
> -
> -	if (strtobool(buf, &value))
> -		return -EINVAL;
> -
> -	if (value)
> -		tsl2x7x_chip_on(indio_dev);
> -	else
> -		tsl2x7x_chip_off(indio_dev);
> -
> -	return len;
> -}
> -
>  static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
>  					   struct device_attribute *attr,
>  					   char *buf)
> @@ -1494,8 +1467,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static DEVICE_ATTR_RW(power_state);
> -
>  static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>  
>  static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
> @@ -1580,7 +1551,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  }
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
>  	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> @@ -1591,13 +1561,11 @@ static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>  };
>  
>  static struct attribute *tsl2x7x_PRX_device_attrs[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_in_proximity0_calibrate.attr,
>  	NULL
>  };
>  
>  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
>  	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> @@ -1609,14 +1577,12 @@ static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  };
>  
>  static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_in_proximity0_calibrate.attr,
>  	&dev_attr_in_proximity0_calibscale_available.attr,
>  	NULL
>  };
>  
>  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
>  	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,

[PATCH v2 4/9] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()

[Brian Masney]

tsl2x7x_i2c_read() would call i2c_smbus_write_byte() and
i2c_smbus_read_byte(). These two i2c functions can be replaced with a
single call to i2c_smbus_read_byte_data(). This patch removes the
tsl2x7x_i2c_read() function and replaces all occurrences with a call to
i2c_smbus_read_byte_data().

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 95ed7cc6..6c9f06b 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -285,35 +285,6 @@ static const u8 device_channel_config[] = {
 };
 
 /**
- * tsl2x7x_i2c_read() - Read a byte from a register.
- * @client:	i2c client
- * @reg:	device register to read from
- * @*val:	pointer to location to store register contents.
- *
- */
-static int
-tsl2x7x_i2c_read(struct i2c_client *client, u8 reg, u8 *val)
-{
-	int ret;
-
-	/* select register to write */
-	ret = i2c_smbus_write_byte(client, (TSL2X7X_CMD_REG | reg));
-	if (ret < 0) {
-		dev_err(&client->dev, "failed to write register %x\n", reg);
-		return ret;
-	}
-
-	/* read the data */
-	ret = i2c_smbus_read_byte(client);
-	if (ret >= 0)
-		*val = (u8)ret;
-	else
-		dev_err(&client->dev, "failed to read register %x\n", reg);
-
-	return ret;
-}
-
-/**
  * tsl2x7x_get_lux() - Reads and calculates current lux value.
  * @indio_dev:	pointer to IIO device
  *
@@ -352,15 +323,15 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 		goto out_unlock;
 	}
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &buf[0]);
+	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__);
 		goto out_unlock;
 	}
 	/* is data new & valid */
-	if (!(buf[0] & TSL2X7X_STA_ADC_VALID)) {
+	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
 		dev_err(&chip->client->dev,
 			"%s: data not valid yet\n", __func__);
 		ret = chip->als_cur_info.lux; /* return LAST VALUE */
@@ -368,14 +339,16 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	}
 
 	for (i = 0; i < 4; i++) {
-		ret = tsl2x7x_i2c_read(chip->client,
-				       (TSL2X7X_CMD_REG |
-				       (TSL2X7X_ALS_CHAN0LO + i)), &buf[i]);
+		int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
+
+		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		if (ret < 0) {
 			dev_err(&chip->client->dev,
 				"failed to read. err=%x\n", ret);
 			goto out_unlock;
 		}
+
+		buf[i] = ret;
 	}
 
 	/* clear any existing interrupt status */
@@ -475,7 +448,6 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 {
 	int i;
 	int ret;
-	u8 status;
 	u8 chdata[2];
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 
@@ -485,8 +457,8 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 		return -EBUSY;
 	}
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &status);
+	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);
 		goto prox_poll_err;
@@ -498,7 +470,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	case tmd2671:
 	case tsl2771:
 	case tmd2771:
-		if (!(status & TSL2X7X_STA_ADC_VALID))
+		if (!(ret & TSL2X7X_STA_ADC_VALID))
 			goto prox_poll_err;
 	break;
 	case tsl2572:
@@ -506,17 +478,19 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
 	case tmd2672:
 	case tsl2772:
 	case tmd2772:
-		if (!(status & TSL2X7X_STA_PRX_VALID))
+		if (!(ret & TSL2X7X_STA_PRX_VALID))
 			goto prox_poll_err;
 	break;
 	}
 
 	for (i = 0; i < 2; i++) {
-		ret = tsl2x7x_i2c_read(chip->client,
-				       (TSL2X7X_CMD_REG |
-				       (TSL2X7X_PRX_LO + i)), &chdata[i]);
+		int reg = TSL2X7X_CMD_REG | (TSL2X7X_PRX_LO + i);
+
+		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		if (ret < 0)
 			goto prox_poll_err;
+
+		chdata[i] = ret;
 	}
 
 	chip->prox_data =
@@ -1486,7 +1460,7 @@ static DEVICE_ATTR_RW(in_intensity0_thresh_period);
 static DEVICE_ATTR_RW(in_proximity0_thresh_period);
 
 /* Use the default register values to identify the Taos device */
-static int tsl2x7x_device_id(unsigned char *id, int target)
+static int tsl2x7x_device_id(int *id, int target)
 {
 	switch (target) {
 	case tsl2571:
@@ -1843,7 +1817,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 			 const struct i2c_device_id *id)
 {
 	int ret;
-	unsigned char device_id;
 	struct iio_dev *indio_dev;
 	struct tsl2X7X_chip *chip;
 
@@ -1855,13 +1828,13 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
 	chip->client = clientp;
 	i2c_set_clientdata(clientp, indio_dev);
 
-	ret = tsl2x7x_i2c_read(chip->client,
-			       TSL2X7X_CHIPID, &device_id);
+	ret = i2c_smbus_read_byte_data(chip->client,
+				       TSL2X7X_CMD_REG | TSL2X7X_CHIPID);
 	if (ret < 0)
 		return ret;
 
-	if ((!tsl2x7x_device_id(&device_id, id->driver_data)) ||
-	    (tsl2x7x_device_id(&device_id, id->driver_data) == -EINVAL)) {
+	if ((!tsl2x7x_device_id(&ret, id->driver_data)) ||
+	    (tsl2x7x_device_id(&ret, id->driver_data) == -EINVAL)) {
 		dev_info(&chip->client->dev,
 			 "%s: i2c device found does not match expected id\n",
 				__func__);
-- 
2.9.4

Re: [PATCH v2 4/9] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:21 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_i2c_read() would call i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two i2c functions can be replaced with a
> single call to i2c_smbus_read_byte_data(). This patch removes the
> tsl2x7x_i2c_read() function and replaces all occurrences with a call to
> i2c_smbus_read_byte_data().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 71 ++++++++++++-------------------------
>  1 file changed, 22 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 95ed7cc6..6c9f06b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -285,35 +285,6 @@ static const u8 device_channel_config[] = {
>  };
>  
>  /**
> - * tsl2x7x_i2c_read() - Read a byte from a register.
> - * @client:	i2c client
> - * @reg:	device register to read from
> - * @*val:	pointer to location to store register contents.
> - *
> - */
> -static int
> -tsl2x7x_i2c_read(struct i2c_client *client, u8 reg, u8 *val)
> -{
> -	int ret;
> -
> -	/* select register to write */
> -	ret = i2c_smbus_write_byte(client, (TSL2X7X_CMD_REG | reg));
> -	if (ret < 0) {
> -		dev_err(&client->dev, "failed to write register %x\n", reg);
> -		return ret;
> -	}
> -
> -	/* read the data */
> -	ret = i2c_smbus_read_byte(client);
> -	if (ret >= 0)
> -		*val = (u8)ret;
> -	else
> -		dev_err(&client->dev, "failed to read register %x\n", reg);
> -
> -	return ret;
> -}
> -
> -/**
>   * tsl2x7x_get_lux() - Reads and calculates current lux value.
>   * @indio_dev:	pointer to IIO device
>   *
> @@ -352,15 +323,15 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  		goto out_unlock;
>  	}
>  
> -	ret = tsl2x7x_i2c_read(chip->client,
> -			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &buf[0]);
> +	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__);
>  		goto out_unlock;
>  	}
>  	/* is data new & valid */
> -	if (!(buf[0] & TSL2X7X_STA_ADC_VALID)) {
> +	if (!(ret & TSL2X7X_STA_ADC_VALID)) {
>  		dev_err(&chip->client->dev,
>  			"%s: data not valid yet\n", __func__);
>  		ret = chip->als_cur_info.lux; /* return LAST VALUE */
> @@ -368,14 +339,16 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
>  	}
>  
>  	for (i = 0; i < 4; i++) {
> -		ret = tsl2x7x_i2c_read(chip->client,
> -				       (TSL2X7X_CMD_REG |
> -				       (TSL2X7X_ALS_CHAN0LO + i)), &buf[i]);
> +		int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
> +
> +		ret = i2c_smbus_read_byte_data(chip->client, reg);
>  		if (ret < 0) {
>  			dev_err(&chip->client->dev,
>  				"failed to read. err=%x\n", ret);
>  			goto out_unlock;
>  		}
> +
> +		buf[i] = ret;
>  	}
>  
>  	/* clear any existing interrupt status */
> @@ -475,7 +448,6 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  {
>  	int i;
>  	int ret;
> -	u8 status;
>  	u8 chdata[2];
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  
> @@ -485,8 +457,8 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  		return -EBUSY;
>  	}
>  
> -	ret = tsl2x7x_i2c_read(chip->client,
> -			       (TSL2X7X_CMD_REG | TSL2X7X_STATUS), &status);
> +	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);
>  		goto prox_poll_err;
> @@ -498,7 +470,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	case tmd2671:
>  	case tsl2771:
>  	case tmd2771:
> -		if (!(status & TSL2X7X_STA_ADC_VALID))
> +		if (!(ret & TSL2X7X_STA_ADC_VALID))
>  			goto prox_poll_err;
>  	break;
>  	case tsl2572:
> @@ -506,17 +478,19 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
>  	case tmd2672:
>  	case tsl2772:
>  	case tmd2772:
> -		if (!(status & TSL2X7X_STA_PRX_VALID))
> +		if (!(ret & TSL2X7X_STA_PRX_VALID))
>  			goto prox_poll_err;
>  	break;
>  	}
>  
>  	for (i = 0; i < 2; i++) {
> -		ret = tsl2x7x_i2c_read(chip->client,
> -				       (TSL2X7X_CMD_REG |
> -				       (TSL2X7X_PRX_LO + i)), &chdata[i]);
> +		int reg = TSL2X7X_CMD_REG | (TSL2X7X_PRX_LO + i);
> +
> +		ret = i2c_smbus_read_byte_data(chip->client, reg);
>  		if (ret < 0)
>  			goto prox_poll_err;
> +
> +		chdata[i] = ret;
>  	}
>  
>  	chip->prox_data =
> @@ -1486,7 +1460,7 @@ static DEVICE_ATTR_RW(in_intensity0_thresh_period);
>  static DEVICE_ATTR_RW(in_proximity0_thresh_period);
>  
>  /* Use the default register values to identify the Taos device */
> -static int tsl2x7x_device_id(unsigned char *id, int target)
> +static int tsl2x7x_device_id(int *id, int target)
>  {
>  	switch (target) {
>  	case tsl2571:
> @@ -1843,7 +1817,6 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  			 const struct i2c_device_id *id)
>  {
>  	int ret;
> -	unsigned char device_id;
>  	struct iio_dev *indio_dev;
>  	struct tsl2X7X_chip *chip;
>  
> @@ -1855,13 +1828,13 @@ static int tsl2x7x_probe(struct i2c_client *clientp,
>  	chip->client = clientp;
>  	i2c_set_clientdata(clientp, indio_dev);
>  
> -	ret = tsl2x7x_i2c_read(chip->client,
> -			       TSL2X7X_CHIPID, &device_id);
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2X7X_CMD_REG | TSL2X7X_CHIPID);
>  	if (ret < 0)
>  		return ret;
>  
> -	if ((!tsl2x7x_device_id(&device_id, id->driver_data)) ||
> -	    (tsl2x7x_device_id(&device_id, id->driver_data) == -EINVAL)) {
> +	if ((!tsl2x7x_device_id(&ret, id->driver_data)) ||
> +	    (tsl2x7x_device_id(&ret, id->driver_data) == -EINVAL)) {
>  		dev_info(&chip->client->dev,
>  			 "%s: i2c device found does not match expected id\n",
>  				__func__);

[PATCH v2 5/9] staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()

[Brian Masney]

The calibration function calls i2c_smbus_write_byte() and
i2c_smbus_read_byte(). These two function calls are replaced with a
single call to i2c_smbus_read_byte_data() by this patch. This patch
also removes an unnecessary call that reads the CNTRL register
a second time. One of the error paths returned -1 if the ADC was not
enabled and this patch changes that return value to -EINVAL.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 6c9f06b..5734628 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -542,39 +542,29 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
 static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-	u8 reg_val;
 	int gain_trim_val;
 	int ret;
 	int lux_val;
 
-	ret = i2c_smbus_write_byte(chip->client,
-				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
+	ret = i2c_smbus_read_byte_data(chip->client,
+				       TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
 	if (ret < 0) {
 		dev_err(&chip->client->dev,
-			"failed to write CNTRL register, ret=%d\n", ret);
+			"%s: failed to read from the CNTRL register\n",
+			__func__);
 		return ret;
 	}
 
-	reg_val = i2c_smbus_read_byte(chip->client);
-	if ((reg_val & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
-		!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
-		dev_err(&chip->client->dev,
-			"%s: failed: ADC not enabled\n", __func__);
-		return -1;
-	}
-
-	ret = i2c_smbus_write_byte(chip->client,
-				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
-	if (ret < 0) {
+	if ((ret & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
+			!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
 		dev_err(&chip->client->dev,
-			"failed to write ctrl reg: ret=%d\n", ret);
-		return ret;
-	}
-
-	reg_val = i2c_smbus_read_byte(chip->client);
-	if ((reg_val & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
+			"%s: Device is not powered on and/or ADC is not enabled\n",
+			__func__);
+		return -EINVAL;
+	} else if ((ret & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
 		dev_err(&chip->client->dev,
-			"%s: failed: STATUS - ADC not valid.\n", __func__);
+			"%s: The two ADC channels have not completed an integration cycle\n",
+			__func__);
 		return -ENODATA;
 	}
 
-- 
2.9.4

Re: [PATCH v2 5/9] staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:22 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The calibration function calls i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two function calls are replaced with a
> single call to i2c_smbus_read_byte_data() by this patch. This patch
> also removes an unnecessary call that reads the CNTRL register
> a second time. One of the error paths returned -1 if the ADC was not
> enabled and this patch changes that return value to -EINVAL.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 6c9f06b..5734628 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -542,39 +542,29 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
>  static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	u8 reg_val;
>  	int gain_trim_val;
>  	int ret;
>  	int lux_val;
>  
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
>  	if (ret < 0) {
>  		dev_err(&chip->client->dev,
> -			"failed to write CNTRL register, ret=%d\n", ret);
> +			"%s: failed to read from the CNTRL register\n",
> +			__func__);
>  		return ret;
>  	}
>  
> -	reg_val = i2c_smbus_read_byte(chip->client);
> -	if ((reg_val & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> -		!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
> -		dev_err(&chip->client->dev,
> -			"%s: failed: ADC not enabled\n", __func__);
> -		return -1;
> -	}
> -
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   (TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> -	if (ret < 0) {
> +	if ((ret & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> +			!= (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
>  		dev_err(&chip->client->dev,
> -			"failed to write ctrl reg: ret=%d\n", ret);
> -		return ret;
> -	}
> -
> -	reg_val = i2c_smbus_read_byte(chip->client);
> -	if ((reg_val & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
> +			"%s: Device is not powered on and/or ADC is not enabled\n",
> +			__func__);
> +		return -EINVAL;
> +	} else if ((ret & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
>  		dev_err(&chip->client->dev,
> -			"%s: failed: STATUS - ADC not valid.\n", __func__);
> +			"%s: The two ADC channels have not completed an integration cycle\n",
> +			__func__);
>  		return -ENODATA;
>  	}
>  

[PATCH v2 6/9] staging: iio: tsl2x7x: refactor {read,write}_event_value to allow handling multiple iio_event_infos

[Brian Masney]

tsl2x7x_read_thresh() and tsl2x7x_write_thresh() currently assumes
that IIO_EV_INFO_VALUE is the only iio_event_info that will be
passed in.  This patch refactors these two functions so that
additional iio_event_infos can be passed in. The functions are
renamed from tsl2x7x_{read,write}_thresh() to
tsl2x7x_{read,write}_event_value(). This patch also adds the
missing return value check to tsl2x7x_invoke_change() since this
was previously missing. This patch is in preparation for moving
the in_intensity0_thresh_period and in_proximity0_thresh_period sysfs
attributes to be created by iio_event_spec.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 5734628..d80f5dc 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1218,78 +1218,101 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static int tsl2x7x_write_thresh(struct iio_dev *indio_dev,
-				const struct iio_chan_spec *chan,
-				enum iio_event_type type,
-				enum iio_event_direction dir,
-				enum iio_event_info info,
-				int val, int val2)
+static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	int ret = -EINVAL;
 
-	if (chan->type == IIO_INTENSITY) {
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			chip->tsl2x7x_settings.als_thresh_high = val;
-			break;
-		case IIO_EV_DIR_FALLING:
-			chip->tsl2x7x_settings.als_thresh_low = val;
-			break;
-		default:
-			return -EINVAL;
-		}
-	} else {
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			chip->tsl2x7x_settings.prox_thres_high = val;
-			break;
-		case IIO_EV_DIR_FALLING:
-			chip->tsl2x7x_settings.prox_thres_low = val;
-			break;
-		default:
-			return -EINVAL;
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		if (chan->type == IIO_INTENSITY) {
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				chip->tsl2x7x_settings.als_thresh_high = val;
+				ret = 0;
+				break;
+			case IIO_EV_DIR_FALLING:
+				chip->tsl2x7x_settings.als_thresh_low = val;
+				ret = 0;
+				break;
+			default:
+				break;
+			}
+		} else {
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				chip->tsl2x7x_settings.prox_thres_high = val;
+				ret = 0;
+				break;
+			case IIO_EV_DIR_FALLING:
+				chip->tsl2x7x_settings.prox_thres_low = val;
+				ret = 0;
+				break;
+			default:
+				break;
+			}
 		}
+		break;
+	default:
+		break;
 	}
 
-	tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
-	return 0;
+	return tsl2x7x_invoke_change(indio_dev);
 }
 
-static int tsl2x7x_read_thresh(struct iio_dev *indio_dev,
-			       const struct iio_chan_spec *chan,
-			       enum iio_event_type type,
-			       enum iio_event_direction dir,
-				   enum iio_event_info info,
-			       int *val, int *val2)
+static int tsl2x7x_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	int ret = -EINVAL;
 
-	if (chan->type == IIO_INTENSITY) {
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			*val = chip->tsl2x7x_settings.als_thresh_high;
-			break;
-		case IIO_EV_DIR_FALLING:
-			*val = chip->tsl2x7x_settings.als_thresh_low;
-			break;
-		default:
-			return -EINVAL;
-		}
-	} else {
-		switch (dir) {
-		case IIO_EV_DIR_RISING:
-			*val = chip->tsl2x7x_settings.prox_thres_high;
-			break;
-		case IIO_EV_DIR_FALLING:
-			*val = chip->tsl2x7x_settings.prox_thres_low;
-			break;
-		default:
-			return -EINVAL;
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		if (chan->type == IIO_INTENSITY) {
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				*val = chip->tsl2x7x_settings.als_thresh_high;
+				ret = IIO_VAL_INT;
+				break;
+			case IIO_EV_DIR_FALLING:
+				*val = chip->tsl2x7x_settings.als_thresh_low;
+				ret = IIO_VAL_INT;
+				break;
+			default:
+				break;
+			}
+		} else {
+			switch (dir) {
+			case IIO_EV_DIR_RISING:
+				*val = chip->tsl2x7x_settings.prox_thres_high;
+				ret = IIO_VAL_INT;
+				break;
+			case IIO_EV_DIR_FALLING:
+				*val = chip->tsl2x7x_settings.prox_thres_low;
+				ret = IIO_VAL_INT;
+				break;
+			default:
+				break;
+			}
 		}
+		break;
+	default:
+		break;
 	}
 
-	return IIO_VAL_INT;
+	return ret;
 }
 
 static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
@@ -1614,8 +1637,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
 		.driver_module = THIS_MODULE,
 		.read_raw = &tsl2x7x_read_raw,
 		.write_raw = &tsl2x7x_write_raw,
-		.read_event_value = &tsl2x7x_read_thresh,
-		.write_event_value = &tsl2x7x_write_thresh,
+		.read_event_value = &tsl2x7x_read_event_value,
+		.write_event_value = &tsl2x7x_write_event_value,
 		.read_event_config = &tsl2x7x_read_interrupt_config,
 		.write_event_config = &tsl2x7x_write_interrupt_config,
 	},
@@ -1625,8 +1648,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
 		.driver_module = THIS_MODULE,
 		.read_raw = &tsl2x7x_read_raw,
 		.write_raw = &tsl2x7x_write_raw,
-		.read_event_value = &tsl2x7x_read_thresh,
-		.write_event_value = &tsl2x7x_write_thresh,
+		.read_event_value = &tsl2x7x_read_event_value,
+		.write_event_value = &tsl2x7x_write_event_value,
 		.read_event_config = &tsl2x7x_read_interrupt_config,
 		.write_event_config = &tsl2x7x_write_interrupt_config,
 	},
@@ -1636,8 +1659,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
 		.driver_module = THIS_MODULE,
 		.read_raw = &tsl2x7x_read_raw,
 		.write_raw = &tsl2x7x_write_raw,
-		.read_event_value = &tsl2x7x_read_thresh,
-		.write_event_value = &tsl2x7x_write_thresh,
+		.read_event_value = &tsl2x7x_read_event_value,
+		.write_event_value = &tsl2x7x_write_event_value,
 		.read_event_config = &tsl2x7x_read_interrupt_config,
 		.write_event_config = &tsl2x7x_write_interrupt_config,
 	},
@@ -1647,8 +1670,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
 		.driver_module = THIS_MODULE,
 		.read_raw = &tsl2x7x_read_raw,
 		.write_raw = &tsl2x7x_write_raw,
-		.read_event_value = &tsl2x7x_read_thresh,
-		.write_event_value = &tsl2x7x_write_thresh,
+		.read_event_value = &tsl2x7x_read_event_value,
+		.write_event_value = &tsl2x7x_write_event_value,
 		.read_event_config = &tsl2x7x_read_interrupt_config,
 		.write_event_config = &tsl2x7x_write_interrupt_config,
 	},
@@ -1658,8 +1681,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
 		.driver_module = THIS_MODULE,
 		.read_raw = &tsl2x7x_read_raw,
 		.write_raw = &tsl2x7x_write_raw,
-		.read_event_value = &tsl2x7x_read_thresh,
-		.write_event_value = &tsl2x7x_write_thresh,
+		.read_event_value = &tsl2x7x_read_event_value,
+		.write_event_value = &tsl2x7x_write_event_value,
 		.read_event_config = &tsl2x7x_read_interrupt_config,
 		.write_event_config = &tsl2x7x_write_interrupt_config,
 	},
-- 
2.9.4

Re: [PATCH v2 6/9] staging: iio: tsl2x7x: refactor {read,write}_event_value to allow handling multiple iio_event_infos

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:23 -0400
Brian Masney <masneyb@onstation.org> wrote:

> tsl2x7x_read_thresh() and tsl2x7x_write_thresh() currently assumes
> that IIO_EV_INFO_VALUE is the only iio_event_info that will be
> passed in.  This patch refactors these two functions so that
> additional iio_event_infos can be passed in. The functions are
> renamed from tsl2x7x_{read,write}_thresh() to
> tsl2x7x_{read,write}_event_value(). This patch also adds the
> missing return value check to tsl2x7x_invoke_change() since this
> was previously missing. This patch is in preparation for moving
> the in_intensity0_thresh_period and in_proximity0_thresh_period sysfs
> attributes to be created by iio_event_spec.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.  Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 157 +++++++++++++++++++++---------------
>  1 file changed, 90 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 5734628..d80f5dc 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1218,78 +1218,101 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static int tsl2x7x_write_thresh(struct iio_dev *indio_dev,
> -				const struct iio_chan_spec *chan,
> -				enum iio_event_type type,
> -				enum iio_event_direction dir,
> -				enum iio_event_info info,
> -				int val, int val2)
> +static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     enum iio_event_type type,
> +				     enum iio_event_direction dir,
> +				     enum iio_event_info info,
> +				     int val, int val2)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
>  
> -	if (chan->type == IIO_INTENSITY) {
> -		switch (dir) {
> -		case IIO_EV_DIR_RISING:
> -			chip->tsl2x7x_settings.als_thresh_high = val;
> -			break;
> -		case IIO_EV_DIR_FALLING:
> -			chip->tsl2x7x_settings.als_thresh_low = val;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -	} else {
> -		switch (dir) {
> -		case IIO_EV_DIR_RISING:
> -			chip->tsl2x7x_settings.prox_thres_high = val;
> -			break;
> -		case IIO_EV_DIR_FALLING:
> -			chip->tsl2x7x_settings.prox_thres_low = val;
> -			break;
> -		default:
> -			return -EINVAL;
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		if (chan->type == IIO_INTENSITY) {
> +			switch (dir) {
> +			case IIO_EV_DIR_RISING:
> +				chip->tsl2x7x_settings.als_thresh_high = val;
> +				ret = 0;
> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				chip->tsl2x7x_settings.als_thresh_low = val;
> +				ret = 0;
> +				break;
> +			default:
> +				break;
> +			}
> +		} else {
> +			switch (dir) {
> +			case IIO_EV_DIR_RISING:
> +				chip->tsl2x7x_settings.prox_thres_high = val;
> +				ret = 0;
> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				chip->tsl2x7x_settings.prox_thres_low = val;
> +				ret = 0;
> +				break;
> +			default:
> +				break;
> +			}
>  		}
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
> -	return 0;
> +	return tsl2x7x_invoke_change(indio_dev);
>  }
>  
> -static int tsl2x7x_read_thresh(struct iio_dev *indio_dev,
> -			       const struct iio_chan_spec *chan,
> -			       enum iio_event_type type,
> -			       enum iio_event_direction dir,
> -				   enum iio_event_info info,
> -			       int *val, int *val2)
> +static int tsl2x7x_read_event_value(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    enum iio_event_type type,
> +				    enum iio_event_direction dir,
> +				    enum iio_event_info info,
> +				    int *val, int *val2)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
>  
> -	if (chan->type == IIO_INTENSITY) {
> -		switch (dir) {
> -		case IIO_EV_DIR_RISING:
> -			*val = chip->tsl2x7x_settings.als_thresh_high;
> -			break;
> -		case IIO_EV_DIR_FALLING:
> -			*val = chip->tsl2x7x_settings.als_thresh_low;
> -			break;
> -		default:
> -			return -EINVAL;
> -		}
> -	} else {
> -		switch (dir) {
> -		case IIO_EV_DIR_RISING:
> -			*val = chip->tsl2x7x_settings.prox_thres_high;
> -			break;
> -		case IIO_EV_DIR_FALLING:
> -			*val = chip->tsl2x7x_settings.prox_thres_low;
> -			break;
> -		default:
> -			return -EINVAL;
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		if (chan->type == IIO_INTENSITY) {
> +			switch (dir) {
> +			case IIO_EV_DIR_RISING:
> +				*val = chip->tsl2x7x_settings.als_thresh_high;
> +				ret = IIO_VAL_INT;
> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				*val = chip->tsl2x7x_settings.als_thresh_low;
> +				ret = IIO_VAL_INT;
> +				break;
> +			default:
> +				break;
> +			}
> +		} else {
> +			switch (dir) {
> +			case IIO_EV_DIR_RISING:
> +				*val = chip->tsl2x7x_settings.prox_thres_high;
> +				ret = IIO_VAL_INT;
> +				break;
> +			case IIO_EV_DIR_FALLING:
> +				*val = chip->tsl2x7x_settings.prox_thres_low;
> +				ret = IIO_VAL_INT;
> +				break;
> +			default:
> +				break;
> +			}
>  		}
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	return IIO_VAL_INT;
> +	return ret;
>  }
>  
>  static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
> @@ -1614,8 +1637,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
>  		.driver_module = THIS_MODULE,
>  		.read_raw = &tsl2x7x_read_raw,
>  		.write_raw = &tsl2x7x_write_raw,
> -		.read_event_value = &tsl2x7x_read_thresh,
> -		.write_event_value = &tsl2x7x_write_thresh,
> +		.read_event_value = &tsl2x7x_read_event_value,
> +		.write_event_value = &tsl2x7x_write_event_value,
>  		.read_event_config = &tsl2x7x_read_interrupt_config,
>  		.write_event_config = &tsl2x7x_write_interrupt_config,
>  	},
> @@ -1625,8 +1648,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
>  		.driver_module = THIS_MODULE,
>  		.read_raw = &tsl2x7x_read_raw,
>  		.write_raw = &tsl2x7x_write_raw,
> -		.read_event_value = &tsl2x7x_read_thresh,
> -		.write_event_value = &tsl2x7x_write_thresh,
> +		.read_event_value = &tsl2x7x_read_event_value,
> +		.write_event_value = &tsl2x7x_write_event_value,
>  		.read_event_config = &tsl2x7x_read_interrupt_config,
>  		.write_event_config = &tsl2x7x_write_interrupt_config,
>  	},
> @@ -1636,8 +1659,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
>  		.driver_module = THIS_MODULE,
>  		.read_raw = &tsl2x7x_read_raw,
>  		.write_raw = &tsl2x7x_write_raw,
> -		.read_event_value = &tsl2x7x_read_thresh,
> -		.write_event_value = &tsl2x7x_write_thresh,
> +		.read_event_value = &tsl2x7x_read_event_value,
> +		.write_event_value = &tsl2x7x_write_event_value,
>  		.read_event_config = &tsl2x7x_read_interrupt_config,
>  		.write_event_config = &tsl2x7x_write_interrupt_config,
>  	},
> @@ -1647,8 +1670,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
>  		.driver_module = THIS_MODULE,
>  		.read_raw = &tsl2x7x_read_raw,
>  		.write_raw = &tsl2x7x_write_raw,
> -		.read_event_value = &tsl2x7x_read_thresh,
> -		.write_event_value = &tsl2x7x_write_thresh,
> +		.read_event_value = &tsl2x7x_read_event_value,
> +		.write_event_value = &tsl2x7x_write_event_value,
>  		.read_event_config = &tsl2x7x_read_interrupt_config,
>  		.write_event_config = &tsl2x7x_write_interrupt_config,
>  	},
> @@ -1658,8 +1681,8 @@ static const struct iio_info tsl2X7X_device_info[] = {
>  		.driver_module = THIS_MODULE,
>  		.read_raw = &tsl2x7x_read_raw,
>  		.write_raw = &tsl2x7x_write_raw,
> -		.read_event_value = &tsl2x7x_read_thresh,
> -		.write_event_value = &tsl2x7x_write_thresh,
> +		.read_event_value = &tsl2x7x_read_event_value,
> +		.write_event_value = &tsl2x7x_write_event_value,
>  		.read_event_config = &tsl2x7x_read_interrupt_config,
>  		.write_event_config = &tsl2x7x_write_interrupt_config,
>  	},

[PATCH v2 7/9] staging: iio: tsl2x7x: use usleep_range() instead of mdelay()

[Brian Masney]

This driver in some cases can busy wait for upwards of 15ms. Since the
kernel at this point is not running in atomic context, and is running in
process context, we can safely use usleep_range() instead. This patch
changes the two occurrences of mdelay() to usleep_range().

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index d80f5dc..c38bd64 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -686,7 +686,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
 		}
 	}
 
-	mdelay(3);	/* Power-on settling time */
+	/* Power-on settling time */
+	usleep_range(3000, 3500);
 
 	/*
 	 * NOW enable the ADC
@@ -853,7 +854,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
 
 	/*gather the samples*/
 	for (i = 0; i < chip->tsl2x7x_settings.prox_max_samples_cal; i++) {
-		mdelay(15);
+		usleep_range(15000, 17500);
 		tsl2x7x_get_prox(indio_dev);
 		prox_history[i] = chip->prox_data;
 		dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",
-- 
2.9.4

Re: [PATCH v2 7/9] staging: iio: tsl2x7x: use usleep_range() instead of mdelay()

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:24 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This driver in some cases can busy wait for upwards of 15ms. Since the
> kernel at this point is not running in atomic context, and is running in
> process context, we can safely use usleep_range() instead. This patch
> changes the two occurrences of mdelay() to usleep_range().
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.  Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index d80f5dc..c38bd64 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -686,7 +686,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
>  		}
>  	}
>  
> -	mdelay(3);	/* Power-on settling time */
> +	/* Power-on settling time */
> +	usleep_range(3000, 3500);
>  
>  	/*
>  	 * NOW enable the ADC
> @@ -853,7 +854,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>  
>  	/*gather the samples*/
>  	for (i = 0; i < chip->tsl2x7x_settings.prox_max_samples_cal; i++) {
> -		mdelay(15);
> +		usleep_range(15000, 17500);
>  		tsl2x7x_get_prox(indio_dev);
>  		prox_history[i] = chip->prox_data;
>  		dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",

[PATCH v2 8/9] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec

[Brian Masney]

The driver explicitly creates the in_illuminance0_integration_time sysfs
attribute outside the IIO core. This attribute is available in the IIO
core so this patches migrates the attribute to be created by
the iio_chan_spec.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index c38bd64..033468d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -905,46 +905,6 @@ static ssize_t in_proximity0_calibscale_available_show(struct device *dev,
 		return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
 }
 
-static ssize_t in_illuminance0_integration_time_show(struct device *dev,
-				     struct device_attribute *attr,
-				     char *buf)
-{
-	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
-	int y, z;
-
-	y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->tsl2x7x_settings.als_time) + 1;
-	z = y * TSL2X7X_MIN_ITIME;
-	y /= 1000;
-	z %= 1000;
-
-	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
-}
-
-static ssize_t in_illuminance0_integration_time_store(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buf, size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
-	struct tsl2x7x_parse_result result;
-	int ret;
-
-	ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
-	if (ret)
-		return ret;
-
-	result.fract /= 3;
-	chip->tsl2x7x_settings.als_time =
-			TSL2X7X_MAX_TIMER_CNT - (u8)result.fract;
-
-	dev_info(&chip->client->dev, "%s: als time = %d",
-		 __func__, chip->tsl2x7x_settings.als_time);
-
-	tsl2x7x_invoke_change(indio_dev);
-
-	return IIO_VAL_INT_PLUS_MICRO;
-}
-
 static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
 		".00272 - .696");
 
@@ -1369,7 +1329,13 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
 		*val = chip->tsl2x7x_settings.als_gain_trim;
 		ret = IIO_VAL_INT;
 		break;
-
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = (TSL2X7X_MAX_TIMER_CNT - chip->tsl2x7x_settings.als_time)
+			+ 1;
+		*val2 = (*val * TSL2X7X_MIN_ITIME) % 1000;
+		*val /= 1000;
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -1445,7 +1411,13 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_CALIBBIAS:
 		chip->tsl2x7x_settings.als_gain_trim = val;
 		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		chip->tsl2x7x_settings.als_time =
+			TSL2X7X_MAX_TIMER_CNT - (val2 / 3);
 
+		dev_info(&chip->client->dev, "%s: als time = %d",
+			 __func__, chip->tsl2x7x_settings.als_time);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -1459,8 +1431,6 @@ static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
 
 static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
 
-static DEVICE_ATTR_RW(in_illuminance0_integration_time);
-
 static DEVICE_ATTR_RW(in_illuminance0_target_input);
 
 static DEVICE_ATTR_WO(in_illuminance0_calibrate);
@@ -1540,7 +1510,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
@@ -1555,7 +1524,6 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
@@ -1572,7 +1540,6 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&dev_attr_in_illuminance0_integration_time.attr,
 	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
@@ -1710,7 +1677,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
 			.type = IIO_LIGHT,
 			.indexed = 1,
 			.channel = 0,
-			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+					      BIT(IIO_CHAN_INFO_INT_TIME),
 			}, {
 			.type = IIO_INTENSITY,
 			.indexed = 1,
@@ -1749,7 +1717,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
 			.type = IIO_LIGHT,
 			.indexed = 1,
 			.channel = 0,
-			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+					      BIT(IIO_CHAN_INFO_INT_TIME),
 			}, {
 			.type = IIO_INTENSITY,
 			.indexed = 1,
@@ -1797,7 +1766,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
 			.type = IIO_LIGHT,
 			.indexed = 1,
 			.channel = 0,
-			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+					      BIT(IIO_CHAN_INFO_INT_TIME),
 			}, {
 			.type = IIO_INTENSITY,
 			.indexed = 1,
-- 
2.9.4

Re: [PATCH v2 8/9] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:25 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The driver explicitly creates the in_illuminance0_integration_time sysfs
> attribute outside the IIO core. This attribute is available in the IIO
> core so this patches migrates the attribute to be created by
> the iio_chan_spec.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 68 +++++++++++--------------------------
>  1 file changed, 19 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index c38bd64..033468d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -905,46 +905,6 @@ static ssize_t in_proximity0_calibscale_available_show(struct device *dev,
>  		return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
>  }
>  
> -static ssize_t in_illuminance0_integration_time_show(struct device *dev,
> -				     struct device_attribute *attr,
> -				     char *buf)
> -{
> -	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
> -	int y, z;
> -
> -	y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->tsl2x7x_settings.als_time) + 1;
> -	z = y * TSL2X7X_MIN_ITIME;
> -	y /= 1000;
> -	z %= 1000;
> -
> -	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
> -}
> -
> -static ssize_t in_illuminance0_integration_time_store(struct device *dev,
> -				      struct device_attribute *attr,
> -				      const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	struct tsl2x7x_parse_result result;
> -	int ret;
> -
> -	ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
> -	if (ret)
> -		return ret;
> -
> -	result.fract /= 3;
> -	chip->tsl2x7x_settings.als_time =
> -			TSL2X7X_MAX_TIMER_CNT - (u8)result.fract;
> -
> -	dev_info(&chip->client->dev, "%s: als time = %d",
> -		 __func__, chip->tsl2x7x_settings.als_time);
> -
> -	tsl2x7x_invoke_change(indio_dev);
> -
> -	return IIO_VAL_INT_PLUS_MICRO;
> -}
> -
>  static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
>  		".00272 - .696");
>  
> @@ -1369,7 +1329,13 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
>  		*val = chip->tsl2x7x_settings.als_gain_trim;
>  		ret = IIO_VAL_INT;
>  		break;
> -
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = (TSL2X7X_MAX_TIMER_CNT - chip->tsl2x7x_settings.als_time)
> +			+ 1;
> +		*val2 = (*val * TSL2X7X_MIN_ITIME) % 1000;
> +		*val /= 1000;
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -1445,7 +1411,13 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		chip->tsl2x7x_settings.als_gain_trim = val;
>  		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		chip->tsl2x7x_settings.als_time =
> +			TSL2X7X_MAX_TIMER_CNT - (val2 / 3);
Is that 3 the same one represented by MIN_ITME?

Looks like it so would be easier to review if it was using the
constant.

Thanks,

Jonathan
>  
> +		dev_info(&chip->client->dev, "%s: als time = %d",
> +			 __func__, chip->tsl2x7x_settings.als_time);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1459,8 +1431,6 @@ static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>  
>  static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
>  
> -static DEVICE_ATTR_RW(in_illuminance0_integration_time);
> -
>  static DEVICE_ATTR_RW(in_illuminance0_target_input);
>  
>  static DEVICE_ATTR_WO(in_illuminance0_calibrate);
> @@ -1540,7 +1510,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1555,7 +1524,6 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1572,7 +1540,6 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1710,7 +1677,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,
> @@ -1749,7 +1717,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,
> @@ -1797,7 +1766,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,

[PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

[Brian Masney]

The return value from tsl2x7x_invoke_change() was not checked in most
places in the driver. This patch adds the proper error checks. The
return values inside tsl2x7x_invoke_change() are now checked by
this patch as well.

Previously, if there was an error turning the chip back on, then the
driver would attempt to turn the chip off and incorrectly return
success. The code to power off the chip is removed by this patch
since we should fail fast.

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

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 033468d..c00278d 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -771,22 +771,24 @@ int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int device_status = chip->tsl2x7x_chip_status;
+	int ret;
 
 	mutex_lock(&chip->als_mutex);
 	mutex_lock(&chip->prox_mutex);
 
-	if (device_status == TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_off(indio_dev);
-
-	tsl2x7x_chip_on(indio_dev);
+	if (device_status == TSL2X7X_CHIP_WORKING) {
+		ret = tsl2x7x_chip_off(indio_dev);
+		if (ret < 0)
+			goto unlock;
+	}
 
-	if (device_status != TSL2X7X_CHIP_WORKING)
-		tsl2x7x_chip_off(indio_dev);
+	ret = tsl2x7x_chip_on(indio_dev);
 
+unlock:
 	mutex_unlock(&chip->prox_mutex);
 	mutex_unlock(&chip->als_mutex);
 
-	return 0;
+	return ret;
 }
 
 static
@@ -925,6 +927,7 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	unsigned long value;
+	int ret;
 
 	if (kstrtoul(buf, 0, &value))
 		return -EINVAL;
@@ -932,7 +935,9 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
 	if (value)
 		chip->tsl2x7x_settings.als_cal_target = value;
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -981,7 +986,9 @@ static ssize_t in_intensity0_thresh_period_store(struct device *dev,
 	dev_info(&chip->client->dev, "%s: als persistence = %d",
 		 __func__, filter_delay);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return IIO_VAL_INT_PLUS_MICRO;
 }
@@ -1029,7 +1036,10 @@ static ssize_t in_proximity0_thresh_period_store(struct device *dev,
 	dev_info(&chip->client->dev, "%s: prox persistence = %d",
 		 __func__, filter_delay);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
+
 
 	return IIO_VAL_INT_PLUS_MICRO;
 }
@@ -1040,6 +1050,7 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	bool value;
+	int ret;
 
 	if (strtobool(buf, &value))
 		return -EINVAL;
@@ -1047,7 +1058,9 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
 	if (value)
 		tsl2x7x_als_calibrate(indio_dev);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1087,7 +1100,7 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
 	int value[ARRAY_SIZE(chip->tsl2x7x_device_lux) * 3 + 1];
-	int n;
+	int n, ret;
 
 	get_options(buf, ARRAY_SIZE(value), value);
 
@@ -1115,7 +1128,9 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
 	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
 	memcpy(chip->tsl2x7x_device_lux, &value[1], (value[0] * 4));
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1126,6 +1141,7 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	bool value;
+	int ret;
 
 	if (strtobool(buf, &value))
 		return -EINVAL;
@@ -1133,7 +1149,9 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
 	if (value)
 		tsl2x7x_prox_cal(indio_dev);
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return len;
 }
@@ -1161,6 +1179,7 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 					  int val)
 {
 	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
+	int ret;
 
 	if (chan->type == IIO_INTENSITY) {
 		if (val)
@@ -1174,7 +1193,9 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
 			chip->tsl2x7x_settings.interrupts_en &= 0x10;
 	}
 
-	tsl2x7x_invoke_change(indio_dev);
+	ret = tsl2x7x_invoke_change(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }
@@ -1422,9 +1443,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
 		return -EINVAL;
 	}
 
-	tsl2x7x_invoke_change(indio_dev);
-
-	return 0;
+	return tsl2x7x_invoke_change(indio_dev);
 }
 
 static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
-- 
2.9.4

Re: [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()

[Jonathan Cameron]

On Thu,  6 Jul 2017 18:56:26 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The return value from tsl2x7x_invoke_change() was not checked in most
> places in the driver. This patch adds the proper error checks. The
> return values inside tsl2x7x_invoke_change() are now checked by
> this patch as well.
> 
> Previously, if there was an error turning the chip back on, then the
> driver would attempt to turn the chip off and incorrectly return
> success. The code to power off the chip is removed by this patch
> since we should fail fast.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This went on cleanly even without patch 8 so:

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

Another good set Brian, looking forward to the next one!

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 55 +++++++++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 033468d..c00278d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -771,22 +771,24 @@ int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int device_status = chip->tsl2x7x_chip_status;
> +	int ret;
>  
>  	mutex_lock(&chip->als_mutex);
>  	mutex_lock(&chip->prox_mutex);
>  
> -	if (device_status == TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> -
> -	tsl2x7x_chip_on(indio_dev);
> +	if (device_status == TSL2X7X_CHIP_WORKING) {
> +		ret = tsl2x7x_chip_off(indio_dev);
> +		if (ret < 0)
> +			goto unlock;
> +	}
>  
> -	if (device_status != TSL2X7X_CHIP_WORKING)
> -		tsl2x7x_chip_off(indio_dev);
> +	ret = tsl2x7x_chip_on(indio_dev);
>  
> +unlock:
>  	mutex_unlock(&chip->prox_mutex);
>  	mutex_unlock(&chip->als_mutex);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static
> @@ -925,6 +927,7 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	unsigned long value;
> +	int ret;
>  
>  	if (kstrtoul(buf, 0, &value))
>  		return -EINVAL;
> @@ -932,7 +935,9 @@ static ssize_t in_illuminance0_target_input_store(struct device *dev,
>  	if (value)
>  		chip->tsl2x7x_settings.als_cal_target = value;
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -981,7 +986,9 @@ static ssize_t in_intensity0_thresh_period_store(struct device *dev,
>  	dev_info(&chip->client->dev, "%s: als persistence = %d",
>  		 __func__, filter_delay);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return IIO_VAL_INT_PLUS_MICRO;
>  }
> @@ -1029,7 +1036,10 @@ static ssize_t in_proximity0_thresh_period_store(struct device *dev,
>  	dev_info(&chip->client->dev, "%s: prox persistence = %d",
>  		 __func__, filter_delay);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
>  
>  	return IIO_VAL_INT_PLUS_MICRO;
>  }
> @@ -1040,6 +1050,7 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	bool value;
> +	int ret;
>  
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
> @@ -1047,7 +1058,9 @@ static ssize_t in_illuminance0_calibrate_store(struct device *dev,
>  	if (value)
>  		tsl2x7x_als_calibrate(indio_dev);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1087,7 +1100,7 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>  	int value[ARRAY_SIZE(chip->tsl2x7x_device_lux) * 3 + 1];
> -	int n;
> +	int n, ret;
>  
>  	get_options(buf, ARRAY_SIZE(value), value);
>  
> @@ -1115,7 +1128,9 @@ static ssize_t in_illuminance0_lux_table_store(struct device *dev,
>  	memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
>  	memcpy(chip->tsl2x7x_device_lux, &value[1], (value[0] * 4));
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1126,6 +1141,7 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	bool value;
> +	int ret;
>  
>  	if (strtobool(buf, &value))
>  		return -EINVAL;
> @@ -1133,7 +1149,9 @@ static ssize_t in_proximity0_calibrate_store(struct device *dev,
>  	if (value)
>  		tsl2x7x_prox_cal(indio_dev);
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return len;
>  }
> @@ -1161,6 +1179,7 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  					  int val)
>  {
>  	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> +	int ret;
>  
>  	if (chan->type == IIO_INTENSITY) {
>  		if (val)
> @@ -1174,7 +1193,9 @@ static int tsl2x7x_write_interrupt_config(struct iio_dev *indio_dev,
>  			chip->tsl2x7x_settings.interrupts_en &= 0x10;
>  	}
>  
> -	tsl2x7x_invoke_change(indio_dev);
> +	ret = tsl2x7x_invoke_change(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	return 0;
>  }
> @@ -1422,9 +1443,7 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  		return -EINVAL;
>  	}
>  
> -	tsl2x7x_invoke_change(indio_dev);
> -
> -	return 0;
> +	return tsl2x7x_invoke_change(indio_dev);
>  }
>  
>  static DEVICE_ATTR_RO(in_proximity0_calibscale_available);