Re: [PATCH 2/2] iio: imu: mpu6050: Add support for the ICM 20602 IMU

From: Randolph Maaßen
Date: Mon Jan 28 2019 - 13:36:42 EST

Next message: Liang, Kan: "Re: [PATCH V2 05/12] perf mem: Clean up output format and sort order string"
Previous message: Marc Zyngier: "Re: [PATCH v5] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable"
In reply to: Jonathan Cameron: "Re: [PATCH 2/2] iio: imu: mpu6050: Add support for the ICM 20602 IMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

first, thanks for the review.

Am Samstag, den 26.01.2019, 20:05 +0000 schrieb Jonathan Cameron:
> On Tue, 22 Jan 2019 13:02:14 +0100
> Randolph MaaÃen <gaireg@xxxxxxxxx> wrote:
>
> > The Invensense ICM-20602 is a 6-axis MotionTracking device that
> > combines a 3-axis gyroscope and an 3-axis accelerometer. It is very
> > similar to the ICM-20608 imu which is already supported by the mpu6050
> > driver. The main difference is that the ICM-20602 has the i2c bus
> > disable bit in a separate register.
> >
> > Signed-off-by: Randolph MaaÃen <gaireg@xxxxxxxxx>
>
> There is a slight oddity in here, in that you introduce a
> field in the chip info to describe the register to control the i2c disable
> but then don't actually use it in the code.
>
> You should check if that is 0 and if not, write the separate
> register it provides.

I see, i mixed the register handling up. I intended to use the variable in the
inv_mpu_i2c_disable function but used the define there. I'll send a v2

Randolph

>
> Jonathan
> > ---
> > Âdrivers/iio/imu/inv_mpu6050/KconfigÂÂÂÂÂÂÂÂ|ÂÂ8 ++++----
> > Âdrivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 29
> > +++++++++++++++++++++++++++++
> > Âdrivers/iio/imu/inv_mpu6050/inv_mpu_i2c.cÂÂ|ÂÂ6 ++++++
> > Âdrivers/iio/imu/inv_mpu6050/inv_mpu_iio.hÂÂ|ÂÂ8 ++++++++
> > Âdrivers/iio/imu/inv_mpu6050/inv_mpu_spi.cÂÂ| 12 +++++++++---
> > Â5 files changed, 56 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig
> > b/drivers/iio/imu/inv_mpu6050/Kconfig
> > index 5483b2ea754d..d2fe9dbddda7 100644
> > --- a/drivers/iio/imu/inv_mpu6050/Kconfig
> > +++ b/drivers/iio/imu/inv_mpu6050/Kconfig
> > @@ -13,8 +13,8 @@ config INV_MPU6050_I2C
> > Â select INV_MPU6050_IIO
> > Â select REGMAP_I2C
> > Â help
> > - ÂÂThis driver supports the Invensense MPU6050/6500/9150 and
> > ICM20608
> > - ÂÂmotion tracking devices over I2C.
> > + ÂÂThis driver supports the Invensense MPU6050/6500/9150 and
> > + ÂÂICM20608/20602 motion tracking devices over I2C.
> > Â ÂÂThis driver can be built as a module. The module will be called
> > Â ÂÂinv-mpu6050-i2c.
> > Â
> > @@ -24,7 +24,7 @@ config INV_MPU6050_SPI
> > Â select INV_MPU6050_IIO
> > Â select REGMAP_SPI
> > Â help
> > - ÂÂThis driver supports the Invensense MPU6050/6500/9150 and
> > ICM20608
> > - ÂÂmotion tracking devices over SPI.
> > + ÂÂThis driver supports the Invensense MPU6050/6500/9150 and
> > + ÂÂICM20608/20602 motion tracking devices over SPI.
> > Â ÂÂThis driver can be built as a module. The module will be called
> > Â ÂÂinv-mpu6050-spi.
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 1e428c196a82..01c856417d8f 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -38,6 +38,29 @@ static const int gyro_scale_6050[] = {133090, 266181,
> > 532362, 1064724};
> > Â */
> > Âstatic const int accel_scale[] = {598, 1196, 2392, 4785};
> > Â
> > +static const struct inv_mpu6050_reg_map reg_set_icm20602 = {
> > + .sample_rate_div = INV_MPU6050_REG_SAMPLE_RATE_DIV,
> > + .lpfÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_CONFIG,
> > + .accel_lpfÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6500_REG_ACCEL_CONFIG_2,
> > + .user_ctrlÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_USER_CTRL,
> > + .fifo_enÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_FIFO_EN,
> > + .gyro_configÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_GYRO_CONFIG,
> > + .accl_configÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_ACCEL_CONFIG,
> > + .fifo_count_hÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_FIFO_COUNT_H,
> > + .fifo_r_wÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_FIFO_R_W,
> > + .raw_gyroÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_RAW_GYRO,
> > + .raw_acclÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_RAW_ACCEL,
> > + .temperatureÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_TEMPERATURE,
> > + .int_enableÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_INT_ENABLE,
> > + .int_statusÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_INT_STATUS,
> > + .pwr_mgmt_1ÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_PWR_MGMT_1,
> > + .pwr_mgmt_2ÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_PWR_MGMT_2,
> > + .int_pin_cfgÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_INT_PIN_CFG,
> > + .accl_offsetÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6500_REG_ACCEL_OFFSET,
> > + .gyro_offsetÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_GYRO_OFFSET,
> > + .i2c_ifÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_ICM20602_REG_I2C_IF,
> > +};
> > +
> > Âstatic const struct inv_mpu6050_reg_map reg_set_6500 = {
> > Â .sample_rate_div = INV_MPU6050_REG_SAMPLE_RATE_DIV,
> > Â .lpfÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ= INV_MPU6050_REG_CONFIG,
> > @@ -140,6 +163,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
> > Â .reg = &reg_set_6500,
> > Â .config = &chip_config_6050,
> > Â },
> > + {
> > + .whoami = INV_ICM20602_WHOAMI_VALUE,
> > + .name = "ICM20602",
> > + .reg = &reg_set_icm20602,
> > + .config = &chip_config_6050,
> > + },
> > Â};
> > Â
> > Âint inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32
> > mask)
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index dd758e3d403d..e46eb4ddea21 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -127,6 +127,7 @@ static int inv_mpu_probe(struct i2c_client *client,
> > Â st = iio_priv(dev_get_drvdata(&client->dev));
> > Â switch (st->chip_type) {
> > Â case INV_ICM20608:
> > + case INV_ICM20602:
> > Â /* no i2c auxiliary bus on the chip */
> > Â break;
> > Â default:
> > @@ -179,6 +180,7 @@ static const struct i2c_device_id inv_mpu_id[] = {
> > Â {"mpu9250", INV_MPU9250},
> > Â {"mpu9255", INV_MPU9255},
> > Â {"icm20608", INV_ICM20608},
> > + {"icm20602", INV_ICM20602},
> > Â {}
> > Â};
> > Â
> > @@ -213,6 +215,10 @@ static const struct of_device_id inv_of_match[] = {
> > Â .compatible = "invensense,icm20608",
> > Â .data = (void *)INV_ICM20608
> > Â },
> > + {
> > + .compatible = "invensense,icm20602",
> > + .data = (void *)INV_ICM20602
> > + },
> > Â { }
> > Â};
> > ÂMODULE_DEVICE_TABLE(of, inv_of_match);
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index 6bcc11fc1b88..325afd9f5f61 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -44,6 +44,7 @@
> > Â *ÂÂ@int_pin_cfg; Controls interrupt pin configuration.
> > Â *ÂÂ@accl_offset: Controls the accelerometer calibration offset.
> > Â *ÂÂ@gyro_offset: Controls the gyroscope calibration offset.
> > + *ÂÂ@i2c_if: Controls the i2c interface
>
> This is introduced but never actually used.
>
> > Â */
> > Âstruct inv_mpu6050_reg_map {
> > Â u8 sample_rate_div;
> > @@ -65,6 +66,7 @@ struct inv_mpu6050_reg_map {
> > Â u8 int_pin_cfg;
> > Â u8 accl_offset;
> > Â u8 gyro_offset;
> > + u8 i2c_if;
> > Â};
> > Â
> > Â/*device enum */
> > @@ -77,6 +79,7 @@ enum inv_devices {
> > Â INV_MPU9250,
> > Â INV_MPU9255,
> > Â INV_ICM20608,
> > + INV_ICM20602,
> > Â INV_NUM_PARTS
> > Â};
> > Â
> > @@ -195,6 +198,10 @@ struct inv_mpu6050_state {
> > Â#define INV_MPU6050_BIT_PWR_ACCL_STBYÂÂÂÂÂÂÂ0x38
> > Â#define INV_MPU6050_BIT_PWR_GYRO_STBYÂÂÂÂÂÂÂ0x07
> > Â
> > +/* ICM20602 register */
> > +#define INV_ICM20602_REG_I2C_IFÂÂÂÂÂÂÂÂÂÂÂÂÂ0x70
> > +#define INV_ICM20602_BIT_I2C_IF_DISÂÂÂÂÂÂÂÂÂ0x40
> > +
> > Â#define INV_MPU6050_REG_FIFO_COUNT_HÂÂÂÂÂÂÂÂ0x72
> > Â#define INV_MPU6050_REG_FIFO_R_WÂÂÂÂÂÂÂÂÂÂÂÂ0x74
> > Â
> > @@ -261,6 +268,7 @@ struct inv_mpu6050_state {
> > Â#define INV_MPU9255_WHOAMI_VALUE 0x73
> > Â#define INV_MPU6515_WHOAMI_VALUE 0x74
> > Â#define INV_ICM20608_WHOAMI_VALUE 0xAF
> > +#define INV_ICM20602_WHOAMI_VALUE 0x12
> > Â
> > Â/* scan element definition */
> > Âenum inv_mpu6050_scan {
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > index 227f50afff22..e5b7213c74f9 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > @@ -31,9 +31,14 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
> > Â if (ret)
> > Â return ret;
> > Â
> > - st->chip_config.user_ctrl |= INV_MPU6050_BIT_I2C_IF_DIS;
> > - ret = regmap_write(st->map, st->reg->user_ctrl,
> > - ÂÂÂst->chip_config.user_ctrl);
> > + if (st->chip_type == INV_ICM20602) {
> > + ret = regmap_write(st->map, INV_ICM20602_REG_I2C_IF,
> > + ÂÂÂINV_ICM20602_BIT_I2C_IF_DIS);
> > + } else {
> > + st->chip_config.user_ctrl |= INV_MPU6050_BIT_I2C_IF_DIS;
> > + ret = regmap_write(st->map, st->reg->user_ctrl,
> > + ÂÂÂst->chip_config.user_ctrl);
> > + }
> > Â if (ret) {
> > Â inv_mpu6050_set_power_itg(st, false);
> > Â return ret;
> > @@ -81,6 +86,7 @@ static const struct spi_device_id inv_mpu_id[] = {
> > Â {"mpu9250", INV_MPU9250},
> > Â {"mpu9255", INV_MPU9255},
> > Â {"icm20608", INV_ICM20608},
> > + {"icm20602", INV_ICM20602},
> > Â {}
> > Â};
> > Â
>
>
--
Mit freundlichen GrÃÃen
Randolph MaaÃen

Next message: Liang, Kan: "Re: [PATCH V2 05/12] perf mem: Clean up output format and sort order string"
Previous message: Marc Zyngier: "Re: [PATCH v5] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable"
In reply to: Jonathan Cameron: "Re: [PATCH 2/2] iio: imu: mpu6050: Add support for the ICM 20602 IMU"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]