I'm not really an expert, and I didn't understand all your remarks
but I can tell you this:
The driver supplied with 2.6.15 looks like dlink's driver version 1.17.
I had a dlink NIC that got stuck once in a while running that driver.
dlink's version 1.19 is written for 2.4 kernels, so all I did was
convert it to 2.6 kernels.
The new version still got stuck once in a while.
Maybe because the bugs you pointed out.
I don't have the dlink NIC anymore so I don't see how I can help here.
Maybe Edward can answer to that.
Sorry.
On Thu, 2006-09-07 at 11:19 +0200, Arjan van de Ven wrote:
> > @@ -335,8 +374,9 @@
> > #endif
> > /* Read eeprom */
> > for (i = 0; i < 128; i++) {
> > - ((u16 *) sromdata)[i] = le16_to_cpu (read_eeprom (ioaddr, i));
> > + ((u16 *) sromdata)[i] = cpu_to_le16 (read_eeprom (ioaddr, i));
> > }
> > + psrom->crc = le32_to_cpu(psrom->crc);
>
> this looks wrong, the data comes from the hw as le, so le*_to_cpu()
> sounds the right direction
>
> > @@ -401,7 +441,7 @@
> > int i;
> > u16 macctrl;
> >
> > - i = request_irq (dev->irq, &rio_interrupt, IRQF_SHARED, dev->name, dev);
> > + i = request_irq (dev->irq, &rio_interrupt, SA_SHIRQ, dev->name, dev);
> > if (i)
> > return i;
>
> this is backing out a fix/conversion to the new API. Bad.
>
>
> >
> > @@ -434,9 +474,12 @@
> > writeb (0x30, ioaddr + RxDMABurstThresh);
> > writeb (0x30, ioaddr + RxDMAUrgentThresh);
> > writel (0x0007ffff, ioaddr + RmonStatMask);
> > +
> > /* clear statistics */
> > clear_stats (dev);
> >
> > + atomic_set(&np->tx_desc_lock, 0);
>
> I'm quite scared by this naming; it suggests home-brew locking
>
> > dev->trans_start = jiffies;
> > + tasklet_enable(&np->tx_tasklet);
> > + writew(DEFAULT_INTR, ioaddr + IntEnable);
> > + return;
>
> this looks like a PCI posting bug
>
>
> > -rio_free_tx (struct net_device *dev, int irq)
> > +rio_free_tx (struct net_device *dev)
> > {
> > struct netdev_private *np = netdev_priv(dev);
> > int entry = np->old_tx % TX_RING_SIZE;
> > - int tx_use = 0;
> > unsigned long flag = 0;
> > + int irq = in_interrupt();
>
> eeeeep
>
> > +
> > + if (atomic_read(&np->tx_desc_lock))
> > + return;
> > + atomic_inc(&np->tx_desc_lock);
>
> and yes.. it is broken self made locking....
> there is a nice race between the _read and the _inc here.
>
>
> >
> > if (irq)
> > spin_lock(&np->tx_lock);
> > else
> > spin_lock_irqsave(&np->tx_lock, flag);
>
> double eeeep
>
> this is wrong to do with in_interrupt() as gating factor!
> Always doing the irqsave() is fine btw
>
>
>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
