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

Gong Q.Y.
2015-09-15T11:47:13Z

> -----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 at freescale.com<mailto:Qianyu.Gong@freescale.com>>
> > Signed-off-by: Hou Zhiqiang <B48286 at freescale.com<mailto:B48286@freescale.com>>
> > Signed-off-by: Shaohui Xie <Shaohui.Xie at freescale.com<mailto:Shaohui.Xie@freescale.com>>
> > Signed-off-by: Mingkai Hu <Mingkai.Hu at freescale.com<mailto: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.

> > +#ifdef CONFIG_SPL_BUILD
> > +void board_init_f(ulong dummy)
> > +{
> > +#ifdef CONFIG_SPL_NAND_SUPPORT
> > + init_early_memctl_regs();
> > +
> > + /*
> > + * There is LS1 SoC issue where NOR, FPGA are during
> > + * NAND boot because IFC signals > IFC_AD7 are not enabled.
> > + * This workaround changes RCW source to make all signals enabled.
> > + */
> > + u32 porsr1, pinctl;
> > + struct ccsr_gur __iomem *gur = (void *)CONFIG_SYS_FSL_GUTS_ADDR;
> > + porsr1 = in_be32(&gur->porsr1);
> > + pinctl = ((porsr1 & ~(FSL_CHASSIS2_CCSR_PORSR1_RCW_MASK)) |
> 0x24800000);
> > + out_be32((unsigned int *)CONFIG_SYS_DCSR_DCFG_ADDR, pinctl);
> > +#endif
>
> This looks like erratum_rcw_src() in arch/cpu/armv8/fsl-lsch3/soc.c. It
> should probably be moved somewhere common.
>
> Did this ever get an erratum number?
>

This is from ls1021a spl code. Seems that it could be removed now since ls1043a won?t get that issue.

> >
> > + timer_init(); /* initialize timer */
>
> This sort of comment is not useful.

OK, thanks.

>
> > +
> > + 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.
I?m not very clear about how spl works on LS2085A. Why does its spl need arch_cpu_init() and env_init()?

> > 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. The max size also depends on SPL image.
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.

> 600 KiB for the payload is an unusual size. It doesn't look erase-block-
> aligned. What components share an erase block in this scheme?

You?re right. Maybe a 640KiB size is more satisfying.

>
> -Scott