[PATCH] Proper perfect filter setup for xircom_tulip_cb.c

[PATCH] Proper perfect filter setup for xircom_tulip_cb.c

[Ion Badulescu]

Hi Jeff,

The reason some CardBus Xircom cards need to be put in promisc mode in 
order to receive packets is because the driver doesn't initialize the 
perfect filter correctly. Although it uses a single Tx descriptor to send 
the initialization data to the card, it doesn't set either of the 
first_segment or last_segment bits in the descriptor. No wonder the 
chipset gets confused...

The attached patch fixed this error by setting both bits, and it also gets
rid of the unnecessary special case for the Xircom -- the chip really does
have a 16-slot perfect filter, like the other tulip clones.

Please apply.

Thanks,
Ion

-- 
  It is better to keep your mouth shut and be thought a fool,
            than to open it and remove all doubt.
----------------------
--- /src/vanilla/linux-2.4.5-ac9/drivers/net/pcmcia/xircom_tulip_cb.c	Sun Mar 25 18:24:31 2001
+++ linux-2.4/drivers/net/pcmcia/xircom_tulip_cb.c	Tue Jun  5 22:16:07 2001
@@ -1272,7 +1273,7 @@
 		*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
 		*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
 		/* Put the setup frame on the Tx list. */
-		tp->tx_ring[0].length = 0x08000000 | 192;
+		tp->tx_ring[0].length = 0x68000000 | 192;
 		tp->tx_ring[0].buffer1 = virt_to_bus(tp->setup_frame);
 		tp->tx_ring[0].status = DescOwned;
 
@@ -1291,7 +1292,7 @@
 		}
 
 		/* Put the setup frame on the Tx list. */
-		tp->tx_ring[tp->cur_tx].length = 0x08000000 | 192;
+		tp->tx_ring[tp->cur_tx].length = 0x68000000 | 192;
 		/* Lie about the address of our setup frame to make the */
 		/* chip happy */
 		tp->tx_ring[tp->cur_tx].buffer1 = virt_to_bus(tp->setup_frame);
@@ -2887,14 +2888,14 @@
 	} else {
 		u16 *eaddrs, *setup_frm = tp->setup_frame;
 		struct dev_mc_list *mclist;
-		u32 tx_flags = 0x08000000 | 192;
+		u32 tx_flags = 0x68000000 | 192;
 		int i;
 
 		/* Note that only the low-address shortword of setup_frame is valid!
 		   The values are doubled for big-endian architectures. */
-		if ((dev->mc_count > 14) || ((dev->mc_count > 6) && (tp->chip_id == X3201_3))) { /* Must use a multicast hash table. */
+		if (dev->mc_count > 14) { /* Must use a multicast hash table. */
 			u16 hash_table[32];
-			tx_flags = 0x08400000 | 192;		/* Use hash filter. */
+			tx_flags = 0x68400000 | 192;		/* Use hash filter. */
 			memset(hash_table, 0, sizeof(hash_table));
 			set_bit(255, hash_table); 			/* Broadcast entry */
 			/* This should work on big-endian machines as well. */
@@ -2907,7 +2908,7 @@
 				*setup_frm++ = hash_table[i];
 			}
 			setup_frm = &tp->setup_frame[13*6];
-		} else if(tp->chip_id != X3201_3) {
+		} else {
 			/* We have <= 14 addresses so we can use the wonderful
 			   16 address perfect filtering of the Tulip. */
 			for (i = 0, mclist = dev->mc_list; i < dev->mc_count;
@@ -2917,30 +2918,6 @@
 				*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
 				*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
 			}
-			/* Fill the unused entries with the broadcast address. */
-			memset(setup_frm, 0xff, (15-i)*12);
-			setup_frm = &tp->setup_frame[15*6];
-		} else {
-			/* fill the first two table entries with our address */
-			eaddrs = (u16 *)dev->dev_addr;
-			for(i=0; i<2; i++) {
-				*setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
-				*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
-				*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
-			}
-			/* Double fill each entry to accomodate chips that */
-			/* don't like to parse these correctly */
-			for (i=0, mclist=dev->mc_list; i<dev->mc_count;
-				 i++, mclist=mclist->next) {
-				eaddrs = (u16 *)mclist->dmi_addr;
-				*setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
-				*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
-				*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
-				*setup_frm++ = eaddrs[0]; *setup_frm++ = eaddrs[0];
-				*setup_frm++ = eaddrs[1]; *setup_frm++ = eaddrs[1];
-				*setup_frm++ = eaddrs[2]; *setup_frm++ = eaddrs[2];
-			}
-			i=((i+1)*2);
 			/* Fill the unused entries with the broadcast address. */
 			memset(setup_frm, 0xff, (15-i)*12);
 			setup_frm = &tp->setup_frame[15*6];

Re: [PATCH] Proper perfect filter setup for xircom_tulip_cb.c

[Keith Owens]

On Tue, 5 Jun 2001 22:34:05 -0700 (PDT), 
Ion Badulescu <ionut@cs.columbia.edu> wrote:
>The reason some CardBus Xircom cards need to be put in promisc mode in 
>order to receive packets is because the driver doesn't initialize the 
>perfect filter correctly. Although it uses a single Tx descriptor to send 
>the initialization data to the card, it doesn't set either of the 
>first_segment or last_segment bits in the descriptor. No wonder the 
>chipset gets confused...

Nicely spotted.  The X3201-3 Software Specification says nothing about
the segment bits for the filter, instead the information is tucked away
in the 21143 PCI/CardBus 10/100Mb/s Ethernet LAN Controller Hardware
Reference Manual.  So Xircom have a software specification manual that
does not include the full software spec, oh the horrors.

Re: [PATCH] Proper perfect filter setup for xircom_tulip_cb.c

[Ion Badulescu]

On Wed, 6 Jun 2001, Keith Owens wrote:

> Nicely spotted.  The X3201-3 Software Specification says nothing about
> the segment bits for the filter, instead the information is tucked away
> in the 21143 PCI/CardBus 10/100Mb/s Ethernet LAN Controller Hardware
> Reference Manual.  So Xircom have a software specification manual that
> does not include the full software spec, oh the horrors.

If that were the only thing missing... At least I was able to catch it by 
simply being tidy -- I was going over the control bits to see which of 
them need to be/make sense to be set. 

But what about setting the hash (the imperfect filter)? The 21143 docs say
the *upper* 9 bits of the crc32 should be used; the Xircom docs are
completely silent, and the driver currently uses the *lower* 9 bits of the
crc32.

Oh, and for that matter, the hash is currently broken on the Xircom: 
unlike the 21143, the Xircom has 4 perfect filter slots in hash mode, not
one, but the driver ignores this fact. The hash layout is also severely
brain-damaged... I'll look into fixing it tomorrow -- by simply copying 
the fix from the patch I wrote for the other xircom driver (xircom_cb). :-)

Thanks,
Ion

-- 
  It is better to keep your mouth shut and be thought a fool,
            than to open it and remove all doubt.