iio: hdc100x: correct IIO_CHAN_INFO_OFFSET value

PATCH In review Matt Ranostay 3 participants 10 messages Sep 27, 2015 – Oct 11, 2015 IIO

Share & tools

Matt Ranostay

author

2015-09-27 06:19 UTC #0 lore raw

PATCH 1/1 iio: hdc100x: correct IIO_CHAN_INFO_OFFSET value

Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.

Signed-off-by Matt Ranostay <[email protected]>

---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/iio/humidity/hdc100x.c +3 -2

index 2824578..a7f61e8 100644
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
 		}
 		break;
 	case IIO_CHAN_INFO_OFFSET:
-		*val = -40;
-		return IIO_VAL_INT;
+		*val = -3971;
+		*val2 = 879096;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}

Signature · 1 line

1.9.1

Jonathan Cameron

maintainer

2015-09-27 13:29 UTC #1 lore raw

Quoted 5 lines · Matt Ranostay

Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>

Oops, missed that.

Given it's provided in the datasheet as effectively a fractional value would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be cleaner and give the same answer?

Speaking of which, for the scale are we loosing any precision by shifting the bottom of the fraction right 2 rather than the top left 2 which would have the same result?

Jonathan

Quoted 21 lines

---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/iio/humidity/hdc100x.c +3 -2

index 2824578..a7f61e8 100644
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
 		}
 		break;
 	case IIO_CHAN_INFO_OFFSET:
-		*val = -40;
-		return IIO_VAL_INT;
+		*val = -3971;
+		*val2 = 879096;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}

Matt Ranostay

author

2015-09-27 22:35 UTC #2 lore raw

Quoted 5 lines · Jonathan Cameron

Quoted 4 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.

Quoted 7 lines

Quoted 1 line
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional value would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be cleaner and give the same answer?

Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)

Quoted 3 lines

Speaking of which, for the scale are we loosing any precision by shifting the bottom of the fraction right 2 rather than the top left 2 which would have the same result?

Ah possibly. But I suspect isn't anything measurable...

Quoted 24 lines

Jonathan

Quoted 21 lines

---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/iio/humidity/hdc100x.c +3 -2

index 2824578..a7f61e8 100644
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev,
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }

Jonathan Cameron

2015-09-28 09:08 UTC #3 lore raw

Quoted 11 lines · Matt Ranostay

Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional

Quoted 8 lines

value
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)

Good point. Can't we do (-40*165)/65536 ?

Quoted 35 lines

Quoted 2 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 7 lines
Jonathan
Review
Quoted 5 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }

Signature · 1 line

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Matt Ranostay

author

2015-09-29 04:19 UTC #4 lore raw

Quoted 15 lines · Jonathan Cameron

Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value

Quoted 8 lines

Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?

Nope.

scale = 165<<2 / 65536
offset = 40 / scale

Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?

Quoted 38 lines

Quoted 15 lines
Quoted 2 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Matt Ranostay

author

2015-09-29 06:21 UTC #5 lore raw

Would it just be clearer to use a IIO_CONST for the offset than using this kinda not clear IIO_CHAN_INFO_OFFSET hacking?

Quoted 73 lines · Matt Ranostay

Quoted 15 lines · Jonathan Cameron
Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?
Nope.
scale = 165<<2 / 65536
offset = 40 / scale
Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?
Quoted 38 lines
Quoted 15 lines
Quoted 2 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Jonathan Cameron

maintainer

2015-09-29 17:30 UTC #6 lore raw

Quoted 2 lines · Matt Ranostay

Would it just be clearer to use a IIO_CONST for the offset than using this kinda not clear IIO_CHAN_INFO_OFFSET hacking?

Clearer, but less functional. The IIO_CHAN_INFO stuff is available in kernel IIO_CONST attributes are not. That is why the fractional type is preferred. Other in kernel users can elect to do maths with it with minimal loss of precision whereas a floating point number is a PITA.

Over time the aim is to get rid of all the attributes in favour of supporting them through the core. All part of a drive to separate the front and backends of IIO so the backend can act as a more generic layer.

Quoted 80 lines · Matt Ranostay

Quoted 26 lines
Quoted 15 lines · Jonathan Cameron
Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?
Quoted 47 lines
Nope.
scale = 165<<2 / 65536
offset = 40 / scale
Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?
Quoted 38 lines
Quoted 15 lines
Quoted 2 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jonathan Cameron

maintainer

2015-09-29 17:33 UTC #7 lore raw

In reply to Matt Ranostay

Quoted 26 lines · Matt Ranostay

Quoted 15 lines · Jonathan Cameron
Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?

Quoted 7 lines

Nope.
scale = 165<<2 / 65536
offset = 40 / scale
Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?

Hmm. Would get messy fast.

Lets just go with your original fixed point option below. If someone later figures out a neater way of doing it, good for them ;)

Quoted 40 lines

Quoted 40 lines
Quoted 17 lines
Quoted 4 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Matt Ranostay

author

2015-10-05 05:57 UTC #8 lore raw

Quoted 32 lines · Jonathan Cameron

Quoted 31 lines · Matt Ranostay
Quoted 15 lines · Jonathan Cameron
Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?
Nope.
scale = 165<<2 / 65536
offset = 40 / scale

Quoted 6 lines

Quoted 2 lines
Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?
Hmm. Would get messy fast.
Lets just go with your original fixed point option below. If someone later figures out a neater way of doing it, good for them ;)

Ok that works.

Quoted 43 lines

J
Quoted 40 lines
Quoted 40 lines
Quoted 17 lines
Quoted 4 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Jonathan Cameron

maintainer

2015-10-11 12:53 UTC #9 lore raw

Quoted 33 lines · Matt Ranostay

Quoted 32 lines · Jonathan Cameron
Quoted 31 lines · Matt Ranostay
Quoted 15 lines · Jonathan Cameron
Quoted 12 lines · Matt Ranostay
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
Previous offset wasn't applied in the correct order and invalid. This patchset fixes this issue, and also has the correct scale value applied to the offset.
Signed-off-by Matt Ranostay <[email protected]>
Oops, missed that.
Given it's provided in the datasheet as effectively a fractional
value
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
would val = -40000, val2 = 65536 and type = IIO_FRACTIONAL not be
cleaner
Quoted 2 lines
and give the same answer?
Actually not because the way the datasheet states it ((value / 2**16)
* 165) - 40, so the scale value needs to be applied to the -40
offset (165/65536)
Good point. Can't we do (-40*165)/65536 ?
Nope.
scale = 165<<2 / 65536
offset = 40 / scale

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
Maybe we should add a new IIO_CHAN_INFO_SCALED_OFFSET that uses the scale value and a divisor that returns an IIO_FRACTIONAL?
Hmm. Would get messy fast.
Lets just go with your original fixed point option below. If someone later figures out a neater way of doing it, good for them ;)
Ok that works.

Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 48 lines

Quoted 43 lines
J
Quoted 40 lines
Quoted 40 lines
Quoted 17 lines
Quoted 4 lines
Speaking of which, for the scale are we loosing any precision by
shifting
Quoted 1 line
the bottom of the fraction right 2 rather than the top left 2 which
would have
Quoted 1 line
the same result?
Ah possibly. But I suspect isn't anything measurable...
Quoted 6 lines
Jonathan
Quoted 4 lines
---
 drivers/iio/humidity/hdc100x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 20 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/iio/humidity/hdc100x.c
Quoted 4 lines
drivers/iio/humidity/hdc100x.c static int hdc100x_read_raw(struct iio_dev
Quoted 4 lines
index 2824578..a7f61e8 100644
drivers/iio/humidity/hdc100x.c +0 -0
@@ -221,8 +221,9 @@ static int hdc100x_read_raw(struct iio_dev
*indio_dev,
Quoted 13 lines
Quoted 12 lines
drivers/iio/humidity/hdc100x.c +3 -2
              }
              break;
      case IIO_CHAN_INFO_OFFSET:
-             *val = -40;
-             return IIO_VAL_INT;
+             *val = -3971;
+             *val2 = 879096;
+             return IIO_VAL_INT_PLUS_MICRO;
      default:
              return -EINVAL;
      }
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html