2014-07-25 11:02:54

by Vineet Gupta

[permalink] [raw]
Subject: ARC fails to boot on linux-next of 20140711

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.

Thx,
-Vineet


2014-07-25 11:26:34

by Vineet Gupta

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Friday 25 July 2014 04:32 PM, Vineet Gupta wrote:
> 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.

Correction - with these two, ARC boots again.

Thx,
-Vineet

2014-07-25 14:15:47

by Rob Herring

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
<[email protected]> 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;
}

Rob

2014-07-26 05:33:49

by Grant Likely

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Fri, 25 Jul 2014 09:15:22 -0500, Rob Herring <[email protected]> wrote:
> On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
> <[email protected]> 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:

Yes, I'm not fond of it either. I've not been able to test it though and
work out a proper bug fix. As far as I can understand, the earlycon code
only works on aarch64, correct? I haven't been able to get an aarch64
boot working in QEMU yet.

g.

>
> @@ -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;
> }
>
> Rob

2014-07-26 06:52:45

by Vineet Gupta

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

Hi Rob,

On Friday 25 July 2014 07:45 PM, Rob Herring wrote:
> On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
> <[email protected]> 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.

Thx,
-Vineet

2014-07-26 06:53:36

by Vineet Gupta

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Saturday 26 July 2014 11:03 AM, Grant Likely wrote:
> On Fri, 25 Jul 2014 09:15:22 -0500, Rob Herring <[email protected]> wrote:
>> On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
>> <[email protected]> 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:
> Yes, I'm not fond of it either. I've not been able to test it though and
> work out a proper bug fix. As far as I can understand, the earlycon code
> only works on aarch64, correct? I haven't been able to get an aarch64
> boot working in QEMU yet.
>
> g.

Well generic earlycon works on ARC too (as it stands today in linux-next). std-out
based console is broken because the relevant core/of fixes from you are no longer
present there. Can we get those back please !

-Vineet

>
>> @@ -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;
>> }
>>
>> Rob
>

2014-07-26 15:17:39

by Guenter Roeck

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On 07/25/2014 10:33 PM, Grant Likely wrote:
> On Fri, 25 Jul 2014 09:15:22 -0500, Rob Herring <[email protected]> wrote:
>> On Fri, Jul 25, 2014 at 6:02 AM, Vineet Gupta
>> <[email protected]> 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:
>
> Yes, I'm not fond of it either. I've not been able to test it though and
> work out a proper bug fix. As far as I can understand, the earlycon code
> only works on aarch64, correct? I haven't been able to get an aarch64
> boot working in QEMU yet.
>

Have a look at [1]. This worked for me with the latest qemu and the latest
pre-built root file system from [2].

Guenter

---
[1] http://www.bennee.com/~alex/blog/2014/05/09/running-linux-in-qemus-aarch64-system-emulation-mode/
[2] http://people.linaro.org/~alex.bennee/images/aarch64-linux-3.16rc3-buildroot.img

2014-07-26 16:51:05

by Grant Likely

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
> > <[email protected]> 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.

g.

2014-07-26 20:10:53

by Rob Herring

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Sat, Jul 26, 2014 at 11:50 AM, Grant Likely
<[email protected]> wrote:
> On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
>> > <[email protected]> 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.
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.

Rob

2014-07-28 04:38:46

by Vineet Gupta

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Saturday 26 July 2014 10:21 PM, Grant Likely wrote:
> On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
>>> <[email protected]> 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.

I understand all of that. Actually 9 out of 10 of my changes were for switching to
earlycon for which corresponding core changes from Rob were already in mainline.
When reviewing my code, Rob pointed me to your on going work on use of stdout-path
so my mistake was to pile that up in the same series vs. asking you to add that
patch in ur tree for precisely the multi dependency reason.

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

If you have an internal branch for this work, can you please add the patch below
to it so that we don't forget abt it.

https://lkml.org/lkml/2014/6/24/127

I'll ask Greg to revert it from tty tree !

Thx,
-Vineet

2014-07-30 23:53:55

by Grant Likely

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Mon, 28 Jul 2014 04:38:38 +0000, Vineet Gupta <[email protected]> wrote:
> On Saturday 26 July 2014 10:21 PM, Grant Likely wrote:
> > On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
> >>> <[email protected]> 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.
>
> I understand all of that. Actually 9 out of 10 of my changes were for switching to
> earlycon for which corresponding core changes from Rob were already in mainline.
> When reviewing my code, Rob pointed me to your on going work on use of stdout-path
> so my mistake was to pile that up in the same series vs. asking you to add that
> patch in ur tree for precisely the multi dependency reason.
>
> > 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.
>
> If you have an internal branch for this work, can you please add the patch below
> to it so that we don't forget abt it.
>
> https://lkml.org/lkml/2014/6/24/127
>
> I'll ask Greg to revert it from tty tree !

Isn't that also broken for bisect? Doesn't that patch depend on the
patches in Greg's tree? If so, then same problem. At this point you'll
need to wait until both are mainlined before this patch gets applied
since I don't want to deal with cross merging trees.

However, this patch is trivial, so I don't see any problem with not
having it in linux-next before the merge window.

g.

2014-07-31 03:25:38

by Grant Likely

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Sat, 26 Jul 2014 15:10:28 -0500, Rob Herring <[email protected]> wrote:
> On Sat, Jul 26, 2014 at 11:50 AM, Grant Likely
> <[email protected]> wrote:
> > On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
> >> > <[email protected]> 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?

g.

2014-07-31 15:13:15

by Rob Herring

[permalink] [raw]
Subject: Re: ARC fails to boot on linux-next of 20140711

On Wed, Jul 30, 2014 at 10:25 PM, Grant Likely
<[email protected]> wrote:
> On Sat, 26 Jul 2014 15:10:28 -0500, Rob Herring <[email protected]> wrote:
>> On Sat, Jul 26, 2014 at 11:50 AM, Grant Likely
>> <[email protected]> wrote:
>> > On Sat, 26 Jul 2014 06:52:36 +0000, Vineet Gupta <[email protected]> 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
>> >> > <[email protected]> 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