Generic serial earlycon

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Quoted 7 lines

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Hi Greg, Rob

This one in tty-next breaks booting: "console=uart8250,io,0x3f8,115200". No early console and regular console anymore.

It should produce "early con and regular con".

Quoted 19 lines · Yinghai Lu

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Quoted 7 lines

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Hi Greg, Rob

This one in tty-next breaks booting: "console=uart8250,io,0x3f8,115200". No early console and regular console anymore.

Quoted 1 line

It should produce "early con and regular con".

This is what I get for running checkpatch and converting simple_strtoul to kstrto* which are not so equivalent. kstrto* will not convert things such as 115200n8 to a number.

Greg, Do you want a fix or for me to respin the series? The fix looks like this:

Quoted 22 lines · Rob Herring

Quoted 20 lines · Yinghai Lu

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Quoted 7 lines

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Hi Greg, Rob

This one in tty-next breaks booting: "console=uart8250,io,0x3f8,115200". No early console and regular console anymore.

It should produce "early con and regular con".

Quoted 5 lines

This is what I get for running checkpatch and converting simple_strtoul to kstrto* which are not so equivalent. kstrto* will not convert things such as 115200n8 to a number.

Greg, Do you want a fix or for me to respin the series? The fix looks like this:

As this is in my tree now (right?), I need a fix to apply to it.

Quoted 15 lines

Quoted 15 lines

Quoted 10 lines

Quoted 3 lines

 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Hi Greg, Rob

This one in tty-next breaks booting: "console=uart8250,io,0x3f8,115200". No early console and regular console anymore.

It should produce "early con and regular con".

This is what I get for running checkpatch and converting simple_strtoul to kstrto* which are not so equivalent. kstrto* will not convert things such as 115200n8 to a number.

Quoted 3 lines

Quoted 1 line

Greg, Do you want a fix or for me to respin the series? The fix looks like this:

As this is in my tree now (right?), I need a fix to apply to it.

Nevermind, you already sent it...

Linus' tree (HEAD=963649d735c8) is spitting out all sorts of garbage on the serial console - as if the baud rate is being periodically set to something weird, and then back to 112500. I see over a dozen blocks of junk interspersed with good output.

Quoted 4 lines · Tony Luck

Linus' tree (HEAD=963649d735c8) is spitting out all sorts of garbage on the serial console - as if the baud rate is being periodically set to something weird, and then back to 112500. I see over a dozen blocks of junk interspersed with good output.

Ah - it seems that I need to be more specific on the cmdline

     console=uart8250,io,0x3f8,115200

makes all the garbage disappear.

Do I need to update: Documentation/ia64/serial.txt which has the example of "console=uart,io,0x3f8"?

It should do auto detect of the baud-rate if it is not specified which for 8250 is just reading the baud-rate divider and calculating the baud-rate using the uart clock. It should just reprogram the divider with the same divider value. I don't see anything obvious why this would have broken.

Quoted 5 lines · Rob Herring

It should do auto detect of the baud-rate if it is not specified which for 8250 is just reading the baud-rate divider and calculating the baud-rate using the uart clock. It should just reprogram the divider with the same divider value. I don't see anything obvious why this would have broken.

Something very weird is happening. Output is good so long as I put the trailing ",115200" on the command line.

But I made early_serial8250_setup() printk() the return value it got from probe_baud() - in case is was somehow getting 115201 or some other silly value ... nope. Exactly 115200.

I also can't explain why the "noise" comes and goes a dozen times during boot,

Does some other place in the kernel look at the "uart..." command line argument?

Quoted 5 lines · Rob Herring

It should do auto detect of the baud-rate if it is not specified which for 8250 is just reading the baud-rate divider and calculating the baud-rate using the uart clock. It should just reprogram the divider with the same divider value. I don't see anything obvious why this would have broken.

Something very weird is happening. Output is good so long as I put the trailing ",115200" on the command line.

But I made early_serial8250_setup() printk() the return value it got from probe_baud() - in case is was somehow getting 115201 or some other silly value ... nope. Exactly 115200.

I also can't explain why the "noise" comes and goes a dozen times during boot,

Does some other place in the kernel look at the "uart..." command line argument?

I think I figured it out. The function serial8250_find_port_for_earlycon is failing to match 'uart' console to 'ttyS' console and transfer the options. It was also failing to create an option string when the baud rate is probed. Can you try out the below patch?

On 06/10/2014 03:52 PM, Tony Luck wrote:
I think I figured it out. The function serial8250_find_port_for_earlycon is failing to match 'uart' console to 'ttyS' console and transfer the options. It was also failing to create an option string when the baud rate is probed. Can you try out the below patch?

Rob

8<-------------------------------------------------------------------

drivers/tty/serial/8250/8250_early.c +4 -1

index cfef801..4858b8a 100644
@@ -144,8 +144,11 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
        if (!(device->port.membase || device->port.iobase))
                return 0;
 
-       if (!device->baud)
+       if (!device->baud) {
                device->baud = probe_baud(&device->port);
+               snprintf(device->options, sizeof(device->options), "%u",
+                        device->baud);
+       }
 
        init_port(device);
 

drivers/tty/serial/earlycon.c +0 -0

index 5131b5e..d20e6d8 100644

Quoted 17 lines · Greg Kroah-Hartman

Quoted 17 lines

Quoted 16 lines

Quoted 10 lines

Quoted 3 lines

 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

Hi Greg, Rob

This one in tty-next breaks booting: "console=uart8250,io,0x3f8,115200". No early console and regular console anymore.

It should produce "early con and regular con".

This is what I get for running checkpatch and converting simple_strtoul to kstrto* which are not so equivalent. kstrto* will not convert things such as 115200n8 to a number.

Greg, Do you want a fix or for me to respin the series? The fix looks like this:

Quoted 3 lines

Quoted 1 line

As this is in my tree now (right?), I need a fix to apply to it.

Nevermind, you already sent it...

Linus' tree (HEAD=963649d735c8) is spitting out all sorts of garbage on the serial console - as if the baud rate is being periodically set to something weird, and then back to 112500. I see over a dozen blocks of junk interspersed with good output.

This in on ia64 with "console=uart,io,0x3f8" on the command line.

git bisect pointed me here:

d2fd6810a823bcde1ee232668f5689837aafa772 is first bad commit commit d2fd6810a823bcde1ee232668f5689837aafa772

Author: Rob Herring <[email protected]>
Date:   Fri Apr 18 17:19:56 2014 -0500

tty/serial: convert 8250 to generic earlycon

-Tony

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

drivers/tty/serial/8250/8250_early.c +0 -0

index c100d63..e83c9db 100644
@@ -35,18 +35,8 @@

Quoted 18 lines · Rob Herring

From: Rob Herring <[email protected]>

With the generic earlycon infrastructure in place, convert the 8250 early console to use it.

Signed-off-by Rob Herring <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

---
 drivers/tty/serial/8250/8250_early.c | 138 ++++-------------------------------
 drivers/tty/serial/8250/Kconfig      |   1 +
 2 files changed, 16 insertions(+), 123 deletions(-)

drivers/tty/serial/8250/8250_early.c +0 -0

index c100d63..e83c9db 100644
@@ -35,18 +35,8 @@

...

drivers/tty/serial/8250/8250_early.c

Quoted 2 lines

drivers/tty/serial/8250/8250_early.c +0 -2

-
-int __init setup_early_serial8250_console(char *cmdline)

You removed global function, but still left declaration and calling...

arch/mips/mti-malta/malta-init.c:
setup_early_serial8250_console(console_string);
drivers/firmware/pcdp.c: return setup_early_serial8250_console(options);
include/linux/serial_8250.h:extern int
setup_early_serial8250_console(char *cmdline);

On Sat, Apr 26, 2014 at 1:29 AM, Yinghai Lu <[email protected]> wrote:

Thanks for finding these. I missed them in my build tests. This should fix them:

drivers/tty/serial/8250/8250_early.c +5 -0

index e83c9db..2094c3b 100644
@@ -156,6 +156,11 @@ 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, "uart8250", early_serial8250_setup);
+}
+
 int serial8250_find_port_for_earlycon(void)
 {

        struct earlycon_device *device = early_device;

Quoted 14 lines · Rob Herring

On Sat, Apr 26, 2014 at 1:29 AM, Yinghai Lu <[email protected]> wrote:

Thanks for finding these. I missed them in my build tests. This should fix them:

drivers/tty/serial/8250/8250_early.c +0 -0

index e83c9db..2094c3b 100644
@@ -156,6 +156,11 @@ static int __init early_serial8250_setup(struct
 earlycon_device *device,
 EARLYCON_DECLARE(uart8250, early_serial8250_setup);
 EARLYCON_DECLARE(uart, early_serial8250_setup);
 

Quoted 8 lines

drivers/tty/serial/8250/8250_early.c +5 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       return setup_earlycon(cmdline, "uart8250", early_serial8250_setup);
+}
+
 int serial8250_find_port_for_earlycon(void)
 {

        struct earlycon_device *device = early_device;

that only handle "uart8250,", may need to add more lines to handle "uart,"

(quoted diff) +6 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       char *options;
+       options = strstr(cmdline, "uart8250,");
+       if (options)
+               return setup_earlycon(cmdline, "uart8250",

early_serial8250_setup);

(quoted diff) +8 -0

+
+       options = strstr(cmdline, "uart,");
+       if (options)
+              return setup_earlycon(cmdline, "uart", early_serial8250_setup);
+
+      return 0;
+}
+

Thanks

Yinghai

Quoted 15 lines · Yinghai Lu

Quoted 14 lines · Rob Herring

On Sat, Apr 26, 2014 at 1:29 AM, Yinghai Lu <[email protected]> wrote:

Thanks for finding these. I missed them in my build tests. This should fix them:

drivers/tty/serial/8250/8250_early.c +0 -0

index e83c9db..2094c3b 100644
@@ -156,6 +156,11 @@ static int __init early_serial8250_setup(struct
 earlycon_device *device,
 EARLYCON_DECLARE(uart8250, early_serial8250_setup);
 EARLYCON_DECLARE(uart, early_serial8250_setup);
 

Quoted 10 lines

Quoted 8 lines

drivers/tty/serial/8250/8250_early.c +5 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       return setup_earlycon(cmdline, "uart8250", early_serial8250_setup);
+}
+
 int serial8250_find_port_for_earlycon(void)
 {

        struct earlycon_device *device = early_device;

that only handle "uart8250,", may need to add more lines to handle "uart,"

That is on purpose because the only 2 users use uart8250. I consider this a legacy interface and use of "uart" is horrible because there are lots of uarts which are not 8250.

Rob

Quoted 1 line

Quoted 7 lines

(quoted diff) +6 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       char *options;
+       options = strstr(cmdline, "uart8250,");
+       if (options)
+               return setup_earlycon(cmdline, "uart8250",

early_serial8250_setup);

Review

Quoted 7 lines

(quoted diff) +7 -0

+
+       options = strstr(cmdline, "uart,");
+       if (options)
+              return setup_earlycon(cmdline, "uart", early_serial8250_setup);
+
+      return 0;
+}

Quoted 35 lines · Rob Herring

Quoted 15 lines · Yinghai Lu

Quoted 14 lines · Rob Herring

On Sat, Apr 26, 2014 at 1:29 AM, Yinghai Lu <[email protected]> wrote:

Thanks for finding these. I missed them in my build tests. This should fix them:

drivers/tty/serial/8250/8250_early.c +0 -0

index e83c9db..2094c3b 100644
@@ -156,6 +156,11 @@ static int __init early_serial8250_setup(struct
 earlycon_device *device,
 EARLYCON_DECLARE(uart8250, early_serial8250_setup);
 EARLYCON_DECLARE(uart, early_serial8250_setup);
 

Quoted 10 lines

Quoted 8 lines

drivers/tty/serial/8250/8250_early.c +5 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       return setup_earlycon(cmdline, "uart8250", early_serial8250_setup);
+}
+
 int serial8250_find_port_for_earlycon(void)
 {

        struct earlycon_device *device = early_device;

that only handle "uart8250,", may need to add more lines to handle "uart,"

That is on purpose because the only 2 users use uart8250. I consider this a legacy interface and use of "uart" is horrible because there are lots of uarts which are not 8250.

Rob

Quoted 1 line

Quoted 14 lines

Quoted 7 lines

(quoted diff) +6 -0

+int __init setup_early_serial8250_console(char *cmdline)
+{
+       char *options;
+       options = strstr(cmdline, "uart8250,");
+       if (options)
+               return setup_earlycon(cmdline, "uart8250",

early_serial8250_setup);

Review

Quoted 7 lines

(quoted diff) +7 -0

+
+       options = strstr(cmdline, "uart,");
+       if (options)
+              return setup_earlycon(cmdline, "uart", early_serial8250_setup);
+
+      return 0;
+}

You want to obsolete "console=uart,io,0x3f8,115200n8" ?

Let's check with Andrew. He suggested to use uart and uart8250 at that time.

Quoted 4 lines · Yinghai Lu

You want to obsolete "console=uart,io,0x3f8,115200n8" ?

Let's check with Andrew. He suggested to use uart and uart8250 at that time.

No, that is not what I'm saying. For the 2 callers of setup_early_serial8250_console which are crafting a console string from firmware data, they can and do use uart8250. I don't expect this mechanism for setting up early console to expand to other users. The whole point of this series is to allow any uart to be supported for earlycon. For anyone using the kernel command line, both uart and uart8250 are still supported.

Rob Herring (7):
x86: move FIX_EARLYCON_MEM kconfig into x86
tty/serial: add generic serial earlycon
tty/serial: convert 8250 to generic earlycon
tty/serial: pl011: add generic earlycon support
tty/serial: add arm/arm64 semihosting earlycon
arm64: enable FIX_EARLYCON_MEM kconfig
arm64: remove arch specific earlyprintk

Quoted 8 lines · Rob Herring

Rob Herring (7):
x86: move FIX_EARLYCON_MEM kconfig into x86
tty/serial: add generic serial earlycon
tty/serial: convert 8250 to generic earlycon
tty/serial: pl011: add generic earlycon support
tty/serial: add arm/arm64 semihosting earlycon
arm64: enable FIX_EARLYCON_MEM kconfig
arm64: remove arch specific earlyprintk

The series looks fine, you can add:

Acked-by Catalin Marinas <[email protected]>

BTW, are you merging all of them via some other tree or would prefer me to take the arm64-specific patches?