Re: [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602

From: Jonathan Cameron
Date: Sat Mar 30 2019 - 14:29:42 EST

Next message: Ferdinand Blomqvist: "[RFC PATCH 3/7] rslib: decode_rs: Fix length parameter check"
Previous message: Hariprasad Kelam: "[PATCH v2] trace: events: fix error directive in argument list"
In reply to: stevemo: "[PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, 26 Mar 2019 02:01:45 -0700
stevemo@xxxxxxxxxx wrote:
+CC Randolph, who I think introduced the support.

> From: Steve Moskovchenko <stevemo@xxxxxxxxxx>
>
> The MPU6050 driver has recently gained support for the
> ICM20602 IMU, which is very similar to MPU6xxx. However,
> the ICM20602's Gyro data specifically includes temperature
> readings, which were not present on MPU6xxx parts. As a
> result, the driver will under-read the ICM20602's FIFO
> register, causing the same (partial) sample to be returned
> for all reads, until the FIFO overflows.
>
> Fix this by adding a table of scan elements speciically for
> the ICM20602, which takes the extra temperature data into
> consideration.
>
> While we're at it, fix the temperature offset and scaling
> on ICM20602, since it uses different scale/offset constants
> than the rest of the MPU6xxx devices.
>
> Signed-off-by: Steve Moskovchenko <stevemo@xxxxxxxxxx>
Hi Steve

This all looks sensible to me. Ideally should have had a fixes tag.

If no one shouts in the next few days I'll pick this up and add one.
Give me a poke if I seem to have forgotten it in a week or so.

Thanks,

Jonathan

> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 46 ++++++++++++++++++++--
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 20 +++++++++-
> drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 8 +++-
> 3 files changed, 67 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 650de0fefb7b..fedd3f2b0135 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -471,7 +471,10 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> *val = 0;
> - *val2 = INV_MPU6050_TEMP_SCALE;
> + if (st->chip_type == INV_ICM20602)
> + *val2 = INV_ICM20602_TEMP_SCALE;
> + else
> + *val2 = INV_MPU6050_TEMP_SCALE;
>
> return IIO_VAL_INT_PLUS_MICRO;
> default:
> @@ -480,7 +483,10 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_OFFSET:
> switch (chan->type) {
> case IIO_TEMP:
> - *val = INV_MPU6050_TEMP_OFFSET;
> + if (st->chip_type == INV_ICM20602)
> + *val = INV_ICM20602_TEMP_OFFSET;
> + else
> + *val = INV_MPU6050_TEMP_OFFSET;
>
> return IIO_VAL_INT;
> default:
> @@ -845,6 +851,32 @@ static const struct iio_chan_spec inv_mpu_channels[] = {
> INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z),
> };
>
> +static const struct iio_chan_spec inv_icm20602_channels[] = {
> + IIO_CHAN_SOFT_TIMESTAMP(INV_ICM20602_SCAN_TIMESTAMP),
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
> + | BIT(IIO_CHAN_INFO_OFFSET)
> + | BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = INV_ICM20602_SCAN_GYRO_TEMP,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .shift = 0,
> + .endianness = IIO_BE,
> + },
> + },
> +
> + INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_X, INV_ICM20602_SCAN_GYRO_X),
> + INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, INV_ICM20602_SCAN_GYRO_Y),
> + INV_MPU6050_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, INV_ICM20602_SCAN_GYRO_Z),
> +
> + INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Y, INV_ICM20602_SCAN_ACCL_Y),
> + INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_X, INV_ICM20602_SCAN_ACCL_X),
> + INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_ICM20602_SCAN_ACCL_Z),
> +};
> +
> /*
> * The user can choose any frequency between INV_MPU6050_MIN_FIFO_RATE and
> * INV_MPU6050_MAX_FIFO_RATE, but only these frequencies are matched by the
> @@ -1100,8 +1132,14 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> indio_dev->name = name;
> else
> indio_dev->name = dev_name(dev);
> - indio_dev->channels = inv_mpu_channels;
> - indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels);
> +
> + if (chip_type == INV_ICM20602) {
> + indio_dev->channels = inv_icm20602_channels;
> + indio_dev->num_channels = ARRAY_SIZE(inv_icm20602_channels);
> + } else {
> + indio_dev->channels = inv_mpu_channels;
> + indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels);
> + }
>
> indio_dev->info = &mpu_info;
> indio_dev->modes = INDIO_BUFFER_TRIGGERED;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 325afd9f5f61..eb04391feaa8 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -208,6 +208,9 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_BYTES_PER_3AXIS_SENSOR 6
> #define INV_MPU6050_FIFO_COUNT_BYTE 2
>
> +/* ICM20602 gyro data includes temperature */
> +#define INV_ICM20602_BYTES_PER_GYRO_SENSOR 8
> +
> /* mpu6500 registers */
> #define INV_MPU6500_REG_ACCEL_CONFIG_2 0x1D
> #define INV_MPU6500_REG_ACCEL_OFFSET 0x77
> @@ -229,6 +232,9 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_GYRO_CONFIG_FSR_SHIFT 3
> #define INV_MPU6050_ACCL_CONFIG_FSR_SHIFT 3
>
> +#define INV_ICM20602_TEMP_OFFSET 8170
> +#define INV_ICM20602_TEMP_SCALE 3060
> +
> /* 6 + 6 round up and plus 8 */
> #define INV_MPU6050_OUTPUT_DATA_SIZE 24
>
> @@ -270,7 +276,7 @@ struct inv_mpu6050_state {
> #define INV_ICM20608_WHOAMI_VALUE 0xAF
> #define INV_ICM20602_WHOAMI_VALUE 0x12
>
> -/* scan element definition */
> +/* scan element definition for generic MPU6xxx devices */
> enum inv_mpu6050_scan {
> INV_MPU6050_SCAN_ACCL_X,
> INV_MPU6050_SCAN_ACCL_Y,
> @@ -281,6 +287,18 @@ enum inv_mpu6050_scan {
> INV_MPU6050_SCAN_TIMESTAMP,
> };
>
> +/* scan element definition for ICM20602, which includes gyro temp */
> +enum inv_icm20602_scan {
> + INV_ICM20602_SCAN_ACCL_X,
> + INV_ICM20602_SCAN_ACCL_Y,
> + INV_ICM20602_SCAN_ACCL_Z,
> + INV_ICM20602_SCAN_GYRO_TEMP,
> + INV_ICM20602_SCAN_GYRO_X,
> + INV_ICM20602_SCAN_GYRO_Y,
> + INV_ICM20602_SCAN_GYRO_Z,
> + INV_ICM20602_SCAN_TIMESTAMP,
> +};
> +
> enum inv_mpu6050_filter_e {
> INV_MPU6050_FILTER_256HZ_NOLPF2 = 0,
> INV_MPU6050_FILTER_188HZ,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 548e042f7b5b..20828d6a13f9 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -204,8 +204,12 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> if (st->chip_config.accl_fifo_enable)
> bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
>
> - if (st->chip_config.gyro_fifo_enable)
> - bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
> + if (st->chip_config.gyro_fifo_enable) {
> + if (st->chip_type == INV_ICM20602)
> + bytes_per_datum += INV_ICM20602_BYTES_PER_GYRO_SENSOR;
> + else
> + bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
> + }
>
> /*
> * read fifo_count register to know how many bytes are inside the FIFO

Next message: Ferdinand Blomqvist: "[RFC PATCH 3/7] rslib: decode_rs: Fix length parameter check"
Previous message: Hariprasad Kelam: "[PATCH v2] trace: events: fix error directive in argument list"
In reply to: stevemo: "[PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]