| Message ID | 20180528153859.813-1-tduszyns@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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 >
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 > -- 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
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 >> -- 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
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 > >> -- 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
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 >>>> > >
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 -- 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
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 >> >> -- 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
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, -- 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
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 -- 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
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 > >> -- 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
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 -- 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
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,
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(-)