[PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver

Russell King - ARM Linux linux at arm.linux.org.uk
Fri Nov 22 12:45:28 EST 2013


On Fri, Nov 15, 2013 at 12:47:58PM +0400, Dmitry Eremin-Solenikov wrote:
> mach-sa1100's irq.c contains a mixture of system and GPIO irqs handling.
> Split out GPIO irqchip to gpio-sa1100.c. To decouple first 11 GPIO IRQs
> handling, make IRQ0-IRQ10 use chained irq handler that just passes the
> IRQ to GPIO IRQs.
> 
> Also during this refactoring introduce irq domain support for sa1100
> gpio irqs.

I'm not sure I quite like this change... How much testing has this had?
I'd much prefer that this code just isn't touched because of all the
problems we've had here in years back with IRQs from CF cards being
dropped and the like.  This was very hard to get right, especially when
interacting with the SA1111.

> @@ -136,3 +301,58 @@ static int __init sa1100_gpio_init(void)
>  	return platform_driver_register(&sa1100_gpio_driver);
>  }
>  postcore_initcall(sa1100_gpio_init);
> +
> +#ifdef CONFIG_PM
> +static unsigned long	saved_gplr;
> +static unsigned long	saved_gpdr;
> +static unsigned long	saved_grer;
> +static unsigned long	saved_gfer;
> +
> +static int sa1100_gpio_suspend(void)
> +{
> +	struct sa1100_gpio_chip *sgc = chip;
> +	saved_gplr = readl_relaxed(sgc->regbase + GPLR_OFFSET);
> +	saved_gpdr = readl_relaxed(sgc->regbase + GPDR_OFFSET);
> +	saved_grer = readl_relaxed(sgc->regbase + GRER_OFFSET);
> +	saved_gfer = readl_relaxed(sgc->regbase + GFER_OFFSET);
> +
> +	/* Clear GPIO transition detect bits */
> +	writel_relaxed(0xffffffff, sgc->regbase + GEDR_OFFSET);
> +
> +	/* FIXME: Original code also reprogramed GRER/GFER here,
> +	 * I don't see the purpose though.
> +	GRER = PWER & sgc->gpio_rising;
> +	GFER = PWER & sgc->gpio_falling;
> +	 */

So you thought you'd just comment it out because you don't understand...
Not really the way things are done.  If you don't understand something,
you shouldn't be touching the code.

In this case, it's quite simple.  GRER and GFER need to be programmed
with the interrupts which we want to be active for _each_ mode of the
system.

Therefore, if we want to have certain GPIOs triggering wakeups (iow,
those GPIOs which have had enable_irq_wake() called on them) but not
those which haven't, we need to reprogram GRER and GFER with the
GPIOs which can wake the system up.

Quite simple and obvious really.



More information about the linux-arm-kernel mailing list