fix VIA Rhine time outs (some)

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.

@@ -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) {

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

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