by Peter Hurley

[permalink] [raw]

Subject: Re: [PATCH -next 03/13] console: Add extensible console matching

On 02/24/2015 11:37 AM, Peter Hurley wrote:
> Add match() method to struct console which allows the console to
> perform console command line matching instead of (or in addition to)
> default console matching (ie., by fixed name and index).
>
> The match() method returns 0 to indicate a successful match; normal
> console matching occurs if no match() method is defined or the
> match() method returns non-zero. The match() method is expected to set
> the console index if required.
>
> Re-implement earlycon-to-console-handoff with direct matching of
> "console=uart|uart8250,..." to the 8250 ttyS console.
>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> drivers/tty/serial/8250/8250_core.c | 64 +++++++++++++++++++++++++++---------
> drivers/tty/serial/8250/8250_early.c | 23 -------------
> include/linux/console.h | 3 +-
> include/linux/serial_8250.h | 2 --
> kernel/printk/printk.c | 50 ++++++++++------------------
> 5 files changed, 66 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e3b9570a..5eb95fd 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3337,9 +3337,54 @@ static int serial8250_console_setup(struct console *co, char *options)
> return uart_set_options(port, co, baud, parity, bits, flow);
> }
>
> -static int serial8250_console_early_setup(void)
> +/**
> + * serial8250_console_match - non-standard console matching
> + * @co: registering console
> + * @name: name from console command line
> + * @idx: index from console command line
> + * @options: ptr to option string from console command line
> + *
> + * Only attempts to match console command lines of the form:
> + * console=uart<>,io|mmio|mmio32,<addr>,<options>
> + * console=uart<>,<addr>,options
> + * This form is used to register an initial earlycon boot console and
> + * replace it with the serial8250_console at 8250 driver init.
> + *
> + * Performs console setup for a match (as required by interface)
> + *
> + * Returns 0 if console matches; otherwise non-zero to use default matching
> + */
> +static int serial8250_console_match(struct console *co, char *name, int idx,
> + char *options)
> {
> - return serial8250_find_port_for_earlycon();
> + char match[] = "uart"; /* 8250-specific earlycon name */
> + unsigned char iotype;
> + unsigned long addr;
> + int i;
> +
> + if (strncmp(name, match, 4) != 0)
> + return -ENODEV;
> +
> + if (uart_parse_earlycon(options, &iotype, &addr, &options))
> + return -ENODEV;
> +
> + /* try to match the port specified on the command line */
> + for (i = 0; i < nr_uarts; i++) {
> + struct uart_port *port = &serial8250_ports[i].port;
> +
> + if (port->iotype != iotype)
> + continue;
> + if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
> + (port->mapbase != addr))
> + continue;
> + if (iotype == UPIO_PORT && port->iobase != addr)
> + continue;
> +
> + co->index = i;
> + return serial8250_console_setup(co, options);
> + }
> +
> + return -ENODEV;
> }
>
> static struct console serial8250_console = {
> @@ -3347,7 +3392,7 @@ static struct console serial8250_console = {
> .write = serial8250_console_write,
> .device = uart_console_device,
> .setup = serial8250_console_setup,
> - .early_setup = serial8250_console_early_setup,
> + .match = serial8250_console_match,
> .flags = CON_PRINTBUFFER | CON_ANYTIME,
> .index = -1,
> .data = &serial8250_reg,
> @@ -3361,19 +3406,6 @@ static int __init serial8250_console_init(void)
> }
> console_initcall(serial8250_console_init);
>
> -int serial8250_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;
> -}
> -
> #define SERIAL8250_CONSOLE &serial8250_console
> #else
> #define SERIAL8250_CONSOLE NULL
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index c31a22b..49bca65 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -173,26 +173,3 @@ int __init setup_early_serial8250_console(char *cmdline)
>
> return setup_earlycon(cmdline, match, early_serial8250_setup);
> }
> -
> -int serial8250_find_port_for_earlycon(void)
> -{
> - struct earlycon_device *device = early_device;
> - struct uart_port *port = device ? &device->port : NULL;
> - int line;
> - int ret;
> -
> - if (!port || (!port->membase && !port->iobase))
> - return -ENODEV;
> -
> - line = serial8250_find_port(port);
> - if (line < 0)
> - return -ENODEV;
> -
> - ret = update_console_cmdline("uart", 8250,
> - "ttyS", line, device->options);
> - if (ret < 0)
> - ret = update_console_cmdline("uart", 0,
> - "ttyS", line, device->options);
> -
> - return ret;
> -}
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 7571a16..9f50fb4 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -123,7 +123,7 @@ struct console {
> struct tty_driver *(*device)(struct console *, int *);
> void (*unblank)(void);
> int (*setup)(struct console *, char *);
> - int (*early_setup)(void);
> + int (*match)(struct console *, char *name, int idx, char *options);
> short flags;
> short index;
> int cflag;
> @@ -141,7 +141,6 @@ extern int console_set_on_cmdline;
> extern struct console *early_console;
>
> extern int add_preferred_console(char *name, int idx, char *options);
> -extern int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options);
> extern void register_console(struct console *);
> extern int unregister_console(struct console *);
> extern struct console *console_drivers;
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index a8efa23..f26ae7f 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -118,8 +118,6 @@ void serial8250_resume_port(int line);
>
> extern int early_serial_setup(struct uart_port *port);
>
> -extern int serial8250_find_port(struct uart_port *p);
> -extern int serial8250_find_port_for_earlycon(void);
> extern unsigned int serial8250_early_in(struct uart_port *port, int offset);
> extern void serial8250_early_out(struct uart_port *port, int offset, int value);
> extern int setup_early_serial8250_console(char *cmdline);
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index ce5c297..040be39 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2017,24 +2017,6 @@ int add_preferred_console(char *name, int idx, char *options)
> return __add_preferred_console(name, idx, options, NULL);
> }
>
> -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, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++)
> - if (strcmp(c->name, name) == 0 && c->index == idx) {
> - strlcpy(c->name, name_new, sizeof(c->name));
> - c->options = options;
> - c->index = idx_new;
> - return i;
> - }
> - /* not found */
> - return -1;
> -}
> -
> bool console_suspend_enabled = true;
> EXPORT_SYMBOL(console_suspend_enabled);
>
> @@ -2436,9 +2418,6 @@ void register_console(struct console *newcon)
> if (preferred_console < 0 || bcon || !console_drivers)
> preferred_console = selected_console;
>
> - if (newcon->early_setup)
> - newcon->early_setup();
> -
> /*
> * See if we want to use this console driver. If we
> * didn't select a console we take the first one
> @@ -2464,20 +2443,25 @@ void register_console(struct console *newcon)
> for (i = 0, c = console_cmdline;
> i < MAX_CMDLINECONSOLES && c->name[0];
> i++, c++) {
> - if (strcmp(c->name, newcon->name) != 0)
> - continue;

Andrew,

I just discovered that struct console_cmdline::name[] and
struct console::name[] are not the same size, which is a bug that
should probably go into -stable.

I'll respin a v2 of this series on top of that bug fix, which I'll
send shortly.

Regards,
Peter Hurley

> - if (newcon->index >= 0 &&
> - newcon->index != c->index)
> - continue;
> - if (newcon->index < 0)
> - newcon->index = c->index;
> + if (!newcon->match ||
> + newcon->match(newcon, c->name, c->index, c->options) != 0) {
> + /* default matching */
> + if (strcmp(c->name, newcon->name) != 0)
> + continue;
> + if (newcon->index >= 0 &&
> + newcon->index != c->index)
> + continue;
> + if (newcon->index < 0)
> + newcon->index = c->index;
>
> - if (_braille_register_console(newcon, c))
> - return;
> + if (_braille_register_console(newcon, c))
> + return;
> +
> + if (newcon->setup &&
> + newcon->setup(newcon, c->options) != 0)
> + break;
> + }
>
> - if (newcon->setup &&
> - newcon->setup(newcon, console_cmdline[i].options) != 0)
> - break;
> newcon->flags |= CON_ENABLED;
> if (i == selected_console) {
> newcon->flags |= CON_CONSDEV;
>