[RFC] IA64: Use early_parm to handle mvec_name and nomca

[RFC] IA64: Use early_parm to handle mvec_name and nomca

[Horms]

I'm not sure of the worthiness of this idea, so please consider it an RFC. 
Its key merits are:

* Reuse existing infrastructure
* Greatly tightens up the parsing of nomca
* Greatly simplifies the parsing of machvec

Signed-Off-By: Horms <horms@verge.net.au>

 setup.c |   47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

4ed9fbfd2b02aea51ee5848966465397eec5a1c6
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 3258e09..53a60fa 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -389,6 +389,24 @@ check_for_logical_procs (void)
 }
 #endif
 
+#ifdef CONFIG_IA64_GENERIC
+static const char *mvec_name = NULL;
+static __init int setup_mvec(char *s)
+{
+	mvec_name = s;
+	return 0;
+}
+early_param("machvec", setup_mvec);
+#endif
+
+static int nomca = 0;
+static __init int setup_nomca(char *s)
+{
+	nomca = 1;
+	return 0;
+}
+early_param("nomca", setup_nomca);
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -402,35 +420,18 @@ setup_arch (char **cmdline_p)
 	efi_init();
 	io_port_init();
 
-#ifdef CONFIG_IA64_GENERIC
-	{
-		const char *mvec_name = strstr (*cmdline_p, "machvec=");
-		char str[64];
+	parse_early_param();
 
-		if (mvec_name) {
-			const char *end;
-			size_t len;
-
-			mvec_name += 8;
-			end = strchr (mvec_name, ' ');
-			if (end)
-				len = end - mvec_name;
-			else
-				len = strlen (mvec_name);
-			len = min(len, sizeof (str) - 1);
-			strncpy (str, mvec_name, len);
-			str[len] = '\0';
-			mvec_name = str;
-		} else
-			mvec_name = acpi_get_sysname();
+#ifdef CONFIG_IA64_GENERIC
+	if (!mvec_name)
+		machvec_init(acpi_get_sysname());
+	else
 		machvec_init(mvec_name);
-	}
 #endif
 
 	if (early_console_setup(*cmdline_p) = 0)
 		mark_bsp_online();
 
-	parse_early_param();
 #ifdef CONFIG_ACPI
 	/* Initialize the ACPI boot-time table parser */
 	acpi_table_init();
@@ -493,7 +494,7 @@ setup_arch (char **cmdline_p)
 #endif
 
 	/* enable IA-64 Machine Check Abort Handling unless disabled */
-	if (!strstr(saved_command_line, "nomca"))
+	if (!nomca)
 		ia64_mca_init();
 
 	platform_setup(cmdline_p);

Re: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Jes Sorensen]

>>>>> "Horms" = Horms  <horms@verge.net.au> writes:

Horms> I'm not sure of the worthiness of this idea, so please consider
Horms> it an RFC.  Its key merits are:

Hi Horms,

Rips out a lot of code so if it still works then I'd say it's a good
win!

You get a D- for these two though:

Horms> +static const char *mvec_name =  NULL;
Horms> +static int nomca = 0;

No need to initialize globals to zero as the BSS is cleared at boot.

Cheers,
Jes

Re: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Horms]

On Fri, Mar 10, 2006 at 04:54:35AM -0500, Jes Sorensen wrote:
> >>>>> "Horms" = Horms  <horms@verge.net.au> writes:
> 
> Horms> I'm not sure of the worthiness of this idea, so please consider
> Horms> it an RFC.  Its key merits are:
> 
> Hi Horms,
> 
> Rips out a lot of code so if it still works then I'd say it's a good
> win!

Thats pretty much what I was thinking. It does seem to work with
the limited ammount of testing that I have done.

> You get a D- for these two though:
> 
> Horms> +static const char *mvec_name =  NULL;
> Horms> +static int nomca = 0;
> 
> No need to initialize globals to zero as the BSS is cleared at boot.

Below is a fressh diff that resolves those initialisation problems.

I was actually thinking that the main drawback of this appoach was the
extra space devoted to these variables. Marking them as __init made the
linker very unhappy. Given their current delaration, will they just
stick around forever?

-- 
Horms

IA64: Use early_parm to handle mvec_name and nomca

I'm not sure of the worthiness of this idea, so please consider it an RFC. 
Its key merits are:

* Reuse existing infrastructure
* Greatly tightens up the parsing of nomca
* Greatly simplifies the parsing of machvec

Signed-Off-By: Horms <horms@verge.net.au>

 setup.c |   47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

4ed9fbfd2b02aea51ee5848966465397eec5a1c6
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 3258e09..53a60fa 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -389,6 +389,24 @@ check_for_logical_procs (void)
 }
 #endif
 
+#ifdef CONFIG_IA64_GENERIC
+static const char *mvec_name = NULL;
+static __init int setup_mvec(char *s)
+{
+	mvec_name = s;
+	return 0;
+}
+early_param("machvec", setup_mvec);
+#endif
+
+static int nomca = 0;
+static __init int setup_nomca(char *s)
+{
+	nomca = 1;
+	return 0;
+}
+early_param("nomca", setup_nomca);
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -402,35 +420,18 @@ setup_arch (char **cmdline_p)
 	efi_init();
 	io_port_init();
 
-#ifdef CONFIG_IA64_GENERIC
-	{
-		const char *mvec_name = strstr (*cmdline_p, "machvec=");
-		char str[64];
+	parse_early_param();
 
-		if (mvec_name) {
-			const char *end;
-			size_t len;
-
-			mvec_name += 8;
-			end = strchr (mvec_name, ' ');
-			if (end)
-				len = end - mvec_name;
-			else
-				len = strlen (mvec_name);
-			len = min(len, sizeof (str) - 1);
-			strncpy (str, mvec_name, len);
-			str[len] = '\0';
-			mvec_name = str;
-		} else
-			mvec_name = acpi_get_sysname();
+#ifdef CONFIG_IA64_GENERIC
+	if (!mvec_name)
+		machvec_init(acpi_get_sysname());
+	else
 		machvec_init(mvec_name);
-	}
 #endif
 
 	if (early_console_setup(*cmdline_p) = 0)
 		mark_bsp_online();
 
-	parse_early_param();
 #ifdef CONFIG_ACPI
 	/* Initialize the ACPI boot-time table parser */
 	acpi_table_init();
@@ -493,7 +494,7 @@ setup_arch (char **cmdline_p)
 #endif
 
 	/* enable IA-64 Machine Check Abort Handling unless disabled */
-	if (!strstr(saved_command_line, "nomca"))
+	if (!nomca)
 		ia64_mca_init();
 
 	platform_setup(cmdline_p);

Re: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Jes Sorensen]

Horms wrote:
> 
> Below is a fressh diff that resolves those initialisation problems.

You're still zero initializingn them in the new diff ;-(

> I was actually thinking that the main drawback of this appoach was the
> extra space devoted to these variables. Marking them as __init made the
> linker very unhappy. Given their current delaration, will they just
> stick around forever?
> 

Yous hould be able to mark them __init, otherwise they just end up
sticking around forever as you say. Grep around some other code in the
vicinity to see how it's declared if it causes problems.

Cheers,
Jes

RE: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Chen, Kenneth W]

Jes Sorensen wrote on Saturday, March 11, 2006 12:52 AM
> > I was actually thinking that the main drawback of this appoach was the
> > extra space devoted to these variables. Marking them as __init made the
> > linker very unhappy. Given their current delaration, will they just
> > stick around forever?
> > 
> 
> Yous hould be able to mark them __init, otherwise they just end up
> sticking around forever as you say. Grep around some other code in the
> vicinity to see how it's declared if it causes problems.

You should use __initdata.

- Ken

Re: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Horms]

On Sat, Mar 11, 2006 at 09:51:41AM +0100, Jes Sorensen wrote:
> Horms wrote:
> >Below is a fressh diff that resolves those initialisation problems.
> 
> You're still zero initializingn them in the new diff ;-(

Sorry, sent the wrong thing :(

[snip]

On Sat, Mar 11, 2006 at 12:17:39PM -0800, Chen, Kenneth W wrote:
> Jes Sorensen wrote on Saturday, March 11, 2006 12:52 AM
> > > I was actually thinking that the main drawback of this appoach was the
> > > extra space devoted to these variables. Marking them as __init made the
> > > linker very unhappy. Given their current delaration, will they just
> > > stick around forever?
> > > 
> > 
> > Yous hould be able to mark them __init, otherwise they just end up
> > sticking around forever as you say. Grep around some other code in the
> > vicinity to see how it's declared if it causes problems.
> 
> You should use __initdata.

Thanks,

The patch below should address the BSS and __initdata deficiencies.
I also did a bit more testing, and it does indeed seem to work.

-- 
Horms

IA64: Use early_parm to handle mvec_name and nomca

I'm not sure of the worthiness of this idea, so please consider it an RFC. 
Its key merits are:

* Reuse existing infrastructure
* Greatly tightens up the parsing of nomca
* Greatly simplifies the parsing of machvec

Signed-Off-By: Horms <horms@verge.net.au>

 setup.c |   47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 3258e09..afabe1b 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -389,6 +389,24 @@ check_for_logical_procs (void)
 }
 #endif
 
+#ifdef CONFIG_IA64_GENERIC
+static __initdata const char *mvec_name;
+static __init int setup_mvec(char *s)
+{
+	mvec_name = s;
+	return 0;
+}
+early_param("machvec", setup_mvec);
+#endif
+
+static __initdata int nomca;
+static __init int setup_nomca(char *s)
+{
+	nomca = 1;
+	return 0;
+}
+early_param("nomca", setup_nomca);
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -402,35 +420,18 @@ setup_arch (char **cmdline_p)
 	efi_init();
 	io_port_init();
 
-#ifdef CONFIG_IA64_GENERIC
-	{
-		const char *mvec_name = strstr (*cmdline_p, "machvec=");
-		char str[64];
+	parse_early_param();
 
-		if (mvec_name) {
-			const char *end;
-			size_t len;
-
-			mvec_name += 8;
-			end = strchr (mvec_name, ' ');
-			if (end)
-				len = end - mvec_name;
-			else
-				len = strlen (mvec_name);
-			len = min(len, sizeof (str) - 1);
-			strncpy (str, mvec_name, len);
-			str[len] = '\0';
-			mvec_name = str;
-		} else
-			mvec_name = acpi_get_sysname();
+#ifdef CONFIG_IA64_GENERIC
+	if (!mvec_name)
+		machvec_init(acpi_get_sysname());
+	else
 		machvec_init(mvec_name);
-	}
 #endif
 
 	if (early_console_setup(*cmdline_p) = 0)
 		mark_bsp_online();
 
-	parse_early_param();
 #ifdef CONFIG_ACPI
 	/* Initialize the ACPI boot-time table parser */
 	acpi_table_init();
@@ -493,7 +494,7 @@ setup_arch (char **cmdline_p)
 #endif
 
 	/* enable IA-64 Machine Check Abort Handling unless disabled */
-	if (!strstr(saved_command_line, "nomca"))
+	if (!nomca)
 		ia64_mca_init();
 
 	platform_setup(cmdline_p);

RE: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Chen, Kenneth W]

Horms wrote on Sunday, March 12, 2006 8:08 PM
> IA64: Use early_parm to handle mvec_name and nomca
> 
> I'm not sure of the worthiness of this idea, so please consider it an RFC. 
> Its key merits are:
> 
> * Reuse existing infrastructure
> * Greatly tightens up the parsing of nomca
> * Greatly simplifies the parsing of machvec

It is kind of odd though that parse_early_param() is called twice,
once from init/main.c:start_kernel and once from arch/ia64/kernel/
setup.c:setup_arch.  Though you are not the one introduces that
Oddity.

The other thing is that the code you changed are going to be thrown
away after initialization, so I take your motivation of the patch
is in some form of uncluttered the source code?

If I were you, I would put machvec code into machvec.c, like this:


 machvec.c |   17 ++++++++++++++---
 setup.c   |   30 +++++-------------------------
 2 files changed, 19 insertions(+), 28 deletions(-)

--- ./arch/ia64/kernel/machvec.c.orig	2006-03-12 22:52:30.208644445 -0800
+++ ./arch/ia64/kernel/machvec.c	2006-03-12 23:07:09.688125859 -0800
@@ -14,6 +14,14 @@
 struct ia64_machine_vector ia64_mv;
 EXPORT_SYMBOL(ia64_mv);
 
+static __initdata const char *mvec_name;
+static __init int setup_mvec(char *s)
+{
+	mvec_name = s;
+	return 0;
+}
+early_param("machvec", setup_mvec);
+
 static struct ia64_machine_vector * __init
 lookup_machvec (const char *name)
 {
@@ -33,10 +41,13 @@ machvec_init (const char *name)
 {
 	struct ia64_machine_vector *mv;
 
+	if (!name)
+		name = mvec_name ? mvec_name : acpi_get_sysname();
 	mv = lookup_machvec(name);
-	if (!mv) {
-		panic("generic kernel failed to find machine vector for platform %s!", name);
-	}
+	if (!mv)
+		panic("generic kernel failed to find machine vector for"
+		      " platform %s!", name);
+
 	ia64_mv = *mv;
 	printk(KERN_INFO "booting generic kernel on platform %s\n", name);
 }
--- ./arch/ia64/kernel/setup.c.orig	2006-03-12 22:53:16.178370444 -0800
+++ ./arch/ia64/kernel/setup.c	2006-03-12 23:01:43.595356416 -0800
@@ -402,35 +402,15 @@ setup_arch (char **cmdline_p)
 	efi_init();
 	io_port_init();
 
-#ifdef CONFIG_IA64_GENERIC
-	{
-		const char *mvec_name = strstr (*cmdline_p, "machvec=");
-		char str[64];
-
-		if (mvec_name) {
-			const char *end;
-			size_t len;
-
-			mvec_name += 8;
-			end = strchr (mvec_name, ' ');
-			if (end)
-				len = end - mvec_name;
-			else
-				len = strlen (mvec_name);
-			len = min(len, sizeof (str) - 1);
-			strncpy (str, mvec_name, len);
-			str[len] = '\0';
-			mvec_name = str;
-		} else
-			mvec_name = acpi_get_sysname();
-		machvec_init(mvec_name);
-	}
-#endif
-
 	if (early_console_setup(*cmdline_p) = 0)
 		mark_bsp_online();
 
 	parse_early_param();
+
+#ifdef CONFIG_IA64_GENERIC
+	machvec_init(NULL);
+#endif
+
 #ifdef CONFIG_ACPI
 	/* Initialize the ACPI boot-time table parser */
 	acpi_table_init();

Re: [RFC] IA64: Use early_parm to handle mvec_name and nomca

[Horms]

On Sun, Mar 12, 2006 at 10:33:27PM -0800, Chen, Kenneth W wrote:
> Horms wrote on Sunday, March 12, 2006 8:08 PM
> > IA64: Use early_parm to handle mvec_name and nomca
> > 
> > I'm not sure of the worthiness of this idea, so please consider it an RFC. 
> > Its key merits are:
> > 
> > * Reuse existing infrastructure
> > * Greatly tightens up the parsing of nomca
> > * Greatly simplifies the parsing of machvec
> 
> It is kind of odd though that parse_early_param() is called twice,
> once from init/main.c:start_kernel and once from arch/ia64/kernel/
> setup.c:setup_arch.  Though you are not the one introduces that
> Oddity.

Yes, that is odd. I guess the arch/ia64/kernel/setup.c one could be
removed. That is, unless the init/main.c is called too late. I'll
investigate that.

> The other thing is that the code you changed are going to be thrown
> away after initialization, so I take your motivation of the patch
> is in some form of uncluttered the source code?

Yes. It seems to me that the parsing is greatly simplified with this
approach.

> If I were you, I would put machvec code into machvec.c, like this:

That is an excellent idea.

-- 
Horms

IA64: Broken Serial Console

[Horms]

Hi,

I have noticed that the serial console on my ia64 machine seems
to have broken since 2.6.16. Thanks to git bisect I have been
able to track this problem down to 111c9bf8c5cfa92363b3719c8956d29368b5b9de

  [SERIAL] remove 8250_acpi (replaced by 8250_pnp and PNPACPI)

  With the combination of PNPACPI and 8250_pnp, we no longer need
  8250_acpi.

  Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
  Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

I haven't had time to look into the code and I have to leave
my desk shortly, so I thought I would post here to see if anyone
has any ideas.

I am boot using the following command line:

  console=tty0 console=uart,io,0x2f8 console=ttyS0

Althogh I'm pretty sure I really only need:

  console=uart,io,0x2f8

A failed boot looks like this:

  io scheduler anticipatory registered (default)
  Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing enabled
  isa bounce pool size: 16 pages
  RAMDISK driver initialized: 16 RAM disks of 4096K size 1024 blocksize
  mice: PS/2 mouse device common for all mice
  EFI Variables Facility v0.08 2004-May-17
  No ttyS device at I/O port 0x2f8 for console

And a successful boot looks like this:

  io scheduler anticipatory registered (default)
  Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing enabled
  acpi_serial_port: zero-length IO port range?
  serial8250: ttyS0 at I/O 0x3f8 (irq = 44) is a 16550A
  acpi_serial_port: zero-length IO port range?
  serial8250: ttyS1 at I/O 0x2f8 (irq = 45) is a 16550A
  isa bounce pool size: 16 pages
  RAMDISK driver initialized: 16 RAM disks of 4096K size 1024 blocksize
  mice: PS/2 mouse device common for all mice
  EFI Variables Facility v0.08 2004-May-17
  Adding console on ttyS1 at I/O port 0x2f8 (options '115200')

  prompt #

The machine in question is a Trus64-21. Sorry I don't have an English
link handy, but if you need me to dig up details please ask.

http://www.plathome.co.jp/factory/ser/tru_ita64_21.html

-- 
Horms                                           http://www.vergenet.net/~horms/

Re: IA64: Broken Serial Console

[Matthew Wilcox]

On Fri, Jun 02, 2006 at 08:48:06PM +0900, Horms wrote:
> I have noticed that the serial console on my ia64 machine seems
> to have broken since 2.6.16. Thanks to git bisect I have been
> able to track this problem down to 111c9bf8c5cfa92363b3719c8956d29368b5b9de
> 
>   [SERIAL] remove 8250_acpi (replaced by 8250_pnp and PNPACPI)
> 
>   With the combination of PNPACPI and 8250_pnp, we no longer need
>   8250_acpi.
> 
>   Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
>   Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> I haven't had time to look into the code and I have to leave
> my desk shortly, so I thought I would post here to see if anyone
> has any ideas.

Well, does your config have PNPACPI and SERIAL_8250_PNP enabled?

Re: IA64: Broken Serial Console

[Francois WELLENREITER]

>I am boot using the following command line:
>
>  console=tty0 console=uart,io,0x2f8 console=ttyS0
>
>Althogh I'm pretty sure I really only need:
>
>  console=uart,io,0x2f8
>  
>

Have you tried the following parameters ?

console=uart,io,0x2f8,115200n8

Re: IA64: Broken Serial Console

[Horms]

On Fri, Jun 02, 2006 at 05:53:59AM -0600, Matthew Wilcox wrote:
> On Fri, Jun 02, 2006 at 08:48:06PM +0900, Horms wrote:
> > I have noticed that the serial console on my ia64 machine seems
> > to have broken since 2.6.16. Thanks to git bisect I have been
> > able to track this problem down to 111c9bf8c5cfa92363b3719c8956d29368b5b9de
> > 
> >   [SERIAL] remove 8250_acpi (replaced by 8250_pnp and PNPACPI)
> > 
> >   With the combination of PNPACPI and 8250_pnp, we no longer need
> >   8250_acpi.
> > 
> >   Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> >   Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > I haven't had time to look into the code and I have to leave
> > my desk shortly, so I thought I would post here to see if anyone
> > has any ideas.
> 
> Well, does your config have PNPACPI and SERIAL_8250_PNP enabled?

Thanks, adding those options (obviously) solved the problem.

-- 
Horms                                           http://www.vergenet.net/~horms/