[PATCH v2 0/3] add support for Sensirion SPS30 PM sensor

[PATCH v2 0/3] add support for Sensirion SPS30 PM sensor

[Tomasz Duszynski]

This patch series adds support for Sensirion SPS30 particulate matter
sensor. Along with a driver itself, new channel type and modifiers
for distinguishing between coarse and fine particles measurements are
introduced.

Sensor datasheet can be downloaded from

https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/0_Datasheets/Particulate_Matter/Sensirion_PM_Sensors_SPS30_Datasheet.pdf

v2:
 * add support for PM1 and PM4 measurements
 * get rid of variable length arrays
 * do proper cleanup via devm_add_action_or_reset()
 * add extra level of precision (two decimal places) while displaying
   measurements
 * improve comments a little bit
 * slight refactoring and general improvements here and there

Tomasz Duszynski (3):
  iio: add IIO_MASSCONCENTRATION channel type
  iio: chemical: add support for Sensirion SPS30 sensor
  iio: chemical: sps30: add device tree support

 Documentation/ABI/testing/sysfs-bus-iio       |  17 +-
 .../bindings/iio/chemical/sensirion,sps30.txt |  12 +
 drivers/iio/chemical/Kconfig                  |  11 +
 drivers/iio/chemical/Makefile                 |   1 +
 drivers/iio/chemical/sps30.c                  | 406 ++++++++++++++++++
 drivers/iio/industrialio-core.c               |   5 +
 include/uapi/linux/iio/types.h                |   5 +
 tools/iio/iio_event_monitor.c                 |  10 +
 8 files changed, 466 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
 create mode 100644 drivers/iio/chemical/sps30.c

--
2.19.2

[PATCH v2 1/3] iio: add IIO_MASSCONCENTRATION channel type

[Tomasz Duszynski]

Measuring particulate matter in ug / m3 (micro-grams per cubic meter)
is de facto standard. Existing air quality sensors usually follow
this convention and are capable of returning measurements using
this unit.

IIO currently does not offer suitable channel type for this
type of measurements hence this patch adds this.

In addition, extra modifiers are introduced used for distinguishing
between fine pm1, pm2p5 and coarse pm4, pm10 particle measurements, i.e
IIO_MOD_PM1, IIO_MOD_PM25 and IIO_MOD_PM4, IIO_MOD_PM10.

pmX consists of particles with aerodynamic diameter less or equal to
X micrometers.

Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 17 ++++++++++++++++-
 drivers/iio/industrialio-core.c         |  5 +++++
 include/uapi/linux/iio/types.h          |  5 +++++
 tools/iio/iio_event_monitor.c           | 10 ++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 8127a08e366d..ad9579a9385f 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1684,4 +1684,19 @@ KernelVersion:	4.18
 Contact:	linux-iio@vger.kernel.org
 Description:
 		Raw (unscaled) phase difference reading from channel Y
-		that can be processed to radians.
\ No newline at end of file
+		that can be processed to radians.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm2p5_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm2p5_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm4_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm4_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm10_input
+What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm10_input
+KernelVersion:	4.21
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Mass concentration reading of particulate matter in ug / m3.
+		pmX consists of particles with aerodynamic diameter less or
+		equal to X micrometers.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index a062cfddc5af..cbc2c5c8bf36 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -87,6 +87,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_GRAVITY]  = "gravity",
 	[IIO_POSITIONRELATIVE]  = "positionrelative",
 	[IIO_PHASE] = "phase",
+	[IIO_MASSCONCENTRATION] = "massconcentration",
 };
 
 static const char * const iio_modifier_names[] = {
@@ -127,6 +128,10 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_Q] = "q",
 	[IIO_MOD_CO2] = "co2",
 	[IIO_MOD_VOC] = "voc",
+	[IIO_MOD_PM1] = "pm1",
+	[IIO_MOD_PM2P5] = "pm2p5",
+	[IIO_MOD_PM4] = "pm4",
+	[IIO_MOD_PM10] = "pm10",
 };
 
 /* relies on pairs of these shared then separate */
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 92baabc103ac..c59adac24b1c 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -46,6 +46,7 @@ enum iio_chan_type {
 	IIO_GRAVITY,
 	IIO_POSITIONRELATIVE,
 	IIO_PHASE,
+	IIO_MASSCONCENTRATION,
 };
 
 enum iio_modifier {
@@ -87,6 +88,10 @@ enum iio_modifier {
 	IIO_MOD_VOC,
 	IIO_MOD_LIGHT_UV,
 	IIO_MOD_LIGHT_DUV,
+	IIO_MOD_PM1,
+	IIO_MOD_PM2P5,
+	IIO_MOD_PM4,
+	IIO_MOD_PM10,
 };
 
 enum iio_event_type {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index ac2de6b7e89f..f6b8003fbe3c 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -60,6 +60,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_GRAVITY] = "gravity",
 	[IIO_POSITIONRELATIVE] = "positionrelative",
 	[IIO_PHASE] = "phase",
+	[IIO_MASSCONCENTRATION] = "massconcentration",
 };
 
 static const char * const iio_ev_type_text[] = {
@@ -115,6 +116,10 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_Q] = "q",
 	[IIO_MOD_CO2] = "co2",
 	[IIO_MOD_VOC] = "voc",
+	[IIO_MOD_PM1] = "pm1",
+	[IIO_MOD_PM2P5] = "pm2p5",
+	[IIO_MOD_PM4] = "pm4",
+	[IIO_MOD_PM10] = "pm10",
 };
 
 static bool event_is_known(struct iio_event_data *event)
@@ -156,6 +161,7 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_GRAVITY:
 	case IIO_POSITIONRELATIVE:
 	case IIO_PHASE:
+	case IIO_MASSCONCENTRATION:
 		break;
 	default:
 		return false;
@@ -200,6 +206,10 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_MOD_Q:
 	case IIO_MOD_CO2:
 	case IIO_MOD_VOC:
+	case IIO_MOD_PM1:
+	case IIO_MOD_PM2P5:
+	case IIO_MOD_PM4:
+	case IIO_MOD_PM10:
 		break;
 	default:
 		return false;
-- 
2.19.2

Re: [PATCH v2 1/3] iio: add IIO_MASSCONCENTRATION channel type

[Jonathan Cameron]

On Thu,  6 Dec 2018 22:04:41 +0100
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> Measuring particulate matter in ug / m3 (micro-grams per cubic meter)
> is de facto standard. Existing air quality sensors usually follow
> this convention and are capable of returning measurements using
> this unit.
> 
> IIO currently does not offer suitable channel type for this
> type of measurements hence this patch adds this.
> 
> In addition, extra modifiers are introduced used for distinguishing
> between fine pm1, pm2p5 and coarse pm4, pm10 particle measurements, i.e
> IIO_MOD_PM1, IIO_MOD_PM25 and IIO_MOD_PM4, IIO_MOD_PM10.
> 
> pmX consists of particles with aerodynamic diameter less or equal to
> X micrometers.
> 
> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
I'm happy with this, but would like to give the whole series
a little more time on the list to let those others who looked
at V1 have plenty of time for another look.

Thanks,

Jonathan

> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 17 ++++++++++++++++-
>  drivers/iio/industrialio-core.c         |  5 +++++
>  include/uapi/linux/iio/types.h          |  5 +++++
>  tools/iio/iio_event_monitor.c           | 10 ++++++++++
>  4 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 8127a08e366d..ad9579a9385f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1684,4 +1684,19 @@ KernelVersion:	4.18
>  Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Raw (unscaled) phase difference reading from channel Y
> -		that can be processed to radians.
> \ No newline at end of file
> +		that can be processed to radians.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm1_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm1_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm2p5_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm2p5_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm4_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm4_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentration_pm10_input
> +What:		/sys/bus/iio/devices/iio:deviceX/in_massconcentrationY_pm10_input
> +KernelVersion:	4.21
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Mass concentration reading of particulate matter in ug / m3.
> +		pmX consists of particles with aerodynamic diameter less or
> +		equal to X micrometers.
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index a062cfddc5af..cbc2c5c8bf36 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -87,6 +87,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_GRAVITY]  = "gravity",
>  	[IIO_POSITIONRELATIVE]  = "positionrelative",
>  	[IIO_PHASE] = "phase",
> +	[IIO_MASSCONCENTRATION] = "massconcentration",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> @@ -127,6 +128,10 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_Q] = "q",
>  	[IIO_MOD_CO2] = "co2",
>  	[IIO_MOD_VOC] = "voc",
> +	[IIO_MOD_PM1] = "pm1",
> +	[IIO_MOD_PM2P5] = "pm2p5",
> +	[IIO_MOD_PM4] = "pm4",
> +	[IIO_MOD_PM10] = "pm10",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 92baabc103ac..c59adac24b1c 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -46,6 +46,7 @@ enum iio_chan_type {
>  	IIO_GRAVITY,
>  	IIO_POSITIONRELATIVE,
>  	IIO_PHASE,
> +	IIO_MASSCONCENTRATION,
>  };
>  
>  enum iio_modifier {
> @@ -87,6 +88,10 @@ enum iio_modifier {
>  	IIO_MOD_VOC,
>  	IIO_MOD_LIGHT_UV,
>  	IIO_MOD_LIGHT_DUV,
> +	IIO_MOD_PM1,
> +	IIO_MOD_PM2P5,
> +	IIO_MOD_PM4,
> +	IIO_MOD_PM10,
>  };
>  
>  enum iio_event_type {
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index ac2de6b7e89f..f6b8003fbe3c 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -60,6 +60,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_GRAVITY] = "gravity",
>  	[IIO_POSITIONRELATIVE] = "positionrelative",
>  	[IIO_PHASE] = "phase",
> +	[IIO_MASSCONCENTRATION] = "massconcentration",
>  };
>  
>  static const char * const iio_ev_type_text[] = {
> @@ -115,6 +116,10 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_Q] = "q",
>  	[IIO_MOD_CO2] = "co2",
>  	[IIO_MOD_VOC] = "voc",
> +	[IIO_MOD_PM1] = "pm1",
> +	[IIO_MOD_PM2P5] = "pm2p5",
> +	[IIO_MOD_PM4] = "pm4",
> +	[IIO_MOD_PM10] = "pm10",
>  };
>  
>  static bool event_is_known(struct iio_event_data *event)
> @@ -156,6 +161,7 @@ static bool event_is_known(struct iio_event_data *event)
>  	case IIO_GRAVITY:
>  	case IIO_POSITIONRELATIVE:
>  	case IIO_PHASE:
> +	case IIO_MASSCONCENTRATION:
>  		break;
>  	default:
>  		return false;
> @@ -200,6 +206,10 @@ static bool event_is_known(struct iio_event_data *event)
>  	case IIO_MOD_Q:
>  	case IIO_MOD_CO2:
>  	case IIO_MOD_VOC:
> +	case IIO_MOD_PM1:
> +	case IIO_MOD_PM2P5:
> +	case IIO_MOD_PM4:
> +	case IIO_MOD_PM10:
>  		break;
>  	default:
>  		return false;

[PATCH v2 2/3] iio: chemical: add support for Sensirion SPS30 sensor

[Tomasz Duszynski]

Add support for Sensirion SPS30 particulate matter sensor.

Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
---
 drivers/iio/chemical/Kconfig  |  11 +
 drivers/iio/chemical/Makefile |   1 +
 drivers/iio/chemical/sps30.c  | 406 ++++++++++++++++++++++++++++++++++
 3 files changed, 418 insertions(+)
 create mode 100644 drivers/iio/chemical/sps30.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index b8e005be4f87..57832b4360e9 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -61,6 +61,17 @@ config IAQCORE
 	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
 	  sensors
 
+config SPS30
+	tristate "SPS30 particulate matter sensor"
+	depends on I2C
+	select CRC8
+	help
+	  Say Y here to build support for the Sensirion SPS30 particulate
+	  matter sensor.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called sps30.
+
 config VZ89X
 	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 2f4c4ba4d781..9f42f4252151 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -9,4 +9,5 @@ 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_SPS30) += sps30.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
new file mode 100644
index 000000000000..4857b6164dd2
--- /dev/null
+++ b/drivers/iio/chemical/sps30.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SPS30 Particulate Matter sensor driver
+ *
+ * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
+ *
+ * I2C slave address: 0x69
+ *
+ * TODO:
+ *  - support for turning on fan cleaning
+ *  - support for reading/setting auto cleaning interval
+ */
+
+#define pr_fmt(fmt) "sps30: " fmt
+
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+
+#define SPS30_CRC8_POLYNOMIAL 0x31
+/* max number of bytes needed to store PM measurements or serial string */
+#define SPS30_MAX_READ_SIZE 48
+/* sensor measures reliably up to 3000 ug / m3 */
+#define SPS30_MAX_PM 3000
+
+/* SPS30 commands */
+#define SPS30_START_MEAS 0x0010
+#define SPS30_STOP_MEAS 0x0104
+#define SPS30_RESET 0xd304
+#define SPS30_READ_DATA_READY_FLAG 0x0202
+#define SPS30_READ_DATA 0x0300
+#define SPS30_READ_SERIAL 0xd033
+
+enum {
+	PM1,
+	PM2P5,
+	PM4,
+	PM10,
+};
+
+struct sps30_state {
+	struct i2c_client *client;
+	/*
+	 * Guards against concurrent access to sensor registers.
+	 * Must be held whenever sequence of commands is to be executed.
+	 */
+	struct mutex lock;
+};
+
+DECLARE_CRC8_TABLE(sps30_crc8_table);
+
+static int sps30_write_then_read(struct sps30_state *state, u8 *txbuf,
+				 int txsize, u8 *rxbuf, int rxsize)
+{
+	int ret;
+
+	/*
+	 * Sensor does not support repeated start so instead of
+	 * sending two i2c messages in a row we just send one by one.
+	 */
+	ret = i2c_master_send(state->client, txbuf, txsize);
+	if (ret != txsize)
+		return ret < 0 ? ret : -EIO;
+
+	if (!rxbuf)
+		return 0;
+
+	ret = i2c_master_recv(state->client, rxbuf, rxsize);
+	if (ret != rxsize)
+		return ret < 0 ? ret : -EIO;
+
+	return 0;
+}
+
+static int sps30_do_cmd(struct sps30_state *state, u16 cmd, u8 *data, int size)
+{
+	/*
+	 * Internally sensor stores measurements in a following manner:
+	 *
+	 * PM1: upper two bytes, crc8, lower two bytes, crc8
+	 * PM2P5: upper two bytes, crc8, lower two bytes, crc8
+	 * PM4: upper two bytes, crc8, lower two bytes, crc8
+	 * PM10: upper two bytes, crc8, lower two bytes, crc8
+	 *
+	 * What follows next are number concentration measurements and
+	 * typical particle size measurement which we omit.
+	 */
+	u8 buf[SPS30_MAX_READ_SIZE] = { cmd >> 8, cmd };
+	int i, ret = 0;
+
+	switch (cmd) {
+	case SPS30_START_MEAS:
+		buf[2] = 0x03;
+		buf[3] = 0x00;
+		buf[4] = crc8(sps30_crc8_table, &buf[2], 2, CRC8_INIT_VALUE);
+		ret = sps30_write_then_read(state, buf, 5, NULL, 0);
+		break;
+	case SPS30_STOP_MEAS:
+	case SPS30_RESET:
+		ret = sps30_write_then_read(state, buf, 2, NULL, 0);
+		break;
+	case SPS30_READ_DATA_READY_FLAG:
+	case SPS30_READ_DATA:
+	case SPS30_READ_SERIAL:
+		/* every two data bytes are checksummed */
+		size += size / 2;
+		ret = sps30_write_then_read(state, buf, 2, buf, size);
+		break;
+	};
+
+	if (ret)
+		return ret;
+
+	/* validate received data and strip off crc bytes */
+	for (i = 0; i < size; i += 3) {
+		u8 crc = crc8(sps30_crc8_table, &buf[i], 2, CRC8_INIT_VALUE);
+
+		if (crc != buf[i + 2]) {
+			dev_err(&state->client->dev,
+				"data integrity check failed\n");
+			return -EIO;
+		}
+
+		*data++ = buf[i];
+		*data++ = buf[i + 1];
+	}
+
+	return 0;
+}
+
+static int sps30_float_to_int_clamped(const u8 *fp)
+{
+	int val = ((int)fp[0] << 24) | ((int)fp[1] << 16) | ((int)fp[2] << 8) |
+		  (int)fp[3];
+	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 300000 */
+	shift = 23 - exp;
+	val = (1 << exp) + (mantissa >> shift);
+	if (val >= SPS30_MAX_PM)
+		return SPS30_MAX_PM * 100;
+
+	fraction = mantissa & GENMASK(shift - 1, 0);
+
+	return val * 100 + ((fraction * 100) >> shift);
+}
+
+static int sps30_do_meas(struct sps30_state *state, int *data, int size)
+{
+	int i, ret, tries = 5;
+	u8 tmp[16];
+
+	while (tries--) {
+		ret = sps30_do_cmd(state, SPS30_READ_DATA_READY_FLAG, tmp, 2);
+		if (ret)
+			return -EIO;
+
+		/* new measurements ready to be read */
+		if (tmp[1] == 1)
+			break;
+
+		msleep_interruptible(300);
+	}
+
+	if (!tries)
+		return -ETIMEDOUT;
+
+	ret = sps30_do_cmd(state, SPS30_READ_DATA, tmp, sizeof(int) * size);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < size; i++)
+		data[i] = sps30_float_to_int_clamped(&tmp[4 * i]);
+
+	return 0;
+}
+
+static irqreturn_t sps30_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct sps30_state *state = iio_priv(indio_dev);
+	int ret, data[4 + 2]; /* PM1, PM2P5, PM4, PM10, timestamp */
+
+	mutex_lock(&state->lock);
+	ret = sps30_do_meas(state, data, 4);
+	mutex_unlock(&state->lock);
+	if (ret)
+		goto err;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, data,
+					   iio_get_time_ns(indio_dev));
+err:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int sps30_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  int *val, int *val2, long mask)
+{
+	struct sps30_state *state = iio_priv(indio_dev);
+	int data[4], ret = -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_MASSCONCENTRATION:
+			mutex_lock(&state->lock);
+			/* read up to the number of bytes actually needed */
+			switch (chan->channel2) {
+			case IIO_MOD_PM1:
+				ret = sps30_do_meas(state, data, 1);
+				break;
+			case IIO_MOD_PM2P5:
+				ret = sps30_do_meas(state, data, 2);
+				break;
+			case IIO_MOD_PM4:
+				ret = sps30_do_meas(state, data, 3);
+				break;
+			case IIO_MOD_PM10:
+				ret = sps30_do_meas(state, data, 4);
+				break;
+			}
+			mutex_unlock(&state->lock);
+			if (ret)
+				return ret;
+
+			*val = data[chan->address] / 100;
+			*val2 = (data[chan->address] % 100) * 10000;
+
+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_MASSCONCENTRATION:
+			switch (chan->channel2) {
+			case IIO_MOD_PM1:
+			case IIO_MOD_PM2P5:
+			case IIO_MOD_PM4:
+			case IIO_MOD_PM10:
+				*val = 0;
+				*val2 = 10000;
+
+				return IIO_VAL_INT_PLUS_MICRO;
+			}
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info sps30_info = {
+	.read_raw = sps30_read_raw,
+};
+
+#define SPS30_CHAN(_index, _mod) { \
+	.type = IIO_MASSCONCENTRATION, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_ ## _mod, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+	.address = _mod, \
+	.scan_index = _index, \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 19, \
+		.storagebits = 32, \
+		.endianness = IIO_CPU, \
+	}, \
+}
+
+static const struct iio_chan_spec sps30_channels[] = {
+	SPS30_CHAN(0, PM1),
+	SPS30_CHAN(1, PM2P5),
+	SPS30_CHAN(2, PM4),
+	SPS30_CHAN(3, PM10),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static void sps30_stop_meas(void *data)
+{
+	struct sps30_state *state = data;
+
+	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
+}
+
+static const unsigned long sps30_scan_masks[] = { 0x0f, 0x00 };
+
+static int sps30_probe(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev;
+	struct sps30_state *state;
+	u8 buf[32];
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	state->client = client;
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &sps30_info;
+	indio_dev->name = client->name;
+	indio_dev->channels = sps30_channels;
+	indio_dev->num_channels = ARRAY_SIZE(sps30_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = sps30_scan_masks;
+
+	mutex_init(&state->lock);
+	crc8_populate_msb(sps30_crc8_table, SPS30_CRC8_POLYNOMIAL);
+
+	ret = sps30_do_cmd(state, SPS30_RESET, NULL, 0);
+	if (ret) {
+		dev_err(&client->dev, "failed to reset device\n");
+		return ret;
+	}
+	msleep(300);
+	/*
+	 * Power-on-reset causes sensor to produce some glitch on i2c bus and
+	 * some controllers end up in error state. Recover simply by placing
+	 * some data on the bus, for example STOP_MEAS command, which
+	 * is NOP in this case.
+	 */
+	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
+
+	ret = sps30_do_cmd(state, SPS30_READ_SERIAL, buf, sizeof(buf));
+	if (ret) {
+		dev_err(&client->dev, "failed to read serial number\n");
+		return ret;
+	}
+	/* returned serial number is already NUL terminated */
+	dev_info(&client->dev, "serial number: %s\n", buf);
+
+	ret = sps30_do_cmd(state, SPS30_START_MEAS, NULL, 0);
+	if (ret) {
+		dev_err(&client->dev, "failed to start measurement\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(&client->dev, sps30_stop_meas, state);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
+					      sps30_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id sps30_id[] = {
+	{ "sps30" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sps30_id);
+
+static const struct of_device_id sps30_of_match[] = {
+	{ .compatible = "sensirion,sps30" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sps30_of_match);
+
+static struct i2c_driver sps30_driver = {
+	.driver = {
+		.name = "sps30",
+		.of_match_table = sps30_of_match,
+	},
+	.id_table = sps30_id,
+	.probe_new = sps30_probe,
+};
+module_i2c_driver(sps30_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>");
+MODULE_DESCRIPTION("Sensirion SPS30 particulate matter sensor driver");
+MODULE_LICENSE("GPL v2");
-- 
2.19.2

Re: [PATCH v2 2/3] iio: chemical: add support for Sensirion SPS30 sensor

[Jonathan Cameron]

On Thu,  6 Dec 2018 22:04:42 +0100
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> Add support for Sensirion SPS30 particulate matter sensor.
> 
> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>

Just one comment inline and if that is all anyone comes up with
I'll take it as it stands and we can tidy that up whenever we get
around to it.

So, as with patch 1, I'd like to let this sit for a few days to
give others time to take a look at V2.

Thanks,

Jonathan

> ---
>  drivers/iio/chemical/Kconfig  |  11 +
>  drivers/iio/chemical/Makefile |   1 +
>  drivers/iio/chemical/sps30.c  | 406 ++++++++++++++++++++++++++++++++++
>  3 files changed, 418 insertions(+)
>  create mode 100644 drivers/iio/chemical/sps30.c
> 
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index b8e005be4f87..57832b4360e9 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -61,6 +61,17 @@ config IAQCORE
>  	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
>  	  sensors
>  
> +config SPS30
> +	tristate "SPS30 particulate matter sensor"
> +	depends on I2C
> +	select CRC8
> +	help
> +	  Say Y here to build support for the Sensirion SPS30 particulate
> +	  matter sensor.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called sps30.
> +
>  config VZ89X
>  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>  	depends on I2C
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index 2f4c4ba4d781..9f42f4252151 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -9,4 +9,5 @@ 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_SPS30) += sps30.o
>  obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> new file mode 100644
> index 000000000000..4857b6164dd2
> --- /dev/null
> +++ b/drivers/iio/chemical/sps30.c
> @@ -0,0 +1,406 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sensirion SPS30 Particulate Matter sensor driver
> + *
> + * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
> + *
> + * I2C slave address: 0x69
> + *
> + * TODO:
> + *  - support for turning on fan cleaning
> + *  - support for reading/setting auto cleaning interval
> + */
> +
> +#define pr_fmt(fmt) "sps30: " fmt
> +
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/module.h>
> +
> +#define SPS30_CRC8_POLYNOMIAL 0x31
> +/* max number of bytes needed to store PM measurements or serial string */
> +#define SPS30_MAX_READ_SIZE 48
> +/* sensor measures reliably up to 3000 ug / m3 */
> +#define SPS30_MAX_PM 3000
> +
> +/* SPS30 commands */
> +#define SPS30_START_MEAS 0x0010
> +#define SPS30_STOP_MEAS 0x0104
> +#define SPS30_RESET 0xd304
> +#define SPS30_READ_DATA_READY_FLAG 0x0202
> +#define SPS30_READ_DATA 0x0300
> +#define SPS30_READ_SERIAL 0xd033
> +
> +enum {
> +	PM1,
> +	PM2P5,
> +	PM4,
> +	PM10,
> +};
> +
> +struct sps30_state {
> +	struct i2c_client *client;
> +	/*
> +	 * Guards against concurrent access to sensor registers.
> +	 * Must be held whenever sequence of commands is to be executed.
> +	 */
> +	struct mutex lock;
> +};
> +
> +DECLARE_CRC8_TABLE(sps30_crc8_table);
> +
> +static int sps30_write_then_read(struct sps30_state *state, u8 *txbuf,
> +				 int txsize, u8 *rxbuf, int rxsize)
> +{
> +	int ret;
> +
> +	/*
> +	 * Sensor does not support repeated start so instead of
> +	 * sending two i2c messages in a row we just send one by one.
> +	 */
> +	ret = i2c_master_send(state->client, txbuf, txsize);
> +	if (ret != txsize)
> +		return ret < 0 ? ret : -EIO;
> +
> +	if (!rxbuf)
> +		return 0;
> +
> +	ret = i2c_master_recv(state->client, rxbuf, rxsize);
> +	if (ret != rxsize)
> +		return ret < 0 ? ret : -EIO;
> +
> +	return 0;
> +}
> +
> +static int sps30_do_cmd(struct sps30_state *state, u16 cmd, u8 *data, int size)
> +{
> +	/*
> +	 * Internally sensor stores measurements in a following manner:
> +	 *
> +	 * PM1: upper two bytes, crc8, lower two bytes, crc8
> +	 * PM2P5: upper two bytes, crc8, lower two bytes, crc8
> +	 * PM4: upper two bytes, crc8, lower two bytes, crc8
> +	 * PM10: upper two bytes, crc8, lower two bytes, crc8
> +	 *
> +	 * What follows next are number concentration measurements and
> +	 * typical particle size measurement which we omit.
> +	 */
> +	u8 buf[SPS30_MAX_READ_SIZE] = { cmd >> 8, cmd };
> +	int i, ret = 0;
> +
> +	switch (cmd) {
> +	case SPS30_START_MEAS:
> +		buf[2] = 0x03;
> +		buf[3] = 0x00;
> +		buf[4] = crc8(sps30_crc8_table, &buf[2], 2, CRC8_INIT_VALUE);
> +		ret = sps30_write_then_read(state, buf, 5, NULL, 0);
> +		break;
> +	case SPS30_STOP_MEAS:
> +	case SPS30_RESET:
> +		ret = sps30_write_then_read(state, buf, 2, NULL, 0);
> +		break;
> +	case SPS30_READ_DATA_READY_FLAG:
> +	case SPS30_READ_DATA:
> +	case SPS30_READ_SERIAL:
> +		/* every two data bytes are checksummed */
> +		size += size / 2;
> +		ret = sps30_write_then_read(state, buf, 2, buf, size);
> +		break;
> +	};
> +
> +	if (ret)
> +		return ret;
> +
> +	/* validate received data and strip off crc bytes */
> +	for (i = 0; i < size; i += 3) {
> +		u8 crc = crc8(sps30_crc8_table, &buf[i], 2, CRC8_INIT_VALUE);
> +
> +		if (crc != buf[i + 2]) {
> +			dev_err(&state->client->dev,
> +				"data integrity check failed\n");
> +			return -EIO;
> +		}
> +
> +		*data++ = buf[i];
> +		*data++ = buf[i + 1];
> +	}
> +
> +	return 0;
> +}
> +
> +static int sps30_float_to_int_clamped(const u8 *fp)
> +{
> +	int val = ((int)fp[0] << 24) | ((int)fp[1] << 16) | ((int)fp[2] << 8) |
> +		  (int)fp[3];

This is to convert from big endian 32bit to cpu endian.  Please use

be32_to_cpu as that means it's free on a big endian system.
(I think I have that the right way around!)

> +	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 300000 */
> +	shift = 23 - exp;
> +	val = (1 << exp) + (mantissa >> shift);
> +	if (val >= SPS30_MAX_PM)
> +		return SPS30_MAX_PM * 100;
> +
> +	fraction = mantissa & GENMASK(shift - 1, 0);
> +
> +	return val * 100 + ((fraction * 100) >> shift);
> +}
> +
> +static int sps30_do_meas(struct sps30_state *state, int *data, int size)
> +{
> +	int i, ret, tries = 5;
> +	u8 tmp[16];
> +
> +	while (tries--) {
> +		ret = sps30_do_cmd(state, SPS30_READ_DATA_READY_FLAG, tmp, 2);
> +		if (ret)
> +			return -EIO;
> +
> +		/* new measurements ready to be read */
> +		if (tmp[1] == 1)
> +			break;
> +
> +		msleep_interruptible(300);
> +	}
> +
> +	if (!tries)
> +		return -ETIMEDOUT;
> +
> +	ret = sps30_do_cmd(state, SPS30_READ_DATA, tmp, sizeof(int) * size);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < size; i++)
> +		data[i] = sps30_float_to_int_clamped(&tmp[4 * i]);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t sps30_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct sps30_state *state = iio_priv(indio_dev);
> +	int ret, data[4 + 2]; /* PM1, PM2P5, PM4, PM10, timestamp */
> +
> +	mutex_lock(&state->lock);
> +	ret = sps30_do_meas(state, data, 4);
> +	mutex_unlock(&state->lock);
> +	if (ret)
> +		goto err;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, data,
> +					   iio_get_time_ns(indio_dev));
> +err:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sps30_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  int *val, int *val2, long mask)
> +{
> +	struct sps30_state *state = iio_priv(indio_dev);
> +	int data[4], ret = -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_MASSCONCENTRATION:
> +			mutex_lock(&state->lock);
> +			/* read up to the number of bytes actually needed */
> +			switch (chan->channel2) {
> +			case IIO_MOD_PM1:
> +				ret = sps30_do_meas(state, data, 1);
> +				break;
> +			case IIO_MOD_PM2P5:
> +				ret = sps30_do_meas(state, data, 2);
> +				break;
> +			case IIO_MOD_PM4:
> +				ret = sps30_do_meas(state, data, 3);
> +				break;
> +			case IIO_MOD_PM10:
> +				ret = sps30_do_meas(state, data, 4);
> +				break;
> +			}
> +			mutex_unlock(&state->lock);
> +			if (ret)
> +				return ret;
> +
> +			*val = data[chan->address] / 100;
> +			*val2 = (data[chan->address] % 100) * 10000;
> +
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_MASSCONCENTRATION:
> +			switch (chan->channel2) {
> +			case IIO_MOD_PM1:
> +			case IIO_MOD_PM2P5:
> +			case IIO_MOD_PM4:
> +			case IIO_MOD_PM10:
> +				*val = 0;
> +				*val2 = 10000;
> +
> +				return IIO_VAL_INT_PLUS_MICRO;
> +			}
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info sps30_info = {
> +	.read_raw = sps30_read_raw,
> +};
> +
> +#define SPS30_CHAN(_index, _mod) { \
> +	.type = IIO_MASSCONCENTRATION, \
> +	.modified = 1, \
> +	.channel2 = IIO_MOD_ ## _mod, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +	.address = _mod, \
> +	.scan_index = _index, \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 19, \
> +		.storagebits = 32, \
> +		.endianness = IIO_CPU, \
> +	}, \
> +}
> +
> +static const struct iio_chan_spec sps30_channels[] = {
> +	SPS30_CHAN(0, PM1),
> +	SPS30_CHAN(1, PM2P5),
> +	SPS30_CHAN(2, PM4),
> +	SPS30_CHAN(3, PM10),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static void sps30_stop_meas(void *data)
> +{
> +	struct sps30_state *state = data;
> +
> +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> +}
> +
> +static const unsigned long sps30_scan_masks[] = { 0x0f, 0x00 };
> +
> +static int sps30_probe(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev;
> +	struct sps30_state *state;
> +	u8 buf[32];
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -EOPNOTSUPP;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	state = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	state->client = client;
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &sps30_info;
> +	indio_dev->name = client->name;
> +	indio_dev->channels = sps30_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(sps30_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->available_scan_masks = sps30_scan_masks;
> +
> +	mutex_init(&state->lock);
> +	crc8_populate_msb(sps30_crc8_table, SPS30_CRC8_POLYNOMIAL);
> +
> +	ret = sps30_do_cmd(state, SPS30_RESET, NULL, 0);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to reset device\n");
> +		return ret;
> +	}
> +	msleep(300);
> +	/*
> +	 * Power-on-reset causes sensor to produce some glitch on i2c bus and
> +	 * some controllers end up in error state. Recover simply by placing
> +	 * some data on the bus, for example STOP_MEAS command, which
> +	 * is NOP in this case.
> +	 */
> +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> +
> +	ret = sps30_do_cmd(state, SPS30_READ_SERIAL, buf, sizeof(buf));
> +	if (ret) {
> +		dev_err(&client->dev, "failed to read serial number\n");
> +		return ret;
> +	}
> +	/* returned serial number is already NUL terminated */
> +	dev_info(&client->dev, "serial number: %s\n", buf);
> +
> +	ret = sps30_do_cmd(state, SPS30_START_MEAS, NULL, 0);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to start measurement\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(&client->dev, sps30_stop_meas, state);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
> +					      sps30_trigger_handler, NULL);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id sps30_id[] = {
> +	{ "sps30" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sps30_id);
> +
> +static const struct of_device_id sps30_of_match[] = {
> +	{ .compatible = "sensirion,sps30" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sps30_of_match);
> +
> +static struct i2c_driver sps30_driver = {
> +	.driver = {
> +		.name = "sps30",
> +		.of_match_table = sps30_of_match,
> +	},
> +	.id_table = sps30_id,
> +	.probe_new = sps30_probe,
> +};
> +module_i2c_driver(sps30_driver);
> +
> +MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>");
> +MODULE_DESCRIPTION("Sensirion SPS30 particulate matter sensor driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH v2 2/3] iio: chemical: add support for Sensirion SPS30 sensor

[Tomasz Duszynski]

On Sat, Dec 08, 2018 at 03:55:26PM +0000, Jonathan Cameron wrote:
> On Thu,  6 Dec 2018 22:04:42 +0100
> Tomasz Duszynski <tduszyns@gmail.com> wrote:
>
> > Add support for Sensirion SPS30 particulate matter sensor.
> >
> > Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>
> Just one comment inline and if that is all anyone comes up with
> I'll take it as it stands and we can tidy that up whenever we get
> around to it.
>

Good. Two things from my side inline, which could be put into separate
patch and squashed once applying this one (in case you're okay with that).

> So, as with patch 1, I'd like to let this sit for a few days to
> give others time to take a look at V2.
>
> Thanks,
>
> Jonathan
>
> > ---
> >  drivers/iio/chemical/Kconfig  |  11 +
> >  drivers/iio/chemical/Makefile |   1 +
> >  drivers/iio/chemical/sps30.c  | 406 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 418 insertions(+)
> >  create mode 100644 drivers/iio/chemical/sps30.c
> >
> > diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> > index b8e005be4f87..57832b4360e9 100644
> > --- a/drivers/iio/chemical/Kconfig
> > +++ b/drivers/iio/chemical/Kconfig
> > @@ -61,6 +61,17 @@ config IAQCORE
> >  	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
> >  	  sensors
> >
> > +config SPS30
> > +	tristate "SPS30 particulate matter sensor"
> > +	depends on I2C
> > +	select CRC8
> > +	help
> > +	  Say Y here to build support for the Sensirion SPS30 particulate
> > +	  matter sensor.
> > +
> > +	  To compile this driver as a module, choose M here: the module will
> > +	  be called sps30.
> > +
> >  config VZ89X
> >  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
> >  	depends on I2C
> > diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> > index 2f4c4ba4d781..9f42f4252151 100644
> > --- a/drivers/iio/chemical/Makefile
> > +++ b/drivers/iio/chemical/Makefile
> > @@ -9,4 +9,5 @@ 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_SPS30) += sps30.o
> >  obj-$(CONFIG_VZ89X)		+= vz89x.o
> > diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> > new file mode 100644
> > index 000000000000..4857b6164dd2
> > --- /dev/null
> > +++ b/drivers/iio/chemical/sps30.c
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sensirion SPS30 Particulate Matter sensor driver
> > + *

Looks I forgot to change 'Particulate Matter' to lower case here.

> > + * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
> > + *
> > + * I2C slave address: 0x69
> > + *
> > + * TODO:
> > + *  - support for turning on fan cleaning
> > + *  - support for reading/setting auto cleaning interval
> > + */
> > +
> > +#define pr_fmt(fmt) "sps30: " fmt
> > +
> > +#include <linux/crc8.h>
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/module.h>
> > +
> > +#define SPS30_CRC8_POLYNOMIAL 0x31
> > +/* max number of bytes needed to store PM measurements or serial string */
> > +#define SPS30_MAX_READ_SIZE 48
> > +/* sensor measures reliably up to 3000 ug / m3 */
> > +#define SPS30_MAX_PM 3000
> > +
> > +/* SPS30 commands */
> > +#define SPS30_START_MEAS 0x0010
> > +#define SPS30_STOP_MEAS 0x0104
> > +#define SPS30_RESET 0xd304
> > +#define SPS30_READ_DATA_READY_FLAG 0x0202
> > +#define SPS30_READ_DATA 0x0300
> > +#define SPS30_READ_SERIAL 0xd033
> > +
> > +enum {
> > +	PM1,
> > +	PM2P5,
> > +	PM4,
> > +	PM10,
> > +};
> > +
> > +struct sps30_state {
> > +	struct i2c_client *client;
> > +	/*
> > +	 * Guards against concurrent access to sensor registers.
> > +	 * Must be held whenever sequence of commands is to be executed.
> > +	 */
> > +	struct mutex lock;
> > +};
> > +
> > +DECLARE_CRC8_TABLE(sps30_crc8_table);
> > +
> > +static int sps30_write_then_read(struct sps30_state *state, u8 *txbuf,
> > +				 int txsize, u8 *rxbuf, int rxsize)
> > +{
> > +	int ret;
> > +
> > +	/*
> > +	 * Sensor does not support repeated start so instead of
> > +	 * sending two i2c messages in a row we just send one by one.
> > +	 */
> > +	ret = i2c_master_send(state->client, txbuf, txsize);
> > +	if (ret != txsize)
> > +		return ret < 0 ? ret : -EIO;
> > +
> > +	if (!rxbuf)
> > +		return 0;
> > +
> > +	ret = i2c_master_recv(state->client, rxbuf, rxsize);
> > +	if (ret != rxsize)
> > +		return ret < 0 ? ret : -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sps30_do_cmd(struct sps30_state *state, u16 cmd, u8 *data, int size)
> > +{
> > +	/*
> > +	 * Internally sensor stores measurements in a following manner:
> > +	 *
> > +	 * PM1: upper two bytes, crc8, lower two bytes, crc8
> > +	 * PM2P5: upper two bytes, crc8, lower two bytes, crc8
> > +	 * PM4: upper two bytes, crc8, lower two bytes, crc8
> > +	 * PM10: upper two bytes, crc8, lower two bytes, crc8
> > +	 *
> > +	 * What follows next are number concentration measurements and
> > +	 * typical particle size measurement which we omit.
> > +	 */
> > +	u8 buf[SPS30_MAX_READ_SIZE] = { cmd >> 8, cmd };
> > +	int i, ret = 0;
> > +
> > +	switch (cmd) {
> > +	case SPS30_START_MEAS:
> > +		buf[2] = 0x03;
> > +		buf[3] = 0x00;
> > +		buf[4] = crc8(sps30_crc8_table, &buf[2], 2, CRC8_INIT_VALUE);
> > +		ret = sps30_write_then_read(state, buf, 5, NULL, 0);
> > +		break;
> > +	case SPS30_STOP_MEAS:
> > +	case SPS30_RESET:
> > +		ret = sps30_write_then_read(state, buf, 2, NULL, 0);
> > +		break;
> > +	case SPS30_READ_DATA_READY_FLAG:
> > +	case SPS30_READ_DATA:
> > +	case SPS30_READ_SERIAL:
> > +		/* every two data bytes are checksummed */
> > +		size += size / 2;
> > +		ret = sps30_write_then_read(state, buf, 2, buf, size);
> > +		break;
> > +	};
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* validate received data and strip off crc bytes */
> > +	for (i = 0; i < size; i += 3) {
> > +		u8 crc = crc8(sps30_crc8_table, &buf[i], 2, CRC8_INIT_VALUE);
> > +
> > +		if (crc != buf[i + 2]) {
> > +			dev_err(&state->client->dev,
> > +				"data integrity check failed\n");
> > +			return -EIO;
> > +		}
> > +
> > +		*data++ = buf[i];
> > +		*data++ = buf[i + 1];
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int sps30_float_to_int_clamped(const u8 *fp)
> > +{
> > +	int val = ((int)fp[0] << 24) | ((int)fp[1] << 16) | ((int)fp[2] << 8) |
> > +		  (int)fp[3];
>
> This is to convert from big endian 32bit to cpu endian.  Please use
>
> be32_to_cpu as that means it's free on a big endian system.
> (I think I have that the right way around!)
>

get_unaligned_be32 may work even better in this case as be32_to_cpu
could cause unaligned memory access I think. Not sure whether all
architectures can handle it gracefully.

> > +	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 300000 */
> > +	shift = 23 - exp;
> > +	val = (1 << exp) + (mantissa >> shift);
> > +	if (val >= SPS30_MAX_PM)
> > +		return SPS30_MAX_PM * 100;
> > +
> > +	fraction = mantissa & GENMASK(shift - 1, 0);
> > +
> > +	return val * 100 + ((fraction * 100) >> shift);
> > +}
> > +
> > +static int sps30_do_meas(struct sps30_state *state, int *data, int size)
> > +{
> > +	int i, ret, tries = 5;
> > +	u8 tmp[16];
> > +
> > +	while (tries--) {
> > +		ret = sps30_do_cmd(state, SPS30_READ_DATA_READY_FLAG, tmp, 2);
> > +		if (ret)
> > +			return -EIO;
> > +
> > +		/* new measurements ready to be read */
> > +		if (tmp[1] == 1)
> > +			break;
> > +
> > +		msleep_interruptible(300);
> > +	}
> > +
> > +	if (!tries)
> > +		return -ETIMEDOUT;
> > +
> > +	ret = sps30_do_cmd(state, SPS30_READ_DATA, tmp, sizeof(int) * size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (i = 0; i < size; i++)
> > +		data[i] = sps30_float_to_int_clamped(&tmp[4 * i]);
> > +
> > +	return 0;
> > +}
> > +
> > +static irqreturn_t sps30_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *indio_dev = pf->indio_dev;
> > +	struct sps30_state *state = iio_priv(indio_dev);
> > +	int ret, data[4 + 2]; /* PM1, PM2P5, PM4, PM10, timestamp */
> > +
> > +	mutex_lock(&state->lock);
> > +	ret = sps30_do_meas(state, data, 4);
> > +	mutex_unlock(&state->lock);
> > +	if (ret)
> > +		goto err;
> > +
> > +	iio_push_to_buffers_with_timestamp(indio_dev, data,
> > +					   iio_get_time_ns(indio_dev));
> > +err:
> > +	iio_trigger_notify_done(indio_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int sps30_read_raw(struct iio_dev *indio_dev,
> > +			  struct iio_chan_spec const *chan,
> > +			  int *val, int *val2, long mask)
> > +{
> > +	struct sps30_state *state = iio_priv(indio_dev);
> > +	int data[4], ret = -EINVAL;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_PROCESSED:
> > +		switch (chan->type) {
> > +		case IIO_MASSCONCENTRATION:
> > +			mutex_lock(&state->lock);
> > +			/* read up to the number of bytes actually needed */
> > +			switch (chan->channel2) {
> > +			case IIO_MOD_PM1:
> > +				ret = sps30_do_meas(state, data, 1);
> > +				break;
> > +			case IIO_MOD_PM2P5:
> > +				ret = sps30_do_meas(state, data, 2);
> > +				break;
> > +			case IIO_MOD_PM4:
> > +				ret = sps30_do_meas(state, data, 3);
> > +				break;
> > +			case IIO_MOD_PM10:
> > +				ret = sps30_do_meas(state, data, 4);
> > +				break;
> > +			}
> > +			mutex_unlock(&state->lock);
> > +			if (ret)
> > +				return ret;
> > +
> > +			*val = data[chan->address] / 100;
> > +			*val2 = (data[chan->address] % 100) * 10000;
> > +
> > +			return IIO_VAL_INT_PLUS_MICRO;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	case IIO_CHAN_INFO_SCALE:
> > +		switch (chan->type) {
> > +		case IIO_MASSCONCENTRATION:
> > +			switch (chan->channel2) {
> > +			case IIO_MOD_PM1:
> > +			case IIO_MOD_PM2P5:
> > +			case IIO_MOD_PM4:
> > +			case IIO_MOD_PM10:
> > +				*val = 0;
> > +				*val2 = 10000;
> > +
> > +				return IIO_VAL_INT_PLUS_MICRO;
> > +			}
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct iio_info sps30_info = {
> > +	.read_raw = sps30_read_raw,
> > +};
> > +
> > +#define SPS30_CHAN(_index, _mod) { \
> > +	.type = IIO_MASSCONCENTRATION, \
> > +	.modified = 1, \
> > +	.channel2 = IIO_MOD_ ## _mod, \
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> > +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > +	.address = _mod, \
> > +	.scan_index = _index, \
> > +	.scan_type = { \
> > +		.sign = 'u', \
> > +		.realbits = 19, \
> > +		.storagebits = 32, \
> > +		.endianness = IIO_CPU, \
> > +	}, \
> > +}
> > +
> > +static const struct iio_chan_spec sps30_channels[] = {
> > +	SPS30_CHAN(0, PM1),
> > +	SPS30_CHAN(1, PM2P5),
> > +	SPS30_CHAN(2, PM4),
> > +	SPS30_CHAN(3, PM10),
> > +	IIO_CHAN_SOFT_TIMESTAMP(4),
> > +};
> > +
> > +static void sps30_stop_meas(void *data)
> > +{
> > +	struct sps30_state *state = data;
> > +
> > +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> > +}
> > +
> > +static const unsigned long sps30_scan_masks[] = { 0x0f, 0x00 };
> > +
> > +static int sps30_probe(struct i2c_client *client)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct sps30_state *state;
> > +	u8 buf[32];
> > +	int ret;
> > +
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > +		return -EOPNOTSUPP;
> > +
> > +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*state));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	state = iio_priv(indio_dev);
> > +	i2c_set_clientdata(client, indio_dev);
> > +	state->client = client;
> > +	indio_dev->dev.parent = &client->dev;
> > +	indio_dev->info = &sps30_info;
> > +	indio_dev->name = client->name;
> > +	indio_dev->channels = sps30_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(sps30_channels);
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->available_scan_masks = sps30_scan_masks;
> > +
> > +	mutex_init(&state->lock);
> > +	crc8_populate_msb(sps30_crc8_table, SPS30_CRC8_POLYNOMIAL);
> > +
> > +	ret = sps30_do_cmd(state, SPS30_RESET, NULL, 0);
> > +	if (ret) {
> > +		dev_err(&client->dev, "failed to reset device\n");
> > +		return ret;
> > +	}
> > +	msleep(300);
> > +	/*
> > +	 * Power-on-reset causes sensor to produce some glitch on i2c bus and
> > +	 * some controllers end up in error state. Recover simply by placing
> > +	 * some data on the bus, for example STOP_MEAS command, which
> > +	 * is NOP in this case.
> > +	 */
> > +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> > +
> > +	ret = sps30_do_cmd(state, SPS30_READ_SERIAL, buf, sizeof(buf));
> > +	if (ret) {
> > +		dev_err(&client->dev, "failed to read serial number\n");
> > +		return ret;
> > +	}
> > +	/* returned serial number is already NUL terminated */
> > +	dev_info(&client->dev, "serial number: %s\n", buf);
> > +
> > +	ret = sps30_do_cmd(state, SPS30_START_MEAS, NULL, 0);
> > +	if (ret) {
> > +		dev_err(&client->dev, "failed to start measurement\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_add_action_or_reset(&client->dev, sps30_stop_meas, state);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
> > +					      sps30_trigger_handler, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return devm_iio_device_register(&client->dev, indio_dev);
> > +}
> > +
> > +static const struct i2c_device_id sps30_id[] = {
> > +	{ "sps30" },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, sps30_id);
> > +
> > +static const struct of_device_id sps30_of_match[] = {
> > +	{ .compatible = "sensirion,sps30" },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, sps30_of_match);
> > +
> > +static struct i2c_driver sps30_driver = {
> > +	.driver = {
> > +		.name = "sps30",
> > +		.of_match_table = sps30_of_match,
> > +	},
> > +	.id_table = sps30_id,
> > +	.probe_new = sps30_probe,
> > +};
> > +module_i2c_driver(sps30_driver);
> > +
> > +MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>");
> > +MODULE_DESCRIPTION("Sensirion SPS30 particulate matter sensor driver");
> > +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v2 2/3] iio: chemical: add support for Sensirion SPS30 sensor

[Jonathan Cameron]

On Sat, 8 Dec 2018 21:09:20 +0100
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> On Sat, Dec 08, 2018 at 03:55:26PM +0000, Jonathan Cameron wrote:
> > On Thu,  6 Dec 2018 22:04:42 +0100
> > Tomasz Duszynski <tduszyns@gmail.com> wrote:
> >  
> > > Add support for Sensirion SPS30 particulate matter sensor.
> > >
> > > Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>  
> >
> > Just one comment inline and if that is all anyone comes up with
> > I'll take it as it stands and we can tidy that up whenever we get
> > around to it.
> >  
> 
> Good. Two things from my side inline, which could be put into separate
> patch and squashed once applying this one (in case you're okay with that).
Reply inline.  Given how close we are to the end of the cycle, lets
make the change in a v3.  Hopefully any comments people make in v2 in
the meantime will be applicable to v2 and v3!

Thanks

Jonathan

> 
> > So, as with patch 1, I'd like to let this sit for a few days to
> > give others time to take a look at V2.
> >
> > Thanks,
> >
> > Jonathan
> >  
> > > ---
> > >  drivers/iio/chemical/Kconfig  |  11 +
> > >  drivers/iio/chemical/Makefile |   1 +
> > >  drivers/iio/chemical/sps30.c  | 406 ++++++++++++++++++++++++++++++++++
> > >  3 files changed, 418 insertions(+)
> > >  create mode 100644 drivers/iio/chemical/sps30.c
> > >
> > > diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> > > index b8e005be4f87..57832b4360e9 100644
> > > --- a/drivers/iio/chemical/Kconfig
> > > +++ b/drivers/iio/chemical/Kconfig
> > > @@ -61,6 +61,17 @@ config IAQCORE
> > >  	  iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds)
> > >  	  sensors
> > >
> > > +config SPS30
> > > +	tristate "SPS30 particulate matter sensor"
> > > +	depends on I2C
> > > +	select CRC8
> > > +	help
> > > +	  Say Y here to build support for the Sensirion SPS30 particulate
> > > +	  matter sensor.
> > > +
> > > +	  To compile this driver as a module, choose M here: the module will
> > > +	  be called sps30.
> > > +
> > >  config VZ89X
> > >  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
> > >  	depends on I2C
> > > diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> > > index 2f4c4ba4d781..9f42f4252151 100644
> > > --- a/drivers/iio/chemical/Makefile
> > > +++ b/drivers/iio/chemical/Makefile
> > > @@ -9,4 +9,5 @@ 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_SPS30) += sps30.o
> > >  obj-$(CONFIG_VZ89X)		+= vz89x.o
> > > diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
> > > new file mode 100644
> > > index 000000000000..4857b6164dd2
> > > --- /dev/null
> > > +++ b/drivers/iio/chemical/sps30.c
> > > @@ -0,0 +1,406 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Sensirion SPS30 Particulate Matter sensor driver
> > > + *  
> 
> Looks I forgot to change 'Particulate Matter' to lower case here.
> 
> > > + * Copyright (c) Tomasz Duszynski <tduszyns@gmail.com>
> > > + *
> > > + * I2C slave address: 0x69
> > > + *
> > > + * TODO:
> > > + *  - support for turning on fan cleaning
> > > + *  - support for reading/setting auto cleaning interval
> > > + */
> > > +
> > > +#define pr_fmt(fmt) "sps30: " fmt
> > > +
> > > +#include <linux/crc8.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/iio/buffer.h>
> > > +#include <linux/iio/iio.h>
> > > +#include <linux/iio/sysfs.h>
> > > +#include <linux/iio/trigger_consumer.h>
> > > +#include <linux/iio/triggered_buffer.h>
> > > +#include <linux/module.h>
> > > +
> > > +#define SPS30_CRC8_POLYNOMIAL 0x31
> > > +/* max number of bytes needed to store PM measurements or serial string */
> > > +#define SPS30_MAX_READ_SIZE 48
> > > +/* sensor measures reliably up to 3000 ug / m3 */
> > > +#define SPS30_MAX_PM 3000
> > > +
> > > +/* SPS30 commands */
> > > +#define SPS30_START_MEAS 0x0010
> > > +#define SPS30_STOP_MEAS 0x0104
> > > +#define SPS30_RESET 0xd304
> > > +#define SPS30_READ_DATA_READY_FLAG 0x0202
> > > +#define SPS30_READ_DATA 0x0300
> > > +#define SPS30_READ_SERIAL 0xd033
> > > +
> > > +enum {
> > > +	PM1,
> > > +	PM2P5,
> > > +	PM4,
> > > +	PM10,
> > > +};
> > > +
> > > +struct sps30_state {
> > > +	struct i2c_client *client;
> > > +	/*
> > > +	 * Guards against concurrent access to sensor registers.
> > > +	 * Must be held whenever sequence of commands is to be executed.
> > > +	 */
> > > +	struct mutex lock;
> > > +};
> > > +
> > > +DECLARE_CRC8_TABLE(sps30_crc8_table);
> > > +
> > > +static int sps30_write_then_read(struct sps30_state *state, u8 *txbuf,
> > > +				 int txsize, u8 *rxbuf, int rxsize)
> > > +{
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Sensor does not support repeated start so instead of
> > > +	 * sending two i2c messages in a row we just send one by one.
> > > +	 */
> > > +	ret = i2c_master_send(state->client, txbuf, txsize);
> > > +	if (ret != txsize)
> > > +		return ret < 0 ? ret : -EIO;
> > > +
> > > +	if (!rxbuf)
> > > +		return 0;
> > > +
> > > +	ret = i2c_master_recv(state->client, rxbuf, rxsize);
> > > +	if (ret != rxsize)
> > > +		return ret < 0 ? ret : -EIO;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sps30_do_cmd(struct sps30_state *state, u16 cmd, u8 *data, int size)
> > > +{
> > > +	/*
> > > +	 * Internally sensor stores measurements in a following manner:
> > > +	 *
> > > +	 * PM1: upper two bytes, crc8, lower two bytes, crc8
> > > +	 * PM2P5: upper two bytes, crc8, lower two bytes, crc8
> > > +	 * PM4: upper two bytes, crc8, lower two bytes, crc8
> > > +	 * PM10: upper two bytes, crc8, lower two bytes, crc8
> > > +	 *
> > > +	 * What follows next are number concentration measurements and
> > > +	 * typical particle size measurement which we omit.
> > > +	 */
> > > +	u8 buf[SPS30_MAX_READ_SIZE] = { cmd >> 8, cmd };
> > > +	int i, ret = 0;
> > > +
> > > +	switch (cmd) {
> > > +	case SPS30_START_MEAS:
> > > +		buf[2] = 0x03;
> > > +		buf[3] = 0x00;
> > > +		buf[4] = crc8(sps30_crc8_table, &buf[2], 2, CRC8_INIT_VALUE);
> > > +		ret = sps30_write_then_read(state, buf, 5, NULL, 0);
> > > +		break;
> > > +	case SPS30_STOP_MEAS:
> > > +	case SPS30_RESET:
> > > +		ret = sps30_write_then_read(state, buf, 2, NULL, 0);
> > > +		break;
> > > +	case SPS30_READ_DATA_READY_FLAG:
> > > +	case SPS30_READ_DATA:
> > > +	case SPS30_READ_SERIAL:
> > > +		/* every two data bytes are checksummed */
> > > +		size += size / 2;
> > > +		ret = sps30_write_then_read(state, buf, 2, buf, size);
> > > +		break;
> > > +	};
> > > +
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* validate received data and strip off crc bytes */
> > > +	for (i = 0; i < size; i += 3) {
> > > +		u8 crc = crc8(sps30_crc8_table, &buf[i], 2, CRC8_INIT_VALUE);
> > > +
> > > +		if (crc != buf[i + 2]) {
> > > +			dev_err(&state->client->dev,
> > > +				"data integrity check failed\n");
> > > +			return -EIO;
> > > +		}
> > > +
> > > +		*data++ = buf[i];
> > > +		*data++ = buf[i + 1];
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sps30_float_to_int_clamped(const u8 *fp)
> > > +{
> > > +	int val = ((int)fp[0] << 24) | ((int)fp[1] << 16) | ((int)fp[2] << 8) |
> > > +		  (int)fp[3];  
> >
> > This is to convert from big endian 32bit to cpu endian.  Please use
> >
> > be32_to_cpu as that means it's free on a big endian system.
> > (I think I have that the right way around!)
> >  
> 
> get_unaligned_be32 may work even better in this case as be32_to_cpu
> could cause unaligned memory access I think. Not sure whether all
> architectures can handle it gracefully.

True, though easily avoided by changing the type of the tmp array which
is probably appropriate to reflect the two data type anyway.

> 
> > > +	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 300000 */
> > > +	shift = 23 - exp;
> > > +	val = (1 << exp) + (mantissa >> shift);
> > > +	if (val >= SPS30_MAX_PM)
> > > +		return SPS30_MAX_PM * 100;
> > > +
> > > +	fraction = mantissa & GENMASK(shift - 1, 0);
> > > +
> > > +	return val * 100 + ((fraction * 100) >> shift);
> > > +}
> > > +
> > > +static int sps30_do_meas(struct sps30_state *state, int *data, int size)
> > > +{
> > > +	int i, ret, tries = 5;
> > > +	u8 tmp[16];
> > > +
> > > +	while (tries--) {
> > > +		ret = sps30_do_cmd(state, SPS30_READ_DATA_READY_FLAG, tmp, 2);
> > > +		if (ret)
> > > +			return -EIO;
> > > +
> > > +		/* new measurements ready to be read */
> > > +		if (tmp[1] == 1)
> > > +			break;
> > > +
> > > +		msleep_interruptible(300);
> > > +	}
> > > +
> > > +	if (!tries)
> > > +		return -ETIMEDOUT;
> > > +
> > > +	ret = sps30_do_cmd(state, SPS30_READ_DATA, tmp, sizeof(int) * size);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	for (i = 0; i < size; i++)
> > > +		data[i] = sps30_float_to_int_clamped(&tmp[4 * i]);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static irqreturn_t sps30_trigger_handler(int irq, void *p)
> > > +{
> > > +	struct iio_poll_func *pf = p;
> > > +	struct iio_dev *indio_dev = pf->indio_dev;
> > > +	struct sps30_state *state = iio_priv(indio_dev);
> > > +	int ret, data[4 + 2]; /* PM1, PM2P5, PM4, PM10, timestamp */
> > > +
> > > +	mutex_lock(&state->lock);
> > > +	ret = sps30_do_meas(state, data, 4);
> > > +	mutex_unlock(&state->lock);
> > > +	if (ret)
> > > +		goto err;
> > > +
> > > +	iio_push_to_buffers_with_timestamp(indio_dev, data,
> > > +					   iio_get_time_ns(indio_dev));
> > > +err:
> > > +	iio_trigger_notify_done(indio_dev->trig);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int sps30_read_raw(struct iio_dev *indio_dev,
> > > +			  struct iio_chan_spec const *chan,
> > > +			  int *val, int *val2, long mask)
> > > +{
> > > +	struct sps30_state *state = iio_priv(indio_dev);
> > > +	int data[4], ret = -EINVAL;
> > > +
> > > +	switch (mask) {
> > > +	case IIO_CHAN_INFO_PROCESSED:
> > > +		switch (chan->type) {
> > > +		case IIO_MASSCONCENTRATION:
> > > +			mutex_lock(&state->lock);
> > > +			/* read up to the number of bytes actually needed */
> > > +			switch (chan->channel2) {
> > > +			case IIO_MOD_PM1:
> > > +				ret = sps30_do_meas(state, data, 1);
> > > +				break;
> > > +			case IIO_MOD_PM2P5:
> > > +				ret = sps30_do_meas(state, data, 2);
> > > +				break;
> > > +			case IIO_MOD_PM4:
> > > +				ret = sps30_do_meas(state, data, 3);
> > > +				break;
> > > +			case IIO_MOD_PM10:
> > > +				ret = sps30_do_meas(state, data, 4);
> > > +				break;
> > > +			}
> > > +			mutex_unlock(&state->lock);
> > > +			if (ret)
> > > +				return ret;
> > > +
> > > +			*val = data[chan->address] / 100;
> > > +			*val2 = (data[chan->address] % 100) * 10000;
> > > +
> > > +			return IIO_VAL_INT_PLUS_MICRO;
> > > +		default:
> > > +			return -EINVAL;
> > > +		}
> > > +	case IIO_CHAN_INFO_SCALE:
> > > +		switch (chan->type) {
> > > +		case IIO_MASSCONCENTRATION:
> > > +			switch (chan->channel2) {
> > > +			case IIO_MOD_PM1:
> > > +			case IIO_MOD_PM2P5:
> > > +			case IIO_MOD_PM4:
> > > +			case IIO_MOD_PM10:
> > > +				*val = 0;
> > > +				*val2 = 10000;
> > > +
> > > +				return IIO_VAL_INT_PLUS_MICRO;
> > > +			}
> > > +		default:
> > > +			return -EINVAL;
> > > +		}
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}
> > > +
> > > +static const struct iio_info sps30_info = {
> > > +	.read_raw = sps30_read_raw,
> > > +};
> > > +
> > > +#define SPS30_CHAN(_index, _mod) { \
> > > +	.type = IIO_MASSCONCENTRATION, \
> > > +	.modified = 1, \
> > > +	.channel2 = IIO_MOD_ ## _mod, \
> > > +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> > > +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > > +	.address = _mod, \
> > > +	.scan_index = _index, \
> > > +	.scan_type = { \
> > > +		.sign = 'u', \
> > > +		.realbits = 19, \
> > > +		.storagebits = 32, \
> > > +		.endianness = IIO_CPU, \
> > > +	}, \
> > > +}
> > > +
> > > +static const struct iio_chan_spec sps30_channels[] = {
> > > +	SPS30_CHAN(0, PM1),
> > > +	SPS30_CHAN(1, PM2P5),
> > > +	SPS30_CHAN(2, PM4),
> > > +	SPS30_CHAN(3, PM10),
> > > +	IIO_CHAN_SOFT_TIMESTAMP(4),
> > > +};
> > > +
> > > +static void sps30_stop_meas(void *data)
> > > +{
> > > +	struct sps30_state *state = data;
> > > +
> > > +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> > > +}
> > > +
> > > +static const unsigned long sps30_scan_masks[] = { 0x0f, 0x00 };
> > > +
> > > +static int sps30_probe(struct i2c_client *client)
> > > +{
> > > +	struct iio_dev *indio_dev;
> > > +	struct sps30_state *state;
> > > +	u8 buf[32];
> > > +	int ret;
> > > +
> > > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*state));
> > > +	if (!indio_dev)
> > > +		return -ENOMEM;
> > > +
> > > +	state = iio_priv(indio_dev);
> > > +	i2c_set_clientdata(client, indio_dev);
> > > +	state->client = client;
> > > +	indio_dev->dev.parent = &client->dev;
> > > +	indio_dev->info = &sps30_info;
> > > +	indio_dev->name = client->name;
> > > +	indio_dev->channels = sps30_channels;
> > > +	indio_dev->num_channels = ARRAY_SIZE(sps30_channels);
> > > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > > +	indio_dev->available_scan_masks = sps30_scan_masks;
> > > +
> > > +	mutex_init(&state->lock);
> > > +	crc8_populate_msb(sps30_crc8_table, SPS30_CRC8_POLYNOMIAL);
> > > +
> > > +	ret = sps30_do_cmd(state, SPS30_RESET, NULL, 0);
> > > +	if (ret) {
> > > +		dev_err(&client->dev, "failed to reset device\n");
> > > +		return ret;
> > > +	}
> > > +	msleep(300);
> > > +	/*
> > > +	 * Power-on-reset causes sensor to produce some glitch on i2c bus and
> > > +	 * some controllers end up in error state. Recover simply by placing
> > > +	 * some data on the bus, for example STOP_MEAS command, which
> > > +	 * is NOP in this case.
> > > +	 */
> > > +	sps30_do_cmd(state, SPS30_STOP_MEAS, NULL, 0);
> > > +
> > > +	ret = sps30_do_cmd(state, SPS30_READ_SERIAL, buf, sizeof(buf));
> > > +	if (ret) {
> > > +		dev_err(&client->dev, "failed to read serial number\n");
> > > +		return ret;
> > > +	}
> > > +	/* returned serial number is already NUL terminated */
> > > +	dev_info(&client->dev, "serial number: %s\n", buf);
> > > +
> > > +	ret = sps30_do_cmd(state, SPS30_START_MEAS, NULL, 0);
> > > +	if (ret) {
> > > +		dev_err(&client->dev, "failed to start measurement\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = devm_add_action_or_reset(&client->dev, sps30_stop_meas, state);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
> > > +					      sps30_trigger_handler, NULL);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return devm_iio_device_register(&client->dev, indio_dev);
> > > +}
> > > +
> > > +static const struct i2c_device_id sps30_id[] = {
> > > +	{ "sps30" },
> > > +	{ }
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, sps30_id);
> > > +
> > > +static const struct of_device_id sps30_of_match[] = {
> > > +	{ .compatible = "sensirion,sps30" },
> > > +	{ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, sps30_of_match);
> > > +
> > > +static struct i2c_driver sps30_driver = {
> > > +	.driver = {
> > > +		.name = "sps30",
> > > +		.of_match_table = sps30_of_match,
> > > +	},
> > > +	.id_table = sps30_id,
> > > +	.probe_new = sps30_probe,
> > > +};
> > > +module_i2c_driver(sps30_driver);
> > > +
> > > +MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>");
> > > +MODULE_DESCRIPTION("Sensirion SPS30 particulate matter sensor driver");
> > > +MODULE_LICENSE("GPL v2");  
> >  

Re: [PATCH v2 2/3] iio: chemical: add support for Sensirion SPS30 sensor

[kbuild test robot]

Hi Tomasz,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on v4.20-rc6 next-20181207]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tomasz-Duszynski/add-support-for-Sensirion-SPS30-PM-sensor/20181209-204432
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg


coccinelle warnings: (new ones prefixed by >>)

>> drivers/iio/chemical/sps30.c:115:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] iio: chemical: fix semicolon.cocci warnings

[kbuild test robot]

From: kbuild test robot <fengguang.wu@intel.com>

drivers/iio/chemical/sps30.c:115:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 2453f6006a69 ("iio: chemical: add support for Sensirion SPS30 sensor")
CC: Tomasz Duszynski <tduszyns@gmail.com>
Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
---

url:    https://github.com/0day-ci/linux/commits/Tomasz-Duszynski/add-support-for-Sensirion-SPS30-PM-sensor/20181209-204432
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg

 sps30.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/iio/chemical/sps30.c
+++ b/drivers/iio/chemical/sps30.c
@@ -112,7 +112,7 @@ static int sps30_do_cmd(struct sps30_sta
 		size += size / 2;
 		ret = sps30_write_then_read(state, buf, 2, buf, size);
 		break;
-	};
+	}
 
 	if (ret)
 		return ret;

[PATCH v2 3/3] iio: chemical: sps30: add device tree support

[Tomasz Duszynski]

Add device tree support for Sensirion SPS30 particulate
matter sensor.

Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
---
 .../bindings/iio/chemical/sensirion,sps30.txt        | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt

diff --git a/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
new file mode 100644
index 000000000000..6eee2709b5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
@@ -0,0 +1,12 @@
+* Sensirion SPS30 particulate matter sensor
+
+Required properties:
+- compatible: must be "sensirion,sps30"
+- reg: the I2C address of the sensor
+
+Example:
+
+sps30@69 {
+	compatible = "sensirion,sps30";
+	reg = <0x69>;
+};
-- 
2.19.2