Re: [BUG] iio: imu: inv_icm42600: read temperature, Invalid Argument

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 14 Jul 2025 05:29:00 +0000
Sean Nyekjaer <sean@geanix.com> wrote:

> On Sun, Jul 13, 2025 at 02:46:32PM +0100, Jonathan Cameron wrote:
> > On Tue, 8 Jul 2025 12:21:39 +0000
> > Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:
> >   
> > > >
> > > >________________________________________
> > > >From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > >Sent: Tuesday, July 8, 2025 12:35
> > > >To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
> > > >Cc: Sean Nyekjaer <sean@geanix.com>; Jonathan Cameron <jic23@kernel.org>; David Lechner <dlechner@baylibre.com>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Andy Shevchenko <andy@kernel.org>
> > > >Subject: Re: [BUG] iio: imu: inv_icm42600: read temperature, Invalid Argument
> > > > 
> > > >This Message Is From an External Sender
> > > >This message came from outside your organization.
> > > > 
> > > >On Mon, 7 Jul 2025 18:13:33 +0000
> > > >Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:
> > > >  
> > > >> >Hi,
> > > >> >
> > > >> >I'm having some weird issues with reading the temperature of the icm42605.
> > > >> >Kernel version: 6.16.0-rc5
> > > >> >
> > > >> ># cat /sys/bus/iio/devices/iio:device2/name
> > > >> >icm42605-accel
> > > >> >
> > > >> >When reading the temperature I get:
> > > >> ># cat /sys/bus/iio/devices/iio:device2/in_temp_raw
> > > >> >cat: read error: Invalid argument
> > > >> >
> > > >> >But if I read from the accelerometer first, I will go better:
> > > >> ># cat /sys/bus/iio/devices/iio:device2/in_accel_x_raw
> > > >> >-378
> > > >> ># cat /sys/bus/iio/devices/iio:device2/in_temp_raw
> > > >> >600
> > > >> >
> > > >> >Then after "some" time, I re-read the temperature I recieve
> > > >> >"Invalid argument" again.
> > > >> >
> > > >> >I have traced the -EINVAL to inv_icm42600_temp_read() because I receive
> > > >> >INV_ICM42600_DATA_INVALID.
> > > >> >
> > > >> >Register dump diff:
> > > >> >--- invalid-read
> > > >> >+++ ok-read
> > > >> >@@ -10,14 +10,14 @@
> > > >> > 0x1a = 0x00000010
> > > >> > 0x1b = 0x00000000
> > > >> > 0x1c = 0x00000000
> > > >> >-0x1d = 0x00000080
> > > >> >-0x1e = 0x00000000
> > > >> >-0x1f = 0x00000080
> > > >> >-0x20 = 0x00000000
> > > >> >-0x21 = 0x00000080
> > > >> >-0x22 = 0x00000000
> > > >> >-0x23 = 0x00000080
> > > >> >-0x24 = 0x00000000
> > > >> >+0x1d = 0x00000002
> > > >> >+0x1e = 0x00000038
> > > >> >+0x1f = 0x000000fe
> > > >> >+0x20 = 0x00000085
> > > >> >+0x21 = 0x000000fe
> > > >> >+0x22 = 0x000000df
> > > >> >+0x23 = 0x000000f8
> > > >> >+0x24 = 0x0000003a
> > > >> > 0x25 = 0x00000080
> > > >> > 0x26 = 0x00000000
> > > >> > 0x27 = 0x00000080
> > > >> >@@ -26,7 +26,7 @@
> > > >> > 0x2a = 0x00000000
> > > >> > 0x2b = 0x00000000
> > > >> > 0x2c = 0x00000000
> > > >> >-0x2d = 0x00000000
> > > >> >+0x2d = 0x00000008
> > > >> > 0x2e = 0x00000000
> > > >> > 0x2f = 0x00000000
> > > >> > 0x30 = 0x000000ff
> > > >> >@@ -59,11 +59,11 @@
> > > >> > 0x4b = 0x00000000
> > > >> > 0x4c = 0x00000032
> > > >> > 0x4d = 0x00000099
> > > >> >-0x4e = 0x00000000
> > > >> >+0x4e = 0x00000002
> > > >> > 0x4f = 0x00000009
> > > >> > 0x50 = 0x00000009
> > > >> > 0x51 = 0x00000016
> > > >> >-0x52 = 0x00000000
> > > >> >+0x52 = 0x00000060
> > > >> > 0x53 = 0x0000000d
> > > >> > 0x54 = 0x00000031
> > > >> > 0x55 = 0x00000000
> > > >> >
> > > >> >Will the iio core retry a read? If the -EINVAL is returned,
> > > >> >inv_icm42600_accel_read_raw() is call once more.
> > > >> >
> > > >> >One more thing...
> > > >> >When I'm removing the module, the kernel prints this:
> > > >> >inv-icm42600-i2c 1-0068: Runtime PM usage count underflow!
> > > >> >
> > > >> >I will continue investigate this
> > > >> >Br,
> > > >> >Sean  
> > > >>
> > > >> Hello Sean,
> > > >>
> > > >> this is expected behavior since the temperature returned is not the external
> > > >> temperature but the temperature of the mechanical component (MEMS). It will
> > > >> only work if the chip is on, meaning accelerometer and/or gyroscope is on.
> > > >>
> > > >> That's why you can get temperature after reading accel data since it is
> > > >> turning the chip. But after a short while autosuspend is putting the chip
> > > >> back off and you cannot read temperature anymore.
> > > >>
> > > >> You need to turn one sensor continuously on with a buffer, and then you can
> > > >> read temperature all the time since the chip is running.
> > > >>
> > > >> Temperature data are here only to do temperature compensation of the accel
> > > >> and gyro data.  
> > > >
> > > >That is rather non-intuitive behavior.  Could we make a read of the temperature
> > > >channel turn on one of the components?  Given expected use case it shouldn't commonly
> > > >happen but if not too horrendous to implement it would be better to avoid the error
> > > >seen here.  
> > >
> > > Hello Jonathan,
> > >
> > > the problem here is which sensor to turn on? Accel or gyro, or accel+gyro?
> > > And the temperature reported will be completely different if it is accel
> > > and/or gyro running, since gyro is heating much more than accel.
> > >
> > > This is not a classical temperature sensor, but an internal one for measuring
> > > temperature of the mechanical part while running. The usual use case on our
> > > side is polling the temperature at low frequency (10Hz, 20Hz) while accel
> > > and/or gyro are running to do temperature compensation on the data read.
> > >
> > > We generally want to avoid temperature data in the FIFO because reporting
> > > temperature at high frequency is not useful, and it is consuming space in
> > > the FIFO. We prefer to have more space in the FIFO for accel and gyro data
> > > and do polling of temperature.
> > >
> > > Temperature reporting while the chip is off makes absolutely no sense that's
> > > why it is not supported by the chip, even if it can be non-intuitive.
> > >
> > > Perhaps we can use another error returning code rather than invalid value?
> > > Otherwise, tell me what you think is the best to do.  
> > 
> > Maybe indicate it is a temporary situation that userspace can resolve
> > by reporting -EBUSY?  I'm not sure what the best path forward here is.
> > 
> > Your explanation seems reasonable to me.  Sean, do you still believe
> > we need to make a change here?  
> 
> I think -EBUSY would be better, returning -EINVAL required me to read the driver,
> and apply some debug printk's to see where it originated from.
> 
> Returning -EBUSY and add a comment about the "issue" in
> inv_icm42600_temp_read(), would be a help.

Anyone want to spin a patch?  I'm marking this thread closed in patchwork
in the meantime.

Thanks,

Jonathan

> 
> /Sean
> 
> > 
> > Jonathan
> >   
> > >
> > > Thanks,
> > > JB
> > >  
> > > >  
> > > >>
> > > >> Thanks,
> > > >> JB  
> > > >
> > > >  
> >   
>