Skip to contentSkip to conversation

Add boot command line parsing for the e100 driver

PATCH In review Corey Minyard 4 participants 9 messages May 19, 2003 – May 20, 2003

Subsystems (3)
Networking Network drivers Documentation
lore
Share & tools
lore

Corey Minyard

author
#0 lore raw

PATCH 1/1 Add boot command line parsing for the e100 driver

Annoyed by the fact that I could set configuration parameters for a compiled-in e100 driver, I've added boot-line parameter parsing. The patch is attached. It would be very helpful if this could be applied. This is relative to 2.5.68, but should be pretty portable.

-Corey

drivers/net/e100/e100_main.c +72 -1
@@ -174,7 +174,7 @@
  * over and over (plus this helps to avoid typo bugs).
  */
 #define E100_PARAM(X, S)                                        \
-        static const int X[E100_MAX_NIC + 1] = E100_PARAM_INIT; \
+        static int X[E100_MAX_NIC + 1] = E100_PARAM_INIT; \
         MODULE_PARM(X, "1-" __MODULE_STRING(E100_MAX_NIC) "i"); \
         MODULE_PARM_DESC(X, S);
 

@@ -375,6 +375,77 @@
 E100_PARAM(BundleSmallFr, "Disable or enable interrupt bundling of small frames");
 E100_PARAM(BundleMax, "Maximum number for CPU saver's packet bundling");
 E100_PARAM(IFS, "Disable or enable the adaptive IFS algorithm");
+
+#if !defined(MODULE)
+static int next_e100_setup = 0;
+
+#define E100_BOOT_PARAM(parm, strparm) \
+	if (strcmp(strparm, name) == 0) {				\
+		int val;						\
+		char *end;						\
+		val = simple_strtoul(strval, &end, 0);			\
+		if (*end != '\0') {					\
+			printk("Invalid value for parm num %d, name"	\
+			       " %s, parm ignored\n", count, name);	\
+			continue;					\
+		}							\
+		printk("  Setting %s to %d\n", strparm, val);		\
+		parm[i] = val;						\
+		continue;						\
+	}
+		
+static int __init
+e100_boot_setup(char *str)
+{
+	int i = next_e100_setup;
+	int count = -1;
+	char *p;
+	char *name;
+	char *strval;
+
+	if (i >= E100_MAX_NIC) {
+		printk("Attempted to configure too many e100 devices\n");
+		return -ENODEV;
+	}
+
+	printk("e100 boot setup for device %d\n", i);
+
+	next_e100_setup++;
+
+	for (p=strsep(&str, ":,"); p; p=strsep(&str, ":,")) {
+		count++;
+		name = p;
+		name = strsep(&p, "=");
+		if (!name) {
+			printk("  error: Empty parm %d, ignored\n", count);
+			continue;
+		}
+		strval = strsep(&p, "");
+		if (!strval) {
+			printk("  error: No value for parm %d, ignored\n",
+			       count);
+			continue;
+		}
+		
+		E100_BOOT_PARAM(TxDescriptors, "TxDescriptors");
+		E100_BOOT_PARAM(RxDescriptors, "RxDescriptors");
+		E100_BOOT_PARAM(XsumRX, "XsumRX");
+		E100_BOOT_PARAM(e100_speed_duplex, "e100_speed_duplex");
+		E100_BOOT_PARAM(ucode, "ucode");
+		E100_BOOT_PARAM(ber, "ber");
+		E100_BOOT_PARAM(flow_control, "flow_control");
+		E100_BOOT_PARAM(IntDelay, "IntDelay");
+		E100_BOOT_PARAM(BundleSmallFr, "BundleSmallFr");
+		E100_BOOT_PARAM(BundleMax, "BundleMax");
+		E100_BOOT_PARAM(IFS, "IFS");
+		printk("  Invalid parameter name %s, ignored\n", name);
+	}
+
+	return 0;
+}
+
+__setup("e100=", e100_boot_setup);
+#endif
 
 /**
  * e100_exec_cmd - issue a comand
Documentation/networking/e100.txt +11 -0
@@ -110,6 +110,17 @@
 resources for the second adapter. This configuration favors the second 
 adapter. The driver supports up to 16 network adapters concurrently.
 
+If the driver is compiled into the kernel, you may also specify these
+on the boot command line for linux using the 'e100' parameter,
+followed by the "option=val" separated by commas or colons.  You may
+put the e100 parameter on the command line multiple times, each one
+configures the next device.  For instance, you can say:
+
+    e100=ucode=0 e100=ucode=1,IntDelay=700
+
+to turn of the microcode download on the first device, and turn on the
+microcode download and set the delay to 700 on the second device.
+
 The default value for each parameter is generally the recommended setting,
 unless otherwise noted.
#1 lore raw
Quoted 4 lines · Corey Minyard

Annoyed by the fact that I could set configuration parameters for a compiled-in e100 driver, I've added boot-line parameter parsing. The patch is attached. It would be very helpful if this could be applied. This is relative to 2.5.68, but should be pretty portable.

Don't do this. 2.5 has the module_parame stuff that works for both static and modular drivers. Just convert e100 to it.

#2 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Corey Minyard

Annoyed by the fact that I could set configuration parameters for a compiled-in e100 driver, I've added boot-line parameter parsing. The patch is attached. It would be very helpful if this could be applied. This is relative to 2.5.68, but should be pretty portable.

Don't do this. 2.5 has the module_parame stuff that works for both static and modular drivers. Just convert e100 to it.

...which totally screws people trying to keep 2.4 and 2.5 sources as close as possible.

If all modules do not require new module_param changes, then logically, e100 does not either. And e100 has a better argument than most against such changes.

Jeff

#3 lore raw
Quoted 6 lines · Jeff Garzik
Quoted 3 lines

Don't do this. 2.5 has the module_parame stuff that works for both static and modular drivers. Just convert e100 to it.

...which totally screws people trying to keep 2.4 and 2.5 sources as close as possible.

So what? It's not that we APIs don't change under Linux.

Quoted 3 lines

If all modules do not require new module_param changes, then logically, e100 does not either. And e100 has a better argument than most against such changes.

Again, we don't convert old drivers just for the sake of it. But instead of adding such horrible cruft Corey did it should just use the proper API.

#4 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 6 lines · Jeff Garzik
Quoted 3 lines

Don't do this. 2.5 has the module_parame stuff that works for both static and modular drivers. Just convert e100 to it.

...which totally screws people trying to keep 2.4 and 2.5 sources as close as possible.

So what? It's not that we APIs don't change under Linux.

If you look carefully, source back-compatibility has been preserved for network drivers. Most API changes are accepted without comment because they are easily made back-compatible. The module_param API breaks that.

Network driver-land is different from SCSI-land, where the amount of 2.5 differences is so high one more change doesn't make a difference.

Quoted 5 lines
Quoted 3 lines

If all modules do not require new module_param changes, then logically, e100 does not either. And e100 has a better argument than most against such changes.

Again, we don't convert old drivers just for the sake of it. But

Well...

Quoted 2 lines

instead of adding such horrible cruft Corey did it should just use the proper API.

An API already exists, and it is source compatible between 2.4 and 2.5:
ethX=.... on the kernel command line.

The proper patch would pick up options from there.

Jeff

author
#5 lore raw
Quoted 10 lines · Jeff Garzik
Quoted 4 lines

instead of adding such horrible cruft Corey did it should just use the proper API.

An API already exists, and it is source compatible between 2.4 and 2.5:
ethX=.... on the kernel command line.

The proper patch would pick up options from there.

Can you tell me where this is? I found the "ether=xxx" and "netdev=xxx", but they are not suitible. I also could not find "module_parame" anywhere on google or in the kernel.

-Corey

#6 lore raw
Quoted 10 lines · Corey Minyard
Quoted 8 lines · Jeff Garzik
Quoted 4 lines

instead of adding such horrible cruft Corey did it should just use the proper API.

An API already exists, and it is source compatible between 2.4 and 2.5:
ethX=.... on the kernel command line.

Quoted 7 lines
Quoted 2 lines

The proper patch would pick up options from there.

Can you tell me where this is? I found the "ether=xxx" and "netdev=xxx", but they are not suitible. I also could not find "module_parame" anywhere on google or in the kernel.

-Corey

:-) module_parm(), look at include/linux/moduleparam.h and scsi for usage examples

--
Bartlomiej

#7 lore raw
Quoted 14 lines · Bartlomiej Zolnierkiewicz
Quoted 12 lines · Corey Minyard
Quoted 10 lines · Jeff Garzik
Quoted 4 lines

instead of adding such horrible cruft Corey did it should just use the proper API.

An API already exists, and it is source compatible between 2.4 and 2.5:
ethX=.... on the kernel command line.

The proper patch would pick up options from there.

Quoted 8 lines
Quoted 5 lines

Can you tell me where this is? I found the "ether=xxx" and "netdev=xxx", but they are not suitible. I also could not find "module_parame" anywhere on google or in the kernel.

-Corey

:-) module_parm(), look at include/linux/moduleparam.h and scsi for usage examples

ugh. s/module_parm/module_param/

--
Bartlomiej

author
#8 lore raw
Quoted 31 lines · Bartlomiej Zolnierkiewicz
Quoted 28 lines · Bartlomiej Zolnierkiewicz
Quoted 24 lines · Corey Minyard
Quoted 6 lines · Jeff Garzik
Quoted 6 lines

instead of adding such horrible cruft Corey did it should just use the proper API.

Quoted 7 lines

An API already exists, and it is source compatible between 2.4 and 2.5:
ethX=.... on the kernel command line.

The proper patch would pick up options from there.

Can you tell me where this is? I found the "ether=xxx" and "netdev=xxx", but they are not suitible. I also could not find "module_parame" anywhere on google or in the kernel.

-Corey

Quoted 7 lines
Quoted 4 lines

:-) module_parm(), look at include/linux/moduleparam.h and scsi for usage examples

ugh. s/module_parm/module_param/

Thank you. Nobody seems to be able to type correctly today :-). I had actually found it, and looked it over. It looks pretty nice, although the lack of documentation is somewhat annoying.

However, if the ethX=.... exists, I would far prefer to use that. (The parameters for what am doing have to be at bootup to work, it can't be after the fact. I hunted for a while, and I still couldn't find it, btw.) Otherwise, it's really up to the driver maintainers. I will adjust as they want, I will use the module_param stuff or the old __setup stuff. I would like to get this in, though.

-Corey