[PATCH v3 0/7]: iio: chemical: bme680: 2nd round of cleanup

[PATCH v3 0/7]: iio: chemical: bme680: 2nd round of cleanup

[Vasileios Amoiridis]

Changes in v3:

Removed applied patches 1,2,3,5,6,7,8

[PATCH v3 1/7]:
	- v2 4/13
	- Set mode of sensor with enum variable and remove macros

[PATCH v3 5/7]:
	- v2 11/13
	- removed regulators from being required, adjusted commit
	  message

[PATCH v3 7/7]:
	- v2 13/13
	- removed unecessary usage of runtime PM functions

---
v2: https://lore.kernel.org/linux-iio/20241021195316.58911-1-vassilisamir@gmail.com/
Changes in v2:

Generally, the patches were rearranged according to comments from Andy
in previous version in order to be more consistent. The refactoring of
the ambient temperature was dropped for now because it was a bit more
complicated than I thought and this series is already heavy enough.

[PATCH v2 01/13]:
	- New patch

[PATCH v2 02/13]:
	- v1 1/13
	- used "optimized" in commit message to not prompt for a fix.
	- added documentation of where this sleep comes from

[PATCH v2 03/13]:
	- v1 2/13
	- Fix indentation of array and removed extra whitespace.

[PATCH v2 04/13]:
	- v1 5/13
	- removed extra check inside the set_mode() function.

[PATCH v2 06/13]:
	- v1 1/13
	- removed indentation fixes which are fixed later since code is
	  changed in those lines in later commits.

[PATCH v2 09/13]:
	- v1 12/13
	- removed unnecessary debug messages
	- Used struture instead of buffer to push data to userspace

[PATCH v2 10/13]:
	- v1 13/13
	- used better naming
	- made channel index to -1

[PATCH v2 11/13]:
	- v1 06/13
	- removed device from trivial-devices

[PATCH v2 12/13]:
	- v1 07/13
	- use devm_regulator_bulk_get_enable()

[PATCH v2 13/13]:
	- v1 08/13
	- removed internal usage of dev structure
	- added missing header in both bme680_core.c and bme680.h
	- used devm_pm_runtime_enable

---
v1: https://lore.kernel.org/linux-iio/20241010210030.33309-1-vassilisamir@gmail.com

This patch series is continuing the work that started on [1] by
improving some small issues of the driver in the commits 1,2,3.

Commits 4,5 are refactorizing existing code.

Commits 6,7,8 are adding DT, regulator and PM support.

Commit 9 is refactorizing one macro to attribute.

Commit 10,11,12 are refactorizing the read/compensate functions
to become generic and add triggered buffer support.

Finally, commit 13 adds support for an *output* channel of type
IIO_CURRENT in order to preheat the plate that is used to measure the
quality of the air.

This and the previous series [1] started with the idea to add support
for the new bme688 device but due to the structure of the driver I
decided that it is better to restructure and improve some things before
adding extra funcitonalities.

[1]: https://lore.kernel.org/linux-iio/20240609233826.330516-1-vassilisamir@gmail.com

Vasileios Amoiridis (7):
  iio: chemical: bme680: refactorize set_mode() mode
  iio: chemical: bme680: Add SCALE and RAW channels
  iio: chemical: bme680: Add triggered buffer support
  iio: chemical: bme680: Add support for preheat current
  dt-bindings: iio: bosch,bme680: Add supply properties
  iio: chemical: bme680: add regulators
  iio: chemical: bme680: add power management

 .../bindings/iio/chemical/bosch,bme680.yaml   |  62 +++
 .../devicetree/bindings/trivial-devices.yaml  |   2 -
 drivers/iio/chemical/Kconfig                  |   2 +
 drivers/iio/chemical/bme680.h                 |   8 +-
 drivers/iio/chemical/bme680_core.c            | 375 ++++++++++++++++--
 drivers/iio/chemical/bme680_i2c.c             |   1 +
 drivers/iio/chemical/bme680_spi.c             |   1 +
 7 files changed, 423 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml


base-commit: c218214db564ca7d6885fa5859541a86197856c0
-- 
2.43.0

[PATCH v3 1/7] iio: chemical: bme680: refactorize set_mode() mode

[Vasileios Amoiridis]

Refactorize the set_mode() function to use an external enum that
describes the possible modes of the BME680 device instead of using
true/false variables for selecting SLEEPING/FORCED mode.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h      |  2 --
 drivers/iio/chemical/bme680_core.c | 31 ++++++++++++++----------------
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index f5be4516dde7..77136b55e7f6 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -27,8 +27,6 @@
 #define   BME680_OSRS_TEMP_MASK			GENMASK(7, 5)
 #define   BME680_OSRS_PRESS_MASK		GENMASK(4, 2)
 #define   BME680_MODE_MASK			GENMASK(1, 0)
-#define     BME680_MODE_FORCED			1
-#define     BME680_MODE_SLEEP			0
 
 #define BME680_REG_CONFIG			0x75
 #define   BME680_FILTER_MASK			GENMASK(4, 2)
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 6d11f9188367..5c2c327c4540 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -95,6 +95,12 @@ struct bme680_calib {
 	s8  range_sw_err;
 };
 
+/* values of CTRL_MEAS register */
+enum bme680_op_mode {
+	BME680_MODE_SLEEP = 0,
+	BME680_MODE_FORCED = 1,
+};
+
 struct bme680_data {
 	struct regmap *regmap;
 	struct bme680_calib bme680;
@@ -502,23 +508,16 @@ static u8 bme680_calc_heater_dur(u16 dur)
 	return durval;
 }
 
-static int bme680_set_mode(struct bme680_data *data, bool mode)
+static int bme680_set_mode(struct bme680_data *data, enum bme680_op_mode mode)
 {
 	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
-	if (mode) {
-		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
-					BME680_MODE_MASK, BME680_MODE_FORCED);
-		if (ret < 0)
-			dev_err(dev, "failed to set forced mode\n");
-
-	} else {
-		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
-					BME680_MODE_MASK, BME680_MODE_SLEEP);
-		if (ret < 0)
-			dev_err(dev, "failed to set sleep mode\n");
-
+	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
+				BME680_MODE_MASK, mode);
+	if (ret < 0) {
+		dev_err(dev, "failed to set ctrl_meas register\n");
+		return ret;
 	}
 
 	return ret;
@@ -613,8 +612,7 @@ static int bme680_gas_config(struct bme680_data *data)
 	int ret;
 	u8 heatr_res, heatr_dur;
 
-	/* Go to sleep */
-	ret = bme680_set_mode(data, false);
+	ret = bme680_set_mode(data, BME680_MODE_SLEEP);
 	if (ret < 0)
 		return ret;
 
@@ -745,8 +743,7 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
 
 	guard(mutex)(&data->lock);
 
-	/* set forced mode to trigger measurement */
-	ret = bme680_set_mode(data, true);
+	ret = bme680_set_mode(data, BME680_MODE_FORCED);
 	if (ret < 0)
 		return ret;
 
-- 
2.43.0

Re: [PATCH v3 1/7] iio: chemical: bme680: refactorize set_mode() mode

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:05 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Refactorize the set_mode() function to use an external enum that
> describes the possible modes of the BME680 device instead of using
> true/false variables for selecting SLEEPING/FORCED mode.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied and pushed out as testing to see if 0-day can find anything we missed.

Thanks,

J

[PATCH v3 2/7] iio: chemical: bme680: Add SCALE and RAW channels

[Vasileios Amoiridis]

Add SCALE,RAW channels to the device. Even though PROCESSED should be
kept for backwards compatibility add comment to avoid using it if the
value is not actually reported in IIO values.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680_core.c | 51 ++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 5c2c327c4540..ea1ee9964870 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -144,17 +144,26 @@ EXPORT_SYMBOL_NS(bme680_regmap_config, IIO_BME680);
 static const struct iio_chan_spec bme680_channels[] = {
 	{
 		.type = IIO_TEMP,
+		/* PROCESSED maintained for ABI backwards compatibility */
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 	},
 	{
 		.type = IIO_PRESSURE,
+		/* PROCESSED maintained for ABI backwards compatibility */
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 	},
 	{
 		.type = IIO_HUMIDITYRELATIVE,
+		/* PROCESSED maintained for ABI backwards compatibility */
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
 	},
 	{
@@ -787,6 +796,48 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_TEMP:
+			ret = bme680_read_temp(data, (s16 *)&chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			return IIO_VAL_INT;
+		case IIO_PRESSURE:
+			ret = bme680_read_press(data, &chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			return IIO_VAL_INT;
+		case IIO_HUMIDITYRELATIVE:
+			ret = bme680_read_humid(data, &chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_TEMP:
+			*val = 10;
+			return IIO_VAL_INT;
+		case IIO_PRESSURE:
+			*val = 1;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_HUMIDITYRELATIVE:
+			*val = 1;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		switch (chan->type) {
 		case IIO_TEMP:
-- 
2.43.0

Re: [PATCH v3 2/7] iio: chemical: bme680: Add SCALE and RAW channels

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:06 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add SCALE,RAW channels to the device. Even though PROCESSED should be
> kept for backwards compatibility add comment to avoid using it if the
> value is not actually reported in IIO values.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied.

[PATCH v3 3/7] iio: chemical: bme680: Add triggered buffer support

[Vasileios Amoiridis]

Add triggered buffer and soft timestamp support. The available scan mask
enables all the channels of the sensor in order to follow the operation of
the sensor. The sensor basically starts to capture from all channels
as long as it enters into FORCED mode.

The bulk read, reads a total of 15 registers from the sensor, 0x1D..0x2B.
Even though some of those registers are not reported in the register map
of the device, this is how the BME680 Sensor API [1] proposes to do it.
This allows to have one bulk read instead of multiple ones.

Link: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L1200
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/Kconfig       |   2 +
 drivers/iio/chemical/bme680.h      |   3 +
 drivers/iio/chemical/bme680_core.c | 137 ++++++++++++++++++++++++++++-
 3 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 6c87223f58d9..330fe0af946f 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -50,6 +50,8 @@ config BME680
 	select REGMAP
 	select BME680_I2C if I2C
 	select BME680_SPI if SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Bosch Sensortec BME680 sensor with
 	  temperature, pressure, humidity and gas sensing capability.
diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index 77136b55e7f6..a0a7794543c8 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -66,6 +66,9 @@
 /* Datasheet Section 1.1, Table 1 */
 #define BME680_STARTUP_TIME_US			2000
 
+#define BME680_NUM_CHANNELS			4
+#define BME680_NUM_BULK_READ_REGS		15
+
 /* Calibration Parameters */
 #define BME680_T2_LSB_REG	0x8A
 #define BME680_H2_MSB_REG	0xE1
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index ea1ee9964870..6df87383c243 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -16,8 +16,11 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #include <linux/unaligned.h>
 
@@ -101,6 +104,13 @@ enum bme680_op_mode {
 	BME680_MODE_FORCED = 1,
 };
 
+enum bme680_scan {
+	BME680_TEMP,
+	BME680_PRESS,
+	BME680_HUMID,
+	BME680_GAS,
+};
+
 struct bme680_data {
 	struct regmap *regmap;
 	struct bme680_calib bme680;
@@ -111,8 +121,13 @@ struct bme680_data {
 	u16 heater_dur;
 	u16 heater_temp;
 
+	struct {
+		s32 chan[4];
+		aligned_s64 ts;
+	} scan;
+
 	union {
-		u8 buf[3];
+		u8 buf[BME680_NUM_BULK_READ_REGS];
 		unsigned int check;
 		__be16 be16;
 		u8 bme680_cal_buf_1[BME680_CALIB_RANGE_1_LEN];
@@ -149,6 +164,13 @@ static const struct iio_chan_spec bme680_channels[] = {
 				      BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_PRESSURE,
@@ -157,6 +179,13 @@ static const struct iio_chan_spec bme680_channels[] = {
 				      BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_HUMIDITYRELATIVE,
@@ -165,11 +194,26 @@ static const struct iio_chan_spec bme680_channels[] = {
 				      BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE) |
 				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_RESISTANCE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
 static int bme680_read_calib(struct bme680_data *data,
@@ -920,6 +964,88 @@ static const struct iio_info bme680_info = {
 	.attrs = &bme680_attribute_group,
 };
 
+static const unsigned long bme680_avail_scan_masks[] = {
+	BIT(BME680_GAS) | BIT(BME680_HUMID) | BIT(BME680_PRESS) | BIT(BME680_TEMP),
+	0
+};
+
+static irqreturn_t bme680_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct bme680_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	u32 adc_temp, adc_press, adc_humid;
+	u16 adc_gas_res, gas_regs_val;
+	u8 gas_range;
+	s32 t_fine;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	ret = bme680_set_mode(data, BME680_MODE_FORCED);
+	if (ret < 0)
+		goto out;
+
+	ret = bme680_wait_for_eoc(data);
+	if (ret)
+		goto out;
+
+	/* Burst read data regs */
+	ret = regmap_bulk_read(data->regmap, BME680_REG_MEAS_STAT_0,
+			       data->buf, sizeof(data->buf));
+	if (ret) {
+		dev_err(dev, "failed to burst read sensor data\n");
+		goto out;
+	}
+	if (data->buf[0] & BME680_GAS_MEAS_BIT) {
+		dev_err(dev, "gas measurement incomplete\n");
+		goto out;
+	}
+
+	/* Temperature calculations */
+	adc_temp = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[5]));
+	if (adc_temp == BME680_MEAS_SKIPPED) {
+		dev_err(dev, "reading temperature skipped\n");
+		goto out;
+	}
+	data->scan.chan[0] = bme680_compensate_temp(data, adc_temp);
+	t_fine = bme680_calc_t_fine(data, adc_temp);
+
+	/* Pressure calculations */
+	adc_press = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[2]));
+	if (adc_press == BME680_MEAS_SKIPPED) {
+		dev_err(dev, "reading pressure skipped\n");
+		goto out;
+	}
+	data->scan.chan[1] = bme680_compensate_press(data, adc_press, t_fine);
+
+	/* Humidity calculations */
+	adc_humid = get_unaligned_be16(&data->buf[8]);
+	if (adc_humid == BME680_MEAS_SKIPPED) {
+		dev_err(dev, "reading humidity skipped\n");
+		goto out;
+	}
+	data->scan.chan[2] = bme680_compensate_humid(data, adc_humid, t_fine);
+
+	/* Gas calculations */
+	gas_regs_val = get_unaligned_be16(&data->buf[13]);
+	adc_gas_res = FIELD_GET(BME680_ADC_GAS_RES, gas_regs_val);
+	if ((gas_regs_val & BME680_GAS_STAB_BIT) == 0) {
+		dev_err(dev, "heater failed to reach the target temperature\n");
+		goto out;
+	}
+	gas_range = FIELD_GET(BME680_GAS_RANGE_MASK, gas_regs_val);
+	data->scan.chan[3] = bme680_compensate_gas(data, adc_gas_res, gas_range);
+
+	/* Push to buffer */
+	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+					   iio_get_time_ns(indio_dev));
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
 int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		      const char *name)
 {
@@ -938,6 +1064,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->name = name;
 	indio_dev->channels = bme680_channels;
 	indio_dev->num_channels = ARRAY_SIZE(bme680_channels);
+	indio_dev->available_scan_masks = bme680_avail_scan_masks;
 	indio_dev->info = &bme680_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
@@ -980,6 +1107,14 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		return dev_err_probe(dev, ret,
 				     "failed to set gas config data\n");
 
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      bme680_trigger_handler,
+					      NULL);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "iio triggered buffer setup failed\n");
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(bme680_core_probe, IIO_BME680);
-- 
2.43.0

Re: [PATCH v3 3/7] iio: chemical: bme680: Add triggered buffer support

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:07 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add triggered buffer and soft timestamp support. The available scan mask
> enables all the channels of the sensor in order to follow the operation of
> the sensor. The sensor basically starts to capture from all channels
> as long as it enters into FORCED mode.
> 
> The bulk read, reads a total of 15 registers from the sensor, 0x1D..0x2B.
> Even though some of those registers are not reported in the register map
> of the device, this is how the BME680 Sensor API [1] proposes to do it.
> This allows to have one bulk read instead of multiple ones.
> 
> Link: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L1200
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied with a couple of (to me) superfluous comments dropped.
> +static irqreturn_t bme680_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct bme680_data *data = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(data->regmap);
> +	u32 adc_temp, adc_press, adc_humid;
> +	u16 adc_gas_res, gas_regs_val;
> +	u8 gas_range;
> +	s32 t_fine;
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = bme680_set_mode(data, BME680_MODE_FORCED);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = bme680_wait_for_eoc(data);
> +	if (ret)
> +		goto out;
> +
> +	/* Burst read data regs */
This one dropped as kind of obvious from the code.
> +	ret = regmap_bulk_read(data->regmap, BME680_REG_MEAS_STAT_0,
> +			       data->buf, sizeof(data->buf));
> +	if (ret) {
> +		dev_err(dev, "failed to burst read sensor data\n");
> +		goto out;
> +	}
> +	if (data->buf[0] & BME680_GAS_MEAS_BIT) {
> +		dev_err(dev, "gas measurement incomplete\n");
> +		goto out;
> +	}
> +
> +	/* Temperature calculations */
> +	adc_temp = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[5]));
> +	if (adc_temp == BME680_MEAS_SKIPPED) {
> +		dev_err(dev, "reading temperature skipped\n");
> +		goto out;
> +	}
> +	data->scan.chan[0] = bme680_compensate_temp(data, adc_temp);
> +	t_fine = bme680_calc_t_fine(data, adc_temp);
> +
> +	/* Pressure calculations */
> +	adc_press = FIELD_GET(BME680_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[2]));
> +	if (adc_press == BME680_MEAS_SKIPPED) {
> +		dev_err(dev, "reading pressure skipped\n");
> +		goto out;
> +	}
> +	data->scan.chan[1] = bme680_compensate_press(data, adc_press, t_fine);
> +
> +	/* Humidity calculations */
> +	adc_humid = get_unaligned_be16(&data->buf[8]);
> +	if (adc_humid == BME680_MEAS_SKIPPED) {
> +		dev_err(dev, "reading humidity skipped\n");
> +		goto out;
> +	}
> +	data->scan.chan[2] = bme680_compensate_humid(data, adc_humid, t_fine);
> +
> +	/* Gas calculations */
> +	gas_regs_val = get_unaligned_be16(&data->buf[13]);
> +	adc_gas_res = FIELD_GET(BME680_ADC_GAS_RES, gas_regs_val);
> +	if ((gas_regs_val & BME680_GAS_STAB_BIT) == 0) {
> +		dev_err(dev, "heater failed to reach the target temperature\n");
> +		goto out;
> +	}
> +	gas_range = FIELD_GET(BME680_GAS_RANGE_MASK, gas_regs_val);
> +	data->scan.chan[3] = bme680_compensate_gas(data, adc_gas_res, gas_range);
> +
> +	/* Push to buffer */
This one is extremely obvious so dropped.

> +	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> +					   iio_get_time_ns(indio_dev));
> +out:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +

[PATCH v3 4/7] iio: chemical: bme680: Add support for preheat current

[Vasileios Amoiridis]

Add functionality to inject a specified amount of current to the heating
plate before the start of the gas measurement to allow the sensor to reach
faster to the requested temperature.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h      |  1 +
 drivers/iio/chemical/bme680_core.c | 41 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index a0a7794543c8..00ab89b3138b 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -42,6 +42,7 @@
 #define   BME680_RHRANGE_MASK			GENMASK(5, 4)
 #define BME680_REG_RES_HEAT_VAL			0x00
 #define   BME680_RSERROR_MASK			GENMASK(7, 4)
+#define BME680_REG_IDAC_HEAT_0			0x50
 #define BME680_REG_RES_HEAT_0			0x5A
 #define BME680_REG_GAS_WAIT_0			0x64
 #define BME680_ADC_GAS_RES			GENMASK(15, 6)
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 6df87383c243..6cc1cb9e0477 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -118,6 +118,7 @@ struct bme680_data {
 	u8 oversampling_temp;
 	u8 oversampling_press;
 	u8 oversampling_humid;
+	u8 preheat_curr_mA;
 	u16 heater_dur;
 	u16 heater_temp;
 
@@ -214,6 +215,12 @@ static const struct iio_chan_spec bme680_channels[] = {
 		},
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(4),
+	{
+		.type = IIO_CURRENT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.output = 1,
+		.scan_index = -1,
+	},
 };
 
 static int bme680_read_calib(struct bme680_data *data,
@@ -561,6 +568,12 @@ static u8 bme680_calc_heater_dur(u16 dur)
 	return durval;
 }
 
+/* Taken from datasheet, section 5.3.3 */
+static u8 bme680_calc_heater_preheat_current(u8 curr)
+{
+	return 8 * curr - 1;
+}
+
 static int bme680_set_mode(struct bme680_data *data, enum bme680_op_mode mode)
 {
 	struct device *dev = regmap_get_device(data->regmap);
@@ -659,6 +672,20 @@ static int bme680_chip_config(struct bme680_data *data)
 	return 0;
 }
 
+static int bme680_preheat_curr_config(struct bme680_data *data, u8 val)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	u8 heatr_curr;
+	int ret;
+
+	heatr_curr = bme680_calc_heater_preheat_current(val);
+	ret = regmap_write(data->regmap, BME680_REG_IDAC_HEAT_0, heatr_curr);
+	if (ret < 0)
+		dev_err(dev, "failed to write idac_heat_0 register\n");
+
+	return ret;
+}
+
 static int bme680_gas_config(struct bme680_data *data)
 {
 	struct device *dev = regmap_get_device(data->regmap);
@@ -687,6 +714,10 @@ static int bme680_gas_config(struct bme680_data *data)
 		return ret;
 	}
 
+	ret = bme680_preheat_curr_config(data, data->preheat_curr_mA);
+	if (ret)
+		return ret;
+
 	/* Enable the gas sensor and select heater profile set-point 0 */
 	ret = regmap_update_bits(data->regmap, BME680_REG_CTRL_GAS_1,
 				 BME680_RUN_GAS_MASK | BME680_NB_CONV_MASK,
@@ -939,6 +970,15 @@ static int bme680_write_raw(struct iio_dev *indio_dev,
 
 		return bme680_chip_config(data);
 	}
+	case IIO_CHAN_INFO_PROCESSED:
+	{
+		switch (chan->type) {
+		case IIO_CURRENT:
+			return bme680_preheat_curr_config(data, (u8)val);
+		default:
+			return -EINVAL;
+		}
+	}
 	default:
 		return -EINVAL;
 	}
@@ -1074,6 +1114,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	data->oversampling_temp = 8;  /* 8X oversampling rate */
 	data->heater_temp = 320; /* degree Celsius */
 	data->heater_dur = 150;  /* milliseconds */
+	data->preheat_curr_mA = 0;
 
 	ret = regmap_write(regmap, BME680_REG_SOFT_RESET, BME680_CMD_SOFTRESET);
 	if (ret < 0)
-- 
2.43.0

Re: [PATCH v3 4/7] iio: chemical: bme680: Add support for preheat current

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:08 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add functionality to inject a specified amount of current to the heating
> plate before the start of the gas measurement to allow the sensor to reach
> faster to the requested temperature.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied.

Thanks,

Jonathan

[PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Vasileios Amoiridis]

Extend dt-binding for BME680 gas sensor device. The device incorporates
as well temperature, pressure and relative humidity sensors.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 .../bindings/iio/chemical/bosch,bme680.yaml   | 62 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  2 -
 2 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
new file mode 100644
index 000000000000..0eac22e465e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BME680 Gas sensor
+
+maintainers:
+  - Vasileios Amoiridis <vassilisamir@gmail.com>
+
+description: >
+  BME680 is a gas sensor which combines relative humidity, barometric pressure,
+  ambient temperature and gas (VOC - Volatile Organic Compounds) measurements.
+
+  https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bme680-ds001.pdf
+
+properties:
+  compatible:
+    const: bosch,bme680
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+  vddio-supply: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bme680@77 {
+            compatible = "bosch,bme680";
+            reg = <0x77>;
+            vddio-supply = <&vddio>;
+            vdd-supply = <&vdd>;
+        };
+    };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bme680@0 {
+            compatible = "bosch,bme680";
+            reg = <0>;
+            spi-max-frequency = <500000>;
+            vddio-supply = <&vddio>;
+            vdd-supply = <&vdd>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 9bf0fb17a05e..b651826e2d21 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -55,8 +55,6 @@ properties:
           - atmel,atsha204a
             # BPA-RS600: Power Supply
           - blutek,bpa-rs600
-            # Bosch Sensortec pressure, temperature, humididty and VOC sensor
-          - bosch,bme680
             # CM32181: Ambient Light Sensor
           - capella,cm32181
             # CM3232: Ambient Light Sensor
-- 
2.43.0

Re: [PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:09 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Extend dt-binding for BME680 gas sensor device. The device incorporates
> as well temperature, pressure and relative humidity sensors.
This description should make it clear it is moving from trivial-devices.yaml

dt-bindings: iio: bosch,bme680: Move from trivial-bindings and add missing supplies.

Then say a little more on why you are moving it.

> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>

There was an open question on the previous version about
setting the supplies as required (which I see you've removed).
My understanding previously was that it is fine to make that change
in a binding if it reflects supplies that are required to be enabled
for the device to function at all.  If there were previously missing
that's a binding bug we should fix.

I'd like a clarification from the DT binding maintainers on that.
Obviously doesn't work for other users of dt bindings but in
Linux this would be fine as they were already on for any board
that worked and the regulator framework will through us a fake
regulator for cases like this.

https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/

Jonathan

https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/

> ---
>  .../bindings/iio/chemical/bosch,bme680.yaml   | 62 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 62 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> new file mode 100644
> index 000000000000..0eac22e465e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bosch BME680 Gas sensor
> +
> +maintainers:
> +  - Vasileios Amoiridis <vassilisamir@gmail.com>
> +
> +description: >
> +  BME680 is a gas sensor which combines relative humidity, barometric pressure,
> +  ambient temperature and gas (VOC - Volatile Organic Compounds) measurements.
> +
> +  https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bme680-ds001.pdf
> +
> +properties:
> +  compatible:
> +    const: bosch,bme680
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +  vddio-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bme680@77 {
> +            compatible = "bosch,bme680";
> +            reg = <0x77>;
> +            vddio-supply = <&vddio>;
> +            vdd-supply = <&vdd>;
> +        };
> +    };
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bme680@0 {
> +            compatible = "bosch,bme680";
> +            reg = <0>;
> +            spi-max-frequency = <500000>;
> +            vddio-supply = <&vddio>;
> +            vdd-supply = <&vdd>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 9bf0fb17a05e..b651826e2d21 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -55,8 +55,6 @@ properties:
>            - atmel,atsha204a
>              # BPA-RS600: Power Supply
>            - blutek,bpa-rs600
> -            # Bosch Sensortec pressure, temperature, humididty and VOC sensor
> -          - bosch,bme680
>              # CM32181: Ambient Light Sensor
>            - capella,cm32181
>              # CM3232: Ambient Light Sensor

Re: [PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Krzysztof Kozlowski]

On Sat, Nov 02, 2024 at 03:33:15PM +0000, Jonathan Cameron wrote:
> On Sat,  2 Nov 2024 14:13:09 +0100
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Extend dt-binding for BME680 gas sensor device. The device incorporates
> > as well temperature, pressure and relative humidity sensors.
> This description should make it clear it is moving from trivial-devices.yaml
> 
> dt-bindings: iio: bosch,bme680: Move from trivial-bindings and add missing supplies.
> 
> Then say a little more on why you are moving it.
> 
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> 
> There was an open question on the previous version about
> setting the supplies as required (which I see you've removed).
> My understanding previously was that it is fine to make that change
> in a binding if it reflects supplies that are required to be enabled
> for the device to function at all.  If there were previously missing
> that's a binding bug we should fix.
> 
> I'd like a clarification from the DT binding maintainers on that.
> Obviously doesn't work for other users of dt bindings but in
> Linux this would be fine as they were already on for any board
> that worked and the regulator framework will through us a fake
> regulator for cases like this.
> 
> https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/
> 
> Jonathan

That was Rob's objection so I will leave it to him, but putting my two
cents in for Linux it is not an ABI break because missing regulator
supplies are substituted with dummy ones. Unless something changed...

Best regards,
Krzysztof

Re: [PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Rob Herring]

On Sun, Nov 03, 2024 at 10:46:46AM +0100, Krzysztof Kozlowski wrote:
> On Sat, Nov 02, 2024 at 03:33:15PM +0000, Jonathan Cameron wrote:
> > On Sat,  2 Nov 2024 14:13:09 +0100
> > Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> > 
> > > Extend dt-binding for BME680 gas sensor device. The device incorporates
> > > as well temperature, pressure and relative humidity sensors.
> > This description should make it clear it is moving from trivial-devices.yaml
> > 
> > dt-bindings: iio: bosch,bme680: Move from trivial-bindings and add missing supplies.
> > 
> > Then say a little more on why you are moving it.
> > 
> > > 
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > 
> > There was an open question on the previous version about
> > setting the supplies as required (which I see you've removed).
> > My understanding previously was that it is fine to make that change
> > in a binding if it reflects supplies that are required to be enabled
> > for the device to function at all.  If there were previously missing
> > that's a binding bug we should fix.
> > 
> > I'd like a clarification from the DT binding maintainers on that.
> > Obviously doesn't work for other users of dt bindings but in
> > Linux this would be fine as they were already on for any board
> > that worked and the regulator framework will through us a fake
> > regulator for cases like this.
> > 
> > https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/
> > 
> > Jonathan
> 
> That was Rob's objection so I will leave it to him, but putting my two
> cents in for Linux it is not an ABI break because missing regulator
> supplies are substituted with dummy ones. Unless something changed...

Shrug. I don't think we're entirely consistent on this. If we're saying 
supplies are always required, then every device in trivial-devices.yaml 
is wrong. Since Linux handles them missing, you can also argue that 
supplies are never required.

I'd prefer not to special case regulators as an exception I have to 
remember. I have some rudimentary ABI checking I'm working on that 
checks for things like new required properties. Though it wouldn't catch 
this particular change given it moves the schema.

Rob

Re: [PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Vasileios Amoiridis]

On Mon, Nov 04, 2024 at 10:10:33AM -0600, Rob Herring wrote:
> On Sun, Nov 03, 2024 at 10:46:46AM +0100, Krzysztof Kozlowski wrote:
> > On Sat, Nov 02, 2024 at 03:33:15PM +0000, Jonathan Cameron wrote:
> > > On Sat,  2 Nov 2024 14:13:09 +0100
> > > Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> > > 
> > > > Extend dt-binding for BME680 gas sensor device. The device incorporates
> > > > as well temperature, pressure and relative humidity sensors.
> > > This description should make it clear it is moving from trivial-devices.yaml
> > > 
> > > dt-bindings: iio: bosch,bme680: Move from trivial-bindings and add missing supplies.
> > > 
> > > Then say a little more on why you are moving it.
> > > 
> > > > 
> > > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > 
> > > There was an open question on the previous version about
> > > setting the supplies as required (which I see you've removed).
> > > My understanding previously was that it is fine to make that change
> > > in a binding if it reflects supplies that are required to be enabled
> > > for the device to function at all.  If there were previously missing
> > > that's a binding bug we should fix.
> > > 
> > > I'd like a clarification from the DT binding maintainers on that.
> > > Obviously doesn't work for other users of dt bindings but in
> > > Linux this would be fine as they were already on for any board
> > > that worked and the regulator framework will through us a fake
> > > regulator for cases like this.
> > > 
> > > https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/
> > > 
> > > Jonathan
> > 
> > That was Rob's objection so I will leave it to him, but putting my two
> > cents in for Linux it is not an ABI break because missing regulator
> > supplies are substituted with dummy ones. Unless something changed...
> 
> Shrug. I don't think we're entirely consistent on this. If we're saying 
> supplies are always required, then every device in trivial-devices.yaml 
> is wrong. Since Linux handles them missing, you can also argue that 
> supplies are never required.
> 
> I'd prefer not to special case regulators as an exception I have to 
> remember. I have some rudimentary ABI checking I'm working on that 
> checks for things like new required properties. Though it wouldn't catch 
> this particular change given it moves the schema.
> 
> Rob

Hi Jonathan,

According to Rob's answer, do you think that we can move on with the
last 3 patches as they are or do you want some changes?

Cheers,
Vasilis

Re: [PATCH v3 5/7] dt-bindings: iio: bosch,bme680: Add supply properties

[Rob Herring]

On Mon, Nov 11, 2024 at 07:48:48PM +0100, Vasileios Amoiridis wrote:
> On Mon, Nov 04, 2024 at 10:10:33AM -0600, Rob Herring wrote:
> > On Sun, Nov 03, 2024 at 10:46:46AM +0100, Krzysztof Kozlowski wrote:
> > > On Sat, Nov 02, 2024 at 03:33:15PM +0000, Jonathan Cameron wrote:
> > > > On Sat,  2 Nov 2024 14:13:09 +0100
> > > > Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> > > > 
> > > > > Extend dt-binding for BME680 gas sensor device. The device incorporates
> > > > > as well temperature, pressure and relative humidity sensors.
> > > > This description should make it clear it is moving from trivial-devices.yaml
> > > > 
> > > > dt-bindings: iio: bosch,bme680: Move from trivial-bindings and add missing supplies.
> > > > 
> > > > Then say a little more on why you are moving it.
> > > > 
> > > > > 
> > > > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > > > 
> > > > There was an open question on the previous version about
> > > > setting the supplies as required (which I see you've removed).
> > > > My understanding previously was that it is fine to make that change
> > > > in a binding if it reflects supplies that are required to be enabled
> > > > for the device to function at all.  If there were previously missing
> > > > that's a binding bug we should fix.
> > > > 
> > > > I'd like a clarification from the DT binding maintainers on that.
> > > > Obviously doesn't work for other users of dt bindings but in
> > > > Linux this would be fine as they were already on for any board
> > > > that worked and the regulator framework will through us a fake
> > > > regulator for cases like this.
> > > > 
> > > > https://lore.kernel.org/all/20241022182451.00007ac0@Huawei.com/
> > > > 
> > > > Jonathan
> > > 
> > > That was Rob's objection so I will leave it to him, but putting my two
> > > cents in for Linux it is not an ABI break because missing regulator
> > > supplies are substituted with dummy ones. Unless something changed...
> > 
> > Shrug. I don't think we're entirely consistent on this. If we're saying 
> > supplies are always required, then every device in trivial-devices.yaml 
> > is wrong. Since Linux handles them missing, you can also argue that 
> > supplies are never required.
> > 
> > I'd prefer not to special case regulators as an exception I have to 
> > remember. I have some rudimentary ABI checking I'm working on that 
> > checks for things like new required properties. Though it wouldn't catch 
> > this particular change given it moves the schema.
> > 
> > Rob
> 
> Hi Jonathan,
> 
> According to Rob's answer, do you think that we can move on with the
> last 3 patches as they are or do you want some changes?

Please update the commit message as Jonathan requested and resend 
(though you might as well wait til the end of the merge window now).

Rob

[PATCH v3 6/7] iio: chemical: bme680: add regulators

[Vasileios Amoiridis]

Add support for the regulators described in the dt-binding.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 6cc1cb9e0477..31769e0df7c2 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -15,6 +15,7 @@
 #include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -111,6 +112,10 @@ enum bme680_scan {
 	BME680_GAS,
 };
 
+static const char *const bme680_supply_names[] = { "vdd", "vddio" };
+
+#define BME680_NUM_SUPPLIES ARRAY_SIZE(bme680_supply_names)
+
 struct bme680_data {
 	struct regmap *regmap;
 	struct bme680_calib bme680;
@@ -1116,6 +1121,14 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	data->heater_dur = 150;  /* milliseconds */
 	data->preheat_curr_mA = 0;
 
+	ret = devm_regulator_bulk_get_enable(dev, BME680_NUM_SUPPLIES,
+					     bme680_supply_names);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get and enable supplies.\n");
+
+	fsleep(BME680_STARTUP_TIME_US);
+
 	ret = regmap_write(regmap, BME680_REG_SOFT_RESET, BME680_CMD_SOFTRESET);
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "Failed to reset chip\n");
-- 
2.43.0

[PATCH v3 7/7] iio: chemical: bme680: add power management

[Vasileios Amoiridis]

Add runtime power management to the device.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h      |   2 +
 drivers/iio/chemical/bme680_core.c | 104 +++++++++++++++++++++++++++--
 drivers/iio/chemical/bme680_i2c.c  |   1 +
 drivers/iio/chemical/bme680_spi.c  |   1 +
 4 files changed, 101 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index 00ab89b3138b..7d86ed8b02e6 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -2,6 +2,7 @@
 #ifndef BME680_H_
 #define BME680_H_
 
+#include <linux/pm.h>
 #include <linux/regmap.h>
 
 #define BME680_REG_CHIP_ID			0xD0
@@ -80,6 +81,7 @@
 #define BME680_CALIB_RANGE_3_LEN               5
 
 extern const struct regmap_config bme680_regmap_config;
+extern const struct dev_pm_ops bme680_dev_pm_ops;
 
 int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		      const char *name);
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 31769e0df7c2..61d095f1d2f6 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -14,6 +14,8 @@
 #include <linux/device.h>
 #include <linux/log2.h>
 #include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -822,9 +824,9 @@ static int bme680_read_gas(struct bme680_data *data, int *comp_gas_res)
 	return 0;
 }
 
-static int bme680_read_raw(struct iio_dev *indio_dev,
-			   struct iio_chan_spec const *chan,
-			   int *val, int *val2, long mask)
+static int __bme680_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
 {
 	struct bme680_data *data = iio_priv(indio_dev);
 	int chan_val, ret;
@@ -937,14 +939,30 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int bme680_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+
+	pm_runtime_get_sync(dev);
+	ret = __bme680_read_raw(indio_dev, chan, val, val2, mask);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
 static bool bme680_is_valid_oversampling(int rate)
 {
 	return (rate > 0 && rate <= 16 && is_power_of_2(rate));
 }
 
-static int bme680_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val, int val2, long mask)
+static int __bme680_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
 {
 	struct bme680_data *data = iio_priv(indio_dev);
 
@@ -989,6 +1007,22 @@ static int bme680_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int bme680_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+
+	pm_runtime_get_sync(dev);
+	ret = __bme680_write_raw(indio_dev, chan, val, val2, mask);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
 static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
 
 static IIO_CONST_ATTR(oversampling_ratio_available,
@@ -1091,6 +1125,30 @@ static irqreturn_t bme680_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static int bme680_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+
+	pm_runtime_get_sync(dev);
+	return 0;
+}
+
+static int bme680_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops bme680_buffer_setup_ops = {
+	.preenable = bme680_buffer_preenable,
+	.postdisable = bme680_buffer_postdisable,
+};
+
 int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		      const char *name)
 {
@@ -1164,15 +1222,47 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,
 					      bme680_trigger_handler,
-					      NULL);
+					      &bme680_buffer_setup_ops);
 	if (ret)
 		return dev_err_probe(dev, ret,
 				     "iio triggered buffer setup failed\n");
 
+	/* Enable runtime PM */
+	pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(bme680_core_probe, IIO_BME680);
 
+static int bme680_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct bme680_data *data = iio_priv(indio_dev);
+
+	return bme680_set_mode(data, BME680_MODE_SLEEP);
+}
+
+static int bme680_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct bme680_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = bme680_chip_config(data);
+	if (ret)
+		return ret;
+
+	return bme680_gas_config(data);
+}
+
+EXPORT_RUNTIME_DEV_PM_OPS(bme680_dev_pm_ops, bme680_runtime_suspend,
+			  bme680_runtime_resume, NULL);
+
 MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
 MODULE_DESCRIPTION("Bosch BME680 Driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
index 7c4224d75955..9998d7fa3e98 100644
--- a/drivers/iio/chemical/bme680_i2c.c
+++ b/drivers/iio/chemical/bme680_i2c.c
@@ -51,6 +51,7 @@ static struct i2c_driver bme680_i2c_driver = {
 	.driver = {
 		.name			= "bme680_i2c",
 		.of_match_table		= bme680_of_i2c_match,
+		.pm = pm_ptr(&bme680_dev_pm_ops),
 	},
 	.probe = bme680_i2c_probe,
 	.id_table = bme680_i2c_id,
diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
index 7c54bd17d4b0..43d59544d903 100644
--- a/drivers/iio/chemical/bme680_spi.c
+++ b/drivers/iio/chemical/bme680_spi.c
@@ -154,6 +154,7 @@ static struct spi_driver bme680_spi_driver = {
 	.driver = {
 		.name			= "bme680_spi",
 		.of_match_table		= bme680_of_spi_match,
+		.pm = pm_ptr(&bme680_dev_pm_ops),
 	},
 	.probe = bme680_spi_probe,
 	.id_table = bme680_spi_id,
-- 
2.43.0

Re: [PATCH v3 7/7] iio: chemical: bme680: add power management

[Jonathan Cameron]

On Sat,  2 Nov 2024 14:13:11 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add runtime power management to the device.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
This and previous patch look fine to me.  Just need to clean up that
dt-binding patch and hopefully get a clear direction on whether to require the
supplies or not.

Thanks,

Jonathan

Re: [PATCH v3 7/7] iio: chemical: bme680: add power management

[Vasileios Amoiridis]

On Sat, Nov 02, 2024 at 03:35:12PM +0000, Jonathan Cameron wrote:
> On Sat,  2 Nov 2024 14:13:11 +0100
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Add runtime power management to the device.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> This and previous patch look fine to me.  Just need to clean up that
> dt-binding patch and hopefully get a clear direction on whether to require the
> supplies or not.
> 
> Thanks,
> 
> Jonathan

Hi Jonathan,

Thank you very much for the review once again!

I will wait as well to see how that evolves and I will send a new
version, thank you!

Cheers,
Vasilis