[PATCH v2 00/13]: chemical: bme680: 2nd set of cleanup

[PATCH v2 00/13]: chemical: bme680: 2nd set of cleanup

[Vasileios Amoiridis]

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 (13):
  iio: chemical: bme680: Fix missing header
  iio: chemical: bme680: optimize startup time
  iio: chemical: bme680: avoid using camel case
  iio: chemical: bme680: refactorize set_mode() mode
  iio: chemical: bme680: move to fsleep()
  iio: chemical: bme680: Fix indentation and unnecessary spaces
  iio: chemical: bme680: generalize read_*() functions
  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: add binding for BME680 driver
  iio: chemical: bme680: add regulators
  iio: chemical: bme680: add power management

 .../bindings/iio/chemical/bosch,bme680.yaml   |  64 +++
 .../devicetree/bindings/trivial-devices.yaml  |   2 -
 drivers/iio/chemical/Kconfig                  |   2 +
 drivers/iio/chemical/bme680.h                 |  11 +-
 drivers/iio/chemical/bme680_core.c            | 505 +++++++++++++++---
 5 files changed, 508 insertions(+), 76 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml

-- 
2.43.0

[PATCH v2 01/13] iio: chemical: bme680: Fix missing header

[Vasileios Amoiridis]

Add the linux/regmap.h header since the struct regmap_config is used
in this file.

Cc: <Stable@vger.kernel.org>
Fixes: 1b3bd8592780 ("iio: chemical: Add support for Bosch BME680 sensor")
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index b2c547ac8d34..dc9ff477da34 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -2,6 +2,8 @@
 #ifndef BME680_H_
 #define BME680_H_
 
+#include <linux/regmap.h>
+
 #define BME680_REG_CHIP_ID			0xD0
 #define   BME680_CHIP_ID_VAL			0x61
 #define BME680_REG_SOFT_RESET			0xE0
-- 
2.43.0

Re: [PATCH v2 01/13] iio: chemical: bme680: Fix missing header

[Greg KH]

On Mon, Oct 21, 2024 at 09:53:04PM +0200, Vasileios Amoiridis wrote:
> Add the linux/regmap.h header since the struct regmap_config is used
> in this file.
> 
> Cc: <Stable@vger.kernel.org>
> Fixes: 1b3bd8592780 ("iio: chemical: Add support for Bosch BME680 sensor")
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index b2c547ac8d34..dc9ff477da34 100644
> --- a/drivers/iio/chemical/bme680.h
> +++ b/drivers/iio/chemical/bme680.h
> @@ -2,6 +2,8 @@
>  #ifndef BME680_H_
>  #define BME680_H_
>  
> +#include <linux/regmap.h>
> +
>  #define BME680_REG_CHIP_ID			0xD0
>  #define   BME680_CHIP_ID_VAL			0x61
>  #define BME680_REG_SOFT_RESET			0xE0

Why is this needed in a stable release?  Does it fix a bug?

Re: [PATCH v2 01/13] iio: chemical: bme680: Fix missing header

[Jonathan Cameron]

On Mon, 21 Oct 2024 22:13:17 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Oct 21, 2024 at 09:53:04PM +0200, Vasileios Amoiridis wrote:
> > Add the linux/regmap.h header since the struct regmap_config is used
> > in this file.
> > 
> > Cc: <Stable@vger.kernel.org>
> > Fixes: 1b3bd8592780 ("iio: chemical: Add support for Bosch BME680 sensor")
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  drivers/iio/chemical/bme680.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> > index b2c547ac8d34..dc9ff477da34 100644
> > --- a/drivers/iio/chemical/bme680.h
> > +++ b/drivers/iio/chemical/bme680.h
> > @@ -2,6 +2,8 @@
> >  #ifndef BME680_H_
> >  #define BME680_H_
> >  
> > +#include <linux/regmap.h>
> > +
> >  #define BME680_REG_CHIP_ID			0xD0
> >  #define   BME680_CHIP_ID_VAL			0x61
> >  #define BME680_REG_SOFT_RESET			0xE0  
> 
> Why is this needed in a stable release?  Does it fix a bug?
> 

Indeed this is just tidying up.  I've applied but dropped the tag and changed
title to Add missing.... as no reason for this to get backported that I know
of.

[PATCH v2 02/13] iio: chemical: bme680: optimize startup time

[Vasileios Amoiridis]

According to datasheet's Section 1.1, Table 1, the startup time for the
device is 2ms and not 5ms.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index dc9ff477da34..f5be4516dde7 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -65,7 +65,8 @@
 
 #define BME680_MEAS_TRIM_MASK			GENMASK(24, 4)
 
-#define BME680_STARTUP_TIME_US			5000
+/* Datasheet Section 1.1, Table 1 */
+#define BME680_STARTUP_TIME_US			2000
 
 /* Calibration Parameters */
 #define BME680_T2_LSB_REG	0x8A
-- 
2.43.0

Re: [PATCH v2 02/13] iio: chemical: bme680: optimize startup time

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:05 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> According to datasheet's Section 1.1, Table 1, the startup time for the
> device is 2ms and not 5ms.
Ok.  If this were just in the probe path  (which it is today)
I would not bother with the risk for a 3msec potential saving.
However, you are going to reuse it in runtime resume where the effects
will be more obvious. 

Hence applied to the togreg branch of iio.git and pushed out as testing.
*fingers crossed* no one has out of spec wiring that needs a little longer
than the spec says.

Jonathan

> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index dc9ff477da34..f5be4516dde7 100644
> --- a/drivers/iio/chemical/bme680.h
> +++ b/drivers/iio/chemical/bme680.h
> @@ -65,7 +65,8 @@
>  
>  #define BME680_MEAS_TRIM_MASK			GENMASK(24, 4)
>  
> -#define BME680_STARTUP_TIME_US			5000
> +/* Datasheet Section 1.1, Table 1 */
> +#define BME680_STARTUP_TIME_US			2000
>  
>  /* Calibration Parameters */
>  #define BME680_T2_LSB_REG	0x8A

[PATCH v2 03/13] iio: chemical: bme680: avoid using camel case

[Vasileios Amoiridis]

Rename camel case variable, as checkpatch.pl complains.

While at it, fix also the indentation of the array for readability.

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

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 0b96534c6867..d228f90b4dc6 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -438,15 +438,15 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
 	u32 calc_gas_res;
 
 	/* Look up table for the possible gas range values */
-	static const u32 lookupTable[16] = {2147483647u, 2147483647u,
-				2147483647u, 2147483647u, 2147483647u,
-				2126008810u, 2147483647u, 2130303777u,
-				2147483647u, 2147483647u, 2143188679u,
-				2136746228u, 2147483647u, 2126008810u,
-				2147483647u, 2147483647u};
+	static const u32 lookup_table[16] = {
+		2147483647u, 2147483647u, 2147483647u, 2147483647u,
+		2147483647u, 2126008810u, 2147483647u, 2130303777u,
+		2147483647u, 2147483647u, 2143188679u, 2136746228u,
+		2147483647u, 2126008810u, 2147483647u, 2147483647u
+	};
 
 	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
-			((s64) lookupTable[gas_range])) >> 16;
+			((s64)lookup_table[gas_range])) >> 16;
 	var2 = ((gas_res_adc << 15) - 16777216) + var1;
 	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
 	var3 += (var2 >> 1);
-- 
2.43.0

Re: [PATCH v2 03/13] iio: chemical: bme680: avoid using camel case

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:06 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Rename camel case variable, as checkpatch.pl complains.
> 
> While at it, fix also the indentation of the array for readability.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied.

> ---
>  drivers/iio/chemical/bme680_core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index 0b96534c6867..d228f90b4dc6 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -438,15 +438,15 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
>  	u32 calc_gas_res;
>  
>  	/* Look up table for the possible gas range values */
> -	static const u32 lookupTable[16] = {2147483647u, 2147483647u,
> -				2147483647u, 2147483647u, 2147483647u,
> -				2126008810u, 2147483647u, 2130303777u,
> -				2147483647u, 2147483647u, 2143188679u,
> -				2136746228u, 2147483647u, 2126008810u,
> -				2147483647u, 2147483647u};
> +	static const u32 lookup_table[16] = {
> +		2147483647u, 2147483647u, 2147483647u, 2147483647u,
> +		2147483647u, 2126008810u, 2147483647u, 2130303777u,
> +		2147483647u, 2147483647u, 2143188679u, 2136746228u,
> +		2147483647u, 2126008810u, 2147483647u, 2147483647u
> +	};
>  
>  	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
> -			((s64) lookupTable[gas_range])) >> 16;
> +			((s64)lookup_table[gas_range])) >> 16;
>  	var2 = ((gas_res_adc << 15) - 16777216) + var1;
>  	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
>  	var3 += (var2 >> 1);

[PATCH v2 04/13] 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_core.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index d228f90b4dc6..9002519d2c33 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -95,6 +95,11 @@ struct bme680_calib {
 	s8  range_sw_err;
 };
 
+enum bme680_op_mode {
+	BME680_SLEEP,
+	BME680_FORCED,
+};
+
 struct bme680_data {
 	struct regmap *regmap;
 	struct bme680_calib bme680;
@@ -502,23 +507,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;
@@ -615,8 +613,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_SLEEP);
 	if (ret < 0)
 		return ret;
 
@@ -756,8 +753,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_FORCED);
 	if (ret < 0)
 		return ret;
 
-- 
2.43.0

Re: [PATCH v2 04/13] iio: chemical: bme680: refactorize set_mode() mode

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:07 +0200
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>
Improves readability so fair enough.

Applied.

Re: [PATCH v2 04/13] iio: chemical: bme680: refactorize set_mode() mode

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:07 +0200
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>
I changed my mind on this one...

> ---
>  drivers/iio/chemical/bme680_core.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index d228f90b4dc6..9002519d2c33 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -95,6 +95,11 @@ struct bme680_calib {
>  	s8  range_sw_err;
>  };
>  
> +enum bme680_op_mode {
> +	BME680_SLEEP,
> +	BME680_FORCED,
Use this enum to replace the existing BME680_MODE_SLEEP etc definitions
rather than adding another one.
Also assign explicit values as you are going to write this into a register
so they matter.


> +};
> +
>  struct bme680_data {
>  	struct regmap *regmap;
>  	struct bme680_calib bme680;
> @@ -502,23 +507,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);
This is the problematic code.  No obvious reason the enum should match the original
values. It does, but that should be made true by only having an enum, not definitions
and an enum.

> +	if (ret < 0) {
> +		dev_err(dev, "failed to set ctrl_meas register\n");
> +		return ret;
>  	}
>  
>  	return ret;
> @@ -615,8 +613,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_SLEEP);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -756,8 +753,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_FORCED);
>  	if (ret < 0)
>  		return ret;
>  

Re: [PATCH v2 04/13] iio: chemical: bme680: refactorize set_mode() mode

[Vasileios Amoiridis]

On Sun, Oct 27, 2024 at 09:59:39AM +0000, Jonathan Cameron wrote:
> On Mon, 21 Oct 2024 21:53:07 +0200
> 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>
> I changed my mind on this one...
> 
> > ---
> >  drivers/iio/chemical/bme680_core.c | 30 +++++++++++++-----------------
> >  1 file changed, 13 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> > index d228f90b4dc6..9002519d2c33 100644
> > --- a/drivers/iio/chemical/bme680_core.c
> > +++ b/drivers/iio/chemical/bme680_core.c
> > @@ -95,6 +95,11 @@ struct bme680_calib {
> >  	s8  range_sw_err;
> >  };
> >  
> > +enum bme680_op_mode {
> > +	BME680_SLEEP,
> > +	BME680_FORCED,
> Use this enum to replace the existing BME680_MODE_SLEEP etc definitions
> rather than adding another one.
> Also assign explicit values as you are going to write this into a register
> so they matter.
> 
> 

Hi Jonathan,

Thank you very much once again for the review! I totally understand what
you mean and I will fix it for next version.

Cheers,
Vasilis

> > +};
> > +
> >  struct bme680_data {
> >  	struct regmap *regmap;
> >  	struct bme680_calib bme680;
> > @@ -502,23 +507,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);
> This is the problematic code.  No obvious reason the enum should match the original
> values. It does, but that should be made true by only having an enum, not definitions
> and an enum.
> 
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to set ctrl_meas register\n");
> > +		return ret;
> >  	}
> >  
> >  	return ret;
> > @@ -615,8 +613,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_SLEEP);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -756,8 +753,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_FORCED);
> >  	if (ret < 0)
> >  		return ret;
> >  
> 

[PATCH v2 05/13] iio: chemical: bme680: move to fsleep()

[Vasileios Amoiridis]

Use the new fsleep() function in the remaining driver instances.

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

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 9002519d2c33..2ff85e29bfc1 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -544,7 +544,7 @@ static int bme680_wait_for_eoc(struct bme680_data *data)
 			   data->oversampling_humid) * 1936) + (477 * 4) +
 			   (477 * 5) + 1000 + (data->heater_dur * 1000);
 
-	usleep_range(wait_eoc_us, wait_eoc_us + 100);
+	fsleep(wait_eoc_us);
 
 	ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
 	if (ret) {
@@ -890,7 +890,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "Failed to reset chip\n");
 
-	usleep_range(BME680_STARTUP_TIME_US, BME680_STARTUP_TIME_US + 1000);
+	fsleep(BME680_STARTUP_TIME_US);
 
 	ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
 	if (ret < 0)
-- 
2.43.0

Re: [PATCH v2 05/13] iio: chemical: bme680: move to fsleep()

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:08 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Use the new fsleep() function in the remaining driver instances.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied. Note I'm applying these having skipped patch 4, so at somepoint I may
hit a conflict with that and stop even if this code in the later patches is fine.

Jonathan

> ---
>  drivers/iio/chemical/bme680_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index 9002519d2c33..2ff85e29bfc1 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -544,7 +544,7 @@ static int bme680_wait_for_eoc(struct bme680_data *data)
>  			   data->oversampling_humid) * 1936) + (477 * 4) +
>  			   (477 * 5) + 1000 + (data->heater_dur * 1000);
>  
> -	usleep_range(wait_eoc_us, wait_eoc_us + 100);
> +	fsleep(wait_eoc_us);
>  
>  	ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
>  	if (ret) {
> @@ -890,7 +890,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "Failed to reset chip\n");
>  
> -	usleep_range(BME680_STARTUP_TIME_US, BME680_STARTUP_TIME_US + 1000);
> +	fsleep(BME680_STARTUP_TIME_US);
>  
>  	ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
>  	if (ret < 0)

[PATCH v2 06/13] iio: chemical: bme680: Fix indentation and unnecessary spaces

[Vasileios Amoiridis]

Fix indentation issues, line breaking and unnecessary spaces
reported by checkpatch.pl.

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

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 2ff85e29bfc1..2ad427f5deb4 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -229,7 +229,7 @@ static int bme680_read_calib(struct bme680_data *data,
 	calib->res_heat_val = data->bme680_cal_buf_3[RES_HEAT_VAL];
 
 	calib->res_heat_range = FIELD_GET(BME680_RHRANGE_MASK,
-					data->bme680_cal_buf_3[RES_HEAT_RANGE]);
+					  data->bme680_cal_buf_3[RES_HEAT_RANGE]);
 
 	calib->range_sw_err = FIELD_GET(BME680_RSERROR_MASK,
 					data->bme680_cal_buf_3[RANGE_SW_ERR]);
@@ -450,12 +450,12 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
 		2147483647u, 2126008810u, 2147483647u, 2147483647u
 	};
 
-	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
-			((s64)lookup_table[gas_range])) >> 16;
+	var1 = ((1340LL + (5 * calib->range_sw_err)) *
+			(lookup_table[gas_range])) >> 16;
 	var2 = ((gas_res_adc << 15) - 16777216) + var1;
 	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
 	var3 += (var2 >> 1);
-	calc_gas_res = div64_s64(var3, (s64) var2);
+	calc_gas_res = div64_s64(var3, (s64)var2);
 
 	return calc_gas_res;
 }
@@ -473,7 +473,7 @@ static u8 bme680_calc_heater_res(struct bme680_data *data, u16 temp)
 	if (temp > 400) /* Cap temperature */
 		temp = 400;
 
-	var1 = (((s32) BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
+	var1 = (((s32)BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
 	var2 = (calib->par_gh1 + 784) * (((((calib->par_gh2 + 154009) *
 						temp * 5) / 100)
 						+ 3276800) / 10);
@@ -569,9 +569,8 @@ static int bme680_chip_config(struct bme680_data *data)
 	int ret;
 	u8 osrs;
 
-	osrs = FIELD_PREP(
-		BME680_OSRS_HUMIDITY_MASK,
-		bme680_oversampling_to_reg(data->oversampling_humid));
+	osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK,
+			  bme680_oversampling_to_reg(data->oversampling_humid));
 	/*
 	 * Highly recommended to set oversampling of humidity before
 	 * temperature/pressure oversampling.
@@ -585,8 +584,7 @@ static int bme680_chip_config(struct bme680_data *data)
 
 	/* IIR filter settings */
 	ret = regmap_update_bits(data->regmap, BME680_REG_CONFIG,
-				 BME680_FILTER_MASK,
-				 BME680_FILTER_COEFF_VAL);
+				 BME680_FILTER_MASK, BME680_FILTER_COEFF_VAL);
 	if (ret < 0) {
 		dev_err(dev, "failed to write config register\n");
 		return ret;
@@ -885,8 +883,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 	data->heater_temp = 320; /* degree Celsius */
 	data->heater_dur = 150;  /* milliseconds */
 
-	ret = regmap_write(regmap, BME680_REG_SOFT_RESET,
-			   BME680_CMD_SOFTRESET);
+	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

Re: [PATCH v2 06/13] iio: chemical: bme680: Fix indentation and unnecessary spaces

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:09 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Fix indentation issues, line breaking and unnecessary spaces
> reported by checkpatch.pl.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680_core.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index 2ff85e29bfc1..2ad427f5deb4 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -229,7 +229,7 @@ static int bme680_read_calib(struct bme680_data *data,
>  	calib->res_heat_val = data->bme680_cal_buf_3[RES_HEAT_VAL];
>  
>  	calib->res_heat_range = FIELD_GET(BME680_RHRANGE_MASK,
> -					data->bme680_cal_buf_3[RES_HEAT_RANGE]);
> +					  data->bme680_cal_buf_3[RES_HEAT_RANGE]);
>  
>  	calib->range_sw_err = FIELD_GET(BME680_RSERROR_MASK,
>  					data->bme680_cal_buf_3[RANGE_SW_ERR]);
> @@ -450,12 +450,12 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
>  		2147483647u, 2126008810u, 2147483647u, 2147483647u
>  	};
>  
> -	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
> -			((s64)lookup_table[gas_range])) >> 16;
> +	var1 = ((1340LL + (5 * calib->range_sw_err)) *
> +			(lookup_table[gas_range])) >> 16;

Everything other than this one is straight forward and requires
checking types of range_sw_err is less than 32 bit for example.

For cleanup I'd normally avoid these subtle cases. 
An extra type case or two does little harm to readability.

Anyhow, with that in mind I've applied anyway.


>  	var2 = ((gas_res_adc << 15) - 16777216) + var1;
>  	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
>  	var3 += (var2 >> 1);
> -	calc_gas_res = div64_s64(var3, (s64) var2);
> +	calc_gas_res = div64_s64(var3, (s64)var2);
>  
>  	return calc_gas_res;
>  }
> @@ -473,7 +473,7 @@ static u8 bme680_calc_heater_res(struct bme680_data *data, u16 temp)
>  	if (temp > 400) /* Cap temperature */
>  		temp = 400;
>  
> -	var1 = (((s32) BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
> +	var1 = (((s32)BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
>  	var2 = (calib->par_gh1 + 784) * (((((calib->par_gh2 + 154009) *
>  						temp * 5) / 100)
>  						+ 3276800) / 10);
> @@ -569,9 +569,8 @@ static int bme680_chip_config(struct bme680_data *data)
>  	int ret;
>  	u8 osrs;
>  
> -	osrs = FIELD_PREP(
> -		BME680_OSRS_HUMIDITY_MASK,
> -		bme680_oversampling_to_reg(data->oversampling_humid));
> +	osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK,
> +			  bme680_oversampling_to_reg(data->oversampling_humid));
>  	/*
>  	 * Highly recommended to set oversampling of humidity before
>  	 * temperature/pressure oversampling.
> @@ -585,8 +584,7 @@ static int bme680_chip_config(struct bme680_data *data)
>  
>  	/* IIR filter settings */
>  	ret = regmap_update_bits(data->regmap, BME680_REG_CONFIG,
> -				 BME680_FILTER_MASK,
> -				 BME680_FILTER_COEFF_VAL);
> +				 BME680_FILTER_MASK, BME680_FILTER_COEFF_VAL);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to write config register\n");
>  		return ret;
> @@ -885,8 +883,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
>  	data->heater_temp = 320; /* degree Celsius */
>  	data->heater_dur = 150;  /* milliseconds */
>  
> -	ret = regmap_write(regmap, BME680_REG_SOFT_RESET,
> -			   BME680_CMD_SOFTRESET);
> +	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");
>  

Re: [PATCH v2 06/13] iio: chemical: bme680: Fix indentation and unnecessary spaces

[Jonathan Cameron]

On Sun, 27 Oct 2024 10:08:41 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 21 Oct 2024 21:53:09 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Fix indentation issues, line breaking and unnecessary spaces
> > reported by checkpatch.pl.
I also rewrote this description as it doesn't mention the type casting change.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  drivers/iio/chemical/bme680_core.c | 21 +++++++++------------
> >  1 file changed, 9 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> > index 2ff85e29bfc1..2ad427f5deb4 100644
> > --- a/drivers/iio/chemical/bme680_core.c
> > +++ b/drivers/iio/chemical/bme680_core.c
> > @@ -229,7 +229,7 @@ static int bme680_read_calib(struct bme680_data *data,
> >  	calib->res_heat_val = data->bme680_cal_buf_3[RES_HEAT_VAL];
> >  
> >  	calib->res_heat_range = FIELD_GET(BME680_RHRANGE_MASK,
> > -					data->bme680_cal_buf_3[RES_HEAT_RANGE]);
> > +					  data->bme680_cal_buf_3[RES_HEAT_RANGE]);
> >  
> >  	calib->range_sw_err = FIELD_GET(BME680_RSERROR_MASK,
> >  					data->bme680_cal_buf_3[RANGE_SW_ERR]);
> > @@ -450,12 +450,12 @@ static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
> >  		2147483647u, 2126008810u, 2147483647u, 2147483647u
> >  	};
> >  
> > -	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
> > -			((s64)lookup_table[gas_range])) >> 16;
> > +	var1 = ((1340LL + (5 * calib->range_sw_err)) *
> > +			(lookup_table[gas_range])) >> 16;  
> 
> Everything other than this one is straight forward and requires
> checking types of range_sw_err is less than 32 bit for example.
> 
> For cleanup I'd normally avoid these subtle cases. 
> An extra type case or two does little harm to readability.
> 
> Anyhow, with that in mind I've applied anyway.
> 
> 
> >  	var2 = ((gas_res_adc << 15) - 16777216) + var1;
> >  	var3 = ((125000 << (15 - gas_range)) * var1) >> 9;
> >  	var3 += (var2 >> 1);
> > -	calc_gas_res = div64_s64(var3, (s64) var2);
> > +	calc_gas_res = div64_s64(var3, (s64)var2);
> >  
> >  	return calc_gas_res;
> >  }
> > @@ -473,7 +473,7 @@ static u8 bme680_calc_heater_res(struct bme680_data *data, u16 temp)
> >  	if (temp > 400) /* Cap temperature */
> >  		temp = 400;
> >  
> > -	var1 = (((s32) BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
> > +	var1 = (((s32)BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
> >  	var2 = (calib->par_gh1 + 784) * (((((calib->par_gh2 + 154009) *
> >  						temp * 5) / 100)
> >  						+ 3276800) / 10);
> > @@ -569,9 +569,8 @@ static int bme680_chip_config(struct bme680_data *data)
> >  	int ret;
> >  	u8 osrs;
> >  
> > -	osrs = FIELD_PREP(
> > -		BME680_OSRS_HUMIDITY_MASK,
> > -		bme680_oversampling_to_reg(data->oversampling_humid));
> > +	osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK,
> > +			  bme680_oversampling_to_reg(data->oversampling_humid));
> >  	/*
> >  	 * Highly recommended to set oversampling of humidity before
> >  	 * temperature/pressure oversampling.
> > @@ -585,8 +584,7 @@ static int bme680_chip_config(struct bme680_data *data)
> >  
> >  	/* IIR filter settings */
> >  	ret = regmap_update_bits(data->regmap, BME680_REG_CONFIG,
> > -				 BME680_FILTER_MASK,
> > -				 BME680_FILTER_COEFF_VAL);
> > +				 BME680_FILTER_MASK, BME680_FILTER_COEFF_VAL);
> >  	if (ret < 0) {
> >  		dev_err(dev, "failed to write config register\n");
> >  		return ret;
> > @@ -885,8 +883,7 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> >  	data->heater_temp = 320; /* degree Celsius */
> >  	data->heater_dur = 150;  /* milliseconds */
> >  
> > -	ret = regmap_write(regmap, BME680_REG_SOFT_RESET,
> > -			   BME680_CMD_SOFTRESET);
> > +	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");
> >    
> 

[PATCH v2 07/13] iio: chemical: bme680: generalize read_*() functions

[Vasileios Amoiridis]

Remove the IIO specific scaling measurement units from the read functions
and add them inside the ->read_raw() function to keep the read_*() generic.
This way they can be used in other parts of the driver.

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

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 2ad427f5deb4..c79ba6d1ece8 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -644,23 +644,20 @@ static int bme680_gas_config(struct bme680_data *data)
 	return ret;
 }
 
-static int bme680_read_temp(struct bme680_data *data, int *val)
+static int bme680_read_temp(struct bme680_data *data, s16 *comp_temp)
 {
 	int ret;
 	u32 adc_temp;
-	s16 comp_temp;
 
 	ret = bme680_read_temp_adc(data, &adc_temp);
 	if (ret)
 		return ret;
 
-	comp_temp = bme680_compensate_temp(data, adc_temp);
-	*val = comp_temp * 10; /* Centidegrees to millidegrees */
-	return IIO_VAL_INT;
+	*comp_temp = bme680_compensate_temp(data, adc_temp);
+	return 0;
 }
 
-static int bme680_read_press(struct bme680_data *data,
-			     int *val, int *val2)
+static int bme680_read_press(struct bme680_data *data, u32 *comp_press)
 {
 	int ret;
 	u32 adc_press;
@@ -674,16 +671,14 @@ static int bme680_read_press(struct bme680_data *data,
 	if (ret)
 		return ret;
 
-	*val = bme680_compensate_press(data, adc_press, t_fine);
-	*val2 = 1000;
-	return IIO_VAL_FRACTIONAL;
+	*comp_press = bme680_compensate_press(data, adc_press, t_fine);
+	return 0;
 }
 
-static int bme680_read_humid(struct bme680_data *data,
-			     int *val, int *val2)
+static int bme680_read_humid(struct bme680_data *data, u32 *comp_humidity)
 {
 	int ret;
-	u32 adc_humidity, comp_humidity;
+	u32 adc_humidity;
 	s32 t_fine;
 
 	ret = bme680_get_t_fine(data, &t_fine);
@@ -694,15 +689,11 @@ static int bme680_read_humid(struct bme680_data *data,
 	if (ret)
 		return ret;
 
-	comp_humidity = bme680_compensate_humid(data, adc_humidity, t_fine);
-
-	*val = comp_humidity;
-	*val2 = 1000;
-	return IIO_VAL_FRACTIONAL;
+	*comp_humidity = bme680_compensate_humid(data, adc_humidity, t_fine);
+	return 0;
 }
 
-static int bme680_read_gas(struct bme680_data *data,
-			   int *val)
+static int bme680_read_gas(struct bme680_data *data, int *comp_gas_res)
 {
 	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
@@ -737,9 +728,8 @@ static int bme680_read_gas(struct bme680_data *data,
 	}
 
 	gas_range = FIELD_GET(BME680_GAS_RANGE_MASK, gas_regs_val);
-
-	*val = bme680_compensate_gas(data, adc_gas_res, gas_range);
-	return IIO_VAL_INT;
+	*comp_gas_res = bme680_compensate_gas(data, adc_gas_res, gas_range);
+	return 0;
 }
 
 static int bme680_read_raw(struct iio_dev *indio_dev,
@@ -747,7 +737,7 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	struct bme680_data *data = iio_priv(indio_dev);
-	int ret;
+	int chan_val, ret;
 
 	guard(mutex)(&data->lock);
 
@@ -763,13 +753,35 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_PROCESSED:
 		switch (chan->type) {
 		case IIO_TEMP:
-			return bme680_read_temp(data, val);
+			ret = bme680_read_temp(data, (s16 *)&chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val * 10;
+			return IIO_VAL_INT;
 		case IIO_PRESSURE:
-			return bme680_read_press(data, val, val2);
+			ret = bme680_read_press(data, &chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
 		case IIO_HUMIDITYRELATIVE:
-			return bme680_read_humid(data, val, val2);
+			ret = bme680_read_humid(data, &chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
 		case IIO_RESISTANCE:
-			return bme680_read_gas(data, val);
+			ret = bme680_read_gas(data, &chan_val);
+			if (ret)
+				return ret;
+
+			*val = chan_val;
+			return IIO_VAL_INT;
 		default:
 			return -EINVAL;
 		}
-- 
2.43.0

Re: [PATCH v2 07/13] iio: chemical: bme680: generalize read_*() functions

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:10 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Remove the IIO specific scaling measurement units from the read functions
> and add them inside the ->read_raw() function to keep the read_*() generic.
> This way they can be used in other parts of the driver.
Applied.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680_core.c | 68 ++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index 2ad427f5deb4..c79ba6d1ece8 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -644,23 +644,20 @@ static int bme680_gas_config(struct bme680_data *data)
>  	return ret;
>  }
>  
> -static int bme680_read_temp(struct bme680_data *data, int *val)
> +static int bme680_read_temp(struct bme680_data *data, s16 *comp_temp)
>  {
>  	int ret;
>  	u32 adc_temp;
> -	s16 comp_temp;
>  
>  	ret = bme680_read_temp_adc(data, &adc_temp);
>  	if (ret)
>  		return ret;
>  
> -	comp_temp = bme680_compensate_temp(data, adc_temp);
> -	*val = comp_temp * 10; /* Centidegrees to millidegrees */
> -	return IIO_VAL_INT;
> +	*comp_temp = bme680_compensate_temp(data, adc_temp);
> +	return 0;
>  }
>  
> -static int bme680_read_press(struct bme680_data *data,
> -			     int *val, int *val2)
> +static int bme680_read_press(struct bme680_data *data, u32 *comp_press)
>  {
>  	int ret;
>  	u32 adc_press;
> @@ -674,16 +671,14 @@ static int bme680_read_press(struct bme680_data *data,
>  	if (ret)
>  		return ret;
>  
> -	*val = bme680_compensate_press(data, adc_press, t_fine);
> -	*val2 = 1000;
> -	return IIO_VAL_FRACTIONAL;
> +	*comp_press = bme680_compensate_press(data, adc_press, t_fine);
> +	return 0;
>  }
>  
> -static int bme680_read_humid(struct bme680_data *data,
> -			     int *val, int *val2)
> +static int bme680_read_humid(struct bme680_data *data, u32 *comp_humidity)
>  {
>  	int ret;
> -	u32 adc_humidity, comp_humidity;
> +	u32 adc_humidity;
>  	s32 t_fine;
>  
>  	ret = bme680_get_t_fine(data, &t_fine);
> @@ -694,15 +689,11 @@ static int bme680_read_humid(struct bme680_data *data,
>  	if (ret)
>  		return ret;
>  
> -	comp_humidity = bme680_compensate_humid(data, adc_humidity, t_fine);
> -
> -	*val = comp_humidity;
> -	*val2 = 1000;
> -	return IIO_VAL_FRACTIONAL;
> +	*comp_humidity = bme680_compensate_humid(data, adc_humidity, t_fine);
> +	return 0;
>  }
>  
> -static int bme680_read_gas(struct bme680_data *data,
> -			   int *val)
> +static int bme680_read_gas(struct bme680_data *data, int *comp_gas_res)
>  {
>  	struct device *dev = regmap_get_device(data->regmap);
>  	int ret;
> @@ -737,9 +728,8 @@ static int bme680_read_gas(struct bme680_data *data,
>  	}
>  
>  	gas_range = FIELD_GET(BME680_GAS_RANGE_MASK, gas_regs_val);
> -
> -	*val = bme680_compensate_gas(data, adc_gas_res, gas_range);
> -	return IIO_VAL_INT;
> +	*comp_gas_res = bme680_compensate_gas(data, adc_gas_res, gas_range);
> +	return 0;
>  }
>  
>  static int bme680_read_raw(struct iio_dev *indio_dev,
> @@ -747,7 +737,7 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
>  			   int *val, int *val2, long mask)
>  {
>  	struct bme680_data *data = iio_priv(indio_dev);
> -	int ret;
> +	int chan_val, ret;
>  
>  	guard(mutex)(&data->lock);
>  
> @@ -763,13 +753,35 @@ static int bme680_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_PROCESSED:
>  		switch (chan->type) {
>  		case IIO_TEMP:
> -			return bme680_read_temp(data, val);
> +			ret = bme680_read_temp(data, (s16 *)&chan_val);
> +			if (ret)
> +				return ret;
> +
> +			*val = chan_val * 10;
> +			return IIO_VAL_INT;
>  		case IIO_PRESSURE:
> -			return bme680_read_press(data, val, val2);
> +			ret = bme680_read_press(data, &chan_val);
> +			if (ret)
> +				return ret;
> +
> +			*val = chan_val;
> +			*val2 = 1000;
> +			return IIO_VAL_FRACTIONAL;
>  		case IIO_HUMIDITYRELATIVE:
> -			return bme680_read_humid(data, val, val2);
> +			ret = bme680_read_humid(data, &chan_val);
> +			if (ret)
> +				return ret;
> +
> +			*val = chan_val;
> +			*val2 = 1000;
> +			return IIO_VAL_FRACTIONAL;
>  		case IIO_RESISTANCE:
> -			return bme680_read_gas(data, val);
> +			ret = bme680_read_gas(data, &chan_val);
> +			if (ret)
> +				return ret;
> +
> +			*val = chan_val;
> +			return IIO_VAL_INT;
>  		default:
>  			return -EINVAL;
>  		}

[PATCH v2 08/13] 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 c79ba6d1ece8..e8098754a85f 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -143,17 +143,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),
 	},
 	{
@@ -785,6 +794,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 v2 08/13] iio: chemical: bme680: Add SCALE and RAW channels

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:11 +0200
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 v2 09/13] 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 678a6adb9a75..447b205f57bd 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 f5be4516dde7..574877bc51c5 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -68,6 +68,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 e8098754a85f..7bf4daa35ba5 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>
 
@@ -100,6 +103,13 @@ enum bme680_op_mode {
 	BME680_FORCED,
 };
 
+enum bme680_scan {
+	BME680_TEMP,
+	BME680_PRESS,
+	BME680_HUMID,
+	BME680_GAS,
+};
+
 struct bme680_data {
 	struct regmap *regmap;
 	struct bme680_calib bme680;
@@ -110,8 +120,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];
@@ -148,6 +163,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,
@@ -156,6 +178,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,
@@ -164,11 +193,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,
@@ -918,6 +962,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_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)
 {
@@ -936,6 +1062,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;
 
@@ -978,6 +1105,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 v2 09/13] iio: chemical: bme680: Add triggered buffer support

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:12 +0200
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>
LGTM, but can't apply until patch 4 is sorted out.
So let's see this one in v3.

Jonathan

[PATCH v2 10/13] 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 574877bc51c5..e5d82a6d5b59 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -44,6 +44,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 7bf4daa35ba5..8c80a5f1ef9e 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -117,6 +117,7 @@ struct bme680_data {
 	u8 oversampling_temp;
 	u8 oversampling_press;
 	u8 oversampling_humid;
+	u8 preheat_curr_mA;
 	u16 heater_dur;
 	u16 heater_temp;
 
@@ -213,6 +214,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,
@@ -560,6 +567,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);
@@ -658,6 +671,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);
@@ -686,6 +713,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,
@@ -937,6 +968,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;
 	}
@@ -1072,6 +1112,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

[PATCH v2 11/13] dt-bindings: iio: add binding for BME680 driver

[Vasileios Amoiridis]

Add 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   | 64 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  2 -
 2 files changed, 64 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..e54df3afa7b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
@@ -0,0 +1,64 @@
+# 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
+  - vdd-supply
+  - vddio-supply
+
+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 0108d7507215..3d9c08ed7bce 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 v2 11/13] dt-bindings: iio: add binding for BME680 driver

[Rob Herring (Arm)]

On Mon, 21 Oct 2024 21:53:14 +0200, Vasileios Amoiridis wrote:
> Add 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   | 64 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-mux.example.dtb: sensor@1: 'vdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-mux.example.dtb: sensor@1: 'vddio-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-controller.example.dtb: sensor@1: 'vdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/spi-controller.example.dtb: sensor@1: 'vddio-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/cirrus,cs42l43.example.dtb: sensor@0: 'vdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/cirrus,cs42l43.example.dtb: sensor@0: 'vddio-supply' is a required property
	from schema $id: http://devicetree.org/schemas/iio/chemical/bosch,bme680.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241021195316.58911-12-vassilisamir@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 11/13] dt-bindings: iio: add binding for BME680 driver

[Rob Herring]

On Mon, Oct 21, 2024 at 09:53:14PM +0200, Vasileios Amoiridis wrote:
> Add dt-binding for BME680 gas sensor device. The device incorporates as
> well temperature, pressure and relative humidity sensors.

You aren't adding a binding for bme680, but extending it.

Drop the 2nd 'bindings' from subject. 

Something like this:

dt-bindings: iio/chemical: bosch,bme680: Add supply properties

> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  .../bindings/iio/chemical/bosch,bme680.yaml   | 64 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 64 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..e54df3afa7b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> @@ -0,0 +1,64 @@
> +# 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:

You need '>' to maintain paragraphs.

> +  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
> +  - vdd-supply
> +  - vddio-supply

These supplies can't be required. That's an ABI change from what was 
already supported.

> +
> +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 0108d7507215..3d9c08ed7bce 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 v2 11/13] dt-bindings: iio: add binding for BME680 driver

[Jonathan Cameron]

On Tue, 22 Oct 2024 08:41:02 -0500
Rob Herring <robh@kernel.org> wrote:

> On Mon, Oct 21, 2024 at 09:53:14PM +0200, Vasileios Amoiridis wrote:
> > Add dt-binding for BME680 gas sensor device. The device incorporates as
> > well temperature, pressure and relative humidity sensors.  
> 
> You aren't adding a binding for bme680, but extending it.
> 
> Drop the 2nd 'bindings' from subject. 
> 
> Something like this:
> 
> dt-bindings: iio/chemical: bosch,bme680: Add supply properties
> 
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  .../bindings/iio/chemical/bosch,bme680.yaml   | 64 +++++++++++++++++++
> >  .../devicetree/bindings/trivial-devices.yaml  |  2 -
> >  2 files changed, 64 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..e54df3afa7b2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/chemical/bosch,bme680.yaml
> > @@ -0,0 +1,64 @@
> > +# 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:  
> 
> You need '>' to maintain paragraphs.
> 
> > +  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
> > +  - vdd-supply
> > +  - vddio-supply  
> 
> These supplies can't be required. That's an ABI change from what was 
> already supported.
Hi Rob,

I thought for supplies the convention was that if the power is needed
for functioning device then to put them as required.

In case were they were missing in an original binding
allow stub regulators to deal with a DT that predates that.
Non linux cases are obviously trickier to predict but these
supplies must be on or the binding wouldn't have worked before this
point.

I remember I was very much on the side that they were optional and
convinced by others that this was the way to go.

We've added supplies for old bindings as required in the past
so probably broken someone if I have this wrong :(

Jonathan

[PATCH v2 12/13] 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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index 8c80a5f1ef9e..2d9d20f203aa 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>
@@ -110,6 +111,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;
@@ -121,6 +126,8 @@ struct bme680_data {
 	u16 heater_dur;
 	u16 heater_temp;
 
+	struct regulator_bulk_data supplies[BME680_NUM_SUPPLIES];
+
 	struct {
 		s32 chan[4];
 		aligned_s64 ts;
@@ -1114,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 v2 13/13] iio: chemical: bme680: add power management

[Vasileios Amoiridis]

Add runtime power management to the device. To facilitate this, add also
a struct dev* inside the bme680_data structure to have the device
accesible from the data structure.

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

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index e5d82a6d5b59..74e97e35e35a 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
@@ -82,6 +83,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 2d9d20f203aa..803aa4f14b37 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>
 
@@ -823,9 +825,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,39 @@ 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,
+};
+
+static void bme680_pm_disable(void *data)
+{
+	struct device *dev = data;
+
+	pm_runtime_get_sync(dev);
+	pm_runtime_put_noidle(dev);
+	pm_runtime_disable(dev);
+}
+
 int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		      const char *name)
 {
@@ -1164,15 +1231,60 @@ 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_get_noresume(dev);
+	pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US * 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_put(dev);
+
+	ret = devm_add_action_or_reset(dev, bme680_pm_disable, 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 regulator_bulk_disable(BME680_NUM_SUPPLIES, data->supplies);
+}
+
+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 = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies);
+	if (ret)
+		return ret;
+
+	fsleep(BME680_STARTUP_TIME_US);
+
+	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");
-- 
2.43.0

Re: [PATCH v2 13/13] iio: chemical: bme680: add power management

[Jonathan Cameron]

On Mon, 21 Oct 2024 21:53:16 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Add runtime power management to the device. To facilitate this, add also
> a struct dev* inside the bme680_data structure to have the device
> accesible from the data structure.

Needs an update as you are now getting that from the regmap.


> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680.h      |   2 +
>  drivers/iio/chemical/bme680_core.c | 126 +++++++++++++++++++++++++++--
>  2 files changed, 121 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index e5d82a6d5b59..74e97e35e35a 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
> @@ -82,6 +83,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;

You seem to have missed the changes that use this in the i2c and spi drivers.


>  static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
>  
>  static IIO_CONST_ATTR(oversampling_ratio_available,
> @@ -1091,6 +1125,39 @@ 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,
> +};
> +
> +static void bme680_pm_disable(void *data)
> +{
> +	struct device *dev = data;
> +
> +	pm_runtime_get_sync(dev);
> +	pm_runtime_put_noidle(dev);
This dance is to get the device powered up on runtime pm tear down
I think?  Whilst we sometimes do this, why is it needed in this particular driver?

> +	pm_runtime_disable(dev);
> +}
> +
>  int bme680_core_probe(struct device *dev, struct regmap *regmap,
>  		      const char *name)
>  {
> @@ -1164,15 +1231,60 @@ 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_get_noresume(dev);
> +	pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US * 100);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_active(dev);
> +	ret = devm_pm_runtime_enable(dev);

Take a look at what this unwinds in the devm handler. You don't need to do
as much (or possibly anything) yourself.


> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_put(dev);
> +
> +	ret = devm_add_action_or_reset(dev, bme680_pm_disable, 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 regulator_bulk_disable(BME680_NUM_SUPPLIES, data->supplies);
> +}
> +
> +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 = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(BME680_STARTUP_TIME_US);
> +
> +	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");

Re: [PATCH v2 13/13] iio: chemical: bme680: add power management

[Vasileios Amoiridis]

On Sun, Oct 27, 2024 at 10:30:13AM +0000, Jonathan Cameron wrote:
> On Mon, 21 Oct 2024 21:53:16 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Add runtime power management to the device. To facilitate this, add also
> > a struct dev* inside the bme680_data structure to have the device
> > accesible from the data structure.
> 
> Needs an update as you are now getting that from the regmap.
> 
> 

Hi Jonathan,

Thanks for noticing, I will fix it for next version.

> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  drivers/iio/chemical/bme680.h      |   2 +
> >  drivers/iio/chemical/bme680_core.c | 126 +++++++++++++++++++++++++++--
> >  2 files changed, 121 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> > index e5d82a6d5b59..74e97e35e35a 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
> > @@ -82,6 +83,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;
> 
> You seem to have missed the changes that use this in the i2c and spi drivers.
> 
> 

You are totally right.

> >  static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
> >  
> >  static IIO_CONST_ATTR(oversampling_ratio_available,
> > @@ -1091,6 +1125,39 @@ 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,
> > +};
> > +
> > +static void bme680_pm_disable(void *data)
> > +{
> > +	struct device *dev = data;
> > +
> > +	pm_runtime_get_sync(dev);
> > +	pm_runtime_put_noidle(dev);
> This dance is to get the device powered up on runtime pm tear down
> I think?  Whilst we sometimes do this, why is it needed in this particular driver?
> 

Actually this one is not needed indeed, I was using it to test that the
device was coming up but it is not really needed. And the whole disable
sequence is perfectly handled by the devm_pm_runtime_enable so it is not
even needed at all actually.

> > +	pm_runtime_disable(dev);
> > +}
> > +
> >  int bme680_core_probe(struct device *dev, struct regmap *regmap,
> >  		      const char *name)
> >  {
> > @@ -1164,15 +1231,60 @@ 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_get_noresume(dev);
> > +	pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US * 100);
> > +	pm_runtime_use_autosuspend(dev);
> > +	pm_runtime_set_active(dev);
> > +	ret = devm_pm_runtime_enable(dev);
> 
> Take a look at what this unwinds in the devm handler. You don't need to do
> as much (or possibly anything) yourself.
> 
> 

Well, in general what I see that could probably be dropped here is the
extra add_action_or_reset because of the devm_*. About the functions
get_noresume() and put(), I feel that they could be dropped as well
because the device registration will happen well before the autosuspend
delay, but does it make sense to depend on something like this?

Cheers,
Vasilis

> > +	if (ret)
> > +		return ret;
> > +
> > +	pm_runtime_put(dev);
> > +
> > +	ret = devm_add_action_or_reset(dev, bme680_pm_disable, 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 regulator_bulk_disable(BME680_NUM_SUPPLIES, data->supplies);
> > +}
> > +
> > +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 = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies);
> > +	if (ret)
> > +		return ret;
> > +
> > +	fsleep(BME680_STARTUP_TIME_US);
> > +
> > +	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");
> 

Re: [PATCH v2 13/13] iio: chemical: bme680: add power management

[Jonathan Cameron]

> > > +
> > >  int bme680_core_probe(struct device *dev, struct regmap *regmap,
> > >  		      const char *name)
> > >  {
> > > @@ -1164,15 +1231,60 @@ 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_get_noresume(dev);
> > > +	pm_runtime_set_autosuspend_delay(dev, BME680_STARTUP_TIME_US * 100);
> > > +	pm_runtime_use_autosuspend(dev);
> > > +	pm_runtime_set_active(dev);
> > > +	ret = devm_pm_runtime_enable(dev);  
> > 
> > Take a look at what this unwinds in the devm handler. You don't need to do
> > as much (or possibly anything) yourself.
> > 
> >   
> 
> Well, in general what I see that could probably be dropped here is the
> extra add_action_or_reset because of the devm_*. About the functions
> get_noresume() and put(), I feel that they could be dropped as well
> because the device registration will happen well before the autosuspend
> delay, but does it make sense to depend on something like this?

If you haven't enabled runtime pm yet all you need to do is set up the
current state  Shouldn't need the get etc as you don't care if it powers
down here anyway as you aren't talking to the device.
> 
> Cheers,
> Vasilis
> 
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pm_runtime_put(dev);
> > > +
> > > +	ret = devm_add_action_or_reset(dev, bme680_pm_disable, 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 regulator_bulk_disable(BME680_NUM_SUPPLIES, data->supplies);
> > > +}
> > > +
> > > +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 = regulator_bulk_enable(BME680_NUM_SUPPLIES, data->supplies);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	fsleep(BME680_STARTUP_TIME_US);
> > > +
> > > +	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");  
> >