[PATCH] iio: pressure: bmp280: fix relative humidity unit

[PATCH] iio: pressure: bmp280: fix relative humidity unit

[Tomasz Duszynski]

According to IIO ABI relative humidity reading should be
returned in milli percent.

This patch addresses that by applying proper scaling and
returning integer instead of fractional format type specifier.

Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 5ec3e41b65f2..fe87d27779d9 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
 	}
 	comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
 
-	*val = comp_humidity;
-	*val2 = 1024;
+	*val = comp_humidity * 1000 / 1024;
 
-	return IIO_VAL_FRACTIONAL;
+	return IIO_VAL_INT;
 }
 
 static int bmp280_read_raw(struct iio_dev *indio_dev,
-- 
2.17.0

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Matt Ranostay]

On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com> wrote:
> According to IIO ABI relative humidity reading should be
> returned in milli percent.
>
> This patch addresses that by applying proper scaling and
> returning integer instead of fractional format type specifier.
>

*sigh* seems this is my mistake, but good catch.  Slight nitpick
below.. otherwise looks good

> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
> ---
>  drivers/iio/pressure/bmp280-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 5ec3e41b65f2..fe87d27779d9 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
>         }
>         comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>
> -       *val = comp_humidity;
> -       *val2 = 1024;
> +       *val = comp_humidity * 1000 / 1024;

Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) * 1000

>
> -       return IIO_VAL_FRACTIONAL;
> +       return IIO_VAL_INT;
>  }
>
>  static int bmp280_read_raw(struct iio_dev *indio_dev,
> --
> 2.17.0
>
> --
> 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: pressure: bmp280: fix relative humidity unit

[Phil Reid]

On 30/05/2018 08:44, Matt Ranostay wrote:
> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com> wrote:
>> According to IIO ABI relative humidity reading should be
>> returned in milli percent.
>>
>> This patch addresses that by applying proper scaling and
>> returning integer instead of fractional format type specifier.
>>
> 
> *sigh* seems this is my mistake, but good catch.  Slight nitpick
> below.. otherwise looks good
> 
>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>> ---
>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
>> index 5ec3e41b65f2..fe87d27779d9 100644
>> --- a/drivers/iio/pressure/bmp280-core.c
>> +++ b/drivers/iio/pressure/bmp280-core.c
>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
>>          }
>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>>
>> -       *val = comp_humidity;
>> -       *val2 = 1024;
>> +       *val = comp_humidity * 1000 / 1024;
> 
> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) * 1000
> 
be careful of integer division.


>>
>> -       return IIO_VAL_FRACTIONAL;
>> +       return IIO_VAL_INT;
>>   }
>>
>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
>> --
>> 2.17.0

> 


-- 
Regards
Phil Reid

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Matt Ranostay]

On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:
> On 30/05/2018 08:44, Matt Ranostay wrote:
>>
>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
>> wrote:
>>>
>>> According to IIO ABI relative humidity reading should be
>>> returned in milli percent.
>>>
>>> This patch addresses that by applying proper scaling and
>>> returning integer instead of fractional format type specifier.
>>>
>>
>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
>> below.. otherwise looks good
>>
>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>>> ---
>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/pressure/bmp280-core.c
>>> b/drivers/iio/pressure/bmp280-core.c
>>> index 5ec3e41b65f2..fe87d27779d9 100644
>>> --- a/drivers/iio/pressure/bmp280-core.c
>>> +++ b/drivers/iio/pressure/bmp280-core.c
>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
>>> *data, int *val, int *val2)
>>>          }
>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>>>
>>> -       *val = comp_humidity;
>>> -       *val2 = 1024;
>>> +       *val = comp_humidity * 1000 / 1024;
>>
>>
>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
>> 1000
>>
> be careful of integer division.
>

Ah yes good point. You will have to check if comp_humidity isn't zero
or it is possible to have a divide-by-zero.

- Matt

>
>>>
>>> -       return IIO_VAL_FRACTIONAL;
>>> +       return IIO_VAL_INT;
>>>   }
>>>
>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
>>> --
>>> 2.17.0
>
>
>>
>
>
> --
> Regards
> Phil Reid
>

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Matt Ranostay]

On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
<matt.ranostay@konsulko.com> wrote:
> On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:
>> On 30/05/2018 08:44, Matt Ranostay wrote:
>>>
>>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
>>> wrote:
>>>>
>>>> According to IIO ABI relative humidity reading should be
>>>> returned in milli percent.
>>>>
>>>> This patch addresses that by applying proper scaling and
>>>> returning integer instead of fractional format type specifier.
>>>>
>>>
>>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
>>> below.. otherwise looks good
>>>
>>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>>>> ---
>>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
>>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/pressure/bmp280-core.c
>>>> b/drivers/iio/pressure/bmp280-core.c
>>>> index 5ec3e41b65f2..fe87d27779d9 100644
>>>> --- a/drivers/iio/pressure/bmp280-core.c
>>>> +++ b/drivers/iio/pressure/bmp280-core.c
>>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
>>>> *data, int *val, int *val2)
>>>>          }
>>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>>>>
>>>> -       *val = comp_humidity;
>>>> -       *val2 = 1024;
>>>> +       *val = comp_humidity * 1000 / 1024;
>>>
>>>
>>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
>>> 1000
>>>
>> be careful of integer division.
>>
>
> Ah yes good point. You will have to check if comp_humidity isn't zero
> or it is possible to have a divide-by-zero.

D'oh actually divide by zero would be an issue. But you'll want to be
sure of possible overflows (doubt that would be an issue here).

However it may be better to just add the scaling factor of 1000 with
IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.

>
> - Matt
>
>>
>>>>
>>>> -       return IIO_VAL_FRACTIONAL;
>>>> +       return IIO_VAL_INT;
>>>>   }
>>>>
>>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
>>>> --
>>>> 2.17.0
>>
>>
>>>
>>
>>
>> --
>> Regards
>> Phil Reid
>>

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Tomasz Duszynski]

On Wed, May 30, 2018 at 03:20:37PM +0800, Matt Ranostay wrote:
> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
> <matt.ranostay@konsulko.com> wrote:
> > On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:
> >> On 30/05/2018 08:44, Matt Ranostay wrote:
> >>>
> >>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
> >>> wrote:
> >>>>
> >>>> According to IIO ABI relative humidity reading should be
> >>>> returned in milli percent.
> >>>>
> >>>> This patch addresses that by applying proper scaling and
> >>>> returning integer instead of fractional format type specifier.
> >>>>
> >>>
> >>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
> >>> below.. otherwise looks good
> >>>
> >>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
> >>>> ---
> >>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
> >>>>   1 file changed, 2 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/iio/pressure/bmp280-core.c
> >>>> b/drivers/iio/pressure/bmp280-core.c
> >>>> index 5ec3e41b65f2..fe87d27779d9 100644
> >>>> --- a/drivers/iio/pressure/bmp280-core.c
> >>>> +++ b/drivers/iio/pressure/bmp280-core.c
> >>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
> >>>> *data, int *val, int *val2)
> >>>>          }
> >>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> >>>>
> >>>> -       *val = comp_humidity;
> >>>> -       *val2 = 1024;
> >>>> +       *val = comp_humidity * 1000 / 1024;
> >>>
> >>>
> >>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
> >>> 1000
> >>>
> >> be careful of integer division.
> >>
> >
> > Ah yes good point. You will have to check if comp_humidity isn't zero
> > or it is possible to have a divide-by-zero.
>
> D'oh actually divide by zero would be an issue. But you'll want to be
> sure of possible overflows (doubt that would be an issue here).

What kind of division-by-zero are you meaning? When would that happen
in this case?

>
> However it may be better to just add the scaling factor of 1000 with
> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.

Is measurement precision your concern here? I would not bother with that
since the sensor is not a top-notch anyway. Even datasheet itself specifies
error margin of +/-3% for relative humidity.

>
> >
> > - Matt
> >
> >>
> >>>>
> >>>> -       return IIO_VAL_FRACTIONAL;
> >>>> +       return IIO_VAL_INT;
> >>>>   }
> >>>>
> >>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
> >>>> --
> >>>> 2.17.0
> >>
> >>
> >>>
> >>
> >>
> >> --
> >> Regards
> >> Phil Reid
> >>

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Phil Reid]

On 30/05/2018 22:23, Tomasz Duszynski wrote:
> On Wed, May 30, 2018 at 03:20:37PM +0800, Matt Ranostay wrote:
>> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
>> <matt.ranostay@konsulko.com> wrote:
>>> On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:
>>>> On 30/05/2018 08:44, Matt Ranostay wrote:
>>>>>
>>>>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> According to IIO ABI relative humidity reading should be
>>>>>> returned in milli percent.
>>>>>>
>>>>>> This patch addresses that by applying proper scaling and
>>>>>> returning integer instead of fractional format type specifier.
>>>>>>
>>>>>
>>>>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
>>>>> below.. otherwise looks good
>>>>>
>>>>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>>>>>> ---
>>>>>>    drivers/iio/pressure/bmp280-core.c | 5 ++---
>>>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/iio/pressure/bmp280-core.c
>>>>>> b/drivers/iio/pressure/bmp280-core.c
>>>>>> index 5ec3e41b65f2..fe87d27779d9 100644
>>>>>> --- a/drivers/iio/pressure/bmp280-core.c
>>>>>> +++ b/drivers/iio/pressure/bmp280-core.c
>>>>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
>>>>>> *data, int *val, int *val2)
>>>>>>           }
>>>>>>           comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>>>>>>
>>>>>> -       *val = comp_humidity;
>>>>>> -       *val2 = 1024;
>>>>>> +       *val = comp_humidity * 1000 / 1024;
>>>>>
>>>>>
>>>>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
>>>>> 1000
>>>>>
>>>> be careful of integer division.
>>>>
>>>
>>> Ah yes good point. You will have to check if comp_humidity isn't zero
>>> or it is possible to have a divide-by-zero.
>>
>> D'oh actually divide by zero would be an issue. But you'll want to be
>> sure of possible overflows (doubt that would be an issue here).
> 
> What kind of division-by-zero are you meaning? When would that happen
> in this case?
> 

I meant something like this:

10 / 1024 = 0
0 * 1000 = 0

10 * 1000 = 10000
10000 / 1024 = 9

patch looks like the right way to me
provided comp_humidity * 1000 is not going to overflow.


>>
>> However it may be better to just add the scaling factor of 1000 with
>> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.
> 
> Is measurement precision your concern here? I would not bother with that
> since the sensor is not a top-notch anyway. Even datasheet itself specifies
> error margin of +/-3% for relative humidity.
> 
>>
>>>
>>> - Matt
>>>
>>>>
>>>>>>
>>>>>> -       return IIO_VAL_FRACTIONAL;
>>>>>> +       return IIO_VAL_INT;
>>>>>>    }
>>>>>>
>>>>>>    static int bmp280_read_raw(struct iio_dev *indio_dev,
>>>>>> --
>>>>>> 2.17.0
>>>>
>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Regards
>>>> Phil Reid
>>>>
> 
> 


-- 
Regards
Phil Reid

ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au

3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Matt Ranostay]

On Thu, May 31, 2018 at 7:31 AM, Phil Reid <preid@electromag.com.au> wrote:
> On 30/05/2018 22:23, Tomasz Duszynski wrote:
>>
>> On Wed, May 30, 2018 at 03:20:37PM +0800, Matt Ranostay wrote:
>>>
>>> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
>>> <matt.ranostay@konsulko.com> wrote:
>>>>
>>>> On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au>
>>>> wrote:
>>>>>
>>>>> On 30/05/2018 08:44, Matt Ranostay wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski
>>>>>> <tduszyns@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> According to IIO ABI relative humidity reading should be
>>>>>>> returned in milli percent.
>>>>>>>
>>>>>>> This patch addresses that by applying proper scaling and
>>>>>>> returning integer instead of fractional format type specifier.
>>>>>>>
>>>>>>
>>>>>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
>>>>>> below.. otherwise looks good
>>>>>>
>>>>>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>>>>>>> ---
>>>>>>>    drivers/iio/pressure/bmp280-core.c | 5 ++---
>>>>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/iio/pressure/bmp280-core.c
>>>>>>> b/drivers/iio/pressure/bmp280-core.c
>>>>>>> index 5ec3e41b65f2..fe87d27779d9 100644
>>>>>>> --- a/drivers/iio/pressure/bmp280-core.c
>>>>>>> +++ b/drivers/iio/pressure/bmp280-core.c
>>>>>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
>>>>>>> *data, int *val, int *val2)
>>>>>>>           }
>>>>>>>           comp_humidity = bmp280_compensate_humidity(data,
>>>>>>> adc_humidity);
>>>>>>>
>>>>>>> -       *val = comp_humidity;
>>>>>>> -       *val2 = 1024;
>>>>>>> +       *val = comp_humidity * 1000 / 1024;
>>>>>>
>>>>>>
>>>>>>
>>>>>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024)
>>>>>> *
>>>>>> 1000
>>>>>>
>>>>> be careful of integer division.
>>>>>
>>>>
>>>> Ah yes good point. You will have to check if comp_humidity isn't zero
>>>> or it is possible to have a divide-by-zero.
>>>
>>>
>>> D'oh actually divide by zero would be an issue. But you'll want to be
>>> sure of possible overflows (doubt that would be an issue here).
>>
>>
>> What kind of division-by-zero are you meaning? When would that happen
>> in this case?
>>
>
> I meant something like this:
>
> 10 / 1024 = 0
> 0 * 1000 = 0
>
> 10 * 1000 = 10000
> 10000 / 1024 = 9
>
> patch looks like the right way to me
> provided comp_humidity * 1000 is not going to overflow.
>
>

Ah right.. forgot about order of operations does matter in integer division.

>
>>>
>>> However it may be better to just add the scaling factor of 1000 with
>>> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.
>>
>>
>> Is measurement precision your concern here? I would not bother with that
>> since the sensor is not a top-notch anyway. Even datasheet itself
>> specifies
>> error margin of +/-3% for relative humidity.
>>
>>>
>>>>
>>>> - Matt
>>>>
>>>>>
>>>>>>>
>>>>>>> -       return IIO_VAL_FRACTIONAL;
>>>>>>> +       return IIO_VAL_INT;
>>>>>>>    }
>>>>>>>
>>>>>>>    static int bmp280_read_raw(struct iio_dev *indio_dev,
>>>>>>> --
>>>>>>> 2.17.0
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Regards
>>>>> Phil Reid
>>>>>
>>
>>
>
>
> --
> Regards
> Phil Reid
>
> ElectroMagnetic Imaging Technology Pty Ltd
> Development of Geophysical Instrumentation & Software
> www.electromag.com.au
>
> 3 The Avenue, Midland WA 6056, AUSTRALIA
> Ph: +61 8 9250 8100
> Fax: +61 8 9250 7100
> Email: preid@electromag.com.au

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Matt Ranostay]

On Wed, May 30, 2018 at 10:23 PM, Tomasz Duszynski <tduszyns@gmail.com> wrote:
> On Wed, May 30, 2018 at 03:20:37PM +0800, Matt Ranostay wrote:
>> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
>> <matt.ranostay@konsulko.com> wrote:
>> > On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:
>> >> On 30/05/2018 08:44, Matt Ranostay wrote:
>> >>>
>> >>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
>> >>> wrote:
>> >>>>
>> >>>> According to IIO ABI relative humidity reading should be
>> >>>> returned in milli percent.
>> >>>>
>> >>>> This patch addresses that by applying proper scaling and
>> >>>> returning integer instead of fractional format type specifier.
>> >>>>
>> >>>
>> >>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
>> >>> below.. otherwise looks good
>> >>>
>> >>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
>> >>>> ---
>> >>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
>> >>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>> >>>>
>> >>>> diff --git a/drivers/iio/pressure/bmp280-core.c
>> >>>> b/drivers/iio/pressure/bmp280-core.c
>> >>>> index 5ec3e41b65f2..fe87d27779d9 100644
>> >>>> --- a/drivers/iio/pressure/bmp280-core.c
>> >>>> +++ b/drivers/iio/pressure/bmp280-core.c
>> >>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
>> >>>> *data, int *val, int *val2)
>> >>>>          }
>> >>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>> >>>>
>> >>>> -       *val = comp_humidity;
>> >>>> -       *val2 = 1024;
>> >>>> +       *val = comp_humidity * 1000 / 1024;
>> >>>
>> >>>
>> >>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
>> >>> 1000
>> >>>
>> >> be careful of integer division.
>> >>
>> >
>> > Ah yes good point. You will have to check if comp_humidity isn't zero
>> > or it is possible to have a divide-by-zero.
>>
>> D'oh actually divide by zero would be an issue. But you'll want to be
>> sure of possible overflows (doubt that would be an issue here).
>
> What kind of division-by-zero are you meaning? When would that happen
> in this case?
>
>>
>> However it may be better to just add the scaling factor of 1000 with
>> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.
>
> Is measurement precision your concern here? I would not bother with that
> since the sensor is not a top-notch anyway. Even datasheet itself specifies
> error margin of +/-3% for relative humidity.

As Phil mentioned the original patchset is okay as it is. Now I agree
with him after thinking it through.

Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>


>
>>
>> >
>> > - Matt
>> >
>> >>
>> >>>>
>> >>>> -       return IIO_VAL_FRACTIONAL;
>> >>>> +       return IIO_VAL_INT;
>> >>>>   }
>> >>>>
>> >>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
>> >>>> --
>> >>>> 2.17.0
>> >>
>> >>
>> >>>
>> >>
>> >>
>> >> --
>> >> Regards
>> >> Phil Reid
>> >>

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Jonathan Cameron]

On Mon, 28 May 2018 17:38:59 +0200
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> According to IIO ABI relative humidity reading should be
> returned in milli percent.
> 
> This patch addresses that by applying proper scaling and
> returning integer instead of fractional format type specifier.
> 
> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>

I'll be wanting input from Linus on this as he seems to be the last person
to have taken particular interest in the driver.

> ---
>  drivers/iio/pressure/bmp280-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 5ec3e41b65f2..fe87d27779d9 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
>  	}
>  	comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
>  
> -	*val = comp_humidity;
> -	*val2 = 1024;
> +	*val = comp_humidity * 1000 / 1024;

Why not just *val = *val * 1000 and leave the rest alone?

>  
> -	return IIO_VAL_FRACTIONAL;
> +	return IIO_VAL_INT;
>  }
>  
>  static int bmp280_read_raw(struct iio_dev *indio_dev,

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Jonathan Cameron]

On Sun, 3 Jun 2018 15:41:54 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 28 May 2018 17:38:59 +0200
> Tomasz Duszynski <tduszyns@gmail.com> wrote:
> 
> > According to IIO ABI relative humidity reading should be
> > returned in milli percent.
> > 
> > This patch addresses that by applying proper scaling and
> > returning integer instead of fractional format type specifier.
> > 
> > Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>  
> 
> I'll be wanting input from Linus on this as he seems to be the last person
> to have taken particular interest in the driver.

Sorry, my email had 'stalled' for some reason so I didn't see the rest of
the thread.

Please ignore.

J
> 
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 5ec3e41b65f2..fe87d27779d9 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
> >  	}
> >  	comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> >  
> > -	*val = comp_humidity;
> > -	*val2 = 1024;
> > +	*val = comp_humidity * 1000 / 1024;  
> 
> Why not just *val = *val * 1000 and leave the rest alone?
> 
> >  
> > -	return IIO_VAL_FRACTIONAL;
> > +	return IIO_VAL_INT;
> >  }
> >  
> >  static int bmp280_read_raw(struct iio_dev *indio_dev,  
> 
> --
> 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: pressure: bmp280: fix relative humidity unit

[Jonathan Cameron]

On Wed, 30 May 2018 15:20:37 +0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
> <matt.ranostay@konsulko.com> wrote:
> > On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:  
> >> On 30/05/2018 08:44, Matt Ranostay wrote:  
> >>>
> >>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
> >>> wrote:  
> >>>>
> >>>> According to IIO ABI relative humidity reading should be
> >>>> returned in milli percent.
> >>>>
> >>>> This patch addresses that by applying proper scaling and
> >>>> returning integer instead of fractional format type specifier.
> >>>>  
> >>>
> >>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
> >>> below.. otherwise looks good
> >>>  
> >>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
> >>>> ---
> >>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
> >>>>   1 file changed, 2 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/iio/pressure/bmp280-core.c
> >>>> b/drivers/iio/pressure/bmp280-core.c
> >>>> index 5ec3e41b65f2..fe87d27779d9 100644
> >>>> --- a/drivers/iio/pressure/bmp280-core.c
> >>>> +++ b/drivers/iio/pressure/bmp280-core.c
> >>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
> >>>> *data, int *val, int *val2)
> >>>>          }
> >>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> >>>>
> >>>> -       *val = comp_humidity;
> >>>> -       *val2 = 1024;
> >>>> +       *val = comp_humidity * 1000 / 1024;  
> >>>
> >>>
> >>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
> >>> 1000
> >>>  
> >> be careful of integer division.
> >>  
> >
> > Ah yes good point. You will have to check if comp_humidity isn't zero
> > or it is possible to have a divide-by-zero.  
> 
> D'oh actually divide by zero would be an issue. But you'll want to be
> sure of possible overflows (doubt that would be an issue here).
> 
> However it may be better to just add the scaling factor of 1000 with
> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.
> 

Don't do that.  It would be an ABI change.  Whilst we can hope that
everyone is using a nice library that would cope with this, there
is no guarantee they are.

Jonathan

> >
> > - Matt
> >  
> >>  
> >>>>
> >>>> -       return IIO_VAL_FRACTIONAL;
> >>>> +       return IIO_VAL_INT;
> >>>>   }
> >>>>
> >>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
> >>>> --
> >>>> 2.17.0  
> >>
> >>  
> >>>  
> >>
> >>
> >> --
> >> Regards
> >> Phil Reid
> >>  

Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit

[Jonathan Cameron]

On Thu, 31 May 2018 10:06:50 +0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> On Wed, May 30, 2018 at 10:23 PM, Tomasz Duszynski <tduszyns@gmail.com> wrote:
> > On Wed, May 30, 2018 at 03:20:37PM +0800, Matt Ranostay wrote:  
> >> On Wed, May 30, 2018 at 1:05 PM, Matt Ranostay
> >> <matt.ranostay@konsulko.com> wrote:  
> >> > On Wed, May 30, 2018 at 9:17 AM, Phil Reid <preid@electromag.com.au> wrote:  
> >> >> On 30/05/2018 08:44, Matt Ranostay wrote:  
> >> >>>
> >> >>> On Mon, May 28, 2018 at 11:38 PM, Tomasz Duszynski <tduszyns@gmail.com>
> >> >>> wrote:  
> >> >>>>
> >> >>>> According to IIO ABI relative humidity reading should be
> >> >>>> returned in milli percent.
> >> >>>>
> >> >>>> This patch addresses that by applying proper scaling and
> >> >>>> returning integer instead of fractional format type specifier.
> >> >>>>  
> >> >>>
> >> >>> *sigh* seems this is my mistake, but good catch.  Slight nitpick
> >> >>> below.. otherwise looks good
> >> >>>  
> >> >>>> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
> >> >>>> ---
> >> >>>>   drivers/iio/pressure/bmp280-core.c | 5 ++---
> >> >>>>   1 file changed, 2 insertions(+), 3 deletions(-)
> >> >>>>
> >> >>>> diff --git a/drivers/iio/pressure/bmp280-core.c
> >> >>>> b/drivers/iio/pressure/bmp280-core.c
> >> >>>> index 5ec3e41b65f2..fe87d27779d9 100644
> >> >>>> --- a/drivers/iio/pressure/bmp280-core.c
> >> >>>> +++ b/drivers/iio/pressure/bmp280-core.c
> >> >>>> @@ -415,10 +415,9 @@ static int bmp280_read_humid(struct bmp280_data
> >> >>>> *data, int *val, int *val2)
> >> >>>>          }
> >> >>>>          comp_humidity = bmp280_compensate_humidity(data, adc_humidity);
> >> >>>>
> >> >>>> -       *val = comp_humidity;
> >> >>>> -       *val2 = 1024;
> >> >>>> +       *val = comp_humidity * 1000 / 1024;  
> >> >>>
> >> >>>
> >> >>> Minor nitpick  that it would look cleaner as:   (comp_humidity / 1024) *
> >> >>> 1000
> >> >>>  
> >> >> be careful of integer division.
> >> >>  
> >> >
> >> > Ah yes good point. You will have to check if comp_humidity isn't zero
> >> > or it is possible to have a divide-by-zero.  
> >>
> >> D'oh actually divide by zero would be an issue. But you'll want to be
> >> sure of possible overflows (doubt that would be an issue here).  
> >
> > What kind of division-by-zero are you meaning? When would that happen
> > in this case?
> >  
> >>
> >> However it may be better to just add the scaling factor of 1000 with
> >> IIO_CHAN_INFO_SCALE  and make the processed value now a raw one.  
> >
> > Is measurement precision your concern here? I would not bother with that
> > since the sensor is not a top-notch anyway. Even datasheet itself specifies
> > error margin of +/-3% for relative humidity.  
> 
> As Phil mentioned the original patchset is okay as it is. Now I agree
> with him after thinking it through.
> 
> Acked-by: Matt Ranostay <matt.ranostay@konsulko.com>

Applied to the fixes-togreg branch of iio.git.

Note this won't go upstream until after the merge window.  It should also
have had a relevant fixes tag.

I added the original humidity one of Matt's though the patch won't apply
that far back due to splitting of code when SPI support was added.
I added a note to that effect.

Jonathan

> 
> 
> >  
> >>  
> >> >
> >> > - Matt
> >> >  
> >> >>  
> >> >>>>
> >> >>>> -       return IIO_VAL_FRACTIONAL;
> >> >>>> +       return IIO_VAL_INT;
> >> >>>>   }
> >> >>>>
> >> >>>>   static int bmp280_read_raw(struct iio_dev *indio_dev,
> >> >>>> --
> >> >>>> 2.17.0  
> >> >>
> >> >>  
> >> >>>  
> >> >>
> >> >>
> >> >> --
> >> >> Regards
> >> >> Phil Reid
> >> >>  
> --
> 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