[PATCH] fix VIA Rhine time outs (some)

[PATCH] fix VIA Rhine time outs (some)

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

If you experience time outs with via-rhine (typically while copying large
files over the network), this patch is for you.

The problem with at least some of the Rhine chips is that they are skipping
one frame when they stop the Tx engine. This means that every time an abort
(due to excessive collisions) occurs, the driver will stall and need a
reset via netdev timeout [1]. The patch addresses that by setting the Tx
ring buffer pointer to where the driver thinks the chip should continue.

Note: this fix is not sufficient for all time out cases, but it should
improve the situation without having negative side effects for anyone. I
have a more comprehensive solution in my development tree, for now I am
curious how many of the time outs people keep reporting persist after this
one is fixed.

Patch is against Jeff's via-rhine.c LK 1.1.14. It should work with the
older version in 2.4.19-pre9 as well.

Roger

[1] The same problem will likely happen with any other Tx error (e.g.
    underrun), abort is just the one I can easily produce here.

    It also seems that the interrupt status is not always reliable. I
    have one report of very mysterious behavior with a VT86C100A which
    seems to produce all kinds of obviously bogus errors. That's
    unrelated though and easier to study once the restart problems
    are fixed.

[-- Attachment #2: via-rhine.c.7.patch --]
[-- Type: text/plain, Size: 663 bytes --]

--- via-rhine.c.org	Sat Jun  1 11:23:33 2002
+++ via-rhine.c	Sat Jun  1 12:19:18 2002
@@ -1502,7 +1502,12 @@ static void via_rhine_error(struct net_d
 		clear_tally_counters(ioaddr);
 	}
 	if (intr_status & IntrTxAbort) {
-		/* Stats counted in Tx-done handler, just restart Tx. */
+		if (debug > 1)
+			printk(KERN_INFO "%s: Abort %4.4x, frame dropped.\n",
+				   dev->name, intr_status);
+		/* We know better than the chip where it should continue */
+		writel(virt_to_bus(&np->tx_ring[np->dirty_tx % TX_RING_SIZE]),
+			   dev->base_addr + TxRingPtr);
 		writew(CmdTxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
 	}
 	if (intr_status & IntrTxUnderrun) {

Re: [PATCH] fix VIA Rhine time outs (some)

[Urban Widmark]

On Sat, 1 Jun 2002, Roger Luethi wrote:

> reset via netdev timeout [1]. The patch addresses that by setting the Tx
> ring buffer pointer to where the driver thinks the chip should continue.

You shouldn't use virt_to_bus().

On x86 it doesn't matter(?) but on other hw it does.
See Documentation/DMA-mapping.txt.


The tx_ring array is allocated with pci_alloc_consistent and the addresses
you are supposed to send to the hardware are the rx_ring_dma and
tx_ring_dma. tx_ring and rx_ring are for the driver.

The initialisation of tx_ring_dma assumes the area allocted by
pci_alloc_consistent is contiguous and that it is ok to use any address
within that area (within any alignment constrinats of the hw). If that
code is correct you could copy it and do:

int entry = np->dirty_tx % TX_RING_SIZE;
writel(np->tx_ring_dma + entry * sizeof(struct tx_desc),
       dev->base_addr + TxRingPtr);

(Or something prettier ...)

/Urban

Re: [PATCH] fix VIA Rhine time outs (some)

[Roger Luethi]

On Sat, 01 Jun 2002 13:55:42 +0200, Urban Widmark wrote:
> You shouldn't use virt_to_bus().

Drats. Of course you're right. Ivan G. told me that, too. Can we agree that
this is a experimental patch (that's why I didn't cc Jeff) and that we
don't care? ;-) For my development version I broke out the whole restart
stuff into a separate inline function which comes with plenty of space to
hide away pointer arithmetics, and that's where I'm basically doing what
you suggest.  Currently I am just trying to a) find out how effective the
smallest fix I could possibly come up with is, and b) save time since I'm
kinda busy right now. Feel free to rewrite the patch to make DMA-mapping.txt
happy, though <g>.

Roger