[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

Stephen Warren
2011-11-08T18:15:15Z

(Resending due to MIME encoding last time. Sorry; I really have to stop
using Outlook for this list for some reason)

On 11/08/2011 09:06 AM, Wolfgang Denk wrote:
> In message <1320164902-24190-1-git-send-email-swarren@nvidia.com> you wrote:
>> image_get_ram_disk() and image_get_kernel() perform operations in a
>> consistent order. Modify image_get_fdt() to do things the same way.
>> This allows a later change to insert some image header manipulations
>> into these three functions in a consistent fashion.
...
>> @@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
>> {
>> const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
>>
>> - image_print_contents(fdt_hdr);
>> + if (!image_check_magic(fdt_hdr)) {
>> + fdt_error("fdt header bad magic number\n");
>> + return NULL;
>> + }
>>
>> - puts(" Verifying Checksum ... ");
>> if (!image_check_hcrc(fdt_hdr)) {
>> fdt_error("fdt header checksum invalid");
>> return NULL;
>> }
>>
>> + image_print_contents(fdt_hdr);
>> +
>> + puts(" Verifying Checksum ... ");
>> if (!image_check_dcrc(fdt_hdr)) {
>> fdt_error("fdt checksum invalid");
>> return NULL;
>
> The rule in U-Boot when generating output is to print a message
> before you start an action, and then either print an OK or an error
> message. The reason for this is debug support: if neither an OK nor
> an error comes you know that the test somehow crashed.
>
> Here this principle is violated as image_check_magic() and
> image_check_hcrc() will run without being announced.
>
> Please move the output so we get a message printed before starting to
> perform the actual tests.

The new code is exactly the same as the existing image_get_kernel() and
image_get_ramdisk(). Are those wrong? I wouldn't want to fix my patch to
conform to some supposed standard when the existing code that's been
accepted doesn't conform to that standard, or would I be responsible for
fixing up that too?

But anyway, I imagine there's no point discussing this patch further,
because its sole purpose is to support the uImage load_address=-1 patch
that follows, and it's pretty clear that won't be accepted, so please
consider this patch series withdrawn too.

--
nvpublic