[U-Boot] [PATCH v4 19/20] SPL: NAND: Enhance drivers/mtd/nand/nand_spl_simple.c

[Scott Wood <scottwood@freescale.com>]

On 08/24/2012 06:58 PM, Tom Rini wrote:
> Takes the load function from arch/arm/cpu/armv7/omap-common/spl_nand.c
> instead.  This will allow for easier integration of SPL-boots-Linux code on
> other arches.
> 
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> Changes in v4:
> - Leave nand_spl_load.c alone, move the new load into nand_spl_simple.c
[snip]
> +void spl_nand_load_image(void)
> +{
> +	struct image_header *header;
> +	int *src __attribute__((unused));
> +	int *dst __attribute__((unused));
> +
> +	nand_init();
> +
> +	/* use CONFIG_SYS_TEXT_BASE as temporary storage area */
> +	header = (struct image_header *)(CONFIG_SYS_TEXT_BASE);
> +#ifdef CONFIG_SPL_OS_BOOT
> +	if (!spl_start_uboot()) {
> +		/*
> +		 * load parameter image
> +		 * load to temp position since nand_spl_load_image reads
> +		 * a whole block which is typically larger than
> +		 * CONFIG_CMD_SPL_WRITE_SIZE therefore may overwrite
> +		 * following sections like BSS
> +		 */
> +		nand_spl_load_image(CONFIG_CMD_SPL_NAND_OFS,
> +			CONFIG_CMD_SPL_WRITE_SIZE,
> +			(void *)CONFIG_SYS_TEXT_BASE);
> +		/* copy to destintion */
> +		for (dst = (int *)CONFIG_SYS_SPL_ARGS_ADDR,
> +				src = (int *)CONFIG_SYS_TEXT_BASE;
> +				src < (int *)(CONFIG_SYS_TEXT_BASE +
> +				CONFIG_CMD_SPL_WRITE_SIZE);
> +				src++, dst++) {
> +			writel(readl(src), dst);
> +		}
> +
> +		/* load linux */
> +		nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS,
> +			CONFIG_SYS_NAND_PAGE_SIZE, (void *)header);
> +		spl_parse_image_header(header);
> +		if (header->ih_os == IH_OS_LINUX) {
> +			/* happy - was a linux */
> +			nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS,
> +				spl_image.size, (void *)spl_image.load_addr);
> +			nand_deselect();
> +			return;
> +		} else {
> +			puts("The Expected Linux image was not "
> +				"found. Please check your NAND "
> +				"configuration.\n");
> +			puts("Trying to start u-boot now...\n");
> +		}
> +	}
> +#endif
> +#ifdef CONFIG_NAND_ENV_DST
> +	nand_spl_load_image(CONFIG_ENV_OFFSET,
> +		CONFIG_SYS_NAND_PAGE_SIZE, (void *)header);
> +	spl_parse_image_header(header);
> +	nand_spl_load_image(CONFIG_ENV_OFFSET, spl_image.size,
> +		(void *)spl_image.load_addr);
> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	nand_spl_load_image(CONFIG_ENV_OFFSET_REDUND,
> +		CONFIG_SYS_NAND_PAGE_SIZE, (void *)header);
> +	spl_parse_image_header(header);
> +	nand_spl_load_image(CONFIG_ENV_OFFSET_REDUND, spl_image.size,
> +		(void *)spl_image.load_addr);
> +#endif
> +#endif
> +	/* Load u-boot */
> +	nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		CONFIG_SYS_NAND_PAGE_SIZE, (void *)header);
> +	spl_parse_image_header(header);
> +	nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		spl_image.size, (void *)spl_image.load_addr);
> +	nand_deselect();
> +}

Will this refuse to link if spl_parse_image_header is not present, or
will gc-sections remove it before the error is given?  Does this
function leave any anonymous data that isn't cleaned up by gc-sections?
Again, this file must not grow for users that don't need the new features.

What is the benefit of putting this in nand_spl_simple.c versus another
file?  What if someone wants to use this with a different NAND boot
implementation?

-Scott