All of lore.kernel.org
 help / color / mirror / Atom feed
From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
To: "Ricardo Cañuelo" <ricardo.canuelo@collabora.com>, robh@kernel.org
Cc: kernel@collabora.com, bleung@chromium.org, groeck@chromium.org,
	sjg@chromium.org, dianders@chromium.org,
	devicetree@vger.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Re: [PATCH 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema
Date: Mon, 5 Oct 2020 11:03:54 +0200	[thread overview]
Message-ID: <3f30a00e-8b1c-4ac7-c5ed-8f23c7af9af0@collabora.com> (raw)
In-Reply-To: <20201005071403.17450-3-ricardo.canuelo@collabora.com>

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>;
> +    };
> 

  reply	other threads:[~2020-10-05  9:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-05  7:13 [PATCH 0/3] Fix checker warnings related to cros-ec binding Ricardo Cañuelo
2020-10-05  7:14 ` [PATCH 1/3] dt-bindings: i2c: convert i2c-cros-ec-tunnel to json-schema Ricardo Cañuelo
2020-10-05  8:52   ` Enric Balletbo i Serra
2020-10-05  9:18     ` Ricardo Cañuelo
2020-10-05  9:27       ` Enric Balletbo i Serra
2020-10-05 14:04   ` Rob Herring
2020-10-05  7:14 ` [PATCH 2/3] dt-bindings: input: convert cros-ec-keyb " Ricardo Cañuelo
2020-10-05  9:03   ` Enric Balletbo i Serra [this message]
2020-10-05  9:35     ` Ricardo Cañuelo
2020-10-05  7:14 ` [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties Ricardo Cañuelo
2020-10-05  9:23   ` Enric Balletbo i Serra
2020-10-05  9:48     ` Ricardo Cañuelo
2020-10-05 15:37   ` Rob Herring
2020-10-05 15:48     ` Enric Balletbo i Serra
2020-10-06  6:13     ` Ricardo Cañuelo
2020-10-06 11:28       ` Guenter Roeck
2020-10-06 13:07         ` Ricardo Cañuelo
2020-10-06 21:35       ` Rob Herring
2020-10-07  6:31         ` Ricardo Cañuelo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f30a00e-8b1c-4ac7-c5ed-8f23c7af9af0@collabora.com \
    --to=enric.balletbo@collabora.com \
    --cc=bleung@chromium.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=groeck@chromium.org \
    --cc=kernel@collabora.com \
    --cc=ricardo.canuelo@collabora.com \
    --cc=robh@kernel.org \
    --cc=sjg@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.