Uwe Kleine-König | 3 Jul 09:25

[PATCH] [RFC] gpio-keys: let platform code specify the trigger type

The intend for this change is that not all platform's irqs support
triggering on both edges.  Examples are ns9xxx[1] and txx9[2], and I
expect that there are more.

Provided that the platform data is initialized with zeros there is no
change in behavior if the new struct member 'trigger' isn't set in
platform code.

open points:
 - if only one trigger direction is used it should match active_low such
   that the button press generates the irq.
 - poll for button release instead of generate the release event
   directly after the press?
 - is it correct to input_sync() between press and release event?
 - sanitize button->trigger &= IRQF_TRIGGER_EDGE in gpio_keys_probe
   before passing it to request_irq?
 - a comment describing the trigger member of struct gpio_keys_button

I'd like to have polling support in this driver.  This could use
button->trigger == 0, so it might be sensible to add a
WARN_ON(!button->trigger) for now and wait some time before implementing
it.

[There is no MAINTAINER entry for gpio-keys, so I Cc: the last
contributors.] 

[1] The code to show that isn't in vanilla yet.  Basic machine support
    is in arch/arm/mach-ns9xxx.
[2] see txx9_irq_set_type in arch/mips/kernel/irq_txx9.c

(Continue reading)

Uwe Kleine-König | 10 Jul 10:58

[PATCH RESENT] [RFC] gpio-keys: let platform code specify the trigger type

The intend for this change is that not all platform's irqs support
triggering on both edges.  Examples are ns9xxx[1] and txx9[2], and I
expect that there are more.

Provided that the platform data is initialized with zeros there is no
change in behavior if the new struct member 'trigger' isn't set in
platform code.

open points:
 - if only one trigger direction is used it should match active_low such
   that the button press generates the irq.
 - poll for button release instead of generate the release event
   directly after the press?
 - is it correct to input_sync() between press and release event?
 - sanitize button->trigger &= IRQF_TRIGGER_EDGE in gpio_keys_probe
   before passing it to request_irq?
 - a comment describing the trigger member of struct gpio_keys_button

I'd like to have polling support in this driver.  This could use
button->trigger == 0, so it might be sensible to add a
WARN_ON(!button->trigger) for now and wait some time before implementing
it.

[There is no MAINTAINER entry for gpio-keys, so I Cc: the last
contributors.]

[1] The code to show that isn't in vanilla yet.  Basic machine support
    is in arch/arm/mach-ns9xxx.
[2] see txx9_irq_set_type in arch/mips/kernel/irq_txx9.c

(Continue reading)

Dmitry Torokhov | 10 Jul 15:54

Re: [PATCH RESENT] [RFC] gpio-keys: let platform code specify the trigger type

Hi Uwe,

On Thu, Jul 10, 2008 at 10:58:44AM +0200, Uwe Kleine-König wrote:
> The intend for this change is that not all platform's irqs support
> triggering on both edges.  Examples are ns9xxx[1] and txx9[2], and I
> expect that there are more.
> 
> Provided that the platform data is initialized with zeros there is no
> change in behavior if the new struct member 'trigger' isn't set in
> platform code.
> 

OK, makes sense. Unfortunately the patch conflicts with debounce support
that is in 'next' branch of my tree so I can't apply it as is. Actually,
with debounce support is may not even be needed in the present form.

> open points:
>  - if only one trigger direction is used it should match active_low such
>    that the button press generates the irq.
>  - poll for button release instead of generate the release event
>    directly after the press?

Yes, I think that would be the best option.

>  - is it correct to input_sync() between press and release event?
Yes, button press and release are 2 distinct states of the device and so
it is prudent to have input_sync in between.

>  - sanitize button->trigger &= IRQF_TRIGGER_EDGE in gpio_keys_probe
>    before passing it to request_irq?
(Continue reading)

Uwe Kleine-König | 14 Jul 07:33

Re: [PATCH RESENT] [RFC] gpio-keys: let platform code specify the trigger type

Hello Dmitry,

Dmitry Torokhov wrote:
> On Thu, Jul 10, 2008 at 10:58:44AM +0200, Uwe Kleine-König wrote:
> > The intend for this change is that not all platform's irqs support
> > triggering on both edges.  Examples are ns9xxx[1] and txx9[2], and I
> > expect that there are more.
> >
> > Provided that the platform data is initialized with zeros there is no
> > change in behavior if the new struct member 'trigger' isn't set in
> > platform code.
> >
> 
> OK, makes sense. Unfortunately the patch conflicts with debounce support
> that is in 'next' branch of my tree so I can't apply it as is. Actually,
> with debounce support is may not even be needed in the present form.
So I will resend an updated version when the details are resolved.

> > open points:
> >  - if only one trigger direction is used it should match active_low such
> >    that the button press generates the irq.
> >  - poll for button release instead of generate the release event
> >    directly after the press?
> 
> Yes, I think that would be the best option.
OK.

> >  - is it correct to input_sync() between press and release event?
> Yes, button press and release are 2 distinct states of the device and so
> it is prudent to have input_sync in between.
(Continue reading)


Gmane