[gPXE-devel] [PATCH] [jme] Adding JMicron Ethernet driver

[Michael Brown]

On Saturday 29 May 2010 09:35:10 Guo-Fu Tseng wrote:
> > > +	rxdesc += idx;
> > > +	if (jme_make_new_rx_buf(rxring->bufinf + idx)) {
> > > +		DBG("Dropped packet due to memory allocation error.\n");
> > > +		netdev_rx_err(netdev, NULL, -ENOMEM);
> > > +	} else {
> >
> > This will behave poorly when memory runs out.  Better is to have a
> > refill routine that runs after the poll(), and always attempts to
> > refill up to a specified level (failing silently if it cannot allocate
> > a buffer).
> 
> I'm sorry I don't quite sure about the idea.
> Do you mean I should treat this packet as it's avaliable(Which makes sence
> in intuition), and try to allocate the receive buffer later in poll()?

Yes.  Take a look at something like drivers/net/b44.c, and the b44_rx_refill() 
function.

> > > +	/*
> > > +	 * Clean all other interrupt status
> > > +	 */
> > > +	jwrite32(jme, JME_IEVE,
> > > +		intrstat & ~(INTR_RX0 | INTR_RX0EMP | INTR_TX0));
> > > +}
> >
> > This has to happen *before* examining the descriptor rings, to avoid a
> > potential race condition that could cause packets to become delayed
> > indefinitely.
> 
> I did not clear any interrupt status that related to packet receive
> or transmit here. The driver would actually work without it.
> In fact this code is only important when the driver was interrupt
> driven, it prevent the hardware somehow stops the interrupts due
> to un-cleared interrupt status.
> 
> Am I missing your point?

Sorry; I misread the line and thought you were clearing the RX0, RX0EMP and 
TX0 interrupts at that point.

Michael