[RFC PATCH 0/3] Add Sensirion STS30 temperature sensor driver

[RFC PATCH 0/3] Add Sensirion STS30 temperature sensor driver

[Maxwell Doose]

Hi all,

This patch adds support for the Sensirion STS30 family of temperature
sensor drivers. It currently supports single-shot non-clock stretched
readings as well as the entirety of the STS30 family.

I'm sending this as an RFC patch as I unfortunately do not have the
hardware right now. I have compile-tested it successfully and have done
a tad bit of testing on QEMU, with it properly registering with no
errors reported in dmesg. Unfortunately due to the way I had compiled
the kernel used to test the driver it did not expose the I2C bus to
userspace properly and I was not able to retrieve values from the
driver, but I can continue more testing over the next few days and am
happy to answer any questions anyone may have. Additionally, regarding
dmesg logs, I neglected to save them while in my testing, but as stated
previously, I can continue more testing over the next couple of days.

Maxwell Doose (3):
  dt-bindings: iio: temperature: Add STS30 devicetree bindings
  iio: temperature: Add STS30 temperature sensor driver
  MAINTAINERS: Add entry for Sensirion STS30 driver

 .../iio/temperature/sensirion,sts30.yaml      |  49 +++
 MAINTAINERS                                   |   6 +
 drivers/iio/temperature/Kconfig               |  11 +
 drivers/iio/temperature/Makefile              |   1 +
 drivers/iio/temperature/sts30.c               | 321 ++++++++++++++++++
 5 files changed, 388 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
 create mode 100644 drivers/iio/temperature/sts30.c

-- 
2.54.0

[RFC PATCH 1/3] dt-bindings: iio: temperature: Add STS30 devicetree bindings

[Maxwell Doose]

Add the devicetree bindings for the STS30 family of temperature sensors.
The STS30 family of sensors includes the STS30, STS31, and STS35.

All devices in the STS30 family share the same commands, timings, etc.
The only difference between them is their measurement accuracy and
tolerance.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 .../iio/temperature/sensirion,sts30.yaml      | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml

diff --git a/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
new file mode 100644
index 000000000000..dc0c6b7832fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/sensirion,sts30.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STS30, STS31, and STS35 temperature iio sensors
+
+maintainers:
+  - Maxwell Doose <m32285159@gmail.com>
+
+description: |
+  Family of digital temperature sensors from Sensirion with I2C interface.
+  All devices are compatible with each other, their only differences are
+  their levels of accuracy.
+
+  Datasheet available at:
+  https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
+
+properties:
+  compatible:
+    enum:
+      - sensirion,sts30
+      - sensirion,sts31
+      - sensirion,sts35
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        temperature@4a {
+          compatible = "sensirion,sts30";
+          reg = <0x4a>;
+          vdd-supply = <&reg_3v3>;
+        };
+      };
-- 
2.54.0

[RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver

[Maxwell Doose]

Add a driver for the Sensirion STS30 family of temperature sensor
drivers over I2C. The STS30 family of sensors includes the STS30, STS31,
and STS35, all of which are supported by this driver, since they all
share the same commands, etc. and only differ in accuracy and tolerance.

The driver currently supports single-shot non-clock stretched readings,
by using a specified delay based on the repeatability/delay specified
by the user. The repeatability/delay can be changed at any time through
sysfs.

Additionally add Kconfig and Makefile entries for the driver.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/temperature/Kconfig  |  11 ++
 drivers/iio/temperature/Makefile |   1 +
 drivers/iio/temperature/sts30.c  | 321 +++++++++++++++++++++++++++++++
 3 files changed, 333 insertions(+)
 create mode 100644 drivers/iio/temperature/sts30.c

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 9328b2250ace..c23a9c40c725 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -88,6 +88,17 @@ config MLX90635
 	  This driver can also be built as a module. If so, the module will
 	  be called mlx90635.
 
+config STS30
+	tristate "STS30/STS31/STS35 temperature sensor"
+	depends on I2C
+	select CRC8
+	help
+	  If you say yes here you get support for the Sensirion STS30,
+	  STS31, and STS35 temperature sensors over I2C.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called sts30.
+
 config TMP006
 	tristate "TMP006 infrared thermopile sensor"
 	depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7..9c9bf8d1b70e 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_MCP9600) += mcp9600.o
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_MLX90632) += mlx90632.o
 obj-$(CONFIG_MLX90632) += mlx90635.o
+obj-$(CONFIG_STS30) += sts30.o
 obj-$(CONFIG_TMP006) += tmp006.o
 obj-$(CONFIG_TMP007) += tmp007.o
 obj-$(CONFIG_TMP117) += tmp117.o
diff --git a/drivers/iio/temperature/sts30.c b/drivers/iio/temperature/sts30.c
new file mode 100644
index 000000000000..b49deb1d62c2
--- /dev/null
+++ b/drivers/iio/temperature/sts30.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026 Maxwell Doose
+ *
+ * Sensirion STS30 temperature sensor driver
+ *
+ * Datasheet: https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
+ *
+ * Author: Maxwell Doose <m32285159@gmail.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+/* Amount of bytes received from the STS30 after a read command */
+#define STS30_MEAS_SIZE 3
+
+#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
+#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
+#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10
+
+/* Soft reset command */
+#define STS30_COMMAND_RESET 0x30A2
+
+/*
+ * sts30 includes a CRC8 checksum at the end of its i2c responses. The polynomial
+ * is used to generate the CRC8 table and the seed is the starting value.
+ */
+#define STS30_CRC8_POLYNOMIAL 0x31
+#define STS30_CRC8_SEED 0xFF
+
+DECLARE_CRC8_TABLE(sts30_crc_table);
+
+enum sts30_read_delays {
+	STS30_REPEAT_LOW = 4500,
+	STS30_REPEAT_MED = 6000,
+	STS30_REPEAT_HIGH = 15000
+};
+
+/**
+ * struct sts30_data - data structure for STS30 driver
+ *
+ * @client: underlying i2c client data structure
+ * @lock: mutex for serialized communication on the i2c bus
+ * @delay: measurement duration for the current repeatability mode
+ */
+struct sts30_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	/*
+	 * sts30 has three potential repeatability/measurement durations. We need to
+	 * account for them while reading the i2c bus.
+	 *
+	 * See section 2.2 in the datasheet for more info on processing times.
+	 */
+	enum sts30_read_delays delay;
+};
+
+static int sts30_verify_crc8(struct sts30_data *data, u8 buf[STS30_MEAS_SIZE])
+{
+	int crc;
+
+	crc = crc8(sts30_crc_table, buf, 2, STS30_CRC8_SEED);
+	if (crc != buf[2]) {
+		dev_err(&data->client->dev, "Expected CRC8 value of 0x%02x, got 0x%02x\n",
+			buf[2], crc);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
+{
+	u8 tmp[2];
+	u8 buf[STS30_MEAS_SIZE];
+	int ret;
+
+	put_unaligned_be16(command, tmp);
+
+	ret = i2c_master_send(data->client, tmp, sizeof(tmp));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(tmp))
+		return -EIO;
+
+	fsleep(data->delay);
+
+	ret = i2c_master_recv(data->client, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(buf))
+		return -EIO;
+
+	*val = get_unaligned_be16(buf);
+
+	ret = sts30_verify_crc8(data, buf);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int sts30_write(struct sts30_data *data, u16 command)
+{
+	u8 buf[2];
+	int ret;
+
+	put_unaligned_be16(command, buf);
+
+	ret = i2c_master_send(data->client, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(buf))
+		return -EIO;
+
+	return 0;
+}
+
+static int sts30_reset(struct sts30_data *data)
+{
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	ret = sts30_write(data, STS30_COMMAND_RESET);
+	if (ret)
+		return ret;
+
+	fsleep(1500);
+
+	return 0;
+}
+
+static int sts30_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, int *val, int *val2,
+			  long mask)
+{
+	struct sts30_data *data = iio_priv(indio_dev);
+	int ret;
+	u16 tmp;
+
+	guard(mutex)(&data->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (data->delay) {
+		case STS30_REPEAT_LOW:
+			ret = sts30_read(data, STS30_COMMAND_READ_LOW_REPEAT, &tmp);
+			break;
+		case STS30_REPEAT_MED:
+			ret = sts30_read(data, STS30_COMMAND_READ_MED_REPEAT, &tmp);
+			break;
+		case STS30_REPEAT_HIGH:
+			ret = sts30_read(data, STS30_COMMAND_READ_HIGH_REPEAT, &tmp);
+			break;
+		default:
+			dev_warn(&data->client->dev, "Repeatability state corrupted, got: %d\n",
+				 data->delay);
+			return -EINVAL;
+		}
+
+		if (ret)
+			return ret;
+
+		*val = tmp;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OFFSET:
+		/*
+		 * We use this constant -16852 as calculated using the formula
+		 * in the datasheet. See section 4.12 in the data sheet for more
+		 * info.
+		 */
+		*val = -16852;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 175000;
+		*val2 = 65535;
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		*val2 = data->delay;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int sts30_write_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int val, int val2,
+			   long mask)
+{
+	struct sts30_data *data = iio_priv(indio_dev);
+
+	if (val)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME: {
+		guard(mutex)(&data->lock);
+
+		switch (val2) {
+		case STS30_REPEAT_LOW:
+			data->delay = STS30_REPEAT_LOW;
+			break;
+		case STS30_REPEAT_MED:
+			data->delay = STS30_REPEAT_MED;
+			break;
+		case STS30_REPEAT_HIGH:
+			data->delay = STS30_REPEAT_HIGH;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info sts30_info = {
+	.read_raw = sts30_read_raw,
+	.write_raw = sts30_write_raw
+};
+
+static const struct iio_chan_spec sts30_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_OFFSET) |
+				      BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_INT_TIME)
+	},
+};
+
+static const struct i2c_device_id sts30_id[] = {
+	{ "sts30" },
+	{ "sts31" },
+	{ "sts35" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sts30_id);
+
+static int sts30_probe(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev;
+	struct sts30_data *data;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	indio_dev->name = client->name;
+	indio_dev->info = &sts30_info;
+	indio_dev->channels = sts30_channels;
+	indio_dev->num_channels = ARRAY_SIZE(sts30_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	data->delay = STS30_REPEAT_HIGH;
+
+	ret = devm_mutex_init(&client->dev, &data->lock);
+	if (ret)
+		return ret;
+
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = sts30_reset(data);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct of_device_id sts30_of_match[] = {
+	{ .compatible = "sensirion,sts30" },
+	{ .compatible = "sensirion,sts31" },
+	{ .compatible = "sensirion,sts35" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sts30_of_match);
+
+static struct i2c_driver sts30_driver = {
+	.driver = {
+		.name = "sts30",
+		.of_match_table = sts30_of_match,
+	},
+	.probe = sts30_probe,
+	.id_table = sts30_id,
+};
+
+static int __init sts30_init(void)
+{
+	crc8_populate_msb(sts30_crc_table, STS30_CRC8_POLYNOMIAL);
+
+	return i2c_add_driver(&sts30_driver);
+}
+module_init(sts30_init);
+
+static void __exit sts30_exit(void)
+{
+	i2c_del_driver(&sts30_driver);
+}
+module_exit(sts30_exit);
+
+MODULE_AUTHOR("Maxwell Doose <m32285159@gmail.com>");
+MODULE_DESCRIPTION("Sensirion STS30 temperature sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.54.0

[RFC PATCH 3/3] MAINTAINERS: Add entry for Sensirion STS30 driver

[Maxwell Doose]

Add the entry for the STS30 temperature sensor driver in MAINTAINERS.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d95d3ef77773..6bb361ac213f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24487,6 +24487,12 @@ F:	drivers/iio/chemical/sps30.c
 F:	drivers/iio/chemical/sps30_i2c.c
 F:	drivers/iio/chemical/sps30_serial.c
 
+SENSIRION STS30 TEMPERATURE SENSOR DRIVER
+M:	Maxwell Doose <m32285159@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
+F:	drivers/iio/temperature/sts30.c
+
 SERIAL DEVICE BUS
 M:	Rob Herring <robh@kernel.org>
 L:	linux-serial@vger.kernel.org
-- 
2.54.0

Re: [RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `sts30_reset()` is ignored during probe, allowing phantom device registration on failure.
- [High] The driver implements a Clock Stretching Disabled I2C sequence but uses Clock Stretching Enabled command codes, resulting in a protocol mismatch.
--

commit 1a183c37bbba5fe87dfcc6019f964e51526fec2b
Author: Maxwell Doose <m32285159@gmail.com>

iio: temperature: Add STS30 temperature sensor driver

Add a driver for the Sensirion STS30 family of temperature sensor
drivers over I2C. The driver supports single-shot non-clock stretched
readings using a specified delay based on the repeatability/delay.

> diff --git a/drivers/iio/temperature/sts30.c b/drivers/iio/temperature/sts30.c
> new file mode 100644
> index 0000000000000..b49deb1d62c21
> --- /dev/null
> +++ b/drivers/iio/temperature/sts30.c

[ ... ]

> +#define STS30_MEAS_SIZE 3
> +
> +#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
> +#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
> +#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10

[Severity: High]
The commit message mentions using non-clock stretched readings, but are
these the correct commands? According to the STS3x datasheet, the 0x2Cxx
series commands are for "Clock Stretching Enabled", which require a
Repeated START condition between the write and read phases.

[ ... ]

> +static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
> +{
> +	u8 tmp[2];
> +	u8 buf[STS30_MEAS_SIZE];
> +	int ret;
> +
> +	put_unaligned_be16(command, tmp);
> +
> +	ret = i2c_master_send(data->client, tmp, sizeof(tmp));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(tmp))
> +		return -EIO;
> +
> +	fsleep(data->delay);

[Severity: High]
Will sending a STOP condition here abort the measurement? Since
i2c_master_send() issues a STOP condition rather than a Repeated START,
this sequence violates the protocol for the Clock Stretching Enabled
commands (0x2Cxx) defined above.

Should this use the "Clock Stretching Disabled" commands (e.g., 0x2400)
instead, or alternatively use a single i2c_transfer() with a Repeated START?

[ ... ]

> +static int sts30_probe(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev;
> +	struct sts30_data *data;
> +	int ret;

[ ... ]

> +	ret = sts30_reset(data);
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}

[Severity: High]
Is it intentional to ignore the return value of sts30_reset() here?

If the reset fails due to the hardware being missing or a broken I2C bus,
this will unconditionally register a phantom device to userspace. Should we
check 'ret' and return the error code instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620044010.1082621-1-m32285159@gmail.com?part=2

Re: [RFC PATCH 1/3] dt-bindings: iio: temperature: Add STS30 devicetree bindings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The STS30 device tree binding omits the `interrupts` property, preventing users from modeling the hardware's ALERT pin.
--

commit 91523cd6a186b322c33e0fb78f1316cdc24a39af
Author: Maxwell Doose <m32285159@gmail.com>

dt-bindings: iio: temperature: Add STS30 devicetree bindings

This commit adds device tree bindings for the Sensirion STS30 family of 
digital temperature sensors, which includes the STS30, STS31, and STS35.

> diff --git a/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
> new file mode 100644
> index 0000000000000..dc0c6b7832fb8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - sensirion,sts30
> +      - sensirion,sts31
> +      - sensirion,sts35
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

[Severity: Medium]
Does this schema prevent modeling the hardware's ALERT pin?

The Sensirion STS3x family hardware features an ALERT pin that can be wired
to an interrupt controller to signal high and low temperature thresholds.
Because this schema enforces additionalProperties: false but omits the
interrupts property from the properties block, any device tree that
accurately models the physical hardware connection will fail
dt_binding_check.

Should the interrupts property be explicitly listed to allow accurately
describing the hardware capabilities, even if the current IIO driver does
not yet utilize the ALERT pin?

> +
> +examples:

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620044010.1082621-1-m32285159@gmail.com?part=1

Re: [RFC PATCH 3/3] MAINTAINERS: Add entry for Sensirion STS30 driver

[Joshua Crofts]

On Fri, 19 Jun 2026 23:40:07 -0500
Maxwell Doose <m32285159@gmail.com> wrote:  
> +SENSIRION STS30 TEMPERATURE SENSOR DRIVER
> +M:	Maxwell Doose <m32285159@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml

Fold the above changes into your dt-bindings patch.

> +F:	drivers/iio/temperature/sts30.c

This change then goes into the driver code patch. If you're
adding new files, ensure that your patch also adds a mention
to said file to MAINTAINERS (IIRC b4 actually checks for this
when you run prep --check).

-- 
Kind regards

CJD

Re: [RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver

[Joshua Crofts]

Hi Max,

comments inline, some nits, some more serious, some Sashiko reviews.

Josh

On Fri, 19 Jun 2026 23:40:06 -0500
Maxwell Doose <m32285159@gmail.com> wrote:
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2026 Maxwell Doose
> + *
> + * Sensirion STS30 temperature sensor driver
> + *
> + * Datasheet: https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
> + *
> + * Author: Maxwell Doose <m32285159@gmail.com>

Nit, but this is probably unnecessary since you already have
the copyright statement above.

> + */
> +

You're missing array_size.h, as used in probe.

> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>

Move the IIO specific headers below the generic linux headers
and group them separately.

> +#include <linux/i2c.h>
> +#include <linux/kernel.h>

Don't use kernel.h for new entries, you should include what
you actually use instead of relying on this. (there is a tool
that can help with this, it's called `iwyu-tool`).

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +/* Amount of bytes received from the STS30 after a read command */
> +#define STS30_MEAS_SIZE 3
> +
> +#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
> +#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
> +#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10
> +
> +/* Soft reset command */

No point in having a comment if your macro is sensibly named.

> +#define STS30_COMMAND_RESET 0x30A2
> +
> +/*
> + * sts30 includes a CRC8 checksum at the end of its i2c responses. The polynomial
> + * is used to generate the CRC8 table and the seed is the starting value.
> + */
> +#define STS30_CRC8_POLYNOMIAL 0x31
> +#define STS30_CRC8_SEED 0xFF
> +
> +DECLARE_CRC8_TABLE(sts30_crc_table);
> +
> +enum sts30_read_delays {
> +	STS30_REPEAT_LOW = 4500,
> +	STS30_REPEAT_MED = 6000,
> +	STS30_REPEAT_HIGH = 15000
> +};
> +
> +/**
> + * struct sts30_data - data structure for STS30 driver
> + *
> + * @client: underlying i2c client data structure
> + * @lock: mutex for serialized communication on the i2c bus
> + * @delay: measurement duration for the current repeatability mode
> + */

I'd remove this comment altogether and just add a comment on why
you need a mutex.

> +struct sts30_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	/*
> +	 * sts30 has three potential repeatability/measurement durations. We need to
> +	 * account for them while reading the i2c bus.
> +	 *
> +	 * See section 2.2 in the datasheet for more info on processing times.
> +	 */
> +	enum sts30_read_delays delay;
> +};
> +
> +static int sts30_verify_crc8(struct sts30_data *data, u8 buf[STS30_MEAS_SIZE])
> +{
> +	int crc;
> +
> +	crc = crc8(sts30_crc_table, buf, 2, STS30_CRC8_SEED);

Please use sizeof() instead of hard coding the buffer length.

> +	if (crc != buf[2]) {
> +		dev_err(&data->client->dev, "Expected CRC8 value of 0x%02x, got 0x%02x\n",
> +			buf[2], crc);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
> +{
> +	u8 tmp[2];
> +	u8 buf[STS30_MEAS_SIZE];
	
Reverse christmas tree order.

> +	int ret;
> +
> +	put_unaligned_be16(command, tmp);
> +
> +	ret = i2c_master_send(data->client, tmp, sizeof(tmp));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(tmp))
> +		return -EIO;
> +
> +	fsleep(data->delay);

Adding Sashiko's comment:

Will sending a STOP condition here abort the measurement? Since
i2c_master_send() issues a STOP condition rather than a Repeated START,
this sequence violates the protocol for the Clock Stretching Enabled
commands (0x2Cxx) defined above.
Should this use the "Clock Stretching Disabled" commands (e.g., 0x2400)
instead, or alternatively use a single i2c_transfer() with a Repeated START?

> +
> +	ret = i2c_master_recv(data->client, buf, sizeof(buf));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(buf))
> +		return -EIO;
> +
> +	*val = get_unaligned_be16(buf);
> +
> +	ret = sts30_verify_crc8(data, buf);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

...

> +static int sts30_reset(struct sts30_data *data)
> +{
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = sts30_write(data, STS30_COMMAND_RESET);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(1500);

Add a comment or change this to a macro to explain why 1500
specifically.

> +
> +	return 0;
> +}
> +
> +static int sts30_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, int *val, int *val2,
> +			  long mask)
> +{
> +	struct sts30_data *data = iio_priv(indio_dev);
> +	int ret;
> +	u16 tmp;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (data->delay) {
> +		case STS30_REPEAT_LOW:
> +			ret = sts30_read(data, STS30_COMMAND_READ_LOW_REPEAT, &tmp);
> +			break;
> +		case STS30_REPEAT_MED:
> +			ret = sts30_read(data, STS30_COMMAND_READ_MED_REPEAT, &tmp);
> +			break;
> +		case STS30_REPEAT_HIGH:
> +			ret = sts30_read(data, STS30_COMMAND_READ_HIGH_REPEAT, &tmp);
> +			break;
> +		default:
> +			dev_warn(&data->client->dev, "Repeatability state corrupted, got: %d\n",
> +				 data->delay);

Do we need this warning? Returning -EINVAL is sufficient
enough.

> +			return -EINVAL;
> +		}
> +
> +		if (ret)
> +			return ret;
> +
> +		*val = tmp;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OFFSET:
> +		/*
> +		 * We use this constant -16852 as calculated using the formula
> +		 * in the datasheet. See section 4.12 in the data sheet for more
> +		 * info.
> +		 */
> +		*val = -16852;

Okay, this would definitely be better if it were a macro. I'd even
be tempted to add the formula in a comment for safe keeping.
 
...

> +static const struct i2c_device_id sts30_id[] = {
> +	{ "sts30" },
> +	{ "sts31" },
> +	{ "sts35" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sts30_id);

Move the struct i2c_device_id struct after probe(). Additionally,
use the `.name` named initializer when defining the IDs (this reflects
on Uwe Kleine-Konig's effort in IIO).

> +
> +static int sts30_probe(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev;
> +	struct sts30_data *data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	indio_dev->name = client->name;
> +	indio_dev->info = &sts30_info;
> +	indio_dev->channels = sts30_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(sts30_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +	data->delay = STS30_REPEAT_HIGH;
> +
> +	ret = devm_mutex_init(&client->dev, &data->lock);
> +	if (ret)
> +		return ret;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = sts30_reset(data);

You're not checking ret here.

> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +

-- 
Kind regards

CJD

Re: [RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver

[Maxwell Doose]

Hi Joshua,

Sorry about the bugs, this is my first driver submission.

On Sat, Jun 20, 2026 at 2:43 AM Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> Hi Max,
>
> comments inline, some nits, some more serious, some Sashiko reviews.
>
> Josh
>
> On Fri, 19 Jun 2026 23:40:06 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2026 Maxwell Doose
> > + *
> > + * Sensirion STS30 temperature sensor driver
> > + *
> > + * Datasheet: https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
> > + *
> > + * Author: Maxwell Doose <m32285159@gmail.com>
>
> Nit, but this is probably unnecessary since you already have
> the copyright statement above.
>

Fair, will remove.

> > + */
> > +
>
> You're missing array_size.h, as used in probe.
>

Nice catch, I didn't see that in my final compilation.

> > +#include <linux/bits.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/crc8.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/export.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/types.h>
>
> Move the IIO specific headers below the generic linux headers
> and group them separately.
>

Will do.

> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
>
> Don't use kernel.h for new entries, you should include what

Righty oh, will remove. I guess that's why it compiled fine on my end
because it's a catch-all.

> you actually use instead of relying on this. (there is a tool
> that can help with this, it's called `iwyu-tool`).
>

I've heard of iwyu-tool, I just don't know how to configure it :(

> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/types.h>
> > +#include <linux/unaligned.h>
> > +
> > +/* Amount of bytes received from the STS30 after a read command */
> > +#define STS30_MEAS_SIZE 3
> > +
> > +#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
> > +#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
> > +#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10
> > +
> > +/* Soft reset command */
>
> No point in having a comment if your macro is sensibly named.
>

Fair point, I'll see about renaming it and removing the comment in v2.

> > +#define STS30_COMMAND_RESET 0x30A2
> > +
> > +/*
> > + * sts30 includes a CRC8 checksum at the end of its i2c responses. The polynomial
> > + * is used to generate the CRC8 table and the seed is the starting value.
> > + */
> > +#define STS30_CRC8_POLYNOMIAL 0x31
> > +#define STS30_CRC8_SEED 0xFF
> > +
> > +DECLARE_CRC8_TABLE(sts30_crc_table);
> > +
> > +enum sts30_read_delays {
> > +     STS30_REPEAT_LOW = 4500,
> > +     STS30_REPEAT_MED = 6000,
> > +     STS30_REPEAT_HIGH = 15000
> > +};
> > +
> > +/**
> > + * struct sts30_data - data structure for STS30 driver
> > + *
> > + * @client: underlying i2c client data structure
> > + * @lock: mutex for serialized communication on the i2c bus
> > + * @delay: measurement duration for the current repeatability mode
> > + */
>
> I'd remove this comment altogether and just add a comment on why
> you need a mutex.
>

Alright then.

> > +struct sts30_data {
> > +     struct i2c_client *client;
> > +     struct mutex lock;
> > +     /*
> > +      * sts30 has three potential repeatability/measurement durations. We need to
> > +      * account for them while reading the i2c bus.
> > +      *
> > +      * See section 2.2 in the datasheet for more info on processing times.
> > +      */
> > +     enum sts30_read_delays delay;
> > +};
> > +
> > +static int sts30_verify_crc8(struct sts30_data *data, u8 buf[STS30_MEAS_SIZE])
> > +{
> > +     int crc;
> > +
> > +     crc = crc8(sts30_crc_table, buf, 2, STS30_CRC8_SEED);
>
> Please use sizeof() instead of hard coding the buffer length.
>

I suppose that we can do sizeof(buf) - 1 since we know the last byte
in the buffer has to be the checksum.

> > +     if (crc != buf[2]) {
> > +             dev_err(&data->client->dev, "Expected CRC8 value of 0x%02x, got 0x%02x\n",
> > +                     buf[2], crc);
> > +             return -EIO;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
> > +{
> > +     u8 tmp[2];
> > +     u8 buf[STS30_MEAS_SIZE];
>
> Reverse christmas tree order.
>

Sounds good.

> > +     int ret;
> > +
> > +     put_unaligned_be16(command, tmp);
> > +
> > +     ret = i2c_master_send(data->client, tmp, sizeof(tmp));
> > +     if (ret < 0)
> > +             return ret;
> > +     if (ret != sizeof(tmp))
> > +             return -EIO;
> > +
> > +     fsleep(data->delay);
>
> Adding Sashiko's comment:
>
> Will sending a STOP condition here abort the measurement? Since
> i2c_master_send() issues a STOP condition rather than a Repeated START,
> this sequence violates the protocol for the Clock Stretching Enabled
> commands (0x2Cxx) defined above.
> Should this use the "Clock Stretching Disabled" commands (e.g., 0x2400)
> instead, or alternatively use a single i2c_transfer() with a Repeated START?
>

Yea, I accidentally mixed up the commands, I saw this on sashiko when
I submitted it last night. It seems like a five minute fix in any case
though.

> > +
> > +     ret = i2c_master_recv(data->client, buf, sizeof(buf));
> > +     if (ret < 0)
> > +             return ret;
> > +     if (ret != sizeof(buf))
> > +             return -EIO;
> > +
> > +     *val = get_unaligned_be16(buf);
> > +
> > +     ret = sts30_verify_crc8(data, buf);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return 0;
> > +}
>
> ...
>
> > +static int sts30_reset(struct sts30_data *data)
> > +{
> > +     int ret;
> > +
> > +     guard(mutex)(&data->lock);
> > +
> > +     ret = sts30_write(data, STS30_COMMAND_RESET);
> > +     if (ret)
> > +             return ret;
> > +
> > +     fsleep(1500);
>
> Add a comment or change this to a macro to explain why 1500
> specifically.
>

Alright. Can also explain here:
Datasheet dictates that maximum soft reset time is 1.5ms or 1500us. We
fsleep() here to ensure that the device is ready before we continue.

> > +
> > +     return 0;
> > +}
> > +
> > +static int sts30_read_raw(struct iio_dev *indio_dev,
> > +                       struct iio_chan_spec const *chan, int *val, int *val2,
> > +                       long mask)
> > +{
> > +     struct sts30_data *data = iio_priv(indio_dev);
> > +     int ret;
> > +     u16 tmp;
> > +
> > +     guard(mutex)(&data->lock);
> > +
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_RAW:
> > +             switch (data->delay) {
> > +             case STS30_REPEAT_LOW:
> > +                     ret = sts30_read(data, STS30_COMMAND_READ_LOW_REPEAT, &tmp);
> > +                     break;
> > +             case STS30_REPEAT_MED:
> > +                     ret = sts30_read(data, STS30_COMMAND_READ_MED_REPEAT, &tmp);
> > +                     break;
> > +             case STS30_REPEAT_HIGH:
> > +                     ret = sts30_read(data, STS30_COMMAND_READ_HIGH_REPEAT, &tmp);
> > +                     break;
> > +             default:
> > +                     dev_warn(&data->client->dev, "Repeatability state corrupted, got: %d\n",
> > +                              data->delay);
>
> Do we need this warning? Returning -EINVAL is sufficient
> enough.
>

Since this is read_raw() the warning might help the user resolve the
error rather than just getting "Invalid argument".

> > +                     return -EINVAL;
> > +             }
> > +
> > +             if (ret)
> > +                     return ret;
> > +
> > +             *val = tmp;
> > +             return IIO_VAL_INT;
> > +     case IIO_CHAN_INFO_OFFSET:
> > +             /*
> > +              * We use this constant -16852 as calculated using the formula
> > +              * in the datasheet. See section 4.12 in the data sheet for more
> > +              * info.
> > +              */
> > +             *val = -16852;
>
> Okay, this would definitely be better if it were a macro. I'd even
> be tempted to add the formula in a comment for safe keeping.
>

At this point I agree, it's probably a good idea to put our constants
at the top of the file as macros and put the formula there :/

> ...
>
> > +static const struct i2c_device_id sts30_id[] = {
> > +     { "sts30" },
> > +     { "sts31" },
> > +     { "sts35" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, sts30_id);
>
> Move the struct i2c_device_id struct after probe(). Additionally,
> use the `.name` named initializer when defining the IDs (this reflects
> on Uwe Kleine-Konig's effort in IIO).
>

Right. When I was first writing up the driver I'd ended up using
i2c_match_id() but after removing it I'd forgotten to put it back at
the bottom. Will fix (alongside the named initializers).

> > +
> > +static int sts30_probe(struct i2c_client *client)
> > +{
> > +     struct iio_dev *indio_dev;
> > +     struct sts30_data *data;
> > +     int ret;
> > +
> > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +
> > +     indio_dev->name = client->name;
> > +     indio_dev->info = &sts30_info;
> > +     indio_dev->channels = sts30_channels;
> > +     indio_dev->num_channels = ARRAY_SIZE(sts30_channels);
> > +     indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +     data = iio_priv(indio_dev);
> > +     data->client = client;
> > +     data->delay = STS30_REPEAT_HIGH;
> > +
> > +     ret = devm_mutex_init(&client->dev, &data->lock);
> > +     if (ret)
> > +             return ret;
> > +
> > +     i2c_set_clientdata(client, indio_dev);
> > +
> > +     ret = sts30_reset(data);
>
> You're not checking ret here.
>

Also saw that from sashiko, and will fix.

-- 
best regards,
max