linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v4 0/4] iio: new chemical sensor framework and channel types
@ 2015-09-10  6:30 Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type Matt Ranostay
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10  6:30 UTC (permalink / raw)
  To: jic23, lars, pmeerw; +Cc: linux-iio, Matt Ranostay

Changes from RFC v3:
* Add missing Kconfig

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            |  14 ++
 .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/chemical/Kconfig                       |  15 ++
 drivers/iio/chemical/Makefile                      |   6 +
 drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
 drivers/iio/industrialio-core.c                    |   2 +
 include/uapi/linux/iio/types.h                     |   2 +
 11 files changed, 318 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
 create mode 100644 drivers/iio/chemical/Kconfig
 create mode 100644 drivers/iio/chemical/Makefile
 create mode 100644 drivers/iio/chemical/vz89x.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type
  2015-09-10  6:30 [RFC v4 0/4] iio: new chemical sensor framework and channel types Matt Ranostay
@ 2015-09-10  6:30 ` Matt Ranostay
  2015-09-12  9:16   ` Jonathan Cameron
  2015-09-10  6:30 ` [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE " Matt Ranostay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10  6:30 UTC (permalink / raw)
  To: jic23, lars, pmeerw; +Cc: linux-iio, 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 that returns a percentage
of substance because no other channels types fit this use case.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 42d360f..48080b7 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1459,3 +1459,10 @@ 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_concentration_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
+KernelVersion:	4.3
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Raw (unscaled no offset etc.) precentage reading of a substance.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index b3fcc2c..58a60a1 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[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 2f8b117..c5a0e3f 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 {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE channel type
  2015-09-10  6:30 [RFC v4 0/4] iio: new chemical sensor framework and channel types Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type Matt Ranostay
@ 2015-09-10  6:30 ` Matt Ranostay
  2015-09-12  9:19   ` Jonathan Cameron
  2015-09-10  6:30 ` [RFC v4 3/4] devicetree: add SGX Sensortech vendor id Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support Matt Ranostay
  3 siblings, 1 reply; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10  6:30 UTC (permalink / raw)
  To: jic23, lars, pmeerw; +Cc: linux-iio, Matt Ranostay

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 48080b7..0f683ed 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1466,3 +1466,10 @@ KernelVersion:	4.3
 Contact:	linux-iio@vger.kernel.org
 Description:
 		Raw (unscaled no offset etc.) precentage reading of a substance.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
+What:		/sys/bus/iio/devices/iio:deviceX/in_resistanceX_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 58a60a1..d61a363 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 c5a0e3f..d58319c 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 3/4] devicetree: add SGX Sensortech vendor id
  2015-09-10  6:30 [RFC v4 0/4] iio: new chemical sensor framework and channel types Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE " Matt Ranostay
@ 2015-09-10  6:30 ` Matt Ranostay
  2015-09-10  6:30 ` [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support Matt Ranostay
  3 siblings, 0 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10  6:30 UTC (permalink / raw)
  To: jic23, lars, pmeerw; +Cc: linux-iio, 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-10  6:30 [RFC v4 0/4] iio: new chemical sensor framework and channel types Matt Ranostay
                   ` (2 preceding siblings ...)
  2015-09-10  6:30 ` [RFC v4 3/4] devicetree: add SGX Sensortech vendor id Matt Ranostay
@ 2015-09-10  6:30 ` Matt Ranostay
  2015-09-10  9:08   ` Peter Meerwald
  2015-09-12  9:34   ` Jonathan Cameron
  3 siblings, 2 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10  6:30 UTC (permalink / raw)
  To: jic23, lars, pmeerw; +Cc: linux-iio, Matt Ranostay

Add support for VZ89X sensors VOC and CO2 reporting channels in
percentage which can be converted to part per million.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
 .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/chemical/Kconfig                       |  15 ++
 drivers/iio/chemical/Makefile                      |   6 +
 drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
 7 files changed, 299 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
 create mode 100644 drivers/iio/chemical/Kconfig
 create mode 100644 drivers/iio/chemical/Makefile
 create mode 100644 drivers/iio/chemical/vz89x.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
new file mode 100644
index 0000000..74f2a35
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
@@ -0,0 +1,30 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
+Date:		September 2015
+KernelVersion:	4.3
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the unscaled, and no offset applied CO2 gas percentage
+		value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
+Date:		September 2015
+KernelVersion:	4.3
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the raw calibration VOC value from the sensor.
+		This value has little application outside of calibration.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
+Date:		September 2015
+KernelVersion:	4.3
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the unscaled, and no offset applied total VOC gas
+		percentage value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
+Date:		September 2015
+KernelVersion:	4.3
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the unscaled sensor resistance value.
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/Kconfig b/drivers/iio/chemical/Kconfig
new file mode 100644
index 0000000..8fb44e5
--- /dev/null
+++ b/drivers/iio/chemical/Kconfig
@@ -0,0 +1,15 @@
+#
+# Chemical sensors
+#
+
+menu "AQI VOC sensors"
+
+config VZ89X
+	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
+	depends on I2C
+	help
+	  Say Y here to build I2C interface support for the SGX
+	  Sensortech MiCS VZ89X VOC (Volatile Organic Compounds) 
+	  sensors 
+
+endmenu
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..847f853
--- /dev/null
+++ b/drivers/iio/chemical/vz89x.c
@@ -0,0 +1,245 @@
+/*
+ * 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_VOC_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,
+		.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),
+		.address = VZ89X_VOC_SHORT_IDX,
+		.extend_name = "VOC_short",
+	},
+	{
+		.type = IIO_CONCENTRATION,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
+			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_VOC_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->buffer[i] = ret;
+	}
+
+	data->last_update = jiffies;
+
+	return 0;
+}
+
+static int vz89x_get_resistance_reading(struct vz89x_data *data)
+{
+	u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
+
+	return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);
+}
+
+static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
+				   int *val, int *val2)
+{
+	switch (chan->address) {
+	case VZ89X_VOC_CO2_IDX:
+		*val = 1600;
+		*val2 = 229;
+		return IIO_VAL_FRACTIONAL;
+	case VZ89X_VOC_TVOC_IDX:
+		*val = 1000;
+		*val2 = 229;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return -EINVAL;
+}
+
+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);
+		mutex_unlock(&data->lock);
+
+		if (ret)
+			return ret;
+
+		switch (chan->address) {
+		case VZ89X_VOC_CO2_IDX:
+			*val = 0;
+			*val2 = data->buffer[chan->address];
+			return IIO_VAL_INT_PLUS_MICRO;
+		case VZ89X_VOC_SHORT_IDX:
+			*val = data->buffer[chan->address];
+			return IIO_VAL_INT;
+		case VZ89X_VOC_TVOC_IDX:
+			*val = 0;
+			*val2 = data->buffer[chan->address];
+			return IIO_VAL_INT_PLUS_NANO;
+		case VZ89X_VOC_RESISTANCE_IDX:
+			*val = vz89x_get_resistance_reading(data);
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_RESISTANCE:
+			*val = 10;
+			return IIO_VAL_INT;
+		case IIO_CONCENTRATION:
+			return vz89x_get_channel_scale(chan, val, val2);
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		switch (chan->address) {
+		case VZ89X_VOC_CO2_IDX:
+			*val = 0;
+			*val2 = 44250;
+			return IIO_VAL_INT_PLUS_NANO;
+		case VZ89X_VOC_TVOC_IDX:
+			*val = 0;
+			*val2 = -13;
+			return IIO_VAL_INT_PLUS_NANO;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-10  6:30 ` [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support Matt Ranostay
@ 2015-09-10  9:08   ` Peter Meerwald
  2015-09-10 16:16     ` Matt Ranostay
  2015-09-11  2:13     ` Matt Ranostay
  2015-09-12  9:34   ` Jonathan Cameron
  1 sibling, 2 replies; 12+ messages in thread
From: Peter Meerwald @ 2015-09-10  9:08 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: jic23, lars, linux-iio


> Add support for VZ89X sensors VOC and CO2 reporting channels in
> percentage which can be converted to part per million.

two comments below
 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/chemical/Kconfig                       |  15 ++
>  drivers/iio/chemical/Makefile                      |   6 +
>  drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
>  7 files changed, 299 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>  create mode 100644 drivers/iio/chemical/Kconfig
>  create mode 100644 drivers/iio/chemical/Makefile
>  create mode 100644 drivers/iio/chemical/vz89x.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
> new file mode 100644
> index 0000000..74f2a35
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
> @@ -0,0 +1,30 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled, and no offset applied CO2 gas percentage
> +		value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the raw calibration VOC value from the sensor.
> +		This value has little application outside of calibration.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled, and no offset applied total VOC gas
> +		percentage value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled sensor resistance value.
> 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/Kconfig b/drivers/iio/chemical/Kconfig
> new file mode 100644
> index 0000000..8fb44e5
> --- /dev/null
> +++ b/drivers/iio/chemical/Kconfig
> @@ -0,0 +1,15 @@
> +#
> +# Chemical sensors
> +#
> +
> +menu "AQI VOC sensors"

what does AQI mean?

> +
> +config VZ89X
> +	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
> +	depends on I2C
> +	help
> +	  Say Y here to build I2C interface support for the SGX
> +	  Sensortech MiCS VZ89X VOC (Volatile Organic Compounds) 
> +	  sensors 
> +
> +endmenu
> 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..847f853
> --- /dev/null
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -0,0 +1,245 @@
> +/*
> + * 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_VOC_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,
> +		.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),
> +		.address = VZ89X_VOC_SHORT_IDX,
> +		.extend_name = "VOC_short",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +			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_VOC_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->buffer[i] = ret;
> +	}
> +
> +	data->last_update = jiffies;
> +
> +	return 0;
> +}
> +
> +static int vz89x_get_resistance_reading(struct vz89x_data *data)
> +{
> +	u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
> +
> +	return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);

the typecasts are just weird, don't do it; the code works fine without them
please check integer promotion in C, e.g. 
https://www.securecoding.cert.org/confluence/display/c/INT02-C.+Understand+integer+conversion+rules

> +}
> +
> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> +				   int *val, int *val2)
> +{
> +	switch (chan->address) {
> +	case VZ89X_VOC_CO2_IDX:
> +		*val = 1600;
> +		*val2 = 229;
> +		return IIO_VAL_FRACTIONAL;
> +	case VZ89X_VOC_TVOC_IDX:
> +		*val = 1000;
> +		*val2 = 229;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +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);
> +		mutex_unlock(&data->lock);
> +
> +		if (ret)
> +			return ret;
> +
> +		switch (chan->address) {
> +		case VZ89X_VOC_CO2_IDX:
> +			*val = 0;
> +			*val2 = data->buffer[chan->address];
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		case VZ89X_VOC_SHORT_IDX:
> +			*val = data->buffer[chan->address];
> +			return IIO_VAL_INT;
> +		case VZ89X_VOC_TVOC_IDX:
> +			*val = 0;
> +			*val2 = data->buffer[chan->address];
> +			return IIO_VAL_INT_PLUS_NANO;
> +		case VZ89X_VOC_RESISTANCE_IDX:
> +			*val = vz89x_get_resistance_reading(data);
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_RESISTANCE:
> +			*val = 10;
> +			return IIO_VAL_INT;
> +		case IIO_CONCENTRATION:
> +			return vz89x_get_channel_scale(chan, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->address) {
> +		case VZ89X_VOC_CO2_IDX:
> +			*val = 0;
> +			*val2 = 44250;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		case VZ89X_VOC_TVOC_IDX:
> +			*val = 0;
> +			*val2 = -13;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	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)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-10  9:08   ` Peter Meerwald
@ 2015-09-10 16:16     ` Matt Ranostay
  2015-09-11  2:13     ` Matt Ranostay
  1 sibling, 0 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-10 16:16 UTC (permalink / raw)
  To: Peter Meerwald
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio@vger.kernel.org

On Thu, Sep 10, 2015 at 2:08 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
>> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> percentage which can be converted to part per million.
>
> two comments below
>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>>  .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
>>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/chemical/Kconfig                       |  15 ++
>>  drivers/iio/chemical/Makefile                      |   6 +
>>  drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
>>  7 files changed, 299 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>>  create mode 100644 drivers/iio/chemical/Kconfig
>>  create mode 100644 drivers/iio/chemical/Makefile
>>  create mode 100644 drivers/iio/chemical/vz89x.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> new file mode 100644
>> index 0000000..74f2a35
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> @@ -0,0 +1,30 @@
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied CO2 gas percentage
>> +             value.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the raw calibration VOC value from the sensor.
>> +             This value has little application outside of calibration.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied total VOC gas
>> +             percentage value.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_resistance_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled sensor resistance value.
>> 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/Kconfig b/drivers/iio/chemical/Kconfig
>> new file mode 100644
>> index 0000000..8fb44e5
>> --- /dev/null
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -0,0 +1,15 @@
>> +#
>> +# Chemical sensors
>> +#
>> +
>> +menu "AQI VOC sensors"
>
> what does AQI mean?

Air Quality Index
>
>> +
>> +config VZ89X
>> +     tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>> +     depends on I2C
>> +     help
>> +       Say Y here to build I2C interface support for the SGX
>> +       Sensortech MiCS VZ89X VOC (Volatile Organic Compounds)
>> +       sensors
>> +
>> +endmenu
>> 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..847f853
>> --- /dev/null
>> +++ b/drivers/iio/chemical/vz89x.c
>> @@ -0,0 +1,245 @@
>> +/*
>> + * 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_VOC_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,
>> +             .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),
>> +             .address = VZ89X_VOC_SHORT_IDX,
>> +             .extend_name = "VOC_short",
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +                     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_VOC_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->buffer[i] = ret;
>> +     }
>> +
>> +     data->last_update = jiffies;
>> +
>> +     return 0;
>> +}
>> +
>> +static int vz89x_get_resistance_reading(struct vz89x_data *data)
>> +{
>> +     u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
>> +
>> +     return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);
>
> the typecasts are just weird, don't do it; the code works fine without them
> please check integer promotion in C, e.g.
> https://www.securecoding.cert.org/confluence/display/c/INT02-C.+Understand+integer+conversion+rules
>
>> +}
>> +
>> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> +                                int *val, int *val2)
>> +{
>> +     switch (chan->address) {
>> +     case VZ89X_VOC_CO2_IDX:
>> +             *val = 1600;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     case VZ89X_VOC_TVOC_IDX:
>> +             *val = 1000;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +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);
>> +             mutex_unlock(&data->lock);
>> +
>> +             if (ret)
>> +                     return ret;
>> +
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
>> +                     return IIO_VAL_INT_PLUS_MICRO;
>> +             case VZ89X_VOC_SHORT_IDX:
>> +                     *val = data->buffer[chan->address];
>> +                     return IIO_VAL_INT;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_RESISTANCE_IDX:
>> +                     *val = vz89x_get_resistance_reading(data);
>> +                     return IIO_VAL_INT;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             switch (chan->type) {
>> +             case IIO_RESISTANCE:
>> +                     *val = 10;
>> +                     return IIO_VAL_INT;
>> +             case IIO_CONCENTRATION:
>> +                     return vz89x_get_channel_scale(chan, val, val2);
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = 44250;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = -13;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +
>> +     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)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-10  9:08   ` Peter Meerwald
  2015-09-10 16:16     ` Matt Ranostay
@ 2015-09-11  2:13     ` Matt Ranostay
  1 sibling, 0 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-11  2:13 UTC (permalink / raw)
  To: Peter Meerwald
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio@vger.kernel.org

On Thu, Sep 10, 2015 at 2:08 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
>> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> percentage which can be converted to part per million.
>
> two comments below
>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>>  .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
>>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/chemical/Kconfig                       |  15 ++
>>  drivers/iio/chemical/Makefile                      |   6 +
>>  drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
>>  7 files changed, 299 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>>  create mode 100644 drivers/iio/chemical/Kconfig
>>  create mode 100644 drivers/iio/chemical/Makefile
>>  create mode 100644 drivers/iio/chemical/vz89x.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> new file mode 100644
>> index 0000000..74f2a35
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> @@ -0,0 +1,30 @@
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied CO2 gas percentage
>> +             value.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the raw calibration VOC value from the sensor.
>> +             This value has little application outside of calibration.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied total VOC gas
>> +             percentage value.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_resistance_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled sensor resistance value.
>> 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/Kconfig b/drivers/iio/chemical/Kconfig
>> new file mode 100644
>> index 0000000..8fb44e5
>> --- /dev/null
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -0,0 +1,15 @@
>> +#
>> +# Chemical sensors
>> +#
>> +
>> +menu "AQI VOC sensors"
>
> what does AQI mean?
>
>> +
>> +config VZ89X
>> +     tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>> +     depends on I2C
>> +     help
>> +       Say Y here to build I2C interface support for the SGX
>> +       Sensortech MiCS VZ89X VOC (Volatile Organic Compounds)
>> +       sensors
>> +
>> +endmenu
>> 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..847f853
>> --- /dev/null
>> +++ b/drivers/iio/chemical/vz89x.c
>> @@ -0,0 +1,245 @@
>> +/*
>> + * 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_VOC_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,
>> +             .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),
>> +             .address = VZ89X_VOC_SHORT_IDX,
>> +             .extend_name = "VOC_short",
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +                     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_VOC_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->buffer[i] = ret;
>> +     }
>> +
>> +     data->last_update = jiffies;
>> +
>> +     return 0;
>> +}
>> +
>> +static int vz89x_get_resistance_reading(struct vz89x_data *data)
>> +{
>> +     u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
>> +
>> +     return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);
>
> the typecasts are just weird, don't do it; the code works fine without them
> please check integer promotion in C, e.g.
> https://www.securecoding.cert.org/confluence/display/c/INT02-C.+Understand+integer+conversion+rules
>
Guess I have paranoid view of compilers. But yes this makes sense will change.

>> +}
>> +
>> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> +                                int *val, int *val2)
>> +{
>> +     switch (chan->address) {
>> +     case VZ89X_VOC_CO2_IDX:
>> +             *val = 1600;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     case VZ89X_VOC_TVOC_IDX:
>> +             *val = 1000;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +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);
>> +             mutex_unlock(&data->lock);
>> +
>> +             if (ret)
>> +                     return ret;
>> +
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
>> +                     return IIO_VAL_INT_PLUS_MICRO;
>> +             case VZ89X_VOC_SHORT_IDX:
>> +                     *val = data->buffer[chan->address];
>> +                     return IIO_VAL_INT;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_RESISTANCE_IDX:
>> +                     *val = vz89x_get_resistance_reading(data);
>> +                     return IIO_VAL_INT;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             switch (chan->type) {
>> +             case IIO_RESISTANCE:
>> +                     *val = 10;
>> +                     return IIO_VAL_INT;
>> +             case IIO_CONCENTRATION:
>> +                     return vz89x_get_channel_scale(chan, val, val2);
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = 44250;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = -13;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +
>> +     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)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type
  2015-09-10  6:30 ` [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type Matt Ranostay
@ 2015-09-12  9:16   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2015-09-12  9:16 UTC (permalink / raw)
  To: Matt Ranostay, lars, pmeerw; +Cc: linux-iio

On 10/09/15 07:30, 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 that returns a percentage
> of substance because no other channels types fit this use case.
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Other than a typo inline looks good to me.
I think you can drop the RFC now :)
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/uapi/linux/iio/types.h          | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 42d360f..48080b7 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1459,3 +1459,10 @@ 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_concentration_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
> +KernelVersion:	4.3
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) precentage reading of a substance.
percentage
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index b3fcc2c..58a60a1 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[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 2f8b117..c5a0e3f 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 {
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE channel type
  2015-09-10  6:30 ` [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE " Matt Ranostay
@ 2015-09-12  9:19   ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2015-09-12  9:19 UTC (permalink / raw)
  To: Matt Ranostay, lars, pmeerw; +Cc: linux-iio

On 10/09/15 07:30, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/uapi/linux/iio/types.h          | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 48080b7..0f683ed 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1466,3 +1466,10 @@ KernelVersion:	4.3
>  Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Raw (unscaled no offset etc.) precentage reading of a substance.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/in_resistanceX_raw
> +KernelVersion:	4.3
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) resistance reading in ohms.
Actually a comment that applies to the previous patch as well. 
The reading should only be in ohms after the application offset and scale. 
Note how that is described for some of the other attributes.
E.g. Units after the application of offset and scale are ohms.

If you want to output directly in ohms (typically either because the hardware
actually outputs in the relevant unit - or the transform is non linear) then
use the processed version (rather confusingly - we lifted it from hwmon, 
called _input which if is probably the worst bit of our ABI with hindsight!)


> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 58a60a1..d61a363 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 c5a0e3f..d58319c 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 {
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-10  6:30 ` [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support Matt Ranostay
  2015-09-10  9:08   ` Peter Meerwald
@ 2015-09-12  9:34   ` Jonathan Cameron
  2015-09-12 23:11     ` Matt Ranostay
  1 sibling, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2015-09-12  9:34 UTC (permalink / raw)
  To: Matt Ranostay, lars, pmeerw; +Cc: linux-iio

On 10/09/15 07:30, Matt Ranostay wrote:
> Add support for VZ89X sensors VOC and CO2 reporting channels in
> percentage which can be converted to part per million.
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Coming along nicely.  My main comment in here is that the chemical type
can easily enough be a modifier (except for the odd 'short' case)
Hence I think they should be.
> ---
>  .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/chemical/Kconfig                       |  15 ++
>  drivers/iio/chemical/Makefile                      |   6 +
>  drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
>  7 files changed, 299 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>  create mode 100644 drivers/iio/chemical/Kconfig
>  create mode 100644 drivers/iio/chemical/Makefile
>  create mode 100644 drivers/iio/chemical/vz89x.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
> new file mode 100644
> index 0000000..74f2a35
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
> @@ -0,0 +1,30 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled, and no offset applied CO2 gas percentage
> +		value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the raw calibration VOC value from the sensor.
> +		This value has little application outside of calibration.
So is this something that people are likely to need direct access to?
Perhaps it is better placed in debugfs.
If not I'd be tempted to do this one with a modifier of IIO_MOD_VOC and
because it's a wierd bit of non standard abi use an extended name to
say what it is.

(comments about using modifiers later in review).
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled, and no offset applied total VOC gas
> +		percentage value.
I'd imagine anyone would assume that an undecorated VOC would be the total
so I'd drop the t designation.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_resistance_raw
> +Date:		September 2015
> +KernelVersion:	4.3
> +Contact:	Matt Ranostay <mranostay@gmail.com>
> +Description:
> +		Get the unscaled sensor resistance value.
You've already documented this in the main ABI.  Doesn't need to be here.

Actually if you add modifiers for C02 / VOC then they should be in the main
ABI docs rather than here.  I'd imagine you'll just end up with the
'special' raw VOC reading in this doc.

> 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/Kconfig b/drivers/iio/chemical/Kconfig
> new file mode 100644
> index 0000000..8fb44e5
> --- /dev/null
> +++ b/drivers/iio/chemical/Kconfig
> @@ -0,0 +1,15 @@
> +#
> +# Chemical sensors
> +#
> +
> +menu "AQI VOC sensors"
As Peter picked up on, expand your acronyms to make life easy for readers.
I'm going to hazard a guess that at we'll end up with a more generic
menu name to cover the other types of related sensors but that can change
when it needs to.

> +
> +config VZ89X
> +	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
> +	depends on I2C
> +	help
> +	  Say Y here to build I2C interface support for the SGX
> +	  Sensortech MiCS VZ89X VOC (Volatile Organic Compounds) 
> +	  sensors 
> +
> +endmenu
> 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..847f853
> --- /dev/null
> +++ b/drivers/iio/chemical/vz89x.c
> @@ -0,0 +1,245 @@
> +/*
> + * 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_VOC_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,
> +		.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",
I think that the gas types we are likely to seen sensors for are actually
relatively few in number so we should effectively enforce the ABI by
using modifiers for these.

IIO_MOD_CO2
IIO_MOD_VOC
and for the _short one that's wierd and likely to be device specific so
using an extended name on top of the VOC modifier is fine.

The other big reason for this is when you get a threshold sensor for these
(I'm sure one will be along at somepoint).  The event codes do not encode
the extended name in anyway so both the C02 and VOC sensors would give
the same code.  Use a modifier and it all becomes straight forward.

Note that adding modifiers also includes updating the bits in tools/iio
to understand the new modifiers.

> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.address = VZ89X_VOC_SHORT_IDX,
> +		.extend_name = "VOC_short",
> +	},
> +	{
> +		.type = IIO_CONCENTRATION,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +			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_VOC_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->buffer[i] = ret;
> +	}
> +
> +	data->last_update = jiffies;
> +
> +	return 0;
> +}
> +
> +static int vz89x_get_resistance_reading(struct vz89x_data *data)
> +{
> +	u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
> +
> +	return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);
> +}
> +
> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
> +				   int *val, int *val2)
> +{
> +	switch (chan->address) {
> +	case VZ89X_VOC_CO2_IDX:
> +		*val = 1600;
> +		*val2 = 229;
> +		return IIO_VAL_FRACTIONAL;
> +	case VZ89X_VOC_TVOC_IDX:
> +		*val = 1000;
> +		*val2 = 229;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +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);
> +		mutex_unlock(&data->lock);
> +
> +		if (ret)
> +			return ret;
> +
> +		switch (chan->address) {
> +		case VZ89X_VOC_CO2_IDX:
> +			*val = 0;
> +			*val2 = data->buffer[chan->address];
> +			return IIO_VAL_INT_PLUS_MICRO;

This is unusual for a raw reading.  Normally you'd output it as
IIO_VAL_INT and take up the device by 1e6 in the scale factor.
(mainly because you can only output integer values via the buffered
interface - but it's become a convention in general).
I wouldn't guarantee all tools will cope with it otherwise.
> +		case VZ89X_VOC_SHORT_IDX:
> +			*val = data->buffer[chan->address];
> +			return IIO_VAL_INT;
> +		case VZ89X_VOC_TVOC_IDX:
> +			*val = 0;
> +			*val2 = data->buffer[chan->address];
Same is true for this one.
> +			return IIO_VAL_INT_PLUS_NANO;
> +		case VZ89X_VOC_RESISTANCE_IDX:
> +			*val = vz89x_get_resistance_reading(data);
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_RESISTANCE:
> +			*val = 10;
> +			return IIO_VAL_INT;
> +		case IIO_CONCENTRATION:
> +			return vz89x_get_channel_scale(chan, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->address) {
> +		case VZ89X_VOC_CO2_IDX:
> +			*val = 0;
> +			*val2 = 44250;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		case VZ89X_VOC_TVOC_IDX:
> +			*val = 0;
> +			*val2 = -13;
> +			return IIO_VAL_INT_PLUS_NANO;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	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");
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support
  2015-09-12  9:34   ` Jonathan Cameron
@ 2015-09-12 23:11     ` Matt Ranostay
  0 siblings, 0 replies; 12+ messages in thread
From: Matt Ranostay @ 2015-09-12 23:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Peter Meerwald, linux-iio@vger.kernel.org

On Sat, Sep 12, 2015 at 2:34 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 10/09/15 07:30, Matt Ranostay wrote:
>> Add support for VZ89X sensors VOC and CO2 reporting channels in
>> percentage which can be converted to part per million.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> Coming along nicely.  My main comment in here is that the chemical type
> can easily enough be a modifier (except for the odd 'short' case)
> Hence I think they should be.
>> ---
>>  .../ABI/testing/sysfs-bus-iio-chemical-vz89x       |  30 +++
>>  .../devicetree/bindings/i2c/trivial-devices.txt    |   1 +
>>  drivers/iio/Kconfig                                |   1 +
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/chemical/Kconfig                       |  15 ++
>>  drivers/iio/chemical/Makefile                      |   6 +
>>  drivers/iio/chemical/vz89x.c                       | 245 +++++++++++++++++++++
>>  7 files changed, 299 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>>  create mode 100644 drivers/iio/chemical/Kconfig
>>  create mode 100644 drivers/iio/chemical/Makefile
>>  create mode 100644 drivers/iio/chemical/vz89x.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> new file mode 100644
>> index 0000000..74f2a35
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-chemical-vz89x
>> @@ -0,0 +1,30 @@
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_CO2_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied CO2 gas percentage
>> +             value.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_VOC_short_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the raw calibration VOC value from the sensor.
>> +             This value has little application outside of calibration.
> So is this something that people are likely to need direct access to?
> Perhaps it is better placed in debugfs.
> If not I'd be tempted to do this one with a modifier of IIO_MOD_VOC and
> because it's a wierd bit of non standard abi use an extended name to
> say what it is.

Ok makes sense. Will update in v5

>
> (comments about using modifiers later in review).
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_concentration_tVOC_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled, and no offset applied total VOC gas
>> +             percentage value.
> I'd imagine anyone would assume that an undecorated VOC would be the total
> so I'd drop the t designation.
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_resistance_raw
>> +Date:                September 2015
>> +KernelVersion:       4.3
>> +Contact:     Matt Ranostay <mranostay@gmail.com>
>> +Description:
>> +             Get the unscaled sensor resistance value.
> You've already documented this in the main ABI.  Doesn't need to be here.
>
> Actually if you add modifiers for C02 / VOC then they should be in the main
> ABI docs rather than here.  I'd imagine you'll just end up with the
> 'special' raw VOC reading in this doc.
>
>> 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/Kconfig b/drivers/iio/chemical/Kconfig
>> new file mode 100644
>> index 0000000..8fb44e5
>> --- /dev/null
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -0,0 +1,15 @@
>> +#
>> +# Chemical sensors
>> +#
>> +
>> +menu "AQI VOC sensors"
> As Peter picked up on, expand your acronyms to make life easy for readers.
> I'm going to hazard a guess that at we'll end up with a more generic
> menu name to cover the other types of related sensors but that can change
> when it needs to.
Ok got it.

>
>> +
>> +config VZ89X
>> +     tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>> +     depends on I2C
>> +     help
>> +       Say Y here to build I2C interface support for the SGX
>> +       Sensortech MiCS VZ89X VOC (Volatile Organic Compounds)
>> +       sensors
>> +
>> +endmenu
>> 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..847f853
>> --- /dev/null
>> +++ b/drivers/iio/chemical/vz89x.c
>> @@ -0,0 +1,245 @@
>> +/*
>> + * 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_VOC_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,
>> +             .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",
> I think that the gas types we are likely to seen sensors for are actually
> relatively few in number so we should effectively enforce the ABI by
> using modifiers for these.
>
> IIO_MOD_CO2
> IIO_MOD_VOC
> and for the _short one that's wierd and likely to be device specific so
> using an extended name on top of the VOC modifier is fine.
>
> The other big reason for this is when you get a threshold sensor for these
> (I'm sure one will be along at somepoint).  The event codes do not encode
> the extended name in anyway so both the C02 and VOC sensors would give
> the same code.  Use a modifier and it all becomes straight forward.
>
> Note that adding modifiers also includes updating the bits in tools/iio
> to understand the new modifiers.
>
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +             .address = VZ89X_VOC_SHORT_IDX,
>> +             .extend_name = "VOC_short",
>> +     },
>> +     {
>> +             .type = IIO_CONCENTRATION,
>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +                     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_VOC_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->buffer[i] = ret;
>> +     }
>> +
>> +     data->last_update = jiffies;
>> +
>> +     return 0;
>> +}
>> +
>> +static int vz89x_get_resistance_reading(struct vz89x_data *data)
>> +{
>> +     u8 *buf = &data->buffer[VZ89X_VOC_TVOC_IDX];
>> +
>> +     return buf[0] | ((u16)buf[1] << 8) | ((u32)buf[2] << 16);
>> +}
>> +
>> +static int vz89x_get_channel_scale(struct iio_chan_spec const *chan,
>> +                                int *val, int *val2)
>> +{
>> +     switch (chan->address) {
>> +     case VZ89X_VOC_CO2_IDX:
>> +             *val = 1600;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     case VZ89X_VOC_TVOC_IDX:
>> +             *val = 1000;
>> +             *val2 = 229;
>> +             return IIO_VAL_FRACTIONAL;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +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);
>> +             mutex_unlock(&data->lock);
>> +
>> +             if (ret)
>> +                     return ret;
>> +
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
>> +                     return IIO_VAL_INT_PLUS_MICRO;
>
> This is unusual for a raw reading.  Normally you'd output it as
> IIO_VAL_INT and take up the device by 1e6 in the scale factor.
> (mainly because you can only output integer values via the buffered
> interface - but it's become a convention in general).
> I wouldn't guarantee all tools will cope with it otherwise.

Yeah the reason i did it this way is calculation is    (value - 13) *
scale + offset.    But I probably could change the maths around to
make this more sane.

Also got my hardware today for this today. So I can test driver out finally..

>> +             case VZ89X_VOC_SHORT_IDX:
>> +                     *val = data->buffer[chan->address];
>> +                     return IIO_VAL_INT;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = data->buffer[chan->address];
> Same is true for this one.
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_RESISTANCE_IDX:
>> +                     *val = vz89x_get_resistance_reading(data);
>> +                     return IIO_VAL_INT;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             switch (chan->type) {
>> +             case IIO_RESISTANCE:
>> +                     *val = 10;
>> +                     return IIO_VAL_INT;
>> +             case IIO_CONCENTRATION:
>> +                     return vz89x_get_channel_scale(chan, val, val2);
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case IIO_CHAN_INFO_OFFSET:
>> +             switch (chan->address) {
>> +             case VZ89X_VOC_CO2_IDX:
>> +                     *val = 0;
>> +                     *val2 = 44250;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             case VZ89X_VOC_TVOC_IDX:
>> +                     *val = 0;
>> +                     *val2 = -13;
>> +                     return IIO_VAL_INT_PLUS_NANO;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +
>> +     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");
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-09-12 23:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-10  6:30 [RFC v4 0/4] iio: new chemical sensor framework and channel types Matt Ranostay
2015-09-10  6:30 ` [RFC v4 1/4] iio: chemical: Add IIO_CONCENTRATION channel type Matt Ranostay
2015-09-12  9:16   ` Jonathan Cameron
2015-09-10  6:30 ` [RFC v4 2/4] iio: resistance: add IIO_RESISTANCE " Matt Ranostay
2015-09-12  9:19   ` Jonathan Cameron
2015-09-10  6:30 ` [RFC v4 3/4] devicetree: add SGX Sensortech vendor id Matt Ranostay
2015-09-10  6:30 ` [RFC v4 4/4] iio: chemical: add SGX VZ89x VOC sensor support Matt Ranostay
2015-09-10  9:08   ` Peter Meerwald
2015-09-10 16:16     ` Matt Ranostay
2015-09-11  2:13     ` Matt Ranostay
2015-09-12  9:34   ` Jonathan Cameron
2015-09-12 23:11     ` Matt Ranostay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).