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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d4fcf76c-5d43-d8fc-bdc7-1a04ab517678@canonical.com>
Date:   Fri, 18 Feb 2022 12:29:53 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To:     Max Buchholz <max.buchholz@....de>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>
Cc:     David Heidelberg <david@...t.cz>, linux-input@...r.kernel.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: nvidia,tegra20-kbc: Convert to json-schema

On 18/02/2022 11:10, Max Buchholz wrote:
> From: Max Buchholz <Max.Buchholz@....de>
> 
> This converts the Nvidia Tegra keyboard controller bindings to YAML
> and fix them up a bit.
> 
> Acked-by: David Heidelberg <david@...t.cz>
> Signed-off-by: Max Buchholz <max.buchholz@....de>
> ---
>  .../bindings/input/nvidia,tegra20-kbc.txt     |  55 ---------
>  .../bindings/input/nvidia,tegra20-kbc.yaml    | 114 ++++++++++++++++++
>  2 files changed, 114 insertions(+), 55 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
>  create mode 100644 Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt b/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
> deleted file mode 100644
> index 1faa7292e21f..000000000000
> --- a/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -* Tegra keyboard controller
> -The key controller has maximum 24 pins to make matrix keypad. Any pin
> -can be configured as row or column. The maximum column pin can be 8
> -and maximum row pins can be 16 for Tegra20/Tegra30.
> -
> -Required properties:
> -- compatible: "nvidia,tegra20-kbc"
> -- reg: Register base address of KBC.
> -- interrupts: Interrupt number for the KBC.
> -- nvidia,kbc-row-pins: The KBC pins which are configured as row. This is an
> -  array of pin numbers which is used as rows.
> -- nvidia,kbc-col-pins: The KBC pins which are configured as column. This is an
> -  array of pin numbers which is used as column.
> -- linux,keymap: The keymap for keys as described in the binding document
> -  devicetree/bindings/input/matrix-keymap.txt.
> -- clocks: Must contain one entry, for the module clock.
> -  See ../clocks/clock-bindings.txt for details.
> -- resets: Must contain an entry for each entry in reset-names.
> -  See ../reset/reset.txt for details.
> -- reset-names: Must include the following entries:
> -  - kbc
> -
> -Optional properties, in addition to those specified by the shared
> -matrix-keyboard bindings:
> -
> -- linux,fn-keymap: a second keymap, same specification as the
> -  matrix-keyboard-controller spec but to be used when the KEY_FN modifier
> -  key is pressed.
> -- nvidia,debounce-delay-ms: delay in milliseconds per row scan for debouncing
> -- nvidia,repeat-delay-ms: delay in milliseconds before repeat starts
> -- nvidia,ghost-filter: enable ghost filtering for this device
> -- wakeup-source: configure keyboard as a wakeup source for suspend/resume
> -		 (Legacy property supported: "nvidia,wakeup-source")
> -
> -Example:
> -
> -keyboard: keyboard {
> -	compatible = "nvidia,tegra20-kbc";
> -	reg = <0x7000e200 0x100>;
> -	interrupts = <0 85 0x04>;
> -	clocks = <&tegra_car 36>;
> -	resets = <&tegra_car 36>;
> -	reset-names = "kbc";
> -	nvidia,ghost-filter;
> -	nvidia,debounce-delay-ms = <640>;
> -	nvidia,kbc-row-pins = <0 1 2>;    /* pin 0, 1, 2 as rows */
> -	nvidia,kbc-col-pins = <11 12 13>; /* pin 11, 12, 13 as columns */
> -	linux,keymap = <0x00000074
> -			0x00010067
> -			0x00020066
> -			0x01010068
> -			0x02000069
> -			0x02010070
> -			0x02020071>;
> -};
> diff --git a/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.yaml b/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.yaml
> new file mode 100644
> index 000000000000..076b347a6f07
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/input/nvidia,tegra20-kbc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Nvidia Tegra keyboard controller
> +
> +maintainers:
> +  - Max Buchholz <max.buchholz@....de>

Maybe also add TEGRA KBC DRIVER maintainer? He was not CC here...

> +
> +description: >
> +  The key controller has maximum 24 pins to make matrix keypad. Any pin
> +  can be configured as row or column. The maximum column pin can be 8
> +  and maximum row pins can be 16 for Tegra20/Tegra30.
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra20-kbc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:

maxItems: 1

> +    description: Interrupt number for the KBC.

It's fairly obvious, so description can be skipped.

> +
> +  nvidia,kbc-row-pins:
> +    description: >
> +      The KBC pins which are configured as row. This is an
> +      array of pin numbers which is used as rows.

You basically duplicate the property name in description. "Row" is
obvious from property name. "Pins" as well. "Array" from the type below.
Please document the field without repeating the type and property name.

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  nvidia,kbc-col-pins:
> +    description: >
> +      The KBC pins which are configured as column. This is an
> +      array of pin numbers which is used as column.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  linux,keymap:
> +    description: >
> +      The keymap for keys as described in the binding document
> +      devicetree/bindings/input/matrix-keymap.txt.

1. The file is empty.
2. Reference matrxi-keymap.yaml instead with allOf.

> +
> +  clocks:
> +    maxItems: 1
> +    description: >
> +      Must contain one entry, for the module clock.
> +      See ../clocks/clock-bindings.txt for details.

Skip description, it's obvious and no need to reference old TXT.

> +
> +  resets:
> +    description: >
> +      Must contain an entry for each entry in reset-names.
> +      See ../reset/reset.txt for details.

maxItems
Skip description.

> +
> +  reset-names:
> +    const: kbc
> +
> +  linux,fn-keymap:
> +    description: >
> +      a second keymap, same specification as the
> +      matrix-keyboard-controller spec but to be used when the KEY_FN modifier
> +      key is pressed.

Does not look like standard property, so you need type.

> +
> +  nvidia,debounce-delay-ms:
> +    description: delay in milliseconds per row scan for debouncing
> +
> +  nvidia,repeat-delay-ms:
> +    description: delay in milliseconds before repeat starts
> +
> +  nvidia,ghost-filter:
> +    description: enable ghost filtering for this device
> +    type: boolean
> +
> +  wakeup-source:
> +    description: configure keyboard as a wakeup source for suspend/resume
> +
> +  nvidia,wakeup-source:
> +    description: configure keyboard as a wakeup source for suspend/resume
> +    deprecated: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - nvidia,kbc-row-pins
> +  - nvidia,kbc-col-pins
> +  - linux,keymap
> +  - clocks
> +  - resets
> +  - reset-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    keyboard: {
> +        compatible = "nvidia,tegra20-kbc";
> +        reg = <0x7000e200 0x100>;
> +        interrupts = <0 85 0x04>;

0 is GIC_SPI?
0x4 is a speficif flag? Use defines for these.

> +        clocks = <&tegra_car 36>;
> +        resets = <&tegra_car 36>;
> +        reset-names = "kbc";
> +        nvidia,ghost-filter;
> +        nvidia,debounce-delay-ms = <640>;
> +        nvidia,kbc-row-pins = <0 1 2>;    /* pin 0, 1, 2 as rows */
> +        nvidia,kbc-col-pins = <11 12 13>; /* pin 11, 12, 13 as columns */
> +        linux,keymap = <0x00000074
> +                0x00010067
> +                0x00020066
> +                0x01010068
> +                0x02000069
> +                0x02010070
> +                0x02020071>;

Align with 0x0000074 in first line.

> +    };
> --
> 2.35.1
> 


Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ