Hi,
This patch fixes two real problems: missing initialization of a register
which broke 100mbit half-duplex, and dereferencing of freed memory. It
also massages the whitespace a bit.
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.
---------------------------
--- ../linux-2.4/drivers/net/starfire.c Fri Jul 6 00:32:24 2001
+++ linux-2.4/drivers/net/starfire.c Fri Jul 6 00:46:26 2001
@@ -85,13 +85,18 @@
- Fixed 2.2.x compatibility issues introduced in 1.3.1
- Fixed ethtool ioctl returning uninitialized memory
+ LK1.3.3 (Ion Badulescu)
+ - Initialize the TxMode register properly
+ - Set the MII registers _after_ resetting it
+ - Don't dereference dev->priv after unregister_netdev() has freed it
+
TODO:
- implement tx_timeout() properly
*/
#define DRV_NAME "starfire"
-#define DRV_VERSION "1.03+LK1.3.2"
-#define DRV_RELDATE "June 04, 2001"
+#define DRV_VERSION "1.03+LK1.3.3"
+#define DRV_RELDATE "July 05, 2001"
/*
* Adaptec's license for their Novell drivers (which is where I got the
@@ -192,12 +197,6 @@
#define skb_first_frag_len(skb) (skb->len)
#endif /* not ZEROCOPY */
-#if !defined(__OPTIMIZE__)
-#warning You must compile this file with the correct options!
-#warning See the last lines of the source file.
-#error You must compile this driver with "-O".
-#endif
-
#include <linux/version.h>
#include <linux/module.h>
#include <linux/kernel.h>
@@ -208,7 +207,6 @@
#include <linux/delay.h>
#include <asm/processor.h> /* Processor type for cache alignment. */
#include <asm/uaccess.h>
-#include <asm/io.h>
#ifdef HAS_FIRMWARE
#include "starfire_firmware.h"
@@ -559,7 +557,7 @@
unsigned int cur_tx, dirty_tx;
unsigned int rx_buf_sz; /* Based on MTU+slack. */
unsigned int tx_full:1, /* The Tx queue is full. */
- /* These values are keep track of the transceiver/media in use. */
+ /* These values keep track of the transceiver/media in use. */
autoneg:1, /* Autonegotiation allowed. */
full_duplex:1, /* Full-duplex operation. */
speed100:1; /* Set if speed == 100MBit. */
@@ -572,6 +570,7 @@
unsigned char phys[PHY_CNT]; /* MII device addresses. */
};
+
static int mdio_read(struct net_device *dev, int phy_id, int location);
static void mdio_write(struct net_device *dev, int phy_id, int location, int value);
static int netdev_open(struct net_device *dev);
@@ -589,7 +588,7 @@
static int netdev_close(struct net_device *dev);
static void netdev_media_change(struct net_device *dev);
-
+
static int __devinit starfire_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
@@ -615,9 +614,9 @@
if (pci_enable_device (pdev))
return -EIO;
- ioaddr = pci_resource_start (pdev, 0);
- io_size = pci_resource_len (pdev, 0);
- if (!ioaddr || ((pci_resource_flags (pdev, 0) & IORESOURCE_MEM) == 0)) {
+ ioaddr = pci_resource_start(pdev, 0);
+ io_size = pci_resource_len(pdev, 0);
+ if (!ioaddr || ((pci_resource_flags(pdev, 0) & IORESOURCE_MEM) == 0)) {
printk (KERN_ERR DRV_NAME " %d: no PCI MEM resources, aborting\n", card_idx);
return -ENODEV;
}
@@ -777,14 +776,13 @@
err_out_free_res:
pci_release_regions (pdev);
err_out_free_netdev:
- unregister_netdev (dev);
- kfree (dev);
+ unregister_netdev(dev);
+ kfree(dev);
return -ENODEV;
}
-
-/* Read the MII Management Data I/O (MDIO) interfaces. */
+/* Read the MII Management Data I/O (MDIO) interfaces. */
static int mdio_read(struct net_device *dev, int phy_id, int location)
{
long mdio_addr = dev->base_addr + MIICtrl + (phy_id<<7) + (location<<2);
@@ -800,6 +798,7 @@
return result & 0xffff;
}
+
static void mdio_write(struct net_device *dev, int phy_id, int location, int value)
{
long mdio_addr = dev->base_addr + MIICtrl + (phy_id<<7) + (location<<2);
@@ -808,7 +807,7 @@
return;
}
-
+
static int netdev_open(struct net_device *dev)
{
struct netdev_private *np = dev->priv;
@@ -931,6 +930,8 @@
/* Initialize other registers. */
/* Configure the PCI bus bursts and FIFO thresholds. */
np->tx_mode = 0x0C04; /* modified when link is up. */
+ writel(0x8000 | np->tx_mode, ioaddr + TxMode);
+ writel(np->tx_mode, ioaddr + TxMode);
np->tx_threshold = 4;
writel(np->tx_threshold, ioaddr + TxThreshold);
@@ -986,12 +987,12 @@
struct netdev_private *np = dev->priv;
u16 reg0;
- mdio_write(dev, np->phys[0], MII_ADVERTISE, np->advertising);
mdio_write(dev, np->phys[0], MII_BMCR, BMCR_RESET);
udelay(500);
while (mdio_read(dev, np->phys[0], MII_BMCR) & BMCR_RESET);
reg0 = mdio_read(dev, np->phys[0], MII_BMCR);
+ mdio_write(dev, np->phys[0], MII_ADVERTISE, np->advertising);
if (np->autoneg) {
reg0 |= BMCR_ANENABLE | BMCR_ANRESTART;
@@ -1098,6 +1099,7 @@
return;
}
+
static int start_tx(struct sk_buff *skb, struct net_device *dev)
{
struct netdev_private *np = dev->priv;
@@ -1214,6 +1216,7 @@
return 0;
}
+
/* The interrupt handler does all of the Rx thread work and cleans up
after the Tx thread. */
static void intr_handler(int irq, void *dev_instance, struct pt_regs *rgs)
@@ -1350,6 +1353,7 @@
#endif
}
+
/* This routine is logically part of the interrupt handler, but separated
for clarity and better register allocation. */
static int netdev_rx(struct net_device *dev)
@@ -1407,11 +1411,9 @@
memcpy(skb_put(skb, pkt_len), np->rx_info[entry].skb->tail, pkt_len);
#endif
} else {
- char *temp;
-
pci_unmap_single(np->pci_dev, np->rx_info[entry].mapping, np->rx_buf_sz, PCI_DMA_FROMDEVICE);
skb = np->rx_info[entry].skb;
- temp = skb_put(skb, pkt_len);
+ skb_put(skb, pkt_len);
np->rx_info[entry].skb = NULL;
np->rx_info[entry].mapping = 0;
}
@@ -1570,6 +1572,7 @@
np->stats.tx_fifo_errors++;
}
+
static struct net_device_stats *get_stats(struct net_device *dev)
{
long ioaddr = dev->base_addr;
@@ -1596,6 +1599,7 @@
return &np->stats;
}
+
/* The little-endian AUTODIN II ethernet CRC calculations.
A big-endian version is also available.
This is slow but compact code. Do not use this routine for bulk data,
@@ -1622,6 +1626,7 @@
return crc;
}
+
static void set_rx_mode(struct net_device *dev)
{
long ioaddr = dev->base_addr;
@@ -1658,7 +1663,7 @@
memset(mc_filter, 0, sizeof(mc_filter));
for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
- i++, mclist = mclist->next) {
+ i++, mclist = mclist->next) {
int bit_nr = ether_crc_le(ETH_ALEN, mclist->dmi_addr) >> 23;
__u32 *fptr = (__u32 *) &mc_filter[(bit_nr >> 4) & ~1];
@@ -1679,7 +1684,6 @@
}
-
static int netdev_ethtool_ioctl(struct net_device *dev, void *useraddr)
{
struct ethtool_cmd ecmd;
@@ -1779,7 +1783,6 @@
}
-
static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
struct netdev_private *np = dev->priv;
@@ -1808,10 +1811,11 @@
u16 value = data->val_in;
switch (data->reg_num) {
case 0:
- if (value & 0x9000) /* Autonegotiation. */
+ if (value & (BMCR_RESET | BMCR_ANENABLE))
+ /* Autonegotiation. */
np->autoneg = 1;
else {
- np->full_duplex = (value & 0x0100) ? 1 : 0;
+ np->full_duplex = (value & BMCR_FULLDPLX) ? 1 : 0;
np->autoneg = 0;
}
break;
@@ -1921,27 +1925,26 @@
BUG();
np = dev->priv;
-
- unregister_netdev(dev);
- iounmap((char *)dev->base_addr);
- pci_release_regions(pdev);
-
if (np->tx_done_q)
- pci_free_consistent(np->pci_dev, PAGE_SIZE,
+ pci_free_consistent(pdev, PAGE_SIZE,
np->tx_done_q, np->tx_done_q_dma);
if (np->rx_done_q)
- pci_free_consistent(np->pci_dev, PAGE_SIZE,
+ pci_free_consistent(pdev,
+ sizeof(struct rx_done_desc) * DONE_Q_SIZE,
np->rx_done_q, np->rx_done_q_dma);
if (np->tx_ring)
- pci_free_consistent(np->pci_dev, PAGE_SIZE,
+ pci_free_consistent(pdev, PAGE_SIZE,
np->tx_ring, np->tx_ring_dma);
if (np->rx_ring)
- pci_free_consistent(np->pci_dev, PAGE_SIZE,
+ pci_free_consistent(pdev, PAGE_SIZE,
np->rx_ring, np->rx_ring_dma);
- kfree(dev);
+ unregister_netdev(dev); /* Will also free np!! */
+ iounmap((char *)dev->base_addr);
+ pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
+ kfree(dev);
}
Ion Badulescu wrote:
> + unregister_netdev(dev); /* Will also free np!! */
> + iounmap((char *)dev->base_addr);
> + pci_release_regions(pdev);
>
> pci_set_drvdata(pdev, NULL);
> + kfree(dev);
no problem with the patch, this comment is wrong though. kfree frees
np.
--
Jeff Garzik | A recent study has shown that too much soup
Building 1024 | can cause malaise in laboratory mice.
MandrakeSoft |
On Tue, 10 Jul 2001, Jeff Garzik wrote:
> Ion Badulescu wrote:
> > + unregister_netdev(dev); /* Will also free np!! */
> > + iounmap((char *)dev->base_addr);
> > + pci_release_regions(pdev);
> >
> > pci_set_drvdata(pdev, NULL);
> > + kfree(dev);
>
> no problem with the patch, this comment is wrong though. kfree frees
> np.
True, the comment should be moved 5 lines down. I'll fix it in the next
version.
Thanks,
Ion
--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.
You wrote:
> Hi,
>
> This patch fixes two real problems: missing initialization of a register
> which broke 100mbit half-duplex, and dereferencing of freed memory. It
> also massages the whitespace a bit.
This patch breaks my starfire totally:
With 2.4.6 on a 100Mbit HUB (100fx-HD) it receives but seems to have problems
to send.
It worked with 2.4.6-pre5.
With 2.4.6 and your patch I get the following immediately:
kernel: NETDEV WATCHDOG: eth5: transmit timed out
kernel: eth5: Transmit timed out, status 00000000, resetting...
kernel: 0c011 0730c811 0730b011 0730b811 0730a011 0730a811 07309011 07309811 07308011 07308811 07307011 07307811 07306011 07306811 07305011 07305811 07304011 07304811 07303011 07303811 07301011 07301811 07300011 07300811 072ff011 072ff811 072fe011 072fe811 072fd011 072fd811 072fc011 072fc811 072fb011 072fb811 072fa011 072fa811 072f9011 072f9811 072f8011 072f8811 072f7011 072f7811 072f6011 072f6811 072f4011 072f4811 072f3011 072f3811 072f2011 072f2811 072f1011 072f1811 072f0011 072f0811 072ef011 072ef811 072ee011 072ee811 072ed011 072ed811 072ec011 072ec811 072eb011 072eb811 072ea011 072ea811 072e9011 072e9811 072e7011 072e7811 072e6011 072e6811 072e5011 072e5811 072e4011 072e4811 072e3011 072e3811 072e2011 072e2811 072e1011 072e1811 072e0011 072e0811 072df011 072df811 072de011 072de811 072dd011 072dd811 072dc011 072dc811 072da011 072da811 072d9011 072d9811 072d8011 072d8811 072d7011 072d7811 072d6011 072d6811 072d5011 072d5811 072d4011 072d4811 072d3011 072d3811 072d2011 072d2811 072d1011
nel: d1811 072d0011 072d0811 072cf011 072cf811 072cd011 072cd811 072cc011 072cc811 072cb011 072cb811 072ca011 072ca811 072c9011 072c9811 072c8011 072c8811 072c7011 072c7811 072c6011 072c6811 072c5011 072c5811 072c4011 072c4811 072c3011 072c3811 072c2011 072c2811 072c0011 072c0811 072bf011 072bf811 072be011 072be811 072bd011 072bd813
Juri
--
Juri Haberland <[email protected]>
On 11 Jul 2001, Juri Haberland wrote:
> This patch breaks my starfire totally:
That's very weird. I really don't see how that could be the result of my
latest changes.
> With 2.4.6 on a 100Mbit HUB (100fx-HD) it receives but seems to have problems
> to send.
Right, that's the problem I was trying to fix. Basically it was unable to
send small packets, because the padding was disabled and it would only
get enabled if the card negotiated a connection other than 100-HD.
> With 2.4.6 and your patch I get the following immediately:
> kernel: NETDEV WATCHDOG: eth5: transmit timed out
> kernel: eth5: Transmit timed out, status 00000000, resetting...
Ok. Can you send me the other stuff the driver prints, especially the line
speed/duplex detection?
Also, if you feel adventurous, search for these lines in the driver:
/* Configure the PCI bus bursts and FIFO thresholds. */
np->tx_mode = 0x0C04; /* modified when link is up. */
* writel(0x8000 | np->tx_mode, ioaddr + TxMode);
* writel(np->tx_mode, ioaddr + TxMode);
and comment out those two marked with a *. At that point you should have
essentially the 2.4.6 driver, so see if they behaves similarly.
I'll also test it myself when I get home.
Thanks,
Ion
--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.
In article <[email protected]> you wrote:
> On 11 Jul 2001, Juri Haberland wrote:
>
>> This patch breaks my starfire totally:
>
> That's very weird. I really don't see how that could be the result of my
> latest changes.
>
>> With 2.4.6 on a 100Mbit HUB (100fx-HD) it receives but seems to have problems
>> to send.
>
> Right, that's the problem I was trying to fix. Basically it was unable to
> send small packets, because the padding was disabled and it would only
> get enabled if the card negotiated a connection other than 100-HD.
>
>> With 2.4.6 and your patch I get the following immediately:
>> kernel: NETDEV WATCHDOG: eth5: transmit timed out
>> kernel: eth5: Transmit timed out, status 00000000, resetting...
>
> Ok. Can you send me the other stuff the driver prints, especially the line
> speed/duplex detection?
kernel: starfire.c:v1.03 7/26/2000 Written by Donald Becker <[email protected]>
kernel: Updates and info at http://www.scyld.com/network/starfire.html
kernel: (unofficial 2.2/2.4 kernel port, version 1.03+LK1.3.3, July 05, 2001)
kernel: eth2: Adaptec Starfire 6915 at 0xc8800000, 00:00:d1:ee:8a:c5, IRQ 15.
kernel: eth2: MII PHY found at address 1, status 0x7809 advertising 01e1.
kernel: eth3: Adaptec Starfire 6915 at 0xc8881000, 00:00:d1:ee:8a:c6, IRQ 14.
kernel: eth3: MII PHY found at address 1, status 0x7809 advertising 01e1.
kernel: eth4: Adaptec Starfire 6915 at 0xc8902000, 00:00:d1:ee:8a:c7, IRQ 11.
kernel: eth4: MII PHY found at address 1, status 0x7809 advertising 01e1.
kernel: eth5: Adaptec Starfire 6915 at 0xc8983000, 00:00:d1:ee:8a:c8, IRQ 10.
kernel: eth5: MII PHY found at address 1, status 0x7809 advertising 01e1.
I couldn't find any other infos related to the starfire. Unfortunately I
cannot reboot the machine today anymore; this must wait until tomorrow.
> Also, if you feel adventurous, search for these lines in the driver:
> /* Configure the PCI bus bursts and FIFO thresholds. */
> np->tx_mode = 0x0C04; /* modified when link is up. */
> * writel(0x8000 | np->tx_mode, ioaddr + TxMode);
> * writel(np->tx_mode, ioaddr + TxMode);
>
> and comment out those two marked with a *. At that point you should have
> essentially the 2.4.6 driver, so see if they behaves similarly.
I'll try that tomorrow.
Juri
--
Juri Haberland <[email protected]>
On 11 Jul 2001, Juri Haberland wrote:
> > Also, if you feel adventurous, search for these lines in the driver:
> > /* Configure the PCI bus bursts and FIFO thresholds. */
> > np->tx_mode = 0x0C04; /* modified when link is up. */
> > * writel(0x8000 | np->tx_mode, ioaddr + TxMode);
> > * writel(np->tx_mode, ioaddr + TxMode);
> >
> > and comment out those two marked with a *. At that point you should have
> > essentially the 2.4.6 driver, so see if they behaves similarly.
>
> I'll try that tomorrow.
Actually, try this patch first. I believe it will fix your problems, but
I'd like to confirm it.
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/drivers/net/starfire.c Thu Jul 12 02:47:25 2001
+++ linux/drivers/net/starfire.c Thu Jul 12 01:35:04 2001
@@ -987,12 +987,12 @@
struct netdev_private *np = dev->priv;
u16 reg0;
+ mdio_write(dev, np->phys[0], MII_ADVERTISE, np->advertising);
mdio_write(dev, np->phys[0], MII_BMCR, BMCR_RESET);
udelay(500);
while (mdio_read(dev, np->phys[0], MII_BMCR) & BMCR_RESET);
reg0 = mdio_read(dev, np->phys[0], MII_BMCR);
- mdio_write(dev, np->phys[0], MII_ADVERTISE, np->advertising);
if (np->autoneg) {
reg0 |= BMCR_ANENABLE | BMCR_ANRESTART;
Ion Badulescu wrote:
> Actually, try this patch first. I believe it will fix your problems, but
> I'd like to confirm it.
Yep, this worked.
Btw, at what log-level is the link status printed?
I only got it via dmesg, but not in the logs. FWIW:
eth2: Link is down
eth5: Link is down
eth2: Link is up, running at 100Mbit half-duplex
eth5: Link is up, running at 100Mbit half-duplex
Juri
--
Juri Haberland <[email protected]>
On 12 Jul 2001, Juri Haberland wrote:
> Ion Badulescu wrote:
>
> > Actually, try this patch first. I believe it will fix your problems, but
> > I'd like to confirm it.
>
> Yep, this worked.
Great, good to hear that. I'll send the patch to Linus shortly.
> Btw, at what log-level is the link status printed?
> I only got it via dmesg, but not in the logs. FWIW:
I'm logging them at the debug level -- which I know some distributions
don't log by default. I'm wondering if I should increase the level to
info or notice... probably just info though, so they are logged but not
displayed on the console.
Thanks,
Ion
--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.
Ion Badulescu wrote:
> On 12 Jul 2001, Juri Haberland wrote:
>
>> Btw, at what log-level is the link status printed?
>> I only got it via dmesg, but not in the logs. FWIW:
>
> I'm logging them at the debug level -- which I know some distributions
> don't log by default. I'm wondering if I should increase the level to
> info or notice... probably just info though, so they are logged but not
> displayed on the console.
Yes, info would be good.
Thanks a lot,
Juri
--
Juri Haberland <[email protected]>