[PATCH] iio: imu: inv_icm42600: fix temperature reading if accel/gyro is off

Sean Nyekjaer posted 1 patch 3 months ago
drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
[PATCH] iio: imu: inv_icm42600: fix temperature reading if accel/gyro is off
Posted by Sean Nyekjaer 3 months ago
Avoid return invalid argument if one tries to read the temperature,.
if both the accelerometer and gyro are off. Power the accelerometer on
before reading the temperature.
The original state will be restored by runtine_suspend() or the next
reading of the accelerometer.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index a4d42e7e21807f7954def431e9cf03dffaa5bd5e..f97376bc8bb3dd225236e3f5036fd58af4af35ac 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -399,9 +399,14 @@ int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
 int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
 			       unsigned int *sleep_ms)
 {
-	return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode,
-					  st->conf.accel.mode, enable,
-					  sleep_ms);
+	enum inv_icm42600_sensor_mode accel = st->conf.accel.mode;
+
+	if (st->conf.gyro.mode == INV_ICM42600_SENSOR_MODE_OFF &&
+	    st->conf.accel.mode == INV_ICM42600_SENSOR_MODE_OFF)
+		accel = INV_ICM42600_SENSOR_MODE_LOW_POWER;
+
+	return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, accel,
+					  enable, sleep_ms);
 }
 
 int inv_icm42600_enable_wom(struct inv_icm42600_state *st)

---
base-commit: 3e28fa06444e7031aba0b3552cce332b776fe267
change-id: 20250708-icm42temp-6292abddb6e6

Best regards,
-- 
Sean Nyekjaer <sean@geanix.com>
Re: [PATCH] iio: imu: inv_icm42600: fix temperature reading if accel/gyro is off
Posted by Jonathan Cameron 2 months, 3 weeks ago
On Tue, 08 Jul 2025 14:09:17 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> Avoid return invalid argument if one tries to read the temperature,.
> if both the accelerometer and gyro are off. Power the accelerometer on
> before reading the temperature.
> The original state will be restored by runtine_suspend() or the next
> reading of the accelerometer.
I'm not sure we are going ahead with this anyway given the other thread,
but if we did, then relying on runtime_put() in read of a different
channel seems like a bad design. If we think someone is interested
in reading the temperature of this sensor, are they guaranteed to
also be reading the acceleration?

> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
>  drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index a4d42e7e21807f7954def431e9cf03dffaa5bd5e..f97376bc8bb3dd225236e3f5036fd58af4af35ac 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -399,9 +399,14 @@ int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
>  int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
>  			       unsigned int *sleep_ms)
>  {
> -	return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode,
> -					  st->conf.accel.mode, enable,
> -					  sleep_ms);
> +	enum inv_icm42600_sensor_mode accel = st->conf.accel.mode;
> +
> +	if (st->conf.gyro.mode == INV_ICM42600_SENSOR_MODE_OFF &&
> +	    st->conf.accel.mode == INV_ICM42600_SENSOR_MODE_OFF)
> +		accel = INV_ICM42600_SENSOR_MODE_LOW_POWER;
> +
> +	return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, accel,
> +					  enable, sleep_ms);
>  }
>  
>  int inv_icm42600_enable_wom(struct inv_icm42600_state *st)
> 
> ---
> base-commit: 3e28fa06444e7031aba0b3552cce332b776fe267
> change-id: 20250708-icm42temp-6292abddb6e6
> 
> Best regards,
Re: [PATCH] iio: imu: inv_icm42600: fix temperature reading if accel/gyro is off
Posted by Andy Shevchenko 3 months ago
On Tue, Jul 08, 2025 at 02:09:17PM +0200, Sean Nyekjaer wrote:
> Avoid return invalid argument if one tries to read the temperature,.

returning

(Stray period at the end)

> if both the accelerometer and gyro are off. Power the accelerometer on
> before reading the temperature.
> The original state will be restored by runtine_suspend() or the next
> reading of the accelerometer.

Why don't you use the room on the previous lines, the formatting looks ugly.

...

Does it need a Fixes tag?

...

Code wise LGTM, though.

-- 
With Best Regards,
Andy Shevchenko