Skip to contentSkip to conversation

tlan: fix earlier fixes, remove big buffer mode

PATCH In review Sakari Ailus 2 participants 5 messages Dec 16, 2008 – Dec 17, 2008

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Sakari Ailus

author
#0 lore raw

cover tlan: fix earlier fixes, remove big buffer mode

Hi,

Unfortunately it seems that my last patch introduced a few problems.

So here are two patches: the first one fixes the pci memory unmapping. The second one removes the big buffer mode.

That mode has never worked for me and I've been using tlan cards since 2001 or so. With Google I could find one case where someone claimed it was working on 2.4.0-test1-ac7. ;-)

<URL:http://lkml.indiana.edu/hypermail/linux/kernel/0006.0/0193.html>

There's another (likely) bug in the bbuf mode (think skb copying there by txlen bytes), introduced by my earlier patch, which is fixed by the second patch by removing the mode altogether.

Cheers,

Signature · 2 lines
Sakari Ailus
[email protected]
author
#1 lore raw

PATCH 1/2 tlan: Fix pci memory unmapping

Fix pci unmapping problem introduced by commit id 8953f1282793882a5444924f7a273dc72a43d0a3 "tlan: Fix small (< 64 bytes) datagram transmissions".

Signed-off-by Sakari Ailus <[email protected]>
---
 drivers/net/tlan.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)
drivers/net/tlan.c +9 -4
index cf8cdaf..e604982 100644
@@ -1433,7 +1433,9 @@ static u32 TLan_HandleTxEOF( struct net_device *dev, u16 host_int )
 		if ( ! bbuf ) {
 			struct sk_buff *skb = TLan_GetSKB(head_list);
 			pci_unmap_single(priv->pciDev, head_list->buffer[0].address,
-					 skb->len, PCI_DMA_TODEVICE);
+					 max(skb->len,
+					     (unsigned int)TLAN_MIN_FRAME_SIZE),
+					 PCI_DMA_TODEVICE);
 			dev_kfree_skb_any(skb);
 			head_list->buffer[8].address = 0;
 			head_list->buffer[9].address = 0;

@@ -2057,9 +2059,12 @@ static void TLan_FreeLists( struct net_device *dev )
 			list = priv->txList + i;
 			skb = TLan_GetSKB(list);
 			if ( skb ) {
-				pci_unmap_single(priv->pciDev,
-						 list->buffer[0].address, skb->len,
-						 PCI_DMA_TODEVICE);
+				pci_unmap_single(
+					priv->pciDev,
+					list->buffer[0].address,
+					max(skb->len,
+					    (unsigned int)TLAN_MIN_FRAME_SIZE),
+					PCI_DMA_TODEVICE);
 				dev_kfree_skb_any( skb );
 				list->buffer[8].address = 0;
 				list->buffer[9].address = 0;
Signature · 1 line
1.5.6.5
author
#2 lore raw

PATCH 2/2 tlan: Remove broken support for big buffers

The big rx/tx buffer support is broken and unlikely to be very useful as such. Remove it.

Signed-off-by Sakari Ailus <[email protected]>
---
 drivers/net/tlan.c |  211 +++++++++++++++++++---------------------------------
 1 files changed, 77 insertions(+), 134 deletions(-)
drivers/net/tlan.c +77 -134
index e604982..2a2aacb 100644
@@ -163,6 +163,11 @@
  *	v1.15 Apr 4, 2002    - Correct operation when aui=1 to be
  *	                       10T half duplex no loopback
  *	                       Thanks to Gunnar Eikman
+ *
+ *	Sakari Ailus <[email protected]>:
+ *
+ *	v1.15a Dec 15 2008   - Remove bbuf support, it doesn't work anyway.
+ *
  *******************************************************************************/
 
 #include <linux/module.h>

@@ -213,12 +218,8 @@ static  int		debug;
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "ThunderLAN debug mask");
 
-static	int		bbuf;
-module_param(bbuf, int, 0);
-MODULE_PARM_DESC(bbuf, "ThunderLAN use big buffer (0-1)");
-
 static	const char TLanSignature[] = "TLAN";
-static  const char tlan_banner[] = "ThunderLAN driver v1.15\n";
+static  const char tlan_banner[] = "ThunderLAN driver v1.15a\n";
 static  int tlan_have_pci;
 static  int tlan_have_eisa;
 

@@ -859,13 +860,8 @@ static int TLan_Init( struct net_device *dev )
 
 	priv = netdev_priv(dev);
 
-	if ( bbuf ) {
-		dma_size = ( TLAN_NUM_RX_LISTS + TLAN_NUM_TX_LISTS )
-	           * ( sizeof(TLanList) + TLAN_MAX_FRAME_SIZE );
-	} else {
-		dma_size = ( TLAN_NUM_RX_LISTS + TLAN_NUM_TX_LISTS )
-	           * ( sizeof(TLanList) );
-	}
+	dma_size = ( TLAN_NUM_RX_LISTS + TLAN_NUM_TX_LISTS )
+		* ( sizeof(TLanList) );
 	priv->dmaStorage = pci_alloc_consistent(priv->pciDev,
 						dma_size, &priv->dmaStorageDMA);
 	priv->dmaSize = dma_size;

@@ -881,16 +877,6 @@ static int TLan_Init( struct net_device *dev )
 	priv->txList = priv->rxList + TLAN_NUM_RX_LISTS;
 	priv->txListDMA = priv->rxListDMA + sizeof(TLanList) * TLAN_NUM_RX_LISTS;
 
-	if ( bbuf ) {
-		priv->rxBuffer = (u8 *) ( priv->txList + TLAN_NUM_TX_LISTS );
-		priv->rxBufferDMA =priv->txListDMA
-			+ sizeof(TLanList) * TLAN_NUM_TX_LISTS;
-		priv->txBuffer = priv->rxBuffer
-			+ ( TLAN_NUM_RX_LISTS * TLAN_MAX_FRAME_SIZE );
-		priv->txBufferDMA = priv->rxBufferDMA
-			+ ( TLAN_NUM_RX_LISTS * TLAN_MAX_FRAME_SIZE );
-	}
-
 	err = 0;
 	for ( i = 0;  i < 6 ; i++ )
 		err |= TLan_EeReadByte( dev,

@@ -1094,9 +1080,8 @@ static void TLan_tx_timeout_work(struct work_struct *work)
 static int TLan_StartTx( struct sk_buff *skb, struct net_device *dev )
 {
 	TLanPrivateInfo *priv = netdev_priv(dev);
-	TLanList	*tail_list;
 	dma_addr_t	tail_list_phys;
-	u8		*tail_buffer;
+	TLanList	*tail_list;
 	unsigned long	flags;
 	unsigned int    txlen;
 

@@ -1125,15 +1110,10 @@ static int TLan_StartTx( struct sk_buff *skb, struct net_device *dev )
 
 	tail_list->forward = 0;
 
-	if ( bbuf ) {
-		tail_buffer = priv->txBuffer + ( priv->txTail * TLAN_MAX_FRAME_SIZE );
-		skb_copy_from_linear_data(skb, tail_buffer, txlen);
-	} else {
-		tail_list->buffer[0].address = pci_map_single(priv->pciDev,
-							      skb->data, txlen,
-							      PCI_DMA_TODEVICE);
-		TLan_StoreSKB(tail_list, skb);
-	}
+	tail_list->buffer[0].address = pci_map_single(priv->pciDev,
+						      skb->data, txlen,
+						      PCI_DMA_TODEVICE);
+	TLan_StoreSKB(tail_list, skb);
 
 	tail_list->frameSize = (u16) txlen;
 	tail_list->buffer[0].count = TLAN_LAST_BUFFER | (u32) txlen;

@@ -1163,9 +1143,6 @@ static int TLan_StartTx( struct sk_buff *skb, struct net_device *dev )
 
 	CIRC_INC( priv->txTail, TLAN_NUM_TX_LISTS );
 
-	if ( bbuf )
-		dev_kfree_skb_any(skb);
-
 	dev->trans_start = jiffies;
 	return 0;
 

@@ -1429,17 +1406,16 @@ static u32 TLan_HandleTxEOF( struct net_device *dev, u16 host_int )
 	head_list = priv->txList + priv->txHead;
 
 	while (((tmpCStat = head_list->cStat ) & TLAN_CSTAT_FRM_CMP) && (ack < 255)) {
+		struct sk_buff *skb = TLan_GetSKB(head_list);
+
 		ack++;
-		if ( ! bbuf ) {
-			struct sk_buff *skb = TLan_GetSKB(head_list);
-			pci_unmap_single(priv->pciDev, head_list->buffer[0].address,
-					 max(skb->len,
-					     (unsigned int)TLAN_MIN_FRAME_SIZE),
-					 PCI_DMA_TODEVICE);
-			dev_kfree_skb_any(skb);
-			head_list->buffer[8].address = 0;
-			head_list->buffer[9].address = 0;
-		}
+		pci_unmap_single(priv->pciDev, head_list->buffer[0].address,
+				 max(skb->len,
+				     (unsigned int)TLAN_MIN_FRAME_SIZE),
+				 PCI_DMA_TODEVICE);
+		dev_kfree_skb_any(skb);
+		head_list->buffer[8].address = 0;
+		head_list->buffer[9].address = 0;
 
 		if ( tmpCStat & TLAN_CSTAT_EOC )
 			eoc = 1;

@@ -1549,7 +1525,6 @@ static u32 TLan_HandleRxEOF( struct net_device *dev, u16 host_int )
 	TLanPrivateInfo	*priv = netdev_priv(dev);
 	u32		ack = 0;
 	int		eoc = 0;
-	u8		*head_buffer;
 	TLanList	*head_list;
 	struct sk_buff	*skb;
 	TLanList	*tail_list;

@@ -1564,53 +1539,33 @@ static u32 TLan_HandleRxEOF( struct net_device *dev, u16 host_int )
 	while (((tmpCStat = head_list->cStat) & TLAN_CSTAT_FRM_CMP) && (ack < 255)) {
 		dma_addr_t frameDma = head_list->buffer[0].address;
 		u32 frameSize = head_list->frameSize;
+		struct sk_buff *new_skb;
+
 		ack++;
 		if (tmpCStat & TLAN_CSTAT_EOC)
 			eoc = 1;
 
-		if (bbuf) {
-			skb = netdev_alloc_skb(dev, frameSize + 7);
-			if ( !skb )
-				goto drop_and_reuse;
-
-			head_buffer = priv->rxBuffer
-				+ (priv->rxHead * TLAN_MAX_FRAME_SIZE);
-			skb_reserve(skb, 2);
-			pci_dma_sync_single_for_cpu(priv->pciDev,
-						    frameDma, frameSize,
-						    PCI_DMA_FROMDEVICE);
-			skb_copy_from_linear_data(skb, head_buffer, frameSize);
-			skb_put(skb, frameSize);
-			dev->stats.rx_bytes += frameSize;
-
-			skb->protocol = eth_type_trans( skb, dev );
-			netif_rx( skb );
-		} else {
-			struct sk_buff *new_skb;
+		new_skb = netdev_alloc_skb(dev, TLAN_MAX_FRAME_SIZE + 7 );
+		if ( !new_skb )
+			goto drop_and_reuse;
 
-			new_skb = netdev_alloc_skb(dev, TLAN_MAX_FRAME_SIZE + 7 );
-			if ( !new_skb )
-				goto drop_and_reuse;
+		skb = TLan_GetSKB(head_list);
+		pci_unmap_single(priv->pciDev, frameDma,
+				 TLAN_MAX_FRAME_SIZE, PCI_DMA_FROMDEVICE);
+		skb_put( skb, frameSize );
 
-			skb = TLan_GetSKB(head_list);
-			pci_unmap_single(priv->pciDev, frameDma,
-					 TLAN_MAX_FRAME_SIZE, PCI_DMA_FROMDEVICE);
-			skb_put( skb, frameSize );
+		dev->stats.rx_bytes += frameSize;
 
-			dev->stats.rx_bytes += frameSize;
+		skb->protocol = eth_type_trans( skb, dev );
+		netif_rx( skb );
 
-			skb->protocol = eth_type_trans( skb, dev );
-			netif_rx( skb );
+		skb_reserve( new_skb, NET_IP_ALIGN );
+		head_list->buffer[0].address = pci_map_single(priv->pciDev,
+							      new_skb->data,
+							      TLAN_MAX_FRAME_SIZE,
+							      PCI_DMA_FROMDEVICE);
 
-			skb_reserve( new_skb, NET_IP_ALIGN );
-			head_list->buffer[0].address = pci_map_single(priv->pciDev,
-								      new_skb->data,
-								      TLAN_MAX_FRAME_SIZE,
-								      PCI_DMA_FROMDEVICE);
-
-			TLan_StoreSKB(head_list, new_skb);
-
-		}
+		TLan_StoreSKB(head_list, new_skb);
 drop_and_reuse:
 		head_list->forward = 0;
 		head_list->cStat = 0;

@@ -1995,12 +1950,7 @@ static void TLan_ResetLists( struct net_device *dev )
 	for ( i = 0; i < TLAN_NUM_TX_LISTS; i++ ) {
 		list = priv->txList + i;
 		list->cStat = TLAN_CSTAT_UNUSED;
-		if ( bbuf ) {
-			list->buffer[0].address = priv->txBufferDMA
-				+ ( i * TLAN_MAX_FRAME_SIZE );
-		} else {
-			list->buffer[0].address = 0;
-		}
+		list->buffer[0].address = 0;
 		list->buffer[2].count = 0;
 		list->buffer[2].address = 0;
 		list->buffer[8].address = 0;

@@ -2015,23 +1965,18 @@ static void TLan_ResetLists( struct net_device *dev )
 		list->cStat = TLAN_CSTAT_READY;
 		list->frameSize = TLAN_MAX_FRAME_SIZE;
 		list->buffer[0].count = TLAN_MAX_FRAME_SIZE | TLAN_LAST_BUFFER;
-		if ( bbuf ) {
-			list->buffer[0].address = priv->rxBufferDMA
-				+ ( i * TLAN_MAX_FRAME_SIZE );
-		} else {
-			skb = netdev_alloc_skb(dev, TLAN_MAX_FRAME_SIZE + 7 );
-			if ( !skb ) {
-				pr_err("TLAN: out of memory for received data.\n" );
-				break;
-			}
-
-			skb_reserve( skb, NET_IP_ALIGN );
-			list->buffer[0].address = pci_map_single(priv->pciDev,
-								 skb->data,
-								 TLAN_MAX_FRAME_SIZE,
-								 PCI_DMA_FROMDEVICE);
-			TLan_StoreSKB(list, skb);
+		skb = netdev_alloc_skb(dev, TLAN_MAX_FRAME_SIZE + 7 );
+		if ( !skb ) {
+			pr_err("TLAN: out of memory for received data.\n" );
+			break;
 		}
+
+		skb_reserve( skb, NET_IP_ALIGN );
+		list->buffer[0].address = pci_map_single(priv->pciDev,
+							 skb->data,
+							 TLAN_MAX_FRAME_SIZE,
+							 PCI_DMA_FROMDEVICE);
+		TLan_StoreSKB(list, skb);
 		list->buffer[1].count = 0;
 		list->buffer[1].address = 0;
 		list->forward = list_phys + sizeof(TLanList);

@@ -2054,35 +1999,33 @@ static void TLan_FreeLists( struct net_device *dev )
 	TLanList	*list;
 	struct sk_buff	*skb;
 
-	if ( ! bbuf ) {
-		for ( i = 0; i < TLAN_NUM_TX_LISTS; i++ ) {
-			list = priv->txList + i;
-			skb = TLan_GetSKB(list);
-			if ( skb ) {
-				pci_unmap_single(
-					priv->pciDev,
-					list->buffer[0].address,
-					max(skb->len,
-					    (unsigned int)TLAN_MIN_FRAME_SIZE),
-					PCI_DMA_TODEVICE);
-				dev_kfree_skb_any( skb );
-				list->buffer[8].address = 0;
-				list->buffer[9].address = 0;
-			}
+	for ( i = 0; i < TLAN_NUM_TX_LISTS; i++ ) {
+		list = priv->txList + i;
+		skb = TLan_GetSKB(list);
+		if ( skb ) {
+			pci_unmap_single(
+				priv->pciDev,
+				list->buffer[0].address,
+				max(skb->len,
+				    (unsigned int)TLAN_MIN_FRAME_SIZE),
+				PCI_DMA_TODEVICE);
+			dev_kfree_skb_any( skb );
+			list->buffer[8].address = 0;
+			list->buffer[9].address = 0;
 		}
+	}
 
-		for ( i = 0; i < TLAN_NUM_RX_LISTS; i++ ) {
-			list = priv->rxList + i;
-			skb = TLan_GetSKB(list);
-			if ( skb ) {
-				pci_unmap_single(priv->pciDev,
-						 list->buffer[0].address,
-						 TLAN_MAX_FRAME_SIZE,
-						 PCI_DMA_FROMDEVICE);
-				dev_kfree_skb_any( skb );
-				list->buffer[8].address = 0;
-				list->buffer[9].address = 0;
-			}
+	for ( i = 0; i < TLAN_NUM_RX_LISTS; i++ ) {
+		list = priv->rxList + i;
+		skb = TLan_GetSKB(list);
+		if ( skb ) {
+			pci_unmap_single(priv->pciDev,
+					 list->buffer[0].address,
+					 TLAN_MAX_FRAME_SIZE,
+					 PCI_DMA_FROMDEVICE);
+			dev_kfree_skb_any( skb );
+			list->buffer[8].address = 0;
+			list->buffer[9].address = 0;
 		}
 	}
 } /* TLan_FreeLists */
Signature · 1 line
1.5.6.5
maintainer
#3 lore raw
In reply to Sakari Ailus

From: Sakari Ailus <[email protected]>
Date: Tue, 16 Dec 2008 17:24:50 +0200

Quoted 4 lines

Unfortunately it seems that my last patch introduced a few problems.

So here are two patches: the first one fixes the pci memory unmapping. The second one removes the big buffer mode.

I think what I'm going to do is put your first patch into net-2.6 and the bigbuf removal into net-next-2.6

Yes I realize that your changes might have made bigbuf even more broken than it already is, that's fine since it's off by default.

Thanks!

author
#4 lore raw
Quoted 10 lines · David Miller

From: Sakari Ailus <[email protected]>
Date: Tue, 16 Dec 2008 17:24:50 +0200

Quoted 4 lines

Unfortunately it seems that my last patch introduced a few problems.

So here are two patches: the first one fixes the pci memory unmapping. The second one removes the big buffer mode.

I think what I'm going to do is put your first patch into net-2.6 and the bigbuf removal into net-next-2.6

Sounds good, thanks!

Quoted 3 lines

Yes I realize that your changes might have made bigbuf even more broken than it already is, that's fine since it's off by default.

On second though, I don't think the bbuf mode was broken by my patches more than it already was. skb_copy_from_linear_data copies txlen bytes from skb->data to tail_buffer. This is fine, since the skb has at least that many bytes and tail_buffer is at least as big, too. One should never make this kind of claims when too tired... ;)

But the bottom line is still that the bbuf mode doesn't work.

Signature · 2 lines
Sakari Ailus
[email protected]