[PATCH] iio/bmp280-core.c: Read calibration data in probe

[PATCH] iio/bmp280-core.c: Read calibration data in probe

[Stefan Tatschner]

This patch affects BME280 and BMP280. The readout of the calibration
data is moved to the probe function. Each sensor data access triggered
reading the full calibration data before this patch. According to the
datasheet, Section 4.4.2., the calibration data is stored in non-volatile
memory.

Since the calibration data does not change, and cannot be changed by the
user, we can reduce bus traffic by reading the calibration data once.
Additionally, proper organization of the data types enables removing
some odd casts in the compensation formulas.

Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
 1 file changed, 134 insertions(+), 71 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index fd1da26a62e4..0316d1003d7e 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -55,6 +55,28 @@ struct bmp180_calib {
 	s16 MD;
 };
 
+/* See datasheet Section 4.2.2. */
+struct bmp280_calib {
+	u16 T1;
+	s16 T2;
+	s16 T3;
+	u16 P1;
+	s16 P2;
+	s16 P3;
+	s16 P4;
+	s16 P5;
+	s16 P6;
+	s16 P7;
+	s16 P8;
+	s16 P9;
+	u8  H1;
+	s16 H2;
+	u8  H3;
+	s16 H4;
+	s16 H5;
+	s8  H6;
+};
+
 struct bmp280_data {
 	struct device *dev;
 	struct mutex lock;
@@ -62,7 +84,10 @@ struct bmp280_data {
 	struct completion done;
 	bool use_eoc;
 	const struct bmp280_chip_info *chip_info;
-	struct bmp180_calib calib;
+	union {
+		struct bmp180_calib bmp180;
+		struct bmp280_calib bmp280;
+	} calib;
 	struct regulator *vddd;
 	struct regulator *vdda;
 	unsigned int start_up_time; /* in microseconds */
@@ -120,67 +145,123 @@ static const struct iio_chan_spec bmp280_channels[] = {
 	},
 };
 
-/*
- * Returns humidity in percent, resolution is 0.01 percent. Output value of
- * "47445" represents 47445/1024 = 46.333 %RH.
- *
- * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
- */
-
-static u32 bmp280_compensate_humidity(struct bmp280_data *data,
-				      s32 adc_humidity)
+static int bmp280_read_calib(struct bmp280_data *data,
+			     struct bmp280_calib *calib,
+			     unsigned int chip)
 {
+	int ret;
+	unsigned int tmp;
 	struct device *dev = data->dev;
-	unsigned int H1, H3, tmp;
-	int H2, H4, H5, H6, ret, var;
+	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
+	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
+
+	/* Read temperature calibration values. */
+	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
+			       t_buf, BMP280_COMP_TEMP_REG_COUNT);
+	if (ret < 0) {
+		dev_err(data->dev,
+			"failed to read temperature calibration parameters\n");
+		return ret;
+	}
+
+	calib->T1 = le16_to_cpu(t_buf[T1]);
+	calib->T2 = le16_to_cpu(t_buf[T2]);
+	calib->T3 = le16_to_cpu(t_buf[T3]);
 
-	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &H1);
+	/* Read pressure calibration values. */
+	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
+			       p_buf, BMP280_COMP_PRESS_REG_COUNT);
+	if (ret < 0) {
+		dev_err(data->dev,
+			"failed to read pressure calibration parameters\n");
+		return ret;
+	}
+
+	calib->P1 = le16_to_cpu(p_buf[P1]);
+	calib->P2 = le16_to_cpu(p_buf[P2]);
+	calib->P3 = le16_to_cpu(p_buf[P3]);
+	calib->P4 = le16_to_cpu(p_buf[P4]);
+	calib->P5 = le16_to_cpu(p_buf[P5]);
+	calib->P6 = le16_to_cpu(p_buf[P6]);
+	calib->P7 = le16_to_cpu(p_buf[P7]);
+	calib->P8 = le16_to_cpu(p_buf[P8]);
+	calib->P9 = le16_to_cpu(p_buf[P9]);
+
+	/* Read humidity calibration values.
+	 * Due to some odd register addressing we cannot just
+	 * do a big bulk read. Instead, we have to read each HX
+	 * value separately and sometimes do some bit shifting...
+	 * Humidity data is only available on BME280.
+	 */
+	if (chip != BME280_CHIP_ID)
+		return 0;
+
+	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H1 comp value\n");
 		return ret;
 	}
+	calib->H1 = tmp;
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H2 comp value\n");
 		return ret;
 	}
-	H2 = sign_extend32(le16_to_cpu(tmp), 15);
+	calib->H2 = sign_extend32(le16_to_cpu(tmp), 15);
 
-	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &H3);
+	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H3 comp value\n");
 		return ret;
 	}
+	calib->H3 = tmp;
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H4 comp value\n");
 		return ret;
 	}
-	H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
-			  (be16_to_cpu(tmp) & 0xf), 11);
+	calib->H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
+				  (be16_to_cpu(tmp) & 0xf), 11);
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H5 comp value\n");
 		return ret;
 	}
-	H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
+	calib->H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
 
 	ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H6 comp value\n");
 		return ret;
 	}
-	H6 = sign_extend32(tmp, 7);
+	calib->H6 = sign_extend32(tmp, 7);
+
+	/* Toss the calibration data into the entropy pool */
+	add_device_randomness(calib, sizeof(struct bmp280_calib));
+
+	return 0;
+}
+/*
+ * Returns humidity in percent, resolution is 0.01 percent. Output value of
+ * "47445" represents 47445/1024 = 46.333 %RH.
+ *
+ * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
+ */
+static u32 bmp280_compensate_humidity(struct bmp280_data *data,
+				      s32 adc_humidity)
+{
+	s32 var;
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
 	var = ((s32)data->t_fine) - (s32)76800;
-	var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var))
-		+ (s32)16384) >> 15) * (((((((var * H6) >> 10)
-		* (((var * (s32)H3) >> 11) + (s32)32768)) >> 10)
-		+ (s32)2097152) * H2 + 8192) >> 14);
-	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)H1) >> 4;
+	var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
+		+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
+		* (((var * calib->H3) >> 11) + 32768)) >> 10)
+		+ (s32)2097152) * calib->H2 + 8192) >> 14);
+	var -= ((((var >> 15) * (var >> 15)) >> 7) * calib->H1) >> 4;
 
 	return var >> 12;
 };
@@ -195,31 +276,14 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
 static s32 bmp280_compensate_temp(struct bmp280_data *data,
 				  s32 adc_temp)
 {
-	int ret;
 	s32 var1, var2;
-	__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
-	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
-			       buf, BMP280_COMP_TEMP_REG_COUNT);
-	if (ret < 0) {
-		dev_err(data->dev,
-			"failed to read temperature calibration parameters\n");
-		return ret;
-	}
-
-	/*
-	 * The double casts are necessary because le16_to_cpu returns an
-	 * unsigned 16-bit value.  Casting that value directly to a
-	 * signed 32-bit will not do proper sign extension.
-	 *
-	 * Conversely, T1 and P1 are unsigned values, so they can be
-	 * cast straight to the larger type.
-	 */
-	var1 = (((adc_temp >> 3) - ((s32)le16_to_cpu(buf[T1]) << 1)) *
-		((s32)(s16)le16_to_cpu(buf[T2]))) >> 11;
-	var2 = (((((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1]))) *
-		  ((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1])))) >> 12) *
-		((s32)(s16)le16_to_cpu(buf[T3]))) >> 14;
+	var1 = (((adc_temp >> 3) - ((s32)calib->T1 << 1)) *
+		((s32)calib->T2)) >> 11;
+	var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
+		  ((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
+		((s32)calib->T3)) >> 14;
 	data->t_fine = var1 + var2;
 
 	return (data->t_fine * 5 + 128) >> 8;
@@ -235,34 +299,25 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
 static u32 bmp280_compensate_press(struct bmp280_data *data,
 				   s32 adc_press)
 {
-	int ret;
 	s64 var1, var2, p;
-	__le16 buf[BMP280_COMP_PRESS_REG_COUNT / 2];
-
-	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
-			       buf, BMP280_COMP_PRESS_REG_COUNT);
-	if (ret < 0) {
-		dev_err(data->dev,
-			"failed to read pressure calibration parameters\n");
-		return ret;
-	}
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
 	var1 = ((s64)data->t_fine) - 128000;
-	var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
-	var2 += (var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17;
-	var2 += ((s64)(s16)le16_to_cpu(buf[P4])) << 35;
-	var1 = ((var1 * var1 * (s64)(s16)le16_to_cpu(buf[P3])) >> 8) +
-		((var1 * (s64)(s16)le16_to_cpu(buf[P2])) << 12);
-	var1 = ((((s64)1) << 47) + var1) * ((s64)le16_to_cpu(buf[P1])) >> 33;
+	var2 = var1 * var1 * (s64)calib->P6;
+	var2 += (var1 * (s64)calib->P5) << 17;
+	var2 += ((s64)calib->P4) << 35;
+	var1 = ((var1 * var1 * (s64)calib->P3) >> 8) +
+		((var1 * (s64)calib->P2) << 12);
+	var1 = ((((s64)1) << 47) + var1) * ((s64)calib->P1) >> 33;
 
 	if (var1 == 0)
 		return 0;
 
 	p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
 	p = div64_s64(p, var1);
-	var1 = (((s64)(s16)le16_to_cpu(buf[P9])) * (p >> 13) * (p >> 13)) >> 25;
-	var2 = (((s64)(s16)le16_to_cpu(buf[P8])) * p) >> 19;
-	p = ((p + var1 + var2) >> 8) + (((s64)(s16)le16_to_cpu(buf[P7])) << 4);
+	var1 = (((s64)calib->P9) * (p >> 13) * (p >> 13)) >> 25;
+	var2 = ((s64)(calib->P8) * p) >> 19;
+	p = ((p + var1 + var2) >> 8) + (((s64)calib->P7) << 4);
 
 	return (u32)p;
 }
@@ -752,7 +807,7 @@ static int bmp180_read_calib(struct bmp280_data *data,
 static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
 {
 	s32 x1, x2;
-	struct bmp180_calib *calib = &data->calib;
+	struct bmp180_calib *calib = &data->calib.bmp180;
 
 	x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
 	x2 = (calib->MC << 11) / (x1 + calib->MD);
@@ -814,7 +869,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
 	s32 b3, b6;
 	u32 b4, b7;
 	s32 oss = data->oversampling_press;
-	struct bmp180_calib *calib = &data->calib;
+	struct bmp180_calib *calib = &data->calib.bmp180;
 
 	b6 = data->t_fine - 4000;
 	x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
@@ -1028,11 +1083,19 @@ int bmp280_common_probe(struct device *dev,
 	dev_set_drvdata(dev, indio_dev);
 
 	/*
-	 * The BMP085 and BMP180 has calibration in an E2PROM, read it out
-	 * at probe time. It will not change.
+	 * Some chips have calibration parameters "programmed into the devices'
+	 * non-volatile memory during production". Let's read them out at probe
+	 * time once. They will not change.
 	 */
 	if (chip_id  == BMP180_CHIP_ID) {
-		ret = bmp180_read_calib(data, &data->calib);
+		ret = bmp180_read_calib(data, &data->calib.bmp180);
+		if (ret < 0) {
+			dev_err(data->dev,
+				"failed to read calibration coefficients\n");
+			goto out_disable_vdda;
+		}
+	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
+		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
 		if (ret < 0) {
 			dev_err(data->dev,
 				"failed to read calibration coefficients\n");
-- 
2.15.1

Re: [PATCH] iio/bmp280-core.c: Read calibration data in probe

[Andreas Klinger]

Hi Stefan,

sie comment below.


Stefan Tatschner <stefan.tatschner@gmail.com> schrieb am Tue, 12. Dec 15:34:
> This patch affects BME280 and BMP280. The readout of the calibration
> data is moved to the probe function. Each sensor data access triggered
> reading the full calibration data before this patch. According to the
> datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> memory.
> 
> Since the calibration data does not change, and cannot be changed by the
> user, we can reduce bus traffic by reading the calibration data once.
> Additionally, proper organization of the data types enables removing
> some odd casts in the compensation formulas.
> 
> Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>
> ---
>  drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
>  1 file changed, 134 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index fd1da26a62e4..0316d1003d7e 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -55,6 +55,28 @@ struct bmp180_calib {
>  	s16 MD;
>  };
>  
> +/* See datasheet Section 4.2.2. */
> +struct bmp280_calib {
> +	u16 T1;
> +	s16 T2;
> +	s16 T3;
> +	u16 P1;
> +	s16 P2;
> +	s16 P3;
> +	s16 P4;
> +	s16 P5;
> +	s16 P6;
> +	s16 P7;
> +	s16 P8;
> +	s16 P9;
> +	u8  H1;
> +	s16 H2;
> +	u8  H3;
> +	s16 H4;
> +	s16 H5;
> +	s8  H6;
> +};
> +
>  struct bmp280_data {
>  	struct device *dev;
>  	struct mutex lock;
> @@ -62,7 +84,10 @@ struct bmp280_data {
>  	struct completion done;
>  	bool use_eoc;
>  	const struct bmp280_chip_info *chip_info;
> -	struct bmp180_calib calib;
> +	union {
> +		struct bmp180_calib bmp180;
> +		struct bmp280_calib bmp280;
> +	} calib;
>  	struct regulator *vddd;
>  	struct regulator *vdda;
>  	unsigned int start_up_time; /* in microseconds */
> @@ -120,67 +145,123 @@ static const struct iio_chan_spec bmp280_channels[] = {
>  	},
>  };
>  
> -/*
> - * Returns humidity in percent, resolution is 0.01 percent. Output value of
> - * "47445" represents 47445/1024 = 46.333 %RH.
> - *
> - * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> - */
> -
> -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> -				      s32 adc_humidity)
> +static int bmp280_read_calib(struct bmp280_data *data,
> +			     struct bmp280_calib *calib,
> +			     unsigned int chip)
>  {
> +	int ret;
> +	unsigned int tmp;
>  	struct device *dev = data->dev;
> -	unsigned int H1, H3, tmp;
> -	int H2, H4, H5, H6, ret, var;
> +	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> +
> +	/* Read temperature calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> +			       t_buf, BMP280_COMP_TEMP_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read temperature calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->T1 = le16_to_cpu(t_buf[T1]);
> +	calib->T2 = le16_to_cpu(t_buf[T2]);
> +	calib->T3 = le16_to_cpu(t_buf[T3]);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &H1);
> +	/* Read pressure calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> +			       p_buf, BMP280_COMP_PRESS_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read pressure calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->P1 = le16_to_cpu(p_buf[P1]);
> +	calib->P2 = le16_to_cpu(p_buf[P2]);
> +	calib->P3 = le16_to_cpu(p_buf[P3]);
> +	calib->P4 = le16_to_cpu(p_buf[P4]);
> +	calib->P5 = le16_to_cpu(p_buf[P5]);
> +	calib->P6 = le16_to_cpu(p_buf[P6]);
> +	calib->P7 = le16_to_cpu(p_buf[P7]);
> +	calib->P8 = le16_to_cpu(p_buf[P8]);
> +	calib->P9 = le16_to_cpu(p_buf[P9]);
> +
> +	/* Read humidity calibration values.
> +	 * Due to some odd register addressing we cannot just
> +	 * do a big bulk read. Instead, we have to read each HX
> +	 * value separately and sometimes do some bit shifting...
> +	 * Humidity data is only available on BME280.
> +	 */
> +	if (chip != BME280_CHIP_ID)
> +		return 0;
> +
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H1 comp value\n");
>  		return ret;
>  	}
> +	calib->H1 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H2 comp value\n");
>  		return ret;
>  	}
> -	H2 = sign_extend32(le16_to_cpu(tmp), 15);
> +	calib->H2 = sign_extend32(le16_to_cpu(tmp), 15);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &H3);
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H3 comp value\n");
>  		return ret;
>  	}
> +	calib->H3 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H4 comp value\n");
>  		return ret;
>  	}
> -	H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> -			  (be16_to_cpu(tmp) & 0xf), 11);
> +	calib->H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> +				  (be16_to_cpu(tmp) & 0xf), 11);
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H5 comp value\n");
>  		return ret;
>  	}
> -	H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
> +	calib->H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
>  
>  	ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H6 comp value\n");
>  		return ret;
>  	}
> -	H6 = sign_extend32(tmp, 7);
> +	calib->H6 = sign_extend32(tmp, 7);
> +
> +	/* Toss the calibration data into the entropy pool */
> +	add_device_randomness(calib, sizeof(struct bmp280_calib));
> +
> +	return 0;
> +}
> +/*
> + * Returns humidity in percent, resolution is 0.01 percent. Output value of
> + * "47445" represents 47445/1024 = 46.333 %RH.
> + *
> + * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> + */
> +static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> +				      s32 adc_humidity)
> +{
> +	s32 var;
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var = ((s32)data->t_fine) - (s32)76800;
> -	var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var))
> -		+ (s32)16384) >> 15) * (((((((var * H6) >> 10)
> -		* (((var * (s32)H3) >> 11) + (s32)32768)) >> 10)
> -		+ (s32)2097152) * H2 + 8192) >> 14);
> -	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)H1) >> 4;
> +	var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
> +		+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
> +		* (((var * calib->H3) >> 11) + 32768)) >> 10)
> +		+ (s32)2097152) * calib->H2 + 8192) >> 14);
> +	var -= ((((var >> 15) * (var >> 15)) >> 7) * calib->H1) >> 4;

This would revert partly commit ed3730c435f1a9f9559ed7762035d22d8a95adfe

There was a discussion about it on this mailing list with the subject 
"[PATCH] IIO: bmp280-core.c: fix error in humidity calculation" in April.

You are right. The casts seem to be not needed. But the humidity is not correct
without them.

Andreas

>  
>  	return var >> 12;
>  };
> @@ -195,31 +276,14 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
>  static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  				  s32 adc_temp)
>  {
> -	int ret;
>  	s32 var1, var2;
> -	__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> -			       buf, BMP280_COMP_TEMP_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read temperature calibration parameters\n");
> -		return ret;
> -	}
> -
> -	/*
> -	 * The double casts are necessary because le16_to_cpu returns an
> -	 * unsigned 16-bit value.  Casting that value directly to a
> -	 * signed 32-bit will not do proper sign extension.
> -	 *
> -	 * Conversely, T1 and P1 are unsigned values, so they can be
> -	 * cast straight to the larger type.
> -	 */
> -	var1 = (((adc_temp >> 3) - ((s32)le16_to_cpu(buf[T1]) << 1)) *
> -		((s32)(s16)le16_to_cpu(buf[T2]))) >> 11;
> -	var2 = (((((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1]))) *
> -		  ((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1])))) >> 12) *
> -		((s32)(s16)le16_to_cpu(buf[T3]))) >> 14;
> +	var1 = (((adc_temp >> 3) - ((s32)calib->T1 << 1)) *
> +		((s32)calib->T2)) >> 11;
> +	var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
> +		  ((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
> +		((s32)calib->T3)) >> 14;
>  	data->t_fine = var1 + var2;
>  
>  	return (data->t_fine * 5 + 128) >> 8;
> @@ -235,34 +299,25 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  static u32 bmp280_compensate_press(struct bmp280_data *data,
>  				   s32 adc_press)
>  {
> -	int ret;
>  	s64 var1, var2, p;
> -	__le16 buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> -
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> -			       buf, BMP280_COMP_PRESS_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read pressure calibration parameters\n");
> -		return ret;
> -	}
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var1 = ((s64)data->t_fine) - 128000;
> -	var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
> -	var2 += (var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17;
> -	var2 += ((s64)(s16)le16_to_cpu(buf[P4])) << 35;
> -	var1 = ((var1 * var1 * (s64)(s16)le16_to_cpu(buf[P3])) >> 8) +
> -		((var1 * (s64)(s16)le16_to_cpu(buf[P2])) << 12);
> -	var1 = ((((s64)1) << 47) + var1) * ((s64)le16_to_cpu(buf[P1])) >> 33;
> +	var2 = var1 * var1 * (s64)calib->P6;
> +	var2 += (var1 * (s64)calib->P5) << 17;
> +	var2 += ((s64)calib->P4) << 35;
> +	var1 = ((var1 * var1 * (s64)calib->P3) >> 8) +
> +		((var1 * (s64)calib->P2) << 12);
> +	var1 = ((((s64)1) << 47) + var1) * ((s64)calib->P1) >> 33;
>  
>  	if (var1 == 0)
>  		return 0;
>  
>  	p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
>  	p = div64_s64(p, var1);
> -	var1 = (((s64)(s16)le16_to_cpu(buf[P9])) * (p >> 13) * (p >> 13)) >> 25;
> -	var2 = (((s64)(s16)le16_to_cpu(buf[P8])) * p) >> 19;
> -	p = ((p + var1 + var2) >> 8) + (((s64)(s16)le16_to_cpu(buf[P7])) << 4);
> +	var1 = (((s64)calib->P9) * (p >> 13) * (p >> 13)) >> 25;
> +	var2 = ((s64)(calib->P8) * p) >> 19;
> +	p = ((p + var1 + var2) >> 8) + (((s64)calib->P7) << 4);
>  
>  	return (u32)p;
>  }
> @@ -752,7 +807,7 @@ static int bmp180_read_calib(struct bmp280_data *data,
>  static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
>  {
>  	s32 x1, x2;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
>  	x2 = (calib->MC << 11) / (x1 + calib->MD);
> @@ -814,7 +869,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
>  	s32 b3, b6;
>  	u32 b4, b7;
>  	s32 oss = data->oversampling_press;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	b6 = data->t_fine - 4000;
>  	x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
> @@ -1028,11 +1083,19 @@ int bmp280_common_probe(struct device *dev,
>  	dev_set_drvdata(dev, indio_dev);
>  
>  	/*
> -	 * The BMP085 and BMP180 has calibration in an E2PROM, read it out
> -	 * at probe time. It will not change.
> +	 * Some chips have calibration parameters "programmed into the devices'
> +	 * non-volatile memory during production". Let's read them out at probe
> +	 * time once. They will not change.
>  	 */
>  	if (chip_id  == BMP180_CHIP_ID) {
> -		ret = bmp180_read_calib(data, &data->calib);
> +		ret = bmp180_read_calib(data, &data->calib.bmp180);
> +		if (ret < 0) {
> +			dev_err(data->dev,
> +				"failed to read calibration coefficients\n");
> +			goto out_disable_vdda;
> +		}
> +	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> +		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de

[PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Stefan Tatschner]

This patch affects BME280 and BMP280. The readout of the calibration
data is moved to the probe function. Each sensor data access triggered
reading the full calibration data before this patch. According to the
datasheet, Section 4.4.2., the calibration data is stored in non-volatile
memory.

Since the calibration data does not change, and cannot be changed by the
user, we can reduce bus traffic by reading the calibration data once.
Additionally, proper organization of the data types enables removing
some odd casts in the compensation formulas.

Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>
---

v2 changes:
    - readded the type casts in temperature calibration values
    - fix typo

 drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
 1 file changed, 134 insertions(+), 71 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index fd1da26a62e4..a326d68dd668 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -55,6 +55,28 @@ struct bmp180_calib {
 	s16 MD;
 };
 
+/* See datasheet Section 4.2.2. */
+struct bmp280_calib {
+	u16 T1;
+	s16 T2;
+	s16 T3;
+	u16 P1;
+	s16 P2;
+	s16 P3;
+	s16 P4;
+	s16 P5;
+	s16 P6;
+	s16 P7;
+	s16 P8;
+	s16 P9;
+	u8  H1;
+	s16 H2;
+	u8  H3;
+	s16 H4;
+	s16 H5;
+	s8  H6;
+};
+
 struct bmp280_data {
 	struct device *dev;
 	struct mutex lock;
@@ -62,7 +84,10 @@ struct bmp280_data {
 	struct completion done;
 	bool use_eoc;
 	const struct bmp280_chip_info *chip_info;
-	struct bmp180_calib calib;
+	union {
+		struct bmp180_calib bmp180;
+		struct bmp280_calib bmp280;
+	} calib;
 	struct regulator *vddd;
 	struct regulator *vdda;
 	unsigned int start_up_time; /* in microseconds */
@@ -120,67 +145,123 @@ static const struct iio_chan_spec bmp280_channels[] = {
 	},
 };
 
-/*
- * Returns humidity in percent, resolution is 0.01 percent. Output value of
- * "47445" represents 47445/1024 = 46.333 %RH.
- *
- * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
- */
-
-static u32 bmp280_compensate_humidity(struct bmp280_data *data,
-				      s32 adc_humidity)
+static int bmp280_read_calib(struct bmp280_data *data,
+			     struct bmp280_calib *calib,
+			     unsigned int chip)
 {
+	int ret;
+	unsigned int tmp;
 	struct device *dev = data->dev;
-	unsigned int H1, H3, tmp;
-	int H2, H4, H5, H6, ret, var;
+	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
+	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
+
+	/* Read temperature calibration values. */
+	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
+			       t_buf, BMP280_COMP_TEMP_REG_COUNT);
+	if (ret < 0) {
+		dev_err(data->dev,
+			"failed to read temperature calibration parameters\n");
+		return ret;
+	}
+
+	calib->T1 = le16_to_cpu(t_buf[T1]);
+	calib->T2 = le16_to_cpu(t_buf[T2]);
+	calib->T3 = le16_to_cpu(t_buf[T3]);
 
-	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &H1);
+	/* Read pressure calibration values. */
+	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
+			       p_buf, BMP280_COMP_PRESS_REG_COUNT);
+	if (ret < 0) {
+		dev_err(data->dev,
+			"failed to read pressure calibration parameters\n");
+		return ret;
+	}
+
+	calib->P1 = le16_to_cpu(p_buf[P1]);
+	calib->P2 = le16_to_cpu(p_buf[P2]);
+	calib->P3 = le16_to_cpu(p_buf[P3]);
+	calib->P4 = le16_to_cpu(p_buf[P4]);
+	calib->P5 = le16_to_cpu(p_buf[P5]);
+	calib->P6 = le16_to_cpu(p_buf[P6]);
+	calib->P7 = le16_to_cpu(p_buf[P7]);
+	calib->P8 = le16_to_cpu(p_buf[P8]);
+	calib->P9 = le16_to_cpu(p_buf[P9]);
+
+	/* Read humidity calibration values.
+	 * Due to some odd register addressing we cannot just
+	 * do a big bulk read. Instead, we have to read each Hx
+	 * value separately and sometimes do some bit shifting...
+	 * Humidity data is only available on BME280.
+	 */
+	if (chip != BME280_CHIP_ID)
+		return 0;
+
+	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H1 comp value\n");
 		return ret;
 	}
+	calib->H1 = tmp;
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H2 comp value\n");
 		return ret;
 	}
-	H2 = sign_extend32(le16_to_cpu(tmp), 15);
+	calib->H2 = sign_extend32(le16_to_cpu(tmp), 15);
 
-	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &H3);
+	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H3 comp value\n");
 		return ret;
 	}
+	calib->H3 = tmp;
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H4 comp value\n");
 		return ret;
 	}
-	H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
-			  (be16_to_cpu(tmp) & 0xf), 11);
+	calib->H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
+				  (be16_to_cpu(tmp) & 0xf), 11);
 
 	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &tmp, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H5 comp value\n");
 		return ret;
 	}
-	H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
+	calib->H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
 
 	ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read H6 comp value\n");
 		return ret;
 	}
-	H6 = sign_extend32(tmp, 7);
+	calib->H6 = sign_extend32(tmp, 7);
+
+	/* Toss the calibration data into the entropy pool */
+	add_device_randomness(calib, sizeof(struct bmp280_calib));
+
+	return 0;
+}
+/*
+ * Returns humidity in percent, resolution is 0.01 percent. Output value of
+ * "47445" represents 47445/1024 = 46.333 %RH.
+ *
+ * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
+ */
+static u32 bmp280_compensate_humidity(struct bmp280_data *data,
+				      s32 adc_humidity)
+{
+	s32 var;
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
 	var = ((s32)data->t_fine) - (s32)76800;
-	var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var))
-		+ (s32)16384) >> 15) * (((((((var * H6) >> 10)
-		* (((var * (s32)H3) >> 11) + (s32)32768)) >> 10)
-		+ (s32)2097152) * H2 + 8192) >> 14);
-	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)H1) >> 4;
+	var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
+		+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
+		* (((var * (s32)calib->H3) >> 11) + (s32)32768)) >> 10)
+		+ (s32)2097152) * calib->H2 + 8192) >> 14);
+	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)calib->H1) >> 4;
 
 	return var >> 12;
 };
@@ -195,31 +276,14 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
 static s32 bmp280_compensate_temp(struct bmp280_data *data,
 				  s32 adc_temp)
 {
-	int ret;
 	s32 var1, var2;
-	__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
-	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
-			       buf, BMP280_COMP_TEMP_REG_COUNT);
-	if (ret < 0) {
-		dev_err(data->dev,
-			"failed to read temperature calibration parameters\n");
-		return ret;
-	}
-
-	/*
-	 * The double casts are necessary because le16_to_cpu returns an
-	 * unsigned 16-bit value.  Casting that value directly to a
-	 * signed 32-bit will not do proper sign extension.
-	 *
-	 * Conversely, T1 and P1 are unsigned values, so they can be
-	 * cast straight to the larger type.
-	 */
-	var1 = (((adc_temp >> 3) - ((s32)le16_to_cpu(buf[T1]) << 1)) *
-		((s32)(s16)le16_to_cpu(buf[T2]))) >> 11;
-	var2 = (((((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1]))) *
-		  ((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1])))) >> 12) *
-		((s32)(s16)le16_to_cpu(buf[T3]))) >> 14;
+	var1 = (((adc_temp >> 3) - ((s32)calib->T1 << 1)) *
+		((s32)calib->T2)) >> 11;
+	var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
+		  ((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
+		((s32)calib->T3)) >> 14;
 	data->t_fine = var1 + var2;
 
 	return (data->t_fine * 5 + 128) >> 8;
@@ -235,34 +299,25 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
 static u32 bmp280_compensate_press(struct bmp280_data *data,
 				   s32 adc_press)
 {
-	int ret;
 	s64 var1, var2, p;
-	__le16 buf[BMP280_COMP_PRESS_REG_COUNT / 2];
-
-	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
-			       buf, BMP280_COMP_PRESS_REG_COUNT);
-	if (ret < 0) {
-		dev_err(data->dev,
-			"failed to read pressure calibration parameters\n");
-		return ret;
-	}
+	struct bmp280_calib *calib = &data->calib.bmp280;
 
 	var1 = ((s64)data->t_fine) - 128000;
-	var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
-	var2 += (var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17;
-	var2 += ((s64)(s16)le16_to_cpu(buf[P4])) << 35;
-	var1 = ((var1 * var1 * (s64)(s16)le16_to_cpu(buf[P3])) >> 8) +
-		((var1 * (s64)(s16)le16_to_cpu(buf[P2])) << 12);
-	var1 = ((((s64)1) << 47) + var1) * ((s64)le16_to_cpu(buf[P1])) >> 33;
+	var2 = var1 * var1 * (s64)calib->P6;
+	var2 += (var1 * (s64)calib->P5) << 17;
+	var2 += ((s64)calib->P4) << 35;
+	var1 = ((var1 * var1 * (s64)calib->P3) >> 8) +
+		((var1 * (s64)calib->P2) << 12);
+	var1 = ((((s64)1) << 47) + var1) * ((s64)calib->P1) >> 33;
 
 	if (var1 == 0)
 		return 0;
 
 	p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
 	p = div64_s64(p, var1);
-	var1 = (((s64)(s16)le16_to_cpu(buf[P9])) * (p >> 13) * (p >> 13)) >> 25;
-	var2 = (((s64)(s16)le16_to_cpu(buf[P8])) * p) >> 19;
-	p = ((p + var1 + var2) >> 8) + (((s64)(s16)le16_to_cpu(buf[P7])) << 4);
+	var1 = (((s64)calib->P9) * (p >> 13) * (p >> 13)) >> 25;
+	var2 = ((s64)(calib->P8) * p) >> 19;
+	p = ((p + var1 + var2) >> 8) + (((s64)calib->P7) << 4);
 
 	return (u32)p;
 }
@@ -752,7 +807,7 @@ static int bmp180_read_calib(struct bmp280_data *data,
 static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
 {
 	s32 x1, x2;
-	struct bmp180_calib *calib = &data->calib;
+	struct bmp180_calib *calib = &data->calib.bmp180;
 
 	x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
 	x2 = (calib->MC << 11) / (x1 + calib->MD);
@@ -814,7 +869,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
 	s32 b3, b6;
 	u32 b4, b7;
 	s32 oss = data->oversampling_press;
-	struct bmp180_calib *calib = &data->calib;
+	struct bmp180_calib *calib = &data->calib.bmp180;
 
 	b6 = data->t_fine - 4000;
 	x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
@@ -1028,11 +1083,19 @@ int bmp280_common_probe(struct device *dev,
 	dev_set_drvdata(dev, indio_dev);
 
 	/*
-	 * The BMP085 and BMP180 has calibration in an E2PROM, read it out
-	 * at probe time. It will not change.
+	 * Some chips have calibration parameters "programmed into the devices'
+	 * non-volatile memory during production". Let's read them out at probe
+	 * time once. They will not change.
 	 */
 	if (chip_id  == BMP180_CHIP_ID) {
-		ret = bmp180_read_calib(data, &data->calib);
+		ret = bmp180_read_calib(data, &data->calib.bmp180);
+		if (ret < 0) {
+			dev_err(data->dev,
+				"failed to read calibration coefficients\n");
+			goto out_disable_vdda;
+		}
+	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
+		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
 		if (ret < 0) {
 			dev_err(data->dev,
 				"failed to read calibration coefficients\n");
-- 
2.15.1

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Stefan Tatschner]

On Tue, 2017-12-12 at 21:35 +0100, Stefan Tatschner wrote:
>     - readded the type casts in temperature calibration values

Oops, sorry. I mean the humidity calibration values.

Stefan

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Stefan Tatschner]

Hi,

On Tue, Dec 12, 2017 at 09:35:37PM +0100, Stefan Tatschner wrote:
> This patch affects BME280 and BMP280. The readout of the calibration
> data is moved to the probe function. Each sensor data access triggered
> reading the full calibration data before this patch. According to the
> datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> memory.
> 
> Since the calibration data does not change, and cannot be changed by the
> user, we can reduce bus traffic by reading the calibration data once.
> Additionally, proper organization of the data types enables removing
> some odd casts in the compensation formulas.
> 
> Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>

what's the status of this? Are there any further opinions on this?  I am
happy to address your comments on my first kernel patch. :)

Stefan

> ---
> 
> v2 changes:
>     - readded the type casts in humidity calibration values
>     - fix typo
> 
>  drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
>  1 file changed, 134 insertions(+), 71 deletions(-)

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Andreas Klinger]

Stefan Tatschner <stefan.tatschner@gmail.com> schrieb am Tue, 12. Dec 21:35:
> This patch affects BME280 and BMP280. The readout of the calibration
> data is moved to the probe function. Each sensor data access triggered
> reading the full calibration data before this patch. According to the
> datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> memory.
> 
> Since the calibration data does not change, and cannot be changed by the
> user, we can reduce bus traffic by reading the calibration data once.
> Additionally, proper organization of the data types enables removing
> some odd casts in the compensation formulas.
> 
> Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>

Tested-by: Andreas Klinger <ak@it-klinger.de>

> ---
> 
> v2 changes:
>     - readded the type casts in temperature calibration values
>     - fix typo
> 
>  drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
>  1 file changed, 134 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index fd1da26a62e4..a326d68dd668 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -55,6 +55,28 @@ struct bmp180_calib {
>  	s16 MD;
>  };
>  
> +/* See datasheet Section 4.2.2. */
> +struct bmp280_calib {
> +	u16 T1;
> +	s16 T2;
> +	s16 T3;
> +	u16 P1;
> +	s16 P2;
> +	s16 P3;
> +	s16 P4;
> +	s16 P5;
> +	s16 P6;
> +	s16 P7;
> +	s16 P8;
> +	s16 P9;
> +	u8  H1;
> +	s16 H2;
> +	u8  H3;
> +	s16 H4;
> +	s16 H5;
> +	s8  H6;
> +};
> +
>  struct bmp280_data {
>  	struct device *dev;
>  	struct mutex lock;
> @@ -62,7 +84,10 @@ struct bmp280_data {
>  	struct completion done;
>  	bool use_eoc;
>  	const struct bmp280_chip_info *chip_info;
> -	struct bmp180_calib calib;
> +	union {
> +		struct bmp180_calib bmp180;
> +		struct bmp280_calib bmp280;
> +	} calib;
>  	struct regulator *vddd;
>  	struct regulator *vdda;
>  	unsigned int start_up_time; /* in microseconds */
> @@ -120,67 +145,123 @@ static const struct iio_chan_spec bmp280_channels[] = {
>  	},
>  };
>  
> -/*
> - * Returns humidity in percent, resolution is 0.01 percent. Output value of
> - * "47445" represents 47445/1024 = 46.333 %RH.
> - *
> - * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> - */
> -
> -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> -				      s32 adc_humidity)
> +static int bmp280_read_calib(struct bmp280_data *data,
> +			     struct bmp280_calib *calib,
> +			     unsigned int chip)
>  {
> +	int ret;
> +	unsigned int tmp;
>  	struct device *dev = data->dev;
> -	unsigned int H1, H3, tmp;
> -	int H2, H4, H5, H6, ret, var;
> +	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> +
> +	/* Read temperature calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> +			       t_buf, BMP280_COMP_TEMP_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read temperature calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->T1 = le16_to_cpu(t_buf[T1]);
> +	calib->T2 = le16_to_cpu(t_buf[T2]);
> +	calib->T3 = le16_to_cpu(t_buf[T3]);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &H1);
> +	/* Read pressure calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> +			       p_buf, BMP280_COMP_PRESS_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read pressure calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->P1 = le16_to_cpu(p_buf[P1]);
> +	calib->P2 = le16_to_cpu(p_buf[P2]);
> +	calib->P3 = le16_to_cpu(p_buf[P3]);
> +	calib->P4 = le16_to_cpu(p_buf[P4]);
> +	calib->P5 = le16_to_cpu(p_buf[P5]);
> +	calib->P6 = le16_to_cpu(p_buf[P6]);
> +	calib->P7 = le16_to_cpu(p_buf[P7]);
> +	calib->P8 = le16_to_cpu(p_buf[P8]);
> +	calib->P9 = le16_to_cpu(p_buf[P9]);
> +
> +	/* Read humidity calibration values.
> +	 * Due to some odd register addressing we cannot just
> +	 * do a big bulk read. Instead, we have to read each Hx
> +	 * value separately and sometimes do some bit shifting...
> +	 * Humidity data is only available on BME280.
> +	 */
> +	if (chip != BME280_CHIP_ID)
> +		return 0;
> +
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H1 comp value\n");
>  		return ret;
>  	}
> +	calib->H1 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H2 comp value\n");
>  		return ret;
>  	}
> -	H2 = sign_extend32(le16_to_cpu(tmp), 15);
> +	calib->H2 = sign_extend32(le16_to_cpu(tmp), 15);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &H3);
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H3 comp value\n");
>  		return ret;
>  	}
> +	calib->H3 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H4 comp value\n");
>  		return ret;
>  	}
> -	H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> -			  (be16_to_cpu(tmp) & 0xf), 11);
> +	calib->H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> +				  (be16_to_cpu(tmp) & 0xf), 11);
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H5 comp value\n");
>  		return ret;
>  	}
> -	H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
> +	calib->H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
>  
>  	ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H6 comp value\n");
>  		return ret;
>  	}
> -	H6 = sign_extend32(tmp, 7);
> +	calib->H6 = sign_extend32(tmp, 7);
> +
> +	/* Toss the calibration data into the entropy pool */
> +	add_device_randomness(calib, sizeof(struct bmp280_calib));
> +
> +	return 0;
> +}
> +/*
> + * Returns humidity in percent, resolution is 0.01 percent. Output value of
> + * "47445" represents 47445/1024 = 46.333 %RH.
> + *
> + * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> + */
> +static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> +				      s32 adc_humidity)
> +{
> +	s32 var;
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var = ((s32)data->t_fine) - (s32)76800;
> -	var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var))
> -		+ (s32)16384) >> 15) * (((((((var * H6) >> 10)
> -		* (((var * (s32)H3) >> 11) + (s32)32768)) >> 10)
> -		+ (s32)2097152) * H2 + 8192) >> 14);
> -	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)H1) >> 4;
> +	var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
> +		+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
> +		* (((var * (s32)calib->H3) >> 11) + (s32)32768)) >> 10)
> +		+ (s32)2097152) * calib->H2 + 8192) >> 14);
> +	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)calib->H1) >> 4;
>  
>  	return var >> 12;
>  };
> @@ -195,31 +276,14 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
>  static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  				  s32 adc_temp)
>  {
> -	int ret;
>  	s32 var1, var2;
> -	__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> -			       buf, BMP280_COMP_TEMP_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read temperature calibration parameters\n");
> -		return ret;
> -	}
> -
> -	/*
> -	 * The double casts are necessary because le16_to_cpu returns an
> -	 * unsigned 16-bit value.  Casting that value directly to a
> -	 * signed 32-bit will not do proper sign extension.
> -	 *
> -	 * Conversely, T1 and P1 are unsigned values, so they can be
> -	 * cast straight to the larger type.
> -	 */
> -	var1 = (((adc_temp >> 3) - ((s32)le16_to_cpu(buf[T1]) << 1)) *
> -		((s32)(s16)le16_to_cpu(buf[T2]))) >> 11;
> -	var2 = (((((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1]))) *
> -		  ((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1])))) >> 12) *
> -		((s32)(s16)le16_to_cpu(buf[T3]))) >> 14;
> +	var1 = (((adc_temp >> 3) - ((s32)calib->T1 << 1)) *
> +		((s32)calib->T2)) >> 11;
> +	var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
> +		  ((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
> +		((s32)calib->T3)) >> 14;
>  	data->t_fine = var1 + var2;
>  
>  	return (data->t_fine * 5 + 128) >> 8;
> @@ -235,34 +299,25 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  static u32 bmp280_compensate_press(struct bmp280_data *data,
>  				   s32 adc_press)
>  {
> -	int ret;
>  	s64 var1, var2, p;
> -	__le16 buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> -
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> -			       buf, BMP280_COMP_PRESS_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read pressure calibration parameters\n");
> -		return ret;
> -	}
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var1 = ((s64)data->t_fine) - 128000;
> -	var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
> -	var2 += (var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17;
> -	var2 += ((s64)(s16)le16_to_cpu(buf[P4])) << 35;
> -	var1 = ((var1 * var1 * (s64)(s16)le16_to_cpu(buf[P3])) >> 8) +
> -		((var1 * (s64)(s16)le16_to_cpu(buf[P2])) << 12);
> -	var1 = ((((s64)1) << 47) + var1) * ((s64)le16_to_cpu(buf[P1])) >> 33;
> +	var2 = var1 * var1 * (s64)calib->P6;
> +	var2 += (var1 * (s64)calib->P5) << 17;
> +	var2 += ((s64)calib->P4) << 35;
> +	var1 = ((var1 * var1 * (s64)calib->P3) >> 8) +
> +		((var1 * (s64)calib->P2) << 12);
> +	var1 = ((((s64)1) << 47) + var1) * ((s64)calib->P1) >> 33;
>  
>  	if (var1 == 0)
>  		return 0;
>  
>  	p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
>  	p = div64_s64(p, var1);
> -	var1 = (((s64)(s16)le16_to_cpu(buf[P9])) * (p >> 13) * (p >> 13)) >> 25;
> -	var2 = (((s64)(s16)le16_to_cpu(buf[P8])) * p) >> 19;
> -	p = ((p + var1 + var2) >> 8) + (((s64)(s16)le16_to_cpu(buf[P7])) << 4);
> +	var1 = (((s64)calib->P9) * (p >> 13) * (p >> 13)) >> 25;
> +	var2 = ((s64)(calib->P8) * p) >> 19;
> +	p = ((p + var1 + var2) >> 8) + (((s64)calib->P7) << 4);
>  
>  	return (u32)p;
>  }
> @@ -752,7 +807,7 @@ static int bmp180_read_calib(struct bmp280_data *data,
>  static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
>  {
>  	s32 x1, x2;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
>  	x2 = (calib->MC << 11) / (x1 + calib->MD);
> @@ -814,7 +869,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
>  	s32 b3, b6;
>  	u32 b4, b7;
>  	s32 oss = data->oversampling_press;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	b6 = data->t_fine - 4000;
>  	x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
> @@ -1028,11 +1083,19 @@ int bmp280_common_probe(struct device *dev,
>  	dev_set_drvdata(dev, indio_dev);
>  
>  	/*
> -	 * The BMP085 and BMP180 has calibration in an E2PROM, read it out
> -	 * at probe time. It will not change.
> +	 * Some chips have calibration parameters "programmed into the devices'
> +	 * non-volatile memory during production". Let's read them out at probe
> +	 * time once. They will not change.
>  	 */
>  	if (chip_id  == BMP180_CHIP_ID) {
> -		ret = bmp180_read_calib(data, &data->calib);
> +		ret = bmp180_read_calib(data, &data->calib.bmp180);
> +		if (ret < 0) {
> +			dev_err(data->dev,
> +				"failed to read calibration coefficients\n");
> +			goto out_disable_vdda;
> +		}
> +	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> +		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Jonathan Cameron]

On Tue, 12 Dec 2017 21:35:37 +0100
Stefan Tatschner <stefan.tatschner@gmail.com> wrote:

> This patch affects BME280 and BMP280. The readout of the calibration
> data is moved to the probe function. Each sensor data access triggered
> reading the full calibration data before this patch. According to the
> datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> memory.
> 
> Since the calibration data does not change, and cannot be changed by the
> user, we can reduce bus traffic by reading the calibration data once.
> Additionally, proper organization of the data types enables removing
> some odd casts in the compensation formulas.
> 
> Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>
One minor inline. Will fix whilst applying as it's just comment syntax.

This was headache inducing to check.  Glad you got to write it ;)

I've also dropped the entropy add that was in here. It should be proposed
as a separate patch rather than in here. Particularly as it isn't mentioned
in the commit message.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> v2 changes:
>     - readded the type casts in temperature calibration values
>     - fix typo
> 
>  drivers/iio/pressure/bmp280-core.c | 205 ++++++++++++++++++++++++-------------
>  1 file changed, 134 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index fd1da26a62e4..a326d68dd668 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -55,6 +55,28 @@ struct bmp180_calib {
>  	s16 MD;
>  };
>  
> +/* See datasheet Section 4.2.2. */
> +struct bmp280_calib {
> +	u16 T1;
> +	s16 T2;
> +	s16 T3;
> +	u16 P1;
> +	s16 P2;
> +	s16 P3;
> +	s16 P4;
> +	s16 P5;
> +	s16 P6;
> +	s16 P7;
> +	s16 P8;
> +	s16 P9;
> +	u8  H1;
> +	s16 H2;
> +	u8  H3;
> +	s16 H4;
> +	s16 H5;
> +	s8  H6;
> +};
> +
>  struct bmp280_data {
>  	struct device *dev;
>  	struct mutex lock;
> @@ -62,7 +84,10 @@ struct bmp280_data {
>  	struct completion done;
>  	bool use_eoc;
>  	const struct bmp280_chip_info *chip_info;
> -	struct bmp180_calib calib;
> +	union {
> +		struct bmp180_calib bmp180;
> +		struct bmp280_calib bmp280;
> +	} calib;
>  	struct regulator *vddd;
>  	struct regulator *vdda;
>  	unsigned int start_up_time; /* in microseconds */
> @@ -120,67 +145,123 @@ static const struct iio_chan_spec bmp280_channels[] = {
>  	},
>  };
>  
> -/*
> - * Returns humidity in percent, resolution is 0.01 percent. Output value of
> - * "47445" represents 47445/1024 = 46.333 %RH.
> - *
> - * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> - */
> -
> -static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> -				      s32 adc_humidity)
> +static int bmp280_read_calib(struct bmp280_data *data,
> +			     struct bmp280_calib *calib,
> +			     unsigned int chip)
>  {
> +	int ret;
> +	unsigned int tmp;
>  	struct device *dev = data->dev;
> -	unsigned int H1, H3, tmp;
> -	int H2, H4, H5, H6, ret, var;
> +	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> +
> +	/* Read temperature calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> +			       t_buf, BMP280_COMP_TEMP_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read temperature calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->T1 = le16_to_cpu(t_buf[T1]);
> +	calib->T2 = le16_to_cpu(t_buf[T2]);
> +	calib->T3 = le16_to_cpu(t_buf[T3]);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &H1);
> +	/* Read pressure calibration values. */
> +	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> +			       p_buf, BMP280_COMP_PRESS_REG_COUNT);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to read pressure calibration parameters\n");
> +		return ret;
> +	}
> +
> +	calib->P1 = le16_to_cpu(p_buf[P1]);
> +	calib->P2 = le16_to_cpu(p_buf[P2]);
> +	calib->P3 = le16_to_cpu(p_buf[P3]);
> +	calib->P4 = le16_to_cpu(p_buf[P4]);
> +	calib->P5 = le16_to_cpu(p_buf[P5]);
> +	calib->P6 = le16_to_cpu(p_buf[P6]);
> +	calib->P7 = le16_to_cpu(p_buf[P7]);
> +	calib->P8 = le16_to_cpu(p_buf[P8]);
> +	calib->P9 = le16_to_cpu(p_buf[P9]);
> +
> +	/* Read humidity calibration values.
/*
 * Read humidity

If this is all I find I'll fix it whilst applying.

> +	 * Due to some odd register addressing we cannot just
> +	 * do a big bulk read. Instead, we have to read each Hx
> +	 * value separately and sometimes do some bit shifting...
> +	 * Humidity data is only available on BME280.
> +	 */
> +	if (chip != BME280_CHIP_ID)
> +		return 0;
> +
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H1, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H1 comp value\n");
>  		return ret;
>  	}
> +	calib->H1 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H2, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H2 comp value\n");
>  		return ret;
>  	}
> -	H2 = sign_extend32(le16_to_cpu(tmp), 15);
> +	calib->H2 = sign_extend32(le16_to_cpu(tmp), 15);
>  
> -	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &H3);
> +	ret = regmap_read(data->regmap, BMP280_REG_COMP_H3, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H3 comp value\n");
>  		return ret;
>  	}
> +	calib->H3 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H4, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H4 comp value\n");
>  		return ret;
>  	}
> -	H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> -			  (be16_to_cpu(tmp) & 0xf), 11);
> +	calib->H4 = sign_extend32(((be16_to_cpu(tmp) >> 4) & 0xff0) |
> +				  (be16_to_cpu(tmp) & 0xf), 11);
>  
>  	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_H5, &tmp, 2);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H5 comp value\n");
>  		return ret;
>  	}
> -	H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
> +	calib->H5 = sign_extend32(((le16_to_cpu(tmp) >> 4) & 0xfff), 11);
>  
>  	ret = regmap_read(data->regmap, BMP280_REG_COMP_H6, &tmp);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read H6 comp value\n");
>  		return ret;
>  	}
> -	H6 = sign_extend32(tmp, 7);
> +	calib->H6 = sign_extend32(tmp, 7);
> +
> +	/* Toss the calibration data into the entropy pool */
> +	add_device_randomness(calib, sizeof(struct bmp280_calib));
> +
> +	return 0;
> +}
> +/*
> + * Returns humidity in percent, resolution is 0.01 percent. Output value of
> + * "47445" represents 47445/1024 = 46.333 %RH.
> + *
> + * Taken from BME280 datasheet, Section 4.2.3, "Compensation formula".
> + */
> +static u32 bmp280_compensate_humidity(struct bmp280_data *data,
> +				      s32 adc_humidity)
> +{
> +	s32 var;
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var = ((s32)data->t_fine) - (s32)76800;
> -	var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var))
> -		+ (s32)16384) >> 15) * (((((((var * H6) >> 10)
> -		* (((var * (s32)H3) >> 11) + (s32)32768)) >> 10)
> -		+ (s32)2097152) * H2 + 8192) >> 14);
> -	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)H1) >> 4;
> +	var = ((((adc_humidity << 14) - (calib->H4 << 20) - (calib->H5 * var))
> +		+ (s32)16384) >> 15) * (((((((var * calib->H6) >> 10)
> +		* (((var * (s32)calib->H3) >> 11) + (s32)32768)) >> 10)
> +		+ (s32)2097152) * calib->H2 + 8192) >> 14);
> +	var -= ((((var >> 15) * (var >> 15)) >> 7) * (s32)calib->H1) >> 4;
>  
>  	return var >> 12;
>  };
> @@ -195,31 +276,14 @@ static u32 bmp280_compensate_humidity(struct bmp280_data *data,
>  static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  				  s32 adc_temp)
>  {
> -	int ret;
>  	s32 var1, var2;
> -	__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> -			       buf, BMP280_COMP_TEMP_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read temperature calibration parameters\n");
> -		return ret;
> -	}
> -
> -	/*
> -	 * The double casts are necessary because le16_to_cpu returns an
> -	 * unsigned 16-bit value.  Casting that value directly to a
> -	 * signed 32-bit will not do proper sign extension.
> -	 *
> -	 * Conversely, T1 and P1 are unsigned values, so they can be
> -	 * cast straight to the larger type.
> -	 */
> -	var1 = (((adc_temp >> 3) - ((s32)le16_to_cpu(buf[T1]) << 1)) *
> -		((s32)(s16)le16_to_cpu(buf[T2]))) >> 11;
> -	var2 = (((((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1]))) *
> -		  ((adc_temp >> 4) - ((s32)le16_to_cpu(buf[T1])))) >> 12) *
> -		((s32)(s16)le16_to_cpu(buf[T3]))) >> 14;
> +	var1 = (((adc_temp >> 3) - ((s32)calib->T1 << 1)) *
> +		((s32)calib->T2)) >> 11;
> +	var2 = (((((adc_temp >> 4) - ((s32)calib->T1)) *
> +		  ((adc_temp >> 4) - ((s32)calib->T1))) >> 12) *
> +		((s32)calib->T3)) >> 14;
>  	data->t_fine = var1 + var2;
>  
>  	return (data->t_fine * 5 + 128) >> 8;
> @@ -235,34 +299,25 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>  static u32 bmp280_compensate_press(struct bmp280_data *data,
>  				   s32 adc_press)
>  {
> -	int ret;
>  	s64 var1, var2, p;
> -	__le16 buf[BMP280_COMP_PRESS_REG_COUNT / 2];
> -
> -	ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_PRESS_START,
> -			       buf, BMP280_COMP_PRESS_REG_COUNT);
> -	if (ret < 0) {
> -		dev_err(data->dev,
> -			"failed to read pressure calibration parameters\n");
> -		return ret;
> -	}
> +	struct bmp280_calib *calib = &data->calib.bmp280;
>  
>  	var1 = ((s64)data->t_fine) - 128000;
> -	var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
> -	var2 += (var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17;
> -	var2 += ((s64)(s16)le16_to_cpu(buf[P4])) << 35;
> -	var1 = ((var1 * var1 * (s64)(s16)le16_to_cpu(buf[P3])) >> 8) +
> -		((var1 * (s64)(s16)le16_to_cpu(buf[P2])) << 12);
> -	var1 = ((((s64)1) << 47) + var1) * ((s64)le16_to_cpu(buf[P1])) >> 33;
> +	var2 = var1 * var1 * (s64)calib->P6;
> +	var2 += (var1 * (s64)calib->P5) << 17;
> +	var2 += ((s64)calib->P4) << 35;
> +	var1 = ((var1 * var1 * (s64)calib->P3) >> 8) +
> +		((var1 * (s64)calib->P2) << 12);
> +	var1 = ((((s64)1) << 47) + var1) * ((s64)calib->P1) >> 33;
>  
>  	if (var1 == 0)
>  		return 0;
>  
>  	p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
>  	p = div64_s64(p, var1);
> -	var1 = (((s64)(s16)le16_to_cpu(buf[P9])) * (p >> 13) * (p >> 13)) >> 25;
> -	var2 = (((s64)(s16)le16_to_cpu(buf[P8])) * p) >> 19;
> -	p = ((p + var1 + var2) >> 8) + (((s64)(s16)le16_to_cpu(buf[P7])) << 4);
> +	var1 = (((s64)calib->P9) * (p >> 13) * (p >> 13)) >> 25;
> +	var2 = ((s64)(calib->P8) * p) >> 19;
> +	p = ((p + var1 + var2) >> 8) + (((s64)calib->P7) << 4);
>  
>  	return (u32)p;
>  }
> @@ -752,7 +807,7 @@ static int bmp180_read_calib(struct bmp280_data *data,
>  static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp)
>  {
>  	s32 x1, x2;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	x1 = ((adc_temp - calib->AC6) * calib->AC5) >> 15;
>  	x2 = (calib->MC << 11) / (x1 + calib->MD);
> @@ -814,7 +869,7 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press)
>  	s32 b3, b6;
>  	u32 b4, b7;
>  	s32 oss = data->oversampling_press;
> -	struct bmp180_calib *calib = &data->calib;
> +	struct bmp180_calib *calib = &data->calib.bmp180;
>  
>  	b6 = data->t_fine - 4000;
>  	x1 = (calib->B2 * (b6 * b6 >> 12)) >> 11;
> @@ -1028,11 +1083,19 @@ int bmp280_common_probe(struct device *dev,
>  	dev_set_drvdata(dev, indio_dev);
>  
>  	/*
> -	 * The BMP085 and BMP180 has calibration in an E2PROM, read it out
> -	 * at probe time. It will not change.
> +	 * Some chips have calibration parameters "programmed into the devices'
> +	 * non-volatile memory during production". Let's read them out at probe
> +	 * time once. They will not change.
>  	 */
>  	if (chip_id  == BMP180_CHIP_ID) {
> -		ret = bmp180_read_calib(data, &data->calib);
> +		ret = bmp180_read_calib(data, &data->calib.bmp180);
> +		if (ret < 0) {
> +			dev_err(data->dev,
> +				"failed to read calibration coefficients\n");
> +			goto out_disable_vdda;
> +		}
> +	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
> +		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Stefan Tatschner]

On Fri, Dec 29, 2017 at 06:48:55PM +0000, Jonathan Cameron wrote:
> On Tue, 12 Dec 2017 21:35:37 +0100
> Stefan Tatschner <stefan.tatschner@gmail.com> wrote:
> 
> > This patch affects BME280 and BMP280. The readout of the calibration
> > data is moved to the probe function. Each sensor data access triggered
> > reading the full calibration data before this patch. According to the
> > datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> > memory.
> > 
> > Since the calibration data does not change, and cannot be changed by the
> > user, we can reduce bus traffic by reading the calibration data once.
> > Additionally, proper organization of the data types enables removing
> > some odd casts in the compensation formulas.
> > 
> > Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>
> One minor inline. Will fix whilst applying as it's just comment syntax.

Sorry for that. I fired off checkpatch.pl and it didn't complain. I
remember that, for network code or something, it complains about comment
style...

> This was headache inducing to check.  Glad you got to write it ;)
> 
> I've also dropped the entropy add that was in here. It should be proposed
> as a separate patch rather than in here. Particularly as it isn't mentioned
> in the commit message.

If that makes sense for you, I'll send a further patch.

> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.

Thanks!

Stefan

Re: [PATCH v2] iio/bmp280-core.c: Read calibration data in probe

[Jonathan Cameron]

On Wed, 3 Jan 2018 21:13:45 +0100
Stefan Tatschner <stefan.tatschner@gmail.com> wrote:

> On Fri, Dec 29, 2017 at 06:48:55PM +0000, Jonathan Cameron wrote:
> > On Tue, 12 Dec 2017 21:35:37 +0100
> > Stefan Tatschner <stefan.tatschner@gmail.com> wrote:
> >   
> > > This patch affects BME280 and BMP280. The readout of the calibration
> > > data is moved to the probe function. Each sensor data access triggered
> > > reading the full calibration data before this patch. According to the
> > > datasheet, Section 4.4.2., the calibration data is stored in non-volatile
> > > memory.
> > > 
> > > Since the calibration data does not change, and cannot be changed by the
> > > user, we can reduce bus traffic by reading the calibration data once.
> > > Additionally, proper organization of the data types enables removing
> > > some odd casts in the compensation formulas.
> > > 
> > > Signed-off-by: Stefan Tatschner <stefan.tatschner@gmail.com>  
> > One minor inline. Will fix whilst applying as it's just comment syntax.  
> 
> Sorry for that. I fired off checkpatch.pl and it didn't complain. I
> remember that, for network code or something, it complains about comment
> style...
> 
> > This was headache inducing to check.  Glad you got to write it ;)
> > 
> > I've also dropped the entropy add that was in here. It should be proposed
> > as a separate patch rather than in here. Particularly as it isn't mentioned
> > in the commit message.  
> 
> If that makes sense for you, I'll send a further patch.

I think it makes reasonable sense so send a patch adding that on it's own
and we'll consider it then.

> 
> > Applied to the togreg branch of iio.git and pushed out as testing for
> > the autobuilders to play with it.  
> 
> Thanks!
> 
> Stefan