diff mbox series

[v1,07/10] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures

Message ID 20240711211558.106327-8-vassilisamir@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series BMP280: Minor cleanup and interrupt support | expand

Commit Message

Vasileios Amoiridis July 11, 2024, 9:15 p.m. UTC
This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
feature so their operation is not affected at all.

Essentially, up to now, the rest of the sensors were used in normal mode
all the time. This means that they are continuously doing measurements
even though these measurements are not used. Even though the sensor does
provide PM support, to cover all the possible use cases, the sensor needs
to go into sleep mode and wake up whenever necessary.

This commit, adds sleep and forced mode support. Essentially, the sensor
sleeps all the time except for when a measurement is requested. When there
is a request for a measurement, the sensor is put into forced mode, starts
the measurement and after it is done we read the output and we put it again
in sleep mode.

For really fast and more deterministic measurements, the triggered buffer
interface can be used, since the sensor is still used in normal mode for
that use case.

This commit does not add though support for DEEP STANDBY, Low Power NORMAL
and CONTINUOUS modes, supported only by the BMP58x version.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 276 +++++++++++++++++++++++++++--
 drivers/iio/pressure/bmp280.h      |  12 ++
 2 files changed, 269 insertions(+), 19 deletions(-)

Comments

Jonathan Cameron July 20, 2024, 11:28 a.m. UTC | #1
On Thu, 11 Jul 2024 23:15:55 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
> and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
> feature so their operation is not affected at all.
> 
> Essentially, up to now, the rest of the sensors were used in normal mode
> all the time. This means that they are continuously doing measurements
> even though these measurements are not used. Even though the sensor does
> provide PM support, to cover all the possible use cases, the sensor needs
> to go into sleep mode and wake up whenever necessary.
> 
> This commit, adds sleep and forced mode support. Essentially, the sensor
> sleeps all the time except for when a measurement is requested. When there
> is a request for a measurement, the sensor is put into forced mode, starts
> the measurement and after it is done we read the output and we put it again
> in sleep mode.
> 
> For really fast and more deterministic measurements, the triggered buffer
> interface can be used, since the sensor is still used in normal mode for
> that use case.
> 
> This commit does not add though support for DEEP STANDBY, Low Power NORMAL
> and CONTINUOUS modes, supported only by the BMP58x version.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Various minor comments inline.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 276 +++++++++++++++++++++++++++--
>  drivers/iio/pressure/bmp280.h      |  12 ++
>  2 files changed, 269 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 9c99373d66ec..fc8d42880eb8 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -145,6 +145,12 @@ enum bmp280_scan {
>  	BME280_HUMID,
>  };
>  }
>  
> +static int bmp280_set_mode(struct bmp280_data *data, u8 mode)
> +{
> +	int ret;
> +
> +	switch (mode) {
> +	case BMP280_SLEEP:
> +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> +					BMP280_MODE_MASK, BMP280_MODE_SLEEP);

Use a local variable for the BMP280_MODE_* and then have the regmap_write_bits()
after the switch statement.

Could even make it a const data look up given you are getting a value
based on the enum.

> +		break;
> +	case BMP280_FORCED:
> +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> +					BMP280_MODE_MASK, BMP280_MODE_FORCED);
> +		break;
> +	case BMP280_NORMAL:
> +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> +					BMP280_MODE_MASK, BMP280_MODE_NORMAL);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ret) {
> +		dev_err(data->dev, "failed to  write ctrl_meas register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bmp280_wait_conv(struct bmp280_data *data)
> +{
> +	unsigned int reg;
> +	int ret, meas_time;
> +
> +	meas_time = BMP280_MEAS_OFFSET;
> +
> +	if (data->oversampling_humid)
> +		meas_time += (1 << data->oversampling_humid) * BMP280_MEAS_DUR +
> +			       BMP280_PRESS_HUMID_MEAS_OFFSET;
Add a comment on why, if oversampling_humid is not set we end up with
no time for measuring humidity.  The MEAS_OFFSET is less than one MEAS_DUR
so not it's not a case of that already incorporating the time.


> +
> +	/* Pressure measurement time */
> +	meas_time += (1 << data->oversampling_press) * BMP280_MEAS_DUR +
> +		      BMP280_PRESS_HUMID_MEAS_OFFSET;
> +
> +	/* Temperature measurement time */
> +	meas_time += (1 << data->oversampling_temp) * BMP280_MEAS_DUR;
> +
> +	usleep_range(meas_time, meas_time * 12 / 10);
> +
> +	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
> +	if (ret) {
> +		dev_err(data->dev, "failed to read status register\n");
> +		return ret;
> +	}
> +	if (reg & BMP280_REG_STATUS_MEAS_BIT) {
> +		dev_err(data->dev, "Measurement cycle didn't complete\n");
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
>  static int bmp280_chip_config(struct bmp280_data *data)
>  {
>  	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> @@ -994,7 +1078,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  				BMP280_OSRS_TEMP_MASK |
>  				BMP280_OSRS_PRESS_MASK |
>  				BMP280_MODE_MASK,
> -				osrs | BMP280_MODE_NORMAL);
> +				osrs | BMP280_MODE_SLEEP);
>  	if (ret) {
>  		dev_err(data->dev, "failed to write ctrl_meas register\n");
>  		return ret;
> @@ -1100,6 +1184,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
>  	.read_temp = bmp280_read_temp,
>  	.read_press = bmp280_read_press,
>  	.read_calib = bmp280_read_calib,
> +	.set_mode = bmp280_set_mode,
> +	.wait_conv = bmp280_wait_conv,
>  	.preinit = bmp280_preinit,
>  
>  	.trigger_handler = bmp280_trigger_handler,
> @@ -1218,6 +1304,8 @@ const struct bmp280_chip_info bme280_chip_info = {
>  	.read_press = bmp280_read_press,
>  	.read_humid = bme280_read_humid,
>  	.read_calib = bme280_read_calib,
> +	.set_mode = bmp280_set_mode,
> +	.wait_conv = bmp280_wait_conv,
>  	.preinit = bmp280_preinit,
>  
>  	.trigger_handler = bme280_trigger_handler,
> @@ -1505,6 +1593,75 @@ static int bmp380_preinit(struct bmp280_data *data)
>  	return bmp380_cmd(data, BMP380_CMD_SOFT_RESET);
>  }
>  
> +static int bmp380_set_mode(struct bmp280_data *data, u8 mode)
> +{
> +	int ret;
> +
> +	switch (mode) {
> +	case BMP280_SLEEP:
> +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> +					BMP380_MODE_MASK,
> +					FIELD_PREP(BMP380_MODE_MASK,
> +						   BMP380_MODE_SLEEP));
As above. I'd use a local variable to stash the MODE* that you are going
to write or just look it up in a const array.

> +		break;
> +	case BMP280_FORCED:
> +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> +					BMP380_MODE_MASK,
> +					FIELD_PREP(BMP380_MODE_MASK,
> +						   BMP380_MODE_FORCED));
> +		break;
> +	case BMP280_NORMAL:
> +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> +					BMP380_MODE_MASK,
> +					FIELD_PREP(BMP380_MODE_MASK,
> +						   BMP380_MODE_NORMAL));
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ret) {
> +		dev_err(data->dev, "failed to  write power control register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bmp380_wait_conv(struct bmp280_data *data)
> +{
> +	unsigned int reg;
> +	int ret, meas_time;
> +
> +	/* Offset measurement time */
> +	meas_time = BMP380_MEAS_OFFSET;
> +
> +	/* Pressure measurement time */
> +	meas_time += (1 << data->oversampling_press) * BMP380_MEAS_DUR +
> +		      BMP380_PRESS_MEAS_OFFSET;
> +
> +	/* Temperature measurement time */
> +	meas_time += (1 << data->oversampling_temp) * BMP380_MEAS_DUR +
> +		      BMP380_TEMP_MEAS_OFFSET;
> +
> +	usleep_range(meas_time, meas_time * 12 / 10);
> +
> +	ret = regmap_read(data->regmap, BMP380_REG_STATUS, &reg);
> +	if (ret) {
> +		dev_err(data->dev, "failed to read status register\n");
> +		return ret;
> +	}
> +
> +	if (!(reg & BMP380_STATUS_DRDY_PRESS_MASK) ||
> +	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
> +		pr_info("Meas_time: %d\n", meas_time);

Why as pr_info?  Seems like part of the dev_err.

> +		dev_err(data->dev, "Measurement cycle didn't complete\n");
> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
>  static int bmp380_chip_config(struct bmp280_data *data)
>  {
>  	bool change = false, aux;
> @@ -1565,17 +1722,15 @@ static int bmp380_chip_config(struct bmp280_data *data)
>  		 * Resets sensor measurement loop toggling between sleep and
>  		 * normal operating modes.
>  		 */
> -		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> -					BMP380_MODE_MASK,
> -					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
> +		ret = bmp380_set_mode(data, BMP280_SLEEP);
>  		if (ret) {
>  			dev_err(data->dev, "failed to set sleep mode\n");
>  			return ret;
>  		}
> -		usleep_range(2000, 2500);
> -		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> -					BMP380_MODE_MASK,
> -					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
> +
> +		usleep_range(data->start_up_time, data->start_up_time + 500);
> +
> +		ret = bmp380_set_mode(data, BMP280_NORMAL);
>  		if (ret) {
>  			dev_err(data->dev, "failed to set normal mode\n");
>  			return ret;
> @@ -1601,6 +1756,17 @@ static int bmp380_chip_config(struct bmp280_data *data)
>  		}
>  	}
>  
> +	/* Dummy read to empty data registers. */
> +	ret = bmp380_read_press(data, &tmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = bmp380_set_mode(data, BMP280_SLEEP);
> +	if (ret) {
> +		dev_err(data->dev, "failed to set sleep mode\n");
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1693,6 +1859,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
>  	.read_temp = bmp380_read_temp,
>  	.read_press = bmp380_read_press,
>  	.read_calib = bmp380_read_calib,
> +	.set_mode = bmp380_set_mode,
> +	.wait_conv = bmp380_wait_conv,
>  	.preinit = bmp380_preinit,
>  
>  	.trigger_handler = bmp380_trigger_handler,
> @@ -2080,6 +2248,65 @@ static int bmp580_preinit(struct bmp280_data *data)
>  	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
>  }
>  
> +static int bmp580_set_mode(struct bmp280_data *data, u8 mode)
> +{
> +	int ret;
> +
> +	switch (mode) {
> +	case BMP280_SLEEP:
> +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> +					BMP580_MODE_MASK,
> +					FIELD_PREP(BMP580_MODE_MASK,
> +						   BMP580_MODE_SLEEP));
> +		break;
> +	case BMP280_FORCED:
> +		ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG,
> +				      BMP580_DSP_IIR_FORCED_FLUSH);
> +
check that ret.

> +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> +					BMP580_MODE_MASK,
> +					FIELD_PREP(BMP580_MODE_MASK,
> +						   BMP580_MODE_FORCED));
This one is more complex so a switch statement makes sense here.
> +		break;
> +	case BMP280_NORMAL:
> +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> +					BMP580_MODE_MASK,
> +					FIELD_PREP(BMP580_MODE_MASK,
> +						   BMP580_MODE_NORMAL));
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (ret) {
> +		dev_err(data->dev, "failed to  write power control register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int bmp580_wait_conv(struct bmp280_data *data)
> +{
> +	/*
> +	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
> +	 * characteristics
> +	 */
> +	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
> +					42420, 84420};
> +	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
> +				       11340, 21840};
> +	int meas_time;
> +
> +	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
> +			   time_conv_press[data->oversampling_press];
> +
> +	usleep_range(meas_time, meas_time * 12 / 10);
> +
> +	return 0;
> +}
> +
one blank line only.
> +
>  static int bmp580_chip_config(struct bmp280_data *data)
>  {
>  	bool change = false, aux;
> @@ -2150,17 +2377,6 @@ static int bmp580_chip_config(struct bmp280_data *data)
>  		return ret;
>  	}
>  
> -	/* Restore sensor to normal operation mode */
> -	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> -				BMP580_MODE_MASK,
> -				FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
> -	if (ret) {
> -		dev_err(data->dev, "failed to set normal mode\n");
> -		return ret;
> -	}
> -	/* From datasheet's table 4: electrical characteristics */
> -	usleep_range(3000, 3500);
> -
>  	if (change) {
>  		/*
>  		 * Check if ODR and OSR settings are valid or we are
> @@ -2256,6 +2472,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
>  	.chip_config = bmp580_chip_config,
>  	.read_temp = bmp580_read_temp,
>  	.read_press = bmp580_read_press,
> +	.set_mode = bmp580_set_mode,
> +	.wait_conv = bmp580_wait_conv,
>  	.preinit = bmp580_preinit,
>  
>  	.trigger_handler = bmp580_trigger_handler,
> @@ -2503,6 +2721,16 @@ static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
>  	return 0;
>  }
>  
> +static int bmp180_set_mode(struct bmp280_data *data, u8 mode)
> +{
> +	return 0;
Add a comment on why these are stubs.  It's in the patch description, but
better to have it recorded in the code.

> +}
> +
> +static int bmp180_wait_conv(struct bmp280_data *data)
> +{
> +	return 0;
> +}
> +
>
Vasileios Amoiridis July 21, 2024, 10:11 p.m. UTC | #2
On Sat, Jul 20, 2024 at 12:28:02PM +0100, Jonathan Cameron wrote:
> On Thu, 11 Jul 2024 23:15:55 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
> > and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
> > feature so their operation is not affected at all.
> > 
> > Essentially, up to now, the rest of the sensors were used in normal mode
> > all the time. This means that they are continuously doing measurements
> > even though these measurements are not used. Even though the sensor does
> > provide PM support, to cover all the possible use cases, the sensor needs
> > to go into sleep mode and wake up whenever necessary.
> > 
> > This commit, adds sleep and forced mode support. Essentially, the sensor
> > sleeps all the time except for when a measurement is requested. When there
> > is a request for a measurement, the sensor is put into forced mode, starts
> > the measurement and after it is done we read the output and we put it again
> > in sleep mode.
> > 
> > For really fast and more deterministic measurements, the triggered buffer
> > interface can be used, since the sensor is still used in normal mode for
> > that use case.
> > 
> > This commit does not add though support for DEEP STANDBY, Low Power NORMAL
> > and CONTINUOUS modes, supported only by the BMP58x version.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Various minor comments inline.
> 
> Thanks,
> 
> Jonathan
> 

Hi Jonathan,

Thanks again for the amazing feedback!

My answers inline.

Cheers,
Vasilis
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 276 +++++++++++++++++++++++++++--
> >  drivers/iio/pressure/bmp280.h      |  12 ++
> >  2 files changed, 269 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 9c99373d66ec..fc8d42880eb8 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -145,6 +145,12 @@ enum bmp280_scan {
> >  	BME280_HUMID,
> >  };
> >  }
> >  
> > +static int bmp280_set_mode(struct bmp280_data *data, u8 mode)
> > +{
> > +	int ret;
> > +
> > +	switch (mode) {
> > +	case BMP280_SLEEP:
> > +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> > +					BMP280_MODE_MASK, BMP280_MODE_SLEEP);
> 
> Use a local variable for the BMP280_MODE_* and then have the regmap_write_bits()
> after the switch statement.
> 
> Could even make it a const data look up given you are getting a value
> based on the enum.
> 

I like a lot both approaches, I feel like the const array one I like it more.

> > +		break;
> > +	case BMP280_FORCED:
> > +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> > +					BMP280_MODE_MASK, BMP280_MODE_FORCED);
> > +		break;
> > +	case BMP280_NORMAL:
> > +		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> > +					BMP280_MODE_MASK, BMP280_MODE_NORMAL);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to  write ctrl_meas register\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int bmp280_wait_conv(struct bmp280_data *data)
> > +{
> > +	unsigned int reg;
> > +	int ret, meas_time;
> > +
> > +	meas_time = BMP280_MEAS_OFFSET;
> > +
> > +	if (data->oversampling_humid)
> > +		meas_time += (1 << data->oversampling_humid) * BMP280_MEAS_DUR +
> > +			       BMP280_PRESS_HUMID_MEAS_OFFSET;
> Add a comment on why, if oversampling_humid is not set we end up with
> no time for measuring humidity.  The MEAS_OFFSET is less than one MEAS_DUR
> so not it's not a case of that already incorporating the time.
> 

This is a check for if we use the BME280 or the BMP280 since humidity is a
feature of only the BME280 sensor. I should add it, thanks.

> 
> > +
> > +	/* Pressure measurement time */
> > +	meas_time += (1 << data->oversampling_press) * BMP280_MEAS_DUR +
> > +		      BMP280_PRESS_HUMID_MEAS_OFFSET;
> > +
> > +	/* Temperature measurement time */
> > +	meas_time += (1 << data->oversampling_temp) * BMP280_MEAS_DUR;
> > +
> > +	usleep_range(meas_time, meas_time * 12 / 10);
> > +
> > +	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to read status register\n");
> > +		return ret;
> > +	}
> > +	if (reg & BMP280_REG_STATUS_MEAS_BIT) {
> > +		dev_err(data->dev, "Measurement cycle didn't complete\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int bmp280_chip_config(struct bmp280_data *data)
> >  {
> >  	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> > @@ -994,7 +1078,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
> >  				BMP280_OSRS_TEMP_MASK |
> >  				BMP280_OSRS_PRESS_MASK |
> >  				BMP280_MODE_MASK,
> > -				osrs | BMP280_MODE_NORMAL);
> > +				osrs | BMP280_MODE_SLEEP);
> >  	if (ret) {
> >  		dev_err(data->dev, "failed to write ctrl_meas register\n");
> >  		return ret;
> > @@ -1100,6 +1184,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
> >  	.read_temp = bmp280_read_temp,
> >  	.read_press = bmp280_read_press,
> >  	.read_calib = bmp280_read_calib,
> > +	.set_mode = bmp280_set_mode,
> > +	.wait_conv = bmp280_wait_conv,
> >  	.preinit = bmp280_preinit,
> >  
> >  	.trigger_handler = bmp280_trigger_handler,
> > @@ -1218,6 +1304,8 @@ const struct bmp280_chip_info bme280_chip_info = {
> >  	.read_press = bmp280_read_press,
> >  	.read_humid = bme280_read_humid,
> >  	.read_calib = bme280_read_calib,
> > +	.set_mode = bmp280_set_mode,
> > +	.wait_conv = bmp280_wait_conv,
> >  	.preinit = bmp280_preinit,
> >  
> >  	.trigger_handler = bme280_trigger_handler,
> > @@ -1505,6 +1593,75 @@ static int bmp380_preinit(struct bmp280_data *data)
> >  	return bmp380_cmd(data, BMP380_CMD_SOFT_RESET);
> >  }
> >  
> > +static int bmp380_set_mode(struct bmp280_data *data, u8 mode)
> > +{
> > +	int ret;
> > +
> > +	switch (mode) {
> > +	case BMP280_SLEEP:
> > +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > +					BMP380_MODE_MASK,
> > +					FIELD_PREP(BMP380_MODE_MASK,
> > +						   BMP380_MODE_SLEEP));
> As above. I'd use a local variable to stash the MODE* that you are going
> to write or just look it up in a const array.
> 

Ack.

> > +		break;
> > +	case BMP280_FORCED:
> > +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > +					BMP380_MODE_MASK,
> > +					FIELD_PREP(BMP380_MODE_MASK,
> > +						   BMP380_MODE_FORCED));
> > +		break;
> > +	case BMP280_NORMAL:
> > +		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > +					BMP380_MODE_MASK,
> > +					FIELD_PREP(BMP380_MODE_MASK,
> > +						   BMP380_MODE_NORMAL));
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to  write power control register\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int bmp380_wait_conv(struct bmp280_data *data)
> > +{
> > +	unsigned int reg;
> > +	int ret, meas_time;
> > +
> > +	/* Offset measurement time */
> > +	meas_time = BMP380_MEAS_OFFSET;
> > +
> > +	/* Pressure measurement time */
> > +	meas_time += (1 << data->oversampling_press) * BMP380_MEAS_DUR +
> > +		      BMP380_PRESS_MEAS_OFFSET;
> > +
> > +	/* Temperature measurement time */
> > +	meas_time += (1 << data->oversampling_temp) * BMP380_MEAS_DUR +
> > +		      BMP380_TEMP_MEAS_OFFSET;
> > +
> > +	usleep_range(meas_time, meas_time * 12 / 10);
> > +
> > +	ret = regmap_read(data->regmap, BMP380_REG_STATUS, &reg);
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to read status register\n");
> > +		return ret;
> > +	}
> > +
> > +	if (!(reg & BMP380_STATUS_DRDY_PRESS_MASK) ||
> > +	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
> > +		pr_info("Meas_time: %d\n", meas_time);
> 
> Why as pr_info?  Seems like part of the dev_err.
> 

This is one of my forgotten "debug" messages...

> > +		dev_err(data->dev, "Measurement cycle didn't complete\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int bmp380_chip_config(struct bmp280_data *data)
> >  {
> >  	bool change = false, aux;
> > @@ -1565,17 +1722,15 @@ static int bmp380_chip_config(struct bmp280_data *data)
> >  		 * Resets sensor measurement loop toggling between sleep and
> >  		 * normal operating modes.
> >  		 */
> > -		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > -					BMP380_MODE_MASK,
> > -					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
> > +		ret = bmp380_set_mode(data, BMP280_SLEEP);
> >  		if (ret) {
> >  			dev_err(data->dev, "failed to set sleep mode\n");
> >  			return ret;
> >  		}
> > -		usleep_range(2000, 2500);
> > -		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > -					BMP380_MODE_MASK,
> > -					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
> > +
> > +		usleep_range(data->start_up_time, data->start_up_time + 500);
> > +
> > +		ret = bmp380_set_mode(data, BMP280_NORMAL);
> >  		if (ret) {
> >  			dev_err(data->dev, "failed to set normal mode\n");
> >  			return ret;
> > @@ -1601,6 +1756,17 @@ static int bmp380_chip_config(struct bmp280_data *data)
> >  		}
> >  	}
> >  
> > +	/* Dummy read to empty data registers. */
> > +	ret = bmp380_read_press(data, &tmp);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = bmp380_set_mode(data, BMP280_SLEEP);
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to set sleep mode\n");
> > +		return ret;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1693,6 +1859,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
> >  	.read_temp = bmp380_read_temp,
> >  	.read_press = bmp380_read_press,
> >  	.read_calib = bmp380_read_calib,
> > +	.set_mode = bmp380_set_mode,
> > +	.wait_conv = bmp380_wait_conv,
> >  	.preinit = bmp380_preinit,
> >  
> >  	.trigger_handler = bmp380_trigger_handler,
> > @@ -2080,6 +2248,65 @@ static int bmp580_preinit(struct bmp280_data *data)
> >  	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
> >  }
> >  
> > +static int bmp580_set_mode(struct bmp280_data *data, u8 mode)
> > +{
> > +	int ret;
> > +
> > +	switch (mode) {
> > +	case BMP280_SLEEP:
> > +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> > +					BMP580_MODE_MASK,
> > +					FIELD_PREP(BMP580_MODE_MASK,
> > +						   BMP580_MODE_SLEEP));
> > +		break;
> > +	case BMP280_FORCED:
> > +		ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG,
> > +				      BMP580_DSP_IIR_FORCED_FLUSH);
> > +
> check that ret.
> 

True, will do.

> > +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> > +					BMP580_MODE_MASK,
> > +					FIELD_PREP(BMP580_MODE_MASK,
> > +						   BMP580_MODE_FORCED));
> This one is more complex so a switch statement makes sense here.
> > +		break;
> > +	case BMP280_NORMAL:
> > +		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> > +					BMP580_MODE_MASK,
> > +					FIELD_PREP(BMP580_MODE_MASK,
> > +						   BMP580_MODE_NORMAL));
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ret) {
> > +		dev_err(data->dev, "failed to  write power control register\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int bmp580_wait_conv(struct bmp280_data *data)
> > +{
> > +	/*
> > +	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
> > +	 * characteristics
> > +	 */
> > +	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
> > +					42420, 84420};
> > +	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
> > +				       11340, 21840};
> > +	int meas_time;
> > +
> > +	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
> > +			   time_conv_press[data->oversampling_press];
> > +
> > +	usleep_range(meas_time, meas_time * 12 / 10);
> > +
> > +	return 0;
> > +}
> > +
> one blank line only.

True, will do.
> > +
> >  static int bmp580_chip_config(struct bmp280_data *data)
> >  {
> >  	bool change = false, aux;
> > @@ -2150,17 +2377,6 @@ static int bmp580_chip_config(struct bmp280_data *data)
> >  		return ret;
> >  	}
> >  
> > -	/* Restore sensor to normal operation mode */
> > -	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
> > -				BMP580_MODE_MASK,
> > -				FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
> > -	if (ret) {
> > -		dev_err(data->dev, "failed to set normal mode\n");
> > -		return ret;
> > -	}
> > -	/* From datasheet's table 4: electrical characteristics */
> > -	usleep_range(3000, 3500);
> > -
> >  	if (change) {
> >  		/*
> >  		 * Check if ODR and OSR settings are valid or we are
> > @@ -2256,6 +2472,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
> >  	.chip_config = bmp580_chip_config,
> >  	.read_temp = bmp580_read_temp,
> >  	.read_press = bmp580_read_press,
> > +	.set_mode = bmp580_set_mode,
> > +	.wait_conv = bmp580_wait_conv,
> >  	.preinit = bmp580_preinit,
> >  
> >  	.trigger_handler = bmp580_trigger_handler,
> > @@ -2503,6 +2721,16 @@ static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
> >  	return 0;
> >  }
> >  
> > +static int bmp180_set_mode(struct bmp280_data *data, u8 mode)
> > +{
> > +	return 0;
> Add a comment on why these are stubs.  It's in the patch description, but
> better to have it recorded in the code.
> 

I didn't add because as you can see below there is another function exactly
like this one that doesn't have one. Should I add also to the other one?

> > +}
> > +
> > +static int bmp180_wait_conv(struct bmp280_data *data)
> > +{
> > +	return 0;
> > +}
> > +
> >
Jonathan Cameron July 22, 2024, 7:15 p.m. UTC | #3
> > >  
> > >  	.trigger_handler = bmp580_trigger_handler,
> > > @@ -2503,6 +2721,16 @@ static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
> > >  	return 0;
> > >  }
> > >  
> > > +static int bmp180_set_mode(struct bmp280_data *data, u8 mode)
> > > +{
> > > +	return 0;  
> > Add a comment on why these are stubs.  It's in the patch description, but
> > better to have it recorded in the code.
> >   
> 
> I didn't add because as you can see below there is another function exactly
> like this one that doesn't have one. Should I add also to the other one?
Hmm. Maybe? :)

> 
> > > +}
> > > +
> > > +static int bmp180_wait_conv(struct bmp280_data *data)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > >
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 9c99373d66ec..fc8d42880eb8 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -145,6 +145,12 @@  enum bmp280_scan {
 	BME280_HUMID,
 };
 
+enum bmp280_power_modes {
+	BMP280_SLEEP,
+	BMP280_NORMAL,
+	BMP280_FORCED,
+};
+
 static const struct iio_chan_spec bmp280_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -610,6 +616,14 @@  static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
+		ret = data->chip_info->set_mode(data, BMP280_FORCED);
+		if (ret)
+			return ret;
+
+		ret = data->chip_info->wait_conv(data);
+		if (ret)
+			return ret;
+
 		switch (chan->type) {
 		case IIO_HUMIDITYRELATIVE:
 			ret = data->chip_info->read_humid(data, &chan_value);
@@ -639,6 +653,14 @@  static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_RAW:
+		ret = data->chip_info->set_mode(data, BMP280_FORCED);
+		if (ret)
+			return ret;
+
+		ret = data->chip_info->wait_conv(data);
+		if (ret)
+			return ret;
+
 		switch (chan->type) {
 		case IIO_HUMIDITYRELATIVE:
 			ret = data->chip_info->read_humid(data, &chan_value);
@@ -984,6 +1006,68 @@  static int bmp280_preinit(struct bmp280_data *data)
 	return 0;
 }
 
+static int bmp280_set_mode(struct bmp280_data *data, u8 mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
+					BMP280_MODE_MASK, BMP280_MODE_SLEEP);
+		break;
+	case BMP280_FORCED:
+		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
+					BMP280_MODE_MASK, BMP280_MODE_FORCED);
+		break;
+	case BMP280_NORMAL:
+		ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
+					BMP280_MODE_MASK, BMP280_MODE_NORMAL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(data->dev, "failed to  write ctrl_meas register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp280_wait_conv(struct bmp280_data *data)
+{
+	unsigned int reg;
+	int ret, meas_time;
+
+	meas_time = BMP280_MEAS_OFFSET;
+
+	if (data->oversampling_humid)
+		meas_time += (1 << data->oversampling_humid) * BMP280_MEAS_DUR +
+			       BMP280_PRESS_HUMID_MEAS_OFFSET;
+
+	/* Pressure measurement time */
+	meas_time += (1 << data->oversampling_press) * BMP280_MEAS_DUR +
+		      BMP280_PRESS_HUMID_MEAS_OFFSET;
+
+	/* Temperature measurement time */
+	meas_time += (1 << data->oversampling_temp) * BMP280_MEAS_DUR;
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "failed to read status register\n");
+		return ret;
+	}
+	if (reg & BMP280_REG_STATUS_MEAS_BIT) {
+		dev_err(data->dev, "Measurement cycle didn't complete\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static int bmp280_chip_config(struct bmp280_data *data)
 {
 	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
@@ -994,7 +1078,7 @@  static int bmp280_chip_config(struct bmp280_data *data)
 				BMP280_OSRS_TEMP_MASK |
 				BMP280_OSRS_PRESS_MASK |
 				BMP280_MODE_MASK,
-				osrs | BMP280_MODE_NORMAL);
+				osrs | BMP280_MODE_SLEEP);
 	if (ret) {
 		dev_err(data->dev, "failed to write ctrl_meas register\n");
 		return ret;
@@ -1100,6 +1184,8 @@  const struct bmp280_chip_info bmp280_chip_info = {
 	.read_temp = bmp280_read_temp,
 	.read_press = bmp280_read_press,
 	.read_calib = bmp280_read_calib,
+	.set_mode = bmp280_set_mode,
+	.wait_conv = bmp280_wait_conv,
 	.preinit = bmp280_preinit,
 
 	.trigger_handler = bmp280_trigger_handler,
@@ -1218,6 +1304,8 @@  const struct bmp280_chip_info bme280_chip_info = {
 	.read_press = bmp280_read_press,
 	.read_humid = bme280_read_humid,
 	.read_calib = bme280_read_calib,
+	.set_mode = bmp280_set_mode,
+	.wait_conv = bmp280_wait_conv,
 	.preinit = bmp280_preinit,
 
 	.trigger_handler = bme280_trigger_handler,
@@ -1505,6 +1593,75 @@  static int bmp380_preinit(struct bmp280_data *data)
 	return bmp380_cmd(data, BMP380_CMD_SOFT_RESET);
 }
 
+static int bmp380_set_mode(struct bmp280_data *data, u8 mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
+					BMP380_MODE_MASK,
+					FIELD_PREP(BMP380_MODE_MASK,
+						   BMP380_MODE_SLEEP));
+		break;
+	case BMP280_FORCED:
+		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
+					BMP380_MODE_MASK,
+					FIELD_PREP(BMP380_MODE_MASK,
+						   BMP380_MODE_FORCED));
+		break;
+	case BMP280_NORMAL:
+		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
+					BMP380_MODE_MASK,
+					FIELD_PREP(BMP380_MODE_MASK,
+						   BMP380_MODE_NORMAL));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(data->dev, "failed to  write power control register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp380_wait_conv(struct bmp280_data *data)
+{
+	unsigned int reg;
+	int ret, meas_time;
+
+	/* Offset measurement time */
+	meas_time = BMP380_MEAS_OFFSET;
+
+	/* Pressure measurement time */
+	meas_time += (1 << data->oversampling_press) * BMP380_MEAS_DUR +
+		      BMP380_PRESS_MEAS_OFFSET;
+
+	/* Temperature measurement time */
+	meas_time += (1 << data->oversampling_temp) * BMP380_MEAS_DUR +
+		      BMP380_TEMP_MEAS_OFFSET;
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	ret = regmap_read(data->regmap, BMP380_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "failed to read status register\n");
+		return ret;
+	}
+
+	if (!(reg & BMP380_STATUS_DRDY_PRESS_MASK) ||
+	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
+		pr_info("Meas_time: %d\n", meas_time);
+		dev_err(data->dev, "Measurement cycle didn't complete\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static int bmp380_chip_config(struct bmp280_data *data)
 {
 	bool change = false, aux;
@@ -1565,17 +1722,15 @@  static int bmp380_chip_config(struct bmp280_data *data)
 		 * Resets sensor measurement loop toggling between sleep and
 		 * normal operating modes.
 		 */
-		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
-					BMP380_MODE_MASK,
-					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
+		ret = bmp380_set_mode(data, BMP280_SLEEP);
 		if (ret) {
 			dev_err(data->dev, "failed to set sleep mode\n");
 			return ret;
 		}
-		usleep_range(2000, 2500);
-		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
-					BMP380_MODE_MASK,
-					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
+
+		usleep_range(data->start_up_time, data->start_up_time + 500);
+
+		ret = bmp380_set_mode(data, BMP280_NORMAL);
 		if (ret) {
 			dev_err(data->dev, "failed to set normal mode\n");
 			return ret;
@@ -1601,6 +1756,17 @@  static int bmp380_chip_config(struct bmp280_data *data)
 		}
 	}
 
+	/* Dummy read to empty data registers. */
+	ret = bmp380_read_press(data, &tmp);
+	if (ret)
+		return ret;
+
+	ret = bmp380_set_mode(data, BMP280_SLEEP);
+	if (ret) {
+		dev_err(data->dev, "failed to set sleep mode\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1693,6 +1859,8 @@  const struct bmp280_chip_info bmp380_chip_info = {
 	.read_temp = bmp380_read_temp,
 	.read_press = bmp380_read_press,
 	.read_calib = bmp380_read_calib,
+	.set_mode = bmp380_set_mode,
+	.wait_conv = bmp380_wait_conv,
 	.preinit = bmp380_preinit,
 
 	.trigger_handler = bmp380_trigger_handler,
@@ -2080,6 +2248,65 @@  static int bmp580_preinit(struct bmp280_data *data)
 	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
 }
 
+static int bmp580_set_mode(struct bmp280_data *data, u8 mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+					BMP580_MODE_MASK,
+					FIELD_PREP(BMP580_MODE_MASK,
+						   BMP580_MODE_SLEEP));
+		break;
+	case BMP280_FORCED:
+		ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG,
+				      BMP580_DSP_IIR_FORCED_FLUSH);
+
+		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+					BMP580_MODE_MASK,
+					FIELD_PREP(BMP580_MODE_MASK,
+						   BMP580_MODE_FORCED));
+		break;
+	case BMP280_NORMAL:
+		ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+					BMP580_MODE_MASK,
+					FIELD_PREP(BMP580_MODE_MASK,
+						   BMP580_MODE_NORMAL));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(data->dev, "failed to  write power control register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp580_wait_conv(struct bmp280_data *data)
+{
+	/*
+	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
+	 * characteristics
+	 */
+	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
+					42420, 84420};
+	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
+				       11340, 21840};
+	int meas_time;
+
+	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
+			   time_conv_press[data->oversampling_press];
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	return 0;
+}
+
+
 static int bmp580_chip_config(struct bmp280_data *data)
 {
 	bool change = false, aux;
@@ -2150,17 +2377,6 @@  static int bmp580_chip_config(struct bmp280_data *data)
 		return ret;
 	}
 
-	/* Restore sensor to normal operation mode */
-	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
-				BMP580_MODE_MASK,
-				FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
-	if (ret) {
-		dev_err(data->dev, "failed to set normal mode\n");
-		return ret;
-	}
-	/* From datasheet's table 4: electrical characteristics */
-	usleep_range(3000, 3500);
-
 	if (change) {
 		/*
 		 * Check if ODR and OSR settings are valid or we are
@@ -2256,6 +2472,8 @@  const struct bmp280_chip_info bmp580_chip_info = {
 	.chip_config = bmp580_chip_config,
 	.read_temp = bmp580_read_temp,
 	.read_press = bmp580_read_press,
+	.set_mode = bmp580_set_mode,
+	.wait_conv = bmp580_wait_conv,
 	.preinit = bmp580_preinit,
 
 	.trigger_handler = bmp580_trigger_handler,
@@ -2503,6 +2721,16 @@  static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
 	return 0;
 }
 
+static int bmp180_set_mode(struct bmp280_data *data, u8 mode)
+{
+	return 0;
+}
+
+static int bmp180_wait_conv(struct bmp280_data *data)
+{
+	return 0;
+}
+
 static int bmp180_chip_config(struct bmp280_data *data)
 {
 	return 0;
@@ -2573,6 +2801,8 @@  const struct bmp280_chip_info bmp180_chip_info = {
 	.read_temp = bmp180_read_temp,
 	.read_press = bmp180_read_press,
 	.read_calib = bmp180_read_calib,
+	.set_mode = bmp180_set_mode,
+	.wait_conv = bmp180_wait_conv,
 
 	.trigger_handler = bmp180_trigger_handler,
 };
@@ -2625,6 +2855,7 @@  static int bmp280_buffer_preenable(struct iio_dev *indio_dev)
 	struct bmp280_data *data = iio_priv(indio_dev);
 
 	pm_runtime_get_sync(data->dev);
+	data->chip_info->set_mode(data, BMP280_NORMAL);
 
 	return 0;
 }
@@ -2795,6 +3026,10 @@  int bmp280_common_probe(struct device *dev,
 			return ret;
 	}
 
+	ret = data->chip_info->set_mode(data, BMP280_SLEEP);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set sleep mode\n");
+
 	/* Enable runtime PM */
 	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
@@ -2820,6 +3055,9 @@  static int bmp280_runtime_suspend(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmp280_data *data = iio_priv(indio_dev);
 
+	data->chip_info->set_mode(data, BMP280_SLEEP);
+
+	usleep_range(2500, 3000);
 	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 }
 
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 02647454bd02..93c006c33552 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -170,6 +170,11 @@ 
 #define BMP380_MODE_FORCED		1
 #define BMP380_MODE_NORMAL		3
 
+#define BMP380_MEAS_OFFSET		234
+#define BMP380_MEAS_DUR			2020
+#define BMP380_TEMP_MEAS_OFFSET		163
+#define BMP380_PRESS_MEAS_OFFSET	392
+
 #define BMP380_MIN_TEMP			-4000
 #define BMP380_MAX_TEMP			8500
 #define BMP380_MIN_PRES			3000000
@@ -206,6 +211,7 @@ 
 #define BMP280_REG_CTRL_MEAS		0xF4
 #define BMP280_REG_STATUS		0xF3
 #define BMP280_REG_STATUS_IM_UPDATE	BIT(0)
+#define BMP280_REG_STATUS_MEAS_BIT	BIT(3)
 #define BMP280_REG_RESET		0xE0
 #define BMP280_RST_SOFT_CMD		0xB6
 
@@ -246,6 +252,10 @@ 
 #define BMP280_MODE_FORCED		1
 #define BMP280_MODE_NORMAL		3
 
+#define BMP280_MEAS_OFFSET		1250
+#define BMP280_MEAS_DUR			2300
+#define BMP280_PRESS_HUMID_MEAS_OFFSET	575
+
 /* BME280 specific registers */
 #define BME280_REG_HUMIDITY_LSB		0xFE
 #define BME280_REG_HUMIDITY_MSB		0xFD
@@ -484,6 +494,8 @@  struct bmp280_chip_info {
 	int (*read_humid)(struct bmp280_data *data, u32 *adc_humidity);
 	int (*read_calib)(struct bmp280_data *data);
 	int (*preinit)(struct bmp280_data *data);
+	int (*set_mode)(struct bmp280_data *data, u8 mode);
+	int (*wait_conv)(struct bmp280_data *data);
 
 	irqreturn_t (*trigger_handler)(int irq, void *p);
 };