[PATCH 00/10] staging: iio: tsl2583: staging cleanups

[PATCH 00/10] staging: iio: tsl2583: staging cleanups

[Brian Masney]

This patch set begins cleaning up some of the major items that is
keeping the tsl2583 driver out of mainline. Highlights include device
tree support, converts the driver over to use the iio_chan_spec,
improved error handling, and fixes for some concurrency issues. There
is more work required to get this driver out of staging that I will
send later as a separate patch set.

Driver was tested using a TSL2581 hooked up to a Raspberry Pi 2.

The sysfs attributes were previously prefixed with illuminance0_ however
they are now prefixed with in_illuminance_. None of the IIO light
drivers in mainline have their sysfs attributes prefixed with
illuminance0_, however 8 of the IIO light drivers in mainline use the
in_illuminance_ prefix so I assume that this is the naming convention
that should be used for this driver as well.

sysfs attribute names before this patch set:

raspberrypi:/sys/bus/iio/devices/iio:device0$ ls -l
total 0
-r--r--r-- 1 root root 4096 Oct 27 20:27 dev
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibbias
--w------- 1 root root 4096 Oct 27 20:27 illuminance0_calibrate
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibscale
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_calibscale_available
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_input
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_input_target
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_integration_time
-r--r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_integration_time_available
-rw-r--r-- 1 root root 4096 Oct 27 20:27 illuminance0_lux_table
-r--r--r-- 1 root root 4096 Oct 27 20:27 name
lrwxrwxrwx 1 root root    0 Oct 27 20:27 of_node ->
 ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/tsl2581@29/
drwxr-xr-x 2 root root    0 Oct 27 20:27 power/
-rw-r--r-- 1 root root 4096 Oct 27 20:27 power_state
lrwxrwxrwx 1 root root    0 Oct 27 20:27 subsystem -> ../../../../../../../bus/iio/
-rw-r--r-- 1 root root 4096 Oct 27 20:27 uevent

sysfs attribute names after this patch set:

raspberrypi:/sys/bus/iio/devices/iio:device0$ ls -l
total 0
-r--r--r-- 1 root root 4096 Oct 27 22:29 dev
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_both_raw
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibbias
--w------- 1 root root 4096 Oct 27 22:29 in_illuminance_calibrate
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibscale
-r--r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_calibscale_available
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_input
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_input_target
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_integration_time
-r--r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_integration_time_available
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_ir_raw
-rw-r--r-- 1 root root 4096 Oct 27 22:29 in_illuminance_lux_table
-r--r--r-- 1 root root 4096 Oct 27 22:29 name
lrwxrwxrwx 1 root root    0 Oct 27 22:29 of_node ->
 ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/tsl2581@29/
drwxr-xr-x 2 root root    0 Oct 27 22:29 power/
lrwxrwxrwx 1 root root    0 Oct 27 22:29 subsystem ->
 ../../../../../../../bus/iio/
-rw-r--r-- 1 root root 4096 Oct 27 22:23 uevent

Brian Masney (10):
  staging: iio: tsl2583: add of_match table for device tree support
  staging: iio: tsl2583: check for error code from i2c_smbus_read_byte()
  staging: iio: tsl2583: return proper error code instead of -1
  staging: iio: tsl2583: remove redundant power_state sysfs attribute
  staging: iio: tsl2583: check return values from taos_chip_{on,off}
  staging: iio: tsl2583: convert to use iio_chan_spec and
    {read,write}_raw
  staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to
    use iio_chan_spec
  staging: iio: tsl2583: use IIO_*_ATTR* macros to create sysfs entries
  staging: iio: tsl2583: add error code to sysfs store functions
  staging: iio: tsl2583: add locking to sysfs attributes

 .../devicetree/bindings/iio/light/tsl2583.txt      |  26 ++
 drivers/staging/iio/light/tsl2583.c                | 509 +++++++++++----------
 2 files changed, 288 insertions(+), 247 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

-- 
2.7.4

[PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

[Brian Masney]

Add device tree support for the tsl2583 IIO driver with no custom
properties.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../devicetree/bindings/iio/light/tsl2583.txt      | 26 ++++++++++++++++++++++
 drivers/staging/iio/light/tsl2583.c                | 13 +++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

diff --git a/Documentation/devicetree/bindings/iio/light/tsl2583.txt b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
new file mode 100644
index 0000000..8e2066c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
@@ -0,0 +1,26 @@
+* TAOS TSL 2580/2581/2583 ALS sensor
+
+Required properties:
+
+  - compatible: Should be one of
+		"amstaos,tsl2580"
+		"amstaos,tsl2581"
+		"amstaos,tsl2583"
+  - reg: the I2C address of the device
+
+Optional properties:
+
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: the sole interrupt generated by the device
+
+  Refer to interrupt-controller/interrupts.txt for generic interrupt client
+  node bindings.
+
+  - vcc-supply: phandle to the regulator that provides power to the sensor.
+
+Example:
+
+tsl2581@29 {
+	compatible = "amstaos,tsl2581";
+	reg = <0x29>;
+};
diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 08f1583..fd4b6ef 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -947,11 +947,24 @@ static struct i2c_device_id taos_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, taos_idtable);
 
+#ifdef CONFIG_OF
+static const struct of_device_id taos2583_of_match[] = {
+	{ .compatible = "amstaos,tsl2580", },
+	{ .compatible = "amstaos,tsl2581", },
+	{ .compatible = "amstaos,tsl2583", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, taos2583_of_match);
+#else
+#define taos2583_of_match NULL
+#endif
+
 /* Driver definition */
 static struct i2c_driver taos_driver = {
 	.driver = {
 		.name = "tsl2583",
 		.pm = TAOS_PM_OPS,
+		.of_match_table = taos2583_of_match,
 	},
 	.id_table = taos_idtable,
 	.probe = taos_probe,
-- 
2.7.4

[PATCH 02/10] staging: iio: tsl2583: check for error code from i2c_smbus_read_byte()

[Brian Masney]

taos_i2c_read() and taos_als_calibrate() does not check to see if the
value returned by i2c_smbus_read_byte() was an error code. This patch
adds the appropriate error handling.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index fd4b6ef..35c1696 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -171,7 +171,14 @@ taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len)
 			return ret;
 		}
 		/* read the data */
-		*val = i2c_smbus_read_byte(client);
+		ret = i2c_smbus_read_byte(client);
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"%s failed to read byte after writing to register %x\n",
+				__func__, reg);
+			return ret;
+		}
+		*val = ret;
 		val++;
 		reg++;
 	}
@@ -355,6 +362,13 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 	}
 
 	reg_val = i2c_smbus_read_byte(chip->client);
+	if (reg_val < 0) {
+		dev_err(&chip->client->dev,
+			"%s failed to read after writing to the CNTRL register\n",
+			__func__);
+		return ret;
+	}
+
 	if ((reg_val & (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON))
 			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
 		dev_err(&chip->client->dev,
@@ -371,6 +385,12 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 		return ret;
 	}
 	reg_val = i2c_smbus_read_byte(chip->client);
+	if (reg_val < 0) {
+		dev_err(&chip->client->dev,
+			"%s failed to read after writing to the STATUS register\n",
+			__func__);
+		return ret;
+	}
 
 	if ((reg_val & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) {
 		dev_err(&chip->client->dev,
-- 
2.7.4

[PATCH 03/10] staging: iio: tsl2583: return proper error code instead of -1

[Brian Masney]

taos_als_calibrate() has a code path where -1 is returned. This patch
changes the code so that a proper error code is returned.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 35c1696..47fd373 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -373,7 +373,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
 			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
 		dev_err(&chip->client->dev,
 			"taos_als_calibrate failed: device not powered on with ADC enabled\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	ret = i2c_smbus_write_byte(chip->client,
-- 
2.7.4

[PATCH 04/10] staging: iio: tsl2583: remove redundant power_state sysfs attribute

[Brian Masney]

IIO devices have a /sys/bus/iio/devices/iio:deviceX/power/ directory
that allows viewing and controling various power parameters. The tsl2583
driver also has an additional custom sysfs attribute named power_state
that is not needed. This patch removes the redundant power_state sysfs
attribute.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 47fd373..f8ccb4d 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -511,33 +511,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_power_state_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%d\n", chip->taos_chip_status);
-}
-
-static ssize_t taos_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);
-	int value;
-
-	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
-
-	if (!value)
-		taos_chip_off(indio_dev);
-	else
-		taos_chip_on(indio_dev);
-
-	return len;
-}
-
 static ssize_t taos_gain_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
@@ -785,9 +758,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	return len;
 }
 
-static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
-		taos_power_state_show, taos_power_state_store);
-
 static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
 		taos_gain_show, taos_gain_store);
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
@@ -810,7 +780,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
 		taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
-	&dev_attr_power_state.attr,
 	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
 	&dev_attr_illuminance0_calibscale_available.attr,
 	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
-- 
2.7.4

[PATCH 05/10] staging: iio: tsl2583: check return values from taos_chip_{on,off}

[Brian Masney]

The return value from taos_chip_on() and taos_chip_off() was not
checked in taos_luxtable_store() and taos_probe(). This patch adds
proper error checking to these function calls.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index f8ccb4d..e975bba 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -727,7 +727,7 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
-	int n;
+	int n, ret = -EINVAL;
 
 	get_options(buf, ARRAY_SIZE(value), value);
 
@@ -739,23 +739,31 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	n = value[0];
 	if ((n % 3) || n < 6 || n > ((ARRAY_SIZE(taos_device_lux) - 1) * 3)) {
 		dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
-		return -EINVAL;
+		goto done;
 	}
 	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
 		dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
-		return -EINVAL;
+		goto done;
 	}
 
-	if (chip->taos_chip_status == TSL258X_CHIP_WORKING)
-		taos_chip_off(indio_dev);
+	if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
+		ret = taos_chip_off(indio_dev);
+		if (ret < 0)
+			goto done;
+	}
 
 	/* Zero out the table */
 	memset(taos_device_lux, 0, sizeof(taos_device_lux));
 	memcpy(taos_device_lux, &value[1], (value[0] * 4));
 
-	taos_chip_on(indio_dev);
+	ret = taos_chip_on(indio_dev);
+	if (ret < 0)
+		goto done;
 
-	return len;
+	ret = len;
+
+done:
+	return ret;
 }
 
 static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
@@ -883,7 +891,9 @@ static int taos_probe(struct i2c_client *clientp,
 	taos_defaults(chip);
 
 	/* Make sure the chip is on */
-	taos_chip_on(indio_dev);
+	ret = taos_chip_on(indio_dev);
+	if (ret < 0)
+		return ret;
 
 	dev_info(&clientp->dev, "Light sensor found.\n");
 	return 0;
-- 
2.7.4

[PATCH 06/10] staging: iio: tsl2583: convert to use iio_chan_spec and {read,write}_raw

[Brian Masney]

The tsl2583 driver directly creates sysfs attributes that should instead
be created by the IIO core on behalf of the driver. This patch adds the
iio_chan_spec array, the relevant info_mask elements and the read_raw()
and write_raw() functions to take advantage of features provided by the
IIO core. These sysfs attributes were migrated with this patch:
illuminance0_input, illuminance0_calibbias,
illuminance0_integration_time. This also exposes the raw values read
from the two channels on the sensor.

With this change, these four sysfs entries have their prefix changed
from illuminance0_ to in_illuminance_. This is deemed to be acceptable
since none of the IIO light drivers in mainline use the illuminance0_
prefix, however 8 of the IIO light drivers in mainline use the
in_illuminance_ prefix.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index e975bba..6a61a86 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -211,28 +211,23 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 	u32 ch0lux = 0;
 	u32 ch1lux = 0;
 
-	if (mutex_trylock(&chip->als_mutex) == 0) {
-		dev_info(&chip->client->dev, "taos_get_lux device is busy\n");
-		return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
-	}
-
 	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
 		/* device is not enabled */
 		dev_err(&chip->client->dev, "taos_get_lux device is not enabled\n");
 		ret = -EBUSY;
-		goto out_unlock;
+		goto done;
 	}
 
 	ret = taos_i2c_read(chip->client, (TSL258X_CMD_REG), &buf[0], 1);
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "taos_get_lux failed to read CMD_REG\n");
-		goto out_unlock;
+		goto done;
 	}
 	/* is data new & valid */
 	if (!(buf[0] & TSL258X_STA_ADC_INTR)) {
 		dev_err(&chip->client->dev, "taos_get_lux data not valid\n");
 		ret = chip->als_cur_info.lux; /* return LAST VALUE */
-		goto out_unlock;
+		goto done;
 	}
 
 	for (i = 0; i < 4; i++) {
@@ -243,7 +238,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 			dev_err(&chip->client->dev,
 				"taos_get_lux failed to read register %x\n",
 				reg);
-			goto out_unlock;
+			goto done;
 		}
 	}
 
@@ -259,7 +254,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 		dev_err(&chip->client->dev,
 			"taos_i2c_write_command failed in taos_get_lux, err = %d\n",
 			ret);
-		goto out_unlock; /* have no data, so return failure */
+		goto done; /* have no data, so return failure */
 	}
 
 	/* extract ALS/lux data */
@@ -276,7 +271,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 		/* have no data, so return LAST VALUE */
 		ret = 0;
 		chip->als_cur_info.lux = 0;
-		goto out_unlock;
+		goto done;
 	}
 	/* calculate ratio */
 	ratio = (ch1 << 15) / ch0;
@@ -302,7 +297,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 		dev_dbg(&chip->client->dev, "No Data - Return last value\n");
 		ret = 0;
 		chip->als_cur_info.lux = 0;
-		goto out_unlock;
+		goto done;
 	}
 
 	/* adjust for active time scale */
@@ -334,8 +329,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
 	chip->als_cur_info.lux = lux;
 	ret = lux;
 
-out_unlock:
-	mutex_unlock(&chip->als_mutex);
+done:
 	return ret;
 }
 
@@ -575,69 +569,12 @@ static ssize_t taos_gain_available_show(struct device *dev,
 	return sprintf(buf, "%s\n", "1 8 16 111");
 }
 
-static ssize_t taos_als_time_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%d\n", chip->taos_settings.als_time);
-}
-
-static ssize_t taos_als_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 tsl2583_chip *chip = iio_priv(indio_dev);
-	int value;
-
-	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
-
-	if ((value < 50) || (value > 650))
-		return -EINVAL;
-
-	if (value % 50)
-		return -EINVAL;
-
-	chip->taos_settings.als_time = value;
-
-	return len;
-}
-
 static ssize_t taos_als_time_available_show(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
 {
 	return sprintf(buf, "%s\n",
-		"50 100 150 200 250 300 350 400 450 500 550 600 650");
-}
-
-static ssize_t taos_als_trim_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-
-	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
-}
-
-static ssize_t taos_als_trim_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 tsl2583_chip *chip = iio_priv(indio_dev);
-	int value;
-
-	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
-
-	if (value)
-		chip->taos_settings.als_gain_trim = value;
-
-	return len;
+		"0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
 }
 
 static ssize_t taos_als_cal_target_show(struct device *dev,
@@ -667,18 +604,6 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
-			     char *buf)
-{
-	int ret;
-
-	ret = taos_get_lux(dev_to_iio_dev(dev));
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", ret);
-}
-
 static ssize_t taos_do_calibrate(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t len)
@@ -771,18 +696,12 @@ static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
 		taos_gain_available_show, NULL);
 
-static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
-		taos_als_time_show, taos_als_time_store);
 static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
 		taos_als_time_available_show, NULL);
 
-static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
-		taos_als_trim_show, taos_als_trim_store);
-
 static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
 		taos_als_cal_target_show, taos_als_cal_target_store);
 
-static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
 static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
 static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
 		taos_luxtable_show, taos_luxtable_store);
@@ -790,11 +709,8 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
 static struct attribute *sysfs_attrs_ctrl[] = {
 	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
 	&dev_attr_illuminance0_calibscale_available.attr,
-	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
 	&dev_attr_illuminance0_integration_time_available.attr,
-	&dev_attr_illuminance0_calibbias.attr,			/* trim  */
 	&dev_attr_illuminance0_input_target.attr,
-	&dev_attr_illuminance0_input.attr,
 	&dev_attr_illuminance0_calibrate.attr,
 	&dev_attr_illuminance0_lux_table.attr,
 	NULL
@@ -810,9 +726,141 @@ static int taos_tsl258x_device(unsigned char *bufp)
 	return ((bufp[TSL258X_CHIPID] & 0xf0) == 0x90);
 }
 
+static const struct iio_chan_spec tsl2583_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_IR,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+	{
+		.type = IIO_LIGHT,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
+				      BIT(IIO_CHAN_INFO_INT_TIME),
+	},
+};
+
+static int tsl2583_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct tsl2583_chip *chip = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	mutex_lock(&chip->als_mutex);
+
+	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
+		ret = -EBUSY;
+		goto read_done;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (chan->type == IIO_LIGHT) {
+			ret = taos_get_lux(indio_dev);
+			if (ret < 0)
+				goto read_done;
+
+			/*
+			 * From page 20 of the TSL2581, TSL2583 data
+			 * sheet (TAOS134 − MARCH 2011):
+			 *
+			 * One of the photodiodes (channel 0) is
+			 * sensitive to both visible and infrared light,
+			 * while the second photodiode (channel 1) is
+			 * sensitive primarily to infrared light.
+			 */
+			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
+				*val = chip->als_cur_info.als_ch0;
+			else
+				*val = chip->als_cur_info.als_ch1;
+
+			ret = IIO_VAL_INT;
+		}
+		break;
+	case IIO_CHAN_INFO_PROCESSED:
+		if (chan->type == IIO_LIGHT) {
+			ret = taos_get_lux(indio_dev);
+			if (ret < 0)
+				goto read_done;
+
+			*val = ret;
+			ret = IIO_VAL_INT;
+		}
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (chan->type == IIO_LIGHT) {
+			*val = chip->taos_settings.als_gain_trim;
+			ret = IIO_VAL_INT;
+		}
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT) {
+			*val = 0;
+			*val2 = chip->taos_settings.als_time;
+			ret = IIO_VAL_INT_PLUS_MICRO;
+		}
+		break;
+	default:
+		break;
+	}
+
+read_done:
+	mutex_unlock(&chip->als_mutex);
+
+	return ret;
+}
+
+static int tsl2583_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct tsl2583_chip *chip = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	mutex_lock(&chip->als_mutex);
+
+	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
+		ret = -EBUSY;
+		goto write_done;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (chan->type == IIO_LIGHT) {
+			chip->taos_settings.als_gain_trim = val;
+			ret = 0;
+		}
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
+		    val2 <= 650 && !(val2 % 50)) {
+			chip->taos_settings.als_time = val2;
+			ret = 0;
+		}
+		break;
+	default:
+		break;
+	}
+
+write_done:
+	mutex_unlock(&chip->als_mutex);
+
+	return ret;
+}
+
 static const struct iio_info tsl2583_info = {
 	.attrs = &tsl2583_attribute_group,
 	.driver_module = THIS_MODULE,
+	.read_raw = tsl2583_read_raw,
+	.write_raw = tsl2583_write_raw,
 };
 
 /*
@@ -878,6 +926,8 @@ static int taos_probe(struct i2c_client *clientp,
 	}
 
 	indio_dev->info = &tsl2583_info;
+	indio_dev->channels = tsl2583_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels);
 	indio_dev->dev.parent = &clientp->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->name = chip->client->name;
-- 
2.7.4

[PATCH 07/10] staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to use iio_chan_spec

[Brian Masney]

The illuminance0_calibscale sysfs attribute is not currently created by
the IIO core. This patch adds the appropriate mask to iio_chan_spec,
along with the appropriate data handling in the read_raw() and
write_raw() functions, so that the sysfs attribute is created by the IIO
core. With this change, this sysfs entry will have its prefix changed
from illuminance0_ to in_illuminance_.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 6a61a86..bfff6ca 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -124,14 +124,15 @@ static struct taos_lux taos_device_lux[11] = {
 struct gainadj {
 	s16 ch0;
 	s16 ch1;
+	s16 mean;
 };
 
 /* Index = (0 - 3) Used to validate the gain selection index */
 static const struct gainadj gainadj[] = {
-	{ 1, 1 },
-	{ 8, 8 },
-	{ 16, 16 },
-	{ 107, 115 }
+	{ 1, 1, 1 },
+	{ 8, 8, 8 },
+	{ 16, 16, 16 },
+	{ 107, 115, 111 }
 };
 
 /*
@@ -505,63 +506,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_gain_show(struct device *dev,
-			      struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	char gain[4] = {0};
-
-	switch (chip->taos_settings.als_gain) {
-	case 0:
-		strcpy(gain, "001");
-		break;
-	case 1:
-		strcpy(gain, "008");
-		break;
-	case 2:
-		strcpy(gain, "016");
-		break;
-	case 3:
-		strcpy(gain, "111");
-		break;
-	}
-
-	return sprintf(buf, "%s\n", gain);
-}
-
-static ssize_t taos_gain_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 tsl2583_chip *chip = iio_priv(indio_dev);
-	int value;
-
-	if (kstrtoint(buf, 0, &value))
-		return -EINVAL;
-
-	switch (value) {
-	case 1:
-		chip->taos_settings.als_gain = 0;
-		break;
-	case 8:
-		chip->taos_settings.als_gain = 1;
-		break;
-	case 16:
-		chip->taos_settings.als_gain = 2;
-		break;
-	case 111:
-		chip->taos_settings.als_gain = 3;
-		break;
-	default:
-		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
-		return -1;
-	}
-
-	return len;
-}
-
 static ssize_t taos_gain_available_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -691,8 +635,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	return ret;
 }
 
-static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
-		taos_gain_show, taos_gain_store);
 static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
 		taos_gain_available_show, NULL);
 
@@ -707,7 +649,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
 		taos_luxtable_show, taos_luxtable_store);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
-	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
 	&dev_attr_illuminance0_calibscale_available.attr,
 	&dev_attr_illuminance0_integration_time_available.attr,
 	&dev_attr_illuminance0_input_target.attr,
@@ -743,6 +684,7 @@ static const struct iio_chan_spec tsl2583_channels[] = {
 		.type = IIO_LIGHT,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
 				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
+				      BIT(IIO_CHAN_INFO_CALIBSCALE) |
 				      BIT(IIO_CHAN_INFO_INT_TIME),
 	},
 };
@@ -801,6 +743,12 @@ static int tsl2583_read_raw(struct iio_dev *indio_dev,
 			ret = IIO_VAL_INT;
 		}
 		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (chan->type == IIO_LIGHT) {
+			*val = gainadj[chip->taos_settings.als_gain].mean;
+			ret = IIO_VAL_INT;
+		}
+		break;
 	case IIO_CHAN_INFO_INT_TIME:
 		if (chan->type == IIO_LIGHT) {
 			*val = 0;
@@ -839,6 +787,19 @@ static int tsl2583_write_raw(struct iio_dev *indio_dev,
 			ret = 0;
 		}
 		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (chan->type == IIO_LIGHT) {
+			int i;
+
+			for (i = 0; i < ARRAY_SIZE(gainadj); i++) {
+				if (gainadj[i].mean == val) {
+					chip->taos_settings.als_gain = i;
+					ret = 0;
+					break;
+				}
+			}
+		}
+		break;
 	case IIO_CHAN_INFO_INT_TIME:
 		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
 		    val2 <= 650 && !(val2 % 50)) {
-- 
2.7.4

[PATCH 08/10] staging: iio: tsl2583: use IIO_*_ATTR* macros to create sysfs entries

[Brian Masney]

Use the IIO_CONST_ATTR, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
macros for creating the in_illuminance_calibscale_available,
in_illuminance_integration_time_available, in_illuminance_input_target,
in_illuminance_calibrate, and in_illuminance_lux_table sysfs entries.
Previously these sysfs entries were prefixed with illuminance0_, however
they are now prefixed with in_illuminance_ to make these sysfs entries
consistent with how the IIO core is creating the other sysfs entries.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index bfff6ca..1462374 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
 
 #define TSL258X_MAX_DEVICE_REGS		32
 
@@ -506,24 +507,9 @@ static int taos_chip_off(struct iio_dev *indio_dev)
 
 /* Sysfs Interface Functions */
 
-static ssize_t taos_gain_available_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return sprintf(buf, "%s\n", "1 8 16 111");
-}
-
-static ssize_t taos_als_time_available_show(struct device *dev,
-					    struct device_attribute *attr,
-					    char *buf)
-{
-	return sprintf(buf, "%s\n",
-		"0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
-}
-
-static ssize_t taos_als_cal_target_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
+static ssize_t in_illuminance_input_target_show(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
@@ -531,9 +517,9 @@ static ssize_t taos_als_cal_target_show(struct device *dev,
 	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
 }
 
-static ssize_t taos_als_cal_target_store(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t len)
+static ssize_t in_illuminance_input_target_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 tsl2583_chip *chip = iio_priv(indio_dev);
@@ -548,9 +534,9 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_do_calibrate(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf, size_t len)
+static ssize_t in_illuminance_calibrate_store(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	int value;
@@ -564,8 +550,9 @@ static ssize_t taos_do_calibrate(struct device *dev,
 	return len;
 }
 
-static ssize_t taos_luxtable_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t in_illuminance_lux_table_show(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
 {
 	int i;
 	int offset = 0;
@@ -589,9 +576,9 @@ static ssize_t taos_luxtable_show(struct device *dev,
 	return offset;
 }
 
-static ssize_t taos_luxtable_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t len)
+static ssize_t in_illuminance_lux_table_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 tsl2583_chip *chip = iio_priv(indio_dev);
@@ -635,25 +622,19 @@ static ssize_t taos_luxtable_store(struct device *dev,
 	return ret;
 }
 
-static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
-		taos_gain_available_show, NULL);
-
-static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
-		taos_als_time_available_show, NULL);
-
-static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
-		taos_als_cal_target_show, taos_als_cal_target_store);
-
-static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
-static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
-		taos_luxtable_show, taos_luxtable_store);
+static IIO_CONST_ATTR(in_illuminance_calibscale_available, "1 8 16 111");
+static IIO_CONST_ATTR(in_illuminance_integration_time_available,
+		      "0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
+static IIO_DEVICE_ATTR_RW(in_illuminance_input_target, 0);
+static IIO_DEVICE_ATTR_WO(in_illuminance_calibrate, 0);
+static IIO_DEVICE_ATTR_RW(in_illuminance_lux_table, 0);
 
 static struct attribute *sysfs_attrs_ctrl[] = {
-	&dev_attr_illuminance0_calibscale_available.attr,
-	&dev_attr_illuminance0_integration_time_available.attr,
-	&dev_attr_illuminance0_input_target.attr,
-	&dev_attr_illuminance0_calibrate.attr,
-	&dev_attr_illuminance0_lux_table.attr,
+	&iio_const_attr_in_illuminance_calibscale_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
+	&iio_dev_attr_in_illuminance_input_target.dev_attr.attr,
+	&iio_dev_attr_in_illuminance_calibrate.dev_attr.attr,
+	&iio_dev_attr_in_illuminance_lux_table.dev_attr.attr,
 	NULL
 };
 
-- 
2.7.4

[PATCH 09/10] staging: iio: tsl2583: add error code to sysfs store functions

[Brian Masney]

in_illuminance_input_target_store() and in_illuminance_calibrate_store()
validated the data from userspace, however it would not return an
error code to userspace if an invalid value was passed in. This patch
changes these functions so that they return -EINVAL if invalid data is
passed in.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 1462374..98afa5b 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -525,11 +525,10 @@ static ssize_t in_illuminance_input_target_store(struct device *dev,
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	int value;
 
-	if (kstrtoint(buf, 0, &value))
+	if (kstrtoint(buf, 0, &value) || !value)
 		return -EINVAL;
 
-	if (value)
-		chip->taos_settings.als_cal_target = value;
+	chip->taos_settings.als_cal_target = value;
 
 	return len;
 }
@@ -541,11 +540,10 @@ static ssize_t in_illuminance_calibrate_store(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	int value;
 
-	if (kstrtoint(buf, 0, &value))
+	if (kstrtoint(buf, 0, &value) || value != 1)
 		return -EINVAL;
 
-	if (value == 1)
-		taos_als_calibrate(indio_dev);
+	taos_als_calibrate(indio_dev);
 
 	return len;
 }
-- 
2.7.4

[PATCH 10/10] staging: iio: tsl2583: add locking to sysfs attributes

[Brian Masney]

in_illuminance_input_target_show(), in_illuminance_input_target_store(),
in_illuminance_calibrate_store(), and in_illuminance_lux_table_store()
accesses data from the tsl2583_chip struct. Some of these fields can be
modified by other parts of the driver concurrently. This patch adds the
mutex locking to these sysfs attributes.

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

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 98afa5b..49b19f5 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -513,8 +513,13 @@ static ssize_t in_illuminance_input_target_show(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&chip->als_mutex);
+	ret = sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
+	mutex_unlock(&chip->als_mutex);
 
-	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
+	return ret;
 }
 
 static ssize_t in_illuminance_input_target_store(struct device *dev,
@@ -528,7 +533,9 @@ static ssize_t in_illuminance_input_target_store(struct device *dev,
 	if (kstrtoint(buf, 0, &value) || !value)
 		return -EINVAL;
 
+	mutex_lock(&chip->als_mutex);
 	chip->taos_settings.als_cal_target = value;
+	mutex_unlock(&chip->als_mutex);
 
 	return len;
 }
@@ -538,12 +545,15 @@ static ssize_t in_illuminance_calibrate_store(struct device *dev,
 					      const char *buf, size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct tsl2583_chip *chip = iio_priv(indio_dev);
 	int value;
 
 	if (kstrtoint(buf, 0, &value) || value != 1)
 		return -EINVAL;
 
+	mutex_lock(&chip->als_mutex);
 	taos_als_calibrate(indio_dev);
+	mutex_unlock(&chip->als_mutex);
 
 	return len;
 }
@@ -583,6 +593,8 @@ static ssize_t in_illuminance_lux_table_store(struct device *dev,
 	int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
 	int n, ret = -EINVAL;
 
+	mutex_lock(&chip->als_mutex);
+
 	get_options(buf, ARRAY_SIZE(value), value);
 
 	/* We now have an array of ints starting at value[1], and
@@ -617,6 +629,8 @@ static ssize_t in_illuminance_lux_table_store(struct device *dev,
 	ret = len;
 
 done:
+	mutex_unlock(&chip->als_mutex);
+
 	return ret;
 }
 
-- 
2.7.4

Re: [PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> Add device tree support for the tsl2583 IIO driver with no custom
> properties.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Trivial enough that I feel I can take this without an explicit devicetree
ack.  Of course one is always welcome if anyone gets a chance to look at it.
I won't have it exposed in a non rebasing tree until perhaps the second half
of the week.
> ---
>  .../devicetree/bindings/iio/light/tsl2583.txt      | 26 ++++++++++++++++++++++
>  drivers/staging/iio/light/tsl2583.c                | 13 +++++++++++
>  2 files changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/tsl2583.txt b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
> new file mode 100644
> index 0000000..8e2066c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
> @@ -0,0 +1,26 @@
> +* TAOS TSL 2580/2581/2583 ALS sensor
> +
> +Required properties:
> +
> +  - compatible: Should be one of
> +		"amstaos,tsl2580"
> +		"amstaos,tsl2581"
> +		"amstaos,tsl2583"
> +  - reg: the I2C address of the device
> +
> +Optional properties:
> +
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts: the sole interrupt generated by the device
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +  - vcc-supply: phandle to the regulator that provides power to the sensor.
> +
> +Example:
> +
> +tsl2581@29 {
> +	compatible = "amstaos,tsl2581";
> +	reg = <0x29>;
> +};
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 08f1583..fd4b6ef 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -947,11 +947,24 @@ static struct i2c_device_id taos_idtable[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, taos_idtable);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id taos2583_of_match[] = {
> +	{ .compatible = "amstaos,tsl2580", },
> +	{ .compatible = "amstaos,tsl2581", },
> +	{ .compatible = "amstaos,tsl2583", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, taos2583_of_match);
> +#else
> +#define taos2583_of_match NULL
> +#endif
> +
>  /* Driver definition */
>  static struct i2c_driver taos_driver = {
>  	.driver = {
>  		.name = "tsl2583",
>  		.pm = TAOS_PM_OPS,
> +		.of_match_table = taos2583_of_match,
>  	},
>  	.id_table = taos_idtable,
>  	.probe = taos_probe,
> 

Re: [PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

[Jonathan Cameron]

On 30/10/16 17:43, Jonathan Cameron wrote:
> On 28/10/16 11:00, Brian Masney wrote:
>> Add device tree support for the tsl2583 IIO driver with no custom
>> properties.
>>
>> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Trivial enough that I feel I can take this without an explicit devicetree
> ack.  Of course one is always welcome if anyone gets a chance to look at it.
> I won't have it exposed in a non rebasing tree until perhaps the second half
> of the week.
oops. Forgot to say - applied to the togreg branch of iio.git - initially pushed
out as testing for the autobuilders to play with it.

Thanks,

Jonathan
>> ---
>>  .../devicetree/bindings/iio/light/tsl2583.txt      | 26 ++++++++++++++++++++++
>>  drivers/staging/iio/light/tsl2583.c                | 13 +++++++++++
>>  2 files changed, 39 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/tsl2583.txt b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
>> new file mode 100644
>> index 0000000..8e2066c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/light/tsl2583.txt
>> @@ -0,0 +1,26 @@
>> +* TAOS TSL 2580/2581/2583 ALS sensor
>> +
>> +Required properties:
>> +
>> +  - compatible: Should be one of
>> +		"amstaos,tsl2580"
>> +		"amstaos,tsl2581"
>> +		"amstaos,tsl2583"
>> +  - reg: the I2C address of the device
>> +
>> +Optional properties:
>> +
>> +  - interrupt-parent: should be the phandle for the interrupt controller
>> +  - interrupts: the sole interrupt generated by the device
>> +
>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
>> +  node bindings.
>> +
>> +  - vcc-supply: phandle to the regulator that provides power to the sensor.
>> +
>> +Example:
>> +
>> +tsl2581@29 {
>> +	compatible = "amstaos,tsl2581";
>> +	reg = <0x29>;
>> +};
>> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
>> index 08f1583..fd4b6ef 100644
>> --- a/drivers/staging/iio/light/tsl2583.c
>> +++ b/drivers/staging/iio/light/tsl2583.c
>> @@ -947,11 +947,24 @@ static struct i2c_device_id taos_idtable[] = {
>>  };
>>  MODULE_DEVICE_TABLE(i2c, taos_idtable);
>>  
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id taos2583_of_match[] = {
>> +	{ .compatible = "amstaos,tsl2580", },
>> +	{ .compatible = "amstaos,tsl2581", },
>> +	{ .compatible = "amstaos,tsl2583", },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, taos2583_of_match);
>> +#else
>> +#define taos2583_of_match NULL
>> +#endif
>> +
>>  /* Driver definition */
>>  static struct i2c_driver taos_driver = {
>>  	.driver = {
>>  		.name = "tsl2583",
>>  		.pm = TAOS_PM_OPS,
>> +		.of_match_table = taos2583_of_match,
>>  	},
>>  	.id_table = taos_idtable,
>>  	.probe = taos_probe,
>>
> 
> --
> 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
> 

Re: [PATCH 02/10] staging: iio: tsl2583: check for error code from i2c_smbus_read_byte()

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> taos_i2c_read() and taos_als_calibrate() does not check to see if the
> value returned by i2c_smbus_read_byte() was an error code. This patch
> adds the appropriate error handling.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders
to play with it.

Also Cc'd Jon on basis he might want to run his eye of these.
> ---
>  drivers/staging/iio/light/tsl2583.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index fd4b6ef..35c1696 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -171,7 +171,14 @@ taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len)
>  			return ret;
>  		}
>  		/* read the data */
> -		*val = i2c_smbus_read_byte(client);
> +		ret = i2c_smbus_read_byte(client);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"%s failed to read byte after writing to register %x\n",
> +				__func__, reg);
> +			return ret;
> +		}
> +		*val = ret;
>  		val++;
>  		reg++;
>  	}
> @@ -355,6 +362,13 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
>  	}
>  
>  	reg_val = i2c_smbus_read_byte(chip->client);
> +	if (reg_val < 0) {
> +		dev_err(&chip->client->dev,
> +			"%s failed to read after writing to the CNTRL register\n",
> +			__func__);
> +		return ret;
> +	}
> +
>  	if ((reg_val & (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON))
>  			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
>  		dev_err(&chip->client->dev,
> @@ -371,6 +385,12 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
>  		return ret;
>  	}
>  	reg_val = i2c_smbus_read_byte(chip->client);
> +	if (reg_val < 0) {
> +		dev_err(&chip->client->dev,
> +			"%s failed to read after writing to the STATUS register\n",
> +			__func__);
> +		return ret;
> +	}
>  
>  	if ((reg_val & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) {
>  		dev_err(&chip->client->dev,
> 

Re: [PATCH 03/10] staging: iio: tsl2583: return proper error code instead of -1

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> taos_als_calibrate() has a code path where -1 is returned. This patch
> changes the code so that a proper error code is returned.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.
(I'm going to begin to get lazy on this message ;)
> ---
>  drivers/staging/iio/light/tsl2583.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 35c1696..47fd373 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -373,7 +373,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev)
>  			!= (TSL258X_CNTL_ADC_ENBL | TSL258X_CNTL_PWR_ON)) {
>  		dev_err(&chip->client->dev,
>  			"taos_als_calibrate failed: device not powered on with ADC enabled\n");
> -		return -1;
> +		return -EINVAL;
>  	}
>  
>  	ret = i2c_smbus_write_byte(chip->client,
> 

Re: [PATCH 04/10] staging: iio: tsl2583: remove redundant power_state sysfs attribute

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> IIO devices have a /sys/bus/iio/devices/iio:deviceX/power/ directory
> that allows viewing and controling various power parameters. The tsl2583
> driver also has an additional custom sysfs attribute named power_state
> that is not needed. This patch removes the redundant power_state sysfs
> attribute.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied. etc.
> ---
>  drivers/staging/iio/light/tsl2583.c | 31 -------------------------------
>  1 file changed, 31 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 47fd373..f8ccb4d 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -511,33 +511,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
>  
>  /* Sysfs Interface Functions */
>  
> -static ssize_t taos_power_state_show(struct device *dev,
> -				     struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%d\n", chip->taos_chip_status);
> -}
> -
> -static ssize_t taos_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);
> -	int value;
> -
> -	if (kstrtoint(buf, 0, &value))
> -		return -EINVAL;
> -
> -	if (!value)
> -		taos_chip_off(indio_dev);
> -	else
> -		taos_chip_on(indio_dev);
> -
> -	return len;
> -}
> -
>  static ssize_t taos_gain_show(struct device *dev,
>  			      struct device_attribute *attr, char *buf)
>  {
> @@ -785,9 +758,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
>  	return len;
>  }
>  
> -static DEVICE_ATTR(power_state, S_IRUGO | S_IWUSR,
> -		taos_power_state_show, taos_power_state_store);
> -
>  static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
>  		taos_gain_show, taos_gain_store);
>  static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
> @@ -810,7 +780,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
>  		taos_luxtable_show, taos_luxtable_store);
>  
>  static struct attribute *sysfs_attrs_ctrl[] = {
> -	&dev_attr_power_state.attr,
>  	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
>  	&dev_attr_illuminance0_calibscale_available.attr,
>  	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
> 

Re: [PATCH 05/10] staging: iio: tsl2583: check return values from taos_chip_{on,off}

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> The return value from taos_chip_on() and taos_chip_off() was not
> checked in taos_luxtable_store() and taos_probe(). This patch adds
> proper error checking to these function calls.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
This does raise the question of whether we should have a remove that turns
the device off, but that's a separate issue!
I don't however like the jumping to a single exit point.
That should have been done in the later patch where you add locking rather
than here.

I'll take it anyway though on the basis the ugliness doesn't last long.

Jonathan
> ---
>  drivers/staging/iio/light/tsl2583.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index f8ccb4d..e975bba 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -727,7 +727,7 @@ static ssize_t taos_luxtable_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
>  	int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
> -	int n;
> +	int n, ret = -EINVAL;
>  
>  	get_options(buf, ARRAY_SIZE(value), value);
>  
> @@ -739,23 +739,31 @@ static ssize_t taos_luxtable_store(struct device *dev,
>  	n = value[0];
>  	if ((n % 3) || n < 6 || n > ((ARRAY_SIZE(taos_device_lux) - 1) * 3)) {
>  		dev_info(dev, "LUX TABLE INPUT ERROR 1 Value[0]=%d\n", n);
> -		return -EINVAL;
> +		goto done;
>  	}
>  	if ((value[(n - 2)] | value[(n - 1)] | value[n]) != 0) {
>  		dev_info(dev, "LUX TABLE INPUT ERROR 2 Value[0]=%d\n", n);
> -		return -EINVAL;
> +		goto done;
>  	}
>  
> -	if (chip->taos_chip_status == TSL258X_CHIP_WORKING)
> -		taos_chip_off(indio_dev);
> +	if (chip->taos_chip_status == TSL258X_CHIP_WORKING) {
> +		ret = taos_chip_off(indio_dev);
> +		if (ret < 0)
> +			goto done;
> +	}
>  
>  	/* Zero out the table */
>  	memset(taos_device_lux, 0, sizeof(taos_device_lux));
>  	memcpy(taos_device_lux, &value[1], (value[0] * 4));
>  
> -	taos_chip_on(indio_dev);
> +	ret = taos_chip_on(indio_dev);
> +	if (ret < 0)
> +		goto done;
>  
> -	return len;
> +	ret = len;
> +
> +done:
> +	return ret;
>  }
>  
>  static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
> @@ -883,7 +891,9 @@ static int taos_probe(struct i2c_client *clientp,
>  	taos_defaults(chip);
>  
>  	/* Make sure the chip is on */
> -	taos_chip_on(indio_dev);
> +	ret = taos_chip_on(indio_dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	dev_info(&clientp->dev, "Light sensor found.\n");
>  	return 0;
> 

Re: [PATCH 06/10] staging: iio: tsl2583: convert to use iio_chan_spec and {read,write}_raw

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> The tsl2583 driver directly creates sysfs attributes that should instead
> be created by the IIO core on behalf of the driver. This patch adds the
> iio_chan_spec array, the relevant info_mask elements and the read_raw()
> and write_raw() functions to take advantage of features provided by the
> IIO core. These sysfs attributes were migrated with this patch:
> illuminance0_input, illuminance0_calibbias,
> illuminance0_integration_time. This also exposes the raw values read
> from the two channels on the sensor.
> 
> With this change, these four sysfs entries have their prefix changed
> from illuminance0_ to in_illuminance_. This is deemed to be acceptable
> since none of the IIO light drivers in mainline use the illuminance0_
> prefix, however 8 of the IIO light drivers in mainline use the
> in_illuminance_ prefix.
Yeah, ABI was previously broken.  We made this change a long time ago
before anything actually moved out of staging.  Possible we'll break
some userspace code, but that's the advantage of being in staging
- no guarantees on ABI remaining the same!

Hmm... I think you also fixed the units of integration time which we
should probably mention.

I'll add a note - applied to the togreg branch of iio.git and pushed out
as testing. Jon if you get a chance, this is the first one with major
changes.

Thanks,

Jonathan
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/staging/iio/light/tsl2583.c | 236 ++++++++++++++++++++++--------------
>  1 file changed, 143 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index e975bba..6a61a86 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -211,28 +211,23 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  	u32 ch0lux = 0;
>  	u32 ch1lux = 0;
>  
> -	if (mutex_trylock(&chip->als_mutex) == 0) {
> -		dev_info(&chip->client->dev, "taos_get_lux device is busy\n");
> -		return chip->als_cur_info.lux; /* busy, so return LAST VALUE */
> -	}
> -
>  	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
>  		/* device is not enabled */
>  		dev_err(&chip->client->dev, "taos_get_lux device is not enabled\n");
>  		ret = -EBUSY;
> -		goto out_unlock;
> +		goto done;
>  	}
>  
>  	ret = taos_i2c_read(chip->client, (TSL258X_CMD_REG), &buf[0], 1);
>  	if (ret < 0) {
>  		dev_err(&chip->client->dev, "taos_get_lux failed to read CMD_REG\n");
> -		goto out_unlock;
> +		goto done;
>  	}
>  	/* is data new & valid */
>  	if (!(buf[0] & TSL258X_STA_ADC_INTR)) {
>  		dev_err(&chip->client->dev, "taos_get_lux data not valid\n");
>  		ret = chip->als_cur_info.lux; /* return LAST VALUE */
> -		goto out_unlock;
> +		goto done;
>  	}
>  
>  	for (i = 0; i < 4; i++) {
> @@ -243,7 +238,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  			dev_err(&chip->client->dev,
>  				"taos_get_lux failed to read register %x\n",
>  				reg);
> -			goto out_unlock;
> +			goto done;
>  		}
>  	}
>  
> @@ -259,7 +254,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  		dev_err(&chip->client->dev,
>  			"taos_i2c_write_command failed in taos_get_lux, err = %d\n",
>  			ret);
> -		goto out_unlock; /* have no data, so return failure */
> +		goto done; /* have no data, so return failure */
>  	}
>  
>  	/* extract ALS/lux data */
> @@ -276,7 +271,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  		/* have no data, so return LAST VALUE */
>  		ret = 0;
>  		chip->als_cur_info.lux = 0;
> -		goto out_unlock;
> +		goto done;
>  	}
>  	/* calculate ratio */
>  	ratio = (ch1 << 15) / ch0;
> @@ -302,7 +297,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  		dev_dbg(&chip->client->dev, "No Data - Return last value\n");
>  		ret = 0;
>  		chip->als_cur_info.lux = 0;
> -		goto out_unlock;
> +		goto done;
>  	}
>  
>  	/* adjust for active time scale */
> @@ -334,8 +329,7 @@ static int taos_get_lux(struct iio_dev *indio_dev)
>  	chip->als_cur_info.lux = lux;
>  	ret = lux;
>  
> -out_unlock:
> -	mutex_unlock(&chip->als_mutex);
> +done:
>  	return ret;
>  }
>  
> @@ -575,69 +569,12 @@ static ssize_t taos_gain_available_show(struct device *dev,
>  	return sprintf(buf, "%s\n", "1 8 16 111");
>  }
>  
> -static ssize_t taos_als_time_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%d\n", chip->taos_settings.als_time);
> -}
> -
> -static ssize_t taos_als_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 tsl2583_chip *chip = iio_priv(indio_dev);
> -	int value;
> -
> -	if (kstrtoint(buf, 0, &value))
> -		return -EINVAL;
> -
> -	if ((value < 50) || (value > 650))
> -		return -EINVAL;
> -
> -	if (value % 50)
> -		return -EINVAL;
> -
> -	chip->taos_settings.als_time = value;
> -
> -	return len;
> -}
> -
>  static ssize_t taos_als_time_available_show(struct device *dev,
>  					    struct device_attribute *attr,
>  					    char *buf)
>  {
>  	return sprintf(buf, "%s\n",
> -		"50 100 150 200 250 300 350 400 450 500 550 600 650");
> -}
> -
> -static ssize_t taos_als_trim_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -
> -	return sprintf(buf, "%d\n", chip->taos_settings.als_gain_trim);
> -}
> -
> -static ssize_t taos_als_trim_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 tsl2583_chip *chip = iio_priv(indio_dev);
> -	int value;
> -
> -	if (kstrtoint(buf, 0, &value))
> -		return -EINVAL;
> -
> -	if (value)
> -		chip->taos_settings.als_gain_trim = value;
> -
> -	return len;
> +		"0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
>  }
>  
>  static ssize_t taos_als_cal_target_show(struct device *dev,
> @@ -667,18 +604,6 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_lux_show(struct device *dev, struct device_attribute *attr,
> -			     char *buf)
> -{
> -	int ret;
> -
> -	ret = taos_get_lux(dev_to_iio_dev(dev));
> -	if (ret < 0)
> -		return ret;
> -
> -	return sprintf(buf, "%d\n", ret);
> -}
> -
>  static ssize_t taos_do_calibrate(struct device *dev,
>  				 struct device_attribute *attr,
>  				 const char *buf, size_t len)
> @@ -771,18 +696,12 @@ static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
>  static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
>  		taos_gain_available_show, NULL);
>  
> -static DEVICE_ATTR(illuminance0_integration_time, S_IRUGO | S_IWUSR,
> -		taos_als_time_show, taos_als_time_store);
>  static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
>  		taos_als_time_available_show, NULL);
>  
> -static DEVICE_ATTR(illuminance0_calibbias, S_IRUGO | S_IWUSR,
> -		taos_als_trim_show, taos_als_trim_store);
> -
>  static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
>  		taos_als_cal_target_show, taos_als_cal_target_store);
>  
> -static DEVICE_ATTR(illuminance0_input, S_IRUGO, taos_lux_show, NULL);
>  static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
>  static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
>  		taos_luxtable_show, taos_luxtable_store);
> @@ -790,11 +709,8 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
>  static struct attribute *sysfs_attrs_ctrl[] = {
>  	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
>  	&dev_attr_illuminance0_calibscale_available.attr,
> -	&dev_attr_illuminance0_integration_time.attr,	/* I time*/
>  	&dev_attr_illuminance0_integration_time_available.attr,
> -	&dev_attr_illuminance0_calibbias.attr,			/* trim  */
>  	&dev_attr_illuminance0_input_target.attr,
> -	&dev_attr_illuminance0_input.attr,
>  	&dev_attr_illuminance0_calibrate.attr,
>  	&dev_attr_illuminance0_lux_table.attr,
>  	NULL
> @@ -810,9 +726,141 @@ static int taos_tsl258x_device(unsigned char *bufp)
>  	return ((bufp[TSL258X_CHIPID] & 0xf0) == 0x90);
>  }
>  
> +static const struct iio_chan_spec tsl2583_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_IR,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_BOTH,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
> +				      BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +};
> +
> +static int tsl2583_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> + 	
> +	mutex_lock(&chip->als_mutex);
> +
> +	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
> +		ret = -EBUSY;
> +		goto read_done;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = taos_get_lux(indio_dev);
> +			if (ret < 0)
> +				goto read_done;
> +
> +			/*
> +			 * From page 20 of the TSL2581, TSL2583 data
> +			 * sheet (TAOS134 − MARCH 2011):
> +			 *
> +			 * One of the photodiodes (channel 0) is
> +			 * sensitive to both visible and infrared light,
> +			 * while the second photodiode (channel 1) is
> +			 * sensitive primarily to infrared light.
> +			 */
> +			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
> +				*val = chip->als_cur_info.als_ch0;
> +			else
> +				*val = chip->als_cur_info.als_ch1;
> +
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = taos_get_lux(indio_dev);
> +			if (ret < 0)
> +				goto read_done;
> +
> +			*val = ret;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = chip->taos_settings.als_gain_trim;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = 0;
> +			*val2 = chip->taos_settings.als_time;
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +read_done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static int tsl2583_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	if (chip->taos_chip_status != TSL258X_CHIP_WORKING) {
> +		ret = -EBUSY;
> +		goto write_done;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (chan->type == IIO_LIGHT) {
> +			chip->taos_settings.als_gain_trim = val;
> +			ret = 0;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
> +		    val2 <= 650 && !(val2 % 50)) {
> +			chip->taos_settings.als_time = val2;
> +			ret = 0;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +write_done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
>  static const struct iio_info tsl2583_info = {
>  	.attrs = &tsl2583_attribute_group,
>  	.driver_module = THIS_MODULE,
> +	.read_raw = tsl2583_read_raw,
> +	.write_raw = tsl2583_write_raw,
>  };
>  
>  /*
> @@ -878,6 +926,8 @@ static int taos_probe(struct i2c_client *clientp,
>  	}
>  
>  	indio_dev->info = &tsl2583_info;
> +	indio_dev->channels = tsl2583_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels);
>  	indio_dev->dev.parent = &clientp->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->name = chip->client->name;
> 

Re: [PATCH 07/10] staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to use iio_chan_spec

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> The illuminance0_calibscale sysfs attribute is not currently created by
> the IIO core. This patch adds the appropriate mask to iio_chan_spec,
> along with the appropriate data handling in the read_raw() and
> write_raw() functions, so that the sysfs attribute is created by the IIO
> core. With this change, this sysfs entry will have its prefix changed
> from illuminance0_ to in_illuminance_.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Hmm. Could probably have computed the mean on the fly but fine as is.

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

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2583.c | 89 +++++++++++--------------------------
>  1 file changed, 25 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 6a61a86..bfff6ca 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -124,14 +124,15 @@ static struct taos_lux taos_device_lux[11] = {
>  struct gainadj {
>  	s16 ch0;
>  	s16 ch1;
> +	s16 mean;
>  };
>  
>  /* Index = (0 - 3) Used to validate the gain selection index */
>  static const struct gainadj gainadj[] = {
> -	{ 1, 1 },
> -	{ 8, 8 },
> -	{ 16, 16 },
> -	{ 107, 115 }
> +	{ 1, 1, 1 },
> +	{ 8, 8, 8 },
> +	{ 16, 16, 16 },
> +	{ 107, 115, 111 }
>  };
>  
>  /*
> @@ -505,63 +506,6 @@ static int taos_chip_off(struct iio_dev *indio_dev)
>  
>  /* Sysfs Interface Functions */
>  
> -static ssize_t taos_gain_show(struct device *dev,
> -			      struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	char gain[4] = {0};
> -
> -	switch (chip->taos_settings.als_gain) {
> -	case 0:
> -		strcpy(gain, "001");
> -		break;
> -	case 1:
> -		strcpy(gain, "008");
> -		break;
> -	case 2:
> -		strcpy(gain, "016");
> -		break;
> -	case 3:
> -		strcpy(gain, "111");
> -		break;
> -	}
> -
> -	return sprintf(buf, "%s\n", gain);
> -}
> -
> -static ssize_t taos_gain_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 tsl2583_chip *chip = iio_priv(indio_dev);
> -	int value;
> -
> -	if (kstrtoint(buf, 0, &value))
> -		return -EINVAL;
> -
> -	switch (value) {
> -	case 1:
> -		chip->taos_settings.als_gain = 0;
> -		break;
> -	case 8:
> -		chip->taos_settings.als_gain = 1;
> -		break;
> -	case 16:
> -		chip->taos_settings.als_gain = 2;
> -		break;
> -	case 111:
> -		chip->taos_settings.als_gain = 3;
> -		break;
> -	default:
> -		dev_err(dev, "Invalid Gain Index (must be 1,8,16,111)\n");
> -		return -1;
> -	}
> -
> -	return len;
> -}
> -
>  static ssize_t taos_gain_available_show(struct device *dev,
>  					struct device_attribute *attr,
>  					char *buf)
> @@ -691,8 +635,6 @@ static ssize_t taos_luxtable_store(struct device *dev,
>  	return ret;
>  }
>  
> -static DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
> -		taos_gain_show, taos_gain_store);
>  static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
>  		taos_gain_available_show, NULL);
>  
> @@ -707,7 +649,6 @@ static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
>  		taos_luxtable_show, taos_luxtable_store);
>  
>  static struct attribute *sysfs_attrs_ctrl[] = {
> -	&dev_attr_illuminance0_calibscale.attr,			/* Gain  */
>  	&dev_attr_illuminance0_calibscale_available.attr,
>  	&dev_attr_illuminance0_integration_time_available.attr,
>  	&dev_attr_illuminance0_input_target.attr,
> @@ -743,6 +684,7 @@ static const struct iio_chan_spec tsl2583_channels[] = {
>  		.type = IIO_LIGHT,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
>  				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
> +				      BIT(IIO_CHAN_INFO_CALIBSCALE) |
>  				      BIT(IIO_CHAN_INFO_INT_TIME),
>  	},
>  };
> @@ -801,6 +743,12 @@ static int tsl2583_read_raw(struct iio_dev *indio_dev,
>  			ret = IIO_VAL_INT;
>  		}
>  		break;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = gainadj[chip->taos_settings.als_gain].mean;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
>  	case IIO_CHAN_INFO_INT_TIME:
>  		if (chan->type == IIO_LIGHT) {
>  			*val = 0;
> @@ -839,6 +787,19 @@ static int tsl2583_write_raw(struct iio_dev *indio_dev,
>  			ret = 0;
>  		}
>  		break;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			int i;
> +
> +			for (i = 0; i < ARRAY_SIZE(gainadj); i++) {
> +				if (gainadj[i].mean == val) {
> +					chip->taos_settings.als_gain = i;
> +					ret = 0;
> +					break;
> +				}
> +			}
> +		}
> +		break;
>  	case IIO_CHAN_INFO_INT_TIME:
>  		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
>  		    val2 <= 650 && !(val2 % 50)) {
> 

Re: [PATCH 08/10] staging: iio: tsl2583: use IIO_*_ATTR* macros to create sysfs entries

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> Use the IIO_CONST_ATTR, IIO_DEVICE_ATTR_RW, and IIO_DEVICE_ATTR_WO
> macros for creating the in_illuminance_calibscale_available,
> in_illuminance_integration_time_available, in_illuminance_input_target,
> in_illuminance_calibrate, and in_illuminance_lux_table sysfs entries.
> Previously these sysfs entries were prefixed with illuminance0_, however
> they are now prefixed with in_illuminance_ to make these sysfs entries
> consistent with how the IIO core is creating the other sysfs entries.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2583.c | 73 ++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index bfff6ca..1462374 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -29,6 +29,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
>  
>  #define TSL258X_MAX_DEVICE_REGS		32
>  
> @@ -506,24 +507,9 @@ static int taos_chip_off(struct iio_dev *indio_dev)
>  
>  /* Sysfs Interface Functions */
>  
> -static ssize_t taos_gain_available_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> -{
> -	return sprintf(buf, "%s\n", "1 8 16 111");
> -}
> -
> -static ssize_t taos_als_time_available_show(struct device *dev,
> -					    struct device_attribute *attr,
> -					    char *buf)
> -{
> -	return sprintf(buf, "%s\n",
> -		"0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
> -}
> -
> -static ssize_t taos_als_cal_target_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> +static ssize_t in_illuminance_input_target_show(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -531,9 +517,9 @@ static ssize_t taos_als_cal_target_show(struct device *dev,
>  	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
>  }
>  
> -static ssize_t taos_als_cal_target_store(struct device *dev,
> -					 struct device_attribute *attr,
> -					 const char *buf, size_t len)
> +static ssize_t in_illuminance_input_target_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 tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -548,9 +534,9 @@ static ssize_t taos_als_cal_target_store(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_do_calibrate(struct device *dev,
> -				 struct device_attribute *attr,
> -				 const char *buf, size_t len)
> +static ssize_t in_illuminance_calibrate_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	int value;
> @@ -564,8 +550,9 @@ static ssize_t taos_do_calibrate(struct device *dev,
>  	return len;
>  }
>  
> -static ssize_t taos_luxtable_show(struct device *dev,
> -				  struct device_attribute *attr, char *buf)
> +static ssize_t in_illuminance_lux_table_show(struct device *dev,
> +					     struct device_attribute *attr,
> +					     char *buf)
>  {
>  	int i;
>  	int offset = 0;
> @@ -589,9 +576,9 @@ static ssize_t taos_luxtable_show(struct device *dev,
>  	return offset;
>  }
>  
> -static ssize_t taos_luxtable_store(struct device *dev,
> -				   struct device_attribute *attr,
> -				   const char *buf, size_t len)
> +static ssize_t in_illuminance_lux_table_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 tsl2583_chip *chip = iio_priv(indio_dev);
> @@ -635,25 +622,19 @@ static ssize_t taos_luxtable_store(struct device *dev,
>  	return ret;
>  }
>  
> -static DEVICE_ATTR(illuminance0_calibscale_available, S_IRUGO,
> -		taos_gain_available_show, NULL);
> -
> -static DEVICE_ATTR(illuminance0_integration_time_available, S_IRUGO,
> -		taos_als_time_available_show, NULL);
> -
> -static DEVICE_ATTR(illuminance0_input_target, S_IRUGO | S_IWUSR,
> -		taos_als_cal_target_show, taos_als_cal_target_store);
> -
> -static DEVICE_ATTR(illuminance0_calibrate, S_IWUSR, NULL, taos_do_calibrate);
> -static DEVICE_ATTR(illuminance0_lux_table, S_IRUGO | S_IWUSR,
> -		taos_luxtable_show, taos_luxtable_store);
> +static IIO_CONST_ATTR(in_illuminance_calibscale_available, "1 8 16 111");
> +static IIO_CONST_ATTR(in_illuminance_integration_time_available,
> +		      "0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
> +static IIO_DEVICE_ATTR_RW(in_illuminance_input_target, 0);
> +static IIO_DEVICE_ATTR_WO(in_illuminance_calibrate, 0);
> +static IIO_DEVICE_ATTR_RW(in_illuminance_lux_table, 0);
>  
>  static struct attribute *sysfs_attrs_ctrl[] = {
> -	&dev_attr_illuminance0_calibscale_available.attr,
> -	&dev_attr_illuminance0_integration_time_available.attr,
> -	&dev_attr_illuminance0_input_target.attr,
> -	&dev_attr_illuminance0_calibrate.attr,
> -	&dev_attr_illuminance0_lux_table.attr,
> +	&iio_const_attr_in_illuminance_calibscale_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_input_target.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_calibrate.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_lux_table.dev_attr.attr,
>  	NULL
>  };
>  
> 

Re: [PATCH 09/10] staging: iio: tsl2583: add error code to sysfs store functions

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> in_illuminance_input_target_store() and in_illuminance_calibrate_store()
> validated the data from userspace, however it would not return an
> error code to userspace if an invalid value was passed in. This patch
> changes these functions so that they return -EINVAL if invalid data is
> passed in.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied thanks.

Jonathan
> ---
>  drivers/staging/iio/light/tsl2583.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 1462374..98afa5b 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -525,11 +525,10 @@ static ssize_t in_illuminance_input_target_store(struct device *dev,
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
>  	int value;
>  
> -	if (kstrtoint(buf, 0, &value))
> +	if (kstrtoint(buf, 0, &value) || !value)
>  		return -EINVAL;
>  
> -	if (value)
> -		chip->taos_settings.als_cal_target = value;
> +	chip->taos_settings.als_cal_target = value;
>  
>  	return len;
>  }
> @@ -541,11 +540,10 @@ static ssize_t in_illuminance_calibrate_store(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	int value;
>  
> -	if (kstrtoint(buf, 0, &value))
> +	if (kstrtoint(buf, 0, &value) || value != 1)
>  		return -EINVAL;
>  
> -	if (value == 1)
> -		taos_als_calibrate(indio_dev);
> +	taos_als_calibrate(indio_dev);
>  
>  	return len;
>  }
> 

Re: [PATCH 10/10] staging: iio: tsl2583: add locking to sysfs attributes

[Jonathan Cameron]

On 28/10/16 11:00, Brian Masney wrote:
> in_illuminance_input_target_show(), in_illuminance_input_target_store(),
> in_illuminance_calibrate_store(), and in_illuminance_lux_table_store()
> accesses data from the tsl2583_chip struct. Some of these fields can be
> modified by other parts of the driver concurrently. This patch adds the
> mutex locking to these sysfs attributes.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Some elements of this were probably a little over paranoid but in theory
you might get a wrong reading (compared to what the hardware is set to) so
fair enough.

Good patch set.

Thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2583.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> index 98afa5b..49b19f5 100644
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ b/drivers/staging/iio/light/tsl2583.c
> @@ -513,8 +513,13 @@ static ssize_t in_illuminance_input_target_show(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&chip->als_mutex);
> +	ret = sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
> +	mutex_unlock(&chip->als_mutex);
>  
> -	return sprintf(buf, "%d\n", chip->taos_settings.als_cal_target);
> +	return ret;
>  }
>  
>  static ssize_t in_illuminance_input_target_store(struct device *dev,
> @@ -528,7 +533,9 @@ static ssize_t in_illuminance_input_target_store(struct device *dev,
>  	if (kstrtoint(buf, 0, &value) || !value)
>  		return -EINVAL;
>  
> +	mutex_lock(&chip->als_mutex);
>  	chip->taos_settings.als_cal_target = value;
> +	mutex_unlock(&chip->als_mutex);
>  
>  	return len;
>  }
> @@ -538,12 +545,15 @@ static ssize_t in_illuminance_calibrate_store(struct device *dev,
>  					      const char *buf, size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
>  	int value;
>  
>  	if (kstrtoint(buf, 0, &value) || value != 1)
>  		return -EINVAL;
>  
> +	mutex_lock(&chip->als_mutex);
>  	taos_als_calibrate(indio_dev);
> +	mutex_unlock(&chip->als_mutex);
>  
>  	return len;
>  }
> @@ -583,6 +593,8 @@ static ssize_t in_illuminance_lux_table_store(struct device *dev,
>  	int value[ARRAY_SIZE(taos_device_lux) * 3 + 1];
>  	int n, ret = -EINVAL;
>  
> +	mutex_lock(&chip->als_mutex);
> +
>  	get_options(buf, ARRAY_SIZE(value), value);
>  
>  	/* We now have an array of ints starting at value[1], and
> @@ -617,6 +629,8 @@ static ssize_t in_illuminance_lux_table_store(struct device *dev,
>  	ret = len;
>  
>  done:
> +	mutex_unlock(&chip->als_mutex);
> +
>  	return ret;
>  }
>  
> 

Re: [PATCH 07/10] staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to use iio_chan_spec

[Brian Masney]

On Sun, Oct 30, 2016 at 06:37:40PM +0000, Jonathan Cameron wrote:
> On 28/10/16 11:00, Brian Masney wrote:
> > The illuminance0_calibscale sysfs attribute is not currently created by
> > the IIO core. This patch adds the appropriate mask to iio_chan_spec,
> > along with the appropriate data handling in the read_raw() and
> > write_raw() functions, so that the sysfs attribute is created by the IIO
> > core. With this change, this sysfs entry will have its prefix changed
> > from illuminance0_ to in_illuminance_.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> Hmm. Could probably have computed the mean on the fly but fine as is.

Hi Jonathan,
   Thanks for your feedback. I'll have another patch set ready for you
by next weekend with my next round of cleanups to that driver. I'll
change this so that it calculates the mean on the fly unless I hear
otherwise from you.

Brian

Re: [PATCH 07/10] staging: iio: tsl2583: convert illuminance0_calibscale sysfs attr to use iio_chan_spec

[Jonathan Cameron]

On 30 October 2016 20:04:09 GMT+00:00, Brian Masney <masneyb@onstation.org> wrote:
>On Sun, Oct 30, 2016 at 06:37:40PM +0000, Jonathan Cameron wrote:
>> On 28/10/16 11:00, Brian Masney wrote:
>> > The illuminance0_calibscale sysfs attribute is not currently
>created by
>> > the IIO core. This patch adds the appropriate mask to
>iio_chan_spec,
>> > along with the appropriate data handling in the read_raw() and
>> > write_raw() functions, so that the sysfs attribute is created by
>the IIO
>> > core. With this change, this sysfs entry will have its prefix
>changed
>> > from illuminance0_ to in_illuminance_.
>> > 
>> > Signed-off-by: Brian Masney <masneyb@onstation.org>
>> Hmm. Could probably have computed the mean on the fly but fine as is.
>
>Hi Jonathan,
>   Thanks for your feedback. I'll have another patch set ready for you
>by next weekend with my next round of cleanups to that driver. I'll
>change this so that it calculates the mean on the fly unless I hear
>otherwise from you.

I don't worry about the mean change. Not worth changing to my mind.

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

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

[Rob Herring]

On Fri, Oct 28, 2016 at 06:00:12AM -0400, Brian Masney wrote:
> Add device tree support for the tsl2583 IIO driver with no custom
> properties.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../devicetree/bindings/iio/light/tsl2583.txt      | 26 ++++++++++++++++++++++
>  drivers/staging/iio/light/tsl2583.c                | 13 +++++++++++
>  2 files changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

Acked-by: Rob Herring <robh@kernel.org>