lkml.org 
[lkml]   [2003]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] 2.6.0-test2 wanXL driver
Krzysztof Halasa <khc@pm.waw.pl> :
[...]
--- 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 torvalds@osdl.org now.

Regards

--
Ueimor
-
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/

\
 
 \ /
  Last update: 2005-03-22 13:47    [W:0.069 / U:0.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site