Krzysztof Halasa <[email protected]> :
[...]
--- linux-2.6.orig/drivers/net/wan/wanxl.c 2003-07-29 19:21:04.000000000 +0200
+++ linux-2.6/drivers/net/wan/wanxl.c 2003-07-23 15:12:31.000000000 +0200
[...]
+static int wanxl_open(struct net_device *dev)
+{
+ hdlc_device *hdlc = dev_to_hdlc(dev);
+ port_t *port = hdlc_to_port(hdlc);
+ u8 *dbr = port->card->plx + PLX_DOORBELL_TO_CARD;
+ long start;
[...]
+ start = jiffies;
+ do
+ if (port->status.open)
+ return 0;
+ while (jiffies - start < HZ);
What about:
unsigned long timeout = jiffies + HZ;
do {
...
} while (time_after(timeout, jiffies));
[...]
+static int __devinit wanxl_pci_init_one(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ card_t *card;
+ u32 ramsize, stat;
+ long start;
+ u32 plx_phy; /* PLX PCI base address */
+ u32 mem_phy; /* memory PCI base addr */
+ u8 *mem; /* memory virtual base addr */
+ int i, ports, alloc_size;
+
+ i = pci_enable_device(pdev);
+ if (i)
+ return i;
+
+ i = pci_request_regions(pdev, "wanXL");
+ if (i)
+ return i;
+
+ 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)
+
+ alloc_size = sizeof(card_t) + ports * sizeof(port_t);
+ card = kmalloc(alloc_size, GFP_KERNEL);
+ if (virt_to_bus(card) + alloc_size > 256 * 1024 * 1024) {
+ /* wanXL can only access first 256 MB */
+ kfree(card);
+ card = kmalloc(alloc_size, GFP_KERNEL | GFP_DMA);
+ }
+ if (card == NULL) {
+ pci_release_regions(pdev);
+ printk(KERN_ERR "wanXL %s: unable to allocate memory\n",
+ card_name(pdev));
+ return -ENOBUFS;
+ }
+ memset(card, 0, alloc_size);
+
+ pci_set_drvdata(pdev, card);
+ card->pdev = pdev;
+ card->n_ports = ports;
+
+ /* set up PLX mapping */
+ plx_phy = pci_resource_start(pdev, 0);
+ card->plx = ioremap_nocache(plx_phy, 0x70);
+
+#if RESET_WHILE_LOADING
+ wanxl_reset(card);
+#endif
+
+ start = jiffies;
+ 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).
[...]
+ 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().
Btw, you can use [email protected] now.
Regards
--
Ueimor
Francois Romieu <[email protected]> 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
Krzysztof Halasa <[email protected]> :
> Francois Romieu <[email protected]> :
[...]
> > dma_map_single() is probably preferred over virt_to_bus().
>
> Never heard of it in fact :-)
> Will check.
It isn't that far from pci_map_single() and friends.
See Documentation/DMA-mapping.txt for more details and drivers/net/ for examples
of use.
--
Ueimor