[PATCH linux dev-4.7 v2 2/2] ARM: dts: Enable checkstop and cooling gpio keys

[Andrew Jeffery]

Hi Brad,

On Fri, Apr 14, 2017, at 05:07, Brad Bishop wrote:
> Enable gpio-keys events for the checkstop and water/air cooled
> gpios for use by applications on the Witherspoon system.
> 
> Signed-off-by: Brad Bishop <bradleyb at fuzziesquirrel.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index e3a7b77..4a68b30 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -31,6 +31,22 @@
>  		};
>  	};
>  
> +       gpio-keys {
> +               compatible = "gpio-keys";
> +
> +               air-water {
> +                       label = "air-water";
> +                       gpios = <&gpio ASPEED_GPIO(B, 5)
> GPIO_ACTIVE_LOW>;
> +                       linux,code = <&gpio ASPEED_GPIO(B, 5)
> GPIO_ACTIVE_LOW>;

This doesn't look right to me. linux,code doesn't appear to be anything
more than a single value cell in the documentation: 

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/input/gpio-keys.txt?id=refs/tags/v4.11-rc6

I expect you just want the macro,  and not phandle or gpio flags. I was
expecting to see:

linux,code = <ASPEED_GPIO(B, 5)>;

Is there a reason you've done it this way? Have you checked reading
values from userspace gives the expected value?

Andrew

> +               };
> +
> +               checkstop {
> +                       label = "checkstop";
> +                       gpios = <&gpio ASPEED_GPIO(J, 2)
> GPIO_ACTIVE_LOW>;
> +                       linux,code = <&gpio ASPEED_GPIO(J, 2)
> GPIO_ACTIVE_LOW>;
> +               };
> +       };
> +
>  	leds {
>  		compatible = "gpio-leds";
>  
> -- 
> 1.8.3.1