* [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor
@ 2021-08-17 15:49 Jacopo Mondi
2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jacopo Mondi @ 2021-08-17 15:49 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, Matt Ranostay
Cc: Jacopo Mondi, linux-iio
Hello,
this is a small driver for the Senseair Sunrise 006-0-0007 CO2
sensor.
The driver supports continuous reads of temperature and CO2 concentration
through two dedicated IIO channels.
While the driver is rather simple I'm not sure calibration is handled in
the correct way. In this version, at probe time, a check on the general
error register is made to verify if a calibration cycle is required.
The calibration takes a time in the order of a few seconds, and currently
can only happen at probe time.
Is there a mechanism available in the IIO framework to expose a trigger to have
userspace decide when the calibration has to happen ? In my understanding IIO
triggers are meant to trigger read events, using them for calibration purpose
seems not the right thing to do, or am I mistaken ?
Thanks
j
Jacopo Mondi (2):
dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor
iio: chemical: Add Senseair Sunrise 006-0-007 driver
.../iio/chemical/senseair,sunrise.yaml | 51 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 6 +
drivers/iio/chemical/Kconfig | 10 +
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/sunrise.c | 310 ++++++++++++++++++
6 files changed, 380 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml
create mode 100644 drivers/iio/chemical/sunrise.c
--
2.32.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor 2021-08-17 15:49 [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Jacopo Mondi @ 2021-08-17 15:49 ` Jacopo Mondi 2021-08-17 16:50 ` Andy Shevchenko 2021-08-18 16:49 ` Rob Herring 2021-08-17 15:49 ` [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver Jacopo Mondi 2021-08-17 17:01 ` [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Matt Ranostay 2 siblings, 2 replies; 13+ messages in thread From: Jacopo Mondi @ 2021-08-17 15:49 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, Matt Ranostay, Rob Herring Cc: Jacopo Mondi, linux-iio, devicetree Add documentation for the Senseair Sunrise 006-0-0007 CO2 NDIR sensor. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- .../iio/chemical/senseair,sunrise.yaml | 51 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + 2 files changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml diff --git a/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml b/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml new file mode 100644 index 000000000000..b77196666187 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/chemical/senseair,sunrise.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Senseair Sunrise 006-0-0007 CO2 Sensor + +maintainers: + - Jacopo Mondi <jacopo@jmondi.org> + +description: | + Senseair Sunrise 006-0-0007 is a NDIR CO2 sensor. It supports I2C or UART buses + for communications and control. + + Datasheets: + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/PSP11704.pdf + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/PSH11649.pdf + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/TDE5531.pdf + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Market/publicerat/TDE7318.pdf + +properties: + compatible: + const: senseair,sunrise-006-0-0007 + + reg: + maxItems: 1 + + ndry-gpios: + description: Phandle to the GPIO line connected to the nDRY pin. Active low. + + en-gpios: + description: Phandle to the GPIO line connected to the EN pin. Active high. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + sunrise@68 { + compatible = "senseair,sunrise-006-0-0007"; + reg = <0x68>; + }; + }; diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 944a14926e02..c60502eb3d36 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1000,6 +1000,8 @@ patternProperties: description: Shenzhen SEI Robotics Co., Ltd "^semtech,.*": description: Semtech Corporation + "^senseair,.*": + description: Senseair AB "^sensirion,.*": description: Sensirion AG "^sensortek,.*": -- 2.32.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor 2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi @ 2021-08-17 16:50 ` Andy Shevchenko 2021-08-18 7:29 ` Jacopo Mondi 2021-08-18 16:49 ` Rob Herring 1 sibling, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-08-17 16:50 UTC (permalink / raw) To: Jacopo Mondi Cc: Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, Rob Herring, linux-iio, devicetree On Tue, Aug 17, 2021 at 05:49:50PM +0200, Jacopo Mondi wrote: > Add documentation for the Senseair Sunrise 006-0-0007 CO2 NDIR sensor. > + ndry-gpios: > + description: Phandle to the GPIO line connected to the nDRY pin. Active low. > + > + en-gpios: > + description: Phandle to the GPIO line connected to the EN pin. Active high. Not sure you have to mention polarity. It can be changed on PCB level easily and this bindings won't satisfy those (valid) changes. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor 2021-08-17 16:50 ` Andy Shevchenko @ 2021-08-18 7:29 ` Jacopo Mondi 2021-08-18 16:47 ` Rob Herring 0 siblings, 1 reply; 13+ messages in thread From: Jacopo Mondi @ 2021-08-18 7:29 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, Rob Herring, linux-iio, devicetree Hi Andy, On Tue, Aug 17, 2021 at 07:50:46PM +0300, Andy Shevchenko wrote: > On Tue, Aug 17, 2021 at 05:49:50PM +0200, Jacopo Mondi wrote: > > Add documentation for the Senseair Sunrise 006-0-0007 CO2 NDIR sensor. > > > + ndry-gpios: > > + description: Phandle to the GPIO line connected to the nDRY pin. Active low. > > + > > + en-gpios: > > + description: Phandle to the GPIO line connected to the EN pin. Active high. > > Not sure you have to mention polarity. It can be changed on PCB level easily > and this bindings won't satisfy those (valid) changes. Well, one can indeed invert the signal on the PCB, it's weird I see most of the bindings reporting the pin's active polarity though... I'll drop it. Thanks j > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor 2021-08-18 7:29 ` Jacopo Mondi @ 2021-08-18 16:47 ` Rob Herring 0 siblings, 0 replies; 13+ messages in thread From: Rob Herring @ 2021-08-18 16:47 UTC (permalink / raw) To: Jacopo Mondi Cc: Andy Shevchenko, Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, linux-iio, devicetree On Wed, Aug 18, 2021 at 09:29:43AM +0200, Jacopo Mondi wrote: > Hi Andy, > > On Tue, Aug 17, 2021 at 07:50:46PM +0300, Andy Shevchenko wrote: > > On Tue, Aug 17, 2021 at 05:49:50PM +0200, Jacopo Mondi wrote: > > > Add documentation for the Senseair Sunrise 006-0-0007 CO2 NDIR sensor. > > > > > + ndry-gpios: > > > + description: Phandle to the GPIO line connected to the nDRY pin. Active low. > > > + > > > + en-gpios: > > > + description: Phandle to the GPIO line connected to the EN pin. Active high. > > > > Not sure you have to mention polarity. It can be changed on PCB level easily > > and this bindings won't satisfy those (valid) changes. > > Well, one can indeed invert the signal on the PCB, it's weird I see most of > the bindings reporting the pin's active polarity though... > > I'll drop it. I'd keep it. It's documenting the pin on the h/w. You can prefix with 'Typically' if you want. Rob ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor 2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi 2021-08-17 16:50 ` Andy Shevchenko @ 2021-08-18 16:49 ` Rob Herring 1 sibling, 0 replies; 13+ messages in thread From: Rob Herring @ 2021-08-18 16:49 UTC (permalink / raw) To: Jacopo Mondi Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, Matt Ranostay, linux-iio, devicetree On Tue, Aug 17, 2021 at 05:49:50PM +0200, Jacopo Mondi wrote: > Add documentation for the Senseair Sunrise 006-0-0007 CO2 NDIR sensor. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > .../iio/chemical/senseair,sunrise.yaml | 51 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > 2 files changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml > > diff --git a/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml b/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml > new file mode 100644 > index 000000000000..b77196666187 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml > @@ -0,0 +1,51 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/chemical/senseair,sunrise.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Senseair Sunrise 006-0-0007 CO2 Sensor > + > +maintainers: > + - Jacopo Mondi <jacopo@jmondi.org> > + > +description: | > + Senseair Sunrise 006-0-0007 is a NDIR CO2 sensor. It supports I2C or UART buses > + for communications and control. > + > + Datasheets: > + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/PSP11704.pdf > + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/PSH11649.pdf > + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Dev/publicerat/TDE5531.pdf > + https://rmtplusstoragesenseair.blob.core.windows.net/docs/Market/publicerat/TDE7318.pdf > + > +properties: > + compatible: > + const: senseair,sunrise-006-0-0007 > + > + reg: > + maxItems: 1 > + > + ndry-gpios: > + description: Phandle to the GPIO line connected to the nDRY pin. Active low. > + > + en-gpios: > + description: Phandle to the GPIO line connected to the EN pin. Active high. You need to define how many (maxItems: 1). > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + sunrise@68 { 'co2-sensor' perhaps. > + compatible = "senseair,sunrise-006-0-0007"; > + reg = <0x68>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 944a14926e02..c60502eb3d36 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -1000,6 +1000,8 @@ patternProperties: > description: Shenzhen SEI Robotics Co., Ltd > "^semtech,.*": > description: Semtech Corporation > + "^senseair,.*": > + description: Senseair AB > "^sensirion,.*": > description: Sensirion AG > "^sensortek,.*": > -- > 2.32.0 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver 2021-08-17 15:49 [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Jacopo Mondi 2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi @ 2021-08-17 15:49 ` Jacopo Mondi 2021-08-17 16:58 ` Andy Shevchenko 2021-08-17 17:01 ` [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Matt Ranostay 2 siblings, 1 reply; 13+ messages in thread From: Jacopo Mondi @ 2021-08-17 15:49 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, Matt Ranostay Cc: Jacopo Mondi, linux-iio Add support for the Senseair Sunrise 006-0-0007 driver through the IIO subsystem. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- MAINTAINERS | 6 + drivers/iio/chemical/Kconfig | 10 ++ drivers/iio/chemical/Makefile | 1 + drivers/iio/chemical/sunrise.c | 310 +++++++++++++++++++++++++++++++++ 4 files changed, 327 insertions(+) create mode 100644 drivers/iio/chemical/sunrise.c diff --git a/MAINTAINERS b/MAINTAINERS index 90ca9df1d3c3..10a33fd2ccb8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16544,6 +16544,12 @@ S: Maintained F: drivers/misc/phantom.c F: include/uapi/linux/phantom.h +SENSEAIR SUNRISE 006-0-0007 +M: Jacopo Mondi <jacopo@jmondi.org> +S: Maintained +F: Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml +F: drivers/iio/chemical/sunrise.c + SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER M: Tomasz Duszynski <tomasz.duszynski@octakon.com> S: Maintained diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig index 10bb431bc3ce..c308379076ba 100644 --- a/drivers/iio/chemical/Kconfig +++ b/drivers/iio/chemical/Kconfig @@ -144,6 +144,16 @@ config SPS30 To compile this driver as a module, choose M here: the module will be called sps30. +config SUNRISE + tristate "Senseair Sunrise 006-0-0007 CO2 sensor" + depends on I2C + help + Say yes here to build support for Senseair Sunrise 006-0-0007 CO2 + sensor. + + To compile this driver as a module, choose M here: the + module will be called sunrise. + config VZ89X tristate "SGX Sensortech MiCS VZ89X VOC sensor" depends on I2C diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile index fef63dd5bf92..24b8d63d05bd 100644 --- a/drivers/iio/chemical/Makefile +++ b/drivers/iio/chemical/Makefile @@ -17,4 +17,5 @@ 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_SUNRISE) += sunrise.o obj-$(CONFIG_VZ89X) += vz89x.o diff --git a/drivers/iio/chemical/sunrise.c b/drivers/iio/chemical/sunrise.c new file mode 100644 index 000000000000..582633e86d1d --- /dev/null +++ b/drivers/iio/chemical/sunrise.c @@ -0,0 +1,310 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * iio/co2/sunrise-006-0-0007.c + * + * Senseair Sunrise 006-0-0007 CO2 sensor driver. + * + * Copyright (C) 2021 Jacopo Mondi + * + * The driver supports continuous measurement operation mode only. + * TODO: Add support for controllable EN pin + * TODO: Add support for single-shot operations by using the nDRY pin. + */ + +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iopoll.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> + +#define DRIVER_NAME "sunrise" + +#define SUNRISE_ERROR_STATUS_REG 0x00 +#define SUNRISE_ERROR_OOR_ERR BIT(5) +#define SUNRISE_CO2_FILTERED_COMP_REG 0x06 +#define SUNRISE_CHIP_TEMPERATURE_REG 0x08 +#define SUNRISE_CALIBRATION_STATUS_REG 0x81 +#define SUNRISE_CALIBRATION_COMMAND_REG 0x082 +#define SUNRISE_CALIBRATION_BACKGROUND_CMD 0x7c06 + +struct sunrise_dev { + struct i2c_client *client; + struct mutex lock; +}; + +static void sunrise_wakeup(struct i2c_client *client) +{ + /* + * Ping to wakeup: the chip returns in low power state after 15 msec + * without communications or after a complete read/write sequence. + */ + i2c_smbus_read_byte(client); +} + +static int sunrise_read_byte(struct i2c_client *client, u8 reg, u8 *val) +{ + int ret; + + sunrise_wakeup(client); + ret = i2c_smbus_read_byte_data(client, reg); + if (ret < 0) { + dev_err(&client->dev, "Read byte failed (%d)\n", ret); + return ret; + } + + *val = ret; + + return 0; +} + +/* Accessors for readx_poll_timeout() function. */ +#define sunrise_readb_client(client, addr) \ +({ \ + u8 _val; \ + sunrise_read_byte(client, addr, &_val); \ + _val; \ +}) +#define sunrise_readb(addr) sunrise_readb_client(client, addr) +#define sunrise_poll_register(reg, val, cond) \ + readx_poll_timeout(sunrise_readb, reg, val, cond, 100, 0) + +static int sunrise_read_word(struct i2c_client *client, u8 reg, u16 *val) +{ + int ret; + + sunrise_wakeup(client); + ret = i2c_smbus_read_word_data(client, reg); + if (ret < 0) { + dev_err(&client->dev, "Read word failed (%d)\n", ret); + return ret; + } + + *val = be16_to_cpu(ret); + + return 0; +} + +static int sunrise_write_byte(struct i2c_client *client, u8 reg, u8 val) +{ + int ret; + + sunrise_wakeup(client); + ret = i2c_smbus_write_byte_data(client, reg, val); + if (ret) { + dev_err(&client->dev, "Write byte failed (%d)\n", ret); + return ret; + } + + return 0; +} + +static int sunrise_write_word(struct i2c_client *client, u8 reg, u16 data) +{ + u16 be_data = cpu_to_be16(data); + int ret; + + sunrise_wakeup(client); + ret = i2c_smbus_write_word_data(client, reg, be_data); + if (ret) { + dev_err(&client->dev, "Write word failed (%d)\n", ret); + return ret; + } + + return 0; +} + +static const struct iio_chan_spec sunrise_channels[] = { + { + .type = IIO_CONCENTRATION, + .modified = 1, + .channel2 = IIO_MOD_CO2, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_TEMP, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 1, + .scan_type = { + .sign = 's', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, +}; + +static int sunrise_read_co2_filtered(struct i2c_client *client, u16 *co2) +{ + int ret; + + ret = sunrise_read_word(client, SUNRISE_CO2_FILTERED_COMP_REG, co2); + if (ret) + return ret; + + return 0; +} + +static int sunrise_read_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int *val, int *val2, long mask) +{ + struct sunrise_dev *sunrise = iio_priv(iio_dev); + struct i2c_client *client = sunrise->client; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + + mutex_lock(&sunrise->lock); + + switch (chan->type) { + case IIO_CONCENTRATION: { + u16 co2; + + ret = sunrise_read_co2_filtered(client, &co2); + *val = co2; + mutex_unlock(&sunrise->lock); + + return ret ? ret : IIO_VAL_INT; + } + + case IIO_TEMP: { + u16 temp; + + ret = sunrise_read_word(client, + SUNRISE_CHIP_TEMPERATURE_REG, + &temp); + *val = temp; + mutex_unlock(&sunrise->lock); + + return ret ? ret : IIO_VAL_INT; + } + + default: + mutex_unlock(&sunrise->lock); + return -EINVAL; + } + + case IIO_CHAN_INFO_SCALE: + /* Chip temperature scale = 1/100 */ + *val = 1; + *val2 = 100; + return IIO_VAL_FRACTIONAL; + + default: + return -EINVAL; + } + + return 0; +} + +static const struct iio_info sunrise_info = { + .read_raw = sunrise_read_raw, +}; + +static int sunrise_calibrate(struct i2c_client *client) +{ + u16 status; + int ret; + + /* Verify if calibration is required. */ + ret = sunrise_read_word(client, SUNRISE_ERROR_STATUS_REG, &status); + if (ret) + return ret; + if (!status) + return 0; + + /* + * A different error than "Out of range": report it in dmseg but + * do not fail. However operations might be compromised. If OOR error + * is set, proceed with a calibration cycle. + */ + if (status & ~SUNRISE_ERROR_OOR_ERR) + dev_err(&client->dev, "Error status: 0x%2x\n", status); + if (!(status & SUNRISE_ERROR_OOR_ERR)) + return 0; + + /* Run an ADC background calibration cycle. */ + ret = sunrise_write_byte(client, SUNRISE_CALIBRATION_STATUS_REG, 0x00); + if (ret) + return ret; + + ret = sunrise_write_word(client, SUNRISE_CALIBRATION_COMMAND_REG, + SUNRISE_CALIBRATION_BACKGROUND_CMD); + if (ret) + return ret; + + /* + * Perform a read to trigger co2 calibration and poll the status bit + * until calibration is completed. + */ + dev_dbg(&client->dev, "ABC backgrounf calibration in progress\n"); + + ret = sunrise_read_co2_filtered(client, &status); + if (ret) + return ret; + + return sunrise_poll_register(SUNRISE_CALIBRATION_STATUS_REG, status, + (status & BIT(5))); +} + +static int sunrise_probe(struct i2c_client *client) +{ + struct sunrise_dev *sunrise; + struct iio_dev *iio_dev; + int ret; + + iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*sunrise)); + if (!iio_dev) + return -ENOMEM; + + i2c_set_clientdata(client, iio_dev); + + sunrise = iio_priv(iio_dev); + sunrise->client = client; + mutex_init(&sunrise->lock); + + iio_dev->info = &sunrise_info; + iio_dev->name = DRIVER_NAME; + iio_dev->channels = sunrise_channels; + iio_dev->num_channels = ARRAY_SIZE(sunrise_channels); + iio_dev->modes = INDIO_DIRECT_MODE; + + ret = sunrise_calibrate(client); + if (ret) { + dev_err(&client->dev, "Calibration failed (%d)\n", ret); + return ret; + } + + return devm_iio_device_register(&client->dev, iio_dev); +} + +static const struct of_device_id sunrise_of_match[] = { + { .compatible = "senseair,sunrise-006-0-0007" }, + {} +}; +MODULE_DEVICE_TABLE(of, sunrise_of_match); + +static struct i2c_driver sunrise_driver = { + .driver = { + .name = DRIVER_NAME, + .of_match_table = sunrise_of_match, + }, + .probe_new = sunrise_probe, +}; +module_i2c_driver(sunrise_driver); + +MODULE_AUTHOR("Jacopo Mondi <jacopo@jmondi.org>"); +MODULE_DESCRIPTION("Senseair Sunrise 006-0-0007 CO2 sensor IIO driver"); +MODULE_LICENSE("GPL v2"); -- 2.32.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver 2021-08-17 15:49 ` [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver Jacopo Mondi @ 2021-08-17 16:58 ` Andy Shevchenko 2021-08-18 8:02 ` Jacopo Mondi 0 siblings, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-08-17 16:58 UTC (permalink / raw) To: Jacopo Mondi Cc: Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, linux-iio On Tue, Aug 17, 2021 at 05:49:51PM +0200, Jacopo Mondi wrote: > Add support for the Senseair Sunrise 006-0-0007 driver through the > IIO subsystem. Datasheet tag / link? ... > +config SUNRISE > + tristate "Senseair Sunrise 006-0-0007 CO2 sensor" > + depends on I2C > + help > + Say yes here to build support for Senseair Sunrise 006-0-0007 CO2 > + sensor. > + > + To compile this driver as a module, choose M here: the > + module will be called sunrise. Too generic name for configuration and module, ... > + * iio/co2/sunrise-006-0-0007.c > + * Redundant noise. ... > + * TODO: Add support for controllable EN pin > + * TODO: Add support for single-shot operations by using the nDRY pin. Ditto. If it's not ready, then make it ready. ... > +#include <linux/of_device.h> Why? Perhaps mod_devicetable,h is what you had in mind. ... > + i2c_smbus_read_byte(client); Can you use regmap I²C API? ... > +#define sunrise_readb_client(client, addr) \ > +({ \ > + u8 _val; \ > + sunrise_read_byte(client, addr, &_val); \ > + _val; \ > +}) > +#define sunrise_readb(addr) sunrise_readb_client(client, addr) Drop ugly macros and use read_poll_timeout() below. > +#define sunrise_poll_register(reg, val, cond) \ > + readx_poll_timeout(sunrise_readb, reg, val, cond, 100, 0) ... > +static int sunrise_read_raw(struct iio_dev *iio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long mask) > +{ > + struct sunrise_dev *sunrise = iio_priv(iio_dev); > + struct i2c_client *client = sunrise->client; + u16 value; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + > + mutex_lock(&sunrise->lock); > + > + switch (chan->type) { > + case IIO_CONCENTRATION: { > + u16 co2; Reuse value. > + ret = sunrise_read_co2_filtered(client, &co2); > + *val = co2; > + mutex_unlock(&sunrise->lock); > + > + return ret ? ret : IIO_VAL_INT; Can be written as return ret ?: IIO_VAL_INT; but up to maintainers. > + } > + > + case IIO_TEMP: { > + u16 temp; Reuse value. > + ret = sunrise_read_word(client, > + SUNRISE_CHIP_TEMPERATURE_REG, > + &temp); > + *val = temp; > + mutex_unlock(&sunrise->lock); > + > + return ret ? ret : IIO_VAL_INT; > + } > + > + default: > + mutex_unlock(&sunrise->lock); > + return -EINVAL; > + } > + > + case IIO_CHAN_INFO_SCALE: > + /* Chip temperature scale = 1/100 */ > + *val = 1; > + *val2 = 100; > + return IIO_VAL_FRACTIONAL; > + > + default: > + return -EINVAL; > + } > + > + return 0; > +} ... > + return sunrise_poll_register(SUNRISE_CALIBRATION_STATUS_REG, status, > + (status & BIT(5))); Too many parentheses. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver 2021-08-17 16:58 ` Andy Shevchenko @ 2021-08-18 8:02 ` Jacopo Mondi 2021-08-18 9:29 ` Andy Shevchenko 2021-08-29 16:05 ` Jonathan Cameron 0 siblings, 2 replies; 13+ messages in thread From: Jacopo Mondi @ 2021-08-18 8:02 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, linux-iio Hi Andy, thanks for review On Tue, Aug 17, 2021 at 07:58:38PM +0300, Andy Shevchenko wrote: > On Tue, Aug 17, 2021 at 05:49:51PM +0200, Jacopo Mondi wrote: > > Add support for the Senseair Sunrise 006-0-0007 driver through the > > IIO subsystem. > > Datasheet tag / link? It's reported in the bindings document, I can copy it here > > ... > > > +config SUNRISE > > + tristate "Senseair Sunrise 006-0-0007 CO2 sensor" > > + depends on I2C > > + help > > + Say yes here to build support for Senseair Sunrise 006-0-0007 CO2 > > + sensor. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called sunrise. > > Too generic name for configuration and module, I got the same feeling but at the same time SUNRISE_006_0_0007 seems a bit too long. Would you suggest to go for that one ? Should I also rename the .c file accordingly ? > > ... > > > + * iio/co2/sunrise-006-0-0007.c > > + * > > Redundant noise. > > ... > > > + * TODO: Add support for controllable EN pin > > + * TODO: Add support for single-shot operations by using the nDRY pin. > > Ditto. > > If it's not ready, then make it ready. If I place myself in the perspective of someone interested in verifying support for this chip in linux, I would first be happy I found a driver for it, then I would even be happier if I got a clear notice of what features the driver supports and which ones are missing. Even more so if I look at bindings and find mention of two GPIO lines which I don't see handled in the driver. A TODO note would make it clear that those features are missing. The "make it ready" it's a little bit a non-sense to me, can you count how many drivers support -all- the features a device provides ? In the test board I'm using those lines are not even wired, should I throw around code I cannot even test ? > > ... > > > > +#include <linux/of_device.h> > > Why? Perhaps mod_devicetable,h is what you had in mind. > Ah yes indeed. I got it wrong in so many places in the past ! > ... > > > + i2c_smbus_read_byte(client); > > Can you use regmap I²C API? > Do you think it's worth it ? I admit I never used regmap API so far, but I always got the impression that for such simple devices it's a bit an overkill. What benefit would it bring here ? > ... > > > +#define sunrise_readb_client(client, addr) \ > > +({ \ > > + u8 _val; \ > > + sunrise_read_byte(client, addr, &_val); \ > > + _val; \ > > +}) > > +#define sunrise_readb(addr) sunrise_readb_client(client, addr) > > Drop ugly macros and use read_poll_timeout() below. Ah! Thanks a lot, I missed the fact read_poll_timeout() accepts a variable number of arguments... However, the API of the driver i2c read function is: int sunrise_read_byte(client, reg, *val); while read_poll_timeout() expects int read(client, reg); I know the answer already: change your i2c accessor to comply. As it is only used by read_poll_timeout() I'll do so :) > > > +#define sunrise_poll_register(reg, val, cond) \ > > + readx_poll_timeout(sunrise_readb, reg, val, cond, 100, 0) > > > ... > > > +static int sunrise_read_raw(struct iio_dev *iio_dev, > > + const struct iio_chan_spec *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct sunrise_dev *sunrise = iio_priv(iio_dev); > > + struct i2c_client *client = sunrise->client; > > + u16 value; > > > + int ret; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + > > + mutex_lock(&sunrise->lock); > > + > > + switch (chan->type) { > > + case IIO_CONCENTRATION: { > > + u16 co2; > > Reuse value. > Ack > > + ret = sunrise_read_co2_filtered(client, &co2); > > + *val = co2; > > + mutex_unlock(&sunrise->lock); > > + > > + return ret ? ret : IIO_VAL_INT; > > Can be written as > > return ret ?: IIO_VAL_INT; > > but up to maintainers. > You know I never saw this syntax before ? :D I'll use it! > > + } > > + > > + case IIO_TEMP: { > > + u16 temp; > > Reuse value. > ack > > + ret = sunrise_read_word(client, > > + SUNRISE_CHIP_TEMPERATURE_REG, > > + &temp); > > + *val = temp; > > + mutex_unlock(&sunrise->lock); > > + > > + return ret ? ret : IIO_VAL_INT; > > + } > > + > > + default: > > + mutex_unlock(&sunrise->lock); > > + return -EINVAL; > > + } > > + > > + case IIO_CHAN_INFO_SCALE: > > + /* Chip temperature scale = 1/100 */ > > + *val = 1; > > + *val2 = 100; > > + return IIO_VAL_FRACTIONAL; > > + > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > ... > > > + return sunrise_poll_register(SUNRISE_CALIBRATION_STATUS_REG, status, > > + (status & BIT(5))); > > Too many parentheses. Right! Thanks for review! > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver 2021-08-18 8:02 ` Jacopo Mondi @ 2021-08-18 9:29 ` Andy Shevchenko 2021-08-29 16:05 ` Jonathan Cameron 1 sibling, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2021-08-18 9:29 UTC (permalink / raw) To: Jacopo Mondi Cc: Jonathan Cameron, Lars-Peter Clausen, Matt Ranostay, linux-iio On Wed, Aug 18, 2021 at 10:02:13AM +0200, Jacopo Mondi wrote: > On Tue, Aug 17, 2021 at 07:58:38PM +0300, Andy Shevchenko wrote: > > On Tue, Aug 17, 2021 at 05:49:51PM +0200, Jacopo Mondi wrote: > > > Add support for the Senseair Sunrise 006-0-0007 driver through the > > > IIO subsystem. > > > > Datasheet tag / link? > > It's reported in the bindings document, I can copy it here Yes, please, as a tag Datasheet: ... Signed-of-by: ... ... > > > +config SUNRISE > > > > Too generic name for configuration and module, > > I got the same feeling but at the same time SUNRISE_006_0_0007 seems a > bit too long. Would you suggest to go for that one ? Should I also > rename the .c file accordingly ? I would go with something like sunrise_air_6 or whatever part of the model. I haven't read datasheet to propose better naming. Also possible to have something like sunrise_air_meter.c (and corresponding Kconfig). ... > > > + * TODO: Add support for controllable EN pin > > > + * TODO: Add support for single-shot operations by using the nDRY pin. > > > > Ditto. > > > > If it's not ready, then make it ready. > > If I place myself in the perspective of someone interested in > verifying support for this chip in linux, I would first be happy I > found a driver for it, then I would even be happier if I got a clear > notice of what features the driver supports and which ones are missing. > Even more so if I look at bindings and find mention of two GPIO lines > which I don't see handled in the driver. A TODO note would make it > clear that those features are missing. > > The "make it ready" it's a little bit a non-sense to me, can you count > how many drivers support -all- the features a device provides ? In the > test board I'm using those lines are not even wired, should I throw > around code I cannot even test ? My point is that TODO means that somebody is working or will work on this. Is it the case? Then my comment applies, if not, replace this with something more realistic, like * Feature list that driver does NOT support (yet): * 1) ... * 2) ... ... > > > + i2c_smbus_read_byte(client); > > > > Can you use regmap I²C API? > > Do you think it's worth it ? > I admit I never used regmap API so far, but I always got the > impression that for such simple devices it's a bit an overkill. What > benefit would it bring here ? Some of nice features for debugging and tracing are in the generic support for free. On top if even this will be part of multi-function device than you won;t need to rewrite the driver. I would definitely go for it. ... > > > + return ret ? ret : IIO_VAL_INT; > > > > Can be written as > > > > return ret ?: IIO_VAL_INT; > > > > but up to maintainers. > > > > You know I never saw this syntax before ? :D > I'll use it! It's the C standard :-) -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver 2021-08-18 8:02 ` Jacopo Mondi 2021-08-18 9:29 ` Andy Shevchenko @ 2021-08-29 16:05 ` Jonathan Cameron 1 sibling, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2021-08-29 16:05 UTC (permalink / raw) To: Jacopo Mondi Cc: Andy Shevchenko, Lars-Peter Clausen, Matt Ranostay, linux-iio > Ack > > > > + ret = sunrise_read_co2_filtered(client, &co2); > > > + *val = co2; > > > + mutex_unlock(&sunrise->lock); > > > + > > > + return ret ? ret : IIO_VAL_INT; > > > > Can be written as > > > > return ret ?: IIO_VAL_INT; > > > > but up to maintainers. > > > > You know I never saw this syntax before ? :D > I'll use it! I'm rather late to the party, but I'd keep all error handling in driver of the form if (ret) return ret; return IIO_VAL_INT; That means lazy reviewers like me have one less thing to think about ;) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor 2021-08-17 15:49 [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Jacopo Mondi 2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi 2021-08-17 15:49 ` [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver Jacopo Mondi @ 2021-08-17 17:01 ` Matt Ranostay 2021-08-18 8:03 ` Jacopo Mondi 2 siblings, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2021-08-17 17:01 UTC (permalink / raw) To: Jacopo Mondi Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, open list:IIO SUBSYSTEM AND DRIVERS On Tue, Aug 17, 2021 at 8:49 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hello, > this is a small driver for the Senseair Sunrise 006-0-0007 CO2 > sensor. > > The driver supports continuous reads of temperature and CO2 concentration > through two dedicated IIO channels. > > While the driver is rather simple I'm not sure calibration is handled in > the correct way. In this version, at probe time, a check on the general > error register is made to verify if a calibration cycle is required. > The calibration takes a time in the order of a few seconds, and currently > can only happen at probe time. > > Is there a mechanism available in the IIO framework to expose a trigger to have > userspace decide when the calibration has to happen ? In my understanding IIO > triggers are meant to trigger read events, using them for calibration purpose > seems not the right thing to do, or am I mistaken ? For sure you shouldn't use the trigger framework for that. You should use an attribute group, e.g. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/proximity/as3935.c?h=v5.14-rc6#n168 - Matt > > Thanks > j > > Jacopo Mondi (2): > dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor > iio: chemical: Add Senseair Sunrise 006-0-007 driver > > .../iio/chemical/senseair,sunrise.yaml | 51 +++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > MAINTAINERS | 6 + > drivers/iio/chemical/Kconfig | 10 + > drivers/iio/chemical/Makefile | 1 + > drivers/iio/chemical/sunrise.c | 310 ++++++++++++++++++ > 6 files changed, 380 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml > create mode 100644 drivers/iio/chemical/sunrise.c > > -- > 2.32.0 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor 2021-08-17 17:01 ` [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Matt Ranostay @ 2021-08-18 8:03 ` Jacopo Mondi 0 siblings, 0 replies; 13+ messages in thread From: Jacopo Mondi @ 2021-08-18 8:03 UTC (permalink / raw) To: Matt Ranostay Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, open list:IIO SUBSYSTEM AND DRIVERS Hi Matt, On Tue, Aug 17, 2021 at 10:01:40AM -0700, Matt Ranostay wrote: > On Tue, Aug 17, 2021 at 8:49 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > > > Hello, > > this is a small driver for the Senseair Sunrise 006-0-0007 CO2 > > sensor. > > > > The driver supports continuous reads of temperature and CO2 concentration > > through two dedicated IIO channels. > > > > While the driver is rather simple I'm not sure calibration is handled in > > the correct way. In this version, at probe time, a check on the general > > error register is made to verify if a calibration cycle is required. > > The calibration takes a time in the order of a few seconds, and currently > > can only happen at probe time. > > > > Is there a mechanism available in the IIO framework to expose a trigger to have > > userspace decide when the calibration has to happen ? In my understanding IIO > > triggers are meant to trigger read events, using them for calibration purpose > > seems not the right thing to do, or am I mistaken ? > > For sure you shouldn't use the trigger framework for that. > > You should use an attribute group, e.g. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/proximity/as3935.c?h=v5.14-rc6#n168 Thanks for the pointer! I'll consider that for v2! > > - Matt > > > > > Thanks > > j > > > > Jacopo Mondi (2): > > dt-bindings: iio: chemical: Document senseair,sunrise CO2 sensor > > iio: chemical: Add Senseair Sunrise 006-0-007 driver > > > > .../iio/chemical/senseair,sunrise.yaml | 51 +++ > > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > > MAINTAINERS | 6 + > > drivers/iio/chemical/Kconfig | 10 + > > drivers/iio/chemical/Makefile | 1 + > > drivers/iio/chemical/sunrise.c | 310 ++++++++++++++++++ > > 6 files changed, 380 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml > > create mode 100644 drivers/iio/chemical/sunrise.c > > > > -- > > 2.32.0 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-29 16:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-17 15:49 [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Jacopo Mondi 2021-08-17 15:49 ` [PATCH 1/2] dt-bindings: iio: chemical: Document senseair,sunrise " Jacopo Mondi 2021-08-17 16:50 ` Andy Shevchenko 2021-08-18 7:29 ` Jacopo Mondi 2021-08-18 16:47 ` Rob Herring 2021-08-18 16:49 ` Rob Herring 2021-08-17 15:49 ` [PATCH 2/2] iio: chemical: Add Senseair Sunrise 006-0-007 driver Jacopo Mondi 2021-08-17 16:58 ` Andy Shevchenko 2021-08-18 8:02 ` Jacopo Mondi 2021-08-18 9:29 ` Andy Shevchenko 2021-08-29 16:05 ` Jonathan Cameron 2021-08-17 17:01 ` [PATCH 0/2] iio: chemical: Add Senseair Sunrise CO2 sensor Matt Ranostay 2021-08-18 8:03 ` Jacopo Mondi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.