Messages in this thread | | | Subject | Re: [PATCH] 2.6.0-test2 wanXL driver | From | Krzysztof Halasa <> | Date | 30 Jul 2003 01:09:34 +0200 |
| |
Francois Romieu <romieu@fr.zoreil.com> writes:
> unsigned long timeout = jiffies + HZ; > > do { > ... > } while (time_after(timeout, jiffies));
That's the same (my version deals with wraps too) but yes, time_after looks nicer. I'll change it in next version.
> + switch (pdev->device) { > + case PCI_DEVICE_ID_SBE_WANXL100: ports = 1; break; > + case PCI_DEVICE_ID_SBE_WANXL200: ports = 2; break; > + default: ports = 4; > + } > > You can do it this way: > static struct pci_device_id wanxl_pci_tbl[] __devinitdata = { > { PCI_VENDOR_ID_SBE, PCI_DEVICE_ID_SBE_WANXL100, PCI_ANY_ID, > PCI_ANY_ID, 0, 0, 1 }, > { PCI_VENDOR_ID_SBE, PCI_DEVICE_ID_SBE_WANXL200, PCI_ANY_ID, > PCI_ANY_ID, 0, 0, 2 }, > { PCI_VENDOR_ID_SBE, PCI_DEVICE_ID_SBE_WANXL400, PCI_ANY_ID, > PCI_ANY_ID, 0, 0, 4 }, > { 0, } > }; > > ports = ent->driver_data; > > (imho turning 1, 2, 4 into #define wouldn't be bad then)
Yes, but I like having the variable local to this function a little more.
> + while ((stat = readl(card->plx + PLX_MAILBOX_0)) != 0) { > + if (jiffies - start >= 20 * HZ) { > + printk(KERN_WARNING "wanXL %s: timeout waiting for" > + " PUTS to complete\n", card_name(pdev)); > + return -ENODEV; > > This return leaks kmalloced card_t *card, pci_requested_regions and > ioremaped area (same thing for the return a few lines below in the > same function).
Right. Will fix in next version.
> + for (i = 0; i < RX_QUEUE_LENGTH; i++) { > + struct sk_buff *skb = dev_alloc_skb(BUFFER_LENGTH); > + card->rx_skbs[i] = skb; > + if (skb) > + card->rx_descs[i].address = virt_to_bus(skb->data); > > dma_map_single() is probably preferred over virt_to_bus().
Never heard of it in fact :-) Will check.
Thanks. -- Krzysztof Halasa Network Administrator - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |