[PATCH v1 0/3] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[PATCH v1 0/3] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Roan van Dijk]

This series adds support for the Sensirion SCD4x sensor.

The driver supports continuous reads of temperature, relative humdity and CO2
concentration. There is an interval of 5 seconds between readings. During
this interval the drivers checks if the sensor has new data available.

The driver is based on the scd30 driver. However, The scd4x has become too
different to just expand the scd30 driver. I made a new driver instead of
expanding the scd30 driver. I hope I made the right choice by doing so?

Changes since v1:
dt-bindings:
  - Separated compatible string for each sensor type
scd4x.c:
  - Changed probe, resume and suspend functions to static
  - Added SIMPLE_DEV_PM_OPS function call for power management
    operations.

Roan van Dijk (3):
  dt-bindings: iio: chemical: sensirion,scd4x: Add yaml description
  MAINTAINERS: Add myself as maintainer of the scd4x driver
  drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor

 .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
 MAINTAINERS                                   |   6 +
 drivers/iio/chemical/Kconfig                  |  13 +
 drivers/iio/chemical/Makefile                 |   1 +
 drivers/iio/chemical/scd4x.c                  | 708 ++++++++++++++++++
 5 files changed, 774 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
 create mode 100644 drivers/iio/chemical/scd4x.c

-- 
2.30.2

[PATCH v1 1/3] dt-bindings: iio: chemical: sensirion,scd4x: Add yaml description

[Roan van Dijk]

Add documentation for the SCD4x carbon dioxide sensor from Sensirion.

Signed-off-by: Roan van Dijk <roan@protonic.nl>
---
 .../iio/chemical/sensirion,scd4x.yaml         | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
new file mode 100644
index 000000000000..798f48d05279
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/sensirion,scd4x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sensirion SCD4X carbon dioxide sensor
+
+maintainers:
+  - Roan van Dijk <roan@protonic.nl>
+
+description: |
+  Air quality sensor capable of measuring co2 concentration, temperature
+  and relative humidity.
+
+properties:
+  compatible:
+    enum:
+      - sensirion,scd40
+      - sensirion,scd41
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      co2-sensor@62 {
+        compatible = "sensirion,scd41";
+        reg = <0x62>;
+      };
+    };
-- 
2.30.2

[PATCH v1 2/3] MAINTAINERS: Add myself as maintainer of the scd4x driver

[Roan van Dijk]

Signed-off-by: Roan van Dijk <roan@protonic.nl>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c6b8a720c0bc..7398ee8fda22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16732,6 +16732,12 @@ F:	drivers/iio/chemical/scd30_core.c
 F:	drivers/iio/chemical/scd30_i2c.c
 F:	drivers/iio/chemical/scd30_serial.c
 
+SENSIRION SCD4X CARBON DIOXIDE SENSOR DRIVER
+M:	Roan van Dijk <roan@protonic.nl>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
+F:	drivers/iio/chemical/scd4x.c
+
 SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
 S:	Maintained
-- 
2.30.2

[PATCH v1 3/3] drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Roan van Dijk]

This is a driver for the SCD4x CO2 sensor from Sensirion. The sensor is
able to measure CO2 concentration, temperature and relative humdity.
The sensor uses a photoacoustic principle for measuring CO2 concentration.
An I2C interface is supported by this driver in order to communicate with
the sensor.

Signed-off-by: Roan van Dijk <roan@protonic.nl>
---
 drivers/iio/chemical/Kconfig  |  13 +
 drivers/iio/chemical/Makefile |   1 +
 drivers/iio/chemical/scd4x.c  | 708 ++++++++++++++++++++++++++++++++++
 3 files changed, 722 insertions(+)
 create mode 100644 drivers/iio/chemical/scd4x.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index a4920646e9be..ce9ec81d4993 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -118,6 +118,19 @@ config SCD30_SERIAL
 	  To compile this driver as a module, choose M here: the module will
 	  be called scd30_serial.
 
+config SCD4X
+	tristate "SCD4X carbon dioxide sensor driver"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on I2C
+	select CRC8
+	help
+	  Say Y here to build support for the Sensirion SCD4X sensor with cabon
+	  dioxide, relative humidity and temperature sensing capabilities
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called scd4x.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 4898690cc155..3a766dd23020 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SCD30_CORE) += scd30_core.o
 obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
 obj-$(CONFIG_SCD30_SERIAL) += scd30_serial.o
+obj-$(CONFIG_SCD4X) += scd4x.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
 obj-$(CONFIG_SPS30_I2C) += sps30_i2c.o
diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
new file mode 100644
index 000000000000..6ec8a5b5d901
--- /dev/null
+++ b/drivers/iio/chemical/scd4x.c
@@ -0,0 +1,708 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD4X carbon dioxide sensor i2c driver
+ *
+ * Copyright (C) 2021 Protonic Holland
+ * Author: Roan van Dijk <roan@protonic.nl>
+ *
+ * I2C slave address: 0x62
+ *
+ * Datasheets:
+ * https://www.sensirion.com/file/datasheet_scd4x
+ */
+
+#include <asm/unaligned.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/types.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#define SCD4X_CRC8_POLYNOMIAL 0x31
+#define SCD4X_TIMEOUT_ERR 1000
+#define SCD4X_READ_BUF_SIZE 9
+#define SCD4X_COMMAND_BUF_SIZE 2
+#define SCD4X_WRITE_BUF_SIZE 5
+#define SCD4X_FRC_MIN_PPM 400
+#define SCD4X_FRC_MAX_PPM 2000
+
+/*Commands SCD4X*/
+enum scd4x_cmd {
+	CMD_START_MEAS          = 0x21b1,
+	CMD_READ_MEAS           = 0xec05,
+	CMD_STOP_MEAS           = 0x3f86,
+	CMD_SET_TEMP_OFFSET     = 0x241d,
+	CMD_GET_TEMP_OFFSET     = 0x2318,
+	CMD_FRC                 = 0x362f,
+	CMD_SET_ASC             = 0x2416,
+	CMD_GET_ASC             = 0x2313,
+	CMD_GET_DATA_READY      = 0xe4b8,
+};
+
+enum scd4x_channel_idx {
+	SCD4X_CO2,
+	SCD4X_TEMP,
+	SCD4X_HR,
+};
+
+struct scd4x_state {
+	struct i2c_client *client;
+	struct mutex lock;
+	struct device *dev;
+	struct regulator *vdd;
+
+	int irq;
+	uint16_t meas[3];
+};
+
+DECLARE_CRC8_TABLE(scd4x_crc8_table);
+
+static int scd4x_i2c_xfer(struct scd4x_state *state, char *txbuf, int txsize,
+				char *rxbuf, int rxsize)
+{
+	struct i2c_client *client = to_i2c_client(state->dev);
+	int ret;
+
+	ret = i2c_master_send(client, txbuf, txsize);
+
+	if (ret < 0)
+		return ret;
+	if (ret != txsize)
+		return -EIO;
+
+	if (rxsize == 0)
+		return 0;
+
+	ret = i2c_master_recv(client, rxbuf, rxsize);
+	if (ret < 0)
+		return ret;
+	if (ret != rxsize)
+		return -EIO;
+
+	return 0;
+}
+
+static int scd4x_send_command(struct scd4x_state *state, enum scd4x_cmd cmd)
+{
+	char buf[SCD4X_COMMAND_BUF_SIZE];
+	int ret;
+
+	/*
+	 * Measurement needs to be stopped before sending commands.
+	 * Except stop and start command.
+	 */
+	if ((cmd != CMD_STOP_MEAS) & (cmd != CMD_START_MEAS)) {
+
+		ret = scd4x_send_command(state, CMD_STOP_MEAS);
+		if (ret)
+			return ret;
+
+		/* execution time for stopping measurement */
+		msleep_interruptible(500);
+	}
+
+	put_unaligned_be16(cmd, buf);
+	ret = scd4x_i2c_xfer(state, buf, 2, buf, 0);
+	if (ret)
+		return ret;
+
+	if ((cmd != CMD_STOP_MEAS) & (cmd != CMD_START_MEAS)) {
+		ret = scd4x_send_command(state, CMD_START_MEAS);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int scd4x_read(struct scd4x_state *state, enum scd4x_cmd cmd,
+			void *response, int byte_cnt)
+{
+	char buf[SCD4X_READ_BUF_SIZE];
+	char *rsp = response;
+	int i, ret;
+	char crc;
+
+	/*
+	 * Measurement needs to be stopped before sending commands.
+	 * Except for reading measurement and data ready command.
+	 */
+	if ((cmd != CMD_GET_DATA_READY) & (cmd != CMD_READ_MEAS)) {
+		ret = scd4x_send_command(state, CMD_STOP_MEAS);
+		if (ret)
+			return ret;
+
+		/* execution time for stopping measurement */
+		msleep_interruptible(500);
+	}
+
+	put_unaligned_be16(cmd, buf);
+	ret = scd4x_i2c_xfer(state, buf, 2, buf, byte_cnt);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < byte_cnt; i += 3) {
+		crc = crc8(scd4x_crc8_table, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(state->dev, "CRC error\n");
+			return -EIO;
+		}
+
+		*rsp++ = buf[i];
+		*rsp++ = buf[i + 1];
+	}
+
+	/* start measurement */
+	if ((cmd != CMD_GET_DATA_READY) & (cmd != CMD_READ_MEAS)) {
+		ret = scd4x_send_command(state, CMD_START_MEAS);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int scd4x_write(struct scd4x_state *state, enum scd4x_cmd cmd, uint16_t arg)
+{
+	char buf[SCD4X_WRITE_BUF_SIZE];
+	int ret;
+	char crc;
+
+	put_unaligned_be16(cmd, buf);
+	put_unaligned_be16(arg, buf + 2);
+
+	crc = crc8(scd4x_crc8_table, buf + 2, 2, CRC8_INIT_VALUE);
+	buf[4] = crc;
+
+	/* measurement needs to be stopped before sending commands */
+	ret = scd4x_send_command(state, CMD_STOP_MEAS);
+	if (ret)
+		return ret;
+
+	/* execution time */
+	msleep_interruptible(500);
+
+	ret = scd4x_i2c_xfer(state, buf, SCD4X_WRITE_BUF_SIZE, buf, 0);
+	if (ret)
+		return ret;
+
+	/* start measurement, except for forced calibration command */
+	if (cmd != CMD_FRC) {
+		ret = scd4x_send_command(state, CMD_START_MEAS);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
+				uint16_t arg, void *response, int byte_cnt)
+{
+	struct i2c_client *client = to_i2c_client(state->dev);
+	char buf[SCD4X_READ_BUF_SIZE];
+	char *rsp = response;
+	int i, ret;
+	char crc;
+
+	ret = scd4x_write(state, CMD_FRC, arg);
+	if (ret) {
+		scd4x_send_command(state, CMD_START_MEAS);
+		return ret;
+	}
+
+	/* Execution time */
+	msleep_interruptible(400);
+
+	ret = i2c_master_recv(client, buf, byte_cnt);
+	if (ret < 0)
+		return ret;
+	if (ret != byte_cnt)
+		return -EIO;
+
+	for (i = 0; i < byte_cnt; i += 3) {
+		crc = crc8(scd4x_crc8_table, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(state->dev, "CRC error\n");
+			scd4x_send_command(state, CMD_START_MEAS);
+			return -EIO;
+		}
+
+		*rsp++ = buf[i];
+		*rsp++ = buf[i + 1];
+	}
+
+	ret = scd4x_send_command(state, CMD_START_MEAS);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int scd4x_read_meas(struct scd4x_state *state)
+{
+	int i, ret;
+
+	ret = scd4x_read(state, CMD_READ_MEAS, state->meas, 9);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(state->meas); i++)
+		state->meas[i] = be16_to_cpu(state->meas[i]);
+	/*
+	 * Temperature and humidity values are convertered and scaled to
+	 * milli deg C and milli percent.
+	 */
+	state->meas[SCD4X_TEMP] = ((175 * state->meas[SCD4X_TEMP]/65536) - 45)*1000;
+	state->meas[SCD4X_HR] = ((100 * state->meas[SCD4X_HR]/65536))*1000;
+
+	return 0;
+}
+
+static int scd4x_wait_meas_poll(struct scd4x_state *state)
+{
+	int tries = 6;
+	int ret;
+
+	do {
+		uint16_t val;
+
+		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, 3);
+		if (ret)
+			return -EIO;
+		val = be16_to_cpu(val);
+
+		/* new measurement available */
+		if (val & 0x7FF)
+			break;
+
+		msleep_interruptible(1000);
+	} while (--tries);
+
+	if (tries == 0) {
+		/* try to start sensor on timeout */
+		ret = scd4x_send_command(state, CMD_START_MEAS);
+		if (ret)
+			dev_err(state->dev, "failed to start measurement: %d\n", ret);
+	}
+
+	return tries ? 0 : -ETIMEDOUT;
+}
+
+static int scd4x_read_poll(struct scd4x_state *state)
+{
+	int ret;
+
+	ret = scd4x_wait_meas_poll(state);
+	if (ret)
+		return ret;
+
+	return scd4x_read_meas(state);
+}
+
+static int scd4x_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask)
+{
+	struct scd4x_state *state = iio_priv(indio_dev);
+	int ret;
+	uint16_t tmp;
+
+	mutex_lock(&state->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			break;
+
+		ret = scd4x_read_poll(state);
+		if (ret) {
+			iio_device_release_direct_mode(indio_dev);
+			break;
+		}
+		*val = state->meas[chan->address];
+		iio_device_release_direct_mode(indio_dev);
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, 3);
+		if (ret)
+			break;
+		*val = 175000 * be16_to_cpu(tmp) / 65536;
+		ret = IIO_VAL_INT;
+		break;
+	}
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static int scd4x_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+				int val, int val2, long mask)
+{
+	struct scd4x_state *state = iio_priv(indio_dev);
+	int ret = 0;
+
+	mutex_lock(&state->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBBIAS:
+		val = val * 65536 / 175000;
+		ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static ssize_t calibration_auto_enable_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd4x_state *state = iio_priv(indio_dev);
+	int ret;
+	uint16_t val;
+
+	mutex_lock(&state->lock);
+	ret = scd4x_read(state, CMD_GET_ASC, &val, 3);
+	mutex_unlock(&state->lock);
+	if (ret)
+		dev_err(dev, "failed to read automatic calibration");
+
+	val = be16_to_cpu(val);
+
+	return ret ?: sprintf(buf, "%d\n", val);
+}
+
+static ssize_t calibration_auto_enable_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd4x_state *state = iio_priv(indio_dev);
+	bool val;
+	int ret;
+	uint16_t value;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	value = val;
+
+	mutex_lock(&state->lock);
+	ret = scd4x_write(state, CMD_SET_ASC, value);
+	mutex_unlock(&state->lock);
+	if (ret)
+		dev_err(dev, "failed to set automatic calibration");
+
+	return ret ?: len;
+}
+
+static ssize_t calibration_forced_value_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd4x_state *state = iio_priv(indio_dev);
+	uint16_t val, arg;
+	int ret;
+
+	ret = kstrtou16(buf, 0, &arg);
+	if (ret)
+		return ret;
+
+	if (arg < SCD4X_FRC_MIN_PPM || arg > SCD4X_FRC_MAX_PPM)
+		return -EINVAL;
+
+	mutex_lock(&state->lock);
+	ret = scd4x_write_and_fetch(state, CMD_FRC, arg, &val, 3);
+	mutex_unlock(&state->lock);
+
+	if (val == 0xff) {
+		dev_err(state->dev, "forced calibration has failed");
+		return -EINVAL;
+	}
+
+	return ret ?: len;
+}
+
+static IIO_DEVICE_ATTR_RW(calibration_auto_enable, 0);
+static IIO_DEVICE_ATTR_WO(calibration_forced_value, 0);
+
+static struct attribute *scd4x_attrs[] = {
+	&iio_dev_attr_calibration_auto_enable.dev_attr.attr,
+	&iio_dev_attr_calibration_forced_value.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group scd4x_attr_group = {
+	.attrs = scd4x_attrs,
+};
+static const struct iio_info scd4x_info = {
+	.attrs = &scd4x_attr_group,
+	.read_raw = scd4x_read_raw,
+	.write_raw = scd4x_write_raw,
+};
+
+static const struct iio_chan_spec scd4x_channels[] = {
+	{
+		.type = IIO_CONCENTRATION,
+		.channel2 = IIO_MOD_CO2,
+		.modified = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.address = SCD4X_CO2,
+		.scan_index = SCD4X_CO2,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_BE,
+		},
+	},
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)|
+				      BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.address = SCD4X_TEMP,
+		.scan_index = SCD4X_TEMP,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_BE,
+		},
+	},
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.address = SCD4X_HR,
+		.scan_index = SCD4X_HR,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_BE,
+		},
+	},
+};
+
+static int __maybe_unused scd4x_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct scd4x_state *state  = iio_priv(indio_dev);
+	int ret;
+
+	ret = scd4x_send_command(state, CMD_STOP_MEAS);
+	if (ret)
+		return ret;
+
+	return regulator_disable(state->vdd);
+}
+
+static int __maybe_unused scd4x_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct scd4x_state *state = iio_priv(indio_dev);
+	int ret;
+
+	ret = regulator_enable(state->vdd);
+	if (ret)
+		return ret;
+
+	return scd4x_send_command(state, CMD_START_MEAS);
+}
+
+static __maybe_unused SIMPLE_DEV_PM_OPS(scd4x_pm_ops, scd4x_suspend, scd4x_resume);
+
+static void scd4x_stop_meas(void *data)
+{
+	struct scd4x_state *state = data;
+
+	scd4x_send_command(state, CMD_STOP_MEAS);
+}
+
+static void scd4x_disable_regulator(void *data)
+{
+	struct scd4x_state *state = data;
+
+	regulator_disable(state->vdd);
+}
+
+static irqreturn_t scd4x_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct scd4x_state *state = iio_priv(indio_dev);
+	struct {
+		int data[3];
+		int64_t ts __aligned(8);
+	} scan;
+	int ret;
+
+	mutex_lock(&state->lock);
+	ret = scd4x_read_poll(state);
+	memset(&scan, 0, sizeof(scan));
+	memcpy(scan.data, state->meas, sizeof(state->meas));
+	mutex_unlock(&state->lock);
+	if (ret)
+		goto out;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static int scd4x_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct scd4x_state *st = iio_priv(indio_dev);
+
+	if (state)
+		enable_irq(st->irq);
+	else
+		disable_irq(st->irq);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops scd4x_trigger_ops = {
+	.set_trigger_state = scd4x_set_trigger_state,
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static int scd4x_setup_trigger(struct iio_dev *indio_dev)
+{
+	struct device *dev = indio_dev->dev.parent;
+	struct iio_trigger *trig;
+	int ret;
+
+	trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+				      iio_device_id(indio_dev));
+	if (!trig) {
+		dev_err(dev, "failed to allocate trigger\n");
+		return -ENOMEM;
+	}
+
+	trig->ops = &scd4x_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = devm_iio_trigger_register(dev, trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(trig);
+
+	return ret;
+}
+
+static int scd4x_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	static const unsigned long scd4x_scan_masks[] = { 0x07, 0x00 };
+	struct device *dev = &client->dev;
+	struct iio_dev *indio_dev;
+	struct scd4x_state *state;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(indio_dev);
+	mutex_init(&state->lock);
+	state->dev = dev;
+	crc8_populate_msb(scd4x_crc8_table, SCD4X_CRC8_POLYNOMIAL);
+
+	indio_dev->info = &scd4x_info;
+	indio_dev->name = client->name;
+	indio_dev->channels = scd4x_channels;
+	indio_dev->num_channels = ARRAY_SIZE(scd4x_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
+	indio_dev->available_scan_masks = scd4x_scan_masks;
+
+	state->vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(state->vdd))
+		return dev_err_probe(dev, PTR_ERR(state->vdd), "failed to get regulator\n");
+
+	ret = regulator_enable(state->vdd);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, scd4x_disable_regulator, state);
+	if (ret)
+		return ret;
+
+	ret = scd4x_send_command(state, CMD_STOP_MEAS);
+	if (ret) {
+		dev_err(dev, "failed to stop measurement: %d\n", ret);
+		return ret;
+	}
+
+	/* execution time */
+	msleep_interruptible(500);
+
+	if (state->irq > 0) {
+		ret = scd4x_setup_trigger(indio_dev);
+		if (ret) {
+			dev_err(dev, "failed to setup trigger: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, scd4x_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to register iio device\n");
+		return ret;
+	}
+
+	ret = scd4x_send_command(state, CMD_START_MEAS);
+	if (ret) {
+		dev_err(dev, "failed to start measurement: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, scd4x_stop_meas, state);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id scd4x_dt_ids[] = {
+	{ .compatible = "sensirion,scd40" },
+	{ .compatible = "sensirion,scd41" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, scd4x_dt_ids);
+
+static struct i2c_driver scd4x_i2c_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = scd4x_dt_ids,
+		.pm = &scd4x_pm_ops
+	},
+	.probe = scd4x_probe,
+};
+module_i2c_driver(scd4x_i2c_driver);
+
+MODULE_AUTHOR("Roan van Dijk <roan@protonic.nl>");
+MODULE_DESCRIPTION("Sensirion SCD4X carbon dioxide sensor core driver");
+MODULE_LICENSE("GPL v2");
-- 
2.30.2

Re: [PATCH v1 1/3] dt-bindings: iio: chemical: sensirion,scd4x: Add yaml description

[Rob Herring]

On Wed, 01 Sep 2021 12:59:09 +0200, Roan van Dijk wrote:
> Add documentation for the SCD4x carbon dioxide sensor from Sensirion.
> 
> Signed-off-by: Roan van Dijk <roan@protonic.nl>
> ---
>  .../iio/chemical/sensirion,scd4x.yaml         | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v1 1/3] dt-bindings: iio: chemical: sensirion,scd4x: Add yaml description

[Roan van Dijk]

On 03-09-2021 21:51, Rob Herring wrote:
> On Wed, 01 Sep 2021 12:59:09 +0200, Roan van Dijk wrote:
>> Add documentation for the SCD4x carbon dioxide sensor from Sensirion.
>>
>> Signed-off-by: Roan van Dijk <roan@protonic.nl>
>> ---
>>   .../iio/chemical/sensirion,scd4x.yaml         | 46 +++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
>>
> Reviewed-by: Rob Herring <robh@kernel.org>
Sorry, I forgot about this. It will be added to the next patch.

Thanks,

Roan

Re: [PATCH v1 3/3] drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Tue, 7 Sep 2021 16:10:40 +0200
Roan van Dijk <roan@protonic.nl> wrote:

> On 05-09-2021 16:22, Jonathan Cameron wrote:
> > On Wed,  1 Sep 2021 12:59:11 +0200
> > Roan van Dijk <roan@protonic.nl> wrote:
> >  
> >> This is a driver for the SCD4x CO2 sensor from Sensirion. The sensor is
> >> able to measure CO2 concentration, temperature and relative humdity.
> >> The sensor uses a photoacoustic principle for measuring CO2 concentration.
> >> An I2C interface is supported by this driver in order to communicate with
> >> the sensor.
> >>
> >> Signed-off-by: Roan van Dijk <roan@protonic.nl>  
> > Hi Roan,
> >
> > Various comments inline.
> >
> > Thanks,
> >
> > Jonathan  
> Hi Jonathan,
> 
> Thank you for your feedback. Changes due to your
> comments will be added in the next patch.
> 
> However, I have some questions about some of the comments.
> I added my questions to your inline comments.
> Could you help me with these questions?
> 
> Thanks,
> 
> Roan
Replies follow,

Jonathan

> >> +static int scd4x_read_raw(struct iio_dev *indio_dev,
> >> +			struct iio_chan_spec const *chan, int *val,
> >> +			int *val2, long mask)
> >> +{
> >> +	struct scd4x_state *state = iio_priv(indio_dev);
> >> +	int ret;
> >> +	uint16_t tmp;
> >> +
> >> +	mutex_lock(&state->lock);
> >> +	switch (mask) {
> >> +	case IIO_CHAN_INFO_RAW:
> >> +	case IIO_CHAN_INFO_PROCESSED:
> >> +		ret = iio_device_claim_direct_mode(indio_dev);
> >> +		if (ret)
> >> +			break;
> >> +
> >> +		ret = scd4x_read_poll(state);  
> > Perhaps neater to wrap scd4x_read_poll() as scd4x_read_channel() that
> > returns the value for the requested channel.  That would make this
> > look like
> > 		
> > 		ret = scd4x_read_poll(state, chan->address);
> > 		iio_device_release_direct_mode();
> > 		if (ret)
> > 			return ret;
> >
> > 		*val = ret;
> > 		return IIO_VAL_INT;  
> The scd4x_read_poll() can return errors, like -ETIMEOUT. In that case
> we need to check here if ret has returned an error or data for the
> requested channel. Which could make the readability a bit harder.

This is pretty common for read type functions so readability impact
is not great.

> Besides the scd4x_read_poll() is also used in scd4x_trigger_handler()
> to read all channels at once instead of just one channel. So I think
> changing it to a request for one channel at a time, will not make it neater.

I'm suggesting a wrapper around that function not removing it.
What I would like to avoid is 'stashing' temporary variables inside
the state structure when there is not need.  An alternative would be to
pass in a buffer on the stack and put the value in that. 

> Or am I mistaken? I could still rename the function to
> scd4x_read_channels() if that makes more sense or do you have
> other suggestions?

The rename is a good idea anyway.

> >> +		if (ret) {
> >> +			iio_device_release_direct_mode(indio_dev);
> >> +			break;
> >> +		}
> >> +		*val = state->meas[chan->address];  
> > If you need to protect a local variable, use a local lock as well as
> > iio_device_release_direct_mode().  Today that function provides the
> > mutual exclusion you want but it isn't necessarily always going to do
> > so...
> >  
> >> +		iio_device_release_direct_mode(indio_dev);
> >> +		ret = IIO_VAL_INT;
> >> +		break;
> >> +	case IIO_CHAN_INFO_CALIBBIAS:
> >> +		ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, 3);
> >> +		if (ret)
> >> +			break;
> >> +		*val = 175000 * be16_to_cpu(tmp) / 65536;
> >> +		ret = IIO_VAL_INT;
> >> +		break;
> >> +	}
> >> +	mutex_unlock(&state->lock);
> >> +
> >> +	return ret;
> >> +}

...


> >> +
> >> +static ssize_t calibration_forced_value_store(struct device *dev,
> >> +					struct device_attribute *attr,
> >> +					const char *buf, size_t len)
> >> +{
> >> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >> +	struct scd4x_state *state = iio_priv(indio_dev);
> >> +	uint16_t val, arg;
> >> +	int ret;
> >> +
> >> +	ret = kstrtou16(buf, 0, &arg);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	if (arg < SCD4X_FRC_MIN_PPM || arg > SCD4X_FRC_MAX_PPM)
> >> +		return -EINVAL;
> >> +
> >> +	mutex_lock(&state->lock);
> >> +	ret = scd4x_write_and_fetch(state, CMD_FRC, arg, &val, 3);
> >> +	mutex_unlock(&state->lock);
> >> +
> >> +	if (val == 0xff) {
> >> +		dev_err(state->dev, "forced calibration has failed");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	return ret ?: len;
> >> +}
> >> +
> >> +static IIO_DEVICE_ATTR_RW(calibration_auto_enable, 0);
> >> +static IIO_DEVICE_ATTR_WO(calibration_forced_value, 0);
> >> +
> >> +static struct attribute *scd4x_attrs[] = {
> >> +	&iio_dev_attr_calibration_auto_enable.dev_attr.attr,  
> > Documentation in Documentation/ABI/sysfs-bus-iio-scd4x or similar needed.  
> What do you think is the best way to do this? To keep the ABI as simple
> as possible, I used the same attributes for scd4x as used by the scd30 
> driver.
> These attributes are already described in
> Documentation/ABI/sysfs-bus-iio-scd30. So should I make a new file for
> the scd4x with the same information or maybe rename the scd30 file
> to a shared file like Documentation/ABI/sysfs-bus-iio-scdxx?

Unfortunately the documentation build system doesn't support repeated
naming in multiple files.  So you need to pull this into a common file.
Given it isn't c02 sensor specific, move it from the scd30 to the
main sysfs-bus-iio file.

> >> +	&iio_dev_attr_calibration_forced_value.dev_attr.attr,
> >> +	NULL
> >> +};
> >> +

...