[PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[PATCH v5 0/4] 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 v5:
scd4x.c:
  - Fix bug in trigger_handler

Changes since v4:
scd4x.c:
  - Minor fixes in documentation
  - Reorder trigger_handler so memcpy is not needed anymore
Documentation:
  - Change information about the KernelVersion for the 
    calibration_forced_value_available

Changes since v3:
scd4x.c
  - Change read and write_and_fetch function parameter. CRC byte is now
    hidden inside the function.
  - Fix minor style issues
  - Add calibration_forced_value_available attribute to the driver
  - Remove including BUFFER_TRIGGERED
  - Change calibbias to raw ADC readings rather than converting it to
    milli degrees C.
Documentation:
  - Change description of driver attributes
  - Add calibration_forced_value_available documentation

Changes since v2:
scd4x.c:
  - Change boolean operations
  - Document scope of lock
  - Remove device *dev from struct
  - Add goto block for errror handling
  - Add function to read value per channel in read_raw
  - Fix bug with lock in error paths
  - Remove conversion of humidity and temperature values
  - Add scale and offset to temperature channel
  - Add scale to humidity channel
  - Move memset out of locked section
  - Remove unused irq functions
  - Move device register at end of probe function
Documentation:
  - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
  - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30

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 (4):
  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: documentation: Document scd4x calibration use

 Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
 Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
 .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
 MAINTAINERS                                   |   6 +
 drivers/iio/chemical/Kconfig                  |  13 +
 drivers/iio/chemical/Makefile                 |   1 +
 drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
 7 files changed, 796 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
 create mode 100644 drivers/iio/chemical/scd4x.c

-- 
2.30.2

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

[Roan van Dijk]

Add documentation for the SCD4x carbon dioxide sensor from Sensirion.

Reviewed-by: Rob Herring <robh@kernel.org>
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 v5 2/4] 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 a4a0c2baaf27..99b559cdc1b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16879,6 +16879,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 SGP40 GAS SENSOR DRIVER
 M:	Andreas Klinger <ak@it-klinger.de>
 S:	Maintained
-- 
2.30.2

[PATCH v5 3/4] 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  | 689 ++++++++++++++++++++++++++++++++++
 3 files changed, 703 insertions(+)
 create mode 100644 drivers/iio/chemical/scd4x.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index c03667e62732..f4d2fcb1b9ac 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 d07af581f234..81b29d9eb6a2 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_SENSIRION_SGP40)	+= sgp40.o
 obj-$(CONFIG_SPS30) += sps30.o
diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
new file mode 100644
index 000000000000..09b34201c42b
--- /dev/null
+++ b/drivers/iio/chemical/scd4x.c
@@ -0,0 +1,689 @@
+// 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 0
+#define SCD4X_FRC_MAX_PPM 2000
+#define SCD4X_READY_MASK 0x01
+
+/*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;
+	/* maintain access to device, to prevent concurrent reads/writes */
+	struct mutex lock;
+	struct regulator *vdd;
+};
+
+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 = state->client;
+	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 response_sz)
+{
+	struct i2c_client *client = state->client;
+	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);
+	}
+
+	/* CRC byte for every 2 bytes of data */
+	response_sz += response_sz / 2;
+
+	put_unaligned_be16(cmd, buf);
+	ret = scd4x_i2c_xfer(state, buf, 2, buf, response_sz);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < response_sz; i += 3) {
+		crc = crc8(scd4x_crc8_table, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(&client->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 response_sz)
+{
+	struct i2c_client *client = state->client;
+	char buf[SCD4X_READ_BUF_SIZE];
+	char *rsp = response;
+	int i, ret;
+	char crc;
+
+	ret = scd4x_write(state, CMD_FRC, arg);
+	if (ret)
+		goto err;
+
+	/* execution time */
+	msleep_interruptible(400);
+
+	/* CRC byte for every 2 bytes of data */
+	response_sz += response_sz / 2;
+
+	ret = i2c_master_recv(client, buf, response_sz);
+	if (ret < 0)
+		goto err;
+	if (ret != response_sz) {
+		ret = -EIO;
+		goto err;
+	}
+
+	for (i = 0; i < response_sz; i += 3) {
+		crc = crc8(scd4x_crc8_table, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(&client->dev, "CRC error\n");
+			ret = -EIO;
+			goto err;
+		}
+
+		*rsp++ = buf[i];
+		*rsp++ = buf[i + 1];
+	}
+
+	return scd4x_send_command(state, CMD_START_MEAS);
+
+err:
+	/*
+	 * on error try to start the measurement,
+	 * puts sensor back into continuous measurement
+	 */
+	scd4x_send_command(state, CMD_START_MEAS);
+
+	return ret;
+}
+
+static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
+{
+	int i, ret;
+	uint16_t buf[3];
+
+	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(buf); i++)
+		meas[i] = be16_to_cpu(buf[i]);
+
+	return 0;
+}
+
+static int scd4x_wait_meas_poll(struct scd4x_state *state)
+{
+	struct i2c_client *client = state->client;
+	int tries = 6;
+	int ret;
+
+	do {
+		uint16_t val;
+
+		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
+		if (ret)
+			return -EIO;
+		val = be16_to_cpu(val);
+
+		/* new measurement available */
+		if (val & 0x7FF)
+			return 0;
+
+		msleep_interruptible(1000);
+	} while (--tries);
+
+	/* try to start sensor on timeout */
+	ret = scd4x_send_command(state, CMD_START_MEAS);
+	if (ret)
+		dev_err(&client->dev, "failed to start measurement: %d\n", ret);
+
+	return -ETIMEDOUT;
+}
+
+static int scd4x_read_poll(struct scd4x_state *state, uint16_t *buf)
+{
+	int ret;
+
+	ret = scd4x_wait_meas_poll(state);
+	if (ret)
+		return ret;
+
+	return scd4x_read_meas(state, buf);
+}
+
+static int scd4x_read_channel(struct scd4x_state *state, int chan)
+{
+	int ret;
+	uint16_t buf[3];
+
+	ret = scd4x_read_poll(state, buf);
+	if (ret)
+		return ret;
+
+	return buf[chan];
+}
+
+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;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+
+		mutex_lock(&state->lock);
+		ret = scd4x_read_channel(state, chan->address);
+		mutex_unlock(&state->lock);
+
+		iio_device_release_direct_mode(indio_dev);
+		if (ret < 0)
+			return ret;
+
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_TEMP) {
+			*val = 175000;
+			*val2 = 65536;
+			return IIO_VAL_FRACTIONAL;
+		} else if (chan->type == IIO_HUMIDITYRELATIVE) {
+			*val = 100000;
+			*val2 = 65536;
+			return IIO_VAL_FRACTIONAL;
+		}
+		return -EINVAL;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -16852;
+		*val2 = 114286;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		mutex_lock(&state->lock);
+		ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
+		mutex_unlock(&state->lock);
+		if (ret)
+			return ret;
+
+		*val = be16_to_cpu(tmp);
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+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;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBBIAS:
+		mutex_lock(&state->lock);
+		ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
+		mutex_unlock(&state->lock);
+
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+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, sizeof(val));
+	mutex_unlock(&state->lock);
+	if (ret) {
+		dev_err(dev, "failed to read automatic calibration");
+		return ret;
+	}
+
+	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
+
+	return 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, sizeof(val));
+	mutex_unlock(&state->lock);
+
+	if (val == 0xff) {
+		dev_err(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 IIO_CONST_ATTR(calibration_forced_value_available,
+	       __stringify([SCD4X_FRC_MIN_PPM 1 SCD4X_FRC_MAX_PPM]));
+
+static struct attribute *scd4x_attrs[] = {
+	&iio_dev_attr_calibration_auto_enable.dev_attr.attr,
+	&iio_dev_attr_calibration_forced_value.dev_attr.attr,
+	&iio_const_attr_calibration_forced_value_available.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_RAW) |
+					BIT(IIO_CHAN_INFO_SCALE) |
+					BIT(IIO_CHAN_INFO_OFFSET) |
+					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_RAW) |
+					BIT(IIO_CHAN_INFO_SCALE),
+		.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 *state)
+{
+	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 {
+		uint16_t data[3];
+		int64_t ts __aligned(8);
+	} scan;
+	int ret;
+
+	memset(&scan, 0, sizeof(scan));
+	mutex_lock(&state->lock);
+	ret = scd4x_read_poll(state, scan.data);
+	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_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->client = client;
+	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_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);
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, scd4x_trigger_handler, NULL);
+	if (ret)
+		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 devm_iio_device_register(dev, indio_dev);
+}
+
+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 v5 3/4] drivers: iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Randy Dunlap]

On 10/8/21 3:17 AM, Roan van Dijk wrote:
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index c03667e62732..f4d2fcb1b9ac 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

	                                                                  carbon

> +	  dioxide, relative humidity and temperature sensing capabilities

	                                                     capabilities.

> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called scd4x.


-- 
~Randy

[PATCH v5 4/4] iio: documentation: Document scd4x calibration use

[Roan van Dijk]

Add entries from Documentation/ABI/testing/sysfs-bus-iio-scd30
to Documentation/ABI/testing/sysfs-bus-iio. The attributes of the scd4x
and scd30 are common.

Remove Documentation/ABI/testing/sysfs-bus-iio-scd30.

Signed-off-by: Roan van Dijk <roan@protonic.nl>
---
 Documentation/ABI/testing/sysfs-bus-iio       | 41 +++++++++++++++++++
 Documentation/ABI/testing/sysfs-bus-iio-scd30 | 34 ---------------
 2 files changed, 41 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 6ad47a67521c..c27347d3608e 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1957,3 +1957,44 @@ Description:
 		Specify the percent for light sensor relative to the channel
 		absolute value that a data field should change before an event
 		is generated. Units are a percentage of the prior reading.
+
+What:		/sys/bus/iio/devices/iio:deviceX/calibration_auto_enable
+Date:		June 2020
+KernelVersion:	5.8
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Some sensors have the ability to apply auto calibration at
+		runtime. For example, it may be necessary to compensate for
+		contaminant build-up in a measurement chamber or optical
+		element deterioration that would otherwise lead to sensor drift.
+
+		Writing 1 or 0 to this attribute will respectively activate or
+		deactivate this auto calibration function.
+
+		Upon reading, the current status is returned.
+
+What:		/sys/bus/iio/devices/iio:deviceX/calibration_forced_value
+Date:		June 2020
+KernelVersion:	5.8
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Some sensors have the ability to apply a manual calibration using
+		a known measurement value, perhaps obtained from an external
+		reference device.
+
+		Writing a value to this function will force such a calibration
+		change. For the scd30 the value should be from the range
+		[400 1 2000].
+
+		Note for the scd30 that a valid value may only be obtained once
+		it is has been written. Until then any read back of this value
+		should be ignored. As for the scd4x an error will be returned
+		immediately if the manual calibration has failed.
+
+What:		/sys/bus/iio/devices/iio:deviceX/calibration_forced_value_available
+KernelVersion:  5.15
+Contact:        linux-iio@vger.kernel.org
+Description:
+		Available range for the forced calibration value, expressed as:
+
+		- a range specified as "[min step max]"
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
deleted file mode 100644
index b9712f390bec..000000000000
--- a/Documentation/ABI/testing/sysfs-bus-iio-scd30
+++ /dev/null
@@ -1,34 +0,0 @@
-What:		/sys/bus/iio/devices/iio:deviceX/calibration_auto_enable
-Date:		June 2020
-KernelVersion:	5.8
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Contaminants build-up in the measurement chamber or optical
-		elements deterioration leads to sensor drift.
-
-		One can compensate for sensor drift by using automatic self
-		calibration procedure (asc).
-
-		Writing 1 or 0 to this attribute will respectively activate or
-		deactivate asc.
-
-		Upon reading current asc status is returned.
-
-What:		/sys/bus/iio/devices/iio:deviceX/calibration_forced_value
-Date:		June 2020
-KernelVersion:	5.8
-Contact:	linux-iio@vger.kernel.org
-Description:
-		Contaminants build-up in the measurement chamber or optical
-		elements deterioration leads to sensor drift.
-
-		One can compensate for sensor drift by using forced
-		recalibration (frc). This is useful in case there's known
-		co2 reference available nearby the sensor.
-
-		Picking value from the range [400 1 2000] and writing it to the
-		sensor will set frc.
-
-		Upon reading current frc value is returned. Note that after
-		power cycling default value (i.e 400) is returned even though
-		internally sensor had recalibrated itself.
-- 
2.30.2

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Fri,  8 Oct 2021 12:17:02 +0200
Roan van Dijk <roan@protonic.nl> wrote:

> 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?

Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
up. Pushed out as testing for 0-day to see if it can find anything we missed

Thanks,

Jonathan

> 
> Changes since v5:
> scd4x.c:
>   - Fix bug in trigger_handler
> 
> Changes since v4:
> scd4x.c:
>   - Minor fixes in documentation
>   - Reorder trigger_handler so memcpy is not needed anymore
> Documentation:
>   - Change information about the KernelVersion for the 
>     calibration_forced_value_available
> 
> Changes since v3:
> scd4x.c
>   - Change read and write_and_fetch function parameter. CRC byte is now
>     hidden inside the function.
>   - Fix minor style issues
>   - Add calibration_forced_value_available attribute to the driver
>   - Remove including BUFFER_TRIGGERED
>   - Change calibbias to raw ADC readings rather than converting it to
>     milli degrees C.
> Documentation:
>   - Change description of driver attributes
>   - Add calibration_forced_value_available documentation
> 
> Changes since v2:
> scd4x.c:
>   - Change boolean operations
>   - Document scope of lock
>   - Remove device *dev from struct
>   - Add goto block for errror handling
>   - Add function to read value per channel in read_raw
>   - Fix bug with lock in error paths
>   - Remove conversion of humidity and temperature values
>   - Add scale and offset to temperature channel
>   - Add scale to humidity channel
>   - Move memset out of locked section
>   - Remove unused irq functions
>   - Move device register at end of probe function
> Documentation:
>   - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
>   - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
> 
> 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 (4):
>   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: documentation: Document scd4x calibration use
> 
>  Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
>  Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
>  .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
>  MAINTAINERS                                   |   6 +
>  drivers/iio/chemical/Kconfig                  |  13 +
>  drivers/iio/chemical/Makefile                 |   1 +
>  drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
>  7 files changed, 796 insertions(+), 34 deletions(-)
>  delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
>  create mode 100644 drivers/iio/chemical/scd4x.c
> 

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Sun, 10 Oct 2021 16:59:19 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  8 Oct 2021 12:17:02 +0200
> Roan van Dijk <roan@protonic.nl> wrote:
> 
> > 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?  
> 
> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
> up. Pushed out as testing for 0-day to see if it can find anything we missed

And indeed - I missed a bunch of places where explicit __be16 types should have
been used.

I've applied the following fixup, shout if it's wrong.


diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
index 09b34201c42b..ebebcb117ba2 100644
--- a/drivers/iio/chemical/scd4x.c
+++ b/drivers/iio/chemical/scd4x.c
@@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
 static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
 {
 	int i, ret;
-	uint16_t buf[3];
+	__be16 buf[3];
 
 	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
 	if (ret)
@@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
 	int ret;
 
 	do {
+		__be16 bval;
 		uint16_t val;
 
-		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
+		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
 		if (ret)
 			return -EIO;
-		val = be16_to_cpu(val);
+		val = be16_to_cpu(bval);
 
 		/* new measurement available */
 		if (val & 0x7FF)
@@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
 {
 	struct scd4x_state *state = iio_priv(indio_dev);
 	int ret;
-	uint16_t tmp;
+	__be16 tmp;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct scd4x_state *state = iio_priv(indio_dev);
 	int ret;
-	uint16_t val;
+	__be16 bval;
+	u16 val;
 
 	mutex_lock(&state->lock);
-	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
+	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
 	mutex_unlock(&state->lock);
 	if (ret) {
 		dev_err(dev, "failed to read automatic calibration");
 		return ret;
 	}
 
-	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
+	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
 
 	return sprintf(buf, "%d\n", val);
 }


> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Changes since v5:
> > scd4x.c:
> >   - Fix bug in trigger_handler
> > 
> > Changes since v4:
> > scd4x.c:
> >   - Minor fixes in documentation
> >   - Reorder trigger_handler so memcpy is not needed anymore
> > Documentation:
> >   - Change information about the KernelVersion for the 
> >     calibration_forced_value_available
> > 
> > Changes since v3:
> > scd4x.c
> >   - Change read and write_and_fetch function parameter. CRC byte is now
> >     hidden inside the function.
> >   - Fix minor style issues
> >   - Add calibration_forced_value_available attribute to the driver
> >   - Remove including BUFFER_TRIGGERED
> >   - Change calibbias to raw ADC readings rather than converting it to
> >     milli degrees C.
> > Documentation:
> >   - Change description of driver attributes
> >   - Add calibration_forced_value_available documentation
> > 
> > Changes since v2:
> > scd4x.c:
> >   - Change boolean operations
> >   - Document scope of lock
> >   - Remove device *dev from struct
> >   - Add goto block for errror handling
> >   - Add function to read value per channel in read_raw
> >   - Fix bug with lock in error paths
> >   - Remove conversion of humidity and temperature values
> >   - Add scale and offset to temperature channel
> >   - Add scale to humidity channel
> >   - Move memset out of locked section
> >   - Remove unused irq functions
> >   - Move device register at end of probe function
> > Documentation:
> >   - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
> >   - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
> > 
> > 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 (4):
> >   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: documentation: Document scd4x calibration use
> > 
> >  Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
> >  Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
> >  .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
> >  MAINTAINERS                                   |   6 +
> >  drivers/iio/chemical/Kconfig                  |  13 +
> >  drivers/iio/chemical/Makefile                 |   1 +
> >  drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
> >  7 files changed, 796 insertions(+), 34 deletions(-)
> >  delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> >  create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
> >  create mode 100644 drivers/iio/chemical/scd4x.c
> >   
> 

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Roan van Dijk]

On 13-10-2021 19:38, Jonathan Cameron wrote:
> On Sun, 10 Oct 2021 16:59:19 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> On Fri,  8 Oct 2021 12:17:02 +0200
>> Roan van Dijk <roan@protonic.nl> wrote:
>>
>>> 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?
>>
>> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
>> up. Pushed out as testing for 0-day to see if it can find anything we missed
> 
> And indeed - I missed a bunch of places where explicit __be16 types should have
> been used.
> 
> I've applied the following fixup, shout if it's wrong.
>
Thank you Jonathan for applying this fixup. No need to shout :) Your 
changes should fix the issue.

However, I have a question about something else. The co2 concentration 
is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this 
moment. Is an _scale always required for an _raw in the ABI? I could not 
find anything in the documentation if there is a rule for this. Someone 
mentioned this to me, so I want to check if I did this right.

The sensor returns the actual co2 value upon reading, like 450 ppm. We 
can set an offset of this co2 value with the calibration_forced_value 
through the ABI, but this offset is handled internally by the sensor. So 
there isn't anything with scaling or an offset needed at the driver side.

Was I right by making it of type RAW? If needed we could make it more 
like the scd30 driver, keeping it of type RAW but with scale = 1. What 
should I do or is it fine as it is?

Sorry for not asking this earlier.

Thanks,

Roan

> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index 09b34201c42b..ebebcb117ba2 100644
> --- a/drivers/iio/chemical/scd4x.c
> +++ b/drivers/iio/chemical/scd4x.c
> @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
>   static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
>   {
>   	int i, ret;
> -	uint16_t buf[3];
> +	__be16 buf[3];
>   
>   	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
>   	if (ret)
> @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
>   	int ret;
>   
>   	do {
> +		__be16 bval;
>   		uint16_t val;
>   
> -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
> +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
>   		if (ret)
>   			return -EIO;
> -		val = be16_to_cpu(val);
> +		val = be16_to_cpu(bval);
>   
>   		/* new measurement available */
>   		if (val & 0x7FF)
> @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>   {
>   	struct scd4x_state *state = iio_priv(indio_dev);
>   	int ret;
> -	uint16_t tmp;
> +	__be16 tmp;
>   
>   	switch (mask) {
>   	case IIO_CHAN_INFO_RAW:
> @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
>   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   	struct scd4x_state *state = iio_priv(indio_dev);
>   	int ret;
> -	uint16_t val;
> +	__be16 bval;
> +	u16 val;
>   
>   	mutex_lock(&state->lock);
> -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
> +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
>   	mutex_unlock(&state->lock);
>   	if (ret) {
>   		dev_err(dev, "failed to read automatic calibration");
>   		return ret;
>   	}
>   
> -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
> +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
>   
>   	return sprintf(buf, "%d\n", val);
>   }
> 
> 
>>
>> Thanks,
>>
>> Jonathan
>>
>>>
>>> Changes since v5:
>>> scd4x.c:
>>>    - Fix bug in trigger_handler
>>>
>>> Changes since v4:
>>> scd4x.c:
>>>    - Minor fixes in documentation
>>>    - Reorder trigger_handler so memcpy is not needed anymore
>>> Documentation:
>>>    - Change information about the KernelVersion for the
>>>      calibration_forced_value_available
>>>
>>> Changes since v3:
>>> scd4x.c
>>>    - Change read and write_and_fetch function parameter. CRC byte is now
>>>      hidden inside the function.
>>>    - Fix minor style issues
>>>    - Add calibration_forced_value_available attribute to the driver
>>>    - Remove including BUFFER_TRIGGERED
>>>    - Change calibbias to raw ADC readings rather than converting it to
>>>      milli degrees C.
>>> Documentation:
>>>    - Change description of driver attributes
>>>    - Add calibration_forced_value_available documentation
>>>
>>> Changes since v2:
>>> scd4x.c:
>>>    - Change boolean operations
>>>    - Document scope of lock
>>>    - Remove device *dev from struct
>>>    - Add goto block for errror handling
>>>    - Add function to read value per channel in read_raw
>>>    - Fix bug with lock in error paths
>>>    - Remove conversion of humidity and temperature values
>>>    - Add scale and offset to temperature channel
>>>    - Add scale to humidity channel
>>>    - Move memset out of locked section
>>>    - Remove unused irq functions
>>>    - Move device register at end of probe function
>>> Documentation:
>>>    - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
>>>    - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>
>>> 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 (4):
>>>    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: documentation: Document scd4x calibration use
>>>
>>>   Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
>>>   Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
>>>   .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
>>>   MAINTAINERS                                   |   6 +
>>>   drivers/iio/chemical/Kconfig                  |  13 +
>>>   drivers/iio/chemical/Makefile                 |   1 +
>>>   drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
>>>   7 files changed, 796 insertions(+), 34 deletions(-)
>>>   delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>   create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
>>>   create mode 100644 drivers/iio/chemical/scd4x.c
>>>    
>>
> 

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Thu, 14 Oct 2021 10:24:54 +0200
Roan van Dijk <roan@protonic.nl> wrote:

> On 13-10-2021 19:38, Jonathan Cameron wrote:
> > On Sun, 10 Oct 2021 16:59:19 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> >> On Fri,  8 Oct 2021 12:17:02 +0200
> >> Roan van Dijk <roan@protonic.nl> wrote:
> >>  
> >>> 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?  
> >>
> >> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
> >> up. Pushed out as testing for 0-day to see if it can find anything we missed  
> > 
> > And indeed - I missed a bunch of places where explicit __be16 types should have
> > been used.
> > 
> > I've applied the following fixup, shout if it's wrong.
> >  
> Thank you Jonathan for applying this fixup. No need to shout :) Your 
> changes should fix the issue.
> 
> However, I have a question about something else. The co2 concentration 
> is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this 
> moment. Is an _scale always required for an _raw in the ABI? I could not 
> find anything in the documentation if there is a rule for this. Someone 
> mentioned this to me, so I want to check if I did this right.
> 
> The sensor returns the actual co2 value upon reading, like 450 ppm. We 
> can set an offset of this co2 value with the calibration_forced_value 
> through the ABI, but this offset is handled internally by the sensor. So 
> there isn't anything with scaling or an offset needed at the driver side.

Ah. We could have mapped this to calibbias, though here it's made more
complex by other calibrations existing that don't use the value so let's
leave it as it is.

> 
> Was I right by making it of type RAW? If needed we could make it more 
> like the scd30 driver, keeping it of type RAW but with scale = 1. What 
> should I do or is it fine as it is?

Hmm. Interesting corner case in the ABI.  A _raw value without a scale
normally means we don't know it for some reason.  The most common case
of this is light sensors where several _raw intensity values are combined
in some (typically non linear) transform to form a single measure of illuminance.
Those intensity_raw channels don't have an meaningful units, but devices
often have threshold events on them so we have to expose them.

I would say make it a processed value, but there is a quirk.
concentrations in IIO are expressed in percent not per million, so you need
a scale anyway, I guess 10000?  See Documentation/ABI/testing/sysfs-bus-iio


No need to do a new driver version, just send a patch tidying up this corner.

Thanks,

Jonathan


> Sorry for not asking this earlier.
> 
> Thanks,
> 
> Roan
> 
> > diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> > index 09b34201c42b..ebebcb117ba2 100644
> > --- a/drivers/iio/chemical/scd4x.c
> > +++ b/drivers/iio/chemical/scd4x.c
> > @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
> >   static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
> >   {
> >   	int i, ret;
> > -	uint16_t buf[3];
> > +	__be16 buf[3];
> >   
> >   	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
> >   	if (ret)
> > @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
> >   	int ret;
> >   
> >   	do {
> > +		__be16 bval;
> >   		uint16_t val;
> >   
> > -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
> > +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
> >   		if (ret)
> >   			return -EIO;
> > -		val = be16_to_cpu(val);
> > +		val = be16_to_cpu(bval);
> >   
> >   		/* new measurement available */
> >   		if (val & 0x7FF)
> > @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> >   {
> >   	struct scd4x_state *state = iio_priv(indio_dev);
> >   	int ret;
> > -	uint16_t tmp;
> > +	__be16 tmp;
> >   
> >   	switch (mask) {
> >   	case IIO_CHAN_INFO_RAW:
> > @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
> >   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >   	struct scd4x_state *state = iio_priv(indio_dev);
> >   	int ret;
> > -	uint16_t val;
> > +	__be16 bval;
> > +	u16 val;
> >   
> >   	mutex_lock(&state->lock);
> > -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
> > +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
> >   	mutex_unlock(&state->lock);
> >   	if (ret) {
> >   		dev_err(dev, "failed to read automatic calibration");
> >   		return ret;
> >   	}
> >   
> > -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
> > +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
> >   
> >   	return sprintf(buf, "%d\n", val);
> >   }
> > 
> >   
> >>
> >> Thanks,
> >>
> >> Jonathan
> >>  
> >>>
> >>> Changes since v5:
> >>> scd4x.c:
> >>>    - Fix bug in trigger_handler
> >>>
> >>> Changes since v4:
> >>> scd4x.c:
> >>>    - Minor fixes in documentation
> >>>    - Reorder trigger_handler so memcpy is not needed anymore
> >>> Documentation:
> >>>    - Change information about the KernelVersion for the
> >>>      calibration_forced_value_available
> >>>
> >>> Changes since v3:
> >>> scd4x.c
> >>>    - Change read and write_and_fetch function parameter. CRC byte is now
> >>>      hidden inside the function.
> >>>    - Fix minor style issues
> >>>    - Add calibration_forced_value_available attribute to the driver
> >>>    - Remove including BUFFER_TRIGGERED
> >>>    - Change calibbias to raw ADC readings rather than converting it to
> >>>      milli degrees C.
> >>> Documentation:
> >>>    - Change description of driver attributes
> >>>    - Add calibration_forced_value_available documentation
> >>>
> >>> Changes since v2:
> >>> scd4x.c:
> >>>    - Change boolean operations
> >>>    - Document scope of lock
> >>>    - Remove device *dev from struct
> >>>    - Add goto block for errror handling
> >>>    - Add function to read value per channel in read_raw
> >>>    - Fix bug with lock in error paths
> >>>    - Remove conversion of humidity and temperature values
> >>>    - Add scale and offset to temperature channel
> >>>    - Add scale to humidity channel
> >>>    - Move memset out of locked section
> >>>    - Remove unused irq functions
> >>>    - Move device register at end of probe function
> >>> Documentation:
> >>>    - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
> >>>    - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>
> >>> 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 (4):
> >>>    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: documentation: Document scd4x calibration use
> >>>
> >>>   Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
> >>>   Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
> >>>   .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
> >>>   MAINTAINERS                                   |   6 +
> >>>   drivers/iio/chemical/Kconfig                  |  13 +
> >>>   drivers/iio/chemical/Makefile                 |   1 +
> >>>   drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
> >>>   7 files changed, 796 insertions(+), 34 deletions(-)
> >>>   delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>   create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
> >>>   create mode 100644 drivers/iio/chemical/scd4x.c
> >>>      
> >>  
> >   

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Roan van Dijk]

On 14-10-2021 19:19, Jonathan Cameron wrote:
> On Thu, 14 Oct 2021 10:24:54 +0200
> Roan van Dijk <roan@protonic.nl> wrote:
> 
>> On 13-10-2021 19:38, Jonathan Cameron wrote:
>>> On Sun, 10 Oct 2021 16:59:19 +0100
>>> Jonathan Cameron <jic23@kernel.org> wrote:
>>>    
>>>> On Fri,  8 Oct 2021 12:17:02 +0200
>>>> Roan van Dijk <roan@protonic.nl> wrote:
>>>>   
>>>>> 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?
>>>>
>>>> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
>>>> up. Pushed out as testing for 0-day to see if it can find anything we missed
>>>
>>> And indeed - I missed a bunch of places where explicit __be16 types should have
>>> been used.
>>>
>>> I've applied the following fixup, shout if it's wrong.
>>>   
>> Thank you Jonathan for applying this fixup. No need to shout :) Your
>> changes should fix the issue.
>>
>> However, I have a question about something else. The co2 concentration
>> is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this
>> moment. Is an _scale always required for an _raw in the ABI? I could not
>> find anything in the documentation if there is a rule for this. Someone
>> mentioned this to me, so I want to check if I did this right.
>>
>> The sensor returns the actual co2 value upon reading, like 450 ppm. We
>> can set an offset of this co2 value with the calibration_forced_value
>> through the ABI, but this offset is handled internally by the sensor. So
>> there isn't anything with scaling or an offset needed at the driver side.
> 
> Ah. We could have mapped this to calibbias, though here it's made more
> complex by other calibrations existing that don't use the value so let's
> leave it as it is.
> 
>>
>> Was I right by making it of type RAW? If needed we could make it more
>> like the scd30 driver, keeping it of type RAW but with scale = 1. What
>> should I do or is it fine as it is?
> 
> Hmm. Interesting corner case in the ABI.  A _raw value without a scale
> normally means we don't know it for some reason.  The most common case
> of this is light sensors where several _raw intensity values are combined
> in some (typically non linear) transform to form a single measure of illuminance.
> Those intensity_raw channels don't have an meaningful units, but devices
> often have threshold events on them so we have to expose them.
> 
> I would say make it a processed value, but there is a quirk.
> concentrations in IIO are expressed in percent not per million, so you need
> a scale anyway, I guess 10000?  See Documentation/ABI/testing/sysfs-bus-iio
> 
> 
> No need to do a new driver version, just send a patch tidying up this corner.
> 

Hi Jonathan,

As you suggested, these are my fixes for the concentration reading.

The co2 reading is now a processed value and has a scale. I also added 
the information in sysfs-bus-iio documentation, because this type of 
processed value is new in the ABI.

diff --git a/Documentation/ABI/testing/sysfs-bus-iio 
b/Documentation/ABI/testing/sysfs-bus-iio
index c27347d3608e..66a17f4c831e 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1716,6 +1716,7 @@ Description:

  What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_raw
  What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
+What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_input
  What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_raw
  What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_co2_raw
  What: 
/sys/bus/iio/devices/iio:deviceX/in_concentration_ethanol_raw
diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
index 09b34201c42b..bc1c6676029d 100644
--- a/drivers/iio/chemical/scd4x.c
+++ b/drivers/iio/chemical/scd4x.c
@@ -337,6 +337,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,

         switch (mask) {
         case IIO_CHAN_INFO_RAW:
+       case IIO_CHAN_INFO_PROCESSED:
                 ret = iio_device_claim_direct_mode(indio_dev);
                 if (ret)
                         return ret;
@@ -352,7 +353,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
                 *val = ret;
                 return IIO_VAL_INT;
         case IIO_CHAN_INFO_SCALE:
-               if (chan->type == IIO_TEMP) {
+               if (chan->type == IIO_CONCENTRATION) {
+                       *val = 0;
+                       *val2 = 100;
+                       return IIO_VAL_INT_PLUS_MICRO;
+               } else if (chan->type == IIO_TEMP) {
                         *val = 175000;
                         *val2 = 65536;
                         return IIO_VAL_FRACTIONAL;
@@ -501,7 +506,8 @@ 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),
+               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+                                       BIT(IIO_CHAN_INFO_SCALE),
                 .address = SCD4X_CO2,
                 .scan_index = SCD4X_CO2,
                 .scan_type = {

Thanks,

Roan

> Thanks,
> 
> Jonathan
> 
> 
>> Sorry for not asking this earlier.
>>
>> Thanks,
>>
>> Roan
>>
>>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
>>> index 09b34201c42b..ebebcb117ba2 100644
>>> --- a/drivers/iio/chemical/scd4x.c
>>> +++ b/drivers/iio/chemical/scd4x.c
>>> @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
>>>    static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
>>>    {
>>>    	int i, ret;
>>> -	uint16_t buf[3];
>>> +	__be16 buf[3];
>>>    
>>>    	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
>>>    	if (ret)
>>> @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
>>>    	int ret;
>>>    
>>>    	do {
>>> +		__be16 bval;
>>>    		uint16_t val;
>>>    
>>> -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
>>> +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
>>>    		if (ret)
>>>    			return -EIO;
>>> -		val = be16_to_cpu(val);
>>> +		val = be16_to_cpu(bval);
>>>    
>>>    		/* new measurement available */
>>>    		if (val & 0x7FF)
>>> @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>>>    {
>>>    	struct scd4x_state *state = iio_priv(indio_dev);
>>>    	int ret;
>>> -	uint16_t tmp;
>>> +	__be16 tmp;
>>>    
>>>    	switch (mask) {
>>>    	case IIO_CHAN_INFO_RAW:
>>> @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
>>>    	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>    	struct scd4x_state *state = iio_priv(indio_dev);
>>>    	int ret;
>>> -	uint16_t val;
>>> +	__be16 bval;
>>> +	u16 val;
>>>    
>>>    	mutex_lock(&state->lock);
>>> -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
>>> +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
>>>    	mutex_unlock(&state->lock);
>>>    	if (ret) {
>>>    		dev_err(dev, "failed to read automatic calibration");
>>>    		return ret;
>>>    	}
>>>    
>>> -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
>>> +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
>>>    
>>>    	return sprintf(buf, "%d\n", val);
>>>    }
>>>
>>>    
>>>>
>>>> Thanks,
>>>>
>>>> Jonathan
>>>>   
>>>>>
>>>>> Changes since v5:
>>>>> scd4x.c:
>>>>>     - Fix bug in trigger_handler
>>>>>
>>>>> Changes since v4:
>>>>> scd4x.c:
>>>>>     - Minor fixes in documentation
>>>>>     - Reorder trigger_handler so memcpy is not needed anymore
>>>>> Documentation:
>>>>>     - Change information about the KernelVersion for the
>>>>>       calibration_forced_value_available
>>>>>
>>>>> Changes since v3:
>>>>> scd4x.c
>>>>>     - Change read and write_and_fetch function parameter. CRC byte is now
>>>>>       hidden inside the function.
>>>>>     - Fix minor style issues
>>>>>     - Add calibration_forced_value_available attribute to the driver
>>>>>     - Remove including BUFFER_TRIGGERED
>>>>>     - Change calibbias to raw ADC readings rather than converting it to
>>>>>       milli degrees C.
>>>>> Documentation:
>>>>>     - Change description of driver attributes
>>>>>     - Add calibration_forced_value_available documentation
>>>>>
>>>>> Changes since v2:
>>>>> scd4x.c:
>>>>>     - Change boolean operations
>>>>>     - Document scope of lock
>>>>>     - Remove device *dev from struct
>>>>>     - Add goto block for errror handling
>>>>>     - Add function to read value per channel in read_raw
>>>>>     - Fix bug with lock in error paths
>>>>>     - Remove conversion of humidity and temperature values
>>>>>     - Add scale and offset to temperature channel
>>>>>     - Add scale to humidity channel
>>>>>     - Move memset out of locked section
>>>>>     - Remove unused irq functions
>>>>>     - Move device register at end of probe function
>>>>> Documentation:
>>>>>     - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
>>>>>     - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>>>
>>>>> 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 (4):
>>>>>     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: documentation: Document scd4x calibration use
>>>>>
>>>>>    Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
>>>>>    Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
>>>>>    .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
>>>>>    MAINTAINERS                                   |   6 +
>>>>>    drivers/iio/chemical/Kconfig                  |  13 +
>>>>>    drivers/iio/chemical/Makefile                 |   1 +
>>>>>    drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
>>>>>    7 files changed, 796 insertions(+), 34 deletions(-)
>>>>>    delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>>>    create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
>>>>>    create mode 100644 drivers/iio/chemical/scd4x.c
>>>>>       
>>>>   
>>>    
> 

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Mon, 18 Oct 2021 10:19:42 +0200
Roan van Dijk <roan@protonic.nl> wrote:

> On 14-10-2021 19:19, Jonathan Cameron wrote:
> > On Thu, 14 Oct 2021 10:24:54 +0200
> > Roan van Dijk <roan@protonic.nl> wrote:
> >   
> >> On 13-10-2021 19:38, Jonathan Cameron wrote:  
> >>> On Sun, 10 Oct 2021 16:59:19 +0100
> >>> Jonathan Cameron <jic23@kernel.org> wrote:
> >>>      
> >>>> On Fri,  8 Oct 2021 12:17:02 +0200
> >>>> Roan van Dijk <roan@protonic.nl> wrote:
> >>>>     
> >>>>> 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?  
> >>>>
> >>>> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
> >>>> up. Pushed out as testing for 0-day to see if it can find anything we missed  
> >>>
> >>> And indeed - I missed a bunch of places where explicit __be16 types should have
> >>> been used.
> >>>
> >>> I've applied the following fixup, shout if it's wrong.
> >>>     
> >> Thank you Jonathan for applying this fixup. No need to shout :) Your
> >> changes should fix the issue.
> >>
> >> However, I have a question about something else. The co2 concentration
> >> is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this
> >> moment. Is an _scale always required for an _raw in the ABI? I could not
> >> find anything in the documentation if there is a rule for this. Someone
> >> mentioned this to me, so I want to check if I did this right.
> >>
> >> The sensor returns the actual co2 value upon reading, like 450 ppm. We
> >> can set an offset of this co2 value with the calibration_forced_value
> >> through the ABI, but this offset is handled internally by the sensor. So
> >> there isn't anything with scaling or an offset needed at the driver side.  
> > 
> > Ah. We could have mapped this to calibbias, though here it's made more
> > complex by other calibrations existing that don't use the value so let's
> > leave it as it is.
> >   
> >>
> >> Was I right by making it of type RAW? If needed we could make it more
> >> like the scd30 driver, keeping it of type RAW but with scale = 1. What
> >> should I do or is it fine as it is?  
> > 
> > Hmm. Interesting corner case in the ABI.  A _raw value without a scale
> > normally means we don't know it for some reason.  The most common case
> > of this is light sensors where several _raw intensity values are combined
> > in some (typically non linear) transform to form a single measure of illuminance.
> > Those intensity_raw channels don't have an meaningful units, but devices
> > often have threshold events on them so we have to expose them.
> > 
> > I would say make it a processed value, but there is a quirk.
> > concentrations in IIO are expressed in percent not per million, so you need
> > a scale anyway, I guess 10000?  See Documentation/ABI/testing/sysfs-bus-iio
> > 
> > 
> > No need to do a new driver version, just send a patch tidying up this corner.
> >   
> 
> Hi Jonathan,
> 
> As you suggested, these are my fixes for the concentration reading.
> 
> The co2 reading is now a processed value and has a scale. I also added 
> the information in sysfs-bus-iio documentation, because this type of 
> processed value is new in the ABI.
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio 
> b/Documentation/ABI/testing/sysfs-bus-iio
> index c27347d3608e..66a17f4c831e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1716,6 +1716,7 @@ Description:
> 
>   What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_raw
>   What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
> +What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_input
>   What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_raw
>   What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_co2_raw
>   What: 
> /sys/bus/iio/devices/iio:deviceX/in_concentration_ethanol_raw
> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index 09b34201c42b..bc1c6676029d 100644
> --- a/drivers/iio/chemical/scd4x.c
> +++ b/drivers/iio/chemical/scd4x.c
> @@ -337,6 +337,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> 
>          switch (mask) {
>          case IIO_CHAN_INFO_RAW:
> +       case IIO_CHAN_INFO_PROCESSED:
>                  ret = iio_device_claim_direct_mode(indio_dev);
>                  if (ret)
>                          return ret;
> @@ -352,7 +353,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>                  *val = ret;
>                  return IIO_VAL_INT;
>          case IIO_CHAN_INFO_SCALE:
> -               if (chan->type == IIO_TEMP) {
> +               if (chan->type == IIO_CONCENTRATION) {
> +                       *val = 0;
> +                       *val2 = 100;
> +                       return IIO_VAL_INT_PLUS_MICRO;
> +               } else if (chan->type == IIO_TEMP) {
>                          *val = 175000;
>                          *val2 = 65536;
>                          return IIO_VAL_FRACTIONAL;
> @@ -501,7 +506,8 @@ 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),
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +                                       BIT(IIO_CHAN_INFO_SCALE),
You shouldn't have scale and processed.  If we need a scale, then it should
be _RAW.

Jonathan

>                  .address = SCD4X_CO2,
>                  .scan_index = SCD4X_CO2,
>                  .scan_type = {
> 
> Thanks,
> 
> Roan
> 
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> >> Sorry for not asking this earlier.
> >>
> >> Thanks,
> >>
> >> Roan
> >>  
> >>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> >>> index 09b34201c42b..ebebcb117ba2 100644
> >>> --- a/drivers/iio/chemical/scd4x.c
> >>> +++ b/drivers/iio/chemical/scd4x.c
> >>> @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
> >>>    static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
> >>>    {
> >>>    	int i, ret;
> >>> -	uint16_t buf[3];
> >>> +	__be16 buf[3];
> >>>    
> >>>    	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
> >>>    	if (ret)
> >>> @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
> >>>    	int ret;
> >>>    
> >>>    	do {
> >>> +		__be16 bval;
> >>>    		uint16_t val;
> >>>    
> >>> -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
> >>> +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
> >>>    		if (ret)
> >>>    			return -EIO;
> >>> -		val = be16_to_cpu(val);
> >>> +		val = be16_to_cpu(bval);
> >>>    
> >>>    		/* new measurement available */
> >>>    		if (val & 0x7FF)
> >>> @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> >>>    {
> >>>    	struct scd4x_state *state = iio_priv(indio_dev);
> >>>    	int ret;
> >>> -	uint16_t tmp;
> >>> +	__be16 tmp;
> >>>    
> >>>    	switch (mask) {
> >>>    	case IIO_CHAN_INFO_RAW:
> >>> @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
> >>>    	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >>>    	struct scd4x_state *state = iio_priv(indio_dev);
> >>>    	int ret;
> >>> -	uint16_t val;
> >>> +	__be16 bval;
> >>> +	u16 val;
> >>>    
> >>>    	mutex_lock(&state->lock);
> >>> -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
> >>> +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
> >>>    	mutex_unlock(&state->lock);
> >>>    	if (ret) {
> >>>    		dev_err(dev, "failed to read automatic calibration");
> >>>    		return ret;
> >>>    	}
> >>>    
> >>> -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
> >>> +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
> >>>    
> >>>    	return sprintf(buf, "%d\n", val);
> >>>    }
> >>>
> >>>      
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Jonathan
> >>>>     
> >>>>>
> >>>>> Changes since v5:
> >>>>> scd4x.c:
> >>>>>     - Fix bug in trigger_handler
> >>>>>
> >>>>> Changes since v4:
> >>>>> scd4x.c:
> >>>>>     - Minor fixes in documentation
> >>>>>     - Reorder trigger_handler so memcpy is not needed anymore
> >>>>> Documentation:
> >>>>>     - Change information about the KernelVersion for the
> >>>>>       calibration_forced_value_available
> >>>>>
> >>>>> Changes since v3:
> >>>>> scd4x.c
> >>>>>     - Change read and write_and_fetch function parameter. CRC byte is now
> >>>>>       hidden inside the function.
> >>>>>     - Fix minor style issues
> >>>>>     - Add calibration_forced_value_available attribute to the driver
> >>>>>     - Remove including BUFFER_TRIGGERED
> >>>>>     - Change calibbias to raw ADC readings rather than converting it to
> >>>>>       milli degrees C.
> >>>>> Documentation:
> >>>>>     - Change description of driver attributes
> >>>>>     - Add calibration_forced_value_available documentation
> >>>>>
> >>>>> Changes since v2:
> >>>>> scd4x.c:
> >>>>>     - Change boolean operations
> >>>>>     - Document scope of lock
> >>>>>     - Remove device *dev from struct
> >>>>>     - Add goto block for errror handling
> >>>>>     - Add function to read value per channel in read_raw
> >>>>>     - Fix bug with lock in error paths
> >>>>>     - Remove conversion of humidity and temperature values
> >>>>>     - Add scale and offset to temperature channel
> >>>>>     - Add scale to humidity channel
> >>>>>     - Move memset out of locked section
> >>>>>     - Remove unused irq functions
> >>>>>     - Move device register at end of probe function
> >>>>> Documentation:
> >>>>>     - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
> >>>>>     - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>>>
> >>>>> 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 (4):
> >>>>>     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: documentation: Document scd4x calibration use
> >>>>>
> >>>>>    Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
> >>>>>    Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
> >>>>>    .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
> >>>>>    MAINTAINERS                                   |   6 +
> >>>>>    drivers/iio/chemical/Kconfig                  |  13 +
> >>>>>    drivers/iio/chemical/Makefile                 |   1 +
> >>>>>    drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
> >>>>>    7 files changed, 796 insertions(+), 34 deletions(-)
> >>>>>    delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>>>    create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
> >>>>>    create mode 100644 drivers/iio/chemical/scd4x.c
> >>>>>         
> >>>>     
> >>>      
> >   

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Roan van Dijk]

On 18-10-2021 19:52, Jonathan Cameron wrote:
> On Mon, 18 Oct 2021 10:19:42 +0200
> Roan van Dijk <roan@protonic.nl> wrote:
> 
>> On 14-10-2021 19:19, Jonathan Cameron wrote:
>>> On Thu, 14 Oct 2021 10:24:54 +0200
>>> Roan van Dijk <roan@protonic.nl> wrote:
>>>    
>>>> On 13-10-2021 19:38, Jonathan Cameron wrote:
>>>>> On Sun, 10 Oct 2021 16:59:19 +0100
>>>>> Jonathan Cameron <jic23@kernel.org> wrote:
>>>>>       
>>>>>> On Fri,  8 Oct 2021 12:17:02 +0200
>>>>>> Roan van Dijk <roan@protonic.nl> wrote:
>>>>>>      
>>>>>>> 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?
>>>>>>
>>>>>> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
>>>>>> up. Pushed out as testing for 0-day to see if it can find anything we missed
>>>>>
>>>>> And indeed - I missed a bunch of places where explicit __be16 types should have
>>>>> been used.
>>>>>
>>>>> I've applied the following fixup, shout if it's wrong.
>>>>>      
>>>> Thank you Jonathan for applying this fixup. No need to shout :) Your
>>>> changes should fix the issue.
>>>>
>>>> However, I have a question about something else. The co2 concentration
>>>> is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this
>>>> moment. Is an _scale always required for an _raw in the ABI? I could not
>>>> find anything in the documentation if there is a rule for this. Someone
>>>> mentioned this to me, so I want to check if I did this right.
>>>>
>>>> The sensor returns the actual co2 value upon reading, like 450 ppm. We
>>>> can set an offset of this co2 value with the calibration_forced_value
>>>> through the ABI, but this offset is handled internally by the sensor. So
>>>> there isn't anything with scaling or an offset needed at the driver side.
>>>
>>> Ah. We could have mapped this to calibbias, though here it's made more
>>> complex by other calibrations existing that don't use the value so let's
>>> leave it as it is.
>>>    
>>>>
>>>> Was I right by making it of type RAW? If needed we could make it more
>>>> like the scd30 driver, keeping it of type RAW but with scale = 1. What
>>>> should I do or is it fine as it is?
>>>
>>> Hmm. Interesting corner case in the ABI.  A _raw value without a scale
>>> normally means we don't know it for some reason.  The most common case
>>> of this is light sensors where several _raw intensity values are combined
>>> in some (typically non linear) transform to form a single measure of illuminance.
>>> Those intensity_raw channels don't have an meaningful units, but devices
>>> often have threshold events on them so we have to expose them.
>>>
>>> I would say make it a processed value, but there is a quirk.
>>> concentrations in IIO are expressed in percent not per million, so you need
>>> a scale anyway, I guess 10000?  See Documentation/ABI/testing/sysfs-bus-iio
>>>
>>>
>>> No need to do a new driver version, just send a patch tidying up this corner.
>>>    
>>
>> Hi Jonathan,
>>
>> As you suggested, these are my fixes for the concentration reading.
>>
>> The co2 reading is now a processed value and has a scale. I also added
>> the information in sysfs-bus-iio documentation, because this type of
>> processed value is new in the ABI.
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
>> b/Documentation/ABI/testing/sysfs-bus-iio
>> index c27347d3608e..66a17f4c831e 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -1716,6 +1716,7 @@ Description:
>>
>>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_raw
>>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
>> +What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_input
>>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_raw
>>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_co2_raw
>>    What:
>> /sys/bus/iio/devices/iio:deviceX/in_concentration_ethanol_raw
>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
>> index 09b34201c42b..bc1c6676029d 100644
>> --- a/drivers/iio/chemical/scd4x.c
>> +++ b/drivers/iio/chemical/scd4x.c
>> @@ -337,6 +337,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>>
>>           switch (mask) {
>>           case IIO_CHAN_INFO_RAW:
>> +       case IIO_CHAN_INFO_PROCESSED:
>>                   ret = iio_device_claim_direct_mode(indio_dev);
>>                   if (ret)
>>                           return ret;
>> @@ -352,7 +353,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>>                   *val = ret;
>>                   return IIO_VAL_INT;
>>           case IIO_CHAN_INFO_SCALE:
>> -               if (chan->type == IIO_TEMP) {
>> +               if (chan->type == IIO_CONCENTRATION) {
>> +                       *val = 0;
>> +                       *val2 = 100;
>> +                       return IIO_VAL_INT_PLUS_MICRO;
>> +               } else if (chan->type == IIO_TEMP) {
>>                           *val = 175000;
>>                           *val2 = 65536;
>>                           return IIO_VAL_FRACTIONAL;
>> @@ -501,7 +506,8 @@ 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),
>> +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
>> +                                       BIT(IIO_CHAN_INFO_SCALE),
> You shouldn't have scale and processed.  If we need a scale, then it should
> be _RAW.
> 
> Jonathan

Okay, it's now a _RAW value with a scale to make the concentration value 
expressed in percent. This should be the fix then.

diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
index 09b34201c42b..b063b378c7d5 100644
--- a/drivers/iio/chemical/scd4x.c
+++ b/drivers/iio/chemical/scd4x.c
@@ -352,7 +352,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
                 *val = ret;
                 return IIO_VAL_INT;
         case IIO_CHAN_INFO_SCALE:
-               if (chan->type == IIO_TEMP) {
+               if (chan->type == IIO_CONCENTRATION) {
+                       *val = 0;
+                       *val2 = 100;
+                       return IIO_VAL_INT_PLUS_MICRO;
+               } else if (chan->type == IIO_TEMP) {
                         *val = 175000;
                         *val2 = 65536;
                         return IIO_VAL_FRACTIONAL;
@@ -501,7 +505,8 @@ 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),
+               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+                                       BIT(IIO_CHAN_INFO_SCALE),
                 .address = SCD4X_CO2,
                 .scan_index = SCD4X_CO2,
                 .scan_type = {

Thanks,

Roan

> 
>>                   .address = SCD4X_CO2,
>>                   .scan_index = SCD4X_CO2,
>>                   .scan_type = {
>>
>> Thanks,
>>
>> Roan
>>
>>> Thanks,
>>>
>>> Jonathan
>>>
>>>    
>>>> Sorry for not asking this earlier.
>>>>
>>>> Thanks,
>>>>
>>>> Roan
>>>>   
>>>>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
>>>>> index 09b34201c42b..ebebcb117ba2 100644
>>>>> --- a/drivers/iio/chemical/scd4x.c
>>>>> +++ b/drivers/iio/chemical/scd4x.c
>>>>> @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
>>>>>     static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
>>>>>     {
>>>>>     	int i, ret;
>>>>> -	uint16_t buf[3];
>>>>> +	__be16 buf[3];
>>>>>     
>>>>>     	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
>>>>>     	if (ret)
>>>>> @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
>>>>>     	int ret;
>>>>>     
>>>>>     	do {
>>>>> +		__be16 bval;
>>>>>     		uint16_t val;
>>>>>     
>>>>> -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
>>>>> +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
>>>>>     		if (ret)
>>>>>     			return -EIO;
>>>>> -		val = be16_to_cpu(val);
>>>>> +		val = be16_to_cpu(bval);
>>>>>     
>>>>>     		/* new measurement available */
>>>>>     		if (val & 0x7FF)
>>>>> @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>>>>>     {
>>>>>     	struct scd4x_state *state = iio_priv(indio_dev);
>>>>>     	int ret;
>>>>> -	uint16_t tmp;
>>>>> +	__be16 tmp;
>>>>>     
>>>>>     	switch (mask) {
>>>>>     	case IIO_CHAN_INFO_RAW:
>>>>> @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
>>>>>     	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>     	struct scd4x_state *state = iio_priv(indio_dev);
>>>>>     	int ret;
>>>>> -	uint16_t val;
>>>>> +	__be16 bval;
>>>>> +	u16 val;
>>>>>     
>>>>>     	mutex_lock(&state->lock);
>>>>> -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
>>>>> +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
>>>>>     	mutex_unlock(&state->lock);
>>>>>     	if (ret) {
>>>>>     		dev_err(dev, "failed to read automatic calibration");
>>>>>     		return ret;
>>>>>     	}
>>>>>     
>>>>> -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
>>>>> +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
>>>>>     
>>>>>     	return sprintf(buf, "%d\n", val);
>>>>>     }
>>>>>
>>>>>       
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Jonathan
>>>>>>      
>>>>>>>
>>>>>>> Changes since v5:
>>>>>>> scd4x.c:
>>>>>>>      - Fix bug in trigger_handler
>>>>>>>
>>>>>>> Changes since v4:
>>>>>>> scd4x.c:
>>>>>>>      - Minor fixes in documentation
>>>>>>>      - Reorder trigger_handler so memcpy is not needed anymore
>>>>>>> Documentation:
>>>>>>>      - Change information about the KernelVersion for the
>>>>>>>        calibration_forced_value_available
>>>>>>>
>>>>>>> Changes since v3:
>>>>>>> scd4x.c
>>>>>>>      - Change read and write_and_fetch function parameter. CRC byte is now
>>>>>>>        hidden inside the function.
>>>>>>>      - Fix minor style issues
>>>>>>>      - Add calibration_forced_value_available attribute to the driver
>>>>>>>      - Remove including BUFFER_TRIGGERED
>>>>>>>      - Change calibbias to raw ADC readings rather than converting it to
>>>>>>>        milli degrees C.
>>>>>>> Documentation:
>>>>>>>      - Change description of driver attributes
>>>>>>>      - Add calibration_forced_value_available documentation
>>>>>>>
>>>>>>> Changes since v2:
>>>>>>> scd4x.c:
>>>>>>>      - Change boolean operations
>>>>>>>      - Document scope of lock
>>>>>>>      - Remove device *dev from struct
>>>>>>>      - Add goto block for errror handling
>>>>>>>      - Add function to read value per channel in read_raw
>>>>>>>      - Fix bug with lock in error paths
>>>>>>>      - Remove conversion of humidity and temperature values
>>>>>>>      - Add scale and offset to temperature channel
>>>>>>>      - Add scale to humidity channel
>>>>>>>      - Move memset out of locked section
>>>>>>>      - Remove unused irq functions
>>>>>>>      - Move device register at end of probe function
>>>>>>> Documentation:
>>>>>>>      - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
>>>>>>>      - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>>>>>
>>>>>>> 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 (4):
>>>>>>>      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: documentation: Document scd4x calibration use
>>>>>>>
>>>>>>>     Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
>>>>>>>     Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
>>>>>>>     .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
>>>>>>>     MAINTAINERS                                   |   6 +
>>>>>>>     drivers/iio/chemical/Kconfig                  |  13 +
>>>>>>>     drivers/iio/chemical/Makefile                 |   1 +
>>>>>>>     drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
>>>>>>>     7 files changed, 796 insertions(+), 34 deletions(-)
>>>>>>>     delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
>>>>>>>     create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
>>>>>>>     create mode 100644 drivers/iio/chemical/scd4x.c
>>>>>>>          
>>>>>>      
>>>>>       
>>>    
> 

Re: [PATCH v5 0/4] iio: chemical: Add support for Sensirion SCD4x CO2 sensor

[Jonathan Cameron]

On Tue, 19 Oct 2021 09:30:34 +0200
Roan van Dijk <roan@protonic.nl> wrote:

> On 18-10-2021 19:52, Jonathan Cameron wrote:
> > On Mon, 18 Oct 2021 10:19:42 +0200
> > Roan van Dijk <roan@protonic.nl> wrote:
> >   
> >> On 14-10-2021 19:19, Jonathan Cameron wrote:  
> >>> On Thu, 14 Oct 2021 10:24:54 +0200
> >>> Roan van Dijk <roan@protonic.nl> wrote:
> >>>      
> >>>> On 13-10-2021 19:38, Jonathan Cameron wrote:  
> >>>>> On Sun, 10 Oct 2021 16:59:19 +0100
> >>>>> Jonathan Cameron <jic23@kernel.org> wrote:
> >>>>>         
> >>>>>> On Fri,  8 Oct 2021 12:17:02 +0200
> >>>>>> Roan van Dijk <roan@protonic.nl> wrote:
> >>>>>>        
> >>>>>>> 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?  
> >>>>>>
> >>>>>> Applied to the togreg branch of iio.git with the issues Randy mentioned tidied
> >>>>>> up. Pushed out as testing for 0-day to see if it can find anything we missed  
> >>>>>
> >>>>> And indeed - I missed a bunch of places where explicit __be16 types should have
> >>>>> been used.
> >>>>>
> >>>>> I've applied the following fixup, shout if it's wrong.
> >>>>>        
> >>>> Thank you Jonathan for applying this fixup. No need to shout :) Your
> >>>> changes should fix the issue.
> >>>>
> >>>> However, I have a question about something else. The co2 concentration
> >>>> is an IIO_CHAN_INFO_RAW, but doesn't have a scale or offset at this
> >>>> moment. Is an _scale always required for an _raw in the ABI? I could not
> >>>> find anything in the documentation if there is a rule for this. Someone
> >>>> mentioned this to me, so I want to check if I did this right.
> >>>>
> >>>> The sensor returns the actual co2 value upon reading, like 450 ppm. We
> >>>> can set an offset of this co2 value with the calibration_forced_value
> >>>> through the ABI, but this offset is handled internally by the sensor. So
> >>>> there isn't anything with scaling or an offset needed at the driver side.  
> >>>
> >>> Ah. We could have mapped this to calibbias, though here it's made more
> >>> complex by other calibrations existing that don't use the value so let's
> >>> leave it as it is.
> >>>      
> >>>>
> >>>> Was I right by making it of type RAW? If needed we could make it more
> >>>> like the scd30 driver, keeping it of type RAW but with scale = 1. What
> >>>> should I do or is it fine as it is?  
> >>>
> >>> Hmm. Interesting corner case in the ABI.  A _raw value without a scale
> >>> normally means we don't know it for some reason.  The most common case
> >>> of this is light sensors where several _raw intensity values are combined
> >>> in some (typically non linear) transform to form a single measure of illuminance.
> >>> Those intensity_raw channels don't have an meaningful units, but devices
> >>> often have threshold events on them so we have to expose them.
> >>>
> >>> I would say make it a processed value, but there is a quirk.
> >>> concentrations in IIO are expressed in percent not per million, so you need
> >>> a scale anyway, I guess 10000?  See Documentation/ABI/testing/sysfs-bus-iio
> >>>
> >>>
> >>> No need to do a new driver version, just send a patch tidying up this corner.
> >>>      
> >>
> >> Hi Jonathan,
> >>
> >> As you suggested, these are my fixes for the concentration reading.
> >>
> >> The co2 reading is now a processed value and has a scale. I also added
> >> the information in sysfs-bus-iio documentation, because this type of
> >> processed value is new in the ABI.
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
> >> b/Documentation/ABI/testing/sysfs-bus-iio
> >> index c27347d3608e..66a17f4c831e 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-iio
> >> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> >> @@ -1716,6 +1716,7 @@ Description:
> >>
> >>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_raw
> >>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_raw
> >> +What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_input
> >>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_raw
> >>    What:          /sys/bus/iio/devices/iio:deviceX/in_concentrationX_co2_raw
> >>    What:
> >> /sys/bus/iio/devices/iio:deviceX/in_concentration_ethanol_raw
> >> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> >> index 09b34201c42b..bc1c6676029d 100644
> >> --- a/drivers/iio/chemical/scd4x.c
> >> +++ b/drivers/iio/chemical/scd4x.c
> >> @@ -337,6 +337,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> >>
> >>           switch (mask) {
> >>           case IIO_CHAN_INFO_RAW:
> >> +       case IIO_CHAN_INFO_PROCESSED:
> >>                   ret = iio_device_claim_direct_mode(indio_dev);
> >>                   if (ret)
> >>                           return ret;
> >> @@ -352,7 +353,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> >>                   *val = ret;
> >>                   return IIO_VAL_INT;
> >>           case IIO_CHAN_INFO_SCALE:
> >> -               if (chan->type == IIO_TEMP) {
> >> +               if (chan->type == IIO_CONCENTRATION) {
> >> +                       *val = 0;
> >> +                       *val2 = 100;
> >> +                       return IIO_VAL_INT_PLUS_MICRO;
> >> +               } else if (chan->type == IIO_TEMP) {
> >>                           *val = 175000;
> >>                           *val2 = 65536;
> >>                           return IIO_VAL_FRACTIONAL;
> >> @@ -501,7 +506,8 @@ 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),
> >> +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> >> +                                       BIT(IIO_CHAN_INFO_SCALE),  
> > You shouldn't have scale and processed.  If we need a scale, then it should
> > be _RAW.
> > 
> > Jonathan  
> 
> Okay, it's now a _RAW value with a scale to make the concentration value 
> expressed in percent. This should be the fix then.

Looks good to me.  Please send as a new patch with Signed-off-by: etc
Technically it's not a 'fix' but I'll probably queue it up as one anyway
as better this definitely makes the same cycle as the driver.

Thanks,

Jonathan

> 
> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index 09b34201c42b..b063b378c7d5 100644
> --- a/drivers/iio/chemical/scd4x.c
> +++ b/drivers/iio/chemical/scd4x.c
> @@ -352,7 +352,11 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>                  *val = ret;
>                  return IIO_VAL_INT;
>          case IIO_CHAN_INFO_SCALE:
> -               if (chan->type == IIO_TEMP) {
> +               if (chan->type == IIO_CONCENTRATION) {
> +                       *val = 0;
> +                       *val2 = 100;
> +                       return IIO_VAL_INT_PLUS_MICRO;
> +               } else if (chan->type == IIO_TEMP) {
>                          *val = 175000;
>                          *val2 = 65536;
>                          return IIO_VAL_FRACTIONAL;
> @@ -501,7 +505,8 @@ 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),
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +                                       BIT(IIO_CHAN_INFO_SCALE),
>                  .address = SCD4X_CO2,
>                  .scan_index = SCD4X_CO2,
>                  .scan_type = {
> 
> Thanks,
> 
> Roan
> 
> >   
> >>                   .address = SCD4X_CO2,
> >>                   .scan_index = SCD4X_CO2,
> >>                   .scan_type = {
> >>
> >> Thanks,
> >>
> >> Roan
> >>  
> >>> Thanks,
> >>>
> >>> Jonathan
> >>>
> >>>      
> >>>> Sorry for not asking this earlier.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Roan
> >>>>     
> >>>>> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> >>>>> index 09b34201c42b..ebebcb117ba2 100644
> >>>>> --- a/drivers/iio/chemical/scd4x.c
> >>>>> +++ b/drivers/iio/chemical/scd4x.c
> >>>>> @@ -263,7 +263,7 @@ static int scd4x_write_and_fetch(struct scd4x_state *state, enum scd4x_cmd cmd,
> >>>>>     static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
> >>>>>     {
> >>>>>     	int i, ret;
> >>>>> -	uint16_t buf[3];
> >>>>> +	__be16 buf[3];
> >>>>>     
> >>>>>     	ret = scd4x_read(state, CMD_READ_MEAS, buf, sizeof(buf));
> >>>>>     	if (ret)
> >>>>> @@ -282,12 +282,13 @@ static int scd4x_wait_meas_poll(struct scd4x_state *state)
> >>>>>     	int ret;
> >>>>>     
> >>>>>     	do {
> >>>>> +		__be16 bval;
> >>>>>     		uint16_t val;
> >>>>>     
> >>>>> -		ret = scd4x_read(state, CMD_GET_DATA_READY, &val, sizeof(val));
> >>>>> +		ret = scd4x_read(state, CMD_GET_DATA_READY, &bval, sizeof(bval));
> >>>>>     		if (ret)
> >>>>>     			return -EIO;
> >>>>> -		val = be16_to_cpu(val);
> >>>>> +		val = be16_to_cpu(bval);
> >>>>>     
> >>>>>     		/* new measurement available */
> >>>>>     		if (val & 0x7FF)
> >>>>> @@ -333,7 +334,7 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> >>>>>     {
> >>>>>     	struct scd4x_state *state = iio_priv(indio_dev);
> >>>>>     	int ret;
> >>>>> -	uint16_t tmp;
> >>>>> +	__be16 tmp;
> >>>>>     
> >>>>>     	switch (mask) {
> >>>>>     	case IIO_CHAN_INFO_RAW:
> >>>>> @@ -405,17 +406,18 @@ static ssize_t calibration_auto_enable_show(struct device *dev,
> >>>>>     	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >>>>>     	struct scd4x_state *state = iio_priv(indio_dev);
> >>>>>     	int ret;
> >>>>> -	uint16_t val;
> >>>>> +	__be16 bval;
> >>>>> +	u16 val;
> >>>>>     
> >>>>>     	mutex_lock(&state->lock);
> >>>>> -	ret = scd4x_read(state, CMD_GET_ASC, &val, sizeof(val));
> >>>>> +	ret = scd4x_read(state, CMD_GET_ASC, &bval, sizeof(bval));
> >>>>>     	mutex_unlock(&state->lock);
> >>>>>     	if (ret) {
> >>>>>     		dev_err(dev, "failed to read automatic calibration");
> >>>>>     		return ret;
> >>>>>     	}
> >>>>>     
> >>>>> -	val = (be16_to_cpu(val) & SCD4X_READY_MASK) ? 1 : 0;
> >>>>> +	val = (be16_to_cpu(bval) & SCD4X_READY_MASK) ? 1 : 0;
> >>>>>     
> >>>>>     	return sprintf(buf, "%d\n", val);
> >>>>>     }
> >>>>>
> >>>>>         
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Jonathan
> >>>>>>        
> >>>>>>>
> >>>>>>> Changes since v5:
> >>>>>>> scd4x.c:
> >>>>>>>      - Fix bug in trigger_handler
> >>>>>>>
> >>>>>>> Changes since v4:
> >>>>>>> scd4x.c:
> >>>>>>>      - Minor fixes in documentation
> >>>>>>>      - Reorder trigger_handler so memcpy is not needed anymore
> >>>>>>> Documentation:
> >>>>>>>      - Change information about the KernelVersion for the
> >>>>>>>        calibration_forced_value_available
> >>>>>>>
> >>>>>>> Changes since v3:
> >>>>>>> scd4x.c
> >>>>>>>      - Change read and write_and_fetch function parameter. CRC byte is now
> >>>>>>>        hidden inside the function.
> >>>>>>>      - Fix minor style issues
> >>>>>>>      - Add calibration_forced_value_available attribute to the driver
> >>>>>>>      - Remove including BUFFER_TRIGGERED
> >>>>>>>      - Change calibbias to raw ADC readings rather than converting it to
> >>>>>>>        milli degrees C.
> >>>>>>> Documentation:
> >>>>>>>      - Change description of driver attributes
> >>>>>>>      - Add calibration_forced_value_available documentation
> >>>>>>>
> >>>>>>> Changes since v2:
> >>>>>>> scd4x.c:
> >>>>>>>      - Change boolean operations
> >>>>>>>      - Document scope of lock
> >>>>>>>      - Remove device *dev from struct
> >>>>>>>      - Add goto block for errror handling
> >>>>>>>      - Add function to read value per channel in read_raw
> >>>>>>>      - Fix bug with lock in error paths
> >>>>>>>      - Remove conversion of humidity and temperature values
> >>>>>>>      - Add scale and offset to temperature channel
> >>>>>>>      - Add scale to humidity channel
> >>>>>>>      - Move memset out of locked section
> >>>>>>>      - Remove unused irq functions
> >>>>>>>      - Move device register at end of probe function
> >>>>>>> Documentation:
> >>>>>>>      - Copy content of sysfs-bus-iio-scd30 to sysfs-bus-iio
> >>>>>>>      - Remove Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>>>>>
> >>>>>>> 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 (4):
> >>>>>>>      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: documentation: Document scd4x calibration use
> >>>>>>>
> >>>>>>>     Documentation/ABI/testing/sysfs-bus-iio       |  41 ++
> >>>>>>>     Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 -
> >>>>>>>     .../iio/chemical/sensirion,scd4x.yaml         |  46 ++
> >>>>>>>     MAINTAINERS                                   |   6 +
> >>>>>>>     drivers/iio/chemical/Kconfig                  |  13 +
> >>>>>>>     drivers/iio/chemical/Makefile                 |   1 +
> >>>>>>>     drivers/iio/chemical/scd4x.c                  | 689 ++++++++++++++++++
> >>>>>>>     7 files changed, 796 insertions(+), 34 deletions(-)
> >>>>>>>     delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> >>>>>>>     create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd4x.yaml
> >>>>>>>     create mode 100644 drivers/iio/chemical/scd4x.c
> >>>>>>>            
> >>>>>>        
> >>>>>         
> >>>      
> >