[U-Boot] [PATCH v3 0/14] split tegra20 arm7 code into separate SPL

Simon Glass
2012-06-09T19:44:17Z

Hi Allen,

On Fri, Jun 8, 2012 at 2:15 PM, Allen Martin <amartin@nvidia.com> wrote:

> This patch series fixes a long standing problem with the tegra20
> u-boot build. Tegra20 contains an ARM7TDMI boot processor and a
> Cortex A9 main processor. Prior to this patch series this was
> accomplished by #ifdefing out any armv7 code from the early boot
> sequence and creating a single binary that runs on both both the
> ARM7TDMI and A9. This was very fragile as changes to compiler options
> or any additions or rearranging of the early boot code could add
> additional armv7 specific code causing it to fail on the ARM7TDMI.

This patch series pulls all the armv4t code out into a separate SPL
> that does nothing more than initialize the A9 and transfer control to
>

Sorry I am so late in looking at this again.

Is there a reason why SPL doesn't also load U-Boot?

> it. The resultint SPL and armv7 u-boot are concatenated together into
> a single image.

This would depend on the needs of a particular build system, I suppose. So
the original files are still there if needed.

I have made a few other comments in the series which I hope are useful.
Certainly you are right that this is fragile at present (where you must
sometimes update the compiler flags for particular U-Boot files using
tegra's config.mk), but I wonder if we should provide the ability to do a
full SPL instead of just a way of building part of U-Boot with different
compiler flags! It seems like a lot of work and change, plus the additional
complexity, and the cpu/tegra20-common.

To be clear, the only difficulty you actually have (and I believe the
entire problem you are solving with this series) is the very first part of
board_init_f(). The code there is something like this:

bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");

/* Pointer is writable since we allocated a register for it */
gd = (gd_t *) ((CONFIG_SYS_INIT_SP_ADDR) & ~0x07);
/* compiler optimization barrier needed for GCC >= 3.4 */
__asm__ __volatile__("": : :"memory");

memset((void *)gd, 0, sizeof(gd_t));

This can cause problems since the compiler might put v7 instructions in the
memcpy().

gd->mon_len = _bss_end_ofs;
#ifdef CONFIG_OF_EMBED
/* Get a pointer to the FDT */
gd->fdt_blob = _binary_dt_dtb_start;
#elif defined CONFIG_OF_SEPARATE
/* FDT is at end of image */
gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
#endif
/* Allow the early environment to override the fdt address */
gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
(uintptr_t)gd->fdt_blob);

This is the cause of a recent problem (the call to getenv_ulong())

for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
if ((*init_fnc_ptr)() != 0) {
hang ();
}
}

The very first call in this init sequence is arch_cpu_init(), and from then
all is well.

So in fact another solution would be to call arch_cpu_init() from start.S,
or right at the start of board_init_f(). This might well be an acceptable
change, and certainly would be simpler than moving to SPL. Maybe you could
even add a new CONFIG_START_MAIN_CPU to control calling a better named
function which does the Tegra init.

Another solution could be to simply call the A9 init from start.S as we
originally did. Then you could be sure that no other files are used.
Originally this logic was pretty contorted, but if we just use the code out
of arch_cpu_init() it need not be.

However I suspect there are other reasons for wanting to move to SPL, and
these reasons are actually driving this. So I am looking forward to your
comments.

>
> Changes:
> v3:
> - git bisect still does not work across this series, I'm saving that
> for the next revision, but I had enough changes that I wanted to get
> this out for review
> - expanded the tegra2 -> tegra20 rename to include functions/variables/
> defines
> - rebased to u-boot-tegra/next
> - removed some extra -march=armv4t flags, kept armv4t flags on
> warmboot_avp since it's special
> - removed bashisms from mkconfig
> - renamed CONFIG_MACH_TEGRA_GENERIC to CONFIG_TEGRA
> - moved SPL overrides to tegra2-common-post.h
> - changed SPL base address to 0x108000, u-boot goes to 0x208000
> - moved warboot_save_sdram_params fix to separate patch
> - remove USE_PRIVATE_LIBGCC from non SPL build
> - expanded SPL support to all tegra20 boards, not just seaboard
> v2:
> - renamed tegra2 to tegra20 to match kernel and devicetree naming
> policy
> - pulled all SPL related config overrides to a separate file to clean
> up ifdefs from seabard.h
> - rebased to TOT u-boot/master and fixed a bug related to init
> sequence changes between this patch series and new EMC code
> - made u-boot.t2 target work even if CONFIG_OF is disabled
> - added back USE_PRIVATE_LIBGCC
>
> [PATCH 01/14] tegra20: rename tegra2 -> tegra20
> [PATCH 02/14] tegra20: move tegra20 SoC code to
> [PATCH 03/14] tegra20: rename CONFIG_MACH_TEGRA_GENERIC
> [PATCH 04/14] mkconfig: add support for SPL CPU
> [PATCH 05/14] ARM: Fix arm720t SPL build
> [PATCH 06/14] tegra20: remove timer_init from SPL build
> [PATCH 07/14] ARM: add tegra20 support to arm720t
> [PATCH 08/14] tegra20: add SPL config options for tegra20 boards
> [PATCH 09/14] tegra20: add u-boot.t2 target
> [PATCH 10/14] tegra: move SDRAM param save to later in boot
> [PATCH 11/14] tegra20: Remove CPU init code from tegra20 u-boot
> [PATCH 12/14] tegra20: Remove armv4t build flags
> [PATCH 13/14] tegra20: plutux: change obj directory mkdir commands
> [PATCH 14/14] tegra20: tec: add tegra20-common-post.h
>
> .gitignore | 1 +
> MAINTAINERS | 18 +--
> Makefile | 14 ++
> arch/arm/cpu/arm720t/cpu.c | 2 +
> arch/arm/cpu/arm720t/interrupts.c | 4 +
> arch/arm/cpu/arm720t/start.S | 19 ++-
> .../arm/cpu/arm720t/tegra20}/Makefile | 27 ++--
> .../arch-tegra2 => cpu/arm720t/tegra20}/board.h | 11 +-
> .../{armv7/tegra2/ap20.c => arm720t/tegra20/cpu.c} | 147
> ++------------------
> .../ap20.h => cpu/arm720t/tegra20/cpu.h} | 14 +-
> arch/arm/cpu/arm720t/tegra20/spl.c | 133
> ++++++++++++++++++
> arch/arm/cpu/armv7/start.S | 2 -
> arch/arm/cpu/armv7/{tegra2 => tegra20}/Makefile | 15 +-
> .../cpu/armv7/{tegra2 => tegra20}/cmd_enterrcm.c | 4 +-
> arch/arm/cpu/armv7/{tegra2 => tegra20}/config.mk | 12 +-
> arch/arm/cpu/armv7/{tegra2 => tegra20}/usb.c | 2 +-
> .../cpu/{armv7/tegra2 => tegra20-common}/Makefile | 32 ++---
> arch/arm/cpu/tegra20-common/ap20.c | 131
> +++++++++++++++++
> .../cpu/{armv7/tegra2 => tegra20-common}/board.c | 44 ++----
> .../cpu/{armv7/tegra2 => tegra20-common}/clock.c | 10 +-
> .../cpu/{armv7/tegra2 => tegra20-common}/crypto.c | 0
> .../cpu/{armv7/tegra2 => tegra20-common}/crypto.h | 0
> .../arm/cpu/{armv7/tegra2 => tegra20-common}/emc.c | 2 +-
> .../cpu/{armv7/tegra2 => tegra20-common}/funcmux.c | 2 +-
> .../tegra2 => tegra20-common}/lowlevel_init.S | 0
> .../cpu/{armv7/tegra2 => tegra20-common}/pinmux.c | 4 +-
> .../arm/cpu/{armv7/tegra2 => tegra20-common}/pmu.c | 2 +-
> .../{armv7/tegra2 => tegra20-common}/sys_info.c | 2 +-
> .../cpu/{armv7/tegra2 => tegra20-common}/timer.c | 2 +-
> .../{armv7/tegra2 => tegra20-common}/warmboot.c | 18 +--
> .../tegra2 => tegra20-common}/warmboot_avp.c | 4 +-
> .../tegra2 => tegra20-common}/warmboot_avp.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/ap20.h | 4 +-
> .../asm/{arch-tegra2 => arch-tegra20}/apb_misc.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/board.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/clk_rst.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/clock.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/emc.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/flow.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/funcmux.h | 2 +-
> .../asm/{arch-tegra2 => arch-tegra20}/fuse.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/gp_padctrl.h | 2 +-
> .../asm/{arch-tegra2 => arch-tegra20}/gpio.h | 2 +-
> arch/arm/include/asm/arch-tegra20/hardware.h | 29 ++++
> .../asm/{arch-tegra2 => arch-tegra20}/mmc.h | 8 +-
> .../asm/{arch-tegra2 => arch-tegra20}/pinmux.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/pmc.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/pmu.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/scu.h | 0
> .../{arch-tegra2 => arch-tegra20}/sdram_param.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/sys_proto.h | 4 +-
> .../tegra2.h => arch-tegra20/tegra20.h} | 18 +--
> .../asm/{arch-tegra2 => arch-tegra20}/tegra_i2c.h | 2 +-
> .../asm/{arch-tegra2 => arch-tegra20}/tegra_spi.h | 4 +-
> .../asm/{arch-tegra2 => arch-tegra20}/timer.h | 6 +-
> .../uart-spi-switch.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/uart.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/usb.h | 0
> .../asm/{arch-tegra2 => arch-tegra20}/warmboot.h | 0
> board/avionic-design/common/tamonten.c | 4 +-
> .../dts/{tegra2-medcom.dts => tegra20-medcom.dts} | 0
> .../dts/{tegra2-plutux.dts => tegra20-plutux.dts} | 0
> .../dts/{tegra2-tec.dts => tegra20-tec.dts} | 0
> board/avionic-design/plutux/Makefile | 7 +-
> .../dts/{tegra2-paz00.dts => tegra20-paz00.dts} | 0
> board/compal/paz00/paz00.c | 6 +-
> ...{tegra2-trimslice.dts => tegra20-trimslice.dts} | 0
> board/compulab/trimslice/trimslice.c | 6 +-
> board/nvidia/common/board.c | 14 +-
> board/nvidia/common/emc.c | 2 +-
> board/nvidia/common/uart-spi-switch.c | 2 +-
> .../{tegra2-harmony.dts => tegra20-harmony.dts} | 2 +-
> .../{tegra2-seaboard.dts => tegra20-seaboard.dts} | 0
> .../{tegra2-ventana.dts => tegra20-ventana.dts} | 2 +-
> .../{tegra2-whistler.dts => tegra20-whistler.dts} | 2 +-
> board/nvidia/harmony/harmony.c | 6 +-
> board/nvidia/seaboard/config.mk | 1 +
> board/nvidia/seaboard/seaboard.c | 6 +-
> board/nvidia/whistler/whistler.c | 6 +-
> boards.cfg | 23 +--
> doc/README.SPL | 12 ++
> drivers/gpio/tegra_gpio.c | 12 +-
> drivers/i2c/tegra_i2c.c | 12 +-
> drivers/input/Makefile | 2 +-
> drivers/mmc/tegra_mmc.c | 32 ++---
> drivers/mmc/tegra_mmc.h | 12 +-
> drivers/spi/tegra_spi.c | 10 +-
> include/configs/harmony.h | 14 +-
> include/configs/medcom.h | 12 +-
> include/configs/paz00.h | 12 +-
> include/configs/plutux.h | 12 +-
> include/configs/seaboard.h | 20 +--
> include/configs/tec.h | 12 +-
> ...{tegra2-common-post.h => tegra20-common-post.h} | 73 +++++++++-
> .../configs/{tegra2-common.h => tegra20-common.h} | 44 +++---
> include/configs/trimslice.h | 14 +-
> include/configs/ventana.h | 12 +-
> include/configs/whistler.h | 14 +-
> include/fdtdec.h | 12 +-
> include/serial.h | 2 +-
> mkconfig | 13 +-
> spl/Makefile | 4 +
> 102 files changed, 734 insertions(+), 476 deletions(-)
>
>
> --
> nvpublic
>

Regards,
Simon