enabling EXT_WAKEUP interrupts on PXA3xx

[Eric Miao]

On Tue, Nov 10, 2009 at 4:14 PM, Mike Rapoport <mike at compulab.co.il> wrote:
> Eric Miao wrote:
>> On Tue, Nov 10, 2009 at 3:34 PM, Eric Miao <eric.y.miao at gmail.com> wrote:
>>> On Tue, Nov 10, 2009 at 3:28 PM, Daniel Mack <daniel at caiaq.de> wrote:
>>>> On Tue, Nov 10, 2009 at 09:22:00AM +0200, Mike Rapoport wrote:
>>>>> We need to enable EXT_WAKEUP interrupts on PXA3xx and I wonder what would be the
>>>>> best way to do it.
>>>>> I see two possibilities:
>>>>> 1) add 'if (irq == IRQ_WAKEUPx)' statements to pxa_{un}mask_irq functions in
>>>>> arch/arm/mach-pxa/irq.c
>>>>> 2) register additional irq_chip in arch/arm/mach-pxa/pxa3xx.c that will be
>>>>> responsible for handling EXT_WAKEUP (and probably other pxa3xx specific) interrupts.
>>>>>
>>>>> What do you prefer?
>>>> I'm just calling
>>>>
>>>>  enable_irq_wake(IRQ_WAKEUP0);
>>>>
>>>> from my board support code, and it works fine - no need to change
>>>> anything else.
>>>>
>>> Daniel,
>>>
>>> I think Mike is talking about "interrupt" on EXT_WAKEUP0, which involves
>>> register PECR.
>>>
>>> My POV is the 2nd way to go, should be better and cleaner, but I'm always
>>> wondering where to add these additional two interrupts, possibly before
>>> the GPIO IRQs or after.
>>>
>>
>> Well, my second thought on this would be after GPIO IRQs and treat
>> them as board specific ones.
>>
>
> Currently we have
> #define IRQ_WAKEUP0     PXA_IRQ(49)     /* EXT_WAKEUP0 */
> #define IRQ_WAKEUP1     PXA_IRQ(50)     /* EXT_WAKEUP1 */
> so probably we'll just treat them in a way similar to IRQ_GPIO[0,1]:
>

Ah, right that's it, almost forgot. The code below looks OK to me,
and I think it would be better to put them into pxa3xx.c, as well
as the registration into pxa3xx_init_irq(). Feel free to file a patch
for this.

Thanks.

> diff --git a/arch/arm/mach-pxa/irq.c b/arch/arm/mach-pxa/irq.c
> index d694ce2..bfc57e6 100644
> --- a/arch/arm/mach-pxa/irq.c
> +++ b/arch/arm/mach-pxa/irq.c
> @@ -118,6 +118,24 @@ static void __init pxa_init_low_gpio_irq(set_wake_t fn)
>        pxa_low_gpio_chip.set_wake = fn;
>  }
>
> +static struct irq_chip pxa_ext_wakeup_chip = {
> +       .name           = "EXT-WAKEUP",
> +       .ack            = pxa_ack_ext_wakeup,
> +       .mask           = pxa_mask_ext_wakeup,
> +       .unmask         = pxa_unmask_ext_wakeup,
> +};
> +
> +static void __init pxa_init_ext_wakeup_irq(set_wake_t fn)
> +{
> +       for (irq = IRQ_WAKEUP0; irq <= IRQ_WAKEUP1; irq++) {
> +               set_irq_chip(irq, &pxa_ext_wakeup_chip);
> +               set_irq_handler(irq, handle_edge_irq);
> +               set_irq_flags(irq, IRQF_VALID);
> +       }
> +
> +       pxa_ext_wakeup_chip.set_wake = fn;
> +}
> +
>  void __init pxa_init_irq(int irq_nr, set_wake_t fn)
>  {
>        int irq, i;
> @@ -146,6 +164,9 @@ void __init pxa_init_irq(int irq_nr, set_wake_t fn)
>
>        pxa_internal_irq_chip.set_wake = fn;
>        pxa_init_low_gpio_irq(fn);
> +
> +       if (cpu_is_pxa3xx())
> +               pxa_init_ext_wakeup_irq(fn);
>  }
>
>
>
> --
> Sincerely yours,
> Mike.
>
>