* [PATCH V2]iio:pressure:bmp280: cleanup
@ 2014-12-19 22:59 Hartmut Knaack
2014-12-22 12:29 ` Vlad Dogaru
0 siblings, 1 reply; 3+ messages in thread
From: Hartmut Knaack @ 2014-12-19 22:59 UTC (permalink / raw)
To: IIO; +Cc: Vlad Dogaru
The calculations for temperature and pressure compensation were already slightly
optimized in comparison to the datasheet. So, it makes sense to optimize even a
bit more, making proper use of C operators:
- variable t in bmp280_compensate_temp() can be eliminated by directly
returning the result of the relevant equation.
- make use of the += operator and eliminate an unnecessary parenthesis level in
bmp280_compensate_press().
When the initialization of the ctrl_meas register fails, the error message will
now mention the right register name.
During probe, i2c_set_clientdata() is called, although it is not necessary. Drop
it.
Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
Changes in V2:
Rebase after refactoring of compensation code.
diff --git a/drivers/iio/pressure/bmp280.c b/drivers/iio/pressure/bmp280.c
index 47dfd34..7c623e2 100644
--- a/drivers/iio/pressure/bmp280.c
+++ b/drivers/iio/pressure/bmp280.c
@@ -148,7 +148,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
s32 adc_temp)
{
int ret;
- s32 var1, var2, t;
+ s32 var1, var2;
__le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
@@ -173,10 +173,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
((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;
-
- return t;
+ return (data->t_fine * 5 + 128) >> 8;
}
/*
@@ -203,8 +200,8 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
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);
+ 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;
@@ -218,7 +215,7 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
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;
+ return (u32)p;
}
static int bmp280_read_temp(struct bmp280_data *data,
@@ -330,7 +327,7 @@ static int bmp280_chip_init(struct bmp280_data *data)
BMP280_MODE_NORMAL);
if (ret < 0) {
dev_err(&data->client->dev,
- "failed to write config register\n");
+ "failed to write ctrl_meas register\n");
return ret;
}
@@ -358,7 +355,6 @@ static int bmp280_probe(struct i2c_client *client,
if (!indio_dev)
return -ENOMEM;
- i2c_set_clientdata(client, indio_dev);
data = iio_priv(indio_dev);
mutex_init(&data->lock);
data->client = client;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V2]iio:pressure:bmp280: cleanup
2014-12-19 22:59 [PATCH V2]iio:pressure:bmp280: cleanup Hartmut Knaack
@ 2014-12-22 12:29 ` Vlad Dogaru
2014-12-26 10:33 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Vlad Dogaru @ 2014-12-22 12:29 UTC (permalink / raw)
To: Hartmut Knaack; +Cc: IIO
On Fri, Dec 19, 2014 at 11:59:25PM +0100, Hartmut Knaack wrote:
> The calculations for temperature and pressure compensation were already slightly
> optimized in comparison to the datasheet. So, it makes sense to optimize even a
> bit more, making proper use of C operators:
> - variable t in bmp280_compensate_temp() can be eliminated by directly
> returning the result of the relevant equation.
> - make use of the += operator and eliminate an unnecessary parenthesis level in
> bmp280_compensate_press().
> When the initialization of the ctrl_meas register fails, the error message will
> now mention the right register name.
> During probe, i2c_set_clientdata() is called, although it is not necessary. Drop
> it.
>
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Reviewed-by: Vlad Dogaru <vlad.dogaru@intel.com>
Thanks for following up with this one, Hartmut.
> ---
> Changes in V2:
> Rebase after refactoring of compensation code.
>
> diff --git a/drivers/iio/pressure/bmp280.c b/drivers/iio/pressure/bmp280.c
> index 47dfd34..7c623e2 100644
> --- a/drivers/iio/pressure/bmp280.c
> +++ b/drivers/iio/pressure/bmp280.c
> @@ -148,7 +148,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> s32 adc_temp)
> {
> int ret;
> - s32 var1, var2, t;
> + s32 var1, var2;
> __le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
>
> ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
> @@ -173,10 +173,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
> ((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;
> -
> - return t;
> + return (data->t_fine * 5 + 128) >> 8;
> }
>
> /*
> @@ -203,8 +200,8 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
>
> 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);
> + 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;
> @@ -218,7 +215,7 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
> 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;
> + return (u32)p;
> }
>
> static int bmp280_read_temp(struct bmp280_data *data,
> @@ -330,7 +327,7 @@ static int bmp280_chip_init(struct bmp280_data *data)
> BMP280_MODE_NORMAL);
> if (ret < 0) {
> dev_err(&data->client->dev,
> - "failed to write config register\n");
> + "failed to write ctrl_meas register\n");
> return ret;
> }
>
> @@ -358,7 +355,6 @@ static int bmp280_probe(struct i2c_client *client,
> if (!indio_dev)
> return -ENOMEM;
>
> - i2c_set_clientdata(client, indio_dev);
> data = iio_priv(indio_dev);
> mutex_init(&data->lock);
> data->client = client;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V2]iio:pressure:bmp280: cleanup
2014-12-22 12:29 ` Vlad Dogaru
@ 2014-12-26 10:33 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2014-12-26 10:33 UTC (permalink / raw)
To: Vlad Dogaru, Hartmut Knaack; +Cc: IIO
On 22/12/14 12:29, Vlad Dogaru wrote:
> On Fri, Dec 19, 2014 at 11:59:25PM +0100, Hartmut Knaack wrote:
>> The calculations for temperature and pressure compensation were already slightly
>> optimized in comparison to the datasheet. So, it makes sense to optimize even a
>> bit more, making proper use of C operators:
>> - variable t in bmp280_compensate_temp() can be eliminated by directly
>> returning the result of the relevant equation.
>> - make use of the += operator and eliminate an unnecessary parenthesis level in
>> bmp280_compensate_press().
>> When the initialization of the ctrl_meas register fails, the error message will
>> now mention the right register name.
>> During probe, i2c_set_clientdata() is called, although it is not necessary. Drop
>> it.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Reviewed-by: Vlad Dogaru <vlad.dogaru@intel.com>
>
> Thanks for following up with this one, Hartmut.
Applied to the togreg branch of iio.git -- initially pushed out as testing
for the autobuilders to play with it. Well, it will be once I've caught up
with some more reviews..
Jonathan
>
>> ---
>> Changes in V2:
>> Rebase after refactoring of compensation code.
>>
>> diff --git a/drivers/iio/pressure/bmp280.c b/drivers/iio/pressure/bmp280.c
>> index 47dfd34..7c623e2 100644
>> --- a/drivers/iio/pressure/bmp280.c
>> +++ b/drivers/iio/pressure/bmp280.c
>> @@ -148,7 +148,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>> s32 adc_temp)
>> {
>> int ret;
>> - s32 var1, var2, t;
>> + s32 var1, var2;
>> __le16 buf[BMP280_COMP_TEMP_REG_COUNT / 2];
>>
>> ret = regmap_bulk_read(data->regmap, BMP280_REG_COMP_TEMP_START,
>> @@ -173,10 +173,7 @@ static s32 bmp280_compensate_temp(struct bmp280_data *data,
>> ((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;
>> -
>> - return t;
>> + return (data->t_fine * 5 + 128) >> 8;
>> }
>>
>> /*
>> @@ -203,8 +200,8 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
>>
>> 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);
>> + 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;
>> @@ -218,7 +215,7 @@ static u32 bmp280_compensate_press(struct bmp280_data *data,
>> 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;
>> + return (u32)p;
>> }
>>
>> static int bmp280_read_temp(struct bmp280_data *data,
>> @@ -330,7 +327,7 @@ static int bmp280_chip_init(struct bmp280_data *data)
>> BMP280_MODE_NORMAL);
>> if (ret < 0) {
>> dev_err(&data->client->dev,
>> - "failed to write config register\n");
>> + "failed to write ctrl_meas register\n");
>> return ret;
>> }
>>
>> @@ -358,7 +355,6 @@ static int bmp280_probe(struct i2c_client *client,
>> if (!indio_dev)
>> return -ENOMEM;
>>
>> - i2c_set_clientdata(client, indio_dev);
>> data = iio_priv(indio_dev);
>> mutex_init(&data->lock);
>> data->client = client;
> --
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-26 10:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-19 22:59 [PATCH V2]iio:pressure:bmp280: cleanup Hartmut Knaack
2014-12-22 12:29 ` Vlad Dogaru
2014-12-26 10:33 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).