Re: [PATCH V2 3/3] ARM: dts: Add node for GPIO keys on SMDK5420

[Tomasz Figa <tomasz.figa@gmail.com>]

Hi,

On 27.03.2014 07:13, Vikas Sajjan wrote:
> From: Abhilash Kesavan <a.kesavan@samsung.com>
>
> Add support for the suspend/resume button (SW7) on SMDK5420 using
> gpio-keys driver.
>
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com>
> ---
>   arch/arm/boot/dts/exynos5420-smdk5420.dts |   11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> index 6910485..5ae0dd4 100644
> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> @@ -158,6 +158,17 @@
>   		};
>   	};
>
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		wakeup {

Is wakeup the label of this key on the PCB? The "label" property below 
would suggest otherwise. This should probably be called key-sw7 or 
something like this.

Moreover, is this the only GPIO key on the board? If there are also 
SW0-SW6, you should add them too.

> +			label = "SW7";
> +			gpios = <&gpx0 0 1>;
> +			linux,code = <143>;	/* KEY_WAKEUP */

Why not use KEY_WAKEUP defined in include/dt-bindings/input/input.h 
directly?

Best regards,
Tomasz