[PATCH V11 3/9] iio: imu: inv_icm42607: Add inv_icm42607 Core Driver

Chris Morgan macromorgan at hotmail.com
Thu Jun 11 07:28:55 PDT 2026


On Thu, Jun 11, 2026 at 10:35:49AM +0300, Andy Shevchenko wrote:
> On Wed, Jun 10, 2026 at 12:54:47PM -0500, Chris Morgan wrote:
> 
> > Add the core component of a new inv_icm42607 driver. This includes
> > a few setup functions and the full register definition in the
> > header file.
> 
> ...
> 
> >  2 files changed, 531 insertions(+)
> >  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h
> >  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> 
> I'm not sure this is correct split by files. These files are not:
> - being build
> - being mentioned in MAINTAINERS

I can merge patches v3 and v4. Early on they were pretty big but now
that they've been paired down some I think it may be manageable.

Early on I was told to try and keep each patch around ~750 lines.
I can add these to together and get it to about ~850 lines, but
it will be compileable at that commit rather than waiting on a
second one.

> 
> ...
> 
> > +#ifndef INV_ICM42607_H_
> > +#define INV_ICM42607_H_
> > +
> > +#include <linux/bits.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/mutex.h>
> 
> mutex_types.h ?
> 

I'm not sure I understand this, can you elaborate why I'd need it?

> > +#include <linux/regmap.h>
> > +#include <linux/types.h>
> 
> Do you need forward declaration for regulator data type?
> 

I think earlier I had regulator-consumer and stripped it out, but I
might have cut a bit too deep so I can add that back.

> ...
> 
> > +#define INV_ICM42607_POWER_UP_TIME_US			100000
> 
> 100 * USEC_PER_MSEC (will require time.h)?
> 

Understood.

> ...
> 
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> 
> + bitfields.h
> 

Understood.

> > +#include <linux/delay.h>
> > +#include <linux/dev_printk.h>
> 
> + device/devres.h
> + err.h
> 

Understood.

> > +#include <linux/iio/iio.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/time.h>
> 
> Also check if you need types.h.
> 

I'll add it, I'm honestly not sure. I never got iwyu working it seems.

> ...
> 
> > +	fsleep(INV_ICM42607_RESET_TIME_MS * USEC_PER_MSEC);
> > +
> > +	/*
> > +	 * No polling interval specified in datasheet, so use reset time as
> > +	 * polling interval and 10x reset time as timeout period.
> > +	 */
> > +	ret = regmap_read_poll_timeout(st->map, INV_ICM42607_REG_INT_STATUS,
> > +				       val, val & INV_ICM42607_INT_STATUS_RESET_DONE,
> > +				       (INV_ICM42607_RESET_TIME_MS * USEC_PER_MSEC),
> > +				       (INV_ICM42607_RESET_TIME_MS * USEC_PER_MSEC * 10));
> 
> Besides too many parentheses, this can be switched to regular patter of "num * what".
> Also, TBH, the plain values would be better here

I'll switch to plain numbers and let the comment above do the explaining.

> 
> 	fsleep(1 * USEC_PER_MSEC);
> 
> 	/*
> 	 * No polling interval specified in datasheet, so use reset time as
> 	 * polling interval and 10x reset time as timeout period.
> 	 */
> 	ret = regmap_read_poll_timeout(st->map, INV_ICM42607_REG_INT_STATUS,
> 				       val, val & INV_ICM42607_INT_STATUS_RESET_DONE,
> 				       1 * USEC_PER_MSEC, 10 * USEC_PER_MSEC);
> 
> And in the similar way in other fsleep() / _read_poll_timeout() cases.
> 

Plain numbers, gotcha.

> > +	if (ret)
> > +		return dev_err_probe(dev, ret,
> > +				     "reset error, reset done bit not set\n");
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Thank you,
Chris



More information about the Linux-rockchip mailing list