[U-Boot-Users] [PATCH] Add 'imload' command

[Bartlomiej Sieka]

Grant Likely wrote:
> On Feb 13, 2008 12:55 PM, Bartlomiej Sieka <tur at semihalf.com> wrote:
>> Kumar Gala wrote:
>>> On Feb 13, 2008, at 4:11 AM, Bartlomiej Sieka wrote:
>>>
>>>> Kumar Gala wrote:
>>>>> 'imload' provides a more direct means to load from an image file.
>>>>> Also created a load_image routine out of the code in do_bootm() that
>>>>> is shared between do_bootm() and do_imload().
>>>>> Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
>>>>> ---
>>>>> Note, this is against the u-boot-testing new-image branch.
>>>> Thanks.
>>>>
>>>> Two comments:
>>>> - The load_image routine (and consequently imload commad) will not
>>>> work when the image is stored in Data Flash.
>>> what's the issue here?
>> Please have a look at code under CONFIG_HAS_DATAFLASH in get_kernel()
>> (formerly in do_bootm()), especially the read_dataflash() function. The
>> issue is that you have to copy data from Data Flash in a specific way in
>> order to have random access to it. So for example this line in your code:
>> type_name = image_get_type_name (image_get_type (hdr));
>> will effectively try to access hdr->ih_type, which will not work when
>> hdr is an address in Data Flash.
> 
> Ugh, please don't continue down that path.  Dataflash is a serial
> flash technology, but the driver pretends that it is memory mapped.
> It is not a good abstraction that I really think needs to be removed.

Hi Grant,

Not sure if your comment was directed to Kumar or me, but I'm replying
just in case.

I'm all for removing the code under CONFIG_HAS_DATAFLASH from at least
boot-related functions (haven't looked at other affected places in
U-Boot). We've not removed it in the new image format work, because by
default we try to preserve the status quo. When the new format handling
is introduced, we don't want to break too many things that people are
used to.

Regards,
Bartlomiej