From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@nvidia.com>
Date: Tue, 08 Nov 2011 11:15:15 -0700
Subject: [U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like
	image_get_{ram_disk, kernel}
In-Reply-To: <20111108160656.2507913BE0C2@gemini.denx.de>
References: <1320164902-24190-1-git-send-email-swarren@nvidia.com>
	<20111108160656.2507913BE0C2@gemini.denx.de>
Message-ID: <4EB971B3.4080807@nvidia.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

(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