All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fix checker warnings related to cros-ec binding
@ 2020-10-08 10:28 Ricardo Cañuelo
  2020-10-08 10:28 ` [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-08 10:28 UTC (permalink / raw)
  To: robh
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

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.

Changes from v1:

  - Update google,cros-ec.yaml in patches 1 and 2 to avoid checker
    warnings when applied individually.

  - Complete the examples in google,cros-ec-i2c-tunnel.yaml and
    google,cros-ec-keyb.yaml, including the enclosing nodes (Enric).

  - Include the additional properties from matrix-keymap.yaml in
    google,cros-ec-keyb.yaml and use additionalProperties: false
    (Enric).

  - Use an intermediate "codecs" node in google,cros-ec.yaml to enclose
    the ec-codec nodes, which require different #address-cells and
    #size-cells values (Rob).

  - Update the example in google,cros-ec-codec.yaml to reflect the
    change in google,cros-ec.yaml

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
  mfd: google,cros-ec: add missing properties

 .../i2c/google,cros-ec-i2c-tunnel.yaml        |  63 +++++++++
 .../bindings/i2c/i2c-cros-ec-tunnel.txt       |  39 ------
 .../bindings/input/cros-ec-keyb.txt           |  72 -----------
 .../bindings/input/google,cros-ec-keyb.yaml   | 120 ++++++++++++++++++
 .../bindings/mfd/google,cros-ec.yaml          |  50 ++++++++
 .../bindings/sound/google,cros-ec-codec.yaml  |  26 ++--
 6 files changed, 249 insertions(+), 121 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


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

* [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema
  2020-10-08 10:28 [PATCH v2 0/3] Fix checker warnings related to cros-ec binding Ricardo Cañuelo
@ 2020-10-08 10:28 ` Ricardo Cañuelo
  2020-10-11 11:17   ` Enric Balletbo i Serra
  2020-10-08 10:28 ` [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb " Ricardo Cañuelo
  2020-10-08 10:28 ` [PATCH v2 3/3] mfd: google,cros-ec: add missing properties Ricardo Cañuelo
  2 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-08 10:28 UTC (permalink / raw)
  To: robh
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

Convert the google,cros-ec-i2c-tunnel binding to YAML and add it as a
property of google,cros-ec.yaml.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../i2c/google,cros-ec-i2c-tunnel.yaml        | 63 +++++++++++++++++++
 .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 ------------
 .../bindings/mfd/google,cros-ec.yaml          |  5 ++
 3 files changed, 68 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..f83ff67596e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
@@ -0,0 +1,63 @@
+# 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:
+  - |
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cros-ec@0 {
+            compatible = "google,cros-ec-spi";
+            reg = <0>;
+            spi-max-frequency = <5000000>;
+
+            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>;
-			};
-		};
-	}
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index f49c0d5d31ad..c45cf30ea3aa 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -71,6 +71,11 @@ properties:
   wakeup-source:
     description: Button can wake-up the system.
 
+patternProperties:
+  "^i2c-tunnel[0-9]*$":
+    type: object
+    $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
+
 required:
   - compatible
 
-- 
2.18.0


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

* [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema
  2020-10-08 10:28 [PATCH v2 0/3] Fix checker warnings related to cros-ec binding Ricardo Cañuelo
  2020-10-08 10:28 ` [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
@ 2020-10-08 10:28 ` Ricardo Cañuelo
  2020-10-08 18:32   ` Rob Herring
  2020-10-08 10:28 ` [PATCH v2 3/3] mfd: google,cros-ec: add missing properties Ricardo Cañuelo
  2 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-08 10:28 UTC (permalink / raw)
  To: robh
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

Convert the google,cros-ec-keyb binding to YAML and add it as a property
of google,cros-ec.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   | 120 ++++++++++++++++++
 .../bindings/mfd/google,cros-ec.yaml          |   3 +
 3 files changed, 123 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..5286d9d8ac45
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
@@ -0,0 +1,120 @@
+# 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
+
+  linux,keymap:
+    $ref: '/schemas/types.yaml#/definitions/uint32-array'
+    description: |
+      An array of packed 1-cell entries containing the equivalent of row,
+      column and linux key-code. The 32-bit big endian cell is packed as:
+          row << 24 | column << 16 | key-code
+
+  keypad,num-rows:
+    $ref: '/schemas/types.yaml#/definitions/uint32'
+    description: Number of row lines connected to the keypad controller.
+
+  keypad,num-columns:
+    $ref: '/schemas/types.yaml#/definitions/uint32'
+    description: Number of column lines connected to the keypad controller.
+
+  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
+  - linux,keymap
+  - keypad,num-rows
+  - keypad,num-columns
+
+additionalProperties: false
+
+examples:
+  - |
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cros-ec@0 {
+            compatible = "google,cros-ec-spi";
+            reg = <0>;
+
+            keyboard-controller {
+                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/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index c45cf30ea3aa..351bfb6d37ba 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -71,6 +71,9 @@ properties:
   wakeup-source:
     description: Button can wake-up the system.
 
+  keyboard-controller:
+    $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
+
 patternProperties:
   "^i2c-tunnel[0-9]*$":
     type: object
-- 
2.18.0


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

* [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-08 10:28 [PATCH v2 0/3] Fix checker warnings related to cros-ec binding Ricardo Cañuelo
  2020-10-08 10:28 ` [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
  2020-10-08 10:28 ` [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb " Ricardo Cañuelo
@ 2020-10-08 10:28 ` Ricardo Cañuelo
  2020-10-08 18:38   ` Rob Herring
  2 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-08 10:28 UTC (permalink / raw)
  To: robh
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

Add missing properties that are currently used in the examples of
subnode bindings and in many DTs.

Also updates the example in sound/google,cros-ec-codec.yaml to comply
with the google,cros-ec binding.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../bindings/mfd/google,cros-ec.yaml          | 42 +++++++++++++++++++
 .../bindings/sound/google,cros-ec-codec.yaml  | 26 +++++++-----
 2 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index 351bfb6d37ba..48929bb07d98 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -59,6 +59,14 @@ 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.
 
@@ -71,14 +79,48 @@ properties:
   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#"
 
+  codecs:
+    type: object
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 2
+
+      '#size-cells':
+        const: 1
+
+    patternProperties:
+      "^ec-codec@[a-f0-9]+$":
+        type: object
+        $ref: "/schemas/sound/google,cros-ec-codec.yaml#"
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+
 patternProperties:
   "^i2c-tunnel[0-9]*$":
     type: object
     $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
 
+  "^regulator@[0-9]+$":
+    type: object
+    $ref: "/schemas/regulator/google,cros-ec-regulator.yaml#"
+
+  "^extcon[0-9]*$":
+    type: object
+    $ref: "/schemas/extcon/extcon-usbc-cros-ec.yaml#"
+
 required:
   - compatible
 
diff --git a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
index c84e656afb0a..acfb9db021dc 100644
--- a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
+++ b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
@@ -11,9 +11,10 @@ maintainers:
 
 description: |
   Google's ChromeOS EC codec is a digital mic codec provided by the
-  Embedded Controller (EC) and is controlled via a host-command interface.
-  An EC codec node should only be found as a sub-node of the EC node (see
-  Documentation/devicetree/bindings/mfd/cros-ec.txt).
+  Embedded Controller (EC) and is controlled via a host-command
+  interface.  An EC codec node should only be found inside the "codecs"
+  subnode of a cros-ec node.
+  (see Documentation/devicetree/bindings/mfd/google,cros-ec.yaml).
 
 properties:
   compatible:
@@ -54,14 +55,19 @@ examples:
         #size-cells = <0>;
         cros-ec@0 {
             compatible = "google,cros-ec-spi";
-            #address-cells = <2>;
-            #size-cells = <1>;
             reg = <0>;
-            cros_ec_codec: ec-codec@10500000 {
-                compatible = "google,cros-ec-codec";
-                #sound-dai-cells = <1>;
-                reg = <0x0 0x10500000 0x80000>;
-                memory-region = <&reserved_mem>;
+
+            codecs {
+                #address-cells = <2>;
+                #size-cells = <1>;
+
+                cros_ec_codec: ec-codec@10500000 {
+                    compatible = "google,cros-ec-codec";
+                    #sound-dai-cells = <1>;
+                    reg = <0x0 0x10500000 0x80000>;
+                    memory-region = <&reserved_mem>;
+                };
+
             };
         };
     };
-- 
2.18.0


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

* Re: [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema
  2020-10-08 10:28 ` [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb " Ricardo Cañuelo
@ 2020-10-08 18:32   ` Rob Herring
  2020-10-09  5:28     ` Ricardo Cañuelo
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2020-10-08 18:32 UTC (permalink / raw)
  To: Ricardo Cañuelo
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

On Thu, Oct 08, 2020 at 12:28:24PM +0200, Ricardo Cañuelo wrote:
> Convert the google,cros-ec-keyb binding to YAML and add it as a property
> of google,cros-ec.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   | 120 ++++++++++++++++++
>  .../bindings/mfd/google,cros-ec.yaml          |   3 +
>  3 files changed, 123 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..5286d9d8ac45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> @@ -0,0 +1,120 @@
> +# 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
> +
> +  linux,keymap:
> +    $ref: '/schemas/types.yaml#/definitions/uint32-array'
> +    description: |
> +      An array of packed 1-cell entries containing the equivalent of row,
> +      column and linux key-code. The 32-bit big endian cell is packed as:
> +          row << 24 | column << 16 | key-code

These already have a type and description, don't repeat it here.

If no other constraints, then just 'linux,keymap: true'. Or you can use 
unevaluatedProperties instead of additionalProperties. The former will 
take the $ref to matrix-keymap.yaml into account.

> +
> +  keypad,num-rows:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: Number of row lines connected to the keypad controller.
> +
> +  keypad,num-columns:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: Number of column lines connected to the keypad controller.
> +
> +  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
> +  - linux,keymap
> +  - keypad,num-rows
> +  - keypad,num-columns
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cros-ec@0 {
> +            compatible = "google,cros-ec-spi";
> +            reg = <0>;
> +
> +            keyboard-controller {
> +                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/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index c45cf30ea3aa..351bfb6d37ba 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -71,6 +71,9 @@ properties:
>    wakeup-source:
>      description: Button can wake-up the system.
>  
> +  keyboard-controller:
> +    $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
> +
>  patternProperties:
>    "^i2c-tunnel[0-9]*$":
>      type: object
> -- 
> 2.18.0
> 

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-08 10:28 ` [PATCH v2 3/3] mfd: google,cros-ec: add missing properties Ricardo Cañuelo
@ 2020-10-08 18:38   ` Rob Herring
  2020-10-09  5:48     ` Ricardo Cañuelo
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2020-10-08 18:38 UTC (permalink / raw)
  To: Ricardo Cañuelo
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

On Thu, Oct 08, 2020 at 12:28:25PM +0200, Ricardo Cañuelo wrote:
> Add missing properties that are currently used in the examples of
> subnode bindings and in many DTs.
> 
> Also updates the example in sound/google,cros-ec-codec.yaml to comply
> with the google,cros-ec binding.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 42 +++++++++++++++++++
>  .../bindings/sound/google,cros-ec-codec.yaml  | 26 +++++++-----
>  2 files changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 351bfb6d37ba..48929bb07d98 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -59,6 +59,14 @@ 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.
>  
> @@ -71,14 +79,48 @@ properties:
>    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#"
>  
> +  codecs:

Doesn't moving this require a driver change? 

If you only need 1 codec, then just drop the unit-address.

> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      '#address-cells':
> +        const: 2
> +
> +      '#size-cells':
> +        const: 1
> +
> +    patternProperties:
> +      "^ec-codec@[a-f0-9]+$":
> +        type: object
> +        $ref: "/schemas/sound/google,cros-ec-codec.yaml#"
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +
>  patternProperties:
>    "^i2c-tunnel[0-9]*$":
>      type: object
>      $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
>  
> +  "^regulator@[0-9]+$":
> +    type: object
> +    $ref: "/schemas/regulator/google,cros-ec-regulator.yaml#"
> +
> +  "^extcon[0-9]*$":
> +    type: object
> +    $ref: "/schemas/extcon/extcon-usbc-cros-ec.yaml#"
> +
>  required:
>    - compatible
>  
> diff --git a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
> index c84e656afb0a..acfb9db021dc 100644
> --- a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
> +++ b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml
> @@ -11,9 +11,10 @@ maintainers:
>  
>  description: |
>    Google's ChromeOS EC codec is a digital mic codec provided by the
> -  Embedded Controller (EC) and is controlled via a host-command interface.
> -  An EC codec node should only be found as a sub-node of the EC node (see
> -  Documentation/devicetree/bindings/mfd/cros-ec.txt).
> +  Embedded Controller (EC) and is controlled via a host-command
> +  interface.  An EC codec node should only be found inside the "codecs"
> +  subnode of a cros-ec node.
> +  (see Documentation/devicetree/bindings/mfd/google,cros-ec.yaml).
>  
>  properties:
>    compatible:
> @@ -54,14 +55,19 @@ examples:
>          #size-cells = <0>;
>          cros-ec@0 {
>              compatible = "google,cros-ec-spi";
> -            #address-cells = <2>;
> -            #size-cells = <1>;
>              reg = <0>;
> -            cros_ec_codec: ec-codec@10500000 {
> -                compatible = "google,cros-ec-codec";
> -                #sound-dai-cells = <1>;
> -                reg = <0x0 0x10500000 0x80000>;
> -                memory-region = <&reserved_mem>;
> +
> +            codecs {
> +                #address-cells = <2>;
> +                #size-cells = <1>;
> +
> +                cros_ec_codec: ec-codec@10500000 {
> +                    compatible = "google,cros-ec-codec";
> +                    #sound-dai-cells = <1>;
> +                    reg = <0x0 0x10500000 0x80000>;
> +                    memory-region = <&reserved_mem>;
> +                };
> +
>              };
>          };
>      };
> -- 
> 2.18.0
> 

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

* Re: [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema
  2020-10-08 18:32   ` Rob Herring
@ 2020-10-09  5:28     ` Ricardo Cañuelo
  2020-10-09 13:23       ` Rob Herring
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-09  5:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov

Hi Rob,

Thanks for reviewing the patch

On jue 08-10-2020 13:32:41, Rob Herring wrote: 
> These already have a type and description, don't repeat it here.
> 
> If no other constraints, then just 'linux,keymap: true'. Or you can use 
> unevaluatedProperties instead of additionalProperties. The former will 
> take the $ref to matrix-keymap.yaml into account.

That's what I did in v1, using input/imx-keypad.yaml as an example along
with the comment in example-schema.yaml about unevaluatedProperties, but
then I also saw you mention in other threads that support for
unevaluatedProperties is not implemented yet and that documenting the
additional properties/nodes is preferred (that's what I understood, at
least):

    https://lore.kernel.org/dri-devel/CAL_JsqKPXJxsHPS34_TCf9bwgKxZNSV4mvQR-WKRnknQVtGGxQ@mail.gmail.com/

Was it right in v1 then?

Cheers,
Ricardo

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-08 18:38   ` Rob Herring
@ 2020-10-09  5:48     ` Ricardo Cañuelo
  2020-10-09 13:34       ` Rob Herring
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-09  5:48 UTC (permalink / raw)
  To: Rob Herring
  Cc: kernel, enric.balletbo, bleung, groeck, sjg, dianders,
	devicetree, dmitry.torokhov, cychiang

Hi Rob,

Thanks for taking the time to review this. Find my answers below:

On jue 08-10-2020 13:38:18, Rob Herring wrote:
> > +  codecs:
> 
> Doesn't moving this require a driver change? 

I studied the driver as thoroughly as I could (what
sound/soc/codecs/cros_ec_codec.c:cros_ec_codec_platform_probe does) and
I think the driver should still be able to handle this. Unfortunately, I
can't test it. Can any of the driver maintainers share their
input about this? I'm adding Cheng-Yi Chiang to cc as well.

> If you only need 1 codec, then just drop the unit-address.

Thank you. Yes, as far as I can tell there's only this codec (so far).

Cheers,
Ricardo

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

* Re: [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema
  2020-10-09  5:28     ` Ricardo Cañuelo
@ 2020-10-09 13:23       ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-10-09 13:23 UTC (permalink / raw)
  To: Ricardo Cañuelo
  Cc: Collabora Kernel ML, Enric Balletbo i Serra, Benson Leung,
	Guenter Roeck, Simon Glass, Doug Anderson, devicetree,
	Dmitry Torokhov

On Fri, Oct 9, 2020 at 12:28 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Hi Rob,
>
> Thanks for reviewing the patch
>
> On jue 08-10-2020 13:32:41, Rob Herring wrote:
> > These already have a type and description, don't repeat it here.
> >
> > If no other constraints, then just 'linux,keymap: true'. Or you can use
> > unevaluatedProperties instead of additionalProperties. The former will
> > take the $ref to matrix-keymap.yaml into account.
>
> That's what I did in v1, using input/imx-keypad.yaml as an example along
> with the comment in example-schema.yaml about unevaluatedProperties, but
> then I also saw you mention in other threads that support for
> unevaluatedProperties is not implemented yet and that documenting the
> additional properties/nodes is preferred (that's what I understood, at
> least):
>
>     https://lore.kernel.org/dri-devel/CAL_JsqKPXJxsHPS34_TCf9bwgKxZNSV4mvQR-WKRnknQVtGGxQ@mail.gmail.com/
>
> Was it right in v1 then?

Yes. There is some value in explicitly listing the properties here if
only a subset of them are valid for this binding, but that's not the
case here.


Rob

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-09  5:48     ` Ricardo Cañuelo
@ 2020-10-09 13:34       ` Rob Herring
  2020-10-13  6:46         ` Ricardo Cañuelo
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2020-10-09 13:34 UTC (permalink / raw)
  To: Ricardo Cañuelo
  Cc: Collabora Kernel ML, Enric Balletbo i Serra, Benson Leung,
	Guenter Roeck, Simon Glass, Doug Anderson, devicetree,
	Dmitry Torokhov, Cheng-Yi Chiang

On Fri, Oct 9, 2020 at 12:48 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Hi Rob,
>
> Thanks for taking the time to review this. Find my answers below:
>
> On jue 08-10-2020 13:38:18, Rob Herring wrote:
> > > +  codecs:
> >
> > Doesn't moving this require a driver change?
>
> I studied the driver as thoroughly as I could (what
> sound/soc/codecs/cros_ec_codec.c:cros_ec_codec_platform_probe does) and
> I think the driver should still be able to handle this. Unfortunately, I
> can't test it. Can any of the driver maintainers share their
> input about this? I'm adding Cheng-Yi Chiang to cc as well.

The question is not what cros_ec_codec_platform_probe does, but how
the platform device is created by the mfd driver. I think that's just
a call to of_platform_populate which will only create immediate child
devices unless there's a simple-bus or simple-mfd compatible. So I
guess you could also add 'simple-mfd' rather than a driver change.
However, there could be some expectation in the codec driver that the
immediate parent is the EC node.

> > If you only need 1 codec, then just drop the unit-address.
>
> Thank you. Yes, as far as I can tell there's only this codec (so far).

I would probably go this route. You could add this level if there's
ever more than one codec. However, I'm still not clear what the
address represents for the codec. Is it needed? The address
space/format is defined by the parent node. So is this defined by the
EC? If so, other components don't have an address?

Rob

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

* Re: [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema
  2020-10-08 10:28 ` [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
@ 2020-10-11 11:17   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 16+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-11 11:17 UTC (permalink / raw)
  To: Ricardo Cañuelo, robh
  Cc: kernel, bleung, groeck, sjg, dianders, devicetree, dmitry.torokhov

Hi Ricardo,

Thank you for the patches.

On 8/10/20 12:28, Ricardo Cañuelo wrote:
> Convert the google,cros-ec-i2c-tunnel binding to YAML and add it as a
> property of google,cros-ec.yaml.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>

Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks,
  Enric

> ---
>  .../i2c/google,cros-ec-i2c-tunnel.yaml        | 63 +++++++++++++++++++
>  .../bindings/i2c/i2c-cros-ec-tunnel.txt       | 39 ------------
>  .../bindings/mfd/google,cros-ec.yaml          |  5 ++
>  3 files changed, 68 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..f83ff67596e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml
> @@ -0,0 +1,63 @@
> +# 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:
> +  - |
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cros-ec@0 {
> +            compatible = "google,cros-ec-spi";
> +            reg = <0>;
> +            spi-max-frequency = <5000000>;
> +
> +            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>;
> -			};
> -		};
> -	}
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index f49c0d5d31ad..c45cf30ea3aa 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -71,6 +71,11 @@ properties:
>    wakeup-source:
>      description: Button can wake-up the system.
>  
> +patternProperties:
> +  "^i2c-tunnel[0-9]*$":
> +    type: object
> +    $ref: "/schemas/i2c/google,cros-ec-i2c-tunnel.yaml#"
> +
>  required:
>    - compatible
>  
> 

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-09 13:34       ` Rob Herring
@ 2020-10-13  6:46         ` Ricardo Cañuelo
  2020-10-14  2:18           ` Tzung-Bi Shih
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-13  6:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: Collabora Kernel ML, Enric Balletbo i Serra, Benson Leung,
	Guenter Roeck, Simon Glass, Doug Anderson, devicetree,
	Dmitry Torokhov, Cheng-Yi Chiang, tzungbi

Hi Rob,

On vie 09-10-2020 08:34:21, Rob Herring wrote:
> I would probably go this route. You could add this level if there's
> ever more than one codec. However, I'm still not clear what the
> address represents for the codec. Is it needed? The address
> space/format is defined by the parent node. So is this defined by the
> EC? If so, other components don't have an address?

The address represents the physical base address and length of a shared
memory region from the EC. This seems to be the only component in the EC
that needs an address AFAICT, so I guess the unit address and the
intermediate node are needed to make it compatible with the existing EC
binding.

CC'ing Tzung-Bi Shih too, maybe he can give us some info about the
driver.

Thanks,
Ricardo

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-13  6:46         ` Ricardo Cañuelo
@ 2020-10-14  2:18           ` Tzung-Bi Shih
  2020-10-15  6:52             ` Ricardo Cañuelo
  0 siblings, 1 reply; 16+ messages in thread
From: Tzung-Bi Shih @ 2020-10-14  2:18 UTC (permalink / raw)
  To: Ricardo Cañuelo
  Cc: Rob Herring, Collabora Kernel ML, Enric Balletbo i Serra,
	Benson Leung, Guenter Roeck, Simon Glass, Doug Anderson,
	devicetree, Dmitry Torokhov, Cheng-Yi Chiang

On Tue, Oct 13, 2020 at 2:46 PM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
> On vie 09-10-2020 08:34:21, Rob Herring wrote:
> > I would probably go this route. You could add this level if there's
> > ever more than one codec. However, I'm still not clear what the
> > address represents for the codec. Is it needed? The address
> > space/format is defined by the parent node. So is this defined by the
> > EC? If so, other components don't have an address?
>
> The address represents the physical base address and length of a shared
> memory region from the EC. This seems to be the only component in the EC
> that needs an address AFAICT, so I guess the unit address and the
> intermediate node are needed to make it compatible with the existing EC
> binding.

Correct.  The address represents where the EC codec is located.  The
driver uses the address to find the corresponding shared memory (if
any, depends on the EC codec's capabilities).

In practice, there is at most only 1 EC codec per machine.  In theory,
it can be multiple EC codecs though (multiple microprocessors run EC
OS for example).

The intermediate layer (i.e. codecs { ... }) breaks current code as
you already discussed about that in previous threads:
- of_platform_populate only creates immediate child devices.
- the codec driver expects its parent is EC node.

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-14  2:18           ` Tzung-Bi Shih
@ 2020-10-15  6:52             ` Ricardo Cañuelo
  2020-10-15  8:25               ` Enric Balletbo i Serra
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Cañuelo @ 2020-10-15  6:52 UTC (permalink / raw)
  To: Tzung-Bi Shih
  Cc: Rob Herring, Collabora Kernel ML, Enric Balletbo i Serra,
	Benson Leung, Guenter Roeck, Simon Glass, Doug Anderson,
	devicetree, Dmitry Torokhov, Cheng-Yi Chiang

On Wed, 2020-10-14 at 10:18 +0800, Tzung-Bi Shih wrote:
> The intermediate layer (i.e. codecs { ... }) breaks current code as
> you already discussed about that in previous threads:
> - of_platform_populate only creates immediate child devices.
> - the codec driver expects its parent is EC node.

Thanks for your input, I'll try to address these issues in the drivers in v3.

Cheers,
Ricardo


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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-15  6:52             ` Ricardo Cañuelo
@ 2020-10-15  8:25               ` Enric Balletbo i Serra
  2020-10-15  9:50                 ` Tzung-Bi Shih
  0 siblings, 1 reply; 16+ messages in thread
From: Enric Balletbo i Serra @ 2020-10-15  8:25 UTC (permalink / raw)
  To: Ricardo Cañuelo, Tzung-Bi Shih
  Cc: Rob Herring, Collabora Kernel ML, Benson Leung, Guenter Roeck,
	Simon Glass, Doug Anderson, devicetree, Dmitry Torokhov,
	Cheng-Yi Chiang

Hi Tzung-Bi,

On 15/10/20 8:52, Ricardo Cañuelo wrote:
> On Wed, 2020-10-14 at 10:18 +0800, Tzung-Bi Shih wrote:
>> The intermediate layer (i.e. codecs { ... }) breaks current code as
>> you already discussed about that in previous threads:
>> - of_platform_populate only creates immediate child devices.
>> - the codec driver expects its parent is EC node.
> 
> Thanks for your input, I'll try to address these issues in the drivers in v3.
> 

Can you remind us which platforms support cros-ec-codec and why there is no an
EC_FEATURE for that?

Thanks,
 Enric

> Cheers,
> Ricardo
> 

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

* Re: [PATCH v2 3/3] mfd: google,cros-ec: add missing properties
  2020-10-15  8:25               ` Enric Balletbo i Serra
@ 2020-10-15  9:50                 ` Tzung-Bi Shih
  0 siblings, 0 replies; 16+ messages in thread
From: Tzung-Bi Shih @ 2020-10-15  9:50 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Ricardo Cañuelo, Rob Herring, Collabora Kernel ML,
	Benson Leung, Guenter Roeck, Simon Glass, Doug Anderson,
	devicetree, Dmitry Torokhov, Cheng-Yi Chiang

On Thu, Oct 15, 2020 at 4:25 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> On 15/10/20 8:52, Ricardo Cañuelo wrote:
> > On Wed, 2020-10-14 at 10:18 +0800, Tzung-Bi Shih wrote:
> >> The intermediate layer (i.e. codecs { ... }) breaks current code as
> >> you already discussed about that in previous threads:
> >> - of_platform_populate only creates immediate child devices.
> >> - the codec driver expects its parent is EC node.
> >
> > Thanks for your input, I'll try to address these issues in the drivers in v3.
> >
>
> Can you remind us which platforms support cros-ec-codec and why there is no an
> EC_FEATURE for that?

I failed to connect the question to this discussion thread.

But:
- Originally, MT8183 (arm64-based) chromebooks aimed to use
cros-ec-codec.  However, we realized the RAM size is insufficient.
For some reason, the cros-ec-codec feature was not enabled.
- Currently, we have some AMD Picasso chromebooks that use cros-ec-codec.
- It used to have EC_FEATURE_AUDIO_CODEC.  But I removed it in
https://patchwork.kernel.org/project/alsa-devel/patch/20191017213539.01.I374c311eaca0d47944a37b07acbe48fdb74f734d@changeid/.
Because we don't rely on the EC_FEATURE to do anything specifically.
As long as there is a corresponding node in DTS or ACPI table, the
cros-ec-codec gets probed.

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

end of thread, other threads:[~2020-10-15  9:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 10:28 [PATCH v2 0/3] Fix checker warnings related to cros-ec binding Ricardo Cañuelo
2020-10-08 10:28 ` [PATCH v2 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
2020-10-11 11:17   ` Enric Balletbo i Serra
2020-10-08 10:28 ` [PATCH v2 2/3] dt-bindings: input: convert cros-ec-keyb " Ricardo Cañuelo
2020-10-08 18:32   ` Rob Herring
2020-10-09  5:28     ` Ricardo Cañuelo
2020-10-09 13:23       ` Rob Herring
2020-10-08 10:28 ` [PATCH v2 3/3] mfd: google,cros-ec: add missing properties Ricardo Cañuelo
2020-10-08 18:38   ` Rob Herring
2020-10-09  5:48     ` Ricardo Cañuelo
2020-10-09 13:34       ` Rob Herring
2020-10-13  6:46         ` Ricardo Cañuelo
2020-10-14  2:18           ` Tzung-Bi Shih
2020-10-15  6:52             ` Ricardo Cañuelo
2020-10-15  8:25               ` Enric Balletbo i Serra
2020-10-15  9:50                 ` Tzung-Bi Shih

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.