| Message ID | 20220311161600.1469-2-michael.srba@seznam.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: imu: inv_mpu6050: Add support for ICM-20608-D | expand |
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
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.
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
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
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
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