Re: [PATCH] dt-bindings: nvidia,tegra20-kbc: Convert to json-schema

[Krzysztof Kozlowski]

On 18/02/2022 11:10, Max Buchholz wrote:
> From: Max Buchholz <Max.Buchholz@xxxxxx>
> 
> This converts the Nvidia Tegra keyboard controller bindings to YAML
> and fix them up a bit.
> 
> Acked-by: David Heidelberg <david@xxxxxxx>
> Signed-off-by: Max Buchholz <max.buchholz@xxxxxx>
> ---
>  .../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@xxxxxx>

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