lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 21 Mar 2021 06:23:08 +0100 From: Lars-Peter Clausen <lars@...afoo.de> To: Puranjay Mohan <puranjay12@...il.com>, alexandru.ardelean@...log.com, jic23@...nel.org, devicetree@...r.kernel.org, knaack.h@....de, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v1 2/2] iio: temperature: add driver support for ti tmp117 On 3/21/21 6:07 AM, Lars-Peter Clausen wrote: > On 3/20/21 7:45 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@...il.com> > > This looks good to me. Just two small bits I overlooked during the > first review, sorry for that. > >> +}; >> + >> [...] >> +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.
Powered by blists - more mailing lists