[PATCH v2 0/5] iio: chemical: sgp40: add funtions for serial number readback, self-test, and heater control.

[PATCH v2 0/5] iio: chemical: sgp40: add funtions for serial number readback, self-test, and heater control.

[Jaakko Koivisto]

Add commands for getting the Sensirion SGP40 VOC-sensor
serial number, controlling the device's internal heater,
and running the device self-test.

Replace the manual mutex_lock()/mutex_unlock() with guard(mutex).
Simplifies the locking patterns, making it easier to add new mutex-guarded
variable 'heater_enabled' in patch 5.
Extends the lock to cover i2c communications with the device.

Fixes issue with converting all i2c errors to EIO in
sgp40_measure_resistance_raw(). Included in the patchset as it was found
due to reviewer flagging the same issue in the v1 patches.

Changes in v2:
- Added patch for replacing mutex_lock()/mutex_unlock() with guard().

- Added guard against concurrent access to the device in
  sgp40_get_serial_number(), sgp40_execute_self_test(),
  and sgp40_turn_heater_off().

- Return EIO if CRC check fails in sgp40_get_serial_number(),
  sgp40_execute_self_test(), sgp40_and turn_heater_off().

- Fixed formatting issues, missing newlines, unnecessary debug-print and
  unnecessary 0LL constant, added comments for sleep times.

- Changed 1 ms sleeps in sgp40_get_serial_number() and
  sgp40_turn_heater_off() to use fsleep instead of msleep
  (checkpatch.pl warning against using short msleep)

- Removed storing serial-number in driver-data, instead retrieve it when
  requested.

- Use the existing serialnumber ABI for presenting the SN to userspace.

- Fixed error paths in sgp40_turn_heater_off(). In v1 positive
  numbers were returned for errors. Fail the driver probe if the self-test
  fails to run, or the response is not PASS or FAIL.

- Use the existing heater_enable ABI for heater control.

- Return any negative errors from i2c_master_send() without
  converting them to -EIO in get_serial_number, execute_self_test,
  and turn_heater_off.

- Added patch for fixing the i2c error conversion issue in
  sgp40_measure_resistance_raw().

- Log errors with dev_err_probe() instead of dev_err().

- Link to v1: https://lore.kernel.org/linux-iio/20260918134019.1101308-1-jmatko@utu.fi/

Jaakko Koivisto (5):
  iio: chemical: sgp40: Do not convert I2C errors to EIO
  iio: chemical: sgp40: Replace manual locking with guard()
  iio: chemical: sgp40: Implement get_serial_number-command
  iio: chemical: sgp40: Implement execute_self_test-command
  iio: chemical: sgp40: Implement turn_heater_off-command

 drivers/iio/chemical/sgp40.c | 231 +++++++++++++++++++++++++++++++----
 1 file changed, 210 insertions(+), 21 deletions(-)

-- 
2.56.0

[PATCH v2 1/5] iio: chemical: sgp40: Do not convert I2C errors to EIO

[Jaakko Koivisto]

Any negative error returned by i2c_master_send() in
sgp40_measure_resistance_raw() was caught by the
branch guarding against short sends, and the error
was subsequently replaced with -EIO.

Check for any negative errors first and propagate
the error without conversion.

Fixes: 1081b9d97152 ("iio: chemical: Add driver support for sgp40")

Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
---
Tested by forcing i2c clock to ground. The resulting ETIMEDOUT and
EREMOTEIO are now correctly communicated to userspace instead of
converting to EIO.

 drivers/iio/chemical/sgp40.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
index b2b5e32a9eb2..3a239fb6847f 100644
--- a/drivers/iio/chemical/sgp40.c
+++ b/drivers/iio/chemical/sgp40.c
@@ -187,6 +187,8 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
 	mutex_unlock(&data->lock);
 
 	ret = i2c_master_send(client, (const char *)&tg, sizeof(tg));
+	if (ret < 0)
+		return ret;
 	if (ret != sizeof(tg)) {
 		dev_warn(data->dev, "i2c_master_send ret: %d sizeof: %zu\n", ret, sizeof(tg));
 		return -EIO;
-- 
2.56.0

[PATCH v2 2/5] iio: chemical: sgp40: Replace manual locking with guard()

[Jaakko Koivisto]

Replace manual mutex_lock() and mutex_unlock() with RAII-style
guard(mutex)().

Extend the lock in sgp40_measure_resistance_raw() to cover the
I2C send and receive cycle, preventing I/O-errors if the device
is currently running a command, or clobbering the output of a previous
command if a new command is issued during the sleep-period between
i2c_master_send() and i2c_master_recv().

Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
---
 drivers/iio/chemical/sgp40.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
index 3a239fb6847f..080d51f9f53c 100644
--- a/drivers/iio/chemical/sgp40.c
+++ b/drivers/iio/chemical/sgp40.c
@@ -148,9 +148,8 @@ static int sgp40_calc_voc(struct sgp40_data *data, u16 resistance_raw, int *voc)
 	u32 exp = 0;
 
 	/* we calculate as a multiple of 16384 (2^14) */
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
 	x = ((int)resistance_raw - data->res_calibbias) * 106;
-	mutex_unlock(&data->lock);
 
 	/* voc = 500 / (1 + e^x) */
 	exp = sgp40_exp(x, SGP40_CALC_POWER, 18);
@@ -172,7 +171,7 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
 	struct sgp40_tg_measure tg = {.command = {0x26, 0x0F}};
 	struct sgp40_tg_result tgres;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
 
 	ticks = (data->rht / 10) * 65535 / 10000;
 	ticks16 = (u16)clamp(ticks, 0u, 65535u); /* clamp between 0 .. 100 %rH */
@@ -184,8 +183,6 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
 	tg.temp_ticks = cpu_to_be16(ticks16);
 	tg.temp_crc = crc8(sgp40_crc8_table, (u8 *)&tg.temp_ticks, 2, SGP40_CRC8_INIT);
 
-	mutex_unlock(&data->lock);
-
 	ret = i2c_master_send(client, (const char *)&tg, sizeof(tg));
 	if (ret < 0)
 		return ret;
@@ -232,16 +229,16 @@ static int sgp40_read_raw(struct iio_dev *indio_dev,
 
 			*val = resistance_raw;
 			return IIO_VAL_INT;
-		case IIO_TEMP:
-			mutex_lock(&data->lock);
+		case IIO_TEMP: {
+			guard(mutex)(&data->lock);
 			*val = data->temp;
-			mutex_unlock(&data->lock);
 			return IIO_VAL_INT;
-		case IIO_HUMIDITYRELATIVE:
-			mutex_lock(&data->lock);
+		}
+		case IIO_HUMIDITYRELATIVE: {
+			guard(mutex)(&data->lock);
 			*val = data->rht;
-			mutex_unlock(&data->lock);
 			return IIO_VAL_INT;
+		}
 		default:
 			return -EINVAL;
 		}
@@ -263,11 +260,11 @@ static int sgp40_read_raw(struct iio_dev *indio_dev,
 		*val2 = ((voc % (1 << SGP40_CALC_POWER)) * 244) / (1 << (SGP40_CALC_POWER - 12));
 		dev_dbg(data->dev, "voc: %d val: %d.%06d\n", voc, *val, *val2);
 		return IIO_VAL_INT_PLUS_MICRO;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		mutex_lock(&data->lock);
+	case IIO_CHAN_INFO_CALIBBIAS: {
+		guard(mutex)(&data->lock);
 		*val = data->res_calibbias;
-		mutex_unlock(&data->lock);
 		return IIO_VAL_INT;
+	}
 	default:
 		return -EINVAL;
 	}
@@ -279,6 +276,8 @@ static int sgp40_write_raw(struct iio_dev *indio_dev,
 {
 	struct sgp40_data *data = iio_priv(indio_dev);
 
+	guard(mutex)(&data->lock);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		switch (chan->type) {
@@ -286,17 +285,13 @@ static int sgp40_write_raw(struct iio_dev *indio_dev,
 			if ((val < -45000) || (val > 130000))
 				return -EINVAL;
 
-			mutex_lock(&data->lock);
 			data->temp = val;
-			mutex_unlock(&data->lock);
 			return 0;
 		case IIO_HUMIDITYRELATIVE:
 			if ((val < 0) || (val > 100000))
 				return -EINVAL;
 
-			mutex_lock(&data->lock);
 			data->rht = val;
-			mutex_unlock(&data->lock);
 			return 0;
 		default:
 			return -EINVAL;
@@ -305,9 +300,7 @@ static int sgp40_write_raw(struct iio_dev *indio_dev,
 		if ((val < 20000) || (val > 52768))
 			return -EINVAL;
 
-		mutex_lock(&data->lock);
 		data->res_calibbias = val;
-		mutex_unlock(&data->lock);
 		return 0;
 	}
 	return -EINVAL;
-- 
2.56.0

[PATCH v2 3/5] iio: chemical: sgp40: Implement get_serial_number-command

[Jaakko Koivisto]

Add functionality for retrieving SGP40 sensor serial number
and present it to the userspace using serialnumber ABI.

Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
---
Currently only the bno055 driver uses the serialnumber ABI, and presents
the SN in hex. Documentation gives hex output as example format, not hard
requirement, but should we use the same format here anyway?

 drivers/iio/chemical/sgp40.c | 76 ++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
index 080d51f9f53c..e0a300720fce 100644
--- a/drivers/iio/chemical/sgp40.c
+++ b/drivers/iio/chemical/sgp40.c
@@ -35,6 +35,7 @@
 #include <linux/mutex.h>
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
 
 /*
  * floating point calculation of voc is done as integer
@@ -70,6 +71,18 @@ struct sgp40_tg_result {
 	u8	res_crc;
 } __packed;
 
+/*
+ * Datasheet table 16. Serial number is given as 48-bit value 0xAAAABBBBCCCC.
+ */
+struct sgp40_serial_number_result {
+	__be16	A;
+	u8	A_crc;
+	__be16	B;
+	u8	B_crc;
+	__be16	C;
+	u8	C_crc;
+} __packed;
+
 static const struct iio_chan_spec sgp40_channels[] = {
 	{
 		.type = IIO_CONCENTRATION,
@@ -161,6 +174,41 @@ static int sgp40_calc_voc(struct sgp40_data *data, u16 resistance_raw, int *voc)
 	return 0;
 }
 
+static int sgp40_get_serial_number(struct sgp40_data *data, u64 *serial_number)
+{
+	int ret;
+	struct i2c_client *client = data->client;
+	struct sgp40_tg_measure get_sn = { .command = {0x36, 0x82} };
+	struct sgp40_serial_number_result res;
+
+	guard(mutex)(&data->lock);
+
+	ret = i2c_master_send(client, (char *)&get_sn, sizeof(get_sn.command));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(get_sn.command))
+		return dev_err_probe(data->dev, -EIO,
+		       "i2c_master_send ret: %d, expected %zu\n", ret, sizeof(get_sn.command));
+
+	fsleep(1000); /* Max duration for get_serial_number command. Datasheet table 8. */
+
+	ret = i2c_master_recv(client, (char *)&res, sizeof(res));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(res))
+		return dev_err_probe(data->dev, -EIO,
+		       "i2c_master_recv ret: %d, expected: %zu\n", ret, sizeof(res));
+
+	if (crc8(sgp40_crc8_table, (u8 *)&res.A, 2, SGP40_CRC8_INIT) != res.A_crc ||
+	    crc8(sgp40_crc8_table, (u8 *)&res.B, 2, SGP40_CRC8_INIT) != res.B_crc ||
+	    crc8(sgp40_crc8_table, (u8 *)&res.C, 2, SGP40_CRC8_INIT) != res.C_crc)
+		return dev_err_probe(data->dev, -EIO, "CRC error in get_serial_number\n");
+
+	*serial_number = ((u64)be16_to_cpu(res.A) << 32) | ((u64)be16_to_cpu(res.B) << 16) | (u64)be16_to_cpu(res.C);
+
+	return 0;
+}
+
 static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance_raw)
 {
 	int ret;
@@ -306,9 +354,37 @@ static int sgp40_write_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static ssize_t serialnumber_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	int ret;
+	u64 serial_number;
+	struct sgp40_data *data = iio_priv(dev_to_iio_dev(dev));
+
+	ret = sgp40_get_serial_number(data, &serial_number);
+	if (ret < 0)
+		return ret;
+
+	return sysfs_emit_at(buf, 0, "%llu\n", serial_number);
+}
+
+static IIO_DEVICE_ATTR_RO(serialnumber, 0);
+
+static struct attribute *sgp40_attributes[] = {
+	&iio_dev_attr_serialnumber.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group sgp40_attribute_group = {
+	.attrs = sgp40_attributes,
+};
+
+
 static const struct iio_info sgp40_info = {
 	.read_raw	= sgp40_read_raw,
 	.write_raw	= sgp40_write_raw,
+	.attrs		= &sgp40_attribute_group,
 };
 
 static int sgp40_probe(struct i2c_client *client)
-- 
2.56.0

[PATCH v2 4/5] iio: chemical: sgp40: Implement execute_self_test-command

[Jaakko Koivisto]

Add functionality for running the SGP40 sensor self-test, testing
the heater and MOX-material.

Datasheet does not clearly state if the chip is completely unusable if
the test fails. Because of this the driver will try to use the chip
normally even if the self-test has failed.

If the test fails to run, the driver probe is aborted.

Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
---
Is it ok to fail the probe if the self-test fails? Currently the driver
will probe succesfully even if the device is powered down, or does
not exist at all, so failing the probe is rather significant change
from current behaviour.

Case where self-test reports FAIL was tested using a MCU
mimicking SGP40 instead of real device, due to problems finding any
failed SPG40s.

 drivers/iio/chemical/sgp40.c | 58 ++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
index e0a300720fce..278390bc6b04 100644
--- a/drivers/iio/chemical/sgp40.c
+++ b/drivers/iio/chemical/sgp40.c
@@ -83,6 +83,19 @@ struct sgp40_serial_number_result {
 	u8	C_crc;
 } __packed;
 
+/*
+ * Datasheet table 13.
+ */
+#define SGP40_SELF_TEST_PASS 0xD4
+#define SGP40_SELF_TEST_FAIL 0x4B
+
+struct sgp40_self_test_result {
+	u8	data;
+	u8	_ignore;
+	u8	crc;
+} __packed;
+
+
 static const struct iio_chan_spec sgp40_channels[] = {
 	{
 		.type = IIO_CONCENTRATION,
@@ -209,6 +222,47 @@ static int sgp40_get_serial_number(struct sgp40_data *data, u64 *serial_number)
 	return 0;
 }
 
+static int sgp40_execute_self_test(struct sgp40_data *data)
+{
+	int ret;
+	struct i2c_client *client = data->client;
+	struct sgp40_tg_measure run_test = { .command = {0x28, 0x0E} };
+	struct sgp40_self_test_result res;
+
+	guard(mutex)(&data->lock);
+
+	ret = i2c_master_send(client, (char *)&run_test, sizeof(run_test.command));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(run_test.command))
+		return dev_err_probe(data->dev, -EIO,
+		       "i2c_master_send ret: %d, expected %zu\n", ret, sizeof(run_test.command));
+
+	msleep(320); /* Max duration for self-test command. Datasheet table 8. */
+
+	ret = i2c_master_recv(client, (char *)&res, sizeof(res));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(res))
+		return dev_err_probe(data->dev, -EIO,
+		       "i2c_master_recv ret: %d, expected: %zu\n", ret, sizeof(res));
+
+	if (crc8(sgp40_crc8_table, (u8 *)&res, 2, SGP40_CRC8_INIT) != res.crc)
+		return dev_err_probe(data->dev, -EIO, "CRC error in execute_self_test\n");
+
+	dev_dbg(data->dev, "self test result: 0x%x\n", res.data);
+	switch (res.data) {
+	case SGP40_SELF_TEST_PASS:
+		return 0;
+	case SGP40_SELF_TEST_FAIL:
+		dev_warn(data->dev, "Device self-test failed, check data validity\n");
+		return 0;
+	default:
+		dev_err(data->dev, "Invalid self-test response from device\n");
+		return -EIO;
+	}
+}
+
 static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance_raw)
 {
 	int ret;
@@ -418,6 +472,10 @@ static int sgp40_probe(struct i2c_client *client)
 	indio_dev->channels = sgp40_channels;
 	indio_dev->num_channels = ARRAY_SIZE(sgp40_channels);
 
+	ret = sgp40_execute_self_test(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to run device self-test\n");
+
 	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		dev_err(dev, "failed to register iio device\n");
-- 
2.56.0

Re: [PATCH v2 1/5] iio: chemical: sgp40: Do not convert I2C errors to EIO

[Joshua Crofts]

On Mon, 2 Nov 2026 11:12:54 +0200
Jaakko Koivisto <jmatko@utu.fi> wrote:

> Any negative error returned by i2c_master_send() in
> sgp40_measure_resistance_raw() was caught by the
> branch guarding against short sends, and the error
> was subsequently replaced with -EIO.

Please wrap the commit message to ~75 characters per line next
time. Otherwise LGTM.

Reviewed-by: Joshua Crofts <crofts@kernel.org>

> Check for any negative errors first and propagate
> the error without conversion.
> 
> Fixes: 1081b9d97152 ("iio: chemical: Add driver support for sgp40")
>

Also, no blank lines in the tag block.

> Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
> ---
> Tested by forcing i2c clock to ground. The resulting ETIMEDOUT and
> EREMOTEIO are now correctly communicated to userspace instead of
> converting to EIO.
> 
>  drivers/iio/chemical/sgp40.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
> index b2b5e32a9eb2..3a239fb6847f 100644
> --- a/drivers/iio/chemical/sgp40.c
> +++ b/drivers/iio/chemical/sgp40.c
> @@ -187,6 +187,8 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
>  	mutex_unlock(&data->lock);
>  
>  	ret = i2c_master_send(client, (const char *)&tg, sizeof(tg));
> +	if (ret < 0)
> +		return ret;
>  	if (ret != sizeof(tg)) {
>  		dev_warn(data->dev, "i2c_master_send ret: %d sizeof: %zu\n", ret, sizeof(tg));
>  		return -EIO;



-- 
Kind regards,
Joshua Crofts

[PATCH v2 5/5] iio: chemical: sgp40: Implement turn_heater_off-command

[Jaakko Koivisto]

Add functionality for turning the SGP40 sensor heating element
on or off manually. After turning the heater off, the sensor enters
idle mode. Track the state of the heater in the driver, as the device
does not expose current heater state via any register.

Turning the heater off saves approx. 2.5 mA compared to regular
operation. The heating element is automatically turned back on
when measurement is performed.

Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
---
 drivers/iio/chemical/sgp40.c | 62 +++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
index 278390bc6b04..87df8d79fe6e 100644
--- a/drivers/iio/chemical/sgp40.c
+++ b/drivers/iio/chemical/sgp40.c
@@ -54,7 +54,8 @@ struct sgp40_data {
 	int			rht;
 	int			temp;
 	int			res_calibbias;
-	/* Prevent concurrent access to rht, tmp, calibbias */
+	bool			heater_enabled;
+	/* Prevent concurrent access to rht, tmp, calibbias, heater_enabled */
 	struct mutex		lock;
 };
 
@@ -263,6 +264,27 @@ static int sgp40_execute_self_test(struct sgp40_data *data)
 	}
 }
 
+static int sgp40_turn_heater_off(struct sgp40_data *data)
+{
+	int ret;
+	struct i2c_client *client = data->client;
+	struct sgp40_tg_measure turn_off = { .command = {0x36, 0x15} };
+
+	guard(mutex)(&data->lock);
+
+	ret = i2c_master_send(client, (char *)&turn_off, sizeof(turn_off.command));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(turn_off.command))
+		return dev_err_probe(data->dev, -EIO,
+		       "i2c_master_send ret: %d, expected %zu", ret, sizeof(turn_off.command));
+
+	fsleep(1000); /* Max duration for command turn_heater_off. Datasheet table 8. */
+	data->heater_enabled = false;
+
+	return 0;
+}
+
 static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance_raw)
 {
 	int ret;
@@ -293,6 +315,7 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
 		return -EIO;
 	}
 	msleep(30);
+	data->heater_enabled = true;
 
 	ret = i2c_master_recv(client, (u8 *)&tgres, sizeof(tgres));
 	if (ret < 0)
@@ -423,10 +446,47 @@ static ssize_t serialnumber_show(struct device *dev,
 	return sysfs_emit_at(buf, 0, "%llu\n", serial_number);
 }
 
+static ssize_t heater_enable_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	int ret;
+	u16 __ignore;
+	bool val;
+	struct sgp40_data *data = iio_priv(dev_to_iio_dev(dev));
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	if (val)
+		ret = sgp40_measure_resistance_raw(data, &__ignore);
+	else
+		ret = sgp40_turn_heater_off(data);
+
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static ssize_t heater_enable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct sgp40_data *data = iio_priv(dev_to_iio_dev(dev));
+
+	guard(mutex)(&data->lock);
+
+	return sysfs_emit(buf, "%d\n", data->heater_enabled);
+}
+
 static IIO_DEVICE_ATTR_RO(serialnumber, 0);
+static IIO_DEVICE_ATTR_RW(heater_enable, 0);
 
 static struct attribute *sgp40_attributes[] = {
 	&iio_dev_attr_serialnumber.dev_attr.attr,
+	&iio_dev_attr_heater_enable.dev_attr.attr,
 	NULL
 };
 
-- 
2.56.0

Re: [PATCH v2 2/5] iio: chemical: sgp40: Replace manual locking with guard()

[Joshua Crofts]

On Mon, 2 Nov 2026 11:12:55 +0200
Jaakko Koivisto <jmatko@utu.fi> wrote:

> Replace manual mutex_lock() and mutex_unlock() with RAII-style
> guard(mutex)().
> 
> Extend the lock in sgp40_measure_resistance_raw() to cover the
> I2C send and receive cycle, preventing I/O-errors if the device
> is currently running a command, or clobbering the output of a previous
> command if a new command is issued during the sleep-period between
> i2c_master_send() and i2c_master_recv().
> 
> Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
> ---
>  drivers/iio/chemical/sgp40.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
> index 3a239fb6847f..080d51f9f53c 100644
> --- a/drivers/iio/chemical/sgp40.c
> +++ b/drivers/iio/chemical/sgp40.c
> @@ -148,9 +148,8 @@ static int sgp40_calc_voc(struct sgp40_data *data, u16 resistance_raw, int *voc)
>  	u32 exp = 0;

+ #include <linux/cleanup.h>

>  	/* we calculate as a multiple of 16384 (2^14) */
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);

Blank lines around any guard() statement. You want to emphasize the fact
that guard is being utilized.

>  	x = ((int)resistance_raw - data->res_calibbias) * 106;
> -	mutex_unlock(&data->lock);

If only a segment of the code has to hold a lock it's better to use
scoped_guard() instead of regular guard().

>  
>  	/* voc = 500 / (1 + e^x) */
>  	exp = sgp40_exp(x, SGP40_CALC_POWER, 18);
> @@ -172,7 +171,7 @@ static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance
>  	struct sgp40_tg_measure tg = {.command = {0x26, 0x0F}};
>  	struct sgp40_tg_result tgres;
>  

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2 3/5] iio: chemical: sgp40: Implement get_serial_number-command

[Joshua Crofts]

On Mon, 2 Nov 2026 11:12:56 +0200
Jaakko Koivisto <jmatko@utu.fi> wrote:

> Add functionality for retrieving SGP40 sensor serial number
> and present it to the userspace using serialnumber ABI.
> 
> Signed-off-by: Jaakko Koivisto <jmatko@utu.fi>
> ---
> Currently only the bno055 driver uses the serialnumber ABI, and presents
> the SN in hex. Documentation gives hex output as example format, not hard
> requirement, but should we use the same format here anyway?
> 
>  drivers/iio/chemical/sgp40.c | 76 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
> index 080d51f9f53c..e0a300720fce 100644
> --- a/drivers/iio/chemical/sgp40.c
> +++ b/drivers/iio/chemical/sgp40.c
> @@ -35,6 +35,7 @@
>  #include <linux/mutex.h>
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

Could you add an additional patch to your series adding any
missing includes? Just looking at this patch I can see that
types.h, asm/byteorder.h and delay.h are missing.

>  
>  /*
>   * floating point calculation of voc is done as integer
> @@ -70,6 +71,18 @@ struct sgp40_tg_result {
>  	u8	res_crc;
>  } __packed;
>  
> +/*
> + * Datasheet table 16. Serial number is given as 48-bit value 0xAAAABBBBCCCC.
> + */
> +struct sgp40_serial_number_result {
> +	__be16	A;
> +	u8	A_crc;
> +	__be16	B;
> +	u8	B_crc;
> +	__be16	C;
> +	u8	C_crc;
> +} __packed;
> +
>  static const struct iio_chan_spec sgp40_channels[] = {
>  	{
>  		.type = IIO_CONCENTRATION,
> @@ -161,6 +174,41 @@ static int sgp40_calc_voc(struct sgp40_data *data, u16 resistance_raw, int *voc)
>  	return 0;
>  }
>  
> +static int sgp40_get_serial_number(struct sgp40_data *data, u64 *serial_number)
> +{
> +	int ret;
> +	struct i2c_client *client = data->client;
> +	struct sgp40_tg_measure get_sn = { .command = {0x36, 0x82} };
> +	struct sgp40_serial_number_result res;

Reverse xmas tree order.

> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = i2c_master_send(client, (char *)&get_sn, sizeof(get_sn.command));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(get_sn.command))
> +		return dev_err_probe(data->dev, -EIO,

Sashiko points out that this function isn't called from probe(),
therefore it shouldn't use dev_err_probe().

Also, you can use a local variable for dev.

> +		       "i2c_master_send ret: %d, expected %zu\n", ret, sizeof(get_sn.command));
> +
> +	fsleep(1000); /* Max duration for get_serial_number command. Datasheet table 8. */
> +
> +	ret = i2c_master_recv(client, (char *)&res, sizeof(res));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(res))
> +		return dev_err_probe(data->dev, -EIO,
> +		       "i2c_master_recv ret: %d, expected: %zu\n", ret, sizeof(res));
> +
> +	if (crc8(sgp40_crc8_table, (u8 *)&res.A, 2, SGP40_CRC8_INIT) != res.A_crc ||
> +	    crc8(sgp40_crc8_table, (u8 *)&res.B, 2, SGP40_CRC8_INIT) != res.B_crc ||
> +	    crc8(sgp40_crc8_table, (u8 *)&res.C, 2, SGP40_CRC8_INIT) != res.C_crc)
> +		return dev_err_probe(data->dev, -EIO, "CRC error in get_serial_number\n");
> +
> +	*serial_number = ((u64)be16_to_cpu(res.A) << 32) | ((u64)be16_to_cpu(res.B) << 16) | (u64)be16_to_cpu(res.C);
> +	return 0;
> +}
> +
>  static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance_raw)
>  {
>  	int ret;
> @@ -306,9 +354,37 @@ static int sgp40_write_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> +static ssize_t serialnumber_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	int ret;
> +	u64 serial_number;
> +	struct sgp40_data *data = iio_priv(dev_to_iio_dev(dev));

Reverse xmas tree.

...


-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2 4/5] iio: chemical: sgp40: Implement execute_self_test-command

[Joshua Crofts]

On Mon, 2 Nov 2026 11:12:57 +0200
Jaakko Koivisto <jmatko@utu.fi> wrote:

...

>  static const struct iio_chan_spec sgp40_channels[] = {
>  	{
>  		.type = IIO_CONCENTRATION,
> @@ -209,6 +222,47 @@ static int sgp40_get_serial_number(struct sgp40_data *data, u64 *serial_number)
>  	return 0;
>  }
>  
> +static int sgp40_execute_self_test(struct sgp40_data *data)
> +{
> +	int ret;
> +	struct i2c_client *client = data->client;
> +	struct sgp40_tg_measure run_test = { .command = {0x28, 0x0E} };
> +	struct sgp40_self_test_result res;

Reverse xmas tree.

Otherwise LGTM.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2 5/5] iio: chemical: sgp40: Implement turn_heater_off-command

[Joshua Crofts]

On Mon, 2 Nov 2026 11:56:02 +0200
Jaakko Koivisto <jmatko@utu.fi> wrote:

...

> +static int sgp40_turn_heater_off(struct sgp40_data *data)
> +{
> +	int ret;
> +	struct i2c_client *client = data->client;
> +	struct sgp40_tg_measure turn_off = { .command = {0x36, 0x15} };

Reverse xmas tree.

> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = i2c_master_send(client, (char *)&turn_off, sizeof(turn_off.command));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(turn_off.command))
> +		return dev_err_probe(data->dev, -EIO,
> +		       "i2c_master_send ret: %d, expected %zu", ret, sizeof(turn_off.command));

Again, this function isn't called from probe(), yet it uses
dev_err_probe().

-- 
Kind regards,
Joshua Crofts