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

[Chris Morgan]

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 at gmail.com> wrote:
> 
> > From: Chris Morgan <macromorgan at 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 at 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).

> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = inv_icm42607_bus_setup(st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_set_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> > +			      INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> > +				 INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK,
> > +				 INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL);
> 
> That should have a FIELD_PREP() to save us having to got check that
> the MASK includes the LSB.  

Okay.

> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	return inv_icm42607_set_conf(st, st->hw->conf);
> > +}
> 
> > +
> > +MODULE_AUTHOR("InvenSense, Inc.");
> > +MODULE_DESCRIPTION("InvenSense ICM-42607x device driver");
> 
> Why does it have a trailing x?  Whilst a wild card is less harmful
> here than in many places I'd still drop it.

I think I missed it from the very beginning. I'll drop.

> 
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS("IIO_INV_SENSORS_TIMESTAMP");
> 
> Do we need this yet?  I think it only gets used later, in which case
> move it to the first patch that needs this.
> 
> 

No, we don't need it at all anymore. I think I can safely drop this now
that I'm no longer calling that code. Was a vestige from the buffers
code.

Thank you,
Chris