On Mon, Aug 27, 2012 at 11:16:45AM -0500, Scott Wood wrote:
On 08/27/2012 09:37 AM, Tom Rini wrote:
On 08/24/2012 05:09 PM, Scott Wood wrote:
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.
Yes, spl_nand_load_image will be garbage collected and not link-error if not called. But note that all users of this file have been converted to CONFIG_SPL_FRAMEWORK and would be using this function.
There are still a lot of nand_spl targets that have not yet been converted, some of which will be future users of this file (such as ppc 4xx). This file is a replacement for nand_spl/nand_boot.c and will be used by the same SPLs.
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?
I would start by questioning the need of a 3rd SPL framework.
The "simple" driver does not work for all hardware. This is why we have nand_spl/nand_boot_fsl_elbc.c and others in addition to nand_spl/nand_boot.c. It's not a "3rd SPL framework", just a different NAND implementation.
The question boils down to, what are your size constraints? I guess what I'm saying is, if it's <4kb, it's not using this file nor the framework. If we've got more than 4kb to work with, it's using the framework (with changes if needed, of course) and I guess we could move the function to common/spl/spl_nand.c and add drivers/mtd/nand/nand_spl_fsl_elbc.c and so on. Now that I've had more coffee, do I follow your suggestion right?