Moritz Muehlenhoff <j...@inutil.org> wrote:
> This is likely fixed by upstream commit
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=3c4dc7115dfdb9e0450b7a3b0649948f5356d4af

The commit you're pointing to only appears to affect buffer reallocation
when changing MTU.  It fixes an entirely different stupid bug in this
terrible driver.

The oops happens during velocity_open():

1912:   ret = velocity_init_rings(vptr);
1913:   if (ret < 0)
1914:           goto out;
1915:
1916:   ret = velocity_init_rd_ring(vptr);
1917:   if (ret < 0)
1918:           goto err_free_desc_rings;
1919:
1920:   ret = velocity_init_td_ring(vptr);
1921:   if (ret < 0)
1922:           goto err_free_rd_ring;
1923:
1924:   /* Ensure chip is running */
1925:   pci_set_power_state(vptr->pdev, PCI_D0);
1926:
1927:   velocity_init_registers(vptr, VELOCITY_INIT_COLD);

in the middle of velocity_init_td_ring().

Just before that, driver just called velocity_init_rd_ring() which did:

1260:   ret = velocity_rx_refill(vptr);

which I think will have done:

1232:           velocity_give_many_rx_descs(vptr);

So it passed RX descriptors to the hardware before configuring it and
before configuring an interrupt handler.  If the hardware is already in
D0 and has interrupts enabled then this will be disastrous - and
apparently it is.

I think the proper solution probably involves resetting the hardware
earlier:

--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -739,12 +739,6 @@
 
        case VELOCITY_INIT_COLD:
        default:
-               /*
-                *      Do reset
-                */
-               velocity_soft_reset(vptr);
-               mdelay(5);
-
                mac_eeprom_reload(regs);
                for (i = 0; i < 6; i++) {
                        writeb(vptr->dev->dev_addr[i], &(regs->PAR[i]));
@@ -1909,6 +1903,12 @@
        struct velocity_info *vptr = netdev_priv(dev);
        int ret;
 
+       /* Ensure chip is running */
+       pci_set_power_state(vptr->pdev, PCI_D0);
+
+       velocity_soft_reset(vptr);
+       mdelay(5);
+
        ret = velocity_init_rings(vptr);
        if (ret < 0)
                goto out;
@@ -1921,9 +1921,6 @@
        if (ret < 0)
                goto err_free_rd_ring;
 
-       /* Ensure chip is running */
-       pci_set_power_state(vptr->pdev, PCI_D0);
-
        velocity_init_registers(vptr, VELOCITY_INIT_COLD);
 
        ret = request_irq(vptr->pdev->irq, &velocity_intr, IRQF_SHARED,
@@ -1988,6 +1985,9 @@
 
                dev->mtu = new_mtu;
 
+               velocity_soft_reset(vptr);
+               mdelay(5);
+
                ret = velocity_init_rd_ring(vptr);
                if (ret < 0)
                        goto out_unlock;
--- END ---

Note that the above is totally untested, but it looks more like what
*sane* network drivers do.

Ben.

-- 
Ben Hutchings
I'm always amazed by the number of people who take up solipsism because
they heard someone else explain it. - E*Borg on alt.fan.pratchett

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to