Move the buffers that are used in order to read data from the
device in a DMA-safe region. 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 <vassilisamir@gmail.com>
---
 drivers/iio/chemical/bme680.h      |  7 ++++++
 drivers/iio/chemical/bme680_core.c | 37 +++++++++++++++---------------
 2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
index 5f602170a3af..17ea59253923 100644
--- a/drivers/iio/chemical/bme680.h
+++ b/drivers/iio/chemical/bme680.h
@@ -54,6 +54,13 @@
 #define BME680_REG_MEAS_STAT_0			0x1D
 #define   BME680_GAS_MEAS_BIT			BIT(6)
 
+#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 a6f425076d36..b055eeee8f1c 100644
--- a/drivers/iio/chemical/bme680_core.c
+++ b/drivers/iio/chemical/bme680_core.c
@@ -115,6 +115,9 @@ struct bme680_data {
 	 * transfer buffers to live in their own cache lines.
 	 */
 	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];
@@ -547,7 +550,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;
 
@@ -559,13 +561,14 @@ static int bme680_read_temp(struct bme680_data *data, int *val)
 	bme680_wait_for_eoc(data);
 
 	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");
@@ -591,7 +594,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 */
@@ -600,13 +602,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");
@@ -623,7 +626,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;
 
@@ -633,13 +635,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");
@@ -657,8 +659,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;
 
@@ -676,19 +676,19 @@ static int bme680_read_gas(struct bme680_data *data,
 
 	bme680_wait_for_eoc(data);
 
-	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 = gas_regs_val >> BME680_ADC_GAS_RES_SHIFT;
 
 	/*
@@ -817,7 +817,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));
@@ -848,15 +847,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

On Mon, 27 May 2024 20:38:01 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Move the buffers that are used in order to read data from the
> device in a DMA-safe region. 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 <vassilisamir@gmail.com>

Same response as previous.  I don't think it's necessary because
of the custom regmap implementation.

My first instinct was the same as yours though!

Jonathan

On Sun, Jun 02, 2024 at 01:59:08PM +0100, Jonathan Cameron wrote:
> On Mon, 27 May 2024 20:38:01 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Move the buffers that are used in order to read data from the
> > device in a DMA-safe region. 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 <vassilisamir@gmail.com>
> 
> Same response as previous.  I don't think it's necessary because
> of the custom regmap implementation.
> 
> My first instinct was the same as yours though!
> 
> Jonathan
> 
> 
Well, even if we end up not needing it, I would keep the values inside
the union just becasue it saves some space, and it keeps all the read
buffers in the same place. What do you think?

Cheers,
Vasilis

On Sun, 2 Jun 2024 21:33:10 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> On Sun, Jun 02, 2024 at 01:59:08PM +0100, Jonathan Cameron wrote:
> > On Mon, 27 May 2024 20:38:01 +0200
> > Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> >   
> > > Move the buffers that are used in order to read data from the
> > > device in a DMA-safe region. 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 <vassilisamir@gmail.com>  
> > 
> > Same response as previous.  I don't think it's necessary because
> > of the custom regmap implementation.
> > 
> > My first instinct was the same as yours though!
> > 
> > Jonathan
> > 
> >   
> Well, even if we end up not needing it, I would keep the values inside
> the union just becasue it saves some space, and it keeps all the read
> buffers in the same place. What do you think?
> 
Sure. The union is fine as long as you have made sure there can't be
concurrent access to the different members. It will save space on x86_64
at least as that has very low IIO_DMA_MINALIGN.  Less likely to make
a difference on arm64.