Skip to contentSkip to conversation

rtl8139.c related issues

PATCH New Shinya Kuribayashi 1 participant 4 messages Jan 16, 2008 Network drivers

lore
Share & tools
lore
#0 lore raw

cover rtl8139.c related issues

These patches fixes a tx timeout problem, and adds two improvements. Lucas posted the similar patch around Oct 2007, but that was not checked in at that time. This is the try #2. Please review, and if possible give it a try.

Thanks,

Shinya

#1 lore raw

PATCH 1/3 drivers/net/rtl8139.c: Fix tx timeout

"to = (currticks() + RTL_TIMEOUT)" has possibilities to wrap around. If it does, the condition "(currticks() < to)" becomes invalid and immediately leads to tx timeout error. This patch introduces the fine-graded udely(10) loops to ease the impact of wrapping around.

---

 drivers/net/rtl8139.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)
drivers/net/rtl8139.c +7 -10
index 4c24805..5d7ae74 100644
@@ -80,10 +80,7 @@
 #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI) && \
 	defined(CONFIG_RTL8139)
 
-#define TICKS_PER_SEC	CFG_HZ
-#define TICKS_PER_MS	(TICKS_PER_SEC/1000)
-
-#define RTL_TIMEOUT	(1*TICKS_PER_SEC)
+#define RTL_TIMEOUT	100000
 
 #define ETH_FRAME_LEN		1514
 #define ETH_ALEN		6
@@ -414,9 +411,10 @@ static void rtl_reset(struct eth_device *dev)
 
 static int rtl_transmit(struct eth_device *dev, volatile void *packet, int length)
 {
-	unsigned int status, to;
+	unsigned int status;
 	unsigned long txstatus;
 	unsigned int len = length;
+	int i = 0;
 
 	ioaddr = dev->iobase;
 
@@ -436,8 +434,6 @@ static int rtl_transmit(struct eth_device *dev, volatile void *packet, int lengt
 	outl(((TX_FIFO_THRESH<<11) & 0x003f0000) | len,
 		ioaddr + TxStatus0 + cur_tx*4);
 
-	to = currticks() + RTL_TIMEOUT;
-
 	do {
 		status = inw(ioaddr + IntrStatus);
 		/* Only acknlowledge interrupt sources we can properly handle
@@ -445,7 +441,8 @@ static int rtl_transmit(struct eth_device *dev, volatile void *packet, int lengt
 		 * rtl_poll() function.	 */
 		outw(status & (TxOK | TxErr | PCIErr), ioaddr + IntrStatus);
 		if ((status & (TxOK | TxErr | PCIErr)) != 0) break;
-	} while (currticks() < to);
+		udelay(10);
+	} while (i++ < RTL_TIMEOUT);
 
 	txstatus = inl(ioaddr + TxStatus0 + cur_tx*4);
 
@@ -458,8 +455,8 @@ static int rtl_transmit(struct eth_device *dev, volatile void *packet, int lengt
 		return length;
 	} else {
 #ifdef	DEBUG_TX
-		printf("tx timeout/error (%d ticks), status %hX txstatus %X\n",
-			currticks()-to, status, txstatus);
+		printf("tx timeout/error (%d usecs), status %hX txstatus %X\n",
+		       10*i, status, txstatus);
 #endif
 		rtl_reset(dev);
 
#2 lore raw

PATCH 2/3 drivers/net/rtl8139.c: Fix cache coherency issues

In reply to Shinya Kuribayashi

Current driver is meant for cache coherent systems. This patch adds flush_cache() routines to support cache non-coherent systems.

---

 drivers/net/rtl8139.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/net/rtl8139.c +5 -1
index 5d7ae74..014f3b4 100644
@@ -389,6 +389,7 @@ static void rtl_reset(struct eth_device *dev)
 #ifdef	DEBUG_RX
 	printf("rx ring address is %X\n",(unsigned long)rx_ring);
 #endif
+	flush_cache((unsigned long)rx_ring, RX_BUF_LEN);
 	outl(phys_to_bus((int)rx_ring), ioaddr + RxBuf);
 
 	/* If we add multicast support, the MAR0 register would have to be
@@ -430,6 +431,7 @@ static int rtl_transmit(struct eth_device *dev, volatile void *packet, int lengt
 		tx_buffer[len++] = '\0';
 	}
 
+	flush_cache((unsigned long)tx_buffer, length);
 	outl(phys_to_bus((int)tx_buffer), ioaddr + TxAddr0 + cur_tx*4);
 	outl(((TX_FIFO_THRESH<<11) & 0x003f0000) | len,
 		ioaddr + TxStatus0 + cur_tx*4);
@@ -486,7 +488,8 @@ static int rtl_poll(struct eth_device *dev)
 #endif
 
 	ring_offs = cur_rx % RX_BUF_LEN;
-	rx_status = *(unsigned int*)KSEG1ADDR((rx_ring + ring_offs));
+	/* ring_offs is guaranteed being 4-byte aligned */
+	rx_status = *(unsigned int *)(rx_ring + ring_offs);
 	rx_size = rx_status >> 16;
 	rx_status &= 0xffff;
 
@@ -516,6 +519,7 @@ static int rtl_poll(struct eth_device *dev)
 		printf("rx packet %d bytes", rx_size-4);
 #endif
 	}
+	flush_cache((unsigned long)rx_ring, RX_BUF_LEN);
 
 	cur_rx = (cur_rx + rx_size + 4 + 3) & ~3;
 	outw(cur_rx - 16, ioaddr + RxBufPtr);
#3 lore raw

PATCH 3/3 drivers/net/rtl8139.c: rx_status should be le32_to_cpu(rx_status).

In reply to Shinya Kuribayashi

rx_status on the memory is basically in LE, but needs to be handled in CPU endian. le32_to_cpu() takes up this mission. Even if on the sane hardware, it'll work fine.

---

 drivers/net/rtl8139.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/rtl8139.c +1 -1
index 014f3b4..097f684 100644
@@ -489,7 +489,7 @@ static int rtl_poll(struct eth_device *dev)
 
 	ring_offs = cur_rx % RX_BUF_LEN;
 	/* ring_offs is guaranteed being 4-byte aligned */
-	rx_status = *(unsigned int *)(rx_ring + ring_offs);
+	rx_status = le32_to_cpu(*(unsigned int *)(rx_ring + ring_offs));
 	rx_size = rx_status >> 16;
 	rx_status &= 0xffff;