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

[Andrew Jeffery <andrew@aj.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]

Hi Brad,

If you do future revisions of these patches, can you please Cc me?

On Wed, 2017-04-19 at 00:18 -0400, 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@fuzziesquirrel.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 20 ++++++++++++++++++++
>  1 file changed, 20 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..aa1708e 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -31,6 +31,26 @@
> >  		};
> >  	};
>  
> +	gpio-keys@0 {

does the @0 give us a stable device name for userspace to open?

Do we really want to go this way? We now have useful unique codes for
the "key"s, why not use the one node? Or is your concern we've now tied
the function to a value that might vary across platforms? If so, are
you relying on always specifying "air-water" in @0? If you are, I think
I'd prefer the arbitrary codes, rather than this.

Andrew

> +		compatible = "gpio-keys";
> +
> > +		air-water {
> > +			label = "air-water";
> > +			gpios = <&gpio ASPEED_GPIO(B, 5) GPIO_ACTIVE_LOW>;
> > +			linux,code = <ASPEED_GPIO(B, 5)>;
> > +		};
> > +	};
> +
> > +	gpio-keys@1 {
> > +		compatible = "gpio-keys";
> +
> > +		checkstop {
> > +			label = "checkstop";
> > +			gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
> > +			linux,code = <ASPEED_GPIO(J, 2)>;
> > +		};
> > +	};
> +
> >  	leds {
> >  		compatible = "gpio-leds";
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]