Message ID | 20241007194945.66192-2-vassilisamir@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | pressure: bmp280: Minor cleanup and interrupt support | expand |
Hi Vasileios, kernel test robot noticed the following build warnings: [auto build test WARNING on 96be67caa0f0420d4128cb67f07bbd7a6f49e03a] url: https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-pressure-bmp280-Use-sleep-and-forced-mode-for-oneshot-captures/20241008-035238 base: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a patch link: https://lore.kernel.org/r/20241007194945.66192-2-vassilisamir%40gmail.com patch subject: [PATCH v8 1/4] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures config: i386-randconfig-006-20241011 (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410111221.YIeXHxOv-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/iio/pressure/bmp280-core.c:1051:3: warning: variable 'meas_time_us' is uninitialized when used here [-Wuninitialized] 1051 | meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + | ^~~~~~~~~~~~ drivers/iio/pressure/bmp280-core.c:1046:32: note: initialize the variable 'meas_time_us' to silence this warning 1046 | unsigned int reg, meas_time_us; | ^ | = 0 drivers/iio/pressure/bmp280-core.c:2452:2: warning: variable 'offset' is uninitialized when used here [-Wuninitialized] 2452 | offset += sizeof(s32); | ^~~~~~ drivers/iio/pressure/bmp280-core.c:2437:17: note: initialize the variable 'offset' to silence this warning 2437 | int ret, offset; | ^ | = 0 2 warnings generated. vim +/meas_time_us +1051 drivers/iio/pressure/bmp280-core.c 1043 1044 static int bmp280_wait_conv(struct bmp280_data *data) 1045 { 1046 unsigned int reg, meas_time_us; 1047 int ret; 1048 1049 /* Check if we are using a BME280 device */ 1050 if (data->oversampling_humid) > 1051 meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + 1052 BIT(data->oversampling_humid) * BMP280_MEAS_DUR; 1053 1054 /* Pressure measurement time */ 1055 meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + 1056 BIT(data->oversampling_press) * BMP280_MEAS_DUR; 1057 1058 /* Temperature measurement time */ 1059 meas_time_us += BIT(data->oversampling_temp) * BMP280_MEAS_DUR; 1060 1061 /* Waiting time according to the BM(P/E)2 Sensor API */ 1062 fsleep(meas_time_us); 1063 1064 ret = regmap_read(data->regmap, BMP280_REG_STATUS, ®); 1065 if (ret) { 1066 dev_err(data->dev, "failed to read status register.\n"); 1067 return ret; 1068 } 1069 1070 if (reg & BMP280_REG_STATUS_MEAS_BIT) { 1071 dev_err(data->dev, "Measurement cycle didn't complete.\n"); 1072 return -EBUSY; 1073 } 1074 1075 return 0; 1076 } 1077
On Fri, Oct 11, 2024 at 12:32:12PM +0800, kernel test robot wrote: > Hi Vasileios, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on 96be67caa0f0420d4128cb67f07bbd7a6f49e03a] > > url: https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-pressure-bmp280-Use-sleep-and-forced-mode-for-oneshot-captures/20241008-035238 > base: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a > patch link: https://lore.kernel.org/r/20241007194945.66192-2-vassilisamir%40gmail.com > patch subject: [PATCH v8 1/4] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures > config: i386-randconfig-006-20241011 (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/config) > compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202410111221.YIeXHxOv-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > >> drivers/iio/pressure/bmp280-core.c:1051:3: warning: variable 'meas_time_us' is uninitialized when used here [-Wuninitialized] > 1051 | meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + > | ^~~~~~~~~~~~ > drivers/iio/pressure/bmp280-core.c:1046:32: note: initialize the variable 'meas_time_us' to silence this warning > 1046 | unsigned int reg, meas_time_us; > | ^ > | = 0 > drivers/iio/pressure/bmp280-core.c:2452:2: warning: variable 'offset' is uninitialized when used here [-Wuninitialized] > 2452 | offset += sizeof(s32); > | ^~~~~~ > drivers/iio/pressure/bmp280-core.c:2437:17: note: initialize the variable 'offset' to silence this warning > 2437 | int ret, offset; > | ^ > | = 0 Rarely, but looks like this suggestion is okay, rather I would do it as 'else' branch and convert '+=' in the 'if' part to be '='.
On Fri, Oct 11, 2024 at 01:53:21PM +0300, Andy Shevchenko wrote: > On Fri, Oct 11, 2024 at 12:32:12PM +0800, kernel test robot wrote: > > Hi Vasileios, > > > > kernel test robot noticed the following build warnings: > > > > [auto build test WARNING on 96be67caa0f0420d4128cb67f07bbd7a6f49e03a] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-pressure-bmp280-Use-sleep-and-forced-mode-for-oneshot-captures/20241008-035238 > > base: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a > > patch link: https://lore.kernel.org/r/20241007194945.66192-2-vassilisamir%40gmail.com > > patch subject: [PATCH v8 1/4] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures > > config: i386-randconfig-006-20241011 (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/config) > > compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241011/202410111221.YIeXHxOv-lkp@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202410111221.YIeXHxOv-lkp@intel.com/ > > > > All warnings (new ones prefixed by >>): > > > > >> drivers/iio/pressure/bmp280-core.c:1051:3: warning: variable 'meas_time_us' is uninitialized when used here [-Wuninitialized] > > 1051 | meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + > > | ^~~~~~~~~~~~ > > drivers/iio/pressure/bmp280-core.c:1046:32: note: initialize the variable 'meas_time_us' to silence this warning > > 1046 | unsigned int reg, meas_time_us; > > | ^ > > | = 0 > > drivers/iio/pressure/bmp280-core.c:2452:2: warning: variable 'offset' is uninitialized when used here [-Wuninitialized] > > 2452 | offset += sizeof(s32); > > | ^~~~~~ > > drivers/iio/pressure/bmp280-core.c:2437:17: note: initialize the variable 'offset' to silence this warning > > 2437 | int ret, offset; > > | ^ > > | = 0 > > Rarely, but looks like this suggestion is okay, rather I would do it as 'else' > branch and convert '+=' in the 'if' part to be '='. Hi Andy, I though exactly the same, thanks for confirming my thoughts and thanks for taking the time to suggest it! Cheers, Vasilis > > -- > With Best Regards, > Andy Shevchenko > >
On Mon, 7 Oct 2024 21:49:42 +0200 Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > Add 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. > > The idea is that the sensor is by default in sleep mode, wakes up in > forced mode when a oneshot capture is requested, or in normal mode > when the buffer is enabled. The difference lays in the fact that in > forced mode, the sensor does only one conversion and goes back to sleep > while in normal mode, the sensor does continuous measurements with the > frequency that was set in the ODR registers. > > The bmpX_chip_config() functions which are responsible for applying > the requested configuration to the sensor, are modified accordingly > in order to set the sensor by default in sleep mode. > > DEEP STANDBY, Low Power NORMAL and CONTINUOUS modes, supported only by > the BMP58x version, are not added. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> Hi Vasilieos Given it looks like you'll be doing a v9 anyway, a few comments inline on some minor simplifications and potential readability improvements. Thanks, Jonathan > --- > drivers/iio/pressure/bmp280-core.c | 296 +++++++++++++++++++++++++++-- > drivers/iio/pressure/bmp280.h | 21 ++ > 2 files changed, 296 insertions(+), 21 deletions(-) > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c > index 6811619c6f11..9ad29cf4c2ac 100644 > --- a/drivers/iio/pressure/bmp280-core.c > +++ b/drivers/iio/pressure/bmp280-core.c > @@ -16,6 +16,11 @@ > @@ -1522,6 +1610,71 @@ static int bmp380_preinit(struct bmp280_data *data) > return bmp380_cmd(data, BMP380_CMD_SOFT_RESET); > } > > +static const u8 bmp380_operation_mode[] = { > + BMP380_MODE_SLEEP, BMP380_MODE_FORCED, BMP380_MODE_NORMAL, > +}; As below - I'd assign these to specific entries to make the fairly obvious association even more obvious! > + > +static int bmp380_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) > +{ > + int ret; > + > + switch (mode) { > + case BMP280_SLEEP: > + case BMP280_FORCED: > + case BMP280_NORMAL: > + break; > + default: > + return -EINVAL; Currently there aren't others. So the compiler should shout if you try to pass something else. Hence this check shouldn't be needed. > + } > + > + ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL, > + BMP380_MODE_MASK, > + FIELD_PREP(BMP380_MODE_MASK, > + bmp380_operation_mode[mode])); > + if (ret) { > + dev_err(data->dev, "failed to write power control register.\n"); > + return ret; > + } > + > + data->op_mode = mode; > + > + return 0; > +} > > return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config)); > } > > +static const u8 bmp580_operation_mode[] = { > + BMP580_MODE_SLEEP, BMP580_MODE_FORCED, BMP580_MODE_NORMAL, For these, explicit setting will make it more obvious. [BMP280_SLEEP] = BMP580_MODE_SLEEP, etc > +}; > + > +static int bmp580_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) > +{ > + struct device *dev = data->dev; > + int ret; > + > + switch (mode) { > + case BMP280_SLEEP: > + case BMP280_NORMAL: > + break; > + case BMP280_FORCED: > + ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG, > + BMP580_DSP_IIR_FORCED_FLUSH); > + if (ret) { > + dev_err(dev, "Could not flush IIR filter constants.\n"); > + return ret; > + } > + break; > + default: There are only the values above, and we should hopefully be able to rely on compiler warnings to shout at us if a future modification adds more. So should be able to drop the default here. > + return -EINVAL; > + } > + > + ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG, > + BMP580_MODE_MASK, > + FIELD_PREP(BMP580_MODE_MASK, > + bmp580_operation_mode[mode])); > + if (ret) { > + dev_err(dev, "failed to write power control register.\n"); > + return ret; > + } > + > + data->op_mode = mode; > + > + return 0; > +}
On Sat, Oct 12, 2024 at 05:03:33PM +0100, Jonathan Cameron wrote: > On Mon, 7 Oct 2024 21:49:42 +0200 > Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > > > Add 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. > > > > The idea is that the sensor is by default in sleep mode, wakes up in > > forced mode when a oneshot capture is requested, or in normal mode > > when the buffer is enabled. The difference lays in the fact that in > > forced mode, the sensor does only one conversion and goes back to sleep > > while in normal mode, the sensor does continuous measurements with the > > frequency that was set in the ODR registers. > > > > The bmpX_chip_config() functions which are responsible for applying > > the requested configuration to the sensor, are modified accordingly > > in order to set the sensor by default in sleep mode. > > > > DEEP STANDBY, Low Power NORMAL and CONTINUOUS modes, supported only by > > the BMP58x version, are not added. > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> > Hi Vasilieos > > Given it looks like you'll be doing a v9 anyway, a few comments inline > on some minor simplifications and potential readability improvements. > > Thanks, > > Jonathan > Hi Jonathan, > > > --- > > drivers/iio/pressure/bmp280-core.c | 296 +++++++++++++++++++++++++++-- > > drivers/iio/pressure/bmp280.h | 21 ++ > > 2 files changed, 296 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c > > index 6811619c6f11..9ad29cf4c2ac 100644 > > --- a/drivers/iio/pressure/bmp280-core.c > > +++ b/drivers/iio/pressure/bmp280-core.c > > @@ -16,6 +16,11 @@ > > > > @@ -1522,6 +1610,71 @@ static int bmp380_preinit(struct bmp280_data *data) > > return bmp380_cmd(data, BMP380_CMD_SOFT_RESET); > > } > > > > +static const u8 bmp380_operation_mode[] = { > > + BMP380_MODE_SLEEP, BMP380_MODE_FORCED, BMP380_MODE_NORMAL, > > +}; > > As below - I'd assign these to specific entries to make the fairly obvious association > even more obvious! > ACK! Makes total sense. Thanks for your time! Cheers, Vasilis > > + > > +static int bmp380_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) > > +{ > > + int ret; > > + > > + switch (mode) { > > + case BMP280_SLEEP: > > + case BMP280_FORCED: > > + case BMP280_NORMAL: > > + break; > > + default: > > + return -EINVAL; > > Currently there aren't others. So the compiler should shout if you try to pass > something else. Hence this check shouldn't be needed. > > > + } > > + > > + ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL, > > + BMP380_MODE_MASK, > > + FIELD_PREP(BMP380_MODE_MASK, > > + bmp380_operation_mode[mode])); > > + if (ret) { > > + dev_err(data->dev, "failed to write power control register.\n"); > > + return ret; > > + } > > + > > + data->op_mode = mode; > > + > > + return 0; > > +} > > > > > return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config)); > > } > > > > +static const u8 bmp580_operation_mode[] = { > > + BMP580_MODE_SLEEP, BMP580_MODE_FORCED, BMP580_MODE_NORMAL, > > For these, explicit setting will make it more obvious. > [BMP280_SLEEP] = BMP580_MODE_SLEEP, > etc > > > +}; > > + > > +static int bmp580_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) > > +{ > > + struct device *dev = data->dev; > > + int ret; > > + > > + switch (mode) { > > + case BMP280_SLEEP: > > + case BMP280_NORMAL: > > + break; > > + case BMP280_FORCED: > > + ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG, > > + BMP580_DSP_IIR_FORCED_FLUSH); > > + if (ret) { > > + dev_err(dev, "Could not flush IIR filter constants.\n"); > > + return ret; > > + } > > + break; > > + default: > There are only the values above, and we should hopefully be able to rely > on compiler warnings to shout at us if a future modification adds more. > > So should be able to drop the default here. > > > + return -EINVAL; > > + } > > + > > + ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG, > > + BMP580_MODE_MASK, > > + FIELD_PREP(BMP580_MODE_MASK, > > + bmp580_operation_mode[mode])); > > + if (ret) { > > + dev_err(dev, "failed to write power control register.\n"); > > + return ret; > > + } > > + > > + data->op_mode = mode; > > + > > + return 0; > > +} >
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 6811619c6f11..9ad29cf4c2ac 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -16,6 +16,11 @@ * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp390-ds002.pdf * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp581-ds004.pdf * + * Sensor API: + * https://github.com/boschsensortec/BME280_SensorAPI + * https://github.com/boschsensortec/BMP3_SensorAPI + * https://github.com/boschsensortec/BMP5_SensorAPI + * * Notice: * The link to the bmp180 datasheet points to an outdated version missing these changes: * - Changed document referral from ANP015 to BST-MPS-AN004-00 on page 26 @@ -616,6 +621,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); @@ -645,6 +658,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); @@ -991,6 +1012,69 @@ static int bmp280_preinit(struct bmp280_data *data) return 0; } +static const u8 bmp280_operation_mode[] = { + BMP280_MODE_SLEEP, BMP280_MODE_FORCED, BMP280_MODE_NORMAL, +}; + +static int bmp280_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) +{ + int ret; + + switch (mode) { + case BMP280_SLEEP: + case BMP280_FORCED: + case BMP280_NORMAL: + break; + default: + return -EINVAL; + } + + ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS, + BMP280_MODE_MASK, bmp280_operation_mode[mode]); + if (ret) { + dev_err(data->dev, "failed to write ctrl_meas register.\n"); + return ret; + } + + data->op_mode = mode; + + return 0; +} + +static int bmp280_wait_conv(struct bmp280_data *data) +{ + unsigned int reg, meas_time_us; + int ret; + + /* Check if we are using a BME280 device */ + if (data->oversampling_humid) + meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + + BIT(data->oversampling_humid) * BMP280_MEAS_DUR; + + /* Pressure measurement time */ + meas_time_us += BMP280_PRESS_HUMID_MEAS_OFFSET + + BIT(data->oversampling_press) * BMP280_MEAS_DUR; + + /* Temperature measurement time */ + meas_time_us += BIT(data->oversampling_temp) * BMP280_MEAS_DUR; + + /* Waiting time according to the BM(P/E)2 Sensor API */ + fsleep(meas_time_us); + + ret = regmap_read(data->regmap, BMP280_REG_STATUS, ®); + 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) | @@ -1001,7 +1085,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; @@ -1111,6 +1195,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, @@ -1235,6 +1321,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, @@ -1522,6 +1610,71 @@ static int bmp380_preinit(struct bmp280_data *data) return bmp380_cmd(data, BMP380_CMD_SOFT_RESET); } +static const u8 bmp380_operation_mode[] = { + BMP380_MODE_SLEEP, BMP380_MODE_FORCED, BMP380_MODE_NORMAL, +}; + +static int bmp380_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) +{ + int ret; + + switch (mode) { + case BMP280_SLEEP: + case BMP280_FORCED: + case BMP280_NORMAL: + break; + default: + return -EINVAL; + } + + ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL, + BMP380_MODE_MASK, + FIELD_PREP(BMP380_MODE_MASK, + bmp380_operation_mode[mode])); + if (ret) { + dev_err(data->dev, "failed to write power control register.\n"); + return ret; + } + + data->op_mode = mode; + + return 0; +} + +static int bmp380_wait_conv(struct bmp280_data *data) +{ + unsigned int reg; + int ret, meas_time_us; + + /* Offset measurement time */ + meas_time_us = BMP380_MEAS_OFFSET; + + /* Pressure measurement time */ + meas_time_us += BMP380_PRESS_MEAS_OFFSET + + BIT(data->oversampling_press) * BMP380_MEAS_DUR; + + /* Temperature measurement time */ + meas_time_us += BMP380_TEMP_MEAS_OFFSET + + BIT(data->oversampling_temp) * BMP380_MEAS_DUR; + + /* Measurement time defined in Datasheet Section 3.9.2 */ + fsleep(meas_time_us); + + ret = regmap_read(data->regmap, BMP380_REG_STATUS, ®); + 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))) { + 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; @@ -1582,17 +1735,19 @@ 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)); + + /* + * According to the BMP3 Sensor API, the sensor needs 5ms + * in order to go to the sleep mode. + */ + fsleep(5 * USEC_PER_MSEC); + + ret = bmp380_set_mode(data, BMP280_NORMAL); if (ret) { dev_err(data->dev, "failed to set normal mode\n"); return ret; @@ -1618,7 +1773,16 @@ static int bmp380_chip_config(struct bmp280_data *data) } } - return 0; + /* 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; } static irqreturn_t bmp380_trigger_handler(int irq, void *p) @@ -1714,6 +1878,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, @@ -2101,6 +2267,75 @@ static int bmp580_preinit(struct bmp280_data *data) return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config)); } +static const u8 bmp580_operation_mode[] = { + BMP580_MODE_SLEEP, BMP580_MODE_FORCED, BMP580_MODE_NORMAL, +}; + +static int bmp580_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) +{ + struct device *dev = data->dev; + int ret; + + switch (mode) { + case BMP280_SLEEP: + case BMP280_NORMAL: + break; + case BMP280_FORCED: + ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG, + BMP580_DSP_IIR_FORCED_FLUSH); + if (ret) { + dev_err(dev, "Could not flush IIR filter constants.\n"); + return ret; + } + break; + default: + return -EINVAL; + } + + ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG, + BMP580_MODE_MASK, + FIELD_PREP(BMP580_MODE_MASK, + bmp580_operation_mode[mode])); + if (ret) { + dev_err(dev, "failed to write power control register.\n"); + return ret; + } + + data->op_mode = mode; + + return 0; +} + +static int bmp580_wait_conv(struct bmp280_data *data) +{ + /* + * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical + * characteristics. + */ + static const int time_conv_press[] = { + 0, 1050, 1785, 3045, 5670, 10920, 21420, 42420, + 84420, + }; + static const int time_conv_temp[] = { + 0, 1050, 1105, 1575, 2205, 3465, 6090, 11340, + 21840, + }; + int meas_time_us; + + meas_time_us = 4 * USEC_PER_MSEC + + time_conv_temp[data->oversampling_temp] + + time_conv_press[data->oversampling_press]; + + /* + * Measurement time mentioned in Chapter 2, Table 4 of the datasheet. + * The extra 4ms is the required mode change to start of measurement + * time. + */ + fsleep(meas_time_us); + + return 0; +} + static int bmp580_chip_config(struct bmp280_data *data) { bool change = false, aux; @@ -2171,17 +2406,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 @@ -2279,6 +2503,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, @@ -2526,6 +2752,19 @@ static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press) return 0; } +/* Keep compatibility with newer generations of the sensor */ +static int bmp180_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode) +{ + return 0; +} + +/* Keep compatibility with newer generations of the sensor */ +static int bmp180_wait_conv(struct bmp280_data *data) +{ + return 0; +} + +/* Keep compatibility with newer generations of the sensor */ static int bmp180_chip_config(struct bmp280_data *data) { return 0; @@ -2597,6 +2836,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, }; @@ -2649,6 +2890,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; } @@ -2819,6 +3061,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); @@ -2844,6 +3090,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); + + fsleep(data->start_up_time); return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies); } @@ -2858,7 +3107,12 @@ static int bmp280_runtime_resume(struct device *dev) return ret; usleep_range(data->start_up_time, data->start_up_time + 100); - return data->chip_info->chip_config(data); + + ret = data->chip_info->chip_config(data); + if (ret) + return ret; + + return data->chip_info->set_mode(data, data->op_mode); } EXPORT_RUNTIME_DEV_PM_OPS(bmp280_dev_pm_ops, bmp280_runtime_suspend, diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index dc1bf04cb0b5..3babf90ce73c 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 @@ -385,6 +395,12 @@ struct bmp380_calib { s8 P11; }; +enum bmp280_op_mode { + BMP280_SLEEP, + BMP280_FORCED, + BMP280_NORMAL, +}; + struct bmp280_data { struct device *dev; struct mutex lock; @@ -423,6 +439,9 @@ struct bmp280_data { u8 sensor_data[ALIGN(sizeof(s32) * BME280_NUM_MAX_CHANNELS, sizeof(s64)) + sizeof(s64)] __aligned(sizeof(s64)); + /* Value to hold the current operation mode of the device */ + enum bmp280_op_mode op_mode; + /* * DMA (thus cache coherency maintenance) may require the * transfer buffers to live in their own cache lines. @@ -487,6 +506,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, enum bmp280_op_mode mode); + int (*wait_conv)(struct bmp280_data *data); irqreturn_t (*trigger_handler)(int irq, void *p); };