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/
This archive was generated by hypermail 2b29 : Thu Jul 31 2003 - 22:00:45 EST