Re: [PATCH] check_duplex bug causes HD operation, carrier errors

Just a "ping" to verify you saw this...

Thanks
- Bhavesh
-- 
Bhavesh P. Davda
Avaya Inc
[email protected]

-------- Original Message --------
Subject: Re: [tulip-bug] [PATCH] check_duplex bug causes HD operation, 
carrier errors
Date: Fri, 28 Jun 2002 13:40:45 -0600
From: "Bhavesh P. Davda" <[email protected]>
Organization: Avaya, Inc.
To: Donald Becker <[email protected]>
CC: [email protected]
References: <Pine.LNX.4.33.0206281213070.1095-100000@presario>

I disagree. Yeah, though this patch restarts the transmitter every
minute, atleast it selects the right duplex setting.

The problem was: if 'tp->full_duplex' is initially set, and if the
negotiated 'duplex' is also set in check_duplex, then the code doesn't
set the FullDuplex bit in CSR6. In that case, like in the case of a
tulip_open(), CSR6 gets set to half-duplex (default setting through
init_media())

I verified this with tulip-diag, and also by setting tulip_debug=3.

As an alternative, an additional parameter could be passed to
check_duplex() to say that this is called from tulip_open(), so set the
FullDuplex bit in CSR6 if necessary, even if tp->full_duplex == duplex

Here is that alternative patch:

diff -Naur orig/tulip.c new/tulip.c
--- orig/tulip.c	Thu Jun 27 15:37:41 2002
+++ new/tulip.c	Fri Jun 28 13:36:15 2002
@@ -623,7 +623,7 @@
   static void mdio_write(struct net_device *dev, int phy_id, int
location, int value);
   static int tulip_open(struct net_device *dev);
   /* Chip-specific media selection (timer functions prototyped above). */
-static int  check_duplex(struct net_device *dev);
+static int  check_duplex(struct net_device *dev, int startup);
   static void select_media(struct net_device *dev, int startup);
   static void init_media(struct net_device *dev);
   static void nway_lnk_change(struct net_device *dev, int csr5);
@@ -1516,7 +1516,7 @@
   		tp->full_duplex = 0;
   	init_media(dev);
   	if (media_cap[dev->if_port] & MediaIsMII)
-
	check_duplex(dev);
+
	check_duplex(dev, 1);
   	set_rx_mode(dev);

   	/* Start the Tx to process setup frame. */
@@ -1887,7 +1887,7 @@
     Return 0 if everything is OK.
     Return < 0 if the transceiver is missing or has no link beat.
     */
-static int check_duplex(struct net_device *dev)
+static int check_duplex(struct net_device *dev, int startup)
   {
   	long ioaddr = dev->base_addr;
   	struct tulip_private *tp = (struct tulip_private *)dev->priv;
@@ -1916,7 +1916,7 @@
   	duplex = ((negotiated & 0x0300) == 0x0100
   	
	  || (negotiated & 0x00C0) == 0x0040);
   	/* 100baseTx-FD  or  10T-FD, but not 100-HD */
-
if (tp->full_duplex != duplex) {
+
if ((startup) || (tp->full_duplex != duplex)) {
   		tp->full_duplex = duplex;
   		if (negotiated & 0x0380)	/* 100mbps. */
   	
	tp->csr6 &= ~0x00400000;
@@ -2079,7 +2079,7 @@
   		}
   		case 1:  case 3:		/* 21140, 21142 MII */
   		actually_mii:
-
		check_duplex(dev);
+
		check_duplex(dev, 0);
   	
	next_tick = 60*HZ;
   	
	break;
   		case 2:					/* 21142 serial block has no link beat. */
@@ -2109,7 +2109,7 @@
   		printk(KERN_INFO"%s: N-Way autonegotiation status %8.8x, %s.\n",
   	
	   dev->name, csr12, medianame[dev->if_port]);
   	if (media_cap[dev->if_port] & MediaIsMII) {
-
	check_duplex(dev);
+
	check_duplex(dev, 0);
   	} else if (tp->nwayset) {
   		/* Do not screw up a negotiated session! */
   		if (tp->msg_level & NETIF_MSG_TIMER)
@@ -2407,7 +2407,7 @@
   	int next_tick = 60*HZ;

   	if (media_cap[dev->if_port] & MediaIsMII) {
-
	if (check_duplex(dev) > 0)
+
	if (check_duplex(dev, 0) > 0)
   	
	next_tick = 3*HZ;
   	} else {
   		int csr12 = inl(ioaddr + CSR12);
@@ -2475,7 +2475,7 @@
   	
	   "%4.4x.\n",
   	
	   dev->name, mdio_read(dev, tp->phys[0], 1),
   	
	   mdio_read(dev, tp->phys[0], 5));
-
check_duplex(dev);
+
check_duplex(dev, 0);
   	tp->timer.expires = jiffies + next_tick;
   	add_timer(&tp->timer);
   }


-- 
Bhavesh P. Davda
Avaya Inc
[email protected]


Donald Becker wrote:
 > On Thu, 27 Jun 2002, Bhavesh P. Davda wrote:
 >
 >
 >>Subject: [tulip-bug] [PATCH] check_duplex bug causes HD operation,
 >>     carrier errors
 >>
 >>Finally was able to track this bug down to a fairly simple set of
 >>operations:
 >
 >
 > Errrmm, this patch stops the transmitter to change the duplex every
 > timer tick.
 >
 > I'm not seeing the bug that this fixes.
 > It appears to fix a problem where the 'tp->full_duplex' variable is
 > initially set, but the chip has not been put in full duplex mode.  Since
 > the driver thinks the duplex setting is fine, it is never updated.
 >
 > This mismatch should be pretty clearly shown with tulip-diag.
 >
 >
 >>diff -Naur orig/tulip.c new/tulip.c
 >>--- orig/tulip.c	Thu Jun 27 15:37:41 2002
 >>+++ new/tulip.c	Thu Jun 27 15:39:05 2002
 >>@@ -1916,21 +1916,18 @@
 >> 	duplex = ((negotiated & 0x0300) == 0x0100
 >> 			  || (negotiated & 0x00C0) == 0x0040);
 >> 	/* 100baseTx-FD  or  10T-FD, but not 100-HD */
 >>-	if (tp->full_duplex != duplex) {
 >>- 
	tp->full_duplex = duplex;
 >>- 
	if (negotiated & 0x0380)	/* 100mbps. */
 >>- 
		tp->csr6 &= ~0x00400000;
 >>- 
	if (tp->full_duplex) tp->csr6 |= FullDuplex;
 >>- 
	else				 tp->csr6 &= ~FullDuplex;
 >>- 
	outl(tp->csr6 | RxOn, ioaddr + CSR6);
 >>- 
	outl(tp->csr6 | TxOn | RxOn, ioaddr + CSR6);
 >>- 
	if (tp->msg_level & NETIF_MSG_LINK)
 >>- 
		printk(KERN_INFO "%s: Setting %s-duplex based on MII "
 >>- 
			   "#%d link partner capability of %4.4x.\n",
 >>- 
			   dev->name, tp->full_duplex ? "full" : "half",
 >>- 
			   tp->phys[0], mii_reg5);
 >>- 
	return 1;
 >>-	}
 >>+	tp->full_duplex = duplex;
 >>+	if (negotiated & 0x0380)	/* 100mbps. */
 >>+ 
	tp->csr6 &= ~0x00400000;
 >>+	if (tp->full_duplex) tp->csr6 |= FullDuplex;
 >>+	else 
			 tp->csr6 &= ~FullDuplex;
 >>+	outl(tp->csr6 | RxOn, ioaddr + CSR6);
 >>+	outl(tp->csr6 | TxOn | RxOn, ioaddr + CSR6);
 >>+	if (tp->msg_level & NETIF_MSG_LINK)
 >>+ 
	printk(KERN_INFO "%s: Setting %s-duplex based on MII "
 >>+ 
		   "#%d link partner capability of %4.4x.\n",
 >>+ 
		   dev->name, tp->full_duplex ? "full" : "half",
 >>+ 
		   tp->phys[0], mii_reg5);
 >> 	return 0;
 >> }
 >>
 >>
 >
 >

_______________________________________________
tulip-bug mailing list, [email protected]
To change to digest mode or unsubscribe visit
http://www.scyld.com/mailman/listinfo/tulip-bug