Re: [PATCH] saa9730

[Angelo Dell'Aera (buffer@antifork.org)]

On Sat, 20 Sep 2003 14:11:46 -0400
Jeff Garzik <jgarzik@xxxxxxxxx> wrote:

>Yes.  To echo David's comments, you need more description.
>
>Further, if you're actually going to change this area of code, please 
>just convert the driver to "struct pci_driver" aka "new PCI API".

I did it and you can find the patch below. Hope to receive some useful
feedback from saa9730 owners since I've got no such board handy. 

Regards,
Angelo Dell'Aera 


--- linux-2.6.0-test5/drivers/net/saa9730.c.old	2003-09-19 18:57:43.000000000 +0200
+++ linux-2.6.0-test5/drivers/net/saa9730.c	2003-09-23 16:19:18.000000000 +0200
@@ -21,6 +21,9 @@
  *
  * SAA9730 ethernet driver.
  *
+ * Changes:
+ * Angelo Dell'Aera <buffer@xxxxxxxxxxxx> : Conversion to the new PCI API (pci_driver).
+ *                                          Error handling fixes.
  */
 
 #include <linux/init.h>
@@ -42,6 +45,15 @@
 int lan_saa9730_debug;
 #endif
 
+#define DRV_MODULE_NAME "saa9730"
+
+static struct pci_device_id saa9730_pci_tbl[] = {
+	{ PCI_VENDOR_ID_PHILIPS, PCI_DEVICE_ID_PHILIPS_SAA9370,
+          PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
+	{ 0, }
+};
+
+MODULE_DEVICE_TABLE(pci, saa9730_pci_tbl);
 
 /* Non-zero only if the current card is a PCI with BIOS-set IRQ. */
 static unsigned int pci_irq_line;
@@ -216,6 +228,9 @@
 	buffer_start =
 	    (unsigned int) kmalloc(mem_size, GFP_DMA | GFP_KERNEL);
 
+	if (!buffer_start)
+		return -ENOMEM;
+
 	/* 
 	 * Set DMA buffer to kseg1 (uncached).
 	 * Make sure to flush before using it uncached.
@@ -971,21 +986,47 @@
 	lan_saa9730_restart(lp);
 }
 
+
+static void __devexit saa9730_remove_one(struct pci_dev *pdev)
+{
+        struct net_device *dev = pci_get_drvdata(pdev);
+
+        if (dev) {
+		
+		if (dev->priv)
+			kfree(dev->priv);
+
+                unregister_netdev(dev);
+                free_netdev(dev);
+                pci_release_regions(pdev);
+                pci_disable_device(pdev);
+                pci_set_drvdata(pdev, NULL);
+        }
+}
+
+
 static int lan_saa9730_init(struct net_device *dev, int ioaddr, int irq)
 {
 	struct lan_saa9730_private *lp;
 	unsigned char ethernet_addr[6];
+	int ret = 0;
 
 	dev = init_etherdev(dev, 0);
 
+	if (!dev) 
+		return -ENOMEM;
+	
 	dev->open = lan_saa9730_open_fail;
-	if (get_ethernet_addr(ethernet_addr))
-		return -1;
 
+	if (get_ethernet_addr(ethernet_addr)) {
+		ret = -ENODEV;
+		goto out;
+	}
+	
 	memcpy(dev->dev_addr, ethernet_addr, 6);
 	dev->base_addr = ioaddr;
 	dev->irq = irq;
-
+	
 	/* 
 	 * Make certain the data structures used by the controller are aligned 
 	 * and DMAble. 
@@ -995,6 +1036,11 @@
 						      GFP_DMA | GFP_KERNEL)
 					      + 7) & ~7);
 
+	if (!lp) {
+		ret = -ENOMEM;
+                goto out;
+        }
+
 	dev->priv = lp;
 	memset(lp, 0, sizeof(*lp));
 
@@ -1007,33 +1053,34 @@
 							 SAA9730_EVM_REGS_ADDR);
 
 	/* Allocate LAN RX/TX frame buffer space. */
-	if (lan_saa9730_allocate_buffers(lp))
-		return -1;
+	if ((ret = lan_saa9730_allocate_buffers(lp)))
+		goto out;
 
 	/* Stop LAN controller. */
-	if (lan_saa9730_stop(lp))
-		return -1;
-
+	if ((ret = lan_saa9730_stop(lp))) 
+		goto out;
+	
 	/* Initialize CAM registers. */
-	if (lan_saa9730_cam_init(dev))
-		return -1;
+	if ((ret = lan_saa9730_cam_init(dev)))
+		goto out;
 
 	/* Initialize MII registers. */
-	if (lan_saa9730_mii_init(lp))
-		return -1;
+	if ((ret = lan_saa9730_mii_init(lp)))
+		goto out;
 
 	/* Initialize control registers. */
-	if (lan_saa9730_control_init(lp))
-		return -1;
-
+	if ((ret = lan_saa9730_control_init(lp))) 
+		goto out;
+        
 	/* Load CAM registers. */
-	if (lan_saa9730_cam_load(lp))
-		return -1;
-
+	if ((ret = lan_saa9730_cam_load(lp))) 
+		goto out;
+	
 	/* Initialize DMA context registers. */
-	if (lan_saa9730_dma_init(lp))
-		return -1;
-
+	if ((ret = lan_saa9730_dma_init(lp)))
+		goto out;
+	
+	
 	dev->open = lan_saa9730_open;
 	dev->hard_start_xmit = lan_saa9730_start_xmit;
 	dev->stop = lan_saa9730_close;
@@ -1042,42 +1089,89 @@
 	dev->tx_timeout = lan_saa9730_tx_timeout;
 	dev->watchdog_timeo = (HZ >> 1);
 	dev->dma = 0;
-
+	
 	return 0;
+
+ out:
+	if (dev) {
+		if (dev->priv)
+			kfree(dev->priv);
+		unregister_netdevice(dev);
+		free_netdev(dev);
+	}
+		
+	return ret;
 }
 
 
-static int __init saa9730_probe(void)
+static int __devinit saa9730_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct net_device *dev = NULL;
-	struct pci_dev *pdev = NULL;
+	unsigned int pci_ioaddr;
+	int err;
 
 	if (lan_saa9730_debug > 1)
-		printk
-		    ("saa9730.c: PCI bios is present, checking for devices...\n");
+		printk("saa9730.c: PCI bios is present, checking for devices...\n");
 
-	while ((pdev = pci_find_device(PCI_VENDOR_ID_PHILIPS,
-				       PCI_DEVICE_ID_PHILIPS_SAA9730,
-				       pdev))) {
-		unsigned int pci_ioaddr;
-
-		pci_irq_line = pdev->irq;
-		/* LAN base address in located at BAR 1. */
-
-		pci_ioaddr = pci_resource_start(pdev, 1);
-		pci_set_master(pdev);
-
-		printk("Found SAA9730 (PCI) at %#x, irq %d.\n",
-		       pci_ioaddr, pci_irq_line);
-		if (!lan_saa9730_init
-		    (dev, pci_ioaddr, pci_irq_line))
-			return 0;
-		else
-			printk("Lan init failed.\n");
+	err = pci_enable_device(pdev);
+        if (err) {
+                printk(KERN_ERR "Cannot enable PCI device, aborting.\n");
+                goto out;
+        }
+
+	err = pci_request_regions(pdev, DRV_MODULE_NAME);
+	if (err) {
+		printk(KERN_ERR "Cannot obtain PCI resources, aborting.\n");
+		goto out_disable_pdev;
+	}
+
+	pci_irq_line = pdev->irq;
+	/* LAN base address in located at BAR 1. */
+	
+	pci_ioaddr = pci_resource_start(pdev, 1);
+	pci_set_master(pdev);
+	
+	printk("Found SAA9730 (PCI) at %#x, irq %d.\n",
+	       pci_ioaddr, pci_irq_line);
+
+	err = lan_saa9730_init(dev, pci_ioaddr, pci_irq_line);
+	if (err) {
+		printk("Lan init failed");
+		goto out_disable_pdev;
 	}
+	
+	pci_set_drvdata(pdev, dev);
+	return 0;
+	
+ out_disable_pdev:
+	pci_disable_device(pdev);
+ out:
+	pci_set_drvdata(pdev, NULL);
+	return err;
+}
 
-	return -ENODEV;
+
+static struct pci_driver saa9730_driver = {
+	.name           = DRV_MODULE_NAME,
+	.id_table       = saa9730_pci_tbl,
+	.probe          = saa9730_init_one,
+	.remove         = __devexit_p(saa9730_remove_one),
+};
+
+
+static int __init saa9730_init(void)
+{
+        return pci_module_init(&saa9730_driver);
+}
+
+static void __exit saa9730_cleanup(void)
+{
+        pci_unregister_driver(&saa9730_driver);
 }
 
-module_init(saa9730_probe);
+module_init(saa9730_init);
+module_exit(saa9730_cleanup);
+
+
+
 MODULE_LICENSE("GPL");


-
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html