Hello, some quick comments on this driver below
I think documentation is missing and the ABI is a bit problematic and unusual > Support Sensirion SGP30 and SGPC3 multi-pixel I2C gas sensors generally, we tend to avoid wildcard driver names; sgp30 would be preferred over sgpxx > Supported Features: > > * Indoor Air Quality (IAQ) concentrations for > SGP30 and SGPC3: > - tVOC (in_concentration_voc_input) > SGP30 only: > - CO2eq (in_concentration_co2_input) > IAQ must first be initialized by writing a non-empty value to > out_iaq_init. After initializing IAQ, at least one IAQ signal must > be read out every second (SGP30) / every two seconds (SGPC3) for the > sensor to correctly maintain its internal baseline > * Baseline support for IAQ (in_iaq_baseline, out_iaq_baseline) > * Gas concentration signals for > SGP30 and SGPC3: > - Ethanol (in_concentration_ethanol_raw) > SGP30 only: > - H2 (in_concentration_h2_raw) > * On-chip self test (in_selftest) > The self test interferes with IAQ operations. If needed, first > retrieve the current baseline, then reset it after the self test > * Sensor interface version (in_feature_set_version) > * Sensor serial number (in_serial_id) > * Humidity compensation for SGP30 > With the help of a humidity signal, the gas signals can be > humidity-compensated. > * Checksummed I2C communication > For all features, refer to the data sheet or the documentation in > Documentation/iio/chemical/sgpxx.txt for more details. may some brief TODOs; heat controller? > Signed-off-by: Andreas Brauchli <andreas.brauc...@sensirion.com> > --- > Documentation/iio/chemical/sgpxx.txt | 112 +++++ > drivers/iio/chemical/Kconfig | 13 + > drivers/iio/chemical/Makefile | 1 + > drivers/iio/chemical/sgpxx.c | 894 > +++++++++++++++++++++++++++++++++++ > 4 files changed, 1020 insertions(+) > create mode 100644 Documentation/iio/chemical/sgpxx.txt > create mode 100644 drivers/iio/chemical/sgpxx.c > > diff --git a/Documentation/iio/chemical/sgpxx.txt > b/Documentation/iio/chemical/sgpxx.txt > new file mode 100644 > index 000000000000..f49b2f365df3 > --- /dev/null > +++ b/Documentation/iio/chemical/sgpxx.txt > @@ -0,0 +1,112 @@ > +sgpxx: Industrial IO driver for Sensirion i2c Multi-Pixel Gas Sensors > + > +1. Overview > + > +The sgpxx driver supports the Sensirion SGP30 and SGPC3 multi-pixel gas > sensors. > + > +Datasheets: > +https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGP30_Datasheet_EN.pdf > +https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGPC3_Datasheet_EN.pdf > + > +2. Modes of Operation > + > +2.1. Driver Instantiation > + > +The sgpxx driver must be instantiated on the corresponding i2c bus with the > +product name (sgp30 or sgpc3) and i2c address (0x58). > + > +Example instantiation of an sgp30 on i2c bus 1 (i2c-1): > + > + $ echo sgp30 0x58 | sudo tee /sys/bus/i2c/devices/i2c-1/new_device > + > +Using the wrong product name results in an instantiation error. Check dmesg. I'd rather drop this section, the only specific information is the I2C address > + > +2.2. Indoor Air Quality (IAQ) concentrations > + > +* tVOC (in_concentration_voc_input) at ppb precision (1e-9) > +* CO2eq (in_concentration_co2_input) at ppm precision (1e-6) -- SGP30 only > + > +2.2.1. IAQ Initialization > +Before Indoor Air Quality (IAQ) values can be read, the IAQ mode must be > +initialized by writing a non-empty value to out_iaq_init: > + > + $ echo init > out_iaq_init can't this be done on probe()? in any case, private API should be documented under Documentation/ABI/testing/sysfs-bus-iio-* > +After initializing IAQ, at least one IAQ signal must be read out every second > +(SGP30) / every two seconds (SGPC3) for the sensor to correctly maintain its > +internal baseline: shouldn't the driver do this? > + > + SGP30: > + $ watch -n1 cat in_concentration_voc_input > + > + SGPC3: > + $ watch -n2 cat in_concentration_voc_input > + > +For the first 15s of operation after writing to out_iaq_init, default values > are > +retured by the sensor. typo: returned > + > +2.2.2. Pausing and Resuming IAQ > + > +For best performance and faster startup times, the baseline should be saved > +once every hour, after 12h of operation. The baseline is restored by writing > a > +non-empty value to out_iaq_init, followed by writing an unmodified retrieved > +baseline value from in_iaq_baseline to out_iaq_baseline. the out_ prefix seems inappropriate here, the sensors doesn't output CO2 :) handling calibration data in a generic way is difficult > + > + Saving the baseline: > + $ baseline=$(cat in_iaq_baseline) > + > + Restoring the baseline: > + $ echo init > out_iaq_init > + $ echo -n $baseline > out_iaq_baseline > + > +2.3. Gas Concentration Signals > + > +* Ethanol (in_concentration_ethanol_raw) we'd need a IIO_MOD_ETHANOL? > +* H2 (in_concentration_h2_raw) -- SGP30 only we'd need a IIO_MOD_H2? > + > +The gas signals in_concentration_ethanol_raw and in_concentration_h2_raw may > be > +used without prior write to out_iaq_init. > + > +2.4. Humidity Compensation (SGP30) > + > +The SGP30 features an on-chip humidity compensation that requires the > +(in-device) environment's absolute humidity. > + > +Set the absolute humidity by writing the absolute humidity concentration (in > +mg/m^3) to out_concentration_ah_raw. The absolute humidity is obtained by not sure about the out_ prefix again absolute humidity is new, IIO has relative humidity so far (jus saying) relative humidity is in milli percent in IIO temperature is in milli degree Celsius > +converting the relative humidity and temperature. The following units are > used: > +AH in mg/m^3, RH in percent (0..100), T in degrees Celsius, and exp() being > the > +base-e exponential function. > + > + RH exp(17.62 * T) > + ----- * 6.112 * -------------- > + 100.0 243.12 + T > + AH = 216.7 * ------------------------------- * 1000 > + 273.15 + T > + > +Writing a value of 0 to out_absolute_humidity disables the humidity out_absolute_humidity is the same as out_concentration_ah_raw? > +compensation. > + > +2.5. On-chip self test > + > + $ cat in_selftest > + > +in_selftest returns OK or FAILED. > + > +The self test interferes with IAQ operations. If needed, first save the > current > +baseline, then restore it after the self test: > + > + $ baseline=$(cat in_iaq_baseline) > + $ cat in_selftest > + $ echo init > out_iaq_init > + $ echo -n $baseline > out_iaq_baseline > + > +If the sensor's current operating duration is less than 12h the baseline > should > +not be restored by skipping the last step. > + > +3. Sensor Interface > + > + $ cat in_feature_set_version > + > +The SGP sensors' minor interface (feature set) version guarantees interface > +stability: a sensor with feature set 1.1 works with a driver for feature set > 1.0 really needed? > diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig > index 5cb5be7612b4..4574dd687513 100644 > --- a/drivers/iio/chemical/Kconfig > +++ b/drivers/iio/chemical/Kconfig > @@ -38,6 +38,19 @@ config IAQCORE > iAQ-Core Continuous/Pulsed VOC (Volatile Organic Compounds) > sensors > > +config SENSIRION_SGPXX > + tristate "Sensirion SGPxx gas sensors" > + depends on I2C > + select CRC8 > + help > + Say Y here to build I2C interface support for the following > + Sensirion SGP gas sensors: > + * SGP30 gas sensor > + * SGPC3 gas sensor > + > + To compile this driver as module, choose M here: the > + module will be called sgpxx. > + > 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 a629b29d1e0b..6090a0ae3981 100644 > --- a/drivers/iio/chemical/Makefile > +++ b/drivers/iio/chemical/Makefile > @@ -6,4 +6,5 @@ > obj-$(CONFIG_ATLAS_PH_SENSOR) += atlas-ph-sensor.o > obj-$(CONFIG_CCS811) += ccs811.o > obj-$(CONFIG_IAQCORE) += ams-iaq-core.o > +obj-$(CONFIG_SENSIRION_SGPXX) += sgpxx.o > obj-$(CONFIG_VZ89X) += vz89x.o > diff --git a/drivers/iio/chemical/sgpxx.c b/drivers/iio/chemical/sgpxx.c > new file mode 100644 > index 000000000000..aea55e41d4cc > --- /dev/null > +++ b/drivers/iio/chemical/sgpxx.c > @@ -0,0 +1,894 @@ > +/* > + * sgpxx.c - Support for Sensirion SGP Gas Sensors > + * > + * Copyright (C) 2017 Andreas Brauchli <andreas.brauc...@sensirion.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * Datasheets: > + * > https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGP30_Datasheet_EN.pdf > + * > https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/9_Gas_Sensors/Sensirion_Gas_Sensors_SGPC3_Datasheet_EN.pdf > + */ > + > +#include <linux/crc8.h> > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/init.h> > +#include <linux/i2c.h> > +#include <linux/of_device.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/sysfs.h> > + > +#define SGP_WORD_LEN 2 > +#define SGP_CRC8_POLYNOMIAL 0x31 > +#define SGP_CRC8_INIT 0xff > +#define SGP_CRC8_LEN 1 > +#define SGP_CMD(cmd_word) cpu_to_be16(cmd_word) > +#define SGP_CMD_DURATION_US 50000 > +#define SGP_SELFTEST_DURATION_US 220000 > +#define SGP_CMD_HANDLING_DURATION_US 10000 > +#define SGP_CMD_LEN SGP_WORD_LEN > +#define SGP30_MEASUREMENT_LEN 2 > +#define SGPC3_MEASUREMENT_LEN 2 both _MEASUREMENT_LEN needed, same value? > +#define SGP30_MEASURE_INTERVAL_HZ 1 > +#define SGPC3_MEASURE_INTERVAL_HZ 2 > +#define SGP_SELFTEST_OK 0xd400 > + > +DECLARE_CRC8_TABLE(sgp_crc8_table); > + > +enum sgp_product_id { > + SGP30 = 0, > + SGPC3 > +}; > + > +enum sgp30_channel_idx { > + SGP30_IAQ_TVOC_IDX = 0, > + SGP30_IAQ_CO2EQ_IDX, > + SGP30_SIG_ETOH_IDX, > + SGP30_SIG_H2_IDX, > + SGP30_SET_AH_IDX, > +}; > + > +enum sgpc3_channel_idx { > + SGPC3_IAQ_TVOC_IDX = 10, > + SGPC3_SIG_ETOH_IDX, > +}; > + > +enum sgp_cmd { > + SGP_CMD_IAQ_INIT = SGP_CMD(0x2003), > + SGP_CMD_IAQ_MEASURE = SGP_CMD(0x2008), > + SGP_CMD_GET_BASELINE = SGP_CMD(0x2015), > + SGP_CMD_SET_BASELINE = SGP_CMD(0x201e), > + SGP_CMD_GET_FEATURE_SET = SGP_CMD(0x202f), > + SGP_CMD_GET_SERIAL_ID = SGP_CMD(0x3682), > + SGP_CMD_MEASURE_TEST = SGP_CMD(0x2032), > + > + SGP30_CMD_MEASURE_SIGNAL = SGP_CMD(0x2050), > + SGP30_CMD_SET_ABSOLUTE_HUMIDITY = SGP_CMD(0x2061), > + > + SGPC3_CMD_IAQ_INIT0 = SGP_CMD(0x2089), > + SGPC3_CMD_IAQ_INIT16 = SGP_CMD(0x2024), > + SGPC3_CMD_IAQ_INIT64 = SGP_CMD(0x2003), > + SGPC3_CMD_IAQ_INIT184 = SGP_CMD(0x206a), > + SGPC3_CMD_MEASURE_RAW = SGP_CMD(0x2046), > +}; > + > +enum sgp_measure_mode { > + SGP_MEASURE_MODE_UNKNOWN, > + SGP_MEASURE_MODE_IAQ, > + SGP_MEASURE_MODE_SIGNAL, > + SGP_MEASURE_MODE_ALL, > +}; > + > +struct sgp_version { > + u8 major; > + u8 minor; > +}; > + > +struct sgp_crc_word { > + __be16 value; > + u8 crc8; > +} __attribute__((__packed__)); > + > +union sgp_reading { > + u8 start; > + struct sgp_crc_word raw_words[4]; > +}; > + > +struct sgp_data { > + struct i2c_client *client; > + struct mutex data_lock; /* mutex to lock access to data buffer */ > + struct mutex i2c_lock; /* mutex to lock access to i2c */ > + unsigned long last_update; > + > + u64 serial_id; > + u16 chip_id; > + u16 feature_set; > + u16 measurement_len; > + int measure_interval_hz; > + enum sgp_cmd measure_iaq_cmd; > + enum sgp_cmd measure_signal_cmd; > + enum sgp_measure_mode measure_mode; > + char *baseline_format; > + bool iaq_initialized; > + u8 baseline_len; > + union sgp_reading buffer; > +}; > + > +struct sgp_device { > + const struct iio_chan_spec *channels; > + int num_channels; > +}; > + > +static const struct sgp_version supported_versions_sgp30[] = { > + { > + .major = 1, > + .minor = 0, > + } end with comma (,) so that it can be extended with minimal change > +}; > + > +static const struct sgp_version supported_versions_sgpc3[] = { > + { > + .major = 0, > + .minor = 4, > + } > +}; > + > +static const struct iio_chan_spec sgp30_channels[] = { > + { > + .type = IIO_CONCENTRATION, > + .channel2 = IIO_MOD_VOC, > + .datasheet_name = "TVOC signal", > + .scan_index = 0, scan_index only needed when adding buffer support > + .modified = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + .address = SGP30_IAQ_TVOC_IDX, > + }, > + { > + .type = IIO_CONCENTRATION, > + .channel2 = IIO_MOD_CO2, > + .datasheet_name = "CO2eq signal", > + .scan_index = 1, > + .modified = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + .address = SGP30_IAQ_CO2EQ_IDX, > + }, > + { > + .type = IIO_CONCENTRATION, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > + .address = SGP30_SIG_ETOH_IDX, > + .extend_name = "ethanol", > + .datasheet_name = "Ethanol signal", > + .scan_index = 2, > + .scan_type = { > + .endianness = IIO_BE, scan_type only neededwhen adding buffer support maybe add IIO_MOD_ETHANOL? > + }, > + }, > + { > + .type = IIO_CONCENTRATION, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > + .address = SGP30_SIG_H2_IDX, maybe add IIO_MOD_H2? > + .extend_name = "h2", > + .datasheet_name = "H2 signal", > + .scan_index = 3, > + .scan_type = { > + .endianness = IIO_BE, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(4), > + { > + .type = IIO_CONCENTRATION, > + .address = SGP30_SET_AH_IDX, > + .extend_name = "ah", > + .datasheet_name = "absolute humidty", typo: humidity > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .output = 1, > + .scan_index = 5 > + }, > +}; > + > +static const struct iio_chan_spec sgpc3_channels[] = { > + { > + .type = IIO_CONCENTRATION, > + .channel2 = IIO_MOD_VOC, > + .datasheet_name = "TVOC signal", > + .modified = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + .address = SGPC3_IAQ_TVOC_IDX, > + }, > + { > + .type = IIO_CONCENTRATION, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > + .address = SGPC3_SIG_ETOH_IDX, > + .extend_name = "ethanol", > + .datasheet_name = "Ethanol signal", > + .scan_index = 0, > + .scan_type = { > + .endianness = IIO_BE, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(2), > +}; > + > +static struct sgp_device sgp_devices[] = { const? > + [SGP30] = { > + .channels = sgp30_channels, > + .num_channels = ARRAY_SIZE(sgp30_channels), > + }, > + [SGPC3] = { > + .channels = sgpc3_channels, > + .num_channels = ARRAY_SIZE(sgpc3_channels), > + }, > +}; > + > +/** > + * sgp_verify_buffer() - verify the checksums of the data buffer words > + * > + * @data: SGP data containing the raw buffer > + * @word_count: Num data words stored in the buffer, excluding CRC bytes > + * > + * Return: 0 on success, negative error code otherwise > + */ > +static int sgp_verify_buffer(struct sgp_data *data, size_t word_count) > +{ > + size_t size = word_count * (SGP_WORD_LEN + SGP_CRC8_LEN); > + int i; > + u8 crc; > + u8 *data_buf = &data->buffer.start; > + > + for (i = 0; i < size; i += SGP_WORD_LEN + SGP_CRC8_LEN) { > + crc = crc8(sgp_crc8_table, &data_buf[i], SGP_WORD_LEN, > + SGP_CRC8_INIT); > + if (crc != data_buf[i + SGP_WORD_LEN]) { > + dev_err(&data->client->dev, "CRC error\n"); > + return -EIO; > + } > + } > + return 0; > +} > + > +/** > + * sgp_read_from_cmd() - reads data from SGP sensor after issuing a command > + * The caller must hold data->data_lock for the duration of the call. > + * @data: SGP data > + * @cmd: SGP Command to issue > + * @word_count: Num words to read, excluding CRC bytes > + * > + * Return: 0 on success, negative error otherwise. > + */ > +static int sgp_read_from_cmd(struct sgp_data *data, > + enum sgp_cmd cmd, > + size_t word_count, > + unsigned long duration_us) > +{ > + int ret; > + struct i2c_client *client = data->client; > + size_t size = word_count * (SGP_WORD_LEN + SGP_CRC8_LEN); > + u8 *data_buf = &data->buffer.start; > + > + mutex_lock(&data->i2c_lock); > + ret = i2c_master_send(client, (const char *)&cmd, SGP_CMD_LEN); > + if (ret != SGP_CMD_LEN) { > + mutex_unlock(&data->i2c_lock); > + return -EIO; > + } > + usleep_range(duration_us, duration_us + 1000); > + > + ret = i2c_master_recv(client, data_buf, size); > + mutex_unlock(&data->i2c_lock); > + > + if (ret < 0) > + ret = -ETXTBSY; > + else if (ret != size) > + ret = -EINTR; > + else > + ret = sgp_verify_buffer(data, word_count); > + > + return ret; > +} > + > +/** > + * sgp_i2c_write_from_cmd() - write data to SGP sensor with a command > + * @data: SGP data > + * @cmd: SGP Command to issue > + * @buf: Data to write > + * @buf_size: Data size of the buffer > + * > + * Return: 0 on success, negative error otherwise. > + */ > +static int sgp_write_from_cmd(struct sgp_data *data, > + enum sgp_cmd cmd, > + u16 *buf, > + size_t buf_size, > + unsigned long duration_us) > +{ > + int ret, ix; > + u16 buf_idx = 0; > + u16 buffer_size = SGP_CMD_LEN + buf_size * > + (SGP_WORD_LEN + SGP_CRC8_LEN); > + u8 buffer[buffer_size]; dynamically sized array, allowed? > + > + /* assemble buffer */ > + *((u16 *)&buffer[0]) = cmd; be16 > + buf_idx += SGP_CMD_LEN; > + for (ix = 0; ix < buf_size; ix++) { > + *((u16 *)&buffer[buf_idx]) = ntohs(buf[ix] & 0xffff); use cpu_to_be16() as everywhere else instead of ntohs() buf is u16, so & 0xffff needed? > + buf_idx += SGP_WORD_LEN; > + buffer[buf_idx] = crc8(sgp_crc8_table, > + &buffer[buf_idx - SGP_WORD_LEN], > + SGP_WORD_LEN, SGP_CRC8_INIT); > + buf_idx += SGP_CRC8_LEN; > + } > + mutex_lock(&data->i2c_lock); > + ret = i2c_master_send(data->client, buffer, buffer_size); > + if (ret != buffer_size) { > + ret = -EIO; > + goto unlock_return_count; > + } > + ret = 0; > + /* Wait inside lock to ensure the chip is ready before next command */ > + usleep_range(duration_us, duration_us + 1000); > + > +unlock_return_count: > + mutex_unlock(&data->i2c_lock); > + return ret; > +} > + > +/** > + * sgp_get_measurement() - retrieve measurement result from sensor > + * The caller must hold data->data_lock for the duration of the call. > + * @data: SGP data > + * @cmd: SGP Command to issue > + * @measure_mode: SGP measurement mode > + * > + * Return: 0 on success, negative error otherwise. > + */ > +static int sgp_get_measurement(struct sgp_data *data, enum sgp_cmd cmd, > + enum sgp_measure_mode measure_mode) > +{ > + int ret; > + > + /* if all channels are measured, we don't need to distinguish between > + * different measure modes > + */ > + if (data->measure_mode == SGP_MEASURE_MODE_ALL) > + measure_mode = SGP_MEASURE_MODE_ALL; > + > + /* Always measure if measure mode changed > + * SGP30 should only be polled once a second > + * SGPC3 should only be polled once every two seconds > + */ > + if (measure_mode == data->measure_mode && > + !time_after(jiffies, > + data->last_update + data->measure_interval_hz * HZ)) { > + return 0; > + } > + > + ret = sgp_read_from_cmd(data, cmd, data->measurement_len, > + SGP_CMD_DURATION_US); > + > + if (ret < 0) > + return ret; > + > + data->measure_mode = measure_mode; > + data->last_update = jiffies; > + > + return 0; > +} > + > +static int sgp_absolute_humidity_store(struct sgp_data *data, > + int val, int val2) > +{ > + u32 ah; > + u16 ah_scaled; > + > + if (val < 0 || val > 256 || (val == 256 && val2 > 0)) > + return -EINVAL; > + > + ah = val * 1000 + val2 / 1000; > + /* ah_scaled = (u16)((ah / 1000.0) * 256.0) */ > + ah_scaled = (u16)(((u64)ah * 256 * 16777) >> 24); > + > + /* ensure we don't disable AH compensation due to rounding */ > + if (ah > 0 && ah_scaled == 0) > + ah_scaled = 1; > + > + return sgp_write_from_cmd(data, SGP30_CMD_SET_ABSOLUTE_HUMIDITY, > + &ah_scaled, 1, SGP_CMD_HANDLING_DURATION_US); > +} > + > +static int sgp_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct sgp_data *data = iio_priv(indio_dev); > + struct sgp_crc_word *words; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + mutex_lock(&data->data_lock); > + if (!data->iaq_initialized) { > + dev_warn(&data->client->dev, > + "IAQ potentially uninitialized\n"); > + } > + ret = sgp_get_measurement(data, data->measure_iaq_cmd, > + SGP_MEASURE_MODE_IAQ); > + if (ret) > + goto unlock_fail; > + words = data->buffer.raw_words; > + switch (chan->address) { > + case SGP30_IAQ_TVOC_IDX: > + case SGPC3_IAQ_TVOC_IDX: > + *val = 0; > + *val2 = be16_to_cpu(words[1].value); > + ret = IIO_VAL_INT_PLUS_NANO; > + break; > + case SGP30_IAQ_CO2EQ_IDX: > + *val = 0; > + *val2 = be16_to_cpu(words[0].value); > + ret = IIO_VAL_INT_PLUS_MICRO; > + break; > + default: > + ret = -EINVAL; > + break; > + } > + mutex_unlock(&data->data_lock); > + break; > + case IIO_CHAN_INFO_RAW: > + mutex_lock(&data->data_lock); > + ret = sgp_get_measurement(data, data->measure_signal_cmd, > + SGP_MEASURE_MODE_SIGNAL); > + if (ret) > + goto unlock_fail; > + words = data->buffer.raw_words; > + switch (chan->address) { > + case SGP30_SIG_ETOH_IDX: > + *val = be16_to_cpu(words[1].value); > + ret = IIO_VAL_INT; > + break; > + case SGPC3_SIG_ETOH_IDX: > + case SGP30_SIG_H2_IDX: > + *val = be16_to_cpu(words[0].value); > + ret = IIO_VAL_INT; > + break; > + } > +unlock_fail: > + mutex_unlock(&data->data_lock); > + break; > + case IIO_CHAN_INFO_SCALE: > + switch (chan->address) { > + case SGP30_SIG_ETOH_IDX: > + case SGPC3_SIG_ETOH_IDX: > + case SGP30_SIG_H2_IDX: > + *val = 0; > + *val2 = 1953125; > + ret = IIO_VAL_INT_PLUS_NANO; > + break; > + default: > + ret = -EINVAL; > + break; > + } > + break; > + default: > + ret = -EINVAL; > + } > + return ret; > +} > + > +static int sgp_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > +{ > + struct sgp_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (chan->address) { > + case SGP30_SET_AH_IDX: > + ret = sgp_absolute_humidity_store(data, val, val2); > + break; > + default: > + ret = -EINVAL; > + } > + > + return ret; > +} > + > +static ssize_t sgp_iaq_init_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + u32 init_time; > + enum sgp_cmd cmd; > + int ret; > + > + cmd = SGP_CMD_IAQ_INIT; > + if (data->chip_id == SGPC3) { > + ret = kstrtou32(buf, 10, &init_time); > + > + if (ret) > + return -EINVAL; > + > + switch (init_time) { > + case 0: > + cmd = SGPC3_CMD_IAQ_INIT0; > + break; > + case 16: > + cmd = SGPC3_CMD_IAQ_INIT16; > + break; > + case 64: > + cmd = SGPC3_CMD_IAQ_INIT64; > + break; > + case 184: > + cmd = SGPC3_CMD_IAQ_INIT184; > + break; > + default: > + return -EINVAL; > + } > + } > + > + mutex_lock(&data->data_lock); > + ret = sgp_read_from_cmd(data, cmd, 0, SGP_CMD_DURATION_US); > + > + if (ret < 0) > + goto unlock_fail; > + > + data->iaq_initialized = true; > + mutex_unlock(&data->data_lock); > + return count; > + > +unlock_fail: > + mutex_unlock(&data->data_lock); > + return ret; > +} > + > +static ssize_t sgp_iaq_baseline_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + u32 baseline; > + u16 baseline_word; > + int ret, ix; > + > + mutex_lock(&data->data_lock); > + ret = sgp_read_from_cmd(data, SGP_CMD_GET_BASELINE, data->baseline_len, > + SGP_CMD_DURATION_US); > + > + if (ret < 0) > + goto unlock_fail; > + > + baseline = 0; > + for (ix = 0; ix < data->baseline_len; ix++) { > + baseline_word = be16_to_cpu(data->buffer.raw_words[ix].value); > + baseline |= baseline_word << (16 * ix); > + } > + > + mutex_unlock(&data->data_lock); > + return sprintf(buf, data->baseline_format, baseline); > + > +unlock_fail: > + mutex_unlock(&data->data_lock); > + return ret; > +} > + > +static ssize_t sgp_iaq_baseline_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + int newline = (count > 0 && buf[count - 1] == '\n'); > + u16 words[2]; > + int ret = 0; > + > + /* 1 word (4 chars) per signal */ > + if (count - newline == (data->baseline_len * 4)) { > + if (data->baseline_len == 1) > + ret = sscanf(buf, "%04hx", &words[0]); > + else if (data->baseline_len == 2) > + ret = sscanf(buf, "%04hx%04hx", &words[0], &words[1]); > + else > + return -EIO; > + } > + > + /* Check if baseline format is correct */ > + if (ret != data->baseline_len) { > + dev_err(&data->client->dev, "invalid baseline format\n"); > + return -EIO; > + } > + > + ret = sgp_write_from_cmd(data, SGP_CMD_SET_BASELINE, words, > + data->baseline_len, SGP_CMD_DURATION_US); > + if (ret < 0) > + return -EIO; > + > + return count; > +} > + > +static ssize_t sgp_selftest_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + u16 measure_test; > + int ret; > + > + mutex_lock(&data->data_lock); > + data->iaq_initialized = false; > + ret = sgp_read_from_cmd(data, SGP_CMD_MEASURE_TEST, 1, > + SGP_SELFTEST_DURATION_US); > + > + if (ret != 0) > + goto unlock_fail; > + > + measure_test = be16_to_cpu(data->buffer.raw_words[0].value); > + mutex_unlock(&data->data_lock); > + > + return sprintf(buf, "%s\n", > + measure_test ^ SGP_SELFTEST_OK ? "FAILED" : "OK"); > + > +unlock_fail: > + mutex_unlock(&data->data_lock); > + return ret; > +} > + > +static ssize_t sgp_serial_id_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + > + return sprintf(buf, "%llu\n", data->serial_id); > +} > + > +static ssize_t sgp_feature_set_version_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct sgp_data *data = iio_priv(dev_to_iio_dev(dev)); > + > + return sprintf(buf, "%hu.%hu\n", (data->feature_set & 0x00e0) >> 5, > + data->feature_set & 0x001f); > +} > + > +static int sgp_get_serial_id(struct sgp_data *data) > +{ > + int ret; > + struct sgp_crc_word *words; > + > + mutex_lock(&data->data_lock); > + ret = sgp_read_from_cmd(data, SGP_CMD_GET_SERIAL_ID, 3, > + SGP_CMD_DURATION_US); > + if (ret != 0) > + goto unlock_fail; > + > + words = data->buffer.raw_words; > + data->serial_id = (u64)(be16_to_cpu(words[2].value) & 0xffff) | > + (u64)(be16_to_cpu(words[1].value) & 0xffff) << 16 | > + (u64)(be16_to_cpu(words[0].value) & 0xffff) << 32; > + > +unlock_fail: > + mutex_unlock(&data->data_lock); > + return ret; > +} > + > +static int setup_and_check_sgp_data(struct sgp_data *data, > + unsigned int chip_id) > +{ > + u16 minor, major, product, eng, ix, num_fs, reserved; > + struct sgp_version *supported_versions; > + > + product = (data->feature_set & 0xf000) >> 12; > + reserved = (data->feature_set & 0x0e00) >> 9; > + eng = (data->feature_set & 0x0100) >> 8; > + major = (data->feature_set & 0x00e0) >> 5; > + minor = data->feature_set & 0x001f; > + > + /* driver does not match product */ > + if (product != chip_id) { > + dev_err(&data->client->dev, > + "sensor reports a different product: 0x%04hx\n", > + product); > + return -ENODEV; > + } > + > + if (reserved != 0) > + dev_warn(&data->client->dev, "reserved bits set: 0x%04hx\n", > + reserved); > + > + /* engineering samples are not supported */ > + if (eng != 0) > + return -ENODEV; > + > + data->iaq_initialized = false; > + switch (product) { > + case SGP30: > + supported_versions = > + (struct sgp_version *)supported_versions_sgp30; > + num_fs = ARRAY_SIZE(supported_versions_sgp30); > + data->measurement_len = SGP30_MEASUREMENT_LEN; > + data->measure_interval_hz = SGP30_MEASURE_INTERVAL_HZ; > + data->measure_iaq_cmd = SGP_CMD_IAQ_MEASURE; > + data->measure_signal_cmd = SGP30_CMD_MEASURE_SIGNAL; > + data->chip_id = SGP30; > + data->baseline_len = 2; > + data->baseline_format = "%08x\n"; > + data->measure_mode = SGP_MEASURE_MODE_UNKNOWN; > + break; > + case SGPC3: > + supported_versions = > + (struct sgp_version *)supported_versions_sgpc3; > + num_fs = ARRAY_SIZE(supported_versions_sgpc3); > + data->measurement_len = SGPC3_MEASUREMENT_LEN; > + data->measure_interval_hz = SGPC3_MEASURE_INTERVAL_HZ; > + data->measure_iaq_cmd = SGPC3_CMD_MEASURE_RAW; > + data->measure_signal_cmd = SGPC3_CMD_MEASURE_RAW; > + data->chip_id = SGPC3; > + data->baseline_len = 1; > + data->baseline_format = "%04x\n"; > + data->measure_mode = SGP_MEASURE_MODE_ALL; > + break; > + default: > + return -ENODEV; > + }; > + > + for (ix = 0; ix < num_fs; ix++) { > + if (supported_versions[ix].major == major && > + minor >= supported_versions[ix].minor) > + return 0; > + } > + > + dev_err(&data->client->dev, "unsupported sgp version: %d.%d\n", > + major, minor); > + return -ENODEV; > +} > + > +static IIO_DEVICE_ATTR(in_serial_id, 0444, sgp_serial_id_show, NULL, 0); > +static IIO_DEVICE_ATTR(in_feature_set_version, 0444, > + sgp_feature_set_version_show, NULL, 0); > +static IIO_DEVICE_ATTR(in_selftest, 0444, sgp_selftest_show, NULL, 0); > +static IIO_DEVICE_ATTR(out_iaq_init, 0220, NULL, sgp_iaq_init_store, 0); > +static IIO_DEVICE_ATTR(in_iaq_baseline, 0444, sgp_iaq_baseline_show, NULL, > 0); > +static IIO_DEVICE_ATTR(out_iaq_baseline, 0220, NULL, sgp_iaq_baseline_store, > 0); lot of private ABI; all needed? needs documentation... > +static struct attribute *sgp_attributes[] = { > + &iio_dev_attr_in_serial_id.dev_attr.attr, > + &iio_dev_attr_in_feature_set_version.dev_attr.attr, > + &iio_dev_attr_in_selftest.dev_attr.attr, > + &iio_dev_attr_out_iaq_init.dev_attr.attr, > + &iio_dev_attr_in_iaq_baseline.dev_attr.attr, > + &iio_dev_attr_out_iaq_baseline.dev_attr.attr, > + NULL > +}; > + > +static const struct attribute_group sgp_attr_group = { > + .attrs = sgp_attributes, > +}; > + > +static const struct iio_info sgp_info = { > + .attrs = &sgp_attr_group, > + .read_raw = sgp_read_raw, > + .write_raw = sgp_write_raw, > +}; > + > +static const struct of_device_id sgp_dt_ids[] = { > + { .compatible = "sensirion,sgp30", .data = (void *)SGP30 }, > + { .compatible = "sensirion,sgpc3", .data = (void *)SGPC3 }, > + { } > +}; > + > +static int sgp_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct sgp_data *data; > + struct sgp_device *chip; > + const struct of_device_id *of_id; > + unsigned long chip_id; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + of_id = of_match_device(sgp_dt_ids, &client->dev); > + if (!of_id) > + chip_id = id->driver_data; > + else > + chip_id = (unsigned long)of_id->data; > + > + chip = &sgp_devices[chip_id]; > + data = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); > + data->client = client; > + crc8_populate_msb(sgp_crc8_table, SGP_CRC8_POLYNOMIAL); > + mutex_init(&data->data_lock); > + mutex_init(&data->i2c_lock); > + > + /* get serial id and write it to client data */ > + ret = sgp_get_serial_id(data); > + > + if (ret != 0) matter of taste: most drivers just do if (ret) > + return ret; > + > + /* get feature set version and write it to client data */ > + ret = sgp_read_from_cmd(data, SGP_CMD_GET_FEATURE_SET, 1, > + SGP_CMD_DURATION_US); > + if (ret != 0) > + return ret; > + > + data->feature_set = be16_to_cpu(data->buffer.raw_words[0].value); > + > + ret = setup_and_check_sgp_data(data, chip_id); > + if (ret < 0) > + goto fail_free; > + > + /* so initial reading will complete */ > + data->last_update = jiffies - data->measure_interval_hz * HZ; > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->info = &sgp_info; > + indio_dev->name = dev_name(&client->dev); > + indio_dev->modes = INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE; is INDIO_BUFFER_SOFTWARE implemented at this stage? maybe added in followup patch > + > + indio_dev->channels = chip->channels; > + indio_dev->num_channels = chip->num_channels; > + > + ret = devm_iio_device_register(&client->dev, indio_dev); > + if (!ret) > + return ret; > + > + dev_err(&client->dev, "failed to register iio device\n"); really message needed? > + > +fail_free: > + mutex_destroy(&data->i2c_lock); > + mutex_destroy(&data->data_lock); no need to explicitly destroy mutex > + iio_device_free(indio_dev); no need to explicitly free devm_ stuff > + return ret; > +} > + > +static int sgp_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + devm_iio_device_unregister(&client->dev, indio_dev); not needed > + return 0; > +} > + > +static const struct i2c_device_id sgp_id[] = { > + { "sgp30", SGP30 }, > + { "sgpc3", SGPC3 }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, sgp_id); > +MODULE_DEVICE_TABLE(of, sgp_dt_ids); > + > +static struct i2c_driver sgp_driver = { > + .driver = { > + .name = "sgpxx", > + .of_match_table = of_match_ptr(sgp_dt_ids), > + }, > + .probe = sgp_probe, > + .remove = sgp_remove, > + .id_table = sgp_id, > +}; > +module_i2c_driver(sgp_driver); > + > +MODULE_AUTHOR("Andreas Brauchli <andreas.brauc...@sensirion.com>"); > +MODULE_AUTHOR("Pascal Sachs <pascal.sa...@sensirion.com>"); > +MODULE_DESCRIPTION("Sensirion SGPxx gas sensors"); > +MODULE_LICENSE("GPL v2"); > +MODULE_VERSION("0.5.0"); > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418
