Re: [PATCH 2/3] iio: accel: Add support for ICM42370P

Kanak Shilledar <[email protected]>
Newsgroups org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hello!

Thanks for reviewing the code.

On Fri, 2026-08-07 at 08:51 +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Thu, Aug 06, 2026 at 02:46:28PM +0200, Kanak Shilledar wrote:
> > diff --git a/drivers/iio/accel/inv_icm42370_core.c
> > b/drivers/iio/accel/inv_icm42370_core.c
> > new file mode 100644
> > index 0000000000000..9f6c302e6f331
> > --- /dev/null
> > +++ b/drivers/iio/accel/inv_icm42370_core.c
> > @@ -0,0 +1,1251 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2020 Invensense, Inc.
> > + * Copyright (C) 2026 Axis Communications AB
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/irq.h>
> > +#include <linux/slab.h>
> > +#include <linux/mod_devicetable.h>
> 
> <linux/i2c.h> already provides structs i2c_device_id and
> of_device_id.
> So please drop the explicit include for <linux/mod_devicetable.h>, as
> this will go away soon.

Will fix them.

> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +
> > +#include <linux/iio/common/inv_sensors_timestamp.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +
> > [...]
> > +static const struct i2c_device_id inv_icm42370_id[] = { {
> > "icm42370",
> > +   INV_CHIP_ICM42370 },
> > + {} };
> 
> Please use named initializers and saner code style. I suggest:
> 
>  static const struct i2c_device_id inv_icm42370_id[] = {
>  { .name = "icm42370", .driver_data = INV_CHIP_ICM42370 },
>  { }
>  };


Thanks for the suggestion. Will fix the formatting.

> > +MODULE_DEVICE_TABLE(i2c, inv_icm42370_id);
> > +
> > +/**
> > + * inv_icm42370_of_matches - struct for all the compatibe strings
> > + *
> > + */
> > +static const struct of_device_id inv_icm42370_of_matches[] = {
> > + {
> > + .compatible = "invensense,icm42370",
> > + .data = (void *)INV_CHIP_ICM42370,
> > + },
> > + {}
> 
> { } please

Here as well.

> 
> > +};
> > +MODULE_DEVICE_TABLE(of, inv_icm42370_of_matches);
> > +
> > +static struct i2c_driver inv_icm42370_driver = {
> > + .driver = {
> > + .name = "inv-icm42370-i2c",
> > + .of_match_table = inv_icm42370_of_matches,
> > + },
> > + .probe = inv_icm42370_probe,
> 
>  .id_table = inv_icm42370_id,

Will add the property.


> > +};
> > +module_i2c_driver(inv_icm42370_driver);
> > +
> > +MODULE_AUTHOR("Kanak Shilledar <[email protected]>");
> > +MODULE_AUTHOR("Henrik Grimler <[email protected]>");
> > +MODULE_DESCRIPTION("InvenSense ICM-42370P I2C driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS("IIO_ICM42370");
> 
> Best regards
> Uwe

Thanks and Regards,
Kanak Shilledar
signature.asc (application/pgp-signature, 228 B)
-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRZbIB7R61KYt5yWsuslrbqDH4eIAUCanXZfgAKCRCslrbqDH4e
INUZAQCawRAvGLsTWntqQZ+HsgFTA+X5cKLIWAvT9GYI4RYIwwEAk+UV4s5WWIch
oHcmlyPNJ/SjHaayiGd/mzgn6FIGcAA=
=aDvx
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.