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

Bartlomiej Sieka
2008-02-21T15:57:47Z

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