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

[Kanak Shilledar <Kanak.Shilledar@axis.com>]

[-- Attachment #1: Type: text/plain, Size: 2812 bytes --]

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 <kanak.shilledar@axis.com>");
> > +MODULE_AUTHOR("Henrik Grimler <henrik.grimler@axis.com>");
> > +MODULE_DESCRIPTION("InvenSense ICM-42370P I2C driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS("IIO_ICM42370");
> 
> Best regards
> Uwe

Thanks and Regards,
Kanak Shilledar


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]