[PATCH] ide parameter parsing code cleanup

[PATCH] ide parameter parsing code cleanup

[Gerald Champagne]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]


Here's a little patch to clean up some of the ide parameter parsing
code.  I have not tested the module version of this code but the changes
seem pretty minor.  The main change is that it replaces the existing: 

__setup("",ide_setup); 

with: 

__setup("hd",hd_setup); 
__setup(ide",ide_setup); 

The setup routine is split into two routines; one to process hd
parameters specific to a device, and one to process ide parameters
specific to a controller.  The ide code was the only code in the kernel
that abused the __setup macro by calling it with a null string, and this
patch gets rid of that.

This is a small change, but it makes it obvious that some of the hd
parameters are actually setting controller parameters, not just device
parameters.

Gerald 


[-- Attachment #2: ide_parms.diff --]
[-- Type: text/x-patch, Size: 7912 bytes --]

diff -ur linux-2.5.29.orig/drivers/ide/main.c linux-2.5.29/drivers/ide/main.c
--- linux-2.5.29.orig/drivers/ide/main.c	Fri Jul 26 21:58:24 2002
+++ linux-2.5.29/drivers/ide/main.c	Sat Jul 27 11:30:39 2002
@@ -701,79 +701,46 @@
 
 /*
  * This gets called VERY EARLY during initialization, to handle kernel "command
- * line" strings beginning with "hdx=" or "ide".It gets called even before the
- * actual module gets initialized.
+ * line" strings beginning with "hdx=".  It gets called even before the actual
+ * module gets initialized.
  *
  * Please look at Documentation/ide.txt to see the complete list of supported
  * options.
  */
-int __init ide_setup(char *s)
+int __init hd_setup(char *s)
 {
-	int i, vals[4];
-	struct ata_channel *ch;
+	int vals[4];
+	struct ata_channel *ch;	/* FIXME:  Channel parms should not be accessed in hd_setup */
 	struct ata_device *drive;
 	unsigned int hw, unit;
 	const char max_drive = 'a' + ((MAX_HWIFS * MAX_DRIVES) - 1);
-	const char max_ch  = '0' + (MAX_HWIFS - 1);
-
-	if (!strncmp(s, "hd=", 3))	/* hd= is for hd.c driver and not us */
-		return 0;
 
-	if (strncmp(s,"ide",3) &&
-	    strncmp(s,"hd",2))		/* hdx= & hdxlun= */
+	if (s[0] == '=')	/* hd= is for hd.c driver and not us */
 		return 0;
 
-	printk(KERN_INFO  "ide_setup: %s", s);
+	printk(KERN_INFO  "hd_setup: hd%s", s);
 	init_global_data();
 
-#ifdef CONFIG_BLK_DEV_IDEDOUBLER
-	if (!strcmp(s, "ide=doubler")) {
-		extern int ide_doubler;
-
-		printk(KERN_INFO" : Enabled support for IDE doublers\n");
-		ide_doubler = 1;
-
-		return 1;
-	}
-#endif
-
-	if (!strcmp(s, "ide=nodma")) {
-		printk(KERN_INFO "ATA: Prevented DMA\n");
-		noautodma = 1;
-
-		return 1;
-	}
-
-#ifdef CONFIG_PCI
-	if (!strcmp(s, "ide=reverse")) {
-		ide_scan_direction = 1;
-		printk(" : Enabled support for IDE inverse scan order.\n");
-
-		return 1;
-	}
-#endif
-
-	/*
-	 * Look for drive options:  "hdx="
-	 */
-	if (!strncmp(s, "hd", 2) && s[2] >= 'a' && s[2] <= max_drive) {
+	if (s[0] >= 'a' && s[0] <= max_drive) {
 		const char *hd_words[] = {"none", "noprobe", "nowerr", "cdrom",
 				"serialize", "autotune", "noautotune",
 				"slow", "flash", "remap", "noremap", "scsi", NULL};
-		unit = s[2] - 'a';
+		unit = s[0] - 'a';
 		hw   = unit / MAX_DRIVES;
 		unit = unit % MAX_DRIVES;
 		ch = &ide_hwifs[hw];
 		drive = &ch->drives[unit];
-		if (!strncmp(s+3, "=ide-", 5)) {
-			strncpy(drive->driver_req, s + 4, 9);
+
+		/* Look for hdx=ide-* */
+		if (!strncmp(s+1, "=ide-", 5)) {
+			strncpy(drive->driver_req, s+2, 9);
 			goto done;
 		}
 		/*
 		 * Look for last lun option:  "hdxlun="
 		 */
-		if (!strncmp(s+3, "lun=", 4)) {
-	                if (*get_options(s+7, 2, vals) || vals[0]!=1)
+		if (!strncmp(s+1, "lun=", 4)) {
+	                if (*get_options(s+5, 2, vals) || vals[0]!=1)
 				goto bad_option;
 			if (vals[1] >= 0 && vals[1] <= 7) {
 				drive->last_lun = vals[1];
@@ -782,7 +749,7 @@
 				printk(" -- BAD LAST LUN! Expected value from 0 to 7");
 			goto done;
 		}
-		switch (match_parm(s+3, hd_words, vals, 3)) {
+		switch (match_parm(s+1, hd_words, vals, 3)) {
 			case -1: /* "none" */
 				drive->nobios = 1;  /* drop into "noprobe" */
 			case -2: /* "noprobe" */
@@ -790,16 +757,16 @@
 				goto done;
 			case -3: /* "nowerr" */
 				drive->bad_wstat = BAD_R_STAT;
-				ch->noprobe = 0;
+				ch->noprobe = 0;	/* FIXME:  Channel parm */
 				goto done;
 			case -4: /* "cdrom" */
 				drive->present = 1;
 				drive->type = ATA_ROM;
-				ch->noprobe = 0;
+				ch->noprobe = 0;	/* FIXME:  Channel parm */
 				goto done;
 			case -5: /* "serialize" */
 				printk(" -- USE \"ide%d=serialize\" INSTEAD", hw);
-				goto do_serialize;
+				goto bad_option;
 			case -6: /* "autotune" */
 				drive->autotune = 1;
 				goto done;
@@ -807,7 +774,7 @@
 				drive->autotune = 2;
 				goto done;
 			case -8: /* "slow" */
-				ch->slow = 1;
+				ch->slow = 1;		/* FIXME:  Channel parm */
 				goto done;
 			case -9: /* "flash" */
 				drive->ata_flash = 1;
@@ -840,11 +807,63 @@
 		}
 	}
 
+bad_option:
+	printk(" -- BAD OPTION\n");
+	return 1;
+
+done:
+	printk("\n");
+
+	return 1;
+}
+
+/*
+ * This gets called VERY EARLY during initialization, to handle kernel "command
+ * line" strings beginning with "ide".  It gets called even before the actual
+ * module gets initialized.
+ *
+ * Please look at Documentation/ide.txt to see the complete list of supported
+ * options.
+ */
+int __init ide_setup(char *s)
+{
+	int i, vals[4];
+	struct ata_channel *ch;
+	unsigned int hw;
+	const char max_ch  = '0' + (MAX_HWIFS - 1);
+
+	printk(KERN_INFO  "ide_setup: ide%s", s);
+	init_global_data();
+
+#ifdef CONFIG_BLK_DEV_IDEDOUBLER
+	if (!strcmp(s, "=doubler")) {
+		extern int ide_doubler;
+
+		printk(KERN_INFO" : Enabled support for IDE doublers\n");
+		ide_doubler = 1;
+		return 1;
+	}
+#endif
+
+	if (!strcmp(s, "=nodma")) {
+		printk(KERN_INFO "ATA: Prevented DMA\n");
+		noautodma = 1;
+		return 1;
+	}
+
+#ifdef CONFIG_PCI
+	if (!strcmp(s, "=reverse")) {
+		ide_scan_direction = 1;
+		printk(" : Enabled support for IDE inverse scan order.\n");
+		return 1;
+	}
+#endif
+
 	/*
 	 * Look for bus speed option:  "idebus="
 	 */
-	if (!strncmp(s, "idebus=", 7)) {
-		if (*get_options(s+7, 2, vals) || vals[0] != 1)
+	if (!strncmp(s, "bus=", 4)) {
+		if (*get_options(s+4, 2, vals) || vals[0] != 1)
 			goto bad_option;
 		idebus_parameter = vals[1];
 		goto done;
@@ -853,7 +872,7 @@
 	/*
 	 * Look for interface options:  "idex="
 	 */
-	if (!strncmp(s, "ide", 3) && s[3] >= '0' && s[3] <= max_ch) {
+	if (s[0] >= '0' && s[0] <= max_ch) {
 		/*
 		 * Be VERY CAREFUL changing this: note hardcoded indexes below
 		 */
@@ -861,11 +880,11 @@
 			"noprobe", "serialize", "autotune", "noautotune", "reset", "dma", "ata66", NULL };
 		const char *ide_words[] = {
 			"qd65xx", "ht6560b", "cmd640_vlb", "dtc2278", "umc8672", "ali14xx", "dc4030", NULL };
-		hw = s[3] - '0';
+		hw = s[0] - '0';
 		ch = &ide_hwifs[hw];
 
 
-		switch (match_parm(s+4, ide_options, vals, 1)) {
+		switch (match_parm(s+1, ide_options, vals, 1)) {
 			case -7: /* ata66 */
 #ifdef CONFIG_PCI
 				ch->udma_four = 1;
@@ -889,7 +908,6 @@
 				ch->drives[1].autotune = 1;
 				goto done;
 			case -2: /* "serialize" */
-			do_serialize:
 				{
 					struct ata_channel *mate;
 
@@ -904,7 +922,10 @@
 				goto done;
 		}
 
-		i = match_parm(&s[4], ide_words, vals, 3);
+		/*
+		 * Check for specific chipset name
+		 */
+		i = match_parm(s+1, ide_words, vals, 3);
 
 		/*
 		 * Cryptic check to ensure chipset not already set for a channel:
@@ -913,7 +934,7 @@
 			if (ide_hwifs[hw].chipset != ide_unknown)
 				goto bad_option;	/* chipset already specified */
 			if (i != -7 && hw != 0)
-				goto bad_channel;		/* chipset drivers are for "ide0=" only */
+				goto bad_channel;	/* chipset drivers are for "ide0=" only */
 			if (i != -7 && ide_hwifs[1].chipset != ide_unknown)
 				goto bad_option;	/* chipset for 2nd port already specified */
 			printk("\n");
@@ -1432,8 +1453,14 @@
 		while ((options = next) != NULL) {
 			if ((next = strchr(options,' ')) != NULL)
 				*next++ = 0;
-			if (!ide_setup(options))
-				printk(KERN_ERR "Unknown option '%s'\n", options);
+			if (!strncmp(options,"hd",2)) {
+				if (!hd_setup(options+2))
+					printk(KERN_ERR "Unknown option '%s'\n", options);
+			}
+			else if (!strncmp(options,"ide",3)) {
+				if (!ide_setup(options+3))
+					printk(KERN_ERR "Unknown option '%s'\n", options);
+			}
 		}
 	}
 	return ata_module_init();
@@ -1457,6 +1484,7 @@
 #ifndef MODULE
 
 /* command line option parser */
-__setup("", ide_setup);
+__setup("ide", ide_setup);
+__setup("hd", hd_setup);
 
 #endif