Thread (8 messages) 8 messages, 4 authors, 2021-03-22

Re: [PATCH v1 2/2] iio: temperature: add driver support for ti tmp117

From: Lars-Peter Clausen <lars@metafoo.de>
Date: 2021-03-21 05:34:59
Also in: linux-iio, lkml

On 3/21/21 6:07 AM, Lars-Peter Clausen wrote:
On 3/20/21 7:45 AM, Puranjay Mohan wrote:
quoted
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 <redacted>
This looks good to me. Just two small bits I overlooked during the 
first review, sorry for that.
quoted
+};
+
[...]
+static int tmp117_read_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);
+    u16 tmp, off;
+
+    switch (mask) {
+    case IIO_CHAN_INFO_RAW:
+        tmp = tmp117_read_reg(data, TMP117_REG_TEMP);
+        *val = tmp;
No need for tmp here. Just directly assign to val.
Actually thinking about this, does the current implementation work 
correctly for negative temperatures?

I think there are two options to fix it. Either cast to s16 or use the 
sign_extend32() function.

Have a look at how the tmp006 driver handles this. It is a good example, 
including the error checking. In general you should check if your I2C 
read failed and return an error in that case rather than a bogus value 
for the measurement. Same for when reading the calibration offset.

Another thing. IIO reports temperature values in milli degrees Celsius. 
I believe in the current implementation the scale is so that it will 
report in degrees Celsius instead.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help