Received-SPF: pass (google.com: domain of linux-kernel+bounces-206459-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Vasileios Amoiridis <vassilisamir@gmail.com>
Date: Fri, 7 Jun 2024 18:11:01 +0200
To: Vasileios Amoiridis <vassilisamir@gmail.com>
Cc: jic23@kernel.org, dpfrey@gmail.com, himanshujha199640@gmail.com,
	lars@metafoo.de, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, mike.looijmans@topic.nl
Subject: Re: [PATCH v2 13/19] iio: chemical: bme680: Add read buffers in DMA
 safe region
Message-ID: <20240607161101.GA258325@vamoiridPC>
References: <20240606212313.207550-1-vassilisamir@gmail.com>
 <20240606212313.207550-14-vassilisamir@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240606212313.207550-14-vassilisamir@gmail.com>

On Thu, Jun 06, 2024 at 11:23:05PM +0200, Vasileios Amoiridis 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.
> 

Well this commit shouldn't have been here, I changed the title and
forgot to delete it from the folder...

Vasilis

> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/chemical/bme680.h      |  7 +++++
>  drivers/iio/chemical/bme680_core.c | 45 +++++++++++++++---------------
>  2 files changed, 29 insertions(+), 23 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 538696ac4205..99259d0cf13d 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];
> @@ -453,7 +456,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) +
> @@ -466,16 +468,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;
>  	}
> @@ -564,7 +566,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;
>  
> @@ -578,13 +579,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");
> @@ -610,7 +612,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 */
> @@ -619,13 +620,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");
> @@ -642,7 +644,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;
>  
> @@ -652,13 +653,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");
> @@ -676,8 +677,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;
>  
> @@ -697,19 +696,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);
>  
>  	/*
> @@ -838,7 +838,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));
> @@ -869,15 +868,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
>