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
