RE: idebus setup problem (2.6.7-rc1)

[Zhu, Yi]

Rusty Russell wrote:
> 
> OK, I've revisited this problem, with my thinking cap ON this time.
> Sorry for the delay. 
> 
> Andrew, please revert kernel-parameter-parsing-fix.patch and
> kernel-parameter-parsing-fix-fix.patch in favor of this one-liner.
> 
> Yi, does this fix your ACPI problem?
> 
> Rusty.
> 
> Name: Handle __early_param and __setup Collision
> Status: Trivial
> Depends: EarlyParam/early_param.patch.gz
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> 
> Yi Zhu (yi.zhu@xxxxxxxxx) points out the following problem:
> 
> In arch/i386/kernel/setup.c:
> 	__early_param("acpi", early_acpi);
> 
> In drivers/acpi/osl.c:
> 	__setup("acpi_os_name=", acpi_os_name_setup);
> 
> The problem command line looks like:
> 
> 	"acpi=force acpi_os_name=my_override_name"
> 
> For simplicity, we overload the __setup section to contain
> both __early_param and __setup, so we can check that all
> options on the command line are taken by at least one of
> them.  However, __early_param have different semantics the
> __setup: in particular, __early_param("acpi"), must not match
> anything but "acpi" and "acpi=", which mirrors
> module_param(), whereas __setup("acpi") would match anything
> which starts with "acpi".

Really? I think currently only ide_setup is an exception for __setup(),
which will match all params given in command line.

> 
> Fix the obsolete_checksetup code to take this difference into account
> correctly. 
> 
> diff -urpN --exclude TAGS -X
> /home/rusty/devel/kernel/kernel-patches/current-dontdiff
> --minimal .22424-linux-2.6.7-rc2-bk7/init/main.c
> .22424-linux-2.6.7-rc2-bk7.updated/init/main.c
> --- .22424-linux-2.6.7-rc2-bk7/init/main.c	2004-06-07
> 09:51:11.000000000 +1000 +++
> .22424-linux-2.6.7-rc2-bk7.updated/init/main.c 2004-06-07
> 09:53:06.000000000 +1000 @@ -159,8 +159,9 @@ static int __init
>  		obsolete_checksetup(ch  	do { int n =
strlen(p->str);
>  		if (!strncmp(line, p->str, n)) {
> -			/* Already done in parse_early_param? */
> -			if (p->early)
> +			/* Already done in parse_early_param?  (Needs
> +			 * exact match on param part) */
> +			if (p->early && (line[n] == '\0' ||
> line[n] == '='))
>  				return 1;
>  			if (!p->setup_func) {
>  				printk(KERN_WARNING "Parameter
> %s is obsolete, ignored\n", p->str);

This doesn't work. The p->setup_func for "acpi" will still be called on
behalf of "acpi_os_name".

Below change should work.

Thanks,
-yi

--- linux-2.6.7-rc2-mm2.orig/init/main.c	2004-06-07
16:01:25.000000000 +0800
+++ linux-2.6.7-rc2-mm2/init/main.c	2004-06-07 18:50:22.256204536
+0800
@@ -154,18 +154,22 @@ static int __init obsolete_checksetup(ch
 {
 	struct obs_kernel_param *p;
 	extern struct obs_kernel_param __setup_start, __setup_end;
-	char *ptr;
-	int len = strlen(line);
 
-	if ((ptr = strchr(line, '=')))
-		len = ptr - line;
 	p = &__setup_start;
 	do {
 		int n = strlen(p->str);
-		if (n == 0 || (len <= n && !strncmp(line, p->str, n))) {
-			/* Already done in parse_early_param? */
-			if (p->early)
-				return 1;
+		if (!strncmp(line, p->str, n)) {
+			if (p->early) {
+				/* Already done in parse_early_param?
(Needs
+				 * exact match on param part) */
+				if (p->early && (line[n] == '\0' ||
+				    line[n] == '='))
+					return 1;
+				else {
+					p++;
+					continue;
+				}
+			}
 			if (!p->setup_func) {
 				printk(KERN_WARNING "Parameter %s is
obsolete,"
 						" ignored\n", p->str);

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