[lkml]   [2016]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards

On 05-11-16 11:40, Paul Burton wrote:
> On Friday, 4 November 2016 14:22:17 GMT Hans de Goede wrote:
>> Hi,
>> On 04-11-16 13:30, Paul Burton wrote:
>>> Hi Hans,
>>> On Friday, 4 November 2016 13:11:34 GMT Hans de Goede wrote:
>>>> Hi All,
>>>> While booting 4.9-rc# for the first time on an Allwinner A33 tablet,
>>>> I noticed that after u-boot the LCD display stayed black. It turns out
>>>> that there was an issue which caused X to never get up, and all kernel
>>>> (and other startup) messages prior to that only went to ttyS0 which
>>>> consists of 2 tiny testpads on the PCB with this tablet.
>>>> The same issue will also happen on any ARM boards which have a HDMI or
>>>> composite video output and which use a stdout-path pointing to their
>>>> serial console. I think this will e.g. also impact the Raspberry Pi,
>>>> I know for certain that this will impact the 99 different Allwinnner
>>>> boards currently supported by mainline u-boot + the mainline kernel.
>>>> This is a behavior changes from previous kernels and I consider this
>>>> a regression. Thus I propose to revert the commit in question, for more
>>>> info here is a partial copy of the commit message of the proposed revert:
>>>> The reverted commit changes existing behavior on which many ARM boards
>>>> rely. Many ARM small-board-computers, like e.g. the Raspberry Pi have
>>>> both a video output and a serial console. Depending on whether the user
>>>> is using the device as a more regular computer; or as a headless device
>>>> we need to have the console on either one or the other.
>>>> Many users rely on the kernel behavior of the console being present on
>>>> both outputs, before the reverted commit the console setup with no
>>>> console= kernel arguments on an ARM board which sets stdout-path in dt
>>>> would look like this:
>>>> [root@localhost ~]# cat /proc/consoles
>>>> ttyS0 -W- (EC p a) 4:64
>>>> tty0 -WU (E p ) 4:1
>>>> Where as after the reverted commit, it looks like this:
>>>> [root@localhost ~]# cat /proc/consoles
>>>> ttyS0 -W- (EC p a) 4:64
>>>> This commit reverts commit 05fd007e4629 ("console: don't prefer first
>>>> registered if DT specifies stdout-path") restoring the original behavior.
>>>> Regards,
>>>> Hans
>>> Ugh... so the devices you're talking about rely upon set stdout-path in
>>> their device tree but effectively rely upon us ignoring it?
>> No they rely on the kernel using stdout-path as an extra console while
>> keeping tty0 as console, not ignoring it. This how stdout-path has always
>> worked (at least as long as the Allwinner boards have used it, which has
>> been 2 - 3 years now).
>> If you want only the console specified by stdout-path you can get this by
>> specifying it with console=... on the kernel cmdline.
>>> If that's the case then I guess reverting is probably the best option, but
>>> it does restore us to a position where we honor stdout-path for earlycon
>>> & then essentially ignore it for the proper kernel console. That seems
>>> pretty bust to me...
>> We do not ignore it, we use both the tty pointed to by stdout-path and tty0.
>> Regards,
>> Hans
> Hi Hans,
> Could you walk me though how you're getting that behaviour from the current
> code? I don't see how that would happen besides perhaps if drivers are probed
> in a fortunate order. Is that what you're relying upon?

I guess so, I never looked carefully at this, it has just always worked
until your patch.

> What I see in my systems, and what 05fd007e4629 ("console: don't prefer first
> registered if DT specifies stdout-path") addressed, is that if there are for
> example 2 UARTs uart0 & uart1 that are probed in that order and stdout-path
> indicates that we should use uart1 we wind up essentially ignoring it because
> the ordering of the relevant calls goes:
> - of_console_check() for uart0
> - add_preferred_console() for uart0
> - register_console() for uart0
> - of_console_check() for uart1
> - add_preferred_console() for uart1
> - register_console() for uart1
> Since of_check_console() doesn't get called for uart1 until after uart0 has
> been probed, we don't add an entry for it to the console_cmdline array until
> after register_console() has already decided to enable uart0 because
> preferred_console == -1.
> I'm not the only one seeing this oddity either, for example see the discussion
> on this patch:
> By simply reverting my patch you restore us to a position where so far as I
> can see we simply do not honor stdout-path for the real kernel console.

As said before, we do still honor it, but in your probe example we also get
a (second) serial console on uart0, where as you only want one on uart1.

So I see a few possible solutions here:

1) Do a new version of your patch which changes the "&& !of_specified_console"
check to "&& (newcon == tty0 || !of_specified_console)", then we would still
always register tty0 (as long as it gets registered first, just like now) and
we would not register uart0 in your above example, note the "newcon == tty0"
check in my example is pseudo-code. I would be happy to try out such a patch

2) Add a new dt property to enable the new behavior you seek

I'm myself tending towards 1 as a better solution: treat tty0 special, because
some existing setups rely on it always being registered as a console even if
stdout-path is specified and otherwise always honor stdout-path.



 \ /
  Last update: 2016-11-06 11:55    [W:0.064 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site