Re: ARC fails to boot on linux-next of 20140711

[Rob Herring]

On Wed, Jul 30, 2014 at 10:25 PM, Grant Likely
<grant.likely@xxxxxxxxxxxx> wrote:
> On Sat, 26 Jul 2014 15:10:28 -0500, Rob Herring <robherring2@xxxxxxxxx> wrote:
>> On Sat, Jul 26, 2014 at 11:50 AM, Grant Likely
>> <grant.likely@xxxxxxxxxxxx> wrote:
>> > On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <Vineet.Gupta1@xxxxxxxxxxxx> wrote:
>> >> Hi Rob,
>> >>
>> >> On Friday 25 July 2014 07:45 PM, Rob Herring wrote:
>> >> > On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
>> >> > <Vineet.Gupta1@xxxxxxxxxxxx> wrote:
>> >> >> > Hi Grant,
>> >> >> >
>> >> >> > linux-next has a series for arc_uart (via tty tree) which converts it to generic
>> >> >> > earlycon and specifies console via /chosen/stdout-path vs.  an explicit param in
>> >> >> > /chose/bootargs
>> >> >> >
>> >> >> > 2014-06-24 9da433c0a0b5 ARC: [arcfpga] stdout-path now suffices for earlycon/console
>> >> >> >
>> >> >> > This relied on prev commit of yours (from linux next of 20140711), which seem to
>> >> >> > have disappeared now.
>> >> >> >
>> >> >> > 2014-03-27 a9296cf2d0b6 of: Create of_console_check() for selecting a console
>> >> >> > specified in /chosen
>> >> >> > 2014-03-27 cfa9cacc5dd3 of: Enable console on serial ports specified by
>> >> >> > /chosen/stdout-path
>> >> >> >
>> >> >> > Is there a specific reason for dropping these patches (or perhaps a merge to be
>> >> >> > merged). I cherry-picked both but still doesn't work.
>> >> >> >
>> >> >> > Can you please advise next step forward, before I go off debugging with those
>> >> >> > patches in.
>> >> > There's an issue that if you have stdout-path and "earlycon" on the
>> >> > command line, the kernel will switch to tty0 and disable the earlycon.
>> >> >
>> >> > This is the "fix", but I don't like adding the DT dependency into generic code:
>> >> >
>> >> > @@ -2382,7 +2386,7 @@ void register_console(struct console *newcon)
>> >> >                 if (newcon->setup == NULL ||
>> >> >                     newcon->setup(newcon, NULL) == 0) {
>> >> >                         newcon->flags |= CON_ENABLED;
>> >> > -                       if (newcon->device) {
>> >> > +                       if (newcon->device  && !of_stdout) {
>> >> >                                 newcon->flags |= CON_CONSDEV;
>> >> >                                 preferred_console = 0;
>> >> >                         }
>> >>
>> >> The DT settings relevant for ARC, which enable generic-earlycon and
>> >> console-with-stdout-path are as follows
>> >>
>> >>         chosen {
>> >>                 bootargs = "earlycon";
>> >>                 stdout-path = &arcuart0;
>> >>         };
>> >>
>> >> ....
>> >>                 arcuart0: serial@c0fc1000 {
>> >>                         compatible = "snps,arc-uart";
>> >>
>> >> And it works w/o above hunk, provided the 2 patches from Grant exist in linux-next
>> >> which they don't at the moment. I'm pretty confused how the hunk above comes into
>> >> picture.
>> >>
>> >> And if not then I will have to get the ARC std-out patch reverted in tty-next as
>> >> it is broken.
>> >
>> > You need to revert it anyway, the dependency chain is broken. Just
>> > because something is in linux-next doesn't mean it will be merged.
>> > Dependencies must always be in the branch to which you commit.
>> >
>> > If that doesn't happen (like here) then bisecting is broken and the
>> > dependencies may not actually get merged.
>> >
>> > When this happens, what you're supposed to do is tell the maintainers
>> > what commits the patch depends on so that it can be applied to the
>> > correct tree. In this case I could take it through my devicetree branch
>> > that contains the console patches.
>> >
>> > If a patch depends on commits in several branches then it is a bit more
>> > complex. What we usually do is create a new branch that merges in each
>> > branch that is depended on, and then apply the commit on top of that.
>> >
>> > As for the console patches, I'm only going to be putting them back if I
>> > can devise a good fix for the earlycon duplication issue.
>>
>> There is also a simple work-around. You have to specify the console
>> when you use earlycon. For example, you can add "earlycon
>> console=ttyAMA0" instead of just earlycon. To summarize, this is the
>> state of combinations of console params:
>>
>> Working:
>> stdout-path
>> console=blah
>> stdout-path + console=blah
>> earlycon=blah
>>
>> Not working:
>> stdout-path + earlycon
>>
>> Also, it is a developer only feature which is new. You will be aware
>> of an issue because the earlycon starts output and then stops on
>> switching to tty0 versus complete silence which could be anything.
>
> Fair enough. Okay, I'm going to put it back into linux-next. The proper
> fix can come later.
>
>> There are other landmines in this area already. For example, setting
>> earlycon on ARM will break the boot because an ioremap is attempted
>> before paging_init. This is nothing new, but has always been the case
>> if the 8250 driver was enabled.
>
> We should not allow earlycon to be selectable on ARM until this is
> fixed. Can you do a patch for this?

There is not a config option as it is always enabled when the console
is enabled, and it is no different than many command line options that
could break booting. More importantly, earlycon does work on ARM with
semihosting since nothing needs to be mapped. And BTW, you can use
semihosting with QEMU.

Hopefully, we'll get fixmap support sorted out soon clearing this
minefield. Part of fixmap support is needed for UEFI early ioremap and
I think there's a few other things that could use it.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/