| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240602194042.GF387181@vamoiridPC>
Date: Sun, 2 Jun 2024 21:40:42 +0200
From: Vasileios Amoiridis <vassilisamir@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Vasileios Amoiridis <vassilisamir@...il.com>, lars@...afoo.de,
himanshujha199640@...il.com, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 14/17] iio: chemical: bme680: Modify startup procedure
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@...il.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@...il.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;
> > }
> >
>
Powered by blists - more mailing lists