
-----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.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.
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. 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?
The max size also depends on SPL image.
What do you mean by "The max size also depends on SPL image"? The max size depends on the size of OCRAM and the offset within OCRAM of the SPL image (minus any areas at the end of OCRAM that need to be reserved).
Thanks. You make it more detailed. Of course the SPL max size mustn't be larger than OCRAM size.
CONFIG_SPL_PAD_TO is actually U-Boot image offset to NAND 0x0 base because the U-Boot image is put right after padded SPL image. CONFIG_SPL_PAD_TO must be larger than CONFIG_SPL_MAX_SIZE. Do you think the spl code could be common for ls1043a and ls2085a ?
Why is board-specific information such as NAND page size being hardcoded in an SoC common header file?
Now both LS1043ARDB and LS1043AQDS boards share the same NAND page size.
So? It's still board-specific. Don't make it hard to add support for a custom board with a different NAND chip. Likewise for any other parameters that depend on page/block size (e.g. to ensure alignment).
Oh, we didn't consider any custom board. Maybe it's better to keep their respective macros for long-term consideration.
-Scott