[PATCH V11 3/9] iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
Jonathan Cameron
jic23 at kernel.org
Thu Jun 11 04:09:43 PDT 2026
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.
> + 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.
> + 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.
> +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.
More information about the Linux-rockchip
mailing list