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

[Brad Bishop]

> On Apr 19, 2017, at 10:29 AM, Andrew Jeffery <andrew at aj.id.au> wrote:
> 
> Hi Brad,
> 
> If you do future revisions of these patches, can you please Cc me?

will do.

> 
> 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 at 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 at 0 {
> 
> does the @0 give us a stable device name for userspace to open?

No it doesn’t.  This is my lack of DT skillz showing.  I was looking
into how we can give userspace a stable device name.  I was going
down the udev path but I can’t find any any of the information from
these DT nodes in the udev event.

> 
> 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

Each gpio will have an application waiting for its state to change.  My
concern was waking up x number of applications every time any gpio changes
state.  Is that a valid concern?

> the function to a value that might vary across platforms? If so, are

No, not worried about this.  Applications will be getting their gpio number
on the command line anyway.

> 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 at 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";