Skip to contentSkip to conversation

gpio_keys: fix gpio key driver to proper support GIC interrupt

PATCH New Y Vo 5 participants 8 messages Jun 4, 2015 – Jun 10, 2015

Subsystems (3)
lore
Share & tools
lore

Y Vo

author
#0 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

---
 drivers/input/keyboard/gpio_keys.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)
drivers/input/keyboard/gpio_keys.c +17 -3
index ddf4045..7c3da2c 100644
@@ -532,9 +532,23 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	error = devm_request_any_context_irq(&pdev->dev, bdata->irq,
 					     isr, irqflags, desc, bdata);
 	if (error < 0) {
-		dev_err(dev, "Unable to claim irq %d; error %d\n",
-			bdata->irq, error);
-		return error;
+		/*
+		 * Attempt to request again with only rising edge trigger
+		 */
+		if ((irqflags & (IRQ_TYPE_EDGE_RISING | IRQF_TRIGGER_FALLING))) {
+			irqflags = IRQ_TYPE_EDGE_RISING;
+			error = devm_request_any_context_irq(&pdev->dev, bdata->irq,
+							     isr, irqflags, desc, bdata);
+			if (error < 0) {
+				dev_err(dev, "Unable to claim irq %d; error %d\n",
+					bdata->irq, error);
+				return error;
+			}
+		} else {
+			dev_err(dev, "Unable to claim irq %d; error %d\n",
+				bdata->irq, error);
+			return error;
+		}
 	}
 
 	return 0;
Signature · 1 line
1.7.1
maintainer
#1 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

Hi Y,

Quoted 5 lines · Y Vo

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

How do we get notified of button release if we only get interrupt on rising edge?

Thanks.

Signature · 1 line
Dmitry
#2 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

Quoted 10 lines · Dmitry Torokhov

Hi Y,

Quoted 5 lines · Y Vo

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

How do we get notified of button release if we only get interrupt on rising edge?

On the arm gic, only the rising edge is supported. So key press can only be defined
as rising edge and no key release.

Quoted 5 lines

Thanks.

--
Dmitry

maintainer
#3 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

Quoted 10 lines · Feng Kan
Quoted 8 lines · Dmitry Torokhov

Hi Y,

Quoted 5 lines · Y Vo

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

Quoted 6 lines
Quoted 2 lines

How do we get notified of button release if we only get interrupt on rising edge?

On the arm gic, only the rising edge is supported. So key press can only be defined
as rising edge and no key release.

Then it is not much of a key if we can't signal release.

gpio_keys
driver supports what it calls "interrupt only" mode where there is no GPIO and we go just by the fact that we received interrupt. In this mode the trigger type is expected to be set by the platform code (or OF), the driver does not force need of both edges.

Thanks.

Signature · 1 line
Dmitry
#4 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

In reply to Y Vo
Quoted 7 lines · On Thursday 04 June 2015 14:25:12 Y Vo wrote:

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

I think you want to use an 'interrupts' property instead of a 'gpios' property (or possibly both), so you can pass the right polarity.

Arnd

author
#5 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

Quoted 11 lines · Arnd Bergmann
Quoted 7 lines · On Thursday 04 June 2015 14:25:12 Y Vo wrote:

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

I think you want to use an 'interrupts' property instead of a 'gpios' property (or possibly both), so you can pass the right polarity.

        gpio-keys {
                compatible = "gpio-keys";
                apm_ctrl_name = "Power Button";
                btn2 {
                        label = "EXT_INT2";
                        gpios = <&sbgpio 13 0x0>;
                        linux,code = <116>;
                        linux,input-type = <0x1>;     /* EV_KEY */
                };
        };

         sbgpio: sbgpio at 17001000{
                        compatible = "apm,xgene-gpio-sb";
                        reg = <0x0 0x17001000 0x0 0x400>;
                        #gpio-cells = <2>;
                        gpio-controller;
                        interrupts =    <0x0 0x28 0x1>,
                                        <0x0 0x29 0x1>,
                                        <0x0 0x2a 0x1>,
                                        <0x0 0x2b 0x1>,
                                        <0x0 0x2c 0x1>,
                                        <0x0 0x2d 0x1>;
                };

I can change the polarity of interrupt in the sbgpio node, but the issue here is the gpio_key driver always set interrupt type to (irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING). And the GIC driver only support edge rising or level high. So that's our issue.

Quoted 2 lines

Arnd

#6 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

Quoted 36 lines · On Monday 08 June 2015 09:48:29 Y Vo wrote:
Quoted 11 lines · Arnd Bergmann
Quoted 7 lines · On Thursday 04 June 2015 14:25:12 Y Vo wrote:

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

I think you want to use an 'interrupts' property instead of a 'gpios' property (or possibly both), so you can pass the right polarity.

        gpio-keys {
                compatible = "gpio-keys";
                apm_ctrl_name = "Power Button";
                btn2 {
                        label = "EXT_INT2";
                        gpios = <&sbgpio 13 0x0>;
                        linux,code = <116>;
                        linux,input-type = <0x1>;     /* EV_KEY */
                };
        };

         sbgpio: sbgpio at 17001000{
                        compatible = "apm,xgene-gpio-sb";
                        reg = <0x0 0x17001000 0x0 0x400>;
                        #gpio-cells = <2>;
                        gpio-controller;
                        interrupts =    <0x0 0x28 0x1>,
                                        <0x0 0x29 0x1>,
                                        <0x0 0x2a 0x1>,
                                        <0x0 0x2b 0x1>,
                                        <0x0 0x2c 0x1>,
                                        <0x0 0x2d 0x1>;
                };
Quoted 4 lines

I can change the polarity of interrupt in the sbgpio node, but the issue here is the gpio_key driver always set interrupt type to (irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING). And the GIC driver only support edge rising or level high. So that's our issue.

No, please do as I wrote, and use an interrupts property in the gpio-keys node.

Arnd

#7 lore raw

PATCH 1/1 gpio_keys: fix gpio key driver to proper support GIC interrupt

In reply to Y Vo

Hello,

Quoted 5 lines · Y Vo

GIC is designed to support two of trigger mechanisms - active level high or edge rising. But in the gpio_keys driver, it tries to use both edge rising and edge falling trigger. This patch fixes the gpio_keys driver to request only the edge rising event when failed to configure the interrupt.

Is it possible to fix that in the GIC driver? I.e. let it switch the trigger direction to falling when it fired for raising?

This would require to read out the current level, not sure this is possible?!

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |