Re: [PATCH 4/5] serial: convert early_uart to earlycon for 8250

From: Yinghai Lu
Date: Wed May 30 2007 - 15:59:32 EST


Andrew Morton wrote:
On Tue, 29 May 2007 18:43:59 -0700
Yinghai Lu <Yinghai.Lu@xxxxxxx> wrote:

[PATCH 4/5] serial: convert early_uart to earlycon for 8250

Beacuse SERIAL_PORT_DFNS is removed from include/asm-i386/serial.h and
include/asm-x86_64/serial.h. the serial8250_ports need to be probed late
in serial initializing stage. the console_init=>serial8250_console_init=>
register_console=>serial8250_console_setup will return -ENDEV, and console
ttyS0 can not be enabled at that time.
need to wait till uart_add_one_port in drivers/serial/serial_core.c to call
register_console to get console ttyS0. that is too late.

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. and it will switch to corresponding normal serial console
automatically.

new command line will be:
console=uart8250,io,0x3f8,9600n8
console=uart8250,mmio,0xff5e0000,115200n8
or
earlycon=uart8250,io,0x3f8,9600n8
earlycon=uart8250,mmio,0xff5e0000,115200n8

it will print in very early stage:
Early serial console at I/O port 0x3f8 (options '9600n8')
console [uart0] enabled
later for console it will print:
console handover: boot [uart0] -> real [ttyS0]


Documentation/kernel-parameters.txt | 11 +++
arch/ia64/kernel/setup.c | 4 -
drivers/serial/8250.c | 28 ++-------
drivers/serial/8250_early.c | 104 ++++++++++++++----------------------
drivers/serial/Kconfig | 10 +++
include/asm-i386/fixmap.h | 2 include/asm-i386/io.h | 13 ++++
include/asm-ia64/io.h | 4 +
include/asm-x86_64/fixmap.h | 4 +
include/asm-x86_64/io.h | 13 ++++
include/linux/console.h | 2 include/linux/serial.h | 6 --
include/linux/serial_8250.h | 3 +
init/main.c | 5 +
kernel/printk.c | 22 +++++++
15 files changed, 133 insertions(+), 98 deletions(-)

This is a lot of tricky stuff in areas which I'm not very familar with. Need help, am not getting it.

@@ -143,12 +140,7 @@ static int __init parse_options(struct early_uart_device *device, char *options)
if (!strncmp(options, "mmio,", 5)) {
port->iotype = UPIO_MEM;
port->mapbase = simple_strtoul(options + 5, &options, 0);
- port->membase = ioremap(port->mapbase, mapsize);
- if (!port->membase) {
- printk(KERN_ERR "%s: Couldn't ioremap 0x%lx\n",
- __FUNCTION__, port->mapbase);
- return -ENOMEM;
- }
+ port->membase = fix_ioremap(FIX_EARLYCON_MEM_BASE, port->mapbase);

A number of useful-looking warnigns such as the above got removed. How come?
earycon, when you can not get fix_ioremap work, you can not get anything from boot.

mmio = 1;
} else if (!strncmp(options, "io,", 3)) {
port->iotype = UPIO_PORT;
@@ -175,9 +167,16 @@ static int __init parse_options(struct early_uart_device *device, char *options)
return 0;
}
-static int __init early_uart_setup(struct console *console, char *options)
+static struct console early_serial8250_console __initdata = {
+ .name = "uart",
+ .write = early_serial8250_write,
+ .flags = CON_PRINTBUFFER,

Should this have CON_BOOT set as well?

See
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc7/2.6.21-rc7-mm2/broken-out/fixes-and-cleanups-for-earlyprintk-aka-boot-console.patch,
which is now in mainline.


I put that into setup_early_serial8250_console.

-int __init early_serial_console_init(char *cmdline)
+static int __init setup_early_serial8250_console(char *cmdline)
{
char *options;
int err;

- options = strstr(cmdline, "console=uart,");
+ options = strstr(cmdline, "uart8250,");
if (!options)
- return -ENODEV;
+ return 0;

options = strchr(cmdline, ',') + 1;
- if ((err = early_uart_setup(NULL, options)) < 0)
+ if ((err = early_serial8250_setup(options)) < 0)
return err;
- return early_uart_console_init();
+
+ early_serial8250_console.flags |= CON_BOOT;
+ register_console(&early_serial8250_console);
+
+ return 0;
}

we may use
>> + .flags = CON_PRINTBUFFER | CAN_BOOT;

instead that line.


+ .index = -1,
+};
+
+static int __init early_serial8250_setup(char *options)
{
- struct early_uart_device *device = &early_device;
+ struct early_serial8250_device *device = &early_device;
int err;
if (device->port.membase || device->port.iobase)
@@ -190,61 +189,40 @@ 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,
-};

hm, that didn't have CON_BOOT.

+static inline void __iomem * fix_ioremap (unsigned idx, unsigned long phys)

I don't suppose it'd help much if I were to complain about the coding style
(no space after the * or after the function name, please).

+{
+ void __iomem * vaddr;
+ set_fixmap_nocache(idx, phys & PAGE_MASK);
+ vaddr = (void __iomem *)__fix_to_virt(idx);
+ vaddr += phys & ~PAGE_MASK;
+
+ return vaddr;
+}

Probably this was a bit too large for inlining.

current only one calling, later will use that for usb debug cable.


/* Use early IO mappings for DMI because it's initialized early */
#define dmi_ioremap bt_ioremap
#define dmi_iounmap bt_iounmap
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
index eb17a86..e29eaf8 100644
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -423,6 +423,10 @@ extern void __iomem * ioremap(unsigned long offset, unsigned long size);
extern void __iomem * ioremap_nocache (unsigned long offset, unsigned long size);
extern void iounmap (volatile void __iomem *addr);
+/* for console=uart8250,mmio,0xffe5000,9600n8 */
+#define FIX_EARLYCON_MEM_BASE 1
+#define fix_ioremap(idx, phys) ioremap(phys, 64)
+
/* Use normal IO mappings for DMI */
#define dmi_ioremap ioremap
#define dmi_iounmap(x,l) iounmap(x)

So we touch ia64 as well?

+int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options)
+{
+ struct console_cmdline *c;
+ int i;
+
+ for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0]; i++)
+ if (strcmp(console_cmdline[i].name, name) == 0 &&
+ console_cmdline[i].index == idx) {
+ c = &console_cmdline[i];
+ memcpy(c->name, name_new, sizeof(c->name));
+ c->name[sizeof(c->name) - 1] = 0;
+ c->options = options;
+ c->index = idx_new;
+ return i;
+ }
+ /* not found */
+ return -1;
+}

Shouldn't this be __init?

serial8250_find_port_for_earlycon() had its __init removed. Why?
with __init, the compiler will cry about .init.text and .text missection.

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