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

[Chris Morgan]

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 at gmail.com> wrote:
> 
> > From: Chris Morgan <macromorgan at hotmail.com>
> > 
> > Add gyroscope functions to the icm42607 driver.
> > 
> > Signed-off-by: Chris Morgan <macromorgan at 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...

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.

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...

> 
> 
> >  
> >  	ret = inv_icm42607_buffer_fifo_read(st, count);
> >  	if (ret)
> 
> 
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> > new file mode 100644
> > index 000000000000..8d59156086b1
> > --- /dev/null
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> 
> > +
> > +static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
> > +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
> > +	{ },
> No comma. Check for any other commas after terminating entries like this one.
> > +};
> 
> 
> > +
> > +int inv_icm42607_gyro_parse_fifo(struct iio_dev *indio_dev)
> > +{
> > +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> > +	struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
> > +	struct inv_sensors_timestamp *ts = &gyro_st->ts;
> > +	ssize_t i, size;
> > +	unsigned int no;
> > +	const void *accel, *gyro, *timestamp;
> > +	const s8 *temp;
> > +	unsigned int odr;
> > +	s64 ts_val;
> > +	struct inv_icm42607_gyro_buffer buffer = { };
> > +
> > +	guard(mutex)(&st->lock);
> Sashiko thinks there is a deadlock here as this lock will already
> be held.  Report looks correct to me + same bug in the accelerometer case.
> 
> I'm surprised you didn't see them in testing as deadlocks tend to be obvious!
> 

Yeah... about that... turns out my IRQ wasn't firing correctly so I was never
hitting this path. I suspect the line isn't even hooked up!

> 
> > +
> > +	/* parse all fifo packets */
> > +	for (i = 0, no = 0; i < st->fifo.count; i += size, ++no) {
> > +		size = inv_icm42607_fifo_decode_packet(&st->fifo.data[i],
> > +				&accel, &gyro, &temp, &timestamp, &odr);
> > +		/* quit if error or FIFO is empty */
> > +		if (size <= 0)
> > +			return size;
> > +
> > +		/* If the packet size could cause us to overflow, return. */
> > +		if (i + size > st->fifo.count)
> > +			return -EIO;
> > +
> > +		/* skip packet if no gyro data or data is invalid */
> > +		if (gyro == NULL || !inv_icm42607_fifo_is_data_valid(gyro))
> > +			continue;
> > +
> > +		/* update odr */
> > +		if (odr & INV_ICM42607_SENSOR_GYRO)
> > +			inv_sensors_timestamp_apply_odr(ts, st->fifo.period,
> > +							st->fifo.nb.total, no);
> > +
> > +		memcpy(&buffer.gyro, gyro, sizeof(buffer.gyro));
> > +		/* convert 8 bits FIFO temperature in high resolution format */
> > +		buffer.temp = temp ? (*temp * 64) : 0;
> > +		ts_val = inv_sensors_timestamp_pop(ts);
> > +		iio_push_to_buffers_with_ts(indio_dev, &buffer,
> > +					    sizeof(buffer), ts_val);
> > +	}
> > +
> > +	return 0;
> > +}
> 

Thank you again for all your help.