[PATCH v1 14/17] iio: chemical: bme680: Modify startup procedure

Vasileios Amoiridis posted 17 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH v1 14/17] iio: chemical: bme680: Modify startup procedure
Posted by Vasileios Amoiridis 1 year, 8 months ago
Modify the startup procedure to reflect the procedure of
the Bosch BME68x Sensor API. The initial readings and
configuration of the sensor need to happen in the
following order:

1) Read calibration data [1,2]
2) Chip general configuration [3]
3) Gas configuration [4]

After the chip configuration it is necessary to ensure that
the sensor is in sleeping mode, in order to apply the gas
configuration settings [5].

Also, after the soft reset, it is advised to wait for 5ms [6].

[1]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L162
[2]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L44
[3]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L53
[4]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L60
[5]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L640
[6]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L294
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h      |  2 ++
 drivers/iio/chemical/bme680_core.c | 27 ++++++++++++++++++---------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index 17ea59253923..3be2f76a5bfb 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -61,6 +61,8 @@
 
 #define BME680_MEAS_TRIM_MASK			GENMASK(24, 4)
 
+#define BME680_STARTUP_TIME_US			5000
+
 /* 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 b055eeee8f1c..afaa43ec3241 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -505,10 +505,12 @@ static int bme680_chip_config(struct bme680_data *data)
 	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
 				BME680_OSRS_TEMP_MASK | BME680_OSRS_PRESS_MASK,
 				osrs);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(dev, "failed to write ctrl_meas register\n");
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static int bme680_gas_config(struct bme680_data *data)
@@ -517,6 +519,11 @@ 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);
+	if (ret < 0)
+		return ret;
+
 	heatr_res = bme680_calc_heater_res(data, data->heater_temp);
 
 	/* set target heater temperature */
@@ -847,6 +854,8 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+	usleep_range(BME680_STARTUP_TIME_US, BME680_STARTUP_TIME_US + 1000);
+
 	ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
 	if (ret < 0) {
 		dev_err(dev, "Error reading chip ID\n");
@@ -859,22 +868,22 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
 		return -ENODEV;
 	}
 
-	ret = bme680_chip_config(data);
+	ret = bme680_read_calib(data, &data->bme680);
 	if (ret < 0) {
-		dev_err(dev, "failed to set chip_config data\n");
+		dev_err(dev,
+			"failed to read calibration coefficients at probe\n");
 		return ret;
 	}
 
-	ret = bme680_gas_config(data);
+	ret = bme680_chip_config(data);
 	if (ret < 0) {
-		dev_err(dev, "failed to set gas config data\n");
+		dev_err(dev, "failed to set chip_config data\n");
 		return ret;
 	}
 
-	ret = bme680_read_calib(data, &data->bme680);
+	ret = bme680_gas_config(data);
 	if (ret < 0) {
-		dev_err(dev,
-			"failed to read calibration coefficients at probe\n");
+		dev_err(dev, "failed to set gas config data\n");
 		return ret;
 	}
 
-- 
2.25.1
Re: [PATCH v1 14/17] iio: chemical: bme680: Modify startup procedure
Posted by Jonathan Cameron 1 year, 8 months ago
On Mon, 27 May 2024 20:38:02 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Modify the startup procedure to reflect the procedure of
> the Bosch BME68x Sensor API. The initial readings and
> configuration of the sensor need to happen in the
> following order:
> 
> 1) Read calibration data [1,2]
> 2) Chip general configuration [3]
> 3) Gas configuration [4]
> 
> After the chip configuration it is necessary to ensure that
> the sensor is in sleeping mode, in order to apply the gas
> configuration settings [5].
> 
> Also, after the soft reset, it is advised to wait for 5ms [6].
> 
> [1]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L162
> [2]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L44
> [3]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L53
> [4]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L60
> [5]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L640
> [6]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L294
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680.h      |  2 ++
>  drivers/iio/chemical/bme680_core.c | 27 ++++++++++++++++++---------
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index 17ea59253923..3be2f76a5bfb 100644
> --- a/drivers/iio/chemical/bme680.h
> +++ b/drivers/iio/chemical/bme680.h
> @@ -61,6 +61,8 @@
>  
>  #define BME680_MEAS_TRIM_MASK			GENMASK(24, 4)
>  
> +#define BME680_STARTUP_TIME_US			5000
> +
>  /* 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 b055eeee8f1c..afaa43ec3241 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -505,10 +505,12 @@ static int bme680_chip_config(struct bme680_data *data)
>  	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
>  				BME680_OSRS_TEMP_MASK | BME680_OSRS_PRESS_MASK,
>  				osrs);
> -	if (ret < 0)
> +	if (ret < 0) {
>  		dev_err(dev, "failed to write ctrl_meas register\n");
> +		return ret;
> +	}
>  
> -	return ret;
> +	return 0;
>  }

I think this is an unrelated change so if you want to do this - different patch.

>  
>  static int bme680_gas_config(struct bme680_data *data)
> @@ -517,6 +519,11 @@ 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);
> +	if (ret < 0)
> +		return ret;
> +
>  	heatr_res = bme680_calc_heater_res(data, data->heater_temp);
>  
>  	/* set target heater temperature */
> @@ -847,6 +854,8 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
>  		return ret;
>  	}
>  
> +	usleep_range(BME680_STARTUP_TIME_US, BME680_STARTUP_TIME_US + 1000);
> +
>  	ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
>  	if (ret < 0) {
>  		dev_err(dev, "Error reading chip ID\n");
> @@ -859,22 +868,22 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
>  		return -ENODEV;
>  	}
>  
> -	ret = bme680_chip_config(data);
> +	ret = bme680_read_calib(data, &data->bme680);
>  	if (ret < 0) {
> -		dev_err(dev, "failed to set chip_config data\n");
> +		dev_err(dev,
> +			"failed to read calibration coefficients at probe\n");
>  		return ret;

Maybe you have it in a later patch (it definitely wants to be a different patch from
this one as different issue), but feels like a bunch of places where
dev_err_probe() would be good.

>  	}
>  
> -	ret = bme680_gas_config(data);
> +	ret = bme680_chip_config(data);
>  	if (ret < 0) {
> -		dev_err(dev, "failed to set gas config data\n");
> +		dev_err(dev, "failed to set chip_config data\n");
>  		return ret;
>  	}
>  
> -	ret = bme680_read_calib(data, &data->bme680);
> +	ret = bme680_gas_config(data);
>  	if (ret < 0) {
> -		dev_err(dev,
> -			"failed to read calibration coefficients at probe\n");
> +		dev_err(dev, "failed to set gas config data\n");
>  		return ret;
>  	}
>
Re: [PATCH v1 14/17] iio: chemical: bme680: Modify startup procedure
Posted by Vasileios Amoiridis 1 year, 8 months ago
On Sun, Jun 02, 2024 at 02:01:23PM +0100, Jonathan Cameron wrote:
> On Mon, 27 May 2024 20:38:02 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Modify the startup procedure to reflect the procedure of
> > the Bosch BME68x Sensor API. The initial readings and
> > configuration of the sensor need to happen in the
> > following order:
> > 
> > 1) Read calibration data [1,2]
> > 2) Chip general configuration [3]
> > 3) Gas configuration [4]
> > 
> > After the chip configuration it is necessary to ensure that
> > the sensor is in sleeping mode, in order to apply the gas
> > configuration settings [5].
> > 
> > Also, after the soft reset, it is advised to wait for 5ms [6].
> > 
> > [1]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L162
> > [2]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L44
> > [3]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L53
> > [4]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/examples/forced_mode/forced_mode.c#L60
> > [5]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L640
> > [6]: https://github.com/boschsensortec/BME68x_SensorAPI/blob/v4.4.8/bme68x.c#L294
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  drivers/iio/chemical/bme680.h      |  2 ++
> >  drivers/iio/chemical/bme680_core.c | 27 ++++++++++++++++++---------
> >  2 files changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> > index 17ea59253923..3be2f76a5bfb 100644
> > --- a/drivers/iio/chemical/bme680.h
> > +++ b/drivers/iio/chemical/bme680.h
> > @@ -61,6 +61,8 @@
> >  
> >  #define BME680_MEAS_TRIM_MASK			GENMASK(24, 4)
> >  
> > +#define BME680_STARTUP_TIME_US			5000
> > +
> >  /* 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 b055eeee8f1c..afaa43ec3241 100644
> > --- a/drivers/iio/chemical/bme680_core.c
> > +++ b/drivers/iio/chemical/bme680_core.c
> > @@ -505,10 +505,12 @@ static int bme680_chip_config(struct bme680_data *data)
> >  	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
> >  				BME680_OSRS_TEMP_MASK | BME680_OSRS_PRESS_MASK,
> >  				osrs);
> > -	if (ret < 0)
> > +	if (ret < 0) {
> >  		dev_err(dev, "failed to write ctrl_meas register\n");
> > +		return ret;
> > +	}
> >  
> > -	return ret;
> > +	return 0;
> >  }
> 
> I think this is an unrelated change so if you want to do this - different patch.
> 

Well, it is not completely unrelated. This function is only doing regmap_reads() 
and after every regmap in case of error it returns in the if statement that exists
after the regmap_read(). In the last check though, instead of exiting inside the if
statement it just sends a dev_err() message, exits the if() and then exits from
the last return. Functionality is the same, it is just not consistent. But I could
split it in 2 commits, no problem!

> >  
> >  static int bme680_gas_config(struct bme680_data *data)
> > @@ -517,6 +519,11 @@ 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);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	heatr_res = bme680_calc_heater_res(data, data->heater_temp);
> >  
> >  	/* set target heater temperature */
> > @@ -847,6 +854,8 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> >  		return ret;
> >  	}
> >  
> > +	usleep_range(BME680_STARTUP_TIME_US, BME680_STARTUP_TIME_US + 1000);
> > +
> >  	ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
> >  	if (ret < 0) {
> >  		dev_err(dev, "Error reading chip ID\n");
> > @@ -859,22 +868,22 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> >  		return -ENODEV;
> >  	}
> >  
> > -	ret = bme680_chip_config(data);
> > +	ret = bme680_read_calib(data, &data->bme680);
> >  	if (ret < 0) {
> > -		dev_err(dev, "failed to set chip_config data\n");
> > +		dev_err(dev,
> > +			"failed to read calibration coefficients at probe\n");
> >  		return ret;
> 
> Maybe you have it in a later patch (it definitely wants to be a different patch from
> this one as different issue), but feels like a bunch of places where
> dev_err_probe() would be good.
> 

Well, since they are in the probe function I guess I could also change those to
dev_err_probe() in a separate commit.

Cheers,
Vasilis

> >  	}
> >  
> > -	ret = bme680_gas_config(data);
> > +	ret = bme680_chip_config(data);
> >  	if (ret < 0) {
> > -		dev_err(dev, "failed to set gas config data\n");
> > +		dev_err(dev, "failed to set chip_config data\n");
> >  		return ret;
> >  	}
> >  
> > -	ret = bme680_read_calib(data, &data->bme680);
> > +	ret = bme680_gas_config(data);
> >  	if (ret < 0) {
> > -		dev_err(dev,
> > -			"failed to read calibration coefficients at probe\n");
> > +		dev_err(dev, "failed to set gas config data\n");
> >  		return ret;
> >  	}
> >  
>