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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
