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
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 <[email protected]>
---
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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.27.0
Sensirion SHTC3 is a humidity and temperature sensor controlled
through I2C interface
Signed-off-by: Gilles Talis <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +MODULE_LICENSE("GPL");
On Sun, 14 Nov 2021 15:58:15 +0100
Gilles Talis <[email protected]> wrote:
> Hi Lars,
>
> Le dim. 14 nov. 2021 à 14:47, Lars-Peter Clausen <[email protected]> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > > + *
> > > + * 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 <[email protected]>");
> > > +MODULE_LICENSE("GPL");
> >
> >
> >
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
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/[email protected]
Hi Jonathan,
Le dim. 14 nov. 2021 à 17:44, Jonathan Cameron <[email protected]> a écrit :
>
> On Sun, 14 Nov 2021 15:58:15 +0100
> Gilles Talis <[email protected]> wrote:
>
> > Hi Lars,
> >
> > Le dim. 14 nov. 2021 à 14:47, Lars-Peter Clausen <[email protected]> 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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > > + *
> > > > + * 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 <[email protected]>");
> > > > +MODULE_LICENSE("GPL");
> > >
> > >
> > >
>