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

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 11 Jun 2026 09:32:38 -0500
Chris Morgan <macromorgan@hotmail.com> wrote:

> On Thu, Jun 11, 2026 at 12:09:43PM +0100, Jonathan Cameron wrote:
> > On Wed, 10 Jun 2026 12:54:47 -0500
> > Chris Morgan <macroalpha82@gmail.com> wrote:
> >   
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
> > Hi Chris,
> > 
> > A couple more minor things from a fresh read.
> > 
> > Thanks
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/iio/imu/inv_icm42607/inv_icm42607.h   | 334 ++++++++++++++++++
> > >  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 197 +++++++++++
> > >  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
> > > 
> > > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > > new file mode 100644
> > > index 000000000000..716fc0f1c3fd
> > > --- /dev/null
> > > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h  
> >   
> > > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > > new file mode 100644
> > > index 000000000000..334264120b42
> > > --- /dev/null
> > > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c  
> >   
> > > +
> > > +static int inv_icm42607_setup(struct inv_icm42607_state *st,
> > > +			      inv_icm42607_bus_setup inv_icm42607_bus_setup)
> > > +{
> > > +	const struct device *dev = regmap_get_device(st->map);
> > > +	unsigned int val;
> > > +	int ret;
> > > +
> > > +	ret = regmap_read(st->map, INV_ICM42607_REG_WHOAMI, &val);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Warn, but don't fail. */
> > > +	if (val != st->hw->whoami)
> > > +		dev_warn(dev, "Unknown whoami %#02x expected %#02x (%s)\n",
> > > +			 val, st->hw->whoami, st->hw->name);
> > > +
> > > +	ret = regmap_write(st->map, INV_ICM42607_REG_SIGNAL_PATH_RESET,
> > > +			   INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	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));
> > > +	if (ret)
> > > +		return dev_err_probe(dev, ret,
> > > +				     "reset error, reset done bit not set\n");
> > > +
> > > +	/* Sync the regcache again after a reset. */
> > > +	regcache_mark_dirty(st->map);
> > > +	ret = regcache_sync(st->map);  
> > 
> > Sashiko raised the point that you don't have a writeable register list for the regmap
> > and so potentially we at very least write a bunch of stuff that isn't needed.
> > I doubt it's actually a problem or you would have seen it, but nice little optimization
> > to reduce what is written.  
> 
> I will add such a list, I'd put it in patch 4 unless you think I should
> merge patch 3 and 4 to make the driver compileable at the first commit
> of code (will push the number of lines to the ~850/900 range though).
> 
I'm fine with them split but don't mind if Andy prefers them combined.
His comment on adding a COMPILE_TEST option would allow that to be
built, but it is a bit odd to do it just for that purpose.

Jonathan