Skip to contentSkip to conversation

Converting drivers/net/rcpci45.c to new PCI API

RFC PATCH In review Rasmus Andersen 3 participants 6 messages Dec 19, 2000 Network drivers

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 Converting drivers/net/rcpci45.c to new PCI API

Hi.

This is my attempt at converting the rcpci45 driver (240t13p2) to the new PCI API interface. I fully expect to have missed something so please comment away. BTW, I do not consider this the final version of this patch; therefore the maintainers are not explicitly on the recipients lists.

There are some other cleanups I want to do, and I need to make my indentation match the drivers, but that will be after the basic conversion is done.

drivers/net/rcpci45.c +57 -103
@@ -36,6 +36,8 @@
 **  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 **
 **   
+**  Rasmus Andersen, December 2000: Converted to new PCI API.
+**
 **  Pete Popov, January 11,99: Fixed a couple of 2.1.x problems 
 **  (virt_to_bus() not called), tested it under 2.2pre5 (as a module), and 
 **  added a #define(s) to enable the use of the same file for both, the 2.0.x 
@@ -47,10 +49,6 @@
 **
 ***************************************************************************/
 
-static char *version =
-"RedCreek Communications PCI linux driver version 2.02\n";
-
-
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
@@ -75,6 +73,9 @@
 #include <linux/skbuff.h>
 
 
+static char version[] __initdata =
+"RedCreek Communications PCI linux driver version 2.03\n";
+
 #define RC_LINUX_MODULE
 #include "rclanmtl.h"
 #include "rcif.h"
@@ -123,7 +124,6 @@
     U32    function;
     struct timer_list timer;        /*  timer */
     struct net_device_stats  stats; /* the statistics structure */
-    struct net_device *next;            /* points to the next RC adapter */
     unsigned long numOutRcvBuffers;/* number of outstanding receive buffers*/
     unsigned char shutdown;
     unsigned char reboot;
@@ -138,8 +138,6 @@
 static PDPA  PCIAdapters[MAX_ADAPTERS];
 
 static int RCinit(struct net_device *dev);
-static int RCscan(void);
-static int RCfound_device(int, int, int, int, int, int);
 
 static int RCopen(struct net_device *);
 static int RC_xmit_packet(struct sk_buff *, struct net_device *);
@@ -155,71 +153,29 @@
 static int RC_allocate_and_post_buffers(struct net_device *, int);
 
 
-/* A list of all installed RC devices, for removing the driver module. */
-static struct net_device *root_RCdev;
+static struct pci_device_id rcpci45_pci_table[] __devinitdata = {
+	{ RC_PCI45_VENDOR_ID, RC_PCI45_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, rcpci_pci_table);
 
-static int __init rcpci_init_module (void)
+static void rcpci45_remove_one(struct pci_dev *pdev)
 {
-    int cards_found;
-
-    cards_found = RCscan();
-    if (cards_found)
-        printk(version);
-    return cards_found ? 0 : -ENODEV;
-}
+	struct net_device *dev = pdev->driver_data;
+	PDPA pDpa = dev->priv;
 
-static int RCscan(void)
-{
-    int cards_found = 0;
-    static int pci_index;
-
-    if (!pcibios_present()) 
-        return cards_found;
-
-    for (;pci_index < 0x8; pci_index++) 
-    {
-        unsigned char pci_bus, pci_device_fn;
-        int scan_status;
-        int board_index = 0;
-        unsigned char pci_irq_line;
-        unsigned int pci_ioaddr;
-	struct pci_dev *pdev;
-
-        scan_status =  
-            (pcibios_find_device (RC_PCI45_VENDOR_ID, 
-                                  RC_PCI45_DEVICE_ID, 
-                                  pci_index, 
-                                  &pci_bus, 
-                                  &pci_device_fn));
-#ifdef RCDEBUG
-        printk("rc scan_status = 0x%X\n", scan_status);
-#endif
-        if (scan_status != PCIBIOS_SUCCESSFUL ||
-	    !((pdev = pci_find_slot(pci_bus, pci_device_fn))))
-            break;
-	pci_irq_line = pdev->irq;
-	pci_ioaddr = pci_resource_start (pdev, 0);
+        if (!dev) {
+                printk (KERN_ERR "remove non-existent device\n");
+                return;
+        }
 
-#ifdef RCDEBUG
-        printk("rc: Found RedCreek PCI adapter\n");
-        printk("rc: pci_bus = %d,  pci_device_fn = %d\n", pci_bus, pci_device_fn);
-        printk("rc: pci_irq_line = 0x%x \n", pci_irq_line);
-        printk("rc: pci_ioaddr = 0x%x\n", pci_ioaddr);
-#endif
+	printk("IOP reset: 0x%x\n", RCResetIOP(pDpa->id));
 
-	if (pci_enable_device(pdev))
-		break;
-	pci_set_master(pdev);
+	unregister_netdev(dev);
+	iounmap((void *)dev->base_addr);
+        free_irq(dev->irq, dev);
 
-        if (!RCfound_device(pci_ioaddr, pci_irq_line,
-                          pci_bus, pci_device_fn,
-                          board_index++, cards_found))
-            cards_found++;
-    }
-#ifdef RCDEBUG
-    printk("rc: found %d cards \n", cards_found);
-#endif
-    return cards_found;
+	kfree(dev);
 }
 
 static int RCinit(struct net_device *dev)
@@ -233,17 +189,22 @@
     return 0;
 }
 
-static int
-RCfound_device(int memaddr, int irq, 
-               int bus, int function, int product_index, int card_idx)
+static int rcpci45_init_one(struct pci_dev *pdev, 
+			    const struct pci_device_id *ent)
 {
     int dev_size = 32768;        
     unsigned long *vaddr=0;
     PDPA pDpa;
     int init_status;
 
+    long memaddr = pci_resource_start(pdev, 0);
+    int card_idx;    
     struct net_device *dev;
-    
+
+    static int card_counter = 0;
+
+    card_idx = card_counter++; /* Yeah, icky hack. */
+
     /* 
      * Allocate and fill new device structure. 
      * We need enough for struct net_device plus DPA plus the LAN API private
@@ -256,10 +217,13 @@
     dev = (struct net_device *) kmalloc(dev_size, GFP_DMA | GFP_KERNEL |GFP_ATOMIC);
     if (!dev)
     {
-        printk("rc: unable to kmalloc dev\n");
+        printk("rcpci45 driver: unable to kmalloc dev struct\n");
         return 1;   
     }
     memset(dev, 0, dev_size);
+
+    pdev->driver_data = dev;
+
     /*
      * dev->priv will point to the start of DPA.
      */
@@ -268,15 +232,16 @@
 #ifdef RCDEBUG
     printk("rc: dev = 0x%x, dev->priv = 0x%x\n", (uint)dev, (uint)dev->priv);
 #endif
-
+    
     pDpa = dev->priv;
 
     pDpa->dev = dev;            /* this is just for easy reference */
-    pDpa->function = function;
-    pDpa->bus = bus;
+    pDpa->function = pdev->devfn;
+    pDpa->bus = (unsigned char)pdev->bus->number;
     pDpa->id = card_idx;        /* the device number */
     pDpa->pci_addr = memaddr;
     PCIAdapters[card_idx] = pDpa;
+
 #ifdef RCDEBUG
     printk("rc: pDpa = 0x%x, id = %d \n", (uint)pDpa, (uint)pDpa->id);
 #endif
@@ -300,7 +265,7 @@
            (uint)dev, (uint)dev->priv, (uint)vaddr);
 #endif
     dev->base_addr = (unsigned long)vaddr;
-    dev->irq = irq;
+    dev->irq = pdev->irq;
 
     /*
      * Request a shared interrupt line.
@@ -342,9 +307,6 @@
     dev->init = &RCinit;
     ether_setup(dev);            /* linux kernel interface */
 
-    pDpa->next = root_RCdev;
-    root_RCdev = dev;
-
     if (register_netdev(dev) != 0) /* linux kernel interface */
     {
         printk("rc: unable to register device \n");
@@ -359,6 +321,22 @@
     return 0; /* success */
 }
 
+static struct pci_driver rcpci45_driver = {
+	name: "rcpci45",
+	id_table: rcpci45_pci_table,
+	probe: rcpci45_init_one,
+	remove: rcpci45_remove_one,
+};
+
+static int __init rcpci_init_module (void)
+{
+	int rc = pci_module_init(&rcpci45_driver);
+
+	if (!rc)
+		printk(version);
+	return rc;
+}
+
 static int
 RCopen(struct net_device *dev)
 {
@@ -1185,31 +1163,7 @@
 
 static void __exit rcpci_cleanup_module (void)
 {
-    PDPA pDpa;
-    struct net_device *next;
-
-
-#ifdef RCDEBUG
-    printk("rc: RC cleanup_module\n");
-    printk("rc: root_RCdev = 0x%x\n", (uint)root_RCdev);
-#endif
-
-
-    while (root_RCdev)
-    {
-        pDpa = (PDPA) root_RCdev->priv;
-#ifdef RCDEBUG
-        printk("rc: cleanup 0x%08X\n", (uint)root_RCdev);
-#endif
-        printk("IOP reset: 0x%x\n", RCResetIOP(pDpa->id));
-        unregister_netdev(root_RCdev);
-        next = pDpa->next;
-
-        iounmap((unsigned long *)root_RCdev->base_addr); 
-        free_irq( root_RCdev->irq, root_RCdev );
-        kfree(root_RCdev);
-        root_RCdev = next;
-    }
+	pci_unregister_driver(&rcpci45_driver);
 }
 
 module_init(rcpci_init_module);
Signature · 9 lines
Regards,
        Rasmus([email protected])

If a man says something in a forest and there are no women around to 
hear him, is he still wrong? -- Anonymous
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#1 lore raw

Rasmus Andersen <[email protected]> écrit :
[...]

Quoted 6 lines

There are some other cleanups I want to do, and I need to make my indentation match the drivers, but that will be after the basic conversion is done.

drivers/net/rcpci45.c +0 -0

[...]

Quoted 4 lines
drivers/net/rcpci45.c +0 -0
@@ -155,71 +153,29 @@
 static int RC_allocate_and_post_buffers(struct net_device *, int);
 
 

drivers/net/rcpci45.c line 162

Quoted 10 lines
drivers/net/rcpci45.c +6 -3
-/* A list of all installed RC devices, for removing the driver module. */
-static struct net_device *root_RCdev;
+static struct pci_device_id rcpci45_pci_table[] __devinitdata = {
+	{ RC_PCI45_VENDOR_ID, RC_PCI45_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID, },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, rcpci_pci_table);
 
-static int __init rcpci_init_module (void)
+static void rcpci45_remove_one(struct pci_dev *pdev)
              ^->  __exit

[...]

drivers/net/rcpci45.c line 176

Quoted 6 lines
drivers/net/rcpci45.c +3 -3
-	if (pci_enable_device(pdev))
-		break;
-	pci_set_master(pdev);
+	unregister_netdev(dev);
+	iounmap((void *)dev->base_addr);
+        free_irq(dev->irq, dev);

I'd rather inhibit irq first then release the ressources.
+ free_irq(dev->irq, dev);
+ iounmap((void *)dev->base_addr);
+ unregister_netdev(dev);

Signature · 5 lines
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#2 lore raw
In reply to Rasmus Andersen

Review

Quoted 8 lines

drivers/net/rcpci45.c line 176

Quoted 3 lines
drivers/net/rcpci45.c +3 -0
+	unregister_netdev(dev);
+	iounmap((void *)dev->base_addr);
+        free_irq(dev->irq, dev);

I'd rather inhibit irq first then release the ressources.

(quoted diff) +3 -0
+       free_irq(dev->irq, dev);
+	iounmap((void *)dev->base_addr);
+	unregister_netdev(dev);

Fair enough. I just copied the sequence used in the original but your suggestion makes sense. I'll do that in my next patch.

Thanks for the eyeballs :),
Rasmus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
In reply to Rasmus Andersen

On Tue, Dec 19 2000, Francois Romieu wrote:

[deleted]

Review

Quoted 12 lines

drivers/net/rcpci45.c line 176

Quoted 6 lines
drivers/net/rcpci45.c +3 -3
-	if (pci_enable_device(pdev))
-		break;
-	pci_set_master(pdev);
+	unregister_netdev(dev);
+	iounmap((void *)dev->base_addr);
+        free_irq(dev->irq, dev);

I'd rather inhibit irq first then release the ressources.

(quoted diff) +3 -0
+       free_irq(dev->irq, dev);
+	iounmap((void *)dev->base_addr);
+	unregister_netdev(dev);

You should release the irq when the adapter is closed, not removed, unless there's some special case that can't be handled if you take ints during init.

And why would you unregister your netdev after releasing resources?

Signature · 7 lines
Torben Mathiasen <[email protected]>
Linux ThunderLAN maintainer 
http://tlan.kernel.dk
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#4 lore raw
In reply to Rasmus Andersen
Quoted 4 lines · Torben Mathiasen

You should release the irq when the adapter is closed, not removed, unless there's some special case that can't be handled if you take ints during init.

You seem to be right. I have moved the free_irq to the close function.

Quoted 3 lines

And why would you unregister your netdev after releasing resources?

Instead of doing it before? Beats me :) I merely/blindly copied the existing sequence, but after RTFS'ing I changed the sequence to:

	unregister_netdev(dev);
	iounmap((void *)dev->base_addr);
	kfree(dev);

Thank you for your comments.

Signature · 9 lines
Regards,
        Rasmus([email protected])

Freedom of the press is limited to those who own one.
                                 - A.J. Liebling 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#5 lore raw
In reply to Rasmus Andersen
Quoted 8 lines · Rasmus Andersen
Quoted 4 lines · Torben Mathiasen

You should release the irq when the adapter is closed, not removed, unless there's some special case that can't be handled if you take ints during init.

You seem to be right. I have moved the free_irq to the close function.

This driver seems a bit odd. It also request irq's durint init instead of device open. Normally one would request any resource as late as possible (to prevent the driver from using resources when it is not even used). It makes one wonder if this driver has just not been updated in a while, or if it does things for a reason.

Signature · 7 lines
Torben Mathiasen <[email protected]>
Linux ThunderLAN maintainer 
http://tlan.kernel.dk
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/