[PATCH v5 0/3] dt-bindings: cros-ec: Update for fingerprint devices

[PATCH v5 0/3] dt-bindings: cros-ec: Update for fingerprint devices

[Stephen Boyd]

This patch series introduces a DT binding for chromeos fingerprint
devices. The first two patches tighten up the existing binding and the
final patch introduces the fingerprint binding. As there aren't any
driver patches this can probably go directly through the DT binding tree
if Lee can ack the patches. Or go through mfd tree if DT maintainers
ack/review.

Changes from v4 (https://lore.kernel.org/r/20220321191100.1993-1-swboyd@chromium.org):
 * Drop last patch that implemented driver logic
 * Drop second to last patch because it's not really needed until
   compatible is used.
 * Rolled cros-ec-spi into cros-ec-fp compatible to get all the pieces

Changes from v3 (https://lore.kernel.org/r/20220318015451.2869388-1-swboyd@chromium.org):
 * Drop spi_device_id because it isn't used
 * Dropped struct members for gpios
 * Picked up tags

Changes from v2 (https://lore.kernel.org/r/20220317005814.2496302-1-swboyd@chromium.org):
 * Dropped cros-ec spi dt properties that aren't of use right now
 * Picked up tags

Changes from v1 (https://lore.kernel.org/r/20220314232214.4183078-1-swboyd@chromium.org):
 * Properly do the boot sequence
 * Add a message that we're booting and delaying a while
 * Fix typo in commit text
 * Change binding to not spell out reset-gpios and indicate that boot0
   is about asserting boot mode
 * Split device id to different patch as it's a different topic from
   booting

Stephen Boyd (3):
  dt-bindings: cros-ec: Fix a typo in description
  dt-bindings: cros-ec: Reorganize property availability
  dt-bindings: cros-ec: Add ChromeOS fingerprint binding

 .../bindings/mfd/google,cros-ec.yaml          | 121 ++++++++++++++----
 1 file changed, 99 insertions(+), 22 deletions(-)

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Craig Hesling <hesling@chromium.org>
Cc: Tom Hughes <tomhughes@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Lee Jones <lee.jones@linaro.org>

base-commit: ffb217a13a2eaf6d5bd974fc83036a53ca69f1e2
-- 
https://chromeos.dev

[PATCH v5 1/3] dt-bindings: cros-ec: Fix a typo in description

[Stephen Boyd]

A 's/pf/of/' on rpmsg-name description.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Craig Hesling <hesling@chromium.org>
Cc: Tom Hughes <tomhughes@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index d1f53bd449f7..99eda9ab0651 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -61,7 +61,7 @@ properties:
   mediatek,rpmsg-name:
     description:
       Must be defined if the cros-ec is a rpmsg device for a Mediatek
-      ARM Cortex M4 Co-processor. Contains the name pf the rpmsg
+      ARM Cortex M4 Co-processor. Contains the name of the rpmsg
       device. Used to match the subnode to the rpmsg device announced by
       the SCP.
     $ref: "/schemas/types.yaml#/definitions/string"
-- 
https://chromeos.dev

[PATCH v5 2/3] dt-bindings: cros-ec: Reorganize property availability

[Stephen Boyd]

Various properties in the cros-ec binding only apply to different
compatible strings. For example, the interrupts and reg property are
required for all cros-ec devices except for the rpmsg version. Add some
conditions to update the availability of properties so that they can't
be used with compatibles that don't support them.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Craig Hesling <hesling@chromium.org>
Cc: Tom Hughes <tomhughes@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../bindings/mfd/google,cros-ec.yaml          | 39 +++++++++++++------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index 99eda9ab0651..409ecef967ce 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -148,18 +148,33 @@ patternProperties:
 required:
   - compatible
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - google,cros-ec-i2c
-          - google,cros-ec-rpmsg
-then:
-  properties:
-    google,cros-ec-spi-pre-delay: false
-    google,cros-ec-spi-msg-delay: false
-    spi-max-frequency: false
+allOf:
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: google,cros-ec-spi
+    then:
+      properties:
+        controller-data: false
+        google,cros-ec-spi-pre-delay: false
+        google,cros-ec-spi-msg-delay: false
+        spi-max-frequency: false
+
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: google,cros-ec-rpmsg
+    then:
+      properties:
+        mediatek,rpmsg-name: false
+
+      required:
+        - reg
+        - interrupts
 
 additionalProperties: false
 
-- 
https://chromeos.dev

[PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Stephen Boyd]

Add a binding to describe the fingerprint processor found on Chromebooks
with a fingerprint sensor. Previously we've been describing this with
the google,cros-ec-spi binding but it lacks gpio and regulator control
used during firmware flashing.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Craig Hesling <hesling@chromium.org>
Cc: Tom Hughes <tomhughes@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../bindings/mfd/google,cros-ec.yaml          | 80 ++++++++++++++++---
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index 409ecef967ce..e5fe60beb9fe 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -19,15 +19,19 @@ description:
 properties:
   compatible:
     oneOf:
-      - description:
-          For implementations of the EC is connected through I2C.
-        const: google,cros-ec-i2c
-      - description:
-          For implementations of the EC is connected through SPI.
-        const: google,cros-ec-spi
-      - description:
-          For implementations of the EC is connected through RPMSG.
-        const: google,cros-ec-rpmsg
+      # For implementations of the EC is connected through I2C.
+      - items:
+          - const: google,cros-ec-i2c
+      # For implementations of the FPMCU connected through SPI.
+      - items:
+          - const: google,cros-ec-fp
+          - const: google,cros-ec-spi
+      # For implementations of the EC is connected through SPI.
+      - items:
+          - const: google,cros-ec-spi
+      # For implementations of the EC is connected through RPMSG.
+      - items:
+          - const: google,cros-ec-rpmsg
 
   controller-data:
     description:
@@ -132,6 +136,15 @@ properties:
 
     additionalProperties: false
 
+  reset-gpios:
+    maxItems: 1
+
+  boot0-gpios:
+    maxItems: 1
+    description: Assert for bootloader mode.
+
+  vdd-supply: true
+
 patternProperties:
   "^i2c-tunnel[0-9]*$":
     type: object
@@ -176,6 +189,37 @@ allOf:
         - reg
         - interrupts
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: google,cros-ec-fp
+    then:
+      properties:
+        '#address-cells': false
+        '#size-cells': false
+        typec: false
+        ec-pwm: false
+        keyboard-controller: false
+        proximity: false
+        codecs: false
+        cbas: false
+
+      patternProperties:
+        "^i2c-tunnel[0-9]*$": false
+        "^regulator@[0-9]+$": false
+        "^extcon[0-9]*$": false
+
+      required:
+        - reset-gpios
+        - boot0-gpios
+        - vdd-supply
+    else:
+      properties:
+        reset-gpios: false
+        boot0-gpios: false
+        vdd-supply: false
+
 additionalProperties: false
 
 examples:
@@ -231,4 +275,22 @@ examples:
             compatible = "google,cros-ec-rpmsg";
         };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+      #address-cells = <0x1>;
+      #size-cells = <0x0>;
+      ec@0 {
+        compatible = "google,cros-ec-fp", "google,cros-ec-spi";
+        reg = <0>;
+        interrupt-parent = <&gpio_controller>;
+        interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
+        spi-max-frequency = <3000000>;
+        reset-gpios = <&gpio_controller 5 GPIO_ACTIVE_LOW>;
+        boot0-gpios = <&gpio_controller 10 GPIO_ACTIVE_HIGH>;
+        vdd-supply = <&pp3300_fp_mcu>;
+      };
+    };
 ...
-- 
https://chromeos.dev

Re: [PATCH v5 1/3] dt-bindings: cros-ec: Fix a typo in description

[Guenter Roeck]

On Wed, May 11, 2022 at 6:39 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> A 's/pf/of/' on rpmsg-name description.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index d1f53bd449f7..99eda9ab0651 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -61,7 +61,7 @@ properties:
>    mediatek,rpmsg-name:
>      description:
>        Must be defined if the cros-ec is a rpmsg device for a Mediatek
> -      ARM Cortex M4 Co-processor. Contains the name pf the rpmsg
> +      ARM Cortex M4 Co-processor. Contains the name of the rpmsg
>        device. Used to match the subnode to the rpmsg device announced by
>        the SCP.
>      $ref: "/schemas/types.yaml#/definitions/string"
> --
> https://chromeos.dev
>

Re: [PATCH v5 2/3] dt-bindings: cros-ec: Reorganize property availability

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 6:39 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Various properties in the cros-ec binding only apply to different
> compatible strings. For example, the interrupts and reg property are
> required for all cros-ec devices except for the rpmsg version. Add some
> conditions to update the availability of properties so that they can't
> be used with compatibles that don't support them.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 39 +++++++++++++------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 99eda9ab0651..409ecef967ce 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -148,18 +148,33 @@ patternProperties:
>  required:
>    - compatible
>
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        enum:
> -          - google,cros-ec-i2c
> -          - google,cros-ec-rpmsg
> -then:
> -  properties:
> -    google,cros-ec-spi-pre-delay: false
> -    google,cros-ec-spi-msg-delay: false
> -    spi-max-frequency: false
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              const: google,cros-ec-spi
> +    then:
> +      properties:
> +        controller-data: false
> +        google,cros-ec-spi-pre-delay: false
> +        google,cros-ec-spi-msg-delay: false
> +        spi-max-frequency: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              const: google,cros-ec-rpmsg
> +    then:
> +      properties:
> +        mediatek,rpmsg-name: false
> +
> +      required:
> +        - reg
> +        - interrupts

Is it worth having an "else" clause here? to explicitly make
"mediatek,rpmsg-name" required for mediatek?

In any case, this seems right from my (albeit) limited yaml knowledge
and seems like an improvement.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Doug Anderson]

Hi,

On Wed, May 11, 2022 at 6:39 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> @@ -176,6 +189,37 @@ allOf:
>          - reg
>          - interrupts
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: google,cros-ec-fp
> +    then:
> +      properties:
> +        '#address-cells': false
> +        '#size-cells': false
> +        typec: false
> +        ec-pwm: false
> +        keyboard-controller: false
> +        proximity: false
> +        codecs: false
> +        cbas: false
> +
> +      patternProperties:
> +        "^i2c-tunnel[0-9]*$": false
> +        "^regulator@[0-9]+$": false
> +        "^extcon[0-9]*$": false
> +
> +      required:
> +        - reset-gpios
> +        - boot0-gpios
> +        - vdd-supply
> +    else:
> +      properties:
> +        reset-gpios: false
> +        boot0-gpios: false
> +        vdd-supply: false

Wow, that's a huge pain to have to specify it this way, but I don't
know of any other way to make things happy and still have the
"google,cros-ec-spi" fallback that we decided on having in previous
versions.


>  additionalProperties: false
>
>  examples:
> @@ -231,4 +275,22 @@ examples:
>              compatible = "google,cros-ec-rpmsg";
>          };
>      };
> +
> +  - |

nit: all the other examples have a little "# For <blah>" comment
before them. It's not really necessary, but do we want one here to
match?

In any case, it seems OK to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v5 1/3] dt-bindings: cros-ec: Fix a typo in description

[Rob Herring]

On Wed, May 11, 2022 at 06:39:19PM -0700, Stephen Boyd wrote:
> A 's/pf/of/' on rpmsg-name description.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Rob Herring]

On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> Add a binding to describe the fingerprint processor found on Chromebooks
> with a fingerprint sensor. Previously we've been describing this with
> the google,cros-ec-spi binding but it lacks gpio and regulator control
> used during firmware flashing.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 80 ++++++++++++++++---
>  1 file changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 409ecef967ce..e5fe60beb9fe 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -19,15 +19,19 @@ description:
>  properties:
>    compatible:
>      oneOf:
> -      - description:
> -          For implementations of the EC is connected through I2C.
> -        const: google,cros-ec-i2c
> -      - description:
> -          For implementations of the EC is connected through SPI.
> -        const: google,cros-ec-spi
> -      - description:
> -          For implementations of the EC is connected through RPMSG.
> -        const: google,cros-ec-rpmsg
> +      # For implementations of the EC is connected through I2C.

Using 'description' was preferred.

> +      - items:
> +          - const: google,cros-ec-i2c
> +      # For implementations of the FPMCU connected through SPI.
> +      - items:
> +          - const: google,cros-ec-fp
> +          - const: google,cros-ec-spi
> +      # For implementations of the EC is connected through SPI.
> +      - items:
> +          - const: google,cros-ec-spi
> +      # For implementations of the EC is connected through RPMSG.
> +      - items:
> +          - const: google,cros-ec-rpmsg
>  
>    controller-data:
>      description:
> @@ -132,6 +136,15 @@ properties:
>  
>      additionalProperties: false
>  
> +  reset-gpios:
> +    maxItems: 1
> +
> +  boot0-gpios:
> +    maxItems: 1
> +    description: Assert for bootloader mode.
> +
> +  vdd-supply: true
> +
>  patternProperties:
>    "^i2c-tunnel[0-9]*$":
>      type: object
> @@ -176,6 +189,37 @@ allOf:
>          - reg
>          - interrupts
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: google,cros-ec-fp
> +    then:
> +      properties:
> +        '#address-cells': false
> +        '#size-cells': false
> +        typec: false
> +        ec-pwm: false
> +        keyboard-controller: false
> +        proximity: false
> +        codecs: false
> +        cbas: false
> +
> +      patternProperties:
> +        "^i2c-tunnel[0-9]*$": false
> +        "^regulator@[0-9]+$": false
> +        "^extcon[0-9]*$": false

Is the list of what's allowed shorter? If so, you could list those 
properties and use 'additionalProperties: false'.

Or maybe this is a sign that this should be a separate schema document.

> +
> +      required:
> +        - reset-gpios
> +        - boot0-gpios
> +        - vdd-supply
> +    else:
> +      properties:
> +        reset-gpios: false
> +        boot0-gpios: false
> +        vdd-supply: false

Re: [PATCH v5 2/3] dt-bindings: cros-ec: Reorganize property availability

[Rob Herring]

On Wed, 11 May 2022 18:39:20 -0700, Stephen Boyd wrote:
> Various properties in the cros-ec binding only apply to different
> compatible strings. For example, the interrupts and reg property are
> required for all cros-ec devices except for the rpmsg version. Add some
> conditions to update the availability of properties so that they can't
> be used with compatibles that don't support them.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 39 +++++++++++++------
>  1 file changed, 27 insertions(+), 12 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Stephen Boyd]

Quoting Rob Herring (2022-05-16 08:20:03)
> On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > index 409ecef967ce..e5fe60beb9fe 100644
> > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > @@ -19,15 +19,19 @@ description:
> >  properties:
> >    compatible:
> >      oneOf:
> > -      - description:
> > -          For implementations of the EC is connected through I2C.
> > -        const: google,cros-ec-i2c
> > -      - description:
> > -          For implementations of the EC is connected through SPI.
> > -        const: google,cros-ec-spi
> > -      - description:
> > -          For implementations of the EC is connected through RPMSG.
> > -        const: google,cros-ec-rpmsg
> > +      # For implementations of the EC is connected through I2C.
>
> Using 'description' was preferred.

I just learned that I should use anyOf instead of oneOf. Confusing!

>
> > +      - items:
> > +          - const: google,cros-ec-i2c
> > +      # For implementations of the FPMCU connected through SPI.
> > +      - items:
> > +          - const: google,cros-ec-fp
> > +          - const: google,cros-ec-spi
> > +      # For implementations of the EC is connected through SPI.
> > +      - items:
> > +          - const: google,cros-ec-spi
> > +      # For implementations of the EC is connected through RPMSG.
> > +      - items:
> > +          - const: google,cros-ec-rpmsg
> >
> >    controller-data:
> >      description:
> > @@ -176,6 +189,37 @@ allOf:
> >          - reg
> >          - interrupts
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: google,cros-ec-fp
> > +    then:
> > +      properties:
> > +        '#address-cells': false
> > +        '#size-cells': false
> > +        typec: false
> > +        ec-pwm: false
> > +        keyboard-controller: false
> > +        proximity: false
> > +        codecs: false
> > +        cbas: false
> > +
> > +      patternProperties:
> > +        "^i2c-tunnel[0-9]*$": false
> > +        "^regulator@[0-9]+$": false
> > +        "^extcon[0-9]*$": false
>
> Is the list of what's allowed shorter? If so, you could list those
> properties and use 'additionalProperties: false'.

Yes.

>
> Or maybe this is a sign that this should be a separate schema document.

I couldn't figure that out. I tried to add new properties here but it
didn't work. And then when I tried to make a different schema document
it complained that the example for google,cros-ec-spi in here had a
problem. Can properties be defined in this section? Or only listed and
then additionalProperties be marked false?

Re: [PATCH v5 2/3] dt-bindings: cros-ec: Reorganize property availability

[Stephen Boyd]

Quoting Doug Anderson (2022-05-12 16:44:44)
> On Wed, May 11, 2022 at 6:39 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          not:
> > +            contains:
> > +              const: google,cros-ec-rpmsg
> > +    then:
> > +      properties:
> > +        mediatek,rpmsg-name: false
> > +
> > +      required:
> > +        - reg
> > +        - interrupts
>
> Is it worth having an "else" clause here? to explicitly make
> "mediatek,rpmsg-name" required for mediatek?

I don't know if mediatek,rpmsg-name is required for a generic
google,cros-ec-rpmsg. I would guess it isn't, because it's a vendor
prefixed property. Maybe it could look at the parent node property to
see that it is inside a mediatek remoteproc node but I'd rather leave
that to future work.

>
> In any case, this seems right from my (albeit) limited yaml knowledge
> and seems like an improvement.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks!

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Rob Herring]

On Mon, May 16, 2022 at 12:23:03PM -0700, Stephen Boyd wrote:
> Quoting Rob Herring (2022-05-16 08:20:03)
> > On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> > > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > index 409ecef967ce..e5fe60beb9fe 100644
> > > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > @@ -19,15 +19,19 @@ description:
> > >  properties:
> > >    compatible:
> > >      oneOf:
> > > -      - description:
> > > -          For implementations of the EC is connected through I2C.
> > > -        const: google,cros-ec-i2c
> > > -      - description:
> > > -          For implementations of the EC is connected through SPI.
> > > -        const: google,cros-ec-spi
> > > -      - description:
> > > -          For implementations of the EC is connected through RPMSG.
> > > -        const: google,cros-ec-rpmsg
> > > +      # For implementations of the EC is connected through I2C.
> >
> > Using 'description' was preferred.
> 
> I just learned that I should use anyOf instead of oneOf. Confusing!

Uhh, no. It can only be one of these entries at a time.


> >
> > > +      - items:
> > > +          - const: google,cros-ec-i2c
> > > +      # For implementations of the FPMCU connected through SPI.
> > > +      - items:
> > > +          - const: google,cros-ec-fp
> > > +          - const: google,cros-ec-spi
> > > +      # For implementations of the EC is connected through SPI.
> > > +      - items:
> > > +          - const: google,cros-ec-spi
> > > +      # For implementations of the EC is connected through RPMSG.
> > > +      - items:
> > > +          - const: google,cros-ec-rpmsg
> > >
> > >    controller-data:
> > >      description:
> > > @@ -176,6 +189,37 @@ allOf:
> > >          - reg
> > >          - interrupts
> > >
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: google,cros-ec-fp
> > > +    then:
> > > +      properties:
> > > +        '#address-cells': false
> > > +        '#size-cells': false
> > > +        typec: false
> > > +        ec-pwm: false
> > > +        keyboard-controller: false
> > > +        proximity: false
> > > +        codecs: false
> > > +        cbas: false
> > > +
> > > +      patternProperties:
> > > +        "^i2c-tunnel[0-9]*$": false
> > > +        "^regulator@[0-9]+$": false
> > > +        "^extcon[0-9]*$": false
> >
> > Is the list of what's allowed shorter? If so, you could list those
> > properties and use 'additionalProperties: false'.
> 
> Yes.
> 
> >
> > Or maybe this is a sign that this should be a separate schema document.
> 
> I couldn't figure that out. I tried to add new properties here but it
> didn't work.

Like this?:

then:
  properties:
    allowed-prop-1: true
    allowed-prop-2: true
    allowed-prop-3: true
  additionalProperties: false

> And then when I tried to make a different schema document
> it complained that the example for google,cros-ec-spi in here had a
> problem.

You probably need a custom 'select' so that both schemas aren't 
matching. Otherwise a schema is applied if any of the compatible strings 
match.

> Can properties be defined in this section? 

Yes, but discouraged for vendor specific properties. The if/then should 
really be just additional constraints.

Rob

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Stephen Boyd]

Quoting Rob Herring (2022-05-18 09:13:48)
> On Mon, May 16, 2022 at 12:23:03PM -0700, Stephen Boyd wrote:
> > Quoting Rob Herring (2022-05-16 08:20:03)
> > > On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> > > > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > index 409ecef967ce..e5fe60beb9fe 100644
> > > > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > +      - items:
> > > > +          - const: google,cros-ec-i2c
> > > > +      # For implementations of the FPMCU connected through SPI.
> > > > +      - items:
> > > > +          - const: google,cros-ec-fp
> > > > +          - const: google,cros-ec-spi
> > > > +      # For implementations of the EC is connected through SPI.
> > > > +      - items:
> > > > +          - const: google,cros-ec-spi
> > > > +      # For implementations of the EC is connected through RPMSG.
> > > > +      - items:
> > > > +          - const: google,cros-ec-rpmsg
> > > >
> > > >    controller-data:
> > > >      description:
> > > > @@ -176,6 +189,37 @@ allOf:
> > > >          - reg
> > > >          - interrupts
> > > >
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: google,cros-ec-fp
> > > > +    then:
> > > > +      properties:
> > > > +        '#address-cells': false
> > > > +        '#size-cells': false
> > > > +        typec: false
> > > > +        ec-pwm: false
> > > > +        keyboard-controller: false
> > > > +        proximity: false
> > > > +        codecs: false
> > > > +        cbas: false
> > > > +
> > > > +      patternProperties:
> > > > +        "^i2c-tunnel[0-9]*$": false
> > > > +        "^regulator@[0-9]+$": false
> > > > +        "^extcon[0-9]*$": false
> > >
> > > Is the list of what's allowed shorter? If so, you could list those
> > > properties and use 'additionalProperties: false'.
> >
> > Yes.
> >
> > >
> > > Or maybe this is a sign that this should be a separate schema document.
> >
> > I couldn't figure that out. I tried to add new properties here but it
> > didn't work.
>
> Like this?:
>
> then:
>   properties:
>     allowed-prop-1: true
>     allowed-prop-2: true
>     allowed-prop-3: true
>   additionalProperties: false

It doesn't seem to combine constraints?

I get this error:

	Documentation/devicetree/bindings/mfd/google,cros-ec.example.dt.yaml:
	ec@0: Additional properties are not allowed ('$nodename' was unexpected)


with this interdiff. I added the spi properties to reduce the additional
properties that aren't allowed.

---8<----
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index 04b847fd5070..d47c7a273026 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -18,24 +18,21 @@ description:

 properties:
   compatible:
-    anyOf:
+    oneOf:
       - description:
           For implementations of the EC connected through I2C.
-      - items:
-          - const: google,cros-ec-i2c
+        const: google,cros-ec-i2c
       - description:
           For implementations of the FPMCU connected through SPI.
-      - items:
+        items:
           - const: google,cros-ec-fp
           - const: google,cros-ec-spi
       - description:
           For implementations of the EC connected through SPI.
-      - items:
-          - const: google,cros-ec-spi
+        const: google,cros-ec-spi
       - description:
           For implementations of the EC connected through RPMSG.
-      - items:
-          - const: google,cros-ec-rpmsg
+        const: google,cros-ec-rpmsg

   controller-data:
     description:
@@ -200,19 +197,14 @@ allOf:
             const: google,cros-ec-fp
     then:
       properties:
-        '#address-cells': false
-        '#size-cells': false
-        typec: false
-        ec-pwm: false
-        keyboard-controller: false
-        proximity: false
-        codecs: false
-        cbas: false
-
-      patternProperties:
-        "^i2c-tunnel[0-9]*$": false
-        "^regulator@[0-9]+$": false
-        "^extcon[0-9]*$": false
+        reset-gpios: true
+        boot0-gpios: true
+        vdd-supply: true
+        interrupts: true
+        compatible: true
+        reg: true
+        spi-max-frequency: true
+      additionalProperties: false

       required:
         - reset-gpios

>
> > And then when I tried to make a different schema document
> > it complained that the example for google,cros-ec-spi in here had a
> > problem.
>
> You probably need a custom 'select' so that both schemas aren't
> matching. Otherwise a schema is applied if any of the compatible strings
> match.

I see. Maybe that's better vs. jamming it all into one file.

>
> > Can properties be defined in this section?
>
> Yes, but discouraged for vendor specific properties. The if/then should
> really be just additional constraints.

Ok.

Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

[Rob Herring]

On Wed, May 18, 2022 at 12:56:17PM -0700, Stephen Boyd wrote:
> Quoting Rob Herring (2022-05-18 09:13:48)
> > On Mon, May 16, 2022 at 12:23:03PM -0700, Stephen Boyd wrote:
> > > Quoting Rob Herring (2022-05-16 08:20:03)
> > > > On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> > > > > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > > index 409ecef967ce..e5fe60beb9fe 100644
> > > > > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > > > > +      - items:
> > > > > +          - const: google,cros-ec-i2c
> > > > > +      # For implementations of the FPMCU connected through SPI.
> > > > > +      - items:
> > > > > +          - const: google,cros-ec-fp
> > > > > +          - const: google,cros-ec-spi
> > > > > +      # For implementations of the EC is connected through SPI.
> > > > > +      - items:
> > > > > +          - const: google,cros-ec-spi
> > > > > +      # For implementations of the EC is connected through RPMSG.
> > > > > +      - items:
> > > > > +          - const: google,cros-ec-rpmsg
> > > > >
> > > > >    controller-data:
> > > > >      description:
> > > > > @@ -176,6 +189,37 @@ allOf:
> > > > >          - reg
> > > > >          - interrupts
> > > > >
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          contains:
> > > > > +            const: google,cros-ec-fp
> > > > > +    then:
> > > > > +      properties:
> > > > > +        '#address-cells': false
> > > > > +        '#size-cells': false
> > > > > +        typec: false
> > > > > +        ec-pwm: false
> > > > > +        keyboard-controller: false
> > > > > +        proximity: false
> > > > > +        codecs: false
> > > > > +        cbas: false
> > > > > +
> > > > > +      patternProperties:
> > > > > +        "^i2c-tunnel[0-9]*$": false
> > > > > +        "^regulator@[0-9]+$": false
> > > > > +        "^extcon[0-9]*$": false
> > > >
> > > > Is the list of what's allowed shorter? If so, you could list those
> > > > properties and use 'additionalProperties: false'.
> > >
> > > Yes.
> > >
> > > >
> > > > Or maybe this is a sign that this should be a separate schema document.
> > >
> > > I couldn't figure that out. I tried to add new properties here but it
> > > didn't work.
> >
> > Like this?:
> >
> > then:
> >   properties:
> >     allowed-prop-1: true
> >     allowed-prop-2: true
> >     allowed-prop-3: true
> >   additionalProperties: false
> 
> It doesn't seem to combine constraints?
> 
> I get this error:
> 
> 	Documentation/devicetree/bindings/mfd/google,cros-ec.example.dt.yaml:
> 	ec@0: Additional properties are not allowed ('$nodename' was unexpected)

That would be a bug or limitation in the tools. We would need to add 
all the other properties we automatically add as well... I think either 
leaving it as-is or a separate file is going to be the better route 
here.

Rob

Re: [PATCH v5 1/3] dt-bindings: cros-ec: Fix a typo in description

[Lee Jones]

On Wed, 11 May 2022, Stephen Boyd wrote:

> A 's/pf/of/' on rpmsg-name description.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 0/3] dt-bindings: cros-ec: Update for fingerprint devices

[patchwork-bot+chrome-platform]

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Lee Jones <lee.jones@linaro.org>:

On Wed, 11 May 2022 18:39:18 -0700 you wrote:
> This patch series introduces a DT binding for chromeos fingerprint
> devices. The first two patches tighten up the existing binding and the
> final patch introduces the fingerprint binding. As there aren't any
> driver patches this can probably go directly through the DT binding tree
> if Lee can ack the patches. Or go through mfd tree if DT maintainers
> ack/review.
> 
> [...]

Here is the summary with links:
  - [v5,1/3] dt-bindings: cros-ec: Fix a typo in description
    https://git.kernel.org/chrome-platform/c/f93afd8e7567
  - [v5,2/3] dt-bindings: cros-ec: Reorganize property availability
    (no matching commit)
  - [v5,3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html