[PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation

Sean Nyekjaer posted 2 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation
Posted by Sean Nyekjaer 9 months, 1 week ago
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@vger.kernel.org
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/iio/accel/fxls8962af-core.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index bf1d3923a181798a1c884ee08b62d86ab5aed26f..f515222e008493687921879a0b0ef44fd4ae5d10 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -134,6 +134,8 @@
 
 /* Raw temp channel offset */
 #define FXLS8962AF_TEMP_CENTER_VAL		25
+/* Raw temp channel scale */
+#define FXLS8962AF_TEMP_SCALE			1000
 
 #define FXLS8962AF_AUTO_SUSPEND_DELAY_MS	2000
 
@@ -439,8 +441,16 @@ static int fxls8962af_read_raw(struct iio_dev *indio_dev,
 		*val = FXLS8962AF_TEMP_CENTER_VAL;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-		return fxls8962af_read_full_scale(data, val2);
+		switch (chan->type) {
+		case IIO_TEMP:
+			*val = FXLS8962AF_TEMP_SCALE;
+			return IIO_VAL_INT;
+		case IIO_ACCEL:
+			*val = 0;
+			return fxls8962af_read_full_scale(data, val2);
+		default:
+			return -EINVAL;
+		}
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return fxls8962af_read_samp_freq(data, val, val2);
 	default:
@@ -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 = { \

-- 
2.47.1
Re: [PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation
Posted by Marcelo Schmitt 9 months, 1 week ago
On 05/02, 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@vger.kernel.org
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Re: [PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation
Posted by Andy Shevchenko 9 months, 1 week ago
On Fri, May 2, 2025 at 9:15 AM Sean Nyekjaer <sean@geanix.com> wrote:
>
> According to spec temperature should be returned in milli degrees Celsius.
> Add in_temp_scale to calculate from Celsius to milli Celsius.

...

> +/* Raw temp channel scale */
> +#define FXLS8962AF_TEMP_SCALE                  1000

Wouldn't constants from units.h be helpful here?

>  #define FXLS8962AF_AUTO_SUSPEND_DELAY_MS       2000

(2 * MSEC_PER_SEC)

This gives immediately that we want 2 seconds of delay.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/2] iio: accel: fxls8962af: Fix temperature calculation
Posted by Jonathan Cameron 9 months, 1 week ago
On Fri, 2 May 2025 17:19:44 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, May 2, 2025 at 9:15 AM Sean Nyekjaer <sean@geanix.com> wrote:
> >
> > According to spec temperature should be returned in milli degrees Celsius.
> > Add in_temp_scale to calculate from Celsius to milli Celsius.  
> 
> ...
> 
> > +/* Raw temp channel scale */
> > +#define FXLS8962AF_TEMP_SCALE                  1000  
> 
> Wouldn't constants from units.h be helpful here?

Whilst you are just continuing local style, I'm not sure
these defines really bring much at all given the TEMP_SCALE
one for instance is only used in one place which is
explicitly getting the temperature scale.  It's not a magic
number that needs a define. It's a number that means it's own
value :)

Using MILLI there would be a nice bit of self documentation
though.
> 
> >  #define FXLS8962AF_AUTO_SUSPEND_DELAY_MS       2000  
> 
> (2 * MSEC_PER_SEC)
> 
> This gives immediately that we want 2 seconds of delay.
> 
True but not part of this patch so that would be a nice
little follow up.  Possibly dropping this define in favour
of using that inline.

Jonathan