* [PATCH] iio: pressure: bmp280: fix relative humidity unit @ 2018-05-28 15:38 Tomasz Duszynski 2018-05-30 0:44 ` Matt Ranostay 2018-06-03 14:41 ` Jonathan Cameron 0 siblings, 2 replies; 13+ messages in thread From: Tomasz Duszynski @ 2018-05-28 15:38 UTC (permalink / raw) To: linux-iio; +Cc: jic23, linus.walleij 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-28 15:38 [PATCH] iio: pressure: bmp280: fix relative humidity unit Tomasz Duszynski @ 2018-05-30 0:44 ` Matt Ranostay 2018-05-30 1:17 ` Phil Reid 2018-06-03 14:41 ` Jonathan Cameron 1 sibling, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2018-05-30 0:44 UTC (permalink / raw) To: Tomasz Duszynski; +Cc: linux-iio, Jonathan Cameron, Linus Walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 0:44 ` Matt Ranostay @ 2018-05-30 1:17 ` Phil Reid 2018-05-30 5:05 ` Matt Ranostay 0 siblings, 1 reply; 13+ messages in thread From: Phil Reid @ 2018-05-30 1:17 UTC (permalink / raw) To: Matt Ranostay, Tomasz Duszynski Cc: linux-iio, Jonathan Cameron, Linus Walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 1:17 ` Phil Reid @ 2018-05-30 5:05 ` Matt Ranostay 2018-05-30 7:20 ` Matt Ranostay 0 siblings, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2018-05-30 5:05 UTC (permalink / raw) To: Phil Reid; +Cc: Tomasz Duszynski, linux-iio, Jonathan Cameron, Linus Walleij 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 5:05 ` Matt Ranostay @ 2018-05-30 7:20 ` Matt Ranostay 2018-05-30 14:23 ` Tomasz Duszynski 2018-06-03 14:45 ` Jonathan Cameron 0 siblings, 2 replies; 13+ messages in thread From: Matt Ranostay @ 2018-05-30 7:20 UTC (permalink / raw) To: Phil Reid; +Cc: Tomasz Duszynski, linux-iio, Jonathan Cameron, Linus Walleij 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 >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 7:20 ` Matt Ranostay @ 2018-05-30 14:23 ` Tomasz Duszynski 2018-05-30 23:31 ` Phil Reid 2018-05-31 2:06 ` Matt Ranostay 2018-06-03 14:45 ` Jonathan Cameron 1 sibling, 2 replies; 13+ messages in thread From: Tomasz Duszynski @ 2018-05-30 14:23 UTC (permalink / raw) To: Matt Ranostay Cc: Phil Reid, Tomasz Duszynski, linux-iio, Jonathan Cameron, Linus Walleij 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 > >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 14:23 ` Tomasz Duszynski @ 2018-05-30 23:31 ` Phil Reid 2018-05-31 1:04 ` Matt Ranostay 2018-05-31 2:06 ` Matt Ranostay 1 sibling, 1 reply; 13+ messages in thread From: Phil Reid @ 2018-05-30 23:31 UTC (permalink / raw) To: Tomasz Duszynski, Matt Ranostay Cc: linux-iio, Jonathan Cameron, Linus Walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 23:31 ` Phil Reid @ 2018-05-31 1:04 ` Matt Ranostay 0 siblings, 0 replies; 13+ messages in thread From: Matt Ranostay @ 2018-05-31 1:04 UTC (permalink / raw) To: Phil Reid; +Cc: Tomasz Duszynski, linux-iio, Jonathan Cameron, Linus Walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 14:23 ` Tomasz Duszynski 2018-05-30 23:31 ` Phil Reid @ 2018-05-31 2:06 ` Matt Ranostay 2018-06-03 14:59 ` Jonathan Cameron 1 sibling, 1 reply; 13+ messages in thread From: Matt Ranostay @ 2018-05-31 2:06 UTC (permalink / raw) To: Tomasz Duszynski; +Cc: Phil Reid, linux-iio, Jonathan Cameron, Linus Walleij 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 >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-31 2:06 ` Matt Ranostay @ 2018-06-03 14:59 ` Jonathan Cameron 0 siblings, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2018-06-03 14:59 UTC (permalink / raw) To: Matt Ranostay; +Cc: Tomasz Duszynski, Phil Reid, linux-iio, Linus Walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-30 7:20 ` Matt Ranostay 2018-05-30 14:23 ` Tomasz Duszynski @ 2018-06-03 14:45 ` Jonathan Cameron 1 sibling, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2018-06-03 14:45 UTC (permalink / raw) To: Matt Ranostay; +Cc: Phil Reid, Tomasz Duszynski, linux-iio, Linus Walleij 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 > >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-05-28 15:38 [PATCH] iio: pressure: bmp280: fix relative humidity unit Tomasz Duszynski 2018-05-30 0:44 ` Matt Ranostay @ 2018-06-03 14:41 ` Jonathan Cameron 2018-06-03 14:43 ` Jonathan Cameron 1 sibling, 1 reply; 13+ messages in thread From: Jonathan Cameron @ 2018-06-03 14:41 UTC (permalink / raw) To: Tomasz Duszynski; +Cc: linux-iio, linus.walleij 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, ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iio: pressure: bmp280: fix relative humidity unit 2018-06-03 14:41 ` Jonathan Cameron @ 2018-06-03 14:43 ` Jonathan Cameron 0 siblings, 0 replies; 13+ messages in thread From: Jonathan Cameron @ 2018-06-03 14:43 UTC (permalink / raw) To: Tomasz Duszynski; +Cc: linux-iio, linus.walleij 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-06-03 14:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-28 15:38 [PATCH] iio: pressure: bmp280: fix relative humidity unit Tomasz Duszynski 2018-05-30 0:44 ` Matt Ranostay 2018-05-30 1:17 ` Phil Reid 2018-05-30 5:05 ` Matt Ranostay 2018-05-30 7:20 ` Matt Ranostay 2018-05-30 14:23 ` Tomasz Duszynski 2018-05-30 23:31 ` Phil Reid 2018-05-31 1:04 ` Matt Ranostay 2018-05-31 2:06 ` Matt Ranostay 2018-06-03 14:59 ` Jonathan Cameron 2018-06-03 14:45 ` Jonathan Cameron 2018-06-03 14:41 ` Jonathan Cameron 2018-06-03 14:43 ` 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).