linux-kernel - Re: [PATCH] iio: accel: fxls8962af: Fix temperature calculation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4i3cracjtbjc5qjfa27d6mytoy27hiaduaftwmpr3zuyipc6jq@mwou3lp6pmue>
Date: Fri, 02 May 2025 05:34:23 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Andy Shevchenko <andy.shevchenko@...il.com>, Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] iio: accel: fxls8962af: Fix temperature calculation

Hi Marcelo,

On Thu, May 01, 2025 at 04:13:29PM +0100, Marcelo Schmitt wrote:
> Hello Sean,
> 
> Overall, this looks good to me.
> Only thing I think need to (not) change is the offset (see comment below).
> 
> Regards,
> Marcelo
> 
> On 05/01, Sean Nyekjaer wrote:
> > According to spec temperature should be returned in milli degrees Celsius.
> > Add in_temp_scale to calculate from Celsius to milli Celsius.
> >
> > Fixes: a3e0b51884ee ("iio: accel: add support for FXLS8962AF/FXLS8964AF accelerometers")
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Sean Nyekjaer <sean@...nix.com>
> > ---
> >  drivers/iio/accel/fxls8962af-core.c | 17 ++++++++++++++---
> ...
> >
> >  /* Raw temp channel offset */
> > -#define FXLS8962AF_TEMP_CENTER_VAL		25
> > +#define FXLS8962AF_TEMP_CENTER_VAL		25000
> I think the offset was correct the way it was before.
> For example, if the sensor at 37°C, the output code will be 0b00001100, 12 (decimal).
> Then (_raw + _offset) * _scale = (12 + 25) * 1000 = 37000 milli °C

Yes you are right. Just checked the documentation.
"If known for a device, offset to be added to <type>[Y]_raw prior to
scaling by <type>[Y]_scale"

> 
> > +/* Raw temp channel scale */
> > +#define FXLS8962AF_TEMP_SCALE			1000
> >
> ...
> > @@ -736,6 +746,7 @@ static const struct iio_event_spec fxls8962af_event[] = {
> >  	.type = IIO_TEMP, \
> >  	.address = FXLS8962AF_TEMP_OUT, \
> >  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > +			      BIT(IIO_CHAN_INFO_SCALE) | \
> >  			      BIT(IIO_CHAN_INFO_OFFSET),\
> >  	.scan_index = -1, \
> >  	.scan_type = { \
> Datasheet page 39 says 'the TEMP_OUT register contains the 8-bit, 2's complement
> temperature value.'
> So I think we can make the temperature scan element signed.
> 		.sign = 's',	\
> 
> Though, I guess that would be a thing for a separate patch.

Good catch will do some testing :)

Thanks
/Sean