[PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

[PATCH 0/6] 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.

Brian Masney (6):
  staging: iio: tsl2x7x: add of_match table for device tree support
  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()

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

-- 
2.9.4

[PATCH 1/6] 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.

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 ++++++++++
 drivers/staging/iio/light/tsl2x7x.c                   | 16 ++++++++++++++++
 2 files changed, 26 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
diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 1467199..3f4f1f3 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 1/6] staging: iio: tsl2x7x: add of_match table for device tree support

[Rob Herring]

On Thu, Jun 29, 2017 at 01:03:47PM -0400, Brian Masney wrote:
> Add device tree support for the tsl2x7x IIO driver with no custom
> properties.
> 
> 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 ++++++++++

It's better if you split this so I can take it and avoid any conflicts.

>  drivers/staging/iio/light/tsl2x7x.c                   | 16 ++++++++++++++++
>  2 files changed, 26 insertions(+)

[PATCH 2/6] 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 3f4f1f3..3ea29e2 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

[PATCH 3/6] 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 occurances 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 3ea29e2..d917b9d 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 3/6] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()

[Frans Klaver]

On Thu, Jun 29, 2017 at 7:03 PM, 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 occurances with a call to
> i2c_smbus_read_byte_data().

s,occurances,occurrences,

Frans

[PATCH 4/6] 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 | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index d917b9d..620f40c 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -542,39 +542,30 @@ 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)) {
+	if ((ret & (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) {
-		dev_err(&chip->client->dev,
-			"failed to write ctrl reg: ret=%d\n", ret);
-		return ret;
+			"%s: Device is not powered on and/or ADC is not enabled\n",
+			__func__);
+		return -EINVAL;
 	}
-
-	reg_val = i2c_smbus_read_byte(chip->client);
-	if ((reg_val & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
+	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

[PATCH 5/6] 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.

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 620f40c..6a5d58c 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1219,78 +1219,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,
@@ -1615,8 +1638,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,
 	},
@@ -1626,8 +1649,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,
 	},
@@ -1637,8 +1660,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,
 	},
@@ -1648,8 +1671,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,
 	},
@@ -1659,8 +1682,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 5/6] staging: iio: tsl2x7x: refactor {read,write}_event_value to allow handling multiple iio_event_infos

[Jonathan Cameron]

On Thu, 29 Jun 2017 13:03:51 -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.
> 
Hmm.. Why make this change?  Are there additional uses of this
function on the way?

If not I wouldn't necessarily worry about the naming or the
assumptions as the assumptions are enforced by the driver
anyway.  Nothing wrong with a bit of paranoid defence against
future  bugs, but it's not strictly necessary.

J
> 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 620f40c..6a5d58c 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1219,78 +1219,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,
> @@ -1615,8 +1638,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,
>  	},
> @@ -1626,8 +1649,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,
>  	},
> @@ -1637,8 +1660,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,
>  	},
> @@ -1648,8 +1671,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,
>  	},
> @@ -1659,8 +1682,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,
>  	},

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

[Brian Masney]

On Sat, Jul 01, 2017 at 10:40:20AM +0100, Jonathan Cameron wrote:
> On Thu, 29 Jun 2017 13:03:51 -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.
> > 
> Hmm.. Why make this change?  Are there additional uses of this
> function on the way?
> 
> If not I wouldn't necessarily worry about the naming or the
> assumptions as the assumptions are enforced by the driver
> anyway.  Nothing wrong with a bit of paranoid defence against
> future  bugs, but it's not strictly necessary.

I should have mentioned in the changelog that this change sets the
driver up for migrating the in_intensity0_thresh_period
and in_proximity0_thresh_period sysfs attributes to be created by
iio_event_spec. This is one of the patches that I held back until
I am able to properly test it.

Brian

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

[Jonathan Cameron]

On Sat, 1 Jul 2017 06:37:07 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Sat, Jul 01, 2017 at 10:40:20AM +0100, Jonathan Cameron wrote:
> > On Thu, 29 Jun 2017 13:03:51 -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.
> > >   
> > Hmm.. Why make this change?  Are there additional uses of this
> > function on the way?
> > 
> > If not I wouldn't necessarily worry about the naming or the
> > assumptions as the assumptions are enforced by the driver
> > anyway.  Nothing wrong with a bit of paranoid defence against
> > future  bugs, but it's not strictly necessary.  
> 
> I should have mentioned in the changelog that this change sets the
> driver up for migrating the in_intensity0_thresh_period
> and in_proximity0_thresh_period sysfs attributes to be created by
> iio_event_spec. This is one of the patches that I held back until
> I am able to properly test it.
Cool.  That's a good reason ;)
> 
> Brian
> --
> 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

[PATCH 6/6] 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 6a5d58c..8195faa 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -687,7 +687,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
@@ -854,7 +855,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 0/6] staging: iio: tsl2x7x: staging cleanups

[Brian Masney]

On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> 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.
> 
> Brian Masney (6):
>   staging: iio: tsl2x7x: add of_match table for device tree support
>   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()
> 
>  .../devicetree/bindings/trivial-devices.txt        |  10 +
>  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
>  2 files changed, 153 insertions(+), 173 deletions(-)

Hi Jonathon,

Hold off on applying this series. There are several other formatting
warnings from checkpatch with this series that I need to fix. Sorry
about the noise. I'd appreciate it if you could at least look at the
functionality of my changes and I'll resubmit next week.

I held back several other changes related to the event subsystem until
I'm able to properly test my changes. I'm having trouble getting the
interrupts to work. I wired the interrupt pin on the sensor to GPIO
pin 17 on my Raspberry Pi 2 and added the following section to
arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:

&i2c1 {
        tsl2771@29 {
                compatible = "amstaos,tsl2771";
                reg = <0x39>;
                interrupt-parent = <&gpio>;
                interrupts = <17 2>;
        };
};

I start up iio_event_monitor, and run these commands:

echo 1 > events/in_intensity0_thresh_rising_en
echo 256 > events/in_intensity0_thresh_rising_value

When I shine a light on the ALS sensor, and the reading goes above
256, I do not get any events back from iio_event_monitor.

I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
I looked at how interrupts were setup in device tree overlays in the
official Raspberry Pi kernel and tried several variations from there.
I have more reading to do. :)

Thanks,

Brian

RE: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

[Jon Brenner]

Hi Brian,
Not sure about the DTS interrupt setting you mentioned but would like to know how you resolve - as I am planning on getting a Pi0w for some 'home personal projects'.

Anyway (and I am sure you probably already know this - but just in case) WRT the device registers (on the TSL2771 for example);
If you enable interrupts (ie. Reg#0x00= 0x37) and set the persistence register to 0x00 (is. Reg# 0x0C = 0x00), you should see interrupts occurring every integration period.
That way you can see if you're at least getting them.

You probably know this too - on earlier parts such as these, interrupts were cleared via use of the "command register" special function bits.  (see data sheets for details).
Now a day - interrupts are cleared in a more conventional manner 

Jon

-----Original Message-----
From: Brian Masney [mailto:masneyb@onstation.org] 
Sent: Friday, June 30, 2017 10:21 AM
To: jic23@kernel.org; linux-iio@vger.kernel.org
Cc: devel@driverdev.osuosl.org; lars@metafoo.de; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; Jon Brenner <Jon.Brenner@ams.com>; pmeerw@pmeerw.net; knaack.h@gmx.de
Subject: Re: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> 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.
> 
> Brian Masney (6):
>   staging: iio: tsl2x7x: add of_match table for device tree support
>   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()
> 
>  .../devicetree/bindings/trivial-devices.txt        |  10 +
>  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
>  2 files changed, 153 insertions(+), 173 deletions(-)

Hi Jonathon,

Hold off on applying this series. There are several other formatting warnings from checkpatch with this series that I need to fix. Sorry about the noise. I'd appreciate it if you could at least look at the functionality of my changes and I'll resubmit next week.

I held back several other changes related to the event subsystem until I'm able to properly test my changes. I'm having trouble getting the interrupts to work. I wired the interrupt pin on the sensor to GPIO pin 17 on my Raspberry Pi 2 and added the following section to arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:

&i2c1 {
        tsl2771@29 {
                compatible = "amstaos,tsl2771";
                reg = <0x39>;
                interrupt-parent = <&gpio>;
                interrupts = <17 2>;
        };
};

I start up iio_event_monitor, and run these commands:

echo 1 > events/in_intensity0_thresh_rising_en
echo 256 > events/in_intensity0_thresh_rising_value

When I shine a light on the ALS sensor, and the reading goes above 256, I do not get any events back from iio_event_monitor.

I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
I looked at how interrupts were setup in device tree overlays in the official Raspberry Pi kernel and tried several variations from there.
I have more reading to do. :)

Thanks,

Brian

Re: [PATCH 0/6] staging: iio: tsl2x7x: staging cleanups

[Jonathan Cameron]

On Fri, 30 Jun 2017 11:21:14 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Thu, Jun 29, 2017 at 01:03:46PM -0400, Brian Masney wrote:
> > 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.
> > 
> > Brian Masney (6):
> >   staging: iio: tsl2x7x: add of_match table for device tree support
> >   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()
> > 
> >  .../devicetree/bindings/trivial-devices.txt        |  10 +
> >  drivers/staging/iio/light/tsl2x7x.c                | 316 ++++++++++-----------
> >  2 files changed, 153 insertions(+), 173 deletions(-)  
> 
> Hi Jonathon,
> 
> Hold off on applying this series.
Will do.
> There are several other formatting
> warnings from checkpatch with this series that I need to fix. Sorry
> about the noise. I'd appreciate it if you could at least look at the
> functionality of my changes and I'll resubmit next week.
All fine except possibly needing a stronger justification for patch 5.
I'm not against the patch - and it may be fine on the basis that
it lines up better with the standard form in other drivers, but
right now there isn't anything actually 'wrong' with the existing code.

J
> 
> I held back several other changes related to the event subsystem until
> I'm able to properly test my changes. I'm having trouble getting the
> interrupts to work. I wired the interrupt pin on the sensor to GPIO
> pin 17 on my Raspberry Pi 2 and added the following section to
> arch/arm/boot/dts/bcm2836-rpi-2-b.dts for my sensor:
> 
> &i2c1 {
>         tsl2771@29 {
>                 compatible = "amstaos,tsl2771";
>                 reg = <0x39>;
>                 interrupt-parent = <&gpio>;
>                 interrupts = <17 2>;
>         };
> };
> 
> I start up iio_event_monitor, and run these commands:
> 
> echo 1 > events/in_intensity0_thresh_rising_en
> echo 256 > events/in_intensity0_thresh_rising_value
> 
> When I shine a light on the ALS sensor, and the reading goes above
> 256, I do not get any events back from iio_event_monitor.
> 
> I'm honestly not sure about the 2 in the 'interrupts = <17 2>;' line.
> I looked at how interrupts were setup in device tree overlays in the
> official Raspberry Pi kernel and tried several variations from there.
> I have more reading to do. :)
> 
> Thanks,
> 
> Brian