[PATCH 2.6.0-test9-bk6] r8169.c update for 8110S support from RTL 1.6 version

[PATCH 2.6.0-test9-bk6] r8169.c update for 8110S support from RTL 1.6 version

[Brad House]

The current in-kernel version of the r8169 driver is v1.2
from RealTek.  This is a patch against 2.6.0-test9-bk6
and later kernels that updates the driver to 1.6 from RealTek.
Just small patches applied to it to port to 2.6 kernel, etc.
It works with the 8110S chip found on many motherboards,
especially x86_64 mobos.

http://dev.gentoo.org/~brad_mssw/kernel_patches/2.6.0/genpatches-0.4/200_r8169-8110S-111203.patch

Please CC me on any replies as I am not subscribed to the list.

-Brad House
brad_mssw@gentoo.org

r8169 and tg3

[Jeff Garzik]

Brad House wrote:
> The current in-kernel version of the r8169 driver is v1.2
> from RealTek.  This is a patch against 2.6.0-test9-bk6
> and later kernels that updates the driver to 1.6 from RealTek.
> Just small patches applied to it to port to 2.6 kernel, etc.
> It works with the 8110S chip found on many motherboards,
> especially x86_64 mobos.


A bit of history...   SiS sent me a driver for their gigabit chip, 
SIS190 (sis190.c).  Full of PCI DMA bugs, alignment problems, and fun 
like that.  After taking flak for merging it :) I fixed all those bugs. 
  Fast forward a bit, I find out that sis190.c was copied from r8169.c, 
and modified a bit to work with SiS chips.  I look at r8169.c, and find 
all those same bugs -- and the RealTek updates don't address any of that.

So, what needs to happen is somebody needs to look at the sis190 changes 
that went in, and apply those same fixes to r8169.

Two tangents, while I have your attention:

1) If you have problems with the tg3 driver, we need to get those bug 
reports and fix them.  bcm5700 is not going to be merged into the 
kernel.  Further, there was at least one serious x86-64 issue that was 
fixed in tg3, and we need to make sure users are testing that driver, 
not a non-standard driver with bugs of its own.

2) Please CC me on all net driver patches, since I am the dude that 
actually puts those patches into the kernel.

Thanks,

	Jeff

Re: r8169 and tg3

[Brad House]

Commenting below:
> 
> So, what needs to happen is somebody needs to look at the sis190 changes 
> that went in, and apply those same fixes to r8169.

Ok, cool. Didn't realize that, if you have a couple of references to
those discussions or patches, that would be awesome (so I don't have
to try to dig them up), otherwise, I'll research it some.
I was also going to look into merging the big endian patches I saw
posted earlier this month.

> 1) If you have problems with the tg3 driver, we need to get those bug 
> reports and fix them.  bcm5700 is not going to be merged into the 
> kernel.  Further, there was at least one serious x86-64 issue that was 
> fixed in tg3, and we need to make sure users are testing that driver, 
> not a non-standard driver with bugs of its own.

Ok, sounds like a plan. I'll get some testing on the latest tg3 driver
then, have those been merged into 2.6.0-test9-bk21, or is there another
patch I need to apply to my local tree (and in turn will be applied
to Gentoo's tree)?

> 
> 2) Please CC me on all net driver patches, since I am the dude that 
> actually puts those patches into the kernel.

Cool, no problem.
I just took over the amd64 port for Gentoo a couple months back,
so I'm having to get into some stuff I haven't had to do before.
But I'm starting to learn some of the kernel internals/driver
infrastructure, so hopefully I'll be of some actual help in the
near future, assuming my actual job doesn't get in the way ;)

-Brad

Re: r8169 and tg3

[Jan Oravec]

Hi Jeff,

> 1) If you have problems with the tg3 driver, we need to get those bug 
> reports and fix them.  bcm5700 is not going to be merged into the 
> kernel.  Further, there was at least one serious x86-64 issue that was 
> fixed in tg3, and we need to make sure users are testing that driver, 
> not a non-standard driver with bugs of its own.


So here is the bug report:

I have a 4-port card with two Broadcom 5704 chips plugged into PCI-X 1 of
Tyan S2880 motherboard (the onboard 5704's are disabled) with single Opteron
240 processor. When loading tg3 driver, this appears in dmesg:

eth0: Tigon3 [partno(BCM95704A6) rev 2003 PHY(5704)] (PCIX:133MHz:64-bit) 10/100/1000BaseT Ethernet <MAC0>
eth1: Tigon3 [partno(BCM95704A6) rev 2003 PHY(5704)] (PCIX:133MHz:64-bit) 10/100/1000BaseT Ethernet <MAC1>
eth2: Tigon3 [partno(BCM95704A6) rev 2003 PHY(5704)] (PCIX:133MHz:64-bit) 10/100/1000BaseT Ethernet <MAC2>
eth3: Tigon3 [partno(BCM95704A6) rev 2003 PHY(5704)] (PCIX:133MHz:64-bit) 10/100/1000BaseT Ethernet <MAC3>

The IRQs of the ports are:

eth0: IRQ 28
eth1: IRQ 28
eth2: IRQ 28
eth3: IRQ 30


When we try configure eth0 only or eth3 only, it works fine. When only eth1
or eth2 are configured, the 'RUNNING' flag of the interface is missing even
if the cable is plugged thus card does not work. When we turn eth0 up, eth1
and eth2 starts working, but the latency of transfers is horrible.

After some debugging I realized this:

When inserting printk() into tg3_interrupt():

When one of eth0, eth3 are up, I get 10 interrups per second for them (I
heard that it's some workaround for HW bug).

When one of eth1, eth2 are up, I get no interrupt.

When one of eth1, eth2 are up and eth0 is up, I get 10 interrupts per second
for each of them, but I get no interrupts when data are received on eth1 or
eth2. That explains the horrible latency like:

64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=10.0 ms
64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=9.00 ms
64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=8.00 ms
64 bytes from 10.0.0.1: icmp_seq=4 ttl=64 time=7.00 ms
64 bytes from 10.0.0.1: icmp_seq=5 ttl=64 time=6.00 ms
64 bytes from 10.0.0.1: icmp_seq=6 ttl=64 time=5.00 ms
64 bytes from 10.0.0.1: icmp_seq=7 ttl=64 time=4.00 ms
64 bytes from 10.0.0.1: icmp_seq=8 ttl=64 time=3.00 ms
64 bytes from 10.0.0.1: icmp_seq=9 ttl=64 time=2.00 ms
64 bytes from 10.0.0.1: icmp_seq=10 ttl=64 time=1.00 ms
64 bytes from 10.0.0.1: icmp_seq=11 ttl=64 time=99.9 ms
64 bytes from 10.0.0.1: icmp_seq=12 ttl=64 time=98.9 ms
64 bytes from 10.0.0.1: icmp_seq=13 ttl=64 time=97.9 ms
64 bytes from 10.0.0.1: icmp_seq=14 ttl=64 time=96.9 ms


Also, I cannot see any interrupts in /proc/interrupts when I receive packets
thru eth1/eth2.

The problem is repeatable on other exactly same system. The NICs works fine
on 32-bit architecture with 64-bit PCI-X. There are some reports that it
works under 32-bit kernel on Opteron, but I have not tried this yet.

The same problem appears with bcm5700 driver (except 10 ints/sec so the only
way to make it partially work is to make traffic on eth0).

If you need any details or have some suggestions what should I try, let me
know.

Thanks,


Jan

Re: r8169 and tg3

[David S. Miller]

On Tue, 18 Nov 2003 11:00:01 +0100
Jan Oravec <jan.oravec@6com.sk> wrote:

> The same problem appears with bcm5700 driver (except 10 ints/sec so the only
> way to make it partially work is to make traffic on eth0).

This smells of interrupt routing issues on this platform.

Re: r8169 and tg3

[Francois Romieu]

Jeff Garzik <jgarzik@pobox.com> :
[...]
> So, what needs to happen is somebody needs to look at the sis190 changes 
> that went in, and apply those same fixes to r8169.

If no one objects, I can do for r8169 the same thing I did for sis190.
Btw the DMA Rx/Tx rework is already in good shape (I rather like seeing
Jeff improving (S)ATA than coming to copy over his RX and TX handling
code :o) ).

Anyway, the bk-commit patches which were related to sis190 are available at:
http://www.fr.zoreil.com/people/francois/misc/sis190-rework.mbx

--
Ueimor

Re: r8169 and tg3

[Jeff Garzik]

Francois Romieu wrote:
> Jeff Garzik <jgarzik@pobox.com> :
> [...]
> 
>>So, what needs to happen is somebody needs to look at the sis190 changes 
>>that went in, and apply those same fixes to r8169.
> 
> 
> If no one objects, I can do for r8169 the same thing I did for sis190.
> Btw the DMA Rx/Tx rework is already in good shape (I rather like seeing
> Jeff improving (S)ATA than coming to copy over his RX and TX handling
> code :o) ).
> 
> Anyway, the bk-commit patches which were related to sis190 are available at:
> http://www.fr.zoreil.com/people/francois/misc/sis190-rework.mbx


hehe.  I certainly don't object...  You probably want to look over the 
r8169 v1.6 that Brad posted a link to, since it does have some improved 
8169 hardware support (modulo bugs, of course).

	Jeff

Re: r8169 and tg3

[Robert Olsson]

Jeff Garzik writes:

 > hehe.  I certainly don't object...  You probably want to look over the 
 > r8169 v1.6 that Brad posted a link to, since it does have some improved 
 > 8169 hardware support (modulo bugs, of course).

 Yes r8169 can be interesting... I played with it a bit from my head the 
 sending performance was about 900 kpps (pktgen) and RX very OK too about 
 780 kpps with just dropping the skb instead of doing netif_receive.

 Cheers.
						--ro

[patches] 2.6.0-test9 - r8169 DMA API conversion

[Francois Romieu]

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

Jeff Garzik <jgarzik@pobox.com> :
[...]
> hehe.  I certainly don't object...  You probably want to look over the 
> r8169 v1.6 that Brad posted a link to, since it does have some improved 
> 8169 hardware support (modulo bugs, of course).

Ok, will do (hmmm... looks like a free ticket for "Whitespace attacks").

I have attached the usual DMA rework for:
- the Tx descriptors;
- the Rx process (with the original big Rx buffer removed).

Feel free to comment/review. Next patch tomorrow.

--
Ueimor

[-- Attachment #2: r8169-dma-api-tx.patch --]
[-- Type: text/plain, Size: 6675 bytes --]


Conversion of Rx/Tx descriptors to consistent DMA:
- use pci_alloc_consistent() for Rx/Tx descriptors in rtl8169_open()
  (balanced by pci_free_consistent() on error path as well as in
  rtl8169_close());
- removal of the fields {Rx/Tx}DescArrays in struct rtl8169_private
  as there is no need to store a non-256 bytes aligned address any more;
- fix for rtl8169_open() leak when RxBufferRings allocation fails.
  Said allocation is pushed to rtl8169_init_ring() as part of an evil
  cunning plan.


 drivers/net/r8169.c |   99 +++++++++++++++++++++++++++-------------------------
 1 files changed, 52 insertions(+), 47 deletions(-)

diff -puN drivers/net/r8169.c~r8169-dma-api-tx drivers/net/r8169.c
--- linux-2.6.0-test9/drivers/net/r8169.c~r8169-dma-api-tx	2003-11-19 20:30:09.000000000 +0100
+++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-19 21:24:44.000000000 +0100
@@ -89,6 +89,8 @@ static int multicast_filter_limit = 32;
 #define NUM_TX_DESC	64	/* Number of Tx descriptor registers */
 #define NUM_RX_DESC	64	/* Number of Rx descriptor registers */
 #define RX_BUF_SIZE	1536	/* Rx Buffer size */
+#define R8169_TX_RING_BYTES	(NUM_TX_DESC * sizeof(struct TxDesc))
+#define R8169_RX_RING_BYTES	(NUM_RX_DESC * sizeof(struct RxDesc))
 
 #define RTL_MIN_IO_SIZE 0x80
 #define TX_TIMEOUT  (6*HZ)
@@ -280,10 +282,10 @@ struct rtl8169_private {
 	unsigned long cur_rx;	/* Index into the Rx descriptor buffer of next Rx pkt. */
 	unsigned long cur_tx;	/* Index into the Tx descriptor buffer of next Rx pkt. */
 	unsigned long dirty_tx;
-	unsigned char *TxDescArrays;	/* Index of Tx Descriptor buffer */
-	unsigned char *RxDescArrays;	/* Index of Rx Descriptor buffer */
 	struct TxDesc *TxDescArray;	/* Index of 256-alignment Tx Descriptor buffer */
 	struct RxDesc *RxDescArray;	/* Index of 256-alignment Rx Descriptor buffer */
+	dma_addr_t TxPhyAddr;
+	dma_addr_t RxPhyAddr;
 	unsigned char *RxBufferRings;	/* Index of Rx Buffer  */
 	unsigned char *RxBufferRing[NUM_RX_DESC];	/* Index of Rx Buffer array */
 	struct sk_buff *Tx_skbuff[NUM_TX_DESC];	/* Index of Transmit data buffer */
@@ -297,7 +299,7 @@ static int rtl8169_open(struct net_devic
 static int rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance,
 			      struct pt_regs *regs);
-static void rtl8169_init_ring(struct net_device *dev);
+static int rtl8169_init_ring(struct net_device *dev);
 static void rtl8169_hw_start(struct net_device *dev);
 static int rtl8169_close(struct net_device *dev);
 static void rtl8169_set_rx_mode(struct net_device *dev);
@@ -654,52 +656,48 @@ static int
 rtl8169_open(struct net_device *dev)
 {
 	struct rtl8169_private *tp = dev->priv;
+	struct pci_dev *pdev = tp->pci_dev;
 	int retval;
-	u8 diff;
-	u32 TxPhyAddr, RxPhyAddr;
 
 	retval =
 	    request_irq(dev->irq, rtl8169_interrupt, SA_SHIRQ, dev->name, dev);
-	if (retval) {
-		return retval;
-	}
+	if (retval < 0)
+		goto out;
 
-	tp->TxDescArrays =
-	    kmalloc(NUM_TX_DESC * sizeof (struct TxDesc) + 256, GFP_KERNEL);
-	// Tx Desscriptor needs 256 bytes alignment;
-	TxPhyAddr = virt_to_bus(tp->TxDescArrays);
-	diff = 256 - (TxPhyAddr - ((TxPhyAddr >> 8) << 8));
-	TxPhyAddr += diff;
-	tp->TxDescArray = (struct TxDesc *) (tp->TxDescArrays + diff);
-
-	tp->RxDescArrays =
-	    kmalloc(NUM_RX_DESC * sizeof (struct RxDesc) + 256, GFP_KERNEL);
-	// Rx Desscriptor needs 256 bytes alignment;
-	RxPhyAddr = virt_to_bus(tp->RxDescArrays);
-	diff = 256 - (RxPhyAddr - ((RxPhyAddr >> 8) << 8));
-	RxPhyAddr += diff;
-	tp->RxDescArray = (struct RxDesc *) (tp->RxDescArrays + diff);
+	retval = -ENOMEM;
 
-	if (tp->TxDescArrays == NULL || tp->RxDescArrays == NULL) {
-		printk(KERN_INFO
-		       "Allocate RxDescArray or TxDescArray failed\n");
-		free_irq(dev->irq, dev);
-		if (tp->TxDescArrays)
-			kfree(tp->TxDescArrays);
-		if (tp->RxDescArrays)
-			kfree(tp->RxDescArrays);
-		return -ENOMEM;
-	}
-	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
-	if (tp->RxBufferRings == NULL) {
-		printk(KERN_INFO "Allocate RxBufferRing failed\n");
-	}
+	/*
+	 * Rx and Tx desscriptors needs 256 bytes alignment.
+	 * pci_alloc_consistent provides more.
+	 */
+	tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
+					       &tp->TxPhyAddr);
+	if (!tp->TxDescArray)
+		goto err_free_irq;
+
+	tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
+					       &tp->RxPhyAddr);
+	if (!tp->RxDescArray)
+		goto err_free_tx;
+
+	retval = rtl8169_init_ring(dev);
+	if (retval < 0)
+		goto err_free_rx;
 
-	rtl8169_init_ring(dev);
 	rtl8169_hw_start(dev);
 
-	return 0;
+out:
+	return retval;
 
+err_free_rx:
+	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
+			    tp->RxPhyAddr);
+err_free_tx:
+	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
+			    tp->TxPhyAddr);
+err_free_irq:
+	free_irq(dev->irq, dev);
+	goto out;
 }
 
 static void
@@ -739,8 +737,8 @@ rtl8169_hw_start(struct net_device *dev)
 
 	tp->cur_rx = 0;
 
-	RTL_W32(TxDescStartAddr, virt_to_bus(tp->TxDescArray));
-	RTL_W32(RxDescStartAddr, virt_to_bus(tp->RxDescArray));
+	RTL_W32(TxDescStartAddr, tp->TxPhyAddr);
+	RTL_W32(RxDescStartAddr, tp->RxPhyAddr);
 	RTL_W8(Cfg9346, Cfg9346_Lock);
 	udelay(10);
 
@@ -758,12 +756,17 @@ rtl8169_hw_start(struct net_device *dev)
 
 }
 
-static void
-rtl8169_init_ring(struct net_device *dev)
+static int rtl8169_init_ring(struct net_device *dev)
 {
 	struct rtl8169_private *tp = dev->priv;
 	int i;
 
+	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
+	if (tp->RxBufferRings == NULL) {
+		printk(KERN_INFO "Allocate RxBufferRing failed\n");
+		return -ENOMEM;
+	}
+
 	tp->cur_rx = 0;
 	tp->cur_tx = 0;
 	tp->dirty_tx = 0;
@@ -783,6 +786,7 @@ rtl8169_init_ring(struct net_device *dev
 		tp->RxBufferRing[i] = &(tp->RxBufferRings[i * RX_BUF_SIZE]);
 		tp->RxDescArray[i].buf_addr = virt_to_bus(tp->RxBufferRing[i]);
 	}
+	return 0;
 }
 
 static void
@@ -1026,6 +1030,7 @@ static int
 rtl8169_close(struct net_device *dev)
 {
 	struct rtl8169_private *tp = dev->priv;
+	struct pci_dev *pdev = tp->pci_dev;
 	void *ioaddr = tp->mmio_addr;
 	int i;
 
@@ -1049,10 +1054,10 @@ rtl8169_close(struct net_device *dev)
 	free_irq(dev->irq, dev);
 
 	rtl8169_tx_clear(tp);
-	kfree(tp->TxDescArrays);
-	kfree(tp->RxDescArrays);
-	tp->TxDescArrays = NULL;
-	tp->RxDescArrays = NULL;
+	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
+			    tp->RxPhyAddr);
+	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
+			    tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
 	tp->RxDescArray = NULL;
 	kfree(tp->RxBufferRings);

_

[-- Attachment #3: r8169-dma-api-data-buffers.patch --]
[-- Type: text/plain, Size: 10696 bytes --]


Conversion of Rx data buffers to PCI DMA
- endianness is kept in a fscked state as it is in the original code
  (will be adressed in a later patch);
- rtl8169_rx_clear() walks the buffer ring and releases the allocated
  data buffers. It needs to be used in two places: 
  - rtl8169_init_ring() failure path;
  - normal device release (i.e. rtl8169_close);
- rtl8169_free_rx_skb() releases a Rx data buffer. Mostly an helper
  for rtl8169_rx_clear(). As such it must:
  - unmap the memory area;
  - release the skb;
  - prevent the ring descriptor from being used again;
- rtl8169_alloc_rx_skb() prepares a Rx data buffer for use.
  As such it must:
  - allocate an skb;
  - map the memory area;
  - reflect the changes in the ring descriptor.
  This function is balanced by rtl8169_free_rx_skb().
- rtl8169_unmap_rx() simply helps with the 80-columns limit.
- rtl8169_rx_fill() walks a given range of the buffer ring and
  try to turn any descriptor into a ready to use one. It returns the
  count of modified descriptors and exits if an allocation fails.
  It can be seen as balanced by rtl8169_rx_clear(). Motivation:
  - partially abstract the (usually big) piece of code for the refill
    logic at the end of the Rx interrupt;
  - factorize the refill logic and the initial ring setup.
- simple conversion of rtl8169_rx_interrupt() without rx_copybreak
  (will be adressed in a later patch).


 drivers/net/r8169.c |  235 +++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 161 insertions(+), 74 deletions(-)

diff -puN drivers/net/r8169.c~r8169-dma-api-data-buffers drivers/net/r8169.c
--- linux-2.6.0-test9/drivers/net/r8169.c~r8169-dma-api-data-buffers	2003-11-19 21:26:45.000000000 +0100
+++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-20 00:45:00.000000000 +0100
@@ -279,15 +279,15 @@ struct rtl8169_private {
 	struct net_device_stats stats;	/* statistics of net device */
 	spinlock_t lock;	/* spin lock flag */
 	int chipset;
-	unsigned long cur_rx;	/* Index into the Rx descriptor buffer of next Rx pkt. */
-	unsigned long cur_tx;	/* Index into the Tx descriptor buffer of next Rx pkt. */
-	unsigned long dirty_tx;
+	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
+	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
+	u32 dirty_rx;
+	u32 dirty_tx;
 	struct TxDesc *TxDescArray;	/* Index of 256-alignment Tx Descriptor buffer */
 	struct RxDesc *RxDescArray;	/* Index of 256-alignment Rx Descriptor buffer */
 	dma_addr_t TxPhyAddr;
 	dma_addr_t RxPhyAddr;
-	unsigned char *RxBufferRings;	/* Index of Rx Buffer  */
-	unsigned char *RxBufferRing[NUM_RX_DESC];	/* Index of Rx Buffer array */
+	struct sk_buff *Rx_skbuff[NUM_RX_DESC];	/* Rx data buffers */
 	struct sk_buff *Tx_skbuff[NUM_TX_DESC];	/* Index of Transmit data buffer */
 };
 
@@ -756,37 +756,119 @@ rtl8169_hw_start(struct net_device *dev)
 
 }
 
-static int rtl8169_init_ring(struct net_device *dev)
+static inline void rtl8169_make_unusable_by_asic(struct RxDesc *desc)
+{
+	desc->buf_addr = 0xdeadbeef;
+	desc->status = EORbit; 
+}
+
+static void rtl8169_free_rx_skb(struct pci_dev *pdev, struct sk_buff **sk_buff,
+				struct RxDesc *desc)
+{
+	pci_unmap_single(pdev, desc->buf_addr, RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+	dev_kfree_skb(*sk_buff);
+	*sk_buff = NULL;
+	rtl8169_make_unusable_by_asic(desc);
+}
+
+static inline void rtl8169_give_to_asic(struct RxDesc *desc, dma_addr_t mapping)
+{
+	desc->buf_addr = mapping;
+	desc->status = OWNbit + RX_BUF_SIZE;
+}
+
+static int rtl8169_alloc_rx_skb(struct pci_dev *pdev, struct net_device *dev,
+				struct sk_buff **sk_buff, struct RxDesc *desc)
+{
+	struct sk_buff *skb;
+	dma_addr_t mapping;
+	int ret = 0;
+
+	skb = dev_alloc_skb(RX_BUF_SIZE);
+	if (!skb)
+		goto err_out;
+
+	skb->dev = dev;
+	skb_reserve(skb, 2);
+	*sk_buff = skb;
+
+	mapping = pci_map_single(pdev, skb->tail, RX_BUF_SIZE,
+				 PCI_DMA_FROMDEVICE);
+
+	rtl8169_give_to_asic(desc, mapping);
+
+out:
+	return ret;
+
+err_out:
+	ret = -ENOMEM;
+	rtl8169_make_unusable_by_asic(desc);
+	goto out;
+}
+
+static void rtl8169_rx_clear(struct rtl8169_private *tp)
 {
-	struct rtl8169_private *tp = dev->priv;
 	int i;
 
-	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
-	if (tp->RxBufferRings == NULL) {
-		printk(KERN_INFO "Allocate RxBufferRing failed\n");
-		return -ENOMEM;
+	for (i = 0; i < NUM_RX_DESC; i++) {
+		if (tp->Rx_skbuff[i]) {
+			rtl8169_free_rx_skb(tp->pci_dev, tp->Rx_skbuff + i,
+					    tp->RxDescArray + i);
+		}
 	}
+}
 
-	tp->cur_rx = 0;
-	tp->cur_tx = 0;
-	tp->dirty_tx = 0;
+static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
+			   u32 start, u32 end)
+{
+	u32 cur;
+	
+	for (cur = start; end - start > 0; cur++) {
+		int ret, i = cur % NUM_RX_DESC;
+
+		if (tp->Rx_skbuff[i])
+			continue;
+			
+		ret = rtl8169_alloc_rx_skb(tp->pci_dev, dev, tp->Rx_skbuff + i,
+					   tp->RxDescArray + i);
+		if (ret < 0)
+			break;
+	}
+	return cur - start;
+}
+
+static inline void rtl8169_mark_as_last_descriptor(struct RxDesc *desc)
+{
+	desc->status |= EORbit;
+}
+
+static inline void rtl8169_unmark_as_last_descriptor(struct RxDesc *desc)
+{
+	desc->status &= ~EORbit;
+}
+
+static int rtl8169_init_ring(struct net_device *dev)
+{
+	struct rtl8169_private *tp = dev->priv;
+
+	tp->cur_rx = tp->dirty_rx = 0;
+	tp->cur_tx = tp->dirty_tx = 0;
 	memset(tp->TxDescArray, 0x0, NUM_TX_DESC * sizeof (struct TxDesc));
 	memset(tp->RxDescArray, 0x0, NUM_RX_DESC * sizeof (struct RxDesc));
 
-	for (i = 0; i < NUM_TX_DESC; i++) {
-		tp->Tx_skbuff[i] = NULL;
-	}
-	for (i = 0; i < NUM_RX_DESC; i++) {
-		if (i == (NUM_RX_DESC - 1))
-			tp->RxDescArray[i].status =
-			    (OWNbit | EORbit) + RX_BUF_SIZE;
-		else
-			tp->RxDescArray[i].status = OWNbit + RX_BUF_SIZE;
+	memset(tp->Tx_skbuff, 0x0, NUM_TX_DESC * sizeof(struct sk_buff *));
+	memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
+
+	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC) != NUM_RX_DESC)
+		goto err_out;
+
+	rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
 
-		tp->RxBufferRing[i] = &(tp->RxBufferRings[i * RX_BUF_SIZE]);
-		tp->RxDescArray[i].buf_addr = virt_to_bus(tp->RxBufferRing[i]);
-	}
 	return 0;
+
+err_out:
+	rtl8169_rx_clear(tp);
+	return -ENOMEM;
 }
 
 static void
@@ -906,70 +988,77 @@ rtl8169_tx_interrupt(struct net_device *
 	}
 }
 
+static inline void rtl8169_unmap_rx(struct pci_dev *pdev, struct RxDesc *desc)
+{
+	pci_dma_sync_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
+			    PCI_DMA_FROMDEVICE);
+	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
+			 PCI_DMA_FROMDEVICE);
+}
+
 static void
 rtl8169_rx_interrupt(struct net_device *dev, struct rtl8169_private *tp,
 		     void *ioaddr)
 {
-	int cur_rx;
-	struct sk_buff *skb;
-	int pkt_size = 0;
+	int cur_rx, delta;
 
 	assert(dev != NULL);
 	assert(tp != NULL);
 	assert(ioaddr != NULL);
 
-	cur_rx = tp->cur_rx;
+	cur_rx = tp->cur_rx % RX_BUF_SIZE;
 
 	while ((tp->RxDescArray[cur_rx].status & OWNbit) == 0) {
+		u32 status = tp->RxDescArray[cur_rx].status;
 
-		if (tp->RxDescArray[cur_rx].status & RxRES) {
+		if (status & RxRES) {
 			printk(KERN_INFO "%s: Rx ERROR!!!\n", dev->name);
 			tp->stats.rx_errors++;
-			if (tp->RxDescArray[cur_rx].status & (RxRWT | RxRUNT))
+			if (status & (RxRWT | RxRUNT))
 				tp->stats.rx_length_errors++;
-			if (tp->RxDescArray[cur_rx].status & RxCRC)
+			if (status & RxCRC)
 				tp->stats.rx_crc_errors++;
 		} else {
-			pkt_size =
-			    (int) (tp->RxDescArray[cur_rx].
-				   status & 0x00001FFF) - 4;
-			skb = dev_alloc_skb(pkt_size + 2);
-			if (skb != NULL) {
-				skb->dev = dev;
-				skb_reserve(skb, 2);	// 16 byte align the IP fields. //
-				eth_copy_and_sum(skb, tp->RxBufferRing[cur_rx],
-						 pkt_size, 0);
-				skb_put(skb, pkt_size);
-				skb->protocol = eth_type_trans(skb, dev);
-				netif_rx(skb);
-
-				if (cur_rx == (NUM_RX_DESC - 1))
-					tp->RxDescArray[cur_rx].status =
-					    (OWNbit | EORbit) + RX_BUF_SIZE;
-				else
-					tp->RxDescArray[cur_rx].status =
-					    OWNbit + RX_BUF_SIZE;
-
-				tp->RxDescArray[cur_rx].buf_addr =
-				    virt_to_bus(tp->RxBufferRing[cur_rx]);
-				dev->last_rx = jiffies;
-				tp->stats.rx_bytes += pkt_size;
-				tp->stats.rx_packets++;
-			} else {
-				printk(KERN_WARNING
-				       "%s: Memory squeeze, deferring packet.\n",
-				       dev->name);
-				/* We should check that some rx space is free.
-				   If not, free one and mark stats->rx_dropped++. */
-				tp->stats.rx_dropped++;
-			}
-		}
+			struct sk_buff *skb = tp->Rx_skbuff[cur_rx];
+			int pkt_size = (status & 0x00001FFF) - 4;
+
+			rtl8169_unmap_rx(tp->pci_dev, tp->RxDescArray + cur_rx);
+
+			skb_put(skb, pkt_size);
+			skb->protocol = eth_type_trans(skb, dev);
+			netif_rx(skb);
 
-		cur_rx = (cur_rx + 1) % NUM_RX_DESC;
+			tp->Rx_skbuff[cur_rx] = NULL;
 
+			dev->last_rx = jiffies;
+			tp->stats.rx_bytes += pkt_size;
+			tp->stats.rx_packets++;
+		}
+		
+		tp->cur_rx++; 
+		cur_rx = tp->cur_rx % NUM_RX_DESC;
 	}
 
-	tp->cur_rx = cur_rx;
+	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
+	if (delta > 0) {
+		u32 old_last = (tp->dirty_rx - 1) % NUM_RX_DESC;
+
+		tp->dirty_rx += delta;
+		rtl8169_mark_as_last_descriptor(tp->RxDescArray +
+						(tp->dirty_rx - 1)%NUM_RX_DESC);
+		rtl8169_unmark_as_last_descriptor(tp->RxDescArray + old_last);
+	} else if (delta < 0)
+		printk(KERN_INFO "%s: no Rx buffer allocated\n", dev->name);
+
+	/*
+	 * FIXME: until there is periodic timer to try and refill the ring,
+	 * a temporary shortage may definitely kill the Rx process.
+	 * - disable the asic to try and avoid an overflow and kick it again
+	 *   after refill ?
+	 * - how do others driver handle this condition (Uh oh...).
+	 */
+	if (tp->dirty_rx + NUM_RX_DESC == tp->cur_rx)
+		printk(KERN_EMERG "%s: Rx buffers exhausted\n", dev->name);
 }
 
 /* The interrupt handler does all of the Rx thread work and cleans up after the Tx thread. */
@@ -1032,7 +1121,6 @@ rtl8169_close(struct net_device *dev)
 	struct rtl8169_private *tp = dev->priv;
 	struct pci_dev *pdev = tp->pci_dev;
 	void *ioaddr = tp->mmio_addr;
-	int i;
 
 	netif_stop_queue(dev);
 
@@ -1054,16 +1142,15 @@ rtl8169_close(struct net_device *dev)
 	free_irq(dev->irq, dev);
 
 	rtl8169_tx_clear(tp);
+
+	rtl8169_rx_clear(tp);
+
 	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			    tp->RxPhyAddr);
 	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
 			    tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
 	tp->RxDescArray = NULL;
-	kfree(tp->RxBufferRings);
-	for (i = 0; i < NUM_RX_DESC; i++) {
-		tp->RxBufferRing[i] = NULL;
-	}
 
 	return 0;
 }

_

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Brad House]

Just because I'm way to lazy to actually look, is this
against the original r8169, or my patch from the v1.6
driver ??

-Brad

Francois Romieu wrote:
> Jeff Garzik <jgarzik@pobox.com> :
> [...]
> 
>>hehe.  I certainly don't object...  You probably want to look over the 
>>r8169 v1.6 that Brad posted a link to, since it does have some improved 
>>8169 hardware support (modulo bugs, of course).
> 
> 
> Ok, will do (hmmm... looks like a free ticket for "Whitespace attacks").
> 
> I have attached the usual DMA rework for:
> - the Tx descriptors;
> - the Rx process (with the original big Rx buffer removed).
> 
> Feel free to comment/review. Next patch tomorrow.
> 
> --
> Ueimor
> 
> 
> ------------------------------------------------------------------------
> 
> 
> Conversion of Rx/Tx descriptors to consistent DMA:
> - use pci_alloc_consistent() for Rx/Tx descriptors in rtl8169_open()
>   (balanced by pci_free_consistent() on error path as well as in
>   rtl8169_close());
> - removal of the fields {Rx/Tx}DescArrays in struct rtl8169_private
>   as there is no need to store a non-256 bytes aligned address any more;
> - fix for rtl8169_open() leak when RxBufferRings allocation fails.
>   Said allocation is pushed to rtl8169_init_ring() as part of an evil
>   cunning plan.
> 
> 
>  drivers/net/r8169.c |   99 +++++++++++++++++++++++++++-------------------------
>  1 files changed, 52 insertions(+), 47 deletions(-)
> 
> diff -puN drivers/net/r8169.c~r8169-dma-api-tx drivers/net/r8169.c
> --- linux-2.6.0-test9/drivers/net/r8169.c~r8169-dma-api-tx	2003-11-19 20:30:09.000000000 +0100
> +++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-19 21:24:44.000000000 +0100
> @@ -89,6 +89,8 @@ static int multicast_filter_limit = 32;
>  #define NUM_TX_DESC	64	/* Number of Tx descriptor registers */
>  #define NUM_RX_DESC	64	/* Number of Rx descriptor registers */
>  #define RX_BUF_SIZE	1536	/* Rx Buffer size */
> +#define R8169_TX_RING_BYTES	(NUM_TX_DESC * sizeof(struct TxDesc))
> +#define R8169_RX_RING_BYTES	(NUM_RX_DESC * sizeof(struct RxDesc))
>  
>  #define RTL_MIN_IO_SIZE 0x80
>  #define TX_TIMEOUT  (6*HZ)
> @@ -280,10 +282,10 @@ struct rtl8169_private {
>  	unsigned long cur_rx;	/* Index into the Rx descriptor buffer of next Rx pkt. */
>  	unsigned long cur_tx;	/* Index into the Tx descriptor buffer of next Rx pkt. */
>  	unsigned long dirty_tx;
> -	unsigned char *TxDescArrays;	/* Index of Tx Descriptor buffer */
> -	unsigned char *RxDescArrays;	/* Index of Rx Descriptor buffer */
>  	struct TxDesc *TxDescArray;	/* Index of 256-alignment Tx Descriptor buffer */
>  	struct RxDesc *RxDescArray;	/* Index of 256-alignment Rx Descriptor buffer */
> +	dma_addr_t TxPhyAddr;
> +	dma_addr_t RxPhyAddr;
>  	unsigned char *RxBufferRings;	/* Index of Rx Buffer  */
>  	unsigned char *RxBufferRing[NUM_RX_DESC];	/* Index of Rx Buffer array */
>  	struct sk_buff *Tx_skbuff[NUM_TX_DESC];	/* Index of Transmit data buffer */
> @@ -297,7 +299,7 @@ static int rtl8169_open(struct net_devic
>  static int rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev);
>  static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance,
>  			      struct pt_regs *regs);
> -static void rtl8169_init_ring(struct net_device *dev);
> +static int rtl8169_init_ring(struct net_device *dev);
>  static void rtl8169_hw_start(struct net_device *dev);
>  static int rtl8169_close(struct net_device *dev);
>  static void rtl8169_set_rx_mode(struct net_device *dev);
> @@ -654,52 +656,48 @@ static int
>  rtl8169_open(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = dev->priv;
> +	struct pci_dev *pdev = tp->pci_dev;
>  	int retval;
> -	u8 diff;
> -	u32 TxPhyAddr, RxPhyAddr;
>  
>  	retval =
>  	    request_irq(dev->irq, rtl8169_interrupt, SA_SHIRQ, dev->name, dev);
> -	if (retval) {
> -		return retval;
> -	}
> +	if (retval < 0)
> +		goto out;
>  
> -	tp->TxDescArrays =
> -	    kmalloc(NUM_TX_DESC * sizeof (struct TxDesc) + 256, GFP_KERNEL);
> -	// Tx Desscriptor needs 256 bytes alignment;
> -	TxPhyAddr = virt_to_bus(tp->TxDescArrays);
> -	diff = 256 - (TxPhyAddr - ((TxPhyAddr >> 8) << 8));
> -	TxPhyAddr += diff;
> -	tp->TxDescArray = (struct TxDesc *) (tp->TxDescArrays + diff);
> -
> -	tp->RxDescArrays =
> -	    kmalloc(NUM_RX_DESC * sizeof (struct RxDesc) + 256, GFP_KERNEL);
> -	// Rx Desscriptor needs 256 bytes alignment;
> -	RxPhyAddr = virt_to_bus(tp->RxDescArrays);
> -	diff = 256 - (RxPhyAddr - ((RxPhyAddr >> 8) << 8));
> -	RxPhyAddr += diff;
> -	tp->RxDescArray = (struct RxDesc *) (tp->RxDescArrays + diff);
> +	retval = -ENOMEM;
>  
> -	if (tp->TxDescArrays == NULL || tp->RxDescArrays == NULL) {
> -		printk(KERN_INFO
> -		       "Allocate RxDescArray or TxDescArray failed\n");
> -		free_irq(dev->irq, dev);
> -		if (tp->TxDescArrays)
> -			kfree(tp->TxDescArrays);
> -		if (tp->RxDescArrays)
> -			kfree(tp->RxDescArrays);
> -		return -ENOMEM;
> -	}
> -	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
> -	if (tp->RxBufferRings == NULL) {
> -		printk(KERN_INFO "Allocate RxBufferRing failed\n");
> -	}
> +	/*
> +	 * Rx and Tx desscriptors needs 256 bytes alignment.
> +	 * pci_alloc_consistent provides more.
> +	 */
> +	tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
> +					       &tp->TxPhyAddr);
> +	if (!tp->TxDescArray)
> +		goto err_free_irq;
> +
> +	tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
> +					       &tp->RxPhyAddr);
> +	if (!tp->RxDescArray)
> +		goto err_free_tx;
> +
> +	retval = rtl8169_init_ring(dev);
> +	if (retval < 0)
> +		goto err_free_rx;
>  
> -	rtl8169_init_ring(dev);
>  	rtl8169_hw_start(dev);
>  
> -	return 0;
> +out:
> +	return retval;
>  
> +err_free_rx:
> +	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> +			    tp->RxPhyAddr);
> +err_free_tx:
> +	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> +			    tp->TxPhyAddr);
> +err_free_irq:
> +	free_irq(dev->irq, dev);
> +	goto out;
>  }
>  
>  static void
> @@ -739,8 +737,8 @@ rtl8169_hw_start(struct net_device *dev)
>  
>  	tp->cur_rx = 0;
>  
> -	RTL_W32(TxDescStartAddr, virt_to_bus(tp->TxDescArray));
> -	RTL_W32(RxDescStartAddr, virt_to_bus(tp->RxDescArray));
> +	RTL_W32(TxDescStartAddr, tp->TxPhyAddr);
> +	RTL_W32(RxDescStartAddr, tp->RxPhyAddr);
>  	RTL_W8(Cfg9346, Cfg9346_Lock);
>  	udelay(10);
>  
> @@ -758,12 +756,17 @@ rtl8169_hw_start(struct net_device *dev)
>  
>  }
>  
> -static void
> -rtl8169_init_ring(struct net_device *dev)
> +static int rtl8169_init_ring(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = dev->priv;
>  	int i;
>  
> +	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
> +	if (tp->RxBufferRings == NULL) {
> +		printk(KERN_INFO "Allocate RxBufferRing failed\n");
> +		return -ENOMEM;
> +	}
> +
>  	tp->cur_rx = 0;
>  	tp->cur_tx = 0;
>  	tp->dirty_tx = 0;
> @@ -783,6 +786,7 @@ rtl8169_init_ring(struct net_device *dev
>  		tp->RxBufferRing[i] = &(tp->RxBufferRings[i * RX_BUF_SIZE]);
>  		tp->RxDescArray[i].buf_addr = virt_to_bus(tp->RxBufferRing[i]);
>  	}
> +	return 0;
>  }
>  
>  static void
> @@ -1026,6 +1030,7 @@ static int
>  rtl8169_close(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = dev->priv;
> +	struct pci_dev *pdev = tp->pci_dev;
>  	void *ioaddr = tp->mmio_addr;
>  	int i;
>  
> @@ -1049,10 +1054,10 @@ rtl8169_close(struct net_device *dev)
>  	free_irq(dev->irq, dev);
>  
>  	rtl8169_tx_clear(tp);
> -	kfree(tp->TxDescArrays);
> -	kfree(tp->RxDescArrays);
> -	tp->TxDescArrays = NULL;
> -	tp->RxDescArrays = NULL;
> +	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> +			    tp->RxPhyAddr);
> +	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> +			    tp->TxPhyAddr);
>  	tp->TxDescArray = NULL;
>  	tp->RxDescArray = NULL;
>  	kfree(tp->RxBufferRings);
> 
> _
> 
> 
> ------------------------------------------------------------------------
> 
> 
> Conversion of Rx data buffers to PCI DMA
> - endianness is kept in a fscked state as it is in the original code
>   (will be adressed in a later patch);
> - rtl8169_rx_clear() walks the buffer ring and releases the allocated
>   data buffers. It needs to be used in two places: 
>   - rtl8169_init_ring() failure path;
>   - normal device release (i.e. rtl8169_close);
> - rtl8169_free_rx_skb() releases a Rx data buffer. Mostly an helper
>   for rtl8169_rx_clear(). As such it must:
>   - unmap the memory area;
>   - release the skb;
>   - prevent the ring descriptor from being used again;
> - rtl8169_alloc_rx_skb() prepares a Rx data buffer for use.
>   As such it must:
>   - allocate an skb;
>   - map the memory area;
>   - reflect the changes in the ring descriptor.
>   This function is balanced by rtl8169_free_rx_skb().
> - rtl8169_unmap_rx() simply helps with the 80-columns limit.
> - rtl8169_rx_fill() walks a given range of the buffer ring and
>   try to turn any descriptor into a ready to use one. It returns the
>   count of modified descriptors and exits if an allocation fails.
>   It can be seen as balanced by rtl8169_rx_clear(). Motivation:
>   - partially abstract the (usually big) piece of code for the refill
>     logic at the end of the Rx interrupt;
>   - factorize the refill logic and the initial ring setup.
> - simple conversion of rtl8169_rx_interrupt() without rx_copybreak
>   (will be adressed in a later patch).
> 
> 
>  drivers/net/r8169.c |  235 +++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 161 insertions(+), 74 deletions(-)
> 
> diff -puN drivers/net/r8169.c~r8169-dma-api-data-buffers drivers/net/r8169.c
> --- linux-2.6.0-test9/drivers/net/r8169.c~r8169-dma-api-data-buffers	2003-11-19 21:26:45.000000000 +0100
> +++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-20 00:45:00.000000000 +0100
> @@ -279,15 +279,15 @@ struct rtl8169_private {
>  	struct net_device_stats stats;	/* statistics of net device */
>  	spinlock_t lock;	/* spin lock flag */
>  	int chipset;
> -	unsigned long cur_rx;	/* Index into the Rx descriptor buffer of next Rx pkt. */
> -	unsigned long cur_tx;	/* Index into the Tx descriptor buffer of next Rx pkt. */
> -	unsigned long dirty_tx;
> +	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
> +	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
> +	u32 dirty_rx;
> +	u32 dirty_tx;
>  	struct TxDesc *TxDescArray;	/* Index of 256-alignment Tx Descriptor buffer */
>  	struct RxDesc *RxDescArray;	/* Index of 256-alignment Rx Descriptor buffer */
>  	dma_addr_t TxPhyAddr;
>  	dma_addr_t RxPhyAddr;
> -	unsigned char *RxBufferRings;	/* Index of Rx Buffer  */
> -	unsigned char *RxBufferRing[NUM_RX_DESC];	/* Index of Rx Buffer array */
> +	struct sk_buff *Rx_skbuff[NUM_RX_DESC];	/* Rx data buffers */
>  	struct sk_buff *Tx_skbuff[NUM_TX_DESC];	/* Index of Transmit data buffer */
>  };
>  
> @@ -756,37 +756,119 @@ rtl8169_hw_start(struct net_device *dev)
>  
>  }
>  
> -static int rtl8169_init_ring(struct net_device *dev)
> +static inline void rtl8169_make_unusable_by_asic(struct RxDesc *desc)
> +{
> +	desc->buf_addr = 0xdeadbeef;
> +	desc->status = EORbit; 
> +}
> +
> +static void rtl8169_free_rx_skb(struct pci_dev *pdev, struct sk_buff **sk_buff,
> +				struct RxDesc *desc)
> +{
> +	pci_unmap_single(pdev, desc->buf_addr, RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
> +	dev_kfree_skb(*sk_buff);
> +	*sk_buff = NULL;
> +	rtl8169_make_unusable_by_asic(desc);
> +}
> +
> +static inline void rtl8169_give_to_asic(struct RxDesc *desc, dma_addr_t mapping)
> +{
> +	desc->buf_addr = mapping;
> +	desc->status = OWNbit + RX_BUF_SIZE;
> +}
> +
> +static int rtl8169_alloc_rx_skb(struct pci_dev *pdev, struct net_device *dev,
> +				struct sk_buff **sk_buff, struct RxDesc *desc)
> +{
> +	struct sk_buff *skb;
> +	dma_addr_t mapping;
> +	int ret = 0;
> +
> +	skb = dev_alloc_skb(RX_BUF_SIZE);
> +	if (!skb)
> +		goto err_out;
> +
> +	skb->dev = dev;
> +	skb_reserve(skb, 2);
> +	*sk_buff = skb;
> +
> +	mapping = pci_map_single(pdev, skb->tail, RX_BUF_SIZE,
> +				 PCI_DMA_FROMDEVICE);
> +
> +	rtl8169_give_to_asic(desc, mapping);
> +
> +out:
> +	return ret;
> +
> +err_out:
> +	ret = -ENOMEM;
> +	rtl8169_make_unusable_by_asic(desc);
> +	goto out;
> +}
> +
> +static void rtl8169_rx_clear(struct rtl8169_private *tp)
>  {
> -	struct rtl8169_private *tp = dev->priv;
>  	int i;
>  
> -	tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
> -	if (tp->RxBufferRings == NULL) {
> -		printk(KERN_INFO "Allocate RxBufferRing failed\n");
> -		return -ENOMEM;
> +	for (i = 0; i < NUM_RX_DESC; i++) {
> +		if (tp->Rx_skbuff[i]) {
> +			rtl8169_free_rx_skb(tp->pci_dev, tp->Rx_skbuff + i,
> +					    tp->RxDescArray + i);
> +		}
>  	}
> +}
>  
> -	tp->cur_rx = 0;
> -	tp->cur_tx = 0;
> -	tp->dirty_tx = 0;
> +static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
> +			   u32 start, u32 end)
> +{
> +	u32 cur;
> +	
> +	for (cur = start; end - start > 0; cur++) {
> +		int ret, i = cur % NUM_RX_DESC;
> +
> +		if (tp->Rx_skbuff[i])
> +			continue;
> +			
> +		ret = rtl8169_alloc_rx_skb(tp->pci_dev, dev, tp->Rx_skbuff + i,
> +					   tp->RxDescArray + i);
> +		if (ret < 0)
> +			break;
> +	}
> +	return cur - start;
> +}
> +
> +static inline void rtl8169_mark_as_last_descriptor(struct RxDesc *desc)
> +{
> +	desc->status |= EORbit;
> +}
> +
> +static inline void rtl8169_unmark_as_last_descriptor(struct RxDesc *desc)
> +{
> +	desc->status &= ~EORbit;
> +}
> +
> +static int rtl8169_init_ring(struct net_device *dev)
> +{
> +	struct rtl8169_private *tp = dev->priv;
> +
> +	tp->cur_rx = tp->dirty_rx = 0;
> +	tp->cur_tx = tp->dirty_tx = 0;
>  	memset(tp->TxDescArray, 0x0, NUM_TX_DESC * sizeof (struct TxDesc));
>  	memset(tp->RxDescArray, 0x0, NUM_RX_DESC * sizeof (struct RxDesc));
>  
> -	for (i = 0; i < NUM_TX_DESC; i++) {
> -		tp->Tx_skbuff[i] = NULL;
> -	}
> -	for (i = 0; i < NUM_RX_DESC; i++) {
> -		if (i == (NUM_RX_DESC - 1))
> -			tp->RxDescArray[i].status =
> -			    (OWNbit | EORbit) + RX_BUF_SIZE;
> -		else
> -			tp->RxDescArray[i].status = OWNbit + RX_BUF_SIZE;
> +	memset(tp->Tx_skbuff, 0x0, NUM_TX_DESC * sizeof(struct sk_buff *));
> +	memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
> +
> +	if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC) != NUM_RX_DESC)
> +		goto err_out;
> +
> +	rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
>  
> -		tp->RxBufferRing[i] = &(tp->RxBufferRings[i * RX_BUF_SIZE]);
> -		tp->RxDescArray[i].buf_addr = virt_to_bus(tp->RxBufferRing[i]);
> -	}
>  	return 0;
> +
> +err_out:
> +	rtl8169_rx_clear(tp);
> +	return -ENOMEM;
>  }
>  
>  static void
> @@ -906,70 +988,77 @@ rtl8169_tx_interrupt(struct net_device *
>  	}
>  }
>  
> +static inline void rtl8169_unmap_rx(struct pci_dev *pdev, struct RxDesc *desc)
> +{
> +	pci_dma_sync_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
> +			    PCI_DMA_FROMDEVICE);
> +	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
> +			 PCI_DMA_FROMDEVICE);
> +}
> +
>  static void
>  rtl8169_rx_interrupt(struct net_device *dev, struct rtl8169_private *tp,
>  		     void *ioaddr)
>  {
> -	int cur_rx;
> -	struct sk_buff *skb;
> -	int pkt_size = 0;
> +	int cur_rx, delta;
>  
>  	assert(dev != NULL);
>  	assert(tp != NULL);
>  	assert(ioaddr != NULL);
>  
> -	cur_rx = tp->cur_rx;
> +	cur_rx = tp->cur_rx % RX_BUF_SIZE;
>  
>  	while ((tp->RxDescArray[cur_rx].status & OWNbit) == 0) {
> +		u32 status = tp->RxDescArray[cur_rx].status;
>  
> -		if (tp->RxDescArray[cur_rx].status & RxRES) {
> +		if (status & RxRES) {
>  			printk(KERN_INFO "%s: Rx ERROR!!!\n", dev->name);
>  			tp->stats.rx_errors++;
> -			if (tp->RxDescArray[cur_rx].status & (RxRWT | RxRUNT))
> +			if (status & (RxRWT | RxRUNT))
>  				tp->stats.rx_length_errors++;
> -			if (tp->RxDescArray[cur_rx].status & RxCRC)
> +			if (status & RxCRC)
>  				tp->stats.rx_crc_errors++;
>  		} else {
> -			pkt_size =
> -			    (int) (tp->RxDescArray[cur_rx].
> -				   status & 0x00001FFF) - 4;
> -			skb = dev_alloc_skb(pkt_size + 2);
> -			if (skb != NULL) {
> -				skb->dev = dev;
> -				skb_reserve(skb, 2);	// 16 byte align the IP fields. //
> -				eth_copy_and_sum(skb, tp->RxBufferRing[cur_rx],
> -						 pkt_size, 0);
> -				skb_put(skb, pkt_size);
> -				skb->protocol = eth_type_trans(skb, dev);
> -				netif_rx(skb);
> -
> -				if (cur_rx == (NUM_RX_DESC - 1))
> -					tp->RxDescArray[cur_rx].status =
> -					    (OWNbit | EORbit) + RX_BUF_SIZE;
> -				else
> -					tp->RxDescArray[cur_rx].status =
> -					    OWNbit + RX_BUF_SIZE;
> -
> -				tp->RxDescArray[cur_rx].buf_addr =
> -				    virt_to_bus(tp->RxBufferRing[cur_rx]);
> -				dev->last_rx = jiffies;
> -				tp->stats.rx_bytes += pkt_size;
> -				tp->stats.rx_packets++;
> -			} else {
> -				printk(KERN_WARNING
> -				       "%s: Memory squeeze, deferring packet.\n",
> -				       dev->name);
> -				/* We should check that some rx space is free.
> -				   If not, free one and mark stats->rx_dropped++. */
> -				tp->stats.rx_dropped++;
> -			}
> -		}
> +			struct sk_buff *skb = tp->Rx_skbuff[cur_rx];
> +			int pkt_size = (status & 0x00001FFF) - 4;
> +
> +			rtl8169_unmap_rx(tp->pci_dev, tp->RxDescArray + cur_rx);
> +
> +			skb_put(skb, pkt_size);
> +			skb->protocol = eth_type_trans(skb, dev);
> +			netif_rx(skb);
>  
> -		cur_rx = (cur_rx + 1) % NUM_RX_DESC;
> +			tp->Rx_skbuff[cur_rx] = NULL;
>  
> +			dev->last_rx = jiffies;
> +			tp->stats.rx_bytes += pkt_size;
> +			tp->stats.rx_packets++;
> +		}
> +		
> +		tp->cur_rx++; 
> +		cur_rx = tp->cur_rx % NUM_RX_DESC;
>  	}
>  
> -	tp->cur_rx = cur_rx;
> +	delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
> +	if (delta > 0) {
> +		u32 old_last = (tp->dirty_rx - 1) % NUM_RX_DESC;
> +
> +		tp->dirty_rx += delta;
> +		rtl8169_mark_as_last_descriptor(tp->RxDescArray +
> +						(tp->dirty_rx - 1)%NUM_RX_DESC);
> +		rtl8169_unmark_as_last_descriptor(tp->RxDescArray + old_last);
> +	} else if (delta < 0)
> +		printk(KERN_INFO "%s: no Rx buffer allocated\n", dev->name);
> +
> +	/*
> +	 * FIXME: until there is periodic timer to try and refill the ring,
> +	 * a temporary shortage may definitely kill the Rx process.
> +	 * - disable the asic to try and avoid an overflow and kick it again
> +	 *   after refill ?
> +	 * - how do others driver handle this condition (Uh oh...).
> +	 */
> +	if (tp->dirty_rx + NUM_RX_DESC == tp->cur_rx)
> +		printk(KERN_EMERG "%s: Rx buffers exhausted\n", dev->name);
>  }
>  
>  /* The interrupt handler does all of the Rx thread work and cleans up after the Tx thread. */
> @@ -1032,7 +1121,6 @@ rtl8169_close(struct net_device *dev)
>  	struct rtl8169_private *tp = dev->priv;
>  	struct pci_dev *pdev = tp->pci_dev;
>  	void *ioaddr = tp->mmio_addr;
> -	int i;
>  
>  	netif_stop_queue(dev);
>  
> @@ -1054,16 +1142,15 @@ rtl8169_close(struct net_device *dev)
>  	free_irq(dev->irq, dev);
>  
>  	rtl8169_tx_clear(tp);
> +
> +	rtl8169_rx_clear(tp);
> +
>  	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
>  			    tp->RxPhyAddr);
>  	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
>  			    tp->TxPhyAddr);
>  	tp->TxDescArray = NULL;
>  	tp->RxDescArray = NULL;
> -	kfree(tp->RxBufferRings);
> -	for (i = 0; i < NUM_RX_DESC; i++) {
> -		tp->RxBufferRing[i] = NULL;
> -	}
>  
>  	return 0;
>  }
> 
> _

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Francois Romieu]

Brad House <brad@mcve.com> :
> Just because I'm way to lazy to actually look, is this
> against the original r8169, or my patch from the v1.6
> driver ??

It is against the original r8169. Once finished, it should look like:
- first cut at dma api conversion (today)
- remaining bits of dma api conversion (tomorrow)
- rx_copybreak (tomorrow)
- your changes (week-end)
- big-endian fixes (week-end)

Each part divided in reasonably small patches to ease the test/review process.

--
Ueimor

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Brad House]

any reason why it's not against the v1.6 ?
the current driver is missing support for the
8110S chip (or at least it doesn't seem to work at all),
so you're cutting out all the chips on the mobos out there.
Haven't looked to see if it's just a PCI ID or other functionality,
but since the V1.2 is from realtek that's in the official
kernel, it would make sense to use the 1.6 patch....

I suppose I could do it, but it would take me much
more time simply because I'm not familiar with the
changes you're making.

-Brad

Francois Romieu wrote:
> Brad House <brad@mcve.com> :
> 
>>Just because I'm way to lazy to actually look, is this
>>against the original r8169, or my patch from the v1.6
>>driver ??
> 
> 
> It is against the original r8169. Once finished, it should look like:
> - first cut at dma api conversion (today)
> - remaining bits of dma api conversion (tomorrow)
> - rx_copybreak (tomorrow)
> - your changes (week-end)
> - big-endian fixes (week-end)
> 
> Each part divided in reasonably small patches to ease the test/review process.
> 
> --
> Ueimor
> 

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Andre Tomt]

On Thu, 2003-11-20 at 01:59, Brad House wrote:
> any reason why it's not against the v1.6 ?

I think you misunderstood. If I intepret the list correctly, the 1.6
changes are in that list (as "your changes (week-end)".)

> the current driver is missing support for the
> 8110S chip (or at least it doesn't seem to work at all),
> so you're cutting out all the chips on the mobos out there.
> Haven't looked to see if it's just a PCI ID or other functionality,
> but since the V1.2 is from realtek that's in the official
> kernel, it would make sense to use the 1.6 patch....

IIRC, the realtek one got cleaned up a great deal before entering
mainline, both bug-fixes and coding style cleanups. Your patch may
revert a lot of that work (I havn't looked at it closely.) Also the
changelog differs for the 1.2 entry between the two versions, indicating
just this.

Lets take one step at a time ;-)

> > It is against the original r8169. Once finished, it should look like:
> > - first cut at dma api conversion (today)
> > - remaining bits of dma api conversion (tomorrow)
> > - rx_copybreak (tomorrow)
> > - your changes (week-end)
> > - big-endian fixes (week-end)
> > 
> > Each part divided in reasonably small patches to ease the test/review process.

-- 
Mvh,
André Tomt
andre@tomt.net

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Brad House]

yes, I definately misunderstood then ...
sorry ;)
I'll just apply the patches to my version and see what happens ;)

-Brad

Andre Tomt wrote:
> On Thu, 2003-11-20 at 01:59, Brad House wrote:
> 
>>any reason why it's not against the v1.6 ?
> 
> 
> I think you misunderstood. If I intepret the list correctly, the 1.6
> changes are in that list (as "your changes (week-end)".)
> 
> 
>>the current driver is missing support for the
>>8110S chip (or at least it doesn't seem to work at all),
>>so you're cutting out all the chips on the mobos out there.
>>Haven't looked to see if it's just a PCI ID or other functionality,
>>but since the V1.2 is from realtek that's in the official
>>kernel, it would make sense to use the 1.6 patch....
> 
> 
> IIRC, the realtek one got cleaned up a great deal before entering
> mainline, both bug-fixes and coding style cleanups. Your patch may
> revert a lot of that work (I havn't looked at it closely.) Also the
> changelog differs for the 1.2 entry between the two versions, indicating
> just this.
> 
> Lets take one step at a time ;-)
> 
> 
>>>It is against the original r8169. Once finished, it should look like:
>>>- first cut at dma api conversion (today)
>>>- remaining bits of dma api conversion (tomorrow)
>>>- rx_copybreak (tomorrow)
>>>- your changes (week-end)
>>>- big-endian fixes (week-end)
>>>
>>>Each part divided in reasonably small patches to ease the test/review process.
> 
> 

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Brad House]

ok, misunderstood again ...
geez .... now I get it ...
he'll apply the 1.6 patch this weekend ... _sigh_
I really must be brain dead today.

-Brad

Brad House wrote:
> yes, I definately misunderstood then ...
> sorry ;)
> I'll just apply the patches to my version and see what happens ;)
> 
> -Brad
> 
> Andre Tomt wrote:
> 
>> On Thu, 2003-11-20 at 01:59, Brad House wrote:
>>
>>> any reason why it's not against the v1.6 ?
>>
>>
>>
>> I think you misunderstood. If I intepret the list correctly, the 1.6
>> changes are in that list (as "your changes (week-end)".)
>>
>>
>>> the current driver is missing support for the
>>> 8110S chip (or at least it doesn't seem to work at all),
>>> so you're cutting out all the chips on the mobos out there.
>>> Haven't looked to see if it's just a PCI ID or other functionality,
>>> but since the V1.2 is from realtek that's in the official
>>> kernel, it would make sense to use the 1.6 patch....
>>
>>
>>
>> IIRC, the realtek one got cleaned up a great deal before entering
>> mainline, both bug-fixes and coding style cleanups. Your patch may
>> revert a lot of that work (I havn't looked at it closely.) Also the
>> changelog differs for the 1.2 entry between the two versions, indicating
>> just this.
>>
>> Lets take one step at a time ;-)
>>
>>
>>>> It is against the original r8169. Once finished, it should look like:
>>>> - first cut at dma api conversion (today)
>>>> - remaining bits of dma api conversion (tomorrow)
>>>> - rx_copybreak (tomorrow)
>>>> - your changes (week-end)
>>>> - big-endian fixes (week-end)
>>>>
>>>> Each part divided in reasonably small patches to ease the 
>>>> test/review process.
>>
>>
>>
> 
> 
> 

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Jeff Garzik]

Brad House wrote:
> any reason why it's not against the v1.6 ?
> the current driver is missing support for the
> 8110S chip (or at least it doesn't seem to work at all),
> so you're cutting out all the chips on the mobos out there.
> Haven't looked to see if it's just a PCI ID or other functionality,
> but since the V1.2 is from realtek that's in the official
> kernel, it would make sense to use the 1.6 patch....
> 
> I suppose I could do it, but it would take me much
> more time simply because I'm not familiar with the
> changes you're making.


He's doing the work, and he included 1.6 in his list of changes... looks 
like everybody will be happy :)

	Jeff

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Francois Romieu]

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

Francois Romieu <romieu@fr.zoreil.com> :
[...]
> Feel free to comment/review. Next patch tomorrow.

The attached patches apply on top of yesterday's ones:
r8169-start-xmit-fixes.patch
r8169-dma-api-tx-buffers.patch
r8169-rx_copybreak.patch

Bugs apart, there shouldn't be anything subtle.

--
Ueimor

[-- Attachment #2: r8169-start-xmit-fixes.patch --]
[-- Type: text/plain, Size: 2087 bytes --]


rtl8169_start_xmit fixes:
- it forgot to update stats if the skb couldn't be expanded;
- it didn't free it either if the descriptor was not available;
- move the spin_unlock nearer of the exit point instead of duplicating
  it in the new branch.


 drivers/net/r8169.c |   31 ++++++++++++++++++-------------
 1 files changed, 18 insertions(+), 13 deletions(-)

diff -puN drivers/net/r8169.c~r8169-start-xmit-fixes drivers/net/r8169.c
--- linux-2.6.0-test9/drivers/net/r8169.c~r8169-start-xmit-fixes	2003-11-21 01:08:38.000000000 +0100
+++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-21 01:08:38.000000000 +0100
@@ -918,11 +918,13 @@ rtl8169_start_xmit(struct sk_buff *skb, 
 	struct rtl8169_private *tp = dev->priv;
 	void *ioaddr = tp->mmio_addr;
 	int entry = tp->cur_tx % NUM_TX_DESC;
+	u32 len = skb->len;
 
-	if (skb->len < ETH_ZLEN) {
+	if (unlikely(skb->len < ETH_ZLEN)) {
 		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
+		if (!skb)
+			goto err_update_stats;
+		len = ETH_ZLEN;
 	}
 	
 	spin_lock_irq(&tp->lock);
@@ -930,29 +932,32 @@ rtl8169_start_xmit(struct sk_buff *skb, 
 	if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
 		tp->Tx_skbuff[entry] = skb;
 		tp->TxDescArray[entry].buf_addr = virt_to_bus(skb->data);
-		if (entry != (NUM_TX_DESC - 1))
-			tp->TxDescArray[entry].status =
-			    (OWNbit | FSbit | LSbit) | ((skb->len > ETH_ZLEN) ?
-							skb->len : ETH_ZLEN);
-		else
-			tp->TxDescArray[entry].status =
-			    (OWNbit | EORbit | FSbit | LSbit) |
-			    ((skb->len > ETH_ZLEN) ? skb->len : ETH_ZLEN);
 
+		tp->TxDescArray[entry].status = OWNbit | FSbit | LSbit | len |
+				(EORbit * !((entry + 1) % NUM_TX_DESC));
+			
 		RTL_W8(TxPoll, 0x40);	//set polling bit
 
 		dev->trans_start = jiffies;
 
 		tp->cur_tx++;
-	}
+	} else
+		goto err_drop;
 
-	spin_unlock_irq(&tp->lock);
 
 	if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx) {
 		netif_stop_queue(dev);
 	}
+out:
+	spin_unlock_irq(&tp->lock);
 
 	return 0;
+
+err_drop:
+	dev_kfree_skb(skb);
+err_update_stats:
+	tp->stats.tx_dropped++;
+	goto out;
 }
 
 static void

_

[-- Attachment #3: r8169-dma-api-tx-buffers.patch --]
[-- Type: text/plain, Size: 2533 bytes --]


Conversion of Tx data buffers to PCI DMA:
- endianness is kept in a fscked state as it is in the original code
  (will be adressed in a later patch);
- buf_addr of an unmapped descriptor is always set to the same value 
  (cf rtl8169_unmap_tx_skb);
- nothing fancy, really.


 drivers/net/r8169.c |   36 +++++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 7 deletions(-)

diff -puN drivers/net/r8169.c~r8169-dma-api-tx-buffers drivers/net/r8169.c
--- linux-2.6.0-test9/drivers/net/r8169.c~r8169-dma-api-tx-buffers	2003-11-21 01:08:42.000000000 +0100
+++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-21 01:08:42.000000000 +0100
@@ -871,6 +871,17 @@ err_out:
 	return -ENOMEM;
 }
 
+static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct sk_buff **sk_buff,
+				 struct TxDesc *desc)
+{
+	u32 len = sk_buff[0]->len;
+
+	pci_unmap_single(pdev, desc->buf_addr, len < ETH_ZLEN ? ETH_ZLEN : len,
+			 PCI_DMA_TODEVICE);
+	desc->buf_addr = 0x00;
+	*sk_buff = NULL;
+}
+
 static void
 rtl8169_tx_clear(struct rtl8169_private *tp)
 {
@@ -878,9 +889,12 @@ rtl8169_tx_clear(struct rtl8169_private 
 
 	tp->cur_tx = 0;
 	for (i = 0; i < NUM_TX_DESC; i++) {
-		if (tp->Tx_skbuff[i] != NULL) {
-			dev_kfree_skb(tp->Tx_skbuff[i]);
-			tp->Tx_skbuff[i] = NULL;
+		struct sk_buff *skb = tp->Tx_skbuff[i];
+
+		if (skb) {
+			rtl8169_unmap_tx_skb(tp->pci_dev, tp->Tx_skbuff + i,
+					     tp->TxDescArray + i);
+			dev_kfree_skb(skb);
 			tp->stats.tx_dropped++;
 		}
 	}
@@ -930,8 +944,13 @@ rtl8169_start_xmit(struct sk_buff *skb, 
 	spin_lock_irq(&tp->lock);
 
 	if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
+		dma_addr_t mapping;
+
+		mapping = pci_map_single(tp->pci_dev, skb->data, len,
+					 PCI_DMA_TODEVICE);
+
 		tp->Tx_skbuff[entry] = skb;
-		tp->TxDescArray[entry].buf_addr = virt_to_bus(skb->data);
+		tp->TxDescArray[entry].buf_addr = mapping;
 
 		tp->TxDescArray[entry].status = OWNbit | FSbit | LSbit | len |
 				(EORbit * !((entry + 1) % NUM_TX_DESC));
@@ -976,9 +995,12 @@ rtl8169_tx_interrupt(struct net_device *
 
 	while (tx_left > 0) {
 		if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
-			dev_kfree_skb_irq(tp->
-					  Tx_skbuff[dirty_tx % NUM_TX_DESC]);
-			tp->Tx_skbuff[dirty_tx % NUM_TX_DESC] = NULL;
+			int cur = dirty_tx % NUM_TX_DESC;
+			struct sk_buff *skb = tp->Tx_skbuff[cur];
+
+			rtl8169_unmap_tx_skb(tp->pci_dev, tp->Tx_skbuff + cur,
+					     tp->TxDescArray + cur);
+			dev_kfree_skb_irq(skb);
 			tp->stats.tx_packets++;
 			dirty_tx++;
 			tx_left--;

_

[-- Attachment #4: r8169-rx_copybreak.patch --]
[-- Type: text/plain, Size: 3154 bytes --]


Rx copybreak for small packets.
- removal of rtl8169_unmap_rx() (unneeded as for now).


 drivers/net/r8169.c |   47 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 39 insertions(+), 8 deletions(-)

diff -puN drivers/net/r8169.c~r8169-rx_copybreak drivers/net/r8169.c
--- linux-2.6.0-test9/drivers/net/r8169.c~r8169-rx_copybreak	2003-11-21 01:08:46.000000000 +0100
+++ linux-2.6.0-test9-fr/drivers/net/r8169.c	2003-11-21 01:08:46.000000000 +0100
@@ -116,6 +116,8 @@ static struct pci_device_id rtl8169_pci_
 
 MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
 
+static int rx_copybreak = 200;
+
 enum RTL8169_registers {
 	MAC0 = 0,		/* Ethernet hardware address. */
 	MAR0 = 8,		/* Multicast filter. */
@@ -294,6 +296,7 @@ struct rtl8169_private {
 MODULE_AUTHOR("Realtek");
 MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
 MODULE_PARM(media, "1-" __MODULE_STRING(MAX_UNITS) "i");
+MODULE_PARM(rx_copybreak, "i");
 
 static int rtl8169_open(struct net_device *dev);
 static int rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev);
@@ -771,6 +774,11 @@ static void rtl8169_free_rx_skb(struct p
 	rtl8169_make_unusable_by_asic(desc);
 }
 
+static inline void rtl8169_return_to_asic(struct RxDesc *desc)
+{
+	desc->status |= OWNbit + RX_BUF_SIZE;
+}
+
 static inline void rtl8169_give_to_asic(struct RxDesc *desc, dma_addr_t mapping)
 {
 	desc->buf_addr = mapping;
@@ -1015,12 +1023,26 @@ rtl8169_tx_interrupt(struct net_device *
 	}
 }
 
-static inline void rtl8169_unmap_rx(struct pci_dev *pdev, struct RxDesc *desc)
+static inline int rtl8169_try_rx_copy(struct sk_buff **sk_buff, int pkt_size,
+				      struct RxDesc *desc,
+				      struct net_device *dev)
 {
-	pci_dma_sync_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
-			    PCI_DMA_FROMDEVICE);
-	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
-			 PCI_DMA_FROMDEVICE);
+	int ret = -1;
+
+	if (pkt_size < rx_copybreak) {
+		struct sk_buff *skb;
+
+		skb = dev_alloc_skb(pkt_size + 2);
+		if (skb) {
+			skb->dev = dev;
+			skb_reserve(skb, 2);
+			eth_copy_and_sum(skb, sk_buff[0]->tail, pkt_size, 0);
+			*sk_buff = skb;
+			rtl8169_return_to_asic(desc);
+			ret = 0;
+		}
+	}
+	return ret;
 }
 
 static void
@@ -1046,17 +1068,26 @@ rtl8169_rx_interrupt(struct net_device *
 			if (status & RxCRC)
 				tp->stats.rx_crc_errors++;
 		} else {
+			struct RxDesc *desc = tp->RxDescArray + cur_rx;
 			struct sk_buff *skb = tp->Rx_skbuff[cur_rx];
 			int pkt_size = (status & 0x00001FFF) - 4;
 
-			rtl8169_unmap_rx(tp->pci_dev, tp->RxDescArray + cur_rx);
+			pci_dma_sync_single(tp->pci_dev,
+					    le32_to_cpu(desc->buf_addr),
+					    RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+
+			if (rtl8169_try_rx_copy(&skb, pkt_size, desc, dev)) {
+				pci_unmap_single(tp->pci_dev,
+						 le32_to_cpu(desc->buf_addr),
+						 RX_BUF_SIZE,
+						 PCI_DMA_FROMDEVICE);
+				tp->Rx_skbuff[cur_rx] = NULL;
+			}
 
 			skb_put(skb, pkt_size);
 			skb->protocol = eth_type_trans(skb, dev);
 			netif_rx(skb);
 
-			tp->Rx_skbuff[cur_rx] = NULL;
-
 			dev->last_rx = jiffies;
 			tp->stats.rx_bytes += pkt_size;
 			tp->stats.rx_packets++;

_

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Jeff Garzik]

The rx_copybreak didn't apply, but everything else did.  Thanks much. 
Just posted a new net-drivers-2.5-exp patch, to which you may sync with.

	Jeff

Re: [patches] 2.6.0-test9 - r8169 DMA API conversion

[Francois Romieu]

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

Jeff Garzik <jgarzik@pobox.com> :
> The rx_copybreak didn't apply, but everything else did.  Thanks much. 

Oops, I forgot there was an extra MODULE_LICENSE() in net-drivers-2.5-exp.

> Just posted a new net-drivers-2.5-exp patch, to which you may sync with.

Updated rx_copybreak attached.

--
Ueimor

[-- Attachment #2: r8169-rx_copybreak.patch --]
[-- Type: text/plain, Size: 3137 bytes --]


Rx copybreak for small packets.
- removal of rtl8169_unmap_rx() (unneeded as for now).


 drivers/net/r8169.c |   47 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 39 insertions(+), 8 deletions(-)

diff -puN drivers/net/r8169.c~r8169-rx_copybreak drivers/net/r8169.c
--- linux-2.6.0-test9-bk24-netdrvr-exp3/drivers/net/r8169.c~r8169-rx_copybreak	2003-11-22 00:14:22.000000000 +0100
+++ linux-2.6.0-test9-bk24-netdrvr-exp3-fr/drivers/net/r8169.c	2003-11-22 00:14:37.000000000 +0100
@@ -116,6 +116,8 @@ static struct pci_device_id rtl8169_pci_
 
 MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
 
+static int rx_copybreak = 200;
+
 enum RTL8169_registers {
 	MAC0 = 0,		/* Ethernet hardware address. */
 	MAR0 = 8,		/* Multicast filter. */
@@ -294,6 +296,7 @@ struct rtl8169_private {
 MODULE_AUTHOR("Realtek");
 MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
 MODULE_PARM(media, "1-" __MODULE_STRING(MAX_UNITS) "i");
+MODULE_PARM(rx_copybreak, "i");
 MODULE_LICENSE("GPL");
 
 static int rtl8169_open(struct net_device *dev);
@@ -772,6 +775,11 @@ static void rtl8169_free_rx_skb(struct p
 	rtl8169_make_unusable_by_asic(desc);
 }
 
+static inline void rtl8169_return_to_asic(struct RxDesc *desc)
+{
+	desc->status |= OWNbit + RX_BUF_SIZE;
+}
+
 static inline void rtl8169_give_to_asic(struct RxDesc *desc, dma_addr_t mapping)
 {
 	desc->buf_addr = mapping;
@@ -1016,12 +1024,26 @@ rtl8169_tx_interrupt(struct net_device *
 	}
 }
 
-static inline void rtl8169_unmap_rx(struct pci_dev *pdev, struct RxDesc *desc)
+static inline int rtl8169_try_rx_copy(struct sk_buff **sk_buff, int pkt_size,
+				      struct RxDesc *desc,
+				      struct net_device *dev)
 {
-	pci_dma_sync_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
-			    PCI_DMA_FROMDEVICE);
-	pci_unmap_single(pdev, le32_to_cpu(desc->buf_addr), RX_BUF_SIZE,
-			 PCI_DMA_FROMDEVICE);
+	int ret = -1;
+
+	if (pkt_size < rx_copybreak) {
+		struct sk_buff *skb;
+
+		skb = dev_alloc_skb(pkt_size + 2);
+		if (skb) {
+			skb->dev = dev;
+			skb_reserve(skb, 2);
+			eth_copy_and_sum(skb, sk_buff[0]->tail, pkt_size, 0);
+			*sk_buff = skb;
+			rtl8169_return_to_asic(desc);
+			ret = 0;
+		}
+	}
+	return ret;
 }
 
 static void
@@ -1047,17 +1069,26 @@ rtl8169_rx_interrupt(struct net_device *
 			if (status & RxCRC)
 				tp->stats.rx_crc_errors++;
 		} else {
+			struct RxDesc *desc = tp->RxDescArray + cur_rx;
 			struct sk_buff *skb = tp->Rx_skbuff[cur_rx];
 			int pkt_size = (status & 0x00001FFF) - 4;
 
-			rtl8169_unmap_rx(tp->pci_dev, tp->RxDescArray + cur_rx);
+			pci_dma_sync_single(tp->pci_dev,
+					    le32_to_cpu(desc->buf_addr),
+					    RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+
+			if (rtl8169_try_rx_copy(&skb, pkt_size, desc, dev)) {
+				pci_unmap_single(tp->pci_dev,
+						 le32_to_cpu(desc->buf_addr),
+						 RX_BUF_SIZE,
+						 PCI_DMA_FROMDEVICE);
+				tp->Rx_skbuff[cur_rx] = NULL;
+			}
 
 			skb_put(skb, pkt_size);
 			skb->protocol = eth_type_trans(skb, dev);
 			netif_rx(skb);
 
-			tp->Rx_skbuff[cur_rx] = NULL;
-
 			dev->last_rx = jiffies;
 			tp->stats.rx_bytes += pkt_size;
 			tp->stats.rx_packets++;

_