[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20070530115717.77e05153.akpm@linux-foundation.org>
Date: Wed, 30 May 2007 11:57:17 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Yinghai Lu <Yinghai.Lu@....COM>
Cc: Andi Kleen <ak@...e.de>, Russell King <rmk@....linux.org.uk>,
Bjorn Helgaas <bjorn.helgaas@...com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Gerd Hoffmann <kraxel@...e.de>,
David Gibson <david@...son.dropbear.id.au>
Subject: Re: [PATCH 4/5] serial: convert early_uart to earlycon for 8250
On Tue, 29 May 2007 18:43:59 -0700
Yinghai Lu <Yinghai.Lu@....COM> 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?
> 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.
> + .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.
> /* 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?
This patch conflicts with David's
serial-use-resource_size_t-for-port-io-addresses.patch in minor ways, but
there may be functional conflicts as well. David's patch seems to be a bit
stuck, so I'll drop it and I'll merge these instead. But I don't know what
I'm doing.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists