[PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jerry Van Baren]

This makes padding out the blob if the user requested extra size much
easer.  The assembly and writing to the file is more straight forward too.

Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
---

Hi David, Jon,

I wasn't happy with David's suggestion on the -S handling.  I also wasn't
all that wild about how the blob was assembled and then written to the
file piecemeal with "ad-hoc" alignment padding.  I realized I could fix
both by assembling the blob in memory as a "struct data" and then write
the whole thing out in one fell swoop.

Makes it a lot simpler and cleaner to my eyes.

Hope y'all agree,
gvb


 flattree.c |   82 +++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/flattree.c b/flattree.c
index 151d16e..caff758 100644
--- a/flattree.c
+++ b/flattree.c
@@ -310,6 +310,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 	
 	return d;
 }
+
 static void make_bph(struct boot_param_header *bph,
 		     struct version_info *vi,
 		     int reservesize, int dtsize, int strsize,
@@ -332,19 +333,8 @@ static void make_bph(struct boot_param_header *bph,
 	bph->off_dt_struct = cpu_to_be32(reserve_off + reservesize);
 	bph->off_dt_strings = cpu_to_be32(reserve_off + reservesize
 					  + dtsize);
-	bph->totalsize = reserve_off + reservesize + dtsize + strsize;
-	if (minsize > 0) {
-		if (bph->totalsize >= minsize) {
-			if (quiet < 1)
-				fprintf(stderr,
-					"Warning: blob size %d >= minimum size %d\n",
-					bph->totalsize, minsize);
+	bph->totalsize = cpu_to_be32(reserve_off + reservesize + dtsize + strsize);
 
-		} else
-			bph->totalsize = minsize;
-	}
-	bph->totalsize = cpu_to_be32(bph->totalsize);
-		
 	if (vi->flags & FTF_BOOTCPUID)
 		bph->boot_cpuid_phys = cpu_to_be32(boot_cpuid_phys);
 	if (vi->flags & FTF_STRTABSIZE)
@@ -358,11 +348,11 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 {
 	struct version_info *vi = NULL;
 	int i;
-	struct data dtbuf = empty_data;
-	struct data strbuf = empty_data;
-	struct data reservebuf;
+	struct data blob       = empty_data;
+	struct data reservebuf = empty_data;
+	struct data dtbuf      = empty_data;
+	struct data strbuf     = empty_data;
 	struct boot_param_header bph;
-	struct reserve_entry termre = {.address = 0, .size = 0};
 
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
@@ -371,9 +361,6 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	if (!vi)
 		die("Unknown device tree blob version %d\n", version);
 
-	dtbuf = empty_data;
-	strbuf = empty_data;
-
 	flatten_tree(bi->dt, &bin_emitter, &dtbuf, &strbuf, vi);
 	bin_emit_cell(&dtbuf, OF_DT_END);
 
@@ -383,28 +370,46 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
 	make_bph(&bph, vi, reservebuf.len, dtbuf.len, strbuf.len,
 		 boot_cpuid_phys);
 
-	fwrite(&bph, vi->hdr_size, 1, f);
-
-	/* Align the reserve map to an 8 byte boundary */
-	for (i = vi->hdr_size; i < be32_to_cpu(bph.off_mem_rsvmap); i++)
-		fputc(0, f);
+	/*
+	 * Assemble the blob: start with the header, add with alignment
+	 * the reserve buffer, add the reserve map terminating zeroes,
+	 * the device tree itself, and finally the strings.
+	 */
+	blob = data_append_data(blob, &bph, sizeof(bph));
+	blob = data_append_align(blob, 8);
+	blob = data_merge(blob, reservebuf);
+	blob = data_append_zeroes(blob, sizeof(struct reserve_entry));
+	blob = data_merge(blob, dtbuf);
+	blob = data_merge(blob, strbuf);
 
 	/*
-	 * Reserve map entries.
-	 * Each entry is an (address, size) pair of u64 values.
-	 * Always supply a zero-sized temination entry.
+	 * If the user asked for more space than is used, pad out the blob.
 	 */
-	fwrite(reservebuf.val, reservebuf.len, 1, f);
-	fwrite(&termre, sizeof(termre), 1, f);
+	if (minsize > 0) {
+		int padlen = minsize - be32_to_cpu(bph.totalsize);
+
+fprintf(stderr, "minsize = %d, totalsize = %d, padlen = %d\n", minsize, be32_to_cpu(bph.totalsize), padlen);
+		if (padlen > 0) {
+			blob = data_append_zeroes(blob, padlen);
+			bph.totalsize = cpu_to_be32(minsize);
+		} else {
+			if (quiet < 1)
+				fprintf(stderr,
+					"Warning: blob size %d >= minimum size %d\n",
+					be32_to_cpu(bph.totalsize), minsize);
+		}
+	}
 
-	fwrite(dtbuf.val, dtbuf.len, 1, f);
-	fwrite(strbuf.val, strbuf.len, 1, f);
+	fwrite(blob.val, blob.len, 1, f);
 
 	if (ferror(f))
 		die("Error writing device tree blob: %s\n", strerror(errno));
 
-	data_free(dtbuf);
-	data_free(strbuf);
+	/*
+	 * data_merge() frees the right-hand element so only the blob
+	 * remains to be freed.
+	 */
+	data_free(blob);
 }
 
 static void dump_stringtable_asm(FILE *f, struct data strbuf)
@@ -447,7 +452,7 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 	emit_label(f, symprefix, "blob_start");
 	emit_label(f, symprefix, "header");
 	fprintf(f, "\t.long\tOF_DT_HEADER /* magic */\n");
-	fprintf(f, "\t.long\t_%s_blob_end - _%s_blob_start /* totalsize */\n",
+	fprintf(f, "\t.long\t_%s_blob_abs_end - _%s_blob_start /* totalsize */\n",
 		symprefix, symprefix);
 	fprintf(f, "\t.long\t_%s_struct_start - _%s_blob_start /* off_dt_struct */\n",
 		symprefix, symprefix);
@@ -504,6 +509,15 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version, int boot_cpuid_phys)
 
 	emit_label(f, symprefix, "blob_end");
 
+	/*
+	 * If the user asked for more space than is used, pad it out.
+	 */
+	if (minsize > 0) {
+		fprintf(f, "\t.space\t%d - (_%s_blob_end - _%s_blob_start), 0\n", 
+			minsize, symprefix, symprefix);
+	}
+	emit_label(f, symprefix, "blob_abs_end");
+
 	data_free(strbuf);
 }
 
-- 
1.4.4.4

Re: [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[David Gibson]

On Wed, Apr 18, 2007 at 09:59:51PM -0400, Jerry Van Baren wrote:
> This makes padding out the blob if the user requested extra size much
> easer.  The assembly and writing to the file is more straight forward too.
> 
> Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
> ---
> 
> Hi David, Jon,
> 
> I wasn't happy with David's suggestion on the -S handling.  I also wasn't
> all that wild about how the blob was assembled and then written to the
> file piecemeal with "ad-hoc" alignment padding.  I realized I could fix
> both by assembling the blob in memory as a "struct data" and then write
> the whole thing out in one fell swoop.
> 
> Makes it a lot simpler and cleaner to my eyes.

I like it.  (In fact my original suggestion about using
data_append_zeroes() was based on a mistaken vague notion that we
already did it this way).

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jon Loeliger]

On Wed, 2007-04-18 at 20:59, Jerry Van Baren wrote:
> This makes padding out the blob if the user requested extra size much
> easer.  The assembly and writing to the file is more straight forward too.
> 
> Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
> ---
> 
> Hi David, Jon,
> 
> I wasn't happy with David's suggestion on the -S handling.  I also wasn't
> all that wild about how the blob was assembled and then written to the
> file piecemeal with "ad-hoc" alignment padding.  I realized I could fix
> both by assembling the blob in memory as a "struct data" and then write
> the whole thing out in one fell swoop.
> 
> Makes it a lot simpler and cleaner to my eyes.
> 
> Hope y'all agree,
> gvb

Jerry and David,

Sorry.  I've been out sick for a couple days here and am
just now catching up.  I've read through the last week's
worth of mail here now, so let me see if I have properly
summarized where things stand and what's needed for the DTC:

These patches need to be applied:

    [PATCH dtc] Add -o <output file> to the usage message.
    Sat, 14 Apr 2007 18:16:47 -0400

    [PATCH dtc take 3] Fix reserve map output for asm format.
    Tue, 17 Apr 2007 18:14:41 -0400

    [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
    Wed, 18 Apr 2007 21:59:51 -0400

The last patch there replaces the "Improve -S handling" patches.
But does the last patch replace or depend on the second one?

And I agree with David that last patch is a better approach.

Thanks,
jdl

Re: [U-Boot-Users] [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jerry Van Baren]

Jon Loeliger wrote:
> On Wed, 2007-04-18 at 20:59, Jerry Van Baren wrote:
>> This makes padding out the blob if the user requested extra size much
>> easer.  The assembly and writing to the file is more straight forward too.
>>
>> Signed-off-by: Gerald Van Baren <vanbaren@cideas.com>
>> ---
>>
>> Hi David, Jon,
>>
>> I wasn't happy with David's suggestion on the -S handling.  I also wasn't
>> all that wild about how the blob was assembled and then written to the
>> file piecemeal with "ad-hoc" alignment padding.  I realized I could fix
>> both by assembling the blob in memory as a "struct data" and then write
>> the whole thing out in one fell swoop.
>>
>> Makes it a lot simpler and cleaner to my eyes.
>>
>> Hope y'all agree,
>> gvb
> 
> Jerry and David,
> 
> Sorry.  I've been out sick for a couple days here and am
> just now catching up.  I've read through the last week's
> worth of mail here now, so let me see if I have properly
> summarized where things stand and what's needed for the DTC:
> 
> These patches need to be applied:
> 
>     [PATCH dtc] Add -o <output file> to the usage message.
>     Sat, 14 Apr 2007 18:16:47 -0400
> 
>     [PATCH dtc take 3] Fix reserve map output for asm format.
>     Tue, 17 Apr 2007 18:14:41 -0400
> 
>     [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
>     Wed, 18 Apr 2007 21:59:51 -0400
> 
> The last patch there replaces the "Improve -S handling" patches.
> But does the last patch replace or depend on the second one?
> 
> And I agree with David that last patch is a better approach.
> 
> Thanks,
> jdl

Yes, you have that right.

Patch #1 is good to go
   [PATCH dtc] Add -o <output file> to the usage message.
   Sat, 14 Apr 2007 18:16:47 -0400

Patch #2 and #3...
   [PATCH dtc take 3] Fix reserve map output for asm format.
   Tue, 17 Apr 2007 18:14:41 -0400
   [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
   Wed, 18 Apr 2007 21:59:51 -0400

...you force me to make a confession.  The third patch depends on the 
second patch because I snuck in a fix to the ASM output into the third 
patch: the last two chunks fix the ASM header output - I padded the blob 
out but didn't change the size properly in the header.  In the third 
patch above, I added a label "%s_blob_abs_end" and used that to 
calculate the total blob size (including the padding) in the header:

-	fprintf(f, "\t.long\t_%s_blob_end - _%s_blob_start /* totalsize */\n",
+	fprintf(f, "\t.long\t_%s_blob_abs_end - _%s_blob_start /* totalsize */\n",

--------

AAAARGH!  Sorry, I see I left a debug statement in flattree.c around 
line 390:

@@ -383,28 +370,46 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int 
version,
:
:
+fprintf(stderr, "minsize = %d, totalsize = %d, padlen = %d\n", minsize, 
be32_to_cpu(bph.totalsize), padlen);

Your option:
1) Take the last two patches as is and remove the fprintf yourself
2) I'll fix it tonight and provide a New Improved patch

Thanks,
gvb

Re: [U-Boot-Users] [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jon Loeliger]

On Thu, 2007-04-19 at 13:49, Jerry Van Baren wrote:

> > 
> > Sorry.  I've been out sick for a couple days here and am
> > just now catching up.  I've read through the last week's
> > worth of mail here now, so let me see if I have properly
> > summarized where things stand and what's needed for the DTC:
> > 
> > These patches need to be applied:
> > 
> >     [PATCH dtc] Add -o <output file> to the usage message.
> >     Sat, 14 Apr 2007 18:16:47 -0400
> > 
> >     [PATCH dtc take 3] Fix reserve map output for asm format.
> >     Tue, 17 Apr 2007 18:14:41 -0400
> > 
> >     [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
> >     Wed, 18 Apr 2007 21:59:51 -0400
> > 
> > The last patch there replaces the "Improve -S handling" patches.
> > But does the last patch replace or depend on the second one?
> > 
> > And I agree with David that last patch is a better approach.
> > 
> > Thanks,
> > jdl
> 
> Yes, you have that right.

Good.  The drugs aren't too strong yet. :-)

> Patch #1 is good to go
>    [PATCH dtc] Add -o <output file> to the usage message.
>    Sat, 14 Apr 2007 18:16:47 -0400
> 
> Patch #2 and #3...
>    [PATCH dtc take 3] Fix reserve map output for asm format.
>    Tue, 17 Apr 2007 18:14:41 -0400
>    [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
>    Wed, 18 Apr 2007 21:59:51 -0400
> 
> ...you force me to make a confession.

It's good for the soul. :-)


> Your option:
> 1) Take the last two patches as is and remove the fprintf yourself
> 2) I'll fix it tonight and provide a New Improved patch

I'll go with Door Number 1!  No problem.

Thanks,
jdl

Re: [U-Boot-Users] [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jon Loeliger]

So, like, the other day Jerry Van Baren mumbled:
> 
> Patch #2 and #3...
>    [PATCH dtc take 3] Fix reserve map output for asm format.
>    Tue, 17 Apr 2007 18:14:41 -0400
>    [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
>    Wed, 18 Apr 2007 21:59:51 -0400

[snip]

> AAAARGH!  Sorry, I see I left a debug statement in flattree.c around 
> line 390:

> Your option:
> 1) Take the last two patches as is and remove the fprintf yourself


Applied #2 and #3 listed above.
Cleaned out the debug fprintf().

Thanks,
jdl

Re: [U-Boot-Users] [PATCH: dtc take 2] Assemble the blob in memory before writing it out.

[Jerry Van Baren]

Jon Loeliger wrote:
> So, like, the other day Jerry Van Baren mumbled:
>> Patch #2 and #3...
>>    [PATCH dtc take 3] Fix reserve map output for asm format.
>>    Tue, 17 Apr 2007 18:14:41 -0400
>>    [PATCH: dtc take 2] Assemble the blob in memory before writing it out.
>>    Wed, 18 Apr 2007 21:59:51 -0400
> 
> [snip]
> 
>> AAAARGH!  Sorry, I see I left a debug statement in flattree.c around 
>> line 390:
> 
>> Your option:
>> 1) Take the last two patches as is and remove the fprintf yourself
> 
> 
> Applied #2 and #3 listed above.
> Cleaned out the debug fprintf().
> 
> Thanks,
> jdl

Oh oh, does NOT look good.  I lost the rewrite of the header size with 
the new, bigger size when -S is specified.

Ahh, I think I see the problem, sloppy programming bites me again. 
Modifying "bph.totalsize" worked before I changed to the struct data 
blob.  Grrrr.   I need to figure out the padding and apply it to the bph 
before starting the blob assembly.  Before the line:

         blob = data_append_data(blob, &bph, sizeof(bph));

Patch will take a bit because I have a meeting to go to :-(.

sorry,
gvb