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

[Andrew Jeffery]

On Thu, 2017-04-20 at 10:24 -0400, Brad Bishop wrote:
> > > > On Apr 20, 2017, at 12:34 AM, Andrew Jeffery <andrew at aj.id.au> wrote:
> > 
> > On Wed, 2017-04-19 at 10:38 -0400, Brad Bishop wrote:
> > > > > > 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?
> > 
> > How many applications do we expect to be reading the evdev? What are
> > our expected interrupt rates? What are the worst expected cases?
> 
> For Witherspoon/Zaius or any OpenBMC platform? 

Yes, this is what I was asking about, given they are concrete systems.

>  I obviously can’t say for the
> latter case. 

Yep.

>  On a bigger/enterprise systems I can see on the order of 10 to 100
> applications looking at these. 

Okay; having 10-100 processes woken is something we'd want to avoid.

>  I don’t have a good sense of how often.
> 
> On Witherspoon/Zaius, sure, we only have a handful of applications and the
> interrupt rate should be very infrequent.

That was my thought, which was why I wanted to check. While it might be
low, if we're waking up to 100 applications it's not ideal.

> 
> 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.
> 
> We aren’t structured that way so rethinking the usual approach seems reasonable. 

Sure; I was never against rethinking it. I just wanted a clear
explanation of why we would choose this route.

>  
> Another reason I went for per gpio devices because it prevents applications from
> reacting to each others events without any special library code.  
> 
> I’m not saying there aren’t disadvantages to this approach - I just don’t know what
> they are?

It's a bit strange from a design perspective as we now have two unique
identifiers for an event; the input device *and* the keycode emitted by
the event on the input device, which is the *only* keycode emitted by
the device.

For someone new to the system I expect this would seem a strange
redundancy if they weren't aware of the performance implications
inherent to the distributed nature of the application design.

> 
> > 
> > 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
> 
> Again, what exactly is being traded-off ?

The "oddity" in the design with respect to the "redundancy" I described
above.

Anyway, I think what you have outlined is reasonable.

Cheers,

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170421/29226d92/attachment-0001.sig>