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

[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

-- 
2.1.1

[PATCH V3 01/10] tty: serial: bcm63xx: Allow bcm63xx_uart to be built on other platforms

[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 <cernekee@gmail.com>
Acked-by: Florian Fainelli <f.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.
-- 
2.1.1

[PATCH V3 02/10] tty: serial: bcm63xx: Add support for unnamed clock outputs from DT

[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 <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;
 
-- 
2.1.1

[PATCH V3 03/10] tty: serial: bcm63xx: Update the Kconfig help text

[Kevin Cernekee]

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

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Acked-by: Florian Fainelli <f.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
-- 
2.1.1

[PATCH V3 04/10] tty: serial: bcm63xx: Fix typo in MODULE_DESCRIPTION

[Kevin Cernekee]

Remove the extra '<' character.

Signed-off-by: Kevin Cernekee <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 <mbizon@freebox.fr>");
-MODULE_DESCRIPTION("Broadcom 63<xx integrated uart driver");
+MODULE_DESCRIPTION("Broadcom 63xx integrated uart driver");
 MODULE_LICENSE("GPL");
-- 
2.1.1

[PATCH V3 05/10] Documentation: DT: Add entries for bcm63xx UART

[Kevin Cernekee]

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

Signed-off-by: Kevin Cernekee <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: serial@14e00520 {
+		compatible = "brcm,bcm6345-uart";
+		reg = <0x14e00520 0x18>;
+		interrupt-parent = <&periph_intc>;
+		interrupts = <2>;
+		clocks = <&periph_clk>;
+	};
+
+	clocks {
+		periph_clk: periph_clk@0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <54000000>;
+		};
+	};
-- 
2.1.1

[PATCH V3 06/10] tty: serial: bcm63xx: Enable DT earlycon support

[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 <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
-- 
2.1.1

[PATCH V3 07/10] tty: serial: bcm63xx: Eliminate unnecessary request/release functions

[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 <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 */
-- 
2.1.1

[PATCH V3 08/10] tty: serial: of-serial: Suppress warnings if OF earlycon is invoked twice

[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 <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
-- 
2.1.1

[PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[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 <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"
-- 
2.1.1

Re: [PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[Rob Herring]

On Wed, Oct 22, 2014 at 6:23 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> 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.

>
> Signed-off-by: Kevin Cernekee <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

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

> +
>         /* 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
>

Re: [PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[Kevin Cernekee]

On Wed, Oct 22, 2014 at 2:27 AM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Oct 22, 2014 at 6:23 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>> 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?

>>  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.

> 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?

Re: [PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[Rob Herring]

On Thu, Oct 23, 2014 at 11:25 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> On Wed, Oct 22, 2014 at 2:27 AM, Rob Herring <robh@kernel.org> wrote:
>> On Wed, Oct 22, 2014 at 6:23 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>>> 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.

>>>  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.
>
>> 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.

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

>  - 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.

> 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

Re: [PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[Kevin Cernekee]

On Wed, Oct 22, 2014 at 9:47 PM, Rob Herring <robh@kernel.org> wrote:
>> 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.

>> 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...

Re: [PATCH V3 09/10] tty: serial: of-serial: Allow OF earlycon to default to "on"

[Kevin Cernekee]

On Thu, Oct 23, 2014 at 7:31 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
>> 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.

Hi Rob,

Do you think we should just drop this patch for now, and use these
options to append "earlycon" to the bootloader-supplied command line:

CONFIG_CMDLINE_BOOL=y
CONFIG_CMDLINE="earlycon"
CONFIG_CMDLINE_OVERRIDE=n

If you have other suggestions on how to enable earlycon by default, I
would be happy to help implement them.

Thanks!

[PATCH V3 10/10] MAINTAINERS: Add entry for rp2 (Rocketport Express/Infinity) driver

[Kevin Cernekee]

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

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Acked-by: Florian Fainelli <f.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 <cernekee@gmail.com>
+L:	linux-serial@vger.kernel.org
+S:	Odd Fixes
+F:	drivers/tty/serial/rp2.*
+
 ROSE NETWORK LAYER
 M:	Ralf Baechle <ralf@linux-mips.org>
 L:	linux-hams@vger.kernel.org
-- 
2.1.1

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

[Greg KH]

On Tue, Oct 21, 2014 at 03:22:56PM -0700, Kevin Cernekee wrote:
> 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

I've applied 8 of these patches, not patches 08 or 09 at this time.

thanks,

greg k-h