| Message ID | 20171212203537.8204-1-stefan.tatschner@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
--
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
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(-) -- 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
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
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"); -- 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
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 -- 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
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 -- 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
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");
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(-)