linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602
@ 2019-03-26  9:01 stevemo
  2019-03-30 18:29 ` Jonathan Cameron
  2019-04-03  6:28 ` [PATCH v2] " stevemo
  0 siblings, 2 replies; 8+ messages in thread
From: stevemo @ 2019-03-26  9:01 UTC (permalink / raw)
  To: gaireg
  Cc: Steve Moskovchenko, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	Jean-Baptiste Maneyrol, Martin Kelly, Jonathan Marek,
	Brian Masney, Rob Herring, Douglas Fischer, linux-iio,
	linux-kernel

From: Steve Moskovchenko <stevemo@skydio.com>

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@skydio.com>
---
 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
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-03-26  9:01 [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602 stevemo
@ 2019-03-30 18:29 ` Jonathan Cameron
  2019-04-01 13:51   ` Jean-Baptiste Maneyrol
  2019-04-03  6:28 ` [PATCH v2] " stevemo
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2019-03-30 18:29 UTC (permalink / raw)
  To: stevemo
  Cc: gaireg, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jean-Baptiste Maneyrol, Martin Kelly,
	Jonathan Marek, Brian Masney, Rob Herring, Douglas Fischer,
	linux-iio, linux-kernel, Randolph Maaßen

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

> From: Steve Moskovchenko <stevemo@skydio.com>
> 
> 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@skydio.com>
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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-03-30 18:29 ` Jonathan Cameron
@ 2019-04-01 13:51   ` Jean-Baptiste Maneyrol
  2019-04-03  6:15     ` Stepan Moskovchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Baptiste Maneyrol @ 2019-04-01 13:51 UTC (permalink / raw)
  To: Jonathan Cameron, stevemo
  Cc: Randolph Maaßen, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Martin Kelly, Jonathan Marek,
	Brian Masney, Rob Herring, Douglas Fischer, linux-iio,
	linux-kernel

Hello,

it is right that temperature is always put inside the FIFO with this chip.

But beware that it should also be the case when accelerometer only is enabled. The FIFO should contain accel data + temp. You can easily verify that by looking at the size of a sample inside the FIFO when only accelerometer is enabled (should be 8 instead of 6).

If this is confirmed, you need to modify your code to handle this case correctly.

Best regards,
Jean-Baptiste Maneyrol



From: Jonathan Cameron <jic23@kernel.org>
Sent: Saturday, March 30, 2019 19:29
To: stevemo@skydio.com
Cc: gaireg@gaireg.de; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Jean-Baptiste Maneyrol; Martin Kelly; Jonathan Marek; Brian Masney; Rob Herring; Douglas Fischer; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Randolph Maaßen
Subject: Re: [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

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

> From: Steve Moskovchenko <stevemo@skydio.com>
>
> 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@skydio.com>
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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-04-01 13:51   ` Jean-Baptiste Maneyrol
@ 2019-04-03  6:15     ` Stepan Moskovchenko
  0 siblings, 0 replies; 8+ messages in thread
From: Stepan Moskovchenko @ 2019-04-03  6:15 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol
  Cc: Jonathan Cameron, Randolph Maaßen, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Martin Kelly,
	Jonathan Marek, Brian Masney, Rob Herring, Douglas Fischer,
	linux-iio, linux-kernel

Hi Jean-Baptiste,
That's a good catch. I didn't realize the accel-only configuration
included temperature data as well. I've confirmed this behavior and
will send out a v2 shortly. Thanks!
-Steve

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-03-26  9:01 [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602 stevemo
  2019-03-30 18:29 ` Jonathan Cameron
@ 2019-04-03  6:28 ` stevemo
  2019-04-07 11:45   ` Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: stevemo @ 2019-04-03  6:28 UTC (permalink / raw)
  To: jmaneyrol
  Cc: Steve Moskovchenko, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Martin Kelly,
	Jonathan Marek, Brian Masney, Randolph Maaßen,
	Douglas Fischer, linux-iio, linux-kernel

From: Steve Moskovchenko <stevemo@skydio.com>

The MPU6050 driver has recently gained support for the
ICM20602 IMU, which is very similar to MPU6xxx. However,
the ICM20602's FIFO 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 specifically
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@skydio.com>
---
v2: Read temperature when running in accel-only mode, too.

 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 |  3 ++
 3 files changed, 64 insertions(+), 5 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..2ed4b98e0cd7 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 FIFO samples include temperature readings */
+#define INV_ICM20602_BYTES_PER_TEMP_SENSOR   2
+
 /* 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 temperature */
+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..57bd11bde56b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -207,6 +207,9 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	if (st->chip_config.gyro_fifo_enable)
 		bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
 
+	if (st->chip_type == INV_ICM20602)
+		bytes_per_datum += INV_ICM20602_BYTES_PER_TEMP_SENSOR;
+
 	/*
 	 * read fifo_count register to know how many bytes are inside the FIFO
 	 * right now
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-04-03  6:28 ` [PATCH v2] " stevemo
@ 2019-04-07 11:45   ` Jonathan Cameron
  2019-04-08 14:32     ` Jean-Baptiste Maneyrol
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2019-04-07 11:45 UTC (permalink / raw)
  To: stevemo
  Cc: jmaneyrol, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Martin Kelly, Jonathan Marek,
	Brian Masney, Randolph Maaßen, Douglas Fischer, linux-iio,
	linux-kernel

On Tue,  2 Apr 2019 23:28:56 -0700
stevemo@skydio.com wrote:

> From: Steve Moskovchenko <stevemo@skydio.com>
> 
> The MPU6050 driver has recently gained support for the
> ICM20602 IMU, which is very similar to MPU6xxx. However,
> the ICM20602's FIFO 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 specifically
> 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@skydio.com>
I'd like a reviewed-by or acked-by from Jean-Baptiste on this before
I take it.

thanks,

Jonathan

> ---
> v2: Read temperature when running in accel-only mode, too.
> 
>  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 |  3 ++
>  3 files changed, 64 insertions(+), 5 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..2ed4b98e0cd7 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 FIFO samples include temperature readings */
> +#define INV_ICM20602_BYTES_PER_TEMP_SENSOR   2
> +
>  /* 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 temperature */
> +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..57bd11bde56b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -207,6 +207,9 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  	if (st->chip_config.gyro_fifo_enable)
>  		bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
>  
> +	if (st->chip_type == INV_ICM20602)
> +		bytes_per_datum += INV_ICM20602_BYTES_PER_TEMP_SENSOR;
> +
>  	/*
>  	 * read fifo_count register to know how many bytes are inside the FIFO
>  	 * right now


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-04-07 11:45   ` Jonathan Cameron
@ 2019-04-08 14:32     ` Jean-Baptiste Maneyrol
  2019-04-14 10:46       ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Jean-Baptiste Maneyrol @ 2019-04-08 14:32 UTC (permalink / raw)
  To: Jonathan Cameron, stevemo
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Martin Kelly, Jonathan Marek, Brian Masney, Randolph Maaßen,
	Douglas Fischer, linux-iio, linux-kernel

Hello,

overall looks good for me.

I would just prefer to change the define name for temperature to INV_ICM20602_SCAN_TEMP. It is the chip temperature that can be used for temperature compensation for both accel and gyro data.

But it is really just a details.

Best regards,
Jean-Baptiste Maneyrol


From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, April 7, 2019 13:45
To: stevemo@skydio.com
Cc: Jean-Baptiste Maneyrol; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Martin Kelly; Jonathan Marek; Brian Masney; Randolph Maaßen; Douglas Fischer; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

On Tue,  2 Apr 2019 23:28:56 -0700
stevemo@skydio.com wrote:

> From: Steve Moskovchenko <stevemo@skydio.com>
>
> The MPU6050 driver has recently gained support for the
> ICM20602 IMU, which is very similar to MPU6xxx. However,
> the ICM20602's FIFO 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 specifically
> 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@skydio.com>
I'd like a reviewed-by or acked-by from Jean-Baptiste on this before
I take it.

thanks,

Jonathan

> ---
> v2: Read temperature when running in accel-only mode, too.
>
>  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 |  3 ++
>  3 files changed, 64 insertions(+), 5 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..2ed4b98e0cd7 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 FIFO samples include temperature readings */
> +#define INV_ICM20602_BYTES_PER_TEMP_SENSOR   2
> +
>  /* 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 temperature */
> +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..57bd11bde56b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -207,6 +207,9 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>        if (st->chip_config.gyro_fifo_enable)
>                bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
> 
> +     if (st->chip_type == INV_ICM20602)
> +             bytes_per_datum += INV_ICM20602_BYTES_PER_TEMP_SENSOR;
> +
>        /*
>         * read fifo_count register to know how many bytes are inside the FIFO
>         * right now


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
  2019-04-08 14:32     ` Jean-Baptiste Maneyrol
@ 2019-04-14 10:46       ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2019-04-14 10:46 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol
  Cc: stevemo, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Martin Kelly, Jonathan Marek,
	Brian Masney, Randolph Maaßen, Douglas Fischer, linux-iio,
	linux-kernel

On Mon, 8 Apr 2019 14:32:00 +0000
Jean-Baptiste Maneyrol <JManeyrol@invensense.com> wrote:

> Hello,
> 
> overall looks good for me.
> 
> I would just prefer to change the define name for temperature to INV_ICM20602_SCAN_TEMP. It is the chip temperature that can be used for temperature compensation for both accel and gyro data.
> 
> But it is really just a details.
> 
> Best regards,
> Jean-Baptiste Maneyrol

Rather than go around again, I've made the change and applied the
patch to the fixes-togreg branch of iio.git.  I also added a
fixes tag.

Thanks,

Jonathan

> 
> 
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, April 7, 2019 13:45
> To: stevemo@skydio.com
> Cc: Jean-Baptiste Maneyrol; Hartmut Knaack; Lars-Peter Clausen; Peter Meerwald-Stadler; Martin Kelly; Jonathan Marek; Brian Masney; Randolph Maaßen; Douglas Fischer; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] iio: imu: mpu6050: Fix FIFO layout for ICM20602
>  
>  CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> On Tue,  2 Apr 2019 23:28:56 -0700
> stevemo@skydio.com wrote:
> 
> > From: Steve Moskovchenko <stevemo@skydio.com>
> >
> > The MPU6050 driver has recently gained support for the
> > ICM20602 IMU, which is very similar to MPU6xxx. However,
> > the ICM20602's FIFO 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 specifically
> > 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@skydio.com>  
> I'd like a reviewed-by or acked-by from Jean-Baptiste on this before
> I take it.
> 
> thanks,
> 
> Jonathan
> 
> > ---
> > v2: Read temperature when running in accel-only mode, too.
> >
> >  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 |  3 ++
> >  3 files changed, 64 insertions(+), 5 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..2ed4b98e0cd7 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 FIFO samples include temperature readings */
> > +#define INV_ICM20602_BYTES_PER_TEMP_SENSOR   2
> > +
> >  /* 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 temperature */
> > +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..57bd11bde56b 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > @@ -207,6 +207,9 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> >        if (st->chip_config.gyro_fifo_enable)
> >                bytes_per_datum += INV_MPU6050_BYTES_PER_3AXIS_SENSOR;
> > 
> > +     if (st->chip_type == INV_ICM20602)
> > +             bytes_per_datum += INV_ICM20602_BYTES_PER_TEMP_SENSOR;
> > +
> >        /*
> >         * read fifo_count register to know how many bytes are inside the FIFO
> >         * right now  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-04-14 10:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26  9:01 [PATCH] iio: imu: mpu6050: Fix FIFO layout for ICM20602 stevemo
2019-03-30 18:29 ` Jonathan Cameron
2019-04-01 13:51   ` Jean-Baptiste Maneyrol
2019-04-03  6:15     ` Stepan Moskovchenko
2019-04-03  6:28 ` [PATCH v2] " stevemo
2019-04-07 11:45   ` Jonathan Cameron
2019-04-08 14:32     ` Jean-Baptiste Maneyrol
2019-04-14 10:46       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).