Dear Haiying.Wang@freescale.com,
In message 1296499317-26616-4-git-send-email-Haiying.Wang@freescale.com you wrote:
From: Haiying Wang Haiying.Wang@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