[U-Boot] [PATCH] tegra: Specify debugging serial port at boot.

[Simon Glass]

Hi Stephen,

On Tue, Mar 20, 2012 at 2:17 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 03/20/2012 02:13 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Tue, Mar 20, 2012 at 12:57 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> This works together with a kernel change that looks at the scratchpad
>>> register to determine which of the many UARTs it should use for early
>>> printing:
>>>
>>> http://www.spinics.net/lists/arm-kernel/msg154633.html
>>>
>>> Note that this configuration only affects the kernel's decompressor and
>>> earlyprintk code. Once the kernel is initialized far enough to parse the
>>> device tree, the console is initialized using information contained
>>> therein.
>>>
>>> Base on work by Doug Anderson <dianders at chromium.org>, but significantly
>>> rewritten.
>>>
>>> Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
>>> ---
>>> Tom, this patch applies on top of u-boot-tegra/master, with the top 4
>>> commits removed and replaced with Simon's latest putc series from last
>>> night.
>>>
>>> Simon, I wonder if tegra_pre_console_panic() shouldn't be modified to
>>> remove the uart_ids parameter and simply call get_uart_ids() internally?
>>
>> I can't help thinking there is a bit of a misunderstanding here. I
>> suspected it from your comments yesterday, so let's try to clear it
>> up.
>
> It's not so much a misunderstanding as a disagreement over the right
> direction.

OK

>
>> This patch is intended (I believe) to signal to Linux which UART is
>> used for the earlyprintk console. This is so when U-Boot cannot
>> decompress the kernel (because we are booting a zImage which has its
>> own decompression wrapper) we can print the 'Uncompressing Linux...'
>> message. Is that correct?
>
> That's one thing it affects, yes.
>
> It also affects the DEBUG_LL/earlyprintk output, i.e. all the kernel
> spew (by the kernel itself, not the decompressor) that happens before
> the real console device is set up.
>
> (When there's no problem and earlyprintk isn't on, this is all buffered
> up and sent out the console when it appears, but if there's a panic
> before the console is initialized, you won't see any messages. If
> earlyprintk is enabled, the messages aren't buffered, but instead sent
> immediately using the DEBUG_LL mechanism, and this patch is what tells
> the DEBUG_LL mechanism which UART to use).

Yes that's true also.

>
>> If so, what you want to find out here is exactly which UART is being
>> used for the console. It will be a single UART, it will be know for
>> sure, whether the information comes from CONFIG or the device tree.
>
> True, implementing it that way would also be useful. Right now, that's
> driven by CONFIG_SYS_NS16550_COM*, but can come from DT too.

OK

>
>> For the pre-console panic stuff, the point is that there may well be
>> one UART used for the console, but *we don't know which one it is!*.
>
> That's where we disagree.
>
> For each board, there is a single fixed UART that should be used for
> both pre-console panic and the "real" console later. This is fixed by
> the single static board design.
>
> In other words: While other boards may be similar to Seaboard, they are
> not Seaboard, and we shouldn't be using the Seaboard board file for
> them. They are different boards that are simply similar in design. But
> for example, many use a different UART for their debug console - a clear
> difference in hardware.
>
> For each one of these derived boards, there is a single static UART that
> should be used, as determined by the board design.
>
> Thus, I argue that there really is no concept of "which UART do we use"
> *for a given board*.
>
> I think perhaps the way to clear this up is to make the existing
> Seaboard and Ventana boards NOT be device-tree-driven, but rather be
> "board-file" driven. Instead of co-opting Seaboard, introduce a new
> "tegra2dt" board that is purely DT driven. (This is exactly what's been
> done in the kernel). Then, it would make a lot more sense to talk about
> there being multiple possible UARTs to use for the console, since the
> board you build U-Boot for would then explicitly be for multiple boards,
> rather than hijacking a particular single board file to work on multiple
> boards. It'd also allow people to boot U-Boot on Seaboard without using
> DT if they wanted, at the very least as a temporary transition mechanism.

Know you of such people? But yes I am thinking beyond Seaboard and yes
we do need a 'generic' tegra2 board instead of using Seaboard. I am
not sure of the value of having separate boards at all though, since
with a the device tree there should be nothing in the board files that
is not generic. I suppose that is what has stopped me from actually
implementing a generic tegra2 board so far - in my mind it is just
Seaboard (or anything else if you prefer, since they are all the same
in that all differences are captured by the device tree).

But I agree we should do it.

>
> As an aside, for the tegra2dt board, perhaps we should have a single
> well-known location in the U-Boot binary that contains the "uart_ids"
> value. This can be initialized to "all UARTs" by default by the code (or
> perhaps "no UARTs" for safety). As part of the manual post-processing to
> convert this common u-boot.bin binary to a particular board prior to
> flashing, one could both (a) append the .dtb file just like is required
> now (b) patch that well-known location to set the specific UARTs for the
> panic putc, if desired. "Well known" could perhaps be determined using
> "nm" and searching for a specific symbol. A helper script could be
> provided to make this easy. What do you think of this?

Well it is sort-of similar to the problem we have with SPL. There we
are looking at a solution where there is a little table placed in SPL
which contains a few critical parameters from the device tree. It is
available to C code very early and could include the console UART
info. From the code's point of view it is just a global C structure
with some useful data. I can point you to this if you like.

>
>> So the concept of selecting a particular UART and writing out a
>> message is not really good enough. Sure it works on Seaboard, and I
>> adjusted my patch to only use UARTD, but doesn't work on a Seaboard
>> variant where we don't yet have the device tree and so don't know
>> which UART is used for the console. The device tree specifies the
>> console UART - it cannot be known at compile time. The best we can do
>> is specify the possible and legal options.
>>
>> Please don't get these two things (asking which is the console UART
>> and asking which UARTs could possible be the console) confused - I
>> realise that this is a bit mess. To be clear, the pre-console panic is
>> NOT:
>>
>> 1. Intended to just print a message on the console - that would be
>> easy if we actually knew which was the console; nor
>>
>> 2. Intended as a general-purpose pre-console message output interface
>> - there is already Graeme's pre-console buffer for that.
>>
>>>
>>> BTW, it seems inconsistent to have all the Tegra code key off
>>> CONFIG_TEGRA2_ENABLE_UART*, whereas the serial driver keys off
>>> CONFIG_SYS_NS16550_COM*. Can the two be unified somehow?

Both of these are not used when device trees are enabled, so it
doesn't much matter to me. But I'm sure we could do this in the
tegra2.h header file or similar.

Regards,
Smion