[U-Boot-Users] [PATCH 3/6] [new uImage] Add gen_get_image() routine

[Bartlomiej Sieka <tur@semihalf.com>]

Kumar Gala wrote:
>>>> +/**
>>>> + * gen_get_image - get image from special storage (if necessary)
>>>> + * @img_addr: image start address
>>>> + *
>>>> + * gen_get_image() checks if provided image start adddress is located
>>>> + * in a dataflash storage. If so, image is moved to a system RAM 
>>>> memory.
>>>> + *
>>>> + * returns:
>>>> + *     image start address after possible relocation from special 
>>>> storage
>>>> + */
>>>> +ulong gen_get_image (ulong img_addr)
>>>> +{
>>>> +    ulong ram_addr, h_size, d_size;
>>>> +
>>>> +    h_size = image_get_header_size ();
>>>> +#if defined(CONFIG_FIT)
>>>> +    if (sizeof(struct fdt_header) > h_size)
>>>> +        h_size = sizeof(struct fdt_header);
>>>> +#endif
>>>> +
>>>> +#ifdef CONFIG_HAS_DATAFLASH
>>>> +    if (addr_dataflash (img_addr)){
>>>> +        ram_addr = CFG_LOAD_ADDR;
>>>> +        debug ("   Reading image header from dataflash address "
>>>> +            "%08lx to RAM address %08lx\n", img_addr, ram_addr);
>>>> +        read_dataflash (img_addr, h_size, (char *)ram_addr);
>>>> +    } else
>>>> +#endif
>>>> +    ram_addr = img_addr;
>>> can we not early out at this point?
>>
>> I'm not sure what you mean here -- could you clarify?
> 
> Just that it seems like the code should be something like:
> 
> #ifdef CONFIG_HAS_DATAFLASH
>     if (addr_dataflash(img_addr)) {
>         ...
>     } else
> #endif
>         return img_addr;
> 
> unless I'm missing something the code should return 'img_addr' if we 
> don't have to deal with dataflash (either by it not being config'd in or 
> 'img_addr' isn't in a dataflash.

You're right, I'll make this change for the re-spin.

Cheers,
Bartlomiej