Hello,
First of all, many thanks for the driver. I've encountered problems
with recent updates to the driver and fixed them and some others. I'm
also interested in improving this driver, and having the programming
manual would be very helpful. Does anybody know how to obtain the
programming manual for this chip? Is it available online somewhere?
Or should I contact VIA?
Thanks.
----------------------
Recent receive ring related updates to via-velocity broke the driver.
My box lost packets, recevied duplicate truncated packets and soon
locked into infinite error interrupt handling. This patch fixes
receive ring handling and some other parts of the driver. List of
fixes follow.
Using cpu_to_le32 on OWNED_BY_NIC is wrong. It will produce
0x10000000 on big endian machines while owner bitfield would still
evaluate to 0 or 1.
In velocity_give_rx_desc(), there should be a wmb() between resetting
the first four bytes of rdesc0 and setting owner. As resetting the
first four bytes isn't necessary, I just removed the function and
directly set owner.
In velocity_init_registers(), init_cam_filter() clears mCAMmask
which might have been set by set_multy() (not sure if this can ever
occur). Modified to invoke init_cam_filter() first. Also,
clear_isr() is called twice. Removed the first invocation.
velocity_nics wasn't managed properly, fixed.
Removed unused velocity_info.xmit_lock.
In velocity_give_many_rx_descs(), index calculation was incorrect.
This and bugs in velocity_rx_srv() described in the following
paragraph caused packet loss, truncation and infinite error
interrupt generation. Fixed.
In velocity_rx_srv(), velocity_rx_refill() could be called without
any dirty slot. With proper timing, This can result in refilling
yet unreceived packets and pushing dirty pointer ahead of the current
pointer. Also, vptr->rd_curr which is used by velocity_rx_refill()
was updated after calling velocity_rx_refill() thus screwing
receive descriptor ring. Fixed.
Other obivous fixes.
--
tejun
Tejun Heo <[email protected]> :
[...]
> First of all, many thanks for the driver. I've encountered problems
> with recent updates to the driver and fixed them and some others. I'm
> also interested in improving this driver, and having the programming
> manual would be very helpful. Does anybody know how to obtain the
> programming manual for this chip? Is it available online somewhere?
See http://www.viaarena.com for a .exe driver which turns out to be a .zip.
[...]
> Recent receive ring related updates to via-velocity broke the driver.
> My box lost packets, recevied duplicate truncated packets and soon
> locked into infinite error interrupt handling. This patch fixes
> receive ring handling and some other parts of the driver. List of
> fixes follow.
>
> Using cpu_to_le32 on OWNED_BY_NIC is wrong. It will produce
> 0x10000000 on big endian machines while owner bitfield would still
> evaluate to 0 or 1.
The rx_desc structure is used in the network device, not in the host
computer. I see nowhere in the code where it is said to the network
adapter that it should change the ordering of the bytes in its registers.
Was this change tested on a big endian machine ?
> In velocity_give_rx_desc(), there should be a wmb() between resetting
> the first four bytes of rdesc0 and setting owner. As resetting the
> first four bytes isn't necessary, I just removed the function and
> directly set owner.
- The function keeps code factored. Feel free to modify the function
but please keep the function in place.
- having the lower bytes cleaned can help when something goes wrong.
I would favor a removal of the bitfield and use dumb accesses to
registers (as u32) like can be found in others drivers (just mho).
Though simple, the changes to velocity_init_td_ring could be commented
(it takes a few seconds to realize that the settings in velocity_info_tbl()
allowed the former buggy code to work as well).
Can you be convinced to post the whole thing as a serie of incremental
patches on [email protected] with Cc: to [email protected] and
[email protected] ?
The patch looks quite good and it clearly fixes several points but
I guess that most people prefer to review small incremental changes.
--
Ueimor