linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
@ 2018-07-20 21:31 Himanshu Jha
  2018-07-21 15:19 ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: Himanshu Jha @ 2018-07-20 21:31 UTC (permalink / raw)
  To: jic23
  Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, Himanshu Jha,
	Daniel Baluta

Bosch BME680 is a 4-in-1 sensor with temperature, pressure, humidity
and gas sensing capability. It supports both I2C and SPI communication
protocol for effective data communication.

The device supports two modes:

1. Sleep mode
2. Forced mode

The measurements only takes place when forced mode is triggered and a
single TPHG cycle is performed by the sensor. The sensor automatically
goes to sleep after afterwards.

The device has various calibration constants/parameters programmed into
devices' non-volatile memory(NVM) during production and can't be altered
by the user. These constants are used in the compensation functions to
get the required compensated readings along with the raw data. The
compensation functions/algorithms are provided by Bosch Sensortec GmbH
via their API[1]. As these don't change during the measurement cycle,
therefore we read and store them at the probe. The default configs
supplied by Bosch are also set at probe.

0-day tested with build success.

GSoC-2018: https://summerofcode.withgoogle.com/projects/#6691473790074880
Mentor: Daniel Baluta
[1] https://github.com/BoschSensortec/BME680_driver
Datasheet:
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---

v4:
   -Added Bosch API links and datasheet link.
   -explained with comments about the compensate functions.
   -fixed SPI ID register address.
   -Used FIELD_PREP macro to avoid unnecessary hard coding.
   -Simplified Kconfig to remove unnecessary parentheses.

v3:
   -moved files to chemical directory instead of a dedicated directory.
   -read calibration parameters serially with endian conversions.
   -drop some return ret. 
   -removed few unnecessary casts safely.
   -added 'u' suffix to explicitly specify unsigned for large values
    and thereby fixing comiler warning.
   -left aligned all comments.
   -added a comment explaining heater stability failure.

v2:
   -Used devm_add_action() to add a generic remove method for
    both I2C & SPI driver.
   -Introduction of compensation functions.
   -chip initialisation routines moved to respective I2C and SPI
    driver.
   -Introduction of gas sensing rountines.
   -Simplified Kconfig to reduce various options.

 drivers/iio/chemical/Kconfig       |  23 +
 drivers/iio/chemical/Makefile      |   3 +
 drivers/iio/chemical/bme680.h      |  96 ++++
 drivers/iio/chemical/bme680_core.c | 976 +++++++++++++++++++++++++++++++++++++
 drivers/iio/chemical/bme680_i2c.c  |  83 ++++
 drivers/iio/chemical/bme680_spi.c  | 123 +++++
 6 files changed, 1304 insertions(+)
 create mode 100644 drivers/iio/chemical/bme680.h
 create mode 100644 drivers/iio/chemical/bme680_core.c
 create mode 100644 drivers/iio/chemical/bme680_i2c.c
 create mode 100644 drivers/iio/chemical/bme680_spi.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 5cb5be7..b8e005b 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -21,6 +21,29 @@ config ATLAS_PH_SENSOR
 	 To compile this driver as module, choose M here: the
 	 module will be called atlas-ph-sensor.
 
+config BME680
+	tristate "Bosch Sensortec BME680 sensor driver"
+	depends on (I2C || SPI)
+	select REGMAP
+	select BME680_I2C if I2C
+	select BME680_SPI if SPI
+	help
+	  Say yes here to build support for Bosch Sensortec BME680 sensor with
+	  temperature, pressure, humidity and gas sensing capability.
+
+	  This driver can also be built as a module. If so, the module for I2C
+	  would be called bme680_i2c and bme680_spi for SPI support.
+
+config BME680_I2C
+	tristate
+	depends on I2C && BME680
+	select REGMAP_I2C
+
+config BME680_SPI
+	tristate
+	depends on SPI && BME680
+	select REGMAP_SPI
+
 config CCS811
 	tristate "AMS CCS811 VOC sensor"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index a629b29..2f4c4ba 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -4,6 +4,9 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
+obj-$(CONFIG_BME680) += bme680_core.o
+obj-$(CONFIG_BME680_I2C) += bme680_i2c.o
+obj-$(CONFIG_BME680_SPI) += bme680_spi.o
 obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
new file mode 100644
index 0000000..b872f66
--- /dev/null
+++ b/drivers/iio/chemical/bme680.h
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef BME680_H_
+#define BME680_H_
+
+#define BME680_REG_CHIP_I2C_ID			0xD0
+#define BME680_REG_CHIP_SPI_ID			0x50
+#define BME680_CHIP_ID_VAL			0x61
+#define BME680_REG_SOFT_RESET_I2C		0xE0
+#define BME680_REG_SOFT_RESET_SPI		0x60
+#define BME680_CMD_SOFTRESET			0xB6
+#define BME680_REG_STATUS			0x73
+#define   BME680_SPI_MEM_PAGE_BIT		BIT(4)
+#define     BME680_SPI_MEM_PAGE_1_VAL		1
+
+#define BME680_REG_TEMP_MSB			0x22
+#define BME680_REG_PRESS_MSB			0x1F
+#define BM6880_REG_HUMIDITY_MSB			0x25
+#define BME680_REG_GAS_MSB			0x2A
+#define BME680_REG_GAS_R_LSB			0x2B
+#define   BME680_GAS_STAB_BIT			BIT(4)
+
+#define BME680_REG_CTRL_HUMIDITY		0x72
+#define   BME680_OSRS_HUMIDITY_MASK		GENMASK(2, 0)
+
+#define BME680_REG_CTRL_MEAS			0x74
+#define   BME680_OSRS_TEMP_MASK			GENMASK(7, 5)
+#define   BME680_OSRS_PRESS_MASK		GENMASK(4, 2)
+#define   BME680_MODE_MASK			GENMASK(1, 0)
+
+#define BME680_MODE_FORCED			1
+#define BME680_MODE_SLEEP			0
+
+#define BME680_REG_CONFIG			0x75
+#define   BME680_FILTER_MASK			GENMASK(4, 2)
+#define     BME680_FILTER_COEFF_VAL		BIT(1)
+
+/* TEMP/PRESS/HUMID reading skipped */
+#define BME680_MEAS_SKIPPED			0x8000
+
+#define BME680_MAX_OVERFLOW_VAL			0x40000000
+#define BME680_HUM_REG_SHIFT_VAL		4
+#define BME680_BIT_H1_DATA_MSK			0x0F
+
+#define BME680_REG_RES_HEAT_RANGE		0x02
+#define BME680_RHRANGE_MSK			0x30
+#define BME680_REG_RES_HEAT_VAL			0x00
+#define BME680_REG_RANGE_SW_ERR			0x04
+#define BME680_RSERROR_MSK			0xF0
+#define BME680_REG_RES_HEAT_0			0x5A
+#define BME680_REG_GAS_WAIT_0			0x64
+#define BME680_GAS_RANGE_MASK			0x0F
+#define BME680_ADC_GAS_RES_SHIFT		6
+#define BME680_AMB_TEMP				25
+
+#define BME680_REG_CTRL_GAS_1			0x71
+#define   BME680_RUN_GAS_MASK			BIT(4)
+#define   BME680_NB_CONV_MASK			GENMASK(3, 0)
+#define	    BME680_RUN_GAS_EN_BIT		BIT(4)
+#define     BME680_NB_CONV_0_VAL		0
+
+#define BME680_REG_MEAS_STAT_0			0x1D
+#define   BME680_GAS_MEAS_BIT			BIT(6)
+
+/* Calibration Parameters */
+#define BME680_T2_LSB_REG	0x8A
+#define BME680_T3_REG		0x8C
+#define BME680_P1_LSB_REG	0x8E
+#define BME680_P2_LSB_REG	0x90
+#define BME680_P3_REG		0x92
+#define BME680_P4_LSB_REG	0x94
+#define BME680_P5_LSB_REG	0x96
+#define BME680_P7_REG		0x98
+#define BME680_P6_REG		0x99
+#define BME680_P8_LSB_REG	0x9C
+#define BME680_P9_LSB_REG	0x9E
+#define BME680_P10_REG		0xA0
+#define BME680_H2_LSB_REG	0xE2
+#define BME680_H2_MSB_REG	0xE1
+#define BME680_H1_MSB_REG	0xE3
+#define BME680_H1_LSB_REG	0xE2
+#define BME680_H3_REG		0xE4
+#define BME680_H4_REG		0xE5
+#define BME680_H5_REG		0xE6
+#define BME680_H6_REG		0xE7
+#define BME680_H7_REG		0xE8
+#define BME680_T1_LSB_REG	0xE9
+#define BME680_GH2_LSB_REG	0xEB
+#define BME680_GH1_REG		0xED
+#define BME680_GH3_REG		0xEE
+
+extern const struct regmap_config bme680_regmap_config;
+
+int bme680_core_probe(struct device *dev, struct regmap *regmap,
+		      const char *name);
+
+#endif  /* BME680_H_ */
diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
new file mode 100644
index 0000000..886e534
--- /dev/null
+++ b/drivers/iio/chemical/bme680_core.c
@@ -0,0 +1,976 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Bosch BME680 - Temperature, Pressure, Humidity & Gas Sensor
+ *
+ * Copyright (C) 2017 - 2018 Bosch Sensortec GmbH
+ * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
+ *
+ * Datasheet:
+ * https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf
+ */
+#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/log2.h>
+#include <linux/regmap.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include "bme680.h"
+
+struct bme680_calib {
+	u16 par_t1;
+	s16 par_t2;
+	s8  par_t3;
+	u16 par_p1;
+	s16 par_p2;
+	s8  par_p3;
+	s16 par_p4;
+	s16 par_p5;
+	s8  par_p6;
+	s8  par_p7;
+	s16 par_p8;
+	s16 par_p9;
+	u8  par_p10;
+	u16 par_h1;
+	u16 par_h2;
+	s8  par_h3;
+	s8  par_h4;
+	s8  par_h5;
+	s8  par_h6;
+	s8  par_h7;
+	s8  par_gh1;
+	s16 par_gh2;
+	s8  par_gh3;
+	u8  res_heat_range;
+	s8  res_heat_val;
+	s8  range_sw_err;
+};
+
+struct bme680_data {
+	struct regmap *regmap;
+	struct bme680_calib bme680;
+	u8 oversampling_temp;
+	u8 oversampling_press;
+	u8 oversampling_humid;
+	u16 heater_dur;
+	u16 heater_temp;
+	/*
+	 * Carryover value from temperature conversion, used in pressure
+	 * and humidity compensation calculations.
+	 */
+	s32 t_fine;
+};
+
+const struct regmap_config bme680_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+EXPORT_SYMBOL(bme680_regmap_config);
+
+static const struct iio_chan_spec bme680_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_RESISTANCE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+	},
+};
+
+static const int bme680_oversampling_avail[] = { 1, 2, 4, 8, 16 };
+
+static int bme680_read_calib(struct bme680_data *data,
+			     struct bme680_calib *calib)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	unsigned int tmp, tmp_msb, tmp_lsb;
+	int ret;
+	__le16 buf;
+
+	/* Temperature related coefficients */
+	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_T1_LSB_REG\n");
+		return ret;
+	}
+	calib->par_t1 = le16_to_cpu(buf);
+
+	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_T2_LSB_REG\n");
+		return ret;
+	}
+	calib->par_t2 = le16_to_cpu(buf);
+
+	ret = regmap_read(data->regmap, BME680_T3_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_T3_REG\n");
+		return ret;
+	}
+	calib->par_t3 = tmp;
+
+	/* Pressure related coefficients */
+	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P1_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p1 = le16_to_cpu(buf);
+
+	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P2_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p2 = le16_to_cpu(buf);
+
+	ret = regmap_read(data->regmap, BME680_P3_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P3_REG\n");
+		return ret;
+	}
+	calib->par_p3 = tmp;
+
+	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P4_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p4 = le16_to_cpu(buf);
+
+	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P5_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p5 = le16_to_cpu(buf);
+
+	ret = regmap_read(data->regmap, BME680_P6_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P6_REG\n");
+		return ret;
+	}
+	calib->par_p6 = tmp;
+
+	ret = regmap_read(data->regmap, BME680_P7_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P7_REG\n");
+		return ret;
+	}
+	calib->par_p7 = tmp;
+
+	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P8_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p8 = le16_to_cpu(buf);
+
+	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P9_LSB_REG\n");
+		return ret;
+	}
+	calib->par_p9 = le16_to_cpu(buf);
+
+	ret = regmap_read(data->regmap, BME680_P10_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_P10_REG\n");
+		return ret;
+	}
+	calib->par_p10 = tmp;
+
+	/* Humidity related coefficients */
+	ret = regmap_read(data->regmap, BME680_H1_MSB_REG, &tmp_msb);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
+		return ret;
+	}
+
+	ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H1_LSB_REG\n");
+		return ret;
+	}
+
+	calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
+				(tmp_lsb & BME680_BIT_H1_DATA_MSK);
+
+	ret = regmap_read(data->regmap, BME680_H2_MSB_REG, &tmp_msb);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H2_MSB_REG\n");
+		return ret;
+	}
+
+	ret = regmap_read(data->regmap, BME680_H2_LSB_REG, &tmp_lsb);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H2_LSB_REG\n");
+		return ret;
+	}
+
+	calib->par_h2 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
+				(tmp_lsb >> BME680_HUM_REG_SHIFT_VAL);
+
+	ret = regmap_read(data->regmap, BME680_H3_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H3_REG\n");
+		return ret;
+	}
+	calib->par_h3 = tmp;
+
+	ret = regmap_read(data->regmap, BME680_H4_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H4_REG\n");
+		return ret;
+	}
+	calib->par_h4 = tmp;
+
+	ret = regmap_read(data->regmap, BME680_H5_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H5_REG\n");
+		return ret;
+	}
+	calib->par_h5 = tmp;
+
+	ret = regmap_read(data->regmap, BME680_H6_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H6_REG\n");
+		return ret;
+	}
+	calib->par_h6 = tmp;
+
+	ret = regmap_read(data->regmap, BME680_H7_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_H7_REG\n");
+		return ret;
+	}
+	calib->par_h7 = tmp;
+
+	/* Gas heater related coefficients */
+	ret = regmap_read(data->regmap, BME680_GH1_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_GH1_REG\n");
+		return ret;
+	}
+	calib->par_gh1 = tmp;
+
+	ret = regmap_bulk_read(data->regmap, BME680_GH2_LSB_REG,
+			       (u8 *) &buf, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_GH2_LSB_REG\n");
+		return ret;
+	}
+	calib->par_gh2 = le16_to_cpu(buf);
+
+	ret = regmap_read(data->regmap, BME680_GH3_REG, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read BME680_GH3_REG\n");
+		return ret;
+	}
+	calib->par_gh3 = tmp;
+
+	/* Other coefficients */
+	ret = regmap_read(data->regmap, BME680_REG_RES_HEAT_RANGE, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read resistance heat range\n");
+		return ret;
+	}
+	calib->res_heat_range = (tmp & BME680_RHRANGE_MSK) / 16;
+
+	ret = regmap_read(data->regmap, BME680_REG_RES_HEAT_VAL, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read resistance heat value\n");
+		return ret;
+	}
+	calib->res_heat_val = tmp;
+
+	ret = regmap_read(data->regmap, BME680_REG_RANGE_SW_ERR, &tmp);
+	if (ret < 0) {
+		dev_err(dev, "failed to read range software error\n");
+		return ret;
+	}
+	calib->range_sw_err = (tmp & BME680_RSERROR_MSK) / 16;
+
+	return 0;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L876
+ *
+ * Returns temperature measurement in DegC, resolutions is 0.01 DegC. Therefore,
+ * output value of "3233" represents 32.33 DegC.
+ */
+static s32 bme680_compensate_temp(struct bme680_data *data,
+				  s32 adc_temp)
+{
+	struct bme680_calib *calib = &data->bme680;
+	s64 var1, var2, var3, calc_temp;
+
+	var1 = ((s32) adc_temp >> 3) - ((s32) calib->par_t1 << 1);
+	var2 = (var1 * (s32) calib->par_t2) >> 11;
+	var3 = ((var1 >> 1) * (var1 >> 1)) >> 12;
+	var3 = ((var3) * ((s32) calib->par_t3 << 4)) >> 14;
+	data->t_fine = (s32) (var2 + var3);
+	calc_temp = (s16) (((data->t_fine * 5) + 128) >> 8);
+
+	return calc_temp;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L896
+ *
+ * Returns pressure measurement in Pa. Output value of "97356" represents
+ * 97356 Pa = 973.56 hPa.
+ */
+static u32 bme680_compensate_press(struct bme680_data *data,
+				   u32 adc_press)
+{
+	struct bme680_calib *calib = &data->bme680;
+	s32 var1, var2, var3, press_comp;
+
+	var1 = (((s32)data->t_fine) >> 1) - 64000;
+	var2 = ((((var1 >> 2) * (var1 >> 2)) >> 11) * (s32)calib->par_p6) >> 2;
+	var2 = var2 + ((var1 * (s32)calib->par_p5) << 1);
+	var2 = (var2 >> 2) + ((s32)calib->par_p4 << 16);
+	var1 = (((((var1 >> 2) * (var1 >> 2)) >> 13) *
+			((s32)calib->par_p3 << 5)) >> 3) +
+			(((s32)calib->par_p2 * var1) >> 1);
+	var1 = var1 >> 18;
+	var1 = ((32768 + var1) * (s32)calib->par_p1) >> 15;
+	press_comp = 1048576 - adc_press;
+	press_comp = ((press_comp - (var2 >> 12)) * 3125);
+
+	if (press_comp >= BME680_MAX_OVERFLOW_VAL)
+		press_comp = ((press_comp / (u32)var1) << 1);
+	else
+		press_comp = ((press_comp << 1) / (u32)var1);
+
+	var1 = ((s32)calib->par_p9 * (((press_comp >> 3) *
+				     (press_comp >> 3)) >> 13)) >> 12;
+	var2 = ((press_comp >> 2) * (s32)calib->par_p8) >> 13;
+	var3 = ((press_comp >> 8) * (press_comp >> 8) *
+			(press_comp >> 8) * calib->par_p10) >> 17;
+
+	press_comp += ((var1 + var2 + var3 + ((s32)calib->par_p7 << 7)) >> 4);
+
+	return press_comp;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L937
+ *
+ * Returns humidity measurement in percent, resolution is 0.001 percent. Output
+ * value of "43215" represents 43.215 %rH.
+ */
+static u32 bme680_compensate_humid(struct bme680_data *data,
+				   u16 adc_humid)
+{
+	struct bme680_calib *calib = &data->bme680;
+	s32 var1, var2, var3, var4, var5, var6, temp_scaled, calc_hum;
+
+	temp_scaled = (((s32) data->t_fine * 5) + 128) >> 8;
+	var1 = (adc_humid - ((s32) ((s32) calib->par_h1 * 16))) -
+			(((temp_scaled * (s32) calib->par_h3) / 100) >> 1);
+	var2 = ((s32) calib->par_h2 * (((temp_scaled * (s32) calib->par_h4) /
+			((s32) 100)) + (((temp_scaled * ((temp_scaled *
+			(s32) calib->par_h5) / 100)) >> 6) / 100) +
+			(s32) (1 << 14))) >> 10;
+	var3 = var1 * var2;
+	var4 = (s32) calib->par_h6 << 7;
+	var4 = (var4 + ((temp_scaled * (s32) calib->par_h7) / 100)) >> 4;
+	var5 = ((var3 >> 14) * (var3 >> 14)) >> 10;
+	var6 = (var4 * var5) >> 1;
+	calc_hum = (((var3 + var6) >> 10) * 1000) >> 12;
+
+	if (calc_hum > 100000) /* Cap at 100%rH */
+		calc_hum = 100000;
+	else if (calc_hum < 0)
+		calc_hum = 0;
+
+	return calc_hum;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L973
+ *
+ * Returns gas measurement in Ohm. Output value of "82986" represent 82986 ohms.
+ */
+static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
+				 u8 gas_range)
+{
+	struct bme680_calib *calib = &data->bme680;
+	s64 var1;
+	u64 var2;
+	s64 var3;
+	u32 calc_gas_res;
+
+	/* Look up table 1 for the possible gas range values */
+	u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
+				2147483647u, 2147483647u, 2126008810u,
+				2147483647u, 2130303777u, 2147483647u,
+				2147483647u, 2143188679u, 2136746228u,
+				2147483647u, 2126008810u, 2147483647u,
+				2147483647u};
+	/* Look up table 2 for the possible gas range values */
+	u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
+				512000000u, 255744255u, 127110228u, 64000000u,
+				32258064u, 16016016u, 8000000u, 4000000u,
+				2000000u, 1000000u, 500000u, 250000u, 125000u};
+
+	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
+			((s64) lookupTable1[gas_range])) >> 16;
+	var2 = (((s64) ((s64) gas_res_adc << 15) - 16777216) + var1);
+	var3 = (((s64) lookupTable2[gas_range] * (s64) var1) >> 9);
+	calc_gas_res = (u32) ((var3 + ((s64) var2 >> 1)) / (s64) var2);
+
+	return calc_gas_res;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L1002
+ */
+static u8 bme680_calc_heater_res(struct bme680_data *data, u16 temp)
+{
+	struct bme680_calib *calib = &data->bme680;
+	s32 var1, var2, var3, var4, var5, heatr_res_x100;
+	u8 heatr_res;
+
+	if (temp > 400) /* Cap temperature */
+		temp = 400;
+
+	var1 = (((s32) BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
+	var2 = (calib->par_gh1 + 784) * (((((calib->par_gh2 + 154009) *
+						temp * 5) / 100)
+						+ 3276800) / 10);
+	var3 = var1 + (var2 / 2);
+	var4 = (var3 / (calib->res_heat_range + 4));
+	var5 = (131 * calib->res_heat_val) + 65536;
+	heatr_res_x100 = ((var4 / var5) - 250) * 34;
+	heatr_res = (heatr_res_x100 + 50) / 100;
+
+	return heatr_res;
+}
+
+/*
+ * Taken from Bosch BME680 API:
+ * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L1188
+ */
+static u8 bme680_calc_heater_dur(u16 dur)
+{
+	u8 durval, factor = 0;
+
+	if (dur >= 0xfc0) {
+		durval = 0xff; /* Max duration */
+	} else {
+		while (dur > 0x3F) {
+			dur = dur / 4;
+			factor += 1;
+		}
+		durval = dur + (factor * 64);
+	}
+
+	return durval;
+}
+
+static int bme680_set_mode(struct bme680_data *data, bool mode)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+
+	if (mode) {
+		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
+					BME680_MODE_MASK, BME680_MODE_FORCED);
+		if (ret < 0)
+			dev_err(dev, "failed to set forced mode\n");
+
+	} else {
+		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
+					BME680_MODE_MASK, BME680_MODE_SLEEP);
+		if (ret < 0)
+			dev_err(dev, "failed to set sleep mode\n");
+
+	}
+
+	return ret;
+}
+
+static int bme680_chip_config(struct bme680_data *data)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+	u8 osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK,
+			     data->oversampling_humid + 1);
+	/*
+	 * Highly recommended to set oversampling of humidity before
+	 * temperature/pressure oversampling.
+	 */
+	ret = regmap_update_bits(data->regmap, BME680_REG_CTRL_HUMIDITY,
+				 BME680_OSRS_HUMIDITY_MASK, osrs);
+	if (ret < 0) {
+		dev_err(dev, "failed to write ctrl_hum register\n");
+		return ret;
+	}
+
+	/* IIR filter settings */
+	ret = regmap_update_bits(data->regmap, BME680_REG_CONFIG,
+				 BME680_FILTER_MASK,
+				 BME680_FILTER_COEFF_VAL);
+	if (ret < 0) {
+		dev_err(dev, "failed to write config register\n");
+		return ret;
+	}
+
+	osrs = FIELD_PREP(BME680_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
+	       FIELD_PREP(BME680_OSRS_PRESS_MASK, data->oversampling_press + 1);
+
+	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
+				BME680_OSRS_TEMP_MASK |
+				BME680_OSRS_PRESS_MASK,
+				osrs);
+	if (ret < 0)
+		dev_err(dev, "failed to write ctrl_meas register\n");
+
+	return ret;
+}
+
+static int bme680_gas_config(struct bme680_data *data)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+	u8 heatr_res, heatr_dur;
+
+	heatr_res = bme680_calc_heater_res(data, data->heater_temp);
+
+	/* set target heater temperature */
+	ret = regmap_write(data->regmap, BME680_REG_RES_HEAT_0, heatr_res);
+	if (ret < 0) {
+		dev_err(dev, "failed to write res_heat_0 register\n");
+		return ret;
+	}
+
+	heatr_dur = bme680_calc_heater_dur(data->heater_dur);
+
+	/* set target heating duration */
+	ret = regmap_write(data->regmap, BME680_REG_GAS_WAIT_0, heatr_dur);
+	if (ret < 0) {
+		dev_err(dev, "failted to write gas_wait_0 register\n");
+		return ret;
+	}
+
+	/* Selecting the runGas and NB conversion settings for the sensor */
+	ret = regmap_update_bits(data->regmap, BME680_REG_CTRL_GAS_1,
+				 BME680_RUN_GAS_MASK | BME680_NB_CONV_MASK,
+				 BME680_RUN_GAS_EN_BIT | BME680_NB_CONV_0_VAL);
+	if (ret < 0)
+		dev_err(dev, "failed to write ctrl_gas_1 register\n");
+
+	return ret;
+}
+
+static int bme680_read_temp(struct bme680_data *data,
+			    int *val, int *val2)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret = 0;
+	__be32 tmp = 0;
+	s32 adc_temp, comp_temp;
+
+	/* set forced mode to trigger measurement */
+	ret = bme680_set_mode(data, true);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, BME680_REG_TEMP_MSB,
+			       (u8 *) &tmp, 3);
+	if (ret < 0) {
+		dev_err(dev, "failed to read temperature\n");
+		return ret;
+	}
+
+	adc_temp = be32_to_cpu(tmp) >> 12;
+	if (adc_temp == BME680_MEAS_SKIPPED) {
+		/* reading was skipped */
+		dev_err(dev, "reading temperature skipped\n");
+		return -EINVAL;
+	}
+	comp_temp = bme680_compensate_temp(data, adc_temp);
+	/*
+	 * val might be NULL if we're called by the read_press/read_humid
+	 * routine which is callled to get t_fine value used in
+	 * compensate_press/compensate_humid to get compensated
+	 * pressure/humidity readings.
+	 */
+	if (val && val2) {
+		*val = comp_temp;
+		*val2 = 100;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return ret;
+}
+
+static int bme680_read_press(struct bme680_data *data,
+			     int *val, int *val2)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+	__be32 tmp = 0;
+	s32 adc_press;
+
+	/* Read and compensate temperature to get a reading of t_fine */
+	ret = bme680_read_temp(data, NULL, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, BME680_REG_PRESS_MSB,
+			       (u8 *) &tmp, 3);
+	if (ret < 0) {
+		dev_err(dev, "failed to read pressure\n");
+		return ret;
+	}
+
+	adc_press = be32_to_cpu(tmp) >> 12;
+	if (adc_press == BME680_MEAS_SKIPPED) {
+		/* reading was skipped */
+		dev_err(dev, "reading pressure skipped\n");
+		return -EINVAL;
+	}
+
+	*val = bme680_compensate_press(data, adc_press);
+	*val2 = 100;
+	return IIO_VAL_FRACTIONAL;
+}
+
+static int bme680_read_humid(struct bme680_data *data,
+			     int *val, int *val2)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+	__be16 tmp = 0;
+	s32 adc_humidity;
+	u32 comp_humidity;
+
+	/* Read and compensate temperature to get a reading of t_fine */
+	ret = bme680_read_temp(data, NULL, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, BM6880_REG_HUMIDITY_MSB,
+			       (u8 *) &tmp, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read humidity\n");
+		return ret;
+	}
+
+	adc_humidity = be16_to_cpu(tmp);
+	if (adc_humidity == BME680_MEAS_SKIPPED) {
+		/* reading was skipped */
+		dev_err(dev, "reading humidity skipped\n");
+		return -EINVAL;
+	}
+	comp_humidity = bme680_compensate_humid(data, adc_humidity);
+
+	*val = comp_humidity;
+	*val2 = 1000;
+	return IIO_VAL_FRACTIONAL;
+}
+
+static int bme680_read_gas(struct bme680_data *data,
+			   int *val)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+	__be16 tmp = 0;
+	unsigned int check;
+	u16 adc_gas_res;
+	u8 gas_range;
+
+	/* Set heater settings */
+	ret = bme680_gas_config(data);
+	if (ret < 0) {
+		dev_err(dev, "failed to set gas config\n");
+		return ret;
+	}
+
+	/* set forced mode to trigger measurement */
+	ret = bme680_set_mode(data, true);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
+	if (check & BME680_GAS_MEAS_BIT) {
+		dev_err(dev, "gas measurement incomplete\n");
+		return -EBUSY;
+	}
+
+	ret = regmap_read(data->regmap, BME680_REG_GAS_R_LSB, &check);
+	if (ret < 0) {
+		dev_err(dev, "failed to read gas_r_lsb register\n");
+		return ret;
+	}
+
+	if ((check & BME680_GAS_STAB_BIT) == 0) {
+	/*
+	 * occurs if either the gas heating duration was insuffient
+	 * to reach the target heater temperature or the target
+	 * heater temperature was too high for the heater sink to
+	 * reach.
+	 */
+		dev_err(dev, "heater failed to reach the target temperature\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_bulk_read(data->regmap, BME680_REG_GAS_MSB,
+			       (u8 *) &tmp, 2);
+	if (ret < 0) {
+		dev_err(dev, "failed to read gas resistance\n");
+		return ret;
+	}
+
+	gas_range = check & BME680_GAS_RANGE_MASK;
+	adc_gas_res = be16_to_cpu(tmp) >> BME680_ADC_GAS_RES_SHIFT;
+
+	*val = bme680_compensate_gas(data, adc_gas_res, gas_range);
+	return IIO_VAL_INT;
+}
+
+static int bme680_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_TEMP:
+			return bme680_read_temp(data, val, val2);
+		case IIO_PRESSURE:
+			return bme680_read_press(data, val, val2);
+		case IIO_HUMIDITYRELATIVE:
+			return bme680_read_humid(data, val, val2);
+		case IIO_RESISTANCE:
+			return bme680_read_gas(data, val);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_TEMP:
+			*val = 1 << data->oversampling_temp;
+			return IIO_VAL_INT;
+		case IIO_PRESSURE:
+			*val = 1 << data->oversampling_press;
+			return IIO_VAL_INT;
+		case IIO_HUMIDITYRELATIVE:
+			*val = 1 << data->oversampling_humid;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int bme680_write_oversampling_ratio_temp(struct bme680_data *data,
+						int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
+		if (bme680_oversampling_avail[i] == val) {
+			data->oversampling_temp = ilog2(val);
+
+			return bme680_chip_config(data);
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int bme680_write_oversampling_ratio_press(struct bme680_data *data,
+						 int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
+		if (bme680_oversampling_avail[i] == val) {
+			data->oversampling_press = ilog2(val);
+
+			return bme680_chip_config(data);
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int bme680_write_oversampling_ratio_humid(struct bme680_data *data,
+						 int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
+		if (bme680_oversampling_avail[i] == val) {
+			data->oversampling_humid = ilog2(val);
+
+			return bme680_chip_config(data);
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int bme680_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct bme680_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_TEMP:
+			return bme680_write_oversampling_ratio_temp(data, val);
+		case IIO_PRESSURE:
+			return bme680_write_oversampling_ratio_press(data, val);
+		case IIO_HUMIDITYRELATIVE:
+			return bme680_write_oversampling_ratio_humid(data, val);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
+
+static IIO_CONST_ATTR(oversampling_ratio_available,
+		      bme680_oversampling_ratio_show);
+
+static struct attribute *bme680_attributes[] = {
+	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group bme680_attribute_group = {
+	.attrs = bme680_attributes,
+};
+
+static const struct iio_info bme680_info = {
+	.read_raw = &bme680_read_raw,
+	.write_raw = &bme680_write_raw,
+	.attrs = &bme680_attribute_group,
+};
+
+static const char *bme680_match_acpi_device(struct device *dev)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return NULL;
+
+	return dev_name(dev);
+}
+
+static void bme680_core_remove(void *arg)
+{
+	iio_device_unregister(arg);
+}
+
+int bme680_core_probe(struct device *dev, struct regmap *regmap,
+		      const char *name)
+{
+	struct iio_dev *indio_dev;
+	struct bme680_data *data;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ret = devm_add_action(dev, bme680_core_remove, indio_dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to register remove action\n");
+		return ret;
+	}
+
+	if (!name && ACPI_HANDLE(dev))
+		name = bme680_match_acpi_device(dev);
+
+	data = iio_priv(indio_dev);
+	dev_set_drvdata(dev, indio_dev);
+	data->regmap = regmap;
+	indio_dev->dev.parent = dev;
+	indio_dev->name = name;
+	indio_dev->channels = bme680_channels;
+	indio_dev->num_channels = ARRAY_SIZE(bme680_channels);
+	indio_dev->info = &bme680_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	/* default values for the sensor */
+	data->oversampling_humid = ilog2(2); /* 2X oversampling rate */
+	data->oversampling_press = ilog2(4); /* 4X oversampling rate */
+	data->oversampling_temp = ilog2(8);  /* 8X oversampling rate */
+	data->heater_temp = 320; /* degree Celsius */
+	data->heater_dur = 150;  /* milliseconds */
+
+	ret = bme680_chip_config(data);
+	if (ret < 0) {
+		dev_err(dev, "failed to set chip_config data\n");
+		return ret;
+	}
+
+	ret = bme680_gas_config(data);
+	if (ret < 0) {
+		dev_err(dev, "failed to set gas config data\n");
+		return ret;
+	}
+
+	ret = bme680_read_calib(data, &data->bme680);
+	if (ret < 0) {
+		dev_err(dev,
+			"failed to read calibration coefficients at probe\n");
+		return ret;
+	}
+
+	return iio_device_register(indio_dev);
+}
+EXPORT_SYMBOL_GPL(bme680_core_probe);
+
+MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
+MODULE_DESCRIPTION("Bosch BME680 Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
new file mode 100644
index 0000000..39a0534
--- /dev/null
+++ b/drivers/iio/chemical/bme680_i2c.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * BME680 - I2C Driver
+ *
+ * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
+ *
+ * 7-Bit I2C slave address is:
+ *	- 0x76 if SDO is pulled to GND
+ *	- 0x77 if SDO is pulled to VDDIO
+ *
+ * Note: SDO pin cannot be left floating otherwise I2C address
+ *	 will be undefined.
+ */
+#include <linux/acpi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "bme680.h"
+
+static int bme680_i2c_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	const char *name = NULL;
+	unsigned int val;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &bme680_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Failed to register i2c regmap %d\n",
+				(int)PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	ret = regmap_write(regmap, BME680_REG_SOFT_RESET_I2C,
+			   BME680_CMD_SOFTRESET);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(regmap, BME680_REG_CHIP_I2C_ID, &val);
+	if (ret < 0) {
+		dev_err(&client->dev, "Error reading I2C chip ID\n");
+		return ret;
+	}
+
+	if (val != BME680_CHIP_ID_VAL) {
+		dev_err(&client->dev, "Wrong chip ID, got %x expected %x\n",
+				val, BME680_CHIP_ID_VAL);
+		return -ENODEV;
+	}
+
+	if (id)
+		name = id->name;
+
+	return bme680_core_probe(&client->dev, regmap, name);
+}
+
+static const struct i2c_device_id bme680_i2c_id[] = {
+	{"bme680", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, bme680_i2c_id);
+
+static const struct acpi_device_id bme680_acpi_match[] = {
+	{"BME0680", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
+
+static struct i2c_driver bme680_i2c_driver = {
+	.driver = {
+		.name			= "bme680_i2c",
+		.acpi_match_table       = ACPI_PTR(bme680_acpi_match),
+	},
+	.probe = bme680_i2c_probe,
+	.id_table = bme680_i2c_id,
+};
+module_i2c_driver(bme680_i2c_driver);
+
+MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
+MODULE_DESCRIPTION("BME680 I2C driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
new file mode 100644
index 0000000..53a7401
--- /dev/null
+++ b/drivers/iio/chemical/bme680_spi.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * BME680 - SPI Driver
+ *
+ * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
+ */
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "bme680.h"
+
+static int bme680_regmap_spi_write(void *context, const void *data,
+				   size_t count)
+{
+	struct spi_device *spi = context;
+	u8 buf[2];
+
+	memcpy(buf, data, 2);
+	/*
+	 * The SPI register address (= full register address without bit 7)
+	 * and the write command (bit7 = RW = '0')
+	 */
+	buf[0] &= ~0x80;
+
+	return spi_write_then_read(spi, buf, 2, NULL, 0);
+}
+
+static int bme680_regmap_spi_read(void *context, const void *reg,
+				  size_t reg_size, void *val, size_t val_size)
+{
+	struct spi_device *spi = context;
+
+	return spi_write_then_read(spi, reg, reg_size, val, val_size);
+}
+
+static struct regmap_bus bme680_regmap_bus = {
+	.write = bme680_regmap_spi_write,
+	.read = bme680_regmap_spi_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static int bme680_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct regmap *regmap;
+	unsigned int val;
+	int ret;
+
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(&spi->dev, "spi_setup failed!\n");
+		return ret;
+	}
+
+	regmap = devm_regmap_init(&spi->dev, &bme680_regmap_bus,
+				  &spi->dev, &bme680_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "Failed to register spi regmap %d\n",
+				(int)PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	ret = regmap_write(regmap, BME680_REG_SOFT_RESET_SPI,
+			   BME680_CMD_SOFTRESET);
+	if (ret < 0)
+		return ret;
+
+	/* after power-on reset, Page 0(0x80-0xFF) of spi_mem_page is active */
+	ret = regmap_read(regmap, BME680_REG_CHIP_SPI_ID, &val);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Error reading SPI chip ID\n");
+		return ret;
+	}
+
+	if (val != BME680_CHIP_ID_VAL) {
+		dev_err(&spi->dev, "Wrong chip ID, got %x expected %x\n",
+				val, BME680_CHIP_ID_VAL);
+		return -ENODEV;
+	}
+	/*
+	 * select Page 1 of spi_mem_page to enable access to
+	 * to registers from address 0x00 to 0x7F.
+	 */
+	ret = regmap_write_bits(regmap, BME680_REG_STATUS,
+				BME680_SPI_MEM_PAGE_BIT,
+				BME680_SPI_MEM_PAGE_1_VAL);
+	if (ret < 0) {
+		dev_err(&spi->dev, "failed to set page 1 of spi_mem_page\n");
+		return ret;
+	}
+
+	return bme680_core_probe(&spi->dev, regmap, id->name);
+}
+
+static const struct spi_device_id bme680_spi_id[] = {
+	{"bme680", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, bme680_spi_id);
+
+static const struct acpi_device_id bme680_acpi_match[] = {
+	{"BME0680", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
+
+static struct spi_driver bme680_spi_driver = {
+	.driver = {
+		.name			= "bme680_spi",
+		.acpi_match_table	= ACPI_PTR(bme680_acpi_match),
+	},
+	.probe = bme680_spi_probe,
+	.id_table = bme680_spi_id,
+};
+module_spi_driver(bme680_spi_driver);
+
+MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
+MODULE_DESCRIPTION("Bosch BME680 SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-20 21:31 [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor Himanshu Jha
@ 2018-07-21 15:19 ` Jonathan Cameron
  2018-07-21 15:36   ` Himanshu Jha
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2018-07-21 15:19 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, Daniel Baluta

On Sat, 21 Jul 2018 03:01:24 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:

> Bosch BME680 is a 4-in-1 sensor with temperature, pressure, humidity
> and gas sensing capability. It supports both I2C and SPI communication
> protocol for effective data communication.
> 
> The device supports two modes:
> 
> 1. Sleep mode
> 2. Forced mode
> 
> The measurements only takes place when forced mode is triggered and a
> single TPHG cycle is performed by the sensor. The sensor automatically
> goes to sleep after afterwards.
> 
> The device has various calibration constants/parameters programmed into
> devices' non-volatile memory(NVM) during production and can't be altered
> by the user. These constants are used in the compensation functions to
> get the required compensated readings along with the raw data. The
> compensation functions/algorithms are provided by Bosch Sensortec GmbH
> via their API[1]. As these don't change during the measurement cycle,
> therefore we read and store them at the probe. The default configs
> supplied by Bosch are also set at probe.
> 
> 0-day tested with build success.
> 
> GSoC-2018: https://summerofcode.withgoogle.com/projects/#6691473790074880
> Mentor: Daniel Baluta
> [1] https://github.com/BoschSensortec/BME680_driver
> Datasheet:
> https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>

Hi Himanshu,

This was close to the point where I'd take it and make the few remaining
fixes myself.  I'm still bothered however by the fact the casts in the
various calibration functions are still not all justified so please take
another look at that.  Frankly it looks like the original author
threw in casts because they didn't want to have to think about which ones
actually do anything!

Few other things to fix up for v5 as well.

Jonathan


> ---
> 
> v4:
>    -Added Bosch API links and datasheet link.
>    -explained with comments about the compensate functions.
>    -fixed SPI ID register address.
>    -Used FIELD_PREP macro to avoid unnecessary hard coding.
>    -Simplified Kconfig to remove unnecessary parentheses.
> 
> v3:
>    -moved files to chemical directory instead of a dedicated directory.
>    -read calibration parameters serially with endian conversions.
>    -drop some return ret. 
>    -removed few unnecessary casts safely.
>    -added 'u' suffix to explicitly specify unsigned for large values
>     and thereby fixing comiler warning.
>    -left aligned all comments.
>    -added a comment explaining heater stability failure.
> 
> v2:
>    -Used devm_add_action() to add a generic remove method for
>     both I2C & SPI driver.
>    -Introduction of compensation functions.
>    -chip initialisation routines moved to respective I2C and SPI
>     driver.
>    -Introduction of gas sensing rountines.
>    -Simplified Kconfig to reduce various options.
> 
>  drivers/iio/chemical/Kconfig       |  23 +
>  drivers/iio/chemical/Makefile      |   3 +
>  drivers/iio/chemical/bme680.h      |  96 ++++
>  drivers/iio/chemical/bme680_core.c | 976 +++++++++++++++++++++++++++++++++++++
>  drivers/iio/chemical/bme680_i2c.c  |  83 ++++
>  drivers/iio/chemical/bme680_spi.c  | 123 +++++
>  6 files changed, 1304 insertions(+)
>  create mode 100644 drivers/iio/chemical/bme680.h
>  create mode 100644 drivers/iio/chemical/bme680_core.c
>  create mode 100644 drivers/iio/chemical/bme680_i2c.c
>  create mode 100644 drivers/iio/chemical/bme680_spi.c
> 
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 5cb5be7..b8e005b 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -21,6 +21,29 @@ config ATLAS_PH_SENSOR
>  	 To compile this driver as module, choose M here: the
>  	 module will be called atlas-ph-sensor.
>  
> +config BME680
> +	tristate "Bosch Sensortec BME680 sensor driver"
> +	depends on (I2C || SPI)
> +	select REGMAP
> +	select BME680_I2C if I2C
> +	select BME680_SPI if SPI
> +	help
> +	  Say yes here to build support for Bosch Sensortec BME680 sensor with
> +	  temperature, pressure, humidity and gas sensing capability.
> +
> +	  This driver can also be built as a module. If so, the module for I2C
> +	  would be called bme680_i2c and bme680_spi for SPI support.
> +
> +config BME680_I2C
> +	tristate
> +	depends on I2C && BME680
> +	select REGMAP_I2C
> +
> +config BME680_SPI
> +	tristate
> +	depends on SPI && BME680
> +	select REGMAP_SPI
> +
>  config CCS811
>  	tristate "AMS CCS811 VOC sensor"
>  	depends on I2C
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index a629b29..2f4c4ba 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -4,6 +4,9 @@
>  
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
> +obj-$(CONFIG_BME680) += bme680_core.o
> +obj-$(CONFIG_BME680_I2C) += bme680_i2c.o
> +obj-$(CONFIG_BME680_SPI) += bme680_spi.o
>  obj-$(CONFIG_CCS811)		+= ccs811.o
>  obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
>  obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/bme680.h b/drivers/iio/chemical/bme680.h
> new file mode 100644
> index 0000000..b872f66
> --- /dev/null
> +++ b/drivers/iio/chemical/bme680.h
> @@ -0,0 +1,96 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef BME680_H_
> +#define BME680_H_
> +
> +#define BME680_REG_CHIP_I2C_ID			0xD0
> +#define BME680_REG_CHIP_SPI_ID			0x50
> +#define BME680_CHIP_ID_VAL			0x61
> +#define BME680_REG_SOFT_RESET_I2C		0xE0
> +#define BME680_REG_SOFT_RESET_SPI		0x60
> +#define BME680_CMD_SOFTRESET			0xB6
> +#define BME680_REG_STATUS			0x73
> +#define   BME680_SPI_MEM_PAGE_BIT		BIT(4)
> +#define     BME680_SPI_MEM_PAGE_1_VAL		1
> +
> +#define BME680_REG_TEMP_MSB			0x22
> +#define BME680_REG_PRESS_MSB			0x1F
> +#define BM6880_REG_HUMIDITY_MSB			0x25
> +#define BME680_REG_GAS_MSB			0x2A
> +#define BME680_REG_GAS_R_LSB			0x2B
> +#define   BME680_GAS_STAB_BIT			BIT(4)
> +
> +#define BME680_REG_CTRL_HUMIDITY		0x72
> +#define   BME680_OSRS_HUMIDITY_MASK		GENMASK(2, 0)
> +
> +#define BME680_REG_CTRL_MEAS			0x74
> +#define   BME680_OSRS_TEMP_MASK			GENMASK(7, 5)
> +#define   BME680_OSRS_PRESS_MASK		GENMASK(4, 2)
> +#define   BME680_MODE_MASK			GENMASK(1, 0)
> +
> +#define BME680_MODE_FORCED			1
> +#define BME680_MODE_SLEEP			0
> +
> +#define BME680_REG_CONFIG			0x75
> +#define   BME680_FILTER_MASK			GENMASK(4, 2)
> +#define     BME680_FILTER_COEFF_VAL		BIT(1)
> +
> +/* TEMP/PRESS/HUMID reading skipped */
> +#define BME680_MEAS_SKIPPED			0x8000
> +
> +#define BME680_MAX_OVERFLOW_VAL			0x40000000
> +#define BME680_HUM_REG_SHIFT_VAL		4
> +#define BME680_BIT_H1_DATA_MSK			0x0F
> +
> +#define BME680_REG_RES_HEAT_RANGE		0x02
> +#define BME680_RHRANGE_MSK			0x30
> +#define BME680_REG_RES_HEAT_VAL			0x00
> +#define BME680_REG_RANGE_SW_ERR			0x04
> +#define BME680_RSERROR_MSK			0xF0
> +#define BME680_REG_RES_HEAT_0			0x5A
> +#define BME680_REG_GAS_WAIT_0			0x64
> +#define BME680_GAS_RANGE_MASK			0x0F
> +#define BME680_ADC_GAS_RES_SHIFT		6
> +#define BME680_AMB_TEMP				25
> +
> +#define BME680_REG_CTRL_GAS_1			0x71
> +#define   BME680_RUN_GAS_MASK			BIT(4)
> +#define   BME680_NB_CONV_MASK			GENMASK(3, 0)
> +#define	    BME680_RUN_GAS_EN_BIT		BIT(4)

odd looking spacing above.

> +#define     BME680_NB_CONV_0_VAL		0
> +
> +#define BME680_REG_MEAS_STAT_0			0x1D
> +#define   BME680_GAS_MEAS_BIT			BIT(6)
> +
> +/* Calibration Parameters */
> +#define BME680_T2_LSB_REG	0x8A
> +#define BME680_T3_REG		0x8C
> +#define BME680_P1_LSB_REG	0x8E
> +#define BME680_P2_LSB_REG	0x90
> +#define BME680_P3_REG		0x92
> +#define BME680_P4_LSB_REG	0x94
> +#define BME680_P5_LSB_REG	0x96
> +#define BME680_P7_REG		0x98
> +#define BME680_P6_REG		0x99
> +#define BME680_P8_LSB_REG	0x9C
> +#define BME680_P9_LSB_REG	0x9E
> +#define BME680_P10_REG		0xA0
> +#define BME680_H2_LSB_REG	0xE2
> +#define BME680_H2_MSB_REG	0xE1
> +#define BME680_H1_MSB_REG	0xE3
> +#define BME680_H1_LSB_REG	0xE2
> +#define BME680_H3_REG		0xE4
> +#define BME680_H4_REG		0xE5
> +#define BME680_H5_REG		0xE6
> +#define BME680_H6_REG		0xE7
> +#define BME680_H7_REG		0xE8
> +#define BME680_T1_LSB_REG	0xE9
> +#define BME680_GH2_LSB_REG	0xEB
> +#define BME680_GH1_REG		0xED
> +#define BME680_GH3_REG		0xEE
> +
> +extern const struct regmap_config bme680_regmap_config;
> +
> +int bme680_core_probe(struct device *dev, struct regmap *regmap,
> +		      const char *name);
> +
> +#endif  /* BME680_H_ */
> diff --git a/drivers/iio/chemical/bme680_core.c b/drivers/iio/chemical/bme680_core.c
> new file mode 100644
> index 0000000..886e534
> --- /dev/null
> +++ b/drivers/iio/chemical/bme680_core.c
> @@ -0,0 +1,976 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Bosch BME680 - Temperature, Pressure, Humidity & Gas Sensor
> + *
> + * Copyright (C) 2017 - 2018 Bosch Sensortec GmbH
> + * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
> + *
> + * Datasheet:
> + * https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME680-DS001-00.pdf
> + */
> +#include <linux/acpi.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/log2.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include "bme680.h"
> +
> +struct bme680_calib {
> +	u16 par_t1;
> +	s16 par_t2;
> +	s8  par_t3;
> +	u16 par_p1;
> +	s16 par_p2;
> +	s8  par_p3;
> +	s16 par_p4;
> +	s16 par_p5;
> +	s8  par_p6;
> +	s8  par_p7;
> +	s16 par_p8;
> +	s16 par_p9;
> +	u8  par_p10;
> +	u16 par_h1;
> +	u16 par_h2;
> +	s8  par_h3;
> +	s8  par_h4;
> +	s8  par_h5;
> +	s8  par_h6;
> +	s8  par_h7;
> +	s8  par_gh1;
> +	s16 par_gh2;
> +	s8  par_gh3;
> +	u8  res_heat_range;
> +	s8  res_heat_val;
> +	s8  range_sw_err;
> +};
> +
> +struct bme680_data {
> +	struct regmap *regmap;
> +	struct bme680_calib bme680;
> +	u8 oversampling_temp;
> +	u8 oversampling_press;
> +	u8 oversampling_humid;
> +	u16 heater_dur;
> +	u16 heater_temp;
> +	/*
> +	 * Carryover value from temperature conversion, used in pressure
> +	 * and humidity compensation calculations.
> +	 */
> +	s32 t_fine;
> +};
> +
> +const struct regmap_config bme680_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +EXPORT_SYMBOL(bme680_regmap_config);
> +
> +static const struct iio_chan_spec bme680_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_PRESSURE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_RESISTANCE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +	},
> +};
> +
> +static const int bme680_oversampling_avail[] = { 1, 2, 4, 8, 16 };
> +
> +static int bme680_read_calib(struct bme680_data *data,
> +			     struct bme680_calib *calib)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	unsigned int tmp, tmp_msb, tmp_lsb;
> +	int ret;
> +	__le16 buf;
> +
> +	/* Temperature related coefficients */
> +	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_T1_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_t1 = le16_to_cpu(buf);
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_T2_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_t2 = le16_to_cpu(buf);
> +
> +	ret = regmap_read(data->regmap, BME680_T3_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_T3_REG\n");
> +		return ret;
> +	}
> +	calib->par_t3 = tmp;
> +
> +	/* Pressure related coefficients */
> +	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P1_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p1 = le16_to_cpu(buf);
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P2_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p2 = le16_to_cpu(buf);
> +
> +	ret = regmap_read(data->regmap, BME680_P3_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P3_REG\n");
> +		return ret;
> +	}
> +	calib->par_p3 = tmp;
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P4_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p4 = le16_to_cpu(buf);
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P5_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p5 = le16_to_cpu(buf);
> +
> +	ret = regmap_read(data->regmap, BME680_P6_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P6_REG\n");
> +		return ret;
> +	}
> +	calib->par_p6 = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_P7_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P7_REG\n");
> +		return ret;
> +	}
> +	calib->par_p7 = tmp;
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P8_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p8 = le16_to_cpu(buf);
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P9_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_p9 = le16_to_cpu(buf);
> +
> +	ret = regmap_read(data->regmap, BME680_P10_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_P10_REG\n");
> +		return ret;
> +	}
> +	calib->par_p10 = tmp;
> +
> +	/* Humidity related coefficients */
> +	ret = regmap_read(data->regmap, BME680_H1_MSB_REG, &tmp_msb);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H1_LSB_REG\n");
> +		return ret;
> +	}
> +
> +	calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
> +				(tmp_lsb & BME680_BIT_H1_DATA_MSK);
> +
> +	ret = regmap_read(data->regmap, BME680_H2_MSB_REG, &tmp_msb);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H2_MSB_REG\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_read(data->regmap, BME680_H2_LSB_REG, &tmp_lsb);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H2_LSB_REG\n");
> +		return ret;
> +	}
> +
> +	calib->par_h2 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
> +				(tmp_lsb >> BME680_HUM_REG_SHIFT_VAL);
> +
> +	ret = regmap_read(data->regmap, BME680_H3_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H3_REG\n");
> +		return ret;
> +	}
> +	calib->par_h3 = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_H4_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H4_REG\n");
> +		return ret;
> +	}
> +	calib->par_h4 = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_H5_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H5_REG\n");
> +		return ret;
> +	}
> +	calib->par_h5 = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_H6_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H6_REG\n");
> +		return ret;
> +	}
> +	calib->par_h6 = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_H7_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_H7_REG\n");
> +		return ret;
> +	}
> +	calib->par_h7 = tmp;
> +
> +	/* Gas heater related coefficients */
> +	ret = regmap_read(data->regmap, BME680_GH1_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_GH1_REG\n");
> +		return ret;
> +	}
> +	calib->par_gh1 = tmp;
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_GH2_LSB_REG,
> +			       (u8 *) &buf, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_GH2_LSB_REG\n");
> +		return ret;
> +	}
> +	calib->par_gh2 = le16_to_cpu(buf);
> +
> +	ret = regmap_read(data->regmap, BME680_GH3_REG, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read BME680_GH3_REG\n");
> +		return ret;
> +	}
> +	calib->par_gh3 = tmp;
> +
> +	/* Other coefficients */
> +	ret = regmap_read(data->regmap, BME680_REG_RES_HEAT_RANGE, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read resistance heat range\n");
> +		return ret;
> +	}
> +	calib->res_heat_range = (tmp & BME680_RHRANGE_MSK) / 16;
> +
> +	ret = regmap_read(data->regmap, BME680_REG_RES_HEAT_VAL, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read resistance heat value\n");
> +		return ret;
> +	}
> +	calib->res_heat_val = tmp;
> +
> +	ret = regmap_read(data->regmap, BME680_REG_RANGE_SW_ERR, &tmp);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read range software error\n");
> +		return ret;
> +	}
> +	calib->range_sw_err = (tmp & BME680_RSERROR_MSK) / 16;
> +
> +	return 0;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L876
> + *
> + * Returns temperature measurement in DegC, resolutions is 0.01 DegC. Therefore,
> + * output value of "3233" represents 32.33 DegC.
> + */
> +static s32 bme680_compensate_temp(struct bme680_data *data,
> +				  s32 adc_temp)
> +{
> +	struct bme680_calib *calib = &data->bme680;
> +	s64 var1, var2, var3, calc_temp;
> +
> +	var1 = ((s32) adc_temp >> 3) - ((s32) calib->par_t1 << 1);
> +	var2 = (var1 * (s32) calib->par_t2) >> 11;
> +	var3 = ((var1 >> 1) * (var1 >> 1)) >> 12;
> +	var3 = ((var3) * ((s32) calib->par_t3 << 4)) >> 14;
> +	data->t_fine = (s32) (var2 + var3);
> +	calc_temp = (s16) (((data->t_fine * 5) + 128) >> 8);
> +
> +	return calc_temp;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L896
> + *
> + * Returns pressure measurement in Pa. Output value of "97356" represents
> + * 97356 Pa = 973.56 hPa.
> + */
> +static u32 bme680_compensate_press(struct bme680_data *data,
> +				   u32 adc_press)
> +{
> +	struct bme680_calib *calib = &data->bme680;
> +	s32 var1, var2, var3, press_comp;
> +
> +	var1 = (((s32)data->t_fine) >> 1) - 64000;
> +	var2 = ((((var1 >> 2) * (var1 >> 2)) >> 11) * (s32)calib->par_p6) >> 2;
> +	var2 = var2 + ((var1 * (s32)calib->par_p5) << 1);
> +	var2 = (var2 >> 2) + ((s32)calib->par_p4 << 16);
> +	var1 = (((((var1 >> 2) * (var1 >> 2)) >> 13) *
> +			((s32)calib->par_p3 << 5)) >> 3) +
> +			(((s32)calib->par_p2 * var1) >> 1);
> +	var1 = var1 >> 18;
> +	var1 = ((32768 + var1) * (s32)calib->par_p1) >> 15;
> +	press_comp = 1048576 - adc_press;
> +	press_comp = ((press_comp - (var2 >> 12)) * 3125);
> +
> +	if (press_comp >= BME680_MAX_OVERFLOW_VAL)
> +		press_comp = ((press_comp / (u32)var1) << 1);
> +	else
> +		press_comp = ((press_comp << 1) / (u32)var1);
> +
> +	var1 = ((s32)calib->par_p9 * (((press_comp >> 3) *
> +				     (press_comp >> 3)) >> 13)) >> 12;
> +	var2 = ((press_comp >> 2) * (s32)calib->par_p8) >> 13;
> +	var3 = ((press_comp >> 8) * (press_comp >> 8) *
> +			(press_comp >> 8) * calib->par_p10) >> 17;
> +
> +	press_comp += ((var1 + var2 + var3 + ((s32)calib->par_p7 << 7)) >> 4);
> +
> +	return press_comp;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L937
> + *
> + * Returns humidity measurement in percent, resolution is 0.001 percent. Output
> + * value of "43215" represents 43.215 %rH.
> + */
> +static u32 bme680_compensate_humid(struct bme680_data *data,
> +				   u16 adc_humid)
> +{
> +	struct bme680_calib *calib = &data->bme680;
> +	s32 var1, var2, var3, var4, var5, var6, temp_scaled, calc_hum;
> +
There is still substantial over the top casting in here.

data->t_fine is already a 32 bit integer for example.

> +	temp_scaled = (((s32) data->t_fine * 5) + 128) >> 8;
> +	var1 = (adc_humid - ((s32) ((s32) calib->par_h1 * 16))) -

the outer s32 looks like it won't do anything to me as the inner bit
will already be an s32.

> +			(((temp_scaled * (s32) calib->par_h3) / 100) >> 1);
> +	var2 = ((s32) calib->par_h2 * (((temp_scaled * (s32) calib->par_h4) /
> +			((s32) 100)) + (((temp_scaled * ((temp_scaled *
> +			(s32) calib->par_h5) / 100)) >> 6) / 100) +
> +			(s32) (1 << 14))) >> 10;

Lots more casts of dubious merit in here..

> +	var3 = var1 * var2;
> +	var4 = (s32) calib->par_h6 << 7;
> +	var4 = (var4 + ((temp_scaled * (s32) calib->par_h7) / 100)) >> 4;
> +	var5 = ((var3 >> 14) * (var3 >> 14)) >> 10;
> +	var6 = (var4 * var5) >> 1;
> +	calc_hum = (((var3 + var6) >> 10) * 1000) >> 12;
> +
> +	if (calc_hum > 100000) /* Cap at 100%rH */
> +		calc_hum = 100000;
> +	else if (calc_hum < 0)
> +		calc_hum = 0;
> +
> +	return calc_hum;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L973
> + *
> + * Returns gas measurement in Ohm. Output value of "82986" represent 82986 ohms.
> + */
> +static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
> +				 u8 gas_range)
> +{
> +	struct bme680_calib *calib = &data->bme680;
> +	s64 var1;
> +	u64 var2;
> +	s64 var3;
> +	u32 calc_gas_res;
> +
> +	/* Look up table 1 for the possible gas range values */
> +	u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> +				2147483647u, 2147483647u, 2126008810u,
> +				2147483647u, 2130303777u, 2147483647u,
> +				2147483647u, 2143188679u, 2136746228u,
> +				2147483647u, 2126008810u, 2147483647u,
> +				2147483647u};
> +	/* Look up table 2 for the possible gas range values */
> +	u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> +				512000000u, 255744255u, 127110228u, 64000000u,
> +				32258064u, 16016016u, 8000000u, 4000000u,
> +				2000000u, 1000000u, 500000u, 250000u, 125000u};

You can mark these two arrays as const I think.

> +
> +	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
> +			((s64) lookupTable1[gas_range])) >> 16;
> +	var2 = (((s64) ((s64) gas_res_adc << 15) - 16777216) + var1);

Unless something odd is going on the outer cast just casts the bit
that has already been forced to s64 to s64 so is pointless.

> +	var3 = (((s64) lookupTable2[gas_range] * (s64) var1) >> 9);
> +	calc_gas_res = (u32) ((var3 + ((s64) var2 >> 1)) / (s64) var2);

64 bit division, does that need to be do_div so as to support 32 bit
platforms? Also, why does var 2 want to be cast to a 64 bit?
You'll need to go back to 32 bit anyway to use do_div.

> +
> +	return calc_gas_res;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L1002
> + */
> +static u8 bme680_calc_heater_res(struct bme680_data *data, u16 temp)
> +{
> +	struct bme680_calib *calib = &data->bme680;
> +	s32 var1, var2, var3, var4, var5, heatr_res_x100;
> +	u8 heatr_res;
> +
> +	if (temp > 400) /* Cap temperature */
> +		temp = 400;
> +
> +	var1 = (((s32) BME680_AMB_TEMP * calib->par_gh3) / 1000) * 256;
> +	var2 = (calib->par_gh1 + 784) * (((((calib->par_gh2 + 154009) *
> +						temp * 5) / 100)
> +						+ 3276800) / 10);
> +	var3 = var1 + (var2 / 2);
> +	var4 = (var3 / (calib->res_heat_range + 4));
> +	var5 = (131 * calib->res_heat_val) + 65536;
> +	heatr_res_x100 = ((var4 / var5) - 250) * 34;
> +	heatr_res = (heatr_res_x100 + 50) / 100;
> +
> +	return heatr_res;
> +}
> +
> +/*
> + * Taken from Bosch BME680 API:
> + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L1188
> + */
> +static u8 bme680_calc_heater_dur(u16 dur)
> +{
> +	u8 durval, factor = 0;
> +
> +	if (dur >= 0xfc0) {
> +		durval = 0xff; /* Max duration */
> +	} else {
> +		while (dur > 0x3F) {
> +			dur = dur / 4;
> +			factor += 1;
> +		}
> +		durval = dur + (factor * 64);
> +	}
> +
> +	return durval;
> +}
> +
> +static int bme680_set_mode(struct bme680_data *data, bool mode)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +
> +	if (mode) {
> +		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
> +					BME680_MODE_MASK, BME680_MODE_FORCED);
> +		if (ret < 0)
> +			dev_err(dev, "failed to set forced mode\n");
> +
> +	} else {
> +		ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
> +					BME680_MODE_MASK, BME680_MODE_SLEEP);
> +		if (ret < 0)
> +			dev_err(dev, "failed to set sleep mode\n");
> +
> +	}
> +
> +	return ret;
> +}
> +
> +static int bme680_chip_config(struct bme680_data *data)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +	u8 osrs = FIELD_PREP(BME680_OSRS_HUMIDITY_MASK,
> +			     data->oversampling_humid + 1);
> +	/*
> +	 * Highly recommended to set oversampling of humidity before
> +	 * temperature/pressure oversampling.
> +	 */
> +	ret = regmap_update_bits(data->regmap, BME680_REG_CTRL_HUMIDITY,
> +				 BME680_OSRS_HUMIDITY_MASK, osrs);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write ctrl_hum register\n");
> +		return ret;
> +	}
> +
> +	/* IIR filter settings */
> +	ret = regmap_update_bits(data->regmap, BME680_REG_CONFIG,
> +				 BME680_FILTER_MASK,
> +				 BME680_FILTER_COEFF_VAL);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write config register\n");
> +		return ret;
> +	}
> +
> +	osrs = FIELD_PREP(BME680_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> +	       FIELD_PREP(BME680_OSRS_PRESS_MASK, data->oversampling_press + 1);
> +
> +	ret = regmap_write_bits(data->regmap, BME680_REG_CTRL_MEAS,
> +				BME680_OSRS_TEMP_MASK |
> +				BME680_OSRS_PRESS_MASK,
> +				osrs);
> +	if (ret < 0)
> +		dev_err(dev, "failed to write ctrl_meas register\n");
> +
> +	return ret;
> +}
> +
> +static int bme680_gas_config(struct bme680_data *data)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +	u8 heatr_res, heatr_dur;
> +
> +	heatr_res = bme680_calc_heater_res(data, data->heater_temp);
> +
> +	/* set target heater temperature */
> +	ret = regmap_write(data->regmap, BME680_REG_RES_HEAT_0, heatr_res);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write res_heat_0 register\n");
> +		return ret;
> +	}
> +
> +	heatr_dur = bme680_calc_heater_dur(data->heater_dur);
> +
> +	/* set target heating duration */
> +	ret = regmap_write(data->regmap, BME680_REG_GAS_WAIT_0, heatr_dur);
> +	if (ret < 0) {
> +		dev_err(dev, "failted to write gas_wait_0 register\n");
> +		return ret;
> +	}
> +
> +	/* Selecting the runGas and NB conversion settings for the sensor */
> +	ret = regmap_update_bits(data->regmap, BME680_REG_CTRL_GAS_1,
> +				 BME680_RUN_GAS_MASK | BME680_NB_CONV_MASK,
> +				 BME680_RUN_GAS_EN_BIT | BME680_NB_CONV_0_VAL);
> +	if (ret < 0)
> +		dev_err(dev, "failed to write ctrl_gas_1 register\n");
> +
> +	return ret;
> +}
> +
> +static int bme680_read_temp(struct bme680_data *data,
> +			    int *val, int *val2)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret = 0;
> +	__be32 tmp = 0;
> +	s32 adc_temp, comp_temp;
> +
> +	/* set forced mode to trigger measurement */
> +	ret = bme680_set_mode(data, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_REG_TEMP_MSB,
> +			       (u8 *) &tmp, 3);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read temperature\n");
> +		return ret;
> +	}
> +
> +	adc_temp = be32_to_cpu(tmp) >> 12;
> +	if (adc_temp == BME680_MEAS_SKIPPED) {
> +		/* reading was skipped */
> +		dev_err(dev, "reading temperature skipped\n");
> +		return -EINVAL;
> +	}
> +	comp_temp = bme680_compensate_temp(data, adc_temp);
> +	/*
> +	 * val might be NULL if we're called by the read_press/read_humid
> +	 * routine which is callled to get t_fine value used in
> +	 * compensate_press/compensate_humid to get compensated
> +	 * pressure/humidity readings.
> +	 */
> +	if (val && val2) {
> +		*val = comp_temp;
> +		*val2 = 100;
> +		return IIO_VAL_FRACTIONAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int bme680_read_press(struct bme680_data *data,
> +			     int *val, int *val2)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +	__be32 tmp = 0;
> +	s32 adc_press;
> +
> +	/* Read and compensate temperature to get a reading of t_fine */
> +	ret = bme680_read_temp(data, NULL, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_REG_PRESS_MSB,
> +			       (u8 *) &tmp, 3);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read pressure\n");
> +		return ret;
> +	}
> +
> +	adc_press = be32_to_cpu(tmp) >> 12;
> +	if (adc_press == BME680_MEAS_SKIPPED) {
> +		/* reading was skipped */
> +		dev_err(dev, "reading pressure skipped\n");
> +		return -EINVAL;
> +	}
> +
> +	*val = bme680_compensate_press(data, adc_press);
> +	*val2 = 100;
> +	return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int bme680_read_humid(struct bme680_data *data,
> +			     int *val, int *val2)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +	__be16 tmp = 0;
> +	s32 adc_humidity;
> +	u32 comp_humidity;
> +
> +	/* Read and compensate temperature to get a reading of t_fine */
> +	ret = bme680_read_temp(data, NULL, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_bulk_read(data->regmap, BM6880_REG_HUMIDITY_MSB,
> +			       (u8 *) &tmp, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read humidity\n");
> +		return ret;
> +	}
> +
> +	adc_humidity = be16_to_cpu(tmp);
> +	if (adc_humidity == BME680_MEAS_SKIPPED) {
> +		/* reading was skipped */
> +		dev_err(dev, "reading humidity skipped\n");
> +		return -EINVAL;
> +	}
> +	comp_humidity = bme680_compensate_humid(data, adc_humidity);
> +
> +	*val = comp_humidity;
> +	*val2 = 1000;
> +	return IIO_VAL_FRACTIONAL;
> +}
> +
> +static int bme680_read_gas(struct bme680_data *data,
> +			   int *val)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	int ret;
> +	__be16 tmp = 0;
> +	unsigned int check;
> +	u16 adc_gas_res;
> +	u8 gas_range;
> +
> +	/* Set heater settings */
> +	ret = bme680_gas_config(data);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set gas config\n");
> +		return ret;
> +	}
> +
> +	/* set forced mode to trigger measurement */
> +	ret = bme680_set_mode(data, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read(data->regmap, BME680_REG_MEAS_STAT_0, &check);
> +	if (check & BME680_GAS_MEAS_BIT) {
> +		dev_err(dev, "gas measurement incomplete\n");
> +		return -EBUSY;
> +	}
> +
> +	ret = regmap_read(data->regmap, BME680_REG_GAS_R_LSB, &check);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read gas_r_lsb register\n");
> +		return ret;
> +	}
> +
> +	if ((check & BME680_GAS_STAB_BIT) == 0) {
> +	/*
> +	 * occurs if either the gas heating duration was insuffient
> +	 * to reach the target heater temperature or the target
> +	 * heater temperature was too high for the heater sink to
> +	 * reach.
> +	 */

Odd comment indenting. I would move it before the if to make it
more 'natural'.  Still clearly applies to this block without looking 'odd'.

> +		dev_err(dev, "heater failed to reach the target temperature\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_bulk_read(data->regmap, BME680_REG_GAS_MSB,
> +			       (u8 *) &tmp, 2);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to read gas resistance\n");
> +		return ret;
> +	}
> +
> +	gas_range = check & BME680_GAS_RANGE_MASK;
> +	adc_gas_res = be16_to_cpu(tmp) >> BME680_ADC_GAS_RES_SHIFT;
> +
> +	*val = bme680_compensate_gas(data, adc_gas_res, gas_range);
> +	return IIO_VAL_INT;
> +}
> +
> +static int bme680_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct bme680_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			return bme680_read_temp(data, val, val2);
> +		case IIO_PRESSURE:
> +			return bme680_read_press(data, val, val2);
> +		case IIO_HUMIDITYRELATIVE:
> +			return bme680_read_humid(data, val, val2);
> +		case IIO_RESISTANCE:
> +			return bme680_read_gas(data, val);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			*val = 1 << data->oversampling_temp;
> +			return IIO_VAL_INT;
> +		case IIO_PRESSURE:
> +			*val = 1 << data->oversampling_press;
> +			return IIO_VAL_INT;
> +		case IIO_HUMIDITYRELATIVE:
> +			*val = 1 << data->oversampling_humid;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +

Don't think you can get here so this code should not be here.

> +	return -EINVAL;
> +}
> +
> +static int bme680_write_oversampling_ratio_temp(struct bme680_data *data,
> +						int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
> +		if (bme680_oversampling_avail[i] == val) {
> +			data->oversampling_temp = ilog2(val);
> +
> +			return bme680_chip_config(data);
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int bme680_write_oversampling_ratio_press(struct bme680_data *data,
> +						 int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
> +		if (bme680_oversampling_avail[i] == val) {
> +			data->oversampling_press = ilog2(val);
> +
> +			return bme680_chip_config(data);
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int bme680_write_oversampling_ratio_humid(struct bme680_data *data,
> +						 int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(bme680_oversampling_avail); i++) {
> +		if (bme680_oversampling_avail[i] == val) {
> +			data->oversampling_humid = ilog2(val);
> +
> +			return bme680_chip_config(data);
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int bme680_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct bme680_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			return bme680_write_oversampling_ratio_temp(data, val);
> +		case IIO_PRESSURE:
> +			return bme680_write_oversampling_ratio_press(data, val);
> +		case IIO_HUMIDITYRELATIVE:
> +			return bme680_write_oversampling_ratio_humid(data, val);
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +

You can't get here so no point in having this return. I'll delete it.

> +	return -EINVAL;
> +}
> +
> +static const char bme680_oversampling_ratio_show[] = "1 2 4 8 16";
> +
> +static IIO_CONST_ATTR(oversampling_ratio_available,
> +		      bme680_oversampling_ratio_show);
> +
> +static struct attribute *bme680_attributes[] = {
> +	&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group bme680_attribute_group = {
> +	.attrs = bme680_attributes,
> +};
> +
> +static const struct iio_info bme680_info = {
> +	.read_raw = &bme680_read_raw,
> +	.write_raw = &bme680_write_raw,
> +	.attrs = &bme680_attribute_group,
> +};
> +
> +static const char *bme680_match_acpi_device(struct device *dev)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!id)
> +		return NULL;
> +
> +	return dev_name(dev);
> +}
> +
> +static void bme680_core_remove(void *arg)
> +{
> +	iio_device_unregister(arg);
> +}
> +
> +int bme680_core_probe(struct device *dev, struct regmap *regmap,
> +		      const char *name)
> +{
> +	struct iio_dev *indio_dev;
> +	struct bme680_data *data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ret = devm_add_action(dev, bme680_core_remove, indio_dev);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register remove action\n");
> +		return ret;
> +	}

I think this is logically in the wrong place.  The things it's actually
doing is unwinding the register below, but at this stage you haven't
performed that registration.

If this is all I fine, I 'might' move it and apply anyway.

I'm assuming that there will shortly be more in there than a simple
unregister (otherwise you could just have used devm_iio_device_register...

I'd prefer that for now you just use devm_iio_device_register
and drop this until you need it.

> +
> +	if (!name && ACPI_HANDLE(dev))
> +		name = bme680_match_acpi_device(dev);
> +
> +	data = iio_priv(indio_dev);
> +	dev_set_drvdata(dev, indio_dev);
> +	data->regmap = regmap;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = name;
> +	indio_dev->channels = bme680_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(bme680_channels);
> +	indio_dev->info = &bme680_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	/* default values for the sensor */
> +	data->oversampling_humid = ilog2(2); /* 2X oversampling rate */
> +	data->oversampling_press = ilog2(4); /* 4X oversampling rate */
> +	data->oversampling_temp = ilog2(8);  /* 8X oversampling rate */
> +	data->heater_temp = 320; /* degree Celsius */
> +	data->heater_dur = 150;  /* milliseconds */
> +
> +	ret = bme680_chip_config(data);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set chip_config data\n");
> +		return ret;
> +	}
> +
> +	ret = bme680_gas_config(data);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set gas config data\n");
> +		return ret;
> +	}
> +
> +	ret = bme680_read_calib(data, &data->bme680);
> +	if (ret < 0) {
> +		dev_err(dev,
> +			"failed to read calibration coefficients at probe\n");
> +		return ret;
> +	}
> +
> +	return iio_device_register(indio_dev);
> +}
> +EXPORT_SYMBOL_GPL(bme680_core_probe);
> +
> +MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
> +MODULE_DESCRIPTION("Bosch BME680 Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c
> new file mode 100644
> index 0000000..39a0534
> --- /dev/null
> +++ b/drivers/iio/chemical/bme680_i2c.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * BME680 - I2C Driver
> + *
> + * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
> + *
> + * 7-Bit I2C slave address is:
> + *	- 0x76 if SDO is pulled to GND
> + *	- 0x77 if SDO is pulled to VDDIO
> + *
> + * Note: SDO pin cannot be left floating otherwise I2C address
> + *	 will be undefined.
> + */
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "bme680.h"
> +
> +static int bme680_i2c_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	struct regmap *regmap;
> +	const char *name = NULL;
> +	unsigned int val;
> +	int ret;
> +
> +	regmap = devm_regmap_init_i2c(client, &bme680_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "Failed to register i2c regmap %d\n",
> +				(int)PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	ret = regmap_write(regmap, BME680_REG_SOFT_RESET_I2C,
> +			   BME680_CMD_SOFTRESET);
> +	if (ret < 0)

It's not something I'm that bothered by, but you are a little random
in whether a given error outputs a debug message or not...

> +		return ret;
> +
> +	ret = regmap_read(regmap, BME680_REG_CHIP_I2C_ID, &val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Error reading I2C chip ID\n");
> +		return ret;
> +	}
> +
> +	if (val != BME680_CHIP_ID_VAL) {
> +		dev_err(&client->dev, "Wrong chip ID, got %x expected %x\n",
> +				val, BME680_CHIP_ID_VAL);
> +		return -ENODEV;
> +	}
> +
> +	if (id)
> +		name = id->name;
> +
> +	return bme680_core_probe(&client->dev, regmap, name);
> +}
> +
> +static const struct i2c_device_id bme680_i2c_id[] = {
> +	{"bme680", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, bme680_i2c_id);
> +
> +static const struct acpi_device_id bme680_acpi_match[] = {
> +	{"BME0680", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
> +
> +static struct i2c_driver bme680_i2c_driver = {
> +	.driver = {
> +		.name			= "bme680_i2c",
> +		.acpi_match_table       = ACPI_PTR(bme680_acpi_match),
> +	},
> +	.probe = bme680_i2c_probe,
> +	.id_table = bme680_i2c_id,
> +};
> +module_i2c_driver(bme680_i2c_driver);
> +
> +MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
> +MODULE_DESCRIPTION("BME680 I2C driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/chemical/bme680_spi.c b/drivers/iio/chemical/bme680_spi.c
> new file mode 100644
> index 0000000..53a7401
> --- /dev/null
> +++ b/drivers/iio/chemical/bme680_spi.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * BME680 - SPI Driver
> + *
> + * Copyright (C) 2018 Himanshu Jha <himanshujha199640@gmail.com>
> + */
> +#include <linux/acpi.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#include "bme680.h"
> +
> +static int bme680_regmap_spi_write(void *context, const void *data,
> +				   size_t count)
> +{
> +	struct spi_device *spi = context;
> +	u8 buf[2];
> +
> +	memcpy(buf, data, 2);
> +	/*
> +	 * The SPI register address (= full register address without bit 7)
> +	 * and the write command (bit7 = RW = '0')
> +	 */
> +	buf[0] &= ~0x80;
> +
> +	return spi_write_then_read(spi, buf, 2, NULL, 0);
> +}
> +
> +static int bme680_regmap_spi_read(void *context, const void *reg,
> +				  size_t reg_size, void *val, size_t val_size)
> +{
> +	struct spi_device *spi = context;
> +
> +	return spi_write_then_read(spi, reg, reg_size, val, val_size);
> +}
> +
> +static struct regmap_bus bme680_regmap_bus = {
> +	.write = bme680_regmap_spi_write,
> +	.read = bme680_regmap_spi_read,
> +	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
> +	.val_format_endian_default = REGMAP_ENDIAN_BIG,
> +};
> +
> +static int bme680_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +	struct regmap *regmap;
> +	unsigned int val;
> +	int ret;
> +
> +	spi->bits_per_word = 8;
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "spi_setup failed!\n");
> +		return ret;
> +	}
> +
> +	regmap = devm_regmap_init(&spi->dev, &bme680_regmap_bus,
> +				  &spi->dev, &bme680_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "Failed to register spi regmap %d\n",
> +				(int)PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	ret = regmap_write(regmap, BME680_REG_SOFT_RESET_SPI,
> +			   BME680_CMD_SOFTRESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* after power-on reset, Page 0(0x80-0xFF) of spi_mem_page is active */
> +	ret = regmap_read(regmap, BME680_REG_CHIP_SPI_ID, &val);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Error reading SPI chip ID\n");
> +		return ret;
> +	}
> +
> +	if (val != BME680_CHIP_ID_VAL) {
> +		dev_err(&spi->dev, "Wrong chip ID, got %x expected %x\n",
> +				val, BME680_CHIP_ID_VAL);
> +		return -ENODEV;
> +	}
> +	/*
> +	 * select Page 1 of spi_mem_page to enable access to
> +	 * to registers from address 0x00 to 0x7F.
> +	 */
> +	ret = regmap_write_bits(regmap, BME680_REG_STATUS,
> +				BME680_SPI_MEM_PAGE_BIT,
> +				BME680_SPI_MEM_PAGE_1_VAL);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "failed to set page 1 of spi_mem_page\n");
> +		return ret;
> +	}
> +
> +	return bme680_core_probe(&spi->dev, regmap, id->name);
> +}
> +
> +static const struct spi_device_id bme680_spi_id[] = {
> +	{"bme680", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, bme680_spi_id);
> +
> +static const struct acpi_device_id bme680_acpi_match[] = {
> +	{"BME0680", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, bme680_acpi_match);
> +
> +static struct spi_driver bme680_spi_driver = {
> +	.driver = {
> +		.name			= "bme680_spi",
> +		.acpi_match_table	= ACPI_PTR(bme680_acpi_match),
> +	},
> +	.probe = bme680_spi_probe,
> +	.id_table = bme680_spi_id,
> +};
> +module_spi_driver(bme680_spi_driver);
> +
> +MODULE_AUTHOR("Himanshu Jha <himanshujha199640@gmail.com>");
> +MODULE_DESCRIPTION("Bosch BME680 SPI driver");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:19 ` Jonathan Cameron
@ 2018-07-21 15:36   ` Himanshu Jha
  2018-07-21 15:43     ` Andy Shevchenko
  2018-07-21 16:31     ` Jonathan Cameron
  0 siblings, 2 replies; 11+ messages in thread
From: Himanshu Jha @ 2018-07-21 15:36 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, Daniel Baluta

> Hi Himanshu,
> 
> This was close to the point where I'd take it and make the few remaining
> fixes myself.  I'm still bothered however by the fact the casts in the
> various calibration functions are still not all justified so please take
> another look at that.  Frankly it looks like the original author
> threw in casts because they didn't want to have to think about which ones
> actually do anything!

Ok. I will remove the ones mentioned below.

> Few other things to fix up for v5 as well.

I will send the fixes in an hour.

> Jonathan
> 
> 
> > +#define   BME680_NB_CONV_MASK			GENMASK(3, 0)
> > +#define	    BME680_RUN_GAS_EN_BIT		BIT(4)
> 
> odd looking spacing above.

I don't know why this is showing like that in the diff output, but I
have checked the code by apllying to my test tree(git am <patch>) and
there was no such spurious spacing!

> > +/*
> > + * Taken from Bosch BME680 API:
> > + * https://github.com/BoschSensortec/BME680_driver/blob/63bb5336/bme680.c#L937
> > + *
> > + * Returns humidity measurement in percent, resolution is 0.001 percent. Output
> > + * value of "43215" represents 43.215 %rH.
> > + */
> > +static u32 bme680_compensate_humid(struct bme680_data *data,
> > +				   u16 adc_humid)
> > +{
> > +	struct bme680_calib *calib = &data->bme680;
> > +	s32 var1, var2, var3, var4, var5, var6, temp_scaled, calc_hum;
> > +
> There is still substantial over the top casting in here.
> 
> data->t_fine is already a 32 bit integer for example.

Will do.

> > +	temp_scaled = (((s32) data->t_fine * 5) + 128) >> 8;
> > +	var1 = (adc_humid - ((s32) ((s32) calib->par_h1 * 16))) -
> 
> the outer s32 looks like it won't do anything to me as the inner bit
> will already be an s32.

OK.

> > +			(((temp_scaled * (s32) calib->par_h3) / 100) >> 1);
> > +	var2 = ((s32) calib->par_h2 * (((temp_scaled * (s32) calib->par_h4) /
> > +			((s32) 100)) + (((temp_scaled * ((temp_scaled *
> > +			(s32) calib->par_h5) / 100)) >> 6) / 100) +
> > +			(s32) (1 << 14))) >> 10;
> 
> Lots more casts of dubious merit in here..

Ok.

> > +	var3 = var1 * var2;
> > +	var4 = (s32) calib->par_h6 << 7;
> > +	var4 = (var4 + ((temp_scaled * (s32) calib->par_h7) / 100)) >> 4;
> > +	var5 = ((var3 >> 14) * (var3 >> 14)) >> 10;
> > +	var6 = (var4 * var5) >> 1;
> > +	calc_hum = (((var3 + var6) >> 10) * 1000) >> 12;
> > +
> > +	if (calc_hum > 100000) /* Cap at 100%rH */
> > +		calc_hum = 100000;
> > +	else if (calc_hum < 0)
> > +		calc_hum = 0;
> > +
> > +	return calc_hum;
> > +}
> > +static u32 bme680_compensate_gas(struct bme680_data *data, u16 gas_res_adc,
> > +				 u8 gas_range)
> > +{
> > +	struct bme680_calib *calib = &data->bme680;
> > +	s64 var1;
> > +	u64 var2;
> > +	s64 var3;
> > +	u32 calc_gas_res;
> > +
> > +	/* Look up table 1 for the possible gas range values */
> > +	u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> > +				2147483647u, 2147483647u, 2126008810u,
> > +				2147483647u, 2130303777u, 2147483647u,
> > +				2147483647u, 2143188679u, 2136746228u,
> > +				2147483647u, 2126008810u, 2147483647u,
> > +				2147483647u};
> > +	/* Look up table 2 for the possible gas range values */
> > +	u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> > +				512000000u, 255744255u, 127110228u, 64000000u,
> > +				32258064u, 16016016u, 8000000u, 4000000u,
> > +				2000000u, 1000000u, 500000u, 250000u, 125000u};
> 
> You can mark these two arrays as const I think.

Sure.

> > +
> > +	var1 = ((1340 + (5 * (s64) calib->range_sw_err)) *
> > +			((s64) lookupTable1[gas_range])) >> 16;
> > +	var2 = (((s64) ((s64) gas_res_adc << 15) - 16777216) + var1);
> 
> Unless something odd is going on the outer cast just casts the bit
> that has already been forced to s64 to s64 so is pointless.

Ok.

> > +	var3 = (((s64) lookupTable2[gas_range] * (s64) var1) >> 9);
> > +	calc_gas_res = (u32) ((var3 + ((s64) var2 >> 1)) / (s64) var2);
> 
> 64 bit division, does that need to be do_div so as to support 32 bit
> platforms? Also, why does var 2 want to be cast to a 64 bit?
> You'll need to go back to 32 bit anyway to use do_div.

I will change to do_div().

> > +
> > +	if ((check & BME680_GAS_STAB_BIT) == 0) {
> > +	/*
> > +	 * occurs if either the gas heating duration was insuffient
> > +	 * to reach the target heater temperature or the target
> > +	 * heater temperature was too high for the heater sink to
> > +	 * reach.
> > +	 */
> 
> Odd comment indenting. I would move it before the if to make it
> more 'natural'.  Still clearly applies to this block without looking 'odd'.

Will do.

> > +static int bme680_read_raw(struct iio_dev *indio_dev,
> > +			   struct iio_chan_spec const *chan,
> > +			   int *val, int *val2, long mask)
> > +{
> > +	struct bme680_data *data = iio_priv(indio_dev);
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_PROCESSED:
> > +		switch (chan->type) {
> > +		case IIO_TEMP:
> > +			return bme680_read_temp(data, val, val2);
> > +		case IIO_PRESSURE:
> > +			return bme680_read_press(data, val, val2);
> > +		case IIO_HUMIDITYRELATIVE:
> > +			return bme680_read_humid(data, val, val2);
> > +		case IIO_RESISTANCE:
> > +			return bme680_read_gas(data, val);
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > +		switch (chan->type) {
> > +		case IIO_TEMP:
> > +			*val = 1 << data->oversampling_temp;
> > +			return IIO_VAL_INT;
> > +		case IIO_PRESSURE:
> > +			*val = 1 << data->oversampling_press;
> > +			return IIO_VAL_INT;
> > +		case IIO_HUMIDITYRELATIVE:
> > +			*val = 1 << data->oversampling_humid;
> > +			return IIO_VAL_INT;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> 
> Don't think you can get here so this code should not be here.

Ok.

> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> 
> You can't get here so no point in having this return. I'll delete it.

OK.

> > +	ret = devm_add_action(dev, bme680_core_remove, indio_dev);
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to register remove action\n");
> > +		return ret;
> > +	}
> 
> I think this is logically in the wrong place.  The things it's actually
> doing is unwinding the register below, but at this stage you haven't
> performed that registration.
> 
> If this is all I fine, I 'might' move it and apply anyway.
> 
> I'm assuming that there will shortly be more in there than a simple
> unregister (otherwise you could just have used devm_iio_device_register...
> 
> I'd prefer that for now you just use devm_iio_device_register
> and drop this until you need it.

OK.

> > +
> > +	ret = regmap_write(regmap, BME680_REG_SOFT_RESET_I2C,
> > +			   BME680_CMD_SOFTRESET);
> > +	if (ret < 0)
> 
> It's not something I'm that bothered by, but you are a little random
> in whether a given error outputs a debug message or not...

I missed this.
Will fix.

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:36   ` Himanshu Jha
@ 2018-07-21 15:43     ` Andy Shevchenko
  2018-07-21 15:52       ` Himanshu Jha
  2018-07-21 17:45       ` Daniel Baluta
  2018-07-21 16:31     ` Jonathan Cameron
  1 sibling, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2018-07-21 15:43 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Linux Kernel Mailing List, linux-iio,
	Daniel Baluta

On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
<himanshujha199640@gmail.com> wrote:

>> > +   /* Look up table 1 for the possible gas range values */
>> > +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
>> > +                           2147483647u, 2147483647u, 2126008810u,
>> > +                           2147483647u, 2130303777u, 2147483647u,
>> > +                           2147483647u, 2143188679u, 2136746228u,
>> > +                           2147483647u, 2126008810u, 2147483647u,
>> > +                           2147483647u};

This one needs perhaps a bit of though, but...

>> > +   /* Look up table 2 for the possible gas range values */
>> > +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
>> > +                           512000000u, 255744255u, 127110228u, 64000000u,
>> > +                           32258064u, 16016016u, 8000000u, 4000000u,
>> > +                           2000000u, 1000000u, 500000u, 250000u, 125000u};

...this one obviously just a not needed one. You may replace it with a
one constant and simple calculation to get either value (index from
value, or value from index).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:43     ` Andy Shevchenko
@ 2018-07-21 15:52       ` Himanshu Jha
  2018-07-21 16:29         ` Jonathan Cameron
  2018-07-21 17:45       ` Daniel Baluta
  1 sibling, 1 reply; 11+ messages in thread
From: Himanshu Jha @ 2018-07-21 15:52 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Linux Kernel Mailing List, linux-iio,
	Daniel Baluta

On Sat, Jul 21, 2018 at 06:43:51PM +0300, Andy Shevchenko wrote:
> On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
> <himanshujha199640@gmail.com> wrote:
> 
> >> > +   /* Look up table 1 for the possible gas range values */
> >> > +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> >> > +                           2147483647u, 2147483647u, 2126008810u,
> >> > +                           2147483647u, 2130303777u, 2147483647u,
> >> > +                           2147483647u, 2143188679u, 2136746228u,
> >> > +                           2147483647u, 2126008810u, 2147483647u,
> >> > +                           2147483647u};
> 
> This one needs perhaps a bit of though, but...
> 
> >> > +   /* Look up table 2 for the possible gas range values */
> >> > +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> >> > +                           512000000u, 255744255u, 127110228u, 64000000u,
> >> > +                           32258064u, 16016016u, 8000000u, 4000000u,
> >> > +                           2000000u, 1000000u, 500000u, 250000u, 125000u};
> 
> ...this one obviously just a not needed one. You may replace it with a
> one constant and simple calculation to get either value (index from
> value, or value from index).

Not sure to understand your opinion!?

Anything related to increase to .ro segment ?

I think adding is better to let compiler optimise the code a bit
further.

> -- 
> With Best Regards,
> Andy Shevchenko

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:52       ` Himanshu Jha
@ 2018-07-21 16:29         ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-07-21 16:29 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: Andy Shevchenko, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Linux Kernel Mailing List, linux-iio,
	Daniel Baluta

On Sat, 21 Jul 2018 21:22:30 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:

> On Sat, Jul 21, 2018 at 06:43:51PM +0300, Andy Shevchenko wrote:
> > On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
> > <himanshujha199640@gmail.com> wrote:
> >   
> > >> > +   /* Look up table 1 for the possible gas range values */
> > >> > +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> > >> > +                           2147483647u, 2147483647u, 2126008810u,
> > >> > +                           2147483647u, 2130303777u, 2147483647u,
> > >> > +                           2147483647u, 2143188679u, 2136746228u,
> > >> > +                           2147483647u, 2126008810u, 2147483647u,
> > >> > +                           2147483647u};  
> > 
> > This one needs perhaps a bit of though, but...
> >   
> > >> > +   /* Look up table 2 for the possible gas range values */
> > >> > +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> > >> > +                           512000000u, 255744255u, 127110228u, 64000000u,
> > >> > +                           32258064u, 16016016u, 8000000u, 4000000u,
> > >> > +                           2000000u, 1000000u, 500000u, 250000u, 125000u};  
> > 
> > ...this one obviously just a not needed one. You may replace it with a
> > one constant and simple calculation to get either value (index from
> > value, or value from index).  
> 
> Not sure to understand your opinion!?
> 
> Anything related to increase to .ro segment ?
> 
> I think adding is better to let compiler optimise the code a bit
> further.

No, I missed this as didn't look too closely at those numbers.
They are almost by not quite simple power's of 2 * 125000.
Question is whether the very very small difference from that matter or not...

Andy is suggesting replacing this block of data with an equation that is
'near enough'.
> 
> > -- 
> > With Best Regards,
> > Andy Shevchenko  
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:36   ` Himanshu Jha
  2018-07-21 15:43     ` Andy Shevchenko
@ 2018-07-21 16:31     ` Jonathan Cameron
  1 sibling, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2018-07-21 16:31 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, Daniel Baluta

On Sat, 21 Jul 2018 21:06:08 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:

> > Hi Himanshu,
> > 
> > This was close to the point where I'd take it and make the few remaining
> > fixes myself.  I'm still bothered however by the fact the casts in the
> > various calibration functions are still not all justified so please take
> > another look at that.  Frankly it looks like the original author
> > threw in casts because they didn't want to have to think about which ones
> > actually do anything!  
> 
> Ok. I will remove the ones mentioned below.
> 
> > Few other things to fix up for v5 as well.  
> 
> I will send the fixes in an hour.
> 
> > Jonathan
> > 
> >   
> > > +#define   BME680_NB_CONV_MASK			GENMASK(3, 0)
> > > +#define	    BME680_RUN_GAS_EN_BIT		BIT(4)  
> > 
> > odd looking spacing above.  
> 
> I don't know why this is showing like that in the diff output, but I
> have checked the code by apllying to my test tree(git am <patch>) and
> there was no such spurious spacing!

You have a tab after the #define in this one instance - in the others it's
all spaces (as it should be)

...

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 15:43     ` Andy Shevchenko
  2018-07-21 15:52       ` Himanshu Jha
@ 2018-07-21 17:45       ` Daniel Baluta
  2018-07-22 22:21         ` Himanshu Jha
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Baluta @ 2018-07-21 17:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Himanshu Jha, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Linux Kernel Mailing List,
	linux-iio

On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
> <himanshujha199640@gmail.com> wrote:
>
>>> > +   /* Look up table 1 for the possible gas range values */
>>> > +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
>>> > +                           2147483647u, 2147483647u, 2126008810u,
>>> > +                           2147483647u, 2130303777u, 2147483647u,
>>> > +                           2147483647u, 2143188679u, 2136746228u,
>>> > +                           2147483647u, 2126008810u, 2147483647u,
>>> > +                           2147483647u};
>
> This one needs perhaps a bit of though, but...
>
>>> > +   /* Look up table 2 for the possible gas range values */
>>> > +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
>>> > +                           512000000u, 255744255u, 127110228u, 64000000u,
>>> > +                           32258064u, 16016016u, 8000000u, 4000000u,
>>> > +                           2000000u, 1000000u, 500000u, 250000u, 125000u};
>
> ...this one obviously just a not needed one. You may replace it with a
> one constant and simple calculation to get either value (index from
> value, or value from index).

Indeed this can be reduce to:

125.000 << (15 - idx).

The real question here is if we approximate 255.744.255u to 256.00.00u how
much different is the result. Being a gas sensor I think it is very
hard to appreciate.

We can go with this formula + adding a comment with the table with the
exact coefficients.

thanks,
Daniel.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-21 17:45       ` Daniel Baluta
@ 2018-07-22 22:21         ` Himanshu Jha
  2018-07-23 22:16           ` David Frey
  0 siblings, 1 reply; 11+ messages in thread
From: Himanshu Jha @ 2018-07-22 22:21 UTC (permalink / raw)
  To: Daniel Baluta, dfrey
  Cc: Andy Shevchenko, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Linux Kernel Mailing List,
	linux-iio

On Sat, Jul 21, 2018 at 08:45:34PM +0300, Daniel Baluta wrote:
> On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
> > <himanshujha199640@gmail.com> wrote:
> >
> >>> > +   /* Look up table 1 for the possible gas range values */
> >>> > +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> >>> > +                           2147483647u, 2147483647u, 2126008810u,
> >>> > +                           2147483647u, 2130303777u, 2147483647u,
> >>> > +                           2147483647u, 2143188679u, 2136746228u,
> >>> > +                           2147483647u, 2126008810u, 2147483647u,
> >>> > +                           2147483647u};
> >
> > This one needs perhaps a bit of though, but...
> >
> >>> > +   /* Look up table 2 for the possible gas range values */
> >>> > +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> >>> > +                           512000000u, 255744255u, 127110228u, 64000000u,
> >>> > +                           32258064u, 16016016u, 8000000u, 4000000u,
> >>> > +                           2000000u, 1000000u, 500000u, 250000u, 125000u};
> >
> > ...this one obviously just a not needed one. You may replace it with a
> > one constant and simple calculation to get either value (index from
> > value, or value from index).
> 
> Indeed this can be reduce to:
> 
> 125.000 << (15 - idx).
> 
> The real question here is if we approximate 255.744.255u to 256.00.00u how
> much different is the result. Being a gas sensor I think it is very
> hard to appreciate.
> 
> We can go with this formula + adding a comment with the table with the
> exact coefficients.

So, I have planned to use this 125000 << (15 - idx) equation with
approximating the array members.

About the difference in results we would get after approximating isn't
much of a problem IMHO because gas sensor is primarily used for IAQ, and
IAQ is relative to the resistance reading.

For eg: 

Resistance(ohm)			IAQ
value < 30K			Very bad
30k < value < 50k		worse
50k < value < 70k		bad
... 
..			
so on..

So, what I simply imply is the scale will be adjusted and nothing else
changes, unlike if it had been pressure, temperature, humidity.

The IAQ implementation is userspace application suggesting
good/bad/ugly air quality.

And since we know David Frey is planning to use this sensor in his
product mangOH board.

So, David, how are you planning to use the gas sensing part in your
product ? RGB leds, buzzer, alarm ?

Thanks Andy for the suggestion :)
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-22 22:21         ` Himanshu Jha
@ 2018-07-23 22:16           ` David Frey
  2018-07-23 23:02             ` Himanshu Jha
  0 siblings, 1 reply; 11+ messages in thread
From: David Frey @ 2018-07-23 22:16 UTC (permalink / raw)
  To: Himanshu Jha, Daniel Baluta; +Cc: Linux Kernel Mailing List, linux-iio

On 7/22/2018 3:21 PM, Himanshu Jha wrote:
> On Sat, Jul 21, 2018 at 08:45:34PM +0300, Daniel Baluta wrote:
>> On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
>>> <himanshujha199640@gmail.com> wrote:
>>>
>>>>>> +   /* Look up table 1 for the possible gas range values */
>>>>>> +   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
>>>>>> +                           2147483647u, 2147483647u, 2126008810u,
>>>>>> +                           2147483647u, 2130303777u, 2147483647u,
>>>>>> +                           2147483647u, 2143188679u, 2136746228u,
>>>>>> +                           2147483647u, 2126008810u, 2147483647u,
>>>>>> +                           2147483647u};
>>>
>>> This one needs perhaps a bit of though, but...
>>>
>>>>>> +   /* Look up table 2 for the possible gas range values */
>>>>>> +   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
>>>>>> +                           512000000u, 255744255u, 127110228u, 64000000u,
>>>>>> +                           32258064u, 16016016u, 8000000u, 4000000u,
>>>>>> +                           2000000u, 1000000u, 500000u, 250000u, 125000u};
>>>
>>> ...this one obviously just a not needed one. You may replace it with a
>>> one constant and simple calculation to get either value (index from
>>> value, or value from index).
>>
>> Indeed this can be reduce to:
>>
>> 125.000 << (15 - idx).
>>
>> The real question here is if we approximate 255.744.255u to 256.00.00u how
>> much different is the result. Being a gas sensor I think it is very
>> hard to appreciate.
>>
>> We can go with this formula + adding a comment with the table with the
>> exact coefficients.
> 
> So, I have planned to use this 125000 << (15 - idx) equation with
> approximating the array members.
> 
> About the difference in results we would get after approximating isn't
> much of a problem IMHO because gas sensor is primarily used for IAQ, and
> IAQ is relative to the resistance reading.
> 
> For eg:
> 
> Resistance(ohm)			IAQ
> value < 30K			Very bad
> 30k < value < 50k		worse
> 50k < value < 70k		bad
> ...
> ..			
> so on..
> 
> So, what I simply imply is the scale will be adjusted and nothing else
> changes, unlike if it had been pressure, temperature, humidity.
> 
> The IAQ implementation is userspace application suggesting
> good/bad/ugly air quality.
> 
> And since we know David Frey is planning to use this sensor in his
> product mangOH board.
> 
> So, David, how are you planning to use the gas sensing part in your
> product ? RGB leds, buzzer, alarm ?
> 
> Thanks Andy for the suggestion :)
> 

My understanding is that the Bosch BSEC (Bosch Sensortec Environmental
Cluster - https://www.bosch-sensortec.com/bst/products/all_products/bsec)
software calculates the indoor air quality (IAQ) which is presented in
the range of 0 to 500.  BSEC is proprietary, pre-compiled static
library.  I don't know how they derive the IAQ, but it seems that it
could be based on smoothing outlying gas resistance values and 
integrating other values such as temperature, humidity and pressure.
Unless this driver can somehow produce IAQ values of equal or greater
reliability to the BSEC library, then I would prefer that it just
present the raw gas resistance value so that a user can write a program
to feed the sensor data into BSEC.

mangOH isn't really a traditional product.  It's an open hardware board
designed around Sierra Wireless cellular modules that run Linux.  So I
don't have any specific use case in mind, but I want to enable our users
(and thus future products) to make use of air quality measurements.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor
  2018-07-23 22:16           ` David Frey
@ 2018-07-23 23:02             ` Himanshu Jha
  0 siblings, 0 replies; 11+ messages in thread
From: Himanshu Jha @ 2018-07-23 23:02 UTC (permalink / raw)
  To: David Frey; +Cc: Daniel Baluta, Linux Kernel Mailing List, linux-iio

On Mon, Jul 23, 2018 at 03:16:10PM -0700, David Frey wrote:
> On 7/22/2018 3:21 PM, Himanshu Jha wrote:
> >On Sat, Jul 21, 2018 at 08:45:34PM +0300, Daniel Baluta wrote:
> >>On Sat, Jul 21, 2018 at 6:43 PM, Andy Shevchenko
> >><andy.shevchenko@gmail.com> wrote:
> >>>On Sat, Jul 21, 2018 at 6:36 PM, Himanshu Jha
> >>><himanshujha199640@gmail.com> wrote:
> >>>
> >>>>>>+   /* Look up table 1 for the possible gas range values */
> >>>>>>+   u32 lookupTable1[16] = {2147483647u, 2147483647u, 2147483647u,
> >>>>>>+                           2147483647u, 2147483647u, 2126008810u,
> >>>>>>+                           2147483647u, 2130303777u, 2147483647u,
> >>>>>>+                           2147483647u, 2143188679u, 2136746228u,
> >>>>>>+                           2147483647u, 2126008810u, 2147483647u,
> >>>>>>+                           2147483647u};
> >>>
> >>>This one needs perhaps a bit of though, but...
> >>>
> >>>>>>+   /* Look up table 2 for the possible gas range values */
> >>>>>>+   u32 lookupTable2[16] = {4096000000u, 2048000000u, 1024000000u,
> >>>>>>+                           512000000u, 255744255u, 127110228u, 64000000u,
> >>>>>>+                           32258064u, 16016016u, 8000000u, 4000000u,
> >>>>>>+                           2000000u, 1000000u, 500000u, 250000u, 125000u};
> >>>
> >>>...this one obviously just a not needed one. You may replace it with a
> >>>one constant and simple calculation to get either value (index from
> >>>value, or value from index).
> >>
> >>Indeed this can be reduce to:
> >>
> >>125.000 << (15 - idx).
> >>
> >>The real question here is if we approximate 255.744.255u to 256.00.00u how
> >>much different is the result. Being a gas sensor I think it is very
> >>hard to appreciate.
> >>
> >>We can go with this formula + adding a comment with the table with the
> >>exact coefficients.
> >
> >So, I have planned to use this 125000 << (15 - idx) equation with
> >approximating the array members.
> >
> >About the difference in results we would get after approximating isn't
> >much of a problem IMHO because gas sensor is primarily used for IAQ, and
> >IAQ is relative to the resistance reading.
> >
> >For eg:
> >
> >Resistance(ohm)			IAQ
> >value < 30K			Very bad
> >30k < value < 50k		worse
> >50k < value < 70k		bad
> >...
> >..			
> >so on..
> >
> >So, what I simply imply is the scale will be adjusted and nothing else
> >changes, unlike if it had been pressure, temperature, humidity.
> >
> >The IAQ implementation is userspace application suggesting
> >good/bad/ugly air quality.
> >
> >And since we know David Frey is planning to use this sensor in his
> >product mangOH board.
> >
> >So, David, how are you planning to use the gas sensing part in your
> >product ? RGB leds, buzzer, alarm ?
> >
> >Thanks Andy for the suggestion :)
> >
> 
> My understanding is that the Bosch BSEC (Bosch Sensortec Environmental
> Cluster - https://www.bosch-sensortec.com/bst/products/all_products/bsec)
> software calculates the indoor air quality (IAQ) which is presented in
> the range of 0 to 500.  BSEC is proprietary, pre-compiled static
> library.  I don't know how they derive the IAQ, but it seems that it
> could be based on smoothing outlying gas resistance values and integrating
> other values such as temperature, humidity and pressure.
> Unless this driver can somehow produce IAQ values of equal or greater
> reliability to the BSEC library, then I would prefer that it just
> present the raw gas resistance value so that a user can write a program
> to feed the sensor data into BSEC.
> 
> mangOH isn't really a traditional product.  It's an open hardware board
> designed around Sierra Wireless cellular modules that run Linux.  So I
> don't have any specific use case in mind, but I want to enable our users
> (and thus future products) to make use of air quality measurements.

I tested the gas sensor with the new bitshifting equation and didn't find any
discrepancy in the resistance readings.

Even I don't know how Bosch implements IAQ algorithm, but one thing I
got around testing the sensor was that the air quality is inversely
proportional to resistance value.

[You can try it yourself, by spraying deodrant/aerosol sprays around the sensor ;)]

But we shouldn't be worried about such issues since these should be handled
fine with the userspace application and depends on the implementation.
We only need to focus on exporting data.

Anyway, thanks again for your time!

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-07-23 23:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-20 21:31 [PATCH v4] iio: chemical: Add support for Bosch BME680 sensor Himanshu Jha
2018-07-21 15:19 ` Jonathan Cameron
2018-07-21 15:36   ` Himanshu Jha
2018-07-21 15:43     ` Andy Shevchenko
2018-07-21 15:52       ` Himanshu Jha
2018-07-21 16:29         ` Jonathan Cameron
2018-07-21 17:45       ` Daniel Baluta
2018-07-22 22:21         ` Himanshu Jha
2018-07-23 22:16           ` David Frey
2018-07-23 23:02             ` Himanshu Jha
2018-07-21 16:31     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).