[PATCH V3 00/10] bcm63xx_uart and of-serial updates

[Kevin Cernekee]

V2->V3:

Change DT clock node based on review feedback (thanks Arnd!)

Rebase on Linus' master branch


Kevin Cernekee (10):
tty: serial: bcm63xx: Allow bcm63xx_uart to be built on other
platforms
tty: serial: bcm63xx: Add support for unnamed clock outputs from DT
tty: serial: bcm63xx: Update the Kconfig help text
tty: serial: bcm63xx: Fix typo in MODULE_DESCRIPTION
Documentation: DT: Add entries for bcm63xx UART
tty: serial: bcm63xx: Enable DT earlycon support
tty: serial: bcm63xx: Eliminate unnecessary request/release functions
tty: serial: of-serial: Suppress warnings if OF earlycon is invoked
twice
tty: serial: of-serial: Allow OF earlycon to default to "on"
MAINTAINERS: Add entry for rp2 (Rocketport Express/Infinity) driver

.../devicetree/bindings/serial/bcm63xx-uart.txt | 30 ++++++++++++
MAINTAINERS | 6 +++
drivers/of/fdt.c | 17 +++++--
drivers/tty/serial/Kconfig | 30 ++++++++----
drivers/tty/serial/bcm63xx_uart.c | 55 +++++++++++++---------
include/linux/serial_bcm63xx.h | 2 -
6 files changed, 104 insertions(+), 36 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/bcm63xx-uart.txt

[Kevin Cernekee]

This device was originally supported on bcm63xx only, but it shows up on
a wide variety of MIPS and ARM chipsets spanning multiple product lines.
Now that the driver has eliminated dependencies on bcm63xx-specific
header files, we can build it on any non-bcm63xx kernel.

Compile-tested on x86, both statically and as a module. Tested for
functionality on bcm3384 (a new MIPS platform under active development).

Signed-off-by: Kevin Cernekee <***@gmail.com>
Acked-by: Florian Fainelli <***@gmail.com>
---
drivers/tty/serial/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 649b784..4a5c0c8 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1283,7 +1283,7 @@ config SERIAL_TIMBERDALE
config SERIAL_BCM63XX
tristate "bcm63xx serial port support"
select SERIAL_CORE
- depends on BCM63XX
+ depends on MIPS || ARM || COMPILE_TEST
help
If you have a bcm63xx CPU, you can enable its onboard
serial port by enabling this options.

[Kevin Cernekee]

The original non-DT bcm63xx clk code ignores the struct device argument
and looks up a global clock name. DT platforms, by contrast, often just
use a phandle to reference a clock node with no "clock-output-names"
property. Modify the UART driver to support both schemes.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/tty/serial/bcm63xx_uart.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 2315190..de95573 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -824,7 +824,8 @@ static int bcm_uart_probe(struct platform_device *pdev)
if (!res_irq)
return -ENODEV;

- clk = clk_get(&pdev->dev, "periph");
+ clk = pdev->dev.of_node ? of_clk_get(pdev->dev.of_node, 0) :
+ clk_get(&pdev->dev, "periph");
if (IS_ERR(clk))
return -ENODEV;

[Kevin Cernekee]

Remove incorrect "bcm963xx_uart" module name; add a list of known users;
tweak grammar/indentation/capitalization.

Signed-off-by: Kevin Cernekee <***@gmail.com>
Acked-by: Florian Fainelli <***@gmail.com>
---
drivers/tty/serial/Kconfig | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4a5c0c8..815b652 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1281,22 +1281,24 @@ config SERIAL_TIMBERDALE
Add support for UART controller on timberdale.

config SERIAL_BCM63XX
- tristate "bcm63xx serial port support"
+ tristate "Broadcom BCM63xx/BCM33xx UART support"
select SERIAL_CORE
depends on MIPS || ARM || COMPILE_TEST
help
- If you have a bcm63xx CPU, you can enable its onboard
- serial port by enabling this options.
+ This enables the driver for the onchip UART core found on
+ the following chipsets:

- To compile this driver as a module, choose M here: the
- module will be called bcm963xx_uart.
+ BCM33xx (cable modem)
+ BCM63xx/BCM63xxx (DSL)
+ BCM68xx (PON)
+ BCM7xxx (STB) - DOCSIS console

config SERIAL_BCM63XX_CONSOLE
- bool "Console on bcm63xx serial port"
+ bool "Console on BCM63xx serial port"
depends on SERIAL_BCM63XX=y
select SERIAL_CORE_CONSOLE
help
- If you have enabled the serial port on the bcm63xx CPU
+ If you have enabled the serial port on the BCM63xx CPU
you can make it the console by answering Y to this option.

config SERIAL_GRLIB_GAISLER_APBUART

[Kevin Cernekee]

Remove the extra '<' character.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/tty/serial/bcm63xx_uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index de95573..b615af2 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -906,5 +906,5 @@ module_init(bcm_uart_init);
module_exit(bcm_uart_exit);

MODULE_AUTHOR("Maxime Bizon <***@freebox.fr>");
-MODULE_DESCRIPTION("Broadcom 63<xx integrated uart driver");
+MODULE_DESCRIPTION("Broadcom 63xx integrated uart driver");
MODULE_LICENSE("GPL");

[Kevin Cernekee]

This squashes a checkpatch warning on my new bcm3384 dts submission.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
.../devicetree/bindings/serial/bcm63xx-uart.txt | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/bcm63xx-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt b/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
new file mode 100644
index 0000000..5c52e5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
@@ -0,0 +1,30 @@
+* BCM63xx UART
+
+Required properties:
+
+- compatible: "brcm,bcm6345-uart"
+
+- reg: The base address of the UART register bank.
+
+- interrupts: A single interrupt specifier.
+
+- clocks: Clock driving the hardware; used to figure out the baud rate
+ divisor.
+
+Example:
+
+ uart0: ***@14e00520 {
+ compatible = "brcm,bcm6345-uart";
+ reg = <0x14e00520 0x18>;
+ interrupt-parent = <&periph_intc>;
+ interrupts = <2>;
+ clocks = <&periph_clk>;
+ };
+
+ clocks {
+ periph_clk: ***@0 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <54000000>;
+ };
+ };

[Kevin Cernekee]

We don't really need to perform the ioremap "on demand" so it's simpler
just to do it from the probe function. This also lets us eliminate the
UART_REG_SIZE constant and rely on the resource information passed in
from the DT or platform code.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/tty/serial/bcm63xx_uart.c | 30 ++++++++++--------------------
include/linux/serial_bcm63xx.h | 2 --
2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 109dea7..e04e580 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -588,20 +588,7 @@ static void bcm_uart_set_termios(struct uart_port *port,
*/
static int bcm_uart_request_port(struct uart_port *port)
{
- unsigned int size;
-
- size = UART_REG_SIZE;
- if (!request_mem_region(port->mapbase, size, "bcm63xx")) {
- dev_err(port->dev, "Memory region busy\n");
- return -EBUSY;
- }
-
- port->membase = ioremap(port->mapbase, size);
- if (!port->membase) {
- dev_err(port->dev, "Unable to map registers\n");
- release_mem_region(port->mapbase, size);
- return -EBUSY;
- }
+ /* UARTs always present */
return 0;
}

@@ -610,8 +597,7 @@ static int bcm_uart_request_port(struct uart_port *port)
*/
static void bcm_uart_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, UART_REG_SIZE);
- iounmap(port->membase);
+ /* Nothing to release ... */
}

/*
@@ -833,13 +819,20 @@ static int bcm_uart_probe(struct platform_device *pdev)
if (pdev->id < 0 || pdev->id >= BCM63XX_NR_UARTS)
return -EINVAL;

- if (ports[pdev->id].membase)
+ port = &ports[pdev->id];
+ if (port->membase)
return -EBUSY;
+ memset(port, 0, sizeof(*port));

res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res_mem)
return -ENODEV;

+ port->mapbase = res_mem->start;
+ port->membase = devm_ioremap_resource(&pdev->dev, res_mem);
+ if (IS_ERR(port->membase))
+ return PTR_ERR(port->membase);
+
res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res_irq)
return -ENODEV;
@@ -849,10 +842,7 @@ static int bcm_uart_probe(struct platform_device *pdev)
if (IS_ERR(clk))
return -ENODEV;

- port = &ports[pdev->id];
- memset(port, 0, sizeof(*port));
port->iotype = UPIO_MEM;
- port->mapbase = res_mem->start;
port->irq = res_irq->start;
port->ops = &bcm_uart_ops;
port->flags = UPF_BOOT_AUTOCONF;
diff --git a/include/linux/serial_bcm63xx.h b/include/linux/serial_bcm63xx.h
index a80aa1a..570e964 100644
--- a/include/linux/serial_bcm63xx.h
+++ b/include/linux/serial_bcm63xx.h
@@ -116,6 +116,4 @@
UART_FIFO_PARERR_MASK | \
UART_FIFO_BRKDET_MASK)

-#define UART_REG_SIZE 24
-
#endif /* _LINUX_SERIAL_BCM63XX_H */

[Kevin Cernekee]

Specifying "earlycon earlycon" on the kernel command line yields this
warning:

bootconsole [uart0] enabled
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/printk/printk.c:2391 register_console+0x244/0x3fc()
console 'uart0' already registered
CPU: 0 PID: 0 Comm: swapper Not tainted 3.18.0-rc1+ #2
Stack : 00000000 00000004 80af0000 80af0000 00000000 00000000 00000000 00000000
80ad4e12 00000036 00000000 00000000 00010000 805abe88 805606b4 805abae7
00000000 00000000 80ad38d8 805abe88 8055f304 43d42d03 9988c6a1 804e2710
805b0000 80032854 00000000 00000000 8056492c 80599c84 80599c84 805606b4
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
...
Call Trace:
[<8001a22c>] show_stack+0x64/0x7c
[<804e47d8>] dump_stack+0xc8/0xfc
[<80032aa8>] warn_slowpath_common+0x7c/0xac
[<80032b38>] warn_slowpath_fmt+0x2c/0x38
[<80076524>] register_console+0x244/0x3fc
[<805d8314>] of_setup_earlycon+0x74/0x98
[<805daa40>] early_init_dt_scan_chosen_serial+0x104/0x134
[<805c51a0>] do_early_param+0xc4/0x13c
[<8004efa0>] parse_args+0x284/0x444
[<805c56cc>] parse_early_options+0x34/0x40
[<805c5714>] parse_early_param+0x3c/0x58
[<805c87a4>] setup_arch+0xec/0x6e4
[<805c57d4>] start_kernel+0x94/0x458

---[ end trace dc8fa200cb88537f ]---

In this case the duplicate "earlycon" was entered directly, but there are
other cases where this could happen inadvertently:

- Some platforms allow user bootargs to be concatenated with builtin
bootargs, e.g. CONFIG_CMDLINE_EXTEND.

- Other platforms may want to hardwire earlycon to ON, so it isn't
nice if a user manually specifying "earlycon" on the command line sees
a big scary warning.

So, we will treat "earlycon" as a flag, and if happens to be requested
multiple times the kernel will not print any warnings.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/of/fdt.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8..20193cc 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -755,6 +755,11 @@ int __init early_init_dt_scan_chosen_serial(void)
int l;
const struct of_device_id *match = __earlycon_of_table;
const void *fdt = initial_boot_params;
+ static int done;
+
+ if (done)
+ return -EBUSY;
+ done = 1;

offset = fdt_path_offset(fdt, "/chosen");
if (offset < 0)
@@ -792,10 +797,9 @@ int __init early_init_dt_scan_chosen_serial(void)

static int __init setup_of_earlycon(char *buf)
{
- if (buf)
- return 0;
-
- return early_init_dt_scan_chosen_serial();
+ if (!buf)
+ early_init_dt_scan_chosen_serial();
+ return 0;
}
early_param("earlycon", setup_of_earlycon);
#endif

[Kevin Cernekee]

This enables early console output if there is a chosen/stdout-path
property referencing a UART node with the "brcm,bcm6345-uart" compatible
string. The bootloader sets up the pinmux and baud/parity/etc.
Tested on bcm3384 (MIPS, DT).

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/bcm63xx_uart.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 815b652..fdd851e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1297,6 +1297,7 @@ config SERIAL_BCM63XX_CONSOLE
bool "Console on BCM63xx serial port"
depends on SERIAL_BCM63XX=y
select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
help
If you have enabled the serial port on the BCM63xx CPU
you can make it the console by answering Y to this option.
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index b615af2..109dea7 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -782,6 +782,26 @@ static int __init bcm63xx_console_init(void)

console_initcall(bcm63xx_console_init);

+static void bcm_early_write(struct console *con, const char *s, unsigned n)
+{
+ struct earlycon_device *dev = con->data;
+
+ uart_console_write(&dev->port, s, n, bcm_console_putchar);
+ wait_for_xmitr(&dev->port);
+}
+
+static int __init bcm_early_console_setup(struct earlycon_device *device,
+ const char *opt)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ device->con->write = bcm_early_write;
+ return 0;
+}
+
+OF_EARLYCON_DECLARE(bcm63xx_uart, "brcm,bcm6345-uart", bcm_early_console_setup);
+
#define BCM63XX_CONSOLE (&bcm63xx_console)
#else
#define BCM63XX_CONSOLE NULL

[Kevin Cernekee]

On many development systems it is very common to see failures during the
early stages of the boot process, e.g. SMP boot or PCIe initialization.
This is one likely reason why some existing earlyprintk implementations,
such as arch/mips/kernel/early_printk.c, are enabled unconditionally
at compile time.

Now that earlycon's operating parameters can be passed into the kernel
via DT, it is helpful to be able to configure the kernel to turn it on
automatically. Introduce a new CONFIG_SERIAL_EARLYCON_FORCE option for
this purpose.

Signed-off-by: Kevin Cernekee <***@gmail.com>
---
drivers/of/fdt.c | 5 +++++
drivers/tty/serial/Kconfig | 11 +++++++++++
2 files changed, 16 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 20193cc..3e2ea1e 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1013,6 +1013,11 @@ bool __init early_init_dt_verify(void *params)

void __init early_init_dt_scan_nodes(void)
{
+#ifdef CONFIG_SERIAL_EARLYCON_FORCE
+ if (early_init_dt_scan_chosen_serial() < 0)
+ pr_warn("Unable to set up earlycon from stdout-path\n");
+#endif
+
/* Retrieve various information from the /chosen node */
of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index fdd851e..bc4ebcc 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -14,6 +14,17 @@ config SERIAL_EARLYCON
the console before standard serial driver is probed. The console is
enabled when early_param is processed.

+config SERIAL_EARLYCON_FORCE
+ bool "Always enable early console"
+ depends on SERIAL_EARLYCON
+ help
+ Traditionally, enabling the early console has required passing in
+ the "earlycon" parameter on the kernel command line. On systems
+ under development it may be desirable to enable earlycon
+ unconditionally rather than to force the user to manually add it
+ to the boot argument string, as boot failures often occur before
+ the standard serial driver is probed.
+
source "drivers/tty/serial/8250/Kconfig"

comment "Non-8250 serial port support"

[Rob Herring]

Post by Kevin Cernekee
On many development systems it is very common to see failures during the
early stages of the boot process, e.g. SMP boot or PCIe initialization.
This is one likely reason why some existing earlyprintk implementations,
such as arch/mips/kernel/early_printk.c, are enabled unconditionally
at compile time.
Now that earlycon's operating parameters can be passed into the kernel
via DT, it is helpful to be able to configure the kernel to turn it on
automatically. Introduce a new CONFIG_SERIAL_EARLYCON_FORCE option for
this purpose.

You can already force this using the CMDLINE_EXTEND option. I'm not
sure we need more options.

Post by Kevin Cernekee
---
drivers/of/fdt.c | 5 +++++
drivers/tty/serial/Kconfig | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 20193cc..3e2ea1e 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1013,6 +1013,11 @@ bool __init early_init_dt_verify(void *params)
void __init early_init_dt_scan_nodes(void)
{
+#ifdef CONFIG_SERIAL_EARLYCON_FORCE
+ if (early_init_dt_scan_chosen_serial() < 0)
+ pr_warn("Unable to set up earlycon from stdout-path\n");
+#endif

Doesn't this make the earlycon get scanned and setup twice? Hopefully
that is safe...

This also introduces the scanning at another point in time during boot
which may not work depending on the arch.

Rob

Post by Kevin Cernekee
+
/* Retrieve various information from the /chosen node */
of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index fdd851e..bc4ebcc 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -14,6 +14,17 @@ config SERIAL_EARLYCON
the console before standard serial driver is probed. The console is
enabled when early_param is processed.
+config SERIAL_EARLYCON_FORCE
+ bool "Always enable early console"
+ depends on SERIAL_EARLYCON
+ help
+ Traditionally, enabling the early console has required passing in
+ the "earlycon" parameter on the kernel command line. On systems
+ under development it may be desirable to enable earlycon
+ unconditionally rather than to force the user to manually add it
+ to the boot argument string, as boot failures often occur before
+ the standard serial driver is probed.
+
source "drivers/tty/serial/8250/Kconfig"
comment "Non-8250 serial port support"
--
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Kevin Cernekee]

Post by Rob Herring

Post by Kevin Cernekee
On many development systems it is very common to see failures during the
early stages of the boot process, e.g. SMP boot or PCIe initialization.
This is one likely reason why some existing earlyprintk implementations,
such as arch/mips/kernel/early_printk.c, are enabled unconditionally
at compile time.
Now that earlycon's operating parameters can be passed into the kernel
via DT, it is helpful to be able to configure the kernel to turn it on
automatically. Introduce a new CONFIG_SERIAL_EARLYCON_FORCE option for
this purpose.

You can already force this using the CMDLINE_EXTEND option. I'm not
sure we need more options.

Hi Rob,

Now that earlycon can get all of its parameters from DT, do you think
it might make sense to drop the command line option entirely from
fdt.c and enable it all of the time if stdout-path is set correctly?

Post by Rob Herring
From the user's standpoint, how important is it to be able to run

without earlycon?

Post by Rob Herring

Post by Kevin Cernekee
void __init early_init_dt_scan_nodes(void)
{
+#ifdef CONFIG_SERIAL_EARLYCON_FORCE
+ if (early_init_dt_scan_chosen_serial() < 0)
+ pr_warn("Unable to set up earlycon from stdout-path\n");
+#endif

Doesn't this make the earlycon get scanned and setup twice? Hopefully
that is safe...

Patch 08/10 makes it safe. Without Patch 08/10, specifying "earlycon
earlycon" also generates a backtrace.

Post by Rob Herring
This also introduces the scanning at another point in time during boot
which may not work depending on the arch.

Currently the sequence looks like:

- arch code calls early_init_dt_scan() to populate boot_command_line
and memory ranges

- arch code might do some other stuff, possibly setting up page
tables, register mappings, etc.

- arch code calls parse_early_param() to look at the final command line

- parse_early_param() might call early_init_dt_scan_chosen_serial()

So we're assuming that the arch code knows not to call
parse_early_param() until the mappings are configured. But this is an
implicit requirement, and might not be totally obvious. Since
SERIAL_EARLYCON is enabled by the UART driver, not the arch code, it
is possible that some platforms have ordering issues here that won't
be discovered until somebody tries to use earlycon.

Would it be more straightforward to have the arch code explicitly call
early_init_dt_scan_chosen_serial() to indicate that it is ready for
the early UART driver to run?
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rob Herring]

Post by Kevin Cernekee

Post by Rob Herring

Post by Kevin Cernekee
On many development systems it is very common to see failures during the
early stages of the boot process, e.g. SMP boot or PCIe initialization.
This is one likely reason why some existing earlyprintk implementations,
such as arch/mips/kernel/early_printk.c, are enabled unconditionally
at compile time.
Now that earlycon's operating parameters can be passed into the kernel
via DT, it is helpful to be able to configure the kernel to turn it on
automatically. Introduce a new CONFIG_SERIAL_EARLYCON_FORCE option for
this purpose.

You can already force this using the CMDLINE_EXTEND option. I'm not
sure we need more options.

Hi Rob,
Now that earlycon can get all of its parameters from DT, do you think
it might make sense to drop the command line option entirely from
fdt.c and enable it all of the time if stdout-path is set correctly?
From the user's standpoint, how important is it to be able to run
without earlycon?

It may affect boot time for one although if you care you probably
disable console altogether.

I think we'd just have to add a noearlycon option instead if we made
it the default. It's never been the default before, so I don't think
we should change now. There's also an implicit requirement that the
bootloader has configured the uart already. You could easily hang if
the uart has not been setup.

Post by Kevin Cernekee

Post by Rob Herring

Post by Kevin Cernekee
void __init early_init_dt_scan_nodes(void)
{
+#ifdef CONFIG_SERIAL_EARLYCON_FORCE
+ if (early_init_dt_scan_chosen_serial() < 0)
+ pr_warn("Unable to set up earlycon from stdout-path\n");
+#endif

Doesn't this make the earlycon get scanned and setup twice? Hopefully
that is safe...

Patch 08/10 makes it safe. Without Patch 08/10, specifying "earlycon
earlycon" also generates a backtrace.

Post by Rob Herring
This also introduces the scanning at another point in time during boot
which may not work depending on the arch.

- arch code calls early_init_dt_scan() to populate boot_command_line
and memory ranges
- arch code might do some other stuff, possibly setting up page
tables, register mappings, etc.

Yes, like the page table needed to map your earlycon uart.

Post by Kevin Cernekee
- arch code calls parse_early_param() to look at the final command line
- parse_early_param() might call early_init_dt_scan_chosen_serial()
So we're assuming that the arch code knows not to call
parse_early_param() until the mappings are configured. But this is an
implicit requirement, and might not be totally obvious. Since
SERIAL_EARLYCON is enabled by the UART driver, not the arch code, it
is possible that some platforms have ordering issues here that won't
be discovered until somebody tries to use earlycon.

Right, if you enable earlycon and the architecture doesn't support it,
you will crash. This is not really new. Doing "earlycon=uart8250..."
on ARM will crash as long as the 8250 driver has supported that
option.

Post by Kevin Cernekee
Would it be more straightforward to have the arch code explicitly call
early_init_dt_scan_chosen_serial() to indicate that it is ready for
the early UART driver to run?

Yes, but then when do you handle earlycon command line option for
non-DT case? Having these at different points in time is asking for
problems.

Also, I've been trying to reduce the number of DT hooks into the arch
code, so adding that would be moving in the wrong direction.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Kevin Cernekee]

Post by Rob Herring

Post by Kevin Cernekee
Now that earlycon can get all of its parameters from DT, do you think
it might make sense to drop the command line option entirely from
fdt.c and enable it all of the time if stdout-path is set correctly?
From the user's standpoint, how important is it to be able to run
without earlycon?

It may affect boot time for one although if you care you probably
disable console altogether.
I think we'd just have to add a noearlycon option instead if we made
it the default. It's never been the default before, so I don't think
we should change now. There's also an implicit requirement that the
bootloader has configured the uart already. You could easily hang if
the uart has not been setup.

EARLY_PRINTK is enabled by default on MIPS platforms that support it.
I have been using this for years and found it very helpful in
debugging failures in the kernel boot process (especially intermittent
ones).

But for a new platform it is better to use earlycon as earlycon
provides a clean way of reading the UART information out of DT. My
original bcm3384 (MIPS) port used EARLY_PRINTK but I recently
converted it to earlycon. The last remaining goal is to get it
enabled by default again.

Post by Rob Herring

Post by Kevin Cernekee
Would it be more straightforward to have the arch code explicitly call
early_init_dt_scan_chosen_serial() to indicate that it is ready for
the early UART driver to run?

Yes, but then when do you handle earlycon command line option for
non-DT case? Having these at different points in time is asking for
problems.

Is it important for the non-DT "earlycon" option to be handled from
parse_early_param() just because parse_early_param() runs at the right
point in the boot sequence (not because we actually wanted it to be a
command line option)?

One other option might be to invoke early_init_dt_scan_chosen_serial()
from parse_early_param(). But it is somewhat clunky to put a special
case in there...
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Kevin Cernekee]

I wrote this driver and use it daily on several machines for work, so
why not.

Signed-off-by: Kevin Cernekee <***@gmail.com>
Acked-by: Florian Fainelli <***@gmail.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a20df9b..d483627 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7787,6 +7787,12 @@ S: Maintained
F: Documentation/serial/rocket.txt
F: drivers/tty/rocket*

+ROCKETPORT EXPRESS/INFINITY DRIVER
+M: Kevin Cernekee <***@gmail.com>
+L: linux-***@vger.kernel.org
+S: Odd Fixes
+F: drivers/tty/serial/rp2.*
+
ROSE NETWORK LAYER
M: Ralf Baechle <***@linux-mips.org>
L: linux-***@vger.kernel.org