[RFC v1 0/4] iio: new chemical sensor framework and channel types

[RFC v1 0/4] iio: new chemical sensor framework and channel types

[Matt Ranostay]

Initial RFC for new chemical sensor framework, IIO_CONCENTRATION,
and IIO_RESISTANCE channel types.

Important notes:
* Not been tested on real hardware yet but that isn't the main RFC reason and
  once hardware is in hand it will be verified
* Reason the IIO_CONCENTRATION type isn't in percent but has modifiers for ppm
  and ppb is the scale value for the latter would cause a integer overflow
  using IIO_VAL_FRACTIONAL

Matt Ranostay (4):
  iio: chemical: Add IIO_CONCENTRATION channel type
  iio: resistance: add IIO_RESISTANCE channel type
  devicetree: add SGX Sensortech vendor id
  iio: chemical: add SGX VZ89x VOC sensor support

 Documentation/ABI/testing/sysfs-bus-iio            |  16 ++
 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/chemical/Makefile                      |   6 +
 drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
 drivers/iio/industrialio-core.c                    |   4 +
 include/uapi/linux/iio/types.h                     |   4 +
 9 files changed, 271 insertions(+)
 create mode 100644 drivers/iio/chemical/Makefile
 create mode 100644 drivers/iio/chemical/vz89x.c

-- 
1.9.1

[RFC v1 1/4] iio: chemical: Add IIO_CONCENTRATION channel type

[Matt Ranostay]

There are air quality sensors that report data back in parts per million
of VOC (Volatile Organic Compounds) which are usually indexed from CO2
or another common pollutant.

This patchset adds an IIO_CONCENTRATION type and IIO_MOD_PPM/PPB modifiers
because no other channels types fit this use case.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++
 drivers/iio/industrialio-core.c         |  3 +++
 include/uapi/linux/iio/types.h          |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 42d360f..a3803a1 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1459,3 +1459,13 @@ Description:
 		measurements and return the average value as output data. Each
 		value resulted from <type>[_name]_oversampling_ratio measurements
 		is considered as one sample for <type>[_name]_sampling_frequency.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppm_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppb_raw
+KernelVersion:	4.3
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Raw (unscaled no offset etc.) concentration reading of data like
+		CO2 or VOC (Volatile Organic Compounds) substances with or without
+		ppm (Part Per Million) or ppb (Parts Per Billion) channel modifiers.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index b3fcc2c..ea9e31a 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -75,6 +75,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_ENERGY] = "energy",
 	[IIO_DISTANCE] = "distance",
 	[IIO_VELOCITY] = "velocity",
+	[IIO_CONCENTRATION] = "concentration",
 };
 
 static const char * const iio_modifier_names[] = {
@@ -111,6 +112,8 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z] = "sqrt(x^2+y^2+z^2)",
 	[IIO_MOD_I] = "i",
 	[IIO_MOD_Q] = "q",
+	[IIO_MOD_PPM] = "ppm",
+	[IIO_MOD_PPB] = "ppb",
 };
 
 /* relies on pairs of these shared then separate */
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 2f8b117..dfb8b8c 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -35,6 +35,7 @@ enum iio_chan_type {
 	IIO_ENERGY,
 	IIO_DISTANCE,
 	IIO_VELOCITY,
+	IIO_CONCENTRATION,
 };
 
 enum iio_modifier {
@@ -72,6 +73,8 @@ enum iio_modifier {
 	IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
 	IIO_MOD_I,
 	IIO_MOD_Q,
+	IIO_MOD_PPM,
+	IIO_MOD_PPB,
 };
 
 enum iio_event_type {
-- 
1.9.1

[RFC v1 2/4] iio: resistance: add IIO_RESISTANCE channel type

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 6 ++++++
 drivers/iio/industrialio-core.c         | 1 +
 include/uapi/linux/iio/types.h          | 1 +
 3 files changed, 8 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a3803a1..1084659 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1469,3 +1469,9 @@ Description:
 		Raw (unscaled no offset etc.) concentration reading of data like
 		CO2 or VOC (Volatile Organic Compounds) substances with or without
 		ppm (Part Per Million) or ppb (Parts Per Billion) channel modifiers.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
+KernelVersion:	4.3
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Raw (unscaled no offset etc.) resistance reading in ohms.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index ea9e31a..862c066 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -76,6 +76,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_DISTANCE] = "distance",
 	[IIO_VELOCITY] = "velocity",
 	[IIO_CONCENTRATION] = "concentration",
+	[IIO_RESISTANCE] = "resistance",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index dfb8b8c..22edd4d 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -36,6 +36,7 @@ enum iio_chan_type {
 	IIO_DISTANCE,
 	IIO_VELOCITY,
 	IIO_CONCENTRATION,
+	IIO_RESISTANCE,
 };
 
 enum iio_modifier {
-- 
1.9.1

[RFC v1 3/4] devicetree: add SGX Sensortech vendor id

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index ac5f0c3..281e8f0 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -191,6 +191,7 @@ sbs	Smart Battery System
 schindler	Schindler
 seagate	Seagate Technology PLC
 semtech	Semtech Corporation
+sgx	SGX Sensortech
 sharp	Sharp Corporation
 sil	Silicon Image
 silabs	Silicon Laboratories
-- 
1.9.1

[RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Matt Ranostay]

Add support for VZ89X sensors VOC and CO2 reporting channels in
ppm/ppb units.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/chemical/Makefile                      |   6 +
 drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
 5 files changed, 246 insertions(+)
 create mode 100644 drivers/iio/chemical/Makefile
 create mode 100644 drivers/iio/chemical/vz89x.c

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index d77d412..a550216 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -88,6 +88,7 @@ ricoh,rs5c372b		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
 ricoh,rv5c386		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
 ricoh,rv5c387a		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
 samsung,24ad0xd1	S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
+sgx,vz89x		SGX Sensortech VZ89X Sensors
 sii,s35390a		2-wire CMOS real-time clock
 skyworks,sky81452	Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
 st-micro,24c256		i2c serial eeprom  (24cxx)
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 4011eff..9664e9c 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
 source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
+source "drivers/iio/chemical/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
 source "drivers/iio/frequency/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 698afc2..2288684 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
 obj-y += accel/
 obj-y += adc/
 obj-y += amplifiers/
+obj-y += chemical/
 obj-y += common/
 obj-y += dac/
 obj-y += gyro/
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
new file mode 100644
index 0000000..7292f2d
--- /dev/null
+++ b/drivers/iio/chemical/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO chemical sensors
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
new file mode 100644
index 0000000..a596a22
--- /dev/null
+++ b/drivers/iio/chemical/vz89x.c
@@ -0,0 +1,237 @@
+/*
+ * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
+ *
+ * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define VZ89X_REG_MEASUREMENT		0x09
+#define VZ89X_REG_MEASUREMENT_SIZE	6
+
+#define VZ89X_VOC_CO2_IDX	0
+#define VZ89X_VOC_SHORT_IDX	1
+#define VZ89X_VOC_TVOC_IDX	2
+#define VZ89X_RESISTANCE_IDX	3
+
+struct vz89x_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	unsigned long last_update;
+
+	u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
+};
+
+static const struct iio_chan_spec vz89x_channels[] = {
+	{
+		.type = IIO_CONCENTRATION,
+		.channel2 = IIO_MOD_PPM,
+		.modified = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.address = VZ89X_VOC_CO2_IDX,
+		.extend_name = "CO2",
+	},
+	{
+		.type = IIO_CONCENTRATION,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.address = VZ89X_VOC_SHORT_IDX,
+		.extend_name = "VOC_short",
+	},
+	{
+		.type = IIO_CONCENTRATION,
+		.channel2 = IIO_MOD_PPB,
+		.modified = 1,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.address = VZ89X_VOC_TVOC_IDX,
+		.extend_name = "tVOC",
+	},
+	{
+		.type = IIO_RESISTANCE,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.address = VZ89X_RESISTANCE_IDX,
+	},
+};
+
+static int vz89x_get_measurement(struct vz89x_data *data)
+{
+	int ret;
+	int i;
+
+	/* sensor can only be polled once a second max per datasheet */
+	if (!time_after(jiffies, data->last_update + HZ))
+		return 0;
+
+	ret = i2c_smbus_write_word_data(data->client,
+					VZ89X_REG_MEASUREMENT, 0);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
+		ret = i2c_smbus_read_byte(data->client);
+		if (ret < 0)
+			return ret;
+	}
+
+	data->last_update = jiffies;
+
+	return 0;
+}
+
+static int vz89x_get_resistance_reading(struct vz89x_data *data,
+					struct iio_chan_spec const *chan)
+{
+	u8 *buf = &data->buffer[chan->address];
+
+	return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
+}
+
+static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
+				   int *val, int *val2)
+{
+	int ret = -EINVAL;
+
+	switch (chan->address) {
+	case VZ89X_VOC_CO2_IDX:
+		*val = 1600;
+		*val2 = 229;
+		ret = IIO_VAL_FRACTIONAL;
+		break;
+	case VZ89X_VOC_SHORT_IDX:
+		*val = 0;
+		ret = IIO_VAL_INT;
+		break;
+	case VZ89X_VOC_TVOC_IDX:
+		*val = 1000;
+		*val2 = 229;
+		ret = IIO_VAL_FRACTIONAL;
+	}
+
+	return ret;
+}
+
+static int vz89x_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, int *val,
+			  int *val2, long mask)
+{
+	struct vz89x_data *data = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&data->lock);
+		ret = vz89x_get_measurement(data);
+
+		if (chan->type == IIO_RESISTANCE)
+			*val = vz89x_get_resistance_reading(data, chan);
+		else
+			*val = data->buffer[chan->address] - 13;
+
+		if (!ret)
+			ret = IIO_VAL_INT;
+
+		mutex_unlock(&data->lock);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_RESISTANCE:
+			*val = 10;
+			ret = IIO_VAL_INT;
+			break;
+		case IIO_CONCENTRATION:
+			ret = vz89x_get_channel_scale(chan, val, val2);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 400;
+		ret = IIO_VAL_INT;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info vz89x_info = {
+	.driver_module	= THIS_MODULE,
+	.read_raw	= vz89x_read_raw,
+};
+
+static int vz89x_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct vz89x_data *data;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
+				     I2C_FUNC_SMBUS_BYTE))
+		return -ENODEV;
+
+	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;
+	data->last_update = jiffies - HZ;
+	mutex_init(&data->lock);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &vz89x_info,
+	indio_dev->name = dev_name(&client->dev);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->channels = vz89x_channels;
+	indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id vz89x_id[] = {
+	{ "vz89x", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, vz89x_id);
+
+static const struct of_device_id vz89x_dt_ids[] = {
+	{ .compatible = "sgx,vz89x" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
+
+static struct i2c_driver vz89x_driver = {
+	.driver = {
+		.name	= "vz89x",
+		.of_match_table = of_match_ptr(vz89x_dt_ids),
+	},
+	.probe = vz89x_probe,
+	.id_table = vz89x_id,
+};
+module_i2c_driver(vz89x_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [RFC v1 1/4] iio: chemical: Add IIO_CONCENTRATION channel type

[Peter Meerwald]

> There are air quality sensors that report data back in parts per million
> of VOC (Volatile Organic Compounds) which are usually indexed from CO2
> or another common pollutant.
> 
> This patchset adds an IIO_CONCENTRATION type and IIO_MOD_PPM/PPB modifiers
> because no other channels types fit this use case.

comments below
can you add these to tools/iio/iio_event_monitor.c as well?
 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++
>  drivers/iio/industrialio-core.c         |  3 +++
>  include/uapi/linux/iio/types.h          |  3 +++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 42d360f..a3803a1 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1459,3 +1459,13 @@ Description:
>  		measurements and return the average value as output data. Each
>  		value resulted from <type>[_name]_oversampling_ratio measurements
>  		is considered as one sample for <type>[_name]_sampling_frequency.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppm_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppb_raw
> +KernelVersion:	4.3
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) concentration reading of data like
> +		CO2 or VOC (Volatile Organic Compounds) substances with or without
> +		ppm (Part Per Million) or ppb (Parts Per Billion) channel modifiers.

Part_s_ Per Million

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index b3fcc2c..ea9e31a 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -75,6 +75,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_ENERGY] = "energy",
>  	[IIO_DISTANCE] = "distance",
>  	[IIO_VELOCITY] = "velocity",
> +	[IIO_CONCENTRATION] = "concentration",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> @@ -111,6 +112,8 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z] = "sqrt(x^2+y^2+z^2)",
>  	[IIO_MOD_I] = "i",
>  	[IIO_MOD_Q] = "q",
> +	[IIO_MOD_PPM] = "ppm",
> +	[IIO_MOD_PPB] = "ppb",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 2f8b117..dfb8b8c 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -35,6 +35,7 @@ enum iio_chan_type {
>  	IIO_ENERGY,
>  	IIO_DISTANCE,
>  	IIO_VELOCITY,
> +	IIO_CONCENTRATION,
>  };
>  
>  enum iio_modifier {
> @@ -72,6 +73,8 @@ enum iio_modifier {
>  	IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
>  	IIO_MOD_I,
>  	IIO_MOD_Q,
> +	IIO_MOD_PPM,
> +	IIO_MOD_PPB,
>  };
>  
>  enum iio_event_type {
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Peter Meerwald]

> Add support for VZ89X sensors VOC and CO2 reporting channels in
> ppm/ppb units.

comments below
link to datasheet?

> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/chemical/Makefile                      |   6 +
>  drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
>  5 files changed, 246 insertions(+)
>  create mode 100644 drivers/iio/chemical/Makefile
>  create mode 100644 drivers/iio/chemical/vz89x.c
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index d77d412..a550216 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -88,6 +88,7 @@ ricoh,rs5c372b		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c386		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c387a		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  samsung,24ad0xd1	S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
> +sgx,vz89x		SGX Sensortech VZ89X Sensors
>  sii,s35390a		2-wire CMOS real-time clock
>  skyworks,sky81452	Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>  st-micro,24c256		i2c serial eeprom  (24cxx)
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 4011eff..9664e9c 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>  source "drivers/iio/accel/Kconfig"
>  source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/amplifiers/Kconfig"
> +source "drivers/iio/chemical/Kconfig"
>  source "drivers/iio/common/Kconfig"
>  source "drivers/iio/dac/Kconfig"
>  source "drivers/iio/frequency/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 698afc2..2288684 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>  obj-y += accel/
>  obj-y += adc/
>  obj-y += amplifiers/
> +obj-y += chemical/
>  obj-y += common/
>  obj-y += dac/
>  obj-y += gyro/
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> new file mode 100644
> index 0000000..7292f2d
> --- /dev/null
> +++ b/drivers/iio/chemical/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO chemical sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
> new file mode 100644
> index 0000000..a596a22
> --- /dev/null
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -0,0 +1,237 @@
> +/*
> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VZ89X_REG_MEASUREMENT		0x09
> +#define VZ89X_REG_MEASUREMENT_SIZE	6
> +
> +#define VZ89X_VOC_CO2_IDX	0
> +#define VZ89X_VOC_SHORT_IDX	1
> +#define VZ89X_VOC_TVOC_IDX	2
> +#define VZ89X_RESISTANCE_IDX	3
> +
> +struct vz89x_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	unsigned long last_update;
> +
> +	u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
> +};
> +
> +static const struct iio_chan_spec vz89x_channels[] = {
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.channel2 = IIO_MOD_PPM,
> +		.modified = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_CO2_IDX,
> +		.extend_name = "CO2",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_SHORT_IDX,
> +		.extend_name = "VOC_short",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.channel2 = IIO_MOD_PPB,
> +		.modified = 1,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_TVOC_IDX,
> +		.extend_name = "tVOC",
> +	},
> +	{
> +		.type = IIO_RESISTANCE,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_RESISTANCE_IDX,
> +	},
> +};
> +
> +static int vz89x_get_measurement(struct vz89x_data *data)
> +{
> +	int ret;
> +	int i;
> +
> +	/* sensor can only be polled once a second max per datasheet */
> +	if (!time_after(jiffies, data->last_update + HZ))
> +		return 0;
> +
> +	ret = i2c_smbus_write_word_data(data->client,
> +					VZ89X_REG_MEASUREMENT, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
> +		ret = i2c_smbus_read_byte(data->client);

could a block transfer be used?

> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	data->last_update = jiffies;
> +
> +	return 0;
> +}
> +
> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
> +					struct iio_chan_spec const *chan)
> +{
> +	u8 *buf = &data->buffer[chan->address];

I think only VZ89X_RESISTANCE_IDX as chan->address makes sense here?

> +
> +	return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);

why cast to u16? no cast is neccessary

> +}
> +
> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> +				   int *val, int *val2)
> +{
> +	int ret = -EINVAL;
> +
> +	switch (chan->address) {
> +	case VZ89X_VOC_CO2_IDX:
> +		*val = 1600;
> +		*val2 = 229;
> +		ret = IIO_VAL_FRACTIONAL;

return directly and save the break, no init of ret

> +		break;
> +	case VZ89X_VOC_SHORT_IDX:
> +		*val = 0;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case VZ89X_VOC_TVOC_IDX:
> +		*val = 1000;
> +		*val2 = 229;
> +		ret = IIO_VAL_FRACTIONAL;

default

> +	}
> +
> +	return ret;
> +}
> +
> +static int vz89x_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, int *val,
> +			  int *val2, long mask)
> +{
> +	struct vz89x_data *data = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = vz89x_get_measurement(data);
> +
> +		if (chan->type == IIO_RESISTANCE)
> +			*val = vz89x_get_resistance_reading(data, chan);

chan argument is not needed

> +		else
> +			*val = data->buffer[chan->address] - 13;

maybe have this block after checking ret, operating on invalid data isn't 
well behaved :)
what is 13? could this be incorporated in _OFFSET?

> +
> +		if (!ret)
> +			ret = IIO_VAL_INT;
> +
> +		mutex_unlock(&data->lock);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_RESISTANCE:
> +			*val = 10;
> +			ret = IIO_VAL_INT;
> +			break;
> +		case IIO_CONCENTRATION:
> +			ret = vz89x_get_channel_scale(chan, val, val2);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 400;
> +		ret = IIO_VAL_INT;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info vz89x_info = {
> +	.driver_module	= THIS_MODULE,
> +	.read_raw	= vz89x_read_raw,
> +};
> +
> +static int vz89x_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct iio_dev *indio_dev;
> +	struct vz89x_data *data;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_BYTE))
> +		return -ENODEV;
> +
> +	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;
> +	data->last_update = jiffies - HZ;
> +	mutex_init(&data->lock);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &vz89x_info,
> +	indio_dev->name = dev_name(&client->dev);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	indio_dev->channels = vz89x_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id vz89x_id[] = {
> +	{ "vz89x", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
> +
> +static const struct of_device_id vz89x_dt_ids[] = {
> +	{ .compatible = "sgx,vz89x" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
> +
> +static struct i2c_driver vz89x_driver = {
> +	.driver = {
> +		.name	= "vz89x",
> +		.of_match_table = of_match_ptr(vz89x_dt_ids),
> +	},
> +	.probe = vz89x_probe,
> +	.id_table = vz89x_id,
> +};
> +module_i2c_driver(vz89x_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Matt Ranostay]

Sent from my iPhone

> On Sep 5, 2015, at 00:45, Peter Meerwald <pmeerw@pmeerw.net> wrote:
> 
> 
>> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> ppm/ppb units.
> 
> comments below
> link to datasheet?

I will post the links when I am not at the Oregon coast. Have no laptop atm...

> 
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>> .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>> drivers/iio/Kconfig                                |   1 +
>> drivers/iio/Makefile                               |   1 +
>> drivers/iio/chemical/Makefile                      |   6 +
>> drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
>> 5 files changed, 246 insertions(+)
>> create mode 100644 drivers/iio/chemical/Makefile
>> create mode 100644 drivers/iio/chemical/vz89x.c
>> 
>> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> index d77d412..a550216 100644
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -88,6 +88,7 @@ ricoh,rs5c372b        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> ricoh,rv5c386        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> ricoh,rv5c387a        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> samsung,24ad0xd1    S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
>> +sgx,vz89x        SGX Sensortech VZ89X Sensors
>> sii,s35390a        2-wire CMOS real-time clock
>> skyworks,sky81452    Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>> st-micro,24c256        i2c serial eeprom  (24cxx)
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 4011eff..9664e9c 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>> source "drivers/iio/accel/Kconfig"
>> source "drivers/iio/adc/Kconfig"
>> source "drivers/iio/amplifiers/Kconfig"
>> +source "drivers/iio/chemical/Kconfig"
>> source "drivers/iio/common/Kconfig"
>> source "drivers/iio/dac/Kconfig"
>> source "drivers/iio/frequency/Kconfig"
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 698afc2..2288684 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>> obj-y += accel/
>> obj-y += adc/
>> obj-y += amplifiers/
>> +obj-y += chemical/
>> obj-y += common/
>> obj-y += dac/
>> obj-y += gyro/
>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> new file mode 100644
>> index 0000000..7292f2d
>> --- /dev/null
>> +++ b/drivers/iio/chemical/Makefile
>> @@ -0,0 +1,6 @@
>> +#
>> +# Makefile for IIO chemical sensors
>> +#
>> +
>> +# When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_VZ89X)        += vz89x.o
>> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
>> new file mode 100644
>> index 0000000..a596a22
>> --- /dev/null
>> +++ b/drivers/iio/chemical/vz89x.c
>> @@ -0,0 +1,237 @@
>> +/*
>> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
>> + *
>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#define VZ89X_REG_MEASUREMENT        0x09
>> +#define VZ89X_REG_MEASUREMENT_SIZE    6
>> +
>> +#define VZ89X_VOC_CO2_IDX    0
>> +#define VZ89X_VOC_SHORT_IDX    1
>> +#define VZ89X_VOC_TVOC_IDX    2
>> +#define VZ89X_RESISTANCE_IDX    3
>> +
>> +struct vz89x_data {
>> +    struct i2c_client *client;
>> +    struct mutex lock;
>> +    unsigned long last_update;
>> +
>> +    u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
>> +};
>> +
>> +static const struct iio_chan_spec vz89x_channels[] = {
>> +    {
>> +        .type = IIO_CONCENTRATION,
>> +        .channel2 = IIO_MOD_PPM,
>> +        .modified = 1,
>> +        .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +        .address = VZ89X_VOC_CO2_IDX,
>> +        .extend_name = "CO2",
>> +    },
>> +    {
>> +        .type = IIO_CONCENTRATION,
>> +        .info_mask_separate =
>> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +        .address = VZ89X_VOC_SHORT_IDX,
>> +        .extend_name = "VOC_short",
>> +    },
>> +    {
>> +        .type = IIO_CONCENTRATION,
>> +        .channel2 = IIO_MOD_PPB,
>> +        .modified = 1,
>> +        .info_mask_separate =
>> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +        .address = VZ89X_VOC_TVOC_IDX,
>> +        .extend_name = "tVOC",
>> +    },
>> +    {
>> +        .type = IIO_RESISTANCE,
>> +        .info_mask_separate =
>> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +        .address = VZ89X_RESISTANCE_IDX,
>> +    },
>> +};
>> +
>> +static int vz89x_get_measurement(struct vz89x_data *data)
>> +{
>> +    int ret;
>> +    int i;
>> +
>> +    /* sensor can only be polled once a second max per datasheet */
>> +    if (!time_after(jiffies, data->last_update + HZ))
>> +        return 0;
>> +
>> +    ret = i2c_smbus_write_word_data(data->client,
>> +                    VZ89X_REG_MEASUREMENT, 0);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
>> +        ret = i2c_smbus_read_byte(data->client);
> 
> could a block transfer be used?

Yah from the data sheet on i2c it can only do byte reads after the "command write data of zero"... I'll know better I have this part in hand.... But from Arduino hippies it seems I have gotten weird sensors to work on this chipset....


>> +        if (ret < 0)
>> +            return ret;
>> +    }
>> +
>> +    data->last_update = jiffies;
>> +
>> +    return 0;
>> +}
>> +
>> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
>> +                    struct iio_chan_spec const *chan)
>> +{
>> +    u8 *buf = &data->buffer[chan->address];
> 
> I think only VZ89X_RESISTANCE_IDX as chan->address makes sense here?
> 
>> +
>> +    return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
> 
> why cast to u16? no cast is neccessary
Maybe but to be safe of compilers not getting the shift left values that could happen doesn't hurt 

> 
>> +}
>> +
>> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> +                   int *val, int *val2)
>> +{
>> +    int ret = -EINVAL;
>> +
>> +    switch (chan->address) {
>> +    case VZ89X_VOC_CO2_IDX:
>> +        *val = 1600;
>> +        *val2 = 229;
>> +        ret = IIO_VAL_FRACTIONAL;
> 
> return directly and save the break, no init of ret

Old habits :)
> 
>> +        break;
>> +    case VZ89X_VOC_SHORT_IDX:
>> +        *val = 0;
>> +        ret = IIO_VAL_INT;
>> +        break;
>> +    case VZ89X_VOC_TVOC_IDX:
>> +        *val = 1000;
>> +        *val2 = 229;
>> +        ret = IIO_VAL_FRACTIONAL;
> 
> default
> 
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int vz89x_read_raw(struct iio_dev *indio_dev,
>> +              struct iio_chan_spec const *chan, int *val,
>> +              int *val2, long mask)
>> +{
>> +    struct vz89x_data *data = iio_priv(indio_dev);
>> +    int ret = -EINVAL;
>> +
>> +    switch (mask) {
>> +    case IIO_CHAN_INFO_RAW:
>> +        mutex_lock(&data->lock);
>> +        ret = vz89x_get_measurement(data);
>> +
>> +        if (chan->type == IIO_RESISTANCE)
>> +            *val = vz89x_get_resistance_reading(data, chan);
> 
> chan argument is not needed
> 
>> +        else
>> +            *val = data->buffer[chan->address] - 13;
> 
> maybe have this block after checking ret, operating on invalid data isn't 
> well behaved :)
> what is 13? could this be incorporated in _OFFSET?
> 

Yeah if scale value wasn't calculated first... Need to ask them why 13.. I mean really 


>> +
>> +        if (!ret)
>> +            ret = IIO_VAL_INT;
>> +
>> +        mutex_unlock(&data->lock);
>> +        break;
>> +    case IIO_CHAN_INFO_SCALE:
>> +        switch (chan->type) {
>> +        case IIO_RESISTANCE:
>> +            *val = 10;
>> +            ret = IIO_VAL_INT;
>> +            break;
>> +        case IIO_CONCENTRATION:
>> +            ret = vz89x_get_channel_scale(chan, val, val2);
>> +            break;
>> +        default:
>> +            ret = -EINVAL;
>> +        }
>> +        break;
>> +    case IIO_CHAN_INFO_OFFSET:
>> +        *val = 400;
>> +        ret = IIO_VAL_INT;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static const struct iio_info vz89x_info = {
>> +    .driver_module    = THIS_MODULE,
>> +    .read_raw    = vz89x_read_raw,
>> +};
>> +
>> +static int vz89x_probe(struct i2c_client *client,
>> +               const struct i2c_device_id *id)
>> +{
>> +    struct iio_dev *indio_dev;
>> +    struct vz89x_data *data;
>> +
>> +    if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
>> +                     I2C_FUNC_SMBUS_BYTE))
>> +        return -ENODEV;
>> +
>> +    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;
>> +    data->last_update = jiffies - HZ;
>> +    mutex_init(&data->lock);
>> +
>> +    indio_dev->dev.parent = &client->dev;
>> +    indio_dev->info = &vz89x_info,
>> +    indio_dev->name = dev_name(&client->dev);
>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +    indio_dev->channels = vz89x_channels;
>> +    indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
>> +
>> +    return devm_iio_device_register(&client->dev, indio_dev);
>> +}
>> +
>> +static const struct i2c_device_id vz89x_id[] = {
>> +    { "vz89x", 0 },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
>> +
>> +static const struct of_device_id vz89x_dt_ids[] = {
>> +    { .compatible = "sgx,vz89x" },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
>> +
>> +static struct i2c_driver vz89x_driver = {
>> +    .driver = {
>> +        .name    = "vz89x",
>> +        .of_match_table = of_match_ptr(vz89x_dt_ids),
>> +    },
>> +    .probe = vz89x_probe,
>> +    .id_table = vz89x_id,
>> +};
>> +module_i2c_driver(vz89x_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
>> +MODULE_LICENSE("GPL v2");
> 
> -- 
> 
> Peter Meerwald
> +43-664-2444418 (mobile)

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Daniel Baluta]

On Sat, Sep 5, 2015 at 8:53 AM, Matt Ranostay <mranostay@gmail.com> wrote:
> Add support for VZ89X sensors VOC and CO2 reporting channels in
> ppm/ppb units.

Adding more info on this would be great.

s/VOC/VOC(volatile organic compound)

Also, link to datasheet?

>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/chemical/Makefile                      |   6 +
>  drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++

Do you have a real part number for vz89x.c. Sometime ago, we agree to
avoid using generic part
numbers in files. So vz890 (if exists) is preferred over vz89x.c.

Is vz89x a family of devices? :)

>  5 files changed, 246 insertions(+)
>  create mode 100644 drivers/iio/chemical/Makefile
>  create mode 100644 drivers/iio/chemical/vz89x.c
>
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index d77d412..a550216 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -88,6 +88,7 @@ ricoh,rs5c372b                I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c386          I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c387a         I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  samsung,24ad0xd1       S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
> +sgx,vz89x              SGX Sensortech VZ89X Sensors

You should also CC devicetree people for this.

>  sii,s35390a            2-wire CMOS real-time clock
>  skyworks,sky81452      Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>  st-micro,24c256                i2c serial eeprom  (24cxx)
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 4011eff..9664e9c 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>  source "drivers/iio/accel/Kconfig"
>  source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/amplifiers/Kconfig"
> +source "drivers/iio/chemical/Kconfig"
>  source "drivers/iio/common/Kconfig"
>  source "drivers/iio/dac/Kconfig"
>  source "drivers/iio/frequency/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 698afc2..2288684 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>  obj-y += accel/
>  obj-y += adc/
>  obj-y += amplifiers/
> +obj-y += chemical/
>  obj-y += common/
>  obj-y += dac/
>  obj-y += gyro/
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> new file mode 100644
> index 0000000..7292f2d
> --- /dev/null
> +++ b/drivers/iio/chemical/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO chemical sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_VZ89X)            += vz89x.o
> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
> new file mode 100644
> index 0000000..a596a22
> --- /dev/null
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -0,0 +1,237 @@
> +/*
> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VZ89X_REG_MEASUREMENT          0x09
> +#define VZ89X_REG_MEASUREMENT_SIZE     6
> +
> +#define VZ89X_VOC_CO2_IDX      0
> +#define VZ89X_VOC_SHORT_IDX    1
> +#define VZ89X_VOC_TVOC_IDX     2
> +#define VZ89X_RESISTANCE_IDX   3
> +
> +struct vz89x_data {
> +       struct i2c_client *client;
> +       struct mutex lock;
> +       unsigned long last_update;
> +
> +       u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
> +};
> +
> +static const struct iio_chan_spec vz89x_channels[] = {
> +       {
> +               .type = IIO_CONCENTRATION,
> +               .channel2 = IIO_MOD_PPM,
> +               .modified = 1,
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +               .address = VZ89X_VOC_CO2_IDX,
> +               .extend_name = "CO2",
> +       },
> +       {
> +               .type = IIO_CONCENTRATION,
> +               .info_mask_separate =
> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +               .address = VZ89X_VOC_SHORT_IDX,
> +               .extend_name = "VOC_short",
> +       },
> +       {
> +               .type = IIO_CONCENTRATION,
> +               .channel2 = IIO_MOD_PPB,
> +               .modified = 1,
> +               .info_mask_separate =
> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +               .address = VZ89X_VOC_TVOC_IDX,
> +               .extend_name = "tVOC",
> +       },
> +       {
> +               .type = IIO_RESISTANCE,
> +               .info_mask_separate =
> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +               .address = VZ89X_RESISTANCE_IDX,
> +       },
> +};
> +
> +static int vz89x_get_measurement(struct vz89x_data *data)
> +{
> +       int ret;
> +       int i;
> +
> +       /* sensor can only be polled once a second max per datasheet */
> +       if (!time_after(jiffies, data->last_update + HZ))
> +               return 0;
> +
> +       ret = i2c_smbus_write_word_data(data->client,
> +                                       VZ89X_REG_MEASUREMENT, 0);
> +       if (ret < 0)
> +               return ret;
> +
> +       for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
> +               ret = i2c_smbus_read_byte(data->client);

i2c_smbus_block_read ?

> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       data->last_update = jiffies;
> +
> +       return 0;
> +}
> +
> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
> +                                       struct iio_chan_spec const *chan)
> +{
> +       u8 *buf = &data->buffer[chan->address];
> +
> +       return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
> +}
> +
> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> +                                  int *val, int *val2)
> +{
> +       int ret = -EINVAL;
> +
> +       switch (chan->address) {
> +       case VZ89X_VOC_CO2_IDX:
> +               *val = 1600;
> +               *val2 = 229;
> +               ret = IIO_VAL_FRACTIONAL;
> +               break;
> +       case VZ89X_VOC_SHORT_IDX:
> +               *val = 0;
> +               ret = IIO_VAL_INT;
> +               break;
> +       case VZ89X_VOC_TVOC_IDX:
> +               *val = 1000;
> +               *val2 = 229;
> +               ret = IIO_VAL_FRACTIONAL;
> +       }
> +
> +       return ret;
> +}
> +
> +static int vz89x_read_raw(struct iio_dev *indio_dev,
> +                         struct iio_chan_spec const *chan, int *val,
> +                         int *val2, long mask)
> +{
> +       struct vz89x_data *data = iio_priv(indio_dev);
> +       int ret = -EINVAL;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               mutex_lock(&data->lock);
> +               ret = vz89x_get_measurement(data);
> +
> +               if (chan->type == IIO_RESISTANCE)
> +                       *val = vz89x_get_resistance_reading(data, chan);
> +               else
> +                       *val = data->buffer[chan->address] - 13;
> +
> +               if (!ret)
> +                       ret = IIO_VAL_INT;
> +
> +               mutex_unlock(&data->lock);
> +               break;
> +       case IIO_CHAN_INFO_SCALE:
> +               switch (chan->type) {
> +               case IIO_RESISTANCE:
> +                       *val = 10;
> +                       ret = IIO_VAL_INT;
> +                       break;
> +               case IIO_CONCENTRATION:
> +                       ret = vz89x_get_channel_scale(chan, val, val2);
> +                       break;
> +               default:
> +                       ret = -EINVAL;
> +               }
> +               break;
> +       case IIO_CHAN_INFO_OFFSET:
> +               *val = 400;
> +               ret = IIO_VAL_INT;
> +               break;
> +       }
> +
> +       return ret;
> +}
> +
> +static const struct iio_info vz89x_info = {
> +       .driver_module  = THIS_MODULE,
> +       .read_raw       = vz89x_read_raw,
> +};
> +
> +static int vz89x_probe(struct i2c_client *client,
> +                      const struct i2c_device_id *id)
> +{
> +       struct iio_dev *indio_dev;
> +       struct vz89x_data *data;
> +
> +       if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> +                                    I2C_FUNC_SMBUS_BYTE))
> +               return -ENODEV;
> +
> +       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;
> +       data->last_update = jiffies - HZ;
> +       mutex_init(&data->lock);
> +
> +       indio_dev->dev.parent = &client->dev;
> +       indio_dev->info = &vz89x_info,
> +       indio_dev->name = dev_name(&client->dev);
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +       indio_dev->channels = vz89x_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
> +
> +       return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id vz89x_id[] = {
> +       { "vz89x", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
> +
> +static const struct of_device_id vz89x_dt_ids[] = {
> +       { .compatible = "sgx,vz89x" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
> +
> +static struct i2c_driver vz89x_driver = {
> +       .driver = {
> +               .name   = "vz89x",
> +               .of_match_table = of_match_ptr(vz89x_dt_ids),
> +       },
> +       .probe = vz89x_probe,
> +       .id_table = vz89x_id,
> +};
> +module_i2c_driver(vz89x_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC v1 3/4] devicetree: add SGX Sensortech vendor id

[Daniel Baluta]

don't forget to cc devicetree people.

On Sat, Sep 5, 2015 at 8:53 AM, Matt Ranostay <mranostay@gmail.com> wrote:
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index ac5f0c3..281e8f0 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -191,6 +191,7 @@ sbs Smart Battery System
>  schindler      Schindler
>  seagate        Seagate Technology PLC
>  semtech        Semtech Corporation
> +sgx    SGX Sensortech
>  sharp  Sharp Corporation
>  sil    Silicon Image
>  silabs Silicon Laboratories
> --
> 1.9.1
>
> --
> 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: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Peter Meerwald]

> >> Add support for VZ89X sensors VOC and CO2 reporting channels in
> >> ppm/ppb units.
> > 
> > comments below
> > link to datasheet?
> 
> I will post the links when I am not at the Oregon coast. Have no laptop atm...
> 
> > 
> >> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> >> ---
> >> .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
> >> drivers/iio/Kconfig                                |   1 +
> >> drivers/iio/Makefile                               |   1 +
> >> drivers/iio/chemical/Makefile                      |   6 +
> >> drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
> >> 5 files changed, 246 insertions(+)
> >> create mode 100644 drivers/iio/chemical/Makefile
> >> create mode 100644 drivers/iio/chemical/vz89x.c
> >> 
> >> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> index d77d412..a550216 100644
> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> >> @@ -88,6 +88,7 @@ ricoh,rs5c372b        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
> >> ricoh,rv5c386        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
> >> ricoh,rv5c387a        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
> >> samsung,24ad0xd1    S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
> >> +sgx,vz89x        SGX Sensortech VZ89X Sensors
> >> sii,s35390a        2-wire CMOS real-time clock
> >> skyworks,sky81452    Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
> >> st-micro,24c256        i2c serial eeprom  (24cxx)
> >> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> >> index 4011eff..9664e9c 100644
> >> --- a/drivers/iio/Kconfig
> >> +++ b/drivers/iio/Kconfig
> >> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
> >> source "drivers/iio/accel/Kconfig"
> >> source "drivers/iio/adc/Kconfig"
> >> source "drivers/iio/amplifiers/Kconfig"
> >> +source "drivers/iio/chemical/Kconfig"
> >> source "drivers/iio/common/Kconfig"
> >> source "drivers/iio/dac/Kconfig"
> >> source "drivers/iio/frequency/Kconfig"
> >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> >> index 698afc2..2288684 100644
> >> --- a/drivers/iio/Makefile
> >> +++ b/drivers/iio/Makefile
> >> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
> >> obj-y += accel/
> >> obj-y += adc/
> >> obj-y += amplifiers/
> >> +obj-y += chemical/
> >> obj-y += common/
> >> obj-y += dac/
> >> obj-y += gyro/
> >> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> >> new file mode 100644
> >> index 0000000..7292f2d
> >> --- /dev/null
> >> +++ b/drivers/iio/chemical/Makefile
> >> @@ -0,0 +1,6 @@
> >> +#
> >> +# Makefile for IIO chemical sensors
> >> +#
> >> +
> >> +# When adding new entries keep the list in alphabetical order
> >> +obj-$(CONFIG_VZ89X)        += vz89x.o
> >> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
> >> new file mode 100644
> >> index 0000000..a596a22
> >> --- /dev/null
> >> +++ b/drivers/iio/chemical/vz89x.c
> >> @@ -0,0 +1,237 @@
> >> +/*
> >> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
> >> + *
> >> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + */
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/mutex.h>
> >> +#include <linux/init.h>
> >> +#include <linux/i2c.h>
> >> +
> >> +#include <linux/iio/iio.h>
> >> +#include <linux/iio/sysfs.h>
> >> +
> >> +#define VZ89X_REG_MEASUREMENT        0x09
> >> +#define VZ89X_REG_MEASUREMENT_SIZE    6
> >> +
> >> +#define VZ89X_VOC_CO2_IDX    0
> >> +#define VZ89X_VOC_SHORT_IDX    1
> >> +#define VZ89X_VOC_TVOC_IDX    2
> >> +#define VZ89X_RESISTANCE_IDX    3
> >> +
> >> +struct vz89x_data {
> >> +    struct i2c_client *client;
> >> +    struct mutex lock;
> >> +    unsigned long last_update;
> >> +
> >> +    u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
> >> +};
> >> +
> >> +static const struct iio_chan_spec vz89x_channels[] = {
> >> +    {
> >> +        .type = IIO_CONCENTRATION,
> >> +        .channel2 = IIO_MOD_PPM,
> >> +        .modified = 1,
> >> +        .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >> +        .address = VZ89X_VOC_CO2_IDX,
> >> +        .extend_name = "CO2",
> >> +    },
> >> +    {
> >> +        .type = IIO_CONCENTRATION,
> >> +        .info_mask_separate =
> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >> +        .address = VZ89X_VOC_SHORT_IDX,
> >> +        .extend_name = "VOC_short",
> >> +    },
> >> +    {
> >> +        .type = IIO_CONCENTRATION,
> >> +        .channel2 = IIO_MOD_PPB,
> >> +        .modified = 1,
> >> +        .info_mask_separate =
> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >> +        .address = VZ89X_VOC_TVOC_IDX,
> >> +        .extend_name = "tVOC",
> >> +    },
> >> +    {
> >> +        .type = IIO_RESISTANCE,
> >> +        .info_mask_separate =
> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >> +        .address = VZ89X_RESISTANCE_IDX,
> >> +    },
> >> +};
> >> +
> >> +static int vz89x_get_measurement(struct vz89x_data *data)
> >> +{
> >> +    int ret;
> >> +    int i;
> >> +
> >> +    /* sensor can only be polled once a second max per datasheet */
> >> +    if (!time_after(jiffies, data->last_update + HZ))
> >> +        return 0;
> >> +
> >> +    ret = i2c_smbus_write_word_data(data->client,
> >> +                    VZ89X_REG_MEASUREMENT, 0);
> >> +    if (ret < 0)
> >> +        return ret;
> >> +
> >> +    for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
> >> +        ret = i2c_smbus_read_byte(data->client);
> > 
> > could a block transfer be used?
> 
> Yah from the data sheet on i2c it can only do byte reads after the 
> "command write data of zero"... I'll know better I have this part in 
> hand.... But from Arduino hippies it seems I have gotten weird sensors 
> to work on this chipset....

and the transfer is at most once per second, so no big performance deal
 
> 
> >> +        if (ret < 0)
> >> +            return ret;
> >> +    }
> >> +
> >> +    data->last_update = jiffies;
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
> >> +                    struct iio_chan_spec const *chan)
> >> +{
> >> +    u8 *buf = &data->buffer[chan->address];
> > 
> > I think only VZ89X_RESISTANCE_IDX as chan->address makes sense here?
> > 
> >> +
> >> +    return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
> > 
> > why cast to u16? no cast is neccessary
> Maybe but to be safe of compilers not getting the shift left values that could happen doesn't hurt 

(u16)buf[2] << 16
u16 still makes no sense
 
> > 
> >> +}
> >> +
> >> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> >> +                   int *val, int *val2)
> >> +{
> >> +    int ret = -EINVAL;
> >> +
> >> +    switch (chan->address) {
> >> +    case VZ89X_VOC_CO2_IDX:
> >> +        *val = 1600;
> >> +        *val2 = 229;
> >> +        ret = IIO_VAL_FRACTIONAL;
> > 
> > return directly and save the break, no init of ret
> 
> Old habits :)

agree, matter of taste; but in this case is saves somes lines
 
> >> +        break;
> >> +    case VZ89X_VOC_SHORT_IDX:
> >> +        *val = 0;
> >> +        ret = IIO_VAL_INT;
> >> +        break;
> >> +    case VZ89X_VOC_TVOC_IDX:
> >> +        *val = 1000;
> >> +        *val2 = 229;
> >> +        ret = IIO_VAL_FRACTIONAL;
> > 
> > default
> > 
> >> +    }
> >> +
> >> +    return ret;
> >> +}
> >> +
> >> +static int vz89x_read_raw(struct iio_dev *indio_dev,
> >> +              struct iio_chan_spec const *chan, int *val,
> >> +              int *val2, long mask)
> >> +{
> >> +    struct vz89x_data *data = iio_priv(indio_dev);
> >> +    int ret = -EINVAL;
> >> +
> >> +    switch (mask) {
> >> +    case IIO_CHAN_INFO_RAW:
> >> +        mutex_lock(&data->lock);
> >> +        ret = vz89x_get_measurement(data);
> >> +
> >> +        if (chan->type == IIO_RESISTANCE)
> >> +            *val = vz89x_get_resistance_reading(data, chan);
> > 
> > chan argument is not needed
> > 
> >> +        else
> >> +            *val = data->buffer[chan->address] - 13;
> > 
> > maybe have this block after checking ret, operating on invalid data isn't 
> > well behaved :)
> > what is 13? could this be incorporated in _OFFSET?
> > 
> 
> Yeah if scale value wasn't calculated first... Need to ask them why 13.. I mean really 
 
I always have to look is up as well: it is (value + offset) * scale
 
> >> +
> >> +        if (!ret)
> >> +            ret = IIO_VAL_INT;
> >> +
> >> +        mutex_unlock(&data->lock);
> >> +        break;
> >> +    case IIO_CHAN_INFO_SCALE:
> >> +        switch (chan->type) {
> >> +        case IIO_RESISTANCE:
> >> +            *val = 10;
> >> +            ret = IIO_VAL_INT;
> >> +            break;
> >> +        case IIO_CONCENTRATION:
> >> +            ret = vz89x_get_channel_scale(chan, val, val2);
> >> +            break;
> >> +        default:
> >> +            ret = -EINVAL;
> >> +        }
> >> +        break;
> >> +    case IIO_CHAN_INFO_OFFSET:
> >> +        *val = 400;
> >> +        ret = IIO_VAL_INT;
> >> +        break;
> >> +    }
> >> +
> >> +    return ret;
> >> +}
> >> +
> >> +static const struct iio_info vz89x_info = {
> >> +    .driver_module    = THIS_MODULE,
> >> +    .read_raw    = vz89x_read_raw,
> >> +};
> >> +
> >> +static int vz89x_probe(struct i2c_client *client,
> >> +               const struct i2c_device_id *id)
> >> +{
> >> +    struct iio_dev *indio_dev;
> >> +    struct vz89x_data *data;
> >> +
> >> +    if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> >> +                     I2C_FUNC_SMBUS_BYTE))
> >> +        return -ENODEV;
> >> +
> >> +    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;
> >> +    data->last_update = jiffies - HZ;
> >> +    mutex_init(&data->lock);
> >> +
> >> +    indio_dev->dev.parent = &client->dev;
> >> +    indio_dev->info = &vz89x_info,
> >> +    indio_dev->name = dev_name(&client->dev);
> >> +    indio_dev->modes = INDIO_DIRECT_MODE;
> >> +
> >> +    indio_dev->channels = vz89x_channels;
> >> +    indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
> >> +
> >> +    return devm_iio_device_register(&client->dev, indio_dev);
> >> +}
> >> +
> >> +static const struct i2c_device_id vz89x_id[] = {
> >> +    { "vz89x", 0 },
> >> +    { }
> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
> >> +
> >> +static const struct of_device_id vz89x_dt_ids[] = {
> >> +    { .compatible = "sgx,vz89x" },
> >> +    { }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
> >> +
> >> +static struct i2c_driver vz89x_driver = {
> >> +    .driver = {
> >> +        .name    = "vz89x",
> >> +        .of_match_table = of_match_ptr(vz89x_dt_ids),
> >> +    },
> >> +    .probe = vz89x_probe,
> >> +    .id_table = vz89x_id,
> >> +};
> >> +module_i2c_driver(vz89x_driver);
> >> +
> >> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> >> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
> >> +MODULE_LICENSE("GPL v2");
> > 
> > -- 
> > 
> > Peter Meerwald
> > +43-664-2444418 (mobile)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [RFC v1 0/4] iio: new chemical sensor framework and channel types

[Daniel Baluta]

On Sat, Sep 5, 2015 at 8:53 AM, Matt Ranostay <mranostay@gmail.com> wrote:
> Initial RFC for new chemical sensor framework, IIO_CONCENTRATION,
> and IIO_RESISTANCE channel types.
>
> Important notes:
> * Not been tested on real hardware yet but that isn't the main RFC reason and
>   once hardware is in hand it will be verified
> * Reason the IIO_CONCENTRATION type isn't in percent but has modifiers for ppm
>   and ppb is the scale value for the latter would cause a integer overflow
>   using IIO_VAL_FRACTIONAL

Please add this details also in the commit message introducing
IIO_CONCENTRATION. Readers
will faster find commit messages than cover letter.

Interesting patches Matt! :)

thanks,
Daniel.

Re: [RFC v1 1/4] iio: chemical: Add IIO_CONCENTRATION channel type

[Jonathan Cameron]

On 05/09/15 06:53, Matt Ranostay wrote:
> There are air quality sensors that report data back in parts per million
> of VOC (Volatile Organic Compounds) which are usually indexed from CO2
> or another common pollutant.
> 
> This patchset adds an IIO_CONCENTRATION type and IIO_MOD_PPM/PPB modifiers
> because no other channels types fit this use case.
Fine with concentration, not with the modifiers. See below.

> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++
>  drivers/iio/industrialio-core.c         |  3 +++
>  include/uapi/linux/iio/types.h          |  3 +++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 42d360f..a3803a1 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1459,3 +1459,13 @@ Description:
>  		measurements and return the average value as output data. Each
>  		value resulted from <type>[_name]_oversampling_ratio measurements
>  		is considered as one sample for <type>[_name]_sampling_frequency.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppm_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_ppb_raw
> +KernelVersion:	4.3
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) concentration reading of data like
> +		CO2 or VOC (Volatile Organic Compounds) substances with or without
> +		ppm (Part Per Million) or ppb (Parts Per Billion) channel modifiers.
See below.  ppm  / ppb should not be modifiers. That's not what they are for.

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index b3fcc2c..ea9e31a 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -75,6 +75,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_ENERGY] = "energy",
>  	[IIO_DISTANCE] = "distance",
>  	[IIO_VELOCITY] = "velocity",
> +	[IIO_CONCENTRATION] = "concentration",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> @@ -111,6 +112,8 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z] = "sqrt(x^2+y^2+z^2)",
>  	[IIO_MOD_I] = "i",
>  	[IIO_MOD_Q] = "q",
> +	[IIO_MOD_PPM] = "ppm",
> +	[IIO_MOD_PPB] = "ppb",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 2f8b117..dfb8b8c 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -35,6 +35,7 @@ enum iio_chan_type {
>  	IIO_ENERGY,
>  	IIO_DISTANCE,
>  	IIO_VELOCITY,
> +	IIO_CONCENTRATION,
>  };
>  
>  enum iio_modifier {
> @@ -72,6 +73,8 @@ enum iio_modifier {
>  	IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z,
>  	IIO_MOD_I,
>  	IIO_MOD_Q,
> +	IIO_MOD_PPM,
The use of modifiers is not about units.  Those should be well specified
in the ABI docs for the given channel type.  Hence I'd expect modifiers
for the type of chemical being detected not the unit.
PPB can be handled by PPM and a different return type
IIO_INT_PLUS_NANO etc.  If there isn't an appropriate return type for
the scale needed, then we can easily add more of those ;)

> +	IIO_MOD_PPB,
>  };
>  
>  enum iio_event_type {
> 

Re: [RFC v1 0/4] iio: new chemical sensor framework and channel types

[Jonathan Cameron]

On 05/09/15 09:20, Daniel Baluta wrote:
> On Sat, Sep 5, 2015 at 8:53 AM, Matt Ranostay <mranostay@gmail.com> wrote:
>> Initial RFC for new chemical sensor framework, IIO_CONCENTRATION,
>> and IIO_RESISTANCE channel types.
>>
>> Important notes:
>> * Not been tested on real hardware yet but that isn't the main RFC reason and
>>   once hardware is in hand it will be verified
>> * Reason the IIO_CONCENTRATION type isn't in percent but has modifiers for ppm
>>   and ppb is the scale value for the latter would cause a integer overflow
>>   using IIO_VAL_FRACTIONAL
Then add a a return type that deals with that issue.
Perhaps IIO_VAL_FRACTIONAL_DIVMILLION or
something like that.  Will just do the normal calculation but when outputting
append a load of leading zeros.  So val/(val2 * 1000000) = (val/val2) / 1000000
so decimal shift right 6 digits.
> 
> Please add this details also in the commit message introducing
> IIO_CONCENTRATION. Readers
> will faster find commit messages than cover letter.
Indeed, I wouldn't even have read the cover letter (tend to like jumping in the
deep end ;) except that I wondered what Daniel was commenting on!
> 
> Interesting patches Matt! :)
Indeed!
> 
> thanks,
> Daniel.
> --
> 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: [RFC v1 2/4] iio: resistance: add IIO_RESISTANCE channel type

[Jonathan Cameron]

On 05/09/15 06:53, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 6 ++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/uapi/linux/iio/types.h          | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index a3803a1..1084659 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1469,3 +1469,9 @@ Description:
>  		Raw (unscaled no offset etc.) concentration reading of data like
>  		CO2 or VOC (Volatile Organic Compounds) substances with or without
>  		ppm (Part Per Million) or ppb (Parts Per Billion) channel modifiers.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
> +KernelVersion:	4.3
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) resistance reading in ohms.
Fair enough I guess, though I would presume this is really a current measurement
at a known voltage.  Still measuring it as a resistance does save having to
specify the particular voltage so looks good to me.
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index ea9e31a..862c066 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -76,6 +76,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_DISTANCE] = "distance",
>  	[IIO_VELOCITY] = "velocity",
>  	[IIO_CONCENTRATION] = "concentration",
> +	[IIO_RESISTANCE] = "resistance",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index dfb8b8c..22edd4d 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -36,6 +36,7 @@ enum iio_chan_type {
>  	IIO_DISTANCE,
>  	IIO_VELOCITY,
>  	IIO_CONCENTRATION,
> +	IIO_RESISTANCE,
>  };
>  
>  enum iio_modifier {
> 

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Jonathan Cameron]

On 05/09/15 06:53, Matt Ranostay wrote:
> Add support for VZ89X sensors VOC and CO2 reporting channels in
> ppm/ppb units.
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Few points inline.  I'm either rather too sleepy or you don't actually
store threadings in your cache during the read measurement function hence
the driver will rather spectacularly not work right now :)

Jonathan
> ---
>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/chemical/Makefile                      |   6 +
>  drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
>  5 files changed, 246 insertions(+)
>  create mode 100644 drivers/iio/chemical/Makefile
>  create mode 100644 drivers/iio/chemical/vz89x.c
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index d77d412..a550216 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -88,6 +88,7 @@ ricoh,rs5c372b		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c386		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  ricoh,rv5c387a		I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>  samsung,24ad0xd1	S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
> +sgx,vz89x		SGX Sensortech VZ89X Sensors
>  sii,s35390a		2-wire CMOS real-time clock
>  skyworks,sky81452	Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>  st-micro,24c256		i2c serial eeprom  (24cxx)
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 4011eff..9664e9c 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>  source "drivers/iio/accel/Kconfig"
>  source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/amplifiers/Kconfig"
> +source "drivers/iio/chemical/Kconfig"
>  source "drivers/iio/common/Kconfig"
>  source "drivers/iio/dac/Kconfig"
>  source "drivers/iio/frequency/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 698afc2..2288684 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>  obj-y += accel/
>  obj-y += adc/
>  obj-y += amplifiers/
> +obj-y += chemical/
>  obj-y += common/
>  obj-y += dac/
>  obj-y += gyro/
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> new file mode 100644
> index 0000000..7292f2d
> --- /dev/null
> +++ b/drivers/iio/chemical/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO chemical sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
> new file mode 100644
> index 0000000..a596a22
> --- /dev/null
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -0,0 +1,237 @@
> +/*
> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VZ89X_REG_MEASUREMENT		0x09
> +#define VZ89X_REG_MEASUREMENT_SIZE	6
> +
> +#define VZ89X_VOC_CO2_IDX	0
> +#define VZ89X_VOC_SHORT_IDX	1
> +#define VZ89X_VOC_TVOC_IDX	2
> +#define VZ89X_RESISTANCE_IDX	3
> +
> +struct vz89x_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	unsigned long last_update;
> +
> +	u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];

Just a small point, but unless I'm missing something you forgot
to actually copy any data into the buffer anywhere.  Guess testing would
make this obviously fairly quickly once you have the part :)

> +};
> +
> +static const struct iio_chan_spec vz89x_channels[] = {
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.channel2 = IIO_MOD_PPM,
> +		.modified = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_CO2_IDX,
> +		.extend_name = "CO2",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_SHORT_IDX,
> +		.extend_name = "VOC_short",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.channel2 = IIO_MOD_PPB,
> +		.modified = 1,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_VOC_TVOC_IDX,
> +		.extend_name = "tVOC",
> +	},
> +	{
I think this is effectively a raw measurement of the signal used
to derive the above values?  I'm I reading the datasheet correctly
about this?  Not that it maters but might be nice to add a comment
to that effect.
> +		.type = IIO_RESISTANCE,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.address = VZ89X_RESISTANCE_IDX,
> +	},
> +};
> +
> +static int vz89x_get_measurement(struct vz89x_data *data)
> +{
> +	int ret;
> +	int i;
> +
> +	/* sensor can only be polled once a second max per datasheet */
> +	if (!time_after(jiffies, data->last_update + HZ))
return -EBUSY? and pass it all the way up to userspace to let
it know why no value was returned.
> +		return 0;
> +
> +	ret = i2c_smbus_write_word_data(data->client,
> +					VZ89X_REG_MEASUREMENT, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
> +		ret = i2c_smbus_read_byte(data->client);
> +		if (ret < 0)
> +			return ret;
Store the result somewhere perhaps?
> +	}
> +
> +	data->last_update = jiffies;
> +
> +	return 0;
> +}
> +
> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
> +					struct iio_chan_spec const *chan)
> +{
> +	u8 *buf = &data->buffer[chan->address];
> +
> +	return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
> +}
> +
> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> +				   int *val, int *val2)
> +{
> +	int ret = -EINVAL;
> +
> +	switch (chan->address) {
> +	case VZ89X_VOC_CO2_IDX:
> +		*val = 1600;
> +		*val2 = 229;
> +		ret = IIO_VAL_FRACTIONAL;
> +		break;
> +	case VZ89X_VOC_SHORT_IDX:
> +		*val = 0;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case VZ89X_VOC_TVOC_IDX:
> +		*val = 1000;
> +		*val2 = 229;
> +		ret = IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int vz89x_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, int *val,
> +			  int *val2, long mask)
> +{
> +	struct vz89x_data *data = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = vz89x_get_measurement(data);
> +
> +		if (chan->type == IIO_RESISTANCE)
> +			*val = vz89x_get_resistance_reading(data, chan);
> +		else
> +			*val = data->buffer[chan->address] - 13;
> +
> +		if (!ret)
> +			ret = IIO_VAL_INT;
> +
> +		mutex_unlock(&data->lock);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_RESISTANCE:
> +			*val = 10;
> +			ret = IIO_VAL_INT;
> +			break;
> +		case IIO_CONCENTRATION:
> +			ret = vz89x_get_channel_scale(chan, val, val2);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 400;
> +		ret = IIO_VAL_INT;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info vz89x_info = {
> +	.driver_module	= THIS_MODULE,
> +	.read_raw	= vz89x_read_raw,
> +};
> +
> +static int vz89x_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct iio_dev *indio_dev;
> +	struct vz89x_data *data;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_BYTE))
> +		return -ENODEV;
> +
> +	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;
> +	data->last_update = jiffies - HZ;
> +	mutex_init(&data->lock);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &vz89x_info,
> +	indio_dev->name = dev_name(&client->dev);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	indio_dev->channels = vz89x_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id vz89x_id[] = {
> +	{ "vz89x", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
> +
> +static const struct of_device_id vz89x_dt_ids[] = {
> +	{ .compatible = "sgx,vz89x" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
> +
> +static struct i2c_driver vz89x_driver = {
> +	.driver = {
> +		.name	= "vz89x",
> +		.of_match_table = of_match_ptr(vz89x_dt_ids),
> +	},
> +	.probe = vz89x_probe,
> +	.id_table = vz89x_id,
> +};
> +module_i2c_driver(vz89x_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
> +MODULE_LICENSE("GPL v2");
> 

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Matt Ranostay]

On Sat, Sep 5, 2015 at 9:14 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 05/09/15 06:53, Matt Ranostay wrote:
>> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> ppm/ppb units.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> Few points inline.  I'm either rather too sleepy or you don't actually
> store threadings in your cache during the read measurement function hence
> the driver will rather spectacularly not work right now :)
>
> Jonathan
>> ---
>>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/chemical/Makefile                      |   6 +
>>  drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
>>  5 files changed, 246 insertions(+)
>>  create mode 100644 drivers/iio/chemical/Makefile
>>  create mode 100644 drivers/iio/chemical/vz89x.c
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> index d77d412..a550216 100644
>> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> @@ -88,6 +88,7 @@ ricoh,rs5c372b              I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>>  ricoh,rv5c386                I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>>  ricoh,rv5c387a               I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>>  samsung,24ad0xd1     S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
>> +sgx,vz89x            SGX Sensortech VZ89X Sensors
>>  sii,s35390a          2-wire CMOS real-time clock
>>  skyworks,sky81452    Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>>  st-micro,24c256              i2c serial eeprom  (24cxx)
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 4011eff..9664e9c 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>>  source "drivers/iio/accel/Kconfig"
>>  source "drivers/iio/adc/Kconfig"
>>  source "drivers/iio/amplifiers/Kconfig"
>> +source "drivers/iio/chemical/Kconfig"
>>  source "drivers/iio/common/Kconfig"
>>  source "drivers/iio/dac/Kconfig"
>>  source "drivers/iio/frequency/Kconfig"
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 698afc2..2288684 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>>  obj-y += accel/
>>  obj-y += adc/
>>  obj-y += amplifiers/
>> +obj-y += chemical/
>>  obj-y += common/
>>  obj-y += dac/
>>  obj-y += gyro/
>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> new file mode 100644
>> index 0000000..7292f2d
>> --- /dev/null
>> +++ b/drivers/iio/chemical/Makefile
>> @@ -0,0 +1,6 @@
>> +#
>> +# Makefile for IIO chemical sensors
>> +#
>> +
>> +# When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_VZ89X)          += vz89x.o
>> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
>> new file mode 100644
>> index 0000000..a596a22
>> --- /dev/null
>> +++ b/drivers/iio/chemical/vz89x.c
>> @@ -0,0 +1,237 @@
>> +/*
>> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
>> + *
>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#define VZ89X_REG_MEASUREMENT                0x09
>> +#define VZ89X_REG_MEASUREMENT_SIZE   6
>> +
>> +#define VZ89X_VOC_CO2_IDX    0
>> +#define VZ89X_VOC_SHORT_IDX  1
>> +#define VZ89X_VOC_TVOC_IDX   2
>> +#define VZ89X_RESISTANCE_IDX 3
>> +
>> +struct vz89x_data {
>> +     struct i2c_client *client;
>> +     struct mutex lock;
>> +     unsigned long last_update;
>> +
>> +     u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
>
> Just a small point, but unless I'm missing something you forgot
> to actually copy any data into the buffer anywhere.  Guess testing would
> make this obviously fairly quickly once you have the part :)
>
>> +};
>> +
>> +static const struct iio_chan_spec vz89x_channels[] = {
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .channel2 = IIO_MOD_PPM,
>> +             .modified = 1,
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .address = VZ89X_VOC_CO2_IDX,
>> +             .extend_name = "CO2",
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .address = VZ89X_VOC_SHORT_IDX,
>> +             .extend_name = "VOC_short",
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .channel2 = IIO_MOD_PPB,
>> +             .modified = 1,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .address = VZ89X_VOC_TVOC_IDX,
>> +             .extend_name = "tVOC",
>> +     },
>> +     {
> I think this is effectively a raw measurement of the signal used
> to derive the above values?  I'm I reading the datasheet correctly
> about this?  Not that it maters but might be nice to add a comment
> to that effect.

Ok.
>> +             .type = IIO_RESISTANCE,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .address = VZ89X_RESISTANCE_IDX,
>> +     },
>> +};
>> +
>> +static int vz89x_get_measurement(struct vz89x_data *data)
>> +{
>> +     int ret;
>> +     int i;
>> +
>> +     /* sensor can only be polled once a second max per datasheet */
>> +     if (!time_after(jiffies, data->last_update + HZ))
> return -EBUSY? and pass it all the way up to userspace to let
> it know why no value was returned.
>> +             return 0;
>> +
>> +     ret = i2c_smbus_write_word_data(data->client,
>> +                                     VZ89X_REG_MEASUREMENT, 0);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
>> +             ret = i2c_smbus_read_byte(data->client);
>> +             if (ret < 0)
>> +                     return ret;
> Store the result somewhere perhaps?

Oops missed the  data->buffer[i] = ret :)

>> +     }
>> +
>> +     data->last_update = jiffies;
>> +
>> +     return 0;
>> +}
>> +
>> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
>> +                                     struct iio_chan_spec const *chan)
>> +{
>> +     u8 *buf = &data->buffer[chan->address];
>> +
>> +     return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
>> +}
>> +
>> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> +                                int *val, int *val2)
>> +{
>> +     int ret = -EINVAL;
>> +
>> +     switch (chan->address) {
>> +     case VZ89X_VOC_CO2_IDX:
>> +             *val = 1600;
>> +             *val2 = 229;
>> +             ret = IIO_VAL_FRACTIONAL;
>> +             break;
>> +     case VZ89X_VOC_SHORT_IDX:
>> +             *val = 0;
>> +             ret = IIO_VAL_INT;
>> +             break;
>> +     case VZ89X_VOC_TVOC_IDX:
>> +             *val = 1000;
>> +             *val2 = 229;
>> +             ret = IIO_VAL_FRACTIONAL;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +static int vz89x_read_raw(struct iio_dev *indio_dev,
>> +                       struct iio_chan_spec const *chan, int *val,
>> +                       int *val2, long mask)
>> +{
>> +     struct vz89x_data *data = iio_priv(indio_dev);
>> +     int ret = -EINVAL;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             mutex_lock(&data->lock);
>> +             ret = vz89x_get_measurement(data);
>> +
>> +             if (chan->type == IIO_RESISTANCE)
>> +                     *val = vz89x_get_resistance_reading(data, chan);
>> +             else
>> +                     *val = data->buffer[chan->address] - 13;
>> +
>> +             if (!ret)
>> +                     ret = IIO_VAL_INT;
>> +
>> +             mutex_unlock(&data->lock);
>> +             break;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             switch (chan->type) {
>> +             case IIO_RESISTANCE:
>> +                     *val = 10;
>> +                     ret = IIO_VAL_INT;
>> +                     break;
>> +             case IIO_CONCENTRATION:
>> +                     ret = vz89x_get_channel_scale(chan, val, val2);
>> +                     break;
>> +             default:
>> +                     ret = -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             *val = 400;
>> +             ret = IIO_VAL_INT;
>> +             break;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +static const struct iio_info vz89x_info = {
>> +     .driver_module  = THIS_MODULE,
>> +     .read_raw       = vz89x_read_raw,
>> +};
>> +
>> +static int vz89x_probe(struct i2c_client *client,
>> +                    const struct i2c_device_id *id)
>> +{
>> +     struct iio_dev *indio_dev;
>> +     struct vz89x_data *data;
>> +
>> +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
>> +                                  I2C_FUNC_SMBUS_BYTE))
>> +             return -ENODEV;
>> +
>> +     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;
>> +     data->last_update = jiffies - HZ;
>> +     mutex_init(&data->lock);
>> +
>> +     indio_dev->dev.parent = &client->dev;
>> +     indio_dev->info = &vz89x_info,
>> +     indio_dev->name = dev_name(&client->dev);
>> +     indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +     indio_dev->channels = vz89x_channels;
>> +     indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
>> +
>> +     return devm_iio_device_register(&client->dev, indio_dev);
>> +}
>> +
>> +static const struct i2c_device_id vz89x_id[] = {
>> +     { "vz89x", 0 },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
>> +
>> +static const struct of_device_id vz89x_dt_ids[] = {
>> +     { .compatible = "sgx,vz89x" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
>> +
>> +static struct i2c_driver vz89x_driver = {
>> +     .driver = {
>> +             .name   = "vz89x",
>> +             .of_match_table = of_match_ptr(vz89x_dt_ids),
>> +     },
>> +     .probe = vz89x_probe,
>> +     .id_table = vz89x_id,
>> +};
>> +module_i2c_driver(vz89x_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
>> +MODULE_LICENSE("GPL v2");
>>
>

Re: [RFC v1 4/4] iio: chemical: add SGX VZ89x VOC sensor support

[Matt Ranostay]

On Sat, Sep 5, 2015 at 1:11 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
>> >> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> >> ppm/ppb units.
>> >
>> > comments below
>> > link to datasheet?
>>
>> I will post the links when I am not at the Oregon coast. Have no laptop atm...
>>
>> >
>> >> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> >> ---
>> >> .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>> >> drivers/iio/Kconfig                                |   1 +
>> >> drivers/iio/Makefile                               |   1 +
>> >> drivers/iio/chemical/Makefile                      |   6 +
>> >> drivers/iio/chemical/vz89x.c                       | 237 +++++++++++++++++++++
>> >> 5 files changed, 246 insertions(+)
>> >> create mode 100644 drivers/iio/chemical/Makefile
>> >> create mode 100644 drivers/iio/chemical/vz89x.c
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> index d77d412..a550216 100644
>> >> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
>> >> @@ -88,6 +88,7 @@ ricoh,rs5c372b        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> >> ricoh,rv5c386        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> >> ricoh,rv5c387a        I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>> >> samsung,24ad0xd1    S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
>> >> +sgx,vz89x        SGX Sensortech VZ89X Sensors
>> >> sii,s35390a        2-wire CMOS real-time clock
>> >> skyworks,sky81452    Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>> >> st-micro,24c256        i2c serial eeprom  (24cxx)
>> >> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> >> index 4011eff..9664e9c 100644
>> >> --- a/drivers/iio/Kconfig
>> >> +++ b/drivers/iio/Kconfig
>> >> @@ -61,6 +61,7 @@ config IIO_CONSUMERS_PER_TRIGGER
>> >> source "drivers/iio/accel/Kconfig"
>> >> source "drivers/iio/adc/Kconfig"
>> >> source "drivers/iio/amplifiers/Kconfig"
>> >> +source "drivers/iio/chemical/Kconfig"
>> >> source "drivers/iio/common/Kconfig"
>> >> source "drivers/iio/dac/Kconfig"
>> >> source "drivers/iio/frequency/Kconfig"
>> >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> >> index 698afc2..2288684 100644
>> >> --- a/drivers/iio/Makefile
>> >> +++ b/drivers/iio/Makefile
>> >> @@ -14,6 +14,7 @@ obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>> >> obj-y += accel/
>> >> obj-y += adc/
>> >> obj-y += amplifiers/
>> >> +obj-y += chemical/
>> >> obj-y += common/
>> >> obj-y += dac/
>> >> obj-y += gyro/
>> >> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> >> new file mode 100644
>> >> index 0000000..7292f2d
>> >> --- /dev/null
>> >> +++ b/drivers/iio/chemical/Makefile
>> >> @@ -0,0 +1,6 @@
>> >> +#
>> >> +# Makefile for IIO chemical sensors
>> >> +#
>> >> +
>> >> +# When adding new entries keep the list in alphabetical order
>> >> +obj-$(CONFIG_VZ89X)        += vz89x.o
>> >> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
>> >> new file mode 100644
>> >> index 0000000..a596a22
>> >> --- /dev/null
>> >> +++ b/drivers/iio/chemical/vz89x.c
>> >> @@ -0,0 +1,237 @@
>> >> +/*
>> >> + * vz89x.c - Support for SGX Sensortech MiCS VZ89X VOC sensors
>> >> + *
>> >> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> >> + *
>> >> + * This program is free software; you can redistribute it and/or modify
>> >> + * it under the terms of the GNU General Public License as published by
>> >> + * the Free Software Foundation; either version 2 of the License, or
>> >> + * (at your option) any later version.
>> >> + *
>> >> + * This program is distributed in the hope that it will be useful,
>> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> >> + * GNU General Public License for more details.
>> >> + *
>> >> + */
>> >> +
>> >> +#include <linux/module.h>
>> >> +#include <linux/mutex.h>
>> >> +#include <linux/init.h>
>> >> +#include <linux/i2c.h>
>> >> +
>> >> +#include <linux/iio/iio.h>
>> >> +#include <linux/iio/sysfs.h>
>> >> +
>> >> +#define VZ89X_REG_MEASUREMENT        0x09
>> >> +#define VZ89X_REG_MEASUREMENT_SIZE    6
>> >> +
>> >> +#define VZ89X_VOC_CO2_IDX    0
>> >> +#define VZ89X_VOC_SHORT_IDX    1
>> >> +#define VZ89X_VOC_TVOC_IDX    2
>> >> +#define VZ89X_RESISTANCE_IDX    3
>> >> +
>> >> +struct vz89x_data {
>> >> +    struct i2c_client *client;
>> >> +    struct mutex lock;
>> >> +    unsigned long last_update;
>> >> +
>> >> +    u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
>> >> +};
>> >> +
>> >> +static const struct iio_chan_spec vz89x_channels[] = {
>> >> +    {
>> >> +        .type = IIO_CONCENTRATION,
>> >> +        .channel2 = IIO_MOD_PPM,
>> >> +        .modified = 1,
>> >> +        .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> >> +        .address = VZ89X_VOC_CO2_IDX,
>> >> +        .extend_name = "CO2",
>> >> +    },
>> >> +    {
>> >> +        .type = IIO_CONCENTRATION,
>> >> +        .info_mask_separate =
>> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> >> +        .address = VZ89X_VOC_SHORT_IDX,
>> >> +        .extend_name = "VOC_short",
>> >> +    },
>> >> +    {
>> >> +        .type = IIO_CONCENTRATION,
>> >> +        .channel2 = IIO_MOD_PPB,
>> >> +        .modified = 1,
>> >> +        .info_mask_separate =
>> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> >> +        .address = VZ89X_VOC_TVOC_IDX,
>> >> +        .extend_name = "tVOC",
>> >> +    },
>> >> +    {
>> >> +        .type = IIO_RESISTANCE,
>> >> +        .info_mask_separate =
>> >> +            BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> >> +        .address = VZ89X_RESISTANCE_IDX,
>> >> +    },
>> >> +};
>> >> +
>> >> +static int vz89x_get_measurement(struct vz89x_data *data)
>> >> +{
>> >> +    int ret;
>> >> +    int i;
>> >> +
>> >> +    /* sensor can only be polled once a second max per datasheet */
>> >> +    if (!time_after(jiffies, data->last_update + HZ))
>> >> +        return 0;
>> >> +
>> >> +    ret = i2c_smbus_write_word_data(data->client,
>> >> +                    VZ89X_REG_MEASUREMENT, 0);
>> >> +    if (ret < 0)
>> >> +        return ret;
>> >> +
>> >> +    for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
>> >> +        ret = i2c_smbus_read_byte(data->client);
>> >
>> > could a block transfer be used?
>>
>> Yah from the data sheet on i2c it can only do byte reads after the
>> "command write data of zero"... I'll know better I have this part in
>> hand.... But from Arduino hippies it seems I have gotten weird sensors
>> to work on this chipset....
>
> and the transfer is at most once per second, so no big performance deal
>
>>
>> >> +        if (ret < 0)
>> >> +            return ret;
>> >> +    }
>> >> +
>> >> +    data->last_update = jiffies;
>> >> +
>> >> +    return 0;
>> >> +}
>> >> +
>> >> +static int vz89x_get_resistance_reading(struct vz89x_data *data,
>> >> +                    struct iio_chan_spec const *chan)
>> >> +{
>> >> +    u8 *buf = &data->buffer[chan->address];
>> >
>> > I think only VZ89X_RESISTANCE_IDX as chan->address makes sense here?

Doesn't matter either, either way work for me.

>> >
>> >> +
>> >> +    return buf[0] | ((u16)buf[1] << 8) | ((u16)buf[2] << 16);
>> >
>> > why cast to u16? no cast is neccessary
>> Maybe but to be safe of compilers not getting the shift left values that could happen doesn't hurt
>
> (u16)buf[2] << 16
> u16 still makes no sense

D'oh missed that it should be u32 :)

>
>> >
>> >> +}
>> >> +
>> >> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> >> +                   int *val, int *val2)
>> >> +{
>> >> +    int ret = -EINVAL;
>> >> +
>> >> +    switch (chan->address) {
>> >> +    case VZ89X_VOC_CO2_IDX:
>> >> +        *val = 1600;
>> >> +        *val2 = 229;
>> >> +        ret = IIO_VAL_FRACTIONAL;
>> >
>> > return directly and save the break, no init of ret
>>
>> Old habits :)
>
> agree, matter of taste; but in this case is saves somes lines
>
>> >> +        break;
>> >> +    case VZ89X_VOC_SHORT_IDX:
>> >> +        *val = 0;
>> >> +        ret = IIO_VAL_INT;
>> >> +        break;
>> >> +    case VZ89X_VOC_TVOC_IDX:
>> >> +        *val = 1000;
>> >> +        *val2 = 229;
>> >> +        ret = IIO_VAL_FRACTIONAL;
>> >
>> > default
>> >
>> >> +    }
>> >> +
>> >> +    return ret;
>> >> +}
>> >> +
>> >> +static int vz89x_read_raw(struct iio_dev *indio_dev,
>> >> +              struct iio_chan_spec const *chan, int *val,
>> >> +              int *val2, long mask)
>> >> +{
>> >> +    struct vz89x_data *data = iio_priv(indio_dev);
>> >> +    int ret = -EINVAL;
>> >> +
>> >> +    switch (mask) {
>> >> +    case IIO_CHAN_INFO_RAW:
>> >> +        mutex_lock(&data->lock);
>> >> +        ret = vz89x_get_measurement(data);
>> >> +
>> >> +        if (chan->type == IIO_RESISTANCE)
>> >> +            *val = vz89x_get_resistance_reading(data, chan);
>> >
>> > chan argument is not needed

It is needed to get the chan->address in that function

>> >
>> >> +        else
>> >> +            *val = data->buffer[chan->address] - 13;
>> >
>> > maybe have this block after checking ret, operating on invalid data isn't
>> > well behaved :)
>> > what is 13? could this be incorporated in _OFFSET?
>> >
>>
>> Yeah if scale value wasn't calculated first... Need to ask them why 13.. I mean really
>
> I always have to look is up as well: it is (value + offset) * scale

Well in this case it is ((value - 13) * scale) + offset.

>
>> >> +
>> >> +        if (!ret)
>> >> +            ret = IIO_VAL_INT;
>> >> +
>> >> +        mutex_unlock(&data->lock);
>> >> +        break;
>> >> +    case IIO_CHAN_INFO_SCALE:
>> >> +        switch (chan->type) {
>> >> +        case IIO_RESISTANCE:
>> >> +            *val = 10;
>> >> +            ret = IIO_VAL_INT;
>> >> +            break;
>> >> +        case IIO_CONCENTRATION:
>> >> +            ret = vz89x_get_channel_scale(chan, val, val2);
>> >> +            break;
>> >> +        default:
>> >> +            ret = -EINVAL;
>> >> +        }
>> >> +        break;
>> >> +    case IIO_CHAN_INFO_OFFSET:
>> >> +        *val = 400;
>> >> +        ret = IIO_VAL_INT;
>> >> +        break;
>> >> +    }
>> >> +
>> >> +    return ret;
>> >> +}
>> >> +
>> >> +static const struct iio_info vz89x_info = {
>> >> +    .driver_module    = THIS_MODULE,
>> >> +    .read_raw    = vz89x_read_raw,
>> >> +};
>> >> +
>> >> +static int vz89x_probe(struct i2c_client *client,
>> >> +               const struct i2c_device_id *id)
>> >> +{
>> >> +    struct iio_dev *indio_dev;
>> >> +    struct vz89x_data *data;
>> >> +
>> >> +    if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
>> >> +                     I2C_FUNC_SMBUS_BYTE))
>> >> +        return -ENODEV;
>> >> +
>> >> +    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;
>> >> +    data->last_update = jiffies - HZ;
>> >> +    mutex_init(&data->lock);
>> >> +
>> >> +    indio_dev->dev.parent = &client->dev;
>> >> +    indio_dev->info = &vz89x_info,
>> >> +    indio_dev->name = dev_name(&client->dev);
>> >> +    indio_dev->modes = INDIO_DIRECT_MODE;
>> >> +
>> >> +    indio_dev->channels = vz89x_channels;
>> >> +    indio_dev->num_channels = ARRAY_SIZE(vz89x_channels);
>> >> +
>> >> +    return devm_iio_device_register(&client->dev, indio_dev);
>> >> +}
>> >> +
>> >> +static const struct i2c_device_id vz89x_id[] = {
>> >> +    { "vz89x", 0 },
>> >> +    { }
>> >> +};
>> >> +MODULE_DEVICE_TABLE(i2c, vz89x_id);
>> >> +
>> >> +static const struct of_device_id vz89x_dt_ids[] = {
>> >> +    { .compatible = "sgx,vz89x" },
>> >> +    { }
>> >> +};
>> >> +MODULE_DEVICE_TABLE(of, vz89x_dt_ids);
>> >> +
>> >> +static struct i2c_driver vz89x_driver = {
>> >> +    .driver = {
>> >> +        .name    = "vz89x",
>> >> +        .of_match_table = of_match_ptr(vz89x_dt_ids),
>> >> +    },
>> >> +    .probe = vz89x_probe,
>> >> +    .id_table = vz89x_id,
>> >> +};
>> >> +module_i2c_driver(vz89x_driver);
>> >> +
>> >> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> >> +MODULE_DESCRIPTION("SGX Sensortech MiCS VZ89X VOC sensors");
>> >> +MODULE_LICENSE("GPL v2");
>> >
>> > --
>> >
>> > Peter Meerwald
>> > +43-664-2444418 (mobile)
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>
> --
>
> Peter Meerwald
> +43-664-2444418 (mobile)