Re: [PATCH] fix e100 rx path on ARM (was [PATCH] e100 rx: or s and el bits)

Milton Miller wrote:

On Jun 1, 2007, at 3:45 PM, David Acker wrote:

quoted

Ok, I took a stab at coding and testing these ideas.  Below is a patch 
against 2.6.22-rc3.
Let me know what you think.

I think you got most of the ideas.   As Auke noted, your coding style is 
showing again.   And your mailer again munged whitespace (fixed by 
s/^<space><space>/<space>/ s/^$/<space>/).

Sorry about the coding style.  I instinctively followed what was there 
instead of kernel coding convention.  I will look into how whitespace is 
getting screwed up.

quoted

Find a buffer that is complete with rx->el not set and rx->s0 set.
    It appears that hardware can read the rfd's el-bit, then software 
can clear the rfd el-bit and set the rfd size, and then hardware can 
come in and read the size.

Yes, since the size is after the EL flag in the descriptor, this can 
happen since the pci read is not atomic.

quoted

I am reading the status back, although I don't think that I have to in 
this instance.

Actually, you are reading it when the rfd still has EL set.  Since the 
cpu will never encounter that case, the if condition is never satisfied.

In my tests, every time I found a completed rfd with the el-bit set, the 
receiver was in the out of resources state.

How about creating a state unknown, for when we think we should check 
the device if its running.
If we are in this state and then encounter a received packet without s0 
set, we can set it back
to running.   We set it when we rx a packet with s0 set.

We then move both io_status reads to the caller.

I can look into that as I clean this up.

quoted

I am testing a version of this code patched against 2.6.18.4 on my PXA 
255 based system.  I will let you all know how it goes.

The testing I did so far did well.  I will try to get some more going 
tonight, hopefully on a cleaned up patch.

I'm assuming this is why the cleanup of the receiver start to always 
start on rx_to_clean got dropped again. :-)

Yep.  I will get that in the next patch.

Also, I would like a few sentences in the Driver Operation section IV 
Receive big comment.  Something like

In order to keep updates to the RFD link field from colliding with 
hardware writes to mark packets complete, we use the feature that 
hardware will not write to a size 0 descriptor and mark the previous 
packet as end-of-list (EL).   After updating the link, we remove EL and 
only then restore the size such that hardware may use the 
previous-to-end RFD.

at the end of the first paragraph, and insert software before "no 
locking is required" in the second.

Sounds good to me.

I will see if I can get into a cleaned up patch today and get it out by 
tomorrow.  Thanks for dealing with me...I have been around kernel code 
for awhile but posting official patches to linux is new to me.
-Ack