[PATCH] Input: gpio-keys: Support for one-directional interrupts

[PATCH] Input: gpio-keys: Support for one-directional interrupts

[Cory Maccarrone]

The current gpio-keys driver assumes that each given GPIO has
the ability to respond to both rising and falling interrupts
at the same time.  For some GPIOs on certain devices, the interrupt
is only one-directional -- either rising or falling, depending on
what's being listened for.  In particular, the HTC Herald keyboard
slide switch interrupt must be switched from rising to falling and
back on each event, otherwise only one transition will be detected.

This change adds a new entry to the gpio_keys_button structure to
allow for this condition to be specified in board configurations.
When set, the driver detects the current state of the GPIO and sets
the direction of the interrupt accordingly.  Then, on each event,
the interrupt direction is switched.

Signed-off-by: Cory Maccarrone <darkstar6262@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c |   27 +++++++++++++++++++++++----
 include/linux/gpio_keys.h          |    1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 77d1309..05c599a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,7 +45,18 @@ static void gpio_keys_report_event(struct work_struct *work)
 	struct gpio_keys_button *button = bdata->button;
 	struct input_dev *input = bdata->input;
 	unsigned int type = button->type ?: EV_KEY;
-	int state = (gpio_get_value(button->gpio) ? 1 : 0) ^ button->active_low;
+	int state = gpio_get_value(button->gpio) ? 1 : 0;
+
+	/* If the hardware can't do both edges, set the appropriate
+	 * interrupt control */
+	if (button->not_both_edges) {
+		if (state)
+			set_irq_type(gpio_to_irq(button->gpio), IRQ_TYPE_EDGE_FALLING);
+		else
+			set_irq_type(gpio_to_irq(button->gpio), IRQ_TYPE_EDGE_RISING);
+	}
+
+	state ^= button->active_low;
 
 	input_event(input, type, button->code, !!state);
 	input_sync(input);
@@ -79,7 +90,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
 	struct gpio_keys_drvdata *ddata;
 	struct input_dev *input;
-	int i, error;
+	int i, error, trigger;
 	int wakeup = 0;
 
 	ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
@@ -146,9 +157,17 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 			goto fail2;
 		}
 
+		if (button->not_both_edges) {
+			if (gpio_get_value(button->gpio))
+				trigger = IRQF_TRIGGER_FALLING;
+			else
+				trigger = IRQF_TRIGGER_RISING;
+		} else {
+			trigger = IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING;
+		}
+
 		error = request_irq(irq, gpio_keys_isr,
-				    IRQF_SHARED |
-				    IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				    IRQF_SHARED | trigger,
 				    button->desc ? button->desc : "gpio_keys",
 				    bdata);
 		if (error) {
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 1289fa7..493426d 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -10,6 +10,7 @@ struct gpio_keys_button {
 	int type;		/* input event type (EV_KEY, EV_SW) */
 	int wakeup;		/* configure the button as a wake-up source */
 	int debounce_interval;	/* debounce ticks interval in msecs */
+	int not_both_edges;	/* some hardware can't do interrupts on both edges */
 };
 
 struct gpio_keys_platform_data {
-- 
1.6.3.3

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Ferenc Wagner]

Cory Maccarrone <darkstar6262@gmail.com> writes:

> The current gpio-keys driver assumes that each given GPIO has
> the ability to respond to both rising and falling interrupts
> at the same time.  For some GPIOs on certain devices, the interrupt
> is only one-directional -- either rising or falling, depending on
> what's being listened for.  In particular, the HTC Herald keyboard
> slide switch interrupt must be switched from rising to falling and
> back on each event, otherwise only one transition will be detected.

Interesting.  Btw. I'd like to convert gpio-keys from edge-triggered to
level-triggered interrupts.  Would that work for your hardware?
-- 
Regards,
Feri.

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Cory Maccarrone]

On Wed, Dec 9, 2009 at 3:32 AM, Ferenc Wagner <wferi@niif.hu> wrote:
> Cory Maccarrone <darkstar6262@gmail.com> writes:
>
> Interesting.  Btw. I'd like to convert gpio-keys from edge-triggered to
> level-triggered interrupts.  Would that work for your hardware?

I'm fairly certain it wouldn't.  Each of the interrupts on my hardware
has a corresponding bit in a control register that determines if it's
rising or falling-edge triggered -- thus the need for my patch.  To
capture both directions, it's necessary to modify the control register
when a falling edge is detected, so that the corresponding rising edge
can be collected afterward.  May I ask why you're thinking of
converting to level-triggering?

Perhaps it would be better to provide an option in the platform_device
structure to set edge- or level-triggering, similar to the change I'm
proposing for interrupts that can only signal one way at a time.

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

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Dmitry Torokhov]

On Wed, Dec 09, 2009 at 08:15:26AM -0800, Cory Maccarrone wrote:
> On Wed, Dec 9, 2009 at 3:32 AM, Ferenc Wagner <wferi@niif.hu> wrote:
> > Cory Maccarrone <darkstar6262@gmail.com> writes:
> >
> > Interesting.  Btw. I'd like to convert gpio-keys from edge-triggered to
> > level-triggered interrupts.  Would that work for your hardware?
> 
> I'm fairly certain it wouldn't.  Each of the interrupts on my hardware
> has a corresponding bit in a control register that determines if it's
> rising or falling-edge triggered -- thus the need for my patch.  To
> capture both directions, it's necessary to modify the control register
> when a falling edge is detected, so that the corresponding rising edge
> can be collected afterward.

This kind of ugliness should be hidden in irqchip driver. See
mfd/asic3.c for an example.

> May I ask why you're thinking of
> converting to level-triggering?
> 
> Perhaps it would be better to provide an option in the platform_device
> structure to set edge- or level-triggering, similar to the change I'm
> proposing for interrupts that can only signal one way at a time.
> 

Yes, we need a way fro platform code to specify desired interrupt flags
but I don't believe we should be reconfiguring them on the fly.

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

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Cory Maccarrone]

On Wed, Dec 9, 2009 at 9:58 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Dec 09, 2009 at 08:15:26AM -0800, Cory Maccarrone wrote:
>> On Wed, Dec 9, 2009 at 3:32 AM, Ferenc Wagner <wferi@niif.hu> wrote:
>>
>> I'm fairly certain it wouldn't.  Each of the interrupts on my hardware
>> has a corresponding bit in a control register that determines if it's
>> rising or falling-edge triggered -- thus the need for my patch.  To
>> capture both directions, it's necessary to modify the control register
>> when a falling edge is detected, so that the corresponding rising edge
>> can be collected afterward.
>
> This kind of ugliness should be hidden in irqchip driver. See
> mfd/asic3.c for an example.
>

I would agree with that -- it should be possible to hide that away as
part of the functionality of the interrupt driver.  Perhaps a fix to
the OMAP1 IRQ handling is warranted.  I know there are some interrupts
that can do both directions out of the box, but that shouldn't be the
responsibility of each driver to know.

>> May I ask why you're thinking of
>> converting to level-triggering?
>>
>> Perhaps it would be better to provide an option in the platform_device
>> structure to set edge- or level-triggering, similar to the change I'm
>> proposing for interrupts that can only signal one way at a time.
>>
>
> Yes, we need a way fro platform code to specify desired interrupt flags
> but I don't believe we should be reconfiguring them on the fly.
>

If the ability to handle this type of interrupt transparently was
added to our board IRQ chip driver, this whole thing would be a
non-issue, as gpio-keys could request edge triggered falling and
rising at the same time, and the driver would Just Work.

I'll take a look and see how possible that would be to do.  Looking
through the code for OMAP1's GPIO IRQ handlers, there's a note that
OMAP1 only supports edge triggering, so if gpio-keys were to move
exclusively to that, itwould stop working with those boards.  But, it
may be possible to do the trigger flip in the chip driver, which would
make this patch unnecessary.

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

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Cory Maccarrone]

On Wed, Dec 9, 2009 at 2:03 PM, Cory Maccarrone <darkstar6262@gmail.com> wrote:
> I'll take a look and see how possible that would be to do.  Looking
> through the code for OMAP1's GPIO IRQ handlers, there's a note that
> OMAP1 only supports edge triggering, so if gpio-keys were to move
> exclusively to that, itwould stop working with those boards.  But, it
> may be possible to do the trigger flip in the chip driver, which would
> make this patch unnecessary.

I've just submitted a patch to linux-omap for this against the
gpio/irq handling code. It implements the ICR flip in there
automatically whenever both RISING and FALLING edge are requested of
one of these gpios.  With it, the patch I've submitted here is moot.
We'll see if it gets accepted.

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

Re: [PATCH] Input: gpio-keys: Support for one-directional interrupts

[Cory Maccarrone]

On Thu, Dec 10, 2009 at 8:03 PM, Cory Maccarrone <darkstar6262@gmail.com> wrote:
> I've just submitted a patch to linux-omap for this against the
> gpio/irq handling code. It implements the ICR flip in there
> automatically whenever both RISING and FALLING edge are requested of
> one of these gpios.  With it, the patch I've submitted here is moot.
> We'll see if it gets accepted.
>

OK, looks like the patch will get approved, so I'm dropping this patch.  Thanks!

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