Add Invensense ICM42607
Subsystems (4)
v4 cover Add Invensense ICM42607
From: Chris Morgan <[email protected]>
Add support for the ICM42607 IMU. This sensor shares the same functionality but a different register layout with the existing ICM42600.
This driver should work with the ICM42607 and ICM42607P over both I2C and SPI, however only the ICM42607P over I2C could be tested.
Changes Since V1:
- Instead of creating a new driver, merged with the existing inv_icm42600 driver. This necessitated adding some code to the existing driver to permit using a different register layout for the same functionality.
- Split changes up a bit more to decrease the size of the individual patches. Note that patch 0004 is still pretty hefty; if I need to split further I may need to create some temporary stub functions.
- Used guard() and PM_RUNTIME_ACQUIRE_AUTOSUSPEND() on the new functions per Jonathan's recommendations.
Changes Since V2:
- Went back to using a new driver on advice from Invensense engineer.
- Further split changes up into smaller chunks of functionality. Note still that the largest patch is approximately 900 lines, and that while the driver compiles cleanly at each commit it is not able to drive the hardware until the commit that adds the Interrupt (as it also adds the Makefile).
- Change the error to a warning when the devicetree binding does not match the hardware ID.
- Dropped the ack on the devicetree bindings, as I am creating a new file (for a new driver) instead of modifying the existing one.
Changes Since V3:
- Numerous small fixes (too many to list here). Thank you to everyone who provided feedback.
- Split power management additions into an additional commit to break things up further.
- Consolidated devicetree documentation in existing invensense,icm42600.yaml file.
- Removed most of the FIELD_PREP from header file to c files to make code easier to read.
- Changed scale values to 2D arrays for Gyro and Accelerometer.
- Removed IIO_CHAN_INFO_CALIBBIAS attribute.
Chris Morgan (10):
dt-bindings: iio: imu: icm42600: Add icm42607 binding
iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
iio: imu: inv_icm42607: Add I2C and SPI For icm42607
iio: imu: inv_icm42607: Add PM support for icm42607
iio: imu: inv_icm42607: Add Buffer support for icm42607
iio: imu: inv_icm42607: Add Temp Support in icm42607
iio: imu: inv_icm42607: Add Accelerometer for icm42607
iio: imu: inv_icm42607: Add Wake on Movement for icm42607
iio: imu: inv_icm42607: Add Gyroscope to icm42607
arm64: dts: rockchip: Add icm42607p IMU for RG-DS
.../bindings/iio/imu/invensense,icm42600.yaml | 4 + .../dts/rockchip/rk3568-anbernic-rg-ds.dts | 20 +- drivers/iio/imu/Kconfig | 1 + drivers/iio/imu/Makefile | 1 + drivers/iio/imu/inv_icm42607/Kconfig | 30 + drivers/iio/imu/inv_icm42607/Makefile | 14 + drivers/iio/imu/inv_icm42607/inv_icm42607.h | 438 ++++++++ .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 989 ++++++++++++++++++ .../imu/inv_icm42607/inv_icm42607_buffer.c | 559 ++++++++++ .../imu/inv_icm42607/inv_icm42607_buffer.h | 101 ++ .../iio/imu/inv_icm42607/inv_icm42607_core.c | 809 ++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_gyro.c | 536 ++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 91 ++ .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 97 ++ .../iio/imu/inv_icm42607/inv_icm42607_temp.c | 81 ++ .../iio/imu/inv_icm42607/inv_icm42607_temp.h | 30 + 16 files changed, 3800 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/imu/inv_icm42607/Kconfig create mode 100644 drivers/iio/imu/inv_icm42607/Makefile create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
Signature · 1 line
2.43.0
PATCH v4 1/10 dt-bindings: iio: imu: icm42600: Add icm42607 binding
From: Chris Morgan <[email protected]>
Add devicetree binding for the Invensense ICM42607 and Invensense ICM42607P inertial measurement unit. This unit is a combined accelerometer, gyroscope, and thermometer available via I2C or SPI.
This device is functionally very similar to the icm42600 series with a very different register layout. Additionally, add mount-matrix attribute to schema.
--- .../devicetree/bindings/iio/imu/invensense,icm42600.yaml | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml +4 -0
@@ -30,6 +30,8 @@ properties: - invensense,icm42600 - invensense,icm42602 - invensense,icm42605 + - invensense,icm42607 + - invensense,icm42607p - invensense,icm42622 - invensense,icm42631 - invensense,icm42686
@@ -53,6 +55,8 @@ properties: drive-open-drain: type: boolean + mount-matrix: true + vdd-supply: description: Regulator that provides power to the sensor
Signature · 1 line
2.43.0
Re: [PATCH V4 01/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding
Quoted 9 lines · Chris Morgan
From: Chris Morgan <[email protected]>
Add devicetree binding for the Invensense ICM42607 and Invensense ICM42607P inertial measurement unit. This unit is a combined accelerometer, gyroscope, and thermometer available via I2C or SPI.
This device is functionally very similar to the icm42600 series with a very different register layout. Additionally, add mount-matrix attribute to schema.
Why adding it? Is it something new? Is it applicable to other variants? If not, why it is allowed for them?
Do not say WHAT you did, say why you did it.
Best regards,
Krzysztof
Re: [PATCH V4 01/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding
Quoted 7 lines · Krzysztof Kozlowski
Quoted 6 lines · Chris Morgan
From: Chris Morgan <[email protected]>
Add devicetree binding for the Invensense ICM42607 and Invensense ICM42607P inertial measurement unit. This unit is a combined accelerometer, gyroscope, and thermometer available via I2C or SPI.
Quoted 6 lines
Quoted 3 lines
This device is functionally very similar to the icm42600 series with a very different register layout. Additionally, add mount-matrix attribute to schema.
Why adding it? Is it something new? Is it applicable to other variants? If not, why it is allowed for them?
It's not new, technically this is a bug/oversight from the very first iteration of the invensense icm42600 driver. The driver requests a mount matrix using iio_read_mount_matrix and then returns an error from the probe function if it can't read one [1]. So it's very much required (and for the next version I'll upgrade it to a required element). I'm adding it because a cursory grep suggests that my use case (for the Anbernic RG-DS and in the near future the Anbernic RG Vita Pro) are the first devices to use this driver in mainline in a device tree based system.
Thank you,
Chris
Quoted 6 lines
Do not say WHAT you did, say why you did it.
Best regards,
Krzysztof
Jonathan Cameron
maintainerRe: [PATCH V4 01/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding
Quoted 12 lines · On Sun, 3 May 2026 15:51:30 -0500 Chris Morgan <[email protected]> wrote:
Quoted 11 lines · Krzysztof Kozlowski
Quoted 9 lines · Chris Morgan
From: Chris Morgan <[email protected]>
Add devicetree binding for the Invensense ICM42607 and Invensense ICM42607P inertial measurement unit. This unit is a combined accelerometer, gyroscope, and thermometer available via I2C or SPI.
This device is functionally very similar to the icm42600 series with a very different register layout. Additionally, add mount-matrix attribute to schema.
Quoted 6 lines
Quoted 2 lines
Why adding it? Is it something new? Is it applicable to other variants? If not, why it is allowed for them?
It's not new, technically this is a bug/oversight from the very first iteration of the invensense icm42600 driver. The driver requests a mount matrix using
iio_read_mount_matrixand then returns an error from
Unless it's broken (always possible) iio_read_mount_matrix() is supposed to return an identity matrix if there isn't any info in the binding.
/* Matrix was not declared at all: fallback to identity. */ return iio_setup_mount_idmatrix(dev, matrix);
So not required by the linux driver at least. Probably not something we should require in general.
Separate patch to introduce it to the binding as optional and say something about what it is for to justify it's inclusion. That patch goes before this one.
Quoted 18 lines
the probe function if it can't read one [1]. So it's very much required (and for the next version I'll upgrade it to a required element). I'm adding it because a cursory grep suggests that my use case (for the Anbernic RG-DS and in the near future the Anbernic RG Vita Pro) are the first devices to use this driver in mainline in a device tree based system.
Thank you,
ChrisQuoted 6 lines
Do not say WHAT you did, say why you did it.
Best regards,
Krzysztof
Re: [PATCH V4 01/10] dt-bindings: iio: imu: icm42600: Add icm42607 binding
Quoted 28 lines · Jonathan Cameron
Quoted 12 lines · On Sun, 3 May 2026 15:51:30 -0500 Chris Morgan <[email protected]> wrote:
Quoted 11 lines · Krzysztof Kozlowski
Quoted 9 lines · Chris Morgan
From: Chris Morgan <[email protected]>
Add devicetree binding for the Invensense ICM42607 and Invensense ICM42607P inertial measurement unit. This unit is a combined accelerometer, gyroscope, and thermometer available via I2C or SPI.
This device is functionally very similar to the icm42600 series with a very different register layout. Additionally, add mount-matrix attribute to schema.
Quoted 6 lines
Quoted 2 lines
Why adding it? Is it something new? Is it applicable to other variants? If not, why it is allowed for them?
It's not new, technically this is a bug/oversight from the very first iteration of the invensense icm42600 driver. The driver requests a mount matrix using
iio_read_mount_matrixand then returns an error fromUnless it's broken (always possible)
iio_read_mount_matrix()is supposed to return an identity matrix if there isn't any info in the binding./* Matrix was not declared at all: fallback to identity. */ return iio_setup_mount_idmatrix(dev, matrix);
Quoted 6 lines
So not required by the linux driver at least. Probably not something we should require in general.
Separate patch to introduce it to the binding as optional and say something about what it is for to justify it's inclusion. That patch goes before this one.
Okay, that makes more sense. I saw the return condition but didn't realize that an empty matrix didn't result in an error.
So for the next version I'll add an additionall commit with this as a new attribute, noting that it has always been supported with this driver but never specified in the binding documentation.
Thank you,
Chris
Quoted 20 lines
Quoted 19 lines
the probe function if it can't read one [1]. So it's very much required (and for the next version I'll upgrade it to a required element). I'm adding it because a cursory grep suggests that my use case (for the Anbernic RG-DS and in the near future the Anbernic RG Vita Pro) are the first devices to use this driver in mainline in a device tree based system.
Thank you,
ChrisQuoted 6 lines
Do not say WHAT you did, say why you did it.
Best regards,
Krzysztof
PATCH v4 2/10 iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
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.
--- drivers/iio/imu/inv_icm42607/inv_icm42607.h | 400 ++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_core.c | 303 +++++++++++++ 2 files changed, 703 insertions(+) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
drivers/iio/imu/inv_icm42607/inv_icm42607.h new file +400 -0
@@ -0,0 +1,400 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2026 InvenSense, Inc. +*/ + +#ifndef INV_ICM42607_H_ +#define INV_ICM42607_H_ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/iio/common/inv_sensors_timestamp.h> +#include <linux/iio/iio.h> +#include <linux/mutex.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +enum inv_icm42607_chip { +INV_CHIP_INVALID , +INV_CHIP_ICM42607P , +INV_CHIP_ICM42607 , +INV_CHIP_NB +}; + +/* serial bus slew rates */ +enum inv_icm42607_slew_rate { +INV_ICM42607_SLEW_RATE_20_60NS , +INV_ICM42607_SLEW_RATE_12_36NS , +INV_ICM42607_SLEW_RATE_6_18NS , +INV_ICM42607_SLEW_RATE_4_12NS , +INV_ICM42607_SLEW_RATE_2_6NS , +INV_ICM42607_SLEW_RATE_INF_2NS +}; + +enum inv_icm42607_sensor_mode { +INV_ICM42607_SENSOR_MODE_OFF , +INV_ICM42607_SENSOR_MODE_STANDBY , +INV_ICM42607_SENSOR_MODE_LOW_POWER , +INV_ICM42607_SENSOR_MODE_LOW_NOISE , +INV_ICM42607_SENSOR_MODE_NB +}; + +/* gyroscope fullscale values */ +enum inv_icm42607_gyro_fs { +INV_ICM42607_GYRO_FS_2000DPS , +INV_ICM42607_GYRO_FS_1000DPS , +INV_ICM42607_GYRO_FS_500DPS , +INV_ICM42607_GYRO_FS_250DPS , +INV_ICM42607_GYRO_FS_NB +}; + +/* accelerometer fullscale values */ +enum inv_icm42607_accel_fs { +INV_ICM42607_ACCEL_FS_16G , +INV_ICM42607_ACCEL_FS_8G , +INV_ICM42607_ACCEL_FS_4G , +INV_ICM42607_ACCEL_FS_2G , +INV_ICM42607_ACCEL_FS_NB +}; + +/* ODR values */ +enum inv_icm42607_odr { +INV_ICM42607_ODR_1600HZ =5 , +INV_ICM42607_ODR_800HZ , +INV_ICM42607_ODR_400HZ , +INV_ICM42607_ODR_200HZ , +INV_ICM42607_ODR_100HZ , +INV_ICM42607_ODR_50HZ , +INV_ICM42607_ODR_25HZ , +INV_ICM42607_ODR_12_5HZ , +INV_ICM42607_ODR_6_25HZ_LP , +INV_ICM42607_ODR_3_125HZ_LP , +INV_ICM42607_ODR_1_5625HZ_LP , +INV_ICM42607_ODR_NB +}; + +enum inv_icm42607_filter_bw { +/* Low-Noise mode sensor data filter (bandwidth) */ +INV_ICM42607_FILTER_BYPASS , +INV_ICM42607_FILTER_BW_180HZ , +INV_ICM42607_FILTER_BW_121HZ , +INV_ICM42607_FILTER_BW_73HZ , +INV_ICM42607_FILTER_BW_53HZ , +INV_ICM42607_FILTER_BW_34HZ , +INV_ICM42607_FILTER_BW_25HZ , +INV_ICM42607_FILTER_BW_16HZ +}; + +enum inv_icm42607_filter_avg { +/* Low-Power mode sensor data filter (averaging) */ +INV_ICM42607_FILTER_AVG_2X =0 , +INV_ICM42607_FILTER_AVG_4X , +INV_ICM42607_FILTER_AVG_8X , +INV_ICM42607_FILTER_AVG_16X , +INV_ICM42607_FILTER_AVG_32X , +INV_ICM42607_FILTER_AVG_64X +/* values 7 and 8 also correspond to 64x. */ +}; + +struct inv_icm42607_sensor_conf { +int mode; +int fs; +int odr; +int filter; +}; +#define INV_ICM42607_SENSOR_CONF_INIT {-1, -1, -1, -1} + +struct inv_icm42607_conf { +struct inv_icm42607_sensor_conf gyro; +struct inv_icm42607_sensor_conf accel; +bool temp_en; +}; + +struct inv_icm42607_suspended { +enum inv_icm42607_sensor_mode gyro; +enum inv_icm42607_sensor_mode accel; +bool temp; +}; + +struct inv_icm42607_apex { +unsigned int on; +struct { +u64 value; +bool enable; + } wom; +}; + +/** +* struct inv_icm42607_state - driver state variables +* @lock: lock for serializing multiple registers access. +* @chip: chip identifier. +* @name: chip name. +* @map: regmap pointer. +* @vddio_supply: I/O voltage regulator for the chip. +* @irq: chip irq, required to enable/disable and set wakeup +* @orientation: sensor chip orientation relative to main hardware. +* @conf: chip sensors configurations. +* @suspended: suspended sensors configuration. +* @indio_gyro: gyroscope IIO device. +* @indio_accel: accelerometer IIO device. +* @timestamp: interrupt timestamps. +* @apex: APEX (Advanced Pedometer and Event detection) management +* @buffer: data transfer buffer aligned for DMA. +*/ +struct inv_icm42607_state { +struct mutex lock; +enum inv_icm42607_chip chip; +const char *name; +struct regmap *map; +struct regulator *vddio_supply; +int irq; +struct iio_mount_matrix orientation; +struct inv_icm42607_conf conf; +struct inv_icm42607_suspended suspended; +struct iio_dev *indio_gyro; +struct iio_dev *indio_accel; +struct { +s64 gyro; +s64 accel; + } timestamp; +struct inv_icm42607_apex apex; +__be16 buffer[3 ]__aligned(IIO_DMA_MINALIGN) ; +}; + +/** +* struct inv_icm42607_sensor_state - sensor state variables +* @power_mode: sensor requested power mode (for common frequencies) +* @filter: sensor filter. +* @ts: timestamp module states. +*/ +struct inv_icm42607_sensor_state { +enum inv_icm42607_sensor_mode power_mode; +int filter; +struct inv_sensors_timestamp ts; +}; + +/* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */ + +/* Register Map for User Bank 0 */ +#define INV_ICM42607_REG_DEVICE_CONFIG 0x01 +#define INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE BIT(2) +#define INV_ICM42607_DEVICE_CONFIG_SPI_MODE BIT(0) + +#define INV_ICM42607_REG_SIGNAL_PATH_RESET 0x02 +#define INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET BIT(4) +#define INV_ICM42607_SIGNAL_PATH_RESET_FIFO_FLUSH BIT(2) + +#define INV_ICM42607_REG_DRIVE_CONFIG1 0x03 +#define INV_ICM42607_DRIVE_CONFIG1_I3C_DDR_MASK GENMASK(5, 3) +#define INV_ICM42607_DRIVE_CONFIG1_I3C_SDR_MASK GENMASK(2, 0) + +#define INV_ICM42607_REG_DRIVE_CONFIG2 0x04 +#define INV_ICM42607_DRIVE_CONFIG2_I2C_MASK GENMASK(5, 3) +#define INV_ICM42607_DRIVE_CONFIG2_ALL_MASK GENMASK(2, 0) + +#define INV_ICM42607_REG_DRIVE_CONFIG3 0x05 +#define INV_ICM42607_DRIVE_CONFIG3_SPI_MASK GENMASK(2, 0) + +#define INV_ICM42607_REG_INT_CONFIG 0x06 +#define INV_ICM42607_INT_CONFIG_INT2_LATCHED BIT(5) +#define INV_ICM42607_INT_CONFIG_INT2_PUSH_PULL BIT(4) +#define INV_ICM42607_INT_CONFIG_INT2_ACTIVE_HIGH BIT(3) +#define INV_ICM42607_INT_CONFIG_INT2_ACTIVE_LOW 0x00 +#define INV_ICM42607_INT_CONFIG_INT1_LATCHED BIT(2) +#define INV_ICM42607_INT_CONFIG_INT1_PUSH_PULL BIT(1) +#define INV_ICM42607_INT_CONFIG_INT1_ACTIVE_HIGH BIT(0) +#define INV_ICM42607_INT_CONFIG_INT1_ACTIVE_LOW 0x00 + +/* all sensor data are 16 bits (2 registers wide) in big-endian */ +#define INV_ICM42607_REG_TEMP_DATA1 0x09 +#define INV_ICM42607_REG_TEMP_DATA0 0x0A +#define INV_ICM42607_REG_ACCEL_DATA_X1 0x0B +#define INV_ICM42607_REG_ACCEL_DATA_X0 0x0C +#define INV_ICM42607_REG_ACCEL_DATA_Y1 0x0D +#define INV_ICM42607_REG_ACCEL_DATA_Y0 0x0E +#define INV_ICM42607_REG_ACCEL_DATA_Z1 0x0F +#define INV_ICM42607_REG_ACCEL_DATA_Z0 0x10 +#define INV_ICM42607_REG_GYRO_DATA_X1 0x11 +#define INV_ICM42607_REG_GYRO_DATA_X0 0x12 +#define INV_ICM42607_REG_GYRO_DATA_Y1 0x13 +#define INV_ICM42607_REG_GYRO_DATA_Y0 0x14 +#define INV_ICM42607_REG_GYRO_DATA_Z1 0x15 +#define INV_ICM42607_REG_GYRO_DATA_Z0 0x16 +#define INV_ICM42607_DATA_INVALID -32768 + +#define INV_ICM42607_REG_TMST_FSYNCH 0x17 +#define INV_ICM42607_REG_TMST_FSYNCL 0x18 + +/* APEX Data Registers */ +#define INV_ICM42607_REG_APEX_DATA0 0x31 +#define INV_ICM42607_REG_APEX_DATA1 0x32 +#define INV_ICM42607_REG_APEX_DATA2 0x33 +#define INV_ICM42607_REG_APEX_DATA3 0x34 +#define INV_ICM42607_REG_APEX_DATA4 0x1D +#define INV_ICM42607_REG_APEX_DATA5 0x1E + +#define INV_ICM42607_REG_PWR_MGMT0 0x1F +#define INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL BIT(7) +#define INV_ICM42607_PWR_MGMT0_IDLE BIT(4) +#define INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK GENMASK(3, 2) +#define INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK GENMASK(1, 0) + +#define INV_ICM42607_REG_GYRO_CONFIG0 0x20 +#define INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK GENMASK(6, 5) +#define INV_ICM42607_GYRO_CONFIG0_ODR_MASK GENMASK(3, 0) + +#define INV_ICM42607_REG_ACCEL_CONFIG0 0x21 +#define INV_ICM42607_ACCEL_CONFIG0_FS_SEL_MASK GENMASK(6, 5) +#define INV_ICM42607_ACCEL_CONFIG0_ODR_MASK GENMASK(3, 0) + +#define INV_ICM42607_REG_TEMP_CONFIG0 0x22 +#define INV_ICM42607_TEMP_CONFIG0_FILTER_MASK GENMASK(6, 4) + +#define INV_ICM42607_REG_GYRO_CONFIG1 0x23 +#define INV_ICM42607_GYRO_CONFIG1_FILTER_MASK GENMASK(2, 0) + +#define INV_ICM42607_REG_ACCEL_CONFIG1 0x24 +#define INV_ICM42607_ACCEL_CONFIG1_AVG_MASK GENMASK(6, 4) +#define INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK GENMASK(2, 0) + +#define INV_ICM42607_REG_APEX_CONFIG0 0x25 +#define INV_ICM42607_APEX_CONFIG0_DMP_POWER_SAVE_EN BIT(3) +#define INV_ICM42607_APEX_CONFIG0_DMP_INIT_EN BIT(2) +#define INV_ICM42607_APEX_CONFIG0_DMP_MEM_RESET_EN BIT(0) + +#define INV_ICM42607_REG_APEX_CONFIG1 0x26 +#define INV_ICM42607_APEX_CONFIG1_SMD_ENABLE BIT(6) +#define INV_ICM42607_APEX_CONFIG1_FF_ENABLE BIT(5) +#define INV_ICM42607_APEX_CONFIG1_TILT_ENABLE BIT(4) +#define INV_ICM42607_APEX_CONFIG1_PED_ENABLE BIT(3) +#define INV_ICM42607_APEX_CONFIG1_DMP_ODR_MASK GENMASK(1, 0) + +#define INV_ICM42607_REG_WOM_CONFIG 0x27 +#define INV_ICM42607_WOM_CONFIG_INT_DUR_MASK GENMASK(4, 3) +#define INV_ICM42607_WOM_CONFIG_INT_MODE BIT(2) +#define INV_ICM42607_WOM_CONFIG_MODE BIT(1) +#define INV_ICM42607_WOM_CONFIG_EN BIT(0) + +#define INV_ICM42607_REG_FIFO_CONFIG1 0x28 +#define INV_ICM42607_FIFO_CONFIG1_MODE BIT(1) +#define INV_ICM42607_FIFO_CONFIG1_BYPASS BIT(0) + +#define INV_ICM42607_REG_FIFO_CONFIG2 0x29 +#define INV_ICM42607_REG_FIFO_CONFIG3 0x2A +#define INV_ICM42607_FIFO_WATERMARK_VAL (_wm) \ + cpu_to_le16((_wm) & GENMASK(11, 0)) +/* FIFO is 2048 bytes, let 12 samples for reading latency */ +#define INV_ICM42607_FIFO_WATERMARK_MAX (2048 - 12 * 16) + +#define INV_ICM42607_REG_INT_SOURCE0 0x2B +#define INV_ICM42607_INT_SOURCE0_ST_INT1_EN BIT(7) +#define INV_ICM42607_INT_SOURCE0_FSYNC_INT1_EN BIT(6) +#define INV_ICM42607_INT_SOURCE0_PLL_RDY_INT1_EN BIT(5) +#define INV_ICM42607_INT_SOURCE0_RESET_DONE_INT1_EN BIT(4) +#define INV_ICM42607_INT_SOURCE0_DRDY_INT1_EN BIT(3) +#define INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN BIT(2) +#define INV_ICM42607_INT_SOURCE0_FIFO_FULL_INT1_EN BIT(1) +#define INV_ICM42607_INT_SOURCE0_AGC_RDY_INT1_EN BIT(0) + +#define INV_ICM42607_REG_INT_SOURCE1 0x2C +#define INV_ICM42607_INT_SOURCE1_I3C_ERROR_INT1_EN BIT(6) +#define INV_ICM42607_INT_SOURCE1_SMD_INT1_EN BIT(3) +#define INV_ICM42607_INT_SOURCE1_WOM_INT1_EN GENMASK(2, 0) + +#define INV_ICM42607_REG_INT_SOURCE3 0x2D +#define INV_ICM42607_INT_SOURCE3_ST_INT2_EN BIT(7) +#define INV_ICM42607_INT_SOURCE3_FSYNC_INT2_EN BIT(6) +#define INV_ICM42607_INT_SOURCE3_PLL_RDY_INT2_EN BIT(5) +#define INV_ICM42607_INT_SOURCE3_RESET_DONE_INT2_EN BIT(4) +#define INV_ICM42607_INT_SOURCE3_DRDY_INT2_EN BIT(3) +#define INV_ICM42607_INT_SOURCE3_FIFO_THS_INT2_EN BIT(2) +#define INV_ICM42607_INT_SOURCE3_FIFO_FULL_INT2_EN BIT(1) +#define INV_ICM42607_INT_SOURCE3_AGC_RDY_INT2_EN BIT(0) + +#define INV_ICM42607_REG_INT_SOURCE4 0x2E +#define INV_ICM42607_INT_SOURCE4_I3C_ERROR_INT2_EN BIT(6) +#define INV_ICM42607_INT_SOURCE4_SMD_INT2_EN BIT(3) +#define INV_ICM42607_INT_SOURCE4_WOM_Z_INT2_EN BIT(2) +#define INV_ICM42607_INT_SOURCE4_WOM_Y_INT2_EN BIT(1) +#define INV_ICM42607_INT_SOURCE4_WOM_X_INT2_EN BIT(0) + +#define INV_ICM42607_REG_FIFO_LOST_PKT0 0x2F +#define INV_ICM42607_REG_FIFO_LOST_PKT1 0x30 + +#define INV_ICM42607_REG_INTF_CONFIG0 0x35 +#define INV_ICM42607_INTF_CONFIG0_FIFO_COUNT_FORMAT BIT(6) +#define INV_ICM42607_INTF_CONFIG0_FIFO_COUNT_ENDIAN BIT(5) +#define INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN BIT(4) +#define INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK GENMASK(1, 0) +#define INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS \ + FIELD_PREP(INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK, 2) +#define INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS \ + FIELD_PREP(INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK, 3) + +#define INV_ICM42607_REG_INTF_CONFIG1 0x36 +#define INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN BIT(3) +#define INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN BIT(2) +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK GENMASK(1, 0) +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_INT 0 +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL 1 +#define INV_ICM42607_INTF_CONFIG1_CLKSEL_OFF 2 + +#define INV_ICM42607_REG_INT_STATUS_DRDY 0x39 +#define INV_ICM42607_INT_STATUS_DRDY_DATA_RDY BIT(0) + +#define INV_ICM42607_REG_INT_STATUS 0x3A +#define INV_ICM42607_INT_STATUS_ST BIT(7) +#define INV_ICM42607_INT_STATUS_FSYNC BIT(6) +#define INV_ICM42607_INT_STATUS_PLL_RDY BIT(5) +#define INV_ICM42607_INT_STATUS_RESET_DONE BIT(4) +#define INV_ICM42607_INT_STATUS_FIFO_THS BIT(2) +#define INV_ICM42607_INT_STATUS_FIFO_FULL BIT(1) +#define INV_ICM42607_INT_STATUS_AGC_RDY BIT(0) + +#define INV_ICM42607_REG_INT_STATUS2 0x3B +#define INV_ICM42607_INT_STATUS2_SMD BIT(3) +#define INV_ICM42607_INT_STATUS2_WOM_INT GENMASK(2, 0) + +#define INV_ICM42607_REG_INT_STATUS3 0x3C +#define INV_ICM42607_INT_STATUS3_STEP_DET BIT(5) +#define INV_ICM42607_INT_STATUS3_STEP_CNT_OVF BIT(4) +#define INV_ICM42607_INT_STATUS3_TILT_DET BIT(3) +#define INV_ICM42607_INT_STATUS3_FF_DET BIT(2) + +/* +* FIFO access registers +* FIFO count is 16 bits (2 registers) big-endian +* FIFO data is a continuous read register to read FIFO content +*/ +#define INV_ICM42607_REG_FIFO_COUNTH 0x3D +#define INV_ICM42607_REG_FIFO_COUNTL 0x3E +#define INV_ICM42607_REG_FIFO_DATA 0x3F + +#define INV_ICM42607_REG_ACCEL_WOM_X_THR 0x4b +#define INV_ICM42607_REG_ACCEL_WOM_Y_THR 0x4c +#define INV_ICM42607_REG_ACCEL_WOM_Z_THR 0x4d + +#define INV_ICM42607_REG_WHOAMI 0x75 +#define INV_ICM42607P_WHOAMI 0x60 +#define INV_ICM42607_WHOAMI 0x67 + +/* Sleep times required by the driver */ +#define INV_ICM42607_POWER_UP_TIME_US 100000 +#define INV_ICM42607_RESET_TIME_MS 1 +#define INV_ICM42607_ACCEL_STARTUP_TIME_MS 20 +#define INV_ICM42607_GYRO_STARTUP_TIME_MS 60 +#define INV_ICM42607_GYRO_STOP_TIME_MS 150 +#define INV_ICM42607_TEMP_STARTUP_TIME_MS 14 +#define INV_ICM42607_SUSPEND_DELAY_MS 2000 + +typedef int (*inv_icm42607_bus_setup )(struct inv_icm42607_state *); + +u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr); + +int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg, +unsigned int writeval,unsigned int *readval); + +int inv_icm42607_core_probe (struct regmap *regmap,int chip, +inv_icm42607_bus_setup bus_setup); + +#endif
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +303 -0
303 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 02/10] iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
Quoted 7 lines · On Fri, 1 May 2026 17:11:41 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add the core component of a new
inv_icm42607driver. This includes a few setup functions and the full register definition in the header file.Signed-off-by Chris Morgan <[email protected]>
Hi Chris. A few comments inline.
Biggest one is probably to shuffle when you introduce structures and elements in other structures so they only turn up when they are used in later patches. You can do that with enums etc as well. I don't mind the register defines all in the first patch but the other stuff would ideally come later.
Thanks
Jonathan
Quoted 12 lines
--- drivers/iio/imu/inv_icm42607/inv_icm42607.h | 400 ++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_core.c | 303 +++++++++++++ 2 files changed, 703 insertions(+) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.cdrivers/iio/imu/inv_icm42607/inv_icm42607.h new file +0
-0
drivers/iio/imu/inv_icm42607/inv_icm42607.h line 125
Quoted 9 lines
drivers/iio/imu/inv_icm42607/inv_icm42607.h new file +8
-0+ +struct inv_icm42607_apex { +unsigned int on; +struct { +u64 value; +bool enable; + } wom; +};
These structures that are only used in later patches, should only be brought in as part of those patches.
Quoted 20 lines
drivers/iio/imu/inv_icm42607/inv_icm42607.h new file +20
-0+ +/** +* struct inv_icm42607_state - driver state variables +* @lock: lock for serializing multiple registers access. +* @chip: chip identifier. +* @name: chip name. +* @map: regmap pointer. +* @vddio_supply: I/O voltage regulator for the chip. +* @irq: chip irq, required to enable/disable and set wakeup +* @orientation: sensor chip orientation relative to main hardware. +* @conf: chip sensors configurations. +* @suspended: suspended sensors configuration. +* @indio_gyro: gyroscope IIO device. +* @indio_accel: accelerometer IIO device. +* @timestamp: interrupt timestamps. +* @apex: APEX (Advanced Pedometer and Event detection) management +* @buffer: data transfer buffer aligned for DMA. +*/ +struct inv_icm42607_state { +struct mutex lock;
drivers/iio/imu/inv_icm42607/inv_icm42607.h line 155
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607.h new file +10
-0+enum inv_icm42607_chip chip; +const char *name; +struct regmap *map; +struct regulator *vddio_supply; +int irq; +struct iio_mount_matrix orientation; +struct inv_icm42607_conf conf; +struct inv_icm42607_suspended suspended; +struct iio_dev *indio_gyro; +struct iio_dev *indio_accel;
Even these should only be added when you add something that uses them.
Quoted 40 lines
drivers/iio/imu/inv_icm42607/inv_icm42607.h new file +7
-0+struct { +s64 gyro; +s64 accel; + } timestamp; +struct inv_icm42607_apex apex; +__be16 buffer[3 ]__aligned(IIO_DMA_MINALIGN) ; +};drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +26
-0+/** +* inv_icm42607_setup() - check and setup chip +* @st: driver internal state +* @bus_setup: callback for setting up bus specific registers +* +* Returns 0 on success, a negative error code otherwise. +*/ +static int inv_icm42607_setup (struct inv_icm42607_state *st, +inv_icm42607_bus_setup bus_setup) +{ +const struct inv_icm42607_hw *hw = &inv_icm42607_hw[st->chip]; +const struct device *dev =regmap_get_device (st->map); +unsigned int val; +int ret; + + ret =regmap_read (st->map,INV_ICM42607_REG_WHOAMI , &val); +if (ret) +return ret; + +if (val != hw->whoami) +dev_warn_probe (dev, -ENODEV , +"invalid whoami %#02x expected %#02x (%s)\n" , + val, hw->whoami, hw->name); + + st->name = hw->name; +
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c line 189
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +10
-0+ ret =regmap_write (st->map,INV_ICM42607_REG_SIGNAL_PATH_RESET , +INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET ); +if (ret) +return ret; + + ret =regmap_read_poll_timeout (st->map,INV_ICM42607_REG_INT_STATUS , + val, val &INV_ICM42607_INT_STATUS_RESET_DONE , +INV_ICM42607_RESET_TIME_MS *100 , +INV_ICM42607_RESET_TIME_MS *1000 ); +
No blank line here. Keep the error check tightly coupled with the source of errors.
Quoted 28 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +27
-0+if (ret) +return dev_err_probe (dev, ret, +"reset error, reset done bit not set\n" ); + + ret =bus_setup (st); +if (ret) +return ret; + + ret =regmap_set_bits (st->map,INV_ICM42607_REG_INTF_CONFIG0 , +INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN ); +if (ret) +return ret; + + ret =regmap_update_bits (st->map,INV_ICM42607_REG_INTF_CONFIG1 , +INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK , +INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL ); +if (ret) +return ret; + +return inv_icm42607_set_conf (st, hw->conf); +} + +int inv_icm42607_core_probe (struct regmap *regmap,int chip, +inv_icm42607_bus_setup bus_setup) +{ +struct device *dev =regmap_get_device (regmap); +struct fwnode_handle *fwnode =dev_fwnode (dev);
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c line 246
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +10
-0+struct inv_icm42607_state *st; +int irq; +bool open_drain; +int ret; + +/* +* Keep bounds checking in case more chips are added, for now only +* 2 are supported. +*/ +if (chip <INV_CHIP_INVALID || chip >=INV_CHIP_NB )
Is INV_CHIP_INVALID valid in any sense? If it is add a comment to the enum. If it's not <= INV_CHIP_INVALID though I would be tempted to just make chip unsigned and not worry about the bottom - if anyone passes a negative wrap around will make it fail the other check. I'm not immediately spotting any way we can end up with an invalid chip id.
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c line 247
Quoted 1 line
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c new file +1
-0+dev_warn_probe (dev, -ENODEV ,"Invalid chip = %d\n" , chip);
We allow for fallback compatibles and hence WHOAMI reg mismatches but not for bugs and I can't see how this is any thing other than a bug? If it's for legacy probe paths we still want to know what it is.
Quoted 1 line
+
PATCH v4 3/10 iio: imu: inv_icm42607: Add I2C and SPI For icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add I2C and SPI driver support for InvenSense ICM-42607 devices. Add necessary Kconfig and Makefile to allow building of (incomplete) driver.
--- drivers/iio/imu/Kconfig | 1 + drivers/iio/imu/Makefile | 1 + drivers/iio/imu/inv_icm42607/Kconfig | 30 ++++++ drivers/iio/imu/inv_icm42607/Makefile | 10 ++ drivers/iio/imu/inv_icm42607/inv_icm42607.h | 2 + .../iio/imu/inv_icm42607/inv_icm42607_core.c | 20 ++++ .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 90 +++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 96 +++++++++++++++++++ 8 files changed, 250 insertions(+) create mode 100644 drivers/iio/imu/inv_icm42607/Kconfig create mode 100644 drivers/iio/imu/inv_icm42607/Makefile create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
drivers/iio/imu/Kconfig +1 -0
@@ -109,6 +109,7 @@ config KMX61 be called kmx61. source "drivers/iio/imu/inv_icm42600/Kconfig" +source "drivers/iio/imu/inv_icm42607/Kconfig" source "drivers/iio/imu/inv_icm45600/Kconfig" source "drivers/iio/imu/inv_mpu6050/Kconfig"
drivers/iio/imu/Makefile +1 -0
@@ -25,6 +25,7 @@ obj-$(CONFIG_FXOS8700_I2C) += fxos8700_i2c.o obj-$(CONFIG_FXOS8700_SPI) += fxos8700_spi.o obj-y += inv_icm42600/ +obj-y += inv_icm42607/ obj-y += inv_icm45600/ obj-y += inv_mpu6050/
drivers/iio/imu/inv_icm42607/Kconfig new file +30 -0
@@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +config INV_ICM42607 + tristate + select IIO_BUFFER + select IIO_INV_SENSORS_TIMESTAMP + +config INV_ICM42607_I2C + tristate "InvenSense ICM-42607X I2C driver" + depends on I2C + select INV_ICM42607 + select REGMAP_I2C + help + This driver supports the InvenSense ICM-42607 motion tracking + device over I2C. + + This driver can be built as a module. The module will be called + inv-icm42607-i2c. + +config INV_ICM42607_SPI + tristate "InvenSense ICM-42607X SPI driver" + depends on SPI_MASTER + select INV_ICM42607 + select REGMAP_SPI + help + This driver supports the InvenSense ICM-42607 motion tracking + device over SPI. + + This driver can be built as a module. The module will be called + inv-icm42607-spi.
drivers/iio/imu/inv_icm42607/Makefile new file +10 -0
@@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o +inv-icm42607-y += inv_icm42607_core.o + +obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o +inv-icm42607-i2c-y += inv_icm42607_i2c.o + +obj-$(CONFIG_INV_ICM42607_SPI) += inv-icm42607-spi.o +inv-icm42607-spi-y += inv_icm42607_spi.o
drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0
@@ -389,6 +389,8 @@ struct inv_icm42607_sensor_state {typedef int (*inv_icm42607_bus_setup )(struct inv_icm42607_state *); +extern const struct regmap_config inv_icm42607_regmap_config; +u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr); int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +20 -0
@@ -16,6 +16,26 @@#include "inv_icm42607.h" +static const struct regmap_range_cfg inv_icm42607_regmap_ranges[] = { + { + .name ="user bank" , + .range_min =0x0000 , + .range_max =0x00FF , + .window_start =0 , + .window_len =0x0100 , + }, +}; + +const struct regmap_config inv_icm42607_regmap_config = { + .reg_bits =8 , + .val_bits =8 , + .max_register =0x00FF , + .ranges = inv_icm42607_regmap_ranges, + .num_ranges =ARRAY_SIZE (inv_icm42607_regmap_ranges), + .cache_type =REGCACHE_NONE , +}; +EXPORT_SYMBOL_NS_GPL (inv_icm42607_regmap_config,"IIO_ICM42607" ); +struct inv_icm42607_hw {uint8_t whoami;const char *name;
drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c new file +90 -0
90 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c new file +96 -0
96 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 03/10] iio: imu: inv_icm42607: Add I2C and SPI For icm42607
Quoted 20 lines · On Fri, 1 May 2026 17:11:42 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add I2C and SPI driver support for InvenSense ICM-42607 devices. Add necessary
KconfigandMakefileto allow building of (incomplete) driver.Signed-off-by Chris Morgan <[email protected]>drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c new file +6
-0+static const struct of_device_id inv_icm42607_of_matches[] = { + { + .compatible ="invensense,icm42607" , + .data = (void *)INV_CHIP_ICM42607 , + }, + {
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c line 78
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c new file +10
-0+ .compatible = "invensense,icm42607p", + .data = (void *)INV_CHIP_ICM42607P, + }, + { } +}; +MODULE_DEVICE_TABLE (of, inv_icm42607_of_matches); + +static const struct spi_device_id inv_icm42607_spi_id_table[] = { + {"icm42607" ,INV_CHIP_ICM42607 }, + { "icm42607p", INV_CHIP_ICM42607P },
Very strong preference in new code to not use enums for the data. That explains why you need the magic invalid in previous patch.
Can we instead just use pointers to the per chip data relevant to each one? That usually means some externs in the header.
We used to do things how you have it here, but the enums always end up proving a pain as drivers get more complex. They also encourage part specific code when it should almost always be part specific constant data.
Currently I'm not sure what is different between these parts. If nothing yet from point of view of what the driver supports then don't bother providing data 'yet'.
Jonathan
Quoted 2 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c new file +2
-0+ { } +};
PATCH v4 4/10 iio: imu: inv_icm42607: Add PM support for icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add power management support for the ICM42607 device driver.
--- drivers/iio/imu/inv_icm42607/inv_icm42607.h | 2 + .../iio/imu/inv_icm42607/inv_icm42607_core.c | 177 ++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 + .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 + 4 files changed, 181 insertions(+)
drivers/iio/imu/inv_icm42607/inv_icm42607.h +2 -0
@@ -11,6 +11,7 @@#include <linux/iio/common/inv_sensors_timestamp.h> #include <linux/iio/iio.h> #include <linux/mutex.h> +#include <linux/pm.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h>
@@ -390,6 +391,7 @@ struct inv_icm42607_sensor_state {typedef int (*inv_icm42607_bus_setup )(struct inv_icm42607_state *);extern const struct regmap_config inv_icm42607_regmap_config; +extern const struct dev_pm_ops inv_icm42607_pm_ops;u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr);
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +177 -0
199 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c +1 -0
@@ -78,6 +78,7 @@ static struct i2c_driver inv_icm42607_driver = { .driver = { .name = "inv-icm42607-i2c", .of_match_table = inv_icm42607_of_matches, + .pm = pm_ptr(&inv_icm42607_pm_ops), }, .id_table = inv_icm42607_id, .probe = inv_icm42607_probe,
drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c +1 -0
@@ -84,6 +84,7 @@ static struct spi_driver inv_icm42607_driver = { .driver = { .name = "inv-icm42607-spi", .of_match_table = inv_icm42607_of_matches, + .pm = &inv_icm42607_pm_ops, }, .id_table = inv_icm42607_spi_id_table, .probe = inv_icm42607_probe,
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 04/10] iio: imu: inv_icm42607: Add PM support for icm42607
Quoted 5 lines · On Fri, 1 May 2026 17:11:43 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add power management support for the ICM42607 device driver.
Signed-off-by Chris Morgan <[email protected]>
I think this suffers from usual problem we get when runtime_pm handles a regulator that we are also registering to be turned off via devm. That devm path needs to check if device is already runtime suspended. Be careful about paths in probe() where we might check runtime suspend state that isn't set yet.
https://sashiko.dev/#/patchset/20260501221152.194251-1-macroalpha82%40gmail.com
Has quite a bit to say no the series in general. It may not always be right but some of the stuff in this patch is.
Quoted 103 lines
--- drivers/iio/imu/inv_icm42607/inv_icm42607.h | 2 + .../iio/imu/inv_icm42607/inv_icm42607_core.c | 177 ++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 + .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 + 4 files changed, 181 insertions(+)drivers/iio/imu/inv_icm42607/inv_icm42607.h +2
-0@@ -11,6 +11,7 @@#include <linux/iio/common/inv_sensors_timestamp.h> #include <linux/iio/iio.h> #include <linux/mutex.h> +#include <linux/pm.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> @@ -390,6 +391,7 @@ struct inv_icm42607_sensor_state {typedef int (*inv_icm42607_bus_setup )(struct inv_icm42607_state *);extern const struct regmap_config inv_icm42607_regmap_config; +extern const struct dev_pm_ops inv_icm42607_pm_ops;u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr);drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +57
-0@@ -9,6 +9,7 @@#include <linux/irq.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/pm_runtime.h> #include <linux/property.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> @@ -106,6 +107,62 @@ u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)return odr_periods[odr]; } +static int inv_icm42607_set_pwr_mgmt0 (struct inv_icm42607_state *st, +enum inv_icm42607_sensor_mode gyro, +enum inv_icm42607_sensor_mode accel, +bool temp,unsigned int *sleep_ms) +{ +enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode; +enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode; +bool oldtemp = st->conf.temp_en; +unsigned int sleepval; +unsigned int val; +int ret; + +if (gyro == oldgyro && accel == oldaccel && temp == oldtemp) +return 0 ; + + val =FIELD_PREP (INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK , gyro); + val |=FIELD_PREP (INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK , accel); +if (!temp) + val |=INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL ; + ret =regmap_write (st->map,INV_ICM42607_REG_PWR_MGMT0 , val); +if (ret) +return ret; + + st->conf.gyro.mode = gyro; + st->conf.accel.mode = accel; + st->conf.temp_en = temp; + + sleepval =0 ; +if (temp && !oldtemp) { +if (sleepval <INV_ICM42607_TEMP_STARTUP_TIME_MS ) + sleepval =INV_ICM42607_TEMP_STARTUP_TIME_MS ; + } +if (accel != oldaccel && oldaccel ==INV_ICM42607_SENSOR_MODE_OFF ) { +usleep_range (200 ,300 ); +if (sleepval <INV_ICM42607_ACCEL_STARTUP_TIME_MS ) + sleepval =INV_ICM42607_ACCEL_STARTUP_TIME_MS ; + } +if (gyro != oldgyro) { +if (oldgyro ==INV_ICM42607_SENSOR_MODE_OFF ) { +usleep_range (200 ,300 ); +if (sleepval <INV_ICM42607_GYRO_STARTUP_TIME_MS ) + sleepval =INV_ICM42607_GYRO_STARTUP_TIME_MS ; + }else if (gyro ==INV_ICM42607_SENSOR_MODE_OFF ) { +if (sleepval <INV_ICM42607_GYRO_STOP_TIME_MS ) + sleepval =INV_ICM42607_GYRO_STOP_TIME_MS ; + } + } + +if (sleep_ms) + *sleep_ms = sleepval; +else if (sleepval) +msleep (sleepval); + +return 0 ; +} +int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg,unsigned int writeval,unsigned int *readval) {
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, int chip, · line 373
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1
-0@@ -313,10 +370,130 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,if (ret)return ret; + ret =devm_pm_runtime_set_active_enabled (dev);
This will interact with the status setup you need as mentioned above for devm code to work right.
Quoted 11 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +7
-0+if (ret) +return ret; + +pm_runtime_set_autosuspend_delay (dev,INV_ICM42607_SUSPEND_DELAY_MS ); +pm_runtime_use_autosuspend (dev); +return 0 ; }EXPORT_SYMBOL_NS_GPL (inv_icm42607_core_probe,"IIO_ICM42607" ); +/*
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, int chip, · line 394
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +10
-0+* Suspend saves sensors state and turns everything off. +* Check first if runtime suspend has not already done the job. +*/ +static int inv_icm42607_suspend (struct device *dev) +{ +struct inv_icm42607_state *st =dev_get_drvdata (dev); +struct device *accel_dev; +bool wakeup; +int accel_conf; +int ret =0 ;
Always set before use so no need to init here.
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, int chip, · line 405
Quoted 11 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +11
-0+ +guard (mutex)(&st->lock); + + st->suspended.gyro = st->conf.gyro.mode; + st->suspended.accel = st->conf.accel.mode; + st->suspended.temp = st->conf.temp_en; +if (pm_runtime_suspended (dev)) +return 0 ; + +/* keep chip on and wake-up capable if APEX and wakeup on */ + accel_dev = &st->indio_accel->dev;
Never set at this point in series. (Sashiko raised this)
Quoted 21 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +21
-0+ wakeup = st->apex.on &&device_may_wakeup (accel_dev); +if (wakeup) { +/* keep accel on and setup irq for wakeup */ + accel_conf = st->conf.accel.mode; +enable_irq_wake (st->irq); +disable_irq (st->irq); + }else { + accel_conf =INV_ICM42607_SENSOR_MODE_OFF ; + } + + ret =inv_icm42607_set_pwr_mgmt0 (st,INV_ICM42607_SENSOR_MODE_OFF , +INV_ICM42607_SENSOR_MODE_OFF , false, +NULL ); +if (ret) +return ret; + +if (!wakeup) +regulator_disable (st->vddio_supply); + +return 0 ; +}
PATCH v4 5/10 iio: imu: inv_icm42607: Add Buffer support for icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add all FIFO parsing and reading functions to support inv_icm42607 hardware.
--- drivers/iio/imu/inv_icm42607/Makefile | 1 + drivers/iio/imu/inv_icm42607/inv_icm42607.h | 4 + .../imu/inv_icm42607/inv_icm42607_buffer.c | 489 ++++++++++++++++++ .../imu/inv_icm42607/inv_icm42607_buffer.h | 101 ++++ .../iio/imu/inv_icm42607/inv_icm42607_core.c | 26 + 5 files changed, 621 insertions(+) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h
drivers/iio/imu/inv_icm42607/Makefile +1 -0
@@ -2,6 +2,7 @@ obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o inv-icm42607-y += inv_icm42607_core.o +inv-icm42607-y += inv_icm42607_buffer.o obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o inv-icm42607-i2c-y += inv_icm42607_i2c.o
drivers/iio/imu/inv_icm42607/inv_icm42607.h +4 -0
@@ -15,6 +15,8 @@#include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include "inv_icm42607_buffer.h" +enum inv_icm42607_chip {INV_CHIP_INVALID ,INV_CHIP_ICM42607P ,
@@ -140,6 +142,7 @@ struct inv_icm42607_apex {* @indio_accel: accelerometer IIO device. * @timestamp: interrupt timestamps. * @apex: APEX (Advanced Pedometer and Event detection) management +* @fifo: FIFO management structure. * @buffer: data transfer buffer aligned for DMA. */ struct inv_icm42607_state {
@@ -159,6 +162,7 @@ struct inv_icm42607_state {s64 accel; } timestamp;struct inv_icm42607_apex apex; +struct inv_icm42607_fifo fifo;__be16 buffer[3 ]__aligned(IIO_DMA_MINALIGN) ; };
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +489 -0
489 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +101 -0
101 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +26 -0
@@ -16,6 +16,7 @@#include <linux/slab.h> #include "inv_icm42607.h" +#include "inv_icm42607_buffer.h" static const struct regmap_range_cfg inv_icm42607_regmap_ranges[] = { {
@@ -163,6 +164,7 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,return 0 ; } +int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg,unsigned int writeval,unsigned int *readval) {
@@ -370,6 +372,11 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,if (ret)return ret; +/* Initialize buffer/FIFO handling */ + ret =inv_icm42607_buffer_init (st); +if (ret) +return ret; + ret =devm_pm_runtime_set_active_enabled (dev);if (ret)return ret;
@@ -401,6 +408,13 @@ static int inv_icm42607_suspend(struct device *dev)if (pm_runtime_suspended (dev))return 0 ; +if (st->fifo.on) { + ret =regmap_write (st->map,INV_ICM42607_REG_FIFO_CONFIG1 , +INV_ICM42607_FIFO_CONFIG1_BYPASS ); +if (ret) +return ret; + } +/* keep chip on and wake-up capable if APEX and wakeup on */ accel_dev = &st->indio_accel->dev; wakeup = st->apex.on &&device_may_wakeup (accel_dev);
@@ -432,6 +446,8 @@ static int inv_icm42607_suspend(struct device *dev)static int inv_icm42607_resume (struct device *dev) {struct inv_icm42607_state *st =dev_get_drvdata (dev); +struct inv_icm42607_sensor_state *gyro_st =iio_priv (st->indio_gyro); +struct inv_icm42607_sensor_state *accel_st =iio_priv (st->indio_accel);struct device *accel_dev;bool wakeup;int ret;
@@ -458,6 +474,16 @@ static int inv_icm42607_resume(struct device *dev) ret =inv_icm42607_set_pwr_mgmt0 (st, st->suspended.gyro, st->suspended.accel, st->suspended.temp,NULL ); +if (ret) +return ret; + +if (st->fifo.on) { +inv_sensors_timestamp_reset (&gyro_st->ts); +inv_sensors_timestamp_reset (&accel_st->ts); + ret =regmap_write (st->map,INV_ICM42607_REG_FIFO_CONFIG1 , +INV_ICM42607_FIFO_CONFIG1_MODE ); + } +return ret; }
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 05/10] iio: imu: inv_icm42607: Add Buffer support for icm42607
Quoted 3 lines · On Fri, 1 May 2026 17:11:44 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add all FIFO parsing and reading functions to support>
inv_icm42607hardware.
Generally wrap commit messages to 75 ish characters.
Quoted 2 lines
Signed-off-by Chris Morgan <[email protected]>
Hi Chris
Various comments inline. Biggest set seem to be on making use of min() / max() to do clamping. They are generally as easy to read as using a conditional and often more compact.
Quoted 9 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +3
-0@@ -0,0 +1,489 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* Copyright (C) 2026 InvenSense, Inc.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 13
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+*/ + +#include <linux/delay.h> +#include <linux/minmax.h> +#include <linux/mutex.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/common/inv_sensors_timestamp.h>
I'd put this one in it's own block between the iio generic headers and the very driver specific ones given its specific to invensense sensors.
Quoted 22 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +22
-0+#include <linux/iio/iio.h> + +#include "inv_icm42607.h" +#include "inv_icm42607_buffer.h" + +/* FIFO header: 1 byte */ +#define INV_ICM42607_FIFO_HEADER_MSG BIT(7) +#define INV_ICM42607_FIFO_HEADER_ACCEL BIT(6) +#define INV_ICM42607_FIFO_HEADER_GYRO BIT(5) +#define INV_ICM42607_FIFO_HEADER_TMST_FSYNC GENMASK(3, 2) +#define INV_ICM42607_FIFO_HEADER_ODR_ACCEL BIT(1) +#define INV_ICM42607_FIFO_HEADER_ODR_GYRO BIT(0) + +struct inv_icm42607_fifo_1sensor_packet { +u8 header; +struct inv_icm42607_fifo_sensor_data data; +s8 temp; +} __packed; +#define INV_ICM42607_FIFO_1SENSOR_PACKET_SIZE 8 + +struct inv_icm42607_fifo_2sensors_packet { +u8 header;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 45
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+struct inv_icm42607_fifo_sensor_data accel; +struct inv_icm42607_fifo_sensor_data gyro; +s8 temp; +__be16 timestamp; +} __packed; +#define INV_ICM42607_FIFO_2SENSORS_PACKET_SIZE 16 + +ssize_t inv_icm42607_fifo_decode_packet (const void *packet,const void **accel, +const void **gyro,const int8_t **temp, +const void **timestamp,unsigned int *odr)
For now at least odr doesn't seem to be used at the caller. So I'd drop it from the function signature until it is useful.
Quoted 21 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +21
-0+{ +const struct inv_icm42607_fifo_1sensor_packet *pack1 = packet; +const struct inv_icm42607_fifo_2sensors_packet *pack2 = packet; +u8 header = *((const u8 *)packet); + +/* FIFO empty */ +if (header &INV_ICM42607_FIFO_HEADER_MSG ) { + *accel =NULL ; + *gyro =NULL ; + *temp =NULL ; + *timestamp =NULL ; + *odr =0 ; +return 0 ; + } + +/* handle odr flags */ + *odr =0 ; +if (header &INV_ICM42607_FIFO_HEADER_ODR_GYRO ) + *odr |=INV_ICM42607_SENSOR_GYRO ; +if (header &INV_ICM42607_FIFO_HEADER_ODR_ACCEL ) + *odr |=INV_ICM42607_SENSOR_ACCEL ;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 76
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+ +/* accel + gyro */ +if ((header &INV_ICM42607_FIFO_HEADER_ACCEL ) && + (header &INV_ICM42607_FIFO_HEADER_GYRO )) { + *accel = &pack2->accel; + *gyro = &pack2->gyro; + *temp = &pack2->temp; + *timestamp = &pack2->timestamp; +return INV_ICM42607_FIFO_2SENSORS_PACKET_SIZE ; + }
Odd indent. Should be one less tab.
Quoted 22 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +22
-0+ +/* accel only */ +if (header &INV_ICM42607_FIFO_HEADER_ACCEL ) { + *accel = &pack1->data; + *gyro =NULL ; + *temp = &pack1->temp; + *timestamp =NULL ; +return INV_ICM42607_FIFO_1SENSOR_PACKET_SIZE ; + } + +/* gyro only */ +if (header &INV_ICM42607_FIFO_HEADER_GYRO ) { + *accel =NULL ; + *gyro = &pack1->data; + *temp = &pack1->temp; + *timestamp =NULL ; +return INV_ICM42607_FIFO_1SENSOR_PACKET_SIZE ; + } + +/* invalid packet if here */ +return -EINVAL ; +}
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 162
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +9
-0+ +static unsigned int inv_icm42607_wm_truncate (unsigned int watermark, +size_t packet_size) +{ +size_t wm_size; + + wm_size = watermark * packet_size; +if (wm_size >INV_ICM42607_FIFO_WATERMARK_MAX ) + wm_size =INV_ICM42607_FIFO_WATERMARK_MAX ;
wm_size = min(watermark * packet_size, INV_ICM42607_FIFO_WATERMARK_MAX);
Quoted 41 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +41
-0+ +return wm_size / packet_size; +} + +/** +* inv_icm42607_buffer_update_watermark - update watermark FIFO threshold +* @st: driver internal state +* +* Returns 0 on success, a negative error code otherwise. +*/ +int inv_icm42607_buffer_update_watermark (struct inv_icm42607_state *st) +{ +size_t packet_size, wm_size; +unsigned int wm_gyro, wm_accel, watermark; +u32 period_gyro, period_accel; +u32 latency_gyro, latency_accel, latency; +bool restore; +__le16 raw_wm; +int ret; + + packet_size =inv_icm42607_get_packet_size (st->fifo.en); + +/* compute sensors latency, depending on sensor watermark and odr */ + wm_gyro =inv_icm42607_wm_truncate (st->fifo.watermark.gyro, packet_size); + wm_accel =inv_icm42607_wm_truncate (st->fifo.watermark.accel, packet_size); +/* use us for odr to avoid overflow using 32 bits values */ + period_gyro =inv_icm42607_odr_to_period (st->conf.gyro.odr) /1000UL ; + period_accel =inv_icm42607_odr_to_period (st->conf.accel.odr) /1000UL ; + latency_gyro = period_gyro * wm_gyro; + latency_accel = period_accel * wm_accel; + +/* 0 value for watermark means that the sensor is turned off */ +if (wm_gyro ==0 && wm_accel ==0 ) +return 0 ; + +if (latency_gyro ==0 ) { + watermark = wm_accel; + st->fifo.watermark.eff_accel = wm_accel; + }else if (latency_accel ==0 ) { + watermark = wm_gyro; + st->fifo.watermark.eff_gyro = wm_gyro;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 213
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+ }else { +/* compute the smallest latency that is a multiple of both */ +if (latency_gyro <= latency_accel) + latency = latency_gyro - (latency_accel % latency_gyro); +else + latency = latency_accel - (latency_gyro % latency_accel); +/* all this works because periods are multiple of each others */ + watermark = latency /min (period_gyro, period_accel); +if (watermark <1 ) + watermark =1 ;
watermark = max(watermark, 1);
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 217
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +4
-0+/* update effective watermark */ + st->fifo.watermark.eff_gyro = latency / period_gyro; +if (st->fifo.watermark.eff_gyro <1 ) + st->fifo.watermark.eff_gyro =1 ;
st->fifo.watermark.eff_gyro = max(latency / period_gyro, 1);
is clear and shorter.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 220
Quoted 3 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +3
-0+ st->fifo.watermark.eff_accel = latency / period_accel; +if (st->fifo.watermark.eff_accel <1 ) + st->fifo.watermark.eff_accel =1 ;
same again
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 224
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +4
-0+ } + +/* compute watermark value in bytes */ + wm_size = watermark * packet_size;
I'd push this down to nearer where it's used.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 232
Quoted 8 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +8
-0+ +/* changing FIFO watermark requires to turn off watermark interrupt */ + ret =regmap_update_bits_check (st->map,INV_ICM42607_REG_INT_SOURCE0 , +INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN , +0 , &restore); +if (ret) +return ret; +
So here.
Quoted 17 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +17
-0+ raw_wm =INV_ICM42607_FIFO_WATERMARK_VAL (wm_size); +memcpy (st->buffer, &raw_wm,sizeof (raw_wm)); + ret =regmap_bulk_write (st->map,INV_ICM42607_REG_FIFO_CONFIG2 , + st->buffer,sizeof (raw_wm)); +if (ret) +return ret; + +/* restore watermark interrupt */ +if (restore) { + ret =regmap_update_bits (st->map,INV_ICM42607_REG_INT_SOURCE0 , +INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN , +INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN ); +if (ret) +return ret; + } + +return 0 ;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 259
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+} + +static int inv_icm42607_buffer_preenable (struct iio_dev *indio_dev) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct device *dev =regmap_get_device (st->map); +struct inv_icm42607_sensor_state *sensor_st =iio_priv (indio_dev); +struct inv_sensors_timestamp *ts = &sensor_st->ts; + +pm_runtime_get_sync (dev);
Maybe check the error return from this. Though as the docs suggest better to use
ret = pm_runtime_resume_and_get(dev);
as that doesn't leave ref count raised on failure.
Quoted 18 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +18
-0+ +guard (mutex)(&st->lock); +inv_sensors_timestamp_reset (ts); + +return 0 ; +} + +/* +* update_scan_mode callback is turning sensors on and setting data FIFO enable +* bits. +*/ +static int inv_icm42607_buffer_postenable (struct iio_dev *indio_dev) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +int ret; + +guard (mutex)(&st->lock); +
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 287
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+/* exit if FIFO is already on */ +if (st->fifo.on) { + st->fifo.on++; +return 0 ; + } + +/* set FIFO threshold interrupt */ + ret =regmap_update_bits (st->map,INV_ICM42607_REG_INT_SOURCE0 , +INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN , +INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN );
ret = regmap_set_bits(st->map, INV_ICM42607_REG_INT_SOURCE0, INV_ICM42607_INT_SOURCE0_FIFO_THS_INT1_EN);
Might as well save a line on these.
Quoted 7 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +7
-0+if (ret) +return ret; + +/* flush FIFO data */ + ret =regmap_write (st->map,INV_ICM42607_REG_SIGNAL_PATH_RESET , +INV_ICM42607_SIGNAL_PATH_RESET_FIFO_FLUSH ); +if (ret)
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 304
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+return ret; + +/* set FIFO in streaming mode */ + ret =regmap_write (st->map,INV_ICM42607_REG_FIFO_CONFIG1 , +INV_ICM42607_FIFO_CONFIG1_MODE ); +if (ret) +return ret; + +/* workaround: first read of FIFO count after reset is always 0 */ + ret =regmap_bulk_read (st->map,INV_ICM42607_REG_FIFO_COUNTH , st->buffer,2 );
I think you read this as explicitly going into a __be16 elsewhere. Good to size this either from sizeof(st->buffer[0]) or sizeof(__be16) rather than a magic 2.
Quoted 6 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +6
-0+if (ret) +return ret; + + st->fifo.on++; + +return 0 ;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 406
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +8
-0+} +int inv_icm42607_buffer_fifo_read (struct inv_icm42607_state *st, +unsigned int max) +{ +size_t max_count; +__be16 *raw_fifo_count; +ssize_t i, size; +const void *accel, *gyro, *timestamp;
Where no other reason for ordering applies use reverse xmas tree. If this is meant to be order of first use, that tends to be much more fragile to code movement than reverse xmas tree hence the preference.
Quoted 8 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +8
-0+const s8 *temp; +unsigned int odr; +int ret; + +/* reset all samples counters */ + st->fifo.count =0 ; + st->fifo.nb.gyro =0 ; + st->fifo.nb.accel =0 ;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 424
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+ st->fifo.nb.total =0 ; + +/* compute maximum FIFO read size */ +if (max ==0 ) + max_count =sizeof (st->fifo.data); +else + max_count = max *inv_icm42607_get_packet_size (st->fifo.en); + +/* read FIFO count value */ + raw_fifo_count = (__be16 *)st->buffer;
I think st->buffer is __be16 [3] so no cast needed here maybe make it clear you want the first element with &st->buffer[0];
However I'd be tempted to just use that directly and drop the local variable as adding little when we only use it so locally.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 433
Quoted 9 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +9
-0+ ret =regmap_bulk_read (st->map,INV_ICM42607_REG_FIFO_COUNTH , + raw_fifo_count,sizeof (*raw_fifo_count)); +if (ret) +return ret; + st->fifo.count =be16_to_cpup (raw_fifo_count); + +/* check and clamp FIFO count value */ +if (st->fifo.count ==0 ) +return 0 ;
I'd add a blank line here, just to make it easier to spot this successful exit path.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 435
Quoted 2 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +2
-0+if (st->fifo.count > max_count) + st->fifo.count = max_count;
st->fifo.count = max(st->fifo.count, max_count);
is slightly more readable.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 444
Quoted 9 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +9
-0+ +/* read all FIFO data in internal buffer */ + ret =regmap_noinc_read (st->map,INV_ICM42607_REG_FIFO_DATA , + st->fifo.data, st->fifo.count); +if (ret) +return ret; + +/* compute number of samples for each sensor */ +for (i =0 ; i < st->fifo.count; i += size) {
for (size_t i = 0; < st->fifo.count...
I'm curious on why it's signed. Should be fine to use an unsigned iterator as only add positive values of size.
Quoted 15 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +15
-0+ size =inv_icm42607_fifo_decode_packet (&st->fifo.data[i], + &accel, &gyro, &temp, ×tamp, &odr); +if (size <=0 ) +break ; +if (gyro !=NULL &&inv_icm42607_fifo_is_data_valid (gyro)) + st->fifo.nb.gyro++; +if (accel !=NULL &&inv_icm42607_fifo_is_data_valid (accel)) + st->fifo.nb.accel++; + st->fifo.nb.total++; + } + +return 0 ; +} + +int inv_icm42607_buffer_hwfifo_flush(struct inv_icm42607_state *st,
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c line 469
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +10
-0+ unsigned int count) +{ +s64 gyro_ts, accel_ts; +int ret; + + gyro_ts =iio_get_time_ns (st->indio_gyro); + accel_ts =iio_get_time_ns (st->indio_accel); + + ret =inv_icm42607_buffer_fifo_read (st, count); +
To avoid a bit of churn later you could do
if (ret) return ret; return 0;
now but not that important.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h line 3
Quoted 12 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c new file +2
-0+return ret; +}drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +3
-0@@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2026 InvenSense, Inc.
Given you are using gmail / hotmail, just to confirm, is this work being paid for by InvenSense? If it's based on other work of theirs then there should be more detail on why this copyright notice so we have that recorded for the long term.
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h line 9
Quoted 6 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +6
-0+*/ + +#ifndef INV_ICM42607_BUFFER_H_ +#define INV_ICM42607_BUFFER_H_ + +#include <linux/kernel.h>
For new code we avoid including linux/kernel.h but instead use more specific headers. Not entirely sure what you need here but definitely something for the endian swap.
Also you need to have a forwards def for the
struct inv_icm42607_state; maybe others.
Quoted 39 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +39
-0+#include <linux/bitops.h> + +struct inv_icm42607_state ; + +#define INV_ICM42607_SENSOR_GYRO BIT(0) +#define INV_ICM42607_SENSOR_ACCEL BIT(1) +#define INV_ICM42607_SENSOR_TEMP BIT(2) + +/** +* struct inv_icm42607_fifo - FIFO state variables +* @on: reference counter for FIFO on. +* @en: bits field of INV_ICM42607_SENSOR_* for FIFO EN bits. +* @period: FIFO internal period. +* @watermark: watermark configuration values for accel and gyro. +* @count: number of bytes in the FIFO data buffer. +* @nb: gyro, accel and total samples in the FIFO data buffer. +* @data: FIFO data buffer aligned for DMA (2kB + 32 bytes of read cache). +*/ +struct inv_icm42607_fifo { +unsigned int on; +unsigned int en; +u32 period; +struct { +unsigned int gyro; +unsigned int accel; +unsigned int eff_gyro; +unsigned int eff_accel; + } watermark; +size_t count; +struct { +size_t gyro; +size_t accel; +size_t total; + } nb; +u8 data[2080 ]__aligned(IIO_DMA_MINALIGN) ; +}; + +/* FIFO data packet */ +struct inv_icm42607_fifo_sensor_data {
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h line 58
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +10
-0+__be16 x; +__be16 y; +__be16 z; +}; + +#define INV_ICM42607_FIFO_DATA_INVALID -32768 + +static inline s16 inv_icm42607_fifo_get_sensor_data (__be16 d) +{ +return be16_to_cpu (d);
Is this helper worth bothering with? Seems like it would be pretty clear what was going on if the be16_to_cpu() was just inline?
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +4
-0+} + +static inline bool +inv_icm42607_fifo_is_data_valid (const struct inv_icm42607_fifo_sensor_data *s)
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h line 72
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +10
-0+{ +s16 x, y, z; + + x =inv_icm42607_fifo_get_sensor_data (s->x); + y =inv_icm42607_fifo_get_sensor_data (s->y); + z =inv_icm42607_fifo_get_sensor_data (s->z); + +if (x ==INV_ICM42607_FIFO_DATA_INVALID && + y ==INV_ICM42607_FIFO_DATA_INVALID && + z ==INV_ICM42607_FIFO_DATA_INVALID )
Align this as:
if (x == INV_ICM42607_FIFO_DATA_INVALID && y == INV_ICM42607_FIFO_DATA_INVALID && z == INV_ICM42607_FIFO_DATA_INVALID)
You could flip this around and do
__be16 be_inval_marker = cpu_to_be16(INV_ICM42607_FIFO_DATA_INVALID); if (s->x == be_inval_marker && s->y == be_inval_marker && s->z == be_inval_marker) return false;
Quoted 24 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.h new file +9
-0+return false; + +return true; +} + +ssize_t inv_icm42607_fifo_decode_packet (const void *packet,const void **accel, +const void **gyro,const s8 **temp, +const void **timestamp,unsigned int *odr); +drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1
-0@@ -16,6 +16,7 @@#include <linux/slab.h> #include "inv_icm42607.h" +#include "inv_icm42607_buffer.h" static const struct regmap_range_cfg inv_icm42607_regmap_ranges[] = { {
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st, · line 167
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +1
-0@@ -163,6 +164,7 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,return 0 ; } +
Stray change. Remember to check your patches for these. we all introduce them whilst editing a series, so it's always a case of taking a look at the final result to catch them.
Quoted 1 line
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +0
-0int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
PATCH v4 6/10 iio: imu: inv_icm42607: Add Temp Support in icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add functions for reading temperature sensor data.
--- drivers/iio/imu/inv_icm42607/Makefile | 1 + drivers/iio/imu/inv_icm42607/inv_icm42607.h | 3 + .../iio/imu/inv_icm42607/inv_icm42607_core.c | 17 ++++ .../iio/imu/inv_icm42607/inv_icm42607_temp.c | 81 +++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_temp.h | 30 +++++++ 5 files changed, 132 insertions(+) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
drivers/iio/imu/inv_icm42607/Makefile +1 -0
@@ -3,6 +3,7 @@ obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o inv-icm42607-y += inv_icm42607_core.o inv-icm42607-y += inv_icm42607_buffer.o +inv-icm42607-y += inv_icm42607_temp.o obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o inv-icm42607-i2c-y += inv_icm42607_i2c.o
drivers/iio/imu/inv_icm42607/inv_icm42607.h +3 -0
@@ -399,6 +399,9 @@ extern const struct dev_pm_ops inv_icm42607_pm_ops;u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr); +int inv_icm42607_set_temp_conf (struct inv_icm42607_state *st,bool enable, +unsigned int *sleep_ms); +int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg,unsigned int writeval,unsigned int *readval);
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +17 -0
@@ -164,6 +164,23 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,return 0 ; } +int inv_icm42607_set_temp_conf (struct inv_icm42607_state *st,bool enable, +unsigned int *sleep_ms) +{ +unsigned int val; +int ret; + + val =FIELD_PREP (INV_ICM42607_TEMP_CONFIG0_FILTER_MASK , +INV_ICM42607_FILTER_BW_34HZ ); + ret =regmap_update_bits (st->map,INV_ICM42607_REG_TEMP_CONFIG0 , +INV_ICM42607_TEMP_CONFIG0_FILTER_MASK , val); +if (ret) +return ret; + +return inv_icm42607_set_pwr_mgmt0 (st, st->conf.gyro.mode, + st->conf.accel.mode, enable, + sleep_ms); +}int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg,unsigned int writeval,unsigned int *readval)
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +81 -0
81 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h new file +30 -0
@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2026 InvenSense, Inc. +*/ + +#ifndef INV_ICM42607_TEMP_H_ +#define INV_ICM42607_TEMP_H_ + +#include <linux/iio/iio.h> + +#define INV_ICM42607_TEMP_CHAN (_index) \ +{ \ + .type = IIO_TEMP, \ + .info_mask_separate = \ + BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_OFFSET) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = _index, \ + .scan_type = { \ + .sign = 's', \ + .realbits = 16, \ + .storagebits = 16, \ + }, \ +} + +int inv_icm42607_temp_read_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +int *val,int *val2,long mask); + +#endif
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 06/10] iio: imu: inv_icm42607: Add Temp Support in icm42607
Quoted 5 lines · On Fri, 1 May 2026 17:11:45 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add functions for reading temperature sensor data.
Signed-off-by Chris Morgan <[email protected]>
A few minor things.
Thanks,
Jonathan
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c line 6
Quoted 12 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +6
-0@@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* Copyright (C) 2026 InvenSense, Inc. +*/ + +#include <linux/kernel.h>
As in previous avoid this by using more specific headers.
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c line 10
Quoted 4 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +4
-0+#include <linux/device.h> +#include <linux/mutex.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h>
blank line here to match other files.
Quoted 19 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +19
-0+#include <linux/iio/iio.h> + +#include "inv_icm42607.h" +#include "inv_icm42607_temp.h" + +static int inv_icm42607_temp_read (struct inv_icm42607_state *st,s16 *temp) +{ +struct device *dev =regmap_get_device (st->map); +__be16 *raw; +int ret; + +PM_RUNTIME_ACQUIRE_AUTOSUSPEND (dev, pm); +if (PM_RUNTIME_ACQUIRE_ERR (&pm)) +return -ENXIO ; + +guard (mutex)(&st->lock); + + ret =inv_icm42607_set_temp_conf (st, true,NULL ); +if (ret)
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c line 39
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +10
-0+return ret; + + raw = &st->buffer[0 ]; + ret =regmap_bulk_read (st->map,INV_ICM42607_REG_TEMP_DATA1 , raw,sizeof (*raw)); +if (ret) +return ret; + + *temp =be16_to_cpup (raw); +if (*temp ==INV_ICM42607_DATA_INVALID ) + ret = -EINVAL ;
return -EINVAL; return 0;
is no longer and to me a tiny bit simpler to read.
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c new file +3
-0+ +return ret; +}drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h new file +0
-0
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h line 9
Quoted 9 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h new file +9
-0@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* +* Copyright (C) 2026 InvenSense, Inc. +*/ + +#ifndef INV_ICM42607_TEMP_H_ +#define INV_ICM42607_TEMP_H_ + +#include <linux/iio/iio.h>
Need header for BIT() and I'm not seeing a need for this one. Just add forward defs for struct iio_dev and struct iio_chan_spec
Quoted 21 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h new file +21
-0+ +#define INV_ICM42607_TEMP_CHAN (_index) \ +{ \ + .type = IIO_TEMP, \ + .info_mask_separate = \ + BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_OFFSET) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = _index, \ + .scan_type = { \ + .sign = 's', \ + .realbits = 16, \ + .storagebits = 16, \ + }, \ +} + +int inv_icm42607_temp_read_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +int *val,int *val2,long mask); + +#endif
PATCH v4 7/10 iio: imu: inv_icm42607: Add Accelerometer for icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add icm42607 accelerometer sensor for icm42607.
--- drivers/iio/imu/inv_icm42607/Makefile | 1 + drivers/iio/imu/inv_icm42607/inv_icm42607.h | 16 + .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 686 ++++++++++++++++++ .../imu/inv_icm42607/inv_icm42607_buffer.c | 49 +- .../iio/imu/inv_icm42607/inv_icm42607_core.c | 61 ++ 5 files changed, 812 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
drivers/iio/imu/inv_icm42607/Makefile +1 -0
@@ -2,6 +2,7 @@ obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o inv-icm42607-y += inv_icm42607_core.o +inv-icm42607-y += inv_icm42607_accel.o inv-icm42607-y += inv_icm42607_buffer.o inv-icm42607-y += inv_icm42607_temp.o
drivers/iio/imu/inv_icm42607/inv_icm42607.h +16 -0
@@ -397,8 +397,16 @@ typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);extern const struct regmap_config inv_icm42607_regmap_config;extern const struct dev_pm_ops inv_icm42607_pm_ops; +const struct iio_mount_matrix * +inv_icm42607_get_mount_matrix (struct iio_dev *indio_dev, +const struct iio_chan_spec *chan); +u32 inv_icm42607_odr_to_period (enum inv_icm42607_odr odr); +int inv_icm42607_set_accel_conf (struct inv_icm42607_state *st, +struct inv_icm42607_sensor_conf *conf, +unsigned int *sleep_ms); +int inv_icm42607_set_temp_conf (struct inv_icm42607_state *st,bool enable,unsigned int *sleep_ms);
@@ -408,4 +416,12 @@ int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,int inv_icm42607_core_probe (struct regmap *regmap,int chip,inv_icm42607_bus_setup bus_setup); +struct iio_dev *inv_icm42607_accel_init (struct inv_icm42607_state *st); + +int inv_icm42607_accel_parse_fifo (struct iio_dev *indio_dev); + +void inv_icm42607_accel_handle_events (struct iio_dev *indio_dev, +unsigned int status2,unsigned int status3, +s64 timestamp); +#endif
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +686 -0
686 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c +48 -1
77 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +61 -0
79 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 07/10] iio: imu: inv_icm42607: Add Accelerometer for icm42607
Quoted 5 lines · On Fri, 1 May 2026 17:11:46 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add icm42607 accelerometer sensor for icm42607.
Signed-off-by Chris Morgan <[email protected]>
Hi Chris
Various comments inline. For now at least drop the power control custom ABI. I've strongly resisted this sort of thing for a very long time because it is exceptionally hard to generalize
to anything that makes sense for userspace. Look at what effects
that control actually has and present those as the means to pick a mode from userspace.
Jonathan
Quoted 18 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +12
-0@@ -0,0 +1,686 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* Copyright (C) 2026 InvenSense, Inc. +*/ + +#include <linux/kernel.h> +#include <linux/device.h> +#include <linux/mutex.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/delay.h> +#include <linux/math64.h>
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 69
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +9
-0+#include <linux/minmax.h> +#include <linux/units.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/common/inv_sensors_timestamp.h> +#include <linux/iio/events.h> +#include <linux/iio/iio.h> +#include <linux/iio/kfifo_buf.h> +static const char *const inv_icm42607_accel_power_mode_items[] = {
This is non standard ABI that userspace will almost never know what to do with. So we don't present it like this. If it is possible to work out which mode makes sense from other characteristics that are ABI use those. Otherwise, default to low-noise and don't provide a control for this.
If you want to present a case for a power control framework it needs to be a lot more descriptive of what is going on than this and suitable for generalizing to lots of devices where the trade offs can be different. Do that as a follow up series however to avoid delaying the rest of this.
Quoted 117 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +117
-0+"low-noise" , + "low-power", +}; + +static const int inv_icm42607_accel_power_mode_values[] = { +INV_ICM42607_SENSOR_MODE_LOW_NOISE , +INV_ICM42607_SENSOR_MODE_LOW_POWER , +}; + +static const int inv_icm42607_accel_filter_values[] = { +INV_ICM42607_FILTER_BW_25HZ , +INV_ICM42607_FILTER_AVG_16X , +}; + +static int inv_icm42607_accel_power_mode_set (struct iio_dev *indio_dev, +const struct iio_chan_spec *chan, +unsigned int idx) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +int power_mode, filter; + +if (chan->type !=IIO_ACCEL ) +return -EINVAL ; + +if (idx >=ARRAY_SIZE (inv_icm42607_accel_power_mode_values)) +return -EINVAL ; + + power_mode = inv_icm42607_accel_power_mode_values[idx]; + filter = inv_icm42607_accel_filter_values[idx]; + +guard (mutex)(&st->lock); + +/* cannot change if accel sensor is on */ +if (st->conf.accel.mode !=INV_ICM42607_SENSOR_MODE_OFF ) +return -EBUSY ; + +/* prevent change if power mode is not supported by the ODR */ +switch (power_mode) { +case INV_ICM42607_SENSOR_MODE_LOW_NOISE : +if (st->conf.accel.odr >=INV_ICM42607_ODR_6_25HZ_LP ) +return -EPERM ; +break ; +case INV_ICM42607_SENSOR_MODE_LOW_POWER : +default : +if (st->conf.accel.odr <=INV_ICM42607_ODR_800HZ ) +return -EPERM ; +break ; + } + + accel_st->power_mode = power_mode; + accel_st->filter = filter; + +return 0 ; +} + +static int inv_icm42607_accel_power_mode_get (struct iio_dev *indio_dev, +const struct iio_chan_spec *chan) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +unsigned int idx; +int power_mode; + +if (chan->type !=IIO_ACCEL ) +return -EINVAL ; + +guard (mutex)(&st->lock); + +/* if sensor is on, returns actual power mode and not configured one */ +switch (st->conf.accel.mode) { +case INV_ICM42607_SENSOR_MODE_LOW_POWER : +case INV_ICM42607_SENSOR_MODE_LOW_NOISE : + power_mode = st->conf.accel.mode; +break ; +default : + power_mode = accel_st->power_mode; +break ; + } + +for (idx =0 ; idx <ARRAY_SIZE (inv_icm42607_accel_power_mode_values); ++idx) { +if (power_mode == inv_icm42607_accel_power_mode_values[idx]) +break ; + } +if (idx >=ARRAY_SIZE (inv_icm42607_accel_power_mode_values)) +return -EINVAL ; + +return idx; +} + +static const struct iio_enum inv_icm42607_accel_power_mode_enum = { + .items = inv_icm42607_accel_power_mode_items, + .num_items =ARRAY_SIZE (inv_icm42607_accel_power_mode_items), + .set = inv_icm42607_accel_power_mode_set, + .get = inv_icm42607_accel_power_mode_get, +}; + +static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = { +IIO_MOUNT_MATRIX (IIO_SHARED_BY_ALL , inv_icm42607_get_mount_matrix), +IIO_ENUM_AVAILABLE ("power_mode" ,IIO_SHARED_BY_TYPE , + &inv_icm42607_accel_power_mode_enum), +IIO_ENUM ("power_mode" ,IIO_SHARED_BY_TYPE , + &inv_icm42607_accel_power_mode_enum), + { } +}; + +static const struct iio_chan_spec inv_icm42607_accel_channels[] = { +INV_ICM42607_ACCEL_CHAN (IIO_MOD_X ,INV_ICM42607_ACCEL_SCAN_X , + inv_icm42607_accel_ext_infos), +INV_ICM42607_ACCEL_CHAN (IIO_MOD_Y ,INV_ICM42607_ACCEL_SCAN_Y , + inv_icm42607_accel_ext_infos), +INV_ICM42607_ACCEL_CHAN (IIO_MOD_Z ,INV_ICM42607_ACCEL_SCAN_Z , + inv_icm42607_accel_ext_infos), +INV_ICM42607_TEMP_CHAN (INV_ICM42607_ACCEL_SCAN_TEMP ), +IIO_CHAN_SOFT_TIMESTAMP (INV_ICM42607_ACCEL_SCAN_TIMESTAMP ), +}; +
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 196
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+static const struct iio_event_spec inv_icm42607_motion_events[] = { + { + .type =IIO_EV_TYPE_THRESH , + .dir =IIO_EV_DIR_EITHER , + .mask_separate =BIT (IIO_EV_INFO_ENABLE ) |BIT (IIO_EV_INFO_VALUE ), + }, +}; + +/* +* IIO buffer data: size must be a power of 2 and timestamp aligned
This is generally true. I don't think we need to call it out in individual drivers.
Quoted 1 line
+ * 16 bytes: 6 bytes acceleration, 2 bytes temperature, 8 bytes timestamp
This is just describing what we can see in the structure. Not sure it brings much additional value so I'd drop it.
Quoted 9 lines
+ */
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +8
-0+struct inv_icm42607_accel_buffer { +struct inv_icm42607_fifo_sensor_data accel; +s16 temp; +aligned_s64 timestamp; +}; + +#define INV_ICM42607_SCAN_MASK_ACCEL_3AXIS \ + (BIT(INV_ICM42607_ACCEL_SCAN_X) | \
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 216
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+ BIT(INV_ICM42607_ACCEL_SCAN_Y) | \ + BIT(INV_ICM42607_ACCEL_SCAN_Z)) + +#define INV_ICM42607_SCAN_MASK_TEMP BIT(INV_ICM42607_ACCEL_SCAN_TEMP) + +static const unsigned long inv_icm42607_accel_scan_masks[] = { +INV_ICM42607_SCAN_MASK_ACCEL_3AXIS , +INV_ICM42607_SCAN_MASK_TEMP , +INV_ICM42607_SCAN_MASK_ACCEL_3AXIS |INV_ICM42607_SCAN_MASK_TEMP , +0 ,
That 0 is a terminating entry to the array so no trailing comma.
Quoted 36 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +36
-0+}; + +/* enable accelerometer sensor and FIFO write */ +static int inv_icm42607_accel_update_scan_mode (struct iio_dev *indio_dev, +const unsigned long *scan_mask) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +struct inv_icm42607_sensor_conf conf =INV_ICM42607_SENSOR_CONF_INIT ; +unsigned int fifo_en =0 ; +unsigned int sleep_temp =0 ; +unsigned int sleep_accel =0 ; +unsigned int sleep; +int ret; + +mutex_lock (&st->lock); + +if (*scan_mask &INV_ICM42607_SCAN_MASK_TEMP ) { +/* enable temp sensor */ + ret =inv_icm42607_set_temp_conf (st, true, &sleep_temp); +if (ret) + goto out_unlock; + fifo_en |=INV_ICM42607_SENSOR_TEMP ; + } + +if (*scan_mask &INV_ICM42607_SCAN_MASK_ACCEL_3AXIS ) { +/* enable accel sensor */ + conf.mode = accel_st->power_mode; + conf.filter = accel_st->filter; + ret =inv_icm42607_set_accel_conf (st, &conf, &sleep_accel); +if (ret) + goto out_unlock; + fifo_en |=INV_ICM42607_SENSOR_ACCEL ; + } + +/* update data FIFO write */
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 262
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+ ret =inv_icm42607_buffer_set_fifo_en (st, fifo_en | st->fifo.en); + +out_unlock: +mutex_unlock (&st->lock); +/* +* Choose the highest enable-delay time of the two sensors being +* enabled, and sleep for that amount of time. +*/ + sleep =max (sleep_accel, sleep_temp); +msleep (sleep);
Might as well do
msleep(max(sleep_accel, sleep_temp)); and save a line or two.
Quoted 38 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +38
-0+ +return ret; +} + +static int inv_icm42607_accel_read_sensor (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +s16 *val) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +struct device *dev =regmap_get_device (st->map); +struct inv_icm42607_sensor_conf conf =INV_ICM42607_SENSOR_CONF_INIT ; +unsigned int reg; +__be16 *data; +int ret; + +if (chan->type !=IIO_ACCEL ) +return -EINVAL ; + +switch (chan->channel2) { +case IIO_MOD_X : + reg =INV_ICM42607_REG_ACCEL_DATA_X1 ; +break ; +case IIO_MOD_Y : + reg =INV_ICM42607_REG_ACCEL_DATA_Y1 ; +break ; +case IIO_MOD_Z : + reg =INV_ICM42607_REG_ACCEL_DATA_Z1 ; +break ; +default : +return -EINVAL ; + } + +PM_RUNTIME_ACQUIRE_AUTOSUSPEND (dev, pm); +if (PM_RUNTIME_ACQUIRE_ERR (&pm)) +return -ENXIO ; + +guard (mutex)(&st->lock);
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 310
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+ +/* enable accel sensor */ + conf.mode = accel_st->power_mode; + conf.filter = accel_st->filter; + ret =inv_icm42607_set_accel_conf (st, &conf,NULL ); +if (ret) +return ret; + +/* read accel register data */ + data = (__be16 *)&st->buffer[0 ];
I believe it is already of that type so the cast isn't doing anything.
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 317
Quoted 7 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +7
-0+ ret =regmap_bulk_read (st->map, reg, data,sizeof (*data)); +if (ret) +return ret; + + *val =be16_to_cpup (data); +if (*val ==INV_ICM42607_DATA_INVALID ) + ret = -EINVAL ;
return -EINVAL; return 0;
to me is slightly easier to read as same patter as other early exits.
Quoted 37 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +36
-0+ +return ret; +} + +static int inv_icm42607_accel_read_scale (struct iio_dev *indio_dev, +int *val,int *val2) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +unsigned int idx; + + idx = st->conf.accel.fs; + + *val = inv_icm42607_accel_scale_nano[idx][0 ]; + *val2 = inv_icm42607_accel_scale_nano[idx][1 ]; +return IIO_VAL_INT_PLUS_NANO ; +} + +static int inv_icm42607_accel_write_scale (struct iio_dev *indio_dev, +int val,int val2) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct device *dev =regmap_get_device (st->map); +unsigned int idx; +struct inv_icm42607_sensor_conf conf =INV_ICM42607_SENSOR_CONF_INIT ; +int ret; +size_t scales_len =ARRAY_SIZE (inv_icm42607_accel_scale_nano); + +for (idx =0 ; idx < scales_len; idx++) { +if (val == inv_icm42607_accel_scale_nano[idx][0 ] && + val2 == inv_icm42607_accel_scale_nano[idx][1 ]) +break ; + } +if (idx >= scales_len) +return -EINVAL ; + + conf.fs = idx;
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 370
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+ +PM_RUNTIME_ACQUIRE_AUTOSUSPEND (dev, pm); +if (PM_RUNTIME_ACQUIRE_ERR (&pm)) +return -ENXIO ; + +guard (mutex)(&st->lock); + + ret =inv_icm42607_set_accel_conf (st, &conf,NULL ); + +return ret;
If this doesn't get updated later.
return inv_icm...
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 376
Quoted 6 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +6
-0+} + +/* IIO format int + micro */ +static const int inv_icm42607_accel_odr[] = { +/* 1.5625Hz */ +1 ,562500 ,
For these we often do them in an [][2] and cast it to int * when using it in available. Tends to make indexing a little easier elsewhere. Same as you've done for scale_nano above.
Quoted 27 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +26
-0+/* 3.125Hz */ +3 ,125000 , +/* 6.25Hz */ +6 ,250000 , +/* 12.5Hz */ +12 ,500000 , +/* 25Hz */ +25 ,0 , +/* 50Hz */ +50 ,0 , +/* 100Hz */ +100 ,0 , +/* 200Hz */ +200 ,0 , +/* 400Hz */ +400 ,0 , +/* 800Hz */ +800 ,0 , +/* 1600Hz */ + 1600, 0, +}; + +static int inv_icm42607_accel_write_odr (struct iio_dev *indio_dev, +int val,int val2) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev);
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 447
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +struct inv_sensors_timestamp *ts = &accel_st->ts; +struct device *dev =regmap_get_device (st->map); +unsigned int idx; +struct inv_icm42607_sensor_conf conf =INV_ICM42607_SENSOR_CONF_INIT ; +int ret; + +for (idx =0 ; idx <ARRAY_SIZE (inv_icm42607_accel_odr); idx +=2 ) { +if (val == inv_icm42607_accel_odr[idx] && + val2 == inv_icm42607_accel_odr[idx +1 ])
Align as:
if (val == val2 ==
Quoted 16 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +16
-0+break ; + } +if (idx >=ARRAY_SIZE (inv_icm42607_accel_odr)) +return -EINVAL ; + + conf.odr = inv_icm42607_accel_odr_conv[idx /2 ]; + +PM_RUNTIME_ACQUIRE_AUTOSUSPEND (dev, pm); +if (PM_RUNTIME_ACQUIRE_ERR (&pm)) +return -ENXIO ; + +guard (mutex)(&st->lock); + + ret =inv_sensors_timestamp_update_odr (ts,inv_icm42607_odr_to_period (conf.odr), +iio_buffer_enabled (indio_dev)); +if (ret)
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 473
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+return ret; + + ret =inv_icm42607_set_accel_conf (st, &conf,NULL ); +if (ret) +return ret; + +inv_icm42607_buffer_update_fifo_period (st); +inv_icm42607_buffer_update_watermark (st); + +return ret;
return 0;
Quoted 27 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +26
-0+} + +struct iio_dev *inv_icm42607_accel_init (struct inv_icm42607_state *st) +{ +struct device *dev =regmap_get_device (st->map); +const char *name; +struct inv_icm42607_sensor_state *accel_st; +struct inv_sensors_timestamp_chip ts_chip; +struct iio_dev *indio_dev; +int ret; + + name =devm_kasprintf (dev,GFP_KERNEL ,"%s-accel" , st->name); +if (!name) +return ERR_PTR (-ENOMEM ); + + indio_dev =devm_iio_device_alloc (dev,sizeof (*accel_st)); +if (!indio_dev) +return ERR_PTR (-ENOMEM ); + accel_st =iio_priv (indio_dev); + + accel_st->power_mode =INV_ICM42607_SENSOR_MODE_LOW_POWER ; + accel_st->filter =INV_ICM42607_FILTER_AVG_16X ; + +/* +* clock period is 32kHz (31250ns) +* jitter is +/- 2% (20 per mille)
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 624
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+*/ + ts_chip.clock_period =31250 ; + ts_chip.jitter =20 ; + ts_chip.init_period =inv_icm42607_odr_to_period (st->conf.accel.odr); +inv_sensors_timestamp_init (&accel_st->ts, &ts_chip); + +iio_device_set_drvdata (indio_dev, st); + indio_dev->name = name; + indio_dev->info = &inv_icm42607_accel_info; + indio_dev->modes =INDIO_DIRECT_MODE |INDIO_BUFFER_SOFTWARE ;
INDIO_BUFFER_SOFTWARE is set in devm_iio_kifo_buffer_setup() so no need to set it here.
Quoted 38 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +38
-0+ indio_dev->channels = inv_icm42607_accel_channels; + indio_dev->num_channels =ARRAY_SIZE (inv_icm42607_accel_channels); + indio_dev->available_scan_masks = inv_icm42607_accel_scan_masks; + + ret =devm_iio_kfifo_buffer_setup (dev, indio_dev, + &inv_icm42607_buffer_ops); +if (ret) +return ERR_PTR (ret); + + ret =devm_iio_device_register (dev, indio_dev); +if (ret) +return ERR_PTR (ret); + +/* accel events are wakeup capable */ + ret =devm_device_init_wakeup (&indio_dev->dev); +if (ret) +return ERR_PTR (ret); + +return indio_dev; +} + +int inv_icm42607_accel_parse_fifo (struct iio_dev *indio_dev) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct inv_icm42607_sensor_state *accel_st =iio_priv (indio_dev); +struct inv_sensors_timestamp *ts = &accel_st->ts; +ssize_t i, size; +unsigned int no; +const void *accel, *gyro, *timestamp; +const int8_t *temp; +unsigned int odr; +int64_t ts_val; +struct inv_icm42607_accel_buffer buffer = { }; + +/* parse all fifo packets */ +for (i =0 , no =0 ; i < st->fifo.count; i += size, ++no) { + size =inv_icm42607_fifo_decode_packet (&st->fifo.data[i], + &accel, &gyro, &temp, ×tamp, &odr);
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c line 672
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +10
-0+/* quit if error or FIFO is empty */ +if (size <=0 ) +return size; + +/* skip packet if no accel data or data is invalid */ +if (accel ==NULL || !inv_icm42607_fifo_is_data_valid (accel)) +continue ; + +/* update odr */ +if (odr &INV_ICM42607_SENSOR_ACCEL ) {
No {} needed for single statement. The kernel coding style gets a bit vague on multi line single statements but here I don't see dropping the {} as adding an confusion.
Quoted 28 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c new file +14
-0+inv_sensors_timestamp_apply_odr (ts, st->fifo.period, + st->fifo.nb.total, no); + } + +memcpy (&buffer.accel, accel,sizeof (buffer.accel)); +/* convert 8 bits FIFO temperature in high resolution format */ + buffer.temp = temp ? (*temp *64 ) :0 ; + ts_val =inv_sensors_timestamp_pop (ts); +iio_push_to_buffers_with_ts (indio_dev, &buffer, +sizeof (buffer), ts_val); + } + +return 0 ; +}drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c +2
-0int inv_icm42607_buffer_hwfifo_flush (struct inv_icm42607_state *st,unsigned int count) { +struct inv_icm42607_sensor_state *accel_st =iio_priv (st->indio_accel); +struct inv_sensors_timestamp *ts;s64 gyro_ts, accel_ts;int ret;
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c int inv_icm42607_buffer_hwfifo_flush(struct inv_icm42607_state *st, · line 470
Quoted 7 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c +2
-1@@ -466,8 +500,21 @@ int inv_icm42607_buffer_hwfifo_flush(struct inv_icm42607_state *st, accel_ts =iio_get_time_ns (st->indio_accel); ret =inv_icm42607_buffer_fifo_read (st, count); +if (ret) +return ret; -return ret;
This is the change I referred to earlier when suggesting a minor edit to reduce churn.
Quoted 21 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c +12
-0+if (st->fifo.nb.total ==0 ) +return 0 ; + +if (st->fifo.nb.accel >0 ) { + ts = &accel_st->ts; +inv_sensors_timestamp_interrupt (ts, st->fifo.nb.accel, accel_ts); + ret =inv_icm42607_accel_parse_fifo (st->indio_accel); +if (ret) +return ret; + } + +return 0 ; }int inv_icm42607_buffer_init (struct inv_icm42607_state *st)drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +0
-0
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st, · line 180
Quoted 8 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +5
-0@@ -164,6 +173,53 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,return 0 ; } +int inv_icm42607_set_accel_conf (struct inv_icm42607_state *st, +struct inv_icm42607_sensor_conf *conf, +unsigned int *sleep_ms) +{ +struct inv_icm42607_sensor_conf *oldconf = &st->conf.accel;
Given you update it throughout, the oldconf naming seems inconsistent. It becomes new conf. Maybe come up with a different name?
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st, · line 191
Quoted 11 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +11
-0+unsigned int val; +int ret; + +if (conf->mode <0 ) + conf->mode = oldconf->mode; +if (conf->fs <0 ) + conf->fs = oldconf->fs; +if (conf->odr <0 ) + conf->odr = oldconf->odr; +if (conf->filter <0 ) + conf->filter = oldconf->filter;
There is a patch kicking around for the older invensense drivers. Whilst it does too much deduplication the helper for this section makes sense to me given it's going to turn up a lot. Probably makes sense to do that here as well.
Quoted 30 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +30
-0+ +if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) { + val =FIELD_PREP (INV_ICM42607_ACCEL_CONFIG0_FS_SEL_MASK , conf->fs); + val |=FIELD_PREP (INV_ICM42607_ACCEL_CONFIG0_ODR_MASK , conf->odr); + ret =regmap_write (st->map,INV_ICM42607_REG_ACCEL_CONFIG0 , val); +if (ret) +return ret; + oldconf->fs = conf->fs; + oldconf->odr = conf->odr; + } + +if (conf->filter != oldconf->filter) { +if (conf->mode ==INV_ICM42607_SENSOR_MODE_LOW_POWER ) { + val =FIELD_PREP (INV_ICM42607_ACCEL_CONFIG1_AVG_MASK , conf->filter); + ret =regmap_update_bits (st->map,INV_ICM42607_REG_ACCEL_CONFIG1 , +INV_ICM42607_ACCEL_CONFIG1_AVG_MASK , val); + }else { + val =FIELD_PREP (INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK , + conf->filter); + ret =regmap_update_bits (st->map,INV_ICM42607_REG_ACCEL_CONFIG1 , +INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK , val); + } +if (ret) +return ret; + oldconf->filter = conf->filter; + } + +return inv_icm42607_set_pwr_mgmt0 (st, st->conf.gyro.mode, conf->mode, + st->conf.temp_en, sleep_ms); +}
PATCH v4 8/10 iio: imu: inv_icm42607: Add Wake on Movement for icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add support for wake on movement for the icm42607 driver.
--- drivers/iio/imu/inv_icm42607/inv_icm42607.h | 3 + .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 303 ++++++++++++++++++ .../iio/imu/inv_icm42607/inv_icm42607_core.c | 159 ++++++++- 3 files changed, 464 insertions(+), 1 deletion(-)
drivers/iio/imu/inv_icm42607/inv_icm42607.h +3 -0
@@ -410,6 +410,9 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,int inv_icm42607_set_temp_conf (struct inv_icm42607_state *st,bool enable,unsigned int *sleep_ms); +int inv_icm42607_enable_wom (struct inv_icm42607_state *st); +int inv_icm42607_disable_wom (struct inv_icm42607_state *st); +int inv_icm42607_debugfs_reg (struct iio_dev *indio_dev,unsigned int reg,unsigned int writeval,unsigned int *readval);
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +303 -0
339 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +158 -1
201 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 08/10] iio: imu: inv_icm42607: Add Wake on Movement for icm42607
Quoted 5 lines · On Fri, 1 May 2026 17:11:47 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add support for wake on movement for the icm42607 driver.
Signed-off-by Chris Morgan <[email protected]>
One general thing as a result of change in how runtime pm calls the stuff to track when it was last busy that I probably missed in earlier patches.
Otherwise minor stuff inline.
Jonathan
Quoted 5 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +0
-0
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c static int inv_icm42607_accel_read_sensor(struct iio_dev *indio_dev, · line 488
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +10
-0+static int inv_icm42607_accel_disable_wom (struct iio_dev *indio_dev) +{ +struct inv_icm42607_state *st =iio_device_get_drvdata (indio_dev); +struct device *pdev =regmap_get_device (st->map); +int ret; + + ret =_inv_icm42607_accel_disable_wom (indio_dev); + +pm_runtime_mark_last_busy (pdev); +pm_runtime_put_autosuspend (pdev);
Check for this throughout. The definition of pm_runtime_put_autosuspend() is now:
static inline int pm_runtime_put_autosuspend(struct device *dev)
{
pm_runtime_mark_last_busy(dev);
return __pm_runtime_put_autosuspend(dev);
}So drop all separate calls to mark_last_busy()
Quoted 8 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +3
-0+ +return ret; +}static const int inv_icm42607_accel_scale_nano[][2 ] = { [INV_ICM42607_ACCEL_FS_16G ] = {0 ,4788403 }, [INV_ICM42607_ACCEL_FS_8G ] = {0 ,2394202 },
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev, · line 655
Quoted 6 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +2
-0@@ -464,6 +650,9 @@ static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev,return ret; ret =inv_icm42607_set_accel_conf (st, &conf,NULL ); +if (ret) +return ret;
Blank line here as the two blocks aren't that closely related. Helps readability (a tiny bit!)
Quoted 31 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c +6
-0+ ret =inv_icm42607_accel_set_wom_threshold (st, st->apex.wom.value, val, val2);if (ret)return ret; +static const struct iio_info inv_icm42607_accel_info = { .read_raw = inv_icm42607_accel_read_raw, .write_raw = inv_icm42607_accel_write_raw,@@ -586,6 +885,10 @@ static const struct iio_info inv_icm42607_accel_info = { .update_scan_mode = inv_icm42607_accel_update_scan_mode, .hwfifo_set_watermark = inv_icm42607_accel_hwfifo_set_watermark, .hwfifo_flush_to_buffer = inv_icm42607_accel_hwfifo_flush, + .read_event_config = inv_icm42607_accel_read_event_config, + .write_event_config = inv_icm42607_accel_write_event_config, + .read_event_value = inv_icm42607_accel_read_event_value, + .write_event_value = inv_icm42607_accel_write_event_value, };struct iio_dev *inv_icm42607_accel_init (struct inv_icm42607_state *st)drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +6
-0+int inv_icm42607_disable_wom (struct inv_icm42607_state *st) +{ +int ret; + +/* disable WoM interrupt */ + ret = regmap_clear_bits(st->map, INV_ICM42607_REG_INT_SOURCE1,
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable, · line 273
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +10
-0+ INV_ICM42607_INT_SOURCE1_WOM_INT1_EN); +if (ret) +return ret; + +/* disable WoM hardware */ +return regmap_clear_bits (st->map,INV_ICM42607_REG_WOM_CONFIG , +INV_ICM42607_WOM_CONFIG_EN ); +} + +
Single line only between functions.
Quoted 35 lines
int inv_icm42607_debugfs_reg(struct
iio_dev*indio_dev, unsigned int reg,drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +33
-0+ +static irqreturn_t inv_icm42607_irq_handler (int irq,void *_data) +{ +struct inv_icm42607_state *st = _data; +struct device *dev =regmap_get_device (st->map); +unsigned int status; +int ret; + +mutex_lock (&st->lock); + +if (st->apex.on) { +unsigned int status2, status3; + +/* read INT_STATUS2 and INT_STATUS3 in 1 operation */ + ret =regmap_bulk_read (st->map,INV_ICM42607_REG_INT_STATUS2 , st->buffer,2 ); +if (ret) + goto out_unlock; + status2 = st->buffer[0 ]; + status3 = st->buffer[1 ]; +inv_icm42607_accel_handle_events (st->indio_accel, status2, status3, + st->timestamp.accel); + } + + ret =regmap_read (st->map,INV_ICM42607_REG_INT_STATUS , &status); +if (ret) + goto out_unlock; + +if (status &INV_ICM42607_INT_STATUS_FIFO_FULL ) +dev_warn (dev,"FIFO full data lost!\n" ); + +if (status &INV_ICM42607_INT_STATUS_FIFO_THS ) { + ret =inv_icm42607_buffer_fifo_read (st,0 ); +if (ret) {
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c static int inv_icm42607_setup(struct inv_icm42607_state *st, · line 448
Quoted 10 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +10
-0+dev_err (dev,"FIFO read error %d\n" , ret); + goto out_unlock; + } + ret =inv_icm42607_buffer_fifo_parse (st); +if (ret) +dev_err (dev,"FIFO parsing error %d\n" , ret); + } + +out_unlock: +mutex_unlock (&st->lock);
Looks like suitable place for guard() and early returns.
Quoted 64 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +52
-1+return IRQ_HANDLED ; +} + +/** +* inv_icm42607_irq_init() - initialize int pin and interrupt handler +* @st: driver internal state +* @irq: irq number +* @irq_type: irq trigger type +* @open_drain: true if irq is open drain, false for push-pull +* +* Returns 0 on success, a negative error code otherwise. +*/ +static int inv_icm42607_irq_init (struct inv_icm42607_state *st,int irq, +int irq_type,bool open_drain) +{ +struct device *dev =regmap_get_device (st->map); +unsigned int val =0 ; +int ret; + +switch (irq_type) { +case IRQF_TRIGGER_RISING : +case IRQF_TRIGGER_HIGH : + val =INV_ICM42607_INT_CONFIG_INT1_ACTIVE_HIGH ; +break ; +default : + val =INV_ICM42607_INT_CONFIG_INT1_ACTIVE_LOW ; +break ; + } + +switch (irq_type) { +case IRQF_TRIGGER_LOW : +case IRQF_TRIGGER_HIGH : + val |=INV_ICM42607_INT_CONFIG_INT1_LATCHED ; +break ; +default : +break ; + } + +if (!open_drain) + val |=INV_ICM42607_INT_CONFIG_INT1_PUSH_PULL ; + + ret =regmap_write (st->map,INV_ICM42607_REG_INT_CONFIG , val); +if (ret) +return ret; + + irq_type |=IRQF_ONESHOT ; +return devm_request_threaded_irq (dev, irq, inv_icm42607_irq_timestamp, + inv_icm42607_irq_handler, irq_type, + st->name, st); +} +static int inv_icm42607_enable_vddio_reg (struct inv_icm42607_state *st) {int ret;@@ -387,7 +523,7 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,struct device *dev =regmap_get_device (regmap);struct fwnode_handle *fwnode =dev_fwnode (dev);struct inv_icm42607_state *st; -int irq; +int irq, irq_type;bool open_drain;int ret;
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c int inv_icm42607_core_probe(struct regmap *regmap, int chip, · line 543
Quoted 6 lines
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +3
-0@@ -402,6 +538,9 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,if (irq <0 )return dev_err_probe (dev, irq,"error missing INT1 interrupt\n" ); + irq_type =irq_get_trigger_type (irq); +if (!irq_type) + irq_type =IRQF_TRIGGER_FALLING ;
This is cut and pasted from elsewhere. The only reason we ever do this is to paper over a driver that set the irq type unconditionally and we should never have done that. The problem in those historical cases is we don't know if boards are relying on that behaviour. Hence this hack.
For new code just let the firmware set it up without the driver doing anything to modify it. Fine to read it here and fail to use the interrupt if nothing is set.
Quoted 1 line
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +0
-0open_drain =device_property_read_bool (dev,"drive-open-drain" );
PATCH v4 9/10 iio: imu: inv_icm42607: Add Gyroscope to icm42607
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add gyroscope functions to the icm42607 driver.
--- drivers/iio/imu/inv_icm42607/Makefile | 1 + drivers/iio/imu/inv_icm42607/inv_icm42607.h | 8 + .../imu/inv_icm42607/inv_icm42607_buffer.c | 25 +- .../iio/imu/inv_icm42607/inv_icm42607_core.c | 48 ++ .../iio/imu/inv_icm42607/inv_icm42607_gyro.c | 536 ++++++++++++++++++ 5 files changed, 617 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
drivers/iio/imu/inv_icm42607/Makefile +1 -0
@@ -2,6 +2,7 @@ obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o inv-icm42607-y += inv_icm42607_core.o +inv-icm42607-y += inv_icm42607_gyro.o inv-icm42607-y += inv_icm42607_accel.o inv-icm42607-y += inv_icm42607_buffer.o inv-icm42607-y += inv_icm42607_temp.o
drivers/iio/imu/inv_icm42607/inv_icm42607.h +8 -0
@@ -407,6 +407,10 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,struct inv_icm42607_sensor_conf *conf, unsigned int *sleep_ms); +int inv_icm42607_set_gyro_conf (struct inv_icm42607_state *st, +struct inv_icm42607_sensor_conf *conf, +unsigned int *sleep_ms); +int inv_icm42607_set_temp_conf (struct inv_icm42607_state *st,bool enable,unsigned int *sleep_ms);
@@ -419,6 +423,10 @@ int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,int inv_icm42607_core_probe (struct regmap *regmap,int chip,inv_icm42607_bus_setup bus_setup); +struct iio_dev *inv_icm42607_gyro_init (struct inv_icm42607_state *st); + +int inv_icm42607_gyro_parse_fifo (struct iio_dev *indio_dev); +struct iio_dev *inv_icm42607_accel_init (struct inv_icm42607_state *st);int inv_icm42607_accel_parse_fifo (struct iio_dev *indio_dev);
drivers/iio/imu/inv_icm42607/inv_icm42607_buffer.c +24 -1
@@ -379,7 +379,10 @@ static int inv_icm42607_buffer_postdisable(struct iio_dev *indio_dev) goto out_unlock; conf.mode =INV_ICM42607_SENSOR_MODE_OFF ; - ret =inv_icm42607_set_accel_conf (st, &conf, &sleep_sensor); +if (sensor ==INV_ICM42607_SENSOR_GYRO ) + ret =inv_icm42607_set_gyro_conf (st, &conf, &sleep_sensor); +else + ret =inv_icm42607_set_accel_conf (st, &conf, &sleep_sensor);if (ret) goto out_unlock;
@@ -468,6 +471,7 @@ int inv_icm42607_buffer_fifo_read(struct inv_icm42607_state *st,int inv_icm42607_buffer_fifo_parse (struct inv_icm42607_state *st) { +struct inv_icm42607_sensor_state *gyro_st =iio_priv (st->indio_gyro);struct inv_icm42607_sensor_state *accel_st =iio_priv (st->indio_accel);struct inv_sensors_timestamp *ts;int ret;
@@ -475,6 +479,16 @@ int inv_icm42607_buffer_fifo_parse(struct inv_icm42607_state *st)if (st->fifo.nb.total ==0 )return 0 ; +/* handle gyroscope timestamp and FIFO data parsing */ +if (st->fifo.nb.gyro >0 ) { + ts = &gyro_st->ts; +inv_sensors_timestamp_interrupt (ts, st->fifo.watermark.eff_gyro, + st->timestamp.gyro); + ret =inv_icm42607_gyro_parse_fifo (st->indio_gyro); +if (ret) +return ret; + } +/* handle accelerometer timestamp and FIFO data parsing */ if (st->fifo.nb.accel >0 ) { ts = &accel_st->ts;
@@ -491,6 +505,7 @@ int inv_icm42607_buffer_fifo_parse(struct inv_icm42607_state *st)int inv_icm42607_buffer_hwfifo_flush (struct inv_icm42607_state *st,unsigned int count) { +struct inv_icm42607_sensor_state *gyro_st =iio_priv (st->indio_gyro);struct inv_icm42607_sensor_state *accel_st =iio_priv (st->indio_accel);struct inv_sensors_timestamp *ts;s64 gyro_ts, accel_ts;
@@ -506,6 +521,14 @@ int inv_icm42607_buffer_hwfifo_flush(struct inv_icm42607_state *st,if (st->fifo.nb.total ==0 )return 0 ; +if (st->fifo.nb.gyro >0 ) { + ts = &gyro_st->ts; +inv_sensors_timestamp_interrupt (ts, st->fifo.nb.gyro, gyro_ts); + ret =inv_icm42607_gyro_parse_fifo (st->indio_gyro); +if (ret) +return ret; + } +if (st->fifo.nb.accel >0 ) { ts = &accel_st->ts;inv_sensors_timestamp_interrupt (ts, st->fifo.nb.accel, accel_ts);
drivers/iio/imu/inv_icm42607/inv_icm42607_core.c +48 -0
60 lines not on this page · raw message
drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c new file +536 -0
536 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH V4 09/10] iio: imu: inv_icm42607: Add Gyroscope to icm42607
Quoted 5 lines · On Fri, 1 May 2026 17:11:48 -0500 Chris Morgan <[email protected]> wrote:
From: Chris Morgan <[email protected]>
Add gyroscope functions to the icm42607 driver.
Signed-off-by Chris Morgan <[email protected]>
From a quick look, feedback will be near identical to the accelerometer driver, so just check this for similar things to change.
Thanks,
Jonathan
PATCH v4 10/10 arm64: dts: rockchip: Add icm42607p IMU for RG-DS
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add the Invensense ICM42607P IMU for the Anbernic RG-DS. Mount-matrix was tested with iio-sensor-proxy and reports correct orientation.
--- .../dts/rockchip/rk3568-anbernic-rg-ds.dts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts +19 -1
@@ -871,7 +871,18 @@ aw87391_pa_r: audio-codec@5b { sound-name-prefix = "Right Amp"; }; - /* invensense,icm42607p at 0x68 */ + icm42607p: imu@68 { + compatible = "invensense,icm42607p"; + reg = <0x68>; + interrupt-names = "INT1"; + interrupt-parent = <&gpio0>; + interrupts = <RK_PD6 IRQ_TYPE_EDGE_FALLING>; + mount-matrix = "-1", "0", "0", + "0", "1", "0", + "0", "0", "-1"; + pinctrl-0 = <&accel_irq>; + pinctrl-names = "default"; + }; }; &i2c3 {
@@ -932,6 +943,13 @@ &i2s1_8ch { }; &pinctrl { + accel { + accel_irq: accel-irq { + rockchip,pins = + <0 RK_PD6 RK_FUNC_GPIO &pcfg_pull_up>; + }; + }; + gpio-keys { vol_keys_l: vol-keys_l { rockchip,pins =
Signature · 1 line
2.43.0