| Message ID | 1461099580-3866-48-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Rejected |
| Delegated to: | Tom Warren |
| Headers | show |
Hi Stephen, On 19 April 2016 at 14:59, Stephen Warren <swarren@wwwdotorg.org> wrote: > From: Stephen Warren <swarren@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@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@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
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) +{ + 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; +}