2024-04-07 17:29:39

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 0/6] Driver cleanup and series to add triggered buffer

Based on next-20240405.

Changes in v4:

Patch [1/6]: *new*:
- Various driver cleanups, no functional changes.

Patch [2/6]: *new*:
- Split reading functions to not hide the t_fine anymore.

Patch [3/6]:
- Patch [2/6] of v3, removed dead code due to guard(mutex)

Patch [4/6]:
- Patch [3/6] of v3, adjusted to the changes introduced by the
new Patch [2/6].

Patch [5/6]:
- Patch [4/6] of v3
- parts of Patch [5/6] of v3 which is the splitting of the channels for
the BMP2xx, BME2xx and the rest of the sensors

Patch [6/6]:
- Patch [6/6] of v3, adjusted to the changes introduced by the
new Patch [2/6]
- Change variable to push data to userspace to a s32 with a size of 5
to fit timestamp as well.
- Add buffer_setup_ops to use the power management functions before
enabling and after disabling the buffer.
- parts of Patch [5/6] of v3 which is to add timestamps and
scan_index/scan_type in the channels.

[v3]: https://lore.kernel.org/linux-iio/[email protected]/
[v2]: https://lore.kernel.org/linux-iio/[email protected]/
[v1]: https://lore.kernel.org/linux-iio/[email protected]/

Vasileios Amoiridis (6):
iio: pressure: bmp280: Various driver cleanups
iio: pressure: bmp280: Refactorize reading functions
iio: pressure: bmp280: Introduce new cleanup routines
iio: pressure: bmp280: Generalize read_{temp,press,humid}() functions
iio: pressure: bmp280: Add SCALE, RAW values in channels and
refactorize them
iio: pressure: bmp280: Add triggered buffer support

drivers/iio/pressure/Kconfig | 2 +
drivers/iio/pressure/bmp280-core.c | 1164 ++++++++++++++++++--------
drivers/iio/pressure/bmp280-regmap.c | 8 +-
drivers/iio/pressure/bmp280-spi.c | 8 +-
drivers/iio/pressure/bmp280.h | 68 +-
5 files changed, 867 insertions(+), 383 deletions(-)

--
2.25.1



2024-04-07 17:29:59

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 3/6] iio: pressure: bmp280: Introduce new cleanup routines

Introduce new linux/cleanup.h with the guard(mutex) functionality
in the {read,write}_raw() functions.

Suggested-by: Andy Shevchenko <[email protected]>
Suggested-by: Jonathan Cameron <[email protected]>
Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 129 +++++++++++++----------------
1 file changed, 58 insertions(+), 71 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 50bdf79011bc..51bcdf8cede6 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -27,6 +27,7 @@

#include <linux/bitops.h>
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -499,77 +500,69 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
return IIO_VAL_INT;
}

-static int bmp_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2, long mask)
+static int bmp_read_raw_impl(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);
- int ret;

- pm_runtime_get_sync(data->dev);
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);

switch (mask) {
case IIO_CHAN_INFO_PROCESSED:
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
- ret = data->chip_info->read_humid(data, val, val2);
- break;
+ return data->chip_info->read_humid(data, val, val2);
case IIO_PRESSURE:
- ret = data->chip_info->read_press(data, val, val2);
- break;
+ return data->chip_info->read_press(data, val, val2);
case IIO_TEMP:
- ret = data->chip_info->read_temp(data, val, val2);
- break;
+ return data->chip_info->read_temp(data, val, val2);
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- break;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
*val = 1 << data->oversampling_humid;
- ret = IIO_VAL_INT;
- break;
+ return IIO_VAL_INT;
case IIO_PRESSURE:
*val = 1 << data->oversampling_press;
- ret = IIO_VAL_INT;
- break;
+ return IIO_VAL_INT;
case IIO_TEMP:
*val = 1 << data->oversampling_temp;
- ret = IIO_VAL_INT;
- break;
+ return IIO_VAL_INT;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- break;
case IIO_CHAN_INFO_SAMP_FREQ:
- if (!data->chip_info->sampling_freq_avail) {
- ret = -EINVAL;
- break;
- }
+ if (!data->chip_info->sampling_freq_avail)
+ return -EINVAL;

*val = data->chip_info->sampling_freq_avail[data->sampling_freq][0];
*val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1];
- ret = IIO_VAL_INT_PLUS_MICRO;
- break;
+ return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
- if (!data->chip_info->iir_filter_coeffs_avail) {
- ret = -EINVAL;
- break;
- }
+ if (!data->chip_info->iir_filter_coeffs_avail)
+ return -EINVAL;

*val = (1 << data->iir_filter_coeff) - 1;
- ret = IIO_VAL_INT;
- break;
+ return IIO_VAL_INT;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}

- mutex_unlock(&data->lock);
+ return 0;
+}
+
+static int bmp_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct bmp280_data *data = iio_priv(indio_dev);
+ int ret;
+
+ pm_runtime_get_sync(data->dev);
+ ret = bmp_read_raw_impl(indio_dev, chan, val, val2, mask);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);

@@ -697,12 +690,13 @@ static int bmp_write_iir_filter_coeffs(struct bmp280_data *data, int val)
return -EINVAL;
}

-static int bmp_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val, int val2, long mask)
+static int bmp_write_raw_impl(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);
- int ret = 0;
+
+ guard(mutex)(&data->lock);

/*
* Helper functions to update sensor running configuration.
@@ -712,46 +706,39 @@ static int bmp_write_raw(struct iio_dev *indio_dev,
*/
switch (mask) {
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
- pm_runtime_get_sync(data->dev);
- mutex_lock(&data->lock);
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
- ret = bmp_write_oversampling_ratio_humid(data, val);
- break;
+ return bmp_write_oversampling_ratio_humid(data, val);
case IIO_PRESSURE:
- ret = bmp_write_oversampling_ratio_press(data, val);
- break;
+ return bmp_write_oversampling_ratio_press(data, val);
case IIO_TEMP:
- ret = bmp_write_oversampling_ratio_temp(data, val);
- break;
+ return bmp_write_oversampling_ratio_temp(data, val);
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- mutex_unlock(&data->lock);
- pm_runtime_mark_last_busy(data->dev);
- pm_runtime_put_autosuspend(data->dev);
- break;
case IIO_CHAN_INFO_SAMP_FREQ:
- pm_runtime_get_sync(data->dev);
- mutex_lock(&data->lock);
- ret = bmp_write_sampling_frequency(data, val, val2);
- mutex_unlock(&data->lock);
- pm_runtime_mark_last_busy(data->dev);
- pm_runtime_put_autosuspend(data->dev);
- break;
+ return bmp_write_sampling_frequency(data, val, val2);
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
- pm_runtime_get_sync(data->dev);
- mutex_lock(&data->lock);
- ret = bmp_write_iir_filter_coeffs(data, val);
- mutex_unlock(&data->lock);
- pm_runtime_mark_last_busy(data->dev);
- pm_runtime_put_autosuspend(data->dev);
- break;
+ return bmp_write_iir_filter_coeffs(data, val);
default:
return -EINVAL;
}

+ return 0;
+}
+
+static int bmp_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct bmp280_data *data = iio_priv(indio_dev);
+ int ret;
+
+ pm_runtime_get_sync(data->dev);
+ ret = bmp_write_raw_impl(indio_dev, chan, val, val2, mask);
+ pm_runtime_mark_last_busy(data->dev);
+ pm_runtime_put_autosuspend(data->dev);
+
return ret;
}

--
2.25.1


2024-04-07 17:30:14

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 2/6] iio: pressure: bmp280: Refactorize reading functions

For BMP18x, BMP28x, BME280, BMP38x the reading of the pressure
value requires an update of the t_fine variable which happens
through reading the temperature value.

So all the bmpxxx_read_press() functions of the above sensors
are internally calling the equivalent bmpxxx_read_temp() function
in order to update the t_fine value. By just looking at the code
this functionality is a bit hidden and is not easy to understand
why those channels are not independent.

This commit tries to clear these things a bit by splitting the
bmpxxx_{read/compensate}_{temp/press/humid}() to the following:

i. bmpxxx_read_{temp/press/humid}_adc(): read the raw value from
the sensor.

ii. bmpxx_calc_t_fine(): calculate the t_fine variable.

iii. bmpxxx_get_t_fine(): get the t_fine variable.

iv. bmpxxx_compensate_{temp/press/humid}(): compensate the adc
values and return the calculated value.

v. bmpxxx_read_{temp/press/humid}(): combine calls of the
aforementioned functions to return the requested value.

Suggested-by: Jonathan Cameron <[email protected]>
Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 352 ++++++++++++++++++-----------
drivers/iio/pressure/bmp280.h | 6 -
2 files changed, 223 insertions(+), 135 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 1c51139cbfcf..50bdf79011bc 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -288,12 +288,33 @@ static int bme280_read_calib(struct bmp280_data *data)
*
* Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
*/
-static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity)
+static int bme280_read_humid_adc(struct bmp280_data *data, s32 *adc_humidity)
+{
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
+ &data->be16, sizeof(data->be16));
+ if (ret) {
+ dev_err(data->dev, "failed to read humidity\n");
+ return ret;
+ }
+
+ *adc_humidity = be16_to_cpu(data->be16);
+ if (*adc_humidity == BMP280_HUMIDITY_SKIPPED) {
+ dev_err(data->dev, "reading humidity skipped\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static u32 bme280_compensate_humidity(struct bmp280_data *data,
+ s32 adc_humidity, s32 t_fine)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s32 var;

- var = ((s32)data->t_fine) - (s32)76800;
+ var = ((s32)t_fine) - (s32)76800;
var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
* (((var * (s32)calib->H3) >> 11) + (s32)32768)) >> 10)
@@ -312,7 +333,27 @@ static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity
*
* Taken from datasheet, Section 3.11.3, "Compensation formula".
*/
-static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
+static int bmp280_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
+{
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
+ data->buf, sizeof(data->buf));
+ if (ret) {
+ dev_err(data->dev, "failed to read temperature\n");
+ return ret;
+ }
+
+ *adc_temp = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(data->buf));
+ if (*adc_temp == BMP280_TEMP_SKIPPED) {
+ dev_err(data->dev, "reading temperature skipped\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static s32 bmp280_calc_t_fine(struct bmp280_data *data, s32 adc_temp)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s32 var1, var2;
@@ -322,9 +363,26 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
((s32)calib->T3)) >> 14;
- data->t_fine = var1 + var2;
+ return var1 + var2; /* t_fine = var1 + var2 */
+}

- return (data->t_fine * 5 + 128) >> 8;
+static int bmp280_get_t_fine(struct bmp280_data *data, s32 *t_fine)
+{
+ s32 adc_temp;
+ int ret;
+
+ ret = bmp280_read_temp_adc(data, &adc_temp);
+ if (ret)
+ return ret;
+
+ *t_fine = bmp280_calc_t_fine(data, adc_temp);
+
+ return 0;
+}
+
+static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
+{
+ return (bmp280_calc_t_fine(data, adc_temp) * 5 + 128) / 256;
}

/*
@@ -334,12 +392,33 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
*
* Taken from datasheet, Section 3.11.3, "Compensation formula".
*/
-static u32 bmp280_compensate_press(struct bmp280_data *data, s32 adc_press)
+static int bmp280_read_press_adc(struct bmp280_data *data, s32 *adc_press)
+{
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
+ data->buf, sizeof(data->buf));
+ if (ret) {
+ dev_err(data->dev, "failed to read pressure\n");
+ return ret;
+ }
+
+ *adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(data->buf));
+ if (*adc_press == BMP280_PRESS_SKIPPED) {
+ dev_err(data->dev, "reading pressure skipped\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static u32 bmp280_compensate_press(struct bmp280_data *data,
+ s32 adc_press, s32 t_fine)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s64 var1, var2, p;

- var1 = ((s64)data->t_fine) - 128000;
+ var1 = ((s64)t_fine) - 128000;
var2 = var1 * var1 * (s64)calib->P6;
var2 += (var1 * (s64)calib->P5) << 17;
var2 += ((s64)calib->P4) << 35;
@@ -364,59 +443,34 @@ static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
s32 adc_temp, comp_temp;
int ret;

- ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
- data->buf, sizeof(data->buf));
- if (ret) {
- dev_err(data->dev, "failed to read temperature\n");
+ ret = bmp280_read_temp_adc(data, &adc_temp);
+ if (ret)
return ret;
- }

- adc_temp = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(data->buf));
- if (adc_temp == BMP280_TEMP_SKIPPED) {
- /* reading was skipped */
- dev_err(data->dev, "reading temperature skipped\n");
- return -EIO;
- }
comp_temp = bmp280_compensate_temp(data, adc_temp);

- /*
- * val might be NULL if we're called by the read_press routine,
- * who only cares about the carry over t_fine value.
- */
- if (val) {
- *val = comp_temp * 10;
- return IIO_VAL_INT;
- }
-
- return 0;
+ /* IIO units are in milli Celsius */
+ *val = comp_temp * 10;
+ return IIO_VAL_INT;
}

static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
{
+ s32 adc_press, t_fine;
u32 comp_press;
- s32 adc_press;
int ret;

- /* Read and compensate temperature so we get a reading of t_fine. */
- ret = bmp280_read_temp(data, NULL, NULL);
+ ret = bmp280_get_t_fine(data, &t_fine);
if (ret)
return ret;

- ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
- data->buf, sizeof(data->buf));
- if (ret) {
- dev_err(data->dev, "failed to read pressure\n");
+ ret = bmp280_read_press_adc(data, &adc_press);
+ if (ret)
return ret;
- }

- adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(data->buf));
- if (adc_press == BMP280_PRESS_SKIPPED) {
- /* reading was skipped */
- dev_err(data->dev, "reading pressure skipped\n");
- return -EIO;
- }
- comp_press = bmp280_compensate_press(data, adc_press);
+ comp_press = bmp280_compensate_press(data, adc_press, t_fine);

+ /* IIO units are in kPa */
*val = comp_press;
*val2 = 256000;

@@ -425,30 +479,21 @@ static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)

static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
{
+ s32 adc_humidity, t_fine;
u32 comp_humidity;
- s32 adc_humidity;
int ret;

- /* Read and compensate temperature so we get a reading of t_fine. */
- ret = bmp280_read_temp(data, NULL, NULL);
+ ret = bmp280_get_t_fine(data, &t_fine);
if (ret)
return ret;

- ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
- &data->be16, sizeof(data->be16));
- if (ret) {
- dev_err(data->dev, "failed to read humidity\n");
+ ret = bme280_read_humid_adc(data, &adc_humidity);
+ if (ret)
return ret;
- }

- adc_humidity = be16_to_cpu(data->be16);
- if (adc_humidity == BMP280_HUMIDITY_SKIPPED) {
- /* reading was skipped */
- dev_err(data->dev, "reading humidity skipped\n");
- return -EIO;
- }
- comp_humidity = bme280_compensate_humidity(data, adc_humidity);
+ comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine);

+ /* IIO units are in 1000 * % */
*val = comp_humidity * 1000 / 1024;

return IIO_VAL_INT;
@@ -921,9 +966,29 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
* Taken from datasheet, Section Appendix 9, "Compensation formula" and repo
* https://github.com/BoschSensortec/BMP3-Sensor-API.
*/
-static s32 bmp380_compensate_temp(struct bmp280_data *data, u32 adc_temp)
+static int bmp380_read_temp_adc(struct bmp280_data *data, u32 *adc_temp)
+{
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
+ data->buf, sizeof(data->buf));
+ if (ret) {
+ dev_err(data->dev, "failed to read temperature\n");
+ return ret;
+ }
+
+ *adc_temp = get_unaligned_le24(data->buf);
+ if (*adc_temp == BMP380_TEMP_SKIPPED) {
+ dev_err(data->dev, "reading temperature skipped\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static s32 bmp380_calc_t_fine(struct bmp280_data *data, u32 adc_temp)
{
- s64 var1, var2, var3, var4, var5, var6, comp_temp;
+ s64 var1, var2, var3, var4, var5, var6;
struct bmp380_calib *calib = &data->calib.bmp380;

var1 = ((s64) adc_temp) - (((s64) calib->T1) << 8);
@@ -932,7 +997,29 @@ static s32 bmp380_compensate_temp(struct bmp280_data *data, u32 adc_temp)
var4 = var3 * ((s64) calib->T3);
var5 = (var2 << 18) + var4;
var6 = var5 >> 32;
- data->t_fine = (s32) var6;
+ return (s32) var6; /* t_fine = var6 */
+}
+
+static int bmp380_get_t_fine(struct bmp280_data *data, s32 *t_fine)
+{
+ s32 adc_temp;
+ int ret;
+
+ ret = bmp380_read_temp_adc(data, &adc_temp);
+ if (ret)
+ return ret;
+
+ *t_fine = bmp380_calc_t_fine(data, adc_temp);
+
+ return 0;
+}
+
+static int bmp380_compensate_temp(struct bmp280_data *data, u32 adc_temp)
+{
+ s64 comp_temp;
+ s32 var6;
+
+ var6 = bmp380_calc_t_fine(data, adc_temp);
comp_temp = (var6 * 25) >> 14;

comp_temp = clamp_val(comp_temp, BMP380_MIN_TEMP, BMP380_MAX_TEMP);
@@ -946,27 +1033,48 @@ static s32 bmp380_compensate_temp(struct bmp280_data *data, u32 adc_temp)
* Taken from datasheet, Section 9.3. "Pressure compensation" and repository
* https://github.com/BoschSensortec/BMP3-Sensor-API.
*/
-static u32 bmp380_compensate_press(struct bmp280_data *data, u32 adc_press)
+static int bmp380_read_press_adc(struct bmp280_data *data, u32 *adc_press)
+{
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
+ data->buf, sizeof(data->buf));
+ if (ret) {
+ dev_err(data->dev, "failed to read pressure\n");
+ return ret;
+ }
+
+ *adc_press = get_unaligned_le24(data->buf);
+ if (*adc_press == BMP380_PRESS_SKIPPED) {
+ dev_err(data->dev, "reading pressure skipped\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static u32 bmp380_compensate_press(struct bmp280_data *data,
+ u32 adc_press, s32 t_fine)
{
s64 var1, var2, var3, var4, var5, var6, offset, sensitivity;
struct bmp380_calib *calib = &data->calib.bmp380;
u32 comp_press;

- var1 = (s64)data->t_fine * (s64)data->t_fine;
+ var1 = (s64)t_fine * (s64)t_fine;
var2 = var1 >> 6;
- var3 = (var2 * ((s64) data->t_fine)) >> 8;
+ var3 = (var2 * ((s64) t_fine)) >> 8;
var4 = ((s64)calib->P8 * var3) >> 5;
var5 = ((s64)calib->P7 * var1) << 4;
- var6 = ((s64)calib->P6 * (s64)data->t_fine) << 22;
+ var6 = ((s64)calib->P6 * (s64)t_fine) << 22;
offset = ((s64)calib->P5 << 47) + var4 + var5 + var6;
var2 = ((s64)calib->P4 * var3) >> 5;
var4 = ((s64)calib->P3 * var1) << 2;
var5 = ((s64)calib->P2 - ((s64)1 << 14)) *
- ((s64)data->t_fine << 21);
+ ((s64)t_fine << 21);
sensitivity = (((s64) calib->P1 - ((s64) 1 << 14)) << 46) +
var2 + var4 + var5;
var1 = (sensitivity >> 24) * (s64)adc_press;
- var2 = (s64)calib->P10 * (s64)data->t_fine;
+ var2 = (s64)calib->P10 * (s64)t_fine;
var3 = var2 + ((s64)calib->P9 << 16);
var4 = (var3 * (s64)adc_press) >> 13;

@@ -992,60 +1100,34 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
u32 adc_temp;
int ret;

- ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
- data->buf, sizeof(data->buf));
- if (ret) {
- dev_err(data->dev, "failed to read temperature\n");
+ ret = bmp380_read_temp_adc(data, &adc_temp);
+ if (ret)
return ret;
- }

- adc_temp = get_unaligned_le24(data->buf);
- if (adc_temp == BMP380_TEMP_SKIPPED) {
- dev_err(data->dev, "reading temperature skipped\n");
- return -EIO;
- }
comp_temp = bmp380_compensate_temp(data, adc_temp);

- /*
- * Val might be NULL if we're called by the read_press routine,
- * who only cares about the carry over t_fine value.
- */
- if (val) {
- /* IIO reports temperatures in milli Celsius */
- *val = comp_temp * 10;
- return IIO_VAL_INT;
- }
-
- return 0;
+ /* IIO units are in milli Celsius */
+ *val = comp_temp * 10;
+ return IIO_VAL_INT;
}

static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
{
- s32 comp_press;
- u32 adc_press;
+ u32 adc_press, comp_press, t_fine;
int ret;

- /* Read and compensate for temperature so we get a reading of t_fine */
- ret = bmp380_read_temp(data, NULL, NULL);
+ ret = bmp380_get_t_fine(data, &t_fine);
if (ret)
return ret;

- ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
- data->buf, sizeof(data->buf));
- if (ret) {
- dev_err(data->dev, "failed to read pressure\n");
+ ret = bmp380_read_press_adc(data, &adc_press);
+ if (ret)
return ret;
- }

- adc_press = get_unaligned_le24(data->buf);
- if (adc_press == BMP380_PRESS_SKIPPED) {
- dev_err(data->dev, "reading pressure skipped\n");
- return -EIO;
- }
- comp_press = bmp380_compensate_press(data, adc_press);
+ comp_press = bmp380_compensate_press(data, adc_press, t_fine);

+ /* IIO units are in kPa */
*val = comp_press;
- /* Compensated pressure is in cPa (centipascals) */
*val2 = 100000;

return IIO_VAL_FRACTIONAL;
@@ -1807,7 +1889,7 @@ static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
return 0;
}

-static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
+static int bmp180_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
{
int ret;

@@ -1824,7 +1906,7 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
return ret;
}

- *val = be16_to_cpu(data->be16);
+ *adc_temp = be16_to_cpu(data->be16);

return 0;
}
@@ -1873,16 +1955,34 @@ static int bmp180_read_calib(struct bmp280_data *data)
*
* Taken from datasheet, Section 3.5, "Calculating pressure and temperature".
*/
-static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
+
+static s32 bmp180_calc_t_fine(struct bmp280_data *data, s32 adc_temp)
{
struct bmp180_calib *calib = &data->calib.bmp180;
s32 x1, x2;

x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
x2 = (calib->MC << 11) / (x1 + calib->MD);
- data->t_fine = x1 + x2;
+ return x1 + x2; /* t_fine = x1 + x2; */
+}
+
+static int bmp180_get_t_fine(struct bmp280_data *data, s32 *t_fine)
+{
+ s32 adc_temp;
+ int ret;
+
+ ret = bmp180_read_temp_adc(data, &adc_temp);
+ if (ret)
+ return ret;
+
+ *t_fine = bmp180_calc_t_fine(data, adc_temp);

- return (data->t_fine + 8) >> 4;
+ return 0;
+}
+
+static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
+{
+ return (bmp180_calc_t_fine(data, adc_temp) + 8) / 16;
}

static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
@@ -1890,25 +1990,18 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
s32 adc_temp, comp_temp;
int ret;

- ret = bmp180_read_adc_temp(data, &adc_temp);
+ ret = bmp180_read_temp_adc(data, &adc_temp);
if (ret)
return ret;

comp_temp = bmp180_compensate_temp(data, adc_temp);

- /*
- * val might be NULL if we're called by the read_press routine,
- * who only cares about the carry over t_fine value.
- */
- if (val) {
- *val = comp_temp * 100;
- return IIO_VAL_INT;
- }
-
- return 0;
+ /* IIO units are in milli Celsius */
+ *val = comp_temp * 100;
+ return IIO_VAL_INT;
}

-static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
+static int bmp180_read_press_adc(struct bmp280_data *data, s32 *adc_press)
{
u8 oss = data->oversampling_press;
int ret;
@@ -1927,7 +2020,7 @@ static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
return ret;
}

- *val = get_unaligned_be24(data->buf) >> (8 - oss);
+ *adc_press = get_unaligned_be24(data->buf) >> (8 - oss);

return 0;
}
@@ -1937,7 +2030,8 @@ static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
*
* Taken from datasheet, Section 3.5, "Calculating pressure and temperature".
*/
-static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
+static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press,
+ s32 t_fine)
{
struct bmp180_calib *calib = &data->calib.bmp180;
s32 oss = data->oversampling_press;
@@ -1945,7 +2039,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
s32 b3, b6;
u32 b4, b7;

- b6 = data->t_fine - 4000;
+ b6 = t_fine - 4000;
x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
x2 = calib->AC2 * b6 >> 11;
x3 = x1 + x2;
@@ -1969,21 +2063,21 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)

static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
{
+ s32 adc_press, t_fine;
u32 comp_press;
- s32 adc_press;
int ret;

- /* Read and compensate temperature so we get a reading of t_fine. */
- ret = bmp180_read_temp(data, NULL, NULL);
+ ret = bmp180_get_t_fine(data, &t_fine);
if (ret)
return ret;

- ret = bmp180_read_adc_press(data, &adc_press);
+ ret = bmp180_read_press_adc(data, &adc_press);
if (ret)
return ret;

- comp_press = bmp180_compensate_press(data, adc_press);
+ comp_press = bmp180_compensate_press(data, adc_press, t_fine);

+ /* IIO units are in kPa */
*val = comp_press;
*val2 = 1000;

diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index ea8eb5691428..6dbd7413ad3f 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -394,12 +394,6 @@ struct bmp280_data {
*/
int sampling_freq;

- /*
- * Carryover value from temperature conversion, used in pressure
- * calculation.
- */
- s32 t_fine;
-
/*
* DMA (thus cache coherency maintenance) may require the
* transfer buffers to live in their own cache lines.
--
2.25.1


2024-04-07 17:30:17

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 1/6] iio: pressure: bmp280: Various driver cleanups

Various driver cleanups including:

a) change names of functions that are used by all sensors from
bmp280_read_raw_* to bmp_read_raw_* to make it more clear that
these functions are general and not tied to the BMP280 sensor.

b) modify some defines that are used only by the BME280 sensor
to use the naming convention BME280_* and not BMP280_*.

c) add various error messages that were not implemented.

d) sort headers.

e) fix various indentation errors which were found by checkpatch.pl.

g) Add identifier names in function definitions which were warned
by checkpatch.pl.

Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 244 ++++++++++++++-------------
drivers/iio/pressure/bmp280-i2c.c | 2 +-
drivers/iio/pressure/bmp280-regmap.c | 8 +-
drivers/iio/pressure/bmp280-spi.c | 8 +-
drivers/iio/pressure/bmp280.h | 50 +++---
5 files changed, 159 insertions(+), 153 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 09f53d987c7d..1c51139cbfcf 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -52,7 +52,6 @@
*/
enum { AC1, AC2, AC3, AC4, AC5, AC6, B1, B2, MB, MC, MD };

-
enum bmp380_odr {
BMP380_ODR_200HZ,
BMP380_ODR_100HZ,
@@ -71,7 +70,7 @@ enum bmp380_odr {
BMP380_ODR_0_01HZ,
BMP380_ODR_0_006HZ,
BMP380_ODR_0_003HZ,
- BMP380_ODR_0_0015HZ,
+ BMP380_ODR_0_0015HZ
};

enum bmp580_odr {
@@ -106,7 +105,7 @@ enum bmp580_odr {
BMP580_ODR_1HZ,
BMP580_ODR_0_5HZ,
BMP580_ODR_0_25HZ,
- BMP580_ODR_0_125HZ,
+ BMP580_ODR_0_125HZ
};

/*
@@ -131,7 +130,7 @@ enum {
BMP380_P8 = 16,
BMP380_P9 = 17,
BMP380_P10 = 19,
- BMP380_P11 = 20,
+ BMP380_P11 = 20
};

static const struct iio_chan_spec bmp280_channels[] = {
@@ -181,11 +180,10 @@ static int bmp280_read_calib(struct bmp280_data *data)
struct bmp280_calib *calib = &data->calib.bmp280;
int ret;

-
/* Read temperature and pressure calibration values. */
ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
- if (ret < 0) {
+ if (ret) {
dev_err(data->dev,
"failed to read temperature and pressure calibration parameters\n");
return ret;
@@ -222,7 +220,7 @@ static int bme280_read_calib(struct bmp280_data *data)

/* Load shared calibration params with bmp280 first */
ret = bmp280_read_calib(data);
- if (ret < 0) {
+ if (ret) {
dev_err(dev, "failed to read common bmp280 calibration parameters\n");
return ret;
}
@@ -235,47 +233,47 @@ static int bme280_read_calib(struct bmp280_data *data)
* Humidity data is only available on BME280.
*/

- ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
- if (ret < 0) {
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
+ if (ret) {
dev_err(dev, "failed to read H1 comp value\n");
return ret;
}
calib->H1 = tmp;

- ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2,
+ ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
&data->le16, sizeof(data->le16));
- if (ret < 0) {
+ if (ret) {
dev_err(dev, "failed to read H2 comp value\n");
return ret;
}
calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);

- ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
- if (ret < 0) {
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
+ if (ret) {
dev_err(dev, "failed to read H3 comp value\n");
return ret;
}
calib->H3 = tmp;

- ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4,
+ ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
&data->be16, sizeof(data->be16));
- if (ret < 0) {
+ if (ret) {
dev_err(dev, "failed to read H4 comp value\n");
return ret;
}
calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
(be16_to_cpu(data->be16) & 0xf), 11);

- ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5,
+ ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
&data->le16, sizeof(data->le16));
- if (ret < 0) {
+ if (ret) {
dev_err(dev, "failed to read H5 comp value\n");
return ret;
}
- calib->H5 = sign_extend32(FIELD_GET(BMP280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
+ calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);

- ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
- if (ret < 0) {
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
+ if (ret) {
dev_err(dev, "failed to read H6 comp value\n");
return ret;
}
@@ -283,14 +281,14 @@ static int bme280_read_calib(struct bmp280_data *data)

return 0;
}
+
/*
* Returns humidity in percent, resolution is 0.01 percent. Output value of
* "47445" represents 47445/1024 = 46.333 %RH.
*
* Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
*/
-static u32 bmp280_compensate_humidity(struct bmp280_data *data,
- s32 adc_humidity)
+static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s32 var;
@@ -305,7 +303,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
var = clamp_val(var, 0, 419430400);

return var >> 12;
-};
+}

/*
* Returns temperature in DegC, resolution is 0.01 DegC. Output value of
@@ -314,8 +312,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
*
* Taken from datasheet, Section 3.11.3, "Compensation formula".
*/
-static s32 bmp280_compensate_temp(struct bmp280_data *data,
- s32 adc_temp)
+static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s32 var1, var2;
@@ -337,8 +334,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
*
* Taken from datasheet, Section 3.11.3, "Compensation formula".
*/
-static u32 bmp280_compensate_press(struct bmp280_data *data,
- s32 adc_press)
+static u32 bmp280_compensate_press(struct bmp280_data *data, s32 adc_press)
{
struct bmp280_calib *calib = &data->calib.bmp280;
s64 var1, var2, p;
@@ -363,15 +359,14 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
return (u32)p;
}

-static int bmp280_read_temp(struct bmp280_data *data,
- int *val, int *val2)
+static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
{
s32 adc_temp, comp_temp;
int ret;

ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
data->buf, sizeof(data->buf));
- if (ret < 0) {
+ if (ret) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
}
@@ -396,8 +391,7 @@ static int bmp280_read_temp(struct bmp280_data *data,
return 0;
}

-static int bmp280_read_press(struct bmp280_data *data,
- int *val, int *val2)
+static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
{
u32 comp_press;
s32 adc_press;
@@ -405,12 +399,12 @@ static int bmp280_read_press(struct bmp280_data *data,

/* Read and compensate temperature so we get a reading of t_fine. */
ret = bmp280_read_temp(data, NULL, NULL);
- if (ret < 0)
+ if (ret)
return ret;

ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
data->buf, sizeof(data->buf));
- if (ret < 0) {
+ if (ret) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
}
@@ -429,7 +423,7 @@ static int bmp280_read_press(struct bmp280_data *data,
return IIO_VAL_FRACTIONAL;
}

-static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
+static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
{
u32 comp_humidity;
s32 adc_humidity;
@@ -437,12 +431,12 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)

/* Read and compensate temperature so we get a reading of t_fine. */
ret = bmp280_read_temp(data, NULL, NULL);
- if (ret < 0)
+ if (ret)
return ret;

- ret = regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB,
+ ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
&data->be16, sizeof(data->be16));
- if (ret < 0) {
+ if (ret) {
dev_err(data->dev, "failed to read humidity\n");
return ret;
}
@@ -453,16 +447,16 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
dev_err(data->dev, "reading humidity skipped\n");
return -EIO;
}
- comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
+ comp_humidity = bme280_compensate_humidity(data, adc_humidity);

*val = comp_humidity * 1000 / 1024;

return IIO_VAL_INT;
}

-static int bmp280_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2, long mask)
+static int bmp_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);
int ret;
@@ -537,8 +531,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
return ret;
}

-static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
- int val)
+static int bmp_write_oversampling_ratio_humid(struct bmp280_data *data, int val)
{
const int *avail = data->chip_info->oversampling_humid_avail;
const int n = data->chip_info->num_oversampling_humid_avail;
@@ -562,8 +555,7 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
return -EINVAL;
}

-static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
- int val)
+static int bmp_write_oversampling_ratio_temp(struct bmp280_data *data, int val)
{
const int *avail = data->chip_info->oversampling_temp_avail;
const int n = data->chip_info->num_oversampling_temp_avail;
@@ -587,8 +579,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
return -EINVAL;
}

-static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
- int val)
+static int bmp_write_oversampling_ratio_press(struct bmp280_data *data, int val)
{
const int *avail = data->chip_info->oversampling_press_avail;
const int n = data->chip_info->num_oversampling_press_avail;
@@ -612,8 +603,7 @@ static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
return -EINVAL;
}

-static int bmp280_write_sampling_frequency(struct bmp280_data *data,
- int val, int val2)
+static int bmp_write_sampling_frequency(struct bmp280_data *data, int val, int val2)
{
const int (*avail)[2] = data->chip_info->sampling_freq_avail;
const int n = data->chip_info->num_sampling_freq_avail;
@@ -637,7 +627,7 @@ static int bmp280_write_sampling_frequency(struct bmp280_data *data,
return -EINVAL;
}

-static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val)
+static int bmp_write_iir_filter_coeffs(struct bmp280_data *data, int val)
{
const int *avail = data->chip_info->iir_filter_coeffs_avail;
const int n = data->chip_info->num_iir_filter_coeffs_avail;
@@ -662,9 +652,9 @@ static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val)
return -EINVAL;
}

-static int bmp280_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val, int val2, long mask)
+static int bmp_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);
int ret = 0;
@@ -681,13 +671,13 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,
mutex_lock(&data->lock);
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
- ret = bmp280_write_oversampling_ratio_humid(data, val);
+ ret = bmp_write_oversampling_ratio_humid(data, val);
break;
case IIO_PRESSURE:
- ret = bmp280_write_oversampling_ratio_press(data, val);
+ ret = bmp_write_oversampling_ratio_press(data, val);
break;
case IIO_TEMP:
- ret = bmp280_write_oversampling_ratio_temp(data, val);
+ ret = bmp_write_oversampling_ratio_temp(data, val);
break;
default:
ret = -EINVAL;
@@ -700,7 +690,7 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SAMP_FREQ:
pm_runtime_get_sync(data->dev);
mutex_lock(&data->lock);
- ret = bmp280_write_sampling_frequency(data, val, val2);
+ ret = bmp_write_sampling_frequency(data, val, val2);
mutex_unlock(&data->lock);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);
@@ -708,7 +698,7 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
pm_runtime_get_sync(data->dev);
mutex_lock(&data->lock);
- ret = bmp280_write_iir_filter_coeffs(data, val);
+ ret = bmp_write_iir_filter_coeffs(data, val);
mutex_unlock(&data->lock);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);
@@ -720,10 +710,10 @@ static int bmp280_write_raw(struct iio_dev *indio_dev,
return ret;
}

-static int bmp280_read_avail(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- const int **vals, int *type, int *length,
- long mask)
+static int bmp_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);

@@ -760,9 +750,9 @@ static int bmp280_read_avail(struct iio_dev *indio_dev,
}

static const struct iio_info bmp280_info = {
- .read_raw = &bmp280_read_raw,
- .read_avail = &bmp280_read_avail,
- .write_raw = &bmp280_write_raw,
+ .read_raw = &bmp_read_raw,
+ .read_avail = &bmp_read_avail,
+ .write_raw = &bmp_write_raw,
};

static int bmp280_chip_config(struct bmp280_data *data)
@@ -772,22 +762,20 @@ static int bmp280_chip_config(struct bmp280_data *data)
int ret;

ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
- BMP280_OSRS_TEMP_MASK |
- BMP280_OSRS_PRESS_MASK |
- BMP280_MODE_MASK,
- osrs | BMP280_MODE_NORMAL);
- if (ret < 0) {
- dev_err(data->dev,
- "failed to write ctrl_meas register\n");
+ BMP280_OSRS_TEMP_MASK |
+ BMP280_OSRS_PRESS_MASK |
+ BMP280_MODE_MASK,
+ osrs | BMP280_MODE_NORMAL);
+ if (ret) {
+ dev_err(data->dev, "failed to write ctrl_meas register\n");
return ret;
}

ret = regmap_update_bits(data->regmap, BMP280_REG_CONFIG,
BMP280_FILTER_MASK,
BMP280_FILTER_4X);
- if (ret < 0) {
- dev_err(data->dev,
- "failed to write config register\n");
+ if (ret) {
+ dev_err(data->dev, "failed to write config register\n");
return ret;
}

@@ -833,18 +821,19 @@ EXPORT_SYMBOL_NS(bmp280_chip_info, IIO_BMP280);

static int bme280_chip_config(struct bmp280_data *data)
{
- u8 osrs = FIELD_PREP(BMP280_OSRS_HUMIDITY_MASK, data->oversampling_humid + 1);
+ u8 osrs = FIELD_PREP(BME280_OSRS_HUMIDITY_MASK, data->oversampling_humid + 1);
int ret;

/*
* Oversampling of humidity must be set before oversampling of
* temperature/pressure is set to become effective.
*/
- ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
- BMP280_OSRS_HUMIDITY_MASK, osrs);
-
- if (ret < 0)
+ ret = regmap_update_bits(data->regmap, BME280_REG_CTRL_HUMIDITY,
+ BME280_OSRS_HUMIDITY_MASK, osrs);
+ if (ret) {
+ dev_err(data->dev, "failed to set humidity oversampling\n");
return ret;
+ }

return bmp280_chip_config(data);
}
@@ -870,12 +859,12 @@ const struct bmp280_chip_info bme280_chip_info = {

.oversampling_humid_avail = bmp280_oversampling_avail,
.num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail),
- .oversampling_humid_default = BMP280_OSRS_HUMIDITY_16X - 1,
+ .oversampling_humid_default = BME280_OSRS_HUMIDITY_16X - 1,

.chip_config = bme280_chip_config,
.read_temp = bmp280_read_temp,
.read_press = bmp280_read_press,
- .read_humid = bmp280_read_humid,
+ .read_humid = bme280_read_humid,
.read_calib = bme280_read_calib,
};
EXPORT_SYMBOL_NS(bme280_chip_info, IIO_BMP280);
@@ -1116,7 +1105,7 @@ static const int bmp380_odr_table[][2] = {
[BMP380_ODR_0_01HZ] = {0, 12207},
[BMP380_ODR_0_006HZ] = {0, 6104},
[BMP380_ODR_0_003HZ] = {0, 3052},
- [BMP380_ODR_0_0015HZ] = {0, 1526},
+ [BMP380_ODR_0_0015HZ] = {0, 1526}
};

static int bmp380_preinit(struct bmp280_data *data)
@@ -1138,8 +1127,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
BMP380_CTRL_SENSORS_PRESS_EN |
BMP380_CTRL_SENSORS_TEMP_EN);
if (ret) {
- dev_err(data->dev,
- "failed to write operation control register\n");
+ dev_err(data->dev, "failed to write operation control register\n");
return ret;
}

@@ -1216,7 +1204,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
}
if (tmp & BMP380_ERR_CONF_MASK) {
dev_warn(data->dev,
- "sensor flagged configuration as incompatible\n");
+ "sensor flagged configuration as incompatible\n");
return -EINVAL;
}
}
@@ -1345,10 +1333,6 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
poll = 50;
timeout = 400;
}
- if (ret) {
- dev_err(data->dev, "failed to write command sequence\n");
- return -EIO;
- }

/* Wait until NVM is ready again */
ret = regmap_read_poll_timeout(data->regmap, BMP580_REG_STATUS, reg,
@@ -1460,7 +1444,7 @@ static const int bmp580_odr_table[][2] = {
[BMP580_ODR_1HZ] = {1, 0},
[BMP580_ODR_0_5HZ] = {0, 500000},
[BMP580_ODR_0_25HZ] = {0, 250000},
- [BMP580_ODR_0_125HZ] = {0, 125000},
+ [BMP580_ODR_0_125HZ] = {0, 125000}
};

static const int bmp580_nvmem_addrs[] = { 0x20, 0x21, 0x22 };
@@ -1501,8 +1485,8 @@ static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
if (ret)
goto exit;

- ret = regmap_bulk_read(data->regmap, BMP580_REG_NVM_DATA_LSB, &data->le16,
- sizeof(data->le16));
+ ret = regmap_bulk_read(data->regmap, BMP580_REG_NVM_DATA_LSB,
+ &data->le16, sizeof(data->le16));
if (ret) {
dev_err(data->dev, "error reading nvm data regs\n");
goto exit;
@@ -1546,7 +1530,8 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
while (bytes >= sizeof(*buf)) {
addr = bmp580_nvmem_addrs[offset / sizeof(*buf)];

- ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR, BMP580_NVM_PROG_EN |
+ ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
+ BMP580_NVM_PROG_EN |
FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
if (ret) {
dev_err(data->dev, "error writing nvm address\n");
@@ -1554,8 +1539,8 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
}
data->le16 = cpu_to_le16(*buf++);

- ret = regmap_bulk_write(data->regmap, BMP580_REG_NVM_DATA_LSB, &data->le16,
- sizeof(data->le16));
+ ret = regmap_bulk_write(data->regmap, BMP580_REG_NVM_DATA_LSB,
+ &data->le16, sizeof(data->le16));
if (ret) {
dev_err(data->dev, "error writing LSB NVM data regs\n");
goto exit;
@@ -1607,20 +1592,24 @@ static int bmp580_preinit(struct bmp280_data *data)

/* Post powerup sequence */
ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, &reg);
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to establish comms with the chip\n");
return ret;
+ }

/* Print warn message if we don't know the chip id */
if (reg != BMP580_CHIP_ID && reg != BMP580_CHIP_ID_ALT)
- dev_warn(data->dev, "preinit: unexpected chip_id\n");
+ dev_warn(data->dev, "unexpected chip_id\n");

ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to read nvm status\n");
return ret;
+ }

/* Check nvm status */
if (!(reg & BMP580_STATUS_NVM_RDY_MASK) || (reg & BMP580_STATUS_NVM_ERR_MASK)) {
- dev_err(data->dev, "preinit: nvm error on powerup sequence\n");
+ dev_err(data->dev, "nvm error on powerup sequence\n");
return -EIO;
}

@@ -1655,6 +1644,10 @@ static int bmp580_chip_config(struct bmp280_data *data)
BMP580_DSP_COMP_MASK |
BMP580_DSP_SHDW_IIR_TEMP_EN |
BMP580_DSP_SHDW_IIR_PRESS_EN, reg_val);
+ if (ret) {
+ dev_err(data->dev, "failed to change DSP mode settings\n");
+ return ret;
+ }

/* Configure oversampling */
reg_val = FIELD_PREP(BMP580_OSR_TEMP_MASK, data->oversampling_temp) |
@@ -1763,7 +1756,7 @@ const struct bmp280_chip_info bmp580_chip_info = {
};
EXPORT_SYMBOL_NS(bmp580_chip_info, IIO_BMP280);

-static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
+static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
{
const int conversion_time_max[] = { 4500, 7500, 13500, 25500 };
unsigned int delay_us;
@@ -1774,8 +1767,10 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
reinit_completion(&data->done);

ret = regmap_write(data->regmap, BMP280_REG_CTRL_MEAS, ctrl_meas);
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to write crtl_meas register\n");
return ret;
+ }

if (data->use_eoc) {
/*
@@ -1798,12 +1793,16 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
}

ret = regmap_read(data->regmap, BMP280_REG_CTRL_MEAS, &ctrl);
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to read ctrl_meas register\n");
return ret;
+ }

/* The value of this bit reset to "0" after conversion is complete */
- if (ctrl & BMP180_MEAS_SCO)
+ if (ctrl & BMP180_MEAS_SCO) {
+ dev_err(data->dev, "conversion didn't complete\n");
return -EIO;
+ }

return 0;
}
@@ -1812,7 +1811,7 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
{
int ret;

- ret = bmp180_measure(data,
+ ret = bmp180_wait_for_eoc(data,
FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_TEMP) |
BMP180_MEAS_SCO);
if (ret)
@@ -1820,8 +1819,10 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)

ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
&data->be16, sizeof(data->be16));
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to read temperature\n");
return ret;
+ }

*val = be16_to_cpu(data->be16);

@@ -1836,9 +1837,10 @@ static int bmp180_read_calib(struct bmp280_data *data)

ret = regmap_bulk_read(data->regmap, BMP180_REG_CALIB_START,
data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
-
- if (ret < 0)
+ if (ret) {
+ dev_err(data->dev, "failed to read calibration parameters\n");
return ret;
+ }

/* None of the words has the value 0 or 0xFFFF */
for (i = 0; i < ARRAY_SIZE(data->bmp180_cal_buf); i++) {
@@ -1911,17 +1913,19 @@ static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
u8 oss = data->oversampling_press;
int ret;

- ret = bmp180_measure(data,
- FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_PRESS) |
- FIELD_PREP(BMP180_OSRS_PRESS_MASK, oss) |
- BMP180_MEAS_SCO);
+ ret = bmp180_wait_for_eoc(data,
+ FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_PRESS) |
+ FIELD_PREP(BMP180_OSRS_PRESS_MASK, oss) |
+ BMP180_MEAS_SCO);
if (ret)
return ret;

ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
data->buf, sizeof(data->buf));
- if (ret)
+ if (ret) {
+ dev_err(data->dev, "failed to read pressure\n");
return ret;
+ }

*val = get_unaligned_be24(data->buf) >> (8 - oss);

@@ -1963,8 +1967,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
return p + ((x1 + x2 + 3791) >> 4);
}

-static int bmp180_read_press(struct bmp280_data *data,
- int *val, int *val2)
+static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
{
u32 comp_press;
s32 adc_press;
@@ -2154,8 +2157,10 @@ int bmp280_common_probe(struct device *dev,
data->regmap = regmap;

ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
- if (ret < 0)
+ if (ret) {
+ dev_err(data->dev, "failed to read chip id\n");
return ret;
+ }

for (i = 0; i < data->chip_info->num_chip_id; i++) {
if (chip_id == data->chip_info->chip_id[i]) {
@@ -2175,7 +2180,7 @@ int bmp280_common_probe(struct device *dev,
}

ret = data->chip_info->chip_config(data);
- if (ret < 0)
+ if (ret)
return ret;

dev_set_drvdata(dev, indio_dev);
@@ -2188,7 +2193,7 @@ int bmp280_common_probe(struct device *dev,

if (data->chip_info->read_calib) {
ret = data->chip_info->read_calib(data);
- if (ret < 0)
+ if (ret)
return dev_err_probe(data->dev, ret,
"failed to read calibration coefficients\n");
}
@@ -2241,6 +2246,7 @@ static int bmp280_runtime_resume(struct device *dev)
ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
if (ret)
return ret;
+
usleep_range(data->start_up_time, data->start_up_time + 100);
return data->chip_info->chip_config(data);
}
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index 34e3bc758493..5c3a63b4327c 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
-#include <linux/module.h>
#include <linux/i2c.h>
+#include <linux/module.h>
#include <linux/regmap.h>

#include "bmp280.h"
diff --git a/drivers/iio/pressure/bmp280-regmap.c b/drivers/iio/pressure/bmp280-regmap.c
index 3ee56720428c..fa52839474b1 100644
--- a/drivers/iio/pressure/bmp280-regmap.c
+++ b/drivers/iio/pressure/bmp280-regmap.c
@@ -45,7 +45,7 @@ static bool bmp280_is_writeable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
case BMP280_REG_CONFIG:
- case BMP280_REG_CTRL_HUMIDITY:
+ case BME280_REG_CTRL_HUMIDITY:
case BMP280_REG_CTRL_MEAS:
case BMP280_REG_RESET:
return true;
@@ -57,8 +57,8 @@ static bool bmp280_is_writeable_reg(struct device *dev, unsigned int reg)
static bool bmp280_is_volatile_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
- case BMP280_REG_HUMIDITY_LSB:
- case BMP280_REG_HUMIDITY_MSB:
+ case BME280_REG_HUMIDITY_LSB:
+ case BME280_REG_HUMIDITY_MSB:
case BMP280_REG_TEMP_XLSB:
case BMP280_REG_TEMP_LSB:
case BMP280_REG_TEMP_MSB:
@@ -167,7 +167,7 @@ const struct regmap_config bmp280_regmap_config = {
.reg_bits = 8,
.val_bits = 8,

- .max_register = BMP280_REG_HUMIDITY_LSB,
+ .max_register = BME280_REG_HUMIDITY_LSB,
.cache_type = REGCACHE_RBTREE,

.writeable_reg = bmp280_is_writeable_reg,
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 4e19ea0b4d39..ce92f283e142 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -5,15 +5,15 @@
* Inspired by the older BMP085 driver drivers/misc/bmp085-spi.c
*/
#include <linux/bits.h>
-#include <linux/module.h>
-#include <linux/spi/spi.h>
#include <linux/err.h>
+#include <linux/module.h>
#include <linux/regmap.h>
+#include <linux/spi/spi.h>

#include "bmp280.h"

static int bmp280_regmap_spi_write(void *context, const void *data,
- size_t count)
+ size_t count)
{
struct spi_device *spi = to_spi_device(context);
u8 buf[2];
@@ -29,7 +29,7 @@ static int bmp280_regmap_spi_write(void *context, const void *data,
}

static int bmp280_regmap_spi_read(void *context, const void *reg,
- size_t reg_size, void *val, size_t val_size)
+ size_t reg_size, void *val, size_t val_size)
{
struct spi_device *spi = to_spi_device(context);

diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 5812a344ed8e..ea8eb5691428 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -1,10 +1,10 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include <linux/bitops.h>
#include <linux/device.h>
-#include <linux/iio/iio.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>

+#include <linux/iio/iio.h>

/* BMP580 specific registers */
#define BMP580_REG_CMD 0x7E
@@ -192,8 +192,8 @@
#define BMP380_PRESS_SKIPPED 0x800000

/* BMP280 specific registers */
-#define BMP280_REG_HUMIDITY_LSB 0xFE
-#define BMP280_REG_HUMIDITY_MSB 0xFD
+#define BME280_REG_HUMIDITY_LSB 0xFE
+#define BME280_REG_HUMIDITY_MSB 0xFD
#define BMP280_REG_TEMP_XLSB 0xFC
#define BMP280_REG_TEMP_LSB 0xFB
#define BMP280_REG_TEMP_MSB 0xFA
@@ -207,15 +207,15 @@
#define BMP280_REG_CONFIG 0xF5
#define BMP280_REG_CTRL_MEAS 0xF4
#define BMP280_REG_STATUS 0xF3
-#define BMP280_REG_CTRL_HUMIDITY 0xF2
+#define BME280_REG_CTRL_HUMIDITY 0xF2

/* Due to non linear mapping, and data sizes we can't do a bulk read */
-#define BMP280_REG_COMP_H1 0xA1
-#define BMP280_REG_COMP_H2 0xE1
-#define BMP280_REG_COMP_H3 0xE3
-#define BMP280_REG_COMP_H4 0xE4
-#define BMP280_REG_COMP_H5 0xE5
-#define BMP280_REG_COMP_H6 0xE7
+#define BME280_REG_COMP_H1 0xA1
+#define BME280_REG_COMP_H2 0xE1
+#define BME280_REG_COMP_H3 0xE3
+#define BME280_REG_COMP_H4 0xE4
+#define BME280_REG_COMP_H5 0xE5
+#define BME280_REG_COMP_H6 0xE7

#define BMP280_REG_COMP_TEMP_START 0x88
#define BMP280_COMP_TEMP_REG_COUNT 6
@@ -223,7 +223,7 @@
#define BMP280_REG_COMP_PRESS_START 0x8E
#define BMP280_COMP_PRESS_REG_COUNT 18

-#define BMP280_COMP_H5_MASK GENMASK(15, 4)
+#define BME280_COMP_H5_MASK GENMASK(15, 4)

#define BMP280_CONTIGUOUS_CALIB_REGS (BMP280_COMP_TEMP_REG_COUNT + \
BMP280_COMP_PRESS_REG_COUNT)
@@ -235,13 +235,13 @@
#define BMP280_FILTER_8X 3
#define BMP280_FILTER_16X 4

-#define BMP280_OSRS_HUMIDITY_MASK GENMASK(2, 0)
-#define BMP280_OSRS_HUMIDITY_SKIP 0
-#define BMP280_OSRS_HUMIDITY_1X 1
-#define BMP280_OSRS_HUMIDITY_2X 2
-#define BMP280_OSRS_HUMIDITY_4X 3
-#define BMP280_OSRS_HUMIDITY_8X 4
-#define BMP280_OSRS_HUMIDITY_16X 5
+#define BME280_OSRS_HUMIDITY_MASK GENMASK(2, 0)
+#define BME280_OSRS_HUMIDITY_SKIP 0
+#define BME280_OSRS_HUMIDITY_1X 1
+#define BME280_OSRS_HUMIDITY_2X 2
+#define BME280_OSRS_HUMIDITY_4X 3
+#define BME280_OSRS_HUMIDITY_8X 4
+#define BME280_OSRS_HUMIDITY_16X 5

#define BMP280_OSRS_TEMP_MASK GENMASK(7, 5)
#define BMP280_OSRS_TEMP_SKIP 0
@@ -449,12 +449,12 @@ struct bmp280_chip_info {
int num_sampling_freq_avail;
int sampling_freq_default;

- int (*chip_config)(struct bmp280_data *);
- int (*read_temp)(struct bmp280_data *, int *, int *);
- int (*read_press)(struct bmp280_data *, int *, int *);
- int (*read_humid)(struct bmp280_data *, int *, int *);
- int (*read_calib)(struct bmp280_data *);
- int (*preinit)(struct bmp280_data *);
+ int (*chip_config)(struct bmp280_data *data);
+ int (*read_temp)(struct bmp280_data *data, int *val, int *val2);
+ int (*read_press)(struct bmp280_data *data, int *val, int *val2);
+ int (*read_humid)(struct bmp280_data *data, int *val, int *val2);
+ int (*read_calib)(struct bmp280_data *data);
+ int (*preinit)(struct bmp280_data *data);
};

/* Chip infos for each variant */
@@ -473,7 +473,7 @@ extern const struct regmap_config bmp580_regmap_config;
/* Probe called from different transports */
int bmp280_common_probe(struct device *dev,
struct regmap *regmap,
- const struct bmp280_chip_info *,
+ const struct bmp280_chip_info *chip_info,
const char *name,
int irq);

--
2.25.1


2024-04-07 17:30:25

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 5/6] iio: pressure: bmp280: Add SCALE, RAW values in channels and refactorize them

Add extra IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_RAW channels in order
to be able to calculate the processed value with standard userspace
IIO tools. Can be used for triggered buffers as well.

Even though it is not a good design choice to have SCALE, RAW and
PROCESSED together, the PROCESSED channel is kept for ABI compatibility.

While at it, separate BMPxxx and BMExxx device channels since BME
supports also humidity measurements.

Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 86 +++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index d05035574683..1b894feb717b 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -137,17 +137,45 @@ enum {
static const struct iio_chan_spec bmp280_channels[] = {
{
.type = IIO_PRESSURE,
+ /* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
},
{
.type = IIO_TEMP,
+ /* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ },
+};
+
+static const struct iio_chan_spec bme280_channels[] = {
+ {
+ .type = IIO_PRESSURE,
+ /* PROCESSED maintained for ABI backwards compatibility */
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ },
+ {
+ .type = IIO_TEMP,
+ /* PROCESSED maintained for ABI backwards compatibility */
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
},
{
.type = IIO_HUMIDITYRELATIVE,
+ /* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
},
};
@@ -155,21 +183,20 @@ static const struct iio_chan_spec bmp280_channels[] = {
static const struct iio_chan_spec bmp380_channels[] = {
{
.type = IIO_PRESSURE,
+ /* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
},
{
.type = IIO_TEMP,
+ /* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
- .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
- },
- {
- .type = IIO_HUMIDITYRELATIVE,
- .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
@@ -529,6 +556,49 @@ static int bmp_read_raw_impl(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_HUMIDITYRELATIVE:
+ ret = data->chip_info->read_humid(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = chan_value;
+ return IIO_VAL_INT;
+ case IIO_PRESSURE:
+ ret = data->chip_info->read_press(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = chan_value;
+ return IIO_VAL_INT;
+ case IIO_TEMP:
+ ret = data->chip_info->read_temp(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = chan_value;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_HUMIDITYRELATIVE:
+ *val = data->chip_info->humid_coeffs[0];
+ *val2 = data->chip_info->humid_coeffs[1];
+ return data->chip_info->humid_coeffs_type;
+ case IIO_PRESSURE:
+ *val = data->chip_info->press_coeffs[0];
+ *val2 = data->chip_info->press_coeffs[1];
+ return data->chip_info->press_coeffs_type;
+ case IIO_TEMP:
+ *val = data->chip_info->temp_coeffs[0];
+ *val2 = data->chip_info->temp_coeffs[1];
+ return data->chip_info->temp_coeffs_type;
+ default:
+ return -EINVAL;
+ }
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
@@ -895,7 +965,7 @@ const struct bmp280_chip_info bme280_chip_info = {
.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
.regmap_config = &bmp280_regmap_config,
.start_up_time = 2000,
- .channels = bmp280_channels,
+ .channels = bme280_channels,
.num_channels = 3,

.oversampling_temp_avail = bmp280_oversampling_avail,
--
2.25.1


2024-04-07 17:30:41

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 4/6] iio: pressure: bmp280: Generalize read_{temp,press,humid}() functions

Add the coefficients for the IIO standard units and the IIO value
inside the chip_info structure.

Move the calculations for the IIO unit compatibility from inside the
read_{temp,press,humid}() functions and move them to the general
read_raw() function.

In this way, all the data for the calculation of the value are
located in the chip_info structure of the respective sensor.

Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/bmp280-core.c | 167 ++++++++++++++++-------------
drivers/iio/pressure/bmp280.h | 13 ++-
2 files changed, 102 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 51bcdf8cede6..d05035574683 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -439,26 +439,23 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
return (u32)p;
}

-static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
+static int bmp280_read_temp(struct bmp280_data *data, s32 *comp_temp)
{
- s32 adc_temp, comp_temp;
+ s32 adc_temp;
int ret;

ret = bmp280_read_temp_adc(data, &adc_temp);
if (ret)
return ret;

- comp_temp = bmp280_compensate_temp(data, adc_temp);
+ *comp_temp = bmp280_compensate_temp(data, adc_temp);

- /* IIO units are in milli Celsius */
- *val = comp_temp * 10;
- return IIO_VAL_INT;
+ return 0;
}

-static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
+static int bmp280_read_press(struct bmp280_data *data, u32 *comp_press)
{
s32 adc_press, t_fine;
- u32 comp_press;
int ret;

ret = bmp280_get_t_fine(data, &t_fine);
@@ -469,19 +466,14 @@ static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
if (ret)
return ret;

- comp_press = bmp280_compensate_press(data, adc_press, t_fine);
+ *comp_press = bmp280_compensate_press(data, adc_press, t_fine);

- /* IIO units are in kPa */
- *val = comp_press;
- *val2 = 256000;
-
- return IIO_VAL_FRACTIONAL;
+ return 0;
}

-static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
+static int bme280_read_humid(struct bmp280_data *data, u32 *comp_humidity)
{
s32 adc_humidity, t_fine;
- u32 comp_humidity;
int ret;

ret = bmp280_get_t_fine(data, &t_fine);
@@ -492,12 +484,9 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
if (ret)
return ret;

- comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine);
+ *comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine);

- /* IIO units are in 1000 * % */
- *val = comp_humidity * 1000 / 1024;
-
- return IIO_VAL_INT;
+ return 0;
}

static int bmp_read_raw_impl(struct iio_dev *indio_dev,
@@ -505,6 +494,8 @@ static int bmp_read_raw_impl(struct iio_dev *indio_dev,
int *val, int *val2, long mask)
{
struct bmp280_data *data = iio_priv(indio_dev);
+ int chan_value;
+ int ret;

guard(mutex)(&data->lock);

@@ -512,11 +503,29 @@ static int bmp_read_raw_impl(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_PROCESSED:
switch (chan->type) {
case IIO_HUMIDITYRELATIVE:
- return data->chip_info->read_humid(data, val, val2);
+ ret = data->chip_info->read_humid(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = data->chip_info->humid_coeffs[0] * chan_value;
+ *val2 = data->chip_info->humid_coeffs[1];
+ return data->chip_info->humid_coeffs_type;
case IIO_PRESSURE:
- return data->chip_info->read_press(data, val, val2);
+ ret = data->chip_info->read_press(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = data->chip_info->press_coeffs[0] * chan_value;
+ *val2 = data->chip_info->press_coeffs[1];
+ return data->chip_info->press_coeffs_type;
case IIO_TEMP:
- return data->chip_info->read_temp(data, val, val2);
+ ret = data->chip_info->read_temp(data, &chan_value);
+ if (ret)
+ return ret;
+
+ *val = data->chip_info->temp_coeffs[0] * chan_value;
+ *val2 = data->chip_info->temp_coeffs[1];
+ return data->chip_info->temp_coeffs_type;
default:
return -EINVAL;
}
@@ -816,6 +825,8 @@ static int bmp280_chip_config(struct bmp280_data *data)

static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
+static const int bmp280_temp_coeffs[] = { 10, 1 };
+static const int bmp280_press_coeffs[] = { 1, 256000 };

const struct bmp280_chip_info bmp280_chip_info = {
.id_reg = BMP280_REG_ID,
@@ -844,6 +855,11 @@ const struct bmp280_chip_info bmp280_chip_info = {
.num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail),
.oversampling_press_default = BMP280_OSRS_PRESS_16X - 1,

+ .temp_coeffs = bmp280_temp_coeffs,
+ .temp_coeffs_type = IIO_VAL_FRACTIONAL,
+ .press_coeffs = bmp280_press_coeffs,
+ .press_coeffs_type = IIO_VAL_FRACTIONAL,
+
.chip_config = bmp280_chip_config,
.read_temp = bmp280_read_temp,
.read_press = bmp280_read_press,
@@ -871,6 +887,7 @@ static int bme280_chip_config(struct bmp280_data *data)
}

static const u8 bme280_chip_ids[] = { BME280_CHIP_ID };
+static const int bme280_humid_coeffs[] = { 1000, 1024 };

const struct bmp280_chip_info bme280_chip_info = {
.id_reg = BMP280_REG_ID,
@@ -893,6 +910,13 @@ const struct bmp280_chip_info bme280_chip_info = {
.num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail),
.oversampling_humid_default = BME280_OSRS_HUMIDITY_16X - 1,

+ .temp_coeffs = bmp280_temp_coeffs,
+ .temp_coeffs_type = IIO_VAL_FRACTIONAL,
+ .press_coeffs = bmp280_press_coeffs,
+ .press_coeffs_type = IIO_VAL_FRACTIONAL,
+ .humid_coeffs = bme280_humid_coeffs,
+ .humid_coeffs_type = IIO_VAL_FRACTIONAL,
+
.chip_config = bme280_chip_config,
.read_temp = bmp280_read_temp,
.read_press = bmp280_read_press,
@@ -1081,9 +1105,8 @@ static u32 bmp380_compensate_press(struct bmp280_data *data,
return comp_press;
}

-static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
+static int bmp380_read_temp(struct bmp280_data *data, s32 *comp_temp)
{
- s32 comp_temp;
u32 adc_temp;
int ret;

@@ -1091,16 +1114,14 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
if (ret)
return ret;

- comp_temp = bmp380_compensate_temp(data, adc_temp);
+ *comp_temp = bmp380_compensate_temp(data, adc_temp);

- /* IIO units are in milli Celsius */
- *val = comp_temp * 10;
- return IIO_VAL_INT;
+ return 0;
}

-static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
+static int bmp380_read_press(struct bmp280_data *data, u32 *comp_press)
{
- u32 adc_press, comp_press, t_fine;
+ u32 adc_press, t_fine;
int ret;

ret = bmp380_get_t_fine(data, &t_fine);
@@ -1111,13 +1132,9 @@ static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
if (ret)
return ret;

- comp_press = bmp380_compensate_press(data, adc_press, t_fine);
+ *comp_press = bmp380_compensate_press(data, adc_press, t_fine);

- /* IIO units are in kPa */
- *val = comp_press;
- *val2 = 100000;
-
- return IIO_VAL_FRACTIONAL;
+ return 0;
}

static int bmp380_read_calib(struct bmp280_data *data)
@@ -1284,6 +1301,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID };
+static const int bmp380_temp_coeffs[] = { 10, 1 };
+static const int bmp380_press_coeffs[] = { 1, 100000 };

const struct bmp280_chip_info bmp380_chip_info = {
.id_reg = BMP380_REG_ID,
@@ -1311,6 +1330,11 @@ const struct bmp280_chip_info bmp380_chip_info = {
.num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail),
.iir_filter_coeff_default = 2,

+ .temp_coeffs = bmp380_temp_coeffs,
+ .temp_coeffs_type = IIO_VAL_FRACTIONAL,
+ .press_coeffs = bmp380_press_coeffs,
+ .press_coeffs_type = IIO_VAL_FRACTIONAL,
+
.chip_config = bmp380_chip_config,
.read_temp = bmp380_read_temp,
.read_press = bmp380_read_press,
@@ -1427,9 +1451,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
* for what is expected on IIO ABI.
*/

-static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
+static int bmp580_read_temp(struct bmp280_data *data, s32 *raw_temp)
{
- s32 raw_temp;
int ret;

ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf,
@@ -1439,25 +1462,17 @@ static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
return ret;
}

- raw_temp = get_unaligned_le24(data->buf);
- if (raw_temp == BMP580_TEMP_SKIPPED) {
+ *raw_temp = get_unaligned_le24(data->buf);
+ if (*raw_temp == BMP580_TEMP_SKIPPED) {
dev_err(data->dev, "reading temperature skipped\n");
return -EIO;
}

- /*
- * Temperature is returned in Celsius degrees in fractional
- * form down 2^16. We rescale by x1000 to return milli Celsius
- * to respect IIO ABI.
- */
- *val = raw_temp * 1000;
- *val2 = 16;
- return IIO_VAL_FRACTIONAL_LOG2;
+ return 0;
}

-static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
+static int bmp580_read_press(struct bmp280_data *data, u32 *raw_press)
{
- u32 raw_press;
int ret;

ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf,
@@ -1467,18 +1482,13 @@ static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
return ret;
}

- raw_press = get_unaligned_le24(data->buf);
- if (raw_press == BMP580_PRESS_SKIPPED) {
+ *raw_press = get_unaligned_le24(data->buf);
+ if (*raw_press == BMP580_PRESS_SKIPPED) {
dev_err(data->dev, "reading pressure skipped\n");
return -EIO;
}
- /*
- * Pressure is returned in Pascals in fractional form down 2^16.
- * We rescale /1000 to convert to kilopascal to respect IIO ABI.
- */
- *val = raw_press;
- *val2 = 64000; /* 2^6 * 1000 */
- return IIO_VAL_FRACTIONAL;
+
+ return 0;
}

static const int bmp580_odr_table[][2] = {
@@ -1792,6 +1802,8 @@ static int bmp580_chip_config(struct bmp280_data *data)

static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT };
+static const int bmp580_temp_coeffs[] = { 1000, 16 };
+static const int bmp580_press_coeffs[] = { 1, 64000};

const struct bmp280_chip_info bmp580_chip_info = {
.id_reg = BMP580_REG_CHIP_ID,
@@ -1818,6 +1830,11 @@ const struct bmp280_chip_info bmp580_chip_info = {
.num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail),
.iir_filter_coeff_default = 2,

+ .temp_coeffs = bmp580_temp_coeffs,
+ .temp_coeffs_type = IIO_VAL_FRACTIONAL_LOG2,
+ .press_coeffs = bmp580_press_coeffs,
+ .press_coeffs_type = IIO_VAL_FRACTIONAL,
+
.chip_config = bmp580_chip_config,
.read_temp = bmp580_read_temp,
.read_press = bmp580_read_press,
@@ -1972,20 +1989,18 @@ static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
return (bmp180_calc_t_fine(data, adc_temp) + 8) / 16;
}

-static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
+static int bmp180_read_temp(struct bmp280_data *data, s32 *comp_temp)
{
- s32 adc_temp, comp_temp;
+ s32 adc_temp;
int ret;

ret = bmp180_read_temp_adc(data, &adc_temp);
if (ret)
return ret;

- comp_temp = bmp180_compensate_temp(data, adc_temp);
+ *comp_temp = bmp180_compensate_temp(data, adc_temp);

- /* IIO units are in milli Celsius */
- *val = comp_temp * 100;
- return IIO_VAL_INT;
+ return 0;
}

static int bmp180_read_press_adc(struct bmp280_data *data, s32 *adc_press)
@@ -2048,10 +2063,9 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press,
return p + ((x1 + x2 + 3791) >> 4);
}

-static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
+static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
{
s32 adc_press, t_fine;
- u32 comp_press;
int ret;

ret = bmp180_get_t_fine(data, &t_fine);
@@ -2062,13 +2076,9 @@ static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
if (ret)
return ret;

- comp_press = bmp180_compensate_press(data, adc_press, t_fine);
-
- /* IIO units are in kPa */
- *val = comp_press;
- *val2 = 1000;
+ *comp_press = bmp180_compensate_press(data, adc_press, t_fine);

- return IIO_VAL_FRACTIONAL;
+ return 0;
}

static int bmp180_chip_config(struct bmp280_data *data)
@@ -2079,6 +2089,8 @@ static int bmp180_chip_config(struct bmp280_data *data)
static const int bmp180_oversampling_temp_avail[] = { 1 };
static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };
static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID };
+static const int bmp180_temp_coeffs[] = { 100, 1 };
+static const int bmp180_press_coeffs[] = { 1, 1000 };

const struct bmp280_chip_info bmp180_chip_info = {
.id_reg = BMP280_REG_ID,
@@ -2099,6 +2111,11 @@ const struct bmp280_chip_info bmp180_chip_info = {
ARRAY_SIZE(bmp180_oversampling_press_avail),
.oversampling_press_default = BMP180_MEAS_PRESS_8X,

+ .temp_coeffs = bmp180_temp_coeffs,
+ .temp_coeffs_type = IIO_VAL_FRACTIONAL,
+ .press_coeffs = bmp180_press_coeffs,
+ .press_coeffs_type = IIO_VAL_FRACTIONAL,
+
.chip_config = bmp180_chip_config,
.read_temp = bmp180_read_temp,
.read_press = bmp180_read_press,
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 6dbd7413ad3f..ccba779d7a83 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -443,10 +443,17 @@ struct bmp280_chip_info {
int num_sampling_freq_avail;
int sampling_freq_default;

+ const int *temp_coeffs;
+ const int temp_coeffs_type;
+ const int *press_coeffs;
+ const int press_coeffs_type;
+ const int *humid_coeffs;
+ const int humid_coeffs_type;
+
int (*chip_config)(struct bmp280_data *data);
- int (*read_temp)(struct bmp280_data *data, int *val, int *val2);
- int (*read_press)(struct bmp280_data *data, int *val, int *val2);
- int (*read_humid)(struct bmp280_data *data, int *val, int *val2);
+ int (*read_temp)(struct bmp280_data *data, s32 *adc_temp);
+ int (*read_press)(struct bmp280_data *data, u32 *adc_press);
+ int (*read_humid)(struct bmp280_data *data, u32 *adc_humidity);
int (*read_calib)(struct bmp280_data *data);
int (*preinit)(struct bmp280_data *data);
};
--
2.25.1


2024-04-07 17:30:54

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v4 6/6] iio: pressure: bmp280: Add triggered buffer support

BMP2xx, BME280, BMP3xx, and BMP5xx use continuous buffers for their
temperature, pressure and humidity readings. This facilitates the
use of burst/bulk reads in order to acquire data faster. The
approach is different from the one used in oneshot captures.

BMP085 & BMP1xx devices use a completely different measurement
process that is well defined and is used in their buffer_handler().

Suggested-by: Angel Iglesias <[email protected]>
Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/pressure/Kconfig | 2 +
drivers/iio/pressure/bmp280-core.c | 338 +++++++++++++++++++++++++++--
drivers/iio/pressure/bmp280-spi.c | 8 +-
drivers/iio/pressure/bmp280.h | 21 +-
4 files changed, 347 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index 3ad38506028e..0b5406a3f85d 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -31,6 +31,8 @@ config BMP280
select REGMAP
select BMP280_I2C if (I2C)
select BMP280_SPI if (SPI_MASTER)
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to build support for Bosch Sensortec BMP180, BMP280, BMP380
and BMP580 pressure and temperature sensors. Also supports the BME280 with
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 1b894feb717b..32dd35475826 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -41,7 +41,10 @@
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>

+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>

#include <asm/unaligned.h>

@@ -134,6 +137,12 @@ enum {
BMP380_P11 = 20
};

+enum bmp280_scan {
+ BMP280_PRESS,
+ BMP280_TEMP,
+ BME280_HUMID,
+};
+
static const struct iio_chan_spec bmp280_channels[] = {
{
.type = IIO_PRESSURE,
@@ -142,6 +151,13 @@ static const struct iio_chan_spec bmp280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
.type = IIO_TEMP,
@@ -150,7 +166,15 @@ static const struct iio_chan_spec bmp280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};

static const struct iio_chan_spec bme280_channels[] = {
@@ -161,6 +185,13 @@ static const struct iio_chan_spec bme280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
.type = IIO_TEMP,
@@ -169,6 +200,13 @@ static const struct iio_chan_spec bme280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
.type = IIO_HUMIDITYRELATIVE,
@@ -177,7 +215,15 @@ static const struct iio_chan_spec bme280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 2,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(3),
};

static const struct iio_chan_spec bmp380_channels[] = {
@@ -190,6 +236,13 @@ static const struct iio_chan_spec bmp380_channels[] = {
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
.type = IIO_TEMP,
@@ -200,7 +253,15 @@ static const struct iio_chan_spec bmp380_channels[] = {
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};

static int bmp280_read_calib(struct bmp280_data *data)
@@ -321,7 +382,7 @@ static int bme280_read_humid_adc(struct bmp280_data *data, s32 *adc_humidity)
int ret;

ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
- &data->be16, sizeof(data->be16));
+ &data->be16, BME280_NUM_HUMIDITY_BYTES);
if (ret) {
dev_err(data->dev, "failed to read humidity\n");
return ret;
@@ -366,7 +427,7 @@ static int bmp280_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
int ret;

ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
- data->buf, sizeof(data->buf));
+ data->buf, BMP280_NUM_TEMP_BYTES);
if (ret) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
@@ -425,7 +486,7 @@ static int bmp280_read_press_adc(struct bmp280_data *data, s32 *adc_press)
int ret;

ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
- data->buf, sizeof(data->buf));
+ data->buf, BMP280_NUM_PRESS_BYTES);
if (ret) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -866,6 +927,16 @@ static const struct iio_info bmp280_info = {
.write_raw = &bmp_write_raw,
};

+static const unsigned long bmp280_avail_scan_masks[] = {
+ BIT(BMP280_TEMP) | BIT(BMP280_PRESS),
+ 0
+};
+
+static const unsigned long bme280_avail_scan_masks[] = {
+ BIT(BME280_HUMID) | BIT(BMP280_TEMP) | BIT(BMP280_PRESS),
+ 0
+};
+
static int bmp280_chip_config(struct bmp280_data *data)
{
u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
@@ -893,6 +964,72 @@ static int bmp280_chip_config(struct bmp280_data *data)
return ret;
}

+static irqreturn_t bmp280_buffer_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct bmp280_data *data = iio_priv(indio_dev);
+ s32 adc_temp, adc_press, adc_humidity, t_fine;
+ u8 sizeof_burst_read;
+ int ret;
+
+ guard(mutex)(&data->lock);
+
+ /* If humidity channel is enabled it means that we are called for the
+ * BME280 humidity sensor.
+ */
+ if (test_bit(BME280_HUMID, indio_dev->active_scan_mask))
+ sizeof_burst_read = BME280_BURST_READ_BYTES;
+ else
+ sizeof_burst_read = BMP280_BURST_READ_BYTES;
+
+ /* Burst read data registers */
+ ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
+ data->buf, sizeof_burst_read);
+ if (ret) {
+ dev_err(data->dev, "failed to burst read sensor data\n");
+ return IRQ_HANDLED;
+ }
+
+ /* Temperature calculations */
+ adc_temp = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[3]));
+ if (adc_temp == BMP280_TEMP_SKIPPED) {
+ dev_err(data->dev, "reading temperature skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ data->sensor_data[1] = bmp280_compensate_temp(data, adc_temp);
+
+ /* Pressure calculations */
+ adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[0]));
+ if (adc_press == BMP280_PRESS_SKIPPED) {
+ dev_err(data->dev, "reading pressure skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ t_fine = bmp280_calc_t_fine(data, adc_temp);
+
+ data->sensor_data[0] = bmp280_compensate_press(data, adc_press, t_fine);
+
+ /* Humidity calculations */
+ if (test_bit(BME280_HUMID, indio_dev->active_scan_mask)) {
+ adc_humidity = get_unaligned_be16(&data->buf[6]);
+
+ if (adc_humidity == BMP280_HUMIDITY_SKIPPED) {
+ dev_err(data->dev, "reading humidity skipped\n");
+ return IRQ_HANDLED;
+ }
+ data->sensor_data[2] = bme280_compensate_humidity(data, adc_humidity, t_fine);
+ }
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
+ iio_get_time_ns(indio_dev));
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
static const int bmp280_temp_coeffs[] = { 10, 1 };
@@ -905,7 +1042,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
.regmap_config = &bmp280_regmap_config,
.start_up_time = 2000,
.channels = bmp280_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,

.oversampling_temp_avail = bmp280_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
@@ -934,6 +1072,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
.read_temp = bmp280_read_temp,
.read_press = bmp280_read_press,
.read_calib = bmp280_read_calib,
+
+ .buffer_handler = bmp280_buffer_handler,
};
EXPORT_SYMBOL_NS(bmp280_chip_info, IIO_BMP280);

@@ -966,7 +1106,8 @@ const struct bmp280_chip_info bme280_chip_info = {
.regmap_config = &bmp280_regmap_config,
.start_up_time = 2000,
.channels = bme280_channels,
- .num_channels = 3,
+ .num_channels = 4,
+ .avail_scan_masks = bme280_avail_scan_masks,

.oversampling_temp_avail = bmp280_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
@@ -992,6 +1133,8 @@ const struct bmp280_chip_info bme280_chip_info = {
.read_press = bmp280_read_press,
.read_humid = bme280_read_humid,
.read_calib = bme280_read_calib,
+
+ .buffer_handler = bmp280_buffer_handler,
};
EXPORT_SYMBOL_NS(bme280_chip_info, IIO_BMP280);

@@ -1052,7 +1195,7 @@ static int bmp380_read_temp_adc(struct bmp280_data *data, u32 *adc_temp)
int ret;

ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
- data->buf, sizeof(data->buf));
+ data->buf, BMP280_NUM_TEMP_BYTES);
if (ret) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
@@ -1119,7 +1262,7 @@ static int bmp380_read_press_adc(struct bmp280_data *data, u32 *adc_press)
int ret;

ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
- data->buf, sizeof(data->buf));
+ data->buf, BMP280_NUM_PRESS_BYTES);
if (ret) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -1368,6 +1511,52 @@ static int bmp380_chip_config(struct bmp280_data *data)
return 0;
}

+static irqreturn_t bmp380_buffer_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct bmp280_data *data = iio_priv(indio_dev);
+ s32 adc_temp, adc_press, t_fine;
+ int ret;
+
+ guard(mutex)(&data->lock);
+
+ /* Burst read data registers */
+ ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
+ data->buf, BMP280_BURST_READ_BYTES);
+ if (ret) {
+ dev_err(data->dev, "failed to burst read sensor data\n");
+ return IRQ_HANDLED;
+ }
+
+ /* Temperature calculations */
+ adc_temp = get_unaligned_le24(&data->buf[3]);
+ if (adc_temp == BMP380_TEMP_SKIPPED) {
+ dev_err(data->dev, "reading temperature skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ data->sensor_data[1] = bmp380_compensate_temp(data, adc_temp);
+
+ /* Pressure calculations */
+ adc_press = get_unaligned_le24(&data->buf[0]);
+ if (adc_press == BMP380_PRESS_SKIPPED) {
+ dev_err(data->dev, "reading pressure skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ t_fine = bmp380_calc_t_fine(data, adc_temp);
+
+ data->sensor_data[0] = bmp380_compensate_press(data, adc_press, t_fine);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
+ iio_get_time_ns(indio_dev));
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID };
@@ -1382,7 +1571,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
.spi_read_extra_byte = true,
.start_up_time = 2000,
.channels = bmp380_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,

.oversampling_temp_avail = bmp380_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp380_oversampling_avail),
@@ -1410,6 +1600,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
.read_press = bmp380_read_press,
.read_calib = bmp380_read_calib,
.preinit = bmp380_preinit,
+
+ .buffer_handler = bmp380_buffer_handler,
};
EXPORT_SYMBOL_NS(bmp380_chip_info, IIO_BMP280);

@@ -1525,8 +1717,8 @@ static int bmp580_read_temp(struct bmp280_data *data, s32 *raw_temp)
{
int ret;

- ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf,
- sizeof(data->buf));
+ ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB,
+ data->buf, BMP280_NUM_TEMP_BYTES);
if (ret) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
@@ -1545,8 +1737,8 @@ static int bmp580_read_press(struct bmp280_data *data, u32 *raw_press)
{
int ret;

- ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf,
- sizeof(data->buf));
+ ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB,
+ data->buf, BMP280_NUM_PRESS_BYTES);
if (ret) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -1870,6 +2062,54 @@ static int bmp580_chip_config(struct bmp280_data *data)
return 0;
}

+static irqreturn_t bmp580_buffer_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct bmp280_data *data = iio_priv(indio_dev);
+ s32 adc_temp, adc_press;
+ int ret;
+
+ guard(mutex)(&data->lock);
+
+ /* If humidity channel is enabled it means that we are called for the
+ * BME280 humidity sensor.
+ */
+
+ /* Burst read data registers */
+ ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB,
+ data->buf, BMP280_BURST_READ_BYTES);
+ if (ret) {
+ dev_err(data->dev, "failed to burst read sensor data\n");
+ return IRQ_HANDLED;
+ }
+
+ /* Temperature calculations */
+ adc_temp = get_unaligned_le24(&data->buf[0]);
+ if (adc_temp == BMP580_TEMP_SKIPPED) {
+ dev_err(data->dev, "reading temperature skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ data->sensor_data[1] = adc_temp;
+
+ /* Pressure calculations */
+ adc_press = get_unaligned_le24(&data->buf[3]);
+ if (adc_press == BMP380_PRESS_SKIPPED) {
+ dev_err(data->dev, "reading pressure skipped\n");
+ return IRQ_HANDLED;
+ }
+
+ data->sensor_data[0] = adc_press;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
+ iio_get_time_ns(indio_dev));
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT };
static const int bmp580_temp_coeffs[] = { 1000, 16 };
@@ -1882,7 +2122,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
.regmap_config = &bmp580_regmap_config,
.start_up_time = 2000,
.channels = bmp380_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,

.oversampling_temp_avail = bmp580_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp580_oversampling_avail),
@@ -1909,6 +2150,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
.read_temp = bmp580_read_temp,
.read_press = bmp580_read_press,
.preinit = bmp580_preinit,
+
+ .buffer_handler = bmp580_buffer_handler,
};
EXPORT_SYMBOL_NS(bmp580_chip_info, IIO_BMP280);

@@ -2086,7 +2329,7 @@ static int bmp180_read_press_adc(struct bmp280_data *data, s32 *adc_press)
return ret;

ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
- data->buf, sizeof(data->buf));
+ data->buf, BMP280_NUM_PRESS_BYTES);
if (ret) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -2156,6 +2399,35 @@ static int bmp180_chip_config(struct bmp280_data *data)
return 0;
}

+static irqreturn_t bmp180_buffer_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct bmp280_data *data = iio_priv(indio_dev);
+ int ret, chan_value;
+
+ guard(mutex)(&data->lock);
+
+ ret = bmp180_read_temp(data, &chan_value);
+ if (ret)
+ return IRQ_HANDLED;
+
+ data->sensor_data[1] = chan_value;
+
+ ret = bmp180_read_press(data, &chan_value);
+ if (ret)
+ return IRQ_HANDLED;
+
+ data->sensor_data[0] = chan_value;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
+ iio_get_time_ns(indio_dev));
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const int bmp180_oversampling_temp_avail[] = { 1 };
static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };
static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID };
@@ -2169,7 +2441,8 @@ const struct bmp280_chip_info bmp180_chip_info = {
.regmap_config = &bmp180_regmap_config,
.start_up_time = 2000,
.channels = bmp280_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,

.oversampling_temp_avail = bmp180_oversampling_temp_avail,
.num_oversampling_temp_avail =
@@ -2190,6 +2463,8 @@ const struct bmp280_chip_info bmp180_chip_info = {
.read_temp = bmp180_read_temp,
.read_press = bmp180_read_press,
.read_calib = bmp180_read_calib,
+
+ .buffer_handler = bmp180_buffer_handler,
};
EXPORT_SYMBOL_NS(bmp180_chip_info, IIO_BMP280);

@@ -2235,6 +2510,30 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
return 0;
}

+static int bmp_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct bmp280_data *data = iio_priv(indio_dev);
+
+ pm_runtime_get_sync(data->dev);
+
+ return 0;
+}
+
+static int bmp_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct bmp280_data *data = iio_priv(indio_dev);
+
+ pm_runtime_mark_last_busy(data->dev);
+ pm_runtime_put_autosuspend(data->dev);
+
+ return 0;
+}
+
+const struct iio_buffer_setup_ops bmp_buffer_setup_ops = {
+ .preenable = bmp_buffer_preenable,
+ .postdisable = bmp_buffer_postdisable,
+};
+
static void bmp280_pm_disable(void *data)
{
struct device *dev = data;
@@ -2281,6 +2580,7 @@ int bmp280_common_probe(struct device *dev,
/* Apply initial values from chip info structure */
indio_dev->channels = chip_info->channels;
indio_dev->num_channels = chip_info->num_channels;
+ indio_dev->available_scan_masks = chip_info->avail_scan_masks;
data->oversampling_press = chip_info->oversampling_press_default;
data->oversampling_humid = chip_info->oversampling_humid_default;
data->oversampling_temp = chip_info->oversampling_temp_default;
@@ -2366,6 +2666,14 @@ int bmp280_common_probe(struct device *dev,
"failed to read calibration coefficients\n");
}

+ ret = devm_iio_triggered_buffer_setup(data->dev, indio_dev,
+ iio_pollfunc_store_time,
+ data->chip_info->buffer_handler,
+ NULL);
+ if (ret)
+ return dev_err_probe(data->dev, ret,
+ "iio triggered buffer setup failed\n");
+
/*
* Attempt to grab an optional EOC IRQ - only the BMP085 has this
* however as it happens, the BMP085 shares the chip ID of BMP180
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index ce92f283e142..29b094b17c84 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -40,14 +40,10 @@ static int bmp380_regmap_spi_read(void *context, const void *reg,
size_t reg_size, void *val, size_t val_size)
{
struct spi_device *spi = to_spi_device(context);
- u8 rx_buf[4];
+ u8 rx_buf[BMP280_BURST_READ_BYTES + 1];
ssize_t status;

- /*
- * Maximum number of consecutive bytes read for a temperature or
- * pressure measurement is 3.
- */
- if (val_size > 3)
+ if (val_size > BMP280_BURST_READ_BYTES)
return -EINVAL;

/*
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index ccba779d7a83..0373d5f9b9a8 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -301,6 +301,16 @@
#define BMP280_PRESS_SKIPPED 0x80000
#define BMP280_HUMIDITY_SKIPPED 0x8000

+/* Number of bytes for each value */
+#define BMP280_NUM_PRESS_BYTES 3
+#define BMP280_NUM_TEMP_BYTES 3
+#define BME280_NUM_HUMIDITY_BYTES 2
+#define BMP280_BURST_READ_BYTES (BMP280_NUM_PRESS_BYTES + \
+ BMP280_NUM_TEMP_BYTES)
+#define BME280_BURST_READ_BYTES (BMP280_NUM_PRESS_BYTES + \
+ BMP280_NUM_TEMP_BYTES + \
+ BME280_NUM_HUMIDITY_BYTES)
+
/* Core exported structs */

static const char *const bmp280_supply_names[] = {
@@ -394,13 +404,19 @@ struct bmp280_data {
*/
int sampling_freq;

+ /*
+ * Data to push to userspace triggered buffer. Up to 3 channels and
+ * s64 timestamp, aligned.
+ */
+ s32 sensor_data[5] __aligned(8);
+
/*
* DMA (thus cache coherency maintenance) may require the
* transfer buffers to live in their own cache lines.
*/
union {
/* Sensor data buffer */
- u8 buf[3];
+ u8 buf[BMP280_BURST_READ_BYTES];
/* Calibration data buffers */
__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
@@ -422,6 +438,7 @@ struct bmp280_chip_info {
const struct iio_chan_spec *channels;
int num_channels;
unsigned int start_up_time;
+ const unsigned long *avail_scan_masks;

const int *oversampling_temp_avail;
int num_oversampling_temp_avail;
@@ -456,6 +473,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);
+
+ irqreturn_t (*buffer_handler)(int irq, void *p);
};

/* Chip infos for each variant */
--
2.25.1


2024-04-07 21:51:36

by Angel Iglesias

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] Driver cleanup and series to add triggered buffer

On Sun, 2024-04-07 at 19:29 +0200, Vasileios Amoiridis wrote:
> Based on next-20240405.

Hi there! Small tip, with git's format-patch, you can include this information
for the people applying the patches with the argument --base=<commit or branch>.
This will point to git the base commit from which the series build the
changeset.

Kind regards,
Angel



2024-04-08 16:19:35

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] Driver cleanup and series to add triggered buffer

On Sun, Apr 07, 2024 at 11:51:22PM +0200, Angel Iglesias wrote:
> On Sun, 2024-04-07 at 19:29 +0200, Vasileios Amoiridis wrote:
> > Based on next-20240405.
>
> Hi there! Small tip, with git's format-patch, you can include this information
> for the people applying the patches with the argument --base=<commit or branch>.
> This will point to git the base commit from which the series build the
> changeset.

Moreover, CIs will be able to properly test it.

--
With Best Regards,
Andy Shevchenko



2024-04-08 17:28:00

by Vasileios Amoiridis

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] Driver cleanup and series to add triggered buffer

On Mon, Apr 08, 2024 at 06:01:55PM +0300, Andy Shevchenko wrote:
> On Sun, Apr 07, 2024 at 11:51:22PM +0200, Angel Iglesias wrote:
> > On Sun, 2024-04-07 at 19:29 +0200, Vasileios Amoiridis wrote:
> > > Based on next-20240405.
> >
> > Hi there! Small tip, with git's format-patch, you can include this information
> > for the people applying the patches with the argument --base=<commit or branch>.
> > This will point to git the base commit from which the series build the
> > changeset.
>
> Moreover, CIs will be able to properly test it.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Hi guys!

Thank you very much for the info, I will use it for the next version!

Cheers,
Vasilis

2024-04-13 16:53:22

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] iio: pressure: bmp280: Various driver cleanups

On Sun, 7 Apr 2024 19:29:15 +0200
Vasileios Amoiridis <[email protected]> wrote:

> Various driver cleanups including:
>
Not sure how we got to a v4 with a patch title various.

If you have to list multiple changes, it should normally
be multiple patches.

White space can all be grouped, but the others should be separate.
Please break it up for v5.

I'll take a look at the actual changes even though I won't merge
a 'various' patch like this.

I may well miss things because there is simply too much in here
and some of the diffs are subtle as it can be hard to spot
if it's a name change or a functional change.

> a) change names of functions that are used by all sensors from
> bmp280_read_raw_* to bmp_read_raw_* to make it more clear that
> these functions are general and not tied to the BMP280 sensor.
Don't. Convention is to naming such function after the first supported
part. We've tried generic naming in the past and often becomes even
less clear. Already you have bmp_x functions applying to bme devices.
Sooner or later you will have them applying to an xyz280 where none
of the letter matter.
>


> b) modify some defines that are used only by the BME280 sensor
> to use the naming convention BME280_* and not BMP280_*.

This is fine, but also move them so they aren't in blocks labeled
BMP280 specific registers.

>
> c) add various error messages that were not implemented.
Also fine in a patch on their own.
>
> d) sort headers.
Separate patch and this is probably fine.

>
> e) fix various indentation errors which were found by checkpatch.pl.

White space fixes always belong in a patch that does nothing else.

>
> g) Add identifier names in function definitions which were warned
> by checkpatch.pl.
This is fine, but again not in a patch making other changes.

I want to be reading a patch whilst just looking at one type of thing.
It is much quicker to review 6 single purpose patches than 1 patch
combining all 6.


Jonathan
>
> Signed-off-by: Vasileios Amoiridis <[email protected]>
> ---
> drivers/iio/pressure/bmp280-core.c | 244 ++++++++++++++-------------
> drivers/iio/pressure/bmp280-i2c.c | 2 +-
> drivers/iio/pressure/bmp280-regmap.c | 8 +-
> drivers/iio/pressure/bmp280-spi.c | 8 +-
> drivers/iio/pressure/bmp280.h | 50 +++---
> 5 files changed, 159 insertions(+), 153 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 09f53d987c7d..1c51139cbfcf 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -52,7 +52,6 @@
> */
> enum { AC1, AC2, AC3, AC4, AC5, AC6, B1, B2, MB, MC, MD };
>
> -
> enum bmp380_odr {
> BMP380_ODR_200HZ,
> BMP380_ODR_100HZ,
> @@ -71,7 +70,7 @@ enum bmp380_odr {
> BMP380_ODR_0_01HZ,
> BMP380_ODR_0_006HZ,
> BMP380_ODR_0_003HZ,
> - BMP380_ODR_0_0015HZ,
> + BMP380_ODR_0_0015HZ

Why? We remove the comma when the last element is clearly a terminator, not
because it happens to be the last element. This isn't NULL, or _COUNT or similar
which must always come at the end.


> };
>
> enum bmp580_odr {
> @@ -106,7 +105,7 @@ enum bmp580_odr {
> BMP580_ODR_1HZ,
> BMP580_ODR_0_5HZ,
> BMP580_ODR_0_25HZ,
> - BMP580_ODR_0_125HZ,
> + BMP580_ODR_0_125HZ

As above, I can't see a reason to change this.

> };
>
> /*
> @@ -131,7 +130,7 @@ enum {
> BMP380_P8 = 16,
> BMP380_P9 = 17,
> BMP380_P10 = 19,
> - BMP380_P11 = 20,
> + BMP380_P11 = 20
and again.

> };
>
> static const struct iio_chan_spec bmp280_channels[] = {
> @@ -181,11 +180,10 @@ static int bmp280_read_calib(struct bmp280_data *data)
> struct bmp280_calib *calib = &data->calib.bmp280;
> int ret;
>
> -
> /* Read temperature and pressure calibration values. */
> ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> - if (ret < 0) {
> + if (ret) {
> dev_err(data->dev,
> "failed to read temperature and pressure calibration parameters\n");
> return ret;
> @@ -222,7 +220,7 @@ static int bme280_read_calib(struct bmp280_data *data)
>
> /* Load shared calibration params with bmp280 first */
> ret = bmp280_read_calib(data);
> - if (ret < 0) {
> + if (ret) {
> dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> return ret;
> }
> @@ -235,47 +233,47 @@ static int bme280_read_calib(struct bmp280_data *data)
> * Humidity data is only available on BME280.
> */
>
> - ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
> - if (ret < 0) {
> + ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
> + if (ret) {
> dev_err(dev, "failed to read H1 comp value\n");
> return ret;
> }
> calib->H1 = tmp;
>
> - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2,
> + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
> &data->le16, sizeof(data->le16));
> - if (ret < 0) {
> + if (ret) {
> dev_err(dev, "failed to read H2 comp value\n");
> return ret;
> }
> calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
>
> - ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
> - if (ret < 0) {
> + ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
> + if (ret) {
> dev_err(dev, "failed to read H3 comp value\n");
> return ret;
> }
> calib->H3 = tmp;
>
> - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4,
> + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
> &data->be16, sizeof(data->be16));
> - if (ret < 0) {
> + if (ret) {
> dev_err(dev, "failed to read H4 comp value\n");
> return ret;
> }
> calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
> (be16_to_cpu(data->be16) & 0xf), 11);
>
> - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5,
> + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
> &data->le16, sizeof(data->le16));
> - if (ret < 0) {
> + if (ret) {
> dev_err(dev, "failed to read H5 comp value\n");
> return ret;
> }
> - calib->H5 = sign_extend32(FIELD_GET(BMP280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> + calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
>
> - ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
> - if (ret < 0) {
> + ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
> + if (ret) {
> dev_err(dev, "failed to read H6 comp value\n");
> return ret;
> }
> @@ -283,14 +281,14 @@ static int bme280_read_calib(struct bmp280_data *data)
>
> return 0;
> }
> +
> /*
> * Returns humidity in percent, resolution is 0.01 percent. Output value of
> * "47445" represents 47445/1024 = 46.333 %RH.
> *
> * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> */
> -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> - s32 adc_humidity)
> +static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity)
> {
> struct bmp280_calib *calib = &data->calib.bmp280;
> s32 var;
> @@ -305,7 +303,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> var = clamp_val(var, 0, 419430400);
>
> return var >> 12;
> -};
> +}
>
> /*
> * Returns temperature in DegC, resolution is 0.01 DegC. Output value of
> @@ -314,8 +312,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> *
> * Taken from datasheet, Section 3.11.3, "Compensation formula".
> */
> -static s32 bmp280_compensate_temp(struct bmp280_data *data,
> - s32 adc_temp)
> +static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
> {
> struct bmp280_calib *calib = &data->calib.bmp280;
> s32 var1, var2;
> @@ -337,8 +334,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> *
> * Taken from datasheet, Section 3.11.3, "Compensation formula".
> */
> -static u32 bmp280_compensate_press(struct bmp280_data *data,
> - s32 adc_press)
> +static u32 bmp280_compensate_press(struct bmp280_data *data, s32 adc_press)
> {
> struct bmp280_calib *calib = &data->calib.bmp280;
> s64 var1, var2, p;
> @@ -363,15 +359,14 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
> return (u32)p;
> }
>
> -static int bmp280_read_temp(struct bmp280_data *data,
> - int *val, int *val2)
> +static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
> {
> s32 adc_temp, comp_temp;
> int ret;
>
> ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
> data->buf, sizeof(data->buf));
> - if (ret < 0) {
> + if (ret) {
> dev_err(data->dev, "failed to read temperature\n");
> return ret;
> }
> @@ -396,8 +391,7 @@ static int bmp280_read_temp(struct bmp280_data *data,
> return 0;
> }
>
> -static int bmp280_read_press(struct bmp280_data *data,
> - int *val, int *val2)
> +static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
> {
> u32 comp_press;
> s32 adc_press;
> @@ -405,12 +399,12 @@ static int bmp280_read_press(struct bmp280_data *data,
>
> /* Read and compensate temperature so we get a reading of t_fine. */
> ret = bmp280_read_temp(data, NULL, NULL);
> - if (ret < 0)
> + if (ret)
> return ret;
>
> ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> data->buf, sizeof(data->buf));
> - if (ret < 0) {
> + if (ret) {
> dev_err(data->dev, "failed to read pressure\n");
> return ret;
> }
> @@ -429,7 +423,7 @@ static int bmp280_read_press(struct bmp280_data *data,
> return IIO_VAL_FRACTIONAL;
> }
>
> -static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> +static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> {
> u32 comp_humidity;
> s32 adc_humidity;
> @@ -437,12 +431,12 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
>
> /* Read and compensate temperature so we get a reading of t_fine. */
> ret = bmp280_read_temp(data, NULL, NULL);
> - if (ret < 0)
> + if (ret)
> return ret;
>
> - ret = regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB,
> + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> &data->be16, sizeof(data->be16));
> - if (ret < 0) {
> + if (ret) {
> dev_err(data->dev, "failed to read humidity\n");
> return ret;
> }
> @@ -453,16 +447,16 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> dev_err(data->dev, "reading humidity skipped\n");
> return -EIO;
> }
> - comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> + comp_humidity = bme280_compensate_humidity(data, adc_humidity);
>
> *val = comp_humidity * 1000 / 1024;
>
> return IIO_VAL_INT;
> }
>
> -static int bmp280_read_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int *val, int *val2, long mask)
> +static int bmp_read_raw(struct iio_dev *indio_dev,

No to this sort of change. bmp280_ is the prefix for the driver - it doesn't
mean that it applies only to that part. As such it is the prefix
we should use throughout the driver unless a function is specific
to a different part. bmp is too generic and may cause namespace issues
like a clash with something in a header at somepoint in future.

> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index 5812a344ed8e..ea8eb5691428 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -1,10 +1,10 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> #include <linux/bitops.h>
> #include <linux/device.h>
> -#include <linux/iio/iio.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
>
> +#include <linux/iio/iio.h>
>
> /* BMP580 specific registers */
> #define BMP580_REG_CMD 0x7E
> @@ -192,8 +192,8 @@
> #define BMP380_PRESS_SKIPPED 0x800000
>
> /* BMP280 specific registers */
> -#define BMP280_REG_HUMIDITY_LSB 0xFE
> -#define BMP280_REG_HUMIDITY_MSB 0xFD
> +#define BME280_REG_HUMIDITY_LSB 0xFE
> +#define BME280_REG_HUMIDITY_MSB 0xFD
They are in a block called BMP280 specific registers why
are they prefixed with BME280?

If they don't apply to the BMP280 add a new block with
a comment to say BME280 specific registers.


> #define BMP280_REG_TEMP_XLSB 0xFC
> #define BMP280_REG_TEMP_LSB 0xFB
> #define BMP280_REG_TEMP_MSB 0xFA
> @@ -207,15 +207,15 @@
> #define BMP280_REG_CONFIG 0xF5
> #define BMP280_REG_CTRL_MEAS 0xF4
> #define BMP280_REG_STATUS 0xF3
> -#define BMP280_REG_CTRL_HUMIDITY 0xF2
> +#define BME280_REG_CTRL_HUMIDITY 0xF2

Jonathan


2024-04-13 16:56:38

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 2/6] iio: pressure: bmp280: Refactorize reading functions

On Sun, 7 Apr 2024 19:29:16 +0200
Vasileios Amoiridis <[email protected]> wrote:

> For BMP18x, BMP28x, BME280, BMP38x the reading of the pressure
> value requires an update of the t_fine variable which happens
> through reading the temperature value.
>
> So all the bmpxxx_read_press() functions of the above sensors
> are internally calling the equivalent bmpxxx_read_temp() function
> in order to update the t_fine value. By just looking at the code
> this functionality is a bit hidden and is not easy to understand
> why those channels are not independent.
>
> This commit tries to clear these things a bit by splitting the
> bmpxxx_{read/compensate}_{temp/press/humid}() to the following:
>
> i. bmpxxx_read_{temp/press/humid}_adc(): read the raw value from
> the sensor.
>
> ii. bmpxx_calc_t_fine(): calculate the t_fine variable.
>
> iii. bmpxxx_get_t_fine(): get the t_fine variable.
>
> iv. bmpxxx_compensate_{temp/press/humid}(): compensate the adc
> values and return the calculated value.
>
> v. bmpxxx_read_{temp/press/humid}(): combine calls of the
> aforementioned functions to return the requested value.
>
> Suggested-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Vasileios Amoiridis <[email protected]>
Looks good to me. Thanks for tidying this up.


2024-04-13 16:58:50

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 3/6] iio: pressure: bmp280: Introduce new cleanup routines

On Sun, 7 Apr 2024 19:29:17 +0200
Vasileios Amoiridis <[email protected]> wrote:

> Introduce new linux/cleanup.h with the guard(mutex) functionality
> in the {read,write}_raw() functions.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Suggested-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Vasileios Amoiridis <[email protected]>
A could of corners of dead code that you can remove.

In general looks good.

Jonathan

> ---
> drivers/iio/pressure/bmp280-core.c | 129 +++++++++++++----------------
> 1 file changed, 58 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 50bdf79011bc..51bcdf8cede6 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -27,6 +27,7 @@
>
> #include <linux/bitops.h>
> #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> #include <linux/completion.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> @@ -499,77 +500,69 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> return IIO_VAL_INT;
> }
>
> -static int bmp_read_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int *val, int *val2, long mask)
> +static int bmp_read_raw_impl(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> {
> struct bmp280_data *data = iio_priv(indio_dev);
> - int ret;
>
> - pm_runtime_get_sync(data->dev);
> - mutex_lock(&data->lock);
> + guard(mutex)(&data->lock);
>
> switch (mask) {
> case IIO_CHAN_INFO_PROCESSED:
> switch (chan->type) {
> case IIO_HUMIDITYRELATIVE:
> - ret = data->chip_info->read_humid(data, val, val2);
> - break;
> + return data->chip_info->read_humid(data, val, val2);
> case IIO_PRESSURE:
> - ret = data->chip_info->read_press(data, val, val2);
> - break;
> + return data->chip_info->read_press(data, val, val2);
> case IIO_TEMP:
> - ret = data->chip_info->read_temp(data, val, val2);
> - break;
> + return data->chip_info->read_temp(data, val, val2);
> default:
> - ret = -EINVAL;
> - break;
> + return -EINVAL;
> }
> - break;
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> switch (chan->type) {
> case IIO_HUMIDITYRELATIVE:
> *val = 1 << data->oversampling_humid;
> - ret = IIO_VAL_INT;
> - break;
> + return IIO_VAL_INT;
> case IIO_PRESSURE:
> *val = 1 << data->oversampling_press;
> - ret = IIO_VAL_INT;
> - break;
> + return IIO_VAL_INT;
> case IIO_TEMP:
> *val = 1 << data->oversampling_temp;
> - ret = IIO_VAL_INT;
> - break;
> + return IIO_VAL_INT;
> default:
> - ret = -EINVAL;
> - break;
> + return -EINVAL;
> }
> - break;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - if (!data->chip_info->sampling_freq_avail) {
> - ret = -EINVAL;
> - break;
> - }
> + if (!data->chip_info->sampling_freq_avail)
> + return -EINVAL;
>
> *val = data->chip_info->sampling_freq_avail[data->sampling_freq][0];
> *val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1];
> - ret = IIO_VAL_INT_PLUS_MICRO;
> - break;
> + return IIO_VAL_INT_PLUS_MICRO;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> - if (!data->chip_info->iir_filter_coeffs_avail) {
> - ret = -EINVAL;
> - break;
> - }
> + if (!data->chip_info->iir_filter_coeffs_avail)
> + return -EINVAL;
>
> *val = (1 << data->iir_filter_coeff) - 1;
> - ret = IIO_VAL_INT;
> - break;
> + return IIO_VAL_INT;
> default:
> - ret = -EINVAL;
> - break;
> + return -EINVAL;
> }
>
> - mutex_unlock(&data->lock);
> + return 0;
As below. I don't think you can get here. I hope all paths have already returned.

> +}
> +
> +static int bmp_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct bmp280_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + pm_runtime_get_sync(data->dev);
> + ret = bmp_read_raw_impl(indio_dev, chan, val, val2, mask);
> pm_runtime_mark_last_busy(data->dev);
> pm_runtime_put_autosuspend(data->dev);
>
> @@ -697,12 +690,13 @@ static int bmp_write_iir_filter_coeffs(struct bmp280_data *data, int val)
> return -EINVAL;
> }
>
> -static int bmp_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val, int val2, long mask)
> +static int bmp_write_raw_impl(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> {
> struct bmp280_data *data = iio_priv(indio_dev);
> - int ret = 0;
> +
> + guard(mutex)(&data->lock);
>
> /*
> * Helper functions to update sensor running configuration.
> @@ -712,46 +706,39 @@ static int bmp_write_raw(struct iio_dev *indio_dev,
> */
> switch (mask) {
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> - pm_runtime_get_sync(data->dev);
> - mutex_lock(&data->lock);
> switch (chan->type) {
> case IIO_HUMIDITYRELATIVE:
> - ret = bmp_write_oversampling_ratio_humid(data, val);
> - break;
> + return bmp_write_oversampling_ratio_humid(data, val);
> case IIO_PRESSURE:
> - ret = bmp_write_oversampling_ratio_press(data, val);
> - break;
> + return bmp_write_oversampling_ratio_press(data, val);
> case IIO_TEMP:
> - ret = bmp_write_oversampling_ratio_temp(data, val);
> - break;
> + return bmp_write_oversampling_ratio_temp(data, val);
> default:
> - ret = -EINVAL;
> - break;
> + return -EINVAL;
> }
> - mutex_unlock(&data->lock);
> - pm_runtime_mark_last_busy(data->dev);
> - pm_runtime_put_autosuspend(data->dev);
> - break;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - pm_runtime_get_sync(data->dev);
> - mutex_lock(&data->lock);
> - ret = bmp_write_sampling_frequency(data, val, val2);
> - mutex_unlock(&data->lock);
> - pm_runtime_mark_last_busy(data->dev);
> - pm_runtime_put_autosuspend(data->dev);
> - break;
> + return bmp_write_sampling_frequency(data, val, val2);
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> - pm_runtime_get_sync(data->dev);
> - mutex_lock(&data->lock);
> - ret = bmp_write_iir_filter_coeffs(data, val);
> - mutex_unlock(&data->lock);
> - pm_runtime_mark_last_busy(data->dev);
> - pm_runtime_put_autosuspend(data->dev);
> - break;
> + return bmp_write_iir_filter_coeffs(data, val);
> default:
> return -EINVAL;
> }
>
> + return 0;
I'm fairly sure you can't get here so this is dead code. Hene
drop this final return.

> +}

>


2024-04-13 17:07:17

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] iio: pressure: bmp280: Add triggered buffer support

On Sun, 7 Apr 2024 19:29:20 +0200
Vasileios Amoiridis <[email protected]> wrote:

> BMP2xx, BME280, BMP3xx, and BMP5xx use continuous buffers for their
> temperature, pressure and humidity readings. This facilitates the
> use of burst/bulk reads in order to acquire data faster. The
> approach is different from the one used in oneshot captures.
>
> BMP085 & BMP1xx devices use a completely different measurement
> process that is well defined and is used in their buffer_handler().
>
> Suggested-by: Angel Iglesias <[email protected]>
> Signed-off-by: Vasileios Amoiridis <[email protected]>
Hi,

A few things inline.

Jonathan

> ---
> drivers/iio/pressure/Kconfig | 2 +
> drivers/iio/pressure/bmp280-core.c | 338 +++++++++++++++++++++++++++--
> drivers/iio/pressure/bmp280-spi.c | 8 +-
> drivers/iio/pressure/bmp280.h | 21 +-
> 4 files changed, 347 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index 3ad38506028e..0b5406a3f85d 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -31,6 +31,8 @@ config BMP280
> select REGMAP
> select BMP280_I2C if (I2C)
> select BMP280_SPI if (SPI_MASTER)
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> help
> Say yes here to build support for Bosch Sensortec BMP180, BMP280, BMP380
> and BMP580 pressure and temperature sensors. Also supports the BME280 with
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 1b894feb717b..32dd35475826 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -41,7 +41,10 @@

>
>
>
> +static irqreturn_t bmp580_buffer_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct bmp280_data *data = iio_priv(indio_dev);
> + s32 adc_temp, adc_press;
> + int ret;
> +
> + guard(mutex)(&data->lock);
> +
> + /* If humidity channel is enabled it means that we are called for the
> + * BME280 humidity sensor.
Please match IIO multiline comment syntax. Like most of the kernel.

/*
* If ...

> + */
> +



..

> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index ccba779d7a83..0373d5f9b9a8 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -301,6 +301,16 @@

> /* Core exported structs */
>
> static const char *const bmp280_supply_names[] = {
> @@ -394,13 +404,19 @@ struct bmp280_data {
> */
> int sampling_freq;
>
> + /*
> + * Data to push to userspace triggered buffer. Up to 3 channels and
> + * s64 timestamp, aligned.
> + */
> + s32 sensor_data[5] __aligned(8);
For the timestamp to be aligned (which is the reason this is __aligned(8))
when landing at the end of the buffer, the buffer needs to be a multiple of 8
bytes.

3 channel, 32bit padding, s64 timestamp. So [6]

On the todo list is to add runtime checking to the IIO core on these buffers
being big enough which would have caught this, but I haven't gotten started
on that project yet.

Jonathan
> +

2024-04-14 16:19:22

by Vasileios Amoiridis

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] iio: pressure: bmp280: Various driver cleanups

On Sat, Apr 13, 2024 at 05:52:57PM +0100, Jonathan Cameron wrote:
> On Sun, 7 Apr 2024 19:29:15 +0200
> Vasileios Amoiridis <[email protected]> wrote:
>
> > Various driver cleanups including:
> >
> Not sure how we got to a v4 with a patch title various.
>
> If you have to list multiple changes, it should normally
> be multiple patches.
>
> White space can all be grouped, but the others should be separate.
> Please break it up for v5.
>
> I'll take a look at the actual changes even though I won't merge
> a 'various' patch like this.
>
> I may well miss things because there is simply too much in here
> and some of the diffs are subtle as it can be hard to spot
> if it's a name change or a functional change.
>

Hi Jonathan,

Thank you once again for the very valuable feedback! We didn't actually
reached v4 with a title various. It is just that the more that I was
working on this driver the more things I noticed that could be cleaned
(apparently not all of them as you mentioned) but still quite some that
could be cleaned. So I took the opportunity before adding the triggered
buffer support to do this cleaning.

The reason that I did it in a big patch is that in a previous version
of this set of patches, you mentioned that we must find a way to make
the commits less, so that's why I thought that a cleaning patch could
include many different types of cleaning since there are no functional
changes but as I understand that's not the case, so I will split them
for v5.

Cheers,
Vasilis

> > a) change names of functions that are used by all sensors from
> > bmp280_read_raw_* to bmp_read_raw_* to make it more clear that
> > these functions are general and not tied to the BMP280 sensor.
> Don't. Convention is to naming such function after the first supported
> part. We've tried generic naming in the past and often becomes even
> less clear. Already you have bmp_x functions applying to bme devices.
> Sooner or later you will have them applying to an xyz280 where none
> of the letter matter.
> >
>
>
> > b) modify some defines that are used only by the BME280 sensor
> > to use the naming convention BME280_* and not BMP280_*.
>
> This is fine, but also move them so they aren't in blocks labeled
> BMP280 specific registers.
>
> >
> > c) add various error messages that were not implemented.
> Also fine in a patch on their own.
> >
> > d) sort headers.
> Separate patch and this is probably fine.
>
> >
> > e) fix various indentation errors which were found by checkpatch.pl.
>
> White space fixes always belong in a patch that does nothing else.
>
> >
> > g) Add identifier names in function definitions which were warned
> > by checkpatch.pl.
> This is fine, but again not in a patch making other changes.
>
> I want to be reading a patch whilst just looking at one type of thing.
> It is much quicker to review 6 single purpose patches than 1 patch
> combining all 6.
>
>
> Jonathan
> >
> > Signed-off-by: Vasileios Amoiridis <[email protected]>
> > ---
> > drivers/iio/pressure/bmp280-core.c | 244 ++++++++++++++-------------
> > drivers/iio/pressure/bmp280-i2c.c | 2 +-
> > drivers/iio/pressure/bmp280-regmap.c | 8 +-
> > drivers/iio/pressure/bmp280-spi.c | 8 +-
> > drivers/iio/pressure/bmp280.h | 50 +++---
> > 5 files changed, 159 insertions(+), 153 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 09f53d987c7d..1c51139cbfcf 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -52,7 +52,6 @@
> > */
> > enum { AC1, AC2, AC3, AC4, AC5, AC6, B1, B2, MB, MC, MD };
> >
> > -
> > enum bmp380_odr {
> > BMP380_ODR_200HZ,
> > BMP380_ODR_100HZ,
> > @@ -71,7 +70,7 @@ enum bmp380_odr {
> > BMP380_ODR_0_01HZ,
> > BMP380_ODR_0_006HZ,
> > BMP380_ODR_0_003HZ,
> > - BMP380_ODR_0_0015HZ,
> > + BMP380_ODR_0_0015HZ
>
> Why? We remove the comma when the last element is clearly a terminator, not
> because it happens to be the last element. This isn't NULL, or _COUNT or similar
> which must always come at the end.
>
>
> > };
> >
> > enum bmp580_odr {
> > @@ -106,7 +105,7 @@ enum bmp580_odr {
> > BMP580_ODR_1HZ,
> > BMP580_ODR_0_5HZ,
> > BMP580_ODR_0_25HZ,
> > - BMP580_ODR_0_125HZ,
> > + BMP580_ODR_0_125HZ
>
> As above, I can't see a reason to change this.
>
> > };
> >
> > /*
> > @@ -131,7 +130,7 @@ enum {
> > BMP380_P8 = 16,
> > BMP380_P9 = 17,
> > BMP380_P10 = 19,
> > - BMP380_P11 = 20,
> > + BMP380_P11 = 20
> and again.
>
> > };
> >
> > static const struct iio_chan_spec bmp280_channels[] = {
> > @@ -181,11 +180,10 @@ static int bmp280_read_calib(struct bmp280_data *data)
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > int ret;
> >
> > -
> > /* Read temperature and pressure calibration values. */
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> > data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev,
> > "failed to read temperature and pressure calibration parameters\n");
> > return ret;
> > @@ -222,7 +220,7 @@ static int bme280_read_calib(struct bmp280_data *data)
> >
> > /* Load shared calibration params with bmp280 first */
> > ret = bmp280_read_calib(data);
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> > return ret;
> > }
> > @@ -235,47 +233,47 @@ static int bme280_read_calib(struct bmp280_data *data)
> > * Humidity data is only available on BME280.
> > */
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H1 comp value\n");
> > return ret;
> > }
> > calib->H1 = tmp;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
> > &data->le16, sizeof(data->le16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H2 comp value\n");
> > return ret;
> > }
> > calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H3 comp value\n");
> > return ret;
> > }
> > calib->H3 = tmp;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
> > &data->be16, sizeof(data->be16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H4 comp value\n");
> > return ret;
> > }
> > calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
> > (be16_to_cpu(data->be16) & 0xf), 11);
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
> > &data->le16, sizeof(data->le16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H5 comp value\n");
> > return ret;
> > }
> > - calib->H5 = sign_extend32(FIELD_GET(BMP280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> > + calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H6 comp value\n");
> > return ret;
> > }
> > @@ -283,14 +281,14 @@ static int bme280_read_calib(struct bmp280_data *data)
> >
> > return 0;
> > }
> > +
> > /*
> > * Returns humidity in percent, resolution is 0.01 percent. Output value of
> > * "47445" represents 47445/1024 = 46.333 %RH.
> > *
> > * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> > */
> > -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > - s32 adc_humidity)
> > +static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s32 var;
> > @@ -305,7 +303,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > var = clamp_val(var, 0, 419430400);
> >
> > return var >> 12;
> > -};
> > +}
> >
> > /*
> > * Returns temperature in DegC, resolution is 0.01 DegC. Output value of
> > @@ -314,8 +312,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > *
> > * Taken from datasheet, Section 3.11.3, "Compensation formula".
> > */
> > -static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > - s32 adc_temp)
> > +static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s32 var1, var2;
> > @@ -337,8 +334,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > *
> > * Taken from datasheet, Section 3.11.3, "Compensation formula".
> > */
> > -static u32 bmp280_compensate_press(struct bmp280_data *data,
> > - s32 adc_press)
> > +static u32 bmp280_compensate_press(struct bmp280_data *data, s32 adc_press)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s64 var1, var2, p;
> > @@ -363,15 +359,14 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
> > return (u32)p;
> > }
> >
> > -static int bmp280_read_temp(struct bmp280_data *data,
> > - int *val, int *val2)
> > +static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
> > {
> > s32 adc_temp, comp_temp;
> > int ret;
> >
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
> > data->buf, sizeof(data->buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read temperature\n");
> > return ret;
> > }
> > @@ -396,8 +391,7 @@ static int bmp280_read_temp(struct bmp280_data *data,
> > return 0;
> > }
> >
> > -static int bmp280_read_press(struct bmp280_data *data,
> > - int *val, int *val2)
> > +static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
> > {
> > u32 comp_press;
> > s32 adc_press;
> > @@ -405,12 +399,12 @@ static int bmp280_read_press(struct bmp280_data *data,
> >
> > /* Read and compensate temperature so we get a reading of t_fine. */
> > ret = bmp280_read_temp(data, NULL, NULL);
> > - if (ret < 0)
> > + if (ret)
> > return ret;
> >
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> > data->buf, sizeof(data->buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read pressure\n");
> > return ret;
> > }
> > @@ -429,7 +423,7 @@ static int bmp280_read_press(struct bmp280_data *data,
> > return IIO_VAL_FRACTIONAL;
> > }
> >
> > -static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > +static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > {
> > u32 comp_humidity;
> > s32 adc_humidity;
> > @@ -437,12 +431,12 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> >
> > /* Read and compensate temperature so we get a reading of t_fine. */
> > ret = bmp280_read_temp(data, NULL, NULL);
> > - if (ret < 0)
> > + if (ret)
> > return ret;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > &data->be16, sizeof(data->be16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read humidity\n");
> > return ret;
> > }
> > @@ -453,16 +447,16 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > dev_err(data->dev, "reading humidity skipped\n");
> > return -EIO;
> > }
> > - comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> > + comp_humidity = bme280_compensate_humidity(data, adc_humidity);
> >
> > *val = comp_humidity * 1000 / 1024;
> >
> > return IIO_VAL_INT;
> > }
> >
> > -static int bmp280_read_raw(struct iio_dev *indio_dev,
> > - struct iio_chan_spec const *chan,
> > - int *val, int *val2, long mask)
> > +static int bmp_read_raw(struct iio_dev *indio_dev,
>
> No to this sort of change. bmp280_ is the prefix for the driver - it doesn't
> mean that it applies only to that part. As such it is the prefix
> we should use throughout the driver unless a function is specific
> to a different part. bmp is too generic and may cause namespace issues
> like a clash with something in a header at somepoint in future.
>
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index 5812a344ed8e..ea8eb5691428 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -1,10 +1,10 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > #include <linux/bitops.h>
> > #include <linux/device.h>
> > -#include <linux/iio/iio.h>
> > #include <linux/regmap.h>
> > #include <linux/regulator/consumer.h>
> >
> > +#include <linux/iio/iio.h>
> >
> > /* BMP580 specific registers */
> > #define BMP580_REG_CMD 0x7E
> > @@ -192,8 +192,8 @@
> > #define BMP380_PRESS_SKIPPED 0x800000
> >
> > /* BMP280 specific registers */
> > -#define BMP280_REG_HUMIDITY_LSB 0xFE
> > -#define BMP280_REG_HUMIDITY_MSB 0xFD
> > +#define BME280_REG_HUMIDITY_LSB 0xFE
> > +#define BME280_REG_HUMIDITY_MSB 0xFD
> They are in a block called BMP280 specific registers why
> are they prefixed with BME280?
>
> If they don't apply to the BMP280 add a new block with
> a comment to say BME280 specific registers.
>
>
> > #define BMP280_REG_TEMP_XLSB 0xFC
> > #define BMP280_REG_TEMP_LSB 0xFB
> > #define BMP280_REG_TEMP_MSB 0xFA
> > @@ -207,15 +207,15 @@
> > #define BMP280_REG_CONFIG 0xF5
> > #define BMP280_REG_CTRL_MEAS 0xF4
> > #define BMP280_REG_STATUS 0xF3
> > -#define BMP280_REG_CTRL_HUMIDITY 0xF2
> > +#define BME280_REG_CTRL_HUMIDITY 0xF2
>
> Jonathan
>

2024-04-14 23:41:33

by Vasileios Amoiridis

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] iio: pressure: bmp280: Various driver cleanups

On Sat, Apr 13, 2024 at 05:52:57PM +0100, Jonathan Cameron wrote:
> On Sun, 7 Apr 2024 19:29:15 +0200
> Vasileios Amoiridis <[email protected]> wrote:
>
> > Various driver cleanups including:
> >
> Not sure how we got to a v4 with a patch title various.
>
> If you have to list multiple changes, it should normally
> be multiple patches.
>
> White space can all be grouped, but the others should be separate.
> Please break it up for v5.
>
> I'll take a look at the actual changes even though I won't merge
> a 'various' patch like this.
>
> I may well miss things because there is simply too much in here
> and some of the diffs are subtle as it can be hard to spot
> if it's a name change or a functional change.
>
> > a) change names of functions that are used by all sensors from
> > bmp280_read_raw_* to bmp_read_raw_* to make it more clear that
> > these functions are general and not tied to the BMP280 sensor.
> Don't. Convention is to naming such function after the first supported
> part. We've tried generic naming in the past and often becomes even
> less clear. Already you have bmp_x functions applying to bme devices.
> Sooner or later you will have them applying to an xyz280 where none
> of the letter matter.
> >
>
>
> > b) modify some defines that are used only by the BME280 sensor
> > to use the naming convention BME280_* and not BMP280_*.
>
> This is fine, but also move them so they aren't in blocks labeled
> BMP280 specific registers.
>
> >
> > c) add various error messages that were not implemented.
> Also fine in a patch on their own.
> >

While reflecting a bit on that, I would like to ask the following:

In case of regmap_{bulk/}_{read/write/update}() functions, they always
return 0 on success and negative errno in error cases. This driver is
a bit incosistent with many cases checking for returned errors with
the following 2 forms:

a) if (ret) {
...
}

b) if (ret < 0) {
...
}

Which one is prefered in general?

Cheers,
Vasilis
> > d) sort headers.
> Separate patch and this is probably fine.
>
> >
> > e) fix various indentation errors which were found by checkpatch.pl.
>
> White space fixes always belong in a patch that does nothing else.
>
> >
> > g) Add identifier names in function definitions which were warned
> > by checkpatch.pl.
> This is fine, but again not in a patch making other changes.
>
> I want to be reading a patch whilst just looking at one type of thing.
> It is much quicker to review 6 single purpose patches than 1 patch
> combining all 6.
>
>
> Jonathan
> >
> > Signed-off-by: Vasileios Amoiridis <[email protected]>
> > ---
> > drivers/iio/pressure/bmp280-core.c | 244 ++++++++++++++-------------
> > drivers/iio/pressure/bmp280-i2c.c | 2 +-
> > drivers/iio/pressure/bmp280-regmap.c | 8 +-
> > drivers/iio/pressure/bmp280-spi.c | 8 +-
> > drivers/iio/pressure/bmp280.h | 50 +++---
> > 5 files changed, 159 insertions(+), 153 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 09f53d987c7d..1c51139cbfcf 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -52,7 +52,6 @@
> > */
> > enum { AC1, AC2, AC3, AC4, AC5, AC6, B1, B2, MB, MC, MD };
> >
> > -
> > enum bmp380_odr {
> > BMP380_ODR_200HZ,
> > BMP380_ODR_100HZ,
> > @@ -71,7 +70,7 @@ enum bmp380_odr {
> > BMP380_ODR_0_01HZ,
> > BMP380_ODR_0_006HZ,
> > BMP380_ODR_0_003HZ,
> > - BMP380_ODR_0_0015HZ,
> > + BMP380_ODR_0_0015HZ
>
> Why? We remove the comma when the last element is clearly a terminator, not
> because it happens to be the last element. This isn't NULL, or _COUNT or similar
> which must always come at the end.
>
>
> > };
> >
> > enum bmp580_odr {
> > @@ -106,7 +105,7 @@ enum bmp580_odr {
> > BMP580_ODR_1HZ,
> > BMP580_ODR_0_5HZ,
> > BMP580_ODR_0_25HZ,
> > - BMP580_ODR_0_125HZ,
> > + BMP580_ODR_0_125HZ
>
> As above, I can't see a reason to change this.
>
> > };
> >
> > /*
> > @@ -131,7 +130,7 @@ enum {
> > BMP380_P8 = 16,
> > BMP380_P9 = 17,
> > BMP380_P10 = 19,
> > - BMP380_P11 = 20,
> > + BMP380_P11 = 20
> and again.
>
> > };
> >
> > static const struct iio_chan_spec bmp280_channels[] = {
> > @@ -181,11 +180,10 @@ static int bmp280_read_calib(struct bmp280_data *data)
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > int ret;
> >
> > -
> > /* Read temperature and pressure calibration values. */
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> > data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev,
> > "failed to read temperature and pressure calibration parameters\n");
> > return ret;
> > @@ -222,7 +220,7 @@ static int bme280_read_calib(struct bmp280_data *data)
> >
> > /* Load shared calibration params with bmp280 first */
> > ret = bmp280_read_calib(data);
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> > return ret;
> > }
> > @@ -235,47 +233,47 @@ static int bme280_read_calib(struct bmp280_data *data)
> > * Humidity data is only available on BME280.
> > */
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H1 comp value\n");
> > return ret;
> > }
> > calib->H1 = tmp;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
> > &data->le16, sizeof(data->le16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H2 comp value\n");
> > return ret;
> > }
> > calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H3 comp value\n");
> > return ret;
> > }
> > calib->H3 = tmp;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
> > &data->be16, sizeof(data->be16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H4 comp value\n");
> > return ret;
> > }
> > calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
> > (be16_to_cpu(data->be16) & 0xf), 11);
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
> > &data->le16, sizeof(data->le16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(dev, "failed to read H5 comp value\n");
> > return ret;
> > }
> > - calib->H5 = sign_extend32(FIELD_GET(BMP280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> > + calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> >
> > - ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
> > - if (ret < 0) {
> > + ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
> > + if (ret) {
> > dev_err(dev, "failed to read H6 comp value\n");
> > return ret;
> > }
> > @@ -283,14 +281,14 @@ static int bme280_read_calib(struct bmp280_data *data)
> >
> > return 0;
> > }
> > +
> > /*
> > * Returns humidity in percent, resolution is 0.01 percent. Output value of
> > * "47445" represents 47445/1024 = 46.333 %RH.
> > *
> > * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> > */
> > -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > - s32 adc_humidity)
> > +static u32 bme280_compensate_humidity(struct bmp280_data *data, s32 adc_humidity)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s32 var;
> > @@ -305,7 +303,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > var = clamp_val(var, 0, 419430400);
> >
> > return var >> 12;
> > -};
> > +}
> >
> > /*
> > * Returns temperature in DegC, resolution is 0.01 DegC. Output value of
> > @@ -314,8 +312,7 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> > *
> > * Taken from datasheet, Section 3.11.3, "Compensation formula".
> > */
> > -static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > - s32 adc_temp)
> > +static s32 bmp280_compensate_temp(struct bmp280_data *data, s32 adc_temp)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s32 var1, var2;
> > @@ -337,8 +334,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > *
> > * Taken from datasheet, Section 3.11.3, "Compensation formula".
> > */
> > -static u32 bmp280_compensate_press(struct bmp280_data *data,
> > - s32 adc_press)
> > +static u32 bmp280_compensate_press(struct bmp280_data *data, s32 adc_press)
> > {
> > struct bmp280_calib *calib = &data->calib.bmp280;
> > s64 var1, var2, p;
> > @@ -363,15 +359,14 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
> > return (u32)p;
> > }
> >
> > -static int bmp280_read_temp(struct bmp280_data *data,
> > - int *val, int *val2)
> > +static int bmp280_read_temp(struct bmp280_data *data, int *val, int *val2)
> > {
> > s32 adc_temp, comp_temp;
> > int ret;
> >
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
> > data->buf, sizeof(data->buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read temperature\n");
> > return ret;
> > }
> > @@ -396,8 +391,7 @@ static int bmp280_read_temp(struct bmp280_data *data,
> > return 0;
> > }
> >
> > -static int bmp280_read_press(struct bmp280_data *data,
> > - int *val, int *val2)
> > +static int bmp280_read_press(struct bmp280_data *data, int *val, int *val2)
> > {
> > u32 comp_press;
> > s32 adc_press;
> > @@ -405,12 +399,12 @@ static int bmp280_read_press(struct bmp280_data *data,
> >
> > /* Read and compensate temperature so we get a reading of t_fine. */
> > ret = bmp280_read_temp(data, NULL, NULL);
> > - if (ret < 0)
> > + if (ret)
> > return ret;
> >
> > ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> > data->buf, sizeof(data->buf));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read pressure\n");
> > return ret;
> > }
> > @@ -429,7 +423,7 @@ static int bmp280_read_press(struct bmp280_data *data,
> > return IIO_VAL_FRACTIONAL;
> > }
> >
> > -static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > +static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > {
> > u32 comp_humidity;
> > s32 adc_humidity;
> > @@ -437,12 +431,12 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> >
> > /* Read and compensate temperature so we get a reading of t_fine. */
> > ret = bmp280_read_temp(data, NULL, NULL);
> > - if (ret < 0)
> > + if (ret)
> > return ret;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB,
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > &data->be16, sizeof(data->be16));
> > - if (ret < 0) {
> > + if (ret) {
> > dev_err(data->dev, "failed to read humidity\n");
> > return ret;
> > }
> > @@ -453,16 +447,16 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> > dev_err(data->dev, "reading humidity skipped\n");
> > return -EIO;
> > }
> > - comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> > + comp_humidity = bme280_compensate_humidity(data, adc_humidity);
> >
> > *val = comp_humidity * 1000 / 1024;
> >
> > return IIO_VAL_INT;
> > }
> >
> > -static int bmp280_read_raw(struct iio_dev *indio_dev,
> > - struct iio_chan_spec const *chan,
> > - int *val, int *val2, long mask)
> > +static int bmp_read_raw(struct iio_dev *indio_dev,
>
> No to this sort of change. bmp280_ is the prefix for the driver - it doesn't
> mean that it applies only to that part. As such it is the prefix
> we should use throughout the driver unless a function is specific
> to a different part. bmp is too generic and may cause namespace issues
> like a clash with something in a header at somepoint in future.
>
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index 5812a344ed8e..ea8eb5691428 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -1,10 +1,10 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > #include <linux/bitops.h>
> > #include <linux/device.h>
> > -#include <linux/iio/iio.h>
> > #include <linux/regmap.h>
> > #include <linux/regulator/consumer.h>
> >
> > +#include <linux/iio/iio.h>
> >
> > /* BMP580 specific registers */
> > #define BMP580_REG_CMD 0x7E
> > @@ -192,8 +192,8 @@
> > #define BMP380_PRESS_SKIPPED 0x800000
> >
> > /* BMP280 specific registers */
> > -#define BMP280_REG_HUMIDITY_LSB 0xFE
> > -#define BMP280_REG_HUMIDITY_MSB 0xFD
> > +#define BME280_REG_HUMIDITY_LSB 0xFE
> > +#define BME280_REG_HUMIDITY_MSB 0xFD
> They are in a block called BMP280 specific registers why
> are they prefixed with BME280?
>
> If they don't apply to the BMP280 add a new block with
> a comment to say BME280 specific registers.
>
>
> > #define BMP280_REG_TEMP_XLSB 0xFC
> > #define BMP280_REG_TEMP_LSB 0xFB
> > #define BMP280_REG_TEMP_MSB 0xFA
> > @@ -207,15 +207,15 @@
> > #define BMP280_REG_CONFIG 0xF5
> > #define BMP280_REG_CTRL_MEAS 0xF4
> > #define BMP280_REG_STATUS 0xF3
> > -#define BMP280_REG_CTRL_HUMIDITY 0xF2
> > +#define BME280_REG_CTRL_HUMIDITY 0xF2
>
> Jonathan
>