> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, September 18, 2015 3:26 AM
> To: Gong Qianyu-B52263
> Cc: u-boot@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
>
> 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@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@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.g...@freescale.com>
> > > > > > Signed-off-by: Hou Zhiqiang <b48...@freescale.com>
> > > > > > Signed-off-by: Shaohui Xie <shaohui....@freescale.com>
> > > > > > Signed-off-by: Mingkai Hu <mingkai...@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?
>
Yes. Which address the bootrom will jump directly depends on the value in
scratch registers.
It is hardcoded. PBI actually set the SPL address in scratch registers for it.
> > 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
