Re: eepro100 - 802.1q - mtu size

On Fri, Jan 17, 2003 at 07:08:40PM +0300, Paul P Komkoff Jr wrote:

Replying to Florian Lohoff:

quoted

Why is this patch not integerated yet ?

Because newer and better e100 driver, which accepts tagged frames and
handles it properly, already in the tree

Regardless, people still use eepro100, so I would still like to get
eepro100 doing VLAN.

The reason why the patch was not accepted is that it changes one magic
number to another magic number, and without chipset docs, I had no idea
what either magic number really meant.

Now that Intel has released chipset docs, this is an excellent time to
re-evaluate those eepro100 VLAN changes.  I still refuse to accept a
"change the magic numbers" patch... any change will need to define
a constant that describes the bits we wish to set/clear.

Download the e100 documentation from the e1000 sourceforge site:
http://sourceforge.net/projects/e1000
["8255x Developer Manual"]

	Jeff

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:

 > The reason why the patch was not accepted is that it changes one magic
 > number to another magic number, and without chipset docs, I had no idea
 > what either magic number really meant.

Whilst on the subject of magic numbers in net drivers, did we ever get
to the bottom of 2.4's ChangeSet 1.587.9.20 

ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff

?

		Dave

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Hi Jeff,

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:

On Fri, Jan 17, 2003 at 07:08:40PM +0300, Paul P Komkoff Jr wrote:

quoted

Replying to Florian Lohoff:

quoted

Why is this patch not integerated yet ?

Because newer and better e100 driver, which accepts tagged frames and
handles it properly, already in the tree

Regardless, people still use eepro100, so I would still like to get
eepro100 doing VLAN.

The reason why the patch was not accepted is that it changes one magic
number to another magic number, and without chipset docs, I had no idea
what either magic number really meant.

Now that Intel has released chipset docs, this is an excellent time to
re-evaluate those eepro100 VLAN changes.  I still refuse to accept a
"change the magic numbers" patch... any change will need to define
a constant that describes the bits we wish to set/clear.

Download the e100 documentation from the e1000 sourceforge site:
http://sourceforge.net/projects/e1000
["8255x Developer Manual"]

	Jeff

BTW, it look like i82557_config_cmd is never used...

--- linux-2.4/drivers/net/eepro100.c	2003/01/17 17:33:41	1.1
+++ linux-2.4/drivers/net/eepro100.c	2003/01/17 17:34:32

@@ -499,11 +499,13 @@
 /* The parameters for a CmdConfigure operation.
    There are so many options that it would be difficult to document each bit.
    We mostly use the default or recommended settings. */
+#if 0
 static const char i82557_config_cmd[CONFIG_DATA_SIZE] = {
 	22, 0x08, 0, 0,  0, 0, 0x32, 0x03,  1, /* 1=Use MII  0=Use AUI */
 	0, 0x2E, 0,  0x60, 0,
 	0xf2, 0x48,   0, 0x40, 0xf2, 0x80, 		/* 0x40=Force full-duplex */
 	0x3f, 0x05, };
+#endif
 static const char i82558_config_cmd[CONFIG_DATA_SIZE] = {
 	22, 0x08, 0, 1,  0, 0, 0x22, 0x03,  1, /* 1=Use MII  0=Use AUI */
 	0, 0x2E, 0,  0x60, 0x08, 0x88,

-- 
Ducrot Bruno
http://www.poupinou.org        Page profaissionelle
http://toto.tu-me-saoules.com  Haume page

On Fri, Jan 17, 2003 at 05:27:19PM +0000, Dave Jones wrote:

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:

 > The reason why the patch was not accepted is that it changes one magic
 > number to another magic number, and without chipset docs, I had no idea
 > what either magic number really meant.

Whilst on the subject of magic numbers in net drivers, did we ever get
to the bottom of 2.4's ChangeSet 1.587.9.20 

ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff

IIRC it came from -ac tree without explanation, and I think akpm said it
broke stuff.  Since it has an alive maintainer (akpm), I would rather
let Alan and Andrew fight it out :)  Whatever they decide is fine with
me for 2.5.

	Jeff

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:

Regardless, people still use eepro100, so I would still like to get
eepro100 doing VLAN.

The reason why the patch was not accepted is that it changes one magic
number to another magic number, and without chipset docs, I had no idea
what either magic number really meant.

I did the basic decoding of the config block - The result from looking
at the gdb output is the same except the IP ARP Filter basic address
which is now the documentations recommended default value and some
mandatory bits which are filled.

I also set the LONGRCV option which enables to use VLANs.

I tried this patch which is against 2.4.20 with IPv4, IPv6, 802.1q and
DHCP so i'd say full feature test ;). I did not try 82557 which
might need different initialization as some bits are unused but that
problem persists from before this change.

--- drivers/net/eepro100.c-vanilla	Fri Jan 17 20:18:17 2003
+++ drivers/net/eepro100.c	Fri Jan 17 20:19:39 2003

@@ -496,19 +496,66 @@
 #endif
 };
 
-/* The parameters for a CmdConfigure operation.
-   There are so many options that it would be difficult to document each bit.
-   We mostly use the default or recommended settings. */
-static const char i82557_config_cmd[CONFIG_DATA_SIZE] = {
-	22, 0x08, 0, 0,  0, 0, 0x32, 0x03,  1, /* 1=Use MII  0=Use AUI */
-	0, 0x2E, 0,  0x60, 0,
-	0xf2, 0x48,   0, 0x40, 0xf2, 0x80, 		/* 0x40=Force full-duplex */
-	0x3f, 0x05, };
+#define i55x_CB1_TXFIFO_LIMIT(x)	((x)<<4)
+#define i55x_CB1_RXFIFO_LIMIT(x)	((x)&0xf)
+
+#define i55x_CB3_MWIENABLE		(1<<0)
+
+#define i55x_CB6_EXTSTATCOUNT		(1<<5)
+#define i55x_CB6_MANDBITS		(1<<1)
+
+#define i55x_CB7_DISCARDSHORT		(1<<0)
+#define	i55x_CB7_UNDERRUNRETRY(x)	((x)<<1)
+
+#define i557_CB8_USEMII			(1<<0)
+#define i558_CB8_MANDBITS		i557_CB8_USEMII
+
+#define i55x_CB10_MANDBITS		(3<<1)
+#define i55x_CB10_PREAMBLELEN(x)	((x&3)<<4)
+#define i55x_CB10_NSAI			(1<<3)
+
+#define i55x_CB12_INTERFRAMESPACE(x)	((x)<<4)
+
+#define i558_CB15_MANDBITS		(1<<3|1<<6)
+#define i559_CB16_CRC16			(1<<5)
+
+#define i558_CB18_MANDBITS		(1<<7)
+#define i558_CB18_FCTHRESH(x)		((x&7)<<4)
+#define i558_CB18_LONGRCV		(1<<3)
+#define i55x_CB18_PAD			(1<<1)
+
+#define i558_CB19_TXFC			(1<<2)
+#define i558_CB19_AUTOFDX		(1<<7)
+
+#define i558_CB20_PRIFCLOC		(1<<5)
+#define i558_CB20_MANDBITS		(0x1f)
+
+#define i558_CB21_MANDBITS		(0x05)
+
 static const char i82558_config_cmd[CONFIG_DATA_SIZE] = {
-	22, 0x08, 0, 1,  0, 0, 0x22, 0x03,  1, /* 1=Use MII  0=Use AUI */
-	0, 0x2E, 0,  0x60, 0x08, 0x88,
-	0x68, 0, 0x40, 0xf2, 0x84,		/* Disable FC */
-	0x31, 0x05, };
+	CONFIG_DATA_SIZE,				
+	i55x_CB1_TXFIFO_LIMIT(0)|i55x_CB1_RXFIFO_LIMIT(8),	
+	0,		
+	i55x_CB3_MWIENABLE,
+	0,			
+	0,				
+	i55x_CB6_EXTSTATCOUNT|i55x_CB6_MANDBITS,
+	i55x_CB7_UNDERRUNRETRY(1)|i55x_CB7_DISCARDSHORT,
+	i558_CB8_MANDBITS,				
+	0,
+	i55x_CB10_MANDBITS|i55x_CB10_NSAI|i55x_CB10_PREAMBLELEN(2),
+	0,
+	i55x_CB12_INTERFRAMESPACE(6),
+	0x00,							/* cb 13 - ARP Filter IP Address Low */
+	0xf2,							/* cb 14 - ARP Filter IP Address High */
+	i558_CB15_MANDBITS|i559_CB16_CRC16,			
+	0,							/* cb 16 - FC Delay - LSB */
+	0x40,							/* cb 17 - FC Delay - MSB */
+	i558_CB18_MANDBITS|i558_CB18_FCTHRESH(7)|i558_CB18_LONGRCV|i55x_CB18_PAD,
+	i558_CB19_TXFC|i558_CB19_AUTOFDX,		
+	i558_CB20_PRIFCLOC|i558_CB20_MANDBITS,
+	i558_CB21_MANDBITS,
+	};
 
 /* PHY media interface chips. */
 static const char *phys[] = {

-- 
Florian Lohoff                  flo@rfc822.org             +49-5201-669912
                        Heisenberg may have been here.

On Fri, 2003-01-17 at 17:49, Jeff Garzik wrote:

quoted

Whilst on the subject of magic numbers in net drivers, did we ever get
to the bottom of 2.4's ChangeSet 1.587.9.20 

ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff

IIRC it came from -ac tree without explanation, and I think akpm said it
broke stuff.  Since it has an alive maintainer (akpm), I would rather
let Alan and Andrew fight it out :)  Whatever they decide is fine with
me for 2.5.

I've had no reports I remember about it breaking stuff, and several that
it fixed stuff. It also seems to match the documentation. Its been in -ac
for ages

Alan wrote:

I've had no reports I remember about it breaking stuff, and several that
it fixed stuff. It also seems to match the documentation. Its been in -ac
for ages

Andrew wrote:

I've seen no reports either way, and appear to have misplaced the
datasheet.

May as well run with it.

Sounds good.  Dave, whatever your preference for patching handling is, 
is all good.  I guess yourself, myself, and akpm are all patch-mongering 
candidates...

	Jeff