Patchwork gpio/mxc: make irqs work for fsl,imx21-gpio devices

login
register
mail settings
Submitter Uwe Kleine-König
Date June 6, 2012, 9:49 a.m.
Message ID <1338976163-23001-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/163310/
State New
Headers show

Comments

Uwe Kleine-König - June 6, 2012, 9:49 a.m.
The chained handler was set for the platform device with id == 0.
When the gpio devices are instantiated by a device tree, all have id ==
-1 and so the handler was unset resulting in unusable gpio irqs on
i.MX21 and i.MX27 (when using oftree).

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

I'm not sure this is material for stable because it never worked before.

Best regards
Uwe

 drivers/gpio/gpio-mxc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Shawn Guo - June 7, 2012, 2:21 a.m.
On Wed, Jun 06, 2012 at 11:49:23AM +0200, Uwe Kleine-König wrote:
> The chained handler was set for the platform device with id == 0.
> When the gpio devices are instantiated by a device tree, all have id ==
> -1 and so the handler was unset resulting in unusable gpio irqs on
> i.MX21 and i.MX27 (when using oftree).
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Hi Uwe,

Thanks for fixing this up. 

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Regards,
Shawn

> ---
> Hello,
> 
> I'm not sure this is material for stable because it never worked before.
> 
> Best regards
> Uwe
> 
>  drivers/gpio/gpio-mxc.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e791476..5c42daf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -398,10 +398,12 @@ static int __devinit mxc_gpio_probe(struct platform_device *pdev)
>  	writel(~0, port->base + GPIO_ISR);
>  
>  	if (mxc_gpio_hwtype == IMX21_GPIO) {
> -		/* setup one handler for all GPIO interrupts */
> -		if (pdev->id == 0)
> -			irq_set_chained_handler(port->irq,
> -						mx2_gpio_irq_handler);
> +		/*
> +		 * Setup one handler for all GPIO interrupts. Actually setting
> +		 * the handler is needed only once, but doing it for every port
> +		 * is more robust and easier.
> +		 */
> +		irq_set_chained_handler(port->irq, mx2_gpio_irq_handler);
>  	} else {
>  		/* setup one handler for each entry */
>  		irq_set_chained_handler(port->irq, mx3_gpio_irq_handler);
> -- 
> 1.7.10
>
Sascha Hauer - July 2, 2012, 9:31 a.m.
[Added Linus to Cc]

Has this been applied? Linus, it seems you handle gpio patches now, you
probably missed it because you haven't been on Cc.

Sascha

On Wed, Jun 06, 2012 at 11:49:23AM +0200, Uwe Kleine-König wrote:
> The chained handler was set for the platform device with id == 0.
> When the gpio devices are instantiated by a device tree, all have id ==
> -1 and so the handler was unset resulting in unusable gpio irqs on
> i.MX21 and i.MX27 (when using oftree).
> 
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> I'm not sure this is material for stable because it never worked before.
> 
> Best regards
> Uwe
> 
>  drivers/gpio/gpio-mxc.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index e791476..5c42daf 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -398,10 +398,12 @@ static int __devinit mxc_gpio_probe(struct platform_device *pdev)
>  	writel(~0, port->base + GPIO_ISR);
>  
>  	if (mxc_gpio_hwtype == IMX21_GPIO) {
> -		/* setup one handler for all GPIO interrupts */
> -		if (pdev->id == 0)
> -			irq_set_chained_handler(port->irq,
> -						mx2_gpio_irq_handler);
> +		/*
> +		 * Setup one handler for all GPIO interrupts. Actually setting
> +		 * the handler is needed only once, but doing it for every port
> +		 * is more robust and easier.
> +		 */
> +		irq_set_chained_handler(port->irq, mx2_gpio_irq_handler);
>  	} else {
>  		/* setup one handler for each entry */
>  		irq_set_chained_handler(port->irq, mx3_gpio_irq_handler);
> -- 
> 1.7.10
> 
>
Linus Walleij - July 2, 2012, 12:59 p.m.
On 07/02/2012 11:31 AM, Sascha Hauer wrote:
> [Added Linus to Cc]
>
> Has this been applied? Linus, it seems you handle gpio patches now, you
> probably missed it because you haven't been on Cc.

No I don't, sorry. I try to help Grant with review but I haven't
taken over any trees or merging. If I did I would only have to
funnel it to Grant anyway.

Yours,
Linus Walleij
Linus Walleij - July 4, 2012, 11:38 p.m.
On Mon, Jul 2, 2012 at 2:59 PM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> On 07/02/2012 11:31 AM, Sascha Hauer wrote:
>>
>> [Added Linus to Cc]
>>
>> Has this been applied? Linus, it seems you handle gpio patches now, you
>> probably missed it because you haven't been on Cc.
>
> No I don't, sorry. I try to help Grant with review but I haven't
> taken over any trees or merging. If I did I would only have to
> funnel it to Grant anyway.

Famous last words, I wasn't in sync with my mail log and Grant
did ask me to do that...

But now I need to set up a GPIO tree and funnel it to linux-next first.

Yours,
Linus Walleij
Sascha Hauer - July 5, 2012, 6:21 a.m.
On Thu, Jul 05, 2012 at 01:38:33AM +0200, Linus Walleij wrote:
> On Mon, Jul 2, 2012 at 2:59 PM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
> > On 07/02/2012 11:31 AM, Sascha Hauer wrote:
> >>
> >> [Added Linus to Cc]
> >>
> >> Has this been applied? Linus, it seems you handle gpio patches now, you
> >> probably missed it because you haven't been on Cc.
> >
> > No I don't, sorry. I try to help Grant with review but I haven't
> > taken over any trees or merging. If I did I would only have to
> > funnel it to Grant anyway.
> 
> Famous last words, I wasn't in sync with my mail log and Grant
> did ask me to do that...

nice ;)

Sascha
Linus Walleij - July 5, 2012, 1:46 p.m.
On Wed, Jun 6, 2012 at 11:49 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The chained handler was set for the platform device with id == 0.
> When the gpio devices are instantiated by a device tree, all have id ==
> -1 and so the handler was unset resulting in unusable gpio irqs on
> i.MX21 and i.MX27 (when using oftree).
>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied.

> I'm not sure this is material for stable because it never worked before.

Why take a chance. It goes to stable.

I'll have to explain to Torvalds why this is coming late.

Yours,
Linus Walleij

Patch

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index e791476..5c42daf 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -398,10 +398,12 @@  static int __devinit mxc_gpio_probe(struct platform_device *pdev)
 	writel(~0, port->base + GPIO_ISR);
 
 	if (mxc_gpio_hwtype == IMX21_GPIO) {
-		/* setup one handler for all GPIO interrupts */
-		if (pdev->id == 0)
-			irq_set_chained_handler(port->irq,
-						mx2_gpio_irq_handler);
+		/*
+		 * Setup one handler for all GPIO interrupts. Actually setting
+		 * the handler is needed only once, but doing it for every port
+		 * is more robust and easier.
+		 */
+		irq_set_chained_handler(port->irq, mx2_gpio_irq_handler);
 	} else {
 		/* setup one handler for each entry */
 		irq_set_chained_handler(port->irq, mx3_gpio_irq_handler);