[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

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

Signed-off-by: Y Vo <yvo@apm.com>
---
 drivers/input/keyboard/gpio_keys.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index ddf4045..7c3da2c 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -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;
-- 
1.7.1

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

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

Signed-off-by: Y Vo <yvo@apm.com>
---
 drivers/input/keyboard/gpio_keys.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index ddf4045..7c3da2c 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -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;
-- 
1.7.1

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Dmitry Torokhov]

Hi Y,
On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.

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

Thanks.

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

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Dmitry Torokhov]

Hi Y,
On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.

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

Thanks.

-- 
Dmitry

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Feng Kan]

On Thu, Jun 4, 2015 at 10:23 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Y,
> On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
>
> 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.
>
> Thanks.
>
> --
> Dmitry

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Feng Kan]

On Thu, Jun 4, 2015 at 10:23 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Y,
> On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
>
> 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.
>
> Thanks.
>
> --
> Dmitry

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Dmitry Torokhov]

On Thu, Jun 04, 2015 at 08:37:50PM -0700, Feng Kan wrote:
> On Thu, Jun 4, 2015 at 10:23 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Y,
> > On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
> >
> > 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.

-- 
Dmitry

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Dmitry Torokhov]

On Thu, Jun 04, 2015 at 08:37:50PM -0700, Feng Kan wrote:
> On Thu, Jun 4, 2015 at 10:23 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Y,
> > On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
> >
> > 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.

-- 
Dmitry

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Arnd Bergmann]

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.
> 
> Signed-off-by: Y Vo <yvo@apm.com>

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

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Arnd Bergmann]

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.
> 
> Signed-off-by: Y Vo <yvo@apm.com>

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

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Y Vo]

On Fri, Jun 5, 2015 at 7:50 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.
>>
>> Signed-off-by: Y Vo <yvo@apm.com>
>
> 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@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.

>
>         Arnd

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Y Vo]

On Fri, Jun 5, 2015 at 7:50 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.
>>
>> Signed-off-by: Y Vo <yvo@apm.com>
>
> 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.

>
>         Arnd

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Uwe Kleine-König]

Hello,

On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Uwe Kleine-König]

Hello,

On Thu, Jun 04, 2015 at 02:25:12PM +0700, 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.
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

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

Re: [PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Arnd Bergmann]

On Monday 08 June 2015 09:48:29 Y Vo wrote:
> On Fri, Jun 5, 2015 at 7:50 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 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.
> >>
> >> Signed-off-by: Y Vo <yvo@apm.com>
> >
> > 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@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.

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

	Arnd

[PATCH v0] gpio_keys: fix gpio key driver to proper support GIC interrupt

[Arnd Bergmann]

On Monday 08 June 2015 09:48:29 Y Vo wrote:
> On Fri, Jun 5, 2015 at 7:50 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 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.
> >>
> >> Signed-off-by: Y Vo <yvo@apm.com>
> >
> > 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.

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

	Arnd