Re: [PATCH v16 04/10] iio: imu: inv_icm42607: Add inv_icm42607 Core Driver

On Tue, 14 Jul 2026 14:18:53 +0200
Uwe Kleine-König <[email protected]> wrote:

> Hello,
> 
> On Mon, Jul 13, 2026 at 04:58:34PM -0500, Chris Morgan wrote:
> > +#include <linux/i2c.h>
> > +#include <linux/mod_devicetable.h>  
> 
> Please don't use <linux/mod_devicetable.h>, to get i2c_device_id and
> of_device_id it's enough to have <linux/i2c.h>.
> 
> > +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",
> > +		.data = &inv_icm42607_hw_data,
> > +	}, {
> > +		.compatible = "invensense,icm42607p",
> > +		.data = &inv_icm42607p_hw_data,
> > +	},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);  
> 
> It would be enough to provide the driver_data pointers in the
> i2c_device_id array only.
I'd prefer to keep them in both.

The match function tries the of_device_id path first and if we
should ever get a typo or mismatch we can end up with a driver
failing in a way that is annoyingly hard to track down.  
It has happened :(  Now I'd love us to get to the point
where we could just have the of_device_id table (and maybe
ACPI on occasion) but we aren't there today.

This came up in v17 where I asked why they were missing and Chris
mentioned it would be fine to drop them.

Jonathan

> 
> Best regards
> Uwe


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip