[PATCH] spidernet: fix interrupt reason recognition

[PATCH] spidernet: fix interrupt reason recognition

[Ishizaki Kou]

This patch solves a problem that the spidernet driver sometimes fails
to handle IRQ.

The problem happens because,
- In Cell architecture, interrupts may arrive at an interrupt
  controller, even if they are masked by the setting on registers of
  devices. It happens when interrupt packets are sent just before 
  the interrupts are masked.
- spidernet interrupt handler compares interrupt reasons with
  interrupt masks, so when such interrupts occurs, spidernet interrupt
  handler returns IRQ_NONE.
- When all of interrupt handler return IRQ_NONE, linux kernel disables
  the IRQ and it no longer delivers interrupts to the interrupt handlers.

spidernet doesn't work after above sequence, because it can't receive
interrupts.
 
This patch changes spidernet interrupt handler that it compares
interrupt reason with SPIDER_NET_INTX_MASK_VALUE.

Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
---

Linas-san,

Please apply this to 2.6.23. Because this problem is sometimes happens
and we cannot use the ethernet port any more.

And also, please apply the following Arnd-san's patch to fix a problem
that spidernet driver sometimes causes a BUG_ON at open.

 http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=12211


Index: linux-powerpc-git/drivers/net/spider_net.c
===================================================================
--- linux-powerpc-git.orig/drivers/net/spider_net.c	2007-07-19 18:42:02.000000000 +0900
+++ linux-powerpc-git/drivers/net/spider_net.c	2007-08-20 20:52:23.000000000 +0900
@@ -1441,17 +1441,14 @@ static void
 spider_net_handle_error_irq(struct spider_net_card *card, u32 status_reg)
 {
 	u32 error_reg1, error_reg2;
-	u32 mask_reg1, mask_reg2;
 	u32 i;
 	int show_error = 1;
 
 	error_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1STS);
 	error_reg2 = spider_net_read_reg(card, SPIDER_NET_GHIINT2STS);
-	mask_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1MSK);
-	mask_reg2 = spider_net_read_reg(card,SPIDER_NET_GHIINT2MSK);
 
-	error_reg1 &= mask_reg1;
-	error_reg2 &= mask_reg2;
+	error_reg1 &= SPIDER_NET_INT1_MASK_VALUE;
+	error_reg2 &= SPIDER_NET_INT2_MASK_VALUE;
 
 	/* check GHIINT0STS ************************************/
 	if (status_reg)
@@ -1679,11 +1676,10 @@ spider_net_interrupt(int irq, void *ptr)
 {
 	struct net_device *netdev = ptr;
 	struct spider_net_card *card = netdev_priv(netdev);
-	u32 status_reg, mask_reg;
+	u32 status_reg;
 
 	status_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0STS);
-	mask_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
-	status_reg &= mask_reg;
+	status_reg &= SPIDER_NET_INT0_MASK_VALUE;
 
 	if (!status_reg)
 		return IRQ_NONE;

Re: [PATCH] spidernet: fix interrupt reason recognition

[Linas Vepstas]

On Mon, Aug 20, 2007 at 10:13:27PM +0900, Ishizaki Kou wrote:
> Please apply this to 2.6.23.

I'll review and forward shortly.  Kick me if you don't see a formal
reply in a few days.

> And also, please apply the following Arnd-san's patch to fix a problem
> that spidernet driver sometimes causes a BUG_ON at open.
> 
>  http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=12211

Are you sure? This patch no longer applies cleanly, in part because
your patch "[PATCH] spidernet: improve interrupt handling" 
from Mon, 09 Jul 2007 added a spider_net_enable_interrupts(card); 
at the end of spider_net_open().  Because of this, it seems like 
Arnd's patch is no longer needed, right?

--linas

Re: [Cbe-oss-dev] [PATCH] spidernet: fix interrupt reason recognition

[Ishizaki Kou]

Linas Vepstas wrote:
> On Mon, Aug 20, 2007 at 10:13:27PM +0900, Ishizaki Kou wrote:
> > Please apply this to 2.6.23.
> 
> I'll review and forward shortly.  Kick me if you don't see a formal
> reply in a few days.
> 
> > And also, please apply the following Arnd-san's patch to fix a problem
> > that spidernet driver sometimes causes a BUG_ON at open.
> > 
> >  http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=12211
> 
> Are you sure? This patch no longer applies cleanly, in part because

I see. I'll send another applicable patch.

> your patch "[PATCH] spidernet: improve interrupt handling" 
> from Mon, 09 Jul 2007 added a spider_net_enable_interrupts(card); 
> at the end of spider_net_open().  Because of this, it seems like 
> Arnd's patch is no longer needed, right?

As you pointed out, we intended that "[PATCH] spidernet: improve
interrupt handling" solves the same problem which Arnd's patch solves.

When spider_net_open() is called, interrupt reasons sometimes remain
on interrupt status register, even though they are masked by mask
register.  With this patch, spider_net_interrupt() compares the value
of interrupt status register with SPIDER_NET_INTX_MASK_VALUE, not with
interrupt mask register value.  As a result, spider_net_interrupt()
(which is called from request_irq() in spider_net_open()) starts
polling and causes BUG_ON().

So, netif_poll_enable() must be called before request_irq() is
called. This is the reason that we also need Arnd's patch.

Best regards,
Kou Ishizaki

Re: [Cbe-oss-dev] [PATCH] spidernet: fix interrupt reason recognition

[Ishizaki Kou]

Linas-san,

Ishizaki Kou wrote:
> Linas Vepstas wrote:
> > On Mon, Aug 20, 2007 at 10:13:27PM +0900, Ishizaki Kou wrote:
> > > Please apply this to 2.6.23.
> > 
> > I'll review and forward shortly.  Kick me if you don't see a formal
> > reply in a few days.
> > 
> > > And also, please apply the following Arnd-san's patch to fix a problem
> > > that spidernet driver sometimes causes a BUG_ON at open.
> > > 
> > >  http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=12211
> > 
> > Are you sure? This patch no longer applies cleanly, in part because
> 
> I see. I'll send another applicable patch.
> 
> > your patch "[PATCH] spidernet: improve interrupt handling" 
> > from Mon, 09 Jul 2007 added a spider_net_enable_interrupts(card); 
> > at the end of spider_net_open().  Because of this, it seems like 
> > Arnd's patch is no longer needed, right?
> 
> As you pointed out, we intended that "[PATCH] spidernet: improve
> interrupt handling" solves the same problem which Arnd's patch solves.
> 
> When spider_net_open() is called, interrupt reasons sometimes remain
> on interrupt status register, even though they are masked by mask
> register.  With this patch, spider_net_interrupt() compares the value
> of interrupt status register with SPIDER_NET_INTX_MASK_VALUE, not with
> interrupt mask register value.  As a result, spider_net_interrupt()
> (which is called from request_irq() in spider_net_open()) starts
> polling and causes BUG_ON().
> 
> So, netif_poll_enable() must be called before request_irq() is
> called. This is the reason that we also need Arnd's patch.


How about following two patches that I posted last week:

 http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=12997
 http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=13049

Best regards,
Kou Ishizaki

Re: [PATCH] spidernet: fix interrupt reason recognition

[Jeff Garzik]

Ishizaki Kou wrote:
> This patch solves a problem that the spidernet driver sometimes fails
> to handle IRQ.
> 
> The problem happens because,
> - In Cell architecture, interrupts may arrive at an interrupt
>   controller, even if they are masked by the setting on registers of
>   devices. It happens when interrupt packets are sent just before 
>   the interrupts are masked.
> - spidernet interrupt handler compares interrupt reasons with
>   interrupt masks, so when such interrupts occurs, spidernet interrupt
>   handler returns IRQ_NONE.
> - When all of interrupt handler return IRQ_NONE, linux kernel disables
>   the IRQ and it no longer delivers interrupts to the interrupt handlers.
> 
> spidernet doesn't work after above sequence, because it can't receive
> interrupts.
>  
> This patch changes spidernet interrupt handler that it compares
> interrupt reason with SPIDER_NET_INTX_MASK_VALUE.
> 
> Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
> ---
> 
> Linas-san,
> 
> Please apply this to 2.6.23. Because this problem is sometimes happens
> and we cannot use the ethernet port any more.
> 
> And also, please apply the following Arnd-san's patch to fix a problem
> that spidernet driver sometimes causes a BUG_ON at open.

Linas?  ACK?  Alive?  :)

	Jeff

Re: [PATCH] spidernet: fix interrupt reason recognition

[Linas Vepstas]

On Fri, Aug 31, 2007 at 06:46:17AM -0400, Jeff Garzik wrote:
> Ishizaki Kou wrote:
> >This patch solves a problem that the spidernet driver sometimes fails
> >to handle IRQ.
> >
> >The problem happens because,
> >- In Cell architecture, interrupts may arrive at an interrupt
> >  controller, even if they are masked by the setting on registers of
> >  devices. It happens when interrupt packets are sent just before 
> >  the interrupts are masked.
> >- spidernet interrupt handler compares interrupt reasons with
> >  interrupt masks, so when such interrupts occurs, spidernet interrupt
> >  handler returns IRQ_NONE.
> >- When all of interrupt handler return IRQ_NONE, linux kernel disables
> >  the IRQ and it no longer delivers interrupts to the interrupt handlers.
> >
> >spidernet doesn't work after above sequence, because it can't receive
> >interrupts.
> > 
> >This patch changes spidernet interrupt handler that it compares
> >interrupt reason with SPIDER_NET_INTX_MASK_VALUE.
> >
> >Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
> >---
> >
> >Linas-san,
> >
> >Please apply this to 2.6.23. Because this problem is sometimes happens
> >and we cannot use the ethernet port any more.
> >
> >And also, please apply the following Arnd-san's patch to fix a problem
> >that spidernet driver sometimes causes a BUG_ON at open.
> 
> Linas?  ACK?  Alive?  :)

Argh. I read the code; it looked fine. I was going to compile it and
forward it formally and etc. and then I got busy ...

Ack'ed by: Linas Vepstas <linas@austin.ibm.com>

--linas

Re: [PATCH] spidernet: fix interrupt reason recognition

[Jeff Garzik]

Ishizaki Kou wrote:
> This patch solves a problem that the spidernet driver sometimes fails
> to handle IRQ.
> 
> The problem happens because,
> - In Cell architecture, interrupts may arrive at an interrupt
>   controller, even if they are masked by the setting on registers of
>   devices. It happens when interrupt packets are sent just before 
>   the interrupts are masked.
> - spidernet interrupt handler compares interrupt reasons with
>   interrupt masks, so when such interrupts occurs, spidernet interrupt
>   handler returns IRQ_NONE.
> - When all of interrupt handler return IRQ_NONE, linux kernel disables
>   the IRQ and it no longer delivers interrupts to the interrupt handlers.
> 
> spidernet doesn't work after above sequence, because it can't receive
> interrupts.
>  
> This patch changes spidernet interrupt handler that it compares
> interrupt reason with SPIDER_NET_INTX_MASK_VALUE.
> 
> Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>

applied