| Message ID | 20240527183805.311501-15-vassilisamir@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: chemical: bme680: Driver cleanup | expand |
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; > } >
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; > > } > > >
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; }
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(-)