Re: cs89x0.c MODULE_PARM

[Andrew Morton <andrewm@uow.edu.au>]

Keith Owens wrote:
> 
> drivers/net/cs89x0.c in 2.4.0-test10 has these lines.
> 
> static char media[8];
> MODULE_PARM(media, "s");
> 
> The MODULE_PARM type does not match the variable declaration.  It
> should either be
> 
> static char media[8];
> MODULE_PARM(media, "c8");
> 
> or
> 
> static char *media;
> MODULE_PARM(media, "s");
> 
> Since the code reads media without checking for NULL pointers, the
> first form would be better.

Thanks.  Here's the current diff.  There's still some more stuff to
do here.  I'll be feeding it upstream RSN.



--- linux-2.4.0-test11-pre4/drivers/net/cs89x0.c	Mon Nov 13 18:23:49 2000
+++ linux-akpm/drivers/net/cs89x0.c	Mon Nov 13 23:14:41 2000
@@ -66,10 +66,15 @@
                     :   abstracted IRQ mapping to support CONFIG_ARCH_CLPS7500 arch
                     :   (Jason Gunthorpe <jgg@ualberta.ca>)
 
+  Andrew Morton     : Kernel 2.4.0-test10
+                    : Use dev->name in request_*() (Andrey Panin)
+                    : Fix an error-path memleak in init_module()
+                    : Preserve return value from request_irq()
+
 */
 
 static char *version =
-"cs89x0.c: v2.3.99-pre1-2 Russell Nelson <nelson@crynwr.com>, Andrew Morton <andrewm@uow.edu.au>\n";
+"cs89x0.c: v2.4.0-test10 Russell Nelson <nelson@crynwr.com>, Andrew Morton <andrewm@uow.edu.au>\n";
 
 /* ======================= end of configuration ======================= */
 
@@ -378,8 +383,8 @@
 	lp = (struct net_local *)dev->priv;
 
 	/* Grab the region so we can find another board if autoIRQ fails. */
-	if (!request_region(ioaddr, NETCARD_IO_EXTENT, "cs89x0")) {
-		retval = -ENODEV;
+	if (!request_region(ioaddr, NETCARD_IO_EXTENT, dev->name)) {
+		retval = -EBUSY;
 		goto out1;
 	}
 
@@ -985,7 +990,7 @@
 	int i;
 
 	if (chip_type == CS8900) {
-		/* Search the mapping table for the corrisponding IRQ pin. */
+		/* Search the mapping table for the corresponding IRQ pin. */
 		for (i = 0; i != sizeof(cs8900_irq_map)/sizeof(cs8900_irq_map[0]); i++)
 			if (cs8900_irq_map[i] == irq)
 				break;
@@ -1027,20 +1032,20 @@
 /* And 2.3.47 had this: */
 		writereg(dev, PP_BusCTL, ENABLE_IRQ | MEMORY_ON);
 
-		for (i = 2; i < CS8920_NO_INTS; i++) if ((1 << dev->irq) & lp->irq_map) {
-			if (request_irq (i, NULL, 0, "cs89x0", dev) != -EBUSY) {
-				write_irq(dev, lp->chip_type, i);
-				writereg(dev, PP_BufCFG, GENERATE_SW_INTERRUPT);
-				if (request_irq (dev->irq = i, &net_interrupt, 0, "cs89x0", dev) == 0)
-					break;
+		for (i = 2; i < CS8920_NO_INTS; i++) {
+			if ((1 << dev->irq) & lp->irq_map) {
+				if (request_irq (i, NULL, 0, dev->name, dev) != -EBUSY) {
+					write_irq(dev, lp->chip_type, i);
+					writereg(dev, PP_BufCFG, GENERATE_SW_INTERRUPT);
+					if (request_irq (dev->irq = i, &net_interrupt, 0, dev->name, dev) == 0)
+						break;
+				}
 			}
 		}
 
-
 		if (i >= CS8920_NO_INTS) {
 			writereg(dev, PP_BusCTL, 0);	/* disable interrupts. */
-			if (net_debug)
-				printk("cs89x0: can't get an interrupt\n");
+			printk(KERN_ERR "cs89x0: can't get an interrupt\n");
 			ret = -EAGAIN;
 			goto bad_out;
 		}
@@ -1061,7 +1066,7 @@
 		ret = request_irq(dev->irq, &net_interrupt, 0, dev->name, dev);
 		if (ret) {
 			if (net_debug)
-				printk("cs89x0: request_irq(%d) failed\n", dev->irq);
+				printk(KERN_DEBUG "cs89x0: request_irq(%d) failed\n", dev->irq);
 			goto bad_out;
 		}
 	}
@@ -1089,7 +1094,7 @@
 				goto release_irq;
 			}
 			memset(lp->dma_buff, 0, lp->dmasize * 1024);	/* Why? */
-			if (request_dma(dev->dma, "cs89x0")) {
+			if (request_dma(dev->dma, dev->name)) {
 				printk(KERN_ERR "%s: cannot get dma channel %d\n", dev->name, dev->dma);
 				goto release_irq;
 			}
@@ -1576,7 +1581,7 @@
 MODULE_PARM(io, "i");
 MODULE_PARM(irq, "i");
 MODULE_PARM(debug, "i");
-MODULE_PARM(media, "s");
+MODULE_PARM(media, "c8");
 MODULE_PARM(duplex, "i");
 MODULE_PARM(dma , "i");
 MODULE_PARM(dmasize , "i");
@@ -1616,6 +1621,7 @@
 init_module(void)
 {
 	struct net_local *lp;
+	int ret = 0;
 
 #if DEBUGGING
 	net_debug = debug;
@@ -1661,21 +1667,27 @@
         if (io == 0) {
                 printk(KERN_ERR "cs89x0.c: Module autoprobing not allowed.\n");
                 printk(KERN_ERR "cs89x0.c: Append io=0xNNN\n");
-                return -EPERM;
+                ret = -EPERM;
+		goto out;
         }
 
 #if ALLOW_DMA
 	if (use_dma && dmasize != 16 && dmasize != 64) {
 		printk(KERN_ERR "cs89x0.c: dma size must be either 16K or 64K, not %dK\n", dmasize);
-		return -EPERM;
+		ret = -EPERM;
+		goto out;
 	}
 #endif
 
         if (register_netdev(&dev_cs89x0) != 0) {
                 printk(KERN_ERR "cs89x0.c: No card found at 0x%x\n", io);
-                return -ENXIO;
+                ret = -ENXIO;
+		goto out;
         }
-	return 0;
+out:
+	if (ret)
+		kfree(dev_cs89x0.priv);
+	return ret;
 }
 
 void
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org