Re: [PATCH v2 0/3] Add driver for Invensense ICM42370P accelerometer

[Jean-Baptiste Maneyrol]

>
>________________________________________
>From: Kanak Shilledar <kanak.shilledar@xxxxxxxx>
>Sent: Thursday, August 13, 2026 14:26
>To: Jonathan Cameron; David Lechner; Nuno Sá; Andy Shevchenko; Rob Herring; Krzysztof Kozlowski; Conor Dooley; Henrik Grimler; Jean-Baptiste Maneyrol
>Cc: linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxx; Kanak Shilledar
>Subject: [PATCH v2 0/3] Add driver for Invensense ICM42370P accelerometer
>
>Invensense ICM42370P is a high performance MEMS MotionTracking 3-axis accelerometer. It supports I2C, I3C and SPI protocols. It has a 2. 25kB FIFO and two programmable interrupts with support for ultra-low-power wake-on-motion support. It has
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>Invensense ICM42370P is a high performance MEMS MotionTracking 3-axis
>accelerometer. It supports I2C, I3C and SPI protocols. It has a 2.25kB
>FIFO and two programmable interrupts with support for ultra-low-power
>wake-on-motion support. It has a built-in temperature sensor. This
>patch series adds basic support for the sensor with functionality of
>performing raw reads and writes along with buffered reads via the I2C
>interface.
>
>This device contains 4 register banks for configuring the device called
>MREG0, MREG1, MREG2 and MREG3. Unlike other devices from the same
>vendor, this contains a very different way of accessing the register
>banks apart from the default user bank 0 (MREG0). The register bank access
>procedure is mentioned in the datasheet [1] Section 13. Due to this
>specific reason and that it doesn't have a gyro sensor, it is implemented
>as a standalone driver.

Hello Kanak,

ICM-42370-P is completely similar to ICM-42607, except that it is accelerometer
only. There is no need to write a completely new driver, you can add support
of ICM-42370-P inside the existing inv_icm42607 driver.

inv_icm42607 driver is lacking a lot of functionalities, like buffer support.
It explains why there is no support for MREG registers.

I recommend you add first basic support of ICM-42370-P inside existing inv_icm42607
driver, and then add new functionalities like buffer support in this driver.
There is really no need to write a new driver here. For MREG registers support, you
can have a look inside inv_icm45600 that is using the same mechanism.

Thanks,
JB

>
>This device has 3 formats for the FIFO structure (datasheet [1] Section
>6), currently only FIFO packet format 1 is supported, others
>formats can be added as part of future work.
>
>The device supports EDGE and LEVEL triggered interrupts, refer the dt-binding
>for example of how the devicetree should be setup.
>
>The driver is inspired from iio/imu/inv_icm42600.
>
>Thanks to the reviewers for all the time you spent going through the
>code and the comments. The changes for v2 are quite a lot and I hope
>that every important change is documented in the changelog. After
>implementing the suggested changes, the driver is in a far better state
>than v1.
>
>[1] https://uat.invensense.com/en-us/products/3-axis/icm-42370-p
>
>Signed-off-by: Kanak Shilledar <kanak.shilledar@xxxxxxxx>
>---
>Changes in v2:
>* Changes across all files
>- Update MAINTAINERS with company mailing list
>- Sort/Cleanup of includes
>- Use `guard(mutex)` and newer `pm_runtime` APIs
>- Fix code formatting and add empty lines
>- Be consistent in inv_icm42370_data variable name
>- Fix MODULE_DESCRIPTION
>- Drop secondary state struct and merge it's properties in
>  `inv_icm42370_data` struct
>- Update mreg_read/write function calls
>- Change the compatible and filename to `icm42370p`
>
>* Changes to dt-binding
>- Add dependencies property
>- Made vdd and vddio supply as required
>- Add description to drive-open-drain property
>- Add mount-matrix property
>- Add interrupt-names property
>
>* Changes to `inv_icm42370.h` and `inv_icm42370_buffer.h`:
>- Resturcture the file according to @Marcelo's advice
>- Move struct __aligned properties to the end
>
>* Changes to `inv_icm42370_core.c`:
>- Fix _accel_scale[] values
>- Add IIO_TIMESTAMP to channel spec
>- Update mreg_read/write to fix bank access
>- Replace usleep_range() with fsleep()
>- Use constants from linux/units.h
>- Call `_update_fifo_period()` after updating the ODR values
>- Fix mathematical error in offset calculation
>- Implement handling of mount matrix
>- Implement handling of named interrupts
>- Use devm_regulator_get_enable for the vdd/vddio regulators
>- Use better error handling
>- Move iio device registration after performing IRQ init
>
>* Changes to `inv_icm42370_i2c.c`
>- Change compatible string as per the binding
>- Use named identifiers
>- Add `id_table` to the i2c_driver struct
>
>* Changes to `inv_icm42370_buffer.c`
>- Update FIFO enable/disable logic
>- Update FIFO buffer to match the specification and handle increased
>  size dynamically.
>
>- Link to v1: https://urldefense.com/v3/__https://patch.msgid.link/20260806-b4-inv_icm42370p-v1-0-670837f5842f@axis.com__;!!FtrhtPsWDhZ6tw!EBAKxSwTBMzsBVH_v_AbCESMgn2wN3OI3fqBEdSH9ChXdbE1NZYUnm6ZFS2n88XJTco7pQ5GABRt4tp90GAalWQltSMY_LtpWw$[patch[.]msgid[.]link]
>
>To: Kanak Shilledar <kanak.shilledar@xxxxxxxx>
>To: Henrik Grimler <henrik.grimler@xxxxxxxx>
>To: Jonathan Cameron <jic23@xxxxxxxxxx>
>To: David Lechner <dlechner@xxxxxxxxxxxx>
>To: Nuno Sá <nuno.sa@xxxxxxxxxx>
>To: Andy Shevchenko <andy@xxxxxxxxxx>
>To: Rob Herring <robh@xxxxxxxxxx>
>To: Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>
>To: Conor Dooley <conor+dt@xxxxxxxxxx>
>Cc: kernel@xxxxxxxx
>Cc: linux-iio@xxxxxxxxxxxxxxx
>Cc: devicetree@xxxxxxxxxxxxxxx
>Cc: linux-kernel@xxxxxxxxxxxxxxx
>
>---
>Kanak Shilledar (3):
>      dt-bindings: Add InvenSense ICM-42370-p accelerometer
>      iio: accel: Add support for ICM42370P
>      iio: accel: icm42370: Add FIFO buffer functionality
>
> .../bindings/iio/accel/invensense,icm42370p.yaml   |   84 ++
> MAINTAINERS                                        |    9 +
> drivers/iio/accel/Kconfig                          |   18 +
> drivers/iio/accel/Makefile                         |    6 +
> drivers/iio/accel/inv_icm42370.h                   |  352 +++++
> drivers/iio/accel/inv_icm42370_buffer.c            |  496 +++++++
> drivers/iio/accel/inv_icm42370_buffer.h            |  100 ++
> drivers/iio/accel/inv_icm42370_core.c              | 1502 ++++++++++++++++++++
> drivers/iio/accel/inv_icm42370_i2c.c               |   93 ++
> 9 files changed, 2660 insertions(+)
>---
>base-commit: 350d1fb9204b13c5f95e511e98b8bcb47574d425
>change-id: 20260629-b4-inv_icm42370p-ccd671066bcf
>
>Best regards,
>--
>Kanak Shilledar <kanak.shilledar@xxxxxxxx>
>
>
>