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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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