Re: [PATCH V2 1/2] Documentation: devicetree: bindings: input

From: Yingjoe Chen <yingjoe.chen@mediatek.com>
To: fengping yu <fengping.yu@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	devicetree@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-kernel@vger.kernel.org, wsd_upstream@mediatek.com
Subject: Re: [PATCH V2 1/2] Documentation: devicetree: bindings: input
Date: Tue, 7 Jan 2020 15:49:25 +0800	[thread overview]
Message-ID: <1578383365.12740.7.camel@mtksdaap41> (raw)
In-Reply-To: <20191227013722.12201-2-fengping.yu@mediatek.com>

Hi, Fengping,

You missed several maintainers and mailing lists. Please
use ./scripts/get_maintainer.pl to find proper maintainers & lists.

Bad summary, You can use git log --oneline to find proper summary.
Maybe:
dt-bindings: input: keyboard: add MediaTek keypad controller binding

On Fri, 2019-12-27 at 09:37 +0800, fengping yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
> 
> Add mtk keypad device tree documentation.
> 
> Change-Id: I415b76ee980784992763c768829b2dda76699dea

Please remove Change-Id

> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
>  .../devicetree/bindings/input/mtk-kpd.txt     | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.txt b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> new file mode 100644
> index 000000000000..2af81e696159
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/mtk-kpd.txt
> @@ -0,0 +1,45 @@
> +* Mediatek's Keypad Controller device tree binding
> +
> +Mediatek's Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +Required properties:
> +- compatible should contain:
> +	* "mediatek,mt6779-keypad" for MT6779 keypad
> +	* "mediatek,kp" for common keypad
> +
> +- reg: The base address of the Keypad register bank.
> +
> +- interrupts: A single interrupt specifier.
> +
> +- mediatek,key-debounce-ms: Debounce interval in milliseconds, maximum value
> +  is 256ms
> +
> +- mediatek,hw-map-num: Number of entries of keycode in mediatek,hw-init-map
> +
> +- mediatek,hw-init-map: An array of packed 1-cell entries containing the
> +  equivalent of hw-map-num linux key-code. Each cell is Linux keymap of
> +  corresponding row/col, arrange like this:
> +
> +  row0col0, row0col1, row0col2,...row0col8,
> +  row1col0, row1col1, row1col2,...row1col8,
> +  ...
> +  row7col0, row7col1, row7col2,...row7col8.
> +
> +Example:
> +
> +	keypad: kp@10010000 {
> +		compatible = "mediatek,mt6779-keypad", "mediatek,kp";
> +		reg = <0 0x10010000 0 0x1000>;
> +		interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +
> +	&keypad {
> +		mediatek,key-debounce-ms = <1024>;

The maximum value is 256ms, why set it to 1024?

> +		/*HW Keycode [0~71] -> Linux Keycode*/
Don't need this line.

> +		mediatek,khw-map-num = <72>;

typo?

> +		mediatek,hw-init-map = <114 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 >;

I think it make sense to arrange it as 9*8 array to make it shorter in
lines and easier to find entries.

Joe.C

> +	};

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek