[PATCH 0/7] 8139cp: misc minor changes

[PATCH 0/7] 8139cp: misc minor changes

[Francois Romieu]

The serie contains the patches summarized below:
- 1/7 trim ring_info
- 2/7 remove gratuitous indirection
- 3/7 ring_info removal for the receive path
- 4/7 sync the device private data with its r8169 counterpart
- 5/7 removal of useless BUG_ON() check
- 6/7 pci_get_drvdata(pdev) can not be NULL in suspend handler
- 7/7 use PCI_DEVICE() to shorten the PCI device table

There is nothing critical in the serie as most of its content has
slowly accumulated in various lost branches here. Some testing and
review would be welcome.

The patches are available at:
- git://electric-eye.fr.zoreil.com/home/romieu/linux-2.6.git 8139cp
- http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.18-rc5/8139cp

--
Ueimor

[PATCH 1/7] 8139cp: trim ring_info

[Francois Romieu]

Fat removal: the mapping address is available from the Rx/Tx descriptors.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

3598b57be449a2ee9178e5c511bdb1a8aaccba20
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 1428bb7..8f5d779 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -316,7 +316,6 @@ struct cp_desc {
 
 struct ring_info {
 	struct sk_buff		*skb;
-	dma_addr_t		mapping;
 	u32			len;
 };
 
@@ -551,7 +550,7 @@ rx_status_loop:
 			break;
 
 		len = (status & 0x1fff) - 4;
-		mapping = cp->rx_skb[rx_tail].mapping;
+		mapping = le64_to_cpu(desc->addr);
 
 		if ((status & (FirstFrag | LastFrag)) != (FirstFrag | LastFrag)) {
 			/* we don't support incoming fragmented frames.
@@ -595,10 +594,8 @@ rx_status_loop:
 
 		skb_put(skb, len);
 
-		mapping =
-		cp->rx_skb[rx_tail].mapping =
-			pci_map_single(cp->pdev, new_skb->data,
-				       buflen, PCI_DMA_FROMDEVICE);
+		mapping = pci_map_single(cp->pdev, new_skb->data, buflen,
+					 PCI_DMA_FROMDEVICE);
 		cp->rx_skb[rx_tail].skb = new_skb;
 
 		cp_rx_skb(cp, skb, desc);
@@ -717,18 +714,19 @@ static void cp_tx (struct cp_private *cp
 	unsigned tx_tail = cp->tx_tail;
 
 	while (tx_tail != tx_head) {
+		struct cp_desc *txd = cp->tx_ring + tx_tail;
 		struct sk_buff *skb;
 		u32 status;
 
 		rmb();
-		status = le32_to_cpu(cp->tx_ring[tx_tail].opts1);
+		status = le32_to_cpu(txd->opts1);
 		if (status & DescOwn)
 			break;
 
 		skb = cp->tx_skb[tx_tail].skb;
 		BUG_ON(!skb);
 
-		pci_unmap_single(cp->pdev, cp->tx_skb[tx_tail].mapping,
+		pci_unmap_single(cp->pdev, le64_to_cpu(txd->addr),
 				 cp->tx_skb[tx_tail].len, PCI_DMA_TODEVICE);
 
 		if (status & LastFrag) {
@@ -827,7 +825,6 @@ #endif
 		wmb();
 
 		cp->tx_skb[entry].skb = skb;
-		cp->tx_skb[entry].mapping = mapping;
 		cp->tx_skb[entry].len = len;
 		entry = NEXT_TX(entry);
 	} else {
@@ -845,7 +842,6 @@ #endif
 		first_mapping = pci_map_single(cp->pdev, skb->data,
 					       first_len, PCI_DMA_TODEVICE);
 		cp->tx_skb[entry].skb = skb;
-		cp->tx_skb[entry].mapping = first_mapping;
 		cp->tx_skb[entry].len = first_len;
 		entry = NEXT_TX(entry);
 
@@ -888,7 +884,6 @@ #endif
 			wmb();
 
 			cp->tx_skb[entry].skb = skb;
-			cp->tx_skb[entry].mapping = mapping;
 			cp->tx_skb[entry].len = len;
 			entry = NEXT_TX(entry);
 		}
@@ -1091,6 +1086,7 @@ static int cp_refill_rx (struct cp_priva
 
 	for (i = 0; i < CP_RX_RING_SIZE; i++) {
 		struct sk_buff *skb;
+		dma_addr_t mapping;
 
 		skb = dev_alloc_skb(cp->rx_buf_sz + RX_OFFSET);
 		if (!skb)
@@ -1099,12 +1095,12 @@ static int cp_refill_rx (struct cp_priva
 		skb->dev = cp->dev;
 		skb_reserve(skb, RX_OFFSET);
 
-		cp->rx_skb[i].mapping = pci_map_single(cp->pdev,
-			skb->data, cp->rx_buf_sz, PCI_DMA_FROMDEVICE);
+		mapping = pci_map_single(cp->pdev, skb->data, cp->rx_buf_sz,
+					 PCI_DMA_FROMDEVICE);
 		cp->rx_skb[i].skb = skb;
 
 		cp->rx_ring[i].opts2 = 0;
-		cp->rx_ring[i].addr = cpu_to_le64(cp->rx_skb[i].mapping);
+		cp->rx_ring[i].addr = cpu_to_le64(mapping);
 		if (i == (CP_RX_RING_SIZE - 1))
 			cp->rx_ring[i].opts1 =
 				cpu_to_le32(DescOwn | RingEnd | cp->rx_buf_sz);
@@ -1152,11 +1148,13 @@ static int cp_alloc_rings (struct cp_pri
 
 static void cp_clean_rings (struct cp_private *cp)
 {
+	struct cp_desc *desc;
 	unsigned i;
 
 	for (i = 0; i < CP_RX_RING_SIZE; i++) {
 		if (cp->rx_skb[i].skb) {
-			pci_unmap_single(cp->pdev, cp->rx_skb[i].mapping,
+			desc = cp->rx_ring + i;
+			pci_unmap_single(cp->pdev, le64_to_cpu(desc->addr),
 					 cp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 			dev_kfree_skb(cp->rx_skb[i].skb);
 		}
@@ -1166,9 +1164,10 @@ static void cp_clean_rings (struct cp_pr
 		if (cp->tx_skb[i].skb) {
 			struct sk_buff *skb = cp->tx_skb[i].skb;
 
-			pci_unmap_single(cp->pdev, cp->tx_skb[i].mapping,
+			desc = cp->tx_ring + i;
+			pci_unmap_single(cp->pdev, le64_to_cpu(desc->addr),
 				 	 cp->tx_skb[i].len, PCI_DMA_TODEVICE);
-			if (le32_to_cpu(cp->tx_ring[i].opts1) & LastFrag)
+			if (le32_to_cpu(desc->opts1) & LastFrag)
 				dev_kfree_skb(skb);
 			cp->net_stats.tx_dropped++;
 		}
-- 
1.3.1

[PATCH 2/7] 8139cp: remove gratuitous indirection

[Francois Romieu]

dev is an argument of the current function.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

c48e9399e895834f26dff9149de1930ba18a0d6c
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 8f5d779..ae9bb75 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -571,7 +571,7 @@ rx_status_loop:
 
 		if (netif_msg_rx_status(cp))
 			printk(KERN_DEBUG "%s: rx slot %d status 0x%x len %d\n",
-			       cp->dev->name, rx_tail, status, len);
+			       dev->name, rx_tail, status, len);
 
 		buflen = cp->rx_buf_sz + RX_OFFSET;
 		new_skb = dev_alloc_skb (buflen);
@@ -581,7 +581,7 @@ rx_status_loop:
 		}
 
 		skb_reserve(new_skb, RX_OFFSET);
-		new_skb->dev = cp->dev;
+		new_skb->dev = dev;
 
 		pci_unmap_single(cp->pdev, mapping,
 				 buflen, PCI_DMA_FROMDEVICE);
-- 
1.3.1

[PATCH 3/7] 8139cp: ring_info removal for the receive path

[Francois Romieu]

The ring_info.len field is not used at all. cp_private.rx_skb is
turned into an array of sk_buff *.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

0ba894d420b845667e2981c2147af974a755fba2
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index ae9bb75..0725681 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -354,7 +354,7 @@ struct cp_private {
 
 	unsigned		rx_tail		____cacheline_aligned;
 	struct cp_desc		*rx_ring;
-	struct ring_info	rx_skb[CP_RX_RING_SIZE];
+	struct sk_buff		*rx_skb[CP_RX_RING_SIZE];
 	unsigned		rx_buf_sz;
 
 	unsigned		tx_head		____cacheline_aligned;
@@ -541,7 +541,7 @@ rx_status_loop:
 		struct cp_desc *desc;
 		unsigned buflen;
 
-		skb = cp->rx_skb[rx_tail].skb;
+		skb = cp->rx_skb[rx_tail];
 		BUG_ON(!skb);
 
 		desc = &cp->rx_ring[rx_tail];
@@ -596,7 +596,7 @@ rx_status_loop:
 
 		mapping = pci_map_single(cp->pdev, new_skb->data, buflen,
 					 PCI_DMA_FROMDEVICE);
-		cp->rx_skb[rx_tail].skb = new_skb;
+		cp->rx_skb[rx_tail] = new_skb;
 
 		cp_rx_skb(cp, skb, desc);
 		rx++;
@@ -1097,7 +1097,7 @@ static int cp_refill_rx (struct cp_priva
 
 		mapping = pci_map_single(cp->pdev, skb->data, cp->rx_buf_sz,
 					 PCI_DMA_FROMDEVICE);
-		cp->rx_skb[i].skb = skb;
+		cp->rx_skb[i] = skb;
 
 		cp->rx_ring[i].opts2 = 0;
 		cp->rx_ring[i].addr = cpu_to_le64(mapping);
@@ -1152,11 +1152,11 @@ static void cp_clean_rings (struct cp_pr
 	unsigned i;
 
 	for (i = 0; i < CP_RX_RING_SIZE; i++) {
-		if (cp->rx_skb[i].skb) {
+		if (cp->rx_skb[i]) {
 			desc = cp->rx_ring + i;
 			pci_unmap_single(cp->pdev, le64_to_cpu(desc->addr),
 					 cp->rx_buf_sz, PCI_DMA_FROMDEVICE);
-			dev_kfree_skb(cp->rx_skb[i].skb);
+			dev_kfree_skb(cp->rx_skb[i]);
 		}
 	}
 
@@ -1176,7 +1176,7 @@ static void cp_clean_rings (struct cp_pr
 	memset(cp->rx_ring, 0, sizeof(struct cp_desc) * CP_RX_RING_SIZE);
 	memset(cp->tx_ring, 0, sizeof(struct cp_desc) * CP_TX_RING_SIZE);
 
-	memset(&cp->rx_skb, 0, sizeof(struct ring_info) * CP_RX_RING_SIZE);
+	memset(cp->rx_skb, 0, sizeof(struct sk_buff *) * CP_RX_RING_SIZE);
 	memset(&cp->tx_skb, 0, sizeof(struct ring_info) * CP_TX_RING_SIZE);
 }
 
-- 
1.3.1

[PATCH 4/7] 8139cp: sync the device private data with its r8169 counterpart

[Francois Romieu]

struct cp_private is reorganized to be more easily compared between the
r8169 and the 8139cp drivers.

The alignment should not be impacted.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

d03d376dd29cae53bf70a21a0c26b306abe37326
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 0725681..9aef517 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -352,23 +352,23 @@ struct cp_private {
 	struct net_device_stats net_stats;
 	struct cp_extra_stats	cp_stats;
 
-	unsigned		rx_tail		____cacheline_aligned;
+	unsigned		rx_head		____cacheline_aligned;
+	unsigned		rx_tail;
 	struct cp_desc		*rx_ring;
 	struct sk_buff		*rx_skb[CP_RX_RING_SIZE];
-	unsigned		rx_buf_sz;
 
 	unsigned		tx_head		____cacheline_aligned;
 	unsigned		tx_tail;
-
 	struct cp_desc		*tx_ring;
 	struct ring_info	tx_skb[CP_TX_RING_SIZE];
-	dma_addr_t		ring_dma;
+
+	unsigned		rx_buf_sz;
+	unsigned		wol_enabled : 1; /* Is Wake-on-LAN enabled? */
 
 #if CP_VLAN_TAG_USED
 	struct vlan_group	*vlgrp;
 #endif
-
-	unsigned int		wol_enabled : 1; /* Is Wake-on-LAN enabled? */
+	dma_addr_t		ring_dma;
 
 	struct mii_if_info	mii_if;
 };
-- 
1.3.1

[PATCH 5/7] 8139cp: removal of useless BUG_ON() check

[Francois Romieu]

netdev_priv() will provide a nice oops a few lines before
the BUG_ON() check.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

e68970e7543815133224f79a858e7c9e0c42f4de
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 9aef517..94ab3c3 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -2009,7 +2009,6 @@ static void cp_remove_one (struct pci_de
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct cp_private *cp = netdev_priv(dev);
 
-	BUG_ON(!dev);
 	unregister_netdev(dev);
 	iounmap(cp->regs);
 	if (cp->wol_enabled)
-- 
1.3.1

[PATCH 6/7] 8139cp: pci_get_drvdata(pdev) can not be NULL in suspend handler

[Francois Romieu]

1) pci_set_drvdata() is used in cp_{init/remove}_one to initialize/reset
   driver_data to the relevant value (resp. net_device * and NULL).

2) each of the 3 relevant functions is issued under (device *)->sem:
   2.1) pci_unregister_driver
        -> driver_unregister
           -> bus_remove_driver
              -> driver_detach (takes (device *)->sem)
                 -> __device_release_driver(dev)
                    -> dev->bus-remove(dev) (== pci_device_remove)
                       -> drv->remove(pdev) (== cp_remove_one)
                       [...]
                       pci_dev->driver = NULL;

   2.2) pci_register_driver
        -> __pci_register_driver
           -> driver_register
              -> bus_add_driver
                 -> driver_attach
                    -> __driver_attach (takes (device *)->sem)
                       -> driver_probe_device(drv, dev)
                          -> dev->bus->probe(dev) (== pci_device_probe)
                             -> _pci_device_probe(drv, pci_dev)
                                -> pci_call_probe(drv, pci_dev, id)
                                   -> drv->probe(dev, id) (== cp_init_one)
                                [...]
                                pci_dev->driver = drv;

   2.3) suspend_device (takes (device *)->sem)
        -> dev->bus->suspend(dev) (== pci_device_suspend)
           checking for drv = pci_dev->driver != NULL
           [...]
           -> drv->suspend(pci_dev, state) (== cp_suspend)

dev->sem and the state of pci_dev->driver provide the expected result.

St Mary's day was a bit rainy here.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

7668a4945ba0e17a61e535a6c67aa64a319a03b4
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 94ab3c3..a123d28 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -2023,14 +2023,12 @@ static void cp_remove_one (struct pci_de
 #ifdef CONFIG_PM
 static int cp_suspend (struct pci_dev *pdev, pm_message_t state)
 {
-	struct net_device *dev;
-	struct cp_private *cp;
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
 
-	dev = pci_get_drvdata (pdev);
-	cp  = netdev_priv(dev);
-
-	if (!dev || !netif_running (dev)) return 0;
+	if (!netif_running(dev))
+		return 0;
 
 	netif_device_detach (dev);
 	netif_stop_queue (dev);
-- 
1.3.1

[PATCH 7/7] 8139cp: use PCI_DEVICE() to shorten the PCI device table

[Francois Romieu]

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

---

 drivers/net/8139cp.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

cccb20d3a9b7c6d4b6e1b52ee02814e6094aaa12
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index a123d28..bbdaa18 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -406,10 +406,8 @@ static int cp_set_eeprom(struct net_devi
 			 struct ethtool_eeprom *eeprom, u8 *data);
 
 static struct pci_device_id cp_pci_tbl[] = {
-	{ PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139,
-	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, },
-	{ PCI_VENDOR_ID_TTTECH, PCI_DEVICE_ID_TTTECH_MC322,
-	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK,	PCI_DEVICE_ID_REALTEK_8139), },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTTECH,	PCI_DEVICE_ID_TTTECH_MC322), },
 	{ },
 };
 MODULE_DEVICE_TABLE(pci, cp_pci_tbl);
-- 
1.3.1

Re: [PATCH 0/7] 8139cp: misc minor changes

[Pierre Ossman]

Ehm... why am I included in this? :)

Re: [PATCH 0/7] 8139cp: misc minor changes

[Francois Romieu]

Pierre Ossman <drzeus-list@drzeus.cx> :
> Ehm... why am I included in this? :)

To preserve an happy 8139cp user :o)

-- 
Ueimor

Re: [PATCH 0/7] 8139cp: misc minor changes

[Pierre Ossman]

Francois Romieu wrote:
> Pierre Ossman <drzeus-list@drzeus.cx> :
>   
>> Ehm... why am I included in this? :)
>>     
>
> To preserve an happy 8139cp user :o)
>
>   

A noble endeavor. Carry on. ;)

Re: [PATCH 3/7] 8139cp: ring_info removal for the receive path

[Jeff Garzik]

Francois Romieu wrote:
> The ring_info.len field is not used at all. cp_private.rx_skb is
> turned into an array of sk_buff *.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

Need to remove the now-dead struct ring_info.

Otherwise, patchset looks OK.

When fixed, I'll pull into #upstream, just let me know...

	Jeff

Re: [PATCH 3/7] 8139cp: ring_info removal for the receive path

[Francois Romieu]

Jeff Garzik <jgarzik@pobox.com> :
> Francois Romieu wrote:
> >The ring_info.len field is not used at all. cp_private.rx_skb is
> >turned into an array of sk_buff *.
> >
> >Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> 
> Need to remove the now-dead struct ring_info.

I should have written "is not used at all in the receive path".

If one wants to remove ring_info completely, something like the
patch below is required:

    8139cp: ring_info removal for the transmit path
    
    <handwaving>
    As long as the descriptor fits on a single cacheline, the change
    should be really free.
    </handwaving>
    
    Now ring_info is not used at all.
    
    Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index bbdaa18..c3b8400 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -314,11 +314,6 @@ struct cp_desc {
 	u64		addr;
 };
 
-struct ring_info {
-	struct sk_buff		*skb;
-	u32			len;
-};
-
 struct cp_dma_stats {
 	u64			tx_ok;
 	u64			rx_ok;
@@ -360,7 +355,7 @@ struct cp_private {
 	unsigned		tx_head		____cacheline_aligned;
 	unsigned		tx_tail;
 	struct cp_desc		*tx_ring;
-	struct ring_info	tx_skb[CP_TX_RING_SIZE];
+	struct sk_buff		*tx_skb[CP_TX_RING_SIZE];
 
 	unsigned		rx_buf_sz;
 	unsigned		wol_enabled : 1; /* Is Wake-on-LAN enabled? */
@@ -721,11 +716,12 @@ static void cp_tx (struct cp_private *cp
 		if (status & DescOwn)
 			break;
 
-		skb = cp->tx_skb[tx_tail].skb;
+		skb = cp->tx_skb[tx_tail];
 		BUG_ON(!skb);
 
 		pci_unmap_single(cp->pdev, le64_to_cpu(txd->addr),
-				 cp->tx_skb[tx_tail].len, PCI_DMA_TODEVICE);
+				 le32_to_cpu(txd->opts1) & 0xffff,
+				 PCI_DMA_TODEVICE);
 
 		if (status & LastFrag) {
 			if (status & (TxError | TxFIFOUnder)) {
@@ -752,7 +748,7 @@ static void cp_tx (struct cp_private *cp
 			dev_kfree_skb_irq(skb);
 		}
 
-		cp->tx_skb[tx_tail].skb = NULL;
+		cp->tx_skb[tx_tail] = NULL;
 
 		tx_tail = NEXT_TX(tx_tail);
 	}
@@ -822,8 +818,7 @@ #endif
 		txd->opts1 = cpu_to_le32(flags);
 		wmb();
 
-		cp->tx_skb[entry].skb = skb;
-		cp->tx_skb[entry].len = len;
+		cp->tx_skb[entry] = skb;
 		entry = NEXT_TX(entry);
 	} else {
 		struct cp_desc *txd;
@@ -839,8 +834,7 @@ #endif
 		first_len = skb_headlen(skb);
 		first_mapping = pci_map_single(cp->pdev, skb->data,
 					       first_len, PCI_DMA_TODEVICE);
-		cp->tx_skb[entry].skb = skb;
-		cp->tx_skb[entry].len = first_len;
+		cp->tx_skb[entry] = skb;
 		entry = NEXT_TX(entry);
 
 		for (frag = 0; frag < skb_shinfo(skb)->nr_frags; frag++) {
@@ -881,8 +875,7 @@ #endif
 			txd->opts1 = cpu_to_le32(ctrl);
 			wmb();
 
-			cp->tx_skb[entry].skb = skb;
-			cp->tx_skb[entry].len = len;
+			cp->tx_skb[entry] = skb;
 			entry = NEXT_TX(entry);
 		}
 
@@ -1159,12 +1152,13 @@ static void cp_clean_rings (struct cp_pr
 	}
 
 	for (i = 0; i < CP_TX_RING_SIZE; i++) {
-		if (cp->tx_skb[i].skb) {
-			struct sk_buff *skb = cp->tx_skb[i].skb;
+		if (cp->tx_skb[i]) {
+			struct sk_buff *skb = cp->tx_skb[i];
 
 			desc = cp->tx_ring + i;
 			pci_unmap_single(cp->pdev, le64_to_cpu(desc->addr),
-				 	 cp->tx_skb[i].len, PCI_DMA_TODEVICE);
+					 le32_to_cpu(desc->opts1) & 0xffff,
+					 PCI_DMA_TODEVICE);
 			if (le32_to_cpu(desc->opts1) & LastFrag)
 				dev_kfree_skb(skb);
 			cp->net_stats.tx_dropped++;
@@ -1175,7 +1169,7 @@ static void cp_clean_rings (struct cp_pr
 	memset(cp->tx_ring, 0, sizeof(struct cp_desc) * CP_TX_RING_SIZE);
 
 	memset(cp->rx_skb, 0, sizeof(struct sk_buff *) * CP_RX_RING_SIZE);
-	memset(&cp->tx_skb, 0, sizeof(struct ring_info) * CP_TX_RING_SIZE);
+	memset(cp->tx_skb, 0, sizeof(struct sk_buff *) * CP_TX_RING_SIZE);
 }
 
 static void cp_free_rings (struct cp_private *cp)