Re: [ PATCH ] PowerPC cascade UIC IRQ handler fix.

[David Gibson <david@gibson.dropbear.id.au>]

On Mon, Jul 30, 2007 at 08:35:17PM +0400, Valentine Barshak wrote:
> PPC44x cascade UIC irq handler fix.
> 
> According to PPC44x UM, if an interrupt is configured as level-sensitive,
> and a clear is attempted on the UIC_SR, the UIC_SR field is not
> cleared if the incoming interrupt signal is at the asserted polarity.
> This causes us to enter a cascade handler twice, since we first ack
> parent UIC interrupt and ack child UIC one after that.
> The patch checks child UIC msr value and returns IRQ_HANDLED
> if there're no pending interrupts. Otherwise we get a kernel panic
> with a "Fatal exception in interrupt" (illegal vector).
> The patch also fixes status flags.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

Hrm... This doesn't seem like the right fix to me.  Instead, I think
the cascaded IRQ handler should ack the interrupt on the child first.
I'm a little surprised it doesn't at the moment.

> ---
> 
> --- linux.orig/arch/powerpc/sysdev/uic.c	2007-07-27 20:37:11.000000000 +0400
> +++ linux/arch/powerpc/sysdev/uic.c	2007-07-30 20:26:48.000000000 +0400
> @@ -142,7 +142,7 @@
>  
>  	desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
>  	desc->status |= flow_type & IRQ_TYPE_SENSE_MASK;
> -	if (trigger)
> +	if (!trigger)
>  		desc->status |= IRQ_LEVEL;
>  
>  	spin_unlock_irqrestore(&uic->lock, flags);
> @@ -207,6 +207,9 @@
>  	int subvirq;
>  
>  	msr = mfdcr(uic->dcrbase + UIC_MSR);
> +	if (!msr)
> +		return IRQ_HANDLED;
> +
>  	src = 32 - ffs(msr);
>  
>  	subvirq = irq_linear_revmap(uic->irqhost, src);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson