* [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 18:51 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 02/10] iio: pressure: bmp280: Use BME prefix for BME280 specifics Vasileios Amoiridis
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
Fix indentations that are not following the standards, remove
extra white lines and add missing white lines.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 108 ++++++++++++++++-------------
drivers/iio/pressure/bmp280-spi.c | 4 +-
2 files changed, 61 insertions(+), 51 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 09f53d987c7d..1a3241a41768 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,
@@ -181,18 +180,19 @@ 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));
+ data->bmp280_cal_buf,
+ sizeof(data->bmp280_cal_buf));
if (ret < 0) {
dev_err(data->dev,
- "failed to read temperature and pressure calibration parameters\n");
+ "failed to read calibration parameters\n");
return ret;
}
- /* Toss the temperature and pressure calibration data into the entropy pool */
- add_device_randomness(data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
+ /* Toss calibration data into the entropy pool */
+ add_device_randomness(data->bmp280_cal_buf,
+ sizeof(data->bmp280_cal_buf));
/* Parse temperature calibration values. */
calib->T1 = le16_to_cpu(data->bmp280_cal_buf[T1]);
@@ -223,7 +223,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) {
- dev_err(dev, "failed to read common bmp280 calibration parameters\n");
+ dev_err(dev, "failed to read calibration parameters\n");
return ret;
}
@@ -283,6 +283,7 @@ 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.
@@ -305,7 +306,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
@@ -538,7 +539,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
}
static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
- int val)
+ int val)
{
const int *avail = data->chip_info->oversampling_humid_avail;
const int n = data->chip_info->num_oversampling_humid_avail;
@@ -563,7 +564,7 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
}
static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
- int val)
+ int val)
{
const int *avail = data->chip_info->oversampling_temp_avail;
const int n = data->chip_info->num_oversampling_temp_avail;
@@ -588,7 +589,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
}
static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
- int val)
+ int val)
{
const int *avail = data->chip_info->oversampling_press_avail;
const int n = data->chip_info->num_oversampling_press_avail;
@@ -772,13 +773,12 @@ 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);
+ 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");
+ dev_err(data->dev, "failed to write ctrl_meas register\n");
return ret;
}
@@ -786,8 +786,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
BMP280_FILTER_MASK,
BMP280_FILTER_4X);
if (ret < 0) {
- dev_err(data->dev,
- "failed to write config register\n");
+ dev_err(data->dev, "failed to write config register\n");
return ret;
}
@@ -926,8 +925,8 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
}
/*
- * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value of
- * "5123" equals 51.2º C. t_fine carries fine temperature as global value.
+ * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value
+ * of "5123" equals 51.2º C. t_fine carries fine temperature as global value.
*
* Taken from datasheet, Section Appendix 9, "Compensation formula" and repo
* https://github.com/BoschSensortec/BMP3-Sensor-API.
@@ -1069,7 +1068,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
/* Read temperature and pressure calibration data */
ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
- data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
+ data->bmp380_cal_buf,
+ sizeof(data->bmp380_cal_buf));
if (ret) {
dev_err(data->dev,
"failed to read temperature calibration parameters\n");
@@ -1077,7 +1077,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
}
/* Toss the temperature calibration data into the entropy pool */
- add_device_randomness(data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
+ add_device_randomness(data->bmp380_cal_buf,
+ sizeof(data->bmp380_cal_buf));
/* Parse calibration values */
calib->T1 = get_unaligned_le16(&data->bmp380_cal_buf[BMP380_T1]);
@@ -1159,7 +1160,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
/* Configure output data rate */
ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
- BMP380_ODRS_MASK, data->sampling_freq, &aux);
+ BMP380_ODRS_MASK, data->sampling_freq,
+ &aux);
if (ret) {
dev_err(data->dev, "failed to write ODR selection register\n");
return ret;
@@ -1178,12 +1180,13 @@ static int bmp380_chip_config(struct bmp280_data *data)
if (change) {
/*
- * The configurations errors are detected on the fly during a measurement
- * cycle. If the sampling frequency is too low, it's faster to reset
- * the measurement loop than wait until the next measurement is due.
+ * The configurations errors are detected on the fly during a
+ * measurement cycle. If the sampling frequency is too low, it's
+ * faster to reset the measurement loop than wait until the next
+ * measurement is due.
*
- * Resets sensor measurement loop toggling between sleep and normal
- * operating modes.
+ * Resets sensor measurement loop toggling between sleep and
+ * normal operating modes.
*/
ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
BMP380_MODE_MASK,
@@ -1201,22 +1204,21 @@ static int bmp380_chip_config(struct bmp280_data *data)
return ret;
}
/*
- * Waits for measurement before checking configuration error flag.
- * Selected longest measure time indicated in section 3.9.1
- * in the datasheet.
+ * Waits for measurement before checking configuration error
+ * flag. Selected longest measure time indicated in
+ * section 3.9.1 in the datasheet.
*/
msleep(80);
/* Check config error flag */
ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
if (ret) {
- dev_err(data->dev,
- "failed to read error register\n");
+ dev_err(data->dev, "failed to read error register\n");
return ret;
}
if (tmp & BMP380_ERR_CONF_MASK) {
dev_warn(data->dev,
- "sensor flagged configuration as incompatible\n");
+ "sensor flagged configuration as incompatible\n");
return -EINVAL;
}
}
@@ -1317,9 +1319,11 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
}
/* Start NVM operation sequence */
- ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_NVM_OP_SEQ_0);
+ ret = regmap_write(data->regmap, BMP580_REG_CMD,
+ BMP580_CMD_NVM_OP_SEQ_0);
if (ret) {
- dev_err(data->dev, "failed to send nvm operation's first sequence\n");
+ dev_err(data->dev,
+ "failed to send nvm operation's first sequence\n");
return ret;
}
if (is_write) {
@@ -1327,7 +1331,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
ret = regmap_write(data->regmap, BMP580_REG_CMD,
BMP580_CMD_NVM_WRITE_SEQ_1);
if (ret) {
- dev_err(data->dev, "failed to send nvm write sequence\n");
+ dev_err(data->dev,
+ "failed to send nvm write sequence\n");
return ret;
}
/* Datasheet says on 4.8.1.2 it takes approximately 10ms */
@@ -1338,7 +1343,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
ret = regmap_write(data->regmap, BMP580_REG_CMD,
BMP580_CMD_NVM_READ_SEQ_1);
if (ret) {
- dev_err(data->dev, "failed to send nvm read sequence\n");
+ dev_err(data->dev,
+ "failed to send nvm read sequence\n");
return ret;
}
/* Datasheet says on 4.8.1.1 it takes approximately 200us */
@@ -1501,8 +1507,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 +1552,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 +1561,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;
@@ -1662,7 +1669,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
BMP580_OSR_PRESS_EN;
ret = regmap_update_bits_check(data->regmap, BMP580_REG_OSR_CONFIG,
- BMP580_OSR_TEMP_MASK | BMP580_OSR_PRESS_MASK |
+ BMP580_OSR_TEMP_MASK |
+ BMP580_OSR_PRESS_MASK |
BMP580_OSR_PRESS_EN,
reg_val, &aux);
if (ret) {
@@ -1713,7 +1721,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
*/
ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
if (ret) {
- dev_err(data->dev, "error reading effective OSR register\n");
+ dev_err(data->dev,
+ "error reading effective OSR register\n");
return ret;
}
if (!(tmp & BMP580_EFF_OSR_VALID_ODR)) {
@@ -1848,7 +1857,8 @@ static int bmp180_read_calib(struct bmp280_data *data)
}
/* Toss the calibration data into the entropy pool */
- add_device_randomness(data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
+ add_device_randomness(data->bmp180_cal_buf,
+ sizeof(data->bmp180_cal_buf));
calib->AC1 = be16_to_cpu(data->bmp180_cal_buf[AC1]);
calib->AC2 = be16_to_cpu(data->bmp180_cal_buf[AC2]);
@@ -1963,8 +1973,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;
@@ -2241,6 +2250,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-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 4e19ea0b4d39..62b4e58104cf 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -13,7 +13,7 @@
#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);
base-commit: b0a2c79c6f3590b74742cbbc76687014d47972d8
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping
2024-04-29 19:00 ` [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping Vasileios Amoiridis
@ 2024-05-05 18:51 ` Jonathan Cameron
2024-05-06 0:04 ` Vasileios Amoiridis
0 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 18:51 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 29 Apr 2024 21:00:37 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> Fix indentations that are not following the standards, remove
> extra white lines and add missing white lines.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Possibly some reviewers will feel one or two of these are a little over zealous,
but this does both improve things and bring consistency to this code.
I'll pick up some of these cleanups now (maybe the whole set but who knows)
to reduce what is left if we end up with a v6.
Applied this one to the togreg branch of iio.git and pushed out as testing
for 0-day to see if we missed anything,
Thanks,
Jonathan
> ---
> drivers/iio/pressure/bmp280-core.c | 108 ++++++++++++++++-------------
> drivers/iio/pressure/bmp280-spi.c | 4 +-
> 2 files changed, 61 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 09f53d987c7d..1a3241a41768 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,
> @@ -181,18 +180,19 @@ 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));
> + data->bmp280_cal_buf,
> + sizeof(data->bmp280_cal_buf));
> if (ret < 0) {
> dev_err(data->dev,
> - "failed to read temperature and pressure calibration parameters\n");
> + "failed to read calibration parameters\n");
> return ret;
> }
>
> - /* Toss the temperature and pressure calibration data into the entropy pool */
> - add_device_randomness(data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> + /* Toss calibration data into the entropy pool */
> + add_device_randomness(data->bmp280_cal_buf,
> + sizeof(data->bmp280_cal_buf));
>
> /* Parse temperature calibration values. */
> calib->T1 = le16_to_cpu(data->bmp280_cal_buf[T1]);
> @@ -223,7 +223,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) {
> - dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> + dev_err(dev, "failed to read calibration parameters\n");
> return ret;
> }
>
> @@ -283,6 +283,7 @@ 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.
> @@ -305,7 +306,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
> @@ -538,7 +539,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
> }
>
> static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> - int val)
> + int val)
> {
> const int *avail = data->chip_info->oversampling_humid_avail;
> const int n = data->chip_info->num_oversampling_humid_avail;
> @@ -563,7 +564,7 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> }
>
> static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> - int val)
> + int val)
> {
> const int *avail = data->chip_info->oversampling_temp_avail;
> const int n = data->chip_info->num_oversampling_temp_avail;
> @@ -588,7 +589,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> }
>
> static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
> - int val)
> + int val)
> {
> const int *avail = data->chip_info->oversampling_press_avail;
> const int n = data->chip_info->num_oversampling_press_avail;
> @@ -772,13 +773,12 @@ 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);
> + 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");
> + dev_err(data->dev, "failed to write ctrl_meas register\n");
> return ret;
> }
>
> @@ -786,8 +786,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
> BMP280_FILTER_MASK,
> BMP280_FILTER_4X);
> if (ret < 0) {
> - dev_err(data->dev,
> - "failed to write config register\n");
> + dev_err(data->dev, "failed to write config register\n");
> return ret;
> }
>
> @@ -926,8 +925,8 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
> }
>
> /*
> - * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value of
> - * "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> + * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value
> + * of "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> *
> * Taken from datasheet, Section Appendix 9, "Compensation formula" and repo
> * https://github.com/BoschSensortec/BMP3-Sensor-API.
> @@ -1069,7 +1068,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
>
> /* Read temperature and pressure calibration data */
> ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
> - data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> + data->bmp380_cal_buf,
> + sizeof(data->bmp380_cal_buf));
> if (ret) {
> dev_err(data->dev,
> "failed to read temperature calibration parameters\n");
> @@ -1077,7 +1077,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
> }
>
> /* Toss the temperature calibration data into the entropy pool */
> - add_device_randomness(data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> + add_device_randomness(data->bmp380_cal_buf,
> + sizeof(data->bmp380_cal_buf));
>
> /* Parse calibration values */
> calib->T1 = get_unaligned_le16(&data->bmp380_cal_buf[BMP380_T1]);
> @@ -1159,7 +1160,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
>
> /* Configure output data rate */
> ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
> - BMP380_ODRS_MASK, data->sampling_freq, &aux);
> + BMP380_ODRS_MASK, data->sampling_freq,
> + &aux);
> if (ret) {
> dev_err(data->dev, "failed to write ODR selection register\n");
> return ret;
> @@ -1178,12 +1180,13 @@ static int bmp380_chip_config(struct bmp280_data *data)
>
> if (change) {
> /*
> - * The configurations errors are detected on the fly during a measurement
> - * cycle. If the sampling frequency is too low, it's faster to reset
> - * the measurement loop than wait until the next measurement is due.
> + * The configurations errors are detected on the fly during a
> + * measurement cycle. If the sampling frequency is too low, it's
> + * faster to reset the measurement loop than wait until the next
> + * measurement is due.
> *
> - * Resets sensor measurement loop toggling between sleep and normal
> - * operating modes.
> + * Resets sensor measurement loop toggling between sleep and
> + * normal operating modes.
> */
> ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> BMP380_MODE_MASK,
> @@ -1201,22 +1204,21 @@ static int bmp380_chip_config(struct bmp280_data *data)
> return ret;
> }
> /*
> - * Waits for measurement before checking configuration error flag.
> - * Selected longest measure time indicated in section 3.9.1
> - * in the datasheet.
> + * Waits for measurement before checking configuration error
> + * flag. Selected longest measure time indicated in
> + * section 3.9.1 in the datasheet.
> */
> msleep(80);
>
> /* Check config error flag */
> ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
> if (ret) {
> - dev_err(data->dev,
> - "failed to read error register\n");
> + dev_err(data->dev, "failed to read error register\n");
> return ret;
> }
> if (tmp & BMP380_ERR_CONF_MASK) {
> dev_warn(data->dev,
> - "sensor flagged configuration as incompatible\n");
> + "sensor flagged configuration as incompatible\n");
> return -EINVAL;
> }
> }
> @@ -1317,9 +1319,11 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> }
>
> /* Start NVM operation sequence */
> - ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_NVM_OP_SEQ_0);
> + ret = regmap_write(data->regmap, BMP580_REG_CMD,
> + BMP580_CMD_NVM_OP_SEQ_0);
> if (ret) {
> - dev_err(data->dev, "failed to send nvm operation's first sequence\n");
> + dev_err(data->dev,
> + "failed to send nvm operation's first sequence\n");
> return ret;
> }
> if (is_write) {
> @@ -1327,7 +1331,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> ret = regmap_write(data->regmap, BMP580_REG_CMD,
> BMP580_CMD_NVM_WRITE_SEQ_1);
> if (ret) {
> - dev_err(data->dev, "failed to send nvm write sequence\n");
> + dev_err(data->dev,
> + "failed to send nvm write sequence\n");
> return ret;
> }
> /* Datasheet says on 4.8.1.2 it takes approximately 10ms */
> @@ -1338,7 +1343,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> ret = regmap_write(data->regmap, BMP580_REG_CMD,
> BMP580_CMD_NVM_READ_SEQ_1);
> if (ret) {
> - dev_err(data->dev, "failed to send nvm read sequence\n");
> + dev_err(data->dev,
> + "failed to send nvm read sequence\n");
> return ret;
> }
> /* Datasheet says on 4.8.1.1 it takes approximately 200us */
> @@ -1501,8 +1507,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 +1552,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 +1561,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;
> @@ -1662,7 +1669,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> BMP580_OSR_PRESS_EN;
>
> ret = regmap_update_bits_check(data->regmap, BMP580_REG_OSR_CONFIG,
> - BMP580_OSR_TEMP_MASK | BMP580_OSR_PRESS_MASK |
> + BMP580_OSR_TEMP_MASK |
> + BMP580_OSR_PRESS_MASK |
> BMP580_OSR_PRESS_EN,
> reg_val, &aux);
> if (ret) {
> @@ -1713,7 +1721,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> */
> ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
> if (ret) {
> - dev_err(data->dev, "error reading effective OSR register\n");
> + dev_err(data->dev,
> + "error reading effective OSR register\n");
> return ret;
> }
> if (!(tmp & BMP580_EFF_OSR_VALID_ODR)) {
> @@ -1848,7 +1857,8 @@ static int bmp180_read_calib(struct bmp280_data *data)
> }
>
> /* Toss the calibration data into the entropy pool */
> - add_device_randomness(data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
> + add_device_randomness(data->bmp180_cal_buf,
> + sizeof(data->bmp180_cal_buf));
>
> calib->AC1 = be16_to_cpu(data->bmp180_cal_buf[AC1]);
> calib->AC2 = be16_to_cpu(data->bmp180_cal_buf[AC2]);
> @@ -1963,8 +1973,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;
> @@ -2241,6 +2250,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-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index 4e19ea0b4d39..62b4e58104cf 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -13,7 +13,7 @@
> #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);
>
>
> base-commit: b0a2c79c6f3590b74742cbbc76687014d47972d8
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping
2024-05-05 18:51 ` Jonathan Cameron
@ 2024-05-06 0:04 ` Vasileios Amoiridis
2024-05-06 12:38 ` Jonathan Cameron
0 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-05-06 0:04 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Vasileios Amoiridis, lars, andriy.shevchenko, ang.iglesiasg,
mazziesaccount, ak, petre.rodan, phil, 579lpy, linus.walleij,
semen.protsenko, linux-iio, linux-kernel
On Sun, May 05, 2024 at 07:51:55PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:37 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
>
> > Fix indentations that are not following the standards, remove
> > extra white lines and add missing white lines.
> >
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
>
> Possibly some reviewers will feel one or two of these are a little over zealous,
> but this does both improve things and bring consistency to this code.
>
> I'll pick up some of these cleanups now (maybe the whole set but who knows)
> to reduce what is left if we end up with a v6.
>
> Applied this one to the togreg branch of iio.git and pushed out as testing
> for 0-day to see if we missed anything,
>
> Thanks,
>
> Jonathan
>
Hi Jonathan,
I noticed that, another commit for this driver has been accepted here [1],
which will affect some patches of this series that I am adding. It's not
difficult at all to apply changes to my patches with respect to this new
addition. Should I move on with a v6 without taking into consideration
this change, and if everything is finally approved, I can send
later a v7 with only the changes needed for this new commit?
Cheers,
Vasilis
[1] https://lore.kernel.org/linux-iio/043f2be49df3c11152aaf32fc5467ed43fd59faa.camel@gmail.com/T/#t
> > ---
> > drivers/iio/pressure/bmp280-core.c | 108 ++++++++++++++++-------------
> > drivers/iio/pressure/bmp280-spi.c | 4 +-
> > 2 files changed, 61 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 09f53d987c7d..1a3241a41768 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,
> > @@ -181,18 +180,19 @@ 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));
> > + data->bmp280_cal_buf,
> > + sizeof(data->bmp280_cal_buf));
> > if (ret < 0) {
> > dev_err(data->dev,
> > - "failed to read temperature and pressure calibration parameters\n");
> > + "failed to read calibration parameters\n");
> > return ret;
> > }
> >
> > - /* Toss the temperature and pressure calibration data into the entropy pool */
> > - add_device_randomness(data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> > + /* Toss calibration data into the entropy pool */
> > + add_device_randomness(data->bmp280_cal_buf,
> > + sizeof(data->bmp280_cal_buf));
> >
> > /* Parse temperature calibration values. */
> > calib->T1 = le16_to_cpu(data->bmp280_cal_buf[T1]);
> > @@ -223,7 +223,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) {
> > - dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> > + dev_err(dev, "failed to read calibration parameters\n");
> > return ret;
> > }
> >
> > @@ -283,6 +283,7 @@ 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.
> > @@ -305,7 +306,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
> > @@ -538,7 +539,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
> > }
> >
> > static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> > - int val)
> > + int val)
> > {
> > const int *avail = data->chip_info->oversampling_humid_avail;
> > const int n = data->chip_info->num_oversampling_humid_avail;
> > @@ -563,7 +564,7 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> > }
> >
> > static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> > - int val)
> > + int val)
> > {
> > const int *avail = data->chip_info->oversampling_temp_avail;
> > const int n = data->chip_info->num_oversampling_temp_avail;
> > @@ -588,7 +589,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> > }
> >
> > static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
> > - int val)
> > + int val)
> > {
> > const int *avail = data->chip_info->oversampling_press_avail;
> > const int n = data->chip_info->num_oversampling_press_avail;
> > @@ -772,13 +773,12 @@ 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);
> > + 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");
> > + dev_err(data->dev, "failed to write ctrl_meas register\n");
> > return ret;
> > }
> >
> > @@ -786,8 +786,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
> > BMP280_FILTER_MASK,
> > BMP280_FILTER_4X);
> > if (ret < 0) {
> > - dev_err(data->dev,
> > - "failed to write config register\n");
> > + dev_err(data->dev, "failed to write config register\n");
> > return ret;
> > }
> >
> > @@ -926,8 +925,8 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
> > }
> >
> > /*
> > - * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value of
> > - * "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> > + * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value
> > + * of "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> > *
> > * Taken from datasheet, Section Appendix 9, "Compensation formula" and repo
> > * https://github.com/BoschSensortec/BMP3-Sensor-API.
> > @@ -1069,7 +1068,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
> >
> > /* Read temperature and pressure calibration data */
> > ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
> > - data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> > + data->bmp380_cal_buf,
> > + sizeof(data->bmp380_cal_buf));
> > if (ret) {
> > dev_err(data->dev,
> > "failed to read temperature calibration parameters\n");
> > @@ -1077,7 +1077,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
> > }
> >
> > /* Toss the temperature calibration data into the entropy pool */
> > - add_device_randomness(data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> > + add_device_randomness(data->bmp380_cal_buf,
> > + sizeof(data->bmp380_cal_buf));
> >
> > /* Parse calibration values */
> > calib->T1 = get_unaligned_le16(&data->bmp380_cal_buf[BMP380_T1]);
> > @@ -1159,7 +1160,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
> >
> > /* Configure output data rate */
> > ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
> > - BMP380_ODRS_MASK, data->sampling_freq, &aux);
> > + BMP380_ODRS_MASK, data->sampling_freq,
> > + &aux);
> > if (ret) {
> > dev_err(data->dev, "failed to write ODR selection register\n");
> > return ret;
> > @@ -1178,12 +1180,13 @@ static int bmp380_chip_config(struct bmp280_data *data)
> >
> > if (change) {
> > /*
> > - * The configurations errors are detected on the fly during a measurement
> > - * cycle. If the sampling frequency is too low, it's faster to reset
> > - * the measurement loop than wait until the next measurement is due.
> > + * The configurations errors are detected on the fly during a
> > + * measurement cycle. If the sampling frequency is too low, it's
> > + * faster to reset the measurement loop than wait until the next
> > + * measurement is due.
> > *
> > - * Resets sensor measurement loop toggling between sleep and normal
> > - * operating modes.
> > + * Resets sensor measurement loop toggling between sleep and
> > + * normal operating modes.
> > */
> > ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > BMP380_MODE_MASK,
> > @@ -1201,22 +1204,21 @@ static int bmp380_chip_config(struct bmp280_data *data)
> > return ret;
> > }
> > /*
> > - * Waits for measurement before checking configuration error flag.
> > - * Selected longest measure time indicated in section 3.9.1
> > - * in the datasheet.
> > + * Waits for measurement before checking configuration error
> > + * flag. Selected longest measure time indicated in
> > + * section 3.9.1 in the datasheet.
> > */
> > msleep(80);
> >
> > /* Check config error flag */
> > ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
> > if (ret) {
> > - dev_err(data->dev,
> > - "failed to read error register\n");
> > + dev_err(data->dev, "failed to read error register\n");
> > return ret;
> > }
> > if (tmp & BMP380_ERR_CONF_MASK) {
> > dev_warn(data->dev,
> > - "sensor flagged configuration as incompatible\n");
> > + "sensor flagged configuration as incompatible\n");
> > return -EINVAL;
> > }
> > }
> > @@ -1317,9 +1319,11 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > }
> >
> > /* Start NVM operation sequence */
> > - ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_NVM_OP_SEQ_0);
> > + ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > + BMP580_CMD_NVM_OP_SEQ_0);
> > if (ret) {
> > - dev_err(data->dev, "failed to send nvm operation's first sequence\n");
> > + dev_err(data->dev,
> > + "failed to send nvm operation's first sequence\n");
> > return ret;
> > }
> > if (is_write) {
> > @@ -1327,7 +1331,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > BMP580_CMD_NVM_WRITE_SEQ_1);
> > if (ret) {
> > - dev_err(data->dev, "failed to send nvm write sequence\n");
> > + dev_err(data->dev,
> > + "failed to send nvm write sequence\n");
> > return ret;
> > }
> > /* Datasheet says on 4.8.1.2 it takes approximately 10ms */
> > @@ -1338,7 +1343,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > BMP580_CMD_NVM_READ_SEQ_1);
> > if (ret) {
> > - dev_err(data->dev, "failed to send nvm read sequence\n");
> > + dev_err(data->dev,
> > + "failed to send nvm read sequence\n");
> > return ret;
> > }
> > /* Datasheet says on 4.8.1.1 it takes approximately 200us */
> > @@ -1501,8 +1507,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 +1552,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 +1561,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;
> > @@ -1662,7 +1669,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> > BMP580_OSR_PRESS_EN;
> >
> > ret = regmap_update_bits_check(data->regmap, BMP580_REG_OSR_CONFIG,
> > - BMP580_OSR_TEMP_MASK | BMP580_OSR_PRESS_MASK |
> > + BMP580_OSR_TEMP_MASK |
> > + BMP580_OSR_PRESS_MASK |
> > BMP580_OSR_PRESS_EN,
> > reg_val, &aux);
> > if (ret) {
> > @@ -1713,7 +1721,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> > */
> > ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
> > if (ret) {
> > - dev_err(data->dev, "error reading effective OSR register\n");
> > + dev_err(data->dev,
> > + "error reading effective OSR register\n");
> > return ret;
> > }
> > if (!(tmp & BMP580_EFF_OSR_VALID_ODR)) {
> > @@ -1848,7 +1857,8 @@ static int bmp180_read_calib(struct bmp280_data *data)
> > }
> >
> > /* Toss the calibration data into the entropy pool */
> > - add_device_randomness(data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
> > + add_device_randomness(data->bmp180_cal_buf,
> > + sizeof(data->bmp180_cal_buf));
> >
> > calib->AC1 = be16_to_cpu(data->bmp180_cal_buf[AC1]);
> > calib->AC2 = be16_to_cpu(data->bmp180_cal_buf[AC2]);
> > @@ -1963,8 +1973,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;
> > @@ -2241,6 +2250,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-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > index 4e19ea0b4d39..62b4e58104cf 100644
> > --- a/drivers/iio/pressure/bmp280-spi.c
> > +++ b/drivers/iio/pressure/bmp280-spi.c
> > @@ -13,7 +13,7 @@
> > #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);
> >
> >
> > base-commit: b0a2c79c6f3590b74742cbbc76687014d47972d8
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping
2024-05-06 0:04 ` Vasileios Amoiridis
@ 2024-05-06 12:38 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-06 12:38 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 6 May 2024 02:04:52 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> On Sun, May 05, 2024 at 07:51:55PM +0100, Jonathan Cameron wrote:
> > On Mon, 29 Apr 2024 21:00:37 +0200
> > Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> >
> > > Fix indentations that are not following the standards, remove
> > > extra white lines and add missing white lines.
> > >
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> >
> > Possibly some reviewers will feel one or two of these are a little over zealous,
> > but this does both improve things and bring consistency to this code.
> >
> > I'll pick up some of these cleanups now (maybe the whole set but who knows)
> > to reduce what is left if we end up with a v6.
> >
> > Applied this one to the togreg branch of iio.git and pushed out as testing
> > for 0-day to see if we missed anything,
> >
> > Thanks,
> >
> > Jonathan
> >
>
> Hi Jonathan,
>
> I noticed that, another commit for this driver has been accepted here [1],
> which will affect some patches of this series that I am adding. It's not
> difficult at all to apply changes to my patches with respect to this new
> addition. Should I move on with a v6 without taking into consideration
> this change, and if everything is finally approved, I can send
> later a v7 with only the changes needed for this new commit?
Thanks for pointing that out. Please carry on as if that patch wasn't queued
up but mention it in your cover letter for v6.
It may take a little while for that patch to get upstream due to timing wrt
to the merge window but it will take a quicker path than this series.
If the merge resolution is similar enough, we may just do it upstream rather
than in my trees.
Jonathan
>
> Cheers,
> Vasilis
>
> [1] https://lore.kernel.org/linux-iio/043f2be49df3c11152aaf32fc5467ed43fd59faa.camel@gmail.com/T/#t
>
> > > ---
> > > drivers/iio/pressure/bmp280-core.c | 108 ++++++++++++++++-------------
> > > drivers/iio/pressure/bmp280-spi.c | 4 +-
> > > 2 files changed, 61 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > > index 09f53d987c7d..1a3241a41768 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,
> > > @@ -181,18 +180,19 @@ 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));
> > > + data->bmp280_cal_buf,
> > > + sizeof(data->bmp280_cal_buf));
> > > if (ret < 0) {
> > > dev_err(data->dev,
> > > - "failed to read temperature and pressure calibration parameters\n");
> > > + "failed to read calibration parameters\n");
> > > return ret;
> > > }
> > >
> > > - /* Toss the temperature and pressure calibration data into the entropy pool */
> > > - add_device_randomness(data->bmp280_cal_buf, sizeof(data->bmp280_cal_buf));
> > > + /* Toss calibration data into the entropy pool */
> > > + add_device_randomness(data->bmp280_cal_buf,
> > > + sizeof(data->bmp280_cal_buf));
> > >
> > > /* Parse temperature calibration values. */
> > > calib->T1 = le16_to_cpu(data->bmp280_cal_buf[T1]);
> > > @@ -223,7 +223,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) {
> > > - dev_err(dev, "failed to read common bmp280 calibration parameters\n");
> > > + dev_err(dev, "failed to read calibration parameters\n");
> > > return ret;
> > > }
> > >
> > > @@ -283,6 +283,7 @@ 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.
> > > @@ -305,7 +306,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
> > > @@ -538,7 +539,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
> > > }
> > >
> > > static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> > > - int val)
> > > + int val)
> > > {
> > > const int *avail = data->chip_info->oversampling_humid_avail;
> > > const int n = data->chip_info->num_oversampling_humid_avail;
> > > @@ -563,7 +564,7 @@ static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
> > > }
> > >
> > > static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> > > - int val)
> > > + int val)
> > > {
> > > const int *avail = data->chip_info->oversampling_temp_avail;
> > > const int n = data->chip_info->num_oversampling_temp_avail;
> > > @@ -588,7 +589,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
> > > }
> > >
> > > static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
> > > - int val)
> > > + int val)
> > > {
> > > const int *avail = data->chip_info->oversampling_press_avail;
> > > const int n = data->chip_info->num_oversampling_press_avail;
> > > @@ -772,13 +773,12 @@ 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);
> > > + 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");
> > > + dev_err(data->dev, "failed to write ctrl_meas register\n");
> > > return ret;
> > > }
> > >
> > > @@ -786,8 +786,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
> > > BMP280_FILTER_MASK,
> > > BMP280_FILTER_4X);
> > > if (ret < 0) {
> > > - dev_err(data->dev,
> > > - "failed to write config register\n");
> > > + dev_err(data->dev, "failed to write config register\n");
> > > return ret;
> > > }
> > >
> > > @@ -926,8 +925,8 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
> > > }
> > >
> > > /*
> > > - * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value of
> > > - * "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> > > + * Returns temperature in Celsius degrees, resolution is 0.01º C. Output value
> > > + * of "5123" equals 51.2º C. t_fine carries fine temperature as global value.
> > > *
> > > * Taken from datasheet, Section Appendix 9, "Compensation formula" and repo
> > > * https://github.com/BoschSensortec/BMP3-Sensor-API.
> > > @@ -1069,7 +1068,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
> > >
> > > /* Read temperature and pressure calibration data */
> > > ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
> > > - data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> > > + data->bmp380_cal_buf,
> > > + sizeof(data->bmp380_cal_buf));
> > > if (ret) {
> > > dev_err(data->dev,
> > > "failed to read temperature calibration parameters\n");
> > > @@ -1077,7 +1077,8 @@ static int bmp380_read_calib(struct bmp280_data *data)
> > > }
> > >
> > > /* Toss the temperature calibration data into the entropy pool */
> > > - add_device_randomness(data->bmp380_cal_buf, sizeof(data->bmp380_cal_buf));
> > > + add_device_randomness(data->bmp380_cal_buf,
> > > + sizeof(data->bmp380_cal_buf));
> > >
> > > /* Parse calibration values */
> > > calib->T1 = get_unaligned_le16(&data->bmp380_cal_buf[BMP380_T1]);
> > > @@ -1159,7 +1160,8 @@ static int bmp380_chip_config(struct bmp280_data *data)
> > >
> > > /* Configure output data rate */
> > > ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
> > > - BMP380_ODRS_MASK, data->sampling_freq, &aux);
> > > + BMP380_ODRS_MASK, data->sampling_freq,
> > > + &aux);
> > > if (ret) {
> > > dev_err(data->dev, "failed to write ODR selection register\n");
> > > return ret;
> > > @@ -1178,12 +1180,13 @@ static int bmp380_chip_config(struct bmp280_data *data)
> > >
> > > if (change) {
> > > /*
> > > - * The configurations errors are detected on the fly during a measurement
> > > - * cycle. If the sampling frequency is too low, it's faster to reset
> > > - * the measurement loop than wait until the next measurement is due.
> > > + * The configurations errors are detected on the fly during a
> > > + * measurement cycle. If the sampling frequency is too low, it's
> > > + * faster to reset the measurement loop than wait until the next
> > > + * measurement is due.
> > > *
> > > - * Resets sensor measurement loop toggling between sleep and normal
> > > - * operating modes.
> > > + * Resets sensor measurement loop toggling between sleep and
> > > + * normal operating modes.
> > > */
> > > ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
> > > BMP380_MODE_MASK,
> > > @@ -1201,22 +1204,21 @@ static int bmp380_chip_config(struct bmp280_data *data)
> > > return ret;
> > > }
> > > /*
> > > - * Waits for measurement before checking configuration error flag.
> > > - * Selected longest measure time indicated in section 3.9.1
> > > - * in the datasheet.
> > > + * Waits for measurement before checking configuration error
> > > + * flag. Selected longest measure time indicated in
> > > + * section 3.9.1 in the datasheet.
> > > */
> > > msleep(80);
> > >
> > > /* Check config error flag */
> > > ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
> > > if (ret) {
> > > - dev_err(data->dev,
> > > - "failed to read error register\n");
> > > + dev_err(data->dev, "failed to read error register\n");
> > > return ret;
> > > }
> > > if (tmp & BMP380_ERR_CONF_MASK) {
> > > dev_warn(data->dev,
> > > - "sensor flagged configuration as incompatible\n");
> > > + "sensor flagged configuration as incompatible\n");
> > > return -EINVAL;
> > > }
> > > }
> > > @@ -1317,9 +1319,11 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > > }
> > >
> > > /* Start NVM operation sequence */
> > > - ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_NVM_OP_SEQ_0);
> > > + ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > > + BMP580_CMD_NVM_OP_SEQ_0);
> > > if (ret) {
> > > - dev_err(data->dev, "failed to send nvm operation's first sequence\n");
> > > + dev_err(data->dev,
> > > + "failed to send nvm operation's first sequence\n");
> > > return ret;
> > > }
> > > if (is_write) {
> > > @@ -1327,7 +1331,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > > ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > > BMP580_CMD_NVM_WRITE_SEQ_1);
> > > if (ret) {
> > > - dev_err(data->dev, "failed to send nvm write sequence\n");
> > > + dev_err(data->dev,
> > > + "failed to send nvm write sequence\n");
> > > return ret;
> > > }
> > > /* Datasheet says on 4.8.1.2 it takes approximately 10ms */
> > > @@ -1338,7 +1343,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
> > > ret = regmap_write(data->regmap, BMP580_REG_CMD,
> > > BMP580_CMD_NVM_READ_SEQ_1);
> > > if (ret) {
> > > - dev_err(data->dev, "failed to send nvm read sequence\n");
> > > + dev_err(data->dev,
> > > + "failed to send nvm read sequence\n");
> > > return ret;
> > > }
> > > /* Datasheet says on 4.8.1.1 it takes approximately 200us */
> > > @@ -1501,8 +1507,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 +1552,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 +1561,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;
> > > @@ -1662,7 +1669,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> > > BMP580_OSR_PRESS_EN;
> > >
> > > ret = regmap_update_bits_check(data->regmap, BMP580_REG_OSR_CONFIG,
> > > - BMP580_OSR_TEMP_MASK | BMP580_OSR_PRESS_MASK |
> > > + BMP580_OSR_TEMP_MASK |
> > > + BMP580_OSR_PRESS_MASK |
> > > BMP580_OSR_PRESS_EN,
> > > reg_val, &aux);
> > > if (ret) {
> > > @@ -1713,7 +1721,8 @@ static int bmp580_chip_config(struct bmp280_data *data)
> > > */
> > > ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
> > > if (ret) {
> > > - dev_err(data->dev, "error reading effective OSR register\n");
> > > + dev_err(data->dev,
> > > + "error reading effective OSR register\n");
> > > return ret;
> > > }
> > > if (!(tmp & BMP580_EFF_OSR_VALID_ODR)) {
> > > @@ -1848,7 +1857,8 @@ static int bmp180_read_calib(struct bmp280_data *data)
> > > }
> > >
> > > /* Toss the calibration data into the entropy pool */
> > > - add_device_randomness(data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
> > > + add_device_randomness(data->bmp180_cal_buf,
> > > + sizeof(data->bmp180_cal_buf));
> > >
> > > calib->AC1 = be16_to_cpu(data->bmp180_cal_buf[AC1]);
> > > calib->AC2 = be16_to_cpu(data->bmp180_cal_buf[AC2]);
> > > @@ -1963,8 +1973,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;
> > > @@ -2241,6 +2250,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-spi.c b/drivers/iio/pressure/bmp280-spi.c
> > > index 4e19ea0b4d39..62b4e58104cf 100644
> > > --- a/drivers/iio/pressure/bmp280-spi.c
> > > +++ b/drivers/iio/pressure/bmp280-spi.c
> > > @@ -13,7 +13,7 @@
> > > #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);
> > >
> > >
> > > base-commit: b0a2c79c6f3590b74742cbbc76687014d47972d8
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 02/10] iio: pressure: bmp280: Use BME prefix for BME280 specifics
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 18:53 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 03/10] iio: pressure: bmp280: Add identifier names in function definitions Vasileios Amoiridis
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
Change the rest of the defines and function names that are
used specifically by the BME280 humidity sensor to BME280
as it is done for the rest of the BMP{0,1,3,5}80 sensors.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 37 +++++++++++------------
drivers/iio/pressure/bmp280-regmap.c | 8 ++---
drivers/iio/pressure/bmp280.h | 45 +++++++++++++++-------------
3 files changed, 46 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 1a3241a41768..edfa66953f87 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -235,14 +235,14 @@ 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);
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
if (ret < 0) {
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) {
dev_err(dev, "failed to read H2 comp value\n");
@@ -250,14 +250,14 @@ static int bme280_read_calib(struct bmp280_data *data)
}
calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
- ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
if (ret < 0) {
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) {
dev_err(dev, "failed to read H4 comp value\n");
@@ -266,15 +266,15 @@ static int bme280_read_calib(struct bmp280_data *data)
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) {
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);
+ ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
if (ret < 0) {
dev_err(dev, "failed to read H6 comp value\n");
return ret;
@@ -290,7 +290,7 @@ static int bme280_read_calib(struct bmp280_data *data)
*
* Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
*/
-static u32 bmp280_compensate_humidity(struct bmp280_data *data,
+static u32 bme280_compensate_humidity(struct bmp280_data *data,
s32 adc_humidity)
{
struct bmp280_calib *calib = &data->calib.bmp280;
@@ -430,7 +430,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;
@@ -441,7 +441,7 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
if (ret < 0)
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) {
dev_err(data->dev, "failed to read humidity\n");
@@ -454,7 +454,7 @@ 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;
@@ -538,7 +538,7 @@ static int bmp280_read_raw(struct iio_dev *indio_dev,
return ret;
}
-static int bmp280_write_oversampling_ratio_humid(struct bmp280_data *data,
+static int bme280_write_oversampling_ratio_humid(struct bmp280_data *data,
int val)
{
const int *avail = data->chip_info->oversampling_humid_avail;
@@ -682,7 +682,7 @@ 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 = bme280_write_oversampling_ratio_humid(data, val);
break;
case IIO_PRESSURE:
ret = bmp280_write_oversampling_ratio_press(data, val);
@@ -832,16 +832,15 @@ 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);
-
+ ret = regmap_update_bits(data->regmap, BME280_REG_CTRL_HUMIDITY,
+ BME280_OSRS_HUMIDITY_MASK, osrs);
if (ret < 0)
return ret;
@@ -869,12 +868,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);
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.h b/drivers/iio/pressure/bmp280.h
index 5812a344ed8e..91d4457a9230 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -192,8 +192,6 @@
#define BMP380_PRESS_SKIPPED 0x800000
/* BMP280 specific registers */
-#define BMP280_REG_HUMIDITY_LSB 0xFE
-#define BMP280_REG_HUMIDITY_MSB 0xFD
#define BMP280_REG_TEMP_XLSB 0xFC
#define BMP280_REG_TEMP_LSB 0xFB
#define BMP280_REG_TEMP_MSB 0xFA
@@ -207,15 +205,6 @@
#define BMP280_REG_CONFIG 0xF5
#define BMP280_REG_CTRL_MEAS 0xF4
#define BMP280_REG_STATUS 0xF3
-#define BMP280_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 BMP280_REG_COMP_TEMP_START 0x88
#define BMP280_COMP_TEMP_REG_COUNT 6
@@ -223,8 +212,6 @@
#define BMP280_REG_COMP_PRESS_START 0x8E
#define BMP280_COMP_PRESS_REG_COUNT 18
-#define BMP280_COMP_H5_MASK GENMASK(15, 4)
-
#define BMP280_CONTIGUOUS_CALIB_REGS (BMP280_COMP_TEMP_REG_COUNT + \
BMP280_COMP_PRESS_REG_COUNT)
@@ -235,14 +222,6 @@
#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 BMP280_OSRS_TEMP_MASK GENMASK(7, 5)
#define BMP280_OSRS_TEMP_SKIP 0
#define BMP280_OSRS_TEMP_1X 1
@@ -264,6 +243,30 @@
#define BMP280_MODE_FORCED 1
#define BMP280_MODE_NORMAL 3
+/* BME280 specific registers */
+#define BME280_REG_HUMIDITY_LSB 0xFE
+#define BME280_REG_HUMIDITY_MSB 0xFD
+
+#define BME280_REG_CTRL_HUMIDITY 0xF2
+
+/* Due to non linear mapping, and data sizes we can't do a bulk read */
+#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 BME280_COMP_H5_MASK GENMASK(15, 4)
+
+#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
+
/* BMP180 specific registers */
#define BMP180_REG_OUT_XLSB 0xF8
#define BMP180_REG_OUT_LSB 0xF7
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 02/10] iio: pressure: bmp280: Use BME prefix for BME280 specifics
2024-04-29 19:00 ` [PATCH v5 02/10] iio: pressure: bmp280: Use BME prefix for BME280 specifics Vasileios Amoiridis
@ 2024-05-05 18:53 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 18:53 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 29 Apr 2024 21:00:38 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> Change the rest of the defines and function names that are
> used specifically by the BME280 humidity sensor to BME280
> as it is done for the rest of the BMP{0,1,3,5}80 sensors.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied,
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 03/10] iio: pressure: bmp280: Add identifier names in function definitions
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 01/10] iio: pressure: bmp280: Improve indentation and line wrapping Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 02/10] iio: pressure: bmp280: Use BME prefix for BME280 specifics Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 18:54 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 04/10] iio: pressure: bmp280: Add more intuitive name for bmp180_measure() Vasileios Amoiridis
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
checkpatch.pl complained about missing identifier names in the input
variables for some function definitions.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 91d4457a9230..fe4d3f127954 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -452,12 +452,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 */
@@ -476,7 +476,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 03/10] iio: pressure: bmp280: Add identifier names in function definitions
2024-04-29 19:00 ` [PATCH v5 03/10] iio: pressure: bmp280: Add identifier names in function definitions Vasileios Amoiridis
@ 2024-05-05 18:54 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 18:54 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 29 Apr 2024 21:00:39 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> checkpatch.pl complained about missing identifier names in the input
> variables for some function definitions.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 04/10] iio: pressure: bmp280: Add more intuitive name for bmp180_measure()
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (2 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 03/10] iio: pressure: bmp280: Add identifier names in function definitions Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent Vasileios Amoiridis
` (5 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
The bmp180_measure() function essentially waits for the end of the
current conversion in order to read the values from the sensors. The
name bmp180_measure() could be misinterpreted because it could be
translated as "measure sensor values" even though it was probably
trying to say "measure time for eoc". Give a more intuitive name
to this function to be less confusing.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index edfa66953f87..ed49e0779d41 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1771,7 +1771,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;
@@ -1820,9 +1820,9 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
{
int ret;
- ret = bmp180_measure(data,
- FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_TEMP) |
- BMP180_MEAS_SCO);
+ ret = bmp180_wait_for_eoc(data,
+ FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_TEMP) |
+ BMP180_MEAS_SCO);
if (ret)
return ret;
@@ -1920,10 +1920,10 @@ 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;
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (3 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 04/10] iio: pressure: bmp280: Add more intuitive name for bmp180_measure() Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 19:08 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions Vasileios Amoiridis
` (4 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
Throughout the driver there are quite a few places were return
values are treated as errors if they are negative or not-zero.
This commit tries to make the return values of those functions
consistent and treat them as errors in case there is a negative
value since the vast majority of the functions are returning
erorrs coming from regmap_*() functions.
While at it, add error messages that were not implemented before.
Finally, remove any extra error checks that are dead code.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 175 ++++++++++++++++-------------
1 file changed, 96 insertions(+), 79 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index ed49e0779d41..8290028824e9 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -222,10 +222,8 @@ static int bme280_read_calib(struct bmp280_data *data)
/* Load shared calibration params with bmp280 first */
ret = bmp280_read_calib(data);
- if (ret < 0) {
- dev_err(dev, "failed to read calibration parameters\n");
+ if (ret < 0)
return ret;
- }
/*
* Read humidity calibration values.
@@ -552,7 +550,7 @@ static int bme280_write_oversampling_ratio_humid(struct bmp280_data *data,
data->oversampling_humid = ilog2(val);
ret = data->chip_info->chip_config(data);
- if (ret) {
+ if (ret < 0) {
data->oversampling_humid = prev;
data->chip_info->chip_config(data);
return ret;
@@ -577,7 +575,7 @@ static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
data->oversampling_temp = ilog2(val);
ret = data->chip_info->chip_config(data);
- if (ret) {
+ if (ret < 0) {
data->oversampling_temp = prev;
data->chip_info->chip_config(data);
return ret;
@@ -602,7 +600,7 @@ static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
data->oversampling_press = ilog2(val);
ret = data->chip_info->chip_config(data);
- if (ret) {
+ if (ret < 0) {
data->oversampling_press = prev;
data->chip_info->chip_config(data);
return ret;
@@ -627,7 +625,7 @@ static int bmp280_write_sampling_frequency(struct bmp280_data *data,
data->sampling_freq = i;
ret = data->chip_info->chip_config(data);
- if (ret) {
+ if (ret < 0) {
data->sampling_freq = prev;
data->chip_info->chip_config(data);
return ret;
@@ -651,7 +649,7 @@ static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val)
data->iir_filter_coeff = i;
ret = data->chip_info->chip_config(data);
- if (ret) {
+ if (ret < 0) {
data->iir_filter_coeff = prev;
data->chip_info->chip_config(data);
return ret;
@@ -841,8 +839,10 @@ static int bme280_chip_config(struct bmp280_data *data)
*/
ret = regmap_update_bits(data->regmap, BME280_REG_CTRL_HUMIDITY,
BME280_OSRS_HUMIDITY_MASK, osrs);
- if (ret < 0)
+ if (ret < 0) {
+ dev_err(data->dev, "failed to set humidity oversampling");
return ret;
+ }
return bmp280_chip_config(data);
}
@@ -892,7 +892,7 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
/* Check if device is ready to process a command */
ret = regmap_read(data->regmap, BMP380_REG_STATUS, ®);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read error register\n");
return ret;
}
@@ -903,7 +903,7 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
/* Send command to process */
ret = regmap_write(data->regmap, BMP380_REG_CMD, cmd);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to send command to device\n");
return ret;
}
@@ -911,7 +911,7 @@ static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
usleep_range(data->start_up_time, data->start_up_time + 100);
/* Check for command processing error */
ret = regmap_read(data->regmap, BMP380_REG_ERROR, ®);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error reading ERROR reg\n");
return ret;
}
@@ -1003,7 +1003,7 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
data->buf, sizeof(data->buf));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
}
@@ -1036,12 +1036,12 @@ static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
/* Read and compensate for temperature so we get a reading of t_fine */
ret = bmp380_read_temp(data, NULL, NULL);
- if (ret)
+ if (ret < 0)
return ret;
ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
data->buf, sizeof(data->buf));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
}
@@ -1069,9 +1069,9 @@ static int bmp380_read_calib(struct bmp280_data *data)
ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
data->bmp380_cal_buf,
sizeof(data->bmp380_cal_buf));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
- "failed to read temperature calibration parameters\n");
+ "failed to read calibration parameters\n");
return ret;
}
@@ -1137,7 +1137,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
BMP380_CTRL_SENSORS_MASK,
BMP380_CTRL_SENSORS_PRESS_EN |
BMP380_CTRL_SENSORS_TEMP_EN);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
"failed to write operation control register\n");
return ret;
@@ -1151,7 +1151,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
BMP380_OSRS_TEMP_MASK |
BMP380_OSRS_PRESS_MASK,
osrs, &aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write oversampling register\n");
return ret;
}
@@ -1161,7 +1161,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
BMP380_ODRS_MASK, data->sampling_freq,
&aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write ODR selection register\n");
return ret;
}
@@ -1171,7 +1171,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
ret = regmap_update_bits_check(data->regmap, BMP380_REG_CONFIG, BMP380_FILTER_MASK,
FIELD_PREP(BMP380_FILTER_MASK, data->iir_filter_coeff),
&aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write config register\n");
return ret;
}
@@ -1190,7 +1190,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
BMP380_MODE_MASK,
FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to set sleep mode\n");
return ret;
}
@@ -1198,7 +1198,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
BMP380_MODE_MASK,
FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to set normal mode\n");
return ret;
}
@@ -1211,7 +1211,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
/* Check config error flag */
ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read error register\n");
return ret;
}
@@ -1269,7 +1269,7 @@ static int bmp580_soft_reset(struct bmp280_data *data)
int ret;
ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_SOFT_RESET);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to send reset command to device\n");
return ret;
}
@@ -1277,13 +1277,13 @@ static int bmp580_soft_reset(struct bmp280_data *data)
/* Dummy read of chip_id */
ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, ®);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to reestablish comms after reset\n");
return ret;
}
ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, ®);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error reading interrupt status register\n");
return ret;
}
@@ -1308,7 +1308,7 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
/* Check NVM ready flag */
ret = regmap_read(data->regmap, BMP580_REG_STATUS, ®);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to check nvm status\n");
return ret;
}
@@ -1320,7 +1320,7 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
/* Start NVM operation sequence */
ret = regmap_write(data->regmap, BMP580_REG_CMD,
BMP580_CMD_NVM_OP_SEQ_0);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
"failed to send nvm operation's first sequence\n");
return ret;
@@ -1329,7 +1329,7 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
/* Send NVM write sequence */
ret = regmap_write(data->regmap, BMP580_REG_CMD,
BMP580_CMD_NVM_WRITE_SEQ_1);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
"failed to send nvm write sequence\n");
return ret;
@@ -1341,7 +1341,7 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
/* Send NVM read sequence */
ret = regmap_write(data->regmap, BMP580_REG_CMD,
BMP580_CMD_NVM_READ_SEQ_1);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
"failed to send nvm read sequence\n");
return ret;
@@ -1350,16 +1350,12 @@ 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,
(reg & BMP580_STATUS_NVM_RDY_MASK),
poll, timeout);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error checking nvm operation status\n");
return ret;
}
@@ -1386,7 +1382,7 @@ static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf,
sizeof(data->buf));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
}
@@ -1414,7 +1410,7 @@ static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf,
sizeof(data->buf));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
}
@@ -1485,7 +1481,7 @@ static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
BMP580_ODR_DEEPSLEEP_DIS |
FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to change sensor to standby mode\n");
goto exit;
}
@@ -1497,18 +1493,18 @@ static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error writing nvm address\n");
goto exit;
}
ret = bmp580_nvm_operation(data, false);
- if (ret)
+ if (ret < 0)
goto exit;
ret = regmap_bulk_read(data->regmap, BMP580_REG_NVM_DATA_LSB,
&data->le16, sizeof(data->le16));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error reading nvm data regs\n");
goto exit;
}
@@ -1541,7 +1537,7 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
BMP580_ODR_DEEPSLEEP_DIS |
FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to change sensor to standby mode\n");
goto exit;
}
@@ -1554,7 +1550,7 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
BMP580_NVM_PROG_EN |
FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error writing nvm address\n");
goto exit;
}
@@ -1562,19 +1558,19 @@ static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
ret = regmap_bulk_write(data->regmap, BMP580_REG_NVM_DATA_LSB,
&data->le16, sizeof(data->le16));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error writing LSB NVM data regs\n");
goto exit;
}
ret = bmp580_nvm_operation(data, true);
- if (ret)
+ if (ret < 0)
goto exit;
/* Disable programming mode bit */
ret = regmap_update_bits(data->regmap, BMP580_REG_NVM_ADDR,
BMP580_NVM_PROG_EN, 0);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "error resetting nvm write\n");
goto exit;
}
@@ -1608,25 +1604,29 @@ static int bmp580_preinit(struct bmp280_data *data)
/* Issue soft-reset command */
ret = bmp580_soft_reset(data);
- if (ret)
+ if (ret < 0)
return ret;
/* Post powerup sequence */
ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, ®);
- if (ret)
+ if (ret < 0) {
+ 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, ®);
- if (ret)
+ if (ret < 0) {
+ 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;
}
@@ -1646,7 +1646,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
BMP580_ODR_DEEPSLEEP_DIS |
FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to change sensor to standby mode\n");
return ret;
}
@@ -1661,6 +1661,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 < 0) {
+ 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) |
@@ -1672,7 +1676,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
BMP580_OSR_PRESS_MASK |
BMP580_OSR_PRESS_EN,
reg_val, &aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write oversampling register\n");
return ret;
}
@@ -1682,7 +1686,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
ret = regmap_update_bits_check(data->regmap, BMP580_REG_ODR_CONFIG, BMP580_ODR_MASK,
FIELD_PREP(BMP580_ODR_MASK, data->sampling_freq),
&aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write ODR configuration register\n");
return ret;
}
@@ -1696,7 +1700,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
BMP580_DSP_IIR_PRESS_MASK |
BMP580_DSP_IIR_TEMP_MASK,
reg_val, &aux);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to write config register\n");
return ret;
}
@@ -1706,7 +1710,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
BMP580_MODE_MASK,
FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev, "failed to set normal mode\n");
return ret;
}
@@ -1719,7 +1723,7 @@ static int bmp580_chip_config(struct bmp280_data *data)
* operating in a degraded mode.
*/
ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
- if (ret) {
+ if (ret < 0) {
dev_err(data->dev,
"error reading effective OSR register\n");
return ret;
@@ -1782,8 +1786,10 @@ static int bmp180_wait_for_eoc(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 < 0) {
+ dev_err(data->dev, "failed to write crtl_meas register\n");
return ret;
+ }
if (data->use_eoc) {
/*
@@ -1806,12 +1812,16 @@ static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
}
ret = regmap_read(data->regmap, BMP280_REG_CTRL_MEAS, &ctrl);
- if (ret)
+ if (ret < 0) {
+ 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;
}
@@ -1823,13 +1833,15 @@ static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
ret = bmp180_wait_for_eoc(data,
FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_TEMP) |
BMP180_MEAS_SCO);
- if (ret)
+ if (ret < 0)
return ret;
ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
&data->be16, sizeof(data->be16));
- if (ret)
+ if (ret < 0) {
+ dev_err(data->dev, "failed to read temperature\n");
return ret;
+ }
*val = be16_to_cpu(data->be16);
@@ -1844,9 +1856,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 < 0) {
+ 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++) {
@@ -1898,7 +1911,7 @@ static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
int ret;
ret = bmp180_read_adc_temp(data, &adc_temp);
- if (ret)
+ if (ret < 0)
return ret;
comp_temp = bmp180_compensate_temp(data, adc_temp);
@@ -1924,13 +1937,15 @@ static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_PRESS) |
FIELD_PREP(BMP180_OSRS_PRESS_MASK, oss) |
BMP180_MEAS_SCO);
- if (ret)
+ if (ret < 0)
return ret;
ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
data->buf, sizeof(data->buf));
- if (ret)
+ if (ret < 0) {
+ dev_err(data->dev, "failed to read pressure\n");
return ret;
+ }
*val = get_unaligned_be24(data->buf) >> (8 - oss);
@@ -1980,11 +1995,11 @@ static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
/* Read and compensate temperature so we get a reading of t_fine. */
ret = bmp180_read_temp(data, NULL, NULL);
- if (ret)
+ if (ret < 0)
return ret;
ret = bmp180_read_adc_press(data, &adc_press);
- if (ret)
+ if (ret < 0)
return ret;
comp_press = bmp180_compensate_press(data, adc_press);
@@ -2062,7 +2077,7 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
irq_trig,
name,
data);
- if (ret) {
+ if (ret < 0) {
/* Bail out without IRQ but keep the driver in place */
dev_err(dev, "unable to request DRDY IRQ\n");
return 0;
@@ -2132,20 +2147,20 @@ int bmp280_common_probe(struct device *dev,
ret = devm_regulator_bulk_get(dev,
BMP280_NUM_SUPPLIES, data->supplies);
- if (ret) {
+ if (ret < 0) {
dev_err(dev, "failed to get regulators\n");
return ret;
}
ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
- if (ret) {
+ if (ret < 0) {
dev_err(dev, "failed to enable regulators\n");
return ret;
}
ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
data->supplies);
- if (ret)
+ if (ret < 0)
return ret;
/* Wait to make sure we started up properly */
@@ -2162,8 +2177,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 < 0) {
+ 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]) {
@@ -2177,7 +2194,7 @@ int bmp280_common_probe(struct device *dev,
if (data->chip_info->preinit) {
ret = data->chip_info->preinit(data);
- if (ret)
+ if (ret < 0)
return dev_err_probe(data->dev, ret,
"error running preinit tasks\n");
}
@@ -2208,7 +2225,7 @@ int bmp280_common_probe(struct device *dev,
*/
if (irq > 0 && (chip_id == BMP180_CHIP_ID)) {
ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
- if (ret)
+ if (ret < 0)
return ret;
}
@@ -2225,7 +2242,7 @@ int bmp280_common_probe(struct device *dev,
pm_runtime_put(dev);
ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
- if (ret)
+ if (ret < 0)
return ret;
return devm_iio_device_register(dev, indio_dev);
@@ -2247,7 +2264,7 @@ static int bmp280_runtime_resume(struct device *dev)
int ret;
ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
- if (ret)
+ if (ret < 0)
return ret;
usleep_range(data->start_up_time, data->start_up_time + 100);
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent
2024-04-29 19:00 ` [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent Vasileios Amoiridis
@ 2024-05-05 19:08 ` Jonathan Cameron
2024-05-05 23:08 ` Vasileios Amoiridis
0 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 19:08 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 29 Apr 2024 21:00:41 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> Throughout the driver there are quite a few places were return
> values are treated as errors if they are negative or not-zero.
> This commit tries to make the return values of those functions
> consistent and treat them as errors in case there is a negative
> value since the vast majority of the functions are returning
> erorrs coming from regmap_*() functions.
The changes are fine, but that argument isn't correct.
regmap_*() functions never (that I can recall) return positive
values, so if (ret) would be valid for those and I'd have expected
the exact opposite outcome if you are looking at regmap*() return
values to make the decision.
The if (ret) pattern is sometimes used throughout because it
makes
return function()
consistent without needing to do
ret = function();
if (ret < 0)
return ret;
return 0;
That pattern isn't particularly common in this driver (there are few cases).
We also tend not to worry too much about that slight inconsistency though
in a few cases it has lead to compilers failing to detect that some paths
are not possible and reporting false warnings.
However, all arguments about which is 'better' aside, key is that consistency
(either choice) is better than a mix. So I'm fine with ret < 0 on basis
it's the most common in this driver being your justification. Just don't
blame regmap*() return values!
>
> While at it, add error messages that were not implemented before.
>
> Finally, remove any extra error checks that are dead code.
Ideally this would be broken up a little more as, whilst all error
code related, these aren't all the same thing.
I'd have preferred:
1) Dead code removal.
2) Message updates.
3) Switch to consistent ret handling.
However it isn't that bad as a single patch, so just address the question
above and I think this will be fine as one patch.
>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Jonathan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent
2024-05-05 19:08 ` Jonathan Cameron
@ 2024-05-05 23:08 ` Vasileios Amoiridis
0 siblings, 0 replies; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-05-05 23:08 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Vasileios Amoiridis, lars, andriy.shevchenko, ang.iglesiasg,
mazziesaccount, ak, petre.rodan, phil, 579lpy, linus.walleij,
semen.protsenko, linux-iio, linux-kernel
On Sun, May 05, 2024 at 08:08:18PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:41 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
>
> > Throughout the driver there are quite a few places were return
> > values are treated as errors if they are negative or not-zero.
> > This commit tries to make the return values of those functions
> > consistent and treat them as errors in case there is a negative
> > value since the vast majority of the functions are returning
> > erorrs coming from regmap_*() functions.
>
> The changes are fine, but that argument isn't correct.
> regmap_*() functions never (that I can recall) return positive
> values, so if (ret) would be valid for those and I'd have expected
> the exact opposite outcome if you are looking at regmap*() return
> values to make the decision.
>
> The if (ret) pattern is sometimes used throughout because it
> makes
> return function()
>
> consistent without needing to do
>
> ret = function();
> if (ret < 0)
> return ret;
>
> return 0;
>
> That pattern isn't particularly common in this driver (there are few cases).
> We also tend not to worry too much about that slight inconsistency though
> in a few cases it has lead to compilers failing to detect that some paths
> are not possible and reporting false warnings.
>
> However, all arguments about which is 'better' aside, key is that consistency
> (either choice) is better than a mix. So I'm fine with ret < 0 on basis
> it's the most common in this driver being your justification. Just don't
> blame regmap*() return values!
>
Hi Jonathan!
Thank you once again for the valueable feedback!
Of course, if (ret) would be valid for the return values of the regmap_*()
functions. I was just trying to understand which of the 2 options is more
widely used in other drivers and I tried to implement that. In general,
the if (ret) is used 65 times while the if (ret < 0) only 20. So, in
terms of noise, changing the if (ret < 0) to if (ret) will create less
noise. I chose the if (ret < 0) because I saw other people using it
and it felt better in my eyes. I could check if if (ret) applies
everywhere and update it in the v6.
> >
> > While at it, add error messages that were not implemented before.
> >
> > Finally, remove any extra error checks that are dead code.
>
> Ideally this would be broken up a little more as, whilst all error
> code related, these aren't all the same thing.
>
> I'd have preferred:
> 1) Dead code removal.
> 2) Message updates.
> 3) Switch to consistent ret handling.
>
> However it isn't that bad as a single patch, so just address the question
> above and I think this will be fine as one patch.
>
Since from your comments in the next patches a v6 is for sure, I could split
this as well!
Cheers,
Vasilis
> >
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
>
> Jonathan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (4 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 05/10] iio: pressure: bmp280: Make return values consistent Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 19:21 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 07/10] iio: pressure: bmp280: Introduce new cleanup routines Vasileios Amoiridis
` (3 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis, Jonathan Cameron
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 <Jonathan.Cameron@huawei.com>
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 351 ++++++++++++++++++-----------
drivers/iio/pressure/bmp280.h | 6 -
2 files changed, 221 insertions(+), 136 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 8290028824e9..5ebce38e99f6 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -288,13 +288,33 @@ static int bme280_read_calib(struct bmp280_data *data)
*
* Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
*/
+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 < 0) {
+ 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 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)
@@ -313,8 +333,27 @@ static u32 bme280_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 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 < 0) {
+ 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;
@@ -324,9 +363,26 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
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 */
+}
+
+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 < 0)
+ return ret;
+
+ *t_fine = bmp280_calc_t_fine(data, adc_temp);
- return (data->t_fine * 5 + 128) >> 8;
+ 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;
}
/*
@@ -336,13 +392,33 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
*
* Taken from datasheet, Section 3.11.3, "Compensation formula".
*/
+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 < 0) {
+ 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 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;
@@ -368,60 +444,35 @@ static int bmp280_read_temp(struct bmp280_data *data,
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) {
- dev_err(data->dev, "failed to read temperature\n");
+ ret = bmp280_read_temp_adc(data, &adc_temp);
+ if (ret < 0)
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 < 0)
return ret;
- ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
- data->buf, sizeof(data->buf));
- if (ret < 0) {
- dev_err(data->dev, "failed to read pressure\n");
+ ret = bmp280_read_press_adc(data, &adc_press);
+ if (ret < 0)
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;
@@ -430,30 +481,21 @@ static int bmp280_read_press(struct bmp280_data *data,
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 < 0)
return ret;
- ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
- &data->be16, sizeof(data->be16));
- if (ret < 0) {
- dev_err(data->dev, "failed to read humidity\n");
+ ret = bme280_read_humid_adc(data, &adc_humidity);
+ if (ret < 0)
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;
@@ -930,9 +972,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 < 0) {
+ 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);
@@ -941,7 +1003,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 < 0)
+ 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);
@@ -955,27 +1039,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 < 0) {
+ 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;
@@ -1001,60 +1106,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 < 0) {
- dev_err(data->dev, "failed to read temperature\n");
+ ret = bmp380_read_temp_adc(data, &adc_temp);
+ if (ret < 0)
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 < 0)
return ret;
- ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
- data->buf, sizeof(data->buf));
- if (ret < 0) {
- dev_err(data->dev, "failed to read pressure\n");
+ ret = bmp380_read_press_adc(data, &adc_press);
+ if (ret < 0)
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;
@@ -1826,7 +1905,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;
@@ -1843,7 +1922,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;
}
@@ -1893,16 +1972,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 < 0)
+ 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)
@@ -1910,25 +2007,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 < 0)
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;
@@ -1947,7 +2037,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;
}
@@ -1957,7 +2047,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;
@@ -1965,7 +2056,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;
@@ -1989,21 +2080,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 < 0)
return ret;
- ret = bmp180_read_adc_press(data, &adc_press);
+ ret = bmp180_read_press_adc(data, &adc_press);
if (ret < 0)
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 fe4d3f127954..7c30e4d523be 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -397,12 +397,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions
2024-04-29 19:00 ` [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions Vasileios Amoiridis
@ 2024-05-05 19:21 ` Jonathan Cameron
2024-05-05 23:47 ` Vasileios Amoiridis
0 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 19:21 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Jonathan Cameron
On Mon, 29 Apr 2024 21:00:42 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> 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 <Jonathan.Cameron@huawei.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
In general looks good, but a few details to consider inline.
Jonathan
> ---
> drivers/iio/pressure/bmp280-core.c | 351 ++++++++++++++++++-----------
> drivers/iio/pressure/bmp280.h | 6 -
> 2 files changed, 221 insertions(+), 136 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 8290028824e9..5ebce38e99f6 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -288,13 +288,33 @@ static int bme280_read_calib(struct bmp280_data *data)
> *
> * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> */
> +static int bme280_read_humid_adc(struct bmp280_data *data, s32 *adc_humidity)
It's an u16, so why use an s32? I can see using a signed value avoids a cast later,
but it makes this more confusing to read, so I'd push that cast up to the user.
> +{
> + int ret;
> +
> + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> + &data->be16, sizeof(data->be16));
> + if (ret < 0) {
> + dev_err(data->dev, "failed to read humidity\n");
> + return ret;
> + }
> +
> + *adc_humidity = be16_to_cpu(data->be16);
Trivial, but on error return we normally aim for side effect free.
To do that here use an internal variable first.
s16 value;
...
value = be16_to_cpu(data->be16)
if (value == BMP280_HUMIDITY_SKIPPED) {
dev_err(data->dev, "...
return -EIO;
}
This is the odd bit due to using an s32 to store a u16.
Have to rely on that size mismatch to avoid the sign accidentally mattering.
Which is ugly!
*adc_humidity = value;
return 0;
> + 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 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;
Casting an s32 to an s32. For the const it shouldn't matter as it'll be at least
32 bits and we don't care about the sign.
> 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)
> @@ -313,8 +333,27 @@ static u32 bme280_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 int bmp280_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
As before, sign of the extra variable is confusing. It's not signed
as it's a raw ADC value. So I'd use a u32 for it.
> +{
> + int ret;
> +
> + ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
> + data->buf, sizeof(data->buf));
> + if (ret < 0) {
> + 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;
Same thing on side effects. Best to avoid them if it is easy to do (like here!)
> + }
> +
> + 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;
> @@ -324,9 +363,26 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> 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 */
> +}
> +
> +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 < 0)
> + return ret;
> +
> + *t_fine = bmp280_calc_t_fine(data, adc_temp);
>
> - return (data->t_fine * 5 + 128) >> 8;
> + 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;
> }
>
> /*
> @@ -336,13 +392,33 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> *
> * Taken from datasheet, Section 3.11.3, "Compensation formula".
> */
> +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 < 0) {
> + 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;
As above; avoid side effects.
> + }
> +
> + return 0;
> +}
> +
> static u32 bmp280_compensate_press(struct bmp280_data *data,
> - s32 adc_press)
> + 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;
> @@ -368,60 +444,35 @@ static int bmp280_read_temp(struct bmp280_data *data,
> 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) {
> - dev_err(data->dev, "failed to read temperature\n");
> + ret = bmp280_read_temp_adc(data, &adc_temp);
> + if (ret < 0)
> 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 < 0)
> return ret;
>
> - ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> - data->buf, sizeof(data->buf));
> - if (ret < 0) {
> - dev_err(data->dev, "failed to read pressure\n");
> + ret = bmp280_read_press_adc(data, &adc_press);
> + if (ret < 0)
> 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;
>
> @@ -430,30 +481,21 @@ static int bmp280_read_press(struct bmp280_data *data,
>
> 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 < 0)
> return ret;
>
> - ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> - &data->be16, sizeof(data->be16));
> - if (ret < 0) {
> - dev_err(data->dev, "failed to read humidity\n");
> + ret = bme280_read_humid_adc(data, &adc_humidity);
> + if (ret < 0)
> 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;
> @@ -930,9 +972,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)
Interesting this one is unsigned.
> +{
> + int ret;
> +
> + ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
> + data->buf, sizeof(data->buf));
> + if (ret < 0) {
> + dev_err(data->dev, "failed to read temperature\n");
> + return ret;
> + }
> +
> + *adc_temp = get_unaligned_le24(data->buf);
> + if (*adc_temp == BMP380_TEMP_SKIPPED) {
Same as above.
> + 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);
> @@ -941,7 +1003,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 < 0)
> + 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);
> @@ -955,27 +1039,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 < 0) {
> + dev_err(data->dev, "failed to read pressure\n");
> + return ret;
> + }
> +
> + *adc_press = get_unaligned_le24(data->buf);
As above
> + if (*adc_press == BMP380_PRESS_SKIPPED) {
> + dev_err(data->dev, "reading pressure skipped\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
Jonathan
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions
2024-05-05 19:21 ` Jonathan Cameron
@ 2024-05-05 23:47 ` Vasileios Amoiridis
2024-05-06 12:46 ` Jonathan Cameron
0 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-05-05 23:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Vasileios Amoiridis, lars, andriy.shevchenko, ang.iglesiasg,
mazziesaccount, ak, petre.rodan, phil, 579lpy, linus.walleij,
semen.protsenko, linux-iio, linux-kernel, Jonathan Cameron
On Sun, May 05, 2024 at 08:21:06PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:42 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> 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 <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> In general looks good, but a few details to consider inline.
>
> Jonathan
>
> > ---
> > drivers/iio/pressure/bmp280-core.c | 351 ++++++++++++++++++-----------
> > drivers/iio/pressure/bmp280.h | 6 -
> > 2 files changed, 221 insertions(+), 136 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 8290028824e9..5ebce38e99f6 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -288,13 +288,33 @@ static int bme280_read_calib(struct bmp280_data *data)
> > *
> > * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> > */
> > +static int bme280_read_humid_adc(struct bmp280_data *data, s32 *adc_humidity)
>
> It's an u16, so why use an s32? I can see using a signed value avoids a cast later,
> but it makes this more confusing to read, so I'd push that cast up to the user.
>
Bosch in general has messed up a bit with the signs on their raw values on all
of those sensors that we use in this driver. I totally agree with you, that this
value does not make any sense to be anything else apart from u16 but in the
datasheet [1] in pages 25-26 you can clearly see that they use an s32 for this
value...
[1]: https://www.mouser.com/datasheet/2/783/BST-BME280-DS002-1509607.pdf
> > +{
> > + int ret;
> > +
> > + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > + &data->be16, sizeof(data->be16));
> > + if (ret < 0) {
> > + dev_err(data->dev, "failed to read humidity\n");
> > + return ret;
> > + }
> > +
> > + *adc_humidity = be16_to_cpu(data->be16);
>
> Trivial, but on error return we normally aim for side effect free.
> To do that here use an internal variable first.
I am sorry, but in this part, I cannot fully understand what you mean
by side effect free. I can understand the issue of storing a u16 to an
s32 might make accidentally the sign to matter but how is this thing
that you proposed no side effect free? You also made this comment
in various other places in this patch (because the same principle
with the SKIPPED is used) and in the other places the values are
20-bit and 24-bit long which confuses me a bit more on what you mean
exactly.
> s16 value;
>
> ...
>
> value = be16_to_cpu(data->be16)
>
> if (value == BMP280_HUMIDITY_SKIPPED) {
> dev_err(data->dev, "...
> return -EIO;
> }
> This is the odd bit due to using an s32 to store a u16.
> Have to rely on that size mismatch to avoid the sign accidentally mattering.
> Which is ugly!
>
> *adc_humidity = value;
>
> return 0;
>
>
> > + 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 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;
>
> Casting an s32 to an s32. For the const it shouldn't matter as it'll be at least
> 32 bits and we don't care about the sign.
>
In general, I kept the casting for the t_fine because it was used from before,
but I don't see the point since it is already an s32 value. The casting in front
of the const, I see it is used in the datasheet [1] in pages 25-26 so maybe they
kept it for this reason. Since it will be again a 32 bit value, I don't see the
point of casting it but I still kept it as it was, there originally.
[1]: https://www.mouser.com/datasheet/2/783/BST-BME280-DS002-1509607.pdf
> > 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)
> > @@ -313,8 +333,27 @@ static u32 bme280_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 int bmp280_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
>
> As before, sign of the extra variable is confusing. It's not signed
> as it's a raw ADC value. So I'd use a u32 for it.
>
Again, as I said before, Bosch has messed this up. I agree (again) with you
that this should have been a u16 but according to the datasheet [2] in pages
45-46 it is an s32...
[2]: https://cdn-shop.adafruit.com/datasheets/BST-BMP280-DS001-11.pdf
> > +{
> > + int ret;
> > +
> > + ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
> > + data->buf, sizeof(data->buf));
> > + if (ret < 0) {
> > + 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;
> Same thing on side effects. Best to avoid them if it is easy to do (like here!)
> > + }
> > +
> > + 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;
> > @@ -324,9 +363,26 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > 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 */
> > +}
> > +
> > +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 < 0)
> > + return ret;
> > +
> > + *t_fine = bmp280_calc_t_fine(data, adc_temp);
> >
> > - return (data->t_fine * 5 + 128) >> 8;
> > + 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;
> > }
> >
> > /*
> > @@ -336,13 +392,33 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> > *
> > * Taken from datasheet, Section 3.11.3, "Compensation formula".
> > */
> > +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 < 0) {
> > + 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;
>
> As above; avoid side effects.
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static u32 bmp280_compensate_press(struct bmp280_data *data,
> > - s32 adc_press)
> > + 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;
> > @@ -368,60 +444,35 @@ static int bmp280_read_temp(struct bmp280_data *data,
> > 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) {
> > - dev_err(data->dev, "failed to read temperature\n");
> > + ret = bmp280_read_temp_adc(data, &adc_temp);
> > + if (ret < 0)
> > 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 < 0)
> > return ret;
> >
> > - ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB,
> > - data->buf, sizeof(data->buf));
> > - if (ret < 0) {
> > - dev_err(data->dev, "failed to read pressure\n");
> > + ret = bmp280_read_press_adc(data, &adc_press);
> > + if (ret < 0)
> > 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;
> >
> > @@ -430,30 +481,21 @@ static int bmp280_read_press(struct bmp280_data *data,
> >
> > 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 < 0)
> > return ret;
> >
> > - ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > - &data->be16, sizeof(data->be16));
> > - if (ret < 0) {
> > - dev_err(data->dev, "failed to read humidity\n");
> > + ret = bme280_read_humid_adc(data, &adc_humidity);
> > + if (ret < 0)
> > 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;
> > @@ -930,9 +972,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)
>
> Interesting this one is unsigned.
>
Yes, and it is also mentioned in the datasheet [3] in page 26.
[3]: https://www.mouser.com/pdfdocs/BST-BMP388-DS001-01.pdf
Cheers,
Vasilis
> > +{
> > + int ret;
> > +
> > + ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
> > + data->buf, sizeof(data->buf));
> > + if (ret < 0) {
> > + dev_err(data->dev, "failed to read temperature\n");
> > + return ret;
> > + }
> > +
> > + *adc_temp = get_unaligned_le24(data->buf);
> > + if (*adc_temp == BMP380_TEMP_SKIPPED) {
> Same as above.
> > + 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);
> > @@ -941,7 +1003,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 < 0)
> > + 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);
> > @@ -955,27 +1039,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 < 0) {
> > + dev_err(data->dev, "failed to read pressure\n");
> > + return ret;
> > + }
> > +
> > + *adc_press = get_unaligned_le24(data->buf);
>
> As above
>
> > + if (*adc_press == BMP380_PRESS_SKIPPED) {
> > + dev_err(data->dev, "reading pressure skipped\n");
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> Jonathan
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions
2024-05-05 23:47 ` Vasileios Amoiridis
@ 2024-05-06 12:46 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-06 12:46 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Jonathan Cameron
On Mon, 6 May 2024 01:47:51 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> On Sun, May 05, 2024 at 08:21:06PM +0100, Jonathan Cameron wrote:
> > On Mon, 29 Apr 2024 21:00:42 +0200
> > Vasileios Amoiridis <vassilisamir@gmail.com> 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 <Jonathan.Cameron@huawei.com>
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > In general looks good, but a few details to consider inline.
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/pressure/bmp280-core.c | 351 ++++++++++++++++++-----------
> > > drivers/iio/pressure/bmp280.h | 6 -
> > > 2 files changed, 221 insertions(+), 136 deletions(-)
> > >
> > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > > index 8290028824e9..5ebce38e99f6 100644
> > > --- a/drivers/iio/pressure/bmp280-core.c
> > > +++ b/drivers/iio/pressure/bmp280-core.c
> > > @@ -288,13 +288,33 @@ static int bme280_read_calib(struct bmp280_data *data)
> > > *
> > > * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> > > */
> > > +static int bme280_read_humid_adc(struct bmp280_data *data, s32 *adc_humidity)
> >
> > It's an u16, so why use an s32? I can see using a signed value avoids a cast later,
> > but it makes this more confusing to read, so I'd push that cast up to the user.
> >
>
> Bosch in general has messed up a bit with the signs on their raw values on all
> of those sensors that we use in this driver. I totally agree with you, that this
> value does not make any sense to be anything else apart from u16 but in the
> datasheet [1] in pages 25-26 you can clearly see that they use an s32 for this
> value...
>
> [1]: https://www.mouser.com/datasheet/2/783/BST-BME280-DS002-1509607.pdf
I would be tempted to ignore that and use the more appropriate size, but be
careful that any necessary casts are in place when you use the value.
>
> > > +{
> > > + int ret;
> > > +
> > > + ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > > + &data->be16, sizeof(data->be16));
> > > + if (ret < 0) {
> > > + dev_err(data->dev, "failed to read humidity\n");
> > > + return ret;
> > > + }
> > > +
> > > + *adc_humidity = be16_to_cpu(data->be16);
> >
> > Trivial, but on error return we normally aim for side effect free.
> > To do that here use an internal variable first.
>
> I am sorry, but in this part, I cannot fully understand what you mean
> by side effect free. I can understand the issue of storing a u16 to an
> s32 might make accidentally the sign to matter but how is this thing
> that you proposed no side effect free? You also made this comment
> in various other places in this patch (because the same principle
> with the SKIPPED is used) and in the other places the values are
> 20-bit and 24-bit long which confuses me a bit more on what you mean
> exactly.
If you get an error, e.g. if (*adc_humidty == BMP280_HUMIDITY_SKIPPED) then you
should not set *adc_humidity. Setting it is the side effect. Normal aim
is that a function that returns an error should ensure it leave no other
effects in data it can access.
This make reasoning about error paths much simpler because you only
have to deal with the return values.
Hence the code example I included though I make it more confusing by
commenting on the types in the middle of the code.
Key is I don't want *adc_humidity to be modified if we aren't going to
return 0.
>
> > s16 value;
> >
> > ...
> >
> > value = be16_to_cpu(data->be16)
> >
> > if (value == BMP280_HUMIDITY_SKIPPED) {
> > dev_err(data->dev, "...
> > return -EIO;
> > }
> > This is the odd bit due to using an s32 to store a u16.
> > Have to rely on that size mismatch to avoid the sign accidentally mattering.
> > Which is ugly!
> >
> > *adc_humidity = value;
> >
> > return 0;
> >
> >
> > > + 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 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;
> >
> > Casting an s32 to an s32. For the const it shouldn't matter as it'll be at least
> > 32 bits and we don't care about the sign.
> >
>
> In general, I kept the casting for the t_fine because it was used from before,
> but I don't see the point since it is already an s32 value. The casting in front
> of the const, I see it is used in the datasheet [1] in pages 25-26 so maybe they
> kept it for this reason. Since it will be again a 32 bit value, I don't see the
> point of casting it but I still kept it as it was, there originally.
>
> [1]: https://www.mouser.com/datasheet/2/783/BST-BME280-DS002-1509607.pdf
As you are tidying up the code, drop the unnecessary cast. Good to mention it
in the patch description though.
>
> > > 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)
> > > @@ -313,8 +333,27 @@ static u32 bme280_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 int bmp280_read_temp_adc(struct bmp280_data *data, s32 *adc_temp)
> >
> > As before, sign of the extra variable is confusing. It's not signed
> > as it's a raw ADC value. So I'd use a u32 for it.
> >
>
> Again, as I said before, Bosch has messed this up. I agree (again) with you
> that this should have been a u16 but according to the datasheet [2] in pages
> 45-46 it is an s32...
>
> [2]: https://cdn-shop.adafruit.com/datasheets/BST-BMP280-DS001-11.pdf
>
What they have is not incorrect, but it is relaxed in a rather lazy fashion!
...
> > > @@ -430,30 +481,21 @@ static int bmp280_read_press(struct bmp280_data *data,
> > >
> > > 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 < 0)
> > > return ret;
> > >
> > > - ret = regmap_bulk_read(data->regmap, BME280_REG_HUMIDITY_MSB,
> > > - &data->be16, sizeof(data->be16));
> > > - if (ret < 0) {
> > > - dev_err(data->dev, "failed to read humidity\n");
> > > + ret = bme280_read_humid_adc(data, &adc_humidity);
> > > + if (ret < 0)
> > > 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;
> > > @@ -930,9 +972,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)
> >
> > Interesting this one is unsigned.
> >
>
> Yes, and it is also mentioned in the datasheet [3] in page 26.
>
> [3]: https://www.mouser.com/pdfdocs/BST-BMP388-DS001-01.pdf
I'd take the datasheet maths as 'an implementation' rather than something
we have to match. So go for types that make sense, not what they used!
Jonathan
>
> Cheers,
> Vasilis
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 07/10] iio: pressure: bmp280: Introduce new cleanup routines
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (5 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 06/10] iio: pressure: bmp280: Refactorize reading functions Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 19:22 ` Jonathan Cameron
2024-04-29 19:00 ` [PATCH v5 08/10] iio: pressure: bmp280: Generalize read_{temp,press,humid}() functions Vasileios Amoiridis
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis, Jonathan Cameron
Introduce new linux/cleanup.h with the guard(mutex) functionality
in the {read,write}_raw() functions.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 125 +++++++++++++----------------
1 file changed, 54 insertions(+), 71 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 5ebce38e99f6..f14277bb882d 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>
@@ -501,77 +502,67 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
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 bmp280_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);
+static int bmp280_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 = bmp280_read_raw_impl(indio_dev, chan, val, val2, mask);
pm_runtime_mark_last_busy(data->dev);
pm_runtime_put_autosuspend(data->dev);
@@ -703,12 +694,13 @@ 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 bmp280_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.
@@ -718,45 +710,36 @@ static int bmp280_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 = bme280_write_oversampling_ratio_humid(data, val);
- break;
+ return bme280_write_oversampling_ratio_humid(data, val);
case IIO_PRESSURE:
- ret = bmp280_write_oversampling_ratio_press(data, val);
- break;
+ return bmp280_write_oversampling_ratio_press(data, val);
case IIO_TEMP:
- ret = bmp280_write_oversampling_ratio_temp(data, val);
- break;
+ return bmp280_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 = bmp280_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 bmp280_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 = bmp280_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 bmp280_write_iir_filter_coeffs(data, val);
default:
return -EINVAL;
}
+}
+
+static int bmp280_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 = bmp280_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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 07/10] iio: pressure: bmp280: Introduce new cleanup routines
2024-04-29 19:00 ` [PATCH v5 07/10] iio: pressure: bmp280: Introduce new cleanup routines Vasileios Amoiridis
@ 2024-05-05 19:22 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 19:22 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Jonathan Cameron
On Mon, 29 Apr 2024 21:00:43 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> Introduce new linux/cleanup.h with the guard(mutex) functionality
> in the {read,write}_raw() functions.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Looks good to me.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 08/10] iio: pressure: bmp280: Generalize read_{temp,press,humid}() functions
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (6 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 07/10] iio: pressure: bmp280: Introduce new cleanup routines Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 09/10] iio: pressure: bmp280: Add SCALE, RAW values in channels and refactorize them Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support Vasileios Amoiridis
9 siblings, 0 replies; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
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 <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 169 ++++++++++++++++-------------
drivers/iio/pressure/bmp280.h | 13 ++-
2 files changed, 102 insertions(+), 80 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index f14277bb882d..668c4e8a2ebf 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -439,28 +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 < 0)
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);
@@ -471,19 +466,14 @@ static int bmp280_read_press(struct bmp280_data *data,
if (ret < 0)
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);
@@ -494,12 +484,9 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2)
if (ret < 0)
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 bmp280_read_raw_impl(struct iio_dev *indio_dev,
@@ -507,6 +494,8 @@ static int bmp280_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);
@@ -514,11 +503,29 @@ static int bmp280_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 < 0)
+ 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 < 0)
+ 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 < 0)
+ 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;
}
@@ -818,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,
@@ -846,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,
@@ -873,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,
@@ -895,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,
@@ -1083,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;
@@ -1093,16 +1114,14 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
if (ret < 0)
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);
@@ -1113,13 +1132,9 @@ static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
if (ret < 0)
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)
@@ -1290,6 +1305,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,
@@ -1317,6 +1334,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,
@@ -1437,9 +1459,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,
@@ -1449,25 +1470,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,
@@ -1477,18 +1490,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] = {
@@ -1804,6 +1812,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,
@@ -1830,6 +1840,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,
@@ -1985,20 +2000,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 < 0)
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)
@@ -2061,10 +2074,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);
@@ -2075,13 +2087,9 @@ static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
if (ret < 0)
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)
@@ -2092,6 +2100,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,
@@ -2112,6 +2122,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 7c30e4d523be..a3d2cd722760 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -446,10 +446,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v5 09/10] iio: pressure: bmp280: Add SCALE, RAW values in channels and refactorize them
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (7 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 08/10] iio: pressure: bmp280: Generalize read_{temp,press,humid}() functions Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-04-29 19:00 ` [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support Vasileios Amoiridis
9 siblings, 0 replies; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
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 <vassilisamir@gmail.com>
---
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 668c4e8a2ebf..e349ed87aad6 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 bmp280_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 < 0)
+ return ret;
+
+ *val = chan_value;
+ return IIO_VAL_INT;
+ case IIO_PRESSURE:
+ ret = data->chip_info->read_press(data, &chan_value);
+ if (ret < 0)
+ return ret;
+
+ *val = chan_value;
+ return IIO_VAL_INT;
+ case IIO_TEMP:
+ ret = data->chip_info->read_temp(data, &chan_value);
+ if (ret < 0)
+ 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
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support
2024-04-29 19:00 [PATCH v5 00/10] iio: pressure: bmp280: Driver cleanup and add triggered buffer support Vasileios Amoiridis
` (8 preceding siblings ...)
2024-04-29 19:00 ` [PATCH v5 09/10] iio: pressure: bmp280: Add SCALE, RAW values in channels and refactorize them Vasileios Amoiridis
@ 2024-04-29 19:00 ` Vasileios Amoiridis
2024-05-05 19:34 ` Jonathan Cameron
9 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-04-29 19:00 UTC (permalink / raw)
To: jic23
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel, Vasileios Amoiridis
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 <ang.iglesiasg@gmail.com>
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/Kconfig | 2 +
drivers/iio/pressure/bmp280-core.c | 335 +++++++++++++++++++++++++++--
drivers/iio/pressure/bmp280-spi.c | 8 +-
drivers/iio/pressure/bmp280.h | 21 +-
4 files changed, 344 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 e349ed87aad6..2feadb56835f 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 < 0) {
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 < 0) {
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 < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -866,6 +927,16 @@ static const struct iio_info bmp280_info = {
.write_raw = &bmp280_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,73 @@ 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 < 0) {
+ 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 +1043,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 +1073,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 +1107,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 +1134,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 +1196,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 < 0) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
@@ -1119,7 +1263,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 < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -1372,6 +1516,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 < 0) {
+ 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 };
@@ -1386,7 +1576,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),
@@ -1414,6 +1605,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);
@@ -1533,8 +1726,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 < 0) {
dev_err(data->dev, "failed to read temperature\n");
return ret;
@@ -1553,8 +1746,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 < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -1880,6 +2073,50 @@ 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);
+
+ /* Burst read data registers */
+ ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB,
+ data->buf, BMP280_BURST_READ_BYTES);
+ if (ret < 0) {
+ 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 };
@@ -1892,7 +2129,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),
@@ -1919,6 +2157,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);
@@ -2097,7 +2337,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 < 0) {
dev_err(data->dev, "failed to read pressure\n");
return ret;
@@ -2167,6 +2407,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 < 0)
+ return IRQ_HANDLED;
+
+ data->sensor_data[1] = chan_value;
+
+ ret = bmp180_read_press(data, &chan_value);
+ if (ret < 0)
+ 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 };
@@ -2180,7 +2449,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 =
@@ -2201,6 +2471,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);
@@ -2246,6 +2518,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;
@@ -2292,6 +2588,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;
@@ -2377,6 +2674,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 < 0)
+ 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 62b4e58104cf..dd1127d493d3 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[BME280_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 a3d2cd722760..756c644354c2 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -304,6 +304,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[] = {
@@ -397,13 +407,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[6] __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[BME280_BURST_READ_BYTES];
/* Calibration data buffers */
__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
@@ -425,6 +441,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;
@@ -459,6 +476,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
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support
2024-04-29 19:00 ` [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support Vasileios Amoiridis
@ 2024-05-05 19:34 ` Jonathan Cameron
2024-05-05 23:57 ` Vasileios Amoiridis
0 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-05 19:34 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 29 Apr 2024 21:00:46 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> 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 <ang.iglesiasg@gmail.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Hi Vasileois,
Just one question on this inline. (patches 8 and 9 look good to me)
For v6, only need to send the patches that I haven't already applied.
Thanks,
Jonathan
>
> +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 < 0)
> + return IRQ_HANDLED;
> +
> + data->sensor_data[1] = chan_value;
> +
> + ret = bmp180_read_press(data, &chan_value);
So I 'think' that after all the refactoring you end up reading the temperature
twice. To avoid that you need to pull the read_temp() and read_press()
function implementations here and only do the (currently duplicated) steps once.
You seem to have done this for the other case, but missed the bmp180?
Maybe I'm missing some reason it doesn't work for this one!
> + if (ret < 0)
> + 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;
> +}
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support
2024-05-05 19:34 ` Jonathan Cameron
@ 2024-05-05 23:57 ` Vasileios Amoiridis
2024-05-06 12:50 ` Jonathan Cameron
0 siblings, 1 reply; 25+ messages in thread
From: Vasileios Amoiridis @ 2024-05-05 23:57 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Vasileios Amoiridis, lars, andriy.shevchenko, ang.iglesiasg,
mazziesaccount, ak, petre.rodan, phil, 579lpy, linus.walleij,
semen.protsenko, linux-iio, linux-kernel
On Sun, May 05, 2024 at 08:34:56PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:46 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> 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 <ang.iglesiasg@gmail.com>
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Hi Vasileois,
>
> Just one question on this inline. (patches 8 and 9 look good to me)
>
> For v6, only need to send the patches that I haven't already applied.
>
> Thanks,
>
> Jonathan
>
> >
> > +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 < 0)
> > + return IRQ_HANDLED;
> > +
> > + data->sensor_data[1] = chan_value;
> > +
> > + ret = bmp180_read_press(data, &chan_value);
>
> So I 'think' that after all the refactoring you end up reading the temperature
> twice. To avoid that you need to pull the read_temp() and read_press()
> function implementations here and only do the (currently duplicated) steps once.
>
> You seem to have done this for the other case, but missed the bmp180?
> Maybe I'm missing some reason it doesn't work for this one!
>
Hi Jonathan!
So, I didn't miss it. This is an old sensor and in order to get data out, the
procedure is much more constrained. As you can see in the datasheet [1] in page
11 there is a well defined process on how to read the data out. It's not
possible to make a burst read here. Hence, the strange bmp180_measure() function
in order to wait for an EOC before reading the values. Indeed I am reading the
temperature 2 times which is not optimal but in order to read both of them I
would have to:
a) either get the temperature out of the bmp180_read_press() function
(which would ruin a bit consistency with the other bmpxxx_read_press() functions)
b) make a bmp180_get_sensor_data() which would look like bmp180_get_press() but
also gives temperature (which won't look that good).
That's why I didn't touch it. If you think it makes more sense to do it, I can
follow one of the 2 approaches, whichever you think would make more sense.
Cheers,
Vasilis
[1]: https://cdn-shop.adafruit.com/datasheets/BST-BMP180-DS000-09.pdf
> > + if (ret < 0)
> > + 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;
> > +}
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v5 10/10] iio: pressure: bmp280: Add triggered buffer support
2024-05-05 23:57 ` Vasileios Amoiridis
@ 2024-05-06 12:50 ` Jonathan Cameron
0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2024-05-06 12:50 UTC (permalink / raw)
To: Vasileios Amoiridis
Cc: lars, andriy.shevchenko, ang.iglesiasg, mazziesaccount, ak,
petre.rodan, phil, 579lpy, linus.walleij, semen.protsenko,
linux-iio, linux-kernel
On Mon, 6 May 2024 01:57:55 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> On Sun, May 05, 2024 at 08:34:56PM +0100, Jonathan Cameron wrote:
> > On Mon, 29 Apr 2024 21:00:46 +0200
> > Vasileios Amoiridis <vassilisamir@gmail.com> 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 <ang.iglesiasg@gmail.com>
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > Hi Vasileois,
> >
> > Just one question on this inline. (patches 8 and 9 look good to me)
> >
> > For v6, only need to send the patches that I haven't already applied.
> >
> > Thanks,
> >
> > Jonathan
> >
> > >
> > > +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 < 0)
> > > + return IRQ_HANDLED;
> > > +
> > > + data->sensor_data[1] = chan_value;
> > > +
> > > + ret = bmp180_read_press(data, &chan_value);
> >
> > So I 'think' that after all the refactoring you end up reading the temperature
> > twice. To avoid that you need to pull the read_temp() and read_press()
> > function implementations here and only do the (currently duplicated) steps once.
> >
> > You seem to have done this for the other case, but missed the bmp180?
> > Maybe I'm missing some reason it doesn't work for this one!
> >
>
> Hi Jonathan!
>
> So, I didn't miss it. This is an old sensor and in order to get data out, the
> procedure is much more constrained. As you can see in the datasheet [1] in page
> 11 there is a well defined process on how to read the data out. It's not
> possible to make a burst read here. Hence, the strange bmp180_measure() function
> in order to wait for an EOC before reading the values. Indeed I am reading the
> temperature 2 times which is not optimal but in order to read both of them I
> would have to:
>
> a) either get the temperature out of the bmp180_read_press() function
> (which would ruin a bit consistency with the other bmpxxx_read_press() functions)
>
> b) make a bmp180_get_sensor_data() which would look like bmp180_get_press() but
> also gives temperature (which won't look that good).
>
> That's why I didn't touch it. If you think it makes more sense to do it, I can
> follow one of the 2 approaches, whichever you think would make more sense.
Ok. As you say, old sensor so fine to not optimize it. If anyone else cares
they can do it ;)
Jonathan
>
> Cheers,
> Vasilis
>
> [1]: https://cdn-shop.adafruit.com/datasheets/BST-BMP180-DS000-09.pdf
>
> > > + if (ret < 0)
> > > + 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;
> > > +}
^ permalink raw reply [flat|nested] 25+ messages in thread