On Mon, Nov 23, 2009 at 07:50:20PM +0100, ext Ferenc Wagner wrote:
> Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> writes:
>
> > On Mon, Nov 23, 2009 at 05:42:08PM +0100, Ferenc Wagner wrote:
> >> Mika Westerberg <ext-mika.1.westerberg@xxxxxxxxx> writes:
> >>
> >>> Added new field to struct gpio_keys_button: irqflags which can
> >>> be used to specify exact irqflags the platform wants. If not specified,
> >>> it defaults to: IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING.
> >>>
> >>> --- a/drivers/input/keyboard/gpio_keys.c
> >>> +++ b/drivers/input/keyboard/gpio_keys.c
> >>> @@ -106,10 +107,14 @@ static int __devinit gpio_keys_setup_key(struct device *dev,
> >>> goto fail3;
> >>> }
> >>>
> >>> - error = request_irq(irq, gpio_keys_isr,
> >>> - IRQF_SHARED |
> >>> - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> >>> - desc, bdata);
> >>> + if (button->irqflags == 0) {
> >>> + irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
> >>> + IRQF_SHARED;
> >>> + } else {
> >>> + irqflags = button->irqflags;
> >>> + }
> >>> +
> >>> + error = request_irq(irq, gpio_keys_isr, irqflags, desc, bdata);
> >>> if (error) {
> >>> dev_err(dev, "Unable to claim irq %d; error %d\n",
> >>> irq, error);
> >>
> >> linux/interrupt.h says:
> >>
> >> When requesting an interrupt without specifying a IRQF_TRIGGER, the
> >> setting should be assumed to be "as already configured", which may
> >> be as per machine or firmware initialisation.
> >>
> >> So I'm not sure it's a good idea to exclude 0 from the possible irqflag
> >> values.
> >
> > Hmm... OTOH we can't just require the new irq flags to be always
> > specified, that would break all current users. Do we need a flag
> > indicating that the button will be using custom IRQ flags?
>
> Or let for example -1 mean 0, which is ugly, to say the least.
How about adding (again) new field: bool custom_irqflags?
> Anyway, I'd much prefer an implementation which doesn't work against
> having multiple buttons on a single IRQ line, which is the case for
> example in arch/mips/bcm47xx/gpio.c. Then the IRQ could still be
> disabled if all its buttons are disabled, unless it's shared with
> another driver (and IRQF_SHARED would control this aspect sharing).
Do you mean something like:
if (button->custom_irqflags) {
if (button->irqflags & IRQF_SHARED) {
/* cannot mute this button */
} else {
...
}
then keeping some count of buttons that share single GPIO line and
after all buttons are disabled, only then call disable_irq() for the line?
In our case, we only have 1:1 mapping between GPIO lines and buttons so
current version works but if there are any (possible) users for this kind
of setup I can implement it.
Dmitry, what do you think?
Thanks,
MW
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
