Discussion on GPIO monitoring and design thoughts.

Discussion on GPIO monitoring and design thoughts.

[vishwa]

We had a technical discussion on HOWTO and what is the right thing to do 
handling the GPIO assertion and here is the gist.

1) Use gpio-keys and utilize libevdev wrappers ( 
https://www.freedesktop.org/software/libevdev/doc/latest/ ) to watch for 
data in /dev/input/event.

2) Define a target with set of services that would be called into by the 
GPIO monitor application when the GPIO state changes to what is expected.

  - 2.1) Another option was to just broadcast a signal that interested 
parties could catch and act but then it was not chosen because there 
would be no guarantee that the signal would ever be delivered / caught 
by needed parties.

  - 2.2 ) Also going the signal way demands a service that just needs to 
watch for that.

*) The services that are part of the target will then own complete 
handling of the condition instead of the application that monitored the 
GPIO.

---------

Here is an example of application that handles the GPIO assertion that 
happens when host does a checkstop.

1) Application's arguments need /dev/input/event<$number> and the 
expected GPIO state as indicated by linux,code in :

http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/gpio-keys.txt

2) Call this app after reaching `Host-Running` state thus avoiding any 
checks to see current state.

2) App will then use libevdev wrappers to watch for the state

3) On seeing the expected state change, app calls into the configured 
target.

4) Configured target right now has 2 services.

   - 4.1 ) Service that Creates an errorlog indicating the checkstop
   - 4.2 ) Service that will start the Quiesce target

--------

This design direction helps decouple applications that monitor and 
analyze the conditions.

Please let me know your thoughts on this.

Thanks.

!! Vishwa !!

Re: Discussion on GPIO monitoring and design thoughts.

[Andrew Jeffery]

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

Hi Vishwa,

I have some queries below. Apologies if they appear curt, but I'm
trying to cut to the point in each case.

On Wed, 2017-04-05 at 13:56 +0530, vishwa wrote:
> We had a technical discussion on HOWTO and what is the right thing to do 
> handling the GPIO assertion and here is the gist.
> 
> 1) Use gpio-keys and utilize libevdev wrappers ( 
> https://www.freedesktop.org/software/libevdev/doc/latest/ ) to watch for 
> data in /dev/input/event.

Okay, I'd like you to step back and detail what problem you're actually
trying to solve here. Going from wanting to handle GPIO interrupts to
using gpio-keys everywhere needs some explanation. We have at least two
other interfaces that would appear to be sufficient: GPIO chardev and
sysfs.

What you've described reads to me like a case of the X-Y problem. This
link is rather blunt, but it illustrates the issue: 

http://mywiki.wooledge.org/XyProblem

> 
> 2) Define a target with set of services that would be called into by the 
> GPIO monitor application when the GPIO state changes to what is expected.
> 
>   - 2.1) Another option was to just broadcast a signal that interested 
> parties could catch and act

I assume this is on D-Bus?

>  but then it was not chosen because there 
> would be no guarantee that the signal would ever be delivered / caught 
> by needed parties.

Isn't this what systemd is for?

Isn't this what you gain by decoupling the monitoring from the
handling? For instance if we're talking in abstract terms, the current
system policy might be to not care about the state change.

> 
>   - 2.2 ) Also going the signal way demands a service that just needs to 
> watch for that.

Is that the case? Can't we have one service handling multiple signals?
Whether that's a good idea is another question.

> 
> *) The services that are part of the target will then own complete 
> handling of the condition instead of the application that monitored the 
> GPIO.

Why are we decoupling the handler services from the GPIO? What is the
advantage?

The disadvantage is the behaviour of the system is much harder to
understand. If we go this route, what tooling will be in place to debug
failures of interactions between components?

I would argue that this is already a significant hurdle in
understanding integration failures on the BMC, and I would prefer not
to add to the problems.

> 
> ---------
> 
> Here is an example of application that handles the GPIO assertion that 
> happens when host does a checkstop.
> 
> 1) Application's arguments need /dev/input/event<$number> and the 
> expected GPIO state as indicated by linux,code in :
> 
> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/gpio-keys.txt

There is a downside in that you will be creating a kernel ABI by
mapping GPIO events to a keycode. This choice appears to me to be
arbitrary, and from the kernel's perspective we don't know how
userspace is going to treat the value so we lose the flexibility to
change it if it's not. This isn't a show-stopper, but it sure feels
like a wart that doesn't need to exist.

> 
> 2) Call this app after reaching `Host-Running` state thus avoiding any 
> checks to see current state.
> 
> 2) App will then use libevdev wrappers to watch for the state

It bothers me that we'd be invoking libevdev to catch an interrupt on a
GPIO. It seems unnecessarily complex in general.

> 
> 3) On seeing the expected state change, app calls into the configured 
> target.
> 
> 4) Configured target right now has 2 services.
> 
>    - 4.1 ) Service that Creates an errorlog indicating the checkstop
>    - 4.2 ) Service that will start the Quiesce target
> 
> --------
> 
> This design direction helps decouple applications that monitor and 
> analyze the conditions.

Sure. But you need to describe the advantage of decoupling these
concepts. Why do we need the abstraction? I need something more than
"decoupling is good", because as described above it can lead to a lot
of overhead trying to track down bugs. Sometimes it's a balancing act,
and we need to understand what we're trading off.

Cheers,

Andrew

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

Re: Discussion on GPIO monitoring and design thoughts.

[vishwa]

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

Hi Andrew,

Thanks for the feedback. I have responded inline.

On 04/07/2017 05:44 AM, Andrew Jeffery wrote:
> Hi Vishwa,
>
> I have some queries below. Apologies if they appear curt, but I'm
> trying to cut to the point in each case.
>
> On Wed, 2017-04-05 at 13:56 +0530, vishwa wrote:
>> We had a technical discussion on HOWTO and what is the right thing to do
>> handling the GPIO assertion and here is the gist.
>>
>> 1) Use gpio-keys and utilize libevdev wrappers (
>> https://www.freedesktop.org/software/libevdev/doc/latest/ ) to watch for
>> data in /dev/input/event.
> Okay, I'd like you to step back and detail what problem you're actually
> trying to solve here. Going from wanting to handle GPIO interrupts to
> using gpio-keys everywhere needs some explanation. We have at least two
> other interfaces that would appear to be sufficient: GPIO chardev and
> sysfs.

Sure. I realize I should have put the use-case here than at the end.

Problem: When the host check-stops, we need a way to detect that.
The existing mechanism will assert a configured GPIO when that happens.
Firmware code needs a way of knowing that it has happened and that is 
why this application.

I was told that /sysfs is being deprecated and that we use the chardev. 
We had 2 options then.

1/. Use the gpio-keys
2/. Use libgpio ( the support is not in yet in 4.7 per the discussion )

Brad Bishop recommended that gpio-keys be used over libgpio due to the 
debouncing support built into gpio-keys implementation.
Since I am not an expert in this area, I suggest you please talk to Brad 
if you see there are better ideas.
> What you've described reads to me like a case of the X-Y problem. This
> link is rather blunt, but it illustrates the issue:
>
> http://mywiki.wooledge.org/XyProblem

This link is really hilarious ( yeah ), Thanks for this :)

>> 2) Define a target with set of services that would be called into by the
>> GPIO monitor application when the GPIO state changes to what is expected.
>>
>>    - 2.1) Another option was to just broadcast a signal that interested
>> parties could catch and act
> I assume this is on D-Bus?

Correct.

>
>>   but then it was not chosen because there
>> would be no guarantee that the signal would ever be delivered / caught
>> by needed parties.
> Isn't this what systemd is for?

It was Patrick's concern on using signals. But I tend to agree with him 
too. Although I understand systemd is already doing a good job here with 
signals.
>
> Isn't this what you gain by decoupling the monitoring from the
> handling? For instance if we're talking in abstract terms, the current
> system policy might be to not care about the state change.

Yeah. This is a valid one. We shall talk about it.

>>    - 2.2 ) Also going the signal way demands a service that just needs to
>> watch for that.
> Is that the case? Can't we have one service handling multiple signals?
> Whether that's a good idea is another question.
>

I don't like the idea of one jumbo application monitoring all signals. 
Whoever that has a need, let them wait.

>> *) The services that are part of the target will then own complete
>> handling of the condition instead of the application that monitored the
>> GPIO.
> Why are we decoupling the handler services from the GPIO? What is the
> advantage?
>
> The disadvantage is the behaviour of the system is much harder to
> understand. If we go this route, what tooling will be in place to debug
> failures of interactions between components?
>
> I would argue that this is already a significant hurdle in
> understanding integration failures on the BMC, and I would prefer not
> to add to the problems.
>

I am not following the argument here. Why is the behavior harder to 
understand when you have defined systemd targets doing what they are
required to do. Its the job of applications that get into that target to 
do the right thing of collecting what is necessary to understand
one specific problem.
>> ---------
>>
>> Here is an example of application that handles the GPIO assertion that
>> happens when host does a checkstop.
>>
>> 1) Application's arguments need /dev/input/event<$number> and the
>> expected GPIO state as indicated by linux,code in :
>>
>> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/gpio-keys.txt
> There is a downside in that you will be creating a kernel ABI by
> mapping GPIO events to a keycode. This choice appears to me to be
> arbitrary, and from the kernel's perspective we don't know how
> userspace is going to treat the value so we lose the flexibility to
> change it if it's not. This isn't a show-stopper, but it sure feels
> like a wart that doesn't need to exist.
>

Is it not expected that the binding always pass the input_event struct 
?. What kind of changes are you thinking ?

Are you saying for instance you will change these values and 
applications may go crazy ?

|#define KEY_UP 103 ||#define KEY_DOWN 108 |


>> 2) Call this app after reaching `Host-Running` state thus avoiding any
>> checks to see current state.
>>
>> 2) App will then use libevdev wrappers to watch for the state
> It bothers me that we'd be invoking libevdev to catch an interrupt on a
> GPIO. It seems unnecessarily complex in general.

I need Brad Bishop help here.

>
>> 3) On seeing the expected state change, app calls into the configured
>> target.
>>
>> 4) Configured target right now has 2 services.
>>
>>     - 4.1 ) Service that Creates an errorlog indicating the checkstop
>>     - 4.2 ) Service that will start the Quiesce target
>>
>> --------
>>
>> This design direction helps decouple applications that monitor and
>> analyze the conditions.
> Sure. But you need to describe the advantage of decoupling these
> concepts. Why do we need the abstraction? I need something more than
> "decoupling is good", because as described above it can lead to a lot
> of overhead trying to track down bugs. Sometimes it's a balancing act,
> and we need to understand what we're trading off.

Sure. The advantage of decoupling is that you can use this gpio monitor 
in any implementation and offloading handling the errors to 
implementation specific targets than consuming implementation specific 
complexity into this code and thus making it specific to some platforms.

> Cheers,
>
> Andrew

Hope I answered. Thank you.

!! Vishwa !!

[-- Attachment #2: Type: text/html, Size: 9337 bytes --]

Re: Discussion on GPIO monitoring and design thoughts.

[Andrew Jeffery]

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

On Fri, 2017-04-07 at 12:11 +0530, vishwa wrote:
> > On Wed, 2017-04-05 at 13:56 +0530, vishwa wrote:
> > > We had a technical discussion on HOWTO and what is the right thing to do 
> > > handling the GPIO assertion and here is the gist.
> > > 
> > > 1) Use gpio-keys and utilize libevdev wrappers ( 
> > > https://www.freedesktop.org/software/libevdev/doc/latest/ ) to watch for 
> > > data in /dev/input/event.
> > 
> > Okay, I'd like you to step back and detail what problem you're actually
> > trying to solve here. Going from wanting to handle GPIO interrupts to
> > using gpio-keys everywhere needs some explanation. We have at least two
> > other interfaces that would appear to be sufficient: GPIO chardev and
> > sysfs.
>  
> Sure. I realize I should have put the use-case here than at the end.
> 
> Problem: When the host check-stops, we need a way to detect that. 
> The existing mechanism will assert a configured GPIO when that happens.
> Firmware code needs a way of knowing that it has happened and that is why this application.
> 
> I was told that /sysfs is being deprecated and that we use the chardev. We had 2 options then.
> 
> 1/. Use the gpio-keys 
> 2/. Use libgpio ( the support is not in yet in 4.7 per the discussion )
> 
> Brad Bishop recommended that gpio-keys be used over libgpio due to the debouncing support built into gpio-keys implementation.
> Since I am not an expert in this area, I suggest you please talk to Brad if you see there are better ideas.

I've since had a chat with Brad and Vishwa. There's currently no
perfect fit solution to the problem, so given:

1. We're only dealing with input GPIOs
2. gpio-keys enables the description of debounce periods (through
devicetree)
3. At least *some* GPIOs need debouncing

I'm okay if we proceed this way.

Considering the current state of the kernel and alternatives to the
above, if 1. weren't the case then we would have a weird split between
/dev/input and either the GPIO chardev interface or sysfs, so given 1.
we've at least got consistency. Similarly for 2. and 3. we will
probably run into issues if we split input GPIOs between gpio-keys and
the alternative interfaces, as we may in the future find we need to
enable debouncing on a GPIO which would trigger user-space breaking
migration from one approach to the other.

We do have the downside of assigning keycodes to GPIOs. This could be
an arbitrary (e.g. linear, as-needed) assignment, or we could do
something like set the keycode to the GPIO line's index (e.g. GPIOA0 =
0, GPIOZ7 = 215, etc). This would at least give some meaning to the
values and so we hopefully have no reason to change them in the future.
I don't think there are any limitations to the keycode value we can use
(e.g. max 255), so even if the Aspeed chips expand the number of
available GPIOs in the future we should be okay.

As to why we can't configure debounce periods via say the chardev
interface, this is something I will ask upstream about.

> > 
> > > *) The services that are part of the target will then own complete 
> > > handling of the condition instead of the application that monitored the 
> > > GPIO.
> > 
> > Why are we decoupling the handler services from the GPIO? What is the
> > advantage?
> > 
> > The disadvantage is the behaviour of the system is much harder to
> > understand. If we go this route, what tooling will be in place to debug
> > failures of interactions between components?
> > 
> > I would argue that this is already a significant hurdle in
> > understanding integration failures on the BMC, and I would prefer not
> > to add to the problems.
> > 
>  
> I am not following the argument here. Why is the behavior harder to understand when you have defined systemd targets doing what they are
> required to do. Its the job of applications that get into that target to do the right thing of collecting what is necessary to understand
> one specific problem.

I apologise, I had a misunderstand stemming from your description in
2.1. Sorry for that - you weren't planning on doing what I thought.

> > > ---------
> > > 
> > > Here is an example of application that handles the GPIO assertion that 
> > > happens when host does a checkstop.
> > > 
> > > 1) Application's arguments need /dev/input/event<$number> and the 
> > > expected GPIO state as indicated by linux,code in :
> > > 
> > > http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/gpio-keys.txt
> > 
> > There is a downside in that you will be creating a kernel ABI by
> > mapping GPIO events to a keycode. This choice appears to me to be
> > arbitrary, and from the kernel's perspective we don't know how
> > userspace is going to treat the value so we lose the flexibility to
> > change it if it's not. This isn't a show-stopper, but it sure feels
> > like a wart that doesn't need to exist.
> > 
>  
> Is it not expected that the binding always pass the input_event struct ?. What kind of changes are you thinking ?
> 
> Are you saying for instance you will change these values and applications may go crazy ?
> 
> > #define KEY_UP			103
> #define KEY_DOWN		108

As you pointed out we need to specify these values in your gpio-keys
subnode via the linux,code property. We mustn't assume anything about
how userspace behaves, e.g. it may very well care about these values
and could react negatively if the value were changed.

You mention above /dev/input/event<$number> - were you planning on
having one or multiple gpio-keys entries in the devicetree? If
multiple, what was your planned strategy for grouping the keys into
gpio-keys nodes?

One approach would be to have one gpio-keys node for each GPIO line.
This would enable userspace not to care about what value was assigned
to linux,code at the cost of knowing *which* /dev/input/event* file to
open. I'm not suggesting this is a good idea (I don't think it is), but
it is possible. It also doesn't change the fact that userspace may
still care about the value regardless. Further, knowing which device to
open may still be a problem anyway if someone plugs in say a USB
keyboard. We probably need more investigation here.

Cheers,

Andrew

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

Re: Discussion on GPIO monitoring and design thoughts.

[Kun Yi]

What about applications that need to monitor and set GPIOs rather than
monitoring them only? gpio-keys won't be suitable, and new cdev
interface doesn't support writing to GPIOs yet.
Also could someone point me to 'libgpio' interfaces mentioned here?
I'm interested whether it provides a good read/write/interrupt trigger
etc. interface.

Thanks,
Kun

On Sun, Apr 9, 2017 at 8:12 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Fri, 2017-04-07 at 12:11 +0530, vishwa wrote:
>> > On Wed, 2017-04-05 at 13:56 +0530, vishwa wrote:
>> > > We had a technical discussion on HOWTO and what is the right thing to do
>> > > handling the GPIO assertion and here is the gist.
>> > >
>> > > 1) Use gpio-keys and utilize libevdev wrappers (
>> > > https://www.freedesktop.org/software/libevdev/doc/latest/ ) to watch for
>> > > data in /dev/input/event.
>> >
>> > Okay, I'd like you to step back and detail what problem you're actually
>> > trying to solve here. Going from wanting to handle GPIO interrupts to
>> > using gpio-keys everywhere needs some explanation. We have at least two
>> > other interfaces that would appear to be sufficient: GPIO chardev and
>> > sysfs.
>>
>> Sure. I realize I should have put the use-case here than at the end.
>>
>> Problem: When the host check-stops, we need a way to detect that.
>> The existing mechanism will assert a configured GPIO when that happens.
>> Firmware code needs a way of knowing that it has happened and that is why this application.
>>
>> I was told that /sysfs is being deprecated and that we use the chardev. We had 2 options then.
>>
>> 1/. Use the gpio-keys
>> 2/. Use libgpio ( the support is not in yet in 4.7 per the discussion )
>>
>> Brad Bishop recommended that gpio-keys be used over libgpio due to the debouncing support built into gpio-keys implementation.
>> Since I am not an expert in this area, I suggest you please talk to Brad if you see there are better ideas.
>
> I've since had a chat with Brad and Vishwa. There's currently no
> perfect fit solution to the problem, so given:
>
> 1. We're only dealing with input GPIOs
> 2. gpio-keys enables the description of debounce periods (through
> devicetree)
> 3. At least *some* GPIOs need debouncing
>
> I'm okay if we proceed this way.
>
> Considering the current state of the kernel and alternatives to the
> above, if 1. weren't the case then we would have a weird split between
> /dev/input and either the GPIO chardev interface or sysfs, so given 1.
> we've at least got consistency. Similarly for 2. and 3. we will
> probably run into issues if we split input GPIOs between gpio-keys and
> the alternative interfaces, as we may in the future find we need to
> enable debouncing on a GPIO which would trigger user-space breaking
> migration from one approach to the other.
>
> We do have the downside of assigning keycodes to GPIOs. This could be
> an arbitrary (e.g. linear, as-needed) assignment, or we could do
> something like set the keycode to the GPIO line's index (e.g. GPIOA0 =
> 0, GPIOZ7 = 215, etc). This would at least give some meaning to the
> values and so we hopefully have no reason to change them in the future.
> I don't think there are any limitations to the keycode value we can use
> (e.g. max 255), so even if the Aspeed chips expand the number of
> available GPIOs in the future we should be okay.
>
> As to why we can't configure debounce periods via say the chardev
> interface, this is something I will ask upstream about.
>
>> >
>> > > *) The services that are part of the target will then own complete
>> > > handling of the condition instead of the application that monitored the
>> > > GPIO.
>> >
>> > Why are we decoupling the handler services from the GPIO? What is the
>> > advantage?
>> >
>> > The disadvantage is the behaviour of the system is much harder to
>> > understand. If we go this route, what tooling will be in place to debug
>> > failures of interactions between components?
>> >
>> > I would argue that this is already a significant hurdle in
>> > understanding integration failures on the BMC, and I would prefer not
>> > to add to the problems.
>> >
>>
>> I am not following the argument here. Why is the behavior harder to understand when you have defined systemd targets doing what they are
>> required to do. Its the job of applications that get into that target to do the right thing of collecting what is necessary to understand
>> one specific problem.
>
> I apologise, I had a misunderstand stemming from your description in
> 2.1. Sorry for that - you weren't planning on doing what I thought.
>
>> > > ---------
>> > >
>> > > Here is an example of application that handles the GPIO assertion that
>> > > happens when host does a checkstop.
>> > >
>> > > 1) Application's arguments need /dev/input/event<$number> and the
>> > > expected GPIO state as indicated by linux,code in :
>> > >
>> > > http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/gpio-keys.txt
>> >
>> > There is a downside in that you will be creating a kernel ABI by
>> > mapping GPIO events to a keycode. This choice appears to me to be
>> > arbitrary, and from the kernel's perspective we don't know how
>> > userspace is going to treat the value so we lose the flexibility to
>> > change it if it's not. This isn't a show-stopper, but it sure feels
>> > like a wart that doesn't need to exist.
>> >
>>
>> Is it not expected that the binding always pass the input_event struct ?. What kind of changes are you thinking ?
>>
>> Are you saying for instance you will change these values and applications may go crazy ?
>>
>> > #define KEY_UP                      103
>> #define KEY_DOWN              108
>
> As you pointed out we need to specify these values in your gpio-keys
> subnode via the linux,code property. We mustn't assume anything about
> how userspace behaves, e.g. it may very well care about these values
> and could react negatively if the value were changed.
>
> You mention above /dev/input/event<$number> - were you planning on
> having one or multiple gpio-keys entries in the devicetree? If
> multiple, what was your planned strategy for grouping the keys into
> gpio-keys nodes?
>
> One approach would be to have one gpio-keys node for each GPIO line.
> This would enable userspace not to care about what value was assigned
> to linux,code at the cost of knowing *which* /dev/input/event* file to
> open. I'm not suggesting this is a good idea (I don't think it is), but
> it is possible. It also doesn't change the fact that userspace may
> still care about the value regardless. Further, knowing which device to
> open may still be a problem anyway if someone plugs in say a USB
> keyboard. We probably need more investigation here.
>
> Cheers,
>
> Andrew



-- 
Regards,
Kun

Re: Discussion on GPIO monitoring and design thoughts.

[Andrew Jeffery]

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

On Thu, 2017-04-13 at 16:24 -0700, Kun Yi wrote:
> What about applications that need to monitor and set GPIOs rather than
> monitoring them only?

The proposed approach isn't great for that as we have the inconsistent
interfaces, but we do have functional debouncing. Ideally we would be
able to set GPIO pin configurations through the chardev interface, but
we can't do that (yet).

>  gpio-keys won't be suitable, and new cdev
> interface doesn't support writing to GPIOs yet.

Writing GPIOs through the chardev interface is supported as of v4.8-
rc1, so moving to 4.10 will enable it.

> Also could someone point me to 'libgpio' interfaces mentioned here?

From my conversations with Vishwa what he meant here was the chardev
interface, and there is no actual 'libgpio'.

Cheers,

Andrew

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