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

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Thu, 16 Jul 2026 13:26:30 -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, as well as the bits necessary to compile and probe the
> device when used on an i2c bus.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

Hi Chris,

Just one small thing below given you are probably going to do
a v18.  If it had just been this I might have tweaked it whilst
applying

Thanks,

Jonathan


> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> new file mode 100644
> index 000000000000..8f74bf38aad1
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c

...

> +static int inv_icm42607_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	const struct inv_icm42607_hw *hw;
> +	struct regmap *regmap;
> +
> +	hw = i2c_get_match_data(client);
> +	if (!hw)
> +		return dev_err_probe(dev, -ENODEV, "Failed to get i2c data\n");
> +
> +	regmap = devm_regmap_init_i2c(client, &inv_icm42607_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "Failed to register i2c regmap\n");
> +
> +	return inv_icm42607_core_probe(regmap, hw, inv_icm42607_i2c_bus_setup);
> +}
> +
> +static const struct i2c_device_id inv_icm42607_id[] = {
> +	{
> +		.name = "icm42607",
> +		.driver_data = (kernel_ulong_t)&inv_icm42607_hw_data,
> +	}, {
> +		.name = "icm42607p",
> +		.driver_data = (kernel_ulong_t)&inv_icm42607p_hw_data,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, inv_icm42607_id);
> +
> +static const struct of_device_id inv_icm42607_of_matches[] = {
> +	{
> +		.compatible = "invensense,icm42607",

I'd expect to see the data here as well.  Look at implementation of i2c_get_match_data()
and consider what would happen if there was a slightly mismatch between which devices
were listed in each table.  To avoid that fragility we normally just put
the data pointers in both places.

> +	}, {
> +		.compatible = "invensense,icm42607p",
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);