[patch] early printk and boot console fixups.

[patch] early printk and boot console fixups.

[Gerd Hoffmann]

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

  Hi,

The console subsystem already has an idea of a boot console, using the
CON_BOOT flag.  The implementation has some flaws though.  The major
problem is that presence of a boot console makes register_console()
ignore any other console devices (unless explicitly specified on the
kernel command line).

This patch fixes the console selection code to *not* consider a boot
console a full-featured one, so the first "normal" console registering
will become the default boot console instead.  This way the unregister
call for the boot console in register_console() actually triggers and
the handover from the boot console to the real console device works
smoothly.

The patch also changes the x86 early_printk code to use this.  The early
console is simply tagged as boot console, the disable_early_printk()
call is gone as it isn't needed any more.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[-- Attachment #2: early-printk-boot-console.patch --]
[-- Type: text/x-patch, Size: 3691 bytes --]

---
 arch/x86_64/kernel/early_printk.c |   20 +++++---------------
 drivers/char/tty_io.c             |    5 -----
 kernel/printk.c                   |   26 ++++++++++++++++----------
 3 files changed, 21 insertions(+), 30 deletions(-)

Index: paravirt-2.6.20-hg770/arch/x86_64/kernel/early_printk.c
===================================================================
--- paravirt-2.6.20-hg770.orig/arch/x86_64/kernel/early_printk.c
+++ paravirt-2.6.20-hg770/arch/x86_64/kernel/early_printk.c
@@ -244,22 +244,12 @@ static int __init setup_early_printk(cha
  		early_console = &simnow_console;
  		keep_early = 1;
 	}
+
+	if (keep_early)
+		early_console->flags &= ~CON_BOOT;
+	else
+		early_console->flags |= CON_BOOT;
 	register_console(early_console);
 	return 0;
 }
-
 early_param("earlyprintk", setup_early_printk);
-
-void __init disable_early_printk(void)
-{
-	if (!early_console_initialized || !early_console)
-		return;
-	if (!keep_early) {
-		printk("disabling early console\n");
-		unregister_console(early_console);
-		early_console_initialized = 0;
-	} else {
-		printk("keeping early console\n");
-	}
-}
-
Index: paravirt-2.6.20-hg770/kernel/printk.c
===================================================================
--- paravirt-2.6.20-hg770.orig/kernel/printk.c
+++ paravirt-2.6.20-hg770/kernel/printk.c
@@ -931,8 +931,16 @@ void register_console(struct console *co
 {
 	int i;
 	unsigned long flags;
+	struct console *bootconsole = NULL;
 
-	if (preferred_console < 0)
+	if (console_drivers) {
+		if (console->flags & CON_BOOT)
+			return;
+		if (console_drivers->flags & CON_BOOT)
+			bootconsole = console_drivers;
+	}
+
+	if (preferred_console < 0 || bootconsole || !console_drivers)
 		preferred_console = selected_console;
 
 	/*
@@ -978,8 +986,11 @@ void register_console(struct console *co
 	if (!(console->flags & CON_ENABLED))
 		return;
 
-	if (console_drivers && (console_drivers->flags & CON_BOOT)) {
-		unregister_console(console_drivers);
+	if (bootconsole) {
+		printk(KERN_INFO "console handover: boot [%s%d] -> real [%s%d]\n",
+		       bootconsole->name, bootconsole->index,
+		       console->name, console->index);
+		unregister_console(bootconsole);
 		console->flags &= ~CON_PRINTBUFFER;
 	}
 
@@ -1030,16 +1041,11 @@ int unregister_console(struct console *c
 		}
 	}
 
-	/* If last console is removed, we re-enable picking the first
-	 * one that gets registered. Without that, pmac early boot console
-	 * would prevent fbcon from taking over.
-	 *
+	/*
 	 * If this isn't the last console and it has CON_CONSDEV set, we
 	 * need to set it on the next preferred console.
 	 */
-	if (console_drivers == NULL)
-		preferred_console = selected_console;
-	else if (console->flags & CON_CONSDEV)
+	if (console_drivers != NULL && console->flags & CON_CONSDEV)
 		console_drivers->flags |= CON_CONSDEV;
 
 	release_console_sem();
Index: paravirt-2.6.20-hg770/drivers/char/tty_io.c
===================================================================
--- paravirt-2.6.20-hg770.orig/drivers/char/tty_io.c
+++ paravirt-2.6.20-hg770/drivers/char/tty_io.c
@@ -141,8 +141,6 @@ static DECLARE_MUTEX(allocated_ptys_lock
 static int ptmx_open(struct inode *, struct file *);
 #endif
 
-extern void disable_early_printk(void);
-
 static void initialize_tty_struct(struct tty_struct *tty);
 
 static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
@@ -3881,9 +3879,6 @@ void __init console_init(void)
 	 * set up the console device so that later boot sequences can 
 	 * inform about problems etc..
 	 */
-#ifdef CONFIG_EARLY_PRINTK
-	disable_early_printk();
-#endif
 	call = __con_initcall_start;
 	while (call < __con_initcall_end) {
 		(*call)();

Re: [patch] early printk and boot console fixups.

[Yinghai Lu]

On 2/16/07, Gerd Hoffmann <kraxel@suse.de> wrote:

> The console subsystem already has an idea of a boot console, using the
> CON_BOOT flag.  The implementation has some flaws though.  The major
> problem is that presence of a boot console makes register_console()
> ignore any other console devices (unless explicitly specified on the
> kernel command line).
>
> This patch fixes the console selection code to *not* consider a boot
> console a full-featured one, so the first "normal" console registering
> will become the default boot console instead.  This way the unregister
> call for the boot console in register_console() actually triggers and
> the handover from the boot console to the real console device works
> smoothly.
>
> The patch also changes the x86 early_printk code to use this.  The early
> console is simply tagged as boot console, the disable_early_printk()
> call is gone as it isn't needed any more.

just notice console handover patch got into mainline 5/8.

for early_uart_console, I have some ideas:
1. merged that into early_serial_console in
arch/x86_64/kernel/early_printk.c, and
move early_printk.c to kernel/, --- make it understand
     earlyprintk=uart,io,0x3f8,9600n8
     earlyprintk=uart,mmio,0xff5e0000,115200n8
in addition to vga, ttyS, serial

2. or make early_uart_console to be another CON_BOOT, and get rid of
late_initcall(early_uart_console_switch).
actually with late_initcall early_uart_console_switch instead of
console handover, we will lose char output from serial drv init to
late_initcall stage.
in this case, we need to EARLY_SERIAL_CONSOLE to select
early_serial_console or early_uart_console for x86_64 platform.

YH

Re: [patch] early printk and boot console fixups.

[Bjorn Helgaas]

On Tuesday 15 May 2007 10:17:50 pm Yinghai Lu wrote:
> for early_uart_console, I have some ideas:
> 1. merged that into early_serial_console in
> arch/x86_64/kernel/early_printk.c, and
> move early_printk.c to kernel/, --- make it understand
>      earlyprintk=uart,io,0x3f8,9600n8
>      earlyprintk=uart,mmio,0xff5e0000,115200n8
> in addition to vga, ttyS, serial

I would rather try to merge 8250_early and early_printk.
"console=uart,io,0x3f8" is already pretty similar to
"earlyprintk=serial", and it would be nice to have only one.

"console=uart" does have the disadvantage that you have to know the
port address.  But it has the advantage that it doesn't depend on
compiled-in legacy serial port names.  If you need early console
output, you're probably a developer and can come up with the address.

I think it would be nice to have a "console=vga".  That could use
the current "earlyprintk=vga" implementation, and automatically
hand off to tty0.

Then we could remove "earlyprintk=" and always use "console=uart"
or "console=vga".

Bjorn

Re: [patch] early printk and boot console fixups.

[Yinghai Lu]

On 5/16/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Tuesday 15 May 2007 10:17:50 pm Yinghai Lu wrote:
> > for early_uart_console, I have some ideas:
> > 1. merged that into early_serial_console in
> > arch/x86_64/kernel/early_printk.c, and
> > move early_printk.c to kernel/, --- make it understand
> >      earlyprintk=uart,io,0x3f8,9600n8
> >      earlyprintk=uart,mmio,0xff5e0000,115200n8
> > in addition to vga, ttyS, serial
>
> I would rather try to merge 8250_early and early_printk.
> "console=uart,io,0x3f8" is already pretty similar to
> "earlyprintk=serial", and it would be nice to have only one.
>
> "console=uart" does have the disadvantage that you have to know the
> port address.  But it has the advantage that it doesn't depend on
> compiled-in legacy serial port names.  If you need early console
> output, you're probably a developer and can come up with the address.
>
> I think it would be nice to have a "console=vga".  That could use
> the current "earlyprintk=vga" implementation, and automatically
> hand off to tty0.
>
> Then we could remove "earlyprintk=" and always use "console=uart"
> or "console=vga".

earlyprintk= is needed for seeting bootconsole, or setting CON_BOOT,
and it is could be loaded earlier with early_param than console_init.

in the console_init (via register_console), the console is handed over
from bootconsole to normal console.

also console_init could be moved to quite late than current, and till
some subsystem is initialized like pci and usb.

YH

Re: [patch] early printk and boot console fixups.

[Bjorn Helgaas]

On Wednesday 16 May 2007 10:29:11 am Yinghai Lu wrote:
> On 5/16/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Tuesday 15 May 2007 10:17:50 pm Yinghai Lu wrote:
> > > for early_uart_console, I have some ideas:
> > > 1. merged that into early_serial_console in
> > > arch/x86_64/kernel/early_printk.c, and
> > > move early_printk.c to kernel/, --- make it understand
> > >      earlyprintk=uart,io,0x3f8,9600n8
> > >      earlyprintk=uart,mmio,0xff5e0000,115200n8
> > > in addition to vga, ttyS, serial
> >
> > I would rather try to merge 8250_early and early_printk.
> > "console=uart,io,0x3f8" is already pretty similar to
> > "earlyprintk=serial", and it would be nice to have only one.
> >
> > "console=uart" does have the disadvantage that you have to know the
> > port address.  But it has the advantage that it doesn't depend on
> > compiled-in legacy serial port names.  If you need early console
> > output, you're probably a developer and can come up with the address.
> >
> > I think it would be nice to have a "console=vga".  That could use
> > the current "earlyprintk=vga" implementation, and automatically
> > hand off to tty0.
> >
> > Then we could remove "earlyprintk=" and always use "console=uart"
> > or "console=vga".
> 
> earlyprintk= is needed for seeting bootconsole, or setting CON_BOOT,
> and it is could be loaded earlier with early_param than console_init.

8250_early.c does basically the same thing as the serial part of
early_printk.c, so we should be able to make it work just as early.
I don't remember why I didn't use early_param() for 8250_early;
I probably should have.  On ia64, it doesn't matter because we call
early_serial_console_init() explicitly at about the time of
parse_early_param().

8250_early and early_printk are so similar, I just don't see the
need for both.

Bjorn

Re: [patch] early printk and boot console fixups.

[Maciej W. Rozycki]

On Wed, 16 May 2007, Bjorn Helgaas wrote:

> > for early_uart_console, I have some ideas:
> > 1. merged that into early_serial_console in
> > arch/x86_64/kernel/early_printk.c, and
> > move early_printk.c to kernel/, --- make it understand
> >      earlyprintk=uart,io,0x3f8,9600n8
> >      earlyprintk=uart,mmio,0xff5e0000,115200n8
> > in addition to vga, ttyS, serial
> 
> I would rather try to merge 8250_early and early_printk.
> "console=uart,io,0x3f8" is already pretty similar to
> "earlyprintk=serial", and it would be nice to have only one.
> 
> "console=uart" does have the disadvantage that you have to know the
> port address.  But it has the advantage that it doesn't depend on
> compiled-in legacy serial port names.  If you need early console
> output, you're probably a developer and can come up with the address.

 Given the generic name of "uart" I am assuming this will work with any 
UART driver making use of the serial_core.c core -- am I correct?

  Maciej

Re: [patch] early printk and boot console fixups.

[Bjorn Helgaas]

On Wednesday 16 May 2007 11:09:14 am Maciej W. Rozycki wrote:
> On Wed, 16 May 2007, Bjorn Helgaas wrote:
> 
> > > for early_uart_console, I have some ideas:
> > > 1. merged that into early_serial_console in
> > > arch/x86_64/kernel/early_printk.c, and
> > > move early_printk.c to kernel/, --- make it understand
> > >      earlyprintk=uart,io,0x3f8,9600n8
> > >      earlyprintk=uart,mmio,0xff5e0000,115200n8
> > > in addition to vga, ttyS, serial
> > 
> > I would rather try to merge 8250_early and early_printk.
> > "console=uart,io,0x3f8" is already pretty similar to
> > "earlyprintk=serial", and it would be nice to have only one.
> > 
> > "console=uart" does have the disadvantage that you have to know the
> > port address.  But it has the advantage that it doesn't depend on
> > compiled-in legacy serial port names.  If you need early console
> > output, you're probably a developer and can come up with the address.
> 
>  Given the generic name of "uart" I am assuming this will work with any 
> UART driver making use of the serial_core.c core -- am I correct?

I knew somebody would ask that eventually :-)

Unfortunately, the answer is "no."  "console=uart" only works with
8250-compatible devices.

Re: [patch] early printk and boot console fixups.

[Maciej W. Rozycki]

On Wed, 16 May 2007, Bjorn Helgaas wrote:

> >  Given the generic name of "uart" I am assuming this will work with any 
> > UART driver making use of the serial_core.c core -- am I correct?
> 
> I knew somebody would ask that eventually :-)
> 
> Unfortunately, the answer is "no."  "console=uart" only works with
> 8250-compatible devices.

 But is it a design limitation or is it just that other UART drivers have 
to be modified to work with this option?  My point is if the former, then 
the option should be something like "console=8250" (the right-hand side 
being the name of the driver involved; others may want to add support for 
other drivers).  If the latter, then it's fine as is as other UART drivers 
may be wired to this code as a need arises.

  Maciej

Re: [patch] early printk and boot console fixups.

[Yinghai Lu]

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

On 5/17/07, Maciej W. Rozycki <macro@linux-mips.org> wrote:
> On Wed, 16 May 2007, Bjorn Helgaas wrote:
>
> > >  Given the generic name of "uart" I am assuming this will work with any
> > > UART driver making use of the serial_core.c core -- am I correct?
> >
> > I knew somebody would ask that eventually :-)
> >
> > Unfortunately, the answer is "no."  "console=uart" only works with
> > 8250-compatible devices.
>
>  But is it a design limitation or is it just that other UART drivers have
> to be modified to work with this option?  My point is if the former, then
> the option should be something like "console=8250" (the right-hand side
> being the name of the driver involved; others may want to add support for
> other drivers).  If the latter, then it's fine as is as other UART drivers
> may be wired to this code as a need arises.
>
>   Maciej
>
Actually that early_uart is not real early one.
it is called via console_initcall.

for IA64, Bjom is calling that directly in setup_arch.

Please check my patch to make it to be called via early_param, that
will be good to other platform.

also you may extend to support other early_uart.

YH

[-- Attachment #2: early_uart_early_param.patch --]
[-- Type: text/x-patch, Size: 5578 bytes --]

[PATCH] serial: make early_uart to use early_prarm instead of console_initcall

Make early_uart to use early_param, so uart console can be used earlier.
Make it to be bootconsole with CON_BOOT flag, so can use console handover
feature.
new command line will be
earlycon=uart,io,0x3f8,9600n8 console=ttyS0,9600
it will print in very early stage
Early serial console at I/O port 0x3f8 (options '9600n8')
later for console it will print
console handover: boot [uart0] -> real [ttyS0]

Signed-off-by: <yinghai.lu@sun.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 48e259a..2a76618 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2533,39 +2538,6 @@ static int __init serial8250_console_init(void)
 }
 console_initcall(serial8250_console_init);
 
-static int __init find_port(struct uart_port *p)
-{
-	int line;
-	struct uart_port *port;
-
-	for (line = 0; line < nr_uarts; line++) {
-		port = &serial8250_ports[line].port;
-		if (uart_match_port(p, port))
-			return line;
-	}
-	return -ENODEV;
-}
-
-int __init serial8250_start_console(struct uart_port *port, char *options)
-{
-	int line;
-
-	line = find_port(port);
-	if (line < 0)
-		return -ENODEV;
-
-	add_preferred_console("ttyS", line, options);
-	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
-		    (unsigned long) port->iobase, options);
-	if (!(serial8250_console.flags & CON_ENABLED)) {
-		serial8250_console.flags &= ~CON_PRINTBUFFER;
-		register_console(&serial8250_console);
-	}
-	return line;
-}
-
 #define SERIAL8250_CONSOLE	&serial8250_console
 #else
 #define SERIAL8250_CONSOLE	NULL
diff --git a/drivers/serial/8250_early.c b/drivers/serial/8250_early.c
index 7e51119..f320f8f 100644
--- a/drivers/serial/8250_early.c
+++ b/drivers/serial/8250_early.c
@@ -17,13 +17,9 @@
  * we locate the device directly by its MMIO or I/O port address.
  *
  * The user can specify the device directly, e.g.,
- *	console=uart,io,0x3f8,9600n8
- *	console=uart,mmio,0xff5e0000,115200n8
- * or platform code can call early_uart_console_init() to set
- * the early UART device.
+ *	earlycon=uart,io,0x3f8,9600n8
+ *	earlycon=uart,mmio,0xff5e0000,115200n8
  *
- * After the normal serial driver starts, we try to locate the
- * matching ttyS device and start a console there.
  */
 
 #include <linux/tty.h>
@@ -42,7 +38,6 @@ struct early_uart_device {
 };
 
 static struct early_uart_device early_device __initdata;
-static int early_uart_registered __initdata;
 
 static unsigned int __init serial_in(struct uart_port *port, int offset)
 {
@@ -175,6 +170,13 @@ static int __init parse_options(struct early_uart_device *device, char *options)
 	return 0;
 }
 
+static struct console early_uart_console __initdata = {
+	.name	= "uart",
+	.write	= early_uart_write,
+	.flags	= CON_PRINTBUFFER,
+	.index	= -1,
+};
+
 static int __init early_uart_setup(struct console *console, char *options)
 {
 	struct early_uart_device *device = &early_device;
@@ -190,61 +192,41 @@ static int __init early_uart_setup(struct console *console, char *options)
 	return 0;
 }
 
-static struct console early_uart_console __initdata = {
-	.name	= "uart",
-	.write	= early_uart_write,
-	.setup	= early_uart_setup,
-	.flags	= CON_PRINTBUFFER,
-	.index	= -1,
-};
-
-static int __init early_uart_console_init(void)
-{
-	if (!early_uart_registered) {
-		register_console(&early_uart_console);
-		early_uart_registered = 1;
-	}
-	return 0;
-}
-console_initcall(early_uart_console_init);
-
-int __init early_serial_console_init(char *cmdline)
+static int __init setup_early_serial_console(char *cmdline)
 {
 	char *options;
 	int err;
 
-	options = strstr(cmdline, "console=uart,");
+	options = strstr(cmdline, "uart,");
 	if (!options)
 		return -ENODEV;
 
 	options = strchr(cmdline, ',') + 1;
 	if ((err = early_uart_setup(NULL, options)) < 0)
 		return err;
-	return early_uart_console_init();
+
+	register_console(&early_uart_console);
+	early_uart_console.flags |= CON_BOOT;
+	return 0;
 }
 
-static int __init early_uart_console_switch(void)
+early_param("earlycon", setup_early_serial_console);
+
+static int __init early_uart_console_post(void)
 {
 	struct early_uart_device *device = &early_device;
 	struct uart_port *port = &device->port;
-	int mmio, line;
+	int mmio;
 
 	if (!(early_uart_console.flags & CON_ENABLED))
 		return 0;
 
-	/* Try to start the normal driver on a matching line.  */
 	mmio = (port->iotype == UPIO_MEM);
-	line = serial8250_start_console(port, device->options);
-	if (line < 0)
-		printk("No ttyS device at %s 0x%lx for console\n",
-			mmio ? "MMIO" : "I/O port",
-			mmio ? port->mapbase :
-			    (unsigned long) port->iobase);
-
-	unregister_console(&early_uart_console);
+
 	if (mmio)
 		iounmap(port->membase);
 
 	return 0;
 }
-late_initcall(early_uart_console_switch);
+
+late_initcall(early_uart_console_post);
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 33fc8cb..deb7143 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -177,11 +177,5 @@ struct serial_icounter_struct {
 #ifdef __KERNEL__
 #include <linux/compiler.h>
 
-/* Allow architectures to override entries in serial8250_ports[] at run time: */
-struct uart_port;	/* forward declaration */
-extern int early_serial_setup(struct uart_port *port);
-extern int early_serial_console_init(char *options);
-extern int serial8250_start_console(struct uart_port *port, char *options);
-
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SERIAL_H */

Re: [patch] early printk and boot console fixups.

[Bjorn Helgaas]

On Thursday 17 May 2007 03:45:36 am Maciej W. Rozycki wrote:
> On Wed, 16 May 2007, Bjorn Helgaas wrote:
> > >  Given the generic name of "uart" I am assuming this will work with any 
> > > UART driver making use of the serial_core.c core -- am I correct?
> > 
> > I knew somebody would ask that eventually :-)
> > 
> > Unfortunately, the answer is "no."  "console=uart" only works with
> > 8250-compatible devices.
> 
>  But is it a design limitation or is it just that other UART drivers have 
> to be modified to work with this option?  My point is if the former, then 
> the option should be something like "console=8250" (the right-hand side 
> being the name of the driver involved; others may want to add support for 
> other drivers).  If the latter, then it's fine as is as other UART drivers 
> may be wired to this code as a need arises.

"console=8250" probably would have been a better choice.  I think
this could be changed without too much hassle.

It might be possible to extend "console=uart" so one could do
"console=uart,io,0x3f8,9600n8,21285" (where "21285" is a non-8250
UART type).  But that seems ugly to me.  I'd rather have
"console=21285", "console=68328", etc.

I just want the "console=<tag>" to explicitly identify the UART
type so we don't have to poke at the thing and guess what it is.

Bjorn