* [PATCH 0/2] iio: humidity: add support for Sensirion SHTC3 @ 2021-11-14 13:23 Gilles Talis 2021-11-14 13:23 ` [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor Gilles Talis 2021-11-14 13:23 ` [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor Gilles Talis 0 siblings, 2 replies; 8+ messages in thread From: Gilles Talis @ 2021-11-14 13:23 UTC (permalink / raw) To: jic23, lars, pmeerw, robh+dt, linux-iio Cc: devicetree, linux-kernel, Gilles Talis Add the dt-bindings and driver for the Sensirion SHTC3 humidity sensor Gilles Talis (2): iio: humidity: Add support for Sensirion SHTC3 sensor dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor .../devicetree/bindings/trivial-devices.yaml | 2 + drivers/iio/humidity/Kconfig | 11 + drivers/iio/humidity/Makefile | 1 + drivers/iio/humidity/shtc3.c | 286 ++++++++++++++++++ 4 files changed, 300 insertions(+) create mode 100644 drivers/iio/humidity/shtc3.c -- 2.27.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor 2021-11-14 13:23 [PATCH 0/2] iio: humidity: add support for Sensirion SHTC3 Gilles Talis @ 2021-11-14 13:23 ` Gilles Talis 2021-11-14 13:47 ` Lars-Peter Clausen 2021-11-15 3:08 ` kernel test robot 2021-11-14 13:23 ` [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor Gilles Talis 1 sibling, 2 replies; 8+ messages in thread From: Gilles Talis @ 2021-11-14 13:23 UTC (permalink / raw) To: jic23, lars, pmeerw, robh+dt, linux-iio Cc: devicetree, linux-kernel, Gilles Talis The SHTC3 is a digital humidity and temperature sensor. It covers humidity measurement range from 0 to 100% relative humidity and temperature measurement range from -45 to 125 deg C. Datasheet: https://www.sensirion.com/file/datasheet_shtc3 Signed-off-by: Gilles Talis <gilles.talis@gmail.com> --- drivers/iio/humidity/Kconfig | 11 ++ drivers/iio/humidity/Makefile | 1 + drivers/iio/humidity/shtc3.c | 286 ++++++++++++++++++++++++++++++++++ 3 files changed, 298 insertions(+) create mode 100644 drivers/iio/humidity/shtc3.c diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig index 2de5494e7c22..7aab6141c64b 100644 --- a/drivers/iio/humidity/Kconfig +++ b/drivers/iio/humidity/Kconfig @@ -98,6 +98,17 @@ config HTU21 This driver can also be built as a module. If so, the module will be called htu21. +config SHTC3 + tristate "Sensirion SHTC3 humidity and temperature sensor" + depends on I2C + select CRC8 + help + Say yes here to build support for the Sensirion SHTC3 + humidity and temperature sensor. + + To compile this driver as a module, choose M here: the module + will be called shtc3. + config SI7005 tristate "SI7005 relative humidity and temperature sensor" depends on I2C diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile index f19ff3de97c5..13020dfad1b3 100644 --- a/drivers/iio/humidity/Makefile +++ b/drivers/iio/humidity/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_HTS221_I2C) += hts221_i2c.o obj-$(CONFIG_HTS221_SPI) += hts221_spi.o obj-$(CONFIG_HTU21) += htu21.o +obj-$(CONFIG_SHTC3) += shtc3.o obj-$(CONFIG_SI7005) += si7005.o obj-$(CONFIG_SI7020) += si7020.o diff --git a/drivers/iio/humidity/shtc3.c b/drivers/iio/humidity/shtc3.c new file mode 100644 index 000000000000..ec3d7215e378 --- /dev/null +++ b/drivers/iio/humidity/shtc3.c @@ -0,0 +1,286 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Sensirion SHTC3 Humidity and Temperature Sensor + * + * Copyright (c) 2021 Gilles Talis <gilles.talis@gmail.com> + * + * Datasheet: https://www.sensirion.com/file/datasheet_shtc3 + * + * I2C slave address: 0x70 + */ + +#include <linux/crc8.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> + +#include <linux/iio/iio.h> + +#define SHTC3_CMD(cmd_word) cpu_to_be16(cmd_word) +#define SHTC3_CMD_LEN 2 + +#define SHTC3_ID_MASK 0x083F +#define SHTC3_ID 0x0807 + +#define SHTC3_CRC8_POLYNOMIAL 0x31 + +enum shtc3_cmd { + SHTC3_CMD_GET_ID = SHTC3_CMD(0xEFC8), + SHTC3_CMD_SOFT_RESET = SHTC3_CMD(0x805D), + SHTC3_CMD_SLEEP = SHTC3_CMD(0xB098), + SHTC3_CMD_WAKEUP = SHTC3_CMD(0x3517), + /* + * Run measurement, low-power mode, clock stretching + * temperature first + */ + SHTC3_CMD_TEMP_MEAS_LP_CS = SHTC3_CMD(0x6458), + /* + * Run measurement, low-power mode, clock stretching + * relative humidity first + */ + SHTC3_CMD_RH_MEAS_LP_CS = SHTC3_CMD(0x44DE), +}; + +DECLARE_CRC8_TABLE(shtc3_crc8_tbl); + +struct shtc3_rx_data { + __be16 data; + u8 crc; +} __packed; + +static int shtc3_send_cmd(struct i2c_client *client, u16 cmd, u16 *data) +{ + int ret; + struct shtc3_rx_data rx_data; + u8 crc; + + ret = i2c_master_send(client, (const char *)&cmd, SHTC3_CMD_LEN); + if (ret != SHTC3_CMD_LEN) + return -EIO; + + /* + * This is used to read temperature and humidity measurements + * as well as the sensor ID. + * Sensor sends 2 bytes of data followed by one byte of CRC + */ + if (data) { + ret = i2c_master_recv(client, (u8 *) &rx_data, + sizeof(struct shtc3_rx_data)); + if (ret < 0) + return ret; + if (ret != sizeof(struct shtc3_rx_data)) + return -EIO; + + crc = crc8(shtc3_crc8_tbl, (u8 *)&rx_data.data, + 2, CRC8_INIT_VALUE); + if (crc != rx_data.crc) + return -EIO; + + *data = be16_to_cpu(rx_data.data); + } + + return 0; +} + +static int shtc3_sleep(struct i2c_client *client) +{ + return shtc3_send_cmd(client, SHTC3_CMD_SLEEP, 0); +} + +static int shtc3_wakeup(struct i2c_client *client) +{ + if (shtc3_send_cmd(client, SHTC3_CMD_WAKEUP, 0) < 0) + return -EIO; + + /* Wait for device to wake up */ + usleep_range(180, 240); + + return 0; +} + +static int shtc3_read_channel(struct i2c_client *client, bool temp) +{ + int ret; + u16 cmd; + u16 meas; + + ret = shtc3_wakeup(client); + if (ret < 0) + return ret; + + /* + * Sensor sends back measurement results after measurement command + * has been issued by the host. + * Sensor sends 3 bytes (2 bytes of data + 1 byte of CRC) for each + * channel sequentially + * The command issued by the host determines the channel for which + * the sensor will first send the data. + * We select the channel for which we need the results + * then only read back the 2 bytes corresponding to this channel. + */ + cmd = temp ? SHTC3_CMD_TEMP_MEAS_LP_CS : SHTC3_CMD_RH_MEAS_LP_CS; + ret = shtc3_send_cmd(client, cmd, &meas); + if (ret < 0) + return ret; + + /* Go back to sleep */ + shtc3_sleep(client); + + return meas; +} + +static int shtc3_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct i2c_client **client = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = shtc3_read_channel(*client, (chan->type == IIO_TEMP)); + if (ret < 0) + return ret; + *val = ret; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + if (chan->type == IIO_TEMP) { + *val = 2; + *val2 = 670000; + } else { + *val = 0; + *val2 = 1525; + } + return IIO_VAL_INT_PLUS_MICRO; + case IIO_CHAN_INFO_OFFSET: + *val = -16852; + return IIO_VAL_INT; + default: + break; + } + + return -EINVAL; +} + +static const struct iio_chan_spec shtc3_channels[] = { + { + .type = IIO_HUMIDITYRELATIVE, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + }, + { + .type = IIO_TEMP, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET), + } +}; + +static const struct iio_info shtc3_info = { + .read_raw = shtc3_read_raw, +}; + +static int shtc3_verify_id(struct i2c_client *client) +{ + int ret; + u16 device_id; + u16 reg_val; + + ret = shtc3_send_cmd(client, SHTC3_CMD_GET_ID, ®_val); + if (ret < 0) + return ret; + + device_id = reg_val & SHTC3_ID_MASK; + if (device_id != SHTC3_ID) + return -ENODEV; + + return 0; +} + +static int shtc3_reset(struct i2c_client *client) +{ + int ret; + + ret = shtc3_send_cmd(client, SHTC3_CMD_SOFT_RESET, 0); + if (ret < 0) + return ret; + + /* Wait for device to enter idle state */ + usleep_range(180, 240); + + return 0; +} + +static int shtc3_setup(struct i2c_client *client) +{ + int ret; + + ret = shtc3_verify_id(client); + if (ret < 0) { + dev_err(&client->dev, "SHTC3 not found\n"); + return -ENODEV; + } + + ret = shtc3_reset(client); + if (ret < 0) + return ret; + + return shtc3_sleep(client); +} + +static int shtc3_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct iio_dev *indio_dev; + struct i2c_client **data; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + *data = client; + + indio_dev->name = dev_name(&client->dev); + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &shtc3_info; + indio_dev->channels = shtc3_channels; + indio_dev->num_channels = ARRAY_SIZE(shtc3_channels); + + crc8_populate_msb(shtc3_crc8_tbl, SHTC3_CRC8_POLYNOMIAL); + + ret = shtc3_setup(client); + if (ret < 0) { + dev_err(&client->dev, "SHTC3 setup failed\n"); + return ret; + } + + return devm_iio_device_register(&client->dev, indio_dev); +} + +static const struct i2c_device_id shtc3_id[] = { + { "shtc3", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, shtc3_id); + +static const struct of_device_id shtc3_dt_ids[] = { + { .compatible = "sensirion,shtc3" }, + { } +}; +MODULE_DEVICE_TABLE(of, shtc3_dt_ids); + +static struct i2c_driver shtc3_driver = { + .driver = { + .name = "shtc3", + .of_match_table = shtc3_dt_ids, + }, + .probe = shtc3_probe, + .id_table = shtc3_id, +}; + +module_i2c_driver(shtc3_driver); +MODULE_DESCRIPTION("Sensirion SHTC3 Humidity and Temperature Sensor"); +MODULE_AUTHOR("Gilles Talis <gilles.talis@gmail.com>"); +MODULE_LICENSE("GPL"); -- 2.27.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor 2021-11-14 13:23 ` [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor Gilles Talis @ 2021-11-14 13:47 ` Lars-Peter Clausen [not found] ` <CAKcgs2x7FCf8xBj-PQ62UQNgkxBjFfQznEVCysNO547Yr5Ct3w@mail.gmail.com> 2021-11-15 3:08 ` kernel test robot 1 sibling, 1 reply; 8+ messages in thread From: Lars-Peter Clausen @ 2021-11-14 13:47 UTC (permalink / raw) To: Gilles Talis, jic23, pmeerw, robh+dt, linux-iio; +Cc: devicetree, linux-kernel On 11/14/21 2:23 PM, Gilles Talis wrote: > The SHTC3 is a digital humidity and temperature sensor. > It covers humidity measurement range from 0 to 100% relative humidity > and temperature measurement range from -45 to 125 deg C. > > Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> Hi, Thanks for the path. This looks really good, very well written driver. But we already have support for the shtc3 in the Linux kernel as part of the hwmon framework, see drivers/hwmon/shtc1.c. - Lars > --- > drivers/iio/humidity/Kconfig | 11 ++ > drivers/iio/humidity/Makefile | 1 + > drivers/iio/humidity/shtc3.c | 286 ++++++++++++++++++++++++++++++++++ > 3 files changed, 298 insertions(+) > create mode 100644 drivers/iio/humidity/shtc3.c > > diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig > index 2de5494e7c22..7aab6141c64b 100644 > --- a/drivers/iio/humidity/Kconfig > +++ b/drivers/iio/humidity/Kconfig > @@ -98,6 +98,17 @@ config HTU21 > This driver can also be built as a module. If so, the module will > be called htu21. > > +config SHTC3 > + tristate "Sensirion SHTC3 humidity and temperature sensor" > + depends on I2C > + select CRC8 > + help > + Say yes here to build support for the Sensirion SHTC3 > + humidity and temperature sensor. > + > + To compile this driver as a module, choose M here: the module > + will be called shtc3. > + > config SI7005 > tristate "SI7005 relative humidity and temperature sensor" > depends on I2C > diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile > index f19ff3de97c5..13020dfad1b3 100644 > --- a/drivers/iio/humidity/Makefile > +++ b/drivers/iio/humidity/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_HTS221_I2C) += hts221_i2c.o > obj-$(CONFIG_HTS221_SPI) += hts221_spi.o > > obj-$(CONFIG_HTU21) += htu21.o > +obj-$(CONFIG_SHTC3) += shtc3.o > obj-$(CONFIG_SI7005) += si7005.o > obj-$(CONFIG_SI7020) += si7020.o > > diff --git a/drivers/iio/humidity/shtc3.c b/drivers/iio/humidity/shtc3.c > new file mode 100644 > index 000000000000..ec3d7215e378 > --- /dev/null > +++ b/drivers/iio/humidity/shtc3.c > @@ -0,0 +1,286 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Sensirion SHTC3 Humidity and Temperature Sensor > + * > + * Copyright (c) 2021 Gilles Talis <gilles.talis@gmail.com> > + * > + * Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > + * > + * I2C slave address: 0x70 > + */ > + > +#include <linux/crc8.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > + > +#include <linux/iio/iio.h> > + > +#define SHTC3_CMD(cmd_word) cpu_to_be16(cmd_word) > +#define SHTC3_CMD_LEN 2 > + > +#define SHTC3_ID_MASK 0x083F > +#define SHTC3_ID 0x0807 > + > +#define SHTC3_CRC8_POLYNOMIAL 0x31 > + > +enum shtc3_cmd { > + SHTC3_CMD_GET_ID = SHTC3_CMD(0xEFC8), > + SHTC3_CMD_SOFT_RESET = SHTC3_CMD(0x805D), > + SHTC3_CMD_SLEEP = SHTC3_CMD(0xB098), > + SHTC3_CMD_WAKEUP = SHTC3_CMD(0x3517), > + /* > + * Run measurement, low-power mode, clock stretching > + * temperature first > + */ > + SHTC3_CMD_TEMP_MEAS_LP_CS = SHTC3_CMD(0x6458), > + /* > + * Run measurement, low-power mode, clock stretching > + * relative humidity first > + */ > + SHTC3_CMD_RH_MEAS_LP_CS = SHTC3_CMD(0x44DE), > +}; > + > +DECLARE_CRC8_TABLE(shtc3_crc8_tbl); > + > +struct shtc3_rx_data { > + __be16 data; > + u8 crc; > +} __packed; > + > +static int shtc3_send_cmd(struct i2c_client *client, u16 cmd, u16 *data) > +{ > + int ret; > + struct shtc3_rx_data rx_data; > + u8 crc; > + > + ret = i2c_master_send(client, (const char *)&cmd, SHTC3_CMD_LEN); > + if (ret != SHTC3_CMD_LEN) > + return -EIO; > + > + /* > + * This is used to read temperature and humidity measurements > + * as well as the sensor ID. > + * Sensor sends 2 bytes of data followed by one byte of CRC > + */ > + if (data) { > + ret = i2c_master_recv(client, (u8 *) &rx_data, > + sizeof(struct shtc3_rx_data)); > + if (ret < 0) > + return ret; > + if (ret != sizeof(struct shtc3_rx_data)) > + return -EIO; > + > + crc = crc8(shtc3_crc8_tbl, (u8 *)&rx_data.data, > + 2, CRC8_INIT_VALUE); > + if (crc != rx_data.crc) > + return -EIO; > + > + *data = be16_to_cpu(rx_data.data); > + } > + > + return 0; > +} > + > +static int shtc3_sleep(struct i2c_client *client) > +{ > + return shtc3_send_cmd(client, SHTC3_CMD_SLEEP, 0); > +} > + > +static int shtc3_wakeup(struct i2c_client *client) > +{ > + if (shtc3_send_cmd(client, SHTC3_CMD_WAKEUP, 0) < 0) > + return -EIO; > + > + /* Wait for device to wake up */ > + usleep_range(180, 240); > + > + return 0; > +} > + > +static int shtc3_read_channel(struct i2c_client *client, bool temp) > +{ > + int ret; > + u16 cmd; > + u16 meas; > + > + ret = shtc3_wakeup(client); > + if (ret < 0) > + return ret; > + > + /* > + * Sensor sends back measurement results after measurement command > + * has been issued by the host. > + * Sensor sends 3 bytes (2 bytes of data + 1 byte of CRC) for each > + * channel sequentially > + * The command issued by the host determines the channel for which > + * the sensor will first send the data. > + * We select the channel for which we need the results > + * then only read back the 2 bytes corresponding to this channel. > + */ > + cmd = temp ? SHTC3_CMD_TEMP_MEAS_LP_CS : SHTC3_CMD_RH_MEAS_LP_CS; > + ret = shtc3_send_cmd(client, cmd, &meas); > + if (ret < 0) > + return ret; > + > + /* Go back to sleep */ > + shtc3_sleep(client); > + > + return meas; > +} > + > +static int shtc3_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + struct i2c_client **client = iio_priv(indio_dev); > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = shtc3_read_channel(*client, (chan->type == IIO_TEMP)); > + if (ret < 0) > + return ret; > + *val = ret; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + if (chan->type == IIO_TEMP) { > + *val = 2; > + *val2 = 670000; > + } else { > + *val = 0; > + *val2 = 1525; > + } > + return IIO_VAL_INT_PLUS_MICRO; > + case IIO_CHAN_INFO_OFFSET: > + *val = -16852; > + return IIO_VAL_INT; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static const struct iio_chan_spec shtc3_channels[] = { > + { > + .type = IIO_HUMIDITYRELATIVE, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE), > + }, > + { > + .type = IIO_TEMP, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET), > + } > +}; > + > +static const struct iio_info shtc3_info = { > + .read_raw = shtc3_read_raw, > +}; > + > +static int shtc3_verify_id(struct i2c_client *client) > +{ > + int ret; > + u16 device_id; > + u16 reg_val; > + > + ret = shtc3_send_cmd(client, SHTC3_CMD_GET_ID, ®_val); > + if (ret < 0) > + return ret; > + > + device_id = reg_val & SHTC3_ID_MASK; > + if (device_id != SHTC3_ID) > + return -ENODEV; > + > + return 0; > +} > + > +static int shtc3_reset(struct i2c_client *client) > +{ > + int ret; > + > + ret = shtc3_send_cmd(client, SHTC3_CMD_SOFT_RESET, 0); > + if (ret < 0) > + return ret; > + > + /* Wait for device to enter idle state */ > + usleep_range(180, 240); > + > + return 0; > +} > + > +static int shtc3_setup(struct i2c_client *client) > +{ > + int ret; > + > + ret = shtc3_verify_id(client); > + if (ret < 0) { > + dev_err(&client->dev, "SHTC3 not found\n"); > + return -ENODEV; > + } > + > + ret = shtc3_reset(client); > + if (ret < 0) > + return ret; > + > + return shtc3_sleep(client); > +} > + > +static int shtc3_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct i2c_client **data; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + *data = client; > + > + indio_dev->name = dev_name(&client->dev); > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->info = &shtc3_info; > + indio_dev->channels = shtc3_channels; > + indio_dev->num_channels = ARRAY_SIZE(shtc3_channels); > + > + crc8_populate_msb(shtc3_crc8_tbl, SHTC3_CRC8_POLYNOMIAL); > + > + ret = shtc3_setup(client); > + if (ret < 0) { > + dev_err(&client->dev, "SHTC3 setup failed\n"); > + return ret; > + } > + > + return devm_iio_device_register(&client->dev, indio_dev); > +} > + > +static const struct i2c_device_id shtc3_id[] = { > + { "shtc3", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, shtc3_id); > + > +static const struct of_device_id shtc3_dt_ids[] = { > + { .compatible = "sensirion,shtc3" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, shtc3_dt_ids); > + > +static struct i2c_driver shtc3_driver = { > + .driver = { > + .name = "shtc3", > + .of_match_table = shtc3_dt_ids, > + }, > + .probe = shtc3_probe, > + .id_table = shtc3_id, > +}; > + > +module_i2c_driver(shtc3_driver); > +MODULE_DESCRIPTION("Sensirion SHTC3 Humidity and Temperature Sensor"); > +MODULE_AUTHOR("Gilles Talis <gilles.talis@gmail.com>"); > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAKcgs2x7FCf8xBj-PQ62UQNgkxBjFfQznEVCysNO547Yr5Ct3w@mail.gmail.com>]
* Re: [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor [not found] ` <CAKcgs2x7FCf8xBj-PQ62UQNgkxBjFfQznEVCysNO547Yr5Ct3w@mail.gmail.com> @ 2021-11-14 16:49 ` Jonathan Cameron 2021-11-15 7:29 ` Gilles Talis 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Cameron @ 2021-11-14 16:49 UTC (permalink / raw) To: Gilles Talis Cc: Lars-Peter Clausen, pmeerw, robh+dt, linux-iio, devicetree, linux-kernel On Sun, 14 Nov 2021 15:58:15 +0100 Gilles Talis <gilles.talis@gmail.com> wrote: > Hi Lars, > > Le dim. 14 nov. 2021 à 14:47, Lars-Peter Clausen <lars@metafoo.de> a écrit : > > > On 11/14/21 2:23 PM, Gilles Talis wrote: > > > The SHTC3 is a digital humidity and temperature sensor. > > > It covers humidity measurement range from 0 to 100% relative humidity > > > and temperature measurement range from -45 to 125 deg C. > > > > > > Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > > > > > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> > > > > Hi, > > > > Thanks for the path. This looks really good, very well written driver. > > > > But we already have support for the shtc3 in the Linux kernel as part of > > the hwmon framework, see drivers/hwmon/shtc1.c. > > > > - Lars > > > Thanks for the review. Oops, I should have been more careful. Next time, I > will try to be. > Sorry for the spamming. The fun question of whether humidity drivers should be in IIO or HWMON was my fault many years ago. Pre IIO being anywhere near ready for mainline (and mostly a concept rather than code) I wanted to get at least one of the drivers I was working with upstream and the characteristics of humidity drivers were rather different from ADCs and Accelerometers etc so I asked if Humidty counted as hardware monitoring and got something like "sure it could be" as a reply so upstreamed one driver in hwmon (sht15). Ever since it's been a bit random on where humidity drivers end up based on who is submitting them and their usecase + most similar drivers already upstream. Sorry you fell into this historical quirk! I took a quick look and agree with Lars: nice clean driver, but as you can guess we don't really want 2 drivers and there isn't a strong reason to propose moving this one between subsystems. Thanks, Jonathan > > Thanks! > Gilles. > > > > > > > > --- > > > drivers/iio/humidity/Kconfig | 11 ++ > > > drivers/iio/humidity/Makefile | 1 + > > > drivers/iio/humidity/shtc3.c | 286 ++++++++++++++++++++++++++++++++++ > > > 3 files changed, 298 insertions(+) > > > create mode 100644 drivers/iio/humidity/shtc3.c > > > > > > diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig > > > index 2de5494e7c22..7aab6141c64b 100644 > > > --- a/drivers/iio/humidity/Kconfig > > > +++ b/drivers/iio/humidity/Kconfig > > > @@ -98,6 +98,17 @@ config HTU21 > > > This driver can also be built as a module. If so, the module will > > > be called htu21. > > > > > > +config SHTC3 > > > + tristate "Sensirion SHTC3 humidity and temperature sensor" > > > + depends on I2C > > > + select CRC8 > > > + help > > > + Say yes here to build support for the Sensirion SHTC3 > > > + humidity and temperature sensor. > > > + > > > + To compile this driver as a module, choose M here: the module > > > + will be called shtc3. > > > + > > > config SI7005 > > > tristate "SI7005 relative humidity and temperature sensor" > > > depends on I2C > > > diff --git a/drivers/iio/humidity/Makefile > > b/drivers/iio/humidity/Makefile > > > index f19ff3de97c5..13020dfad1b3 100644 > > > --- a/drivers/iio/humidity/Makefile > > > +++ b/drivers/iio/humidity/Makefile > > > @@ -16,6 +16,7 @@ obj-$(CONFIG_HTS221_I2C) += hts221_i2c.o > > > obj-$(CONFIG_HTS221_SPI) += hts221_spi.o > > > > > > obj-$(CONFIG_HTU21) += htu21.o > > > +obj-$(CONFIG_SHTC3) += shtc3.o > > > obj-$(CONFIG_SI7005) += si7005.o > > > obj-$(CONFIG_SI7020) += si7020.o > > > > > > diff --git a/drivers/iio/humidity/shtc3.c b/drivers/iio/humidity/shtc3.c > > > new file mode 100644 > > > index 000000000000..ec3d7215e378 > > > --- /dev/null > > > +++ b/drivers/iio/humidity/shtc3.c > > > @@ -0,0 +1,286 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Sensirion SHTC3 Humidity and Temperature Sensor > > > + * > > > + * Copyright (c) 2021 Gilles Talis <gilles.talis@gmail.com> > > > + * > > > + * Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > > > + * > > > + * I2C slave address: 0x70 > > > + */ > > > + > > > +#include <linux/crc8.h> > > > +#include <linux/delay.h> > > > +#include <linux/i2c.h> > > > +#include <linux/module.h> > > > +#include <linux/mod_devicetable.h> > > > + > > > +#include <linux/iio/iio.h> > > > + > > > +#define SHTC3_CMD(cmd_word) cpu_to_be16(cmd_word) > > > +#define SHTC3_CMD_LEN 2 > > > + > > > +#define SHTC3_ID_MASK 0x083F > > > +#define SHTC3_ID 0x0807 > > > + > > > +#define SHTC3_CRC8_POLYNOMIAL 0x31 > > > + > > > +enum shtc3_cmd { > > > + SHTC3_CMD_GET_ID = SHTC3_CMD(0xEFC8), > > > + SHTC3_CMD_SOFT_RESET = SHTC3_CMD(0x805D), > > > + SHTC3_CMD_SLEEP = SHTC3_CMD(0xB098), > > > + SHTC3_CMD_WAKEUP = SHTC3_CMD(0x3517), > > > + /* > > > + * Run measurement, low-power mode, clock stretching > > > + * temperature first > > > + */ > > > + SHTC3_CMD_TEMP_MEAS_LP_CS = SHTC3_CMD(0x6458), > > > + /* > > > + * Run measurement, low-power mode, clock stretching > > > + * relative humidity first > > > + */ > > > + SHTC3_CMD_RH_MEAS_LP_CS = SHTC3_CMD(0x44DE), > > > +}; > > > + > > > +DECLARE_CRC8_TABLE(shtc3_crc8_tbl); > > > + > > > +struct shtc3_rx_data { > > > + __be16 data; > > > + u8 crc; > > > +} __packed; > > > + > > > +static int shtc3_send_cmd(struct i2c_client *client, u16 cmd, u16 *data) > > > +{ > > > + int ret; > > > + struct shtc3_rx_data rx_data; > > > + u8 crc; > > > + > > > + ret = i2c_master_send(client, (const char *)&cmd, SHTC3_CMD_LEN); > > > + if (ret != SHTC3_CMD_LEN) That might eat another error, so convention is to check ret < 0 first and then check the length. That way you can return a more informative error if there is one. I see you do that on the recv below. > > > + return -EIO; > > > + > > > + /* > > > + * This is used to read temperature and humidity measurements > > > + * as well as the sensor ID. > > > + * Sensor sends 2 bytes of data followed by one byte of CRC > > > + */ > > > + if (data) { > > > + ret = i2c_master_recv(client, (u8 *) &rx_data, > > > + sizeof(struct shtc3_rx_data)); > > > + if (ret < 0) > > > + return ret; > > > + if (ret != sizeof(struct shtc3_rx_data)) > > > + return -EIO; > > > + > > > + crc = crc8(shtc3_crc8_tbl, (u8 *)&rx_data.data, > > > + 2, CRC8_INIT_VALUE); > > > + if (crc != rx_data.crc) > > > + return -EIO; > > > + > > > + *data = be16_to_cpu(rx_data.data); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int shtc3_sleep(struct i2c_client *client) > > > +{ > > > + return shtc3_send_cmd(client, SHTC3_CMD_SLEEP, 0); > > > +} > > > + > > > +static int shtc3_wakeup(struct i2c_client *client) > > > +{ > > > + if (shtc3_send_cmd(client, SHTC3_CMD_WAKEUP, 0) < 0) > > > + return -EIO; > > > + > > > + /* Wait for device to wake up */ > > > + usleep_range(180, 240); > > > + > > > + return 0; > > > +} > > > + > > > +static int shtc3_read_channel(struct i2c_client *client, bool temp) > > > +{ > > > + int ret; > > > + u16 cmd; > > > + u16 meas; > > > + > > > + ret = shtc3_wakeup(client); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* > > > + * Sensor sends back measurement results after measurement command > > > + * has been issued by the host. > > > + * Sensor sends 3 bytes (2 bytes of data + 1 byte of CRC) for each > > > + * channel sequentially > > > + * The command issued by the host determines the channel for which > > > + * the sensor will first send the data. > > > + * We select the channel for which we need the results > > > + * then only read back the 2 bytes corresponding to this channel. > > > + */ > > > + cmd = temp ? SHTC3_CMD_TEMP_MEAS_LP_CS : SHTC3_CMD_RH_MEAS_LP_CS; > > > + ret = shtc3_send_cmd(client, cmd, &meas); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* Go back to sleep */ > > > + shtc3_sleep(client); > > > + > > > + return meas; > > > +} > > > + > > > +static int shtc3_read_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, int *val, > > > + int *val2, long mask) > > > +{ > > > + struct i2c_client **client = iio_priv(indio_dev); > > > + int ret; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_RAW: > > > + ret = shtc3_read_channel(*client, (chan->type == > > IIO_TEMP)); > > > + if (ret < 0) > > > + return ret; > > > + *val = ret; > > > + return IIO_VAL_INT; > > > + case IIO_CHAN_INFO_SCALE: > > > + if (chan->type == IIO_TEMP) { > > > + *val = 2; > > > + *val2 = 670000; > > > + } else { > > > + *val = 0; > > > + *val2 = 1525; > > > + } > > > + return IIO_VAL_INT_PLUS_MICRO; > > > + case IIO_CHAN_INFO_OFFSET: > > > + *val = -16852; > > > + return IIO_VAL_INT; > > > + default: > > > + break; > > > + } > > > + > > > + return -EINVAL; I'd move this into the default: statement as saves a few lines and makes it slightly easier to read. > > > +} > > > + > > > +static const struct iio_chan_spec shtc3_channels[] = { > > > + { > > > + .type = IIO_HUMIDITYRELATIVE, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > + BIT(IIO_CHAN_INFO_SCALE), > > > + }, > > > + { > > > + .type = IIO_TEMP, > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > + BIT(IIO_CHAN_INFO_SCALE) | > > BIT(IIO_CHAN_INFO_OFFSET), > > > + } > > > +}; > > > + > > > +static const struct iio_info shtc3_info = { > > > + .read_raw = shtc3_read_raw, > > > +}; > > > + > > > +static int shtc3_verify_id(struct i2c_client *client) > > > +{ > > > + int ret; > > > + u16 device_id; > > > + u16 reg_val; > > > + > > > + ret = shtc3_send_cmd(client, SHTC3_CMD_GET_ID, ®_val); > > > + if (ret < 0) > > > + return ret; > > > + > > > + device_id = reg_val & SHTC3_ID_MASK; > > > + if (device_id != SHTC3_ID) > > > + return -ENODEV; > > > + > > > + return 0; > > > +} > > > + > > > +static int shtc3_reset(struct i2c_client *client) > > > +{ > > > + int ret; > > > + > > > + ret = shtc3_send_cmd(client, SHTC3_CMD_SOFT_RESET, 0); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* Wait for device to enter idle state */ > > > + usleep_range(180, 240); > > > + > > > + return 0; > > > +} > > > + > > > +static int shtc3_setup(struct i2c_client *client) > > > +{ > > > + int ret; > > > + > > > + ret = shtc3_verify_id(client); > > > + if (ret < 0) { > > > + dev_err(&client->dev, "SHTC3 not found\n"); > > > + return -ENODEV; > > > + } > > > + > > > + ret = shtc3_reset(client); > > > + if (ret < 0) > > > + return ret; > > > + > > > + return shtc3_sleep(client); > > > +} > > > + > > > +static int shtc3_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + struct iio_dev *indio_dev; > > > + struct i2c_client **data; > > > + int ret; > > > + > > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > > + if (!indio_dev) > > > + return -ENOMEM; > > > + > > > + data = iio_priv(indio_dev); > > > + *data = client; It's normally better to put a structure in there because the chances of a later change requiring more data is rather high... Obviously for now it will only have one element so will end up compiling to pretty much the same you have here. > > > + > > > + indio_dev->name = dev_name(&client->dev); > > > + indio_dev->modes = INDIO_DIRECT_MODE; > > > + indio_dev->info = &shtc3_info; > > > + indio_dev->channels = shtc3_channels; > > > + indio_dev->num_channels = ARRAY_SIZE(shtc3_channels); > > > + > > > + crc8_populate_msb(shtc3_crc8_tbl, SHTC3_CRC8_POLYNOMIAL); > > > + > > > + ret = shtc3_setup(client); > > > + if (ret < 0) { > > > + dev_err(&client->dev, "SHTC3 setup failed\n"); > > > + return ret; > > > + } > > > + > > > + return devm_iio_device_register(&client->dev, indio_dev); > > > +} > > > + > > > +static const struct i2c_device_id shtc3_id[] = { > > > + { "shtc3", 0 }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(i2c, shtc3_id); > > > + > > > +static const struct of_device_id shtc3_dt_ids[] = { > > > + { .compatible = "sensirion,shtc3" }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, shtc3_dt_ids); > > > + > > > +static struct i2c_driver shtc3_driver = { > > > + .driver = { > > > + .name = "shtc3", > > > + .of_match_table = shtc3_dt_ids, > > > + }, > > > + .probe = shtc3_probe, > > > + .id_table = shtc3_id, > > > +}; > > > + > > > +module_i2c_driver(shtc3_driver); > > > +MODULE_DESCRIPTION("Sensirion SHTC3 Humidity and Temperature Sensor"); > > > +MODULE_AUTHOR("Gilles Talis <gilles.talis@gmail.com>"); > > > +MODULE_LICENSE("GPL"); > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor 2021-11-14 16:49 ` Jonathan Cameron @ 2021-11-15 7:29 ` Gilles Talis 0 siblings, 0 replies; 8+ messages in thread From: Gilles Talis @ 2021-11-15 7:29 UTC (permalink / raw) To: Jonathan Cameron Cc: Lars-Peter Clausen, pmeerw, robh+dt, linux-iio, devicetree, linux-kernel Hi Jonathan, Le dim. 14 nov. 2021 à 17:44, Jonathan Cameron <jic23@kernel.org> a écrit : > > On Sun, 14 Nov 2021 15:58:15 +0100 > Gilles Talis <gilles.talis@gmail.com> wrote: > > > Hi Lars, > > > > Le dim. 14 nov. 2021 à 14:47, Lars-Peter Clausen <lars@metafoo.de> a écrit : > > > > > On 11/14/21 2:23 PM, Gilles Talis wrote: > > > > The SHTC3 is a digital humidity and temperature sensor. > > > > It covers humidity measurement range from 0 to 100% relative humidity > > > > and temperature measurement range from -45 to 125 deg C. > > > > > > > > Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > > > > > > > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> > > > > > > Hi, > > > > > > Thanks for the path. This looks really good, very well written driver. > > > > > > But we already have support for the shtc3 in the Linux kernel as part of > > > the hwmon framework, see drivers/hwmon/shtc1.c. > > > > > > - Lars > > > > > Thanks for the review. Oops, I should have been more careful. Next time, I > > will try to be. > > Sorry for the spamming. > > The fun question of whether humidity drivers should be in IIO or HWMON was my > fault many years ago. Pre IIO being anywhere near ready for mainline (and mostly > a concept rather than code) I wanted to get at least one of the drivers I was > working with upstream and the characteristics of humidity drivers were rather > different from ADCs and Accelerometers etc so I asked if Humidty counted as > hardware monitoring and got something like "sure it could be" as a reply so > upstreamed one driver in hwmon (sht15). > > Ever since it's been a bit random on where humidity drivers end up based on > who is submitting them and their usecase + most similar drivers already upstream. > > Sorry you fell into this historical quirk! No problem. Thanks for taking the time to explain the history. Next time, I will make sure I also look in other subsystems before submitting such a driver. > > I took a quick look and agree with Lars: nice clean driver, but as you can guess > we don't really want 2 drivers and there isn't a strong reason to propose > moving this one between subsystems. This obviously makes no sense having 2 drivers for this device. Thanks for the review and the kind words. > > Thanks, > > Jonathan Thanks, Gilles. > > > > > Thanks! > > Gilles. > > > > > > > > > > > > > --- > > > > drivers/iio/humidity/Kconfig | 11 ++ > > > > drivers/iio/humidity/Makefile | 1 + > > > > drivers/iio/humidity/shtc3.c | 286 ++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 298 insertions(+) > > > > create mode 100644 drivers/iio/humidity/shtc3.c > > > > > > > > diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig > > > > index 2de5494e7c22..7aab6141c64b 100644 > > > > --- a/drivers/iio/humidity/Kconfig > > > > +++ b/drivers/iio/humidity/Kconfig > > > > @@ -98,6 +98,17 @@ config HTU21 > > > > This driver can also be built as a module. If so, the module will > > > > be called htu21. > > > > > > > > +config SHTC3 > > > > + tristate "Sensirion SHTC3 humidity and temperature sensor" > > > > + depends on I2C > > > > + select CRC8 > > > > + help > > > > + Say yes here to build support for the Sensirion SHTC3 > > > > + humidity and temperature sensor. > > > > + > > > > + To compile this driver as a module, choose M here: the module > > > > + will be called shtc3. > > > > + > > > > config SI7005 > > > > tristate "SI7005 relative humidity and temperature sensor" > > > > depends on I2C > > > > diff --git a/drivers/iio/humidity/Makefile > > > b/drivers/iio/humidity/Makefile > > > > index f19ff3de97c5..13020dfad1b3 100644 > > > > --- a/drivers/iio/humidity/Makefile > > > > +++ b/drivers/iio/humidity/Makefile > > > > @@ -16,6 +16,7 @@ obj-$(CONFIG_HTS221_I2C) += hts221_i2c.o > > > > obj-$(CONFIG_HTS221_SPI) += hts221_spi.o > > > > > > > > obj-$(CONFIG_HTU21) += htu21.o > > > > +obj-$(CONFIG_SHTC3) += shtc3.o > > > > obj-$(CONFIG_SI7005) += si7005.o > > > > obj-$(CONFIG_SI7020) += si7020.o > > > > > > > > diff --git a/drivers/iio/humidity/shtc3.c b/drivers/iio/humidity/shtc3.c > > > > new file mode 100644 > > > > index 000000000000..ec3d7215e378 > > > > --- /dev/null > > > > +++ b/drivers/iio/humidity/shtc3.c > > > > @@ -0,0 +1,286 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * Sensirion SHTC3 Humidity and Temperature Sensor > > > > + * > > > > + * Copyright (c) 2021 Gilles Talis <gilles.talis@gmail.com> > > > > + * > > > > + * Datasheet: https://www.sensirion.com/file/datasheet_shtc3 > > > > + * > > > > + * I2C slave address: 0x70 > > > > + */ > > > > + > > > > +#include <linux/crc8.h> > > > > +#include <linux/delay.h> > > > > +#include <linux/i2c.h> > > > > +#include <linux/module.h> > > > > +#include <linux/mod_devicetable.h> > > > > + > > > > +#include <linux/iio/iio.h> > > > > + > > > > +#define SHTC3_CMD(cmd_word) cpu_to_be16(cmd_word) > > > > +#define SHTC3_CMD_LEN 2 > > > > + > > > > +#define SHTC3_ID_MASK 0x083F > > > > +#define SHTC3_ID 0x0807 > > > > + > > > > +#define SHTC3_CRC8_POLYNOMIAL 0x31 > > > > + > > > > +enum shtc3_cmd { > > > > + SHTC3_CMD_GET_ID = SHTC3_CMD(0xEFC8), > > > > + SHTC3_CMD_SOFT_RESET = SHTC3_CMD(0x805D), > > > > + SHTC3_CMD_SLEEP = SHTC3_CMD(0xB098), > > > > + SHTC3_CMD_WAKEUP = SHTC3_CMD(0x3517), > > > > + /* > > > > + * Run measurement, low-power mode, clock stretching > > > > + * temperature first > > > > + */ > > > > + SHTC3_CMD_TEMP_MEAS_LP_CS = SHTC3_CMD(0x6458), > > > > + /* > > > > + * Run measurement, low-power mode, clock stretching > > > > + * relative humidity first > > > > + */ > > > > + SHTC3_CMD_RH_MEAS_LP_CS = SHTC3_CMD(0x44DE), > > > > +}; > > > > + > > > > +DECLARE_CRC8_TABLE(shtc3_crc8_tbl); > > > > + > > > > +struct shtc3_rx_data { > > > > + __be16 data; > > > > + u8 crc; > > > > +} __packed; > > > > + > > > > +static int shtc3_send_cmd(struct i2c_client *client, u16 cmd, u16 *data) > > > > +{ > > > > + int ret; > > > > + struct shtc3_rx_data rx_data; > > > > + u8 crc; > > > > + > > > > + ret = i2c_master_send(client, (const char *)&cmd, SHTC3_CMD_LEN); > > > > + if (ret != SHTC3_CMD_LEN) > > That might eat another error, so convention is to check ret < 0 first and then > check the length. That way you can return a more informative error if there > is one. I see you do that on the recv below. > > > > > + return -EIO; > > > > + > > > > + /* > > > > + * This is used to read temperature and humidity measurements > > > > + * as well as the sensor ID. > > > > + * Sensor sends 2 bytes of data followed by one byte of CRC > > > > + */ > > > > + if (data) { > > > > + ret = i2c_master_recv(client, (u8 *) &rx_data, > > > > + sizeof(struct shtc3_rx_data)); > > > > + if (ret < 0) > > > > + return ret; > > > > + if (ret != sizeof(struct shtc3_rx_data)) > > > > + return -EIO; > > > > + > > > > + crc = crc8(shtc3_crc8_tbl, (u8 *)&rx_data.data, > > > > + 2, CRC8_INIT_VALUE); > > > > + if (crc != rx_data.crc) > > > > + return -EIO; > > > > + > > > > + *data = be16_to_cpu(rx_data.data); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int shtc3_sleep(struct i2c_client *client) > > > > +{ > > > > + return shtc3_send_cmd(client, SHTC3_CMD_SLEEP, 0); > > > > +} > > > > + > > > > +static int shtc3_wakeup(struct i2c_client *client) > > > > +{ > > > > + if (shtc3_send_cmd(client, SHTC3_CMD_WAKEUP, 0) < 0) > > > > + return -EIO; > > > > + > > > > + /* Wait for device to wake up */ > > > > + usleep_range(180, 240); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int shtc3_read_channel(struct i2c_client *client, bool temp) > > > > +{ > > > > + int ret; > > > > + u16 cmd; > > > > + u16 meas; > > > > + > > > > + ret = shtc3_wakeup(client); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + /* > > > > + * Sensor sends back measurement results after measurement command > > > > + * has been issued by the host. > > > > + * Sensor sends 3 bytes (2 bytes of data + 1 byte of CRC) for each > > > > + * channel sequentially > > > > + * The command issued by the host determines the channel for which > > > > + * the sensor will first send the data. > > > > + * We select the channel for which we need the results > > > > + * then only read back the 2 bytes corresponding to this channel. > > > > + */ > > > > + cmd = temp ? SHTC3_CMD_TEMP_MEAS_LP_CS : SHTC3_CMD_RH_MEAS_LP_CS; > > > > + ret = shtc3_send_cmd(client, cmd, &meas); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + /* Go back to sleep */ > > > > + shtc3_sleep(client); > > > > + > > > > + return meas; > > > > +} > > > > + > > > > +static int shtc3_read_raw(struct iio_dev *indio_dev, > > > > + struct iio_chan_spec const *chan, int *val, > > > > + int *val2, long mask) > > > > +{ > > > > + struct i2c_client **client = iio_priv(indio_dev); > > > > + int ret; > > > > + > > > > + switch (mask) { > > > > + case IIO_CHAN_INFO_RAW: > > > > + ret = shtc3_read_channel(*client, (chan->type == > > > IIO_TEMP)); > > > > + if (ret < 0) > > > > + return ret; > > > > + *val = ret; > > > > + return IIO_VAL_INT; > > > > + case IIO_CHAN_INFO_SCALE: > > > > + if (chan->type == IIO_TEMP) { > > > > + *val = 2; > > > > + *val2 = 670000; > > > > + } else { > > > > + *val = 0; > > > > + *val2 = 1525; > > > > + } > > > > + return IIO_VAL_INT_PLUS_MICRO; > > > > + case IIO_CHAN_INFO_OFFSET: > > > > + *val = -16852; > > > > + return IIO_VAL_INT; > > > > + default: > > > > + break; > > > > + } > > > > + > > > > + return -EINVAL; > > I'd move this into the default: statement as saves a few lines and makes it > slightly easier to read. > > > > > +} > > > > + > > > > +static const struct iio_chan_spec shtc3_channels[] = { > > > > + { > > > > + .type = IIO_HUMIDITYRELATIVE, > > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > > + BIT(IIO_CHAN_INFO_SCALE), > > > > + }, > > > > + { > > > > + .type = IIO_TEMP, > > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > > > > + BIT(IIO_CHAN_INFO_SCALE) | > > > BIT(IIO_CHAN_INFO_OFFSET), > > > > + } > > > > +}; > > > > + > > > > +static const struct iio_info shtc3_info = { > > > > + .read_raw = shtc3_read_raw, > > > > +}; > > > > + > > > > +static int shtc3_verify_id(struct i2c_client *client) > > > > +{ > > > > + int ret; > > > > + u16 device_id; > > > > + u16 reg_val; > > > > + > > > > + ret = shtc3_send_cmd(client, SHTC3_CMD_GET_ID, ®_val); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + device_id = reg_val & SHTC3_ID_MASK; > > > > + if (device_id != SHTC3_ID) > > > > + return -ENODEV; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int shtc3_reset(struct i2c_client *client) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = shtc3_send_cmd(client, SHTC3_CMD_SOFT_RESET, 0); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + /* Wait for device to enter idle state */ > > > > + usleep_range(180, 240); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int shtc3_setup(struct i2c_client *client) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = shtc3_verify_id(client); > > > > + if (ret < 0) { > > > > + dev_err(&client->dev, "SHTC3 not found\n"); > > > > + return -ENODEV; > > > > + } > > > > + > > > > + ret = shtc3_reset(client); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + return shtc3_sleep(client); > > > > +} > > > > + > > > > +static int shtc3_probe(struct i2c_client *client, > > > > + const struct i2c_device_id *id) > > > > +{ > > > > + struct iio_dev *indio_dev; > > > > + struct i2c_client **data; > > > > + int ret; > > > > + > > > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > > > + if (!indio_dev) > > > > + return -ENOMEM; > > > > + > > > > + data = iio_priv(indio_dev); > > > > + *data = client; > > It's normally better to put a structure in there because the chances > of a later change requiring more data is rather high... > > Obviously for now it will only have one element so will end up > compiling to pretty much the same you have here. > > > > > + > > > > + indio_dev->name = dev_name(&client->dev); > > > > + indio_dev->modes = INDIO_DIRECT_MODE; > > > > + indio_dev->info = &shtc3_info; > > > > + indio_dev->channels = shtc3_channels; > > > > + indio_dev->num_channels = ARRAY_SIZE(shtc3_channels); > > > > + > > > > + crc8_populate_msb(shtc3_crc8_tbl, SHTC3_CRC8_POLYNOMIAL); > > > > + > > > > + ret = shtc3_setup(client); > > > > + if (ret < 0) { > > > > + dev_err(&client->dev, "SHTC3 setup failed\n"); > > > > + return ret; > > > > + } > > > > + > > > > + return devm_iio_device_register(&client->dev, indio_dev); > > > > +} > > > > + > > > > +static const struct i2c_device_id shtc3_id[] = { > > > > + { "shtc3", 0 }, > > > > + { } > > > > +}; > > > > +MODULE_DEVICE_TABLE(i2c, shtc3_id); > > > > + > > > > +static const struct of_device_id shtc3_dt_ids[] = { > > > > + { .compatible = "sensirion,shtc3" }, > > > > + { } > > > > +}; > > > > +MODULE_DEVICE_TABLE(of, shtc3_dt_ids); > > > > + > > > > +static struct i2c_driver shtc3_driver = { > > > > + .driver = { > > > > + .name = "shtc3", > > > > + .of_match_table = shtc3_dt_ids, > > > > + }, > > > > + .probe = shtc3_probe, > > > > + .id_table = shtc3_id, > > > > +}; > > > > + > > > > +module_i2c_driver(shtc3_driver); > > > > +MODULE_DESCRIPTION("Sensirion SHTC3 Humidity and Temperature Sensor"); > > > > +MODULE_AUTHOR("Gilles Talis <gilles.talis@gmail.com>"); > > > > +MODULE_LICENSE("GPL"); > > > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor 2021-11-14 13:23 ` [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor Gilles Talis 2021-11-14 13:47 ` Lars-Peter Clausen @ 2021-11-15 3:08 ` kernel test robot 1 sibling, 0 replies; 8+ messages in thread From: kernel test robot @ 2021-11-15 3:08 UTC (permalink / raw) To: Gilles Talis, jic23, lars, pmeerw, robh+dt, linux-iio Cc: kbuild-all, devicetree, linux-kernel, Gilles Talis [-- Attachment #1: Type: text/plain, Size: 5295 bytes --] Hi Gilles, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on jic23-iio/togreg] [also build test WARNING on robh/for-next v5.16-rc1 next-20211112] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Gilles-Talis/iio-humidity-add-support-for-Sensirion-SHTC3/20211114-212618 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: s390-randconfig-s032-20211115 (attached as .config) compiler: s390-linux-gcc (GCC) 11.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/d4b528fcce840650354227c99ec7f6c0bdd076b0 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Gilles-Talis/iio-humidity-add-support-for-Sensirion-SHTC3/20211114-212618 git checkout d4b528fcce840650354227c99ec7f6c0bdd076b0 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/iio/humidity/shtc3.c:88:39: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short [addressable] [usertype] cmd @@ got restricted __be16 @@ drivers/iio/humidity/shtc3.c:88:39: sparse: expected unsigned short [addressable] [usertype] cmd drivers/iio/humidity/shtc3.c:88:39: sparse: got restricted __be16 >> drivers/iio/humidity/shtc3.c:88:56: sparse: sparse: Using plain integer as NULL pointer drivers/iio/humidity/shtc3.c:93:36: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short [addressable] [usertype] cmd @@ got restricted __be16 @@ drivers/iio/humidity/shtc3.c:93:36: sparse: expected unsigned short [addressable] [usertype] cmd drivers/iio/humidity/shtc3.c:93:36: sparse: got restricted __be16 drivers/iio/humidity/shtc3.c:93:54: sparse: sparse: Using plain integer as NULL pointer >> drivers/iio/humidity/shtc3.c:122:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [usertype] cmd @@ got restricted __be16 @@ drivers/iio/humidity/shtc3.c:122:13: sparse: expected unsigned short [usertype] cmd drivers/iio/humidity/shtc3.c:122:13: sparse: got restricted __be16 drivers/iio/humidity/shtc3.c:189:38: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short [addressable] [usertype] cmd @@ got restricted __be16 @@ drivers/iio/humidity/shtc3.c:189:38: sparse: expected unsigned short [addressable] [usertype] cmd drivers/iio/humidity/shtc3.c:189:38: sparse: got restricted __be16 drivers/iio/humidity/shtc3.c:204:38: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned short [addressable] [usertype] cmd @@ got restricted __be16 @@ drivers/iio/humidity/shtc3.c:204:38: sparse: expected unsigned short [addressable] [usertype] cmd drivers/iio/humidity/shtc3.c:204:38: sparse: got restricted __be16 drivers/iio/humidity/shtc3.c:204:60: sparse: sparse: Using plain integer as NULL pointer vim +88 drivers/iio/humidity/shtc3.c 85 86 static int shtc3_sleep(struct i2c_client *client) 87 { > 88 return shtc3_send_cmd(client, SHTC3_CMD_SLEEP, 0); 89 } 90 91 static int shtc3_wakeup(struct i2c_client *client) 92 { 93 if (shtc3_send_cmd(client, SHTC3_CMD_WAKEUP, 0) < 0) 94 return -EIO; 95 96 /* Wait for device to wake up */ 97 usleep_range(180, 240); 98 99 return 0; 100 } 101 102 static int shtc3_read_channel(struct i2c_client *client, bool temp) 103 { 104 int ret; 105 u16 cmd; 106 u16 meas; 107 108 ret = shtc3_wakeup(client); 109 if (ret < 0) 110 return ret; 111 112 /* 113 * Sensor sends back measurement results after measurement command 114 * has been issued by the host. 115 * Sensor sends 3 bytes (2 bytes of data + 1 byte of CRC) for each 116 * channel sequentially 117 * The command issued by the host determines the channel for which 118 * the sensor will first send the data. 119 * We select the channel for which we need the results 120 * then only read back the 2 bytes corresponding to this channel. 121 */ > 122 cmd = temp ? SHTC3_CMD_TEMP_MEAS_LP_CS : SHTC3_CMD_RH_MEAS_LP_CS; 123 ret = shtc3_send_cmd(client, cmd, &meas); 124 if (ret < 0) 125 return ret; 126 127 /* Go back to sleep */ 128 shtc3_sleep(client); 129 130 return meas; 131 } 132 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 14470 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor 2021-11-14 13:23 [PATCH 0/2] iio: humidity: add support for Sensirion SHTC3 Gilles Talis 2021-11-14 13:23 ` [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor Gilles Talis @ 2021-11-14 13:23 ` Gilles Talis 2021-11-15 2:10 ` Rob Herring 1 sibling, 1 reply; 8+ messages in thread From: Gilles Talis @ 2021-11-14 13:23 UTC (permalink / raw) To: jic23, lars, pmeerw, robh+dt, linux-iio Cc: devicetree, linux-kernel, Gilles Talis Sensirion SHTC3 is a humidity and temperature sensor controlled through I2C interface Signed-off-by: Gilles Talis <gilles.talis@gmail.com> --- Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml index 1e4b3464d734..c6cce9826afc 100644 --- a/Documentation/devicetree/bindings/trivial-devices.yaml +++ b/Documentation/devicetree/bindings/trivial-devices.yaml @@ -277,6 +277,8 @@ properties: - sensirion,sgp30 # Sensirion gas sensor with I2C interface - sensirion,sgp40 + # Sensirion humidity and temperature sensor with I2C interface + - sensirion,shtc3 # Sensortek 3 axis accelerometer - sensortek,stk8312 # Sensortek 3 axis accelerometer -- 2.27.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor 2021-11-14 13:23 ` [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor Gilles Talis @ 2021-11-15 2:10 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2021-11-15 2:10 UTC (permalink / raw) To: Gilles Talis Cc: devicetree, robh+dt, lars, jic23, linux-kernel, linux-iio, pmeerw On Sun, 14 Nov 2021 14:23:35 +0100, Gilles Talis wrote: > Sensirion SHTC3 is a humidity and temperature sensor controlled > through I2C interface > > Signed-off-by: Gilles Talis <gilles.talis@gmail.com> > --- > Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ > 1 file changed, 2 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/sensirion,shtc1.example.dt.yaml: shtc3@70: 'sensirion,blocking-io' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/trivial-devices.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1554826 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-15 7:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-14 13:23 [PATCH 0/2] iio: humidity: add support for Sensirion SHTC3 Gilles Talis
2021-11-14 13:23 ` [PATCH 1/2] iio: humidity: Add support for Sensirion SHTC3 sensor Gilles Talis
2021-11-14 13:47 ` Lars-Peter Clausen
[not found] ` <CAKcgs2x7FCf8xBj-PQ62UQNgkxBjFfQznEVCysNO547Yr5Ct3w@mail.gmail.com>
2021-11-14 16:49 ` Jonathan Cameron
2021-11-15 7:29 ` Gilles Talis
2021-11-15 3:08 ` kernel test robot
2021-11-14 13:23 ` [PATCH 2/2] dt-bindings: trivial-devices: Add Sensirion SHTC3 humidity sensor Gilles Talis
2021-11-15 2:10 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).