[U-Boot] [PATCH 09/14] armv8/ls1043ardb: Add nand boot support

Scott Wood
2015-09-17T19:25:41Z

On Wed, 2015-09-16 at 04:25 -0500, Gong Qianyu-B52263 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, September 16, 2015 6:33 AM
> > To: Gong Qianyu-B52263
> > Cc: u-boot at lists.denx.de; Xie Shaohui-B21989; Hou Zhiqiang-B48286; Hu
> > Mingkai-B21284; Song Wenbin-B53747; Sun York-R58495
> > Subject: Re: [U-Boot] [PATCH 09/14] armv8/ls1043ardb: Add nand boot
> > support
> >
> > [Added York Sun -- please CC him on future patches]
> >
> > On Tue, 2015-09-15 at 06:47 -0500, Gong Qianyu-B52263 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Tuesday, September 15, 2015 7:08 AM
> > > > To: Gong Qianyu-B52263
> > > > Cc: u-boot at lists.denx.de; Xie Shaohui-B21989; Hou Zhiqiang-B48286;
> > > > Hu Mingkai-B21284; Song Wenbin-B53747
> > > > Subject: Re: [U-Boot] [PATCH 09/14] armv8/ls1043ardb: Add nand boot
> > > > support
> > > >
> > > > On Fri, 2015-09-11 at 19:07 +0800, Gong Qianyu wrote:
> > > > > Signed-off-by: Gong Qianyu <Qianyu.Gong@freescale.com>
> > > > > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > > > > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > > > > Signed-off-by: Mingkai Hu <Mingkai.Hu@freescale.com>
> > > > > ---
> > > > > arch/arm/Kconfig | 1 +
> > > > > arch/arm/cpu/armv8/fsl-lsch2/Makefile | 1 +
> > > > > arch/arm/cpu/armv8/fsl-lsch2/spl.c | 91
> > > > > ++++++++++++++++++++++
> > > > > arch/arm/include/asm/arch-fsl-lsch2/config.h | 2 +
> > > > > board/freescale/ls1043ardb/ls1043ardb_pbi.cfg | 14 ++++
> > > > > board/freescale/ls1043ardb/ls1043ardb_rcw_nand.cfg | 7 ++
> > > > > configs/ls1043ardb_nand_defconfig | 4 +
> > > > > include/configs/ls1043a_common.h | 31 ++++++++
> > > > > include/configs/ls1043ardb.h | 40 ++++++++++
> > > > > 9 files changed, 191 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> > > > > f935f19..197c72d 100644
> > > > > --- a/arch/arm/Kconfig
> > > > > +++ b/arch/arm/Kconfig
> > > > > @@ -612,6 +612,7 @@ config TARGET_VEXPRESS64_BASE_FVP config
> > > > > TARGET_VEXPRESS64_JUNO
> > > > > bool "Support Versatile Express Juno Development Platform"
> > > > > select ARM64
> > > > > + select SUPPORT_SPL
> > > >
> > > > The subject line says you're adding nand boot support to ls1043ardb,
> > > > not Juno.
> > > >
> > > > Also, the previous patch adds SUPPORT_SPL to ls1043ardb -- was it
> > > > supported in that patch (for non-NAND boot) or is that an error?
> > > >
> > >
> > > Sorry, this is really a patching mistake.:(
> > >
> > > > >
> > > > > config TARGET_LS2085A_EMU
> > > > > bool "Support ls2085a_emu"
> > > > > diff --git a/arch/arm/cpu/armv8/fsl-lsch2/Makefile
> > > > > b/arch/arm/cpu/armv8/fsl- lsch2/Makefile index 23c5bf9..0573659
> > > > > 100644
> > > > > --- a/arch/arm/cpu/armv8/fsl-lsch2/Makefile
> > > > > +++ b/arch/arm/cpu/armv8/fsl-lsch2/Makefile
> > > > > @@ -10,3 +10,4 @@ obj-y += lowlevel.o obj-y += speed.o
> > > > > obj-$(CONFIG_SYS_HAS_SERDES) += fsl_lsch2_serdes.o
> > > > > ls1043a_serdes.o
> > > > > obj-$(CONFIG_OF_LIBFDT) += fdt.o
> > > > > +obj-$(CONFIG_SPL) += spl.o
> > > > > diff --git a/arch/arm/cpu/armv8/fsl-lsch2/spl.c
> > > > > b/arch/arm/cpu/armv8/fsl- lsch2/spl.c new file mode 100644 index
> > > > > 0000000..980901a
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/cpu/armv8/fsl-lsch2/spl.c
> > > > > @@ -0,0 +1,91 @@
> > > > > +/*
> > > > > + * Copyright 2014 Freescale Semiconductor, Inc.
> > > > > + *
> > > > > + * SPDX-License-Identifier: GPL-2.0+ */
> > > > > +
> > > > > +#include <common.h>
> > > > > +#include <spl.h>
> > > > > +#include <asm/io.h>
> > > > > +#include <fsl_ifc.h>
> > > > > +#include <i2c.h>
> > > > > +#include <asm/arch-fsl-lsch2/immap_lsch2.h>
> > > > > +#include "../../../../../board/freescale/common/ns_access.h"
> > > >
> > > > Why is this header in board/freescale/common if code outside that
> > > > directory needs it?
> > > >
> > > > Where did you note the dependency on "ARMv7/ls1021a: move ns_access
> > > > to common file" which is not in this patchset?
> > > >
> > > >
> > >
> > > The ns_access.h is common and shared by not only LS1043A but also
> > > LS1021A boards.
> > >
> > > The "ARMv7/ls1021a? patch had been sent out much earlier while the
> > > ls1043a patches were not ready.
> > > Some details are still overlooked, though.
> >
> > That doesn't answer my question. If the header needs to be accessed from
> > outside board/freescale/common it should not go in board/freescale/common.
> > How about include/fsl-ns-access.h? What does "ns" stand for here?
> >
>
> OK. Our team discussed and now decide to create a include/fsl_csu.h to
> include the csu common parts.
>
> > >
> > > >
> > > > > +
> > > > > + get_clocks();
> > > > > +
> > > > > + preloader_console_init();
> > > > > +
> > > > > +#ifdef CONFIG_SPL_I2C_SUPPORT
> > > > > + i2c_init_all();
> > > > > +#endif
> > > > > + dram_init();
> > > > > +
> > > > > + /* Clear the BSS */
> > > > > + memset(__bss_start, 0, __bss_end - __bss_start);
> > > > > +
> > > > > +#ifdef CONFIG_LAYERSCAPE_NS_ACCESS
> > > > > + enable_layerscape_ns_access(); #endif
> > > > > + board_init_r(NULL, 0);
> > > > > +}
> > > >
> > > > Can you explain the differences between this and the fsl-lsch3
> > version?
> > > >
> > >
> > > The basic board_init_f() for spl contains necessary initializations
> > > such as clocks, serial, ddr and BSS.
> > > So they are generally the same.
> >
> > Right. It would be nice if things that "are generally the same" between
> > similar chips actually be the same, except where differences are
> > justified.
> >
> > > I?m not very clear about how spl works on LS2085A. Why does its spl
> > > need
> > > arch_cpu_init() and env_init()?
> >
> > arch_cpu_init() is low-level CPU initialization -- primarily setting up
> > caches. In theory you may not need it in SPL, but it speeds things up.
> > It was especially important when we were running in a hardware emulator,
> > but it's still nice to have.
>
> OK. It's nice of it to "speed things up".
>
> >
> > env_init() initializes the environment, though we need to enable
> > CONFIG_NAND_ENV_DST for it to actually get loaded from NAND.
> >
>
> But why do you need to get env variables in early spl? Is it indispensable
> for ls2085?
> U-Boot will just do the same initialization later.

I don't remember the exact consequences of not having it, given that (unless
I'm missing something) only the default environment is currently used in the
SPL on this target, but env variables are used by DDR init and serial output.

>
> > > > > diff --git a/include/configs/ls1043a_common.h
> > > > > b/include/configs/ls1043a_common.h
> > > > > index 139005c..4bda296 100644
> > > > > --- a/include/configs/ls1043a_common.h
> > > > > +++ b/include/configs/ls1043a_common.h
> > > > > @@ -60,6 +60,37 @@
> > > > > #define CONFIG_BAUDRATE 115200
> > > > > #define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600,
> > > > 115200 }
> > > > >
> > > > > +/* NAND SPL */
> > > > > +#ifdef CONFIG_NAND_BOOT
> > > > > +#define CONFIG_SPL_PBL_PAD
> > > > > +#define CONFIG_SPL_FRAMEWORK
> > > > > +#define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv8/u-boot-
> > > > spl.lds"
> > > > > +#define CONFIG_SPL_TARGET "u-boot-with-spl.bin"
> > > > > +#define CONFIG_SPL_LIBCOMMON_SUPPORT #define
> > > > > +CONFIG_SPL_LIBGENERIC_SUPPORT #define CONFIG_SPL_ENV_SUPPORT
> > > > > +#define CONFIG_SPL_WATCHDOG_SUPPORT #define
> > > > > +CONFIG_SPL_I2C_SUPPORT #define CONFIG_SPL_SERIAL_SUPPORT #define
> > > > > +CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
> > > > > +#define CONFIG_SPL_NAND_SUPPORT
> > > > > +#define CONFIG_SPL_DRIVERS_MISC_SUPPORT
> > > > > +#define CONFIG_SPL_TEXT_BASE 0x10000000
> > > > > +#define CONFIG_SPL_MAX_SIZE 0x1a000
> > > > > +#define CONFIG_SPL_STACK 0x1001d000
> > > > > +#define CONFIG_SPL_PAD_TO 0x1c000
> > > > > +#define CONFIG_SYS_NAND_U_BOOT_SIZE (600 << 10) #define
> > > > > +CONFIG_SYS_NAND_U_BOOT_OFFS CONFIG_SPL_PAD_TO
> > > > > +#define CONFIG_SYS_NAND_PAGE_SIZE 2048
> > > > > +#define CONFIG_SYS_NAND_U_BOOT_DST CONFIG_SYS_TEXT_BASE
> > > > > +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define
> > > > > +CONFIG_SYS_SPL_MALLOC_START 0x80200000
> > > > > +#define CONFIG_SPL_BSS_START_ADDR 0x80100000
> > > > > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000
> > > > > +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000
> > > > > +#define CONFIG_SYS_MONITOR_LEN 0xa0000
> > > > > +#endif
> > > >
> > > > Can you explain the differences relative to ls2085a, especially
> > > > addresses, offsets, sizes, and padding?
> > > >
> > >
> > > I?m not clear about how ls102085a specifies the values.
> > > As I know on ls1043a boards,
> > > CONFIG_SPL_TEXT_BASE is where SPL code is put.
> >
> > Which parts of OCRAM does the bootrom use while processing PBI? On
> > ls208x we avoided the first 0xa000 bytes because of this.
> >
>
> At the end of ls1043a bootrom, it just jumps to OCRAM offset 0x0 to execute
> spl code.

Is this because there's something in PBI telling it to do so, or is it
hardcoded in the bootrom?

> So the CONFIG_SPL_TEXT_BASE is 0x1000_0000 which is the OCRAM base address
> on ls1043a.
>
> "Avoided the first 0xa000 bytes" means the ls208x bootrom reserves the
> first 0xa000 bytes for itself?

Yes, the bootrom uses those bytes while it is running. Once the SPL takes
control we can reuse that region, but the SPL image itself can't be there.

-Scott