Dear haiying.w...@freescale.com, In message <1296499317-26616-4-git-send-email-haiying.w...@freescale.com> you wrote: > From: Haiying Wang <haiying.w...@freescale.com> > > Support P1021MDS board to boot from NAND flash (No NOR flash on this > board). And because P1021 only has 256K L2 SRAM, which can not used for final > uboot image, this patch also enables the TPL BOOT on P1021MDS so that DDR can > be initialized in L2 SRAM through SPD code. So there are three stage uboot > images: > * nand_spl, pad from 4KB size to 16KB, load tpl_boot from offset 16KB in NAND. > * tpl_boot, 112KB size. The env variables are copied to offset 128KB > in L2 SRAM, so that ddr spd code can get the interleaving mode setting in > env. > It loads final uboot image from offset 128KB in NAND. > * final uboot image, size is variable depends on the functions enabled.
> diff --git a/board/freescale/p1021mds/config.mk > b/board/freescale/p1021mds/config.mk > new file mode 100644 > index 0000000..3888f61 > --- /dev/null > +++ b/board/freescale/p1021mds/config.mk ... > +ifndef NAND_SPL > +ifndef IN_TPL > +ifeq ($(CONFIG_NAND), y) > +LDSCRIPT := $(TOPDIR)/$(CPUDIR)/u-boot-nand.lds > +endif > +endif > +endif Why is this config.mk needed? Can you not do all this in the board config file instead? > diff --git a/board/freescale/p1021mds/ddr.c b/board/freescale/p1021mds/ddr.c > new file mode 100644 > index 0000000..594a4a8 > --- /dev/null > +++ b/board/freescale/p1021mds/ddr.c It seems there are a number of functions here which ar actually shared with other files, for example board/freescale/p1022ds/ddr.c. I wonder if it is not possible to use more common code here - especially given the fact that we already have a nice collection of such files: board/freescale/corenet_ds/ddr.c board/freescale/mpc8536ds/ddr.c board/freescale/mpc8540ads/ddr.c board/freescale/mpc8541cds/ddr.c board/freescale/mpc8544ds/ddr.c board/freescale/mpc8548cds/ddr.c board/freescale/mpc8555cds/ddr.c board/freescale/mpc8560ads/ddr.c board/freescale/mpc8568mds/ddr.c board/freescale/mpc8569mds/ddr.c board/freescale/mpc8572ds/ddr.c board/freescale/mpc8610hpcd/ddr.c board/freescale/mpc8641hpcn/ddr.c board/freescale/p1022ds/ddr.c board/freescale/p1_p2_rdb/ddr.c board/freescale/p2020ds/ddr.c > diff --git a/board/freescale/p1021mds/p1021mds.c > b/board/freescale/p1021mds/p1021mds.c > new file mode 100644 > index 0000000..c7a7e57 > --- /dev/null > +++ b/board/freescale/p1021mds/p1021mds.c ... > +extern void cpu_mp_lmb_reserve(struct lmb *lmb); Please move prototypes to header file. > +void board_lmb_reserve(struct lmb *lmb) > +{ > + cpu_mp_lmb_reserve(lmb); > +} How many board/freescale/<name>/<name>.c file share this same code? > diff --git a/board/freescale/p1021mds/tlb.c b/board/freescale/p1021mds/tlb.c > new file mode 100644 > index 0000000..30af6dd > --- /dev/null > +++ b/board/freescale/p1021mds/tlb.c How much of this is actually different from - say - board/freescale/p1022ds/tlb.c ? ... > +/* > + * Environment Configuration > + */ > +#define CONFIG_HOSTNAME p1021mds > +#define CONFIG_ROOTPATH /nfsroot > +#define CONFIG_BOOTFILE your.uImage Please rather omit the setting instead of using fillers that are of no practical value. > +#define CONFIG_LOADADDR 1000000 /*default location for tftp and > bootm*/ > + > +#define CONFIG_BOOTDELAY 10 /* -1 disables auto-boot */ > +#undef CONFIG_BOOTARGS /* the boot command will set bootargs*/ > + > +#define CONFIG_EXTRA_ENV_SETTINGS > \ > + "netdev=eth0\0" \ > + "consoledev=ttyS0\0" \ > + "ramdiskaddr=2000000\0" \ > + "ramdiskfile=your.ramdisk.u-boot\0" \ Ditto. [BTW: why "....ramdisk.u-boot"? U-Boot does not use ramdisks. The ramdisk is only used for some OS, so that should probably be "...ramdisk.linux" instead?] > + "fdtaddr=c00000\0" \ > + "fdtfile=your.fdt.dtb\0" \ Ditto. [Are "fdt" and "dtb" not redundant?] > diff --git a/tpl/board/freescale/p1021mds/tpl_boot.c > b/tpl/board/freescale/p1021mds/tpl_boot.c > new file mode 100644 > index 0000000..386d76c > --- /dev/null > +++ b/tpl/board/freescale/p1021mds/tpl_boot.c ... > +extern void nand_load(unsigned int offs, int uboot_size, uchar *dst); > +extern phys_size_t init_ddr_dram(void); Please move prototypes to header files. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Any sufficiently advanced bug is indistinguishable from a feature. - Rich Kulawiec _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot