Re: [PATCH V9 10/11] iio: imu: inv_icm42607: Add Gyroscope to icm42607

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 1 Jun 2026 09:37:56 -0500
Chris Morgan <macromorgan@hotmail.com> wrote:

> On Sun, May 31, 2026 at 01:58:23PM +0100, Jonathan Cameron wrote:
> > On Fri, 29 May 2026 22:17:37 -0500
> > Chris Morgan <macroalpha82@gmail.com> wrote:
> >   
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add gyroscope functions to the icm42607 driver.
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
> > 
> > Hi Chris,
> > Various things inline.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c
> > > index 5b69bf895b35..c45239613344 100644
> > > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c
> > > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c  
> >   
> > >  int inv_icm42607_buffer_fifo_parse(struct inv_icm42607_state *st)
> > >  {
> > > +	struct inv_icm42607_sensor_state *gyro_st = iio_priv(st->indio_gyro);
> > >  	struct inv_icm42607_sensor_state *accel_st = iio_priv(st->indio_accel);
> > >  	struct inv_sensors_timestamp *ts;
> > >  	int ret;
> > > @@ -491,6 +494,16 @@ int inv_icm42607_buffer_fifo_parse(struct inv_icm42607_state *st)
> > >  	if (st->fifo.nb.total == 0)
> > >  		return 0;
> > >  
> > > +	/* handle gyroscope timestamp and FIFO data parsing */
> > > +	if (st->fifo.nb.gyro > 0) {
> > > +		ts = &gyro_st->ts;
> > > +		inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_gyro,
> > > +						st->timestamp.gyro);
> > > +		ret = inv_icm42607_gyro_parse_fifo(st->indio_gyro);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > >  	/* handle accelerometer timestamp and FIFO data parsing */
> > >  	if (st->fifo.nb.accel > 0) {
> > >  		ts = &accel_st->ts;
> > > @@ -507,12 +520,14 @@ int inv_icm42607_buffer_fifo_parse(struct inv_icm42607_state *st)
> > >  int inv_icm42607_buffer_hwfifo_flush(struct inv_icm42607_state *st,
> > >  				     unsigned int count)
> > >  {
> > > +	struct inv_icm42607_sensor_state *gyro_st = iio_priv(st->indio_gyro);
> > >  	struct inv_icm42607_sensor_state *accel_st = iio_priv(st->indio_accel);
> > >  	struct inv_sensors_timestamp *ts;
> > > -	s64 accel_ts;
> > > +	s64 gyro_ts, accel_ts;
> > >  	int ret;
> > >  
> > >  	accel_ts = iio_get_time_ns(st->indio_accel);
> > > +	gyro_ts = iio_get_time_ns(st->indio_gyro);  
> > 
> > Sashiko calls out correctly that there is a race with devices finishing
> > coming up that might be hit here.  Probably need some sort of presence
> > check and locking to be sure those are both valid.
> >   
> 
> While I'm not sure how to fix this exactly, I think (attempting to)
> test deadlocks I found a bigger issue... I don't think my interrupt
> line is hooked up at all...
ouch.  Wouldn't be the first time a given device has saved on pins
to the extent of not wiring any interrupts.

> 
> Assuming I should be getting interrupts when enabling the buffer
> (and also assuming I can do that with sysfs) I'm not getting any calls
> to the IRQ routine.
> 
> Would you know a better way to test the hardware buffers? Thus far
> my tests for data correctness involved either `monitor-sensor -a`
> or reading the values directly from sysfs.

There is a basic buffer example in the kernel tree under tools/iio
that should work for this. iio_generic_buffer 

> 
> Assuming I am in fact working with a device with no interrupt line, I
> can just modify this to remove the hardware buffer stuff and the IRQ
> stuff and work with an even more simplified driver. Probably going to
> miss the 7.2 merge window at this time, but still...

Indeed an option to do this as a temporary step. If we get
other users hopefully someone has the interrupt line wired!

Jonathan