Re: [RFT] pcnet32 NAPI changes

[Lennart Sorensen <lsorense@csclub.uwaterloo.ca>]

On Tue, Jun 20, 2006 at 11:05:04AM -0500, Jon Mason wrote:
> The point of my comment was CPU utilization.
> 
> It appears that a bug is trying to be fixed by adding NAPI. This
> sounds a bit hackish to me, and could hide the root cause of the
> problem. So I'm not sure that is the best idea, but I will defer to
> the maintainer.

No it isn't a bug.  If the hardware generates enough interrupts to keep
the cpu at 100% handling them, starving user space (since interrupts
have high priority compared to just running user code of course), then
the watchdog daemon which of course runs in user space will never run
and hence the watchdog hardware times out and resets the system, as it
is designed to do.  There is no bug, just a problem of too many
interrupts generated by the network hardware.  NAPI elliminates the
receive interrupts when the system is busy, solving the problem at it's
root cause.

> But your example is just one instance.  Here is one without a comment:
> 
> lp->a.write_csr(ioaddr, 4, 0x0915);

Hmm.  0x0915 = 0000 1001 0001 0101 =>
*Auto Pad Transmit (bit 11).  Enabled auto padding of packets.
*Missed Frame Counter Overflow Mask (bit 8):  Masks out interrupts on
 overflow of the missed frame counter.
*Receive Collision Counter Overflow Mask (bit 4):  Masks out interrupts on
 overflow of the receive collision counter.
*Transmit Start Mask (bit 2):  Masks out interrupts on start of
 transmit.

So every CSR has a different meaning for all its bits.  Defining each
one, and combining all of them could make a lot of the code really
messy.  Perhaps more comments on those places would be clearer.

> What is it doing?  Is it still needed?  Can it be done anywhere else?  
> Who knows, because it is magic.  The 4 can be defined as CSR0_STOP, per
> your example above, but what does value 0x0915 do?

No the 4 has a different meaning in CSR4.  It means stop in CSR0.  in
CSR4 it means Transmit Start Mask.  It masks interrupts on transmit
start.  I think the value is wrong, since my data sheet says bit 0 and 1
are reserved and should be written as 0.  0x0915 would write bit 0 as a
1 which violates the data sheet of the 972 at least.

> My point was that there are certain parts of the code which are
> non-intuative and should be commented and there are others which a
> good descrptive value would be nice.

Well I agree the code could get a bit better.  I did think overall that
the code was rather nice actually.

Len Sorensen