linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Vlad Dogaru <vlad.dogaru@intel.com>, Hartmut Knaack <knaack.h@gmx.de>
Cc: IIO <linux-iio@vger.kernel.org>
Subject: Re: [PATCH V2]iio:pressure:bmp280: cleanup
Date: Fri, 26 Dec 2014 10:33:33 +0000	[thread overview]
Message-ID: <549D397D.6090305@kernel.org> (raw)
In-Reply-To: <20141222122925.GA16917@vdogaru>

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
> 


      reply	other threads:[~2014-12-26 10:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 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=549D397D.6090305@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).