Re: [PATCH v4] earlycon: 8250: Fix command line regression
From: Yinghai Lu
Date: Sun Apr 05 2015 - 03:10:00 EST
On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
...
> Documentation/kernel-parameters.txt | 18 +++++++++++++++---
> drivers/tty/serial/8250/8250_core.c | 37 +++++++++++++++++++++++++++++++++---
> drivers/tty/serial/8250/8250_early.c | 19 ------------------
> 3 files changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index bfcb1a6..1facf0b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> + uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
put this to other patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address,
> - switching to the matching ttyS device later. The
> - options are the same as for ttyS, above.
> + switching to the matching ttyS device later.
> + MMIO inter-register address stride is either 8-bit
> + (mmio) or 32-bit (mmio32).
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for ttyS above; if unspecified,
> + the h/w is not re-initialized.
> +
> hvc<n> Use the hypervisor console device <n>. This is for
> both Xen and PowerPC hypervisors.
>
> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
and this to another patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> MMIO inter-register address stride is either 8-bit
> (mmio) or 32-bit (mmio32).
> - The options are the same as for ttyS, above.
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for "console=ttyS<n>"; if
> + unspecified, the h/w is not initialized.
>
> pl011,<addr>
> Start an early, polled-mode console on a pl011 serial
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e0fb5f0..456606f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3447,6 +3447,21 @@ static int univ8250_console_setup(struct console *co, char *options)
> return serial8250_console_setup(up, options);
> }
>
> +static unsigned int probe_baud(struct uart_port *port)
> +{
> + unsigned char lcr, dll, dlm;
> + unsigned int quot;
> +
> + lcr = serial_port_in(port, UART_LCR);
> + serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> + dll = serial_port_in(port, UART_DLL);
> + dlm = serial_port_in(port, UART_DLM);
> + serial_port_out(port, UART_LCR, lcr);
> +
> + quot = (dlm << 8) | dll;
> + return (port->uartclk / 16) / quot;
> +}
> +
You said some "newer" chips do not support probe baud. why do you move
code to core ?
I was thinking some embedded guys could comment out 8280_early.c, now you get
extra work for them to comment out code from 8250_core.c.
> /**
> * univ8250_console_match - non-standard console matching
> * @co: registering console
> @@ -3455,13 +3470,18 @@ static int univ8250_console_setup(struct console *co, char *options)
> * @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
> + * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
> + * console=uart[8250],0x<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)
> *
> + * ** HACK ALERT **
That is not HACK original.
but your fix make it to be hack.
> + * If no <options> are specified, then assume the h/w is already setup.
> + * This was the undocumented behavior of the original implementation so
> + * it is cast in stone forever.
> + *
> * Returns 0 if console matches; otherwise non-zero to use default matching
> */
> static int univ8250_console_match(struct console *co, char *name, int idx,
> @@ -3475,12 +3495,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (strncmp(name, match, 4) != 0)
> return -ENODEV;
>
> + if (idx && idx != 8250)
> + return -ENODEV;
> +
Your fix is getting ugly now.
> 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;
> + int baud, bits = 8, parity = 'n', flow = 'n';
>
> if (port->iotype != iotype)
> continue;
> @@ -3490,8 +3514,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (iotype == UPIO_PORT && port->iobase != addr)
> continue;
>
> + /* link port to console */
> co->index = i;
> - return univ8250_console_setup(co, options);
> + port->cons = co;
> +
> + if (options && *options)
> + uart_parse_options(options, &baud, &parity, &bits, &flow);
> + else
> + baud = probe_baud(port);
> + return uart_set_options(port, port->cons, baud, parity, bits, flow);
what a mess.
Now sure if it is safe to move down probe_baud, when serial port is still used.
> }
>
> return -ENODEV;
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index e95ebfe..8e11968 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
> serial8250_early_out(port, UART_IER, ier);
> }
>
> -static unsigned int __init probe_baud(struct uart_port *port)
> -{
> - unsigned char lcr, dll, dlm;
> - unsigned int quot;
> -
> - lcr = serial8250_early_in(port, UART_LCR);
> - serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> - dll = serial8250_early_in(port, UART_DLL);
> - dlm = serial8250_early_in(port, UART_DLM);
> - serial8250_early_out(port, UART_LCR, lcr);
> -
> - quot = (dlm << 8) | dll;
> - return (port->uartclk / 16) / quot;
> -}
> -
> static void __init init_port(struct earlycon_device *device)
> {
> struct uart_port *port = &device->port;
> @@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
> struct uart_port *port = &device->port;
> unsigned int ier;
>
> - device->baud = probe_baud(&device->port);
> - snprintf(device->options, sizeof(device->options), "%u",
> - device->baud);
> -
> /* assume the device was initialized, only mask interrupts */
> ier = serial8250_early_in(port, UART_IER);
> serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
> --
Greg, Please consider apply my fix as attached, it is much cleaner.
Or just drop commit c7cef0a84912 ("console: Add extensible console matching")
from the tty-next now.
Thanks
Yinghai
Subject: [PATCH -v3] earlycon: Fix earlycon/console handover without options
commit c7cef0a84912 ("console: Add extensible console matching")
broke the earlycon/handover when booting
console=uart8250,io,0x3f8
the bootloader is using 115200, and the earlycon continue
use 115200, but console revert back to 9600.
Before the commit, probed baud rate is passed via console_cmdline
from earlycon to normal console.
That commit remove that and only check boot command line.
This patch use port match to get hold earlycon, and use earlycon
device options to update options for console.
With that we restore the original behavior.
Fixes: commit c7cef0a84912 ("console: Add extensible console matching")
Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
---
-v2: simplify serial8250_get_earlycon_options and don't update
console_cmdline.
-v3: add earlycon_match to restore original behavior.
---
---
drivers/tty/serial/8250/8250_core.c | 3 +++
drivers/tty/serial/8250/8250_early.c | 7 +++++++
drivers/tty/serial/earlycon.c | 15 +++++++++++++++
include/linux/serial_core.h | 2 ++
4 files changed, 27 insertions(+)
Index: linux-2.6/drivers/tty/serial/8250/8250_core.c
===================================================================
--- linux-2.6.orig/drivers/tty/serial/8250/8250_core.c
+++ linux-2.6/drivers/tty/serial/8250/8250_core.c
@@ -3490,6 +3490,9 @@ static int univ8250_console_match(struct
if (iotype == UPIO_PORT && port->iobase != addr)
continue;
+ if (!earlycon_match(port, &options))
+ return -ENODEV;
+
co->index = i;
return univ8250_console_setup(co, options);
}
Index: linux-2.6/drivers/tty/serial/8250/8250_early.c
===================================================================
--- linux-2.6.orig/drivers/tty/serial/8250/8250_early.c
+++ linux-2.6/drivers/tty/serial/8250/8250_early.c
@@ -105,6 +105,12 @@ static void __init early_serial8250_writ
serial8250_early_out(port, UART_IER, ier);
}
+static int serial8250_earlycon_match(struct earlycon_device *device,
+ struct uart_port *up)
+{
+ return uart_match_port(up, &device->port);
+}
+
static unsigned int __init probe_baud(struct uart_port *port)
{
unsigned char lcr, dll, dlm;
@@ -161,6 +167,7 @@ static int __init early_serial8250_setup
} else
init_port(device);
+ device->match = serial8250_earlycon_match;
device->con->write = early_serial8250_write;
return 0;
}
Index: linux-2.6/drivers/tty/serial/earlycon.c
===================================================================
--- linux-2.6.orig/drivers/tty/serial/earlycon.c
+++ linux-2.6/drivers/tty/serial/earlycon.c
@@ -127,6 +127,21 @@ static int __init register_earlycon(char
return 0;
}
+int earlycon_match(struct uart_port *up, char **options_p)
+{
+ struct earlycon_device *device = &early_console_dev;
+
+ if (!device->con || !(device->con->flags & CON_ENABLED))
+ return 0;
+
+ if (device->match && device->match(device, up)) {
+ *options_p = device->options;
+ return 1;
+ }
+
+ return 0;
+}
+
/**
* setup_earlycon - match and register earlycon console
* @buf: earlycon param string
Index: linux-2.6/include/linux/serial_core.h
===================================================================
--- linux-2.6.orig/include/linux/serial_core.h
+++ linux-2.6/include/linux/serial_core.h
@@ -337,6 +337,7 @@ struct earlycon_device {
struct uart_port port;
char options[16]; /* e.g., 115200n8 */
unsigned int baud;
+ int (*match)(struct earlycon_device *, struct uart_port *);
};
struct earlycon_id {
@@ -344,6 +345,7 @@ struct earlycon_id {
int (*setup)(struct earlycon_device *, const char *options);
};
+extern int earlycon_match(struct uart_port *up, char **options_p);
extern int setup_earlycon(char *buf);
extern int of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *));