On Thu, Oct 23, 2014 at 11:25 AM, Kevin Cernekee [off-list ref] wrote:

On Wed, Oct 22, 2014 at 2:27 AM, Rob Herring [off-list ref] wrote:

quoted

On Wed, Oct 22, 2014 at 6:23 AM, Kevin Cernekee [off-list ref] wrote:

quoted

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.

quoted

quoted

 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.

quoted

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