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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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?

Would be nice, together with WARN() in case it needed stanitizing.

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

I am not sure if addin a pure polling mode to this driver is a best
idea. Maybe writing a separate one based on input-polldev will allow
keep them both simpler than combined one.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html