Skip to contentSkip to conversation

iio: gyro: mpu3050: Fix reported temperature value

PATCH Superseded Dmitry Osipenko 2 participants 4 messages Apr 21, 2021 – Apr 22, 2021 1 message not in the archive

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 iio: gyro: mpu3050: Fix reported temperature value

The raw temperature value is a signed 16bit integer. The sign casting is missed in the code, which results in a wrong temperature reported by userspace tools, fix it.

---
 drivers/iio/gyro/mpu3050-core.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/iio/gyro/mpu3050-core.c +11 -2
index ac90be03332a..9885428ca12e 100644
@@ -272,7 +272,16 @@ static int mpu3050_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_OFFSET:
 		switch (chan->type) {
 		case IIO_TEMP:
-			/* The temperature scaling is (x+23000)/280 Celsius */
+			/*
+			 * The temperature scaling is (x+23000)/280 Celsius,
+			 * where 23000 includes room temperature offset of
+			 * +35C, 280 is the precision scale and x is the signed
+			 * 16bit integer which corresponds to the temperature
+			 * range of -40C..85C.
+			 *
+			 * Temperature value itself represents temperature of
+			 * the sensor die.
+			 */
 			*val = 23000;
 			return IIO_VAL_INT;
 		default:
@@ -329,7 +338,7 @@ static int mpu3050_read_raw(struct iio_dev *indio_dev,
 				goto out_read_raw_unlock;
 			}
 
-			*val = be16_to_cpu(raw_val);
+			*val = (s16)be16_to_cpu(raw_val);
 			ret = IIO_VAL_INT;
 
 			goto out_read_raw_unlock;
Signature · 1 line
2.30.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Dmitry Osipenko

The raw temperature value is a signed 16bit integer. The sign casting is missed in the code, which results in a wrong temperature reported by userspace tools, fix it.

+/- Andy's comments:

I never thought this driver would have so many users (3 people signed testing it!) but I realize it is more widely deployed than I thought.

I have totally ignored the MPU3050's ability to act as a "sensor hub" and talk to accelerometers and magnetometers directly. I always thought it would be better to just route the I2C right through it and put Linux in direct control, but I realize this was not Invensese's intention. I don't know if it can be actually utilized in some generic way, all kernels using that have separate hacky drivers for all the sub-sensors duplicating the kernel drivers we already have ...

Yours,
Linus Walleij

#2 lore raw

22.04.2021 03:38, Linus Walleij пишет:

Quoted 19 lines · Dmitry Osipenko
Quoted 11 lines

The raw temperature value is a signed 16bit integer. The sign casting is missed in the code, which results in a wrong temperature reported by userspace tools, fix it.

+/- Andy's comments:

I never thought this driver would have so many users (3 people signed testing it!) but I realize it is more widely deployed than I thought.

Quoted 7 lines

I have totally ignored the MPU3050's ability to act as a "sensor hub" and talk to accelerometers and magnetometers directly. I always thought it would be better to just route the I2C right through it and put Linux in direct control, but I realize this was not Invensese's intention. I don't know if it can be actually utilized in some generic way, all kernels using that have separate hacky drivers for all the sub-sensors duplicating the kernel drivers we already have ...

I don't think that MPU3050 could talk to the sensors behind it directly. It's has "I2C gate" which allows to route the I2C communication to the chained sensors, which is done in order to reduce noise on the I2C bus such that only one sensor is active at a time. Those chained sensors are working good using upstream kernel drivers, the accelerometer is particularly useful for display autorotation. Modern DEs like Gnome and KDE are using iio-sensor-proxy library that knows how to work with the mainline sensor drivers.

Thank you and Andy for the review, I'll prepare v2.

A message that is not in the archive (sent off-list or not yet received): CAHp75VcuU_=OxdFo=9cxDtguCTjQ4jQytFzHUb3WoxJgcwDFxA@mail.gmail.com

#4 lore raw

22.04.2021 03:34, Andy Shevchenko пишет:

Quoted 13 lines

-                       /* The temperature scaling is (x+23000)/280 Celsius */
+                       /*
+                        * The temperature scaling is (x+23000)/280 Celsius,
+                        * where 23000 includes room temperature offset of
+                        * +35C, 280 is the precision scale and x is the signed
+                        * 16bit integer which corresponds to the temperature

Quoted 8 lines

“...16-bit signed integer...”

+                        * range of -40C..85C.

Datasheet says -30°C..+85°C.

Datasheet says this for the "Best fit straight line", while the min value is -40C.