[U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc

[Stefan Roese]

Hi Tom,

On 08/23/2012 09:31 PM, Tom Rini wrote:
>>>> @@ -89,7 +106,11 @@ void spl_parse_image_header(const struct image_header *header)
>>>>  		spl_image.size = __be32_to_cpu(header->ih_size) + header_size;
>>>>  		spl_image.entry_point = __be32_to_cpu(header->ih_load);
>>>>  		/* Load including the header */
>>>> +#ifdef CONFIG_ARM
>>>>  		spl_image.load_addr = spl_image.entry_point - header_size;
>>>> +#else
>>>> +		spl_image.load_addr = __be32_to_cpu(header->ih_load);
>>>> +#endif
>>>
>>> This isn't an ARM-ism but is instead because spl_nor.c isn't offsetting
>>> where the header is like mmc/nand/ymodem do, yes?  Would it be possible
>>> to make spl_nor.c behave like the others?  One of the reasons I ask is
>>> I'm looking at a NOR chip on my desk...
>>
>> I was wondering about this line as well. Please explain: Why can't ARM
>> just use header->ih_load as load_addr?
> 
> Off the top of my head, I believe what goes on is that we read things
> into SDRAM such that the header is taken into account and we don't need
> to relocate the payload (U-Boot or Linux).

Hmmm. So for example, when ih_load is set to 0x100000, then you load the
image to (0x100000 - 0x40) = 0xfffc0? Is this correct?

This can't work for powerpc. As here for Linux both load-address and
entry-point are set to 0. So when loading the image (e.g. from NOR flash)
can't copy the image header in front of the image.

Another thing I'm wondering about: Why is only ih_load from the mkimage
header used and not ih_ep (entry-point)?

I suggest that we switch to copying the "real" image (payload) to the load
address, skipping the header. Then "ih_load" and "ih_ep" can be used
without modification.

BTW: There also seems to be a bug in some of the SPL loaders:

For example in drivers/mtd/nand/nand_spl_load.c:

		...
		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);

The problem here is that the last 64 bytes of the image are not
copied to SDRAM. Since the header is copied which is not included
in the spl_image.size variable. With my idea of only copying
the payload (skipping the header) this would be:

			nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS +
				sizeof(struct image_header),
				spl_image.size, (void *)spl_image.load_addr);

What do you think? Should we switch to this way of loading images?
Seems more logical to me. And we don't run into problems where the
load address is 0.

Thanks,
Stefan