Skip to contentSkip to conversation

iio: chemical: add support for Dynament Premier series single gas sensor

PATCH Superseded ruantu 2 participants 2 messages Dec 10, 2019 – Dec 15, 2019 IIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

ruantu

author
#0 lore raw

PATCH 1/2 iio: chemical: add support for Dynament Premier series single gas sensor

Add support for Dynament Premier series single gas sensor.

---
 MAINTAINERS                    |   5 +
 drivers/iio/chemical/Kconfig   |  11 ++
 drivers/iio/chemical/Makefile  |   1 +
 drivers/iio/chemical/premier.c | 349 +++++++++++++++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 drivers/iio/chemical/premier.c
MAINTAINERS +5 -0
index 02d5278a4c9a..18c26558ddfe 100644
@@ -13133,6 +13133,11 @@ F:	drivers/i2c/busses/i2c-puv3.c
 F:	drivers/video/fbdev/fb-puv3.c
 F:	drivers/rtc/rtc-puv3.c
 
+DYNAMENT PREMIER SERIES SINGLE GAS SENSOR DRIVER
+M:	ruantu <[email protected]>
+S:	Maintained
+F:	drivers/iio/chemical/premier.c
+
 PLANTOWER PMS7003 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <[email protected]>
 S:	Maintained
drivers/iio/chemical/Kconfig +11 -0
index fa4586037bb8..b75903a5a15c 100644
@@ -62,6 +62,17 @@ config IAQCORE
 	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
 	  sensors
 
+config PREMIER
+	tristate "Dynament Premier series sensor"
+	depends on SERIAL_DEV_BUS
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say Y here to build support for the Dynament Premier
+	  series sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called premier.
+
 config PMS7003
 	tristate "Plantower PMS7003 particulate matter sensor"
 	depends on SERIAL_DEV_BUS
drivers/iio/chemical/Makefile +1 -0
index f97270bc4034..c8e779d7cf4a 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_BME680_I2C) += bme680_i2c.o
 obj-$(CONFIG_BME680_SPI) += bme680_spi.o
 obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
+obj-$(CONFIG_PREMIER)		+= premier.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
drivers/iio/chemical/premier.c new file +349 -0

349 lines not on this page · raw message

Signature · 1 line
2.24.0
maintainer
#1 lore raw
Quoted 3 lines · On Tue, 10 Dec 2019 13:37:21 +0800 ruantu <[email protected]> wrote:

Add support for Dynament Premier series single gas sensor.

Hi ruantu,

Welcome to IIO.

Just to check, could you confirm your signed-off-by meets the requirements of Documentation/process/submitting-patches.rst wrt to use your full name? If it does, then my apologies!

Looks pretty good. A few minor cleanups needed though. See inline.

Thanks,

Jonathan

Quoted 13 lines
---
 MAINTAINERS                    |   5 +
 drivers/iio/chemical/Kconfig   |  11 ++
 drivers/iio/chemical/Makefile  |   1 +
 drivers/iio/chemical/premier.c | 349 +++++++++++++++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 drivers/iio/chemical/premier.c
MAINTAINERS +0 -0
index 02d5278a4c9a..18c26558ddfe 100644

MAINTAINERS F: drivers/i2c/busses/i2c-puv3.c · line 13139

Quoted 7 lines
MAINTAINERS +4 -0
@@ -13133,6 +13133,11 @@ F:	drivers/i2c/busses/i2c-puv3.c
 F:	drivers/video/fbdev/fb-puv3.c
 F:	drivers/rtc/rtc-puv3.c
 
+DYNAMENT PREMIER SERIES SINGLE GAS SENSOR DRIVER
+M:	ruantu <[email protected]>
+S:	Maintained
+F:	drivers/iio/chemical/premier.c

MAINTAINERS is in alphabetical order, not grouped by location.

Quoted 9 lines
MAINTAINERS +1 -0
+
 PLANTOWER PMS7003 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <[email protected]>
 S:	Maintained
drivers/iio/chemical/Kconfig +0 -0
index fa4586037bb8..b75903a5a15c 100644

drivers/iio/chemical/Kconfig config IAQCORE · line 68

Quoted 7 lines
drivers/iio/chemical/Kconfig +4 -0
@@ -62,6 +62,17 @@ config IAQCORE
 	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
 	  sensors
 
+config PREMIER
+	tristate "Dynament Premier series sensor"
+	depends on SERIAL_DEV_BUS
+	select IIO_TRIGGERED_BUFFER

It looks to me like you started adding triggered buffer support here but haven't finished. For this patch, make sure you clean out any initial support. That can all come in a follow up patch.

Quoted 41 lines
drivers/iio/chemical/Kconfig +7 -0
+	help
+	  Say Y here to build support for the Dynament Premier
+	  series sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called premier.
+
 config PMS7003
 	tristate "Plantower PMS7003 particulate matter sensor"
 	depends on SERIAL_DEV_BUS
drivers/iio/chemical/Makefile +1 -0
index f97270bc4034..c8e779d7cf4a 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_BME680_I2C) += bme680_i2c.o
 obj-$(CONFIG_BME680_SPI) += bme680_spi.o
 obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
+obj-$(CONFIG_PREMIER)		+= premier.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
drivers/iio/chemical/premier.c new file +13 -0
new file mode 100644
index 000000000000..5681e2caa03b
@@ -0,0 +1,349 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Dynament Premier series single gas sensor driver
+ *
+ * Copyright (c) ruantu <[email protected]>
+ */
+
+#include <asm/unaligned.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>

drivers/iio/chemical/premier.c line 23

Quoted 10 lines
drivers/iio/chemical/premier.c new file +10 -0
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/serdev.h>
+
+#define PREMIER_DRIVER_NAME "premier"

This is a bit of a generic name, perhaps
dynament-premier?

drivers/iio/chemical/premier.c line 25

Quoted 2 lines
drivers/iio/chemical/premier.c new file +2 -0
+
+#define DLE (0x10)

Please prefix these defines. The chance of them clashing with something in a generic header sometime in the future is too high.

PREMIER_CMD_RD etc.

Quoted 115 lines
drivers/iio/chemical/premier.c new file +115 -0
+#define CMD_RD (0x13)
+#define CMD_NAK (0x19)
+#define CMD_DAT (0x1a)
+#define EOFF (0x1f)
+
+#define PREMIER_TIMEOUT msecs_to_jiffies(6000)
+
+/*
+ * commands have following format:
+ *
+ * +-----+-----+---------+-----+-----+-----------+-----------+
+ * | DLE | CMD | PAYLOAD | DLE | EOF | CKSUM MSB | CKSUM LSB |
+ * +-----+-----+---------+-----+-----+-----------+-----------+
+ */
+static const u8 premier_cmd_read_live_data_simple[] = {
+	0x10, 0x13, 0x06, 0x10, 0x1F, 0x00, 0x58
+};
+
+struct premier_frame {
+	u8 state;
+	u8 is_dat;
+	u8 is_nak;
+	u8 data_len;
+	u8 vi, si, gi, gj;
+	u8 gas[4];
+	u8 byte_stuffing;
+	u8 checksum_received[2];
+	u16 checksum_calculated;
+};
+
+struct premier_state {
+	struct serdev_device *serdev;
+	struct premier_frame frame;
+	struct completion frame_ready;
+	struct mutex lock;	/* must be held whenever state gets touched */
+};
+
+static int premier_do_cmd_read_live_data(struct premier_state *state)
+{
+	int ret;
+
+	ret =
+	    serdev_device_write(state->serdev,
+				premier_cmd_read_live_data_simple,
+				sizeof(premier_cmd_read_live_data_simple),
+				PREMIER_TIMEOUT);
+	if (ret < sizeof(premier_cmd_read_live_data_simple))
+		return ret < 0 ? ret : -EIO;
+
+	ret = wait_for_completion_interruptible_timeout(&state->frame_ready,
+							PREMIER_TIMEOUT);
+
+	if (!ret)
+		ret = -ETIMEDOUT;
+
+	return ret < 0 ? ret : 0;
+}
+
+static s32 premier_float_to_int_clamped(const u8 *fp)
+{
+	int val = get_unaligned_le32(fp);
+	int mantissa = val & GENMASK(22, 0);
+	/* this is fine since passed float is always non-negative */
+	int exp = val >> 23;
+	int fraction, shift;
+
+	/* special case 0 */
+	if (!exp && !mantissa)
+		return 0;
+
+	exp -= 127;
+	if (exp < 0) {
+		/* return values ranging from 1 to 99 */
+		return ((((1 << 23) + mantissa) * 100) >> 23) >> (-exp);
+	}
+
+	/* return values ranging from 100 to int_max */
+	shift = 23 - exp;
+	val = (1 << exp) + (mantissa >> shift);
+
+	fraction = mantissa & GENMASK(shift - 1, 0);
+
+	return val * 100 + ((fraction * 100) >> shift);
+}
+
+static int premier_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct premier_state *state = iio_priv(indio_dev);
+	struct premier_frame *frame = &state->frame;
+	int ret;
+	s32 val_tmp;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+
+		mutex_lock(&state->lock);
+		ret = premier_do_cmd_read_live_data(state);
+		if (ret) {
+			mutex_unlock(&state->lock);
+			return ret;
+		}
+		val_tmp = premier_float_to_int_clamped(frame->gas);
+		mutex_unlock(&state->lock);
+
+		*val = val_tmp / 100;
+		*val2 = (val_tmp % 100) * 10000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}

drivers/iio/chemical/premier.c line 150

Quoted 10 lines
drivers/iio/chemical/premier.c new file +10 -0
+
+static const struct iio_info premier_info = {
+	.read_raw = premier_read_raw,
+};
+
+static const struct iio_chan_spec premier_channels[] = {
+	{
+	 .type = IIO_CONCENTRATION,
+	 .channel = 1,
+	 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),

The indentation in here needs fixing up.

Also, could you confirm what the units of this reading are and compare with the Documented IIO base units in Documentation/ABI/testings/sysfs-bus-iio-*

We have a nasty habit of getting inconsistent ones in new drivers so good to check!

Quoted 15 lines
drivers/iio/chemical/premier.c new file +15 -0
+	 .scan_index = -1,
+	  },
+	IIO_CHAN_SOFT_TIMESTAMP(0),
+};
+
+static int premier_receive_buf(struct serdev_device *serdev,
+			       const unsigned char *buf, size_t size)
+{
+	struct iio_dev *indio_dev = serdev_device_get_drvdata(serdev);
+	struct premier_state *state = iio_priv(indio_dev);
+	struct premier_frame *frame = &state->frame;
+	int i;
+
+	for (i = 0; i < size; i++) {
+

drivers/iio/chemical/premier.c line 175

Quoted 10 lines
drivers/iio/chemical/premier.c new file +10 -0
+		if (frame->state > 0 && frame->state <= 7)
+			frame->checksum_calculated += buf[i];
+
+		switch (frame->state) {
+		case 0:
+			if (buf[i] == DLE) {
+				frame->is_dat = 0;
+				frame->is_nak = 0;
+				frame->checksum_calculated = buf[i];
+				/* we don't initialize checksum_calculated in

Multiline comment syntax in most of the kernel (including IIO) is

				/*
				 * We don't...
Quoted 122 lines
drivers/iio/chemical/premier.c new file +122 -0
+				 * the last state in case we didn't go
+				 * there because of noise
+				 */
+				frame->state++;
+			}
+			break;
+		case 1:
+			/*
+			 * if noise corrupts a byte in the FSM sequence,
+			 * we loop between state 0 and 1,
+			 * until we have a valid sequence of DLE&DAT or DLE&NAK
+			 */
+			if (buf[i] == CMD_DAT) {
+				frame->is_dat = 1;
+				frame->state++;
+			} else if (buf[i] == CMD_NAK) {
+				frame->is_nak = 1;
+				frame->state++;
+			} else
+				frame->state = 0;
+			break;
+		case 2:
+			if (frame->is_nak)
+				frame->state = 0;
+			else if (frame->is_dat) {
+				frame->data_len = buf[i] - 4;
+				/* remove version and status bytes from count */
+				if (frame->data_len < 4)
+					frame->state = 0;
+				/* we check for the upper limit in state 5 */
+				else
+					frame->state++;
+			} else
+				frame->state = 0;
+			break;
+		case 3:
+			/* just do nothing for 2 rounds to bypass
+			 * the 2 version bytes
+			 */
+			if (frame->vi < 2 - 1)
+				frame->vi++;
+			else {
+				frame->vi = 0;
+				frame->state++;
+			}
+			break;
+		case 4:
+			if (frame->si < 2 - 1)
+				frame->si++;
+			else {
+				frame->si = 0;
+				frame->state++;
+			}
+			break;
+		case 5:
+			if (frame->gi < frame->data_len - 1) {
+				if (buf[i] != 0x10 || frame->byte_stuffing) {
+					frame->gas[frame->gj] = buf[i];
+					frame->byte_stuffing = 0;
+					frame->gj++;
+					if (frame->gj >= 4)
+						frame->state = 0;
+					/* don't violate array limits
+					 * if data_len corrupt
+					 */
+				} else
+					frame->byte_stuffing = 1;
+				frame->gi++;
+			} else {
+				frame->gas[frame->gj] = buf[i];
+				frame->byte_stuffing = 0;
+				frame->gi = 0;
+				frame->gj = 0;
+				frame->state++;
+			}
+			break;
+		case 6:
+			if (buf[i] == DLE)
+				frame->state++;
+			else
+				frame->state = 0;
+			break;
+		case 7:
+			if (buf[i] == EOFF)
+				frame->state++;
+			else
+				frame->state = 0;
+			break;
+		case 8:
+			frame->checksum_received[1] = buf[i];
+
+			frame->state++;
+			break;
+		case 9:
+			frame->checksum_received[0] = buf[i];
+
+			if (frame->checksum_calculated ==
+			    get_unaligned_le16(frame->checksum_received))
+				complete(&state->frame_ready);
+
+			frame->state = 0;
+			break;
+		}
+	}
+
+	return size;
+}
+
+static const struct serdev_device_ops premier_serdev_ops = {
+	.receive_buf = premier_receive_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+static const unsigned long premier_scan_masks[] = { 0x07, 0x00 };
+
+static int premier_probe(struct serdev_device *serdev)
+{
+	struct premier_state *state;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&serdev->dev, sizeof(*state));

drivers/iio/chemical/premier.c line 307

Quoted 10 lines
drivers/iio/chemical/premier.c new file +10 -0
+	if (!indio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(indio_dev);
+	serdev_device_set_drvdata(serdev, indio_dev);
+	state->serdev = serdev;
+	indio_dev->dev.parent = &serdev->dev;
+	indio_dev->info = &premier_info;
+	indio_dev->name = PREMIER_DRIVER_NAME;
+	indio_dev->channels = premier_channels,

The line above should end in a semicolon and next one not be indented. Although they are related, kernel code conventions don't allow for multiple assignment in one line.

drivers/iio/chemical/premier.c line 310

Quoted 3 lines
drivers/iio/chemical/premier.c new file +3 -0
+	    indio_dev->num_channels = ARRAY_SIZE(premier_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = premier_scan_masks;

available_scan_masks is only used if you have buffered support which you don't seem to offer in this patch.

Quoted 11 lines
drivers/iio/chemical/premier.c new file +11 -0
+
+	mutex_init(&state->lock);
+	init_completion(&state->frame_ready);
+
+	serdev_device_set_client_ops(serdev, &premier_serdev_ops);
+	ret = devm_serdev_device_open(&serdev->dev, serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, 9600);
+	serdev_device_set_flow_control(serdev, false);

drivers/iio/chemical/premier.c line 331

Quoted 10 lines
drivers/iio/chemical/premier.c new file +10 -0
+
+	ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&serdev->dev, indio_dev);
+}
+
+static const struct of_device_id premier_of_match[] = {
+	{.compatible = "dynament,premier" },

Space needed before .compatible

drivers/iio/chemical/premier.c line 340

Quoted 9 lines
drivers/iio/chemical/premier.c new file +9 -0
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, premier_of_match);
+
+static struct serdev_device_driver premier_driver = {
+	.driver = {
+		   .name = PREMIER_DRIVER_NAME,
+		   .of_match_table = premier_of_match,

Something strange with indenting here. Normally structure members are indented a full extra tab from the structure. Here we seem to have som spaces.

Quoted 9 lines
drivers/iio/chemical/premier.c new file +9 -0
+		    },
+	.probe = premier_probe,
+};
+
+module_serdev_device_driver(premier_driver);
+
+MODULE_AUTHOR("ruantu <[email protected]>");
+MODULE_DESCRIPTION("Dynament Premier series single gas sensor driver");
+MODULE_LICENSE("GPL v2");