[PATCH v2 0/2] iio: imu: inv_mpu6050: Add support for ICM-20608-D

[PATCH v2 0/2] iio: imu: inv_mpu6050: Add support for ICM-20608-D

[michael.srba]

From: Michael Srba <Michael.Srba@seznam.cz>

This series copies the invensense icm20608 support in the inv_mpu6050
driver for icm20608d, which is for all intents and purposes identical,
except for the inclusion of a DMP (Digital Motion Processor), which
is deemed significant enough to change the WHOAMI value, thereby making
the driver fail if the invensense,icm20608 compatible is specified.

Since the driver doesn't currently acknowledge that there is such thing
as a DMP core, all that is needed is to copy the icm20608 support and
change the WHOAMI value.

changelog:
 -v2: require specifying "invensense,icm20608" as a fallback compatible
      in the binding, as suggested

Michael Srba (2):
  dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d
  iio: imu: inv_mpu6050: Add support for ICM-20608-D

 .../bindings/iio/imu/invensense,mpu6050.yaml  | 34 +++++++++++--------
 drivers/iio/imu/inv_mpu6050/Kconfig           |  4 +--
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    |  9 +++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  6 ++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  2 ++
 drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  5 +++
 6 files changed, 43 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[michael.srba]

From: Michael Srba <Michael.Srba@seznam.cz>

ICM-20608-D differs from the other ICM-20608 variants by having
a DMP (Digital Motion Processor) core tacked on.
Despite having a different WHOAMI register, this variant is
completely interchangeable with the other ICM-20608 variants
by simply pretending the DMP core doesn't exist.

Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
---
changelog:
 - v2: require specifying "invensense,icm20608" as a fallback compatible
---
 .../bindings/iio/imu/invensense,mpu6050.yaml  | 34 +++++++++++--------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
index d69595a524c1..dbd214e7baba 100644
--- a/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
@@ -14,21 +14,25 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - invensense,iam20680
-      - invensense,icm20608
-      - invensense,icm20609
-      - invensense,icm20689
-      - invensense,icm20602
-      - invensense,icm20690
-      - invensense,mpu6000
-      - invensense,mpu6050
-      - invensense,mpu6500
-      - invensense,mpu6515
-      - invensense,mpu6880
-      - invensense,mpu9150
-      - invensense,mpu9250
-      - invensense,mpu9255
+    oneOf:
+      - enum:
+        - invensense,iam20680
+        - invensense,icm20608
+        - invensense,icm20609
+        - invensense,icm20689
+        - invensense,icm20602
+        - invensense,icm20690
+        - invensense,mpu6000
+        - invensense,mpu6050
+        - invensense,mpu6500
+        - invensense,mpu6515
+        - invensense,mpu6880
+        - invensense,mpu9150
+        - invensense,mpu9250
+        - invensense,mpu9255
+      - items:
+        - const: invensense,icm20608d
+        - const: invensense,icm20608
 
   reg:
     maxItems: 1
-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Krzysztof Kozlowski]

On 11/03/2022 17:15, michael.srba@seznam.cz wrote:
> From: Michael Srba <Michael.Srba@seznam.cz>
> 
> ICM-20608-D differs from the other ICM-20608 variants by having
> a DMP (Digital Motion Processor) core tacked on.
> Despite having a different WHOAMI register, this variant is
> completely interchangeable with the other ICM-20608 variants
> by simply pretending the DMP core doesn't exist.
> 
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> ---
> changelog:
>  - v2: require specifying "invensense,icm20608" as a fallback compatible
> ---
>  .../bindings/iio/imu/invensense,mpu6050.yaml  | 34 +++++++++++--------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Rob Herring]

On Fri, 11 Mar 2022 17:15:59 +0100, michael.srba@seznam.cz wrote:
> From: Michael Srba <Michael.Srba@seznam.cz>
> 
> ICM-20608-D differs from the other ICM-20608 variants by having
> a DMP (Digital Motion Processor) core tacked on.
> Despite having a different WHOAMI register, this variant is
> completely interchangeable with the other ICM-20608 variants
> by simply pretending the DMP core doesn't exist.
> 
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> ---
> changelog:
>  - v2: require specifying "invensense,icm20608" as a fallback compatible
> ---
>  .../bindings/iio/imu/invensense,mpu6050.yaml  | 34 +++++++++++--------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml:19:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml:34:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1604436

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Jonathan Cameron]

On Fri, 11 Mar 2022 17:15:59 +0100
michael.srba@seznam.cz wrote:

> From: Michael Srba <Michael.Srba@seznam.cz>
> 
> ICM-20608-D differs from the other ICM-20608 variants by having
> a DMP (Digital Motion Processor) core tacked on.
> Despite having a different WHOAMI register, this variant is
> completely interchangeable with the other ICM-20608 variants
> by simply pretending the DMP core doesn't exist.
> 
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> ---
> changelog:
>  - v2: require specifying "invensense,icm20608" as a fallback compatible

Apologies that I joined the thread for v1 late, but no. That doesn't work.
If the older driver before the new ID is present with this binding
it won't probe because of the WHOAMI value difference so it's not
compatible.

I'm fine with the v1 version.

> ---
>  .../bindings/iio/imu/invensense,mpu6050.yaml  | 34 +++++++++++--------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
> index d69595a524c1..dbd214e7baba 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,mpu6050.yaml
> @@ -14,21 +14,25 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - invensense,iam20680
> -      - invensense,icm20608
> -      - invensense,icm20609
> -      - invensense,icm20689
> -      - invensense,icm20602
> -      - invensense,icm20690
> -      - invensense,mpu6000
> -      - invensense,mpu6050
> -      - invensense,mpu6500
> -      - invensense,mpu6515
> -      - invensense,mpu6880
> -      - invensense,mpu9150
> -      - invensense,mpu9250
> -      - invensense,mpu9255
> +    oneOf:
> +      - enum:
> +        - invensense,iam20680
> +        - invensense,icm20608
> +        - invensense,icm20609
> +        - invensense,icm20689
> +        - invensense,icm20602
> +        - invensense,icm20690
> +        - invensense,mpu6000
> +        - invensense,mpu6050
> +        - invensense,mpu6500
> +        - invensense,mpu6515
> +        - invensense,mpu6880
> +        - invensense,mpu9150
> +        - invensense,mpu9250
> +        - invensense,mpu9255
> +      - items:
> +        - const: invensense,icm20608d
> +        - const: invensense,icm20608
>  
>    reg:
>      maxItems: 1

Re: [PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Rob Herring]

On Sun, Mar 20, 2022 at 03:15:25PM +0000, Jonathan Cameron wrote:
> On Fri, 11 Mar 2022 17:15:59 +0100
> michael.srba@seznam.cz wrote:
> 
> > From: Michael Srba <Michael.Srba@seznam.cz>
> > 
> > ICM-20608-D differs from the other ICM-20608 variants by having
> > a DMP (Digital Motion Processor) core tacked on.
> > Despite having a different WHOAMI register, this variant is
> > completely interchangeable with the other ICM-20608 variants
> > by simply pretending the DMP core doesn't exist.
> > 
> > Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> > ---
> > changelog:
> >  - v2: require specifying "invensense,icm20608" as a fallback compatible
> 
> Apologies that I joined the thread for v1 late, but no. That doesn't work.
> If the older driver before the new ID is present with this binding
> it won't probe because of the WHOAMI value difference so it's not
> compatible.
> 
> I'm fine with the v1 version.

If the driver didn't check WHOAMI then it would be compatible. So does 
driver implementation determine what's compatible or not? It shouldn't 
as those are supposed to be decoupled.

Generally, if there are h/w id registers, then we'll rely on them and 
don't need a compatible for every variant.

Rob

Re: [PATCH v2 1/2] dt-bindings: iio: imu: mpu6050: Document invensense,icm20608d

[Jonathan Cameron]

On Sun, 20 Mar 2022 13:56:18 -0400
Rob Herring <robh@kernel.org> wrote:

> On Sun, Mar 20, 2022 at 03:15:25PM +0000, Jonathan Cameron wrote:
> > On Fri, 11 Mar 2022 17:15:59 +0100
> > michael.srba@seznam.cz wrote:
> >   
> > > From: Michael Srba <Michael.Srba@seznam.cz>
> > > 
> > > ICM-20608-D differs from the other ICM-20608 variants by having
> > > a DMP (Digital Motion Processor) core tacked on.
> > > Despite having a different WHOAMI register, this variant is
> > > completely interchangeable with the other ICM-20608 variants
> > > by simply pretending the DMP core doesn't exist.
> > > 
> > > Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> > > ---
> > > changelog:
> > >  - v2: require specifying "invensense,icm20608" as a fallback compatible  
> > 
> > Apologies that I joined the thread for v1 late, but no. That doesn't work.
> > If the older driver before the new ID is present with this binding
> > it won't probe because of the WHOAMI value difference so it's not
> > compatible.
> > 
> > I'm fine with the v1 version.  
> 
> If the driver didn't check WHOAMI then it would be compatible. So does 
> driver implementation determine what's compatible or not? It shouldn't 
> as those are supposed to be decoupled.

OK. That decoupling needs perhaps to be more clearly stated as it wasn't
something I was keeping an eye open for in drivers.

It does check them and warns if there isn't a match. This is partly historical
because we had board implementers switch to an 'mostly' compatible part
that was fine with their software stack but not with the Linux drivers and
those bugs were hard to diagnose. We didn't support he particular WHOAMI
at the time so the check was dded.

The code could be improved as it carries on using the chip
specified when it should perhaps use the one matched on WHOAMI.  
The discussion of the original patch that added the check
"iio: inv_mpu6050: Check WHO_AM_I register on probe"
included a request for such a change as a follow up patch.
I guess that never showed up.

The driver supports a bunch of parts that aren't completely compatible
from a register interface point of view + we have other drivers supporting
additional parts. The particular question of which part is supported
by what driver is a choice that other software stacks may have made
differently. So we can't have a general compatible covering everything
supported by the driver.

Having said that we could do subsets where a particular compatible
maps to some of the supported parts where the compatibility is such that
it is unlikely another OS would chose to support them with different
drivers.

> 
> Generally, if there are h/w id registers, then we'll rely on them and 
> don't need a compatible for every variant.

I don't mind the driver moving to that model, but it's not true today
and we'd still have to be careful with what we describe with each
compatible.

Jonathan
> 
> Rob

[PATCH v2 2/2] iio: imu: inv_mpu6050: Add support for ICM-20608-D

[michael.srba]

From: Michael Srba <Michael.Srba@seznam.cz>

The difference between the ICM-20608-D and the other ICM-20608
variants is the addition of a DMP (Digital Motion Processor) core.
This difference is deemed substantial enough to change the WHOAMI
register value.
Since this driver doesn't currently acknowledge the exisence of
something like a DMP core, simply copy ICM-20608 except for the
aforementioned WHOAMI register.

Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
Acked-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
changes:
 - v2: none
---
 drivers/iio/imu/inv_mpu6050/Kconfig        | 4 ++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 9 +++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c  | 6 ++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  | 2 ++
 drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c  | 5 +++++
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig b/drivers/iio/imu/inv_mpu6050/Kconfig
index 9c625517173a..3636b1bc90f1 100644
--- a/drivers/iio/imu/inv_mpu6050/Kconfig
+++ b/drivers/iio/imu/inv_mpu6050/Kconfig
@@ -16,7 +16,7 @@ config INV_MPU6050_I2C
 	select REGMAP_I2C
 	help
 	  This driver supports the Invensense MPU6050/9150,
-	  MPU6500/6515/6880/9250/9255, ICM20608/20609/20689, ICM20602/ICM20690
+	  MPU6500/6515/6880/9250/9255, ICM20608(D)/20609/20689, ICM20602/ICM20690
 	  and IAM20680 motion tracking devices over I2C.
 	  This driver can be built as a module. The module will be called
 	  inv-mpu6050-i2c.
@@ -28,7 +28,7 @@ config INV_MPU6050_SPI
 	select REGMAP_SPI
 	help
 	  This driver supports the Invensense MPU6000,
-	  MPU6500/6515/6880/9250/9255, ICM20608/20609/20689, ICM20602/ICM20690
+	  MPU6500/6515/6880/9250/9255, ICM20608(D)/20609/20689, ICM20602/ICM20690
 	  and IAM20680 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 597768c29a72..86fbbe904050 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -217,6 +217,15 @@ static const struct inv_mpu6050_hw hw_info[] = {
 		.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
 		.startup_time = {INV_MPU6500_GYRO_STARTUP_TIME, INV_MPU6500_ACCEL_STARTUP_TIME},
 	},
+	{
+		.whoami = INV_ICM20608D_WHOAMI_VALUE,
+		.name = "ICM20608D",
+		.reg = &reg_set_6500,
+		.config = &chip_config_6500,
+		.fifo_size = 512,
+		.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
+		.startup_time = {INV_MPU6500_GYRO_STARTUP_TIME, INV_MPU6500_ACCEL_STARTUP_TIME},
+	},
 	{
 		.whoami = INV_ICM20609_WHOAMI_VALUE,
 		.name = "ICM20609",
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index fe03707ec2d3..ed52b27409ac 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -29,6 +29,7 @@ static bool inv_mpu_i2c_aux_bus(struct device *dev)
 
 	switch (st->chip_type) {
 	case INV_ICM20608:
+	case INV_ICM20608D:
 	case INV_ICM20609:
 	case INV_ICM20689:
 	case INV_ICM20602:
@@ -182,6 +183,7 @@ static const struct i2c_device_id inv_mpu_id[] = {
 	{"mpu9250", INV_MPU9250},
 	{"mpu9255", INV_MPU9255},
 	{"icm20608", INV_ICM20608},
+	{"icm20608d", INV_ICM20608D},
 	{"icm20609", INV_ICM20609},
 	{"icm20689", INV_ICM20689},
 	{"icm20602", INV_ICM20602},
@@ -225,6 +227,10 @@ static const struct of_device_id inv_of_match[] = {
 		.compatible = "invensense,icm20608",
 		.data = (void *)INV_ICM20608
 	},
+	{
+		.compatible = "invensense,icm20608d",
+		.data = (void *)INV_ICM20608D
+	},
 	{
 		.compatible = "invensense,icm20609",
 		.data = (void *)INV_ICM20609
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index c6aa36ee966a..8e14f20b1314 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -76,6 +76,7 @@ enum inv_devices {
 	INV_MPU9250,
 	INV_MPU9255,
 	INV_ICM20608,
+	INV_ICM20608D,
 	INV_ICM20609,
 	INV_ICM20689,
 	INV_ICM20602,
@@ -394,6 +395,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_ICM20608D_WHOAMI_VALUE		0xAE
 #define INV_ICM20609_WHOAMI_VALUE		0xA6
 #define INV_ICM20689_WHOAMI_VALUE		0x98
 #define INV_ICM20602_WHOAMI_VALUE		0x12
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index 6800356b25fb..ce8ab6db2bf2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -74,6 +74,7 @@ static const struct spi_device_id inv_mpu_id[] = {
 	{"mpu9250", INV_MPU9250},
 	{"mpu9255", INV_MPU9255},
 	{"icm20608", INV_ICM20608},
+	{"icm20608d", INV_ICM20608D},
 	{"icm20609", INV_ICM20609},
 	{"icm20689", INV_ICM20689},
 	{"icm20602", INV_ICM20602},
@@ -113,6 +114,10 @@ static const struct of_device_id inv_of_match[] = {
 		.compatible = "invensense,icm20608",
 		.data = (void *)INV_ICM20608
 	},
+	{
+		.compatible = "invensense,icm20608d",
+		.data = (void *)INV_ICM20608D
+	},
 	{
 		.compatible = "invensense,icm20609",
 		.data = (void *)INV_ICM20609
-- 
2.34.1

Re: [PATCH v2 2/2] iio: imu: inv_mpu6050: Add support for ICM-20608-D

[Krzysztof Kozlowski]

On 11/03/2022 17:16, michael.srba@seznam.cz wrote:
> From: Michael Srba <Michael.Srba@seznam.cz>
> 
> The difference between the ICM-20608-D and the other ICM-20608
> variants is the addition of a DMP (Digital Motion Processor) core.
> This difference is deemed substantial enough to change the WHOAMI
> register value.
> Since this driver doesn't currently acknowledge the exisence of
> something like a DMP core, simply copy ICM-20608 except for the
> aforementioned WHOAMI register.
> 
> Signed-off-by: Michael Srba <Michael.Srba@seznam.cz>
> Acked-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> ---
> changes:
>  - v2: none
> ---
>  drivers/iio/imu/inv_mpu6050/Kconfig        | 4 ++--
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 9 +++++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c  | 6 ++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  | 2 ++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c  | 5 +++++
>  5 files changed, 24 insertions(+), 2 deletions(-)
> 

FWIW:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof