Add the correct scale to get temperature in mili degree Celcius. Add sign component to temperature scan element.
Signed-off-by: Sean Nyekjaer sean@geanix.com --- Changes in v2: - Correct offset is applied before scaling component - Added sign component to temperature scan element - Link to v1: https://lore.kernel.org/r/20250501-fxls-v1-1-f54061a07099@geanix.com
--- Sean Nyekjaer (2): iio: accel: fxls8962af: Fix temperature calculation iio: accel: fxls8962af: Fix sign temperature scan element
drivers/iio/accel/fxls8962af-core.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) --- base-commit: 609bc31eca06c7408e6860d8b46311ebe45c1fef change-id: 20250501-fxls-307ef3d6d065
Best regards,
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 = { \
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.
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
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
TEMP_OUT register contains the 8-bit, 2's complement temperature value. Let's mark the temperature scan element signed.
Fixes: a3e0b51884ee ("iio: accel: add support for FXLS8962AF/FXLS8964AF accelerometers") Suggested-by: Marcelo Schmitt marcelo.schmitt1@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Sean Nyekjaer sean@geanix.com --- drivers/iio/accel/fxls8962af-core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c index f515222e008493687921879a0b0ef44fd4ae5d10..e1b752e202b877db606a55a978d63ef52894c60d 100644 --- a/drivers/iio/accel/fxls8962af-core.c +++ b/drivers/iio/accel/fxls8962af-core.c @@ -750,6 +750,7 @@ static const struct iio_event_spec fxls8962af_event[] = { BIT(IIO_CHAN_INFO_OFFSET),\ .scan_index = -1, \ .scan_type = { \ + .sign = 's', \ .realbits = 8, \ .storagebits = 8, \ }, \
On 05/02, Sean Nyekjaer wrote:
TEMP_OUT register contains the 8-bit, 2's complement temperature value. Let's mark the temperature scan element signed.
Fixes: a3e0b51884ee ("iio: accel: add support for FXLS8962AF/FXLS8964AF accelerometers") Suggested-by: Marcelo Schmitt marcelo.schmitt1@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Sean Nyekjaer sean@geanix.com
Sort of nitpinking but I think the commit description could be more assertive. The main idea is that we want to make the scan element signed because the data read from the TEMP_OUT register is in two's complement format and not having the scan element marked as a signed number may cause it to be mishandled and miss displayed. Nevertheless, I do think the patch is good so
Reviewed-by: Marcelo Schmitt marcelo.schmitt1@gmail.com
On Sat, 3 May 2025 10:57:09 -0300 Marcelo Schmitt marcelo.schmitt1@gmail.com wrote:
On 05/02, Sean Nyekjaer wrote:
TEMP_OUT register contains the 8-bit, 2's complement temperature value. Let's mark the temperature scan element signed.
Fixes: a3e0b51884ee ("iio: accel: add support for FXLS8962AF/FXLS8964AF accelerometers") Suggested-by: Marcelo Schmitt marcelo.schmitt1@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Sean Nyekjaer sean@geanix.com
Sort of nitpinking but I think the commit description could be more assertive.
Agreed. I might have just tweaked it to "Mark the temperature element signed." + some of what Marcelo has below. but given Andy's request on patch 1 means you are probably doing a v3, please tidy this up as well.
Thanks and good catches on both of them!
Jonathan
The main idea is that we want to make the scan element signed because the data read from the TEMP_OUT register is in two's complement format and not having the scan element marked as a signed number may cause it to be mishandled and miss displayed. Nevertheless, I do think the patch is good so
Reviewed-by: Marcelo Schmitt marcelo.schmitt1@gmail.com
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Jonathan Cameron -
Marcelo Schmitt -
Sean Nyekjaer