On 4/3/21 4:58 PM, Puranjay Mohan wrote:
On Fri, Apr 2, 2021 at 1:43 PM Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
On 4/1/21 11:16 AM, Puranjay Mohan wrote:
TMP117 is a Digital temperature sensor with integrated NV memory.
Add support for tmp117 driver in iio subsystem.
Datasheet:-https://www.ti.com/lit/gpn/tmp117
Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx>
Nice and clean driver. Just some comments about the CALIBBIAS.
[...]
+#define TMP117_RESOLUTION_10UC 78125
Isn't the unit here 100 uC?
it is 7.8125 milli degrees_C so 78125 x 10^-4 milli degrees_C
which is 78125 x 10^-4 x 10^3 micro degrees_C
so it becomes 78125 x 10^-1 micro degrees_C = 78125 10_microdegrees_C.
Did it in detail so I remember it in the future. I guess you thought
it as 0.78125 millidegrees_C?
Ah, I get it, it is a tenth micro degree, not tens of micro degrees,
sorry got confused.
[...]
I think that would be quite unexpected behavior. The unit should be the
same. Here in the read function you can just return the register value.
Okay, if you feel that would be right then I will do it.
Yea, I think reading and writing in different units would be a bit
confusing.
Just make sure to properly sign extend like for the RAW property.
+ return IIO_VAL_INT_PLUS_MICRO;
[...]
+}
+
+static int tmp117_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int val,
+ int val2, long mask)
+{
+ struct tmp117_data *data = iio_priv(indio_dev);
+ s16 off;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBBIAS:
+ off = (s16)val;
This should have some input validation to avoid writing bogus values to
the register when the value is out of range. You can either reject out
of range values or clamp them into the valid range (using the clamp()
macro).
the maximum value which this register takes is 0xffff, so it should
get clamped automatically when casting to s16?
I might be wrong here.
Casting will truncate the upper bits. So something like 0x12345 gets
turned into 0x2345.
+ return i2c_smbus_write_word_swapped(data->client,
+ TMP117_REG_TEMP_OFFSET, off);
+
+ default:
+ return -EINVAL;
+ }
+}
+
[...]
