[PATCH] Add boot command line parsing for the e100 driver

[PATCH] Add boot command line parsing for the e100 driver

[Corey Minyard]

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

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

[-- Attachment #2: e100-bootparm.diff --]
[-- Type: text/plain, Size: 3508 bytes --]

--- linux.orig/drivers/net/e100/e100_main.c	Mon Apr 21 11:20:11 2003
+++ linux/drivers/net/e100/e100_main.c	Mon May 19 10:57:33 2003
@@ -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
--- linux.orig/Documentation/networking/e100.txt	Wed Mar 26 08:00:53 2003
+++ linux/Documentation/networking/e100.txt	Mon May 19 10:50:47 2003
@@ -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.
 

Re: [PATCH] Add boot command line parsing for the e100 driver

[Christoph Hellwig]

On Mon, May 19, 2003 at 11:09:31AM -0500, Corey Minyard wrote:
> 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.

Re: [PATCH] Add boot command line parsing for the e100 driver

[Jeff Garzik]

On Mon, May 19, 2003 at 05:17:14PM +0100, Christoph Hellwig wrote:
> On Mon, May 19, 2003 at 11:09:31AM -0500, Corey Minyard wrote:
> > 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

Re: [PATCH] Add boot command line parsing for the e100 driver

[Christoph Hellwig]

On Mon, May 19, 2003 at 12:30:52PM -0400, Jeff Garzik wrote:
> > 
> > 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 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.

Re: [PATCH] Add boot command line parsing for the e100 driver

[Jeff Garzik]

On Mon, May 19, 2003 at 05:33:23PM +0100, Christoph Hellwig wrote:
> On Mon, May 19, 2003 at 12:30:52PM -0400, Jeff Garzik wrote:
> > > 
> > > 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.


> > 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...


> 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

Re: [PATCH] Add boot command line parsing for the e100 driver

[Corey Minyard]

Jeff Garzik wrote:

>>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

Re: [PATCH] Add boot command line parsing for the e100 driver

[Bartlomiej Zolnierkiewicz]

On Mon, 19 May 2003, Corey Minyard wrote:

> Jeff Garzik wrote:
>
> >>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

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

--
Bartlomiej

Re: [PATCH] Add boot command line parsing for the e100 driver

[Bartlomiej Zolnierkiewicz]

On Tue, 20 May 2003, Bartlomiej Zolnierkiewicz wrote:
> On Mon, 19 May 2003, Corey Minyard wrote:
>
> > Jeff Garzik wrote:
> >
> > >>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
>
> :-) module_parm(), look at include/linux/moduleparam.h
> and scsi for usage examples

ugh. s/module_parm/module_param/

--
Bartlomiej

Re: [PATCH] Add boot command line parsing for the e100 driver

[Corey Minyard]

Bartlomiej Zolnierkiewicz wrote:

>On Tue, 20 May 2003, Bartlomiej Zolnierkiewicz wrote:
>  
>
>>On Mon, 19 May 2003, Corey Minyard wrote:
>>
>>    
>>
>>>Jeff Garzik wrote:
>>>
>>>      
>>>
>>>>>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
>>>      
>>>
>>:-) 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