* [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
@ 2016-05-30 2:52 Matt Ranostay
2016-05-30 2:53 ` Matt Ranostay
0 siblings, 1 reply; 6+ messages in thread
From: Matt Ranostay @ 2016-05-30 2:52 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, amsfield22, Matt Ranostay
Shifting sensor data to the right 2 bits was incorrect and caused the
scaling values + offsets to be invalid.
Reported-by: Alison Schofield <amsfield22@gmail.com>
Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 3070983..a03832a 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
dev_err(&client->dev, "cannot read high byte measurement");
return ret;
}
- val = ret << 6;
+ val = ret << 8;
ret = i2c_smbus_read_byte(client);
if (ret < 0) {
dev_err(&client->dev, "cannot read low byte measurement");
return ret;
}
- val |= ret >> 2;
+ val |= ret;
return val;
}
@@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
if (chan->type == IIO_TEMP) {
*val = 165000;
- *val2 = 65536 >> 2;
+ *val2 = 65536;
return IIO_VAL_FRACTIONAL;
} else {
- *val = 0;
- *val2 = 10000;
- return IIO_VAL_INT_PLUS_MICRO;
+ *val = 100;
+ *val2 = 65536;
+ return IIO_VAL_FRACTIONAL;
}
break;
case IIO_CHAN_INFO_OFFSET:
- *val = -3971;
- *val2 = 879096;
+ *val = -15887;
+ *val2 = 515151;
return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
2016-05-30 2:52 [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling Matt Ranostay
@ 2016-05-30 2:53 ` Matt Ranostay
2016-05-31 17:45 ` Alison Schofield
0 siblings, 1 reply; 6+ messages in thread
From: Matt Ranostay @ 2016-05-30 2:53 UTC (permalink / raw)
To: linux-iio@vger.kernel.org
Cc: Jonathan Cameron, Alison Schofield, Matt Ranostay
Alison,
Can you give a Tested-by: since I don't own this sensor anymore.
Thanks,
Matt
On Sun, May 29, 2016 at 7:52 PM, Matt Ranostay <mranostay@gmail.com> wrote:
> Shifting sensor data to the right 2 bits was incorrect and caused the
> scaling values + offsets to be invalid.
>
> Reported-by: Alison Schofield <amsfield22@gmail.com>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
> drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index 3070983..a03832a 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
> dev_err(&client->dev, "cannot read high byte measurement");
> return ret;
> }
> - val = ret << 6;
> + val = ret << 8;
>
> ret = i2c_smbus_read_byte(client);
> if (ret < 0) {
> dev_err(&client->dev, "cannot read low byte measurement");
> return ret;
> }
> - val |= ret >> 2;
> + val |= ret;
>
> return val;
> }
> @@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_SCALE:
> if (chan->type == IIO_TEMP) {
> *val = 165000;
> - *val2 = 65536 >> 2;
> + *val2 = 65536;
> return IIO_VAL_FRACTIONAL;
> } else {
> - *val = 0;
> - *val2 = 10000;
> - return IIO_VAL_INT_PLUS_MICRO;
> + *val = 100;
> + *val2 = 65536;
> + return IIO_VAL_FRACTIONAL;
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> - *val = -3971;
> - *val2 = 879096;
> + *val = -15887;
> + *val2 = 515151;
> return IIO_VAL_INT_PLUS_MICRO;
> default:
> return -EINVAL;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
2016-05-30 2:53 ` Matt Ranostay
@ 2016-05-31 17:45 ` Alison Schofield
2016-05-31 18:57 ` Matt Ranostay
2016-06-03 12:26 ` Jonathan Cameron
0 siblings, 2 replies; 6+ messages in thread
From: Alison Schofield @ 2016-05-31 17:45 UTC (permalink / raw)
To: Matt Ranostay, jic23; +Cc: linux-iio@vger.kernel.org
On Sun, May 29, 2016 at 07:53:08PM -0700, Matt Ranostay wrote:
> Alison,
>
> Can you give a Tested-by: since I don't own this sensor anymore.
>
> Thanks,
>
> Matt
Tested it. Verified temp & humidty calcs. Looks great!
And, (seeing a pattern here ;)) this comes with another question,
probably directed a Jonathan...
with regards to temperature calculations:
Datasheets give numbers for this method:
temp = ((raw * scale) + offset)
IIO drivers report numbers for this method:
temp = ((raw + offset) * scale)
I've figured out the math for translating between them and creating
the offset for the driver to report in sysfs, but don't know why we
are doing that. 'Our' way seems kind of ugly. For example, for HDC1008
the offset is -40. It's the minimum expected temp value. But, we munge
it to be -15887.515151 so we can apply it before scaling.
Why do we do it this way in IIO?
alisons
>
> On Sun, May 29, 2016 at 7:52 PM, Matt Ranostay <mranostay@gmail.com> wrote:
> > Shifting sensor data to the right 2 bits was incorrect and caused the
> > scaling values + offsets to be invalid.
> >
> > Reported-by: Alison Schofield <amsfield22@gmail.com>
> > Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> > ---
> > drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> > index 3070983..a03832a 100644
> > --- a/drivers/iio/humidity/hdc100x.c
> > +++ b/drivers/iio/humidity/hdc100x.c
> > @@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
> > dev_err(&client->dev, "cannot read high byte measurement");
> > return ret;
> > }
> > - val = ret << 6;
> > + val = ret << 8;
> >
> > ret = i2c_smbus_read_byte(client);
> > if (ret < 0) {
> > dev_err(&client->dev, "cannot read low byte measurement");
> > return ret;
> > }
> > - val |= ret >> 2;
> > + val |= ret;
> >
> > return val;
> > }
> > @@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
> > case IIO_CHAN_INFO_SCALE:
> > if (chan->type == IIO_TEMP) {
> > *val = 165000;
> > - *val2 = 65536 >> 2;
> > + *val2 = 65536;
> > return IIO_VAL_FRACTIONAL;
> > } else {
> > - *val = 0;
> > - *val2 = 10000;
> > - return IIO_VAL_INT_PLUS_MICRO;
> > + *val = 100;
> > + *val2 = 65536;
> > + return IIO_VAL_FRACTIONAL;
> > }
> > break;
> > case IIO_CHAN_INFO_OFFSET:
> > - *val = -3971;
> > - *val2 = 879096;
> > + *val = -15887;
> > + *val2 = 515151;
> > return IIO_VAL_INT_PLUS_MICRO;
> > default:
> > return -EINVAL;
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
2016-05-31 17:45 ` Alison Schofield
@ 2016-05-31 18:57 ` Matt Ranostay
2016-05-31 21:05 ` Jonathan Cameron
2016-06-03 12:26 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Matt Ranostay @ 2016-05-31 18:57 UTC (permalink / raw)
To: Alison Schofield; +Cc: Jonathan Cameron, linux-iio@vger.kernel.org
On Tue, May 31, 2016 at 10:45 AM, Alison Schofield <amsfield22@gmail.com> wrote:
> On Sun, May 29, 2016 at 07:53:08PM -0700, Matt Ranostay wrote:
>> Alison,
>>
>> Can you give a Tested-by: since I don't own this sensor anymore.
>>
>> Thanks,
>>
>> Matt
>
> Tested it. Verified temp & humidty calcs. Looks great!
>
> And, (seeing a pattern here ;)) this comes with another question,
> probably directed a Jonathan...
>
> with regards to temperature calculations:
>
> Datasheets give numbers for this method:
> temp = ((raw * scale) + offset)
> IIO drivers report numbers for this method:
> temp = ((raw + offset) * scale)
>
We do it this way in iio.. hence why the offset needs the scaling
value applied to it.
> I've figured out the math for translating between them and creating
> the offset for the driver to report in sysfs, but don't know why we
> are doing that. 'Our' way seems kind of ugly. For example, for HDC1008
> the offset is -40. It's the minimum expected temp value. But, we munge
> it to be -15887.515151 so we can apply it before scaling.
>
> Why do we do it this way in IIO?
>
> alisons
>
>>
>> On Sun, May 29, 2016 at 7:52 PM, Matt Ranostay <mranostay@gmail.com> wrote:
>> > Shifting sensor data to the right 2 bits was incorrect and caused the
>> > scaling values + offsets to be invalid.
>> >
>> > Reported-by: Alison Schofield <amsfield22@gmail.com>
>> > Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> > ---
>> > drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>> > index 3070983..a03832a 100644
>> > --- a/drivers/iio/humidity/hdc100x.c
>> > +++ b/drivers/iio/humidity/hdc100x.c
>> > @@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>> > dev_err(&client->dev, "cannot read high byte measurement");
>> > return ret;
>> > }
>> > - val = ret << 6;
>> > + val = ret << 8;
>> >
>> > ret = i2c_smbus_read_byte(client);
>> > if (ret < 0) {
>> > dev_err(&client->dev, "cannot read low byte measurement");
>> > return ret;
>> > }
>> > - val |= ret >> 2;
>> > + val |= ret;
>> >
>> > return val;
>> > }
>> > @@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
>> > case IIO_CHAN_INFO_SCALE:
>> > if (chan->type == IIO_TEMP) {
>> > *val = 165000;
>> > - *val2 = 65536 >> 2;
>> > + *val2 = 65536;
>> > return IIO_VAL_FRACTIONAL;
>> > } else {
>> > - *val = 0;
>> > - *val2 = 10000;
>> > - return IIO_VAL_INT_PLUS_MICRO;
>> > + *val = 100;
>> > + *val2 = 65536;
>> > + return IIO_VAL_FRACTIONAL;
>> > }
>> > break;
>> > case IIO_CHAN_INFO_OFFSET:
>> > - *val = -3971;
>> > - *val2 = 879096;
>> > + *val = -15887;
>> > + *val2 = 515151;
>> > return IIO_VAL_INT_PLUS_MICRO;
>> > default:
>> > return -EINVAL;
>> > --
>> > 2.7.4
>> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
2016-05-31 18:57 ` Matt Ranostay
@ 2016-05-31 21:05 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2016-05-31 21:05 UTC (permalink / raw)
To: Matt Ranostay, Alison Schofield; +Cc: linux-iio@vger.kernel.org
On 31 May 2016 19:57:26 BST, Matt Ranostay <mranostay@gmail.com> wrote:
>On Tue, May 31, 2016 at 10:45 AM, Alison Schofield
><amsfield22@gmail.com> wrote:
>> On Sun, May 29, 2016 at 07:53:08PM -0700, Matt Ranostay wrote:
>>> Alison,
>>>
>>> Can you give a Tested-by: since I don't own this sensor anymore.
>>>
>>> Thanks,
>>>
>>> Matt
>>
>> Tested it. Verified temp & humidty calcs. Looks great!
>>
>> And, (seeing a pattern here ;)) this comes with another question,
>> probably directed a Jonathan...
>>
>> with regards to temperature calculations:
>>
>> Datasheets give numbers for this method:
>> temp = ((raw * scale) + offset)
>> IIO drivers report numbers for this method:
>> temp = ((raw + offset) * scale)
>>
>We do it this way in iio.. hence why the offset needs the scaling
>value applied to it.
>
>> I've figured out the math for translating between them and creating
>> the offset for the driver to report in sysfs, but don't know why we
>> are doing that. 'Our' way seems kind of ugly. For example, for
>HDC1008
>> the offset is -40. It's the minimum expected temp value. But, we
>munge
>> it to be -15887.515151 so we can apply it before scaling.
>>
>> Why do we do it this way in IIO?
It is very device dependant or more correctly very data sheet dependant. By coincidence a
lot of early IIO drivers were for parts that had it the other way around so we ended
up with that. A classic example is a differential ADC that does an offset
rather than 2's complement. Those are much neater offset first.
So six of one and half a dozen of the other...
Jonathan
>>
>> alisons
>>
>>>
>>> On Sun, May 29, 2016 at 7:52 PM, Matt Ranostay <mranostay@gmail.com>
>wrote:
>>> > Shifting sensor data to the right 2 bits was incorrect and caused
>the
>>> > scaling values + offsets to be invalid.
>>> >
>>> > Reported-by: Alison Schofield <amsfield22@gmail.com>
>>> > Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>> > ---
>>> > drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
>>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>>> >
>>> > diff --git a/drivers/iio/humidity/hdc100x.c
>b/drivers/iio/humidity/hdc100x.c
>>> > index 3070983..a03832a 100644
>>> > --- a/drivers/iio/humidity/hdc100x.c
>>> > +++ b/drivers/iio/humidity/hdc100x.c
>>> > @@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct
>hdc100x_data *data,
>>> > dev_err(&client->dev, "cannot read high byte
>measurement");
>>> > return ret;
>>> > }
>>> > - val = ret << 6;
>>> > + val = ret << 8;
>>> >
>>> > ret = i2c_smbus_read_byte(client);
>>> > if (ret < 0) {
>>> > dev_err(&client->dev, "cannot read low byte
>measurement");
>>> > return ret;
>>> > }
>>> > - val |= ret >> 2;
>>> > + val |= ret;
>>> >
>>> > return val;
>>> > }
>>> > @@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev
>*indio_dev,
>>> > case IIO_CHAN_INFO_SCALE:
>>> > if (chan->type == IIO_TEMP) {
>>> > *val = 165000;
>>> > - *val2 = 65536 >> 2;
>>> > + *val2 = 65536;
>>> > return IIO_VAL_FRACTIONAL;
>>> > } else {
>>> > - *val = 0;
>>> > - *val2 = 10000;
>>> > - return IIO_VAL_INT_PLUS_MICRO;
>>> > + *val = 100;
>>> > + *val2 = 65536;
>>> > + return IIO_VAL_FRACTIONAL;
>>> > }
>>> > break;
>>> > case IIO_CHAN_INFO_OFFSET:
>>> > - *val = -3971;
>>> > - *val2 = 879096;
>>> > + *val = -15887;
>>> > + *val2 = 515151;
>>> > return IIO_VAL_INT_PLUS_MICRO;
>>> > default:
>>> > return -EINVAL;
>>> > --
>>> > 2.7.4
>>> >
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling
2016-05-31 17:45 ` Alison Schofield
2016-05-31 18:57 ` Matt Ranostay
@ 2016-06-03 12:26 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2016-06-03 12:26 UTC (permalink / raw)
To: Alison Schofield, Matt Ranostay; +Cc: linux-iio@vger.kernel.org
On 31/05/16 18:45, Alison Schofield wrote:
> On Sun, May 29, 2016 at 07:53:08PM -0700, Matt Ranostay wrote:
>> Alison,
>>
>> Can you give a Tested-by: since I don't own this sensor anymore.
>>
>> Thanks,
>>
>> Matt
>
> Tested it. Verified temp & humidty calcs. Looks great!
Ideally please give an explicit tag.
Tested-by: Alison Schofield <amsfield22@gmail.com>
(I've add this time on the basis you meant that I think...)
Applied to fixes-togreg and marked for stable. Thanks,
Jonathan
>
> And, (seeing a pattern here ;)) this comes with another question,
> probably directed a Jonathan...
>
> with regards to temperature calculations:
>
> Datasheets give numbers for this method:
> temp = ((raw * scale) + offset)
> IIO drivers report numbers for this method:
> temp = ((raw + offset) * scale)
>
> I've figured out the math for translating between them and creating
> the offset for the driver to report in sysfs, but don't know why we
> are doing that. 'Our' way seems kind of ugly. For example, for HDC1008
> the offset is -40. It's the minimum expected temp value. But, we munge
> it to be -15887.515151 so we can apply it before scaling.
>
> Why do we do it this way in IIO?
>
> alisons
>
>>
>> On Sun, May 29, 2016 at 7:52 PM, Matt Ranostay <mranostay@gmail.com> wrote:
>>> Shifting sensor data to the right 2 bits was incorrect and caused the
>>> scaling values + offsets to be invalid.
>>>
>>> Reported-by: Alison Schofield <amsfield22@gmail.com>
>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>> ---
>>> drivers/iio/humidity/hdc100x.c | 16 ++++++++--------
>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
>>> index 3070983..a03832a 100644
>>> --- a/drivers/iio/humidity/hdc100x.c
>>> +++ b/drivers/iio/humidity/hdc100x.c
>>> @@ -164,14 +164,14 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>>> dev_err(&client->dev, "cannot read high byte measurement");
>>> return ret;
>>> }
>>> - val = ret << 6;
>>> + val = ret << 8;
>>>
>>> ret = i2c_smbus_read_byte(client);
>>> if (ret < 0) {
>>> dev_err(&client->dev, "cannot read low byte measurement");
>>> return ret;
>>> }
>>> - val |= ret >> 2;
>>> + val |= ret;
>>>
>>> return val;
>>> }
>>> @@ -212,17 +212,17 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
>>> case IIO_CHAN_INFO_SCALE:
>>> if (chan->type == IIO_TEMP) {
>>> *val = 165000;
>>> - *val2 = 65536 >> 2;
>>> + *val2 = 65536;
>>> return IIO_VAL_FRACTIONAL;
>>> } else {
>>> - *val = 0;
>>> - *val2 = 10000;
>>> - return IIO_VAL_INT_PLUS_MICRO;
>>> + *val = 100;
>>> + *val2 = 65536;
>>> + return IIO_VAL_FRACTIONAL;
>>> }
>>> break;
>>> case IIO_CHAN_INFO_OFFSET:
>>> - *val = -3971;
>>> - *val2 = 879096;
>>> + *val = -15887;
>>> + *val2 = 515151;
>>> return IIO_VAL_INT_PLUS_MICRO;
>>> default:
>>> return -EINVAL;
>>> --
>>> 2.7.4
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-03 12:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-30 2:52 [PATCH] iio: hudmidity: hdc100x: fix incorrect shifting and scaling Matt Ranostay
2016-05-30 2:53 ` Matt Ranostay
2016-05-31 17:45 ` Alison Schofield
2016-05-31 18:57 ` Matt Ranostay
2016-05-31 21:05 ` Jonathan Cameron
2016-06-03 12:26 ` 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).