From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bartlomiej Sieka <tur@semihalf.com>
Date: Thu, 21 Feb 2008 16:57:47 +0100
Subject: [U-Boot-Users] [PATCH 3/6] [new uImage] Add gen_get_image()
	routine
In-Reply-To: <3D010FD2-EE46-44C3-9266-B4F0643CF099@kernel.crashing.org>
References: <20080220171950.25624.71201.stgit@pollux.denx.de>
	<20080220172009.25624.30095.stgit@pollux.denx.de>
	<7814D974-88CD-4C3C-B716-7022658C810B@kernel.crashing.org>
	<47BC8FE3.6090500@semihalf.com>
	<3D010FD2-EE46-44C3-9266-B4F0643CF099@kernel.crashing.org>
Message-ID: <47BD9F7B.6080607@semihalf.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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