Re: [PATCH] Alchemy: fix edge irq handling

[Kevin Hickey <khickey@rmicorp.com>]

Manuel,

Have you actually seen this happen (outside of inducing it manually)?  I
have some concern that by doing this we may either miss interrupts on
devices that send a lot (by design) or miss a design bug in a system
because we are masking out some interrupts.  I know that system
stability is important, but I don't like hiding problems.

=Kevin

On Tue, 2009-01-20 at 11:03 +0100, Manuel Lauss wrote:
> Introduce separate mack_ack callbacks which really do shut up the
> edge-triggered irqs when called.  Without this change, high-frequency
> edge interrupts can result in an endless irq storm, hanging the system.
> 
> This can be easily triggered for example by setting an irq to falling
> edge type and manually connecting the associated pin to ground.
> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  arch/mips/alchemy/common/irq.c |   32 ++++++++++++++++++++++++--------
>  1 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/mips/alchemy/common/irq.c b/arch/mips/alchemy/common/irq.c
> index c88c821..60da581 100644
> --- a/arch/mips/alchemy/common/irq.c
> +++ b/arch/mips/alchemy/common/irq.c
> @@ -320,6 +320,16 @@ static void au1x_ic0_mask(unsigned int irq_nr)
>  	au_sync();
>  }
>  
> +static void au1x_ic0_maskack(unsigned int irq_nr)
> +{
> +	unsigned int bit = irq_nr - AU1000_INTC0_INT_BASE;
> +	au_writel(1 << bit, IC0_MASKCLR);
> +	au_writel(1 << bit, IC0_WAKECLR);
> +	au_writel(1 << bit, IC0_FALLINGCLR);
> +	au_writel(1 << bit, IC0_RISINGCLR);
> +	au_sync();
> +}
> +
>  static void au1x_ic1_mask(unsigned int irq_nr)
>  {
>  	unsigned int bit = irq_nr - AU1000_INTC1_INT_BASE;
> @@ -328,6 +338,16 @@ static void au1x_ic1_mask(unsigned int irq_nr)
>  	au_sync();
>  }
>  
> +static void au1x_ic1_maskack(unsigned int irq_nr)
> +{
> +	unsigned int bit = irq_nr - AU1000_INTC1_INT_BASE;
> +	au_writel(1 << bit, IC1_MASKCLR);
> +	au_writel(1 << bit, IC1_WAKECLR);
> +	au_writel(1 << bit, IC1_FALLINGCLR);
> +	au_writel(1 << bit, IC1_RISINGCLR);
> +	au_sync();
> +}
> +
>  static void au1x_ic0_ack(unsigned int irq_nr)
>  {
>  	unsigned int bit = irq_nr - AU1000_INTC0_INT_BASE;
> @@ -379,25 +399,21 @@ static int au1x_ic1_setwake(unsigned int irq, unsigned int on)
>  /*
>   * irq_chips for both ICs; this way the mask handlers can be
>   * as short as possible.
> - *
> - * NOTE: the ->ack() callback is used by the handle_edge_irq
> - *	 flowhandler only, the ->mask_ack() one by handle_level_irq,
> - *	 so no need for an irq_chip for each type of irq (level/edge).
>   */
>  static struct irq_chip au1x_ic0_chip = {
>  	.name		= "Alchemy-IC0",
> -	.ack		= au1x_ic0_ack,		/* edge */
> +	.ack		= au1x_ic0_ack,
>  	.mask		= au1x_ic0_mask,
> -	.mask_ack	= au1x_ic0_mask,	/* level */
> +	.mask_ack	= au1x_ic0_maskack,
>  	.unmask		= au1x_ic0_unmask,
>  	.set_type	= au1x_ic_settype,
>  };
>  
>  static struct irq_chip au1x_ic1_chip = {
>  	.name		= "Alchemy-IC1",
> -	.ack		= au1x_ic1_ack,		/* edge */
> +	.ack		= au1x_ic1_ack,
>  	.mask		= au1x_ic1_mask,
> -	.mask_ack	= au1x_ic1_mask,	/* level */
> +	.mask_ack	= au1x_ic1_maskack,
>  	.unmask		= au1x_ic1_unmask,
>  	.set_type	= au1x_ic_settype,
>  	.set_wake	= au1x_ic1_setwake,
-- 
=Kevin