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

[Jaakko Koivisto]

On Sun Sep 20, 2026 at 8:52 PM EEST, Jonathan Cameron wrote:
> On Sat, 19 Sep 2026 01:36:45 +0200
> Andreas Klinger <ak@xxxxxxxxxxxxx> wrote:
>
>> Hi Jaakko,
>> 
>> Jaakko Koivisto <jmatko@xxxxxx> schrieb am Fr, 18. Sep 16:40:
>> > -Turn the heating element off and enter idle mode.
>> > -Present the functionality as device attribute,
>> >  'echo 1 > turn_heater_off'.  
>> 
>> Instead of introducing a device specific attribute couldn't this be implemented
>> as standard power management operations (RUNTIME_PM_OPS)?
>
> May not apply in this case but normally the warm up time of these sorts of
> heaters are in the seconds.  No one wants that latency when they want
> a measurement.  As such normal runtime pm doesn't work - it needs to
> be a specific userspace opt in.

For SGP40 the heater must be on for 60 seconds for reliable data, and up
to 60 minutes to reach all datasheet specs. These are very long times, so I
would not want to turn the heater off automatically.

> We have defined ABI for this though and this isn't it
> See Documentation/ABI/testing/sysfs-bus-iio (and more in -humidity)

Thanks, missed that there was already ABI for heaters. I will change to
use this instead.

While looking at this I noticed this driver is using mutex_lock() and
mutex_unlock(). Recent commits, e.g. eb60a24b35bfb9e85a272e561379833e49a12a79,
say using the newer guard() is the preferred way these days. Should this be
updated as well?

>
> Jonathan
>
>> 
>> Andreas
>> 
>> > 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@xxxxxx>
>> > ---
>> >  drivers/iio/chemical/sgp40.c | 28 ++++++++++++++++++++++++++++
>> >  1 file changed, 28 insertions(+)
>> > 
>> > diff --git a/drivers/iio/chemical/sgp40.c b/drivers/iio/chemical/sgp40.c
>> > index 28d5e737d1dc..a4fc5c778303 100644
>> > --- a/drivers/iio/chemical/sgp40.c
>> > +++ b/drivers/iio/chemical/sgp40.c
>> > @@ -29,6 +29,7 @@
>> >   * by writing to the out values of temp and humidityrelative.
>> >   */
>> >  
>> > +#include "linux/device.h"
>> >  #include <linux/delay.h>
>> >  #include <linux/crc8.h>
>> >  #include <linux/module.h>
>> > @@ -259,6 +260,21 @@ 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_command turn_off = {.command = {0x36, 0x15}};
>> > +
>> > +	ret = i2c_master_send(client, (char*)&turn_off, sizeof(turn_off.command));
>> > +	if (ret != sizeof(turn_off.command)) {
>> > +		dev_err(data->dev, "i2c_master_send ret: %d, expected %zu", ret, sizeof(turn_off.command));
>> > +		return -EIO;
>> > +	}
>> > +	msleep(1);
>> > +	return 0;
>> > +}
>> > +
>> >  static int sgp40_measure_resistance_raw(struct sgp40_data *data, u16 *resistance_raw)
>> >  {
>> >  	int ret;
>> > @@ -417,10 +433,22 @@ static ssize_t serial_number_show(struct device *dev,
>> >  	return sysfs_emit_at(buf, 0, "%llu\n", data->serial_number);
>> >  }
>> >  
>> > +static ssize_t turn_heater_off_store(struct device *dev,
>> > +				     struct device_attribute *attr,
>> > +				     const char *buf, size_t len)
>> > +{
>> > +	struct sgp40_data *data = iio_priv(dev_to_iio_dev(dev));
>> > +	sgp40_turn_heater_off(data);
>> > +
>> > +	return len;
>> > +}
>> > +
>> >  static IIO_DEVICE_ATTR_RO(serial_number, 0);
>> > +static IIO_DEVICE_ATTR_WO(turn_heater_off, 0);
>> >  
>> >  static struct attribute *sgp40_attributes[] = {
>> >  	&iio_dev_attr_serial_number.dev_attr.attr,
>> > +	&iio_dev_attr_turn_heater_off.dev_attr.attr,
>> >  	NULL
>> >  };
>> >  
>> > -- 
>> > 2.55.0
>> >   
>>