Hi,
Name: PCnet32 cable status patch
Author: Kent Yoder
Status: Tested on 2.4.20-pre8
D: A patch for the pcnet32 driver to check the status of the
D: cable via a watchdog timer as the e100/e1000 drivers do.
Comments and criticism are welcome, as I've gotten no response from the
maintainer.
Thanks,
Kent
--- linux-2.4.19.vanilla/drivers/net/pcnet32.c Fri Aug 2 19:39:44 2002
+++ linux-2.4.19/drivers/net/pcnet32.c Mon Sep 30 10:59:27 2002
@@ -22,8 +22,8 @@
*************************************************************************/
#define DRV_NAME "pcnet32"
-#define DRV_VERSION "1.27a"
-#define DRV_RELDATE "10.02.2002"
+#define DRV_VERSION "1.27b"
+#define DRV_RELDATE "30.09.2002"
#define PFX DRV_NAME ": "
static const char *version =
@@ -212,6 +212,8 @@
* fix pci probe not increment cards_found
* FD auto negotiate error workaround for xSeries250
* clean up and using new mii module
+ * v1.27b Sep 30 2002 Kent Yoder <[email protected]>
+ * Added timer for cable connection state changes.
*/
@@ -316,9 +318,11 @@
int shared_irq:1, /* shared irq possible */
ltint:1, /* enable TxDone-intr inhibitor */
dxsuflo:1, /* disable transmit stop on uflo */
- mii:1; /* mii port available */
+ mii:1, /* mii port available */
+ link:1; /* cable link up/down */
struct net_device *next;
struct mii_if_info mii_if;
+ struct timer_list watchdog_timer;
};
static void pcnet32_probe_vlbus(void);
@@ -334,6 +338,7 @@
static struct net_device_stats *pcnet32_get_stats(struct net_device *);
static void pcnet32_set_multicast_list(struct net_device *);
static int pcnet32_ioctl(struct net_device *, struct ifreq *, int);
+static void pcnet32_watchdog(struct net_device *);
static int mdio_read(struct net_device *dev, int phy_id, int reg_num);
static void mdio_write(struct net_device *dev, int phy_id, int reg_num, int val);
@@ -778,6 +783,13 @@
}
}
+ /* Set the mii phy_id so that we can query the link state */
+ if (lp->mii)
+ lp->mii_if.phy_id = ((lp->a.read_bcr (ioaddr, 33)) >> 5) & 0x1f;
+
+ init_timer(&lp->watchdog_timer);
+ lp->watchdog_timer.data = (unsigned long) dev;
+ lp->watchdog_timer.function = (void *) &pcnet32_watchdog;
/* The PCNET32-specific entries in the device structure. */
dev->open = &pcnet32_open;
@@ -902,6 +914,19 @@
netif_start_queue(dev);
+ /* If we have mii, print the link status and start the watchdog */
+ if (lp->mii) {
+ if(mii_link_ok (&lp->mii_if)) {
+ printk (KERN_INFO PFX "%s: Cable link is up.\n", dev->name);
+ lp->link = 1;
+ } else {
+ printk (KERN_INFO PFX "%s: Cable not connected.\n", dev->name);
+ lp->link = 0;
+ }
+
+ mod_timer(&(lp->watchdog_timer), jiffies + (2 * HZ));
+ }
+
i = 0;
while (i++ < 100)
if (lp->a.read_csr (ioaddr, 0) & 0x0100)
@@ -1372,6 +1397,8 @@
struct pcnet32_private *lp = dev->priv;
int i;
+ del_timer_sync(&lp->watchdog_timer);
+
netif_stop_queue(dev);
lp->stats.rx_missed_errors = lp->a.read_csr (ioaddr, 112);
@@ -1650,6 +1677,22 @@
}
}
return -EOPNOTSUPP;
+}
+
+static void pcnet32_watchdog(struct net_device *dev)
+{
+ struct pcnet32_private *lp = dev->priv;
+
+ if(!netif_running(dev))
+ return;
+
+ if(mii_link_ok(&lp->mii_if) != (lp->link & 1)) {
+ printk(KERN_INFO PFX "%s: Cable link is %s.\n",
+ dev->name, (lp->link ? "down" : "up"));
+ lp->link ^= 1;
+ }
+
+ mod_timer(&(lp->watchdog_timer), jiffies + (2 * HZ));
}
static struct pci_driver pcnet32_driver = {
Comments:
It looks good as a starting point :)
I just added mii_check_media() to drivers/net/mii.c. It's in the latest
2.5.x snapshot,
ftp://ftp.kernel.org/pub/linux/kernel/v2.5/snapshots/patch-2.5.39-bk2.bz2
and is in Marcelo's inbox as well. For simple implementations (and I
think pcnet32 qualifies), the timer should not need to do anything
beyond calling mii_check_media(). One important feature of this is use
of the standard netif_carrier_{off,on} to indicate link to the system.
netif_carrier_xxx also means you don't need lp->link...
>I just added mii_check_media() to drivers/net/mii.c. It's in the latest
>2.5.x snapshot,
>ftp://ftp.kernel.org/pub/linux/kernel/v2.5/snapshots/patch-2.5.39-bk2.bz2
That looks like just what the doctor ordered, but unfortunately it looks
like 2.5.39-bk2 is oopsing on boot on this machine. The boot freezes
and at the top of the screen I can see what looks like the tail end of an
oops (this is hand typed):
--------
wait_for_completion+0x12a/0x1e0
default_wake_function+0x0/0x40
try_to_wake_up+0x332/0x340
default_wake_function+0x0/0x40
set_cpus_allowed+0x22f/0x250
ksoftirqd+0x5b/0x110
ksoftirqd+0x0/0x110
kernel_thread_helper+0x5/0xc
[.. Linux NET starting.. other ACPI info ..]
ACPI Namespace successfully loaded at root c053f4bc
<freeze>
Kent
Kent Yoder wrote:
>>I just added mii_check_media() to drivers/net/mii.c. It's in the latest
>>2.5.x snapshot,
>>ftp://ftp.kernel.org/pub/linux/kernel/v2.5/snapshots/patch-2.5.39-bk2.bz2
>
>
> That looks like just what the doctor ordered, but unfortunately it looks
> like 2.5.39-bk2 is oopsing on boot on this machine.
for the purposes of testing, you can copy drivers/net/mii.c and
include/linux/mii.h directly into 2.4.x at those same locations.
Whups that's mine.
I'll get a fix out today.
-- Andy
> From: Kent Yoder [mailto:[email protected]]
[.. Linux NET starting.. other ACPI info ..]
ACPI Namespace successfully loaded at root c053f4bc
<freeze>
Kent
Hi,
Here's the updated version, now dependent on Jeff's new mii code. This is
a bit more modular as well and new functionality can be added inside the
watchdog function without anything depending on mii.
Thanks,
Kent
--- linux-2.5.39.vanilla/drivers/net/pcnet32.c Sat Sep 21 23:25:05 2002
+++ linux-2.5.39/drivers/net/pcnet32.c Tue Oct 1 11:21:13 2002
@@ -22,8 +22,8 @@
*************************************************************************/
#define DRV_NAME "pcnet32"
-#define DRV_VERSION "1.27a"
-#define DRV_RELDATE "10.02.2002"
+#define DRV_VERSION "1.27b"
+#define DRV_RELDATE "01.10.2002"
#define PFX DRV_NAME ": "
static const char *version =
@@ -96,6 +96,8 @@
#define PCNET32_DMA_MASK 0xffffffff
+#define PCNET32_WATCHDOG_TIMEOUT (jiffies + (2 * HZ))
+
/*
* table to translate option values from tulip
* to internal options
@@ -211,6 +213,8 @@
* fix pci probe not increment cards_found
* FD auto negotiate error workaround for xSeries250
* clean up and using new mii module
+ * v1.27b Sep 30 2002 Kent Yoder <[email protected]>
+ * Added timer for cable connection state changes.
*/
@@ -318,6 +322,7 @@
mii:1; /* mii port available */
struct net_device *next;
struct mii_if_info mii_if;
+ struct timer_list watchdog_timer;
};
static void pcnet32_probe_vlbus(void);
@@ -333,6 +338,7 @@
static struct net_device_stats *pcnet32_get_stats(struct net_device *);
static void pcnet32_set_multicast_list(struct net_device *);
static int pcnet32_ioctl(struct net_device *, struct ifreq *, int);
+static void pcnet32_watchdog(struct net_device *);
static int mdio_read(struct net_device *dev, int phy_id, int reg_num);
static void mdio_write(struct net_device *dev, int phy_id, int reg_num, int val);
@@ -777,6 +783,13 @@
}
}
+ /* Set the mii phy_id so that we can query the link state */
+ if (lp->mii)
+ lp->mii_if.phy_id = ((lp->a.read_bcr (ioaddr, 33)) >> 5) & 0x1f;
+
+ init_timer (&lp->watchdog_timer);
+ lp->watchdog_timer.data = (unsigned long) dev;
+ lp->watchdog_timer.function = (void *) &pcnet32_watchdog;
/* The PCNET32-specific entries in the device structure. */
dev->open = &pcnet32_open;
@@ -901,6 +914,12 @@
netif_start_queue(dev);
+ /* If we have mii, print the link status and start the watchdog */
+ if (lp->mii) {
+ mii_check_media (&lp->mii_if, 1, 1);
+ mod_timer (&(lp->watchdog_timer), PCNET32_WATCHDOG_TIMEOUT);
+ }
+
i = 0;
while (i++ < 100)
if (lp->a.read_csr (ioaddr, 0) & 0x0100)
@@ -1371,6 +1390,8 @@
struct pcnet32_private *lp = dev->priv;
int i;
+ del_timer_sync(&lp->watchdog_timer);
+
netif_stop_queue(dev);
lp->stats.rx_missed_errors = lp->a.read_csr (ioaddr, 112);
@@ -1651,6 +1672,17 @@
return -EOPNOTSUPP;
}
+static void pcnet32_watchdog(struct net_device *dev)
+{
+ struct pcnet32_private *lp = dev->priv;
+
+ /* Print the link status if it has changed */
+ if (lp->mii)
+ mii_check_media (&lp->mii_if, 1, 0);
+
+ mod_timer (&(lp->watchdog_timer), PCNET32_WATCHDOG_TIMEOUT);
+}
+
static struct pci_driver pcnet32_driver = {
name: DRV_NAME,
probe: pcnet32_probe_pci,
----- Original Message -----
From: "Kent Yoder" <[email protected]>
To: "Jeff Garzik" <[email protected]>
Cc: <[email protected]>; <[email protected]>
Sent: Tuesday, October 01, 2002 1:34 PM
Subject: Re: [PATCH] pcnet32 cable status check
>
> Hi,
>
> Here's the updated version, now dependent on Jeff's new mii code. This
is
> a bit more modular as well and new functionality can be added inside the
> watchdog function without anything depending on mii.
You should use netif_carrier_{on|off} to notify the upper layer of a
link change/loss (until the otherwise is true). Check the 8139cp driver.
Also, you shouldn't need the timer stuff to keep track of link change.
Just the mii_check_media and netif_carrier_{on|off} and you should be fine.
Felipe
Kent Yoder wrote:
> +static void pcnet32_watchdog(struct net_device *dev)
> +{
> + struct pcnet32_private *lp = dev->priv;
> +
> + /* Print the link status if it has changed */
> + if (lp->mii)
> + mii_check_media (&lp->mii_if, 1, 0);
> +
> + mod_timer (&(lp->watchdog_timer), PCNET32_WATCHDOG_TIMEOUT);
> +}
Looks good ;-)
One small thing -- since you appear to test all cases for (lp->mii)
before calling mod_timer, I don't think you need to test lp->mii inside
the timer...
As Felipe mentioned, using the link interrupt instead of a timer is
preferred -- but my own preference would be to apply your patch with the
small remove-lp->mii-check fixup, and then investigate the support of
link interrupts. The reasoning is that, pcnet32 covers a ton of chips,
and not all may support a link interrupt.
Jeff
Felipe W Damasio wrote:
> Also, you shouldn't need the timer stuff to keep track of link change.
> Just the mii_check_media and netif_carrier_{on|off} and you should be fine.
That depends on the hardware, specifically the presence of a reliable
link interrupt....
----- Original Message -----
From: "Jeff Garzik" <[email protected]>
To: "Kent Yoder" <[email protected]>
Cc: <[email protected]>; <[email protected]>
Sent: Tuesday, October 01, 2002 2:19 PM
Subject: Re: [PATCH] pcnet32 cable status check
> Kent Yoder wrote:
> > +static void pcnet32_watchdog(struct net_device *dev)
> > +{
> > + struct pcnet32_private *lp = dev->priv;
> > +
> > + /* Print the link status if it has changed */
> > + if (lp->mii)
> > + mii_check_media (&lp->mii_if, 1, 0);
> > +
> > + mod_timer (&(lp->watchdog_timer), PCNET32_WATCHDOG_TIMEOUT);
> > +}
>
>
> Looks good ;-)
>
> One small thing -- since you appear to test all cases for (lp->mii)
> before calling mod_timer, I don't think you need to test lp->mii inside
> the timer...
>
> As Felipe mentioned, using the link interrupt instead of a timer is
> preferred -- but my own preference would be to apply your patch with the
> small remove-lp->mii-check fixup, and then investigate the support of
> link interrupts. The reasoning is that, pcnet32 covers a ton of chips,
> and not all may support a link interrupt.
Sounds nice.
When identified which chips support Link Change, a bit indicating so
could be add to the capabilities of the NIC so we could choose link
interrupt instead of timer when appropriate.
I'll try and investigate some chips soon and let you know.
Felipe
>Felipe W Damasio wrote:
>> Also, you shouldn't need the timer stuff to keep track of link change.
>> Just the mii_check_media and netif_carrier_{on|off} and you should be fine.
>
>That depends on the hardware, specifically the presence of a reliable
>link interrupt....
Yeah, there's no interrupt generated by the cable being taken out on the
card I am testing with, a PCnet/FAST III 79C975. I am looking around for
another in the series though..
Kent
Thus Spake Jeff Garzik:
>
>Looks good ;-)
Thanks,
>
>One small thing -- since you appear to test all cases for (lp->mii)
>before calling mod_timer, I don't think you need to test lp->mii inside
>the timer...
Well, the reason I left that in there was so that another person could add
functionality to the watchdog if they wanted on a non-mii enabled card
without having to know that the check would need to be added. If that's not
that big a deal, I can remove it...
>As Felipe mentioned, using the link interrupt instead of a timer is
>preferred -- but my own preference would be to apply your patch with the
>small remove-lp->mii-check fixup, and then investigate the support of
>link interrupts. The reasoning is that, pcnet32 covers a ton of chips,
>and not all may support a link interrupt.
>
> Jeff
>
>
>
Kent Yoder wrote:
> Thus Spake Jeff Garzik:
>
>>Looks good ;-)
>
>
> Thanks,
>
>
>>One small thing -- since you appear to test all cases for (lp->mii)
>>before calling mod_timer, I don't think you need to test lp->mii inside
>>the timer...
>
>
> Well, the reason I left that in there was so that another person could add
> functionality to the watchdog if they wanted on a non-mii enabled card
> without having to know that the check would need to be added. If that's not
> that big a deal, I can remove it...
That's a fair enough argument, patch applied.
>
>That's a fair enough argument, patch applied.
>
Excellent, thanks. Are the mii changes planned to be included in a 2.4
tree at some time?
Kent
Thus Spake Jeff Garzik:
>As Felipe mentioned, using the link interrupt instead of a timer is
>preferred -- but my own preference would be to apply your patch with the
>small remove-lp->mii-check fixup, and then investigate the support of
>link interrupts. The reasoning is that, pcnet32 covers a ton of chips,
>and not all may support a link interrupt.
Jeff, I came across a PCNET FAST/79C971, which apparently also does not
give an interrupt when the cable is pulled. (My original tested card was a
PCnet/FAST III 79C975).
Kent