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.