On Tue, Nov 24, 2009 at 12:05:57PM +0100, ext Ferenc Wagner wrote:
[snip]

quoted

quoted

Or let for example -1 mean 0, which is ugly, to say the least.

How about adding (again) new field: bool custom_irqflags?

Thinking about it, since multiple buttons can share a single IRQ, this
can't be a per-button field after all.  Perhaps a better solution would
be adding a new mapping to the platform data:

struct gpio_keys_irq {
        int irq;
        int irqflags;
        int wakeup;                 /* replace the per-button field */
}

struct gpio_keys_platform_data {
        struct gpio_keys_button *buttons;
        int nbuttons;
        unsigned int rep:1;
        struct gpio_keys_irq *irqs; /* new member */
        int nirqs;                  /* new member */
};

OK. This seems reasonable.

quoted

quoted

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

Not quite, because the default irqflags contain IRQF_SHARED.  More like:

disable_button (this);
if (button->custom_irqflags && !(button->irqflags & IRQF_SHARED)
    && (all buttons on this IRQ are disabled)) {
        disable_irq (gpio_to_irq (this));
}

I actually meant this code snippet to be in the function gpio_keys_setup_key()
where it only checks whether button can be disabled. Sorry for forgotting
to mention the context.

quoted

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.

I quoted (and own) a platform with such arrangement.  I'm somewhat short
on time to work on this hobby project, but of course I don't expect you
to do the heavy lifting.  Unfortunately I haven't yet managed to get a
clear picture about how to properly attack it (the problem is GPIO
access from the IRQ handler, if you didn't read the thread), so I
couldn't produce acceptable code (even though in my case gpio_get_value
can't sleep, so I've got working code).  Meanwhile, I'm trying to do my
best to not let things go in a direction which makes this change harder
that it currently is.  I'm really sorry this hurts your efforts.

Thanks for clarifying things. It really don't hurt my efforts; I get
paid for doing this :)

So I'll try to implement this gpio-keys muting so that it allows
also multiple buttons to share single IRQ line.

Thanks,
MW