[BUG?] vic MULTI_IRQ_HANDLER (was [PATCH] ep93xx: Implement double buffering for M2M DMA channels)

[H Hartley Sweeten]

On Tuesday, April 03, 2012 10:08 AM, Will Deacon wrote:
> On Mon, Apr 02, 2012 at 10:46:14PM +0100, Russell King - ARM Linux wrote:
>> On Mon, Apr 02, 2012 at 11:00:34PM +0300, Mika Westerberg wrote:
>>> Anyway it looks like handle_IRQ() enables interrupts when it is finished with
>>> the current interrupt which then causes hw to interrupt second time resulting
>>> failure in case of ep93xx.
>> 
>> Soft IRQ processing in the irq exit path will enable interrupts, and
>> this is probably where the problem is showing up.
>> 
>> You've identified an important difference between the level 1 interrupt
>> controller handlers and the chained handlers, and I suggest that folk
>> re-implement their level 1 interrupt handlers in the same way as the
>> assembly code was: re-read the interrupt register each time round the
>> loop.
>
> I'm also seeing this on the Versatile AB, with spurious interrupts reported
> from eth0.
>
> As you suggested, re-reading the VIC status solves the problem:
>
>
> diff --git a/arch/arm/common/vic.c b/arch/arm/common/vic.c
> index dcb004a..cb6b49a 100644
> --- a/arch/arm/common/vic.c
> +++ b/arch/arm/common/vic.c
> @@ -441,11 +441,9 @@ static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs)
>         u32 stat, irq;
>         int handled = 0;
>  
> -       stat = readl_relaxed(vic->base + VIC_IRQ_STATUS);
> -       while (stat) {
> +       while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
>                 irq = ffs(stat) - 1;
>                 handle_IRQ(irq_domain_to_irq(&vic->domain, irq), regs);
> -               stat &= ~(1 << irq);
>                 handled = 1;
>         }

This should be posted as a proper patch with your Signed-off-by.

But for what it's worth, on ep93xx:

Tested-by: H Hartley Sweeten <hsweeten at visionengravers.com>
Acked-by: H Hartley Sweeten <hsweeten at visionengravers.com>