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

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5415 at 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 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.
...
> 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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Intel's new motto: United we stand. Divided we fall!