On 04/05/2015 03:09 AM, Yinghai Lu wrote: > 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 ? There's no functional difference, but here it's at least possible to add support for exar, synopsys, ti omap, intel, etc. based on either port type or a function vector initialized by the sub-driver. > 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. Huh? That's just ridiculous. >> /** >> * 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. This is required anyway. I'm not the one that decided it would be cute to have "uart" and "uart8250" mean the same thing. >> 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. This was the mess: > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > > -static int serial8250_console_early_setup(void) > -{ > - return serial8250_find_port_for_earlycon(); > -} > > @@ -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, > .flags = CON_PRINTBUFFER | CON_ANYTIME, > .index = -1, > .data = &serial8250_reg, > @@ -3361,19 +3406,6 @@ static int __init serial8250_console_init(void) > > -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; > -} > - > > diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c > > - > -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 > > -extern int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options); > > diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h > > -extern int serial8250_find_port(struct uart_port *p); > -extern int serial8250_find_port_for_earlycon(void); > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > -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; > -} > - > > @@ -2436,9 +2418,6 @@ void register_console(struct console *newcon) > > - if (newcon->early_setup) > - newcon->early_setup(); > - > 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. On what planet is 27 loc across 4 source files cleaner than 6 loc that might be reducible to 2? -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html