From: Jonathan Cameron <jic23@kernel.org>
To: Hartmut Knaack <knaack.h@gmx.de>,
Vlad Dogaru <vlad.dogaru@intel.com>,
linux-iio@vger.kernel.org
Subject: Re: [PATCH] iio: bmp280: refactor compensation code
Date: Sat, 22 Nov 2014 20:43:02 +0000 [thread overview]
Message-ID: <5470F556.3030405@kernel.org> (raw)
In-Reply-To: <5470F1AA.5040802@gmx.de>
On 22/11/14 20:27, Hartmut Knaack wrote:
> Vlad Dogaru schrieb am 20.11.2014 13:00:
>> This version of the code avoids extra memory copy operations and is
>> somewhat smaller in code size.
>>
>> Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
> Acked-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git.
>> ---
>> drivers/iio/pressure/bmp280.c | 140 ++++++++++++++++--------------------------
>> 1 file changed, 52 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/bmp280.c b/drivers/iio/pressure/bmp280.c
>> index 75038da..47dfd34 100644
>> --- a/drivers/iio/pressure/bmp280.c
>> +++ b/drivers/iio/pressure/bmp280.c
>> @@ -80,16 +80,12 @@ struct bmp280_data {
>> s32 t_fine;
>> };
>>
>> -/* Compensation parameters. */
>> -struct bmp280_comp_temp {
>> - u16 dig_t1;
>> - s16 dig_t2, dig_t3;
>> -};
>> -
>> -struct bmp280_comp_press {
>> - u16 dig_p1;
>> - s16 dig_p2, dig_p3, dig_p4, dig_p5, dig_p6, dig_p7, dig_p8, dig_p9;
>> -};
>> +/*
>> + * These enums are used for indexing into the array of compensation
>> + * parameters.
>> + */
>> +enum { T1, T2, T3 };
>> +enum { P1, P2, P3, P4, P5, P6, P7, P8, P9 };
>>
>> static const struct iio_chan_spec bmp280_channels[] = {
>> {
>> @@ -141,54 +137,6 @@ static const struct regmap_config bmp280_regmap_config = {
>> .volatile_reg = bmp280_is_volatile_reg,
>> };
>>
>> -static int bmp280_read_compensation_temp(struct bmp280_data *data,
>> - struct bmp280_comp_temp *comp)
>> -{
>> - int ret;
>> - __le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
>> -
>> - ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
>> - buf, BMP280_COMP_TEMP_REG_COUNT);
>> - if (ret < 0) {
>> - dev_err(&data->client->dev,
>> - "failed to read temperature calibration parameters\n");
>> - return ret;
>> - }
>> -
>> - comp->dig_t1 = (u16) le16_to_cpu(buf[0]);
>> - comp->dig_t2 = (s16) le16_to_cpu(buf[1]);
>> - comp->dig_t3 = (s16) le16_to_cpu(buf[2]);
>> -
>> - return 0;
>> -}
>> -
>> -static int bmp280_read_compensation_press(struct bmp280_data *data,
>> - struct bmp280_comp_press *comp)
>> -{
>> - int ret;
>> - __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->client->dev,
>> - "failed to read pressure calibration parameters\n");
>> - return ret;
>> - }
>> -
>> - comp->dig_p1 = (u16) le16_to_cpu(buf[0]);
>> - comp->dig_p2 = (s16) le16_to_cpu(buf[1]);
>> - comp->dig_p3 = (s16) le16_to_cpu(buf[2]);
>> - comp->dig_p4 = (s16) le16_to_cpu(buf[3]);
>> - comp->dig_p5 = (s16) le16_to_cpu(buf[4]);
>> - comp->dig_p6 = (s16) le16_to_cpu(buf[5]);
>> - comp->dig_p7 = (s16) le16_to_cpu(buf[6]);
>> - comp->dig_p8 = (s16) le16_to_cpu(buf[7]);
>> - comp->dig_p9 = (s16) le16_to_cpu(buf[8]);
>> -
>> - return 0;
>> -}
>> -
>> /*
>> * Returns temperature in DegC, resolution is 0.01 DegC. Output value of
>> * "5123" equals 51.23 DegC. t_fine carries fine temperature as global
>> @@ -197,16 +145,33 @@ static int bmp280_read_compensation_press(struct bmp280_data *data,
>> * Taken from datasheet, Section 3.11.3, "Compensation formula".
>> */
>> static s32 bmp280_compensate_temp(struct bmp280_data *data,
>> - struct bmp280_comp_temp *comp,
>> s32 adc_temp)
>> {
>> + int ret;
>> s32 var1, var2, t;
>> + __le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
>> +
>> + ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
>> + buf, BMP280_COMP_TEMP_REG_COUNT);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev,
>> + "failed to read temperature calibration parameters\n");
>> + return ret;
>> + }
>>
>> - var1 = (((adc_temp >> 3) - ((s32) comp->dig_t1 << 1)) *
>> - ((s32) comp->dig_t2)) >> 11;
>> - var2 = (((((adc_temp >> 4) - ((s32) comp->dig_t1)) *
>> - ((adc_temp >> 4) - ((s32) comp->dig_t1))) >> 12) *
>> - ((s32) comp->dig_t3)) >> 14;
>> + /*
>> + * 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;
>>
>> data->t_fine = var1 + var2;
>> t = (data->t_fine * 5 + 128) >> 8;
>> @@ -222,27 +187,36 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>> * Taken from datasheet, Section 3.11.3, "Compensation formula".
>> */
>> static u32 bmp280_compensate_press(struct bmp280_data *data,
>> - struct bmp280_comp_press *comp,
>> 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->client->dev,
>> + "failed to read pressure calibration parameters\n");
>> + return ret;
>> + }
>>
>> - var1 = ((s64) data->t_fine) - 128000;
>> - var2 = var1 * var1 * (s64) comp->dig_p6;
>> - var2 = var2 + ((var1 * (s64) comp->dig_p5) << 17);
>> - var2 = var2 + (((s64) comp->dig_p4) << 35);
>> - var1 = ((var1 * var1 * (s64) comp->dig_p3) >> 8) +
>> - ((var1 * (s64) comp->dig_p2) << 12);
>> - var1 = (((((s64) 1) << 47) + var1)) * ((s64) comp->dig_p1) >> 33;
>> + var1 = ((s64)data->t_fine) - 128000;
>> + var2 = var1 * var1 * (s64)(s16)le16_to_cpu(buf[P6]);
>> + var2 = var2 + ((var1 * (s64)(s16)le16_to_cpu(buf[P5])) << 17);
>> + var2 = 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;
>>
>> if (var1 == 0)
>> return 0;
>>
>> - p = ((((s64) 1048576 - adc_press) << 31) - var2) * 3125;
>> + p = ((((s64)1048576 - adc_press) << 31) - var2) * 3125;
>> p = div64_s64(p, var1);
>> - var1 = (((s64) comp->dig_p9) * (p >> 13) * (p >> 13)) >> 25;
>> - var2 = (((s64) comp->dig_p8) * p) >> 19;
>> - p = ((p + var1 + var2) >> 8) + (((s64) comp->dig_p7) << 4);
>> + 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);
>>
>> return (u32) p;
>> }
>> @@ -253,11 +227,6 @@ static int bmp280_read_temp(struct bmp280_data *data,
>> int ret;
>> __be32 tmp = 0;
>> s32 adc_temp, comp_temp;
>> - struct bmp280_comp_temp comp;
>> -
>> - ret = bmp280_read_compensation_temp(data, &comp);
>> - if (ret < 0)
>> - return ret;
>>
>> ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB,
>> (u8 *) &tmp, 3);
>> @@ -267,7 +236,7 @@ static int bmp280_read_temp(struct bmp280_data *data,
>> }
>>
>> adc_temp = be32_to_cpu(tmp) >> 12;
>> - comp_temp = bmp280_compensate_temp(data, &comp, adc_temp);
>> + comp_temp = bmp280_compensate_temp(data, adc_temp);
>>
>> /*
>> * val might be NULL if we're called by the read_press routine,
>> @@ -288,11 +257,6 @@ static int bmp280_read_press(struct bmp280_data *data,
>> __be32 tmp = 0;
>> s32 adc_press;
>> u32 comp_press;
>> - struct bmp280_comp_press comp;
>> -
>> - ret = bmp280_read_compensation_press(data, &comp);
>> - if (ret < 0)
>> - return ret;
>>
>> /* Read and compensate temperature so we get a reading of t_fine. */
>> ret = bmp280_read_temp(data, NULL);
>> @@ -307,7 +271,7 @@ static int bmp280_read_press(struct bmp280_data *data,
>> }
>>
>> adc_press = be32_to_cpu(tmp) >> 12;
>> - comp_press = bmp280_compensate_press(data, &comp, adc_press);
>> + comp_press = bmp280_compensate_press(data, adc_press);
>>
>> *val = comp_press;
>> *val2 = 256000;
>>
>
prev parent reply other threads:[~2014-11-22 20:43 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-31 1:25 [PATCH 3/3]iio:pressure:bmp280: read compensation data only at init Hartmut Knaack
2014-10-31 11:47 ` Vlad Dogaru
2014-10-31 19:16 ` Hartmut Knaack
2014-11-05 15:53 ` Jonathan Cameron
2014-11-06 13:02 ` Vlad Dogaru
2014-11-10 23:11 ` Hartmut Knaack
2014-11-15 16:06 ` Jonathan Cameron
2014-11-17 15:16 ` Vlad Dogaru
2014-11-20 12:00 ` [PATCH] iio: bmp280: refactor compensation code Vlad Dogaru
2014-11-22 12:05 ` Jonathan Cameron
2014-11-22 20:33 ` Hartmut Knaack
2014-11-22 20:27 ` Hartmut Knaack
2014-11-22 20:43 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5470F556.3030405@kernel.org \
--to=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=linux-iio@vger.kernel.org \
--cc=vlad.dogaru@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).