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

[Stefan Roese <sr@denx.de>]

Hi Daniel,

On 08/24/2012 01:49 PM, Daniel Schwierzeck wrote:
>>> I still would like to move to my suggestion to not copy the header and
>>> use the mkimage header values ih_load and ih_ep directly. Right now I
>>> don't see any showstopper for doing it this way. I'll send a patch to
>>> change this shortly (if everything works out).
>>
>> Hmmm. As it seems some SPL loading drivers (block like mmc, streaming
>> like ymodem) are not that easily converted to skipping the header. So
>> I'm not so sure if we should go this way after all...
>>
>> Comments?
> 
> I did similar work for my upcoming Lantiq MIPS SoC port. In my
> approach I also support compressed u-boot
> images as payload (LZO, LZMA).

Yes, I thought about adding this as well. Nice.

> The u-boot image is merged with SPL
> image without any padding or
> fixed flash offsets to achieve a maximum reduction of flash footprint.

Interesting. I'm still padding to the fixed offset. Let me look into
squeezing those two images together as well...

> I ended up with doing something like this:
> 
> static int spl_parse_image(const image_header_t *hdr, struct spl_image *spl)
> {
>        u32 magic;
> 
>        magic = image_get_magic(hdr);
>        if (magic != IH_MAGIC)
>                return -1;
> 
>        spl->data_addr += image_get_header_size();
>        spl->entry_addr = image_get_load(hdr);
>        spl->size = image_get_data_size(hdr);
>        spl->comp = image_get_comp(hdr);
> 
>        if (spl->comp == IH_COMP_NONE)
>                spl->load_addr = spl->entry_addr;
>        else
>                spl->load_addr = CONFIG_LOADADDR;
> 
>        return 0;
> }

So you introduced a new "struct spl_image" and did not use "struct
spl_image_info" from the SPL framework? Why is that? We should
definitely consolidate to one structure here.

> spl->data_addr points to the image header of the payload and is
> initialized by the caller
> dependent on the used load mechanism.

Good idea. Looks like your spl_parse_image might be a potential
replacement for spl_parse_image_header() from this common SPL framework.

> If the payload is uncompressed
> it can be directly copied to
> its final RAM location. A compressed payload needs an intermediate
> copy step if it is stored in SPI or NAND flash.
> I chose CONFIG_LOADADDR. After this the decompression function
> extracts the image to its final RAM location.

Sounds good. Do you have some patches that you could send for review?
Did you try to keep as much platform/arch independent as possible?

> I think we should keep an architecture-neutral SPL image context
> inside the generic SPL framework.
> The content of this context should be filled by architecture/SoC/board
> specific code.

Ack. It would be great to review your code here. Please keep us informed.

Thanks,
Stefan