On 05/08/2013 05:04:23 AM, ying.zhang@freescale.com wrote:
From: Ying Zhang b40530@freescale.com
This patch introduces SPL to enable a loader stub that runs in the L2 SRAM, after being loaded by the code from the internal on-chip ROM. It loads the final uboot image into DDR, then jump to it to begin execution.
The SPL's size is sizeable, the maximum size must not exceed the size of L2 SRAM.It initializes the DDR through SPD code, and copys final uboot image to DDR. So there are two stage uboot images: * spl_boot, 96KB size. The env variables are copied to offset 96KB. in L2 SRAM, so that ddr spd code can get the interleaving mode setting in env. It loads final uboot image from offset 96KB. * final uboot image, size is variable depends on the functions enabled.
Signed-off-by: Ying Zhang b40530@freescale.com
Andy Fleming afleming@gmail.com is the MMC and mpc85xx maintainer; please CC him on these patches.
This patch needs to be broken into several patches that each take care of one logical problem; it's too hard to properly review (and have the right people pay attention to certain parts) in its current state.
Is this on top of the already-posted P1022DS NAND patch? If so, please say so.
diff --git a/README b/README index 862bb3e..f974bca 100644 --- a/README +++ b/README @@ -2887,6 +2887,10 @@ FIT uImage format: CONFIG_SPL_INIT_MINIMAL Arch init code should be built for a very small image
CONFIG_SPL_INIT_NORMALThis is relative to MINIMAL, this init code containssome
features that the minimal SPL doesn't contains.
Why do we need a symbol for this? The only place you use it is already limited to CONFIG_SPL_BUILD and !CONFIG_SPL_INIT_MINIMAL.
diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c index 0dff37f..d21b324 100644 --- a/arch/powerpc/cpu/mpc85xx/tlb.c +++ b/arch/powerpc/cpu/mpc85xx/tlb.c @@ -55,7 +55,7 @@ void init_tlbs(void) return ; }
-#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD_MINIMAL) void read_tlbcam_entry(int idx, u32 *valid, u32 *tsize, unsigned long *epn, phys_addr_t *rpn) {
Aren't you breaking the existing minimal targets? CONFIG_SPL_BUILD_MINIMAL does not currently exist, and you add it only for P1022DS.
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds index f2b7bff..f1a9ac9 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds @@ -26,6 +26,13 @@ #include "config.h" /* CONFIG_BOARDDIR */
OUTPUT_ARCH(powerpc) +#ifdef CONFIG_SYS_MPC85XX_NO_RESETVEC +PHDRS +{
- text PT_LOAD;
- bss PT_LOAD;
+} +#endif
Whitespace.
@@ -68,9 +80,21 @@ SECTIONS #else #error unknown NAND controller #endif
+#ifndef CONFIG_FSL_IFC +#ifdef CONFIG_SYS_MPC85XX_NO_RESETVEC
- .bootpg ADDR(.text) - 0x1000 :
- {
KEEP(*(.bootpg))- } :text = 0xffff
+#endif +#endif
+#ifndef CONFIG_SYS_MPC85XX_NO_RESETVEC .resetvec ADDR(.text) + RESET_VECTOR_OFFSET : { KEEP(*(.resetvec)) } = 0xffff +#endif
Get rid of the IFC ifdef -- this should apply equally well to IFC.
Can you make the "no resetvec" stuff a separate patch?
@@ -83,5 +107,6 @@ SECTIONS *(.sbss*) *(.bss*) }
- . = ALIGN(4); __bss_end = .;
}
This seems unrelated.
diff --git a/board/freescale/common/sdhc_boot.c b/board/freescale/common/sdhc_boot.c index e432318..96b0680 100644 --- a/board/freescale/common/sdhc_boot.c +++ b/board/freescale/common/sdhc_boot.c @@ -24,6 +24,7 @@ #include <mmc.h> #include <malloc.h>
+DECLARE_GLOBAL_DATA_PTR; /*
- The environment variables are written to just after the u-boot
image
- on SDCard, so we must read the MBR to get the start address and
code @@ -31,6 +32,9 @@ */ #define ESDHC_BOOT_IMAGE_SIZE 0x48 #define ESDHC_BOOT_IMAGE_ADDR 0x50 +#define ESDHC_BOOT_IMAGE_SIGN 0x55AA +#define ESDHC_BOOT_IMAGE_SIGN_ADDR 0x1FE +#define CONFIG_CFG_DATA_SECTOR 0
int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) { @@ -61,3 +65,122 @@ int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr)
return 0; }
+#ifdef CONFIG_SPL_BUILD +void mmc_get_env(void) +{
- /* load environment */
- struct mmc *mmc;
- int err;
- u32 offset;
- uint blk_start, blk_cnt, ret;
- mmc_initialize(gd->bd);
- /* We register only one device. So, the dev id is always 0 */
- mmc = find_mmc_device(0);
- if (!mmc) {
puts("spl: mmc device not found!!\n");hang();- }
- err = mmc_init(mmc);
- if (err) {
puts("spl: mmc init failed!");hang();- }
- if (1 == mmc_get_env_addr(mmc, &offset))
puts("spl: mmc get env error!!\n");
Constants go on the right hand side of comparisons.
- val = *(u16 *)(tmp_buf + ESDHC_BOOT_IMAGE_SIGN_ADDR);
- if ((u16)ESDHC_BOOT_IMAGE_SIGN != val) {
free(tmp_buf);return;- }
Why do you need this cast?
- offset = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_ADDR);
- offset += CONFIG_SYS_MMC_U_BOOT_OFFS;
- /* Get the code size from offset 0x48 */
- code_len = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_SIZE);
- code_len -= CONFIG_SYS_MMC_U_BOOT_OFFS;
- /*
* Load U-Boot image from mmc into RAM- */
- /*
- SDHC card: code offset and length is stored in block units
rather
- than a single byte
- */
/* * U-Boot multiline * comment style is * like this. */
- blk_start = ALIGN(offset, mmc->read_bl_len) / mmc->read_bl_len;
- blk_cnt = ALIGN(code_len, mmc->read_bl_len) / mmc->read_bl_len;
- err = mmc->block_dev.block_read(0, blk_start, blk_cnt,
(uchar*)CONFIG_SYS_MMC_U_BOOT_DST);
- if (err != blk_cnt) {
free(tmp_buf);return ;- }
+} +void mmc_boot(void)
No space before ;
That return is pointless since you're at the end of the function anyway.
Please put a blank line between functions.
diff --git a/board/freescale/common/sdhc_boot.h b/board/freescale/common/sdhc_boot.h new file mode 100644 index 0000000..2b5dcb9 --- /dev/null +++ b/board/freescale/common/sdhc_boot.h @@ -0,0 +1,29 @@ +/*
- Copyright 2011 Freescale Semiconductor, Inc.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#ifndef __SDHC_BOOT_H_ +#define __SDHC_BOOT_H_ 1
+int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr); +void mmc_get_env(void); +void mmc_boot(void);
+#endif /* __SDHC_BOOT_H_ */
Does this stuff really belong in board/freescale? Should probably at least be in arch/powerpc/cpu/mpc85xx, if not more generic.
+#include <common.h> +#include <ns16550.h> +#include <nand.h> +#include <asm/fsl_law.h> +#include <asm/fsl_ddr_sdram.h> +#include <malloc.h> +#ifdef CONFIG_SDCARD +#include <mmc.h> +#include <i2c.h> +#include "../common/sdhc_boot.h" +#endif +#ifdef CONFIG_SPIFLASH +#include <i2c.h> +#include "../common/ngpixis.h" +#include "../common/spi_boot.h" +#endif
Don't ifdef includes.
Don't use boards.cfg-defined config symbols outside the board config header, unless they're proper U-Boot config symbols which requires better naming, and documentation in README.
+void hang(void) +{
- puts("### ERROR ### Please RESET the board ###\n");
- for (;;)
;+}
Whitespace
+ulong get_effective_memsize(void) +{
- return CONFIG_SYS_L2_SIZE;
+}
+void board_init_f(ulong bootflag) +{
- int px_spd;
- u32 plat_ratio, sys_clk, bus_clk;
- ccsr_gur_t *gur = (void *)CONFIG_SYS_MPC85xx_GUTS_ADDR;
- console_init_f();
+#if defined(CONFIG_SDCARD) || defined(CONFIG_SPIFLASH)
- /* Set pmuxcr to allow both i2c1 and i2c2 */
- setbits_be32(&gur->pmuxcr, in_be32(&gur->pmuxcr) | 0x1000);
- setbits_be32(&gur->pmuxcr,
in_be32(&gur->pmuxcr) | MPC85xx_PMUXCR_SD_DATA);+#ifdef CONFIG_SPIFLASH
- /* Enable the SPI */
- clrsetbits_8(&pixis->brdcfg0, PIXIS_ELBC_SPI_MASK, PIXIS_SPI);
+#endif
- /* Read back the register to synchronize the write. */
- in_be32(&gur->pmuxcr);
+#endif
- /* initialize selected port with appropriate baud rate */
- px_spd = in_8((unsigned char *)(PIXIS_BASE + PIXIS_SPD));
- sys_clk = sysclk_tbl[px_spd & PIXIS_SPD_SYSCLK_MASK];
- plat_ratio = in_be32(&gur->porpllsr) &
MPC85xx_PORPLLSR_PLAT_RATIO;
- bus_clk = sys_clk * plat_ratio / 2;
- NS16550_init((NS16550_t)CONFIG_SYS_NS16550_COM1,
bus_clk / 16 / CONFIG_BAUDRATE);+#ifdef CONFIG_SDCARD
- puts("\nSD boot...\n");
+#endif +#ifdef CONFIG_NAND
- puts("\nNAND boot...\n");
+#endif +#ifdef CONFIG_SPIFLASH
- puts("\nSPI Flash boot...\n");
+#endif
Is NAND handled by this file? Isn't this on top of the patch that already adds NAND boot support? I don't see this print being removed from somewhere else.
diff --git a/common/Makefile b/common/Makefile index 0e0fff1..bc80414 100644 --- a/common/Makefile +++ b/common/Makefile @@ -225,6 +225,11 @@ COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_common.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_flags.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_nowhere.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o +COBJS-$(CONFIG_SPL_HWCONFIG_SUPPORT) += hwconfig.o +COBJS-$(CONFIG_SPL_INIT_DDR_SUPPORT) += ddr_spd.o +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_attr.o +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_flags.o +COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_callback.o
CONFIG_SPL_ENV_SUPPORT should replace CONFIG_SPL_NET_SUPPORT here (and add it to the boards that already have CONFIG_SPL_NET_SUPPORT). This sort of refactoring needs to be a separate patch, BTW.
Can ddr_spd.o and hwconfig just use their normal CONFIG symbols (i.e. move the existing makefile line out of the !SPL ifdef)? It's getting a bit excessive with all the new SPL symbols.
Maybe one day we can redo it to be a separate config namespace, like I suggested a while ago. :-P It would certainly help with three-stage boot support.
endif COBJS-$(CONFIG_BOUNCE_BUFFER) += bouncebuf.o COBJS-y += console.o diff --git a/doc/README.mpc85xx-sd-spi-boot b/doc/README.mpc85xx-sd-spi-boot new file mode 100644 index 0000000..79f91fc --- /dev/null +++ b/doc/README.mpc85xx-sd-spi-boot @@ -0,0 +1,82 @@ +---------------------------------------- +Booting from On-Chip ROM (eSDHC or eSPI) +----------------------------------------
+boot_format is a tool to write SD bootable images to a filesystem and build +SD/SPI images to a binary file for writing later.
+When booting from an SD card/MMC, boot_format puts the configuration file and +the RAM-based U-Boot image on the card. +When booting from an EEPROM, boot_format generates a binary image that is used +to boot from this EEPROM.
+Where to get boot_format: +========================
+you can browse it online at: +http://git.freescale.com/git/cgit.cgi/ppc/sdk/boot-format.git/
+Building +========
+Run the following to build this project
- $ make
+Execution +=========
+boot_format runs under a regular Linux machine and requires a super user mode +to run. Execute boot_format as follows.
+For building SD images by writing directly to a file system on SD media:
- $ boot_format $config u-boot.bin -sd $device
+Where $config is the included config.dat file for your platform and $device +is the target block device for the SD media on your computer.
+For build binary images directly a local file:
- $ boot_format $config u-boot.bin -spi $file
+Where $file is the target file. Also keep in mind the u-boot.bin file needs +to be the u-boot built for your particular platform and target media.
+Hint: To generate a u-boot.bin for a P1022DS booting from SD I would run the +following in the u-boot repository:
- $ make P1022DS_SDCARD
s/Hint/Example/ and s/from SD I would run/from SD, run/
diff --git a/lib/Makefile b/lib/Makefile index e901cc7..ab12e63 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -66,6 +66,11 @@ endif COBJS-$(CONFIG_SPL_NET_SUPPORT) += errno.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += hashtable.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += net_utils.o +COBJS-$(CONFIG_SPL_ADDR_MAP_SUPPORT) += addr_map.o
+COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += hashtable.o +COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += display_options.o +COBJS-$(CONFIG_SPL_UTILITYLIB_SUPPORT) += errno.o
As with common/Makefile, Can these just be the normal makefile lines, moved outside the SPL ifdef (and no duplications with CONFIG_SPL_NET_SUPPORT)?
diff --git a/spl/Makefile b/spl/Makefile index b5a8de7..3a3b868 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -51,6 +51,9 @@ LIBS-y += arch/powerpc/cpu/mpc8xxx/lib8xxx.o endif ifeq ($(CPU),mpc85xx) LIBS-y += arch/powerpc/cpu/mpc8xxx/lib8xxx.o +ifdef CONFIG_SPL_INIT_DDR_SUPPORT +LIBS-y += arch/powerpc/cpu/mpc8xxx/ddr/libddr.o +endif
Why isn't this handled as part of lib8xxx.o? We should avoid putting hardware-specific things in generic Makefiles. There ones that are already there should be fixed at some point.
-Scott