lkml.org 
[lkml]   [2020]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 01/12] iio: imu: inv_icm42600: add core of new inv_icm42600 driver
    On Thu, 7 May 2020 16:42:11 +0200
    Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

    > Core component of a new driver for InvenSense ICM-426xx devices.
    > It includes registers definition, main probe/setup, and device
    > utility functions.
    >
    > ICM-426xx devices are latest generation of 6-axis IMU,
    > gyroscope+accelerometer and temperature sensor. This device
    > includes a 2K FIFO, supports I2C/I3C/SPI, and provides
    > intelligent motion features like pedometer, tilt detection,
    > and tap detection.
    >
    > Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

    Hi Jean-Baptiste,

    A few minor things inline.

    Thanks,

    Jonathan

    > ---
    > drivers/iio/imu/inv_icm42600/inv_icm42600.h | 372 +++++++++++
    > .../iio/imu/inv_icm42600/inv_icm42600_core.c | 618 ++++++++++++++++++
    > 2 files changed, 990 insertions(+)
    > create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600.h
    > create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
    >
    > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
    > new file mode 100644
    > index 000000000000..8da4c8249aed
    > --- /dev/null
    > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
    > @@ -0,0 +1,372 @@
    > +/* SPDX-License-Identifier: GPL-2.0-or-later */
    > +/*
    > + * Copyright (C) 2020 Invensense, Inc.
    > + */
    > +
    > +#ifndef INV_ICM42600_H_
    > +#define INV_ICM42600_H_
    > +
    > +#include <linux/bits.h>
    > +#include <linux/bitfield.h>
    > +#include <linux/regmap.h>
    > +#include <linux/mutex.h>
    > +#include <linux/regulator/consumer.h>
    > +#include <linux/pm.h>
    > +#include <linux/iio/iio.h>
    > +
    > +enum inv_icm42600_chip {
    > + INV_CHIP_ICM42600,
    > + INV_CHIP_ICM42602,
    > + INV_CHIP_ICM42605,
    > + INV_CHIP_ICM42622,
    > + INV_CHIP_NB,
    > +};
    > +
    > +/* serial bus slew rates */
    > +enum inv_icm42600_slew_rate {
    > + INV_ICM42600_SLEW_RATE_20_60NS,
    > + INV_ICM42600_SLEW_RATE_12_36NS,
    > + INV_ICM42600_SLEW_RATE_6_18NS,
    > + INV_ICM42600_SLEW_RATE_4_12NS,
    > + INV_ICM42600_SLEW_RATE_2_6NS,
    > + INV_ICM42600_SLEW_RATE_INF_2NS,
    > +};
    > +
    > +enum inv_icm42600_sensor_mode {
    > + INV_ICM42600_SENSOR_MODE_OFF,
    > + INV_ICM42600_SENSOR_MODE_STANDBY,
    > + INV_ICM42600_SENSOR_MODE_LOW_POWER,
    > + INV_ICM42600_SENSOR_MODE_LOW_NOISE,
    > + INV_ICM42600_SENSOR_MODE_NB,
    > +};
    > +
    > +/* gyroscope fullscale values */
    > +enum inv_icm42600_gyro_fs {
    > + INV_ICM42600_GYRO_FS_2000DPS,
    > + INV_ICM42600_GYRO_FS_1000DPS,
    > + INV_ICM42600_GYRO_FS_500DPS,
    > + INV_ICM42600_GYRO_FS_250DPS,
    > + INV_ICM42600_GYRO_FS_125DPS,
    > + INV_ICM42600_GYRO_FS_62_5DPS,
    > + INV_ICM42600_GYRO_FS_31_25DPS,
    > + INV_ICM42600_GYRO_FS_15_625DPS,
    > + INV_ICM42600_GYRO_FS_NB,
    > +};
    > +
    > +/* accelerometer fullscale values */
    > +enum inv_icm42600_accel_fs {
    > + INV_ICM42600_ACCEL_FS_16G,
    > + INV_ICM42600_ACCEL_FS_8G,
    > + INV_ICM42600_ACCEL_FS_4G,
    > + INV_ICM42600_ACCEL_FS_2G,
    > + INV_ICM42600_ACCEL_FS_NB,
    > +};
    > +
    > +/* ODR suffixed by LN or LP are Low-Noise or Low-Power mode only */
    > +enum inv_icm42600_odr {
    > + INV_ICM42600_ODR_8KHZ_LN = 3,
    > + INV_ICM42600_ODR_4KHZ_LN,
    > + INV_ICM42600_ODR_2KHZ_LN,
    > + INV_ICM42600_ODR_1KHZ_LN,
    > + INV_ICM42600_ODR_200HZ,
    > + INV_ICM42600_ODR_100HZ,
    > + INV_ICM42600_ODR_50HZ,
    > + INV_ICM42600_ODR_25HZ,
    > + INV_ICM42600_ODR_12_5HZ,
    > + INV_ICM42600_ODR_6_25HZ_LP,
    > + INV_ICM42600_ODR_3_125HZ_LP,
    > + INV_ICM42600_ODR_1_5625HZ_LP,
    > + INV_ICM42600_ODR_500HZ,
    > + INV_ICM42600_ODR_NB,
    > +};
    > +
    > +enum inv_icm42600_filter {
    > + /* Low-Noise mode sensor data filter (3rd order filter by default) */
    > + INV_ICM42600_FILTER_BW_ODR_DIV_2,
    > +
    > + /* Low-Power mode sensor data filter (averaging) */
    > + INV_ICM42600_FILTER_AVG_1X = 1,
    > + INV_ICM42600_FILTER_AVG_16X = 6,
    > +};
    > +
    > +struct inv_icm42600_sensor_conf {
    > + int mode;
    > + int fs;
    > + int odr;
    > + int filter;
    > +};
    > +#define INV_ICM42600_SENSOR_CONF_INIT {-1, -1, -1, -1}
    > +
    > +struct inv_icm42600_conf {
    > + struct inv_icm42600_sensor_conf gyro;
    > + struct inv_icm42600_sensor_conf accel;
    > + bool temp_en;
    > +};
    > +
    > +struct inv_icm42600_suspended {
    > + enum inv_icm42600_sensor_mode gyro;
    > + enum inv_icm42600_sensor_mode accel;
    > + bool temp;
    > +};
    > +
    > +/*
    /**

    It's valid kernel doc so lets mark it as such.

    > + * struct inv_icm42600_state - driver state variables
    > + * @lock: chip access lock.

    Nice to be a bit more specific on that. What about the chip needs
    a lock at this level as opposed to bus locks etc?

    > + * @chip: chip identifier.
    > + * @name: chip name.
    > + * @map: regmap pointer.
    > + * @vdd_supply: VDD voltage regulator for the chip.
    > + * @vddio_supply: I/O voltage regulator for the chip.
    > + * @orientation: sensor chip orientation relative to main hardware.
    > + * @conf: chip sensors configurations.
    > + * @suspended: suspended sensors configuration.
    > + */
    > +struct inv_icm42600_state {
    > + struct mutex lock;
    > + enum inv_icm42600_chip chip;
    > + const char *name;
    > + struct regmap *map;
    > + struct regulator *vdd_supply;
    > + struct regulator *vddio_supply;
    > + struct iio_mount_matrix orientation;
    > + struct inv_icm42600_conf conf;
    > + struct inv_icm42600_suspended suspended;
    > +};
    > +
    > +/* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
    > +
    > +/* Bank selection register, available in all banks */
    > +#define INV_ICM42600_REG_BANK_SEL 0x76
    > +#define INV_ICM42600_BANK_SEL_MASK GENMASK(2, 0)
    > +
    > +/* User bank 0 (MSB 0x00) */
    > +#define INV_ICM42600_REG_DEVICE_CONFIG 0x0011
    > +#define INV_ICM42600_DEVICE_CONFIG_SOFT_RESET BIT(0)
    > +
    > +#define INV_ICM42600_REG_DRIVE_CONFIG 0x0013
    > +#define INV_ICM42600_DRIVE_CONFIG_I2C_MASK GENMASK(5, 3)
    > +#define INV_ICM42600_DRIVE_CONFIG_I2C(_rate) \
    > + FIELD_PREP(INV_ICM42600_DRIVE_CONFIG_I2C_MASK, (_rate))
    > +#define INV_ICM42600_DRIVE_CONFIG_SPI_MASK GENMASK(2, 0)
    > +#define INV_ICM42600_DRIVE_CONFIG_SPI(_rate) \
    > + FIELD_PREP(INV_ICM42600_DRIVE_CONFIG_SPI_MASK, (_rate))
    > +
    > +#define INV_ICM42600_REG_INT_CONFIG 0x0014
    > +#define INV_ICM42600_INT_CONFIG_INT2_LATCHED BIT(5)
    > +#define INV_ICM42600_INT_CONFIG_INT2_PUSH_PULL BIT(4)
    > +#define INV_ICM42600_INT_CONFIG_INT2_ACTIVE_HIGH BIT(3)
    > +#define INV_ICM42600_INT_CONFIG_INT2_ACTIVE_LOW 0x00
    > +#define INV_ICM42600_INT_CONFIG_INT1_LATCHED BIT(2)
    > +#define INV_ICM42600_INT_CONFIG_INT1_PUSH_PULL BIT(1)
    > +#define INV_ICM42600_INT_CONFIG_INT1_ACTIVE_HIGH BIT(0)
    > +#define INV_ICM42600_INT_CONFIG_INT1_ACTIVE_LOW 0x00
    > +
    > +#define INV_ICM42600_REG_FIFO_CONFIG 0x0016
    > +#define INV_ICM42600_FIFO_CONFIG_MASK GENMASK(7, 6)
    > +#define INV_ICM42600_FIFO_CONFIG_BYPASS \
    > + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 0)
    > +#define INV_ICM42600_FIFO_CONFIG_STREAM \
    > + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 1)
    > +#define INV_ICM42600_FIFO_CONFIG_STOP_ON_FULL \
    > + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 2)
    > +
    > +/* all sensor data are 16 bits (2 registers wide) in big-endian */
    > +#define INV_ICM42600_REG_TEMP_DATA 0x001D
    > +#define INV_ICM42600_REG_ACCEL_DATA_X 0x001F
    > +#define INV_ICM42600_REG_ACCEL_DATA_Y 0x0021
    > +#define INV_ICM42600_REG_ACCEL_DATA_Z 0x0023
    > +#define INV_ICM42600_REG_GYRO_DATA_X 0x0025
    > +#define INV_ICM42600_REG_GYRO_DATA_Y 0x0027
    > +#define INV_ICM42600_REG_GYRO_DATA_Z 0x0029
    > +#define INV_ICM42600_DATA_INVALID -32768
    > +
    > +#define INV_ICM42600_REG_INT_STATUS 0x002D
    > +#define INV_ICM42600_INT_STATUS_UI_FSYNC BIT(6)
    > +#define INV_ICM42600_INT_STATUS_PLL_RDY BIT(5)
    > +#define INV_ICM42600_INT_STATUS_RESET_DONE BIT(4)
    > +#define INV_ICM42600_INT_STATUS_DATA_RDY BIT(3)
    > +#define INV_ICM42600_INT_STATUS_FIFO_THS BIT(2)
    > +#define INV_ICM42600_INT_STATUS_FIFO_FULL BIT(1)
    > +#define INV_ICM42600_INT_STATUS_AGC_RDY BIT(0)
    > +
    > +/*
    > + * 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_ICM42600_REG_FIFO_COUNT 0x002E
    > +#define INV_ICM42600_REG_FIFO_DATA 0x0030
    > +
    > +#define INV_ICM42600_REG_SIGNAL_PATH_RESET 0x004B
    > +#define INV_ICM42600_SIGNAL_PATH_RESET_DMP_INIT_EN BIT(6)
    > +#define INV_ICM42600_SIGNAL_PATH_RESET_DMP_MEM_RESET BIT(5)
    > +#define INV_ICM42600_SIGNAL_PATH_RESET_RESET BIT(3)
    > +#define INV_ICM42600_SIGNAL_PATH_RESET_TMST_STROBE BIT(2)
    > +#define INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH BIT(1)
    > +
    > +/* default configuration: all data big-endian and fifo count in bytes */
    > +#define INV_ICM42600_REG_INTF_CONFIG0 0x004C
    > +#define INV_ICM42600_INTF_CONFIG0_FIFO_HOLD_LAST_DATA BIT(7)
    > +#define INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_REC BIT(6)
    > +#define INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_ENDIAN BIT(5)
    > +#define INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN BIT(4)
    > +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK GENMASK(1, 0)
    > +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS \
    > + FIELD_PREP(INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK, 2)
    > +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS \
    > + FIELD_PREP(INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK, 3)
    > +
    > +#define INV_ICM42600_REG_INTF_CONFIG1 0x004D
    > +#define INV_ICM42600_INTF_CONFIG1_ACCEL_LP_CLK_RC BIT(3)
    > +
    > +#define INV_ICM42600_REG_PWR_MGMT0 0x004E
    > +#define INV_ICM42600_PWR_MGMT0_TEMP_DIS BIT(5)
    > +#define INV_ICM42600_PWR_MGMT0_IDLE BIT(4)
    > +#define INV_ICM42600_PWR_MGMT0_GYRO(_mode) \
    > + FIELD_PREP(GENMASK(3, 2), (_mode))
    > +#define INV_ICM42600_PWR_MGMT0_ACCEL(_mode) \
    > + FIELD_PREP(GENMASK(1, 0), (_mode))
    > +
    > +#define INV_ICM42600_REG_GYRO_CONFIG0 0x004F
    > +#define INV_ICM42600_GYRO_CONFIG0_FS(_fs) \
    > + FIELD_PREP(GENMASK(7, 5), (_fs))
    > +#define INV_ICM42600_GYRO_CONFIG0_ODR(_odr) \
    > + FIELD_PREP(GENMASK(3, 0), (_odr))
    > +
    > +#define INV_ICM42600_REG_ACCEL_CONFIG0 0x0050
    > +#define INV_ICM42600_ACCEL_CONFIG0_FS(_fs) \
    > + FIELD_PREP(GENMASK(7, 5), (_fs))
    > +#define INV_ICM42600_ACCEL_CONFIG0_ODR(_odr) \
    > + FIELD_PREP(GENMASK(3, 0), (_odr))
    > +
    > +#define INV_ICM42600_REG_GYRO_ACCEL_CONFIG0 0x0052
    > +#define INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(_f) \
    > + FIELD_PREP(GENMASK(7, 4), (_f))
    > +#define INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(_f) \
    > + FIELD_PREP(GENMASK(3, 0), (_f))
    > +
    > +#define INV_ICM42600_REG_TMST_CONFIG 0x0054
    > +#define INV_ICM42600_TMST_CONFIG_MASK GENMASK(4, 0)
    > +#define INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN BIT(4)
    > +#define INV_ICM42600_TMST_CONFIG_TMST_RES_16US BIT(3)
    > +#define INV_ICM42600_TMST_CONFIG_TMST_DELTA_EN BIT(2)
    > +#define INV_ICM42600_TMST_CONFIG_TMST_FSYNC_EN BIT(1)
    > +#define INV_ICM42600_TMST_CONFIG_TMST_EN BIT(0)
    > +
    > +#define INV_ICM42600_REG_FIFO_CONFIG1 0x005F
    > +#define INV_ICM42600_FIFO_CONFIG1_RESUME_PARTIAL_RD BIT(6)
    > +#define INV_ICM42600_FIFO_CONFIG1_WM_GT_TH BIT(5)
    > +#define INV_ICM42600_FIFO_CONFIG1_TMST_FSYNC_EN BIT(3)
    > +#define INV_ICM42600_FIFO_CONFIG1_TEMP_EN BIT(2)
    > +#define INV_ICM42600_FIFO_CONFIG1_GYRO_EN BIT(1)
    > +#define INV_ICM42600_FIFO_CONFIG1_ACCEL_EN BIT(0)
    > +
    > +/* FIFO watermark is 16 bits (2 registers wide) in little-endian */
    > +#define INV_ICM42600_REG_FIFO_WATERMARK 0x0060
    > +#define INV_ICM42600_FIFO_WATERMARK_VAL(_wm) \
    > + cpu_to_le16((_wm) & GENMASK(11, 0))
    > +/* FIFO is 2048 bytes, let 12 samples for reading latency */
    > +#define INV_ICM42600_FIFO_WATERMARK_MAX (2048 - 12 * 16)
    > +
    > +#define INV_ICM42600_REG_INT_CONFIG1 0x0064
    > +#define INV_ICM42600_INT_CONFIG1_TPULSE_DURATION BIT(6)
    > +#define INV_ICM42600_INT_CONFIG1_TDEASSERT_DISABLE BIT(5)
    > +#define INV_ICM42600_INT_CONFIG1_ASYNC_RESET BIT(4)
    > +
    > +#define INV_ICM42600_REG_INT_SOURCE0 0x0065
    > +#define INV_ICM42600_INT_SOURCE0_UI_FSYNC_INT1_EN BIT(6)
    > +#define INV_ICM42600_INT_SOURCE0_PLL_RDY_INT1_EN BIT(5)
    > +#define INV_ICM42600_INT_SOURCE0_RESET_DONE_INT1_EN BIT(4)
    > +#define INV_ICM42600_INT_SOURCE0_UI_DRDY_INT1_EN BIT(3)
    > +#define INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN BIT(2)
    > +#define INV_ICM42600_INT_SOURCE0_FIFO_FULL_INT1_EN BIT(1)
    > +#define INV_ICM42600_INT_SOURCE0_UI_AGC_RDY_INT1_EN BIT(0)
    > +
    > +#define INV_ICM42600_REG_WHOAMI 0x0075
    > +#define INV_ICM42600_WHOAMI_ICM42600 0x40
    > +#define INV_ICM42600_WHOAMI_ICM42602 0x41
    > +#define INV_ICM42600_WHOAMI_ICM42605 0x42
    > +#define INV_ICM42600_WHOAMI_ICM42622 0x46
    > +
    > +/* User bank 1 (MSB 0x10) */
    > +#define INV_ICM42600_REG_SENSOR_CONFIG0 0x1003
    > +#define INV_ICM42600_SENSOR_CONFIG0_ZG_DISABLE BIT(5)
    > +#define INV_ICM42600_SENSOR_CONFIG0_YG_DISABLE BIT(4)
    > +#define INV_ICM42600_SENSOR_CONFIG0_XG_DISABLE BIT(3)
    > +#define INV_ICM42600_SENSOR_CONFIG0_ZA_DISABLE BIT(2)
    > +#define INV_ICM42600_SENSOR_CONFIG0_YA_DISABLE BIT(1)
    > +#define INV_ICM42600_SENSOR_CONFIG0_XA_DISABLE BIT(0)
    > +
    > +/* Timestamp value is 20 bits (3 registers) in little-endian */
    > +#define INV_ICM42600_REG_TMSTVAL 0x1062
    > +#define INV_ICM42600_TMSTVAL_MASK GENMASK(19, 0)
    > +
    > +#define INV_ICM42600_REG_INTF_CONFIG4 0x107A
    > +#define INV_ICM42600_INTF_CONFIG4_I3C_BUS_ONLY BIT(6)
    > +#define INV_ICM42600_INTF_CONFIG4_SPI_AP_4WIRE BIT(1)
    > +
    > +#define INV_ICM42600_REG_INTF_CONFIG6 0x107C
    > +#define INV_ICM42600_INTF_CONFIG6_MASK GENMASK(4, 0)
    > +#define INV_ICM42600_INTF_CONFIG6_I3C_EN BIT(4)
    > +#define INV_ICM42600_INTF_CONFIG6_I3C_IBI_BYTE_EN BIT(3)
    > +#define INV_ICM42600_INTF_CONFIG6_I3C_IBI_EN BIT(2)
    > +#define INV_ICM42600_INTF_CONFIG6_I3C_DDR_EN BIT(1)
    > +#define INV_ICM42600_INTF_CONFIG6_I3C_SDR_EN BIT(0)
    > +
    > +/* User bank 4 (MSB 0x40) */
    > +#define INV_ICM42600_REG_INT_SOURCE8 0x404F
    > +#define INV_ICM42600_INT_SOURCE8_FSYNC_IBI_EN BIT(5)
    > +#define INV_ICM42600_INT_SOURCE8_PLL_RDY_IBI_EN BIT(4)
    > +#define INV_ICM42600_INT_SOURCE8_UI_DRDY_IBI_EN BIT(3)
    > +#define INV_ICM42600_INT_SOURCE8_FIFO_THS_IBI_EN BIT(2)
    > +#define INV_ICM42600_INT_SOURCE8_FIFO_FULL_IBI_EN BIT(1)
    > +#define INV_ICM42600_INT_SOURCE8_AGC_RDY_IBI_EN BIT(0)
    > +
    > +#define INV_ICM42600_REG_OFFSET_USER0 0x4077
    > +#define INV_ICM42600_REG_OFFSET_USER1 0x4078
    > +#define INV_ICM42600_REG_OFFSET_USER2 0x4079
    > +#define INV_ICM42600_REG_OFFSET_USER3 0x407A
    > +#define INV_ICM42600_REG_OFFSET_USER4 0x407B
    > +#define INV_ICM42600_REG_OFFSET_USER5 0x407C
    > +#define INV_ICM42600_REG_OFFSET_USER6 0x407D
    > +#define INV_ICM42600_REG_OFFSET_USER7 0x407E
    > +#define INV_ICM42600_REG_OFFSET_USER8 0x407F
    > +
    > +/* Sleep times required by the driver */
    > +#define INV_ICM42600_POWER_UP_TIME_MS 100
    > +#define INV_ICM42600_RESET_TIME_MS 1
    > +#define INV_ICM42600_ACCEL_STARTUP_TIME_MS 20
    > +#define INV_ICM42600_GYRO_STARTUP_TIME_MS 60
    > +#define INV_ICM42600_GYRO_STOP_TIME_MS 150
    > +#define INV_ICM42600_TEMP_STARTUP_TIME_MS 14
    > +#define INV_ICM42600_SUSPEND_DELAY_MS 2000
    > +
    > +typedef int (*inv_icm42600_bus_setup)(struct inv_icm42600_state *);
    > +
    > +extern const struct regmap_config inv_icm42600_regmap_config;
    > +extern const struct dev_pm_ops inv_icm42600_pm_ops;
    > +
    > +const struct iio_mount_matrix *
    > +inv_icm42600_get_mount_matrix(const struct iio_dev *indio_dev,
    > + const struct iio_chan_spec *chan);
    > +
    > +uint32_t inv_icm42600_odr_to_period(enum inv_icm42600_odr odr);
    > +
    > +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
    > + struct inv_icm42600_sensor_conf *conf,
    > + unsigned int *sleep);
    > +
    > +int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
    > + struct inv_icm42600_sensor_conf *conf,
    > + unsigned int *sleep);
    > +
    > +int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
    > + unsigned int *sleep);
    > +
    > +int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
    > + unsigned int writeval, unsigned int *readval);
    > +
    > +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
    > + inv_icm42600_bus_setup bus_setup);
    > +
    > +#endif
    > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
    > new file mode 100644
    > index 000000000000..35bdf4f9d31e
    > --- /dev/null
    > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
    > @@ -0,0 +1,618 @@
    > +// SPDX-License-Identifier: GPL-2.0-or-later
    > +/*
    > + * Copyright (C) 2020 Invensense, Inc.
    > + */
    > +
    > +#include <linux/device.h>
    > +#include <linux/module.h>
    > +#include <linux/slab.h>
    > +#include <linux/delay.h>
    > +#include <linux/interrupt.h>
    > +#include <linux/regulator/consumer.h>
    > +#include <linux/pm_runtime.h>
    > +#include <linux/regmap.h>
    > +#include <linux/iio/iio.h>
    > +
    > +#include "inv_icm42600.h"
    > +
    > +static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
    > + {
    > + .name = "user banks",
    > + .range_min = 0x0000,
    > + .range_max = 0x4FFF,
    > + .selector_reg = INV_ICM42600_REG_BANK_SEL,
    > + .selector_mask = INV_ICM42600_BANK_SEL_MASK,
    > + .selector_shift = 0,
    > + .window_start = 0,
    > + .window_len = 0x1000,
    > + },
    > +};
    > +
    > +const struct regmap_config inv_icm42600_regmap_config = {
    > + .reg_bits = 8,
    > + .val_bits = 8,
    > + .max_register = 0x4FFF,
    > + .ranges = inv_icm42600_regmap_ranges,
    > + .num_ranges = ARRAY_SIZE(inv_icm42600_regmap_ranges),
    > +};
    > +EXPORT_SYMBOL_GPL(inv_icm42600_regmap_config);
    > +
    > +struct inv_icm42600_hw {
    > + uint8_t whoami;
    > + const char *name;
    > + const struct inv_icm42600_conf *conf;
    > +};
    > +
    > +/* chip initial default configuration */
    > +static const struct inv_icm42600_conf inv_icm42600_default_conf = {
    > + .gyro = {
    > + .mode = INV_ICM42600_SENSOR_MODE_OFF,
    > + .fs = INV_ICM42600_GYRO_FS_2000DPS,
    > + .odr = INV_ICM42600_ODR_50HZ,
    > + .filter = INV_ICM42600_FILTER_BW_ODR_DIV_2,
    > + },
    > + .accel = {
    > + .mode = INV_ICM42600_SENSOR_MODE_OFF,
    > + .fs = INV_ICM42600_ACCEL_FS_16G,
    > + .odr = INV_ICM42600_ODR_50HZ,
    > + .filter = INV_ICM42600_FILTER_BW_ODR_DIV_2,
    > + },
    > + .temp_en = false,
    > +};
    > +
    > +static const struct inv_icm42600_hw inv_icm42600_hw[] = {
    > + [INV_CHIP_ICM42600] = {
    > + .whoami = INV_ICM42600_WHOAMI_ICM42600,
    > + .name = "icm42600",
    > + .conf = &inv_icm42600_default_conf,
    > + },
    > + [INV_CHIP_ICM42602] = {
    > + .whoami = INV_ICM42600_WHOAMI_ICM42602,
    > + .name = "icm42602",
    > + .conf = &inv_icm42600_default_conf,
    > + },
    > + [INV_CHIP_ICM42605] = {
    > + .whoami = INV_ICM42600_WHOAMI_ICM42605,
    > + .name = "icm42605",
    > + .conf = &inv_icm42600_default_conf,
    > + },
    > + [INV_CHIP_ICM42622] = {
    > + .whoami = INV_ICM42600_WHOAMI_ICM42622,
    > + .name = "icm42622",
    > + .conf = &inv_icm42600_default_conf,
    > + },
    > +};
    > +
    > +const struct iio_mount_matrix *
    > +inv_icm42600_get_mount_matrix(const struct iio_dev *indio_dev,
    > + const struct iio_chan_spec *chan)
    > +{
    > + const struct inv_icm42600_state *st =
    > + iio_device_get_drvdata((struct iio_dev *)indio_dev);

    Interesting... iio_device_get_drvdata is never going to modify
    the struct iio_dev. Should we just change that to take a
    const struct iio_dev * ?

    > +
    > + return &st->orientation;
    > +}
    > +
    > +uint32_t inv_icm42600_odr_to_period(enum inv_icm42600_odr odr)
    > +{
    > + static uint32_t odr_periods[INV_ICM42600_ODR_NB] = {
    > + /* reserved values */
    > + 0, 0, 0,
    > + /* 8kHz */
    > + 125000,
    > + /* 4kHz */
    > + 250000,
    > + /* 2kHz */
    > + 500000,
    > + /* 1kHz */
    > + 1000000,
    > + /* 200Hz */
    > + 5000000,
    > + /* 100Hz */
    > + 10000000,
    > + /* 50Hz */
    > + 20000000,
    > + /* 25Hz */
    > + 40000000,
    > + /* 12.5Hz */
    > + 80000000,
    > + /* 6.25Hz */
    > + 160000000,
    > + /* 3.125Hz */
    > + 320000000,
    > + /* 1.5625Hz */
    > + 640000000,
    > + /* 500Hz */
    > + 2000000,
    > + };
    > +
    > + return odr_periods[odr];
    > +}
    > +
    > +static int inv_icm42600_set_pwr_mgmt0(struct inv_icm42600_state *st,
    > + enum inv_icm42600_sensor_mode gyro,
    > + enum inv_icm42600_sensor_mode accel,
    > + bool temp, unsigned int *sleep)

    msleep or similar that indicates the units of the sleep time.

    > +{
    > + enum inv_icm42600_sensor_mode oldgyro = st->conf.gyro.mode;
    > + enum inv_icm42600_sensor_mode oldaccel = st->conf.accel.mode;
    > + bool oldtemp = st->conf.temp_en;
    > + unsigned int sleepval;
    > + unsigned int val;
    > + int ret;
    > +
    > + /* if nothing changed, exit */
    > + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
    > + return 0;
    > +
    > + val = INV_ICM42600_PWR_MGMT0_GYRO(gyro) |
    > + INV_ICM42600_PWR_MGMT0_ACCEL(accel);
    > + if (!temp)
    > + val |= INV_ICM42600_PWR_MGMT0_TEMP_DIS;
    > + dev_dbg(regmap_get_device(st->map), "pwr_mgmt0: %#02x\n", val);

    I wonder if you have a little too much in the way of debug prints.
    These are internal to the code and so could only be wrong due to a local
    bug. Once you've finished writing the driver I'd hope we won't need these!

    > + ret = regmap_write(st->map, INV_ICM42600_REG_PWR_MGMT0, val);
    > + if (ret)
    > + return ret;
    > +
    > + st->conf.gyro.mode = gyro;
    > + st->conf.accel.mode = accel;
    > + st->conf.temp_en = temp;
    > +
    > + /* compute required wait time for sensors to stabilize */
    > + sleepval = 0;
    > + /* temperature stabilization time */
    > + if (temp && !oldtemp) {
    > + if (sleepval < INV_ICM42600_TEMP_STARTUP_TIME_MS)
    > + sleepval = INV_ICM42600_TEMP_STARTUP_TIME_MS;
    > + }
    > + /* accel startup time */
    > + if (accel != oldaccel && oldaccel == INV_ICM42600_SENSOR_MODE_OFF) {
    > + /* block any register write for at least 200 µs */
    > + usleep_range(200, 300);
    > + if (sleepval < INV_ICM42600_ACCEL_STARTUP_TIME_MS)
    > + sleepval = INV_ICM42600_ACCEL_STARTUP_TIME_MS;
    > + }
    > + if (gyro != oldgyro) {
    > + /* gyro startup time */
    > + if (oldgyro == INV_ICM42600_SENSOR_MODE_OFF) {
    > + /* block any register write for at least 200 µs */
    > + usleep_range(200, 300);
    > + if (sleepval < INV_ICM42600_GYRO_STARTUP_TIME_MS)
    > + sleepval = INV_ICM42600_GYRO_STARTUP_TIME_MS;
    > + /* gyro stop time */
    > + } else if (gyro == INV_ICM42600_SENSOR_MODE_OFF) {
    > + if (sleepval < INV_ICM42600_GYRO_STOP_TIME_MS)
    > + sleepval = INV_ICM42600_GYRO_STOP_TIME_MS;
    > + }
    > + }
    > +
    > + /* deferred sleep value if sleep pointer is provided or direct sleep */
    > + if (sleep)
    > + *sleep = sleepval;
    > + else if (sleepval)
    > + msleep(sleepval);
    > +
    > + return 0;
    > +}
    > +
    > +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
    > + struct inv_icm42600_sensor_conf *conf,
    > + unsigned int *sleep)
    > +{
    > + struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
    > + unsigned int val;
    > + int ret;
    > +
    > + /* Sanitize missing values with current values */
    > + 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;
    > +
    > + /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
    > + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
    > + val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
    > + INV_ICM42600_ACCEL_CONFIG0_ODR(conf->odr);
    > + dev_dbg(regmap_get_device(st->map), "accel_config0: %#02x\n", val);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > + oldconf->fs = conf->fs;
    > + oldconf->odr = conf->odr;
    > + }
    > +
    > + /* set GYRO_ACCEL_CONFIG0 register (accel filter) */
    > + if (conf->filter != oldconf->filter) {
    > + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->filter) |
    > + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(st->conf.gyro.filter);
    > + dev_dbg(regmap_get_device(st->map), "gyro_accel_config0: %#02x\n", val);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > + oldconf->filter = conf->filter;
    > + }
    > +
    > + /* set PWR_MGMT0 register (accel sensor mode) */
    > + return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
    > + st->conf.temp_en, sleep);
    > +}
    > +
    > +int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
    > + struct inv_icm42600_sensor_conf *conf,
    > + unsigned int *sleep)
    > +{
    > + struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
    > + unsigned int val;
    > + int ret;
    > +
    > + /* sanitize missing values with current values */
    > + 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;
    > +
    > + /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
    > + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
    > + val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
    > + INV_ICM42600_GYRO_CONFIG0_ODR(conf->odr);
    > + dev_dbg(regmap_get_device(st->map), "gyro_config0: %#02x\n", val);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > + oldconf->fs = conf->fs;
    > + oldconf->odr = conf->odr;
    > + }
    > +
    > + /* set GYRO_ACCEL_CONFIG0 register (gyro filter) */
    > + if (conf->filter != oldconf->filter) {
    > + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(st->conf.accel.filter) |
    > + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->filter);
    > + dev_dbg(regmap_get_device(st->map), "gyro_accel_config0: %#02x\n", val);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > + oldconf->filter = conf->filter;
    > + }
    > +
    > + /* set PWR_MGMT0 register (gyro sensor mode) */
    > + return inv_icm42600_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
    > + st->conf.temp_en, sleep);
    > +
    > + return 0;
    > +}
    > +
    > +int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
    > + unsigned int *sleep)
    > +{
    > + return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode,
    > + st->conf.accel.mode, enable,
    > + sleep);
    > +}
    > +
    > +int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
    > + unsigned int writeval, unsigned int *readval)
    > +{
    > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
    > + int ret;
    > +
    > + mutex_lock(&st->lock);
    > +
    > + if (readval)
    > + ret = regmap_read(st->map, reg, readval);
    > + else
    > + ret = regmap_write(st->map, reg, writeval);
    > +
    > + mutex_unlock(&st->lock);
    > +
    > + return ret;
    > +}
    > +
    > +static int inv_icm42600_set_conf(struct inv_icm42600_state *st,
    > + const struct inv_icm42600_conf *conf)
    > +{
    > + unsigned int val;
    > + int ret;
    > +
    > + /* set PWR_MGMT0 register (gyro & accel sensor mode, temp enabled) */
    > + val = INV_ICM42600_PWR_MGMT0_GYRO(conf->gyro.mode) |
    > + INV_ICM42600_PWR_MGMT0_ACCEL(conf->accel.mode);
    > + if (!conf->temp_en)
    > + val |= INV_ICM42600_PWR_MGMT0_TEMP_DIS;
    > + ret = regmap_write(st->map, INV_ICM42600_REG_PWR_MGMT0, val);
    > + if (ret)
    > + return ret;
    > +
    > + /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
    > + val = INV_ICM42600_GYRO_CONFIG0_FS(conf->gyro.fs) |
    > + INV_ICM42600_GYRO_CONFIG0_ODR(conf->gyro.odr);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > +
    > + /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
    > + val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->accel.fs) |
    > + INV_ICM42600_ACCEL_CONFIG0_ODR(conf->accel.odr);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > +
    > + /* set GYRO_ACCEL_CONFIG0 register (gyro & accel filters) */
    > + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->accel.filter) |
    > + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->gyro.filter);
    > + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
    > + if (ret)
    > + return ret;
    > +
    > + /* update internal conf */
    > + st->conf = *conf;
    > +
    > + return 0;
    > +}
    > +
    > +/**
    > + * inv_icm42600_setup() - check and setup chip.

    If doing kernel-doc (which is good) you should do it all.
    So document the parameters as well.
    It's worth running the kernel-doc script over any file where
    you put some and fixing up any warnings / errors.

    > + */
    > +static int inv_icm42600_setup(struct inv_icm42600_state *st,
    > + inv_icm42600_bus_setup bus_setup)
    > +{
    > + const struct inv_icm42600_hw *hw = &inv_icm42600_hw[st->chip];
    > + const struct device *dev = regmap_get_device(st->map);
    > + unsigned int mask, val;
    > + int ret;
    > +
    > + /* check chip self-identification value */
    > + ret = regmap_read(st->map, INV_ICM42600_REG_WHOAMI, &val);
    > + if (ret)
    > + return ret;
    > + if (val != hw->whoami) {
    > + dev_err(dev, "invalid whoami %#02x expected %#02x (%s)\n",
    > + val, hw->whoami, hw->name);
    > + return -ENODEV;
    > + }
    > + dev_info(dev, "found %s (%#02x)\n", hw->name, hw->whoami);

    Hmm. I'm never that keen on this sort of log noise. Why do you need it
    except for initial debug?

    > + st->name = hw->name;
    > +
    > + /* reset to make sure previous state are not there */
    > + ret = regmap_write(st->map, INV_ICM42600_REG_DEVICE_CONFIG,
    > + INV_ICM42600_DEVICE_CONFIG_SOFT_RESET);
    > + if (ret)
    > + return ret;
    > + msleep(INV_ICM42600_RESET_TIME_MS);

    blank line here to separate two logical blocks of code.
    Slightly helps readability.

    > + ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS, &val);
    > + if (ret)
    > + return ret;
    > + if (!(val & INV_ICM42600_INT_STATUS_RESET_DONE)) {
    > + dev_err(dev, "reset error, reset done bit not set\n");
    > + return -ENODEV;
    > + }
    > +
    > + /* set chip bus configuration */
    > + ret = bus_setup(st);
    > + if (ret)
    > + return ret;
    > +
    > + /* sensor data in big-endian (default) */
    > + mask = INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN;
    > + val = INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN;
    > + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG0,
    > + mask, val);

    Long line, but I'd rather you just didn't bother will local variables
    in cases like this where you just set them to a constant.
    Take the 80 chars thing as guidance not a rule :)

    > + if (ret)
    > + return ret;
    > +
    > + return inv_icm42600_set_conf(st, hw->conf);
    > +}
    > +
    > +static int inv_icm42600_enable_regulator_vddio(struct inv_icm42600_state *st)
    > +{
    > + int ret;
    > +
    > + ret = regulator_enable(st->vddio_supply);
    > + if (ret)
    > + return ret;
    > +
    > + /* wait a little for supply ramp */
    > + usleep_range(3000, 4000);
    > +
    > + return 0;
    > +}
    > +
    > +static void inv_icm42600_disable_regulators(void *_data)
    > +{
    > + struct inv_icm42600_state *st = _data;
    > + const struct device *dev = regmap_get_device(st->map);
    > + int ret;
    > +
    > + ret = regulator_disable(st->vddio_supply);
    > + if (ret)
    > + dev_err(dev, "failed to disable vddio error %d\n", ret);
    > +
    > + ret = regulator_disable(st->vdd_supply);
    > + if (ret)
    > + dev_err(dev, "failed to disable vdd error %d\n", ret);
    > +}
    > +
    > +static void inv_icm42600_disable_pm(void *_data)
    > +{
    > + struct device *dev = _data;
    > +
    > + pm_runtime_put_sync(dev);
    > + pm_runtime_disable(dev);
    > +}
    > +
    > +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
    > + inv_icm42600_bus_setup bus_setup)
    > +{
    > + struct device *dev = regmap_get_device(regmap);
    > + struct inv_icm42600_state *st;
    > + int ret;
    > +
    > + BUILD_BUG_ON(ARRAY_SIZE(inv_icm42600_hw) != INV_CHIP_NB);

    Why not just give the array an explicit size when you define it above?
    I guess it would in theory be possible to not instantiate all of the array
    but relying on different size of a variable length array seems less than
    ideal.

    > + if (chip < 0 || chip >= INV_CHIP_NB) {
    > + dev_err(dev, "invalid chip = %d\n", chip);
    > + return -ENODEV;
    > + }
    > +
    > + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
    > + if (!st)
    > + return -ENOMEM;
    nitpick: blank line here.

    > + dev_set_drvdata(dev, st);
    > + mutex_init(&st->lock);
    > + st->chip = chip;
    > + st->map = regmap;
    > +
    > + ret = iio_read_mount_matrix(dev, "mount-matrix", &st->orientation);
    > + if (ret) {
    > + dev_err(dev, "failed to retrieve mounting matrix %d\n", ret);
    > + return ret;
    > + }
    > +
    > + st->vdd_supply = devm_regulator_get(dev, "vdd");
    > + if (IS_ERR(st->vdd_supply))
    > + return PTR_ERR(st->vdd_supply);
    > +
    > + st->vddio_supply = devm_regulator_get(dev, "vddio");
    > + if (IS_ERR(st->vddio_supply))
    > + return PTR_ERR(st->vddio_supply);
    > +
    > + ret = regulator_enable(st->vdd_supply);
    > + if (ret)
    > + return ret;
    > + msleep(INV_ICM42600_POWER_UP_TIME_MS);
    > +
    > + ret = inv_icm42600_enable_regulator_vddio(st);
    > + if (ret) {
    > + regulator_disable(st->vdd_supply);
    > + return ret;
    > + }
    > +
    > + ret = devm_add_action_or_reset(dev, inv_icm42600_disable_regulators,
    > + st);

    I'd prefer to see two devm_add_action_or_reset calls. One for each regulator.
    That means you don't have to do the extra disable logic above which is
    a bit fragile in amongst a whole load of device managed calls.

    > + if (ret)
    > + return ret;
    > +
    > + /* setup chip registers */
    > + ret = inv_icm42600_setup(st, bus_setup);
    > + if (ret)
    > + return ret;
    > +
    > + /* setup runtime power management */
    > + ret = pm_runtime_set_active(dev);
    > + if (ret)
    > + return ret;
    > + pm_runtime_get_noresume(dev);
    > + pm_runtime_enable(dev);
    > + pm_runtime_set_autosuspend_delay(dev, INV_ICM42600_SUSPEND_DELAY_MS);
    > + pm_runtime_use_autosuspend(dev);
    > + pm_runtime_put(dev);
    > +
    > + return devm_add_action_or_reset(dev, inv_icm42600_disable_pm, dev);
    > +}
    > +EXPORT_SYMBOL_GPL(inv_icm42600_core_probe);
    > +
    > +static int __maybe_unused inv_icm42600_suspend(struct device *dev)
    > +{
    > + struct inv_icm42600_state *st = dev_get_drvdata(dev);
    > + int ret;
    > +
    > + mutex_lock(&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)) {
    > + ret = 0;
    > + goto out_unlock;
    > + }
    > +
    > + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
    > + INV_ICM42600_SENSOR_MODE_OFF, false,
    > + NULL);
    > + if (ret)
    > + goto out_unlock;
    > +
    > + regulator_disable(st->vddio_supply);
    > +
    > +out_unlock:
    > + mutex_unlock(&st->lock);
    > + return ret;
    > +}
    > +
    > +static int __maybe_unused inv_icm42600_resume(struct device *dev)
    > +{
    > + struct inv_icm42600_state *st = dev_get_drvdata(dev);
    > + int ret;
    > +
    > + mutex_lock(&st->lock);
    > +
    > + ret = inv_icm42600_enable_regulator_vddio(st);
    > + if (ret)
    > + goto out_unlock;
    > +
    > + pm_runtime_disable(dev);
    > + pm_runtime_set_active(dev);
    > + pm_runtime_enable(dev);
    > +
    > + /* restore sensors state */
    > + ret = inv_icm42600_set_pwr_mgmt0(st, st->suspended.gyro,
    > + st->suspended.accel,
    > + st->suspended.temp, NULL);
    > + if (ret)
    > + goto out_unlock;

    You may need this later, but for now it's a bit comic so ideally introduce
    it only when needed.

    > +
    > +out_unlock:
    > + mutex_unlock(&st->lock);
    > + return ret;
    > +}
    > +
    > +static int __maybe_unused inv_icm42600_runtime_suspend(struct device *dev)
    > +{
    > + struct inv_icm42600_state *st = dev_get_drvdata(dev);
    > + int ret;
    > +
    > + mutex_lock(&st->lock);
    > +
    > + /* disable all sensors */
    > + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
    > + INV_ICM42600_SENSOR_MODE_OFF, false,
    > + NULL);
    > + if (ret)
    > + goto error_unlock;
    > +
    > + regulator_disable(st->vddio_supply);
    > +
    > +error_unlock:
    > + mutex_unlock(&st->lock);
    > + return ret;
    > +}
    > +
    > +static int __maybe_unused inv_icm42600_runtime_resume(struct device *dev)
    > +{
    > + struct inv_icm42600_state *st = dev_get_drvdata(dev);
    > + int ret;
    > +
    > + mutex_lock(&st->lock);
    > +

    Why don't we need to reenable all the sensors we disabled in runtime suspend?
    I can guess why we might not, but a comment here to explain would save on
    possible confusion..

    > + ret = inv_icm42600_enable_regulator_vddio(st);
    > +
    > + mutex_unlock(&st->lock);
    > + return ret;
    > +}
    > +
    > +const struct dev_pm_ops inv_icm42600_pm_ops = {
    > + SET_SYSTEM_SLEEP_PM_OPS(inv_icm42600_suspend, inv_icm42600_resume)
    > + SET_RUNTIME_PM_OPS(inv_icm42600_runtime_suspend,
    > + inv_icm42600_runtime_resume, NULL)
    > +};
    > +EXPORT_SYMBOL_GPL(inv_icm42600_pm_ops);
    > +
    > +MODULE_AUTHOR("InvenSense, Inc.");
    > +MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
    > +MODULE_LICENSE("GPL");


    \
     
     \ /
      Last update: 2020-05-08 15:29    [W:4.801 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site