[PATCH 0/2] gpio-mxc: Fix issue with gpios used as wake source on iMX27

[PATCH 0/2] gpio-mxc: Fix issue with gpios used as wake source on iMX27

[Philipp Rosenberger]

The gpio-mxc driver is able to use two interrupts per gpio bank. One for
the lower 16 and the other for the higher 16 gpios. The iMX27 has only one
interrupt per bank. An error in the driver leads to the problem that the
driver uses an invalid interrupt for the higher 16 gpios. Which in turn
prevents the upper 16 gpios per bank to be used as wake source.

The first patch fixes the assignment for the irq_high. The second patch
adds proper return values. Which should make finding any problems in the
future much easier.

Philipp Rosenberger (2):
  gpio: gpio-mxc: Fix: higher 16 GPIOs usable as wake source
  gpio: gpio-mxc: gpio_set_wake_irq() use proper return values

 drivers/gpio/gpio-mxc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

-- 
2.1.4

[PATCH 1/2] gpio: gpio-mxc: Fix: higher 16 GPIOs usable as wake source

[Philipp Rosenberger]

In the function gpio_set_wake_irq(), port->irq_high is only checked for
zero. As platform_get_irq() returns a value less then zero if no interrupt
was found, any gpio >= 16 was handled like an irq_high interrupt was
available. On iMX27 for example no high interrupt is available. This lead
to the problem that only some gpios (the lower 16) were useable as wake
sources.

Signed-off-by: Philipp Rosenberger <p.rosenberger@linutronix.de>
---
 drivers/gpio/gpio-mxc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 3abea3f..9269225 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -424,6 +424,9 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 		return PTR_ERR(port->base);
 
 	port->irq_high = platform_get_irq(pdev, 1);
+	if (port->irq_high < 0)
+		port->irq_high = 0;
+
 	port->irq = platform_get_irq(pdev, 0);
 	if (port->irq < 0)
 		return port->irq;
-- 
2.1.4

Re: [PATCH 1/2] gpio: gpio-mxc: Fix: higher 16 GPIOs usable as wake source

[Linus Walleij]

On Wed, Jul 12, 2017 at 10:36 AM, Philipp Rosenberger
<p.rosenberger@linutronix.de> wrote:

> In the function gpio_set_wake_irq(), port->irq_high is only checked for
> zero. As platform_get_irq() returns a value less then zero if no interrupt
> was found, any gpio >= 16 was handled like an irq_high interrupt was
> available. On iMX27 for example no high interrupt is available. This lead
> to the problem that only some gpios (the lower 16) were useable as wake
> sources.
>
> Signed-off-by: Philipp Rosenberger <p.rosenberger@linutronix.de>

That looks like a clear bug, patch applied for fixes.

Yours,
Linus Walleij

[PATCH 2/2] gpio: gpio-mxc: gpio_set_wake_irq() use proper return values

[Philipp Rosenberger]

Errors from enable_irq_wake() in gpio_set_wake_irq() were silently ignored.
Thus led to the problem that gpio_set_wake_irq() always returned
successfully, even if enable_irq_wake() returned an error.

Signed-off-by: Philipp Rosenberger <p.rosenberger@linutronix.de>
---
 drivers/gpio/gpio-mxc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 9269225..3dcd990 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -324,20 +324,21 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	struct mxc_gpio_port *port = gc->private;
 	u32 gpio_idx = d->hwirq;
+	int ret;
 
 	if (enable) {
 		if (port->irq_high && (gpio_idx >= 16))
-			enable_irq_wake(port->irq_high);
+			ret = enable_irq_wake(port->irq_high);
 		else
-			enable_irq_wake(port->irq);
+			ret = enable_irq_wake(port->irq);
 	} else {
 		if (port->irq_high && (gpio_idx >= 16))
-			disable_irq_wake(port->irq_high);
+			ret = disable_irq_wake(port->irq_high);
 		else
-			disable_irq_wake(port->irq);
+			ret = disable_irq_wake(port->irq);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int mxc_gpio_init_gc(struct mxc_gpio_port *port, int irq_base)
-- 
2.1.4

Re: [PATCH 2/2] gpio: gpio-mxc: gpio_set_wake_irq() use proper return values

[Linus Walleij]

On Wed, Jul 12, 2017 at 10:36 AM, Philipp Rosenberger
<p.rosenberger@linutronix.de> wrote:

> Errors from enable_irq_wake() in gpio_set_wake_irq() were silently ignored.
> Thus led to the problem that gpio_set_wake_irq() always returned
> successfully, even if enable_irq_wake() returned an error.
>
> Signed-off-by: Philipp Rosenberger <p.rosenberger@linutronix.de>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH 0/2] gpio-mxc: Fix issue with gpios used as wake source on iMX27

[Thomas Gleixner]

Gentle ping ....

Thanks,

	tglx

On Wed, 12 Jul 2017, Philipp Rosenberger wrote:

> The gpio-mxc driver is able to use two interrupts per gpio bank. One for
> the lower 16 and the other for the higher 16 gpios. The iMX27 has only one
> interrupt per bank. An error in the driver leads to the problem that the
> driver uses an invalid interrupt for the higher 16 gpios. Which in turn
> prevents the upper 16 gpios per bank to be used as wake source.
> 
> The first patch fixes the assignment for the irq_high. The second patch
> adds proper return values. Which should make finding any problems in the
> future much easier.
> 
> Philipp Rosenberger (2):
>   gpio: gpio-mxc: Fix: higher 16 GPIOs usable as wake source
>   gpio: gpio-mxc: gpio_set_wake_irq() use proper return values
> 
>  drivers/gpio/gpio-mxc.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> -- 
> 2.1.4
> 
>