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

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 20 Jul 2026 12:15:06 -0500
Chris Morgan <macromorgan@hotmail.com> wrote:

> On Sun, Jul 19, 2026 at 12:05:30AM +0100, Jonathan Cameron wrote:
> > 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.  
> 
> I was advised in the notes for patch v16 that it could be removed. Should I go back and
> undo the remove?

Ah. I missed Uwe's comment. I've replied to that email. Let's give
it a day or two to see if Uwe comes back with an reason beyond it
working without it (which is true as long as the IDs always match!)

Jonathan

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