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>
---
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 @@
* 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);
};
--
2.25.1On 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;
> > +}
>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
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 '='. -- With Best Regards, Andy Shevchenko
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 > >
© 2016 - 2024 Red Hat, Inc.