[U-Boot] [PATCH 47/60] ARM: tegra: provide API for SPL code to init UART

[Simon Glass]

Hi Stephen,

On 19 April 2016 at 14:59, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> Currently, SPL console initialization on Tegra suffers from two problems:
>
> 1) It's a monolithic function that knows about all possibilities using
> tables and ifdefs set by board config.h, and contained in core files that
> are always built into U-Boot. Some of the code can't be ported to future
> SoCs since the clock APIs will be different. Equally, future SoCs don't
> need the code since earlier boot FW will always initialized the UART.
>
> 2) It's unnecessarily invoked twice, once by SPL and once by the main
> U-Boot binary.
>
> This patch adds simpler APIs to initialize the UART from SPL. This code
> can be omitted from non-SPL builds.
>
> A future patch will add the code to the Makefile when board files are
> converted. Adding it now would cause duplicate symbols for the UART
> device itself which will cause the link to fail. Even if that were
> resolved by changing the symbol name, duplicate UART devices would be
> registered, which would likely cause runtime problems.
>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
>  arch/arm/mach-tegra/include/mach/spl_uart.h | 20 +++++++++++++++
>  arch/arm/mach-tegra/spl_uart.c              | 39 +++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 arch/arm/mach-tegra/include/mach/spl_uart.h
>  create mode 100644 arch/arm/mach-tegra/spl_uart.c

Reviewed-by: Simon Glass <sjg at chromium.org>

One thought below.

>
> diff --git a/arch/arm/mach-tegra/include/mach/spl_uart.h b/arch/arm/mach-tegra/include/mach/spl_uart.h
> new file mode 100644
> index 000000000000..28b14acce93b
> --- /dev/null
> +++ b/arch/arm/mach-tegra/include/mach/spl_uart.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _MACH_SPL_UART_H
> +#define _MACH_SPL_UART_H
> +
> +enum tegra_spl_uart {
> +       TEGRA_SPL_UART_A,
> +       TEGRA_SPL_UART_B,
> +       TEGRA_SPL_UART_C,
> +       TEGRA_SPL_UART_D,
> +       TEGRA_SPL_UART_E,
> +};
> +
> +void tegra_spl_setup_uart(enum tegra_spl_uart uart_id);
> +
> +#endif
> diff --git a/arch/arm/mach-tegra/spl_uart.c b/arch/arm/mach-tegra/spl_uart.c
> new file mode 100644
> index 000000000000..2c1d237174fd
> --- /dev/null
> +++ b/arch/arm/mach-tegra/spl_uart.c
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright (c) 2014-2016, NVIDIA CORPORATION. All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <ns16550.h>
> +#include <asm/arch/clock.h>
> +#include <mach/spl_uart.h>
> +
> +static struct ns16550_platdata ns16550_com1_pdata = {
> +       .reg_shift = 2,
> +       .clock = CONFIG_SYS_NS16550_CLK,
> +};
> +
> +U_BOOT_DEVICE(ns16550_com1) = {
> +       "ns16550_serial", &ns16550_com1_pdata
> +};
> +
> +static const struct {
> +       unsigned long addr;
> +       enum periph_id periph_id;
> +} uart_info[] = {
> +       { NV_PA_APB_UARTA_BASE, PERIPH_ID_UART1, },
> +       { NV_PA_APB_UARTB_BASE, PERIPH_ID_UART2, },
> +       { NV_PA_APB_UARTC_BASE, PERIPH_ID_UART3, },
> +       { NV_PA_APB_UARTD_BASE, PERIPH_ID_UART4, },
> +       { NV_PA_APB_UARTE_BASE, PERIPH_ID_UART5, },
> +};
> +
> +void tegra_spl_setup_uart(unsigned int uart_id)

I wonder whether we could eliminate the uart_id concept at some point?

> +{
> +       if (uart_id >= ARRAY_SIZE(uart_info))
> +               return;
> +       clock_ll_start_uart(uart_info[uart_id].periph_id);
> +       ns16550_com1_pdata.base = uart_info[uart_id].addr;
> +}
> --
> 2.8.1
>

Regards,
Simon