diff mbox

[U-Boot,6/7] arm: ls102xa: Add SD boot support for LS1021AQDS board

Message ID 1411019239-12360-7-git-send-email-b18965@freescale.com
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Alison Wang Sept. 18, 2014, 5:47 a.m. UTC
This patch adds SD boot support for LS1021AQDS board. SPL
framework is used. PBL initialize the internal RAM and copy
SPL to it, then SPL initialize DDR using SPD and copy u-boot
from SD card to DDR, finally SPL transfer control to u-boot.

Signed-off-by: Alison Wang <alison.wang@freescale.com>
Signed-off-by: Jason Jin <jason.jin@freescale.com>
---
 arch/arm/cpu/armv7/ls102xa/Makefile           |  1 +
 arch/arm/cpu/armv7/ls102xa/spl.c              | 35 +++++++++++
 arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds     | 89 +++++++++++++++++++++++++++
 arch/arm/include/asm/arch-ls102xa/spl.h       | 20 ++++++
 board/freescale/ls1021aqds/MAINTAINERS        |  1 +
 board/freescale/ls1021aqds/ddr.c              |  5 +-
 board/freescale/ls1021aqds/ls1021aqds.c       | 31 ++++++++++
 board/freescale/ls1021aqds/ls102xa_pbi.cfg    |  8 +++
 board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg | 14 +++++
 configs/ls1021aqds_sdcard_defconfig           |  4 ++
 include/configs/ls1021aqds.h                  | 67 ++++++++++++++++++++
 11 files changed, 274 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/cpu/armv7/ls102xa/spl.c
 create mode 100644 arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
 create mode 100644 arch/arm/include/asm/arch-ls102xa/spl.h
 create mode 100644 board/freescale/ls1021aqds/ls102xa_pbi.cfg
 create mode 100644 board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
 create mode 100644 configs/ls1021aqds_sdcard_defconfig

Comments

Albert ARIBAUD Sept. 18, 2014, 11:20 a.m. UTC | #1
Hi Alison,

On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang <b18965@freescale.com>
wrote:

> This patch adds SD boot support for LS1021AQDS board. SPL
> framework is used. PBL initialize the internal RAM and copy
> SPL to it, then SPL initialize DDR using SPD and copy u-boot
> from SD card to DDR, finally SPL transfer control to u-boot.
> 
> Signed-off-by: Alison Wang <alison.wang@freescale.com>
> Signed-off-by: Jason Jin <jason.jin@freescale.com>
> ---
>  arch/arm/cpu/armv7/ls102xa/Makefile           |  1 +
>  arch/arm/cpu/armv7/ls102xa/spl.c              | 35 +++++++++++
>  arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds     | 89 +++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-ls102xa/spl.h       | 20 ++++++
>  board/freescale/ls1021aqds/MAINTAINERS        |  1 +
>  board/freescale/ls1021aqds/ddr.c              |  5 +-
>  board/freescale/ls1021aqds/ls1021aqds.c       | 31 ++++++++++
>  board/freescale/ls1021aqds/ls102xa_pbi.cfg    |  8 +++
>  board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg | 14 +++++
>  configs/ls1021aqds_sdcard_defconfig           |  4 ++
>  include/configs/ls1021aqds.h                  | 67 ++++++++++++++++++++
>  11 files changed, 274 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/cpu/armv7/ls102xa/spl.c
>  create mode 100644 arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
>  create mode 100644 arch/arm/include/asm/arch-ls102xa/spl.h
>  create mode 100644 board/freescale/ls1021aqds/ls102xa_pbi.cfg
>  create mode 100644 board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
>  create mode 100644 configs/ls1021aqds_sdcard_defconfig
> 
> diff --git a/arch/arm/cpu/armv7/ls102xa/Makefile b/arch/arm/cpu/armv7/ls102xa/Makefile
> index d82ce8d..56ef3a7 100644
> --- a/arch/arm/cpu/armv7/ls102xa/Makefile
> +++ b/arch/arm/cpu/armv7/ls102xa/Makefile
> @@ -10,3 +10,4 @@ obj-y	+= timer.o
>  
>  obj-$(CONFIG_OF_LIBFDT) += fdt.o
>  obj-$(CONFIG_SYS_HAS_SERDES) += fsl_ls1_serdes.o ls102xa_serdes.o
> +obj-$(CONFIG_SPL) += spl.o
> diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c b/arch/arm/cpu/armv7/ls102xa/spl.c
> new file mode 100644
> index 0000000..77ea1ee
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/ls102xa/spl.c
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <spl.h>
> +
> +u32 spl_boot_device(void)
> +{
> +#ifdef CONFIG_SPL_MMC_SUPPORT
> +	return BOOT_DEVICE_MMC1;
> +#endif
> +	return BOOT_DEVICE_NAND;
> +}
> +
> +u32 spl_boot_mode(void)
> +{
> +	switch (spl_boot_device()) {
> +	case BOOT_DEVICE_MMC1:
> +#ifdef CONFIG_SPL_FAT_SUPPORT
> +		return MMCSD_MODE_FAT;
> +#else
> +		return MMCSD_MODE_RAW;
> +#endif
> +		break;
> +	case BOOT_DEVICE_NAND:
> +		return 0;
> +		break;
> +	default:
> +		puts("spl: error: unsupported device\n");
> +		hang();
> +	}
> +}
> diff --git a/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds b/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
> new file mode 100644
> index 0000000..10671e7
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (c) 2004-2008 Texas Instruments
> + *
> + * (C) Copyright 2002
> + * Gary Jennejohn, DENX Software Engineering, <garyj@denx.de>
> + *
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> +	. = 0x00000000;
> +
> +	. = ALIGN(4);
> +	.text :
> +	{
> +		__image_copy_start = .;
> +		*(.vectors)
> +		CPUDIR/start.o (.text*)
> +		*(.text*)
> +	}
> +
> +	. = ALIGN(4);
> +	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
> +
> +	. = ALIGN(4);
> +	.data : {
> +		*(.data*)
> +	}
> +
> +	. = ALIGN(4);
> +	.u_boot_list : {
> +		KEEP(*(SORT(.u_boot_list*_i2c_*)));
> +	}

IS this required? And if it is, could it not be added to the
arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds
file at all.

> +	. = .;
> +
> +	__image_copy_end = .;
> +
> +	.rel.dyn : {
> +		__rel_dyn_start = .;
> +		*(.rel*)
> +		__rel_dyn_end = .;
> +	}
> +
> +	.end :
> +	{
> +		*(.__end)
> +	}
> +
> +	_image_binary_end = .;
> +
> +	.bss __rel_dyn_start (OVERLAY) : {
> +		__bss_start = .;
> +		*(.bss*)
> +		 . = ALIGN(4);
> +		__bss_end = .;
> +	}
> +
> +	.dynsym _image_binary_end : { *(.dynsym) }
> +	.dynbss : { *(.dynbss) }
> +	.dynstr : { *(.dynstr*) }
> +	.dynamic : { *(.dynamic*) }
> +	.hash : { *(.hash*) }
> +	.plt : { *(.plt*) }
> +	.interp : { *(.interp*) }
> +	.gnu : { *(.gnu*) }
> +	.ARM.exidx : { *(.ARM.exidx*) }
> +}
> +
> +#if defined(CONFIG_SPL_MAX_SIZE)
> +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> +	"SPL image too big");
> +#endif
> +
> +#if defined(CONFIG_SPL_BSS_MAX_SIZE)
> +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +	"SPL image BSS too big");
> +#endif
> +
> +#if defined(CONFIG_SPL_MAX_FOOTPRINT)
> +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
> +	"SPL image plus BSS too big");
> +#endif
> diff --git a/arch/arm/include/asm/arch-ls102xa/spl.h b/arch/arm/include/asm/arch-ls102xa/spl.h
> new file mode 100644
> index 0000000..26e4ea1
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-ls102xa/spl.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __ASM_ARCH_SPL_H__
> +#define __ASM_ARCH_SPL_H__
> +
> +#define BOOT_DEVICE_NONE	0
> +#define BOOT_DEVICE_XIP		1
> +#define BOOT_DEVICE_XIPWAIT	2
> +#define BOOT_DEVICE_NAND	3
> +#define BOOT_DEVICE_ONENAND	4
> +#define BOOT_DEVICE_MMC1	5
> +#define BOOT_DEVICE_MMC2	6
> +#define BOOT_DEVICE_MMC2_2	7
> +#define BOOT_DEVICE_SPI		10
> +
> +#endif	/* __ASM_ARCH_SPL_H__ */
> diff --git a/board/freescale/ls1021aqds/MAINTAINERS b/board/freescale/ls1021aqds/MAINTAINERS
> index 021d82b..9244057 100644
> --- a/board/freescale/ls1021aqds/MAINTAINERS
> +++ b/board/freescale/ls1021aqds/MAINTAINERS
> @@ -4,3 +4,4 @@ S:	Maintained
>  F:	board/freescale/ls1021aqds/
>  F:	include/configs/ls1021aqds.h
>  F:	configs/ls1021aqds_nor_defconfig
> +F:	configs/ls1021aqds_sdcard_defconfig
> diff --git a/board/freescale/ls1021aqds/ddr.c b/board/freescale/ls1021aqds/ddr.c
> index 679c654..369088e 100644
> --- a/board/freescale/ls1021aqds/ddr.c
> +++ b/board/freescale/ls1021aqds/ddr.c
> @@ -146,9 +146,12 @@ phys_size_t initdram(int board_type)
>  {
>  	phys_size_t dram_size;
>  
> +#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL)
>  	puts("Initializing DDR....using SPD\n");
>  	dram_size = fsl_ddr_sdram();
> -
> +#else
> +	dram_size =  fsl_ddr_sdram_size();
> +#endif
>  	return dram_size;
>  }
>  
> diff --git a/board/freescale/ls1021aqds/ls1021aqds.c b/board/freescale/ls1021aqds/ls1021aqds.c
> index 12e83f7..85a53c9 100644
> --- a/board/freescale/ls1021aqds/ls1021aqds.c
> +++ b/board/freescale/ls1021aqds/ls1021aqds.c
> @@ -13,6 +13,7 @@
>  #include <mmc.h>
>  #include <fsl_esdhc.h>
>  #include <fsl_ifc.h>
> +#include <spl.h>
>  
>  #include "../common/qixis.h"
>  #include "ls1021aqds_qixis.h"
> @@ -29,10 +30,13 @@ enum {
>  int checkboard(void)
>  {
>  	char buf[64];
> +#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
>  	u8 sw;
> +#endif
>  
>  	puts("Board: LS1021AQDS\n");
>  
> +#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
>  	sw = QIXIS_READ(brdcfg[0]);
>  	sw = (sw & QIXIS_LBMAP_MASK) >> QIXIS_LBMAP_SHIFT;
>  
> @@ -46,6 +50,7 @@ int checkboard(void)
>  		printf("IFCCard\n");
>  	else
>  		printf("invalid setting of SW%u\n", QIXIS_LBMAP_SWITCH);
> +#endif
>  
>  	printf("Sys ID:0x%02x, Sys Ver: 0x%02x\n",
>  	       QIXIS_READ(id), QIXIS_READ(arch));
> @@ -155,6 +160,32 @@ int board_early_init_f(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_SPL_BUILD
> +void board_init_f(ulong dummy)
> +{
> +	struct ccsr_cci400 *cci = (struct ccsr_cci400 *)CONFIG_SYS_CCI400_ADDR;
> +
> +	/* Set global data pointer */
> +	gd = &gdata;
> +
> +	/* Clear the BSS */
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +
> +	get_clocks();
> +
> +	preloader_console_init();
> +
> +#ifdef CONFIG_SPL_I2C_SUPPORT
> +	i2c_init_all();
> +#endif
> +	out_le32(&cci->ctrl_ord, CCI400_CTRLORD_TERM_BARRIER);
> +
> +	dram_init();
> +
> +	board_init_r(NULL, 0);
> +}
> +#endif
> +
>  int config_board_mux(int ctrl_type)
>  {
>  	u8 reg12;
> diff --git a/board/freescale/ls1021aqds/ls102xa_pbi.cfg b/board/freescale/ls1021aqds/ls102xa_pbi.cfg
> new file mode 100644
> index 0000000..edf9f94
> --- /dev/null
> +++ b/board/freescale/ls1021aqds/ls102xa_pbi.cfg
> @@ -0,0 +1,8 @@
> +#PBI commands
> +
> +#Configure Scratch register
> +09ee0200 10000000
> +#Configure alternate space
> +09570158 00080000
> +#Flush PBL data
> +096100c0 000FFFFF
> diff --git a/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg b/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
> new file mode 100644
> index 0000000..e05ec16
> --- /dev/null
> +++ b/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
> @@ -0,0 +1,14 @@
> +#PBL preamble and RCW header
> +aa55aa55 01ee0100
> +
> +#enable IFC, disable QSPI and DSPI
> +#0608000a 00000000 00000000 00000000
> +#00000000 00404000 60025a00 21042000
> +#00200000 00000000 00000000 01038000
> +#00000000 001b1200 00000000 00000000
> +
> +#disable IFC, enable QSPI and DSPI
> +0608000a 00000000 00000000 00000000
> +60000000 00407900 e0025a00 21046000
> +00000000 00000000 00000000 00038000
> +20024800 001b7200 00000000 00000000
> diff --git a/configs/ls1021aqds_sdcard_defconfig b/configs/ls1021aqds_sdcard_defconfig
> new file mode 100644
> index 0000000..e03c3b4
> --- /dev/null
> +++ b/configs/ls1021aqds_sdcard_defconfig
> @@ -0,0 +1,4 @@
> +CONFIG_SPL=y
> +CONFIG_SYS_EXTRA_OPTIONS="RAMBOOT_PBL,SPL_FSL_PBL,SD_BOOT"
> ++S:CONFIG_ARM=y
> ++S:CONFIG_TARGET_LS1021AQDS=y
> diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h
> index 657e3b6..440c118 100644
> --- a/include/configs/ls1021aqds.h
> +++ b/include/configs/ls1021aqds.h
> @@ -37,8 +37,48 @@ unsigned long get_board_sys_clk(void);
>  unsigned long get_board_ddr_clk(void);
>  #endif
>  
> +#if defined(CONFIG_SD_BOOT) || defined(CONFIG_QSPI_BOOT)
> +#define CONFIG_SYS_CLK_FREQ            100000000
> +#define CONFIG_DDR_CLK_FREQ            100000000
> +#define CONFIG_QIXIS_I2C_ACCESS
> +#else
>  #define CONFIG_SYS_CLK_FREQ		get_board_sys_clk()
>  #define CONFIG_DDR_CLK_FREQ		get_board_ddr_clk()
> +#endif
> +
> +#ifdef CONFIG_RAMBOOT_PBL
> +#define CONFIG_SYS_FSL_PBL_PBI	board/freescale/ls1021aqds/ls102xa_pbi.cfg
> +#endif
> +
> +#ifdef CONFIG_SD_BOOT
> +#define CONFIG_SYS_FSL_PBL_RCW	board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
> +#define CONFIG_SPL_PBL_PAD
> +#define CONFIG_SPL_FRAMEWORK
> +#define CONFIG_SPL_LDSCRIPT	"arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds"
> +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_ENV_SUPPORT
> +#define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
> +#define CONFIG_SPL_I2C_SUPPORT
> +#define CONFIG_SPL_WATCHDOG_SUPPORT
> +#define CONFIG_SPL_SERIAL_SUPPORT
> +#define CONFIG_SPL_MMC_SUPPORT
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR		0xe8
> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS		0x400
> +
> +#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_TEXT_BASE		0x82000000
> +
> +#define CONFIG_SYS_SPL_MALLOC_START	0x80200000
> +#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
> +#define CONFIG_SPL_BSS_START_ADDR	0x80100000
> +#define CONFIG_SPL_BSS_MAX_SIZE		0x80000
> +#define CONFIG_SYS_MONITOR_LEN		0x80000
> +#define CONFIG_SYS_NO_FLASH
> +#endif
>  
>  #ifndef CONFIG_SYS_TEXT_BASE
>  #define CONFIG_SYS_TEXT_BASE		0x67f80000
> @@ -70,6 +110,7 @@ unsigned long get_board_ddr_clk(void);
>  /*
>   * IFC Definitions
>   */
> +#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
>  #define CONFIG_FSL_IFC
>  #define CONFIG_SYS_FLASH_BASE		0x60000000
>  #define CONFIG_SYS_FLASH_BASE_PHYS	CONFIG_SYS_FLASH_BASE
> @@ -161,6 +202,7 @@ unsigned long get_board_ddr_clk(void);
>  #define CONFIG_CMD_NAND
>  
>  #define CONFIG_SYS_NAND_BLOCK_SIZE	(128 * 1024)
> +#endif
>  
>  /*
>   * QIXIS Definitions
> @@ -322,7 +364,12 @@ unsigned long get_board_ddr_clk(void);
>  
>  #define CONFIG_CMDLINE_TAG
>  #define CONFIG_CMDLINE_EDITING
> +
> +#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
>  #define CONFIG_CMD_IMLS
> +#else
> +#undef CONFIG_CMD_IMLS
> +#endif
>  
>  #define CONFIG_HWCONFIG
>  #define HWCONFIG_BUFFER_SIZE		128
> @@ -370,17 +417,37 @@ unsigned long get_board_ddr_clk(void);
>  #define CONFIG_SYS_INIT_SP_ADDR \
>  	(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
>  
> +#ifdef CONFIG_SPL_BUILD
> +#define CONFIG_SYS_MONITOR_BASE CONFIG_SPL_TEXT_BASE
> +#else
>  #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE    /* start of monitor */
> +#endif
>  
>  /*
>   * Environment
>   */
>  #define CONFIG_ENV_OVERWRITE
>  
> +#if defined(CONFIG_SD_BOOT)
> +#define CONFIG_ENV_OFFSET		(1024 * 1024)
> +#define CONFIG_ENV_IS_IN_MMC
> +#define CONFIG_SYS_MMC_ENV_DEV		0
> +#define CONFIG_ENV_SIZE			0x2000
> +#elif defined(CONFIG_NAND_BOOT)
> +#define CONFIG_ENV_IS_IN_NAND
> +#define CONFIG_ENV_SIZE			0x2000
> +#define CONFIG_ENV_OFFSET		(10 * CONFIG_SYS_NAND_BLOCK_SIZE)
> +#elif defined(CONFIG_QSPI_BOOT)
> +#define CONFIG_ENV_IS_IN_SPI_FLASH
> +#define CONFIG_ENV_SIZE			0x2000          /* 8KB */
> +#define CONFIG_ENV_OFFSET		0x100000        /* 1MB */
> +#define CONFIG_ENV_SECT_SIZE		0x10000
> +#else
>  #define CONFIG_ENV_IS_IN_FLASH
>  #define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE - CONFIG_ENV_SECT_SIZE)
>  #define CONFIG_ENV_SIZE			0x2000
>  #define CONFIG_ENV_SECT_SIZE		0x20000 /* 128K (one sector) */
> +#endif
>  
>  #define CONFIG_OF_LIBFDT
>  #define CONFIG_OF_BOARD_SETUP


Amicalement,
Alison Wang Sept. 19, 2014, 3:40 a.m. UTC | #2
Hi, Albert,

On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang <b18965@freescale.com>
wrote:

> +
> +     . = ALIGN(4);
> +     .u_boot_list : {
> +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> +     }

IS this required? And if it is, could it not be added to the
arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds file
at all.

[Alison Wang] Yes, it is required. I would like to add it in
arch/arm/cpu/u-boot-spl.lds. I was not sure adding it in
arch/arm/cpu/u-boot-spl.lds is acceptable or not.

Alison



--
View this message in context: http://u-boot.10912.n7.nabble.com/PATCH-0-7-Add-SD-boot-support-for-LS1021AQDS-TWR-board-tp189678p189759.html
Sent from the U-Boot mailing list archive at Nabble.com.
alison wang Sept. 19, 2014, 5:10 a.m. UTC | #3
Hi, Albert,

> > +	. = ALIGN(4);
> > +	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
> > +
> > +	. = ALIGN(4);
> > +	.data : {
> > +		*(.data*)
> > +	}
> > +
> > +	. = ALIGN(4);
> > +	.u_boot_list : {
> > +		KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > +	}
> 
> IS this required? And if it is, could it not be added to the
> arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds
> file at all.
> 
[Alison Wang] Yes, it is required. I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not sure adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or not.


Best Regards,
Alison Wang
Albert ARIBAUD Sept. 19, 2014, 3:56 p.m. UTC | #4
Hi Huan,

On Thu, 18 Sep 2014 15:15:54 +0000, Huan Wang
<alison.wang@freescale.com> wrote:

> Hi, Albert,
> 
> On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang <b18965@freescale.com>
> wrote:
> 
> > +
> > +     . = ALIGN(4);
> > +     .u_boot_list : {
> > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > +     }
> 
> IS this required? And if it is, could it not be added to the
> arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds
> file at all.
> 
> [Alison Wang] Yes, it is required.

Ok -- what for? :)

> I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not sure
> adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or not.

(assuming the reason why it is needed is valid) If it causes no change
to boards which do not use it right now (and I mean 'no change' ad
'binary identical') then this is acceptable. Make sure you check the
binary invariance and that you mention it in the commit.

> Best Regards,
> Alison Wang

Amicalement,
alison wang Sept. 22, 2014, 6:46 a.m. UTC | #5
Hi, Albert,

> > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang <b18965@freescale.com>
> > wrote:
> >
> > > +
> > > +     . = ALIGN(4);
> > > +     .u_boot_list : {
> > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > +     }
> >
> > IS this required? And if it is, could it not be added to the
> > arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds
> > file at all.
> >
> > [Alison Wang] Yes, it is required.
> 
> Ok -- what for? :)
[Alison Wang] In SPL part, DDR is initialized by reading SPD through I2C interface.
For I2C, ll_entry_count() is called, and it returns the number of elements of a
linker-generated array placed into subsection of .u_boot_list section specified
by _list argument. So I need to add this to make I2C work in SPL.

> 
> > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not sure
> > adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or not.
> 
> (assuming the reason why it is needed is valid) If it causes no change
> to boards which do not use it right now (and I mean 'no change' ad
> 'binary identical') then this is acceptable. Make sure you check the
> binary invariance and that you mention it in the commit.
> 
[Alison Wang] It will cause the binary is not identical for other board. I think
it may be not good to add in arch/arm/cpu/u-boot-spl.lds. What's your opinion about it?

Best Regards,
Alison Wang
Albert ARIBAUD Sept. 22, 2014, 11:01 a.m. UTC | #6
Hi Huan,

On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
<alison.wang@freescale.com> wrote:

> Hi, Albert,
> 
> > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang <b18965@freescale.com>
> > > wrote:
> > >
> > > > +
> > > > +     . = ALIGN(4);
> > > > +     .u_boot_list : {
> > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > +     }
> > >
> > > IS this required? And if it is, could it not be added to the
> > > arch/arm/cpu/u-boot-spl.lds file? This way you would not need an .lds
> > > file at all.
> > >
> > > [Alison Wang] Yes, it is required.
> > 
> > Ok -- what for? :)
> [Alison Wang] In SPL part, DDR is initialized by reading SPD through I2C interface.
> For I2C, ll_entry_count() is called, and it returns the number of elements of a
> linker-generated array placed into subsection of .u_boot_list section specified
> by _list argument. So I need to add this to make I2C work in SPL.

Understood. So your SPL code uses I2C, and for I2C, you need a linker
list. But then:

> > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not sure
> > > adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or not.
> > 
> > (assuming the reason why it is needed is valid) If it causes no change
> > to boards which do not use it right now (and I mean 'no change' ad
> > 'binary identical') then this is acceptable. Make sure you check the
> > binary invariance and that you mention it in the commit.
> > 
> [Alison Wang] It will cause the binary is not identical for other board.

Is this a prediction or an actual observation of compared builds with
and without the I2C linker liste addition to the generic SPL .lds?

> I think
> it may be not good to add in arch/arm/cpu/u-boot-spl.lds. What's your opinion about it?

If there are SPLs which use I2C linker lists *and* the generic .lds and
build without an error, then this should be fixed, because it means the
build process should complain when an input section is not mapped to an
output section.

(and that should be fixed even though in this case, adding the I2C
linker lists in the .lds would 'fix' the build, but they would actually
paper over the real issue of sections being mapped without control)

> Best Regards,
> Alison Wang

Amicalement,
alison wang Sept. 25, 2014, 6:45 a.m. UTC | #7
Hi, Albert,

> On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> <alison.wang@freescale.com> wrote:
> 
> > Hi, Albert,
> >
> > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > <b18965@freescale.com>
> > > > wrote:
> > > >
> > > > > +
> > > > > +     . = ALIGN(4);
> > > > > +     .u_boot_list : {
> > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > +     }
> > > >
> > > > IS this required? And if it is, could it not be added to the
> > > > arch/arm/cpu/u-boot-spl.lds file? This way you would not need an
> > > > .lds file at all.
> > > >
> > > > [Alison Wang] Yes, it is required.
> > >
> > > Ok -- what for? :)
> > [Alison Wang] In SPL part, DDR is initialized by reading SPD through
> I2C interface.
> > For I2C, ll_entry_count() is called, and it returns the number of
> > elements of a linker-generated array placed into subsection of
> > .u_boot_list section specified by _list argument. So I need to add
> this to make I2C work in SPL.
> 
> Understood. So your SPL code uses I2C, and for I2C, you need a linker
> list. But then:
> 
> > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not
> > > > sure adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or
> not.
> > >
> > > (assuming the reason why it is needed is valid) If it causes no
> > > change to boards which do not use it right now (and I mean 'no
> > > change' ad 'binary identical') then this is acceptable. Make sure
> > > you check the binary invariance and that you mention it in the
> commit.
> > >
> > [Alison Wang] It will cause the binary is not identical for other
> board.
> 
> Is this a prediction or an actual observation of compared builds with
> and without the I2C linker liste addition to the generic SPL .lds?

[Alison Wang] I use mx31pdk as example. I compared the binaries with and
Without the I2C linker list in arch/arm/cpu/u-boot-spl.lds. The binaries
are not identical.
> 
> > I think
> > it may be not good to add in arch/arm/cpu/u-boot-spl.lds. What's your
> opinion about it?
> 
> If there are SPLs which use I2C linker lists *and* the generic .lds and
> build without an error, then this should be fixed, because it means the
> build process should complain when an input section is not mapped to an
> output section.
> 
> (and that should be fixed even though in this case, adding the I2C
> linker lists in the .lds would 'fix' the build, but they would actually
> paper over the real issue of sections being mapped without control)
> 

Best Regards,
Alison Wang
Albert ARIBAUD Oct. 1, 2014, 6:08 p.m. UTC | #8
Hi Huan,

On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
<alison.wang@freescale.com> wrote:

> Hi, Albert,
> 
> > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > <alison.wang@freescale.com> wrote:
> > 
> > > Hi, Albert,
> > >
> > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > <b18965@freescale.com>
> > > > > wrote:
> > > > >
> > > > > > +
> > > > > > +     . = ALIGN(4);
> > > > > > +     .u_boot_list : {
> > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > +     }
> > > > >
> > > > > IS this required? And if it is, could it not be added to the
> > > > > arch/arm/cpu/u-boot-spl.lds file? This way you would not need an
> > > > > .lds file at all.
> > > > >
> > > > > [Alison Wang] Yes, it is required.
> > > >
> > > > Ok -- what for? :)
> > > [Alison Wang] In SPL part, DDR is initialized by reading SPD through
> > I2C interface.
> > > For I2C, ll_entry_count() is called, and it returns the number of
> > > elements of a linker-generated array placed into subsection of
> > > .u_boot_list section specified by _list argument. So I need to add
> > this to make I2C work in SPL.
> > 
> > Understood. So your SPL code uses I2C, and for I2C, you need a linker
> > list. But then:
> > 
> > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was not
> > > > > sure adding it in arch/arm/cpu/u-boot-spl.lds is acceptable or
> > not.
> > > >
> > > > (assuming the reason why it is needed is valid) If it causes no
> > > > change to boards which do not use it right now (and I mean 'no
> > > > change' ad 'binary identical') then this is acceptable. Make sure
> > > > you check the binary invariance and that you mention it in the
> > commit.
> > > >
> > > [Alison Wang] It will cause the binary is not identical for other
> > board.
> > 
> > Is this a prediction or an actual observation of compared builds with
> > and without the I2C linker liste addition to the generic SPL .lds?
> 
> [Alison Wang] I use mx31pdk as example. I compared the binaries with and
> Without the I2C linker list in arch/arm/cpu/u-boot-spl.lds. The binaries
> are not identical.

I have just checked mx31pdk: the u-boot binaries (u-boot, u-boot.bin,
u-boot-with-spl.bin, u-boot.map, u-boot.srec) are indeed different, but
that's just normal considering the repository state and build date and
time are included in the binaries [1].

OTOH, I see that the u-boot-spl.bin files are identical.

The only change I made between the two builds was inserting

	>         . = ALIGN(4);
	> +       .u_boot_list : {
	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
	> +       }  

in arch/arm/cpu/u-boot-spl.lds at line 34.

Can you re-check?

> Best Regards,
> Alison Wang

[1] BTW, how do you folks out here proceed when trying to compare
u-boot.bin files from different builds of the same target without
the repo state or build date and time affecting the comparison? I use a
patch to Makefile that fakes the commit and repo state, and I also use
fakelib to force timestamps, but there might be a simpler way.

Amicalement,
alison wang Oct. 8, 2014, 9:53 a.m. UTC | #9
Hi, Albert,

> On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
> <alison.wang@freescale.com> wrote:
> 
> > Hi, Albert,
> >
> > > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > > <alison.wang@freescale.com> wrote:
> > >
> > > > Hi, Albert,
> > > >
> > > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > > <b18965@freescale.com>
> > > > > > wrote:
> > > > > >
> > > > > > > +
> > > > > > > +     . = ALIGN(4);
> > > > > > > +     .u_boot_list : {
> > > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > > +     }
> > > > > >
> > > > > > IS this required? And if it is, could it not be added to the
> > > > > > arch/arm/cpu/u-boot-spl.lds file? This way you would not need
> > > > > > an .lds file at all.
> > > > > >
> > > > > > [Alison Wang] Yes, it is required.
> > > > >
> > > > > Ok -- what for? :)
> > > > [Alison Wang] In SPL part, DDR is initialized by reading SPD
> > > > through
> > > I2C interface.
> > > > For I2C, ll_entry_count() is called, and it returns the number of
> > > > elements of a linker-generated array placed into subsection of
> > > > .u_boot_list section specified by _list argument. So I need to
> add
> > > this to make I2C work in SPL.
> > >
> > > Understood. So your SPL code uses I2C, and for I2C, you need a
> > > linker list. But then:
> > >
> > > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was
> > > > > > not sure adding it in arch/arm/cpu/u-boot-spl.lds is
> > > > > > acceptable or
> > > not.
> > > > >
> > > > > (assuming the reason why it is needed is valid) If it causes no
> > > > > change to boards which do not use it right now (and I mean 'no
> > > > > change' ad 'binary identical') then this is acceptable. Make
> > > > > sure you check the binary invariance and that you mention it in
> > > > > the
> > > commit.
> > > > >
> > > > [Alison Wang] It will cause the binary is not identical for other
> > > board.
> > >
> > > Is this a prediction or an actual observation of compared builds
> > > with and without the I2C linker liste addition to the generic
> SPL .lds?
> >
> > [Alison Wang] I use mx31pdk as example. I compared the binaries with
> > and Without the I2C linker list in arch/arm/cpu/u-boot-spl.lds. The
> > binaries are not identical.
> 
> I have just checked mx31pdk: the u-boot binaries (u-boot, u-boot.bin,
> u-boot-with-spl.bin, u-boot.map, u-boot.srec) are indeed different, but
> that's just normal considering the repository state and build date and
> time are included in the binaries [1].
> 
> OTOH, I see that the u-boot-spl.bin files are identical.
> 
> The only change I made between the two builds was inserting
> 
> 	>         . = ALIGN(4);
> 	> +       .u_boot_list : {
> 	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
> 	> +       }
> 
> in arch/arm/cpu/u-boot-spl.lds at line 34.
> 
> Can you re-check?

[Alison Wang] Yes, you are right. u-boot-spl.bin files are identical. The u-boot binaries
(u-boot, u-boot.bin, u-boot-with-spl.bin, u-boot.map, u-boot.srec) are different only in build date and time.
> 
> [1] BTW, how do you folks out here proceed when trying to compare u-
> boot.bin files from different builds of the same target without the
> repo state or build date and time affecting the comparison? I use a
> patch to Makefile that fakes the commit and repo state, and I also use
> fakelib to force timestamps, but there might be a simpler way.
> 
[Alison Wang] Oh, your way is very good. I just used vimdiff.


Best Regards,
Alison Wang
Albert ARIBAUD Oct. 11, 2014, 11:30 a.m. UTC | #10
Hi Huan,

On Wed, 8 Oct 2014 09:53:03 +0000, Huan Wang
<alison.wang@freescale.com> wrote:

> Hi, Albert,
> 
> > On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
> > <alison.wang@freescale.com> wrote:
> > 
> > > Hi, Albert,
> > >
> > > > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > > > <alison.wang@freescale.com> wrote:
> > > >
> > > > > Hi, Albert,
> > > > >
> > > > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > > > <b18965@freescale.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > +
> > > > > > > > +     . = ALIGN(4);
> > > > > > > > +     .u_boot_list : {
> > > > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > > > +     }
> > > > > > >
> > > > > > > IS this required? And if it is, could it not be added to the
> > > > > > > arch/arm/cpu/u-boot-spl.lds file? This way you would not need
> > > > > > > an .lds file at all.
> > > > > > >
> > > > > > > [Alison Wang] Yes, it is required.
> > > > > >
> > > > > > Ok -- what for? :)
> > > > > [Alison Wang] In SPL part, DDR is initialized by reading SPD
> > > > > through
> > > > I2C interface.
> > > > > For I2C, ll_entry_count() is called, and it returns the number of
> > > > > elements of a linker-generated array placed into subsection of
> > > > > .u_boot_list section specified by _list argument. So I need to
> > add
> > > > this to make I2C work in SPL.
> > > >
> > > > Understood. So your SPL code uses I2C, and for I2C, you need a
> > > > linker list. But then:
> > > >
> > > > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I was
> > > > > > > not sure adding it in arch/arm/cpu/u-boot-spl.lds is
> > > > > > > acceptable or
> > > > not.
> > > > > >
> > > > > > (assuming the reason why it is needed is valid) If it causes no
> > > > > > change to boards which do not use it right now (and I mean 'no
> > > > > > change' ad 'binary identical') then this is acceptable. Make
> > > > > > sure you check the binary invariance and that you mention it in
> > > > > > the
> > > > commit.
> > > > > >
> > > > > [Alison Wang] It will cause the binary is not identical for other
> > > > board.
> > > >
> > > > Is this a prediction or an actual observation of compared builds
> > > > with and without the I2C linker liste addition to the generic
> > SPL .lds?
> > >
> > > [Alison Wang] I use mx31pdk as example. I compared the binaries with
> > > and Without the I2C linker list in arch/arm/cpu/u-boot-spl.lds. The
> > > binaries are not identical.
> > 
> > I have just checked mx31pdk: the u-boot binaries (u-boot, u-boot.bin,
> > u-boot-with-spl.bin, u-boot.map, u-boot.srec) are indeed different, but
> > that's just normal considering the repository state and build date and
> > time are included in the binaries [1].
> > 
> > OTOH, I see that the u-boot-spl.bin files are identical.
> > 
> > The only change I made between the two builds was inserting
> > 
> > 	>         . = ALIGN(4);
> > 	> +       .u_boot_list : {
> > 	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > 	> +       }
> > 
> > in arch/arm/cpu/u-boot-spl.lds at line 34.
> > 
> > Can you re-check?
> 
> [Alison Wang] Yes, you are right. u-boot-spl.bin files are identical. The u-boot binaries
> (u-boot, u-boot.bin, u-boot-with-spl.bin, u-boot.map, u-boot.srec) are different only in build date and time.
> > 
> > [1] BTW, how do you folks out here proceed when trying to compare u-
> > boot.bin files from different builds of the same target without the
> > repo state or build date and time affecting the comparison? I use a
> > patch to Makefile that fakes the commit and repo state, and I also use
> > fakelib to force timestamps, but there might be a simpler way.
> > 
> [Alison Wang] Oh, your way is very good. I just used vimdiff.

I'll run a larger-scale check today, to see which SPLs are affected by
adding u-boot_list*, not just u_boot_list*_i2c_*, i.e., maybe we can
make this identical in u-boot.lds and u-boot-spl.lds for ARM.

> Best Regards,
> Alison Wang

Amicalement,
alison wang Oct. 15, 2014, 6:56 a.m. UTC | #11
Hi, Albert,

> On Wed, 8 Oct 2014 09:53:03 +0000, Huan Wang <alison.wang@freescale.com>
> wrote:
> 
> > Hi, Albert,
> >
> > > On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
> > > <alison.wang@freescale.com> wrote:
> > >
> > > > Hi, Albert,
> > > >
> > > > > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > > > > <alison.wang@freescale.com> wrote:
> > > > >
> > > > > > Hi, Albert,
> > > > > >
> > > > > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > > > > <b18965@freescale.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +
> > > > > > > > > +     . = ALIGN(4);
> > > > > > > > > +     .u_boot_list : {
> > > > > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > > > > +     }
> > > > > > > >
> > > > > > > > IS this required? And if it is, could it not be added to
> > > > > > > > the arch/arm/cpu/u-boot-spl.lds file? This way you would
> > > > > > > > not need an .lds file at all.
> > > > > > > >
> > > > > > > > [Alison Wang] Yes, it is required.
> > > > > > >
> > > > > > > Ok -- what for? :)
> > > > > > [Alison Wang] In SPL part, DDR is initialized by reading SPD
> > > > > > through
> > > > > I2C interface.
> > > > > > For I2C, ll_entry_count() is called, and it returns the
> number
> > > > > > of elements of a linker-generated array placed into
> subsection
> > > > > > of .u_boot_list section specified by _list argument. So I
> need
> > > > > > to
> > > add
> > > > > this to make I2C work in SPL.
> > > > >
> > > > > Understood. So your SPL code uses I2C, and for I2C, you need a
> > > > > linker list. But then:
> > > > >
> > > > > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I
> > > > > > > > was not sure adding it in arch/arm/cpu/u-boot-spl.lds is
> > > > > > > > acceptable or
> > > > > not.
> > > > > > >
> > > > > > > (assuming the reason why it is needed is valid) If it
> causes
> > > > > > > no change to boards which do not use it right now (and I
> > > > > > > mean 'no change' ad 'binary identical') then this is
> > > > > > > acceptable. Make sure you check the binary invariance and
> > > > > > > that you mention it in the
> > > > > commit.
> > > > > > >
> > > > > > [Alison Wang] It will cause the binary is not identical for
> > > > > > other
> > > > > board.
> > > > >
> > > > > Is this a prediction or an actual observation of compared
> builds
> > > > > with and without the I2C linker liste addition to the generic
> > > SPL .lds?
> > > >
> > > > [Alison Wang] I use mx31pdk as example. I compared the binaries
> > > > with and Without the I2C linker list in
> > > > arch/arm/cpu/u-boot-spl.lds. The binaries are not identical.
> > >
> > > I have just checked mx31pdk: the u-boot binaries (u-boot,
> > > u-boot.bin, u-boot-with-spl.bin, u-boot.map, u-boot.srec) are
> indeed
> > > different, but that's just normal considering the repository state
> > > and build date and time are included in the binaries [1].
> > >
> > > OTOH, I see that the u-boot-spl.bin files are identical.
> > >
> > > The only change I made between the two builds was inserting
> > >
> > > 	>         . = ALIGN(4);
> > > 	> +       .u_boot_list : {
> > > 	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > 	> +       }
> > >
> > > in arch/arm/cpu/u-boot-spl.lds at line 34.
> > >
> > > Can you re-check?
> >
> > [Alison Wang] Yes, you are right. u-boot-spl.bin files are identical.
> > The u-boot binaries (u-boot, u-boot.bin, u-boot-with-spl.bin, u-
> boot.map, u-boot.srec) are different only in build date and time.
> > >
> > > [1] BTW, how do you folks out here proceed when trying to compare
> u-
> > > boot.bin files from different builds of the same target without the
> > > repo state or build date and time affecting the comparison? I use a
> > > patch to Makefile that fakes the commit and repo state, and I also
> > > use fakelib to force timestamps, but there might be a simpler way.
> > >
> > [Alison Wang] Oh, your way is very good. I just used vimdiff.
> 
> I'll run a larger-scale check today, to see which SPLs are affected by
> adding u-boot_list*, not just u_boot_list*_i2c_*, i.e., maybe we can
> make this identical in u-boot.lds and u-boot-spl.lds for ARM.
> 
[Alison Wang] I can't agree with you more on making this identical in u-boot.lds
and u-boot-spl.lds for ARM. Anyway, I need to confirm it through your larger-scale
check result.


Best Regards,
Alison Wang
Albert ARIBAUD Oct. 15, 2014, 10:30 a.m. UTC | #12
Hi Huan,

On Wed, 15 Oct 2014 06:56:37 +0000, Huan Wang
<alison.wang@freescale.com> wrote:

> Hi, Albert,
> 
> > On Wed, 8 Oct 2014 09:53:03 +0000, Huan Wang <alison.wang@freescale.com>
> > wrote:
> > 
> > > Hi, Albert,
> > >
> > > > On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
> > > > <alison.wang@freescale.com> wrote:
> > > >
> > > > > Hi, Albert,
> > > > >
> > > > > > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > > > > > <alison.wang@freescale.com> wrote:
> > > > > >
> > > > > > > Hi, Albert,
> > > > > > >
> > > > > > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > > > > > <b18965@freescale.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > +
> > > > > > > > > > +     . = ALIGN(4);
> > > > > > > > > > +     .u_boot_list : {
> > > > > > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > > > > > +     }
> > > > > > > > >
> > > > > > > > > IS this required? And if it is, could it not be added to
> > > > > > > > > the arch/arm/cpu/u-boot-spl.lds file? This way you would
> > > > > > > > > not need an .lds file at all.
> > > > > > > > >
> > > > > > > > > [Alison Wang] Yes, it is required.
> > > > > > > >
> > > > > > > > Ok -- what for? :)
> > > > > > > [Alison Wang] In SPL part, DDR is initialized by reading SPD
> > > > > > > through
> > > > > > I2C interface.
> > > > > > > For I2C, ll_entry_count() is called, and it returns the
> > number
> > > > > > > of elements of a linker-generated array placed into
> > subsection
> > > > > > > of .u_boot_list section specified by _list argument. So I
> > need
> > > > > > > to
> > > > add
> > > > > > this to make I2C work in SPL.
> > > > > >
> > > > > > Understood. So your SPL code uses I2C, and for I2C, you need a
> > > > > > linker list. But then:
> > > > > >
> > > > > > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I
> > > > > > > > > was not sure adding it in arch/arm/cpu/u-boot-spl.lds is
> > > > > > > > > acceptable or
> > > > > > not.
> > > > > > > >
> > > > > > > > (assuming the reason why it is needed is valid) If it
> > causes
> > > > > > > > no change to boards which do not use it right now (and I
> > > > > > > > mean 'no change' ad 'binary identical') then this is
> > > > > > > > acceptable. Make sure you check the binary invariance and
> > > > > > > > that you mention it in the
> > > > > > commit.
> > > > > > > >
> > > > > > > [Alison Wang] It will cause the binary is not identical for
> > > > > > > other
> > > > > > board.
> > > > > >
> > > > > > Is this a prediction or an actual observation of compared
> > builds
> > > > > > with and without the I2C linker liste addition to the generic
> > > > SPL .lds?
> > > > >
> > > > > [Alison Wang] I use mx31pdk as example. I compared the binaries
> > > > > with and Without the I2C linker list in
> > > > > arch/arm/cpu/u-boot-spl.lds. The binaries are not identical.
> > > >
> > > > I have just checked mx31pdk: the u-boot binaries (u-boot,
> > > > u-boot.bin, u-boot-with-spl.bin, u-boot.map, u-boot.srec) are
> > indeed
> > > > different, but that's just normal considering the repository state
> > > > and build date and time are included in the binaries [1].
> > > >
> > > > OTOH, I see that the u-boot-spl.bin files are identical.
> > > >
> > > > The only change I made between the two builds was inserting
> > > >
> > > > 	>         . = ALIGN(4);
> > > > 	> +       .u_boot_list : {
> > > > 	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > 	> +       }
> > > >
> > > > in arch/arm/cpu/u-boot-spl.lds at line 34.
> > > >
> > > > Can you re-check?
> > >
> > > [Alison Wang] Yes, you are right. u-boot-spl.bin files are identical.
> > > The u-boot binaries (u-boot, u-boot.bin, u-boot-with-spl.bin, u-
> > boot.map, u-boot.srec) are different only in build date and time.
> > > >
> > > > [1] BTW, how do you folks out here proceed when trying to compare
> > u-
> > > > boot.bin files from different builds of the same target without the
> > > > repo state or build date and time affecting the comparison? I use a
> > > > patch to Makefile that fakes the commit and repo state, and I also
> > > > use fakelib to force timestamps, but there might be a simpler way.
> > > >
> > > [Alison Wang] Oh, your way is very good. I just used vimdiff.
> > 
> > I'll run a larger-scale check today, to see which SPLs are affected by
> > adding u-boot_list*, not just u_boot_list*_i2c_*, i.e., maybe we can
> > make this identical in u-boot.lds and u-boot-spl.lds for ARM.
> > 
> [Alison Wang] I can't agree with you more on making this identical in u-boot.lds
> and u-boot-spl.lds for ARM. Anyway, I need to confirm it through your larger-scale
> check result.

I have been slightly side-tracked on this, but I'll run the test in a
few hours and publish the results in this thread.

> Best Regards,
> Alison Wang

Amicalement,
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/ls102xa/Makefile b/arch/arm/cpu/armv7/ls102xa/Makefile
index d82ce8d..56ef3a7 100644
--- a/arch/arm/cpu/armv7/ls102xa/Makefile
+++ b/arch/arm/cpu/armv7/ls102xa/Makefile
@@ -10,3 +10,4 @@  obj-y	+= timer.o
 
 obj-$(CONFIG_OF_LIBFDT) += fdt.o
 obj-$(CONFIG_SYS_HAS_SERDES) += fsl_ls1_serdes.o ls102xa_serdes.o
+obj-$(CONFIG_SPL) += spl.o
diff --git a/arch/arm/cpu/armv7/ls102xa/spl.c b/arch/arm/cpu/armv7/ls102xa/spl.c
new file mode 100644
index 0000000..77ea1ee
--- /dev/null
+++ b/arch/arm/cpu/armv7/ls102xa/spl.c
@@ -0,0 +1,35 @@ 
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <spl.h>
+
+u32 spl_boot_device(void)
+{
+#ifdef CONFIG_SPL_MMC_SUPPORT
+	return BOOT_DEVICE_MMC1;
+#endif
+	return BOOT_DEVICE_NAND;
+}
+
+u32 spl_boot_mode(void)
+{
+	switch (spl_boot_device()) {
+	case BOOT_DEVICE_MMC1:
+#ifdef CONFIG_SPL_FAT_SUPPORT
+		return MMCSD_MODE_FAT;
+#else
+		return MMCSD_MODE_RAW;
+#endif
+		break;
+	case BOOT_DEVICE_NAND:
+		return 0;
+		break;
+	default:
+		puts("spl: error: unsupported device\n");
+		hang();
+	}
+}
diff --git a/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds b/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
new file mode 100644
index 0000000..10671e7
--- /dev/null
+++ b/arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds
@@ -0,0 +1,89 @@ 
+/*
+ * Copyright (c) 2004-2008 Texas Instruments
+ *
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering, <garyj@denx.de>
+ *
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
+OUTPUT_ARCH(arm)
+ENTRY(_start)
+SECTIONS
+{
+	. = 0x00000000;
+
+	. = ALIGN(4);
+	.text :
+	{
+		__image_copy_start = .;
+		*(.vectors)
+		CPUDIR/start.o (.text*)
+		*(.text*)
+	}
+
+	. = ALIGN(4);
+	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
+
+	. = ALIGN(4);
+	.data : {
+		*(.data*)
+	}
+
+	. = ALIGN(4);
+	.u_boot_list : {
+		KEEP(*(SORT(.u_boot_list*_i2c_*)));
+	}
+
+	. = .;
+
+	__image_copy_end = .;
+
+	.rel.dyn : {
+		__rel_dyn_start = .;
+		*(.rel*)
+		__rel_dyn_end = .;
+	}
+
+	.end :
+	{
+		*(.__end)
+	}
+
+	_image_binary_end = .;
+
+	.bss __rel_dyn_start (OVERLAY) : {
+		__bss_start = .;
+		*(.bss*)
+		 . = ALIGN(4);
+		__bss_end = .;
+	}
+
+	.dynsym _image_binary_end : { *(.dynsym) }
+	.dynbss : { *(.dynbss) }
+	.dynstr : { *(.dynstr*) }
+	.dynamic : { *(.dynamic*) }
+	.hash : { *(.hash*) }
+	.plt : { *(.plt*) }
+	.interp : { *(.interp*) }
+	.gnu : { *(.gnu*) }
+	.ARM.exidx : { *(.ARM.exidx*) }
+}
+
+#if defined(CONFIG_SPL_MAX_SIZE)
+ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
+	"SPL image too big");
+#endif
+
+#if defined(CONFIG_SPL_BSS_MAX_SIZE)
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
+	"SPL image BSS too big");
+#endif
+
+#if defined(CONFIG_SPL_MAX_FOOTPRINT)
+ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
+	"SPL image plus BSS too big");
+#endif
diff --git a/arch/arm/include/asm/arch-ls102xa/spl.h b/arch/arm/include/asm/arch-ls102xa/spl.h
new file mode 100644
index 0000000..26e4ea1
--- /dev/null
+++ b/arch/arm/include/asm/arch-ls102xa/spl.h
@@ -0,0 +1,20 @@ 
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_SPL_H__
+#define __ASM_ARCH_SPL_H__
+
+#define BOOT_DEVICE_NONE	0
+#define BOOT_DEVICE_XIP		1
+#define BOOT_DEVICE_XIPWAIT	2
+#define BOOT_DEVICE_NAND	3
+#define BOOT_DEVICE_ONENAND	4
+#define BOOT_DEVICE_MMC1	5
+#define BOOT_DEVICE_MMC2	6
+#define BOOT_DEVICE_MMC2_2	7
+#define BOOT_DEVICE_SPI		10
+
+#endif	/* __ASM_ARCH_SPL_H__ */
diff --git a/board/freescale/ls1021aqds/MAINTAINERS b/board/freescale/ls1021aqds/MAINTAINERS
index 021d82b..9244057 100644
--- a/board/freescale/ls1021aqds/MAINTAINERS
+++ b/board/freescale/ls1021aqds/MAINTAINERS
@@ -4,3 +4,4 @@  S:	Maintained
 F:	board/freescale/ls1021aqds/
 F:	include/configs/ls1021aqds.h
 F:	configs/ls1021aqds_nor_defconfig
+F:	configs/ls1021aqds_sdcard_defconfig
diff --git a/board/freescale/ls1021aqds/ddr.c b/board/freescale/ls1021aqds/ddr.c
index 679c654..369088e 100644
--- a/board/freescale/ls1021aqds/ddr.c
+++ b/board/freescale/ls1021aqds/ddr.c
@@ -146,9 +146,12 @@  phys_size_t initdram(int board_type)
 {
 	phys_size_t dram_size;
 
+#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL)
 	puts("Initializing DDR....using SPD\n");
 	dram_size = fsl_ddr_sdram();
-
+#else
+	dram_size =  fsl_ddr_sdram_size();
+#endif
 	return dram_size;
 }
 
diff --git a/board/freescale/ls1021aqds/ls1021aqds.c b/board/freescale/ls1021aqds/ls1021aqds.c
index 12e83f7..85a53c9 100644
--- a/board/freescale/ls1021aqds/ls1021aqds.c
+++ b/board/freescale/ls1021aqds/ls1021aqds.c
@@ -13,6 +13,7 @@ 
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <fsl_ifc.h>
+#include <spl.h>
 
 #include "../common/qixis.h"
 #include "ls1021aqds_qixis.h"
@@ -29,10 +30,13 @@  enum {
 int checkboard(void)
 {
 	char buf[64];
+#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
 	u8 sw;
+#endif
 
 	puts("Board: LS1021AQDS\n");
 
+#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
 	sw = QIXIS_READ(brdcfg[0]);
 	sw = (sw & QIXIS_LBMAP_MASK) >> QIXIS_LBMAP_SHIFT;
 
@@ -46,6 +50,7 @@  int checkboard(void)
 		printf("IFCCard\n");
 	else
 		printf("invalid setting of SW%u\n", QIXIS_LBMAP_SWITCH);
+#endif
 
 	printf("Sys ID:0x%02x, Sys Ver: 0x%02x\n",
 	       QIXIS_READ(id), QIXIS_READ(arch));
@@ -155,6 +160,32 @@  int board_early_init_f(void)
 	return 0;
 }
 
+#ifdef CONFIG_SPL_BUILD
+void board_init_f(ulong dummy)
+{
+	struct ccsr_cci400 *cci = (struct ccsr_cci400 *)CONFIG_SYS_CCI400_ADDR;
+
+	/* Set global data pointer */
+	gd = &gdata;
+
+	/* Clear the BSS */
+	memset(__bss_start, 0, __bss_end - __bss_start);
+
+	get_clocks();
+
+	preloader_console_init();
+
+#ifdef CONFIG_SPL_I2C_SUPPORT
+	i2c_init_all();
+#endif
+	out_le32(&cci->ctrl_ord, CCI400_CTRLORD_TERM_BARRIER);
+
+	dram_init();
+
+	board_init_r(NULL, 0);
+}
+#endif
+
 int config_board_mux(int ctrl_type)
 {
 	u8 reg12;
diff --git a/board/freescale/ls1021aqds/ls102xa_pbi.cfg b/board/freescale/ls1021aqds/ls102xa_pbi.cfg
new file mode 100644
index 0000000..edf9f94
--- /dev/null
+++ b/board/freescale/ls1021aqds/ls102xa_pbi.cfg
@@ -0,0 +1,8 @@ 
+#PBI commands
+
+#Configure Scratch register
+09ee0200 10000000
+#Configure alternate space
+09570158 00080000
+#Flush PBL data
+096100c0 000FFFFF
diff --git a/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg b/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
new file mode 100644
index 0000000..e05ec16
--- /dev/null
+++ b/board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
@@ -0,0 +1,14 @@ 
+#PBL preamble and RCW header
+aa55aa55 01ee0100
+
+#enable IFC, disable QSPI and DSPI
+#0608000a 00000000 00000000 00000000
+#00000000 00404000 60025a00 21042000
+#00200000 00000000 00000000 01038000
+#00000000 001b1200 00000000 00000000
+
+#disable IFC, enable QSPI and DSPI
+0608000a 00000000 00000000 00000000
+60000000 00407900 e0025a00 21046000
+00000000 00000000 00000000 00038000
+20024800 001b7200 00000000 00000000
diff --git a/configs/ls1021aqds_sdcard_defconfig b/configs/ls1021aqds_sdcard_defconfig
new file mode 100644
index 0000000..e03c3b4
--- /dev/null
+++ b/configs/ls1021aqds_sdcard_defconfig
@@ -0,0 +1,4 @@ 
+CONFIG_SPL=y
+CONFIG_SYS_EXTRA_OPTIONS="RAMBOOT_PBL,SPL_FSL_PBL,SD_BOOT"
++S:CONFIG_ARM=y
++S:CONFIG_TARGET_LS1021AQDS=y
diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h
index 657e3b6..440c118 100644
--- a/include/configs/ls1021aqds.h
+++ b/include/configs/ls1021aqds.h
@@ -37,8 +37,48 @@  unsigned long get_board_sys_clk(void);
 unsigned long get_board_ddr_clk(void);
 #endif
 
+#if defined(CONFIG_SD_BOOT) || defined(CONFIG_QSPI_BOOT)
+#define CONFIG_SYS_CLK_FREQ            100000000
+#define CONFIG_DDR_CLK_FREQ            100000000
+#define CONFIG_QIXIS_I2C_ACCESS
+#else
 #define CONFIG_SYS_CLK_FREQ		get_board_sys_clk()
 #define CONFIG_DDR_CLK_FREQ		get_board_ddr_clk()
+#endif
+
+#ifdef CONFIG_RAMBOOT_PBL
+#define CONFIG_SYS_FSL_PBL_PBI	board/freescale/ls1021aqds/ls102xa_pbi.cfg
+#endif
+
+#ifdef CONFIG_SD_BOOT
+#define CONFIG_SYS_FSL_PBL_RCW	board/freescale/ls1021aqds/ls102xa_rcw_sd.cfg
+#define CONFIG_SPL_PBL_PAD
+#define CONFIG_SPL_FRAMEWORK
+#define CONFIG_SPL_LDSCRIPT	"arch/arm/cpu/armv7/ls102xa/u-boot-spl.lds"
+#define CONFIG_SPL_LIBCOMMON_SUPPORT
+#define CONFIG_SPL_LIBGENERIC_SUPPORT
+#define CONFIG_SPL_ENV_SUPPORT
+#define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
+#define CONFIG_SPL_I2C_SUPPORT
+#define CONFIG_SPL_WATCHDOG_SUPPORT
+#define CONFIG_SPL_SERIAL_SUPPORT
+#define CONFIG_SPL_MMC_SUPPORT
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR		0xe8
+#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS		0x400
+
+#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_TEXT_BASE		0x82000000
+
+#define CONFIG_SYS_SPL_MALLOC_START	0x80200000
+#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
+#define CONFIG_SPL_BSS_START_ADDR	0x80100000
+#define CONFIG_SPL_BSS_MAX_SIZE		0x80000
+#define CONFIG_SYS_MONITOR_LEN		0x80000
+#define CONFIG_SYS_NO_FLASH
+#endif
 
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE		0x67f80000
@@ -70,6 +110,7 @@  unsigned long get_board_ddr_clk(void);
 /*
  * IFC Definitions
  */
+#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
 #define CONFIG_FSL_IFC
 #define CONFIG_SYS_FLASH_BASE		0x60000000
 #define CONFIG_SYS_FLASH_BASE_PHYS	CONFIG_SYS_FLASH_BASE
@@ -161,6 +202,7 @@  unsigned long get_board_ddr_clk(void);
 #define CONFIG_CMD_NAND
 
 #define CONFIG_SYS_NAND_BLOCK_SIZE	(128 * 1024)
+#endif
 
 /*
  * QIXIS Definitions
@@ -322,7 +364,12 @@  unsigned long get_board_ddr_clk(void);
 
 #define CONFIG_CMDLINE_TAG
 #define CONFIG_CMDLINE_EDITING
+
+#if !defined(CONFIG_SD_BOOT) && !defined(CONFIG_QSPI_BOOT)
 #define CONFIG_CMD_IMLS
+#else
+#undef CONFIG_CMD_IMLS
+#endif
 
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
@@ -370,17 +417,37 @@  unsigned long get_board_ddr_clk(void);
 #define CONFIG_SYS_INIT_SP_ADDR \
 	(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
 
+#ifdef CONFIG_SPL_BUILD
+#define CONFIG_SYS_MONITOR_BASE CONFIG_SPL_TEXT_BASE
+#else
 #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE    /* start of monitor */
+#endif
 
 /*
  * Environment
  */
 #define CONFIG_ENV_OVERWRITE
 
+#if defined(CONFIG_SD_BOOT)
+#define CONFIG_ENV_OFFSET		(1024 * 1024)
+#define CONFIG_ENV_IS_IN_MMC
+#define CONFIG_SYS_MMC_ENV_DEV		0
+#define CONFIG_ENV_SIZE			0x2000
+#elif defined(CONFIG_NAND_BOOT)
+#define CONFIG_ENV_IS_IN_NAND
+#define CONFIG_ENV_SIZE			0x2000
+#define CONFIG_ENV_OFFSET		(10 * CONFIG_SYS_NAND_BLOCK_SIZE)
+#elif defined(CONFIG_QSPI_BOOT)
+#define CONFIG_ENV_IS_IN_SPI_FLASH
+#define CONFIG_ENV_SIZE			0x2000          /* 8KB */
+#define CONFIG_ENV_OFFSET		0x100000        /* 1MB */
+#define CONFIG_ENV_SECT_SIZE		0x10000
+#else
 #define CONFIG_ENV_IS_IN_FLASH
 #define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE - CONFIG_ENV_SECT_SIZE)
 #define CONFIG_ENV_SIZE			0x2000
 #define CONFIG_ENV_SECT_SIZE		0x20000 /* 128K (one sector) */
+#endif
 
 #define CONFIG_OF_LIBFDT
 #define CONFIG_OF_BOARD_SETUP