[PATCH] IIO: BME280: Humidity: Changes to the humidity ctrl_hum require a write to ctrl_meas.

[PATCH] IIO: BME280: Humidity: Changes to the humidity ctrl_hum require a write to ctrl_meas.

[Colin Parker]

From: Colin Parker <colin.parker@aclima.io>

This results in inaccurate Humidity readings that tend to not fluxuate.

Update the ordering of the register writes so that ctrl_meas is written
after ctrl_hum. This is taken from section 5.4.3 of the datasheet.

"https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME280_DS001-11.pdf"

Also writes to the config register can be ignored if the device is in Normal Mode.
Change the device to sleep mode before updating the config register.
This has the nice side effect of guaranteeing that the regmap_update_bits
function actually updates the driver and not just verifies the setting in its cache.

Signed-off-by: Colin Parker <colin.parker@aclima.io>
---
 drivers/iio/pressure/bmp280-core.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index d82b788..85aa24c 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -559,10 +559,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
 		  BMP280_OSRS_PRESS_X(data->oversampling_press + 1);
 
 	ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
-				 BMP280_OSRS_TEMP_MASK |
-				 BMP280_OSRS_PRESS_MASK |
-				 BMP280_MODE_MASK,
-				 osrs | BMP280_MODE_NORMAL);
+				 BMP280_MODE_MASK, BMP280_MODE_SLEEP);
 	if (ret < 0) {
 		dev_err(data->dev,
 			"failed to write ctrl_meas register\n");
@@ -578,6 +575,17 @@ static int bmp280_chip_config(struct bmp280_data *data)
 		return ret;
 	}
 
+	ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
+				 BMP280_OSRS_TEMP_MASK |
+				 BMP280_OSRS_PRESS_MASK |
+				 BMP280_MODE_MASK,
+				 osrs | BMP280_MODE_NORMAL);
+	if (ret < 0) {
+		dev_err(data->dev,
+			"failed to write ctrl_meas register\n");
+		return ret;
+	}
+
 	return ret;
 }
 
@@ -597,14 +605,14 @@ static const struct bmp280_chip_info bmp280_chip_info = {
 
 static int bme280_chip_config(struct bmp280_data *data)
 {
-	int ret = bmp280_chip_config(data);
 	u8 osrs = BMP280_OSRS_HUMIDITIY_X(data->oversampling_humid + 1);
+	int ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
+				  BMP280_OSRS_HUMIDITY_MASK, osrs);
 
 	if (ret < 0)
 		return ret;
 
-	return regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
-				  BMP280_OSRS_HUMIDITY_MASK, osrs);
+	return bmp280_chip_config(data);
 }
 
 static const struct bmp280_chip_info bme280_chip_info = {
-- 
2.1.4

Re: [PATCH] IIO: BME280: Humidity: Changes to the humidity ctrl_hum require a write to ctrl_meas.

[Andreas Klinger]

Hi Colin,

Colin Parker <colin.foeparker@aclima.io> schrieb am Thu, 24. Aug 16:54:
> From: Colin Parker <colin.parker@aclima.io>
> 
> This results in inaccurate Humidity readings that tend to not fluxuate.
> 
> Update the ordering of the register writes so that ctrl_meas is written
> after ctrl_hum. This is taken from section 5.4.3 of the datasheet.
> 
> "https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME280_DS001-11.pdf"

this part of the patch is already covered by commit afb2c3851b: "iio: bmp280:
properly initialize device for humidity reading"

Andreas

> 
> Also writes to the config register can be ignored if the device is in Normal Mode.
> Change the device to sleep mode before updating the config register.
> This has the nice side effect of guaranteeing that the regmap_update_bits
> function actually updates the driver and not just verifies the setting in its cache.
> 
> Signed-off-by: Colin Parker <colin.parker@aclima.io>
> ---
>  drivers/iio/pressure/bmp280-core.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index d82b788..85aa24c 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -559,10 +559,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  		  BMP280_OSRS_PRESS_X(data->oversampling_press + 1);
>  
>  	ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> -				 BMP280_OSRS_TEMP_MASK |
> -				 BMP280_OSRS_PRESS_MASK |
> -				 BMP280_MODE_MASK,
> -				 osrs | BMP280_MODE_NORMAL);
> +				 BMP280_MODE_MASK, BMP280_MODE_SLEEP);
>  	if (ret < 0) {
>  		dev_err(data->dev,
>  			"failed to write ctrl_meas register\n");
> @@ -578,6 +575,17 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  		return ret;
>  	}
>  
> +	ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> +				 BMP280_OSRS_TEMP_MASK |
> +				 BMP280_OSRS_PRESS_MASK |
> +				 BMP280_MODE_MASK,
> +				 osrs | BMP280_MODE_NORMAL);
> +	if (ret < 0) {
> +		dev_err(data->dev,
> +			"failed to write ctrl_meas register\n");
> +		return ret;
> +	}
> +
>  	return ret;
>  }
>  
> @@ -597,14 +605,14 @@ static const struct bmp280_chip_info bmp280_chip_info = {
>  
>  static int bme280_chip_config(struct bmp280_data *data)
>  {
> -	int ret = bmp280_chip_config(data);
>  	u8 osrs = BMP280_OSRS_HUMIDITIY_X(data->oversampling_humid + 1);
> +	int ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
> +				  BMP280_OSRS_HUMIDITY_MASK, osrs);
>  
>  	if (ret < 0)
>  		return ret;
>  
> -	return regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
> -				  BMP280_OSRS_HUMIDITY_MASK, osrs);
> +	return bmp280_chip_config(data);
>  }
>  
>  static const struct bmp280_chip_info bme280_chip_info = {
> -- 
> 2.1.4
> 
> --
> 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

-- 

Re: [PATCH] IIO: BME280: Humidity: Changes to the humidity ctrl_hum require a write to ctrl_meas.

[Colin Foe-Parker]

Hi Andreas,

On Thu, Aug 24, 2017 at 10:52 PM, Andreas Klinger <ak@it-klinger.de> wrote:
> Hi Colin,
>
> Colin Parker <colin.foeparker@aclima.io> schrieb am Thu, 24. Aug 16:54:
>> From: Colin Parker <colin.parker@aclima.io>
>>
>> This results in inaccurate Humidity readings that tend to not fluxuate.
>>
>> Update the ordering of the register writes so that ctrl_meas is written
>> after ctrl_hum. This is taken from section 5.4.3 of the datasheet.
>>
>> "https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME280_DS001-11.pdf"
>
> this part of the patch is already covered by commit afb2c3851b: "iio: bmp280:
> properly initialize device for humidity reading"

Thanks for pointing out the duplication of efforts!  I dug through the
fixes-togreg branch and found the changes you are referencing.

I think that there might still be an issue there.  I believe that the
regmap_update_bits function will only write to the ctrl_meas register
if there is a difference in the register value vs what is in the
regmap cache.  So in the situation where someone wanted to update the
humidity sampling configuration, I don't think the write to the
ctrl_meas write would go out on the bus.  And as a consequence, the
changes to the humidity settings wouldn't become effective.

What is your take?  I can refactor my patch to just fix that part if
you see the same bug.

-Colin

>
> Andreas
>
>>
>> Also writes to the config register can be ignored if the device is in Normal Mode.
>> Change the device to sleep mode before updating the config register.
>> This has the nice side effect of guaranteeing that the regmap_update_bits
>> function actually updates the driver and not just verifies the setting in its cache.
>>
>> Signed-off-by: Colin Parker <colin.parker@aclima.io>
>> ---
>>  drivers/iio/pressure/bmp280-core.c | 22 +++++++++++++++-------
>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
>> index d82b788..85aa24c 100644
>> --- a/drivers/iio/pressure/bmp280-core.c
>> +++ b/drivers/iio/pressure/bmp280-core.c
>> @@ -559,10 +559,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
>>                 BMP280_OSRS_PRESS_X(data->oversampling_press + 1);
>>
>>       ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
>> -                              BMP280_OSRS_TEMP_MASK |
>> -                              BMP280_OSRS_PRESS_MASK |
>> -                              BMP280_MODE_MASK,
>> -                              osrs | BMP280_MODE_NORMAL);
>> +                              BMP280_MODE_MASK, BMP280_MODE_SLEEP);
>>       if (ret < 0) {
>>               dev_err(data->dev,
>>                       "failed to write ctrl_meas register\n");
>> @@ -578,6 +575,17 @@ static int bmp280_chip_config(struct bmp280_data *data)
>>               return ret;
>>       }
>>
>> +     ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
>> +                              BMP280_OSRS_TEMP_MASK |
>> +                              BMP280_OSRS_PRESS_MASK |
>> +                              BMP280_MODE_MASK,
>> +                              osrs | BMP280_MODE_NORMAL);
>> +     if (ret < 0) {
>> +             dev_err(data->dev,
>> +                     "failed to write ctrl_meas register\n");
>> +             return ret;
>> +     }
>> +
>>       return ret;
>>  }
>>
>> @@ -597,14 +605,14 @@ static const struct bmp280_chip_info bmp280_chip_info = {
>>
>>  static int bme280_chip_config(struct bmp280_data *data)
>>  {
>> -     int ret = bmp280_chip_config(data);
>>       u8 osrs = BMP280_OSRS_HUMIDITIY_X(data->oversampling_humid + 1);
>> +     int ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
>> +                               BMP280_OSRS_HUMIDITY_MASK, osrs);
>>
>>       if (ret < 0)
>>               return ret;
>>
>> -     return regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
>> -                               BMP280_OSRS_HUMIDITY_MASK, osrs);
>> +     return bmp280_chip_config(data);
>>  }
>>
>>  static const struct bmp280_chip_info bme280_chip_info = {
>> --
>> 2.1.4
>>
>> --
>> 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
>
> --



-- 
Colin Parker


ACLIMA INC

Direct (415) 735-5062 |  Email colin.foeparker@aclima.io

10 Lombard St. Suite 250  |  San Francisco, CA 94111


Hello, we're Aclima.


This email and any attachments may contain private, confidential and
privileged material for the sole use of the intended recipient. If you
are not the intended recipient, please immediately delete this email
and any attachments.

Re: [PATCH] IIO: BME280: Humidity: Changes to the humidity ctrl_hum require a write to ctrl_meas.

[Andreas Klinger]

Hi Colin,

Colin Foe-Parker <colin.foeparker@aclima.io> schrieb am Fri, 25. Aug 08:15:
> Hi Andreas,
> 
> On Thu, Aug 24, 2017 at 10:52 PM, Andreas Klinger <ak@it-klinger.de> wrote:
> > Hi Colin,
> >
> > Colin Parker <colin.foeparker@aclima.io> schrieb am Thu, 24. Aug 16:54:
> >> From: Colin Parker <colin.parker@aclima.io>
> >>
> >> This results in inaccurate Humidity readings that tend to not fluxuate.
> >>
> >> Update the ordering of the register writes so that ctrl_meas is written
> >> after ctrl_hum. This is taken from section 5.4.3 of the datasheet.
> >>
> >> "https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME280_DS001-11.pdf"
> >
> > this part of the patch is already covered by commit afb2c3851b: "iio: bmp280:
> > properly initialize device for humidity reading"
> 
> Thanks for pointing out the duplication of efforts!  I dug through the
> fixes-togreg branch and found the changes you are referencing.
> 
> I think that there might still be an issue there.  I believe that the
> regmap_update_bits function will only write to the ctrl_meas register
> if there is a difference in the register value vs what is in the
> regmap cache.  So in the situation where someone wanted to update the
> humidity sampling configuration, I don't think the write to the
> ctrl_meas write would go out on the bus.  And as a consequence, the
> changes to the humidity settings wouldn't become effective.
> 
> What is your take?  I can refactor my patch to just fix that part if
> you see the same bug.

Yes, i can see this bug, but i haven't tried so far. It's really worth to send
it out as a patch

Andreas

> 
> -Colin
> 
> >
> > Andreas
> >
> >>
> >> Also writes to the config register can be ignored if the device is in Normal Mode.
> >> Change the device to sleep mode before updating the config register.
> >> This has the nice side effect of guaranteeing that the regmap_update_bits
> >> function actually updates the driver and not just verifies the setting in its cache.
> >>
> >> Signed-off-by: Colin Parker <colin.parker@aclima.io>
> >> ---
> >>  drivers/iio/pressure/bmp280-core.c | 22 +++++++++++++++-------
> >>  1 file changed, 15 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> >> index d82b788..85aa24c 100644
> >> --- a/drivers/iio/pressure/bmp280-core.c
> >> +++ b/drivers/iio/pressure/bmp280-core.c
> >> @@ -559,10 +559,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
> >>                 BMP280_OSRS_PRESS_X(data->oversampling_press + 1);
> >>
> >>       ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> >> -                              BMP280_OSRS_TEMP_MASK |
> >> -                              BMP280_OSRS_PRESS_MASK |
> >> -                              BMP280_MODE_MASK,
> >> -                              osrs | BMP280_MODE_NORMAL);
> >> +                              BMP280_MODE_MASK, BMP280_MODE_SLEEP);
> >>       if (ret < 0) {
> >>               dev_err(data->dev,
> >>                       "failed to write ctrl_meas register\n");
> >> @@ -578,6 +575,17 @@ static int bmp280_chip_config(struct bmp280_data *data)
> >>               return ret;
> >>       }
> >>
> >> +     ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> >> +                              BMP280_OSRS_TEMP_MASK |
> >> +                              BMP280_OSRS_PRESS_MASK |
> >> +                              BMP280_MODE_MASK,
> >> +                              osrs | BMP280_MODE_NORMAL);
> >> +     if (ret < 0) {
> >> +             dev_err(data->dev,
> >> +                     "failed to write ctrl_meas register\n");
> >> +             return ret;
> >> +     }
> >> +
> >>       return ret;
> >>  }
> >>
> >> @@ -597,14 +605,14 @@ static const struct bmp280_chip_info bmp280_chip_info = {
> >>
> >>  static int bme280_chip_config(struct bmp280_data *data)
> >>  {
> >> -     int ret = bmp280_chip_config(data);
> >>       u8 osrs = BMP280_OSRS_HUMIDITIY_X(data->oversampling_humid + 1);
> >> +     int ret = regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
> >> +                               BMP280_OSRS_HUMIDITY_MASK, osrs);
> >>
> >>       if (ret < 0)
> >>               return ret;
> >>
> >> -     return regmap_update_bits(data->regmap, BMP280_REG_CTRL_HUMIDITY,
> >> -                               BMP280_OSRS_HUMIDITY_MASK, osrs);
> >> +     return bmp280_chip_config(data);
> >>  }
> >>
> >>  static const struct bmp280_chip_info bme280_chip_info = {
> >> --
> >> 2.1.4
> >>
> >> --
> >> 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
> >
> > --
> 
> 
> 
> -- 
> Colin Parker
> 
> 
> ACLIMA INC
> 
> Direct (415) 735-5062 |  Email colin.foeparker@aclima.io
> 
> 10 Lombard St. Suite 250  |  San Francisco, CA 94111
> 
> 
> Hello, we're Aclima.
> 
> 
> This email and any attachments may contain private, confidential and
> privileged material for the sole use of the intended recipient. If you
> are not the intended recipient, please immediately delete this email
> and any attachments.

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