2020-04-22 14:24:26

by Tomasz Duszynski

[permalink] [raw]
Subject: [PATCH 0/6] Add support for SCD30 sensor

Following series adds support for Sensirion SCD30 sensor module capable of
measuring carbon dioxide, temperature and relative humidity. CO2 measurements
base on NDIR principle while temperature and relative humidity are measured by
the on board SHT31. As for sensor communication, both I2C and serial interfaces
are supported.

Tomasz Duszynski (6):
iio: chemical: scd30: add core driver
iio: chemical: scd30: add I2C interface driver
iio: chemical: scd30: add serial interface driver
Documentation: ABI: testing: scd30: document iio attributes
dt-bindings: iio: scd30: add device binding file
MAINTAINERS: add myself as a SCD30 driver maintainer

Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++
.../iio/chemical/sensirion,scd30.yaml | 71 ++
MAINTAINERS | 9 +
drivers/iio/chemical/Kconfig | 33 +
drivers/iio/chemical/Makefile | 3 +
drivers/iio/chemical/scd30.h | 72 ++
drivers/iio/chemical/scd30_core.c | 796 ++++++++++++++++++
drivers/iio/chemical/scd30_i2c.c | 141 ++++
drivers/iio/chemical/scd30_serial.c | 262 ++++++
9 files changed, 1484 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
create mode 100644 drivers/iio/chemical/scd30.h
create mode 100644 drivers/iio/chemical/scd30_core.c
create mode 100644 drivers/iio/chemical/scd30_i2c.c
create mode 100644 drivers/iio/chemical/scd30_serial.c

--
2.26.1


2020-04-22 14:25:31

by Tomasz Duszynski

[permalink] [raw]
Subject: [PATCH 3/6] iio: chemical: scd30: add serial interface driver

Add serial interface driver for the SCD30 sensor.

Signed-off-by: Tomasz Duszynski <[email protected]>
---
drivers/iio/chemical/Kconfig | 11 ++
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/scd30_serial.c | 262 ++++++++++++++++++++++++++++
3 files changed, 274 insertions(+)
create mode 100644 drivers/iio/chemical/scd30_serial.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 0c99f37b6bb0..1b7a58d320e7 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -96,6 +96,17 @@ config SCD30_I2C
To compile this driver as a module, choose M here: the module will
be called scd30_i2c.

+config SCD30_SERIAL
+ tristate "SCD30 carbon dioxide sensor serial driver"
+ depends on SCD30_CORE && SERIAL_DEV_BUS
+ select CRC16
+ help
+ Say Y here to build support for the Sensirion SCD30 serial interface
+ driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called scd30_serial.
+
config SENSIRION_SGP30
tristate "Sensirion SGPxx gas sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index f60c8ef358c3..1fac1b14f825 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_IAQCORE) += ams-iaq-core.o
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_SENSIRION_SGP30) += sgp30.o
obj-$(CONFIG_SPS30) += sps30.o
obj-$(CONFIG_VZ89X) += vz89x.o
diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c
new file mode 100644
index 000000000000..93b98d5f2cc4
--- /dev/null
+++ b/drivers/iio/chemical/scd30_serial.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor serial driver
+ *
+ * Copyright (c) Tomasz Duszynski <[email protected]>
+ */
+#include <asm/unaligned.h>
+#include <linux/crc16.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/jiffies.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/serdev.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "scd30.h"
+
+#define SCD30_SERDEV_ADDR 0x61
+#define SCD30_SERDEV_WRITE 0x06
+#define SCD30_SERDEV_READ 0x03
+#define SCD30_SERDEV_MAX_BUF_SIZE 17
+#define SCD30_SERDEV_RX_HEADER_SIZE 3
+#define SCD30_SERDEV_CRC_SIZE 2
+#define SCD30_SERDEV_TIMEOUT msecs_to_jiffies(500)
+
+struct scd30_serdev_priv {
+ struct completion meas_ready;
+ char *buf;
+ int num_expected;
+ int num;
+};
+
+static u16 scd30_serdev_cmd_lookup_tbl[] = {
+ [CMD_START_MEAS] = 0x0036,
+ [CMD_STOP_MEAS] = 0x0037,
+ [CMD_MEAS_INTERVAL] = 0x0025,
+ [CMD_MEAS_READY] = 0x0027,
+ [CMD_READ_MEAS] = 0x0028,
+ [CMD_ASC] = 0x003a,
+ [CMD_FRC] = 0x0039,
+ [CMD_TEMP_OFFSET] = 0x003b,
+ [CMD_FW_VERSION] = 0x0020,
+ [CMD_RESET] = 0x0034,
+};
+
+static u16 scd30_serdev_calc_crc(const char *buf, int size)
+{
+ return crc16(0xffff, buf, size);
+}
+
+static int scd30_serdev_xfer(struct scd30_state *state, char *txbuf, int txsize,
+ char *rxbuf, int rxsize)
+{
+ struct serdev_device *serdev = to_serdev_device(state->dev);
+ struct scd30_serdev_priv *priv = state->priv;
+ int ret;
+
+ priv->buf = rxbuf;
+ priv->num_expected = rxsize;
+ priv->num = 0;
+
+ ret = serdev_device_write(serdev, txbuf, txsize, SCD30_SERDEV_TIMEOUT);
+ if (ret < txsize)
+ return ret < 0 ? ret : -EIO;
+
+ ret = wait_for_completion_interruptible_timeout(&priv->meas_ready,
+ SCD30_SERDEV_TIMEOUT);
+ if (ret > 0)
+ ret = 0;
+ else if (!ret)
+ ret = -ETIMEDOUT;
+
+ return ret;
+}
+
+static int scd30_serdev_command(struct scd30_state *state, enum scd30_cmd cmd,
+ u16 arg, char *rsp, int size)
+{
+ /*
+ * Communication over serial line is based on modbus protocol (or rather
+ * its variation called modbus over serial to be precise). Upon
+ * receiving a request device should reply with response.
+ *
+ * Frame below represents a request message. Each field takes
+ * exactly one byte.
+ *
+ * +------+------+-----+-----+-------+-------+-----+-----+
+ * | dev | op | reg | reg | byte1 | byte0 | crc | crc |
+ * | addr | code | msb | lsb | | | lsb | msb |
+ * +------+------+-----+-----+-------+-------+-----+-----+
+ *
+ * The message device replies with depends on the 'op code' field from
+ * the request. In case it was set to SCD30_SERDEV_WRITE sensor should
+ * reply with unchanged request. Otherwise 'op code' was set to
+ * SCD30_SERDEV_READ and response looks like the one below. As with
+ * request, each field takes one byte.
+ *
+ * +------+------+--------+-------+-----+-------+-----+-----+
+ * | dev | op | num of | byte0 | ... | byteN | crc | crc |
+ * | addr | code | bytes | | | | lsb | msb |
+ * +------+------+--------+-------+-----+-------+-----+-----+
+ */
+ char rxbuf[SCD30_SERDEV_MAX_BUF_SIZE];
+ char txbuf[SCD30_SERDEV_MAX_BUF_SIZE] = { SCD30_SERDEV_ADDR };
+ int ret, rxsize, txsize = 2;
+ u16 crc;
+
+ put_unaligned_be16(scd30_serdev_cmd_lookup_tbl[cmd], txbuf + txsize);
+ txsize += 2;
+
+ if (rsp) {
+ txbuf[1] = SCD30_SERDEV_READ;
+ if (cmd == CMD_READ_MEAS)
+ /* number of u16 words to read */
+ put_unaligned_be16(size / 2, txbuf + txsize);
+ else
+ put_unaligned_be16(0x0001, txbuf + txsize);
+ txsize += 2;
+ crc = scd30_serdev_calc_crc(txbuf, txsize);
+ put_unaligned_le16(crc, txbuf + txsize);
+ txsize += 2;
+ rxsize = SCD30_SERDEV_RX_HEADER_SIZE + size +
+ SCD30_SERDEV_CRC_SIZE;
+ } else {
+ if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+ arg = 0x0001;
+
+ txbuf[1] = SCD30_SERDEV_WRITE;
+ put_unaligned_be16(arg, txbuf + txsize);
+ txsize += 2;
+ crc = scd30_serdev_calc_crc(txbuf, txsize);
+ put_unaligned_le16(crc, txbuf + txsize);
+ txsize += 2;
+ rxsize = txsize;
+ }
+
+ ret = scd30_serdev_xfer(state, txbuf, txsize, rxbuf, rxsize);
+ if (ret)
+ return ret;
+
+ switch (txbuf[1]) {
+ case SCD30_SERDEV_WRITE:
+ if (memcmp(txbuf, txbuf, txsize)) {
+ dev_err(state->dev, "wrong message received\n");
+ return -EIO;
+ }
+ break;
+ case SCD30_SERDEV_READ:
+ if (rxbuf[2] != (rxsize -
+ SCD30_SERDEV_RX_HEADER_SIZE -
+ SCD30_SERDEV_CRC_SIZE)) {
+ dev_err(state->dev,
+ "received data size does not match header\n");
+ return -EIO;
+ }
+
+ rxsize -= SCD30_SERDEV_CRC_SIZE;
+ crc = get_unaligned_le16(rxbuf + rxsize);
+ if (crc != scd30_serdev_calc_crc(rxbuf, rxsize)) {
+ dev_err(state->dev, "data integrity check failed\n");
+ return -EIO;
+ }
+
+ rxsize -= SCD30_SERDEV_RX_HEADER_SIZE;
+ memcpy(rsp, rxbuf + SCD30_SERDEV_RX_HEADER_SIZE, rxsize);
+ break;
+ default:
+ dev_err(state->dev, "received unknown op code\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int scd30_serdev_receive_buf(struct serdev_device *serdev,
+ const unsigned char *buf, size_t size)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ struct scd30_serdev_priv *priv = state->priv;
+ int num;
+
+ /* just in case sensor puts some unexpected bytes on the bus */
+ if (!priv->buf)
+ return 0;
+
+ if (priv->num + size >= priv->num_expected)
+ num = priv->num_expected - priv->num;
+ else
+ num = size;
+
+ memcpy(priv->buf + priv->num, buf, num);
+ priv->num += num;
+
+ if (priv->num == priv->num_expected) {
+ priv->buf = NULL;
+ complete(&priv->meas_ready);
+ }
+
+ return num;
+}
+
+static const struct serdev_device_ops scd30_serdev_ops = {
+ .receive_buf = scd30_serdev_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static int scd30_serdev_probe(struct serdev_device *serdev)
+{
+ struct device *dev = &serdev->dev;
+ struct scd30_serdev_priv *priv;
+ int irq, ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ init_completion(&priv->meas_ready);
+ serdev_device_set_client_ops(serdev, &scd30_serdev_ops);
+
+ ret = devm_serdev_device_open(dev, serdev);
+ if (ret)
+ return ret;
+
+ serdev_device_set_baudrate(serdev, 19200);
+ serdev_device_set_flow_control(serdev, false);
+
+ ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+ if (ret)
+ return ret;
+
+ irq = of_irq_get(dev->of_node, 0);
+ if (irq <= 0)
+ irq = 0;
+
+ return scd30_probe(dev, irq, KBUILD_MODNAME, priv,
+ scd30_serdev_command);
+}
+
+static const struct of_device_id scd30_serdev_of_match[] = {
+ { .compatible = "sensirion,scd30" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, scd30_serdev_of_match);
+
+static struct serdev_device_driver scd30_serdev_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = scd30_serdev_of_match,
+ .pm = &scd30_pm_ops,
+ },
+ .probe = scd30_serdev_probe,
+};
+module_serdev_device_driver(scd30_serdev_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <[email protected]>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor serial driver");
+MODULE_LICENSE("GPL v2");
--
2.26.1

2020-04-22 14:26:06

by Tomasz Duszynski

[permalink] [raw]
Subject: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

Add documentation for sensor specific iio attributes.

Signed-off-by: Tomasz Duszynski <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
1 file changed, 97 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
new file mode 100644
index 000000000000..0431a718447d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
@@ -0,0 +1,97 @@
+What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Given that sensor's CO2 measurement chamber has fixed volume
+ pressure changes will affect concentration readings. Writing
+ current ambient pressure here will allow senor to make necessary
+ adjustments. Upon reading previously set value is returned.
+ Units are millibars.
+
+What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ The range of available values in millibars represented as the
+ minimum value, the step and the maximum value, all enclosed in
+ square brackets.
+
+What: /sys/bus/iio/devices/iio:deviceX/meas_interval
+Date: January 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Amount of time between subsequent measurements. Writing this
+ attribute will change measurement interval. Upon reading
+ current measurement interval is returned. Units are seconds.
+
+What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ The range of available values in seconds represented as the
+ minimum value, the step and the maximum value, all enclosed in
+ square brackets.
+
+What: /sys/bus/iio/devices/iio:deviceX/asc
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Writing 1 or 0 to this attribute will respectively activate or
+ deactivate automatic self calibration procedure. Upon reading 1
+ is returned if asc is ongoing, 0 otherwise.
+
+What: /sys/bus/iio/devices/iio:deviceX/frc
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Forced recalibration is used to compensate for sensor drifts
+ when a reference value of CO2 concentration in close proximity
+ to the sensor is available. Writing attribute will set frc
+ value. Upon reading current frc is returned. Units are
+ millibars.
+
+What: /sys/bus/iio/devices/iio:deviceX/frc_available
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ The range of available values in millibars represented as the
+ minimum value, the step and the maximum value, all enclosed in
+ square brackets.
+
+What: /sys/bus/iio/devices/iio:deviceX/temp_offset
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Sensor readings may be affected by ambient temperature.
+ Writing temperature offset will compensate for unwanted changes.
+ Note that written offset gets multiplied by a factor of 100
+ by a sensor internally.
+
+ For example, writing 10 here will correspond to 0.1 degree
+ Celsius.
+
+What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ The range of available values in degrees Celsius represented as
+ the minimum value, the step and the maximum value, all enclosed
+ in square brackets.
+
+What: /sys/bus/iio/devices/iio:deviceX/reset
+Date: April 2020
+KernelVersion: 5.8
+Contact: [email protected]
+Description:
+ Software reset mechanism forces sensor into the same state
+ as after powering up without the need for removing power supply.
+ Writing any value will reset sensor.
--
2.26.1

2020-04-22 16:41:44

by Peter Meerwald-Stadler

[permalink] [raw]
Subject: Re: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

On Wed, 22 Apr 2020, Tomasz Duszynski wrote:

> Add documentation for sensor specific iio attributes.

minor comments below

> Signed-off-by: Tomasz Duszynski <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
> 1 file changed, 97 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> new file mode 100644
> index 000000000000..0431a718447d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> @@ -0,0 +1,97 @@
> +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Given that sensor's CO2 measurement chamber has fixed volume
> + pressure changes will affect concentration readings. Writing
> + current ambient pressure here will allow senor to make necessary

sensor

> + adjustments. Upon reading previously set value is returned.
> + Units are millibars.

unit for pressure in IIO is kilopascal (e.g.
/sys/bus/iio/devices/iio:deviceX/in_pressure_raw)

> +
> +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + The range of available values in millibars represented as the
> + minimum value, the step and the maximum value, all enclosed in
> + square brackets.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/meas_interval
> +Date: January 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Amount of time between subsequent measurements. Writing this
> + attribute will change measurement interval. Upon reading
> + current measurement interval is returned. Units are seconds.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + The range of available values in seconds represented as the
> + minimum value, the step and the maximum value, all enclosed in
> + square brackets.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/asc
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Writing 1 or 0 to this attribute will respectively activate or
> + deactivate automatic self calibration procedure. Upon reading 1

deactivate automatic self calibration (asc) procedure

> + is returned if asc is ongoing, 0 otherwise.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/frc
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Forced recalibration is used to compensate for sensor drifts
> + when a reference value of CO2 concentration in close proximity
> + to the sensor is available. Writing attribute will set frc
> + value. Upon reading current frc is returned. Units are
> + millibars.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/frc_available
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + The range of available values in millibars represented as the
> + minimum value, the step and the maximum value, all enclosed in
> + square brackets.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/temp_offset
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Sensor readings may be affected by ambient temperature.
> + Writing temperature offset will compensate for unwanted changes.
> + Note that written offset gets multiplied by a factor of 100
> + by a sensor internally.
> +
> + For example, writing 10 here will correspond to 0.1 degree
> + Celsius.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + The range of available values in degrees Celsius represented as
> + the minimum value, the step and the maximum value, all enclosed
> + in square brackets.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/reset
> +Date: April 2020
> +KernelVersion: 5.8
> +Contact: [email protected]
> +Description:
> + Software reset mechanism forces sensor into the same state
> + as after powering up without the need for removing power supply.
> + Writing any value will reset sensor.
>

--

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

2020-04-22 19:56:41

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 3/6] iio: chemical: scd30: add serial interface driver

On Wed, Apr 22, 2020 at 5:22 PM Tomasz Duszynski
<[email protected]> wrote:
>
> Add serial interface driver for the SCD30 sensor.

...

> +#include <linux/of_irq.h>

Do you need this?

> +static int scd30_serdev_probe(struct serdev_device *serdev)
> +{
> + struct device *dev = &serdev->dev;
> + struct scd30_serdev_priv *priv;
> + int irq, ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;

> + irq = of_irq_get(dev->of_node, 0);

fwnode_irq_get() ?

> + if (irq <= 0)
> + irq = 0;
> +
> + return scd30_probe(dev, irq, KBUILD_MODNAME, priv,
> + scd30_serdev_command);
> +}

--
With Best Regards,
Andy Shevchenko

2020-04-23 15:57:48

by Tomasz Duszynski

[permalink] [raw]
Subject: Re: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

On Wed, Apr 22, 2020 at 06:40:17PM +0200, Peter Meerwald-Stadler wrote:
> On Wed, 22 Apr 2020, Tomasz Duszynski wrote:
>
> > Add documentation for sensor specific iio attributes.
>
> minor comments below

Thanks.

>
> > Signed-off-by: Tomasz Duszynski <[email protected]>
> > ---
> > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
> > 1 file changed, 97 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > new file mode 100644
> > index 000000000000..0431a718447d
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > @@ -0,0 +1,97 @@
> > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Given that sensor's CO2 measurement chamber has fixed volume
> > + pressure changes will affect concentration readings. Writing
> > + current ambient pressure here will allow senor to make necessary
>
> sensor
>

Okay.

> > + adjustments. Upon reading previously set value is returned.
> > + Units are millibars.
>
> unit for pressure in IIO is kilopascal (e.g.
> /sys/bus/iio/devices/iio:deviceX/in_pressure_raw)
>

My thinking here was that since these are sensor specific attributes
they don't need to stick to iio conventions and millibars were somewhat
more natural to use. But I guess that's just matter of habit.

So generally I am okay with reworking all attrs to accept values in iio
preferred units.

> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + The range of available values in millibars represented as the
> > + minimum value, the step and the maximum value, all enclosed in
> > + square brackets.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval
> > +Date: January 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Amount of time between subsequent measurements. Writing this
> > + attribute will change measurement interval. Upon reading
> > + current measurement interval is returned. Units are seconds.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + The range of available values in seconds represented as the
> > + minimum value, the step and the maximum value, all enclosed in
> > + square brackets.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/asc
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Writing 1 or 0 to this attribute will respectively activate or
> > + deactivate automatic self calibration procedure. Upon reading 1
>
> deactivate automatic self calibration (asc) procedure
>

That shouldn't be too difficult to realize what asc actually stands for after
reading this short description.

> > + is returned if asc is ongoing, 0 otherwise.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/frc
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Forced recalibration is used to compensate for sensor drifts
> > + when a reference value of CO2 concentration in close proximity
> > + to the sensor is available. Writing attribute will set frc
> > + value. Upon reading current frc is returned. Units are
> > + millibars.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/frc_available
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + The range of available values in millibars represented as the
> > + minimum value, the step and the maximum value, all enclosed in
> > + square brackets.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Sensor readings may be affected by ambient temperature.
> > + Writing temperature offset will compensate for unwanted changes.
> > + Note that written offset gets multiplied by a factor of 100
> > + by a sensor internally.
> > +
> > + For example, writing 10 here will correspond to 0.1 degree
> > + Celsius.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + The range of available values in degrees Celsius represented as
> > + the minimum value, the step and the maximum value, all enclosed
> > + in square brackets.
> > +
> > +What: /sys/bus/iio/devices/iio:deviceX/reset
> > +Date: April 2020
> > +KernelVersion: 5.8
> > +Contact: [email protected]
> > +Description:
> > + Software reset mechanism forces sensor into the same state
> > + as after powering up without the need for removing power supply.
> > + Writing any value will reset sensor.
> >
>
> --
>
> Peter Meerwald-Stadler
> Mobile: +43 664 24 44 418

2020-04-23 16:27:24

by Tomasz Duszynski

[permalink] [raw]
Subject: Re: [PATCH 3/6] iio: chemical: scd30: add serial interface driver

On Wed, Apr 22, 2020 at 10:55:05PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 22, 2020 at 5:22 PM Tomasz Duszynski
> <[email protected]> wrote:
> >
> > Add serial interface driver for the SCD30 sensor.
>
> ...
>
> > +#include <linux/of_irq.h>
>
> Do you need this?
>
> > +static int scd30_serdev_probe(struct serdev_device *serdev)
> > +{
> > + struct device *dev = &serdev->dev;
> > + struct scd30_serdev_priv *priv;
> > + int irq, ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
>
> > + irq = of_irq_get(dev->of_node, 0);
>
> fwnode_irq_get() ?

Okay, that should work equally good.

>
> > + if (irq <= 0)
> > + irq = 0;
> > +
> > + return scd30_probe(dev, irq, KBUILD_MODNAME, priv,
> > + scd30_serdev_command);
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko

2020-04-25 19:23:19

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

On Thu, 23 Apr 2020 17:53:17 +0200
Tomasz Duszynski <[email protected]> wrote:

> On Wed, Apr 22, 2020 at 06:40:17PM +0200, Peter Meerwald-Stadler wrote:
> > On Wed, 22 Apr 2020, Tomasz Duszynski wrote:
> >
> > > Add documentation for sensor specific iio attributes.
> >
> > minor comments below
>
> Thanks.
>
> >
> > > Signed-off-by: Tomasz Duszynski <[email protected]>
> > > ---
> > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
> > > 1 file changed, 97 insertions(+)
> > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > new file mode 100644
> > > index 000000000000..0431a718447d
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > @@ -0,0 +1,97 @@
> > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Given that sensor's CO2 measurement chamber has fixed volume
> > > + pressure changes will affect concentration readings. Writing
> > > + current ambient pressure here will allow senor to make necessary
> >
> > sensor
> >
>
> Okay.
>
> > > + adjustments. Upon reading previously set value is returned.
> > > + Units are millibars.
> >
> > unit for pressure in IIO is kilopascal (e.g.
> > /sys/bus/iio/devices/iio:deviceX/in_pressure_raw)
> >
>
> My thinking here was that since these are sensor specific attributes
> they don't need to stick to iio conventions and millibars were somewhat
> more natural to use. But I guess that's just matter of habit.

You absolutely have to stick to standard units. Userspace programs
aren't going to come read your docs...

For other sensors that take a calibration value like this we've reported
them via an output channel. For example the atlas-ph sensor has
an 'output temp' channel used for this purpose.

It's not ideal or totally intuitive but it does let us avoid expanding
the overall ABI. The argument was something along the lines of
1) Imagine your sensor could control the pressure in the measurement space...
2) An output channel would provide the value to set it to.
3) Now instead we provide a means of saying 'what it is'
4) End result is we write a value and the pressure in the chamber is
that value :)

As I said not ideal but the best we can do without having to define a lot
of ABI just to deal with compensation factors.

This is a rare case where I would document the 'standard' ABI in here
to make the point that it is actually providing an estimate of the pressure
not controlling it...

>
> So generally I am okay with reworking all attrs to accept values in iio
> preferred units.
>
> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + The range of available values in millibars represented as the
> > > + minimum value, the step and the maximum value, all enclosed in
> > > + square brackets.
> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval
> > > +Date: January 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Amount of time between subsequent measurements. Writing this
> > > + attribute will change measurement interval. Upon reading
> > > + current measurement interval is returned. Units are seconds.

Use the existing ABI sampling frequency which is sort of the inverse of this.

> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + The range of available values in seconds represented as the
> > > + minimum value, the step and the maximum value, all enclosed in
> > > + square brackets.
> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/asc
Spends some characters to easy of understanding ;)

auto_calib_proc_enable maybe? Or can we get away with the 'somewhat standard
calibration (it's used in at least one other driver IIRC)

> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Writing 1 or 0 to this attribute will respectively activate or
> > > + deactivate automatic self calibration procedure. Upon reading 1
> >
> > deactivate automatic self calibration (asc) procedure
> >
>
> That shouldn't be too difficult to realize what asc actually stands for after
> reading this short description.
>
> > > + is returned if asc is ongoing, 0 otherwise.
> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/frc
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Forced recalibration is used to compensate for sensor drifts
> > > + when a reference value of CO2 concentration in close proximity
> > > + to the sensor is available. Writing attribute will set frc
> > > + value. Upon reading current frc is returned. Units are
> > > + millibars.

Could we implement this by just writing to the main channel value?
Bit of a clunky ABI but sort of logically fits in my head given we are basically
forcing the value we read to be this one?

> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/frc_available
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + The range of available values in millibars represented as the
> > > + minimum value, the step and the maximum value, all enclosed in
> > > + square brackets.
> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Sensor readings may be affected by ambient temperature.
> > > + Writing temperature offset will compensate for unwanted changes.
> > > + Note that written offset gets multiplied by a factor of 100
> > > + by a sensor internally.
> > > +
> > > + For example, writing 10 here will correspond to 0.1 degree
> > > + Celsius.

This sounds like a calibbias to me which is standard ABI.

> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + The range of available values in degrees Celsius represented as
> > > + the minimum value, the step and the maximum value, all enclosed
> > > + in square brackets.

Wrong units for temperature (which is an odd one as we
lifted them from hwmon before learning the error of our ways and starting to use
SI units as the base).


> > > +
> > > +What: /sys/bus/iio/devices/iio:deviceX/reset
> > > +Date: April 2020
> > > +KernelVersion: 5.8
> > > +Contact: [email protected]
> > > +Description:
> > > + Software reset mechanism forces sensor into the same state
> > > + as after powering up without the need for removing power supply.
> > > + Writing any value will reset sensor.

Not seeing an argument here for why you might want to do that other than on
power up or module probe to get the driver into a known state.
So currently it's a no to this one - just don't expose it to userspace.


> > >
> >
> > --
> >
> > Peter Meerwald-Stadler
> > Mobile: +43 664 24 44 418

2020-04-26 11:14:19

by Tomasz Duszynski

[permalink] [raw]
Subject: Re: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

On Sat, Apr 25, 2020 at 08:20:57PM +0100, Jonathan Cameron wrote:
> On Thu, 23 Apr 2020 17:53:17 +0200
> Tomasz Duszynski <[email protected]> wrote:
>
> > On Wed, Apr 22, 2020 at 06:40:17PM +0200, Peter Meerwald-Stadler wrote:
> > > On Wed, 22 Apr 2020, Tomasz Duszynski wrote:
> > >
> > > > Add documentation for sensor specific iio attributes.
> > >
> > > minor comments below
> >
> > Thanks.
> >
> > >
> > > > Signed-off-by: Tomasz Duszynski <[email protected]>
> > > > ---
> > > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
> > > > 1 file changed, 97 insertions(+)
> > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > > new file mode 100644
> > > > index 000000000000..0431a718447d
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > > @@ -0,0 +1,97 @@
> > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Given that sensor's CO2 measurement chamber has fixed volume
> > > > + pressure changes will affect concentration readings. Writing
> > > > + current ambient pressure here will allow senor to make necessary
> > >
> > > sensor
> > >
> >
> > Okay.
> >
> > > > + adjustments. Upon reading previously set value is returned.
> > > > + Units are millibars.
> > >
> > > unit for pressure in IIO is kilopascal (e.g.
> > > /sys/bus/iio/devices/iio:deviceX/in_pressure_raw)
> > >
> >
> > My thinking here was that since these are sensor specific attributes
> > they don't need to stick to iio conventions and millibars were somewhat
> > more natural to use. But I guess that's just matter of habit.
>
> You absolutely have to stick to standard units. Userspace programs
> aren't going to come read your docs...
>
> For other sensors that take a calibration value like this we've reported
> them via an output channel. For example the atlas-ph sensor has
> an 'output temp' channel used for this purpose.
>

Fair enough.

> It's not ideal or totally intuitive but it does let us avoid expanding
> the overall ABI. The argument was something along the lines of
> 1) Imagine your sensor could control the pressure in the measurement space...
> 2) An output channel would provide the value to set it to.
> 3) Now instead we provide a means of saying 'what it is'
> 4) End result is we write a value and the pressure in the chamber is
> that value :)
>
> As I said not ideal but the best we can do without having to define a lot
> of ABI just to deal with compensation factors.
>
> This is a rare case where I would document the 'standard' ABI in here
> to make the point that it is actually providing an estimate of the pressure
> not controlling it...
>
> >
> > So generally I am okay with reworking all attrs to accept values in iio
> > preferred units.
> >
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + The range of available values in millibars represented as the
> > > > + minimum value, the step and the maximum value, all enclosed in
> > > > + square brackets.
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval
> > > > +Date: January 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Amount of time between subsequent measurements. Writing this
> > > > + attribute will change measurement interval. Upon reading
> > > > + current measurement interval is returned. Units are seconds.
>
> Use the existing ABI sampling frequency which is sort of the inverse of this.
>

Was thinking about it but long periods in Hz simply don't look appealing :).

No other strong opinions so I'll rework that.

> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + The range of available values in seconds represented as the
> > > > + minimum value, the step and the maximum value, all enclosed in
> > > > + square brackets.
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/asc
> Spends some characters to easy of understanding ;)
>
> auto_calib_proc_enable maybe? Or can we get away with the 'somewhat standard
> calibration (it's used in at least one other driver IIRC)
>

Just self_calibration would do?

> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Writing 1 or 0 to this attribute will respectively activate or
> > > > + deactivate automatic self calibration procedure. Upon reading 1
> > >
> > > deactivate automatic self calibration (asc) procedure
> > >
> >
> > That shouldn't be too difficult to realize what asc actually stands for after
> > reading this short description.
> >
> > > > + is returned if asc is ongoing, 0 otherwise.
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/frc
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Forced recalibration is used to compensate for sensor drifts
> > > > + when a reference value of CO2 concentration in close proximity
> > > > + to the sensor is available. Writing attribute will set frc
> > > > + value. Upon reading current frc is returned. Units are
> > > > + millibars.
>
> Could we implement this by just writing to the main channel value?
> Bit of a clunky ABI but sort of logically fits in my head given we are basically
> forcing the value we read to be this one?
>

So the similar to the pressure compensation. Okay.

> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/frc_available
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + The range of available values in millibars represented as the
> > > > + minimum value, the step and the maximum value, all enclosed in
> > > > + square brackets.
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Sensor readings may be affected by ambient temperature.
> > > > + Writing temperature offset will compensate for unwanted changes.
> > > > + Note that written offset gets multiplied by a factor of 100
> > > > + by a sensor internally.
> > > > +
> > > > + For example, writing 10 here will correspond to 0.1 degree
> > > > + Celsius.
>
> This sounds like a calibbias to me which is standard ABI.
>

Right, that could work.

> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + The range of available values in degrees Celsius represented as
> > > > + the minimum value, the step and the maximum value, all enclosed
> > > > + in square brackets.
>
> Wrong units for temperature (which is an odd one as we
> lifted them from hwmon before learning the error of our ways and starting to use
> SI units as the base).
>

Does calibbias have _available counterpart?

>
> > > > +
> > > > +What: /sys/bus/iio/devices/iio:deviceX/reset
> > > > +Date: April 2020
> > > > +KernelVersion: 5.8
> > > > +Contact: [email protected]
> > > > +Description:
> > > > + Software reset mechanism forces sensor into the same state
> > > > + as after powering up without the need for removing power supply.
> > > > + Writing any value will reset sensor.
>
> Not seeing an argument here for why you might want to do that other than on
> power up or module probe to get the driver into a known state.
> So currently it's a no to this one - just don't expose it to userspace.
>

If one writes some odd configuration (though allowed) into sensor, for example
out of sheer curiosity and then writes the sane values back sensor needs some
time to recover (i.e start reporting valid measurements again).

So rationale here was that after reset sensor recovers immediately. I'd
say that reset is sometimes useful. Perhaps that could be exported by
means of iio debug api?

>
> > > >
> > >
> > > --
> > >
> > > Peter Meerwald-Stadler
> > > Mobile: +43 664 24 44 418
>

2020-04-27 09:48:58

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 4/6] Documentation: ABI: testing: scd30: document iio attributes

Hi Tomasz,

Replies inline.

On Sun, 26 Apr 2020 13:11:04 +0200
Tomasz Duszynski <[email protected]> wrote:

> On Sat, Apr 25, 2020 at 08:20:57PM +0100, Jonathan Cameron wrote:
> > On Thu, 23 Apr 2020 17:53:17 +0200
> > Tomasz Duszynski <[email protected]> wrote:
> >
> > > On Wed, Apr 22, 2020 at 06:40:17PM +0200, Peter Meerwald-Stadler wrote:
> > > > On Wed, 22 Apr 2020, Tomasz Duszynski wrote:
> > > >
> > > > > Add documentation for sensor specific iio attributes.
> > > >
> > > > minor comments below
> > >
> > > Thanks.
> > >
> > > >
> > > > > Signed-off-by: Tomasz Duszynski <[email protected]>
> > > > > ---
> > > > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++
> > > > > 1 file changed, 97 insertions(+)
> > > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > > > new file mode 100644
> > > > > index 000000000000..0431a718447d
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
> > > > > @@ -0,0 +1,97 @@
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Given that sensor's CO2 measurement chamber has fixed volume
> > > > > + pressure changes will affect concentration readings. Writing
> > > > > + current ambient pressure here will allow senor to make necessary
> > > >
> > > > sensor
> > > >
> > >
> > > Okay.
> > >
> > > > > + adjustments. Upon reading previously set value is returned.
> > > > > + Units are millibars.
> > > >
> > > > unit for pressure in IIO is kilopascal (e.g.
> > > > /sys/bus/iio/devices/iio:deviceX/in_pressure_raw)
> > > >
> > >
> > > My thinking here was that since these are sensor specific attributes
> > > they don't need to stick to iio conventions and millibars were somewhat
> > > more natural to use. But I guess that's just matter of habit.
> >
> > You absolutely have to stick to standard units. Userspace programs
> > aren't going to come read your docs...
> >
> > For other sensors that take a calibration value like this we've reported
> > them via an output channel. For example the atlas-ph sensor has
> > an 'output temp' channel used for this purpose.
> >
>
> Fair enough.
>
> > It's not ideal or totally intuitive but it does let us avoid expanding
> > the overall ABI. The argument was something along the lines of
> > 1) Imagine your sensor could control the pressure in the measurement space...
> > 2) An output channel would provide the value to set it to.
> > 3) Now instead we provide a means of saying 'what it is'
> > 4) End result is we write a value and the pressure in the chamber is
> > that value :)
> >
> > As I said not ideal but the best we can do without having to define a lot
> > of ABI just to deal with compensation factors.
> >
> > This is a rare case where I would document the 'standard' ABI in here
> > to make the point that it is actually providing an estimate of the pressure
> > not controlling it...
> >
> > >
> > > So generally I am okay with reworking all attrs to accept values in iio
> > > preferred units.
> > >
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + The range of available values in millibars represented as the
> > > > > + minimum value, the step and the maximum value, all enclosed in
> > > > > + square brackets.
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval
> > > > > +Date: January 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Amount of time between subsequent measurements. Writing this
> > > > > + attribute will change measurement interval. Upon reading
> > > > > + current measurement interval is returned. Units are seconds.
> >
> > Use the existing ABI sampling frequency which is sort of the inverse of this.
> >
>
> Was thinking about it but long periods in Hz simply don't look appealing :).
>
> No other strong opinions so I'll rework that.

Agreed it can look a bit odd, but we don't want to have multiple controls for the
same thing so we are stuck with it.

>
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + The range of available values in seconds represented as the
> > > > > + minimum value, the step and the maximum value, all enclosed in
> > > > > + square brackets.
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/asc
> > Spends some characters to easy of understanding ;)
> >
> > auto_calib_proc_enable maybe? Or can we get away with the 'somewhat standard
> > calibration (it's used in at least one other driver IIRC)
> >
>
> Just self_calibration would do?

I'll think a bit more on this one but probably fine.

>
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Writing 1 or 0 to this attribute will respectively activate or
> > > > > + deactivate automatic self calibration procedure. Upon reading 1
> > > >
> > > > deactivate automatic self calibration (asc) procedure
> > > >
> > >
> > > That shouldn't be too difficult to realize what asc actually stands for after
> > > reading this short description.
> > >
> > > > > + is returned if asc is ongoing, 0 otherwise.
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/frc
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Forced recalibration is used to compensate for sensor drifts
> > > > > + when a reference value of CO2 concentration in close proximity
> > > > > + to the sensor is available. Writing attribute will set frc
> > > > > + value. Upon reading current frc is returned. Units are
> > > > > + millibars.
> >
> > Could we implement this by just writing to the main channel value?
> > Bit of a clunky ABI but sort of logically fits in my head given we are basically
> > forcing the value we read to be this one?
> >
>
> So the similar to the pressure compensation. Okay.
>
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/frc_available
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + The range of available values in millibars represented as the
> > > > > + minimum value, the step and the maximum value, all enclosed in
> > > > > + square brackets.
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Sensor readings may be affected by ambient temperature.
> > > > > + Writing temperature offset will compensate for unwanted changes.
> > > > > + Note that written offset gets multiplied by a factor of 100
> > > > > + by a sensor internally.
> > > > > +
> > > > > + For example, writing 10 here will correspond to 0.1 degree
> > > > > + Celsius.
> >
> > This sounds like a calibbias to me which is standard ABI.
> >
>
> Right, that could work.
>
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + The range of available values in degrees Celsius represented as
> > > > > + the minimum value, the step and the maximum value, all enclosed
> > > > > + in square brackets.
> >
> > Wrong units for temperature (which is an odd one as we
> > lifted them from hwmon before learning the error of our ways and starting to use
> > SI units as the base).
> >
>
> Does calibbias have _available counterpart?

It might not be documented yet (as not sure it's been used) but any attribute has
an available counterpart. That's effectively standard ABI.

>
> >
> > > > > +
> > > > > +What: /sys/bus/iio/devices/iio:deviceX/reset
> > > > > +Date: April 2020
> > > > > +KernelVersion: 5.8
> > > > > +Contact: [email protected]
> > > > > +Description:
> > > > > + Software reset mechanism forces sensor into the same state
> > > > > + as after powering up without the need for removing power supply.
> > > > > + Writing any value will reset sensor.
> >
> > Not seeing an argument here for why you might want to do that other than on
> > power up or module probe to get the driver into a known state.
> > So currently it's a no to this one - just don't expose it to userspace.
> >
>
> If one writes some odd configuration (though allowed) into sensor, for example
> out of sheer curiosity and then writes the sane values back sensor needs some
> time to recover (i.e start reporting valid measurements again).
>
> So rationale here was that after reset sensor recovers immediately. I'd
> say that reset is sometimes useful. Perhaps that could be exported by
> means of iio debug api?

Debugfs would be fine

>
> >
> > > > >
> > > >
> > > > --
> > > >
> > > > Peter Meerwald-Stadler
> > > > Mobile: +43 664 24 44 418
> >