[PATCH v3 -next 00/11] Extensible console matching & direct earlycon

[PATCH v3 -next 00/11] Extensible console matching & direct earlycon

[Peter Hurley]

Changes from v2:
* remainder of unapplied series
* Changed the title and commit log for
  "serial: earlycon: Allow earlycon params with name only" to
  "serial: earlycon: Skip parse_options() for empty string" per Rob's
  comment.

Changes from v1:
* rebased on and requires "console: Fix console name size mismatch"


Hi Greg & Andrew,

This patch series implements:
  1. console-definable (aka extensible) matching
  2. generic earlycon-to-console handoff via extensible matching
  3. arch/prom support for direct earlycon


Extensible console matching

Extensible console matching enables the console itself to define the
conditions for a "command line" match. This mimics the design of
device matching in the driver model. Two important use-cases which this
feature enables are generic earlycon-to-console handoff and support
for driver migration.

Earlycon-to-console handoff was implemented in 2007. Console command
lines of the form:
   console=uart,io,0x3f8,115200n8
start an earlycon and later allow the 8250 driver console to takeover.
Unfortunately this implementation requires direct coupling between
the earlycon and the console and is facilitated by ugly hacks like
editing the console command line in-place.

Extensible console matching allows the 8250 driver to directly match
that console command line instead, and enables other serial drivers
to trivially support console handoff themselves.

In addition, extensible console matching allows a new driver to
provide support for a different driver's console. This requirement
stems from needing to minimize breakage when migrating serial drivers.
Since many devices are based on the original 8250/16550 designs
but sometimes have features incompatible with the existing 8250 driver
support, the initial driver is often standalone. When/if the standalone
driver is migrated to the 8250 driver, the problem of console names in
the command line remains. Extensible console matching enables a simple
migration path.


Direct earlycon

This feature enables arches and proms to start an earlycon directly,
rather than requiring an "earlycon=" command line parameter.
Devicetree can already do this via the 'linux,stdout-path' property,
but arch and prom code requires direct coupling to the serial driver.

This support is implemented by judicious refactoring and the same
construct that devicetree and early_param use: a link table containing
the necessary information (name and setup() function) to find and
bind the appropriate earlycon "driver".


NB: I combined these two features in this series because their
implementations heavily overlap in the same source files.

Regards,

Peter Hurley (11):
  console: Add extensible console matching
  serial: core: Fix kernel doc for uart_console_write()
  serial: 8250_early: Remove early_device variable
  serial: earlycon: Move ->uartclk initialize
  serial: 8250_early: Assume uart already initialized if no baud option
  serial: 8250_early: Fix setup() error code
  serial: earlycon: Ignore parse_options() error code
  serial: earlycon: Skip parse_options() if empty string
  serial: earlycon: Refactor earlycon registration
  serial: earlycon: Enable earlycon without command line param
  serial: 8250_early: Remove setup_early_serial8250_console()

 arch/mips/mti-malta/malta-init.c     |   4 +-
 drivers/firmware/pcdp.c              |   4 +-
 drivers/tty/serial/8250/8250_core.c  |  64 ++++++++++++++++------
 drivers/tty/serial/8250/8250_early.c |  51 ++++--------------
 drivers/tty/serial/earlycon.c        | 101 ++++++++++++++++++++++++++++-------
 drivers/tty/serial/serial_core.c     |   2 +-
 include/asm-generic/vmlinux.lds.h    |   9 ++++
 include/linux/console.h              |   3 +-
 include/linux/serial_8250.h          |   3 --
 include/linux/serial_core.h          |  19 ++++---
 kernel/printk/printk.c               |  52 +++++++-----------
 11 files changed, 185 insertions(+), 127 deletions(-)

-- 
2.3.1

[PATCH v3 -next 01/11] console: Add extensible console matching

[Peter Hurley]

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.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 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               | 52 ++++++++++-------------------
 5 files changed, 67 insertions(+), 77 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 26f8998..dda9592 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,21 +2443,26 @@ void register_console(struct console *newcon)
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
-		BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
-		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 (!newcon->match ||
+		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+			/* default matching */
+			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
+			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;
-- 
2.3.1

[PATCH v3 -next 02/11] serial: core: Fix kernel doc for uart_console_write()

[Peter Hurley]

'/**' is required to start a kernel-doc comment block.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/serial_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3f823c26..373cfde 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1766,7 +1766,7 @@ static const struct file_operations uart_proc_fops = {
 #endif
 
 #if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(CONFIG_CONSOLE_POLL)
-/*
+/**
  *	uart_console_write - write a console message to a serial port
  *	@port: the port to write the message
  *	@s: array of characters
-- 
2.3.1

[PATCH v3 -next 03/11] serial: 8250_early: Remove early_device variable

[Peter Hurley]

early_device was only required for serial8250_find_port_for_earlycon(),
which was replaced by extensible console matching.

Fixup early_serial8250_write() to get the earlycon_device * from
console->data (which is initialized by {of_}setup_earlycon()).

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_early.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 49bca65..d7b831b 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -36,8 +36,6 @@
 #include <asm/io.h>
 #include <asm/serial.h>
 
-static struct earlycon_device *early_device;
-
 unsigned int __weak __init serial8250_early_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
@@ -90,7 +88,8 @@ static void __init serial_putc(struct uart_port *port, int c)
 static void __init early_serial8250_write(struct console *console,
 					const char *s, unsigned int count)
 {
-	struct uart_port *port = &early_device->port;
+	struct earlycon_device *device = console->data;
+	struct uart_port *port = &device->port;
 	unsigned int ier;
 
 	/* Save the IER and disable interrupts preserving the UUE bit */
@@ -157,7 +156,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
 
 	init_port(device);
 
-	early_device = device;
 	device->con->write = early_serial8250_write;
 	return 0;
 }
-- 
2.3.1

[PATCH v3 -next 04/11] serial: earlycon: Move ->uartclk initialize

[Peter Hurley]

Initializing the ->uartclk field is not related to option parsing;
relocate from parse_options() to setup_earlycon() (which mirrors the
behavior of of_setup_earlycon()).

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/earlycon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 58d6bcd..0480c8f 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -76,8 +76,6 @@ static int __init parse_options(struct earlycon_device *device, char *options)
 		return -EINVAL;
 	}
 
-	port->uartclk = BASE_BAUD * 16;
-
 	if (options) {
 		device->baud = simple_strtoul(options, NULL, 0);
 		length = min(strcspn(options, " ") + 1,
@@ -121,6 +119,7 @@ int __init setup_earlycon(char *buf, const char *match,
 	if (!err)
 		buf = NULL;
 
+	port->uartclk = BASE_BAUD * 16;
 	if (port->mapbase)
 		port->membase = earlycon_map(port->mapbase, 64);
 
-- 
2.3.1

[PATCH v3 -next 05/11] serial: 8250_early: Assume uart already initialized if no baud option

[Peter Hurley]

The <baud><parity><bit> option string is not supplied if the earlycon
is started via devicetree and OF_EARLYCON_DECLARE(). The option string
is also not required if started via kernel command line parameters of
the form:
  earlycon=uart,mmio,<addr>
  console=uart,mmio,<addr>

If earlycon_device->baud is 0, then an option string was not supplied.
In this case, assume the uart has already been initialized by the
bootloader or firmware.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_early.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index d7b831b..1701d00 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -149,12 +149,18 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
 		return 0;
 
 	if (!device->baud) {
+		struct uart_port *port = &device->port;
+		unsigned int ier;
+
 		device->baud = probe_baud(&device->port);
 		snprintf(device->options, sizeof(device->options), "%u",
 			 device->baud);
-	}
 
-	init_port(device);
+		/* 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);
+	} else
+		init_port(device);
 
 	device->con->write = early_serial8250_write;
 	return 0;
-- 
2.3.1

[PATCH v3 -next 06/11] serial: 8250_early: Fix setup() error code

[Peter Hurley]

If parsing failed to decode a valid uart addr, return -ENODEV instead
of success. Although setup_earlycon() will detect the failure anyway
(because the write() method has not been set), that behavior is not
obvious and should not be relied on.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_early.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 1701d00..b199c10 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -146,7 +146,7 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
 					 const char *options)
 {
 	if (!(device->port.membase || device->port.iobase))
-		return 0;
+		return -ENODEV;
 
 	if (!device->baud) {
 		struct uart_port *port = &device->port;
-- 
2.3.1

[PATCH v3 -next 07/11] serial: earlycon: Ignore parse_options() error code

[Peter Hurley]

Because setup_earlycon() continues to attempt console registration
if an error occurred parsing the option string, the actual value of
the error code from parse_options() is ignored.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/earlycon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 0480c8f..da5e8c8 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -114,9 +114,8 @@ int __init setup_earlycon(char *buf, const char *match,
 
 	buf += len + 1;
 
-	err = parse_options(&early_console_dev, buf);
 	/* On parsing error, pass the options buf to the setup function */
-	if (!err)
+	if (!parse_options(&early_console_dev, buf))
 		buf = NULL;
 
 	port->uartclk = BASE_BAUD * 16;
-- 
2.3.1

[PATCH v3 -next 08/11] serial: earlycon: Skip parse_options() if empty string

[Peter Hurley]

Earlycon param strings of the form
   earlycon=<name>
are rejected from parse_options() with an error (which, in turn,
results in a NULL argument for the setup() method options parameter).

Only pass non-empty string to parse_options(); this will enable
handling actual parse errors differently than expected and allow
formats.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/earlycon.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index da5e8c8..025ea01 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -109,13 +109,16 @@ int __init setup_earlycon(char *buf, const char *match,
 	len = strlen(match);
 	if (strncmp(buf, match, len))
 		return 0;
-	if (buf[len] && (buf[len] != ','))
-		return 0;
 
-	buf += len + 1;
+	if (buf[len]) {
+		if (buf[len] != ',')
+			return 0;
+		buf += len + 1;
+	} else
+		buf = NULL;
 
 	/* On parsing error, pass the options buf to the setup function */
-	if (!parse_options(&early_console_dev, buf))
+	if (buf && !parse_options(&early_console_dev, buf))
 		buf = NULL;
 
 	port->uartclk = BASE_BAUD * 16;
-- 
2.3.1

[PATCH v3 -next 09/11] serial: earlycon: Refactor earlycon registration

[Peter Hurley]

Separate earlycon matching from registration; add register_earlycon
which initializes and registers the matched earlycon.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/earlycon.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 025ea01..9fb76b6 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -96,27 +96,13 @@ static int __init parse_options(struct earlycon_device *device, char *options)
 	return 0;
 }
 
-int __init setup_earlycon(char *buf, const char *match,
-			  int (*setup)(struct earlycon_device *, const char *))
+
+static int __init
+register_earlycon(char *buf, int (*setup)(struct earlycon_device *, const char *))
 {
 	int err;
-	size_t len;
 	struct uart_port *port = &early_console_dev.port;
 
-	if (!buf || !match || !setup)
-		return 0;
-
-	len = strlen(match);
-	if (strncmp(buf, match, len))
-		return 0;
-
-	if (buf[len]) {
-		if (buf[len] != ',')
-			return 0;
-		buf += len + 1;
-	} else
-		buf = NULL;
-
 	/* On parsing error, pass the options buf to the setup function */
 	if (buf && !parse_options(&early_console_dev, buf))
 		buf = NULL;
@@ -136,6 +122,28 @@ int __init setup_earlycon(char *buf, const char *match,
 	return 0;
 }
 
+int __init setup_earlycon(char *buf, const char *match,
+			  int (*setup)(struct earlycon_device *, const char *))
+{
+	size_t len;
+
+	if (!buf || !match || !setup)
+		return 0;
+
+	len = strlen(match);
+	if (strncmp(buf, match, len))
+		return 0;
+
+	if (buf[len]) {
+		if (buf[len] != ',')
+			return 0;
+		buf += len + 1;
+	} else
+		buf = NULL;
+
+	return register_earlycon(buf, setup);
+}
+
 int __init of_setup_earlycon(unsigned long addr,
 			     int (*setup)(struct earlycon_device *, const char *))
 {
-- 
2.3.1

[PATCH v3 -next 10/11] serial: earlycon: Enable earlycon without command line param

[Peter Hurley]

Earlycon matching can only be triggered if 'earlycon=...' has been
specified on the kernel command line. To workaround this limitation
requires tight coupling between arches and specific serial drivers
in order to start an earlycon. Devicetree avoids this limitation
with a link table that contains the required data to match earlycons.

Mirror this approach for earlycon match by name. Re-purpose
EARLYCON_DECLARE to generate a table entry which associates name with
setup() function. Re-purpose setup_earlycon() to scan this table for
an earlycon match, which is registered if found.

Declare one "earlycon" early_param, which calls setup_earlycon().

This design allows setup_earlycon() to be called directly with a
param string (as if 'earlycon=...' had been set on the command line).
Re-registration (either directly or by early_param) is prevented.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_early.c |  7 +--
 drivers/tty/serial/earlycon.c        | 92 ++++++++++++++++++++++++++++--------
 include/asm-generic/vmlinux.lds.h    |  9 ++++
 include/linux/serial_core.h          | 19 ++++----
 4 files changed, 94 insertions(+), 33 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index b199c10..d272139 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -170,10 +170,5 @@ EARLYCON_DECLARE(uart, early_serial8250_setup);
 
 int __init setup_early_serial8250_console(char *cmdline)
 {
-	char match[] = "uart8250";
-
-	if (cmdline && cmdline[4] == ',')
-		match[4] = '\0';
-
-	return setup_earlycon(cmdline, match, early_serial8250_setup);
+	return setup_earlycon(cmdline);
 }
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 9fb76b6..5fdc9f3 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -10,6 +10,9 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
 #include <linux/console.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -34,6 +37,10 @@ static struct earlycon_device early_console_dev = {
 	.con = &early_con,
 };
 
+extern struct earlycon_id __earlycon_table[];
+static const struct earlycon_id __earlycon_table_sentinel
+	__used __section(__earlycon_table_end);
+
 static const struct of_device_id __earlycon_of_table_sentinel
 	__used __section(__earlycon_of_table_end);
 
@@ -96,9 +103,7 @@ static int __init parse_options(struct earlycon_device *device, char *options)
 	return 0;
 }
 
-
-static int __init
-register_earlycon(char *buf, int (*setup)(struct earlycon_device *, const char *))
+static int __init register_earlycon(char *buf, const struct earlycon_id *match)
 {
 	int err;
 	struct uart_port *port = &early_console_dev.port;
@@ -112,7 +117,7 @@ register_earlycon(char *buf, int (*setup)(struct earlycon_device *, const char *
 		port->membase = earlycon_map(port->mapbase, 64);
 
 	early_console_dev.con->data = &early_console_dev;
-	err = setup(&early_console_dev, buf);
+	err = match->setup(&early_console_dev, buf);
 	if (err < 0)
 		return err;
 	if (!early_console_dev.con->write)
@@ -122,27 +127,76 @@ register_earlycon(char *buf, int (*setup)(struct earlycon_device *, const char *
 	return 0;
 }
 
-int __init setup_earlycon(char *buf, const char *match,
-			  int (*setup)(struct earlycon_device *, const char *))
+/**
+ *	setup_earlycon - match and register earlycon console
+ *	@buf:	earlycon param string
+ *
+ *	Registers the earlycon console matching the earlycon specified
+ *	in the param string @buf. Acceptable param strings are of the form
+ *	   <name>,io|mmio|mmio32,<addr>,<options>
+ *	   <name>,0x<addr>,<options>
+ *	   <name>,<options>
+ *	   <name>
+ *
+ *	Only for the third form does the earlycon setup() method receive the
+ *	<options> string in the 'options' parameter; all other forms set
+ *	the parameter to NULL.
+ *
+ *	Returns 0 if an attempt to register the earlycon was made,
+ *	otherwise negative error code
+ */
+int __init setup_earlycon(char *buf)
 {
-	size_t len;
+	const struct earlycon_id *match;
 
-	if (!buf || !match || !setup)
-		return 0;
+	if (!buf || !buf[0])
+		return -EINVAL;
 
-	len = strlen(match);
-	if (strncmp(buf, match, len))
-		return 0;
+	if (early_con.flags & CON_ENABLED)
+		return -EALREADY;
 
-	if (buf[len]) {
-		if (buf[len] != ',')
-			return 0;
-		buf += len + 1;
-	} else
-		buf = NULL;
+	for (match = __earlycon_table; match->name[0]; match++) {
+		size_t len = strlen(match->name);
 
-	return register_earlycon(buf, setup);
+		if (strncmp(buf, match->name, len))
+			continue;
+
+		if (buf[len]) {
+			if (buf[len] != ',')
+				continue;
+			buf += len + 1;
+		} else
+			buf = NULL;
+
+		return register_earlycon(buf, match);
+	}
+
+	return -ENOENT;
+}
+
+/* early_param wrapper for setup_earlycon() */
+static int __init param_setup_earlycon(char *buf)
+{
+	int err;
+
+	/*
+	 * Just 'earlycon' is a valid param for devicetree earlycons;
+	 * don't generate a warning from parse_early_params() in that case
+	 */
+	if (!buf || !buf[0])
+		return 0;
+
+	err = setup_earlycon(buf);
+	if (err == -ENOENT) {
+		pr_warn("no match for %s\n", buf);
+		err = 0;
+	} else if (err == -EALREADY) {
+		pr_warn("already registered\n");
+		err = 0;
+	}
+	return err;
 }
+early_param("earlycon", param_setup_earlycon);
 
 int __init of_setup_earlycon(unsigned long addr,
 			     int (*setup)(struct earlycon_device *, const char *))
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ac78910..87e5b6f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -150,6 +150,14 @@
 #define TRACE_SYSCALLS()
 #endif
 
+#ifdef CONFIG_SERIAL_EARLYCON
+#define EARLYCON_TABLE() . = ALIGN(8);				\
+			 VMLINUX_SYMBOL(__earlycon_table) = .;	\
+			 *(__earlycon_table)			\
+			 *(__earlycon_table_end)
+#else
+#define EARLYCON_TABLE()
+#endif
 
 #define ___OF_TABLE(cfg, name)	_OF_TABLE_##cfg(name)
 #define __OF_TABLE(cfg, name)	___OF_TABLE(cfg, name)
@@ -503,6 +511,7 @@
 	CPU_METHOD_OF_TABLES()						\
 	KERNEL_DTB()							\
 	IRQCHIP_OF_MATCH_TABLE()					\
+	EARLYCON_TABLE()						\
 	EARLYCON_OF_TABLES()
 
 #define INIT_TEXT							\
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cc5c506..a3fd182 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -336,18 +336,21 @@ struct earlycon_device {
 	char options[16];		/* e.g., 115200n8 */
 	unsigned int baud;
 };
-int setup_earlycon(char *buf, const char *match,
-		   int (*setup)(struct earlycon_device *, const char *));
 
+struct earlycon_id {
+	char	name[16];
+	int	(*setup)(struct earlycon_device *, const char *options);
+};
+
+extern int setup_earlycon(char *buf);
 extern int of_setup_earlycon(unsigned long addr,
 			     int (*setup)(struct earlycon_device *, const char *));
 
-#define EARLYCON_DECLARE(name, func) \
-static int __init name ## _setup_earlycon(char *buf) \
-{ \
-	return setup_earlycon(buf, __stringify(name), func); \
-} \
-early_param("earlycon", name ## _setup_earlycon);
+#define EARLYCON_DECLARE(_name, func)					\
+	static const struct earlycon_id __earlycon_##_name		\
+		__used __section(__earlycon_table)			\
+		 = { .name  = __stringify(_name),			\
+		     .setup = func  }
 
 #define OF_EARLYCON_DECLARE(name, compat, fn)				\
 	_OF_DECLARE(earlycon, name, compat, fn, void *)
-- 
2.3.1

[PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

setup_earlycon() will now match and register the desired earlycon
from the param string (as if 'earlycon=...' had been set on the
command line). Use setup_earlycon() from existing arch call sites
which start an earlycon directly.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 arch/mips/mti-malta/malta-init.c     | 4 ++--
 drivers/firmware/pcdp.c              | 4 ++--
 drivers/tty/serial/8250/8250_early.c | 5 -----
 include/linux/serial_8250.h          | 1 -
 4 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
index 6849f53..cec3e18 100644
--- a/arch/mips/mti-malta/malta-init.c
+++ b/arch/mips/mti-malta/malta-init.c
@@ -14,7 +14,7 @@
 #include <linux/init.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
-#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
 
 #include <asm/cacheflush.h>
 #include <asm/smp-ops.h>
@@ -75,7 +75,7 @@ static void __init console_config(void)
 	if ((strstr(fw_getcmdline(), "earlycon=")) == NULL) {
 		sprintf(console_string, "uart8250,io,0x3f8,%d%c%c", baud,
 			parity, bits);
-		setup_early_serial8250_console(console_string);
+		setup_earlycon(console_string);
 	}
 
 	if ((strstr(fw_getcmdline(), "console=")) == NULL) {
diff --git a/drivers/firmware/pcdp.c b/drivers/firmware/pcdp.c
index a330492..75273a25 100644
--- a/drivers/firmware/pcdp.c
+++ b/drivers/firmware/pcdp.c
@@ -15,7 +15,7 @@
 #include <linux/console.h>
 #include <linux/efi.h>
 #include <linux/serial.h>
-#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
 #include <asm/vga.h>
 #include "pcdp.h"
 
@@ -43,7 +43,7 @@ setup_serial_console(struct pcdp_uart *uart)
 	}
 
 	add_preferred_console("uart", 8250, &options[9]);
-	return setup_early_serial8250_console(options);
+	return setup_earlycon(options);
 #else
 	return -ENODEV;
 #endif
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index d272139..a2b4e58 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -167,8 +167,3 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
 }
 EARLYCON_DECLARE(uart8250, early_serial8250_setup);
 EARLYCON_DECLARE(uart, early_serial8250_setup);
-
-int __init setup_early_serial8250_console(char *cmdline)
-{
-	return setup_earlycon(cmdline);
-}
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index f26ae7f..ca9f87b 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -120,7 +120,6 @@ extern int early_serial_setup(struct uart_port *port);
 
 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);
 extern void serial8250_do_set_termios(struct uart_port *port,
 		struct ktermios *termios, struct ktermios *old);
 extern int serial8250_do_startup(struct uart_port *port);
-- 
2.3.1

Re: [PATCH v3 -next 00/11] Extensible console matching & direct earlycon

[Greg Kroah-Hartman]

On Mon, Mar 09, 2015 at 04:27:11PM -0400, Peter Hurley wrote:
> Changes from v2:
> * remainder of unapplied series
> * Changed the title and commit log for
>   "serial: earlycon: Allow earlycon params with name only" to
>   "serial: earlycon: Skip parse_options() for empty string" per Rob's
>   comment.
> 
> Changes from v1:
> * rebased on and requires "console: Fix console name size mismatch"
> 
> 
> Hi Greg & Andrew,
> 
> This patch series implements:
>   1. console-definable (aka extensible) matching
>   2. generic earlycon-to-console handoff via extensible matching
>   3. arch/prom support for direct earlycon
> 
> 
> Extensible console matching
> 
> Extensible console matching enables the console itself to define the
> conditions for a "command line" match. This mimics the design of
> device matching in the driver model. Two important use-cases which this
> feature enables are generic earlycon-to-console handoff and support
> for driver migration.
> 
> Earlycon-to-console handoff was implemented in 2007. Console command
> lines of the form:
>    console=uart,io,0x3f8,115200n8
> start an earlycon and later allow the 8250 driver console to takeover.
> Unfortunately this implementation requires direct coupling between
> the earlycon and the console and is facilitated by ugly hacks like
> editing the console command line in-place.
> 
> Extensible console matching allows the 8250 driver to directly match
> that console command line instead, and enables other serial drivers
> to trivially support console handoff themselves.
> 
> In addition, extensible console matching allows a new driver to
> provide support for a different driver's console. This requirement
> stems from needing to minimize breakage when migrating serial drivers.
> Since many devices are based on the original 8250/16550 designs
> but sometimes have features incompatible with the existing 8250 driver
> support, the initial driver is often standalone. When/if the standalone
> driver is migrated to the 8250 driver, the problem of console names in
> the command line remains. Extensible console matching enables a simple
> migration path.
> 
> 
> Direct earlycon
> 
> This feature enables arches and proms to start an earlycon directly,
> rather than requiring an "earlycon=" command line parameter.
> Devicetree can already do this via the 'linux,stdout-path' property,
> but arch and prom code requires direct coupling to the serial driver.
> 
> This support is implemented by judicious refactoring and the same
> construct that devicetree and early_param use: a link table containing
> the necessary information (name and setup() function) to find and
> bind the appropriate earlycon "driver".
> 
> 
> NB: I combined these two features in this series because their
> implementations heavily overlap in the same source files.

All now queued up, thanks.

greg k-h

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Mon, Mar 9, 2015 at 1:27 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> setup_earlycon() will now match and register the desired earlycon
> from the param string (as if 'earlycon=...' had been set on the
> command line). Use setup_earlycon() from existing arch call sites
> which start an earlycon directly.
>

Hi,

Looks like this patcheset cause regression:
when set grub console to 115200, and later kernel only have

console=uart8250,io,0x3f8

the kernel will revert baud rate to 9600 instead of keeping 115200.

in setup_earlycon: you say:

 *      Registers the earlycon console matching the earlycon specified
 *      in the param string @buf. Acceptable param strings are of the form
 *         <name>,io|mmio|mmio32,<addr>,<options>
 *         <name>,0x<addr>,<options>
 *         <name>,<options>
 *         <name>
 *
 *      Only for the third form does the earlycon setup() method receive the
 *      <options> string in the 'options' parameter; all other forms set
 *      the parameter to NULL.


so that change the old behavior that we defined in
Documentation/kernel-parameters.txt

                uart[8250],io,<addr>[,options]
                uart[8250],mmio,<addr>[,options]
                uart[8250],mmio32,<addr>[,options]
                        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.

The old behavior: options is optional , and will use baud rate that is
set by bootloader.

Please fix the problem and restore to old behavior.

Thanks

Yinghai

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

Hi Yinghai,

On 04/01/2015 10:04 PM, Yinghai Lu wrote:
> On Mon, Mar 9, 2015 at 1:27 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> setup_earlycon() will now match and register the desired earlycon
>> from the param string (as if 'earlycon=...' had been set on the
>> command line). Use setup_earlycon() from existing arch call sites
>> which start an earlycon directly.
>>
> 
> Hi,
> 
> Looks like this patcheset cause regression:
> when set grub console to 115200, and later kernel only have
> 
> console=uart8250,io,0x3f8
> 
> the kernel will revert baud rate to 9600 instead of keeping 115200.
> 
> in setup_earlycon: you say:
> 
>  *      Registers the earlycon console matching the earlycon specified
>  *      in the param string @buf. Acceptable param strings are of the form
>  *         <name>,io|mmio|mmio32,<addr>,<options>
>  *         <name>,0x<addr>,<options>
>  *         <name>,<options>
>  *         <name>
>  *
>  *      Only for the third form does the earlycon setup() method receive the
>  *      <options> string in the 'options' parameter; all other forms set
>  *      the parameter to NULL.
> 
> 
> so that change the old behavior that we defined in
> Documentation/kernel-parameters.txt
> 
>                 uart[8250],io,<addr>[,options]
>                 uart[8250],mmio,<addr>[,options]
>                 uart[8250],mmio32,<addr>[,options]
>                         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.
                                      ^^^^^^^^^^^^
The documented behavior of console=ttyS options, to which your
quote refers, clearly states:

		Default is "9600n8".

> The old behavior: options is optional , and will use baud rate that is
> set by bootloader.

so the previous behavior was actually at odds with the documentation.

> Please fix the problem and restore to old behavior.

Is this really necessary (or even desirable)?
I think it's a bad idea to have one console type (ttyS) initialize its
options to default settings, but yet allow another console type (uart) to
probe the existing state.

Also, this expectation is an impediment when adding support for other
8250-like designs that don't have the same 8250 divisor registers
(ie., _every_ new design). To properly support this requirement for
just the existing 8250 hardware will require special probe_baud()
functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
omap 1510.

Is specifying the line speed on the command line really a burden?

Regards,
Peter Hurley

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On Wed, Apr 1, 2015 at 8:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> The documented behavior of console=ttyS options, to which your
> quote refers, clearly states:
>
>                 Default is "9600n8".

drivers/tty/serial/8250/8250_early.c:early_serial8250_setup
still have calling to probe_baud, but it is not triggered.

Here is root cause.
The gap between entries in earlycon_table cause
iteration fail to find next entry, so uart8250 handler is
not called proplerly.

attached patch fix the problem.

Thanks

Yinghai

[-- Attachment #2: fix_earlycon_alignment.patch --]
[-- Type: text/x-patch, Size: 4743 bytes --]

Subject: [PATCH] serial: Fix earlycon section iteration

Found when booting with console=uart8250,io,0x3f8
the kernel will revert baud rate to 9600 instead of keeping 115200.

root causes: The gap between entries in earlycon_table cause
iteration fail to find next entry, so uart8250 handler is
not called proplerly.

commit d2fd6810a823 ("tty/serial: convert 8250 to generic earlycon")
add two entries into earlycon_table.
 EARLYCON_DECLARE(uart8250, early_serial8250_setup);
 EARLYCON_DECLARE(uart, early_serial8250_setup);

and according to System.map, earlycon_uart is before earlycon_uart8250.
  ffffffff832049a0 t __earlycon_uart
  ffffffff832049c0 t __earlycon_uart8250
And offset between two entries is 0x20.

commit 470ca0de69fe ("serial: earlycon: Enable earlycon without command line param")
setup_earlycon() will loop __earlycon_table with pointer to earlycon_id,
	for (match = __earlycon_table; match->name[0]; match++) {
               size_t len = strlen(match->name);

               if (strncmp(buf, match->name, len))
                       continue;

but match size (size of struct earlycon_id) is only 0x18.  so will point to
wrong place for next match and will get random result.

Make the earlycon_table section have pointer to earlycon_id struct instead.

Fixes: commit 470ca0de69fe ("serial: earlycon: Enable earlycon without command line param")
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/tty/serial/earlycon.c     |   13 +++++++------
 include/asm-generic/vmlinux.lds.h |    8 ++++----
 include/linux/serial_core.h       |    8 +++++---
 3 files changed, 16 insertions(+), 13 deletions(-)

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
@@ -349,10 +349,12 @@ extern int of_setup_earlycon(unsigned lo
 			     int (*setup)(struct earlycon_device *, const char *));
 
 #define EARLYCON_DECLARE(_name, func)					\
-	static const struct earlycon_id __earlycon_##_name		\
-		__used __section(__earlycon_table)			\
+	static struct earlycon_id __earlycon_##_name __initdata	\
 		 = { .name  = __stringify(_name),			\
-		     .setup = func  }
+		     .setup = func  };					\
+	static struct earlycon_id *__p_earlycon_##_name	__used		\
+		__aligned(sizeof(struct earlycon_id *))			\
+		__section(__earlycon_table) = { &__earlycon_##_name }
 
 #define OF_EARLYCON_DECLARE(name, compat, fn)				\
 	_OF_DECLARE(earlycon, name, compat, fn, void *)
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
@@ -37,9 +37,8 @@ static struct earlycon_device early_cons
 	.con = &early_con,
 };
 
-extern struct earlycon_id __earlycon_table[];
-static const struct earlycon_id __earlycon_table_sentinel
-	__used __section(__earlycon_table_end);
+extern struct earlycon_id *__start_earlycon_table[];
+extern struct earlycon_id *__stop_earlycon_table[];
 
 static const struct of_device_id __earlycon_of_table_sentinel
 	__used __section(__earlycon_of_table_end);
@@ -103,7 +102,7 @@ static int __init parse_options(struct e
 	return 0;
 }
 
-static int __init register_earlycon(char *buf, const struct earlycon_id *match)
+static int __init register_earlycon(char *buf, struct earlycon_id *match)
 {
 	int err;
 	struct uart_port *port = &early_console_dev.port;
@@ -147,7 +146,7 @@ static int __init register_earlycon(char
  */
 int __init setup_earlycon(char *buf)
 {
-	const struct earlycon_id *match;
+	struct earlycon_id **pmatch;
 
 	if (!buf || !buf[0])
 		return -EINVAL;
@@ -155,7 +154,9 @@ int __init setup_earlycon(char *buf)
 	if (early_con.flags & CON_ENABLED)
 		return -EALREADY;
 
-	for (match = __earlycon_table; match->name[0]; match++) {
+	for (pmatch = __start_earlycon_table; pmatch < __stop_earlycon_table;
+	     pmatch++) {
+		struct earlycon_id *match = *pmatch;
 		size_t len = strlen(match->name);
 
 		if (strncmp(buf, match->name, len))
Index: linux-2.6/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-2.6.orig/include/asm-generic/vmlinux.lds.h
+++ linux-2.6/include/asm-generic/vmlinux.lds.h
@@ -151,10 +151,10 @@
 #endif
 
 #ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() . = ALIGN(8);				\
-			 VMLINUX_SYMBOL(__earlycon_table) = .;	\
-			 *(__earlycon_table)			\
-			 *(__earlycon_table_end)
+#define EARLYCON_TABLE() . = ALIGN(8);					\
+			 VMLINUX_SYMBOL(__start_earlycon_table) = .;	\
+			 *(__earlycon_table)				\
+			 VMLINUX_SYMBOL(__stop_earlycon_table) = .;
 #else
 #define EARLYCON_TABLE()
 #endif

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

Hi Yinghai,

On 04/02/2015 05:15 AM, Yinghai Lu wrote:
> On Wed, Apr 1, 2015 at 8:22 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> The documented behavior of console=ttyS options, to which your
>> quote refers, clearly states:
>>
>>                 Default is "9600n8".
> 
> drivers/tty/serial/8250/8250_early.c:early_serial8250_setup
> still have calling to probe_baud, but it is not triggered.
> 
> Here is root cause.
> The gap between entries in earlycon_table cause
> iteration fail to find next entry, so uart8250 handler is
> not called proplerly.

Thanks for finding that bug; so the earlycon never started, right?

> attached patch fix the problem.

Would you please try the patch below instead?

Regards,
Peter Hurley

--- >% ---
From: Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH] earlycon: Fix __earlycon_table stride

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/asm-generic/vmlinux.lds.h | 2 +-
 include/linux/serial_core.h       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 7b0ef49..2e11f31 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -151,7 +151,7 @@
 #endif
 
 #ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() . = ALIGN(8);				\
+#define EARLYCON_TABLE() STRUCT_ALIGN();			\
 			 VMLINUX_SYMBOL(__earlycon_table) = .;	\
 			 *(__earlycon_table)			\
 			 *(__earlycon_table_end)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 34de168..025dad9 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -342,7 +342,7 @@ struct earlycon_device {
 struct earlycon_id {
 	char	name[16];
 	int	(*setup)(struct earlycon_device *, const char *options);
-};
+} __aligned(32);
 
 extern int setup_earlycon(char *buf);
 extern int of_setup_earlycon(unsigned long addr,
-- 
2.3.5

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <peter@hurleysoftware.com> wrote:

> Would you please try the patch below instead?
>
> --- >% ---
> From: Peter Hurley <peter@hurleysoftware.com>
> Subject: [PATCH] earlycon: Fix __earlycon_table stride
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  include/asm-generic/vmlinux.lds.h | 2 +-
>  include/linux/serial_core.h       | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 7b0ef49..2e11f31 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -151,7 +151,7 @@
>  #endif
>
>  #ifdef CONFIG_SERIAL_EARLYCON
> -#define EARLYCON_TABLE() . = ALIGN(8);                         \
> +#define EARLYCON_TABLE() STRUCT_ALIGN();                       \
>                          VMLINUX_SYMBOL(__earlycon_table) = .;  \
>                          *(__earlycon_table)                    \
>                          *(__earlycon_table_end)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 34de168..025dad9 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -342,7 +342,7 @@ struct earlycon_device {
>  struct earlycon_id {
>         char    name[16];
>         int     (*setup)(struct earlycon_device *, const char *options);
> -};
> +} __aligned(32);
>
>  extern int setup_earlycon(char *buf);
>  extern int of_setup_earlycon(unsigned long addr,
> --

Great. that works, and less lines change than my version.

ffffffff832049a0 T __earlycon_table
ffffffff832049a0 t __earlycon_uart
ffffffff832049c0 t __earlycon_uart8250
ffffffff832049e0 t __earlycon_table_sentinel

[    0.000000] bootconsole [uart0] enabled
[    0.000000] uart8250 probed_baud_rate: 115200
[    0.000000] size of earlycon_id: 0x20

Thanks

Yinghai

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>
>> Would you please try the patch below instead?
>>
> Great. that works, and less lines change than my version.
>

still have another problem.
when using console=uart8250,io,0x3f8
it works as earlycon at first.
but after handover to normal console
it will revert back to 9600 again.

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>
>>> Would you please try the patch below instead?
>>>
>> Great. that works, and less lines change than my version.
>>
>
> still have another problem.
> when using console=uart8250,io,0x3f8
> it works as earlycon at first.
> but after handover to normal console
> it will revert back to 9600 again.

this regression should be caused by:

commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <peter@hurleysoftware.com>
Date:   Mon Mar 9 16:27:12 2015 -0400

    console: Add extensible console matching

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Thu, Apr 2, 2015 at 3:36 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>
>>>> Would you please try the patch below instead?
>>>>
>>> Great. that works, and less lines change than my version.
>>>
>>
>> still have another problem.
>> when using console=uart8250,io,0x3f8
>> it works as earlycon at first.
>> but after handover to normal console
>> it will revert back to 9600 again.
>
> this regression should be caused by:
>
> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
> Author: Peter Hurley <peter@hurleysoftware.com>
> Date:   Mon Mar 9 16:27:12 2015 -0400
>
>     console: Add extensible console matching

current code will:
for earlycon, will probe baud rate, and save it back to device->options.

and later update the command line
uart to ttyS, update the options with baud rate.

And your change will not try to find the exact port to have baud rate.
and just use command.
So will have have that probed baud rate passed.

Please check if you can fix this regression.
Maybe you have to put back command line update code back.

Thanks

Yinghai

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Thu, Apr 2, 2015 at 5:02 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Apr 2, 2015 at 3:36 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>
>>>>> Would you please try the patch below instead?
>>>>>
>>>> Great. that works, and less lines change than my version.
>>>>
>>>
>>> still have another problem.
>>> when using console=uart8250,io,0x3f8
>>> it works as earlycon at first.
>>> but after handover to normal console
>>> it will revert back to 9600 again.
>>
>> this regression should be caused by:
>>
>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>> Author: Peter Hurley <peter@hurleysoftware.com>
>> Date:   Mon Mar 9 16:27:12 2015 -0400
>>
>>     console: Add extensible console matching
>
So your whole patchset will need the first patch ?

or can you just drop the first one patch ?

Thanks

Yinghai

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:

>>>> still have another problem.
>>>> when using console=uart8250,io,0x3f8
>>>> it works as earlycon at first.
>>>> but after handover to normal console
>>>> it will revert back to 9600 again.
>>>
>>> this regression should be caused by:
>>>
>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>> Author: Peter Hurley <peter@hurleysoftware.com>
>>> Date:   Mon Mar 9 16:27:12 2015 -0400
>>>
>>>     console: Add extensible console matching
>>
> So your whole patchset will need the first patch ?
>
> or can you just drop the first one patch ?

Please check attached patch that fix regresion by. commit c7cef0a849
("console: Add extensible console matching")

or you have better way?

Thanks

Yinghai

[-- Attachment #2: fix_earlycon_console_handover.patch --]
[-- Type: text/x-patch, Size: 5269 bytes --]

Subject: [PATCH] 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.

Fixes: commit c7cef0a84912 ("console: Add extensible console matching")
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/tty/serial/8250/8250_core.c  |   14 ++++++++++++--
 drivers/tty/serial/8250/8250_early.c |   19 +++++++++++++++++++
 include/linux/console.h              |    2 +-
 include/linux/serial_8250.h          |    1 +
 kernel/printk/printk.c               |    2 +-
 5 files changed, 34 insertions(+), 4 deletions(-)

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
@@ -3452,7 +3452,7 @@ static int univ8250_console_setup(struct
  *	@co:	  registering console
  *	@name:	  name from console command line
  *	@idx:	  index from console command line
- *	@options: ptr to option string from console command line
+ *	@options_p: ptr to 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>
@@ -3465,11 +3465,12 @@ static int univ8250_console_setup(struct
  *	Returns 0 if console matches; otherwise non-zero to use default matching
  */
 static int univ8250_console_match(struct console *co, char *name, int idx,
-				  char *options)
+				  char **options_p)
 {
 	char match[] = "uart";	/* 8250-specific earlycon name */
 	unsigned char iotype;
 	unsigned long addr;
+	char *options = *options_p;
 	int i;
 
 	if (strncmp(name, match, 4) != 0)
@@ -3491,6 +3492,15 @@ static int univ8250_console_match(struct
 			continue;
 
 		co->index = i;
+
+		if (!options || !options[0]) {
+			char *new_options = NULL;
+
+			if (!serial8250_get_earlycon_options(port,
+							     &new_options))
+				options = *options_p = new_options;
+		}
+
 		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,24 @@ static void __init early_serial8250_writ
 		serial8250_early_out(port, UART_IER, ier);
 }
 
+static struct earlycon_device *early_device;
+
+int serial8250_get_earlycon_options(struct uart_port *up, char **options_p)
+{
+	struct earlycon_device *device = early_device;
+	struct uart_port *port = device ? &device->port : NULL;
+
+	if (!port || (!port->membase && !port->iobase))
+		return -ENODEV;
+
+	if (!uart_match_port(up, port))
+		return -ENODEV;
+
+	*options_p = device->options;
+
+	return 0;
+}
+
 static unsigned int __init probe_baud(struct uart_port *port)
 {
 	unsigned char lcr, dll, dlm;
@@ -161,6 +179,7 @@ static int __init early_serial8250_setup
 	} else
 		init_port(device);
 
+	early_device = device;
 	device->con->write = early_serial8250_write;
 	return 0;
 }
Index: linux-2.6/include/linux/serial_8250.h
===================================================================
--- linux-2.6.orig/include/linux/serial_8250.h
+++ linux-2.6/include/linux/serial_8250.h
@@ -135,6 +135,7 @@ void serial8250_resume_port(int line);
 
 extern int early_serial_setup(struct uart_port *port);
 
+extern int serial8250_get_earlycon_options(struct uart_port *up, char **p);
 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 void serial8250_do_set_termios(struct uart_port *port,
Index: linux-2.6/kernel/printk/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk/printk.c
+++ linux-2.6/kernel/printk/printk.c
@@ -2444,7 +2444,7 @@ void register_console(struct console *ne
 	     i < MAX_CMDLINECONSOLES && c->name[0];
 	     i++, c++) {
 		if (!newcon->match ||
-		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+		    !newcon->match(newcon, c->name, c->index, &c->options)) {
 			/* default matching */
 			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
 			if (strcmp(c->name, newcon->name) != 0)
Index: linux-2.6/include/linux/console.h
===================================================================
--- linux-2.6.orig/include/linux/console.h
+++ linux-2.6/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	(*match)(struct console *, char *name, int idx, char *options);
+	int	(*match)(struct console *, char *name, int idx, char **p);
 	short	flags;
 	short	index;
 	int	cflag;

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

On 04/02/2015 10:38 PM, Yinghai Lu wrote:
> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> 
>>>>> still have another problem.
>>>>> when using console=uart8250,io,0x3f8
>>>>> it works as earlycon at first.
>>>>> but after handover to normal console
>>>>> it will revert back to 9600 again.
>>>>
>>>> this regression should be caused by:
>>>>
>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>> Author: Peter Hurley <peter@hurleysoftware.com>
>>>> Date:   Mon Mar 9 16:27:12 2015 -0400
>>>>
>>>>     console: Add extensible console matching
>>>
>> So your whole patchset will need the first patch ?
>>
>> or can you just drop the first one patch ?
> 
> Please check attached patch that fix regresion by. commit c7cef0a849
> ("console: Add extensible console matching")
> 
> or you have better way?

Please re-read my first response to this problem.

Regards,
Peter Hurley

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Fri, Apr 3, 2015 at 3:37 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/02/2015 10:38 PM, Yinghai Lu wrote:
>> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>>>>>> still have another problem.
>>>>>> when using console=uart8250,io,0x3f8
>>>>>> it works as earlycon at first.
>>>>>> but after handover to normal console
>>>>>> it will revert back to 9600 again.
>>>>>
>>>>> this regression should be caused by:
>>>>>
>>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>>> Author: Peter Hurley <peter@hurleysoftware.com>
>>>>> Date:   Mon Mar 9 16:27:12 2015 -0400
>>>>>
>>>>>     console: Add extensible console matching
>>>>
>>> So your whole patchset will need the first patch ?
>>>
>>> or can you just drop the first one patch ?
>>
>> Please check attached patch that fix regresion by. commit c7cef0a849
>> ("console: Add extensible console matching")
>>
>> or you have better way?
>
> Please re-read my first response to this problem.
>

> Also, this expectation is an impediment when adding support for other
> 8250-like designs that don't have the same 8250 divisor registers
> (ie., _every_ new design). To properly support this requirement for
> just the existing 8250 hardware will require special probe_baud()
> functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
> omap 1510.

But you can not break old setup!

old behavior: console=uart8250,io,0x3f8

when bootloader or firmware has 115200 used already,
earlycon, and console all keep 115200.

Now with your change, earlycon will use 115200 still,
but console will switch to 9600.

>
> Is specifying the line speed on the command line really a burden?
>

Yes.  We have two setups, and two BIOS versions.
product version is using 9600 and devel/debug is using 115200.

This is a regression, and must be addressed.

Greg,

Please don't send merge request for tty-next to Linus before
this regression get resolved.

Thanks

Yinghai

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

On 04/03/2015 12:57 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 3:37 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/02/2015 10:38 PM, Yinghai Lu wrote:
>>> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>>>>> still have another problem.
>>>>>>> when using console=uart8250,io,0x3f8
>>>>>>> it works as earlycon at first.
>>>>>>> but after handover to normal console
>>>>>>> it will revert back to 9600 again.
>>>>>>
>>>>>> this regression should be caused by:
>>>>>>
>>>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>>>> Author: Peter Hurley <peter@hurleysoftware.com>
>>>>>> Date:   Mon Mar 9 16:27:12 2015 -0400
>>>>>>
>>>>>>     console: Add extensible console matching
>>>>>
>>>> So your whole patchset will need the first patch ?
>>>>
>>>> or can you just drop the first one patch ?
>>>
>>> Please check attached patch that fix regresion by. commit c7cef0a849
>>> ("console: Add extensible console matching")
>>>
>>> or you have better way?
>>
>> Please re-read my first response to this problem.
>>
> 
>> Also, this expectation is an impediment when adding support for other
>> 8250-like designs that don't have the same 8250 divisor registers
>> (ie., _every_ new design). To properly support this requirement for
>> just the existing 8250 hardware will require special probe_baud()
>> functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
>> omap 1510.
> 
> But you can not break old setup!
> 
> old behavior: console=uart8250,io,0x3f8
> 
> when bootloader or firmware has 115200 used already,
> earlycon, and console all keep 115200.
> 
> Now with your change, earlycon will use 115200 still,
> but console will switch to 9600.
> 
>>
>> Is specifying the line speed on the command line really a burden?
>>
> 
> Yes.  We have two setups, and two BIOS versions.
> product version is using 9600 and devel/debug is using 115200.

Wait -- you have earlycon in a product??

> This is a regression, and must be addressed.
> 
> Greg,
> 
> Please don't send merge request for tty-next to Linus before
> this regression get resolved.

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>
> Wait -- you have earlycon in a product??

What do you mean?

when you are using "console=uart8250,io,0x3f8"
you will have earlycon at first, and handover to console.

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

On 04/03/2015 01:44 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>
>> Wait -- you have earlycon in a product??
> 
> What do you mean?

I mean, what will happen if I put in a big debug banner like Steven
did for ftrace?

Kernel developers need earlycon for debugging arch code; often the earlycon
is just hacked together especially when it requires fixmap support.

Putting it in a product and _relying on undocumented behavior_ is a bad idea.


> when you are using "console=uart8250,io,0x3f8"
> you will have earlycon at first, and handover to console.

which means you have to have CONFIG_SERIAL_EARLYCON to get a serial console
at all.

I have no problem fixing this _if this is causing actual regressions_,
and not simply some minor inconvenience that can be fixed by editing your
grub command line.

However, I see this undocumented behavior as a cautionary tale for why not
every kernel command line hack should be accepted in mainline.

Regards,
Peter Hurley

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Greg Kroah-Hartman]

On Fri, Apr 03, 2015 at 02:27:30PM -0400, Peter Hurley wrote:
> I have no problem fixing this _if this is causing actual regressions_,
> and not simply some minor inconvenience that can be fixed by editing your
> grub command line.

But, you can't break working command lines from previous kernel
releases.  Undocumented or not, sorry.  It's a working configuration.
Upgrading a kernel can not break that.

sorry,

greg k-h

[PATCH] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[####],io|mmio|mmio32,<addr>[,options]
    console=uart[####],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Document the required behavior of the original implementation.

Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 Documentation/kernel-parameters.txt  | 15 +++++++++++----
 drivers/tty/serial/8250/8250_core.c  | 15 ++++++++++++++-
 drivers/tty/serial/8250/8250_early.c | 19 -------------------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..59c9832 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -711,12 +711,19 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Documentation/networking/netconsole.txt for an
 			alternative.
 
-		uart[8250],io,<addr>[,options]
-		uart[8250],mmio,<addr>[,options]
+		uart[<n>],io,<addr>[,options]
+		uart[<n>],mmio,<addr>[,options]
+		uart[<n>],mmio32,<addr>[,options]
+		uart[<n>],0x<addr>[,options]
 			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.
+			Any optional number <n> is accepted and ignored.
+			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.
 
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..5f41a9c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3456,12 +3456,17 @@ static int univ8250_console_setup(struct console *co, char *options)
  *
  *	Only attempts to match console command lines of the form:
  *	    console=uart<>,io|mmio|mmio32,<addr>,<options>
- *	    console=uart<>,<addr>,options
+ *	    console=uart<>,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 **
+ *	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,
@@ -3491,6 +3496,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
 			continue;
 
 		co->index = i;
+
+		/* if no line settings, then assume h/w was setup */
+		if (!options) {
+			/* link port to console */
+			port->cons = co;
+			return 0;
+		}
+
 		return univ8250_console_setup(co, options);
 	}
 
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);
-- 
2.3.5

[PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[8250],io|mmio|mmio32,<addr>[,options]
    console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Document the required behavior of the original implementation.

Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---

v2: Fixed regression which allowed "console=uart1337,..." to start a
    console (but not an earlycon)
  + fixed earlycon= documentation related required behavior fixed by
    this patch

 Documentation/kernel-parameters.txt  | 18 +++++++++++++++---
 drivers/tty/serial/8250/8250_core.c  | 20 ++++++++++++++++++--
 drivers/tty/serial/8250/8250_early.c | 19 -------------------
 3 files changed, 33 insertions(+), 24 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]
 			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]
 			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..bda2a57 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3455,13 +3455,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 **
+ *	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,6 +3480,9 @@ 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;
+
 	if (uart_parse_earlycon(options, &iotype, &addr, &options))
 		return -ENODEV;
 
@@ -3491,6 +3499,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
 			continue;
 
 		co->index = i;
+
+		/* if no line settings, then assume h/w was setup */
+		if (!options) {
+			/* link port to console */
+			port->cons = co;
+			return 0;
+		}
+
 		return univ8250_console_setup(co, options);
 	}
 
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);
-- 
2.3.5

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>>
>>> Wait -- you have earlycon in a product??
>>
>> What do you mean?
>
> I mean, what will happen if I put in a big debug banner like Steven
> did for ftrace?
>
> Kernel developers need earlycon for debugging arch code; often the earlycon
> is just hacked together especially when it requires fixmap support.
>
> Putting it in a product and _relying on undocumented behavior_ is a bad idea.

let me repeat again:
when you have "console=uart8250,io,0x3f8", you will have earlycon and
then console.

That is just for kernel developer for debugging.

When you have x86 with bunch of dimms and cpus, user will have to wait couple
of minutes to output from console=ttyS0....and if it hang early, no
one would know what happen.

Also it is only boot time only....

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Yinghai Lu]

On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>
> which means you have to have CONFIG_SERIAL_EARLYCON to get a serial console
> at all.

what do you mean? Now in drivers/tty/serial/8250/Kconfig we have

config SERIAL_8250_CONSOLE
        bool "Console on 8250/16550 and compatible serial port"
        depends on SERIAL_8250=y
        select SERIAL_CORE_CONSOLE
        select SERIAL_EARLYCON

do you mean, we should not have that "select SERIAL_EARLYCON" ?

Re: [PATCH v3 -next 11/11] serial: 8250_early: Remove setup_early_serial8250_console()

[Peter Hurley]

On 04/03/2015 08:52 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>>>
>>>> Wait -- you have earlycon in a product??
>>>
>>> What do you mean?
>>
>> I mean, what will happen if I put in a big debug banner like Steven
>> did for ftrace?
>>
>> Kernel developers need earlycon for debugging arch code; often the earlycon
>> is just hacked together especially when it requires fixmap support.
>>
>> Putting it in a product and _relying on undocumented behavior_ is a bad idea.
> 
> let me repeat again:
> when you have "console=uart8250,io,0x3f8", you will have earlycon and
> then console.
> 
> That is just for kernel developer for debugging.
> 
> When you have x86 with bunch of dimms and cpus, user will have to wait couple
> of minutes to output from console=ttyS0....and if it hang early, no
> one would know what happen.
> 
> Also it is only boot time only....

The fix is sitting in your inbox. Pointless to discuss it now.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>     console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.


Please separated bug fix and other documentation change to different patches.

also fix the patch title to make it clear and you need to mention which commit
cause the regression.

This patch fix regression for the hand over.  Thanks.

Another regression.
when user have
console=uart8250,io,0x3f8 console=uart8250,io,0x2f8

before your patchset:
port_0x3f8 is early console, and will be console later.
and port_0x2f8 is ignored, because only ONE early console is allowed.
       and old console setup, only handle ttyS0.

after your patchset:
port_0x3f8 is early console, and will be console later.
port_0x2f8 will become default console, as new console with match method
   treat all uart8250 as ttyS0.

Please fix that too.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 10:19 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>     console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
> 
> 
> Please separated bug fix and other documentation change to different patches.

No. The documentation reflects exactly what the bug fix is for.

> also fix the patch title to make it clear and you need to mention which commit
> cause the regression.

Please bisect and send bisect log so I know which commit broke
your setup.

> This patch fix regression for the hand over.  Thanks.
> 
> Another regression.
> when user have
> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
> 
> before your patchset:
> port_0x3f8 is early console, and will be console later.
> and port_0x2f8 is ignored, because only ONE early console is allowed.
>        and old console setup, only handle ttyS0.
> 
> after your patchset:
> port_0x3f8 is early console, and will be console later.
> port_0x2f8 will become default console, as new console with match method
>    treat all uart8250 as ttyS0.
> 
> Please fix that too.

That's a new feature, not a regression.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 10:29 PM, Peter Hurley wrote:
> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>> Another regression.
>> when user have
>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>
>> before your patchset:
>> port_0x3f8 is early console, and will be console later.
>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>        and old console setup, only handle ttyS0.
>>
>> after your patchset:
>> port_0x3f8 is early console, and will be console later.
>> port_0x2f8 will become default console, as new console with match method
>>    treat all uart8250 as ttyS0.
>>
>> Please fix that too.
> 
> That's a new feature, not a regression.

So, in all seriousness, you actually have this setup and want me to
fix this?

IOW, to support more than 1 earlycon in the future will require
using a totally different command line parameter. Awesome.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 7:29 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> Restore undocumented behavior of kernel command line parameters of
>>> the forms:
>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>     console=uart[8250],<addr>[,options]
>>> where 'options' have not been specified; in this case, the hardware
>>> is assumed to be initialized.
>>
>>
>> Please separated bug fix and other documentation change to different patches.
>
> No. The documentation reflects exactly what the bug fix is for.

what is

+               uart[<n>],mmio,<addr>[,options]
+               uart[<n>],mmio32,<addr>[,options]

>
>> also fix the patch title to make it clear and you need to mention which commit
>> cause the regression.
>
> Please bisect and send bisect log so I know which commit broke
> your setup.

I already gave your the patch "title". Here it is again:

commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <peter@hurleysoftware.com>
Date:   Mon Mar 9 16:27:12 2015 -0400

    console: Add extensible console matching


>
>> This patch fix regression for the hand over.  Thanks.
>>
>> Another regression.
>> when user have
>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>
>> before your patchset:
>> port_0x3f8 is early console, and will be console later.
>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>        and old console setup, only handle ttyS0.
>>
>> after your patchset:
>> port_0x3f8 is early console, and will be console later.
>> port_0x2f8 will become default console, as new console with match method
>>    treat all uart8250 as ttyS0.
>>
>> Please fix that too.
>
> That's a new feature, not a regression.

ok, think that more.

with this patch:

[PATCH] earlycon: 8250: Fix command line regression

When you have console=uart8250,io,0x3f8 console=uart,io,0x2f8

you will skip the setup to 0x2f8, as the options is null.
then if the bios or bootloader only setup baud rate for 0x3f8.
the user could get nothing from 0x2f8.

Do you need to call univ8250_console_setup(co, options)
to set the default baud rate ?

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 7:50 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 10:29 PM, Peter Hurley wrote:
>> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>>> Another regression.
>>> when user have
>>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>>
>>> before your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>>        and old console setup, only handle ttyS0.
>>>
>>> after your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> port_0x2f8 will become default console, as new console with match method
>>>    treat all uart8250 as ttyS0.
>>>
>>> Please fix that too.
>>
>> That's a new feature, not a regression.
>
> So, in all seriousness, you actually have this setup and want me to
> fix this?

No.

Just add one count in the console match for it.

>
> IOW, to support more than 1 earlycon in the future will require
> using a totally different command line parameter. Awesome.
>

That is future, right?  Show me patch.

Thanks

Yinghai

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>     console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
>
>
> This patch fix regression for the hand over.  Thanks.

Just now noticed, this patch only fix partial problem. Kernel console is ok.

But later initrd/init scripts revert back to 9600 again.

So the patch does not fix the regression from

commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <peter@hurleysoftware.com>
Date:   Mon Mar 9 16:27:12 2015 -0400

    console: Add extensible console matching

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 10:56 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 7:29 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> Restore undocumented behavior of kernel command line parameters of
>>>> the forms:
>>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>     console=uart[8250],<addr>[,options]
>>>> where 'options' have not been specified; in this case, the hardware
>>>> is assumed to be initialized.
>>>
>>>
>>> Please separated bug fix and other documentation change to different patches.
>>
>> No. The documentation reflects exactly what the bug fix is for.
> 
> what is
> 
> +               uart[<n>],mmio,<addr>[,options]
> +               uart[<n>],mmio32,<addr>[,options]

Wrong patch version; that's v1.


>>> also fix the patch title to make it clear and you need to mention which commit
>>> cause the regression.
>>
>> Please bisect and send bisect log so I know which commit broke
>> your setup.
> 
> I already gave your the patch "title". Here it is again:
> 
> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
> Author: Peter Hurley <peter@hurleysoftware.com>
> Date:   Mon Mar 9 16:27:12 2015 -0400
> 
>     console: Add extensible console matching

Ok.

>>> This patch fix regression for the hand over.  Thanks.
>>>
>>> Another regression.
>>> when user have
>>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>>
>>> before your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>>        and old console setup, only handle ttyS0.
>>>
>>> after your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> port_0x2f8 will become default console, as new console with match method
>>>    treat all uart8250 as ttyS0.
>>>
>>> Please fix that too.
>>
>> That's a new feature, not a regression.
> 
> ok, think that more.
> 
> with this patch:
> 
> [PATCH] earlycon: 8250: Fix command line regression
> 
> When you have console=uart8250,io,0x3f8 console=uart,io,0x2f8
> 
> you will skip the setup to 0x2f8, as the options is null.
> then if the bios or bootloader only setup baud rate for 0x3f8.
> the user could get nothing from 0x2f8.
> 
> Do you need to call univ8250_console_setup(co, options)
> to set the default baud rate ?

No. The user specified that the console had already been initialized
by leaving out the option string. Isn't that what this whole
email thread has been about?

All this automatic behavior and aliases for the same things just
adds unnecessary complexity and maintenance burden, without measurable
gain.

The real tragedy is that all this was totally unnecessary; what was
wrong with "earlycon=uart,io,0x3f8 console=ttyS0"?

Regards,
Peter Hurley

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 11:09 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> Restore undocumented behavior of kernel command line parameters of
>>> the forms:
>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>     console=uart[8250],<addr>[,options]
>>> where 'options' have not been specified; in this case, the hardware
>>> is assumed to be initialized.
>>
>>
>> This patch fix regression for the hand over.  Thanks.
> 
> Just now noticed, this patch only fix partial problem. Kernel console is ok.
> 
> But later initrd/init scripts revert back to 9600 again.

Please share the init scripts.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> Restore undocumented behavior of kernel command line parameters of
>>>> the forms:
>>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>     console=uart[8250],<addr>[,options]
>>>> where 'options' have not been specified; in this case, the hardware
>>>> is assumed to be initialized.
>>>
>>>
>>> This patch fix regression for the hand over.  Thanks.
>>
>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>
>> But later initrd/init scripts revert back to 9600 again.
>
> Please share the init scripts.

I am using opensuse 13.1 rescue disk as ramdisk.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 8:09 PM, Peter Hurley <peter@hurleysoftware.com> wrote:

>
> All this automatic behavior and aliases for the same things just
> adds unnecessary complexity and maintenance burden, without measurable
> gain.
>
> The real tragedy is that all this was totally unnecessary; what was
> wrong with "earlycon=uart,io,0x3f8 console=ttyS0"?

When I worked on the patch

| commit 18a8bd949d6adb311ea816125ff65050df1f3f6e
| Date:   Sun Jul 15 23:37:59 2007 -0700
|
|    serial: convert early_uart to earlycon for 8250

Andrew requested that.

Thanks

Yinghai

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 8:24 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>> the forms:
>>>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>>     console=uart[8250],<addr>[,options]
>>>>> where 'options' have not been specified; in this case, the hardware
>>>>> is assumed to be initialized.
>>>>
>>>>
>>>> This patch fix regression for the hand over.  Thanks.
>>>
>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>
>>> But later initrd/init scripts revert back to 9600 again.
>>
>> Please share the init scripts.
>
> I am using opensuse 13.1 rescue disk as ramdisk.

or opensuse 12.3 rescue disk.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 11:24 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>> the forms:
>>>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>>     console=uart[8250],<addr>[,options]
>>>>> where 'options' have not been specified; in this case, the hardware
>>>>> is assumed to be initialized.
>>>>
>>>>
>>>> This patch fix regression for the hand over.  Thanks.
>>>
>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>
>>> But later initrd/init scripts revert back to 9600 again.
>>
>> Please share the init scripts.
> 
> I am using opensuse 13.1 rescue disk as ramdisk.

NVM, I know what the problem is.

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Fri, Apr 3, 2015 at 8:32 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>
>>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>>
>>>> But later initrd/init scripts revert back to 9600 again.
>>>
>>> Please share the init scripts.
>>
>> I am using opensuse 13.1 rescue disk as ramdisk.
>
> NVM, I know what the problem is.
>

What is NVM here?

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/03/2015 11:37 PM, Yinghai Lu wrote:
> What is NVM here?

nvm == nevermind

Re: [PATCH v2] earlycon: 8250: Fix command line regression

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Fri, Apr 3, 2015 at 8:32 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/03/2015 11:24 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>>> the forms:
>>>>>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>>>     console=uart[8250],<addr>[,options]
>>>>>> where 'options' have not been specified; in this case, the hardware
>>>>>> is assumed to be initialized.
>>>>>
>>>>>
>>>>> This patch fix regression for the hand over.  Thanks.
>>>>
>>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>>
>>>> But later initrd/init scripts revert back to 9600 again.
>>>

I updated my fix a little bit. please check attached.

Thanks

Yinghai

[-- Attachment #2: fix_earlycon_console_handover_v2.patch --]
[-- Type: text/x-patch, Size: 3050 bytes --]

Subject: [PATCH -v2] 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.

Fixes: commit c7cef0a84912 ("console: Add extensible console matching")
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
-v2: simplify serial8250_get_earlycon_options and don't update
     console_cmdline.
---
 drivers/tty/serial/8250/8250_core.c  |    4 ++++
 drivers/tty/serial/8250/8250_early.c |   17 +++++++++++++++++
 include/linux/serial_8250.h          |    1 +
 3 files changed, 22 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
@@ -3491,6 +3491,10 @@ static int univ8250_console_match(struct
 			continue;
 
 		co->index = i;
+
+		if (!options)
+			options = serial8250_get_earlycon_options(port);
+
 		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,22 @@ static void __init early_serial8250_writ
 		serial8250_early_out(port, UART_IER, ier);
 }
 
+static struct earlycon_device *early_device;
+
+char *serial8250_get_earlycon_options(struct uart_port *up)
+{
+	struct earlycon_device *device = early_device;
+	struct uart_port *port = device ? &device->port : NULL;
+
+	if (!port || (!port->membase && !port->iobase))
+		return NULL;
+
+	if (!uart_match_port(up, port))
+		return NULL;
+
+	return device->options;
+}
+
 static unsigned int __init probe_baud(struct uart_port *port)
 {
 	unsigned char lcr, dll, dlm;
@@ -161,6 +177,7 @@ static int __init early_serial8250_setup
 	} else
 		init_port(device);
 
+	early_device = device;
 	device->con->write = early_serial8250_write;
 	return 0;
 }
Index: linux-2.6/include/linux/serial_8250.h
===================================================================
--- linux-2.6.orig/include/linux/serial_8250.h
+++ linux-2.6/include/linux/serial_8250.h
@@ -135,6 +135,7 @@ void serial8250_resume_port(int line);
 
 extern int early_serial_setup(struct uart_port *port);
 
+extern char *serial8250_get_earlycon_options(struct uart_port *up);
 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 void serial8250_do_set_termios(struct uart_port *port,

[PATCH v3] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[8250],io|mmio|mmio32,<addr>[,options]
    console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Document the required behavior of the original implementation.

Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---

v3: Fixed automatic console to port line settings initialization;
    open-coded serial8250_console_setup() so the baud can be probed;
    added sha reference in commit log

v2: Fixed regression which allowed "console=uart1337,..." to start a
    console (but not an earlycon)
  + fixed earlycon= documentation related required behavior fixed by
    this patch

 Documentation/kernel-parameters.txt  | 18 ++++++++++++++---
 drivers/tty/serial/8250/8250_core.c  | 38 +++++++++++++++++++++++++++++++++---
 drivers/tty/serial/8250/8250_early.c | 19 ------------------
 3 files changed, 50 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]
 			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]
 			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..f59c7a0 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
 	return serial8250_console_setup(up, options);
 }
 
+/* FIXME: this is broken on most other 8250 h/w */
+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;
+}
+
 /**
  *	univ8250_console_match - non-standard console matching
  *	@co:	  registering console
@@ -3455,13 +3471,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 **
+ *	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 +3496,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;
+
 	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 +3515,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);
 	}
 
 	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);
-- 
2.3.5

Re: [PATCH v3] earlycon: 8250: Fix command line regression

[Greg Kroah-Hartman]

On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>     console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
> 
> Document the required behavior of the original implementation.
> 
> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> Reported-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> 
> v3: Fixed automatic console to port line settings initialization;
>     open-coded serial8250_console_setup() so the baud can be probed;
>     added sha reference in commit log
> 
> v2: Fixed regression which allowed "console=uart1337,..." to start a
>     console (but not an earlycon)
>   + fixed earlycon= documentation related required behavior fixed by
>     this patch
> 
>  Documentation/kernel-parameters.txt  | 18 ++++++++++++++---
>  drivers/tty/serial/8250/8250_core.c  | 38 +++++++++++++++++++++++++++++++++---
>  drivers/tty/serial/8250/8250_early.c | 19 ------------------
>  3 files changed, 50 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]
>  			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]
>  			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..f59c7a0 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
>  	return serial8250_console_setup(up, options);
>  }
>  
> +/* FIXME: this is broken on most other 8250 h/w */

What do you mean by "most other"?  What hardware does this work for?
What is it broken for?  What is someone supposed to think/do with this
comment?

thanks,

greg k-h

Re: [PATCH v3] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>     console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
>>
>> Document the required behavior of the original implementation.
>>
>> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
>> Reported-by: Yinghai Lu <yinghai@kernel.org>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>>
>> v3: Fixed automatic console to port line settings initialization;
>>     open-coded serial8250_console_setup() so the baud can be probed;
>>     added sha reference in commit log
>>
>> v2: Fixed regression which allowed "console=uart1337,..." to start a
>>     console (but not an earlycon)
>>   + fixed earlycon= documentation related required behavior fixed by
>>     this patch
>>
>>  Documentation/kernel-parameters.txt  | 18 ++++++++++++++---
>>  drivers/tty/serial/8250/8250_core.c  | 38 +++++++++++++++++++++++++++++++++---
>>  drivers/tty/serial/8250/8250_early.c | 19 ------------------
>>  3 files changed, 50 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]
>>  			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]
>>  			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..f59c7a0 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
>>  	return serial8250_console_setup(up, options);
>>  }
>>  
>> +/* FIXME: this is broken on most other 8250 h/w */
> 
> What do you mean by "most other"?  What hardware does this work for?
> What is it broken for?  What is someone supposed to think/do with this
> comment?

It's a direct copy of the existing behavior for 8250 earlycon, which is
broken for h/w with non-standard divisor registers. That's basically
_every_ new design, because that's what vendors need to extend to support
modern bitrates.

Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
omap15xx, intel byt, intel mid.

But it was already broken for these and so not a regression.

Regards,
Peter Hurley

Re: [PATCH v3] earlycon: 8250: Fix command line regression

[Greg Kroah-Hartman]

On Sat, Apr 04, 2015 at 12:23:14PM -0400, Peter Hurley wrote:
> On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
> > On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
> >> Restore undocumented behavior of kernel command line parameters of
> >> the forms:
> >>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
> >>     console=uart[8250],<addr>[,options]
> >> where 'options' have not been specified; in this case, the hardware
> >> is assumed to be initialized.
> >>
> >> Document the required behavior of the original implementation.
> >>
> >> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> >> Reported-by: Yinghai Lu <yinghai@kernel.org>
> >> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> >> ---
> >>
> >> v3: Fixed automatic console to port line settings initialization;
> >>     open-coded serial8250_console_setup() so the baud can be probed;
> >>     added sha reference in commit log
> >>
> >> v2: Fixed regression which allowed "console=uart1337,..." to start a
> >>     console (but not an earlycon)
> >>   + fixed earlycon= documentation related required behavior fixed by
> >>     this patch
> >>
> >>  Documentation/kernel-parameters.txt  | 18 ++++++++++++++---
> >>  drivers/tty/serial/8250/8250_core.c  | 38 +++++++++++++++++++++++++++++++++---
> >>  drivers/tty/serial/8250/8250_early.c | 19 ------------------
> >>  3 files changed, 50 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]
> >>  			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]
> >>  			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..f59c7a0 100644
> >> --- a/drivers/tty/serial/8250/8250_core.c
> >> +++ b/drivers/tty/serial/8250/8250_core.c
> >> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
> >>  	return serial8250_console_setup(up, options);
> >>  }
> >>  
> >> +/* FIXME: this is broken on most other 8250 h/w */
> > 
> > What do you mean by "most other"?  What hardware does this work for?
> > What is it broken for?  What is someone supposed to think/do with this
> > comment?
> 
> It's a direct copy of the existing behavior for 8250 earlycon, which is
> broken for h/w with non-standard divisor registers. That's basically
> _every_ new design, because that's what vendors need to extend to support
> modern bitrates.
> 
> Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
> omap15xx, intel byt, intel mid.
> 
> But it was already broken for these and so not a regression.

Sorry, I know the function is a copy, you haven't broken anything that
wasn't already broken.  I see this comment as a "rant", but we might as
well turn that "rant" into something descriptive to let other people
know what is really going on here.

Sound reasonable?

thanks,

greg k-h

Re: [PATCH v3] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/04/2015 12:52 PM, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2015 at 12:23:14PM -0400, Peter Hurley wrote:
>> On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
>>> On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
[...]
>>>> +/* FIXME: this is broken on most other 8250 h/w */
>>>
>>> What do you mean by "most other"?  What hardware does this work for?
>>> What is it broken for?  What is someone supposed to think/do with this
>>> comment?
>>
>> It's a direct copy of the existing behavior for 8250 earlycon, which is
>> broken for h/w with non-standard divisor registers. That's basically
>> _every_ new design, because that's what vendors need to extend to support
>> modern bitrates.
>>
>> Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
>> omap15xx, intel byt, intel mid.
>>
>> But it was already broken for these and so not a regression.
> 
> Sorry, I know the function is a copy, you haven't broken anything that
> wasn't already broken.  I see this comment as a "rant", but we might as
> well turn that "rant" into something descriptive to let other people
> know what is really going on here.

It's not intended as rant. My concern is that it appears as "desirable"
code because it's new, when that's not my intention.

When others go to implement earlycon-to-console handoff, my hope is
that they don't follow this exemplar. Unfortunately, since this is
the only driver that implements earlycon-to-console handoff (right now),
some might just simply copy the behavior here. My intention is to
discourage that.

> Sound reasonable?

Ok. How about "most new"?

Regards,
Peter Hurley

[PATCH v4] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[8250],io|mmio|mmio32,<addr>[,options]
    console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Document the required behavior of the original implementation.

Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
v4: Removed FIXME comment

v3: Fixed automatic console to port line settings initialization;
    open-coded serial8250_console_setup() so the baud can be probed;
    added sha reference in commit log

v2: Fixed regression which allowed "console=uart1337,..." to start a
    console (but not an earlycon)
  + fixed earlycon= documentation related required behavior fixed by
    this patch


 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]
 			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]
 			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;
+}
+
 /**
  *	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 **
+ *	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;
+
 	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);
 	}
 
 	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);
-- 
2.3.5

Re: [PATCH v4] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/04/2015 01:19 PM, Peter Hurley wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>     console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
> 
> Document the required behavior of the original implementation.

Greg,

Please wait for a Tested-by from Yinghai before pushing; I have no
platform to test if this fixes his regression.

Regards,
Peter Hurley

Re: [PATCH v4] earlycon: 8250: Fix command line regression

[Greg Kroah-Hartman]

On Sat, Apr 04, 2015 at 01:24:51PM -0400, Peter Hurley wrote:
> On 04/04/2015 01:19 PM, Peter Hurley wrote:
> > Restore undocumented behavior of kernel command line parameters of
> > the forms:
> >     console=uart[8250],io|mmio|mmio32,<addr>[,options]
> >     console=uart[8250],<addr>[,options]
> > where 'options' have not been specified; in this case, the hardware
> > is assumed to be initialized.
> > 
> > Document the required behavior of the original implementation.
> 
> Greg,
> 
> Please wait for a Tested-by from Yinghai before pushing; I have no
> platform to test if this fixes his regression.

Ok, will do, thanks for redoing this and spinning up a fix so quickly.

greg k-h

Re: [PATCH v4] earlycon: 8250: Fix command line regression

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 8587 bytes --]

On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <peter@hurleysoftware.com> 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

[-- Attachment #2: fix_earlycon_console_handover_v3.patch --]
[-- Type: text/x-patch, Size: 3884 bytes --]

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@kernel.org>

---
-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 *));

Re: [PATCH v4] earlycon: 8250: Fix command line regression

[Peter Hurley]

On 04/05/2015 03:09 AM, Yinghai Lu wrote:
> On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <peter@hurleysoftware.com> 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?

[PATCH v5] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[8250],io|mmio|mmio32,<addr>[,options]
    console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Document the required behavior of the original implementation.

Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
v5: Refactored serial8250_console_setup() & univ8250_console_setup()
    to eliminate open-coding; net +1 LOC (excludes documentation)

v4: Removed FIXME comment

v3: Fixed automatic console to port line settings initialization;
    open-coded serial8250_console_setup() so the baud can be probed;
    added sha reference in commit log

v2: Fixed regression which allowed "console=uart1337,..." to start a
    console (but not an earlycon)
  + fixed earlycon= documentation related required behavior fixed by
    this patch

 Documentation/kernel-parameters.txt  | 18 ++++++++++++---
 drivers/tty/serial/8250/8250_core.c  | 45 ++++++++++++++++++++++++++++--------
 drivers/tty/serial/8250/8250_early.c | 19 ---------------
 3 files changed, 50 insertions(+), 32 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]
 			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]
 			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..f8c27de 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3412,9 +3412,23 @@ static void univ8250_console_write(struct console *co, const char *s,
 	serial8250_console_write(up, s, count);
 }
 
-static int serial8250_console_setup(struct uart_8250_port *up, char *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;
+}
+
+static int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
 {
-	struct uart_port *port = &up->port;
 	int baud = 9600;
 	int bits = 8;
 	int parity = 'n';
@@ -3423,15 +3437,17 @@ static int serial8250_console_setup(struct uart_8250_port *up, char *options)
 	if (!port->iobase && !port->membase)
 		return -ENODEV;
 
-	if (options)
+	if (options && (!probe || *options))
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
+	else if (probe)
+		baud = probe_baud(port);
 
 	return uart_set_options(port, port->cons, baud, parity, bits, flow);
 }
 
 static int univ8250_console_setup(struct console *co, char *options)
 {
-	struct uart_8250_port *up;
+	struct uart_port *port;
 
 	/*
 	 * Check whether an invalid uart number has been specified, and
@@ -3440,11 +3456,11 @@ static int univ8250_console_setup(struct console *co, char *options)
 	 */
 	if (co->index >= nr_uarts)
 		co->index = 0;
-	up = &serial8250_ports[co->index];
+	port = &serial8250_ports[co->index].port;
 	/* link port to console */
-	up->port.cons = co;
+	port->cons = co;
 
-	return serial8250_console_setup(up, options);
+	return serial8250_console_setup(port, options, false);
 }
 
 /**
@@ -3455,13 +3471,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 **
+ *	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,6 +3496,9 @@ 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;
+
 	if (uart_parse_earlycon(options, &iotype, &addr, &options))
 		return -ENODEV;
 
@@ -3491,7 +3515,8 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
 			continue;
 
 		co->index = i;
-		return univ8250_console_setup(co, options);
+		port->cons = co;
+		return serial8250_console_setup(port, options, true);
 	}
 
 	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);
-- 
2.3.5

Re: [PATCH v5] earlycon: 8250: Fix command line regression

[Yinghai Lu]

On Sun, Apr 5, 2015 at 7:52 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
>     console=uart[8250],io|mmio|mmio32,<addr>[,options]
>     console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
>
> Document the required behavior of the original implementation.
>
> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> Reported-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> v5: Refactored serial8250_console_setup() & univ8250_console_setup()
>     to eliminate open-coding; net +1 LOC (excludes documentation)
>
> v4: Removed FIXME comment
>
> v3: Fixed automatic console to port line settings initialization;
>     open-coded serial8250_console_setup() so the baud can be probed;
>     added sha reference in commit log
>
> v2: Fixed regression which allowed "console=uart1337,..." to start a
>     console (but not an earlycon)
>   + fixed earlycon= documentation related required behavior fixed by
>     this patch
>
>  Documentation/kernel-parameters.txt  | 18 ++++++++++++---
>  drivers/tty/serial/8250/8250_core.c  | 45 ++++++++++++++++++++++++++++--------
>  drivers/tty/serial/8250/8250_early.c | 19 ---------------
>  3 files changed, 50 insertions(+), 32 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]

You just don't listen.

I wasted too much time on this.

Greg,

Please just drop c7cef0a84912cab3c9df8 ("console: Add extensible
console matching").

Thanks

Yinghai

Re: [PATCH v4] earlycon: 8250: Fix command line regression

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Sun, Apr 5, 2015 at 6:06 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On 04/05/2015 03:09 AM, Yinghai Lu wrote:

> On what planet is 27 loc across 4 source files cleaner than
> 6 loc that might be reducible to 2?

loc is not only thing to decide if it is cleaner.

Let's Greg and Andrew to check which one is clean.

[-- Attachment #2: fix_earlycon_console_handover_v3.patch --]
[-- Type: text/x-patch, Size: 3884 bytes --]

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@kernel.org>

---
-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 *));

[PATCH v6] earlycon: 8250: Fix command line regression

[Peter Hurley]

Restore undocumented behavior of kernel command line parameters of
the forms:
    console=uart[8250],io|mmio|mmio32,<addr>[,options]
    console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.

Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
v6: Removed documentation
  + removed unnecessary checks; this fix does not attempt to replicate
    the _exact_ original behavior, but only the reported breakage.

v5: Refactored serial8250_console_setup() & univ8250_console_setup()
    to eliminate open-coding; net +1 LOC (excludes documentation)

v4: Removed FIXME comment

v3: Fixed automatic console to port line settings initialization;
    open-coded serial8250_console_setup() so the baud can be probed;
    added sha reference in commit log

v2: Fixed regression which allowed "console=uart1337,..." to start a
    console (but not an earlycon)
  + fixed earlycon= documentation related required behavior fixed by
    this patch

 drivers/tty/serial/8250/8250_core.c  | 31 ++++++++++++++++++++++++-------
 drivers/tty/serial/8250/8250_early.c | 19 -------------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..18142ee 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3412,9 +3412,23 @@ static void univ8250_console_write(struct console *co, const char *s,
 	serial8250_console_write(up, s, count);
 }
 
-static int serial8250_console_setup(struct uart_8250_port *up, char *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;
+}
+
+static int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
 {
-	struct uart_port *port = &up->port;
 	int baud = 9600;
 	int bits = 8;
 	int parity = 'n';
@@ -3425,13 +3439,15 @@ static int serial8250_console_setup(struct uart_8250_port *up, char *options)
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
+	else if (probe)
+		baud = probe_baud(port);
 
 	return uart_set_options(port, port->cons, baud, parity, bits, flow);
 }
 
 static int univ8250_console_setup(struct console *co, char *options)
 {
-	struct uart_8250_port *up;
+	struct uart_port *port;
 
 	/*
 	 * Check whether an invalid uart number has been specified, and
@@ -3440,11 +3456,11 @@ static int univ8250_console_setup(struct console *co, char *options)
 	 */
 	if (co->index >= nr_uarts)
 		co->index = 0;
-	up = &serial8250_ports[co->index];
+	port = &serial8250_ports[co->index].port;
 	/* link port to console */
-	up->port.cons = co;
+	port->cons = co;
 
-	return serial8250_console_setup(up, options);
+	return serial8250_console_setup(port, options, false);
 }
 
 /**
@@ -3491,7 +3507,8 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
 			continue;
 
 		co->index = i;
-		return univ8250_console_setup(co, options);
+		port->cons = co;
+		return serial8250_console_setup(port, options, true);
 	}
 
 	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);
-- 
2.3.5