Re: b44: Reset due to FIFO overflow.

[Eric Dumazet <eric.dumazet@gmail.com>]

Le lundi 28 juin 2010 à 11:24 +0100, James Courtier-Dutton a écrit :
> On 28 June 2010 10:13, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > Hi
> >
> > Problem is we dont know if a Receive Fifo overflow is a minor or major
> > indication from b44 chip.
> >
> > A minor indication would be : Chip tells us one or more frame were lost.
> > No special action needed from driver.
> >
> > A major indication (as of current implemented in b44 driver) is :
> > I am completely out of order and need a reset. Please do it.
> >
> > Patch to switch from major to minor indication is easy, but we dont know
> > if its valid or not.
> >
> > diff --git a/drivers/net/b44.h b/drivers/net/b44.h
> > index e1905a4..514dc3a 100644
> > --- a/drivers/net/b44.h
> > +++ b/drivers/net/b44.h
> > @@ -42,7 +42,7 @@
> >  #define  ISTAT_EMAC            0x04000000 /* EMAC Interrupt */
> >  #define  ISTAT_MII_WRITE       0x08000000 /* MII Write Interrupt */
> >  #define  ISTAT_MII_READ                0x10000000 /* MII Read Interrupt */
> > -#define  ISTAT_ERRORS (ISTAT_DSCE|ISTAT_DATAE|ISTAT_DPE|ISTAT_RDU|ISTAT_RFO|ISTAT_TFU)
> > +#define  ISTAT_ERRORS (ISTAT_DSCE|ISTAT_DATAE|ISTAT_DPE|ISTAT_RDU|ISTAT_TFU)
> >  #define B44_IMASK      0x0024UL /* Interrupt Mask */
> >  #define  IMASK_DEF             (ISTAT_ERRORS | ISTAT_TO | ISTAT_RX | ISTAT_TX)
> >  #define B44_GPTIMER    0x0028UL /* General Purpose Timer */
> >
> >
> >
> 
> Ok, are you saying that all I have to do is apply this patch,
> reproduce the problem condition, and if it recovers OK, then we can go
> with this fix?
> If so, I will try it out after work.
> 

Yes, please try the patch and tell us what happens.

Note : It can be better, it can be worse.

It can work on your b44 chip, and freeze another computer with another
b44 chip. Use at your own risk.


> I will probably add a printk in before the ISTAT_ERRORS test, to
> inform me when that ISTAT_RFO has actually happened.
> 

In this case, you also need to add ISTAT_RFO to IMASK_DEF. My patch
completely ignores ISTAT_RFO and NIC wont generate an interrupt for this
event.

> But is doing nothing the right thing?

If chip only signals an overflow and can revover, its pretty like losing
a frame on a busy network. It happens and you dont have a report for
this.

> I would have thought that one would have to at least start and stop
> the FIFO in order for the write/read pointers to be in the correct
> positions or at least change the read pointer to do the equivalent of
> flush the buffer.

All this depends on hardware bits I dont have access to.

> Is there any of this sort of control over the FIFO possible?
> 


Probably, but I guess if broadcam guys did not implement it, they
probably have a good reason :)