2024-06-06 21:28:30

by Vasileios Amoiridis

[permalink] [raw]
Subject: [PATCH v2 13/19] iio: chemical: bme680: Add read buffers in union

Move the buffers that are used in order to read data from the
device in the union which handles all the device read/write
buffers. Also create defines for the number of bytes that are
being read from the device and don't use magic numbers.

Signed-off-by: Vasileios Amoiridis <[email protected]>
---
drivers/iio/chemical/bme680.h | 7 +++++
drivers/iio/chemical/bme680_core.c | 47 +++++++++++++++---------------
2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index 8d0f53c05d7d..7d0ff294725a 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -56,6 +56,13 @@
#define BME680_GAS_MEAS_BIT BIT(6)
#define BME680_MEAS_BIT BIT(5)

+#define BME680_TEMP_NUM_BYTES 3
+#define BME680_PRESS_NUM_BYTES 3
+#define BME680_HUMID_NUM_BYTES 2
+#define BME680_GAS_NUM_BYTES 2
+
+#define BME680_MEAS_TRIM_MASK GENMASK(24, 4)
+
/* Calibration Parameters */
#define BME680_T2_LSB_REG 0x8A
#define BME680_H2_MSB_REG 0xE1
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
index b13797f7d873..3c33c21b5d6a 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -111,10 +111,13 @@ struct bme680_data {
s32 t_fine;

union {
+ u8 buf[3];
+ unsigned int check;
+ __be16 be16;
u8 bme680_cal_buf_1[BME680_CALIB_RANGE_1_LEN];
u8 bme680_cal_buf_2[BME680_CALIB_RANGE_2_LEN];
u8 bme680_cal_buf_3[BME680_CALIB_RANGE_3_LEN];
- };
+ };
};

static const struct regmap_range bme680_volatile_ranges[] = {
@@ -449,7 +452,6 @@ static u8 bme680_oversampling_to_reg(u8 val)
static int bme680_wait_for_eoc(struct bme680_data *data)
{
struct device *dev = regmap_get_device(data->regmap);
- unsigned int check;
int ret;
/*
* (Sum of oversampling ratios * time per oversampling) +
@@ -462,16 +464,16 @@ static int bme680_wait_for_eoc(struct bme680_data *data)

usleep_range(wait_eoc_us, wait_eoc_us + 100);

- ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
+ ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
if (ret) {
dev_err(dev, "failed to read measurement status register.\n");
return ret;
}
- if (check & BME680_MEAS_BIT) {
+ if (data->check & BME680_MEAS_BIT) {
dev_err(dev, "Device measurement cycle incomplete.\n");
return -EBUSY;
}
- if (!(check & BME680_NEW_DATA_BIT)) {
+ if (!(data->check & BME680_NEW_DATA_BIT)) {
dev_err(dev, "No new data available from the device.\n");
return -ENODATA;
}
@@ -560,7 +562,6 @@ static int bme680_read_temp(struct bme680_data *data, int *val)
{
struct device *dev = regmap_get_device(data->regmap);
int ret;
- __be32 tmp = 0;
u32 adc_temp;
s16 comp_temp;

@@ -574,13 +575,14 @@ static int bme680_read_temp(struct bme680_data *data, int *val)
return ret;

ret = regmap_bulk_read(data->regmap, BME680_REG_TEMP_MSB,
- &tmp, 3);
+ data->buf, BME680_TEMP_NUM_BYTES);
if (ret < 0) {
dev_err(dev, "failed to read temperature\n");
return ret;
}

- adc_temp = be32_to_cpu(tmp) >> 12;
+ adc_temp = FIELD_GET(BME680_MEAS_TRIM_MASK,
+ get_unaligned_be24(data->buf));
if (adc_temp == BME680_MEAS_SKIPPED) {
/* reading was skipped */
dev_err(dev, "reading temperature skipped\n");
@@ -606,7 +608,6 @@ static int bme680_read_press(struct bme680_data *data,
{
struct device *dev = regmap_get_device(data->regmap);
int ret;
- __be32 tmp = 0;
u32 adc_press;

/* Read and compensate temperature to get a reading of t_fine */
@@ -615,13 +616,14 @@ static int bme680_read_press(struct bme680_data *data,
return ret;

ret = regmap_bulk_read(data->regmap, BME680_REG_PRESS_MSB,
- &tmp, 3);
+ data->buf, BME680_PRESS_NUM_BYTES);
if (ret < 0) {
dev_err(dev, "failed to read pressure\n");
return ret;
}

- adc_press = be32_to_cpu(tmp) >> 12;
+ adc_press = FIELD_GET(BME680_MEAS_TRIM_MASK,
+ get_unaligned_be24(data->buf));
if (adc_press == BME680_MEAS_SKIPPED) {
/* reading was skipped */
dev_err(dev, "reading pressure skipped\n");
@@ -638,7 +640,6 @@ static int bme680_read_humid(struct bme680_data *data,
{
struct device *dev = regmap_get_device(data->regmap);
int ret;
- __be16 tmp = 0;
u16 adc_humidity;
u32 comp_humidity;

@@ -648,13 +649,13 @@ static int bme680_read_humid(struct bme680_data *data,
return ret;

ret = regmap_bulk_read(data->regmap, BME680_REG_HUMIDITY_MSB,
- &tmp, sizeof(tmp));
+ &data->be16, BME680_HUMID_NUM_BYTES);
if (ret < 0) {
dev_err(dev, "failed to read humidity\n");
return ret;
}

- adc_humidity = be16_to_cpu(tmp);
+ adc_humidity = be16_to_cpu(data->be16);
if (adc_humidity == BME680_MEAS_SKIPPED) {
/* reading was skipped */
dev_err(dev, "reading humidity skipped\n");
@@ -672,8 +673,6 @@ static int bme680_read_gas(struct bme680_data *data,
{
struct device *dev = regmap_get_device(data->regmap);
int ret;
- __be16 tmp = 0;
- unsigned int check;
u16 adc_gas_res, gas_regs_val;
u8 gas_range;

@@ -693,19 +692,20 @@ static int bme680_read_gas(struct bme680_data *data,
if (ret)
return ret;

- ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
- if (check & BME680_GAS_MEAS_BIT) {
+ ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
+ if (data->check & BME680_GAS_MEAS_BIT) {
dev_err(dev, "gas measurement incomplete\n");
return -EBUSY;
}

ret = regmap_bulk_read(data->regmap, BME680_REG_GAS_MSB,
- &tmp, sizeof(tmp));
+ &data->be16, BME680_GAS_NUM_BYTES);
if (ret < 0) {
dev_err(dev, "failed to read gas resistance\n");
return ret;
}
- gas_regs_val = be16_to_cpu(tmp);
+
+ gas_regs_val = be16_to_cpu(data->be16);
adc_gas_res = FIELD_GET(BME680_ADC_GAS_RES, gas_regs_val);

/*
@@ -834,7 +834,6 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
{
struct iio_dev *indio_dev;
struct bme680_data *data;
- unsigned int val;
int ret;

indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -865,15 +864,15 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
return ret;
}

- ret = regmap_read(regmap, BME680_REG_CHIP_ID, &val);
+ ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
if (ret < 0) {
dev_err(dev, "Error reading chip ID\n");
return ret;
}

- if (val != BME680_CHIP_ID_VAL) {
+ if (data->check != BME680_CHIP_ID_VAL) {
dev_err(dev, "Wrong chip ID, got %x expected %x\n",
- val, BME680_CHIP_ID_VAL);
+ data->check, BME680_CHIP_ID_VAL);
return -ENODEV;
}

--
2.25.1



2024-06-07 16:12:41

by Vasileios Amoiridis

[permalink] [raw]
Subject: Re: [PATCH v2 13/19] iio: chemical: bme680: Add read buffers in union

On Thu, Jun 06, 2024 at 11:23:07PM +0200, Vasileios Amoiridis wrote:
> Move the buffers that are used in order to read data from the
> device in the union which handles all the device read/write
> buffers. Also create defines for the number of bytes that are
> being read from the device and don't use magic numbers.
>

Also this shouldn't have been here for the same reason. Ah, should
have been more careful...

The rest are fine though, so let me know and I submit a v3 with any
new potential changes.

Vasilis

> Signed-off-by: Vasileios Amoiridis <[email protected]>
> ---
> drivers/iio/chemical/bme680.h | 7 +++++
> drivers/iio/chemical/bme680_core.c | 47 +++++++++++++++---------------
> 2 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> index 8d0f53c05d7d..7d0ff294725a 100644
> --- a/drivers/iio/chemical/bme680.h
> +++ b/drivers/iio/chemical/bme680.h
> @@ -56,6 +56,13 @@
> #define BME680_GAS_MEAS_BIT BIT(6)
> #define BME680_MEAS_BIT BIT(5)
>
> +#define BME680_TEMP_NUM_BYTES 3
> +#define BME680_PRESS_NUM_BYTES 3
> +#define BME680_HUMID_NUM_BYTES 2
> +#define BME680_GAS_NUM_BYTES 2
> +
> +#define BME680_MEAS_TRIM_MASK GENMASK(24, 4)
> +
> /* Calibration Parameters */
> #define BME680_T2_LSB_REG 0x8A
> #define BME680_H2_MSB_REG 0xE1
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> index b13797f7d873..3c33c21b5d6a 100644
> --- a/drivers/iio/chemical/bme680_core.c
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -111,10 +111,13 @@ struct bme680_data {
> s32 t_fine;
>
> union {
> + u8 buf[3];
> + unsigned int check;
> + __be16 be16;
> u8 bme680_cal_buf_1[BME680_CALIB_RANGE_1_LEN];
> u8 bme680_cal_buf_2[BME680_CALIB_RANGE_2_LEN];
> u8 bme680_cal_buf_3[BME680_CALIB_RANGE_3_LEN];
> - };
> + };
> };
>
> static const struct regmap_range bme680_volatile_ranges[] = {
> @@ -449,7 +452,6 @@ static u8 bme680_oversampling_to_reg(u8 val)
> static int bme680_wait_for_eoc(struct bme680_data *data)
> {
> struct device *dev = regmap_get_device(data->regmap);
> - unsigned int check;
> int ret;
> /*
> * (Sum of oversampling ratios * time per oversampling) +
> @@ -462,16 +464,16 @@ static int bme680_wait_for_eoc(struct bme680_data *data)
>
> usleep_range(wait_eoc_us, wait_eoc_us + 100);
>
> - ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
> + ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
> if (ret) {
> dev_err(dev, "failed to read measurement status register.\n");
> return ret;
> }
> - if (check & BME680_MEAS_BIT) {
> + if (data->check & BME680_MEAS_BIT) {
> dev_err(dev, "Device measurement cycle incomplete.\n");
> return -EBUSY;
> }
> - if (!(check & BME680_NEW_DATA_BIT)) {
> + if (!(data->check & BME680_NEW_DATA_BIT)) {
> dev_err(dev, "No new data available from the device.\n");
> return -ENODATA;
> }
> @@ -560,7 +562,6 @@ static int bme680_read_temp(struct bme680_data *data, int *val)
> {
> struct device *dev = regmap_get_device(data->regmap);
> int ret;
> - __be32 tmp = 0;
> u32 adc_temp;
> s16 comp_temp;
>
> @@ -574,13 +575,14 @@ static int bme680_read_temp(struct bme680_data *data, int *val)
> return ret;
>
> ret = regmap_bulk_read(data->regmap, BME680_REG_TEMP_MSB,
> - &tmp, 3);
> + data->buf, BME680_TEMP_NUM_BYTES);
> if (ret < 0) {
> dev_err(dev, "failed to read temperature\n");
> return ret;
> }
>
> - adc_temp = be32_to_cpu(tmp) >> 12;
> + adc_temp = FIELD_GET(BME680_MEAS_TRIM_MASK,
> + get_unaligned_be24(data->buf));
> if (adc_temp == BME680_MEAS_SKIPPED) {
> /* reading was skipped */
> dev_err(dev, "reading temperature skipped\n");
> @@ -606,7 +608,6 @@ static int bme680_read_press(struct bme680_data *data,
> {
> struct device *dev = regmap_get_device(data->regmap);
> int ret;
> - __be32 tmp = 0;
> u32 adc_press;
>
> /* Read and compensate temperature to get a reading of t_fine */
> @@ -615,13 +616,14 @@ static int bme680_read_press(struct bme680_data *data,
> return ret;
>
> ret = regmap_bulk_read(data->regmap, BME680_REG_PRESS_MSB,
> - &tmp, 3);
> + data->buf, BME680_PRESS_NUM_BYTES);
> if (ret < 0) {
> dev_err(dev, "failed to read pressure\n");
> return ret;
> }
>
> - adc_press = be32_to_cpu(tmp) >> 12;
> + adc_press = FIELD_GET(BME680_MEAS_TRIM_MASK,
> + get_unaligned_be24(data->buf));
> if (adc_press == BME680_MEAS_SKIPPED) {
> /* reading was skipped */
> dev_err(dev, "reading pressure skipped\n");
> @@ -638,7 +640,6 @@ static int bme680_read_humid(struct bme680_data *data,
> {
> struct device *dev = regmap_get_device(data->regmap);
> int ret;
> - __be16 tmp = 0;
> u16 adc_humidity;
> u32 comp_humidity;
>
> @@ -648,13 +649,13 @@ static int bme680_read_humid(struct bme680_data *data,
> return ret;
>
> ret = regmap_bulk_read(data->regmap, BME680_REG_HUMIDITY_MSB,
> - &tmp, sizeof(tmp));
> + &data->be16, BME680_HUMID_NUM_BYTES);
> if (ret < 0) {
> dev_err(dev, "failed to read humidity\n");
> return ret;
> }
>
> - adc_humidity = be16_to_cpu(tmp);
> + adc_humidity = be16_to_cpu(data->be16);
> if (adc_humidity == BME680_MEAS_SKIPPED) {
> /* reading was skipped */
> dev_err(dev, "reading humidity skipped\n");
> @@ -672,8 +673,6 @@ static int bme680_read_gas(struct bme680_data *data,
> {
> struct device *dev = regmap_get_device(data->regmap);
> int ret;
> - __be16 tmp = 0;
> - unsigned int check;
> u16 adc_gas_res, gas_regs_val;
> u8 gas_range;
>
> @@ -693,19 +692,20 @@ static int bme680_read_gas(struct bme680_data *data,
> if (ret)
> return ret;
>
> - ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
> - if (check & BME680_GAS_MEAS_BIT) {
> + ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &data->check);
> + if (data->check & BME680_GAS_MEAS_BIT) {
> dev_err(dev, "gas measurement incomplete\n");
> return -EBUSY;
> }
>
> ret = regmap_bulk_read(data->regmap, BME680_REG_GAS_MSB,
> - &tmp, sizeof(tmp));
> + &data->be16, BME680_GAS_NUM_BYTES);
> if (ret < 0) {
> dev_err(dev, "failed to read gas resistance\n");
> return ret;
> }
> - gas_regs_val = be16_to_cpu(tmp);
> +
> + gas_regs_val = be16_to_cpu(data->be16);
> adc_gas_res = FIELD_GET(BME680_ADC_GAS_RES, gas_regs_val);
>
> /*
> @@ -834,7 +834,6 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> {
> struct iio_dev *indio_dev;
> struct bme680_data *data;
> - unsigned int val;
> int ret;
>
> indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> @@ -865,15 +864,15 @@ int bme680_core_probe(struct device *dev, struct regmap *regmap,
> return ret;
> }
>
> - ret = regmap_read(regmap, BME680_REG_CHIP_ID, &val);
> + ret = regmap_read(regmap, BME680_REG_CHIP_ID, &data->check);
> if (ret < 0) {
> dev_err(dev, "Error reading chip ID\n");
> return ret;
> }
>
> - if (val != BME680_CHIP_ID_VAL) {
> + if (data->check != BME680_CHIP_ID_VAL) {
> dev_err(dev, "Wrong chip ID, got %x expected %x\n",
> - val, BME680_CHIP_ID_VAL);
> + data->check, BME680_CHIP_ID_VAL);
> return -ENODEV;
> }
>
> --
> 2.25.1
>