[PATCH 0/4] add INT_TIME and drivers using it

[PATCH 0/4] add INT_TIME and drivers using it

[Peter Meerwald]

the INT_TIME introduction received an informal nod by Jon Brenner,
the tsl4531 driver has been updated to v3 due to comments by Lars-Peter
Clausen

Peter Meerwald (4):
  iio: Add INT_TIME (integration time) channel info attribute
  iio: adjd_s311: Use INT_TIME channel info
  iio: Add tsl4531 ambient light sensor driver
  iio: Add tcs3472 color light sensor driver

 Documentation/ABI/testing/sysfs-bus-iio |   12 +
 drivers/iio/industrialio-core.c         |    1 +
 drivers/iio/light/Kconfig               |   20 ++
 drivers/iio/light/Makefile              |    2 +
 drivers/iio/light/adjd_s311.c           |   72 ++----
 drivers/iio/light/tcs3472.c             |  373 +++++++++++++++++++++++++++++++
 drivers/iio/light/tsl4531.c             |  258 +++++++++++++++++++++
 include/linux/iio/iio.h                 |    1 +
 include/linux/iio/sysfs.h               |   15 ++
 9 files changed, 703 insertions(+), 51 deletions(-)
 create mode 100644 drivers/iio/light/tcs3472.c
 create mode 100644 drivers/iio/light/tsl4531.c

-- 
1.7.9.5

[PATCH 1/4] iio: Add INT_TIME (integration time) channel info attribute

[Peter Meerwald]

Integration time is in seconds; it controls the measurement
time and influences the gain of a sensor.

There are two typical ways that scaling is implemented in a device:
1) input amplifier,
2) reference to the ADC is changed.
These both result in the accuracy of the ADC varying (by applying its
sampling over a more relevant range).

Integration time is a way of dealing with noise inherent in the analog
sensor itself.  In the case of a light sensor, a mixture of photon noise
and device specific noise.  Photon noise is dealt with by either improving
the efficiency of the sensor, (more photons actually captured) which is not
easily varied dynamically, or by integrating the measurement over a longer
time period.  Note that this can also be thought of as an averaging of a
number of individual samples and is infact sometimes implemented this way.
Altering integration time implies that the duration of a measurement changes,
a fact the device's user may be interested in.

Hence it makes sense to distinguish between integration time and simple
scale. In some devices both types of control are present and whilst they
will have similar effects on the amplitude of the reading, their effect
on the noise of the measurements will differ considerably.

Used by adjd_s311, tsl4531, tcs3472
The following drivers have similar controls (and could be adapted):
* tsl2563 (integration time is controlled via CALIBSCALE among other things)
* tsl2583 (has integration_time device_attr, but driver doesn't use channels yet)
* tsl2x7x (has integration_time attr)

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Jon Brenner <jon.brenner@ams.com>
---
 Documentation/ABI/testing/sysfs-bus-iio |   12 ++++++++++++
 drivers/iio/industrialio-core.c         |    1 +
 include/linux/iio/iio.h                 |    1 +
 include/linux/iio/sysfs.h               |   15 +++++++++++++++
 4 files changed, 29 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 5caaab9..d75147c 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -800,3 +800,15 @@ Description:
 		Writing '1' stores the current device configuration into
 		on-chip EEPROM. After power-up or chip reset the device will
 		automatically load the saved configuration.
+
+What:		/sys/.../iio:deviceX/in_intensity_red_integration_time
+What:		/sys/.../iio:deviceX/in_intensity_green_integration_time
+What:		/sys/.../iio:deviceX/in_intensity_blue_integration_time
+What:		/sys/.../iio:deviceX/in_intensity_clear_integration_time
+What:		/sys/.../iio:deviceX/in_illuminance_integration_time
+KernelVersion:	3.12
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute is used to get/set the integration time in
+		seconds.
+
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 97f0297..2cb4841 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -101,6 +101,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_PHASE] = "phase",
 	[IIO_CHAN_INFO_HARDWAREGAIN] = "hardwaregain",
 	[IIO_CHAN_INFO_HYSTERESIS] = "hysteresis",
+	[IIO_CHAN_INFO_INT_TIME] = "integration_time",
 };
 
 const struct iio_chan_spec
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 8e7a813..01edd67 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -36,6 +36,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_PHASE,
 	IIO_CHAN_INFO_HARDWAREGAIN,
 	IIO_CHAN_INFO_HYSTERESIS,
+	IIO_CHAN_INFO_INT_TIME,
 };
 
 enum iio_endian {
diff --git a/include/linux/iio/sysfs.h b/include/linux/iio/sysfs.h
index 2958c96..8a1d186 100644
--- a/include/linux/iio/sysfs.h
+++ b/include/linux/iio/sysfs.h
@@ -100,6 +100,21 @@ struct iio_const_attr {
 #define IIO_CONST_ATTR_SAMP_FREQ_AVAIL(_string)			\
 	IIO_CONST_ATTR(sampling_frequency_available, _string)
 
+/**
+ * IIO_DEV_ATTR_INT_TIME_AVAIL - list available integration times
+ * @_show: output method for the attribute
+ **/
+#define IIO_DEV_ATTR_INT_TIME_AVAIL(_show)		\
+	IIO_DEVICE_ATTR(integration_time_available, S_IRUGO, _show, NULL, 0)
+/**
+ * IIO_CONST_ATTR_INT_TIME_AVAIL - list available integration times
+ * @_string: frequency string for the attribute
+ *
+ * Constant version
+ **/
+#define IIO_CONST_ATTR_INT_TIME_AVAIL(_string)		\
+	IIO_CONST_ATTR(integration_time_available, _string)
+
 #define IIO_DEV_ATTR_TEMP_RAW(_show)			\
 	IIO_DEVICE_ATTR(in_temp_raw, S_IRUGO, _show, NULL, 0)
 
-- 
1.7.9.5

Re: [PATCH 1/4] iio: Add INT_TIME (integration time) channel info attribute

[Jonathan Cameron]

On 09/08/13 16:20, Peter Meerwald wrote:
> Integration time is in seconds; it controls the measurement
> time and influences the gain of a sensor.
> 
> There are two typical ways that scaling is implemented in a device:
> 1) input amplifier,
> 2) reference to the ADC is changed.
> These both result in the accuracy of the ADC varying (by applying its
> sampling over a more relevant range).
> 
> Integration time is a way of dealing with noise inherent in the analog
> sensor itself.  In the case of a light sensor, a mixture of photon noise
> and device specific noise.  Photon noise is dealt with by either improving
> the efficiency of the sensor, (more photons actually captured) which is not
> easily varied dynamically, or by integrating the measurement over a longer
> time period.  Note that this can also be thought of as an averaging of a
> number of individual samples and is infact sometimes implemented this way.
> Altering integration time implies that the duration of a measurement changes,
> a fact the device's user may be interested in.
> 
> Hence it makes sense to distinguish between integration time and simple
> scale. In some devices both types of control are present and whilst they
> will have similar effects on the amplitude of the reading, their effect
> on the noise of the measurements will differ considerably.
> 
> Used by adjd_s311, tsl4531, tcs3472
> The following drivers have similar controls (and could be adapted):
> * tsl2563 (integration time is controlled via CALIBSCALE among other things)
> * tsl2583 (has integration_time device_attr, but driver doesn't use channels yet)
> * tsl2x7x (has integration_time attr)
> 
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Jon Brenner <jon.brenner@ams.com>
We may want to put some of this detail into the abi document if we get many
people trying to use this for the wrong purpose.  Anyhow
Applied to the togreg branch of iio.git

Thanks,

> ---
>  Documentation/ABI/testing/sysfs-bus-iio |   12 ++++++++++++
>  drivers/iio/industrialio-core.c         |    1 +
>  include/linux/iio/iio.h                 |    1 +
>  include/linux/iio/sysfs.h               |   15 +++++++++++++++
>  4 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 5caaab9..d75147c 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -800,3 +800,15 @@ Description:
>  		Writing '1' stores the current device configuration into
>  		on-chip EEPROM. After power-up or chip reset the device will
>  		automatically load the saved configuration.
> +
> +What:		/sys/.../iio:deviceX/in_intensity_red_integration_time
> +What:		/sys/.../iio:deviceX/in_intensity_green_integration_time
> +What:		/sys/.../iio:deviceX/in_intensity_blue_integration_time
> +What:		/sys/.../iio:deviceX/in_intensity_clear_integration_time
> +What:		/sys/.../iio:deviceX/in_illuminance_integration_time
> +KernelVersion:	3.12
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute is used to get/set the integration time in
> +		seconds.
> +
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 97f0297..2cb4841 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -101,6 +101,7 @@ static const char * const iio_chan_info_postfix[] = {
>  	[IIO_CHAN_INFO_PHASE] = "phase",
>  	[IIO_CHAN_INFO_HARDWAREGAIN] = "hardwaregain",
>  	[IIO_CHAN_INFO_HYSTERESIS] = "hysteresis",
> +	[IIO_CHAN_INFO_INT_TIME] = "integration_time",
>  };
>  
>  const struct iio_chan_spec
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 8e7a813..01edd67 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -36,6 +36,7 @@ enum iio_chan_info_enum {
>  	IIO_CHAN_INFO_PHASE,
>  	IIO_CHAN_INFO_HARDWAREGAIN,
>  	IIO_CHAN_INFO_HYSTERESIS,
> +	IIO_CHAN_INFO_INT_TIME,
>  };
>  
>  enum iio_endian {
> diff --git a/include/linux/iio/sysfs.h b/include/linux/iio/sysfs.h
> index 2958c96..8a1d186 100644
> --- a/include/linux/iio/sysfs.h
> +++ b/include/linux/iio/sysfs.h
> @@ -100,6 +100,21 @@ struct iio_const_attr {
>  #define IIO_CONST_ATTR_SAMP_FREQ_AVAIL(_string)			\
>  	IIO_CONST_ATTR(sampling_frequency_available, _string)
>  
> +/**
> + * IIO_DEV_ATTR_INT_TIME_AVAIL - list available integration times
> + * @_show: output method for the attribute
> + **/
> +#define IIO_DEV_ATTR_INT_TIME_AVAIL(_show)		\
> +	IIO_DEVICE_ATTR(integration_time_available, S_IRUGO, _show, NULL, 0)
> +/**
> + * IIO_CONST_ATTR_INT_TIME_AVAIL - list available integration times
> + * @_string: frequency string for the attribute
> + *
> + * Constant version
> + **/
> +#define IIO_CONST_ATTR_INT_TIME_AVAIL(_string)		\
> +	IIO_CONST_ATTR(integration_time_available, _string)
> +
>  #define IIO_DEV_ATTR_TEMP_RAW(_show)			\
>  	IIO_DEVICE_ATTR(in_temp_raw, S_IRUGO, _show, NULL, 0)
>  
> 

[PATCH 2/4] iio: adjd_s311: Use INT_TIME channel info

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/light/adjd_s311.c |   72 ++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 51 deletions(-)

diff --git a/drivers/iio/light/adjd_s311.c b/drivers/iio/light/adjd_s311.c
index 23cff79..8853251 100644
--- a/drivers/iio/light/adjd_s311.c
+++ b/drivers/iio/light/adjd_s311.c
@@ -114,43 +114,6 @@ static int adjd_s311_read_data(struct iio_dev *indio_dev, u8 reg, int *val)
 	return 0;
 }
 
-static ssize_t adjd_s311_read_int_time(struct iio_dev *indio_dev,
-	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
-{
-	struct adjd_s311_data *data = iio_priv(indio_dev);
-	s32 ret;
-
-	ret = i2c_smbus_read_word_data(data->client,
-		ADJD_S311_INT_REG(chan->address));
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", ret & ADJD_S311_INT_MASK);
-}
-
-static ssize_t adjd_s311_write_int_time(struct iio_dev *indio_dev,
-	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
-	 size_t len)
-{
-	struct adjd_s311_data *data = iio_priv(indio_dev);
-	unsigned long int_time;
-	int ret;
-
-	ret = kstrtoul(buf, 10, &int_time);
-	if (ret)
-		return ret;
-
-	if (int_time > ADJD_S311_INT_MASK)
-		return -EINVAL;
-
-	ret = i2c_smbus_write_word_data(data->client,
-		ADJD_S311_INT_REG(chan->address), int_time);
-	if (ret < 0)
-		return ret;
-
-	return len;
-}
-
 static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -186,25 +149,16 @@ done:
 	return IRQ_HANDLED;
 }
 
-static const struct iio_chan_spec_ext_info adjd_s311_ext_info[] = {
-	{
-		.name = "integration_time",
-		.read = adjd_s311_read_int_time,
-		.write = adjd_s311_write_int_time,
-	},
-	{ }
-};
-
 #define ADJD_S311_CHANNEL(_color, _scan_idx) { \
 	.type = IIO_INTENSITY, \
 	.modified = 1, \
 	.address = (IDX_##_color), \
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
-		BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
+		BIT(IIO_CHAN_INFO_HARDWAREGAIN) | \
+		BIT(IIO_CHAN_INFO_INT_TIME), \
 	.channel2 = (IIO_MOD_LIGHT_##_color), \
 	.scan_index = (_scan_idx), \
 	.scan_type = IIO_ST('u', 10, 16, 0), \
-	.ext_info = adjd_s311_ext_info, \
 }
 
 static const struct iio_chan_spec adjd_s311_channels[] = {
@@ -236,6 +190,18 @@ static int adjd_s311_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		*val = ret & ADJD_S311_CAP_MASK;
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_INT_TIME:
+		ret = i2c_smbus_read_word_data(data->client,
+			ADJD_S311_INT_REG(chan->address));
+		if (ret < 0)
+			return ret;
+		*val = 0;
+		/*
+		 * not documented, based on measurement:
+		 * 4095 LSBs correspond to roughly 4 ms
+		 */
+		*val2 = ret & ADJD_S311_INT_MASK;
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
 }
@@ -245,16 +211,20 @@ static int adjd_s311_write_raw(struct iio_dev *indio_dev,
 			       int val, int val2, long mask)
 {
 	struct adjd_s311_data *data = iio_priv(indio_dev);
-	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_HARDWAREGAIN:
 		if (val < 0 || val > ADJD_S311_CAP_MASK)
 			return -EINVAL;
 
-		ret = i2c_smbus_write_byte_data(data->client,
+		return i2c_smbus_write_byte_data(data->client,
 			ADJD_S311_CAP_REG(chan->address), val);
-		return ret;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0 || val2 < 0 || val2 > ADJD_S311_INT_MASK)
+			return -EINVAL;
+
+		return i2c_smbus_write_word_data(data->client,
+			ADJD_S311_INT_REG(chan->address), val2);
 	}
 	return -EINVAL;
 }
-- 
1.7.9.5

Re: [PATCH 2/4] iio: adjd_s311: Use INT_TIME channel info

[Jonathan Cameron]

On 09/08/13 16:20, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the togreg branch of iio.git

While we are here, Hardware gain is not being correctly used here.  That is only
documented for output devices. For input it probably corresponds to calibscale
but I haven't looked in any real detail.  Ah well, this is in the ABI now
so even if we fix it we'll have to carry this interface for quite some time anyway.

Jonathan
> ---
>  drivers/iio/light/adjd_s311.c |   72 ++++++++++++-----------------------------
>  1 file changed, 21 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/iio/light/adjd_s311.c b/drivers/iio/light/adjd_s311.c
> index 23cff79..8853251 100644
> --- a/drivers/iio/light/adjd_s311.c
> +++ b/drivers/iio/light/adjd_s311.c
> @@ -114,43 +114,6 @@ static int adjd_s311_read_data(struct iio_dev *indio_dev, u8 reg, int *val)
>  	return 0;
>  }
>  
> -static ssize_t adjd_s311_read_int_time(struct iio_dev *indio_dev,
> -	uintptr_t private, const struct iio_chan_spec *chan, char *buf)
> -{
> -	struct adjd_s311_data *data = iio_priv(indio_dev);
> -	s32 ret;
> -
> -	ret = i2c_smbus_read_word_data(data->client,
> -		ADJD_S311_INT_REG(chan->address));
> -	if (ret < 0)
> -		return ret;
> -
> -	return sprintf(buf, "%d\n", ret & ADJD_S311_INT_MASK);
> -}
> -
> -static ssize_t adjd_s311_write_int_time(struct iio_dev *indio_dev,
> -	 uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> -	 size_t len)
> -{
> -	struct adjd_s311_data *data = iio_priv(indio_dev);
> -	unsigned long int_time;
> -	int ret;
> -
> -	ret = kstrtoul(buf, 10, &int_time);
> -	if (ret)
> -		return ret;
> -
> -	if (int_time > ADJD_S311_INT_MASK)
> -		return -EINVAL;
> -
> -	ret = i2c_smbus_write_word_data(data->client,
> -		ADJD_S311_INT_REG(chan->address), int_time);
> -	if (ret < 0)
> -		return ret;
> -
> -	return len;
> -}
> -
>  static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -186,25 +149,16 @@ done:
>  	return IRQ_HANDLED;
>  }
>  
> -static const struct iio_chan_spec_ext_info adjd_s311_ext_info[] = {
> -	{
> -		.name = "integration_time",
> -		.read = adjd_s311_read_int_time,
> -		.write = adjd_s311_write_int_time,
> -	},
> -	{ }
> -};
> -
>  #define ADJD_S311_CHANNEL(_color, _scan_idx) { \
>  	.type = IIO_INTENSITY, \
>  	.modified = 1, \
>  	.address = (IDX_##_color), \
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> -		BIT(IIO_CHAN_INFO_HARDWAREGAIN), \
> +		BIT(IIO_CHAN_INFO_HARDWAREGAIN) | \
> +		BIT(IIO_CHAN_INFO_INT_TIME), \
>  	.channel2 = (IIO_MOD_LIGHT_##_color), \
>  	.scan_index = (_scan_idx), \
>  	.scan_type = IIO_ST('u', 10, 16, 0), \
> -	.ext_info = adjd_s311_ext_info, \
>  }
>  
>  static const struct iio_chan_spec adjd_s311_channels[] = {
> @@ -236,6 +190,18 @@ static int adjd_s311_read_raw(struct iio_dev *indio_dev,
>  			return ret;
>  		*val = ret & ADJD_S311_CAP_MASK;
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		ret = i2c_smbus_read_word_data(data->client,
> +			ADJD_S311_INT_REG(chan->address));
> +		if (ret < 0)
> +			return ret;
> +		*val = 0;
> +		/*
> +		 * not documented, based on measurement:
> +		 * 4095 LSBs correspond to roughly 4 ms
> +		 */
> +		*val2 = ret & ADJD_S311_INT_MASK;
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
>  }
> @@ -245,16 +211,20 @@ static int adjd_s311_write_raw(struct iio_dev *indio_dev,
>  			       int val, int val2, long mask)
>  {
>  	struct adjd_s311_data *data = iio_priv(indio_dev);
> -	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_HARDWAREGAIN:
>  		if (val < 0 || val > ADJD_S311_CAP_MASK)
>  			return -EINVAL;
>  
> -		ret = i2c_smbus_write_byte_data(data->client,
> +		return i2c_smbus_write_byte_data(data->client,
>  			ADJD_S311_CAP_REG(chan->address), val);
> -		return ret;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (val != 0 || val2 < 0 || val2 > ADJD_S311_INT_MASK)
> +			return -EINVAL;
> +
> +		return i2c_smbus_write_word_data(data->client,
> +			ADJD_S311_INT_REG(chan->address), val2);
>  	}
>  	return -EINVAL;
>  }
> 

[PATCH 3/4] iio: Add tsl4531 ambient light sensor driver

[Peter Meerwald]

driver for the TSL4531 family of 16-bit I2C ambient light
sensors; information is here:
http://www.ams.com/eng/Products/Light-Sensors/Ambient-Light-Sensor-ALS/TSL45315

the chip offers simple lux output

v3 (thanks Lars-Peter Clausen):
* add mutex to when updating integration time
* fix chip ID checking
* code cleanups
v2:
* rename to tsl4351
* use INT_TIME

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Jon Brenner <jon.brenner@ams.com>
---
 drivers/iio/light/Kconfig   |   10 ++
 drivers/iio/light/Makefile  |    1 +
 drivers/iio/light/tsl4531.c |  258 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 269 insertions(+)
 create mode 100644 drivers/iio/light/tsl4531.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index bf9fa0d..488fff7 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -65,6 +65,16 @@ config SENSORS_TSL2563
 	 This driver can also be built as a module.  If so, the module
 	 will be called tsl2563.
 
+config TSL4531
+	tristate "TAOS TSL4531 ambient light sensors"
+	depends on I2C
+	help
+	 Say Y here if you want to build a driver for the TAOS TSL4531 family
+	 of ambient light sensors with direct lux output.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called tsl4531.
+
 config VCNL4000
 	tristate "VCNL4000 combined ALS and proximity sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 354ee9a..cd6f3cf 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
 obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
+obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
diff --git a/drivers/iio/light/tsl4531.c b/drivers/iio/light/tsl4531.c
new file mode 100644
index 0000000..a15006e
--- /dev/null
+++ b/drivers/iio/light/tsl4531.c
@@ -0,0 +1,258 @@
+/*
+ * tsl4531.c - Support for TAOS TSL4531 ambient light sensor
+ *
+ * Copyright 2013 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * IIO driver for the TSL4531x family
+ *   TSL45311/TSL45313: 7-bit I2C slave address 0x39
+ *   TSL45315/TSL45317: 7-bit I2C slave address 0x29
+ *
+ * TODO: single cycle measurement
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define TSL4531_DRV_NAME "tsl4531"
+
+#define TCS3472_COMMAND BIT(7)
+
+#define TSL4531_CONTROL (TCS3472_COMMAND | 0x00)
+#define TSL4531_CONFIG (TCS3472_COMMAND | 0x01)
+#define TSL4531_DATA (TCS3472_COMMAND | 0x04)
+#define TSL4531_ID (TCS3472_COMMAND | 0x0a)
+
+/* operating modes in control register */
+#define TSL4531_MODE_POWERDOWN 0x00
+#define TSL4531_MODE_SINGLE_ADC 0x02
+#define TSL4531_MODE_NORMAL 0x03
+
+/* integration time control in config register */
+#define TSL4531_TCNTRL_400MS 0x00
+#define TSL4531_TCNTRL_200MS 0x01
+#define TSL4531_TCNTRL_100MS 0x02
+
+/* part number in id register */
+#define TSL45311_ID 0x8
+#define TSL45313_ID 0x9
+#define TSL45315_ID 0xa
+#define TSL45317_ID 0xb
+#define TSL4531_ID_SHIFT 4
+
+struct tsl4531_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	int int_time;
+};
+
+static IIO_CONST_ATTR_INT_TIME_AVAIL("0.1 0.2 0.4");
+
+static struct attribute *tsl4531_attributes[] = {
+	&iio_const_attr_integration_time_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tsl4531_attribute_group = {
+	.attrs = tsl4531_attributes,
+};
+
+static const struct iio_chan_spec tsl4531_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_INT_TIME)
+	}
+};
+
+static int tsl4531_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct tsl4531_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = i2c_smbus_read_word_data(data->client,
+			TSL4531_DATA);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* 0.. 1x, 1 .. 2x, 2 .. 4x */
+		*val = 1 << data->int_time;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (data->int_time == 0)
+			*val2 = 400000;
+		else if (data->int_time == 1)
+			*val2 = 200000;
+		else if (data->int_time == 2)
+			*val2 = 100000;
+		else
+			return -EINVAL;
+		*val = 0;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int tsl4531_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct tsl4531_data *data = iio_priv(indio_dev);
+	int int_time, ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0)
+			return -EINVAL;
+		if (val2 == 400000)
+			int_time = 0;
+		else if (val2 == 200000)
+			int_time = 1;
+		else if (val2 == 100000)
+			int_time = 2;
+		else
+			return -EINVAL;
+		mutex_lock(&data->lock);
+		ret = i2c_smbus_write_byte_data(data->client,
+			TSL4531_CONFIG, int_time);
+		if (ret >= 0)
+			data->int_time = int_time;
+		mutex_unlock(&data->lock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info tsl4531_info = {
+	.read_raw = tsl4531_read_raw,
+	.write_raw = tsl4531_write_raw,
+	.attrs = &tsl4531_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int tsl4531_check_id(struct i2c_client *client)
+{
+	int ret = i2c_smbus_read_byte_data(client, TSL4531_ID);
+	if (ret < 0)
+		return ret;
+
+	switch (ret >> TSL4531_ID_SHIFT) {
+	case TSL45311_ID:
+	case TSL45313_ID:
+	case TSL45315_ID:
+	case TSL45317_ID:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int tsl4531_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	struct tsl4531_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	mutex_init(&data->lock);
+
+	if (!tsl4531_check_id(client)) {
+		dev_err(&client->dev, "no TSL4531 sensor\n");
+		return -ENODEV;
+	}
+
+	ret = i2c_smbus_write_byte_data(data->client, TSL4531_CONTROL,
+		TSL4531_MODE_NORMAL);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_smbus_write_byte_data(data->client, TSL4531_CONFIG,
+		TSL4531_TCNTRL_400MS);
+	if (ret < 0)
+		return ret;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tsl4531_info;
+	indio_dev->channels = tsl4531_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tsl4531_channels);
+	indio_dev->name = TSL4531_DRV_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return iio_device_register(indio_dev);
+}
+
+static int tsl4531_powerdown(struct i2c_client *client)
+{
+	return i2c_smbus_write_byte_data(client, TSL4531_CONTROL,
+		TSL4531_MODE_POWERDOWN);
+}
+
+static int tsl4531_remove(struct i2c_client *client)
+{
+	iio_device_unregister(i2c_get_clientdata(client));
+	tsl4531_powerdown(client);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tsl4531_suspend(struct device *dev)
+{
+	return tsl4531_powerdown(to_i2c_client(dev));
+}
+
+static int tsl4531_resume(struct device *dev)
+{
+	return i2c_smbus_write_byte_data(to_i2c_client(dev), TSL4531_CONTROL,
+		TSL4531_MODE_NORMAL);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(tsl4531_pm_ops, tsl4531_suspend, tsl4531_resume);
+
+static const struct i2c_device_id tsl4531_id[] = {
+	{ "tsl4531", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tsl4531_id);
+
+static struct i2c_driver tsl4531_driver = {
+	.driver = {
+		.name   = TSL4531_DRV_NAME,
+		.pm	= &tsl4531_pm_ops,
+		.owner  = THIS_MODULE,
+	},
+	.probe  = tsl4531_probe,
+	.remove = tsl4531_remove,
+	.id_table = tsl4531_id,
+};
+
+module_i2c_driver(tsl4531_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("TAOS TSL4531 ambient light sensors driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

Re: [PATCH 3/4] iio: Add tsl4531 ambient light sensor driver

[Jonathan Cameron]

On 09/08/13 16:20, Peter Meerwald wrote:
> driver for the TSL4531 family of 16-bit I2C ambient light
> sensors; information is here:
> http://www.ams.com/eng/Products/Light-Sensors/Ambient-Light-Sensor-ALS/TSL45315
>
> the chip offers simple lux output
>
> v3 (thanks Lars-Peter Clausen):
> * add mutex to when updating integration time
> * fix chip ID checking
> * code cleanups
> v2:
> * rename to tsl4351
> * use INT_TIME
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Jon Brenner <jon.brenner@ams.com>
Clean, concise and easy to review.  This is the sort of driver
I like!  To be fair you were assisted here by the hardware designers
making a device that fits remarkably well with our interfaces ;)

Applied to the togreg branch of iio.git.

One little question, what is 'single cycle measurement' referring to in the TODO note?

Thanks,

Jonathan

Re: [PATCH 3/4] iio: Add tsl4531 ambient light sensor driver

[Peter Meerwald]

> > driver for the TSL4531 family of 16-bit I2C ambient light
> > sensors; information is here:
> > http://www.ams.com/eng/Products/Light-Sensors/Ambient-Light-Sensor-ALS/TSL45315

> Applied to the togreg branch of iio.git.

there may be your Signed-off-by: missing

> One little question, what is 'single cycle measurement' referring to in the TODO note?

the chip can run in continuous measurement mode and one-shot measurement 
mode; the driver implements continuous measurement mode (aka normal mode) 
thus wasting energy

it is not clear how to know when the measurement is complete in one-shot 
mode, hence I'm sticking with continuous mode

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 3/4] iio: Add tsl4531 ambient light sensor driver

[Jonathan Cameron]

On 09/14/13 20:13, Peter Meerwald wrote:
> 
>>> driver for the TSL4531 family of 16-bit I2C ambient light
>>> sensors; information is here:
>>> http://www.ams.com/eng/Products/Light-Sensors/Ambient-Light-Sensor-ALS/TSL45315
> 
>> Applied to the togreg branch of iio.git.
> 
> there may be your Signed-off-by: missing
Gah!  Fixed and thanks for pointing that out.
> 
>> One little question, what is 'single cycle measurement' referring to in the TODO note?
> 
> the chip can run in continuous measurement mode and one-shot measurement 
> mode; the driver implements continuous measurement mode (aka normal mode) 
> thus wasting energy
> 
> it is not clear how to know when the measurement is complete in one-shot 
> mode, hence I'm sticking with continuous mode
>
Thanks, perhaps Jon can dig out some details on that when he has a few mins.

Jonathan
> p.
> 

[PATCH 4/4] iio: Add tcs3472 color light sensor driver

[Peter Meerwald]

chip has four 16-bit channels for red, green, blue, clear color
intensity; driver supports the TCS3x7x family of devices and was
tested with a TCS34725 chip; further information here:
http://www.ams.com/eng/Products/Light-Sensors/Color-Sensor/TCS34725

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Jon Brenner <jon.brenner@ams.com>
---
 drivers/iio/light/Kconfig   |   10 ++
 drivers/iio/light/Makefile  |    1 +
 drivers/iio/light/tcs3472.c |  373 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 384 insertions(+)
 create mode 100644 drivers/iio/light/tcs3472.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 488fff7..e73a1ab 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -55,6 +55,16 @@ config SENSORS_LM3533
 	  changes. The ALS-control output values can be set per zone for the
 	  three current output channels.
 
+config TCS3472
+	tristate "TAOS TCS3472 color light-to-digital converter"
+	depends on I2C
+	help
+	 If you say yes here you get support for the TAOS TCS3472
+	 family of color light-to-digital converters with IR filter.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called tcs3472.
+
 config SENSORS_TSL2563
 	tristate "TAOS TSL2560, TSL2561, TSL2562 and TSL2563 ambient light sensors"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index cd6f3cf..fb31140 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
 obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
+obj-$(CONFIG_TCS3472)		+= tcs3472.o
 obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
new file mode 100644
index 0000000..ed94b42
--- /dev/null
+++ b/drivers/iio/light/tcs3472.c
@@ -0,0 +1,373 @@
+/*
+ * tcs3472.c - Support for TAOS TCS3472 color light-to-digital converter
+ *
+ * Copyright (c) 2013 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Color light sensor with 16-bit channels for red, green, blue, clear);
+ * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725,
+ * TCS34727)
+ *
+ * TODO: interrupt support, thresholds, wait time
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define TCS3472_DRV_NAME "tcs3472"
+
+#define TCS3472_COMMAND BIT(7)
+#define TCS3472_AUTO_INCR BIT(5)
+
+#define TCS3472_ENABLE (TCS3472_COMMAND | 0x00)
+#define TCS3472_ATIME (TCS3472_COMMAND | 0x01)
+#define TCS3472_WTIME (TCS3472_COMMAND | 0x03)
+#define TCS3472_AILT (TCS3472_COMMAND | 0x04)
+#define TCS3472_AIHT (TCS3472_COMMAND | 0x06)
+#define TCS3472_PERS (TCS3472_COMMAND | 0x0c)
+#define TCS3472_CONFIG (TCS3472_COMMAND | 0x0d)
+#define TCS3472_CONTROL (TCS3472_COMMAND | 0x0f)
+#define TCS3472_ID (TCS3472_COMMAND | 0x12)
+#define TCS3472_STATUS (TCS3472_COMMAND | 0x13)
+#define TCS3472_CDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x14)
+#define TCS3472_RDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x16)
+#define TCS3472_GDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x18)
+#define TCS3472_BDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x1a)
+
+#define TCS3472_STATUS_AVALID BIT(0)
+#define TCS3472_ENABLE_AEN BIT(1)
+#define TCS3472_ENABLE_PON BIT(0)
+#define TCS3472_CONTROL_AGAIN_MASK (BIT(0) | BIT(1))
+
+struct tcs3472_data {
+	struct i2c_client *client;
+	u8 enable;
+	u8 control;
+	u8 atime;
+	u16 *buffer;
+};
+
+/* 4 16-bit channels + timestamp = 16 bytes */
+#define TSL3472_BUFFER_SIZE 16
+
+#define TCS3472_CHANNEL(_color, _si, _addr) { \
+	.type = IIO_INTENSITY, \
+	.modified = 1, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
+		BIT(IIO_CHAN_INFO_INT_TIME), \
+	.channel2 = IIO_MOD_LIGHT_##_color, \
+	.address = _addr, \
+	.scan_index = _si, \
+	.scan_type = IIO_ST('u', 16, 16, 0), \
+}
+
+static const int tcs3472_agains[] = { 1, 4, 16, 60 };
+
+static const struct iio_chan_spec tcs3472_channels[] = {
+	TCS3472_CHANNEL(CLEAR, 0, TCS3472_CDATA),
+	TCS3472_CHANNEL(RED, 1, TCS3472_RDATA),
+	TCS3472_CHANNEL(GREEN, 2, TCS3472_GDATA),
+	TCS3472_CHANNEL(BLUE, 3, TCS3472_BDATA),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int tcs3472_req_data(struct tcs3472_data *data)
+{
+	int tries = 50;
+	int ret;
+
+	while (tries--) {
+		ret = i2c_smbus_read_byte_data(data->client, TCS3472_STATUS);
+		if (ret < 0)
+			return ret;
+		if (ret & TCS3472_STATUS_AVALID)
+			break;
+		msleep(20);
+	}
+
+	if (tries < 0) {
+		dev_err(&data->client->dev, "data not ready\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int tcs3472_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = tcs3472_req_data(data);
+		if (ret < 0)
+			return ret;
+		ret = i2c_smbus_read_word_data(data->client, chan->address);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*val = tcs3472_agains[data->control &
+			TCS3472_CONTROL_AGAIN_MASK];
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = (256 - data->atime) * 2400;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int tcs3472_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		for (i = 0; i < ARRAY_SIZE(tcs3472_agains); i++) {
+			if (val == tcs3472_agains[i]) {
+				data->control &= ~TCS3472_CONTROL_AGAIN_MASK;
+				data->control |= i;
+				return i2c_smbus_write_byte_data(
+					data->client, TCS3472_CONTROL,
+					data->control);
+			}
+		}
+		return -EINVAL;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0)
+			return -EINVAL;
+		for (i = 0; i < 256; i++) {
+			if (val2 == (256 - i) * 2400) {
+				data->atime = i;
+				return i2c_smbus_write_word_data(
+					data->client, TCS3472_ATIME,
+					data->atime);
+			}
+
+		}
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+
+static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	s64 time_ns = iio_get_time_ns();
+	int len = 0;
+	int i, j = 0;
+
+	int ret = tcs3472_req_data(data);
+	if (ret < 0)
+		goto done;
+
+	for_each_set_bit(i, indio_dev->active_scan_mask,
+		indio_dev->masklength) {
+		ret = i2c_smbus_read_word_data(data->client,
+			TCS3472_CDATA + 2*i);
+		if (ret < 0)
+			goto done;
+
+		data->buffer[j++] = ret;
+		len += 2;
+	}
+
+	if (indio_dev->scan_timestamp)
+		*(s64 *)((u8 *)data->buffer + ALIGN(len, sizeof(s64)))
+			= time_ns;
+	iio_push_to_buffers(indio_dev, (u8 *)data->buffer);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t tcs3472_show_int_time_available(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	ssize_t total_n = 0;
+	int i;
+
+	for (i = 1; i <= 256; i++) {
+		ssize_t n = sprintf(buf, "0.%06d ", 2400 * i);
+		buf += n;
+		total_n += n;
+	}
+	/* replace trailing space by newline */
+	buf[-1] = '\n';
+
+	return total_n;
+}
+
+static IIO_CONST_ATTR(calibscale_available, "1 4 16 60");
+static IIO_DEV_ATTR_INT_TIME_AVAIL(tcs3472_show_int_time_available);
+
+static struct attribute *tcs3472_attributes[] = {
+	&iio_const_attr_calibscale_available.dev_attr.attr,
+	&iio_dev_attr_integration_time_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tcs3472_attribute_group = {
+	.attrs = tcs3472_attributes,
+};
+
+static const struct iio_info tcs3472_info = {
+	.read_raw = tcs3472_read_raw,
+	.write_raw = tcs3472_write_raw,
+	.attrs = &tcs3472_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int tcs3472_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct tcs3472_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	data->buffer = devm_kzalloc(&indio_dev->dev, TSL3472_BUFFER_SIZE,
+		GFP_KERNEL);
+	if (data->buffer == NULL)
+		return -ENOMEM;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tcs3472_info;
+	indio_dev->name = TCS3472_DRV_NAME;
+	indio_dev->channels = tcs3472_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tcs3472_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ID);
+	if (ret < 0)
+		return ret;
+
+	if (ret == 0x44)
+		dev_info(&client->dev, "TCS34721/34725 found\n");
+	else if (ret == 0x4d)
+		dev_info(&client->dev, "TCS34723/34727 found\n");
+	else
+		return -ENODEV;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_CONTROL);
+	if (ret < 0)
+		return ret;
+	data->control = ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ATIME);
+	if (ret < 0)
+		return ret;
+	data->atime = ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ENABLE);
+	if (ret < 0)
+		return ret;
+
+	/* enable device */
+	data->enable = ret | TCS3472_ENABLE_PON | TCS3472_ENABLE_AEN;
+	ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+		tcs3472_trigger_handler, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto buffer_cleanup;
+
+	return 0;
+
+buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
+	return ret;
+}
+
+static int tcs3472_powerdown(struct tcs3472_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable & ~(TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
+}
+
+static int tcs3472_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	tcs3472_powerdown(iio_priv(indio_dev));
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tcs3472_suspend(struct device *dev)
+{
+	return tcs3472_powerdown(iio_priv(dev_to_iio_dev(dev)));
+}
+
+static int tcs3472_resume(struct device *dev)
+{
+	struct tcs3472_data *data = iio_priv(dev_to_iio_dev(dev));
+	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable | (TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(tcs3472_pm_ops, tcs3472_suspend, tcs3472_resume);
+
+static const struct i2c_device_id tcs3472_id[] = {
+	{ "tcs3472", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tcs3472_id);
+
+static struct i2c_driver tcs3472_driver = {
+	.driver = {
+		.name	= TCS3472_DRV_NAME,
+		.pm	= &tcs3472_pm_ops,
+		.owner	= THIS_MODULE,
+	},
+	.probe		= tcs3472_probe,
+	.remove		= tcs3472_remove,
+	.id_table	= tcs3472_id,
+};
+module_i2c_driver(tcs3472_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("TCS3472 color light sensors driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

Re: [PATCH 4/4] iio: Add tcs3472 color light sensor driver

[Jonathan Cameron]

On 09/08/13 16:20, Peter Meerwald wrote:
> chip has four 16-bit channels for red, green, blue, clear color
> intensity; driver supports the TCS3x7x family of devices and was
> tested with a TCS34725 chip; further information here:
> http://www.ams.com/eng/Products/Light-Sensors/Color-Sensor/TCS34725
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Jon Brenner <jon.brenner@ams.com>


A few trivial bits:

I2C does not have any alignment constraints on the buffers used (these
are handled in the i2c driver by using a bounce buffer if needed). That means
you can just embed you data->buffer as an array directly inside data.  Saves
an allocation and 2 lines of code, hence whilst nicer it can be in a follow
up patch if you like.

For long writes to sysfs buffers make sure you use snprintf.  It's entirely
possible your write will always fit (and indeed it should) but as a reviewer
I'm happier letting it go with the added protection that it won't overrun
the buffer.

Anyhow, must be all of 2 mins work for you to roll a new version of with these
trivial bits cleaned up. (I very nearly just made the changes and merged, but
they are just a tiny bit too invasive given I can't test!)

Another nice driver. Thanks Peter.

Jonathan
...
> +struct tcs3472_data {
> +	struct i2c_client *client;
> +	u8 enable;
> +	u8 control;
> +	u8 atime;
As mentioned below,
u16 buffer[8];
> +	u16 *buffer;
> +};
> +
...
> +static int tcs3472_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct tcs3472_data *data = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		for (i = 0; i < ARRAY_SIZE(tcs3472_agains); i++) {
Do you want to check val2 here as well just to confirm it is 0?
You could otherwise implement the write_raw_get_fmt callback to
specify that this is IIO_VAL_INT to let the front end handle
any unwanted decimals.
> +			if (val == tcs3472_agains[i]) {
> +				data->control &= ~TCS3472_CONTROL_AGAIN_MASK;
> +				data->control |= i;
> +				return i2c_smbus_write_byte_data(
> +					data->client, TCS3472_CONTROL,
> +					data->control);
> +			}
> +		}
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (val != 0)
> +			return -EINVAL;
> +		for (i = 0; i < 256; i++) {
> +			if (val2 == (256 - i) * 2400) {
> +				data->atime = i;
> +				return i2c_smbus_write_word_data(
> +					data->client, TCS3472_ATIME,
> +					data->atime);
> +			}
> +
> +		}
> +		return -EINVAL;
> +	}
> +	return -EINVAL;
> +}
...
> +static ssize_t tcs3472_show_int_time_available(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	ssize_t total_n = 0;
> +	int i;
> +
> +	for (i = 1; i <= 256; i++) {
> +		ssize_t n = sprintf(buf, "0.%06d ", 2400 * i);
Ouch, that is quite a long text string.  Handling this sort of available
case nicely has been on the todo list for ages.  I've been meaning to
at least get proposed docs on how to do this out there and it might happen
one day soon!  For this I'd propose something like 2400..2400..614400 to
indicate that it is even steps size 2400 between 2400 and 614400.  Right
now what you have is all the ABI allows unfortunately.

Snprintf limited to a page would certainly be wise here.

> +		buf += n;
> +		total_n += n;
> +	}
> +	/* replace trailing space by newline */
> +	buf[-1] = '\n';
> +
> +	return total_n;
> +}
> +
> +static IIO_CONST_ATTR(calibscale_available, "1 4 16 60");
> +static IIO_DEV_ATTR_INT_TIME_AVAIL(tcs3472_show_int_time_available);
> +
> +static struct attribute *tcs3472_attributes[] = {
> +	&iio_const_attr_calibscale_available.dev_attr.attr,
> +	&iio_dev_attr_integration_time_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group tcs3472_attribute_group = {
> +	.attrs = tcs3472_attributes,
> +};
> +
> +static const struct iio_info tcs3472_info = {
> +	.read_raw = tcs3472_read_raw,
> +	.write_raw = tcs3472_write_raw,
> +	.attrs = &tcs3472_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int tcs3472_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct tcs3472_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
I2C has no alignment requirements (and even if there were there are ways
of ensuring all is fine - see spi drivers). Hence just put the buffer array
directly inside data and lose this separate allocation.

> +	data->buffer = devm_kzalloc(&indio_dev->dev, TSL3472_BUFFER_SIZE,
> +		GFP_KERNEL);
> +	if (data->buffer == NULL)
> +		return -ENOMEM;
...

[PATCH v2] iio: Add tcs3472 color light sensor driver

[Peter Meerwald]

chip has four 16-bit channels for red, green, blue, clear color
intensity; driver supports the TCS3x7x family of devices and was
tested with a TCS34725 chip; further information here:
http://www.ams.com/eng/Products/Light-Sensors/Color-Sensor/TCS34725

v2 (thanks to Jonathan Cameron):
* drop dynamic buffer allocation, buffer is in tcs3472_data
* limit sysfs output to PAGE_SIZE
* check val2 == 0 when writing CALIBSCALE

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Jon Brenner <jon.brenner@ams.com>
---
 drivers/iio/light/Kconfig   |  10 ++
 drivers/iio/light/Makefile  |   1 +
 drivers/iio/light/tcs3472.c | 369 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 380 insertions(+)
 create mode 100644 drivers/iio/light/tcs3472.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 488fff7..e73a1ab 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -55,6 +55,16 @@ config SENSORS_LM3533
 	  changes. The ALS-control output values can be set per zone for the
 	  three current output channels.
 
+config TCS3472
+	tristate "TAOS TCS3472 color light-to-digital converter"
+	depends on I2C
+	help
+	 If you say yes here you get support for the TAOS TCS3472
+	 family of color light-to-digital converters with IR filter.
+
+	 This driver can also be built as a module.  If so, the module
+	 will be called tcs3472.
+
 config SENSORS_TSL2563
 	tristate "TAOS TSL2560, TSL2561, TSL2562 and TSL2563 ambient light sensors"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index cd6f3cf..fb31140 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
 obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
+obj-$(CONFIG_TCS3472)		+= tcs3472.o
 obj-$(CONFIG_TSL4531)		+= tsl4531.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
new file mode 100644
index 0000000..95510bd
--- /dev/null
+++ b/drivers/iio/light/tcs3472.c
@@ -0,0 +1,369 @@
+/*
+ * tcs3472.c - Support for TAOS TCS3472 color light-to-digital converter
+ *
+ * Copyright (c) 2013 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Color light sensor with 16-bit channels for red, green, blue, clear);
+ * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725,
+ * TCS34727)
+ *
+ * TODO: interrupt support, thresholds, wait time
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define TCS3472_DRV_NAME "tcs3472"
+
+#define TCS3472_COMMAND BIT(7)
+#define TCS3472_AUTO_INCR BIT(5)
+
+#define TCS3472_ENABLE (TCS3472_COMMAND | 0x00)
+#define TCS3472_ATIME (TCS3472_COMMAND | 0x01)
+#define TCS3472_WTIME (TCS3472_COMMAND | 0x03)
+#define TCS3472_AILT (TCS3472_COMMAND | 0x04)
+#define TCS3472_AIHT (TCS3472_COMMAND | 0x06)
+#define TCS3472_PERS (TCS3472_COMMAND | 0x0c)
+#define TCS3472_CONFIG (TCS3472_COMMAND | 0x0d)
+#define TCS3472_CONTROL (TCS3472_COMMAND | 0x0f)
+#define TCS3472_ID (TCS3472_COMMAND | 0x12)
+#define TCS3472_STATUS (TCS3472_COMMAND | 0x13)
+#define TCS3472_CDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x14)
+#define TCS3472_RDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x16)
+#define TCS3472_GDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x18)
+#define TCS3472_BDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x1a)
+
+#define TCS3472_STATUS_AVALID BIT(0)
+#define TCS3472_ENABLE_AEN BIT(1)
+#define TCS3472_ENABLE_PON BIT(0)
+#define TCS3472_CONTROL_AGAIN_MASK (BIT(0) | BIT(1))
+
+struct tcs3472_data {
+	struct i2c_client *client;
+	u8 enable;
+	u8 control;
+	u8 atime;
+	u16 buffer[4];
+};
+
+/* 4 16-bit channels + timestamp = 16 bytes */
+#define TSL3472_BUFFER_SIZE 16
+
+#define TCS3472_CHANNEL(_color, _si, _addr) { \
+	.type = IIO_INTENSITY, \
+	.modified = 1, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
+		BIT(IIO_CHAN_INFO_INT_TIME), \
+	.channel2 = IIO_MOD_LIGHT_##_color, \
+	.address = _addr, \
+	.scan_index = _si, \
+	.scan_type = IIO_ST('u', 16, 16, 0), \
+}
+
+static const int tcs3472_agains[] = { 1, 4, 16, 60 };
+
+static const struct iio_chan_spec tcs3472_channels[] = {
+	TCS3472_CHANNEL(CLEAR, 0, TCS3472_CDATA),
+	TCS3472_CHANNEL(RED, 1, TCS3472_RDATA),
+	TCS3472_CHANNEL(GREEN, 2, TCS3472_GDATA),
+	TCS3472_CHANNEL(BLUE, 3, TCS3472_BDATA),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int tcs3472_req_data(struct tcs3472_data *data)
+{
+	int tries = 50;
+	int ret;
+
+	while (tries--) {
+		ret = i2c_smbus_read_byte_data(data->client, TCS3472_STATUS);
+		if (ret < 0)
+			return ret;
+		if (ret & TCS3472_STATUS_AVALID)
+			break;
+		msleep(20);
+	}
+
+	if (tries < 0) {
+		dev_err(&data->client->dev, "data not ready\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int tcs3472_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = tcs3472_req_data(data);
+		if (ret < 0)
+			return ret;
+		ret = i2c_smbus_read_word_data(data->client, chan->address);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*val = tcs3472_agains[data->control &
+			TCS3472_CONTROL_AGAIN_MASK];
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = (256 - data->atime) * 2400;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+	return -EINVAL;
+}
+
+static int tcs3472_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (val2 != 0)
+			return -EINVAL;
+		for (i = 0; i < ARRAY_SIZE(tcs3472_agains); i++) {
+			if (val == tcs3472_agains[i]) {
+				data->control &= ~TCS3472_CONTROL_AGAIN_MASK;
+				data->control |= i;
+				return i2c_smbus_write_byte_data(
+					data->client, TCS3472_CONTROL,
+					data->control);
+			}
+		}
+		return -EINVAL;
+	case IIO_CHAN_INFO_INT_TIME:
+		if (val != 0)
+			return -EINVAL;
+		for (i = 0; i < 256; i++) {
+			if (val2 == (256 - i) * 2400) {
+				data->atime = i;
+				return i2c_smbus_write_word_data(
+					data->client, TCS3472_ATIME,
+					data->atime);
+			}
+
+		}
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+
+static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tcs3472_data *data = iio_priv(indio_dev);
+	int len = 0;
+	int i, j = 0;
+
+	int ret = tcs3472_req_data(data);
+	if (ret < 0)
+		goto done;
+
+	for_each_set_bit(i, indio_dev->active_scan_mask,
+		indio_dev->masklength) {
+		ret = i2c_smbus_read_word_data(data->client,
+			TCS3472_CDATA + 2*i);
+		if (ret < 0)
+			goto done;
+
+		data->buffer[j++] = ret;
+		len += 2;
+	}
+
+	if (indio_dev->scan_timestamp)
+		*(s64 *)((u8 *)data->buffer + ALIGN(len, sizeof(s64)))
+			= iio_get_time_ns();
+	iio_push_to_buffers(indio_dev, (u8 *)data->buffer);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static ssize_t tcs3472_show_int_time_available(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	size_t len = 0;
+	int i;
+
+	for (i = 1; i <= 256; i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06d ",
+			2400 * i);
+
+	/* replace trailing space by newline */
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static IIO_CONST_ATTR(calibscale_available, "1 4 16 60");
+static IIO_DEV_ATTR_INT_TIME_AVAIL(tcs3472_show_int_time_available);
+
+static struct attribute *tcs3472_attributes[] = {
+	&iio_const_attr_calibscale_available.dev_attr.attr,
+	&iio_dev_attr_integration_time_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tcs3472_attribute_group = {
+	.attrs = tcs3472_attributes,
+};
+
+static const struct iio_info tcs3472_info = {
+	.read_raw = tcs3472_read_raw,
+	.write_raw = tcs3472_write_raw,
+	.attrs = &tcs3472_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int tcs3472_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct tcs3472_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &tcs3472_info;
+	indio_dev->name = TCS3472_DRV_NAME;
+	indio_dev->channels = tcs3472_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tcs3472_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ID);
+	if (ret < 0)
+		return ret;
+
+	if (ret == 0x44)
+		dev_info(&client->dev, "TCS34721/34725 found\n");
+	else if (ret == 0x4d)
+		dev_info(&client->dev, "TCS34723/34727 found\n");
+	else
+		return -ENODEV;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_CONTROL);
+	if (ret < 0)
+		return ret;
+	data->control = ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ATIME);
+	if (ret < 0)
+		return ret;
+	data->atime = ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ENABLE);
+	if (ret < 0)
+		return ret;
+
+	/* enable device */
+	data->enable = ret | TCS3472_ENABLE_PON | TCS3472_ENABLE_AEN;
+	ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+		tcs3472_trigger_handler, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto buffer_cleanup;
+
+	return 0;
+
+buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
+	return ret;
+}
+
+static int tcs3472_powerdown(struct tcs3472_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable & ~(TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
+}
+
+static int tcs3472_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	tcs3472_powerdown(iio_priv(indio_dev));
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tcs3472_suspend(struct device *dev)
+{
+	return tcs3472_powerdown(iio_priv(dev_to_iio_dev(dev)));
+}
+
+static int tcs3472_resume(struct device *dev)
+{
+	struct tcs3472_data *data = iio_priv(dev_to_iio_dev(dev));
+	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
+		data->enable | (TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(tcs3472_pm_ops, tcs3472_suspend, tcs3472_resume);
+
+static const struct i2c_device_id tcs3472_id[] = {
+	{ "tcs3472", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tcs3472_id);
+
+static struct i2c_driver tcs3472_driver = {
+	.driver = {
+		.name	= TCS3472_DRV_NAME,
+		.pm	= &tcs3472_pm_ops,
+		.owner	= THIS_MODULE,
+	},
+	.probe		= tcs3472_probe,
+	.remove		= tcs3472_remove,
+	.id_table	= tcs3472_id,
+};
+module_i2c_driver(tcs3472_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("TCS3472 color light sensors driver");
+MODULE_LICENSE("GPL");
-- 
1.8.4

Re: [PATCH v2] iio: Add tcs3472 color light sensor driver

[Jonathan Cameron]

On 09/14/13 20:38, Peter Meerwald wrote:
> chip has four 16-bit channels for red, green, blue, clear color
> intensity; driver supports the TCS3x7x family of devices and was
> tested with a TCS34725 chip; further information here:
> http://www.ams.com/eng/Products/Light-Sensors/Color-Sensor/TCS34725
> 
> v2 (thanks to Jonathan Cameron):
> * drop dynamic buffer allocation, buffer is in tcs3472_data
> * limit sysfs output to PAGE_SIZE
> * check val2 == 0 when writing CALIBSCALE
> 
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Jon Brenner <jon.brenner@ams.com>
Applied to the togreg branch of iio.git

Thanks

Jonathan
> ---
>  drivers/iio/light/Kconfig   |  10 ++
>  drivers/iio/light/Makefile  |   1 +
>  drivers/iio/light/tcs3472.c | 369 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 380 insertions(+)
>  create mode 100644 drivers/iio/light/tcs3472.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 488fff7..e73a1ab 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -55,6 +55,16 @@ config SENSORS_LM3533
>  	  changes. The ALS-control output values can be set per zone for the
>  	  three current output channels.
>  
> +config TCS3472
> +	tristate "TAOS TCS3472 color light-to-digital converter"
> +	depends on I2C
> +	help
> +	 If you say yes here you get support for the TAOS TCS3472
> +	 family of color light-to-digital converters with IR filter.
> +
> +	 This driver can also be built as a module.  If so, the module
> +	 will be called tcs3472.
> +
>  config SENSORS_TSL2563
>  	tristate "TAOS TSL2560, TSL2561, TSL2562 and TSL2563 ambient light sensors"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index cd6f3cf..fb31140 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -8,5 +8,6 @@ obj-$(CONFIG_APDS9300)		+= apds9300.o
>  obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
>  obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
>  obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
> +obj-$(CONFIG_TCS3472)		+= tcs3472.o
>  obj-$(CONFIG_TSL4531)		+= tsl4531.o
>  obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
> diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
> new file mode 100644
> index 0000000..95510bd
> --- /dev/null
> +++ b/drivers/iio/light/tcs3472.c
> @@ -0,0 +1,369 @@
> +/*
> + * tcs3472.c - Support for TAOS TCS3472 color light-to-digital converter
> + *
> + * Copyright (c) 2013 Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Color light sensor with 16-bit channels for red, green, blue, clear);
> + * 7-bit I2C slave address 0x39 (TCS34721, TCS34723) or 0x29 (TCS34725,
> + * TCS34727)
> + *
> + * TODO: interrupt support, thresholds, wait time
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/pm.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +
> +#define TCS3472_DRV_NAME "tcs3472"
> +
> +#define TCS3472_COMMAND BIT(7)
> +#define TCS3472_AUTO_INCR BIT(5)
> +
> +#define TCS3472_ENABLE (TCS3472_COMMAND | 0x00)
> +#define TCS3472_ATIME (TCS3472_COMMAND | 0x01)
> +#define TCS3472_WTIME (TCS3472_COMMAND | 0x03)
> +#define TCS3472_AILT (TCS3472_COMMAND | 0x04)
> +#define TCS3472_AIHT (TCS3472_COMMAND | 0x06)
> +#define TCS3472_PERS (TCS3472_COMMAND | 0x0c)
> +#define TCS3472_CONFIG (TCS3472_COMMAND | 0x0d)
> +#define TCS3472_CONTROL (TCS3472_COMMAND | 0x0f)
> +#define TCS3472_ID (TCS3472_COMMAND | 0x12)
> +#define TCS3472_STATUS (TCS3472_COMMAND | 0x13)
> +#define TCS3472_CDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x14)
> +#define TCS3472_RDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x16)
> +#define TCS3472_GDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x18)
> +#define TCS3472_BDATA (TCS3472_COMMAND | TCS3472_AUTO_INCR | 0x1a)
> +
> +#define TCS3472_STATUS_AVALID BIT(0)
> +#define TCS3472_ENABLE_AEN BIT(1)
> +#define TCS3472_ENABLE_PON BIT(0)
> +#define TCS3472_CONTROL_AGAIN_MASK (BIT(0) | BIT(1))
> +
> +struct tcs3472_data {
> +	struct i2c_client *client;
> +	u8 enable;
> +	u8 control;
> +	u8 atime;
> +	u16 buffer[4];
> +};
> +
> +/* 4 16-bit channels + timestamp = 16 bytes */
> +#define TSL3472_BUFFER_SIZE 16
> +
> +#define TCS3472_CHANNEL(_color, _si, _addr) { \
> +	.type = IIO_INTENSITY, \
> +	.modified = 1, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> +		BIT(IIO_CHAN_INFO_INT_TIME), \
> +	.channel2 = IIO_MOD_LIGHT_##_color, \
> +	.address = _addr, \
> +	.scan_index = _si, \
> +	.scan_type = IIO_ST('u', 16, 16, 0), \
> +}
> +
> +static const int tcs3472_agains[] = { 1, 4, 16, 60 };
> +
> +static const struct iio_chan_spec tcs3472_channels[] = {
> +	TCS3472_CHANNEL(CLEAR, 0, TCS3472_CDATA),
> +	TCS3472_CHANNEL(RED, 1, TCS3472_RDATA),
> +	TCS3472_CHANNEL(GREEN, 2, TCS3472_GDATA),
> +	TCS3472_CHANNEL(BLUE, 3, TCS3472_BDATA),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int tcs3472_req_data(struct tcs3472_data *data)
> +{
> +	int tries = 50;
> +	int ret;
> +
> +	while (tries--) {
> +		ret = i2c_smbus_read_byte_data(data->client, TCS3472_STATUS);
> +		if (ret < 0)
> +			return ret;
> +		if (ret & TCS3472_STATUS_AVALID)
> +			break;
> +		msleep(20);
> +	}
> +
> +	if (tries < 0) {
> +		dev_err(&data->client->dev, "data not ready\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tcs3472_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct tcs3472_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = tcs3472_req_data(data);
> +		if (ret < 0)
> +			return ret;
> +		ret = i2c_smbus_read_word_data(data->client, chan->address);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		*val = tcs3472_agains[data->control &
> +			TCS3472_CONTROL_AGAIN_MASK];
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = 0;
> +		*val2 = (256 - data->atime) * 2400;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int tcs3472_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int val, int val2, long mask)
> +{
> +	struct tcs3472_data *data = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (val2 != 0)
> +			return -EINVAL;
> +		for (i = 0; i < ARRAY_SIZE(tcs3472_agains); i++) {
> +			if (val == tcs3472_agains[i]) {
> +				data->control &= ~TCS3472_CONTROL_AGAIN_MASK;
> +				data->control |= i;
> +				return i2c_smbus_write_byte_data(
> +					data->client, TCS3472_CONTROL,
> +					data->control);
> +			}
> +		}
> +		return -EINVAL;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (val != 0)
> +			return -EINVAL;
> +		for (i = 0; i < 256; i++) {
> +			if (val2 == (256 - i) * 2400) {
> +				data->atime = i;
> +				return i2c_smbus_write_word_data(
> +					data->client, TCS3472_ATIME,
> +					data->atime);
> +			}
> +
> +		}
> +		return -EINVAL;
> +	}
> +	return -EINVAL;
> +}
> +
> +static irqreturn_t tcs3472_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct tcs3472_data *data = iio_priv(indio_dev);
> +	int len = 0;
> +	int i, j = 0;
> +
> +	int ret = tcs3472_req_data(data);
> +	if (ret < 0)
> +		goto done;
> +
> +	for_each_set_bit(i, indio_dev->active_scan_mask,
> +		indio_dev->masklength) {
> +		ret = i2c_smbus_read_word_data(data->client,
> +			TCS3472_CDATA + 2*i);
> +		if (ret < 0)
> +			goto done;
> +
> +		data->buffer[j++] = ret;
> +		len += 2;
> +	}
> +
> +	if (indio_dev->scan_timestamp)
> +		*(s64 *)((u8 *)data->buffer + ALIGN(len, sizeof(s64)))
> +			= iio_get_time_ns();
> +	iio_push_to_buffers(indio_dev, (u8 *)data->buffer);
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static ssize_t tcs3472_show_int_time_available(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	size_t len = 0;
> +	int i;
> +
> +	for (i = 1; i <= 256; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06d ",
> +			2400 * i);
> +
> +	/* replace trailing space by newline */
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static IIO_CONST_ATTR(calibscale_available, "1 4 16 60");
> +static IIO_DEV_ATTR_INT_TIME_AVAIL(tcs3472_show_int_time_available);
> +
> +static struct attribute *tcs3472_attributes[] = {
> +	&iio_const_attr_calibscale_available.dev_attr.attr,
> +	&iio_dev_attr_integration_time_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group tcs3472_attribute_group = {
> +	.attrs = tcs3472_attributes,
> +};
> +
> +static const struct iio_info tcs3472_info = {
> +	.read_raw = tcs3472_read_raw,
> +	.write_raw = tcs3472_write_raw,
> +	.attrs = &tcs3472_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int tcs3472_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct tcs3472_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &tcs3472_info;
> +	indio_dev->name = TCS3472_DRV_NAME;
> +	indio_dev->channels = tcs3472_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tcs3472_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ID);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret == 0x44)
> +		dev_info(&client->dev, "TCS34721/34725 found\n");
> +	else if (ret == 0x4d)
> +		dev_info(&client->dev, "TCS34723/34727 found\n");
> +	else
> +		return -ENODEV;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, TCS3472_CONTROL);
> +	if (ret < 0)
> +		return ret;
> +	data->control = ret;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ATIME);
> +	if (ret < 0)
> +		return ret;
> +	data->atime = ret;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, TCS3472_ENABLE);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* enable device */
> +	data->enable = ret | TCS3472_ENABLE_PON | TCS3472_ENABLE_AEN;
> +	ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
> +		data->enable);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +		tcs3472_trigger_handler, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto buffer_cleanup;
> +
> +	return 0;
> +
> +buffer_cleanup:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	return ret;
> +}
> +
> +static int tcs3472_powerdown(struct tcs3472_data *data)
> +{
> +	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
> +		data->enable & ~(TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
> +}
> +
> +static int tcs3472_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	tcs3472_powerdown(iio_priv(indio_dev));
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tcs3472_suspend(struct device *dev)
> +{
> +	return tcs3472_powerdown(iio_priv(dev_to_iio_dev(dev)));
> +}
> +
> +static int tcs3472_resume(struct device *dev)
> +{
> +	struct tcs3472_data *data = iio_priv(dev_to_iio_dev(dev));
> +	return i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
> +		data->enable | (TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON));
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(tcs3472_pm_ops, tcs3472_suspend, tcs3472_resume);
> +
> +static const struct i2c_device_id tcs3472_id[] = {
> +	{ "tcs3472", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, tcs3472_id);
> +
> +static struct i2c_driver tcs3472_driver = {
> +	.driver = {
> +		.name	= TCS3472_DRV_NAME,
> +		.pm	= &tcs3472_pm_ops,
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= tcs3472_probe,
> +	.remove		= tcs3472_remove,
> +	.id_table	= tcs3472_id,
> +};
> +module_i2c_driver(tcs3472_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("TCS3472 color light sensors driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH v2] iio: Add tcs3472 color light sensor driver

[Peter Meerwald]

Hi,

> > chip has four 16-bit channels for red, green, blue, clear color
> > intensity; driver supports the TCS3x7x family of devices and was
> > tested with a TCS34725 chip; further information here:
> > http://www.ams.com/eng/Products/Light-Sensors/Color-Sensor/TCS34725
> > 
> > v2 (thanks to Jonathan Cameron):
> > * drop dynamic buffer allocation, buffer is in tcs3472_data
> > * limit sysfs output to PAGE_SIZE
> > * check val2 == 0 when writing CALIBSCALE

> Applied to the togreg branch of iio.git

> > +struct tcs3472_data {
> > +	struct i2c_client *client;
> > +	u8 enable;
> > +	u8 control;
> > +	u8 atime;
> > +	u16 buffer[4];
> > +};
> > +
> > +/* 4 16-bit channels + timestamp = 16 bytes */
> > +#define TSL3472_BUFFER_SIZE 16

I messed this up, sorry
buffer must be of size 8 (hence 16 bytes) obviously;

patch following (removing the obsolete #define also)

thanks, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

[PATCH] iio: Fix tcs3472 buffer size

[Peter Meerwald]

and drop obsolete #define

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/light/tcs3472.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
index 95510bd..dc7d823 100644
--- a/drivers/iio/light/tcs3472.c
+++ b/drivers/iio/light/tcs3472.c
@@ -55,12 +55,9 @@ struct tcs3472_data {
 	u8 enable;
 	u8 control;
 	u8 atime;
-	u16 buffer[4];
+	u16 buffer[8]; /* 4 16-bit channels + 64-bit timestamp */
 };
 
-/* 4 16-bit channels + timestamp = 16 bytes */
-#define TSL3472_BUFFER_SIZE 16
-
 #define TCS3472_CHANNEL(_color, _si, _addr) { \
 	.type = IIO_INTENSITY, \
 	.modified = 1, \
-- 
1.8.4

Re: [PATCH] iio: Fix tcs3472 buffer size

[Jonathan Cameron]

On 09/15/13 16:52, Peter Meerwald wrote:
> and drop obsolete #define
> 
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Oops, applied to the togreg branch of iio.git

Thanks for the fast fix up.
> ---
>  drivers/iio/light/tcs3472.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c
> index 95510bd..dc7d823 100644
> --- a/drivers/iio/light/tcs3472.c
> +++ b/drivers/iio/light/tcs3472.c
> @@ -55,12 +55,9 @@ struct tcs3472_data {
>  	u8 enable;
>  	u8 control;
>  	u8 atime;
> -	u16 buffer[4];
> +	u16 buffer[8]; /* 4 16-bit channels + 64-bit timestamp */
>  };
>  
> -/* 4 16-bit channels + timestamp = 16 bytes */
> -#define TSL3472_BUFFER_SIZE 16
> -
>  #define TCS3472_CHANNEL(_color, _si, _addr) { \
>  	.type = IIO_INTENSITY, \
>  	.modified = 1, \
>