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

[Simon Glass]

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.

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?

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.

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!*.
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?
>
>  arch/arm/cpu/armv7/tegra2/board.c |   31 +++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
> index c7ea96b..27831f3 100644
> --- a/arch/arm/cpu/armv7/tegra2/board.c
> +++ b/arch/arm/cpu/armv7/tegra2/board.c
> @@ -115,7 +115,7 @@ void tegra_setup_uarts(int uart_ids)
>        }
>  }
>
> -void board_init_uart_f(void)
> +static int get_uart_ids(void)
>  {
>        int uart_ids = 0;       /* bit mask of which UART ids to enable */
>
> @@ -128,7 +128,13 @@ void board_init_uart_f(void)
>  #ifdef CONFIG_TEGRA2_ENABLE_UARTD
>        uart_ids |= TEGRA_UARTD;
>  #endif
> -       tegra_setup_uarts(uart_ids);
> +
> +       return uart_ids;
> +}

If you want this, then I suggest tegra_get_console_uart_id() might be
a better, to avoid confusion. It can just return an enum.

> +
> +void board_init_uart_f(void)
> +{
> +       tegra_setup_uarts(get_uart_ids());
>  }
>
>  #ifndef CONFIG_SYS_DCACHE_OFF
> @@ -150,6 +156,27 @@ static const u32 uart_reg_addr[TEGRA_UART_COUNT] = {
>        NV_PA_APB_UARTD_BASE,
>  };
>
> +void arch_preboot_os(void)
> +{
> +       int uart_ids, i;
> +
> +       /*
> +        * The Linux kernel earlyprintk code detects which UART to use by
> +        * searching for one with 'D' in the UART scratch register. This
> +        * is set up here.
> +        */
> +       uart_ids = get_uart_ids();
> +
> +       for (i = 0; i < TEGRA_UART_COUNT; i++) {

I feel there should only be a single UART here. After all, what is
Linux supposed to do in early printk if it seems more than one UART?

> +               if (uart_ids & (1 << i)) {
> +                       NS16550_t regs = (NS16550_t)uart_reg_addr[i];
> +                       if (!regs)
> +                               continue;
> +                       writeb('D', &regs->spr);
> +               }
> +       }
> +}
> +
>  void tegra_pre_console_panic(int uart_ids, unsigned clock_freq,
>                             unsigned multiplier, const char *str)
>  {
> --
> 1.7.0.4
>

Regards,
Simon