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

[Brad Bishop]

Andrew

Thanks for looking.

> On Apr 13, 2017, at 6:53 PM, Andrew Jeffery <andrew at aj.id.au> wrote:
> 
> 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)>;

Agreed and fixed.

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

No and No.  Busted.  I fully exercised v3 on a Palmetto ( I can’t easily control the state of these specific Witherspoon gpios ).

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