From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bajor.fuzziesquirrel.com (mail.fuzziesquirrel.com [173.167.31.197]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3w8Lrv3d12zDqL2 for ; Fri, 21 Apr 2017 13:32:15 +1000 (AEST) X-Virus-Scanned: amavisd-new at fuzziesquirrel.com Sender: bradleyb@fuzziesquirrel.com Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: Re: [PATCH linux dev-4.7 v3 2/2] ARM: dts: Enable checkstop and cooling gpio keys From: Brad Bishop In-Reply-To: Date: Thu, 20 Apr 2017 23:32:12 -0400 Cc: Andrew Jeffery , OpenBMC Maillist Content-Transfer-Encoding: quoted-printable Message-Id: <6871B647-C223-4D25-ABAE-3778E1CB4D19@fuzziesquirrel.com> References: <1492575497-1419-1-git-send-email-bradleyb@fuzziesquirrel.com> <1492575497-1419-3-git-send-email-bradleyb@fuzziesquirrel.com> <1492612195.1011.4.camel@aj.id.au> <1492662892.1011.7.camel@aj.id.au> <86B3586F-7CEC-4389-8CCC-38D5423A4CE7@fuzziesquirrel.com> To: Joel Stanley X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Apr 2017 03:32:16 -0000 > On Apr 20, 2017, at 10:14 PM, Joel Stanley wrote: >=20 > Hi Brad, >=20 > On Thu, Apr 20, 2017 at 11:54 PM, Brad Bishop > wrote: >>>> 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? >>>=20 >>> How many applications do we expect to be reading the evdev? What are >>> our expected interrupt rates? What are the worst expected cases? >>=20 >> For Witherspoon/Zaius or any OpenBMC platform? I obviously can=E2=80=99= t say for the >> latter case. On a bigger/enterprise systems I can see on the order = of 10 to 100 >> applications looking at these. I don=E2=80=99t have a good sense of = how often. >>=20 >> On Witherspoon/Zaius, sure, we only have a handful of applications = and the >> interrupt rate should be very infrequent. >>=20 >> I think you typically see all the gpio keys mapped to a single device = because >> the usual thing to do is have a single application (Xorg) treat it = like a keyboard. >=20 > I've been following the discussion you've been having. I went to merge > the patches today, but first decided to satisfy myself that this was > the right way to go. I didn't arrive at the same conclusion as you and > Andrew. >=20 > The userspace use cases you've presented would be served well by a > single node. We have the flexibility to revisit this decision when > someone builds a machine that has 100 GPIOs they want to monitor; in > addition I think we would revisit the decision to have 100 userspace > programs. >=20 > By having separate nodes, you're creating a new instance of the kernel > driver for each node. >=20 > The driver, and the binding, is designed to support your use case - > multiple key events generated by the hardware. Honestly I=E2=80=99m OK with either approach. I think we will have = userspace configurable enough that switching back and forth would be pretty trivial, if this = would need to be revisited. v5 en-route. >=20 > Finally, I looked at the other machines in the tree. None of them have > separate gpio-keys nodes Yeah I noticed this too. I chalked it up to my previously mentioned user-is-probably-Xorg logic, but standard practices are not usually a hard sell for me=E2=80=A6. >=20 > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts: >=20 > gpio-keys { > compatible =3D "gpio-keys"; > pinctrl-0 =3D <&power_button_pin &reset_button_pin>; > pinctrl-names =3D "default"; >=20 > power-button { > label =3D "Power Button"; > linux,code =3D ; > gpios =3D <&gpio0 27 GPIO_ACTIVE_HIGH>; > }; >=20 > reset-button { > label =3D "Reset Button"; > linux,code =3D ; > gpios =3D <&gpio1 9 GPIO_ACTIVE_LOW>; > }; > }; >=20 > arch/arm/boot/dts/at91sam9261ek.dts > gpio_keys { > compatible =3D "gpio-keys"; >=20 > button_0 { > label =3D "button_0"; > gpios =3D <&pioA 27 GPIO_ACTIVE_LOW>; > linux,code =3D <256>; > wakeup-source; > }; >=20 > button_1 { > label =3D "button_1"; > gpios =3D <&pioA 26 GPIO_ACTIVE_LOW>; > linux,code =3D <257>; > wakeup-source; > }; >=20 > button_2 { > label =3D "button_2"; > gpios =3D <&pioA 25 GPIO_ACTIVE_LOW>; > linux,code =3D <258>; > wakeup-source; > }; >=20 > button_3 { > label =3D "button_3"; > gpios =3D <&pioA 24 GPIO_ACTIVE_LOW>; > linux,code =3D <259>; > wakeup-source; > }; > }; >=20 > Cheers, >=20 > Joel >=20 >=20 >=20 >>=20 >> We aren=E2=80=99t structured that way so rethinking the usual = approach seems reasonable. >> Another reason I went for per gpio devices because it prevents = applications from >> reacting to each others events without any special library code. >>=20 >> I=E2=80=99m not saying there aren=E2=80=99t disadvantages to this = approach - I just don=E2=80=99t know what >> they are? >>=20 >>>=20 >>> It's hard to judge without knowing the numbers, but considering the >>> chips we run on I agree we should generally favour performance if >>> design is getting in the way. But to make that trade off we should = be >>=20 >> Again, what exactly is being traded-off ? >>=20 >>> clear on the performance numbers. >>>=20 >>> Andrew