Dear Stephen Warren,

In message 74CDBE0F657A3D45AFBB94109FB122FF173F9A5415@HQMAIL01.nvidia.com you wrote:

Wolfgang Denk wrote at Tuesday, November 08, 2011 9:07 AM:

...

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.

...

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?

The new code is different from what was there before.

Now we have:

1134 image_print_contents(fdt_hdr); 1135 1136 puts(" Verifying Checksum ... "); 1137 if (!image_check_hcrc(fdt_hdr)) { 1138 fdt_error("fdt header checksum invalid"); 1139 return NULL; 1140 } 1141 1142 if (!image_check_dcrc(fdt_hdr)) { 1143 fdt_error("fdt checksum invalid"); 1144 return NULL;

i.e. image_print_contents(fdt_hdr); puts(" Verifying Checksum ... "); image_check_hcrc(fdt_hdr); image_check_dcrc(fdt_hdr)

After your patch we have:

image_check_magic(); image_check_hcrc() image_print_contents((); puts(" Verifying Checksum ... "); image_check_dcrc();

So before, we have the "Verifying Checksum" before running any of the image_check_*() functions, while with your patch we run two of them before that.

Best regards,

Wolfgang Denk