[PATCH 0/3] Fix checker warnings related to cros-ec binding

[PATCH 0/3] Fix checker warnings related to cros-ec binding

[Ricardo Cañuelo]

This series fixes a bunch of warnings related to the google,cros-ec
binding, originally reported by Rob Herring.
The patches involve adding missing subnode definitions in the
google,cros-ec binding and the conversion of two existing bindings to
json-schema.

All the related warnings should be fixed after applying the patches,
except for a couple of warnings in the device trees of Qualcomm's
Trogdor and Cheza chromebooks. They define a pdupdate subnode inside
cros-ec that has no binding, the development of drivers and support for
these chromebooks is ongoing and not completely upstreamed yet.

Bindings tested with:

  make dt_binding_check ARCH=<arch> DT_SCHEMA_FILES=...
  make dtbs_check ARCH=<arch> DT_SCHEMA_FILES=...

for <arch> = arm and arm64.

Kind regards,
Ricardo

Ricardo Cañuelo (3):
  dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema
  dt-bindings: input: convert cros-ec-keyb to json-schema
  dt-bindings: mfd: google,cros-ec: add missing properties

 .../i2c/google,cros-ec-i2c-tunnel.yaml        | 56 +++++++++++
 .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 --------
 .../bindings/input/cros-ec-keyb.txt           | 72 ---------------
 .../bindings/input/google,cros-ec-keyb.yaml   | 92 +++++++++++++++++++
 .../bindings/mfd/google,cros-ec.yaml          | 40 ++++++++
 5 files changed, 188 insertions(+), 111 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 delete mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
 create mode 100644 Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml

-- 
2.18.0

[PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema

[Ricardo Cañuelo]

Convert the google,cros-ec-i2c-tunnel binding to YAML.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../i2c/google,cros-ec-i2c-tunnel.yaml        | 56 +++++++++++++++++++
 .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 -------------
 2 files changed, 56 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt

diff --git a/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
new file mode 100644
index 000000000000..905dbc788dc0
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: I2C bus that tunnels through the ChromeOS EC (cros-ec)
+
+maintainers:
+  - Doug Anderson <dianders@chromium.org>
+  - Benson Leung <bleung@chromium.org>
+  - Enric Balletbo i Serra <enric.balletbo@collabora.com>
+
+description: |
+  On some ChromeOS board designs we've got a connection to the EC
+  (embedded controller) but no direct connection to some devices on the
+  other side of the EC (like a battery and PMIC).  To get access to
+  those devices we need to tunnel our i2c commands through the EC.
+
+  The node for this device should be under a cros-ec node like
+  google,cros-ec-spi or google,cros-ec-i2c.
+
+properties:
+  compatible:
+    const: google,cros-ec-i2c-tunnel
+
+  google,remote-bus:
+    description: The EC bus we'd like to talk to.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - google,remote-bus
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    cros-ec {
+        compatible = "google,cros-ec-spi";
+
+        i2c-tunnel {
+            compatible = "google,cros-ec-i2c-tunnel";
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            google,remote-bus = <0>;
+
+            battery: sbs-battery@b {
+                compatible = "sbs,sbs-battery";
+                reg = <0xb>;
+                sbs,poll-retry-count = <1>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
deleted file mode 100644
index 898f030eba62..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-I2C bus that tunnels through the ChromeOS EC (cros-ec)
-======================================================
-On some ChromeOS board designs we've got a connection to the EC (embedded
-controller) but no direct connection to some devices on the other side of
-the EC (like a battery and PMIC).  To get access to those devices we need
-to tunnel our i2c commands through the EC.
-
-The node for this device should be under a cros-ec node like google,cros-ec-spi
-or google,cros-ec-i2c.
-
-
-Required properties:
-- compatible: google,cros-ec-i2c-tunnel
-- google,remote-bus: The EC bus we'd like to talk to.
-
-Optional child nodes:
-- One node per I2C device connected to the tunnelled I2C bus.
-
-
-Example:
-	cros-ec@0 {
-		compatible = "google,cros-ec-spi";
-
-		...
-
-		i2c-tunnel {
-			compatible = "google,cros-ec-i2c-tunnel";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			google,remote-bus = <0>;
-
-			battery: sbs-battery@b {
-				compatible = "sbs,sbs-battery";
-				reg = <0xb>;
-				sbs,poll-retry-count = <1>;
-			};
-		};
-	}
-- 
2.18.0

Re: [PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema

[Enric Balletbo i Serra]

Hi Ricardo,

Many thanks to work on this. Just a few comment below.

On 5/10/20 9:14, Ricardo Cañuelo wrote:
> Convert the google,cros-ec-i2c-tunnel binding to YAML.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../i2c/google,cros-ec-i2c-tunnel.yaml        | 56 +++++++++++++++++++
>  .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 -------------
>  2 files changed, 56 insertions(+), 39 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
> new file mode 100644
> index 000000000000..905dbc788dc0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: I2C bus that tunnels through the ChromeOS EC (cros-ec)
> +
> +maintainers:
> +  - Doug Anderson <dianders@chromium.org>
> +  - Benson Leung <bleung@chromium.org>
> +  - Enric Balletbo i Serra <enric.balletbo@collabora.com>
> +
> +description: |
> +  On some ChromeOS board designs we've got a connection to the EC
> +  (embedded controller) but no direct connection to some devices on the
> +  other side of the EC (like a battery and PMIC).  To get access to
> +  those devices we need to tunnel our i2c commands through the EC.
> +
> +  The node for this device should be under a cros-ec node like
> +  google,cros-ec-spi or google,cros-ec-i2c.
> +
> +properties:
> +  compatible:
> +    const: google,cros-ec-i2c-tunnel
> +
> +  google,remote-bus:
> +    description: The EC bus we'd like to talk to.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> +  - compatible
> +  - google,remote-bus
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    cros-ec {

We try to use always a complete example, and I think that, Rob also prefers
complete examples, so here you are missing the spi node.

> +        compatible = "google,cros-ec-spi";

And, at least, should have a reg. Did not give you an error?

> +
> +        i2c-tunnel {
> +            compatible = "google,cros-ec-i2c-tunnel";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            google,remote-bus = <0>;
> +
> +            battery: sbs-battery@b {
> +                compatible = "sbs,sbs-battery";
> +                reg = <0xb>;
> +                sbs,poll-retry-count = <1>;
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> deleted file mode 100644
> index 898f030eba62..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -I2C bus that tunnels through the ChromeOS EC (cros-ec)
> -======================================================
> -On some ChromeOS board designs we've got a connection to the EC (embedded
> -controller) but no direct connection to some devices on the other side of
> -the EC (like a battery and PMIC).  To get access to those devices we need
> -to tunnel our i2c commands through the EC.
> -
> -The node for this device should be under a cros-ec node like google,cros-ec-spi
> -or google,cros-ec-i2c.
> -
> -
> -Required properties:
> -- compatible: google,cros-ec-i2c-tunnel
> -- google,remote-bus: The EC bus we'd like to talk to.
> -
> -Optional child nodes:
> -- One node per I2C device connected to the tunnelled I2C bus.
> -
> -
> -Example:
> -	cros-ec@0 {
> -		compatible = "google,cros-ec-spi";
> -
> -		...
> -
> -		i2c-tunnel {
> -			compatible = "google,cros-ec-i2c-tunnel";
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			google,remote-bus = <0>;
> -
> -			battery: sbs-battery@b {
> -				compatible = "sbs,sbs-battery";
> -				reg = <0xb>;
> -				sbs,poll-retry-count = <1>;
> -			};
> -		};
> -	}
> 

Re: [PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema

[Ricardo Cañuelo]

Hi Enric, thanks for reviewing the patch.

On lun 05-10-2020 10:52:26, Enric Balletbo i Serra wrote:
> > +examples:
> > +  - |
> > +    cros-ec {
> 
> We try to use always a complete example, and I think that, Rob also prefers
> complete examples, so here you are missing the spi node.

Ok, I'll prepare a new patch with an extended example.

> > +        compatible = "google,cros-ec-spi";
> 
> And, at least, should have a reg. Did not give you an error?

AFAIK, the reg property is only enforced if the node name includes the
unit-address.

Cheers,
Ricardo

Re: [PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema

[Enric Balletbo i Serra]

Hi Ricardo,

On 5/10/20 11:18, Ricardo Cañuelo wrote:
> Hi Enric, thanks for reviewing the patch.
> 
> On lun 05-10-2020 10:52:26, Enric Balletbo i Serra wrote:
>>> +examples:
>>> +  - |
>>> +    cros-ec {
>>
>> We try to use always a complete example, and I think that, Rob also prefers
>> complete examples, so here you are missing the spi node.
> 
> Ok, I'll prepare a new patch with an extended example.
> 
>>> +        compatible = "google,cros-ec-spi";
>>
>> And, at least, should have a reg. Did not give you an error?
> 
> AFAIK, the reg property is only enforced if the node name includes the
> unit-address.
> 

Exactly, and because this is a spi driver it should have both, the node name
include the unit-address and the reg property. I.e:

    spi0 {
        #address-cells = <1>;
        #size-cells = <0>;

        cros-ec@0 {
            compatible = "google,cros-ec-spi";
            reg = <0x0>;
            spi-max-frequency = <5000000>;
        };
    };

Thanks,
 Enric

> Cheers,
> Ricardo
> 

Re: [PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema

[Rob Herring]

On Mon, 05 Oct 2020 09:14:00 +0200, Ricardo Cañuelo wrote:
> Convert the google,cros-ec-i2c-tunnel binding to YAML.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../i2c/google,cros-ec-i2c-tunnel.yaml        | 56 +++++++++++++++++++
>  .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 -------------
>  2 files changed, 56 insertions(+), 39 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.example.dt.yaml: cros-ec: 'i2c-tunnel' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml


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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

[PATCH 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema

[Ricardo Cañuelo]

Convert the google,cros-ec-keyb binding to YAML.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../bindings/input/cros-ec-keyb.txt           | 72 ---------------
 .../bindings/input/google,cros-ec-keyb.yaml   | 92 +++++++++++++++++++
 2 files changed, 92 insertions(+), 72 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
 create mode 100644 Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml

diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
deleted file mode 100644
index 0f6355ce39b5..000000000000
--- a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
+++ /dev/null
@@ -1,72 +0,0 @@
-ChromeOS EC Keyboard
-
-Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
-a separate EC (Embedded Controller) device. It provides a message for reading
-key scans from the EC. These are then converted into keycodes for processing
-by the kernel.
-
-This binding is based on matrix-keymap.txt and extends/modifies it as follows:
-
-Required properties:
-- compatible: "google,cros-ec-keyb"
-
-Optional properties:
-- google,needs-ghost-filter: True to enable a ghost filter for the matrix
-keyboard. This is recommended if the EC does not have its own logic or
-hardware for this.
-
-
-Example:
-
-cros-ec-keyb {
-	compatible = "google,cros-ec-keyb";
-	keypad,num-rows = <8>;
-	keypad,num-columns = <13>;
-	google,needs-ghost-filter;
-	/*
-	 * Keymap entries take the form of 0xRRCCKKKK where
-	 * RR=Row CC=Column KKKK=Key Code
-	 * The values below are for a US keyboard layout and
-	 * are taken from the Linux driver. Note that the
-	 * 102ND key is not used for US keyboards.
-	 */
-	linux,keymap = <
-		/* CAPSLCK F1         B          F10     */
-		0x0001003a 0x0002003b 0x00030030 0x00040044
-		/* N       =          R_ALT      ESC     */
-		0x00060031 0x0008000d 0x000a0064 0x01010001
-		/* F4      G          F7         H       */
-		0x0102003e 0x01030022 0x01040041 0x01060023
-		/* '       F9         BKSPACE    L_CTRL  */
-		0x01080028 0x01090043 0x010b000e 0x0200001d
-		/* TAB     F3         T          F6      */
-		0x0201000f 0x0202003d 0x02030014 0x02040040
-		/* ]       Y          102ND      [       */
-		0x0205001b 0x02060015 0x02070056 0x0208001a
-		/* F8      GRAVE      F2         5       */
-		0x02090042 0x03010029 0x0302003c 0x03030006
-		/* F5      6          -          \       */
-		0x0304003f 0x03060007 0x0308000c 0x030b002b
-		/* R_CTRL  A          D          F       */
-		0x04000061 0x0401001e 0x04020020 0x04030021
-		/* S       K          J          ;       */
-		0x0404001f 0x04050025 0x04060024 0x04080027
-		/* L       ENTER      Z          C       */
-		0x04090026 0x040b001c 0x0501002c 0x0502002e
-		/* V       X          ,          M       */
-		0x0503002f 0x0504002d 0x05050033 0x05060032
-		/* L_SHIFT /          .          SPACE   */
-		0x0507002a 0x05080035 0x05090034 0x050B0039
-		/* 1       3          4          2       */
-		0x06010002 0x06020004 0x06030005 0x06040003
-		/* 8       7          0          9       */
-		0x06050009 0x06060008 0x0608000b 0x0609000a
-		/* L_ALT   DOWN       RIGHT      Q       */
-		0x060a0038 0x060b006c 0x060c006a 0x07010010
-		/* E       R          W          I       */
-		0x07020012 0x07030013 0x07040011 0x07050017
-		/* U       R_SHIFT    P          O       */
-		0x07060016 0x07070036 0x07080019 0x07090018
-		/* UP      LEFT    */
-		0x070b0067 0x070c0069>;
-};
diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
new file mode 100644
index 000000000000..384df2fc21f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/input/google,cros-ec-keyb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS EC Keyboard
+
+maintainers:
+  - Simon Glass <sjg@chromium.org>
+  - Benson Leung <bleung@chromium.org>
+  - Enric Balletbo i Serra <enric.balletbo@collabora.com>
+
+description: |
+  Google's ChromeOS EC Keyboard is a simple matrix keyboard
+  implemented on a separate EC (Embedded Controller) device. It provides
+  a message for reading key scans from the EC. These are then converted
+  into keycodes for processing by the kernel.
+
+allOf:
+  - $ref: "/schemas/input/matrix-keymap.yaml#"
+
+properties:
+  compatible:
+    const: google,cros-ec-keyb
+
+  google,needs-ghost-filter:
+    description:
+      Enable a ghost filter for the matrix keyboard. This is recommended
+      if the EC does not have its own logic or hardware for this.
+    type: boolean
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    cros-ec-keyb {
+        compatible = "google,cros-ec-keyb";
+        keypad,num-rows = <8>;
+        keypad,num-columns = <13>;
+        google,needs-ghost-filter;
+        /*
+         * Keymap entries take the form of 0xRRCCKKKK where
+         * RR=Row CC=Column KKKK=Key Code
+         * The values below are for a US keyboard layout and
+         * are taken from the Linux driver. Note that the
+         * 102ND key is not used for US keyboards.
+         */
+        linux,keymap = <
+            /* CAPSLCK F1         B          F10     */
+            0x0001003a 0x0002003b 0x00030030 0x00040044
+            /* N       =          R_ALT      ESC     */
+            0x00060031 0x0008000d 0x000a0064 0x01010001
+            /* F4      G          F7         H       */
+            0x0102003e 0x01030022 0x01040041 0x01060023
+            /* '       F9         BKSPACE    L_CTRL  */
+            0x01080028 0x01090043 0x010b000e 0x0200001d
+            /* TAB     F3         T          F6      */
+            0x0201000f 0x0202003d 0x02030014 0x02040040
+            /* ]       Y          102ND      [       */
+            0x0205001b 0x02060015 0x02070056 0x0208001a
+            /* F8      GRAVE      F2         5       */
+            0x02090042 0x03010029 0x0302003c 0x03030006
+            /* F5      6          -          \       */
+            0x0304003f 0x03060007 0x0308000c 0x030b002b
+            /* R_CTRL  A          D          F       */
+            0x04000061 0x0401001e 0x04020020 0x04030021
+            /* S       K          J          ;       */
+            0x0404001f 0x04050025 0x04060024 0x04080027
+            /* L       ENTER      Z          C       */
+            0x04090026 0x040b001c 0x0501002c 0x0502002e
+            /* V       X          ,          M       */
+            0x0503002f 0x0504002d 0x05050033 0x05060032
+            /* L_SHIFT /          .          SPACE   */
+            0x0507002a 0x05080035 0x05090034 0x050B0039
+            /* 1       3          4          2       */
+            0x06010002 0x06020004 0x06030005 0x06040003
+            /* 8       7          0          9       */
+            0x06050009 0x06060008 0x0608000b 0x0609000a
+            /* L_ALT   DOWN       RIGHT      Q       */
+            0x060a0038 0x060b006c 0x060c006a 0x07010010
+            /* E       R          W          I       */
+            0x07020012 0x07030013 0x07040011 0x07050017
+            /* U       R_SHIFT    P          O       */
+            0x07060016 0x07070036 0x07080019 0x07090018
+            /* UP      LEFT    */
+            0x070b0067 0x070c0069>;
+    };
-- 
2.18.0

Re: [PATCH 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema

[Enric Balletbo i Serra]

Hi Ricardo,

Thank you for your on this. Some few comments below.

Note that there was already an attempt for this here [1]. So I think you should
address those comments too.

cc'ing Dmitry as he is the input maintainer.

[1] https://patchwork.kernel.org/patch/11350059/

On 5/10/20 9:14, Ricardo Cañuelo wrote:
> Convert the google,cros-ec-keyb binding to YAML.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/input/cros-ec-keyb.txt           | 72 ---------------
>  .../bindings/input/google,cros-ec-keyb.yaml   | 92 +++++++++++++++++++
>  2 files changed, 92 insertions(+), 72 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
>  create mode 100644 Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> deleted file mode 100644
> index 0f6355ce39b5..000000000000
> --- a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> +++ /dev/null
> @@ -1,72 +0,0 @@
> -ChromeOS EC Keyboard
> -
> -Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
> -a separate EC (Embedded Controller) device. It provides a message for reading
> -key scans from the EC. These are then converted into keycodes for processing
> -by the kernel.
> -
> -This binding is based on matrix-keymap.txt and extends/modifies it as follows:
> -
> -Required properties:
> -- compatible: "google,cros-ec-keyb"
> -
> -Optional properties:
> -- google,needs-ghost-filter: True to enable a ghost filter for the matrix
> -keyboard. This is recommended if the EC does not have its own logic or
> -hardware for this.
> -
> -
> -Example:
> -
> -cros-ec-keyb {
> -	compatible = "google,cros-ec-keyb";
> -	keypad,num-rows = <8>;
> -	keypad,num-columns = <13>;
> -	google,needs-ghost-filter;
> -	/*
> -	 * Keymap entries take the form of 0xRRCCKKKK where
> -	 * RR=Row CC=Column KKKK=Key Code
> -	 * The values below are for a US keyboard layout and
> -	 * are taken from the Linux driver. Note that the
> -	 * 102ND key is not used for US keyboards.
> -	 */
> -	linux,keymap = <
> -		/* CAPSLCK F1         B          F10     */
> -		0x0001003a 0x0002003b 0x00030030 0x00040044
> -		/* N       =          R_ALT      ESC     */
> -		0x00060031 0x0008000d 0x000a0064 0x01010001
> -		/* F4      G          F7         H       */
> -		0x0102003e 0x01030022 0x01040041 0x01060023
> -		/* '       F9         BKSPACE    L_CTRL  */
> -		0x01080028 0x01090043 0x010b000e 0x0200001d
> -		/* TAB     F3         T          F6      */
> -		0x0201000f 0x0202003d 0x02030014 0x02040040
> -		/* ]       Y          102ND      [       */
> -		0x0205001b 0x02060015 0x02070056 0x0208001a
> -		/* F8      GRAVE      F2         5       */
> -		0x02090042 0x03010029 0x0302003c 0x03030006
> -		/* F5      6          -          \       */
> -		0x0304003f 0x03060007 0x0308000c 0x030b002b
> -		/* R_CTRL  A          D          F       */
> -		0x04000061 0x0401001e 0x04020020 0x04030021
> -		/* S       K          J          ;       */
> -		0x0404001f 0x04050025 0x04060024 0x04080027
> -		/* L       ENTER      Z          C       */
> -		0x04090026 0x040b001c 0x0501002c 0x0502002e
> -		/* V       X          ,          M       */
> -		0x0503002f 0x0504002d 0x05050033 0x05060032
> -		/* L_SHIFT /          .          SPACE   */
> -		0x0507002a 0x05080035 0x05090034 0x050B0039
> -		/* 1       3          4          2       */
> -		0x06010002 0x06020004 0x06030005 0x06040003
> -		/* 8       7          0          9       */
> -		0x06050009 0x06060008 0x0608000b 0x0609000a
> -		/* L_ALT   DOWN       RIGHT      Q       */
> -		0x060a0038 0x060b006c 0x060c006a 0x07010010
> -		/* E       R          W          I       */
> -		0x07020012 0x07030013 0x07040011 0x07050017
> -		/* U       R_SHIFT    P          O       */
> -		0x07060016 0x07070036 0x07080019 0x07090018
> -		/* UP      LEFT    */
> -		0x070b0067 0x070c0069>;
> -};
> diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> new file mode 100644
> index 000000000000..384df2fc21f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/input/google,cros-ec-keyb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS EC Keyboard
> +
> +maintainers:
> +  - Simon Glass <sjg@chromium.org>
> +  - Benson Leung <bleung@chromium.org>
> +  - Enric Balletbo i Serra <enric.balletbo@collabora.com>
> +
> +description: |
> +  Google's ChromeOS EC Keyboard is a simple matrix keyboard
> +  implemented on a separate EC (Embedded Controller) device. It provides
> +  a message for reading key scans from the EC. These are then converted
> +  into keycodes for processing by the kernel.
> +
> +allOf:
> +  - $ref: "/schemas/input/matrix-keymap.yaml#"
> +
> +properties:
> +  compatible:
> +    const: google,cros-ec-keyb
> +
> +  google,needs-ghost-filter:
> +    description:
> +      Enable a ghost filter for the matrix keyboard. This is recommended
> +      if the EC does not have its own logic or hardware for this.
> +    type: boolean
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +

Not sure about unevaluatedProperties does here, I might miss something. But,
shouldn't you add `additionalProperties: false` instead?


> +examples:
> +  - |
> +    cros-ec-keyb {

The keyboard controller is always a subnode inside the cros_ec, please use a
complete example.

Thanks,
 Enric

> +        compatible = "google,cros-ec-keyb";
> +        keypad,num-rows = <8>;
> +        keypad,num-columns = <13>;
> +        google,needs-ghost-filter;
> +        /*
> +         * Keymap entries take the form of 0xRRCCKKKK where
> +         * RR=Row CC=Column KKKK=Key Code
> +         * The values below are for a US keyboard layout and
> +         * are taken from the Linux driver. Note that the
> +         * 102ND key is not used for US keyboards.
> +         */
> +        linux,keymap = <
> +            /* CAPSLCK F1         B          F10     */
> +            0x0001003a 0x0002003b 0x00030030 0x00040044
> +            /* N       =          R_ALT      ESC     */
> +            0x00060031 0x0008000d 0x000a0064 0x01010001
> +            /* F4      G          F7         H       */
> +            0x0102003e 0x01030022 0x01040041 0x01060023
> +            /* '       F9         BKSPACE    L_CTRL  */
> +            0x01080028 0x01090043 0x010b000e 0x0200001d
> +            /* TAB     F3         T          F6      */
> +            0x0201000f 0x0202003d 0x02030014 0x02040040
> +            /* ]       Y          102ND      [       */
> +            0x0205001b 0x02060015 0x02070056 0x0208001a
> +            /* F8      GRAVE      F2         5       */
> +            0x02090042 0x03010029 0x0302003c 0x03030006
> +            /* F5      6          -          \       */
> +            0x0304003f 0x03060007 0x0308000c 0x030b002b
> +            /* R_CTRL  A          D          F       */
> +            0x04000061 0x0401001e 0x04020020 0x04030021
> +            /* S       K          J          ;       */
> +            0x0404001f 0x04050025 0x04060024 0x04080027
> +            /* L       ENTER      Z          C       */
> +            0x04090026 0x040b001c 0x0501002c 0x0502002e
> +            /* V       X          ,          M       */
> +            0x0503002f 0x0504002d 0x05050033 0x05060032
> +            /* L_SHIFT /          .          SPACE   */
> +            0x0507002a 0x05080035 0x05090034 0x050B0039
> +            /* 1       3          4          2       */
> +            0x06010002 0x06020004 0x06030005 0x06040003
> +            /* 8       7          0          9       */
> +            0x06050009 0x06060008 0x0608000b 0x0609000a
> +            /* L_ALT   DOWN       RIGHT      Q       */
> +            0x060a0038 0x060b006c 0x060c006a 0x07010010
> +            /* E       R          W          I       */
> +            0x07020012 0x07030013 0x07040011 0x07050017
> +            /* U       R_SHIFT    P          O       */
> +            0x07060016 0x07070036 0x07080019 0x07090018
> +            /* UP      LEFT    */
> +            0x070b0067 0x070c0069>;
> +    };
> 

Re: [PATCH 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema

[Ricardo Cañuelo]

Hi Enric,

Thanks for reviewing the patch and for your suggestions. I'll prepare a
new series with the fixes.

On lun 05-10-2020 11:03:54, Enric Balletbo i Serra wrote:
> Note that there was already an attempt for this here [1]. So I think you should
> address those comments too.
> 
> cc'ing Dmitry as he is the input maintainer.
> 
> [1] https://patchwork.kernel.org/patch/11350059/

Thanks, I wasn't aware of that.

> > +unevaluatedProperties: false
> > +
> 
> Not sure about unevaluatedProperties does here, I might miss something. But,
> shouldn't you add `additionalProperties: false` instead?

The idea of using this came from
Documentation/devicetree/bindings/example-schema.yaml, when it explains
the "additionalProperties: false" line:

    This can't be used in cases where another schema is referenced
    (i.e. allOf: [{$ref: ...}]).  If and only if another schema is
    referenced and arbitrary children nodes can appear,
    "unevaluatedProperties: false" could be used.  Typical example is
    I2C controller where no name pattern matching for children can be
    added.

This binding references matrix-keymap.yaml and it may use some
properties defined in it (although they're not
subnodes). bindings/input/imx-keypad.yaml does the same.

The alternative would be to define "additionalProperties: false" and
redefine the matrix-keymap properties used in this binding too
(keypad,num-rows keypad,num-columns and linux,keymap). Or to ditch
"additionalProperties: false" altogether, but I don't think that's a
proper solution.

> > +examples:
> > +  - |
> > +    cros-ec-keyb {
> 
> The keyboard controller is always a subnode inside the cros_ec, please use a
> complete example.

Ok, I'll do that.

Cheers,
Ricardo

[PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Ricardo Cañuelo]

Add missing properties that are currently used in the examples of
subnode bindings and in many DTs.
This fixes all current dt_binding_check and dtbs_check warnings related
to this binding.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../bindings/mfd/google,cros-ec.yaml          | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index f49c0d5d31ad..c2dc05cdef9f 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -59,18 +59,58 @@ properties:
       whether this nvram is present or not.
     type: boolean
 
+  mtk,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
+      device. Used to match the subnode to the rpmsg device announced by
+      the SCP.
+    $ref: /schemas/types.yaml#/definitions/string
+
   spi-max-frequency:
     description: Maximum SPI frequency of the device in Hz.
 
   reg:
     maxItems: 1
 
+  '#address-cells':
+    enum: [1, 2]
+
+  '#size-cells':
+    enum: [0, 1]
+
   interrupts:
     maxItems: 1
 
   wakeup-source:
     description: Button can wake-up the system.
 
+  typec:
+    $ref: "/schemas/chrome/google,cros-ec-typec.yaml#"
+
+  ec-pwm:
+    $ref: "/schemas/pwm/google,cros-ec-pwm.yaml#"
+
+  keyboard-controller:
+    $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
+
+patternProperties:
+  "^regulator@[a-f0-9]+$":
+    type: object
+    $ref: "/schemas/regulator/google,cros-ec-regulator.yaml#"
+
+  "^extcon[0-9]*$":
+    type: object
+    $ref: "/schemas/extcon/extcon-usbc-cros-ec.yaml#"
+
+  "^ec-codec@[a-f0-9]+$":
+    type: object
+    $ref: "/schemas/sound/google,cros-ec-codec.yaml#"
+
+  "^i2c-tunnel[0-9]*$":
+    type: object
+    $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
+
 required:
   - compatible
 
-- 
2.18.0

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Enric Balletbo i Serra]

Hi Ricardo,

Thank you to work on this.

On 5/10/20 9:14, Ricardo Cañuelo wrote:
> Add missing properties that are currently used in the examples of
> subnode bindings and in many DTs.
> This fixes all current dt_binding_check and dtbs_check warnings related
> to this binding.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index f49c0d5d31ad..c2dc05cdef9f 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -59,18 +59,58 @@ properties:
>        whether this nvram is present or not.
>      type: boolean
>  
> +  mtk,rpmsg-name:

AFAIK mtk is not a valid vendor prefix (vendor-prefixes.yaml), so I am wondering
if this should be mediatek,rpmsg-name. I see this being used in
drivers/rpmsg/mtk_rpmsg.c file, but there isn't a devitree using it. So maybe
we're on time to fix it.

Thanks,
 Enric

> +    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
> +      device. Used to match the subnode to the rpmsg device announced by
> +      the SCP.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>    spi-max-frequency:
>      description: Maximum SPI frequency of the device in Hz.
>  
>    reg:
>      maxItems: 1
>  
> +  '#address-cells':
> +    enum: [1, 2]
> +
> +  '#size-cells':
> +    enum: [0, 1]
> +
>    interrupts:
>      maxItems: 1
>  
>    wakeup-source:
>      description: Button can wake-up the system.
>  
> +  typec:
> +    $ref: "/schemas/chrome/google,cros-ec-typec.yaml#"
> +
> +  ec-pwm:
> +    $ref: "/schemas/pwm/google,cros-ec-pwm.yaml#"
> +
> +  keyboard-controller:
> +    $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
> +
> +patternProperties:
> +  "^regulator@[a-f0-9]+$":
> +    type: object
> +    $ref: "/schemas/regulator/google,cros-ec-regulator.yaml#"
> +
> +  "^extcon[0-9]*$":
> +    type: object
> +    $ref: "/schemas/extcon/extcon-usbc-cros-ec.yaml#"
> +
> +  "^ec-codec@[a-f0-9]+$":
> +    type: object
> +    $ref: "/schemas/sound/google,cros-ec-codec.yaml#"
> +
> +  "^i2c-tunnel[0-9]*$":
> +    type: object
> +    $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
> +
>  required:
>    - compatible
>  
> 

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Ricardo Cañuelo]

Hi Enric,

Thank you for reviewing

On lun 05-10-2020 11:23:15, Enric Balletbo i Serra wrote:
> > +  mtk,rpmsg-name:
> 
> AFAIK mtk is not a valid vendor prefix (vendor-prefixes.yaml), so I am wondering
> if this should be mediatek,rpmsg-name. I see this being used in
> drivers/rpmsg/mtk_rpmsg.c file, but there isn't a devitree using it. So maybe
> we're on time to fix it.

It is used in arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi.

I'm not sure if defining this kind of vendor-specific properties in
google,cros-ec is the right approach, but I can't think of a better
alternative (any suggestion is welcome). I thought about making this
conditional to the "compatible" string but in the case of
mt8183-kukui.dtsi the string looks too generic for it to be associated
with a particular vendor.

Shall I go and fix the vendor prefix in all places (driver, binding and
DT)?

Cheers,
Ricardo

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Rob Herring]

On Mon, Oct 5, 2020 at 2:14 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Add missing properties that are currently used in the examples of
> subnode bindings and in many DTs.
> This fixes all current dt_binding_check and dtbs_check warnings related
> to this binding.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index f49c0d5d31ad..c2dc05cdef9f 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -59,18 +59,58 @@ properties:
>        whether this nvram is present or not.
>      type: boolean
>
> +  mtk,rpmsg-name:

This should have been mediatek,rpmsg-name, but I guess we're stuck with it.

> +    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
> +      device. Used to match the subnode to the rpmsg device announced by
> +      the SCP.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>    spi-max-frequency:
>      description: Maximum SPI frequency of the device in Hz.
>
>    reg:
>      maxItems: 1
>
> +  '#address-cells':
> +    enum: [1, 2]
> +
> +  '#size-cells':
> +    enum: [0, 1]

This doesn't really make sense. Either there's a size or there isn't.

[...]

> +  "^regulator@[a-f0-9]+$":
> +  "^ec-codec@[a-f0-9]+$":

What does the number space represent and is it the same for each of
these? If not, then this is kind of broken. There's only 1 number
space at a given level.

Rob

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Enric Balletbo i Serra]

Hi,

On 5/10/20 17:37, Rob Herring wrote:
> On Mon, Oct 5, 2020 at 2:14 AM Ricardo Cañuelo
> <ricardo.canuelo@collabora.com> wrote:
>>
>> Add missing properties that are currently used in the examples of
>> subnode bindings and in many DTs.
>> This fixes all current dt_binding_check and dtbs_check warnings related
>> to this binding.
>>
>> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
>> ---
>>  .../bindings/mfd/google,cros-ec.yaml          | 40 +++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
>> index f49c0d5d31ad..c2dc05cdef9f 100644
>> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
>> @@ -59,18 +59,58 @@ properties:
>>        whether this nvram is present or not.
>>      type: boolean
>>
>> +  mtk,rpmsg-name:
> 
> This should have been mediatek,rpmsg-name, but I guess we're stuck with it.
> 

cc'ing the remote-proc maintainers and some ChromeOS people to be aware.

Seems that the patches that introduce the use of this propietry in the bindings
are still in linux-next, so maybe we're on time to fix it?

In such case, Ricardo can you take care of it and send patches fixing it?

Thanks,
 Enric

>> +    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
>> +      device. Used to match the subnode to the rpmsg device announced by
>> +      the SCP.
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +
>>    spi-max-frequency:
>>      description: Maximum SPI frequency of the device in Hz.
>>
>>    reg:
>>      maxItems: 1
>>
>> +  '#address-cells':
>> +    enum: [1, 2]
>> +
>> +  '#size-cells':
>> +    enum: [0, 1]
> 
> This doesn't really make sense. Either there's a size or there isn't.
> 
> [...]
> 
>> +  "^regulator@[a-f0-9]+$":
>> +  "^ec-codec@[a-f0-9]+$":
> 
> What does the number space represent and is it the same for each of
> these? If not, then this is kind of broken. There's only 1 number
> space at a given level.
> 
> Rob
> 

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Ricardo Cañuelo]

Hi Rob,

thanks for reviewing the patch. Find my comments below:

On lun 05-10-2020 10:37:09, Rob Herring wrote:
> > +  '#address-cells':
> > +    enum: [1, 2]
> > +
> > +  '#size-cells':
> > +    enum: [0, 1]
> 
> This doesn't really make sense. Either there's a size or there isn't.
> 
> [...]
> 
> > +  "^regulator@[a-f0-9]+$":
> > +  "^ec-codec@[a-f0-9]+$":
> 
> What does the number space represent and is it the same for each of
> these? If not, then this is kind of broken. There's only 1 number
> space at a given level.

I see what you mean. In the regulator, the unit-address means the identifier
for the voltage regulator and I guess it could also be defined as
simply "^regulator@[0-9]+$". In the codec, though, it's a physical base
address.

The reg property in these has a different format, that's why I
defined #address-cells and #size-cells above to have a range of values
instead of fixed values.

From your experience, what's the best course of action here? I can't
find a driver managing google,cros-ec-codec yet, although the binding
was submitted in January.

Thanks,
Ricardo

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Guenter Roeck]

On Mon, Oct 5, 2020 at 11:13 PM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Hi Rob,
>
> thanks for reviewing the patch. Find my comments below:
>
> On lun 05-10-2020 10:37:09, Rob Herring wrote:
> > > +  '#address-cells':
> > > +    enum: [1, 2]
> > > +
> > > +  '#size-cells':
> > > +    enum: [0, 1]
> >
> > This doesn't really make sense. Either there's a size or there isn't.
> >
> > [...]
> >
> > > +  "^regulator@[a-f0-9]+$":
> > > +  "^ec-codec@[a-f0-9]+$":
> >
> > What does the number space represent and is it the same for each of
> > these? If not, then this is kind of broken. There's only 1 number
> > space at a given level.
>
> I see what you mean. In the regulator, the unit-address means the identifier
> for the voltage regulator and I guess it could also be defined as
> simply "^regulator@[0-9]+$". In the codec, though, it's a physical base
> address.
>
> The reg property in these has a different format, that's why I
> defined #address-cells and #size-cells above to have a range of values
> instead of fixed values.
>
> From your experience, what's the best course of action here? I can't
> find a driver managing google,cros-ec-codec yet, although the binding
> was submitted in January.
>

It seems like I am missing something. In the mainline kernel:

sound/soc/codecs/cros_ec_codec.c:       { .compatible =
"google,cros-ec-codec" },

Can you explain what you mean with "can't find a driver managing
google,cros-ec-codec yet" ?

Thanks,
Guenter

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Ricardo Cañuelo]

On mar 06-10-2020 04:28:36, Guenter Roeck wrote:
> It seems like I am missing something. In the mainline kernel:
> 
> sound/soc/codecs/cros_ec_codec.c:       { .compatible =
> "google,cros-ec-codec" },
> 
> Can you explain what you mean with "can't find a driver managing
> google,cros-ec-codec yet" ?

You're right, I should grep the entire code base instead of the drivers
subdir next time...

Sorry about that and thanks,

Ricardo

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Rob Herring]

On Tue, Oct 06, 2020 at 08:13:17AM +0200, Ricardo Cañuelo wrote:
> Hi Rob,
> 
> thanks for reviewing the patch. Find my comments below:
> 
> On lun 05-10-2020 10:37:09, Rob Herring wrote:
> > > +  '#address-cells':
> > > +    enum: [1, 2]
> > > +
> > > +  '#size-cells':
> > > +    enum: [0, 1]
> > 
> > This doesn't really make sense. Either there's a size or there isn't.
> > 
> > [...]
> > 
> > > +  "^regulator@[a-f0-9]+$":
> > > +  "^ec-codec@[a-f0-9]+$":
> > 
> > What does the number space represent and is it the same for each of
> > these? If not, then this is kind of broken. There's only 1 number
> > space at a given level.
> 
> I see what you mean. In the regulator, the unit-address means the identifier
> for the voltage regulator and I guess it could also be defined as
> simply "^regulator@[0-9]+$". In the codec, though, it's a physical base
> address.
> 
> The reg property in these has a different format, that's why I
> defined #address-cells and #size-cells above to have a range of values
> instead of fixed values.

But in any given DT it is going to be fixed, so that doesn't help you.

> >From your experience, what's the best course of action here? I can't
> find a driver managing google,cros-ec-codec yet, although the binding
> was submitted in January.

Normally, you just add another layer with a 'regulators' and/or 'codecs' 
node. Do you really have more than 1 codec?

Rob

Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

[Ricardo Cañuelo]

Hi Rob,

On mar 06-10-2020 16:35:43, Rob Herring wrote:
> Normally, you just add another layer with a 'regulators' and/or 'codecs' 
> node. Do you really have more than 1 codec?
> 
> Rob

Thanks for the tip. There's only this codec afaict, so I'll enclose the
codec subnode inside a new intermediate node for the next patch series.

Cheers,
Ricardo