[tulip-bug] [PATCH] check_duplex bug causes HD operation, carrier
errors
Bhavesh P. Davda
bhavesh@avaya.com
Fri Jun 28 15:41:01 2002
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
bhavesh@avaya.com
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;
>> }
>>
>>
>
>