[PATCH V11 3/9] iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
Andy Shevchenko
andriy.shevchenko at intel.com
Thu Jun 11 00:35:49 PDT 2026
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
...
> +#ifndef INV_ICM42607_H_
> +#define INV_ICM42607_H_
> +
> +#include <linux/bits.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mutex.h>
mutex_types.h ?
> +#include <linux/regmap.h>
> +#include <linux/types.h>
Do you need forward declaration for regulator data type?
...
> +#define INV_ICM42607_POWER_UP_TIME_US 100000
100 * USEC_PER_MSEC (will require time.h)?
...
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+ bitfields.h
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
+ device/devres.h
+ err.h
> +#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.
...
> + 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
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.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "reset error, reset done bit not set\n");
--
With Best Regards,
Andy Shevchenko
More information about the Linux-rockchip
mailing list