linux-kernel - Re: [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YgZjWh6dQQJEK21K@robh.at.kernel.org>
Date:   Fri, 11 Feb 2022 07:23:38 -0600
From:   Rob Herring <robh@...nel.org>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-i2c@...r.kernel.org,
        Wolfram Sang <wsa@...nel.org>, Ondrej Jirman <x@....cz>
Subject: Re: [PATCH 1/5] dt-bindings: input: Add the PinePhone keyboard
 binding

On Sat, Jan 29, 2022 at 05:00:38PM -0600, Samuel Holland wrote:
> Add devicetree support for the PinePhone keyboard case, which provides a
> matrix keyboard interface and a proxied I2C bus.
> 
> Signed-off-by: Samuel Holland <samuel@...lland.org>
> ---
> 
>  .../input/pine64,pinephone-keyboard.yaml      | 90 +++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml
> new file mode 100644
> index 000000000000..00f084b263f0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/pine64,pinephone-keyboard.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/pine64,pinephone-keyboard.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Pine64 PinePhone keyboard device tree bindings
> +
> +maintainers:
> +  - Samuel Holland <samuel@...lland.org>
> +
> +description:
> +  A keyboard accessory is available for the Pine64 PinePhone and PinePhone Pro.
> +  It connects via I2C, providing a raw scan matrix, a flashing interface, and a
> +  subordinate I2C bus for communication with a battery charger IC.
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: pine64,pinephone-keyboard
> +
> +  reg:
> +    const: 0x15
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  linux,fn-keymap:

This should be handled in a common way. Not sure if there's anything 
existing for alternate key maps. Child nodes of alternate maps would 
scale better than new property name for every alternate map. Or you 
could make linux,keymap contain multiple maps (e.g. 2x XxY entries) 

Or if the map doesn't change, just put it in the driver.

> +    $ref: /schemas/input/matrix-keymap.yaml#/properties/linux,keymap

Referencing individual properties should be avoided.

> +    description: keymap used when the Fn key is pressed
> +
> +  wakeup-source: true
> +
> +  i2c-bus:
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +dependencies:
> +  linux,fn-keymap: [ 'keypad,num-columns', 'keypad,num-rows' ]
> +  linux,keymap: [ 'keypad,num-columns', 'keypad,num-rows' ]
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      keyboard@15 {
> +        compatible = "pine64,pinephone-keyboard";
> +        reg = <0x15>;
> +        interrupt-parent = <&r_pio>;
> +        interrupts = <0 12 IRQ_TYPE_EDGE_FALLING>; /* PL12 */
> +        keypad,num-rows = <6>;
> +        keypad,num-columns = <12>;
> +        linux,fn-keymap = <MATRIX_KEY(0,  0, KEY_FN_ESC)
> +                           MATRIX_KEY(0,  1, KEY_F1)
> +                           MATRIX_KEY(0,  2, KEY_F2)
> +                           /* ... */
> +                           MATRIX_KEY(5,  2, KEY_FN)
> +                           MATRIX_KEY(5,  3, KEY_LEFTALT)
> +                           MATRIX_KEY(5,  5, KEY_RIGHTALT)>;
> +        linux,keymap = <MATRIX_KEY(0,  0, KEY_ESC)
> +                        MATRIX_KEY(0,  1, KEY_1)
> +                        MATRIX_KEY(0,  2, KEY_2)
> +                        /* ... */
> +                        MATRIX_KEY(5,  2, KEY_FN)
> +                        MATRIX_KEY(5,  3, KEY_LEFTALT)
> +                        MATRIX_KEY(5,  5, KEY_RIGHTALT)>;
> +
> +        i2c-bus {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          charger@75 {
> +            reg = <0x75>;
> +          };
> +        };
> +      };
> +    };
> -- 
> 2.33.1
> 
>