eepro100

Donald Becker becker@tidalwave.net
Mon Dec 13 21:23:54 1999


On Thu, 9 Dec 1999, Stephane Eranian wrote:

[[ Ahhh, good, you got the email.  My original reply mail was rejected by
your machine.  The mail relay for cesdis.gsfc.nasa.gov is on some anti-spam
lists, since the mailing lists forward mail without requiring list
membership.  This means that we do occasionally forward spam, but that's
unavoidable. ]]


> > > The way the macro clear_suspend() is defined IS NOT
> > > SAFE. We ran into a problem with this driver because
> > 
> > It has been changed to 
> > #define clear_suspend(cmd)   ((char *)(&(cmd)->cmd_status))[3] &= ~0x40
> > for little-endian machines.
> > e inconvenience to new list users.
> How is this better than:
> 
> #define clear_suspend(cmd)   clear_bit(30, &(cmd)->cmd_status)

Nominally it's not better.  But it appears that some motherboards don't
enforce atomicity with clear_bit() vs. PCI operations.

Arguably doing a byte write without a read is more efficient, since the
write can be buffered.

> My understanding of the operation is:
>	- The suspend bit is the marker for the end-of-transmit list.
> 	  When the chip reaches this point. It enters the suspended state
> 	  and interrupts.
> 	- When you want to add packets to the list, you need to extend the list.
> 	  Because you're using a fixed ring buffer, you don't need to link in your
> 	  new list. However, you need to tell the chip that the end of list has now
> 	  moved (one entry farther). That's why you need to clear the Suspend
> 	  bit of the previous tail.

Yes.  Without overwriting the completion marker in the low bytes of the
status word, should the eepro100 chip decide to do its write at just the
wrong time.  (Although some versions of the eepro100 chip seem to forget to
write the completion bits on their own.)

> So I still think the code should rather look like this:
> 
> #if defined(__i386__) || defined(__alpha__) /* || defined(__ia64__)*/
> #define clear_suspend(cmd)   clear_bit(30, &(cmd)->cmd_status)
> #elif defined(__powerpc__)
> #define clear_suspend(cmd)      clear_bit(6, &(cmd)->cmd_status)
> #else
> #error "You need the clear_bit() atomic operation for your platform"
> #endif


Donald Becker
Scyld Computing Corporation, becker@tidalwave.net