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

[Stephen Warren]

(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 at 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