PATCH: Add memreserve to DTC

PATCH: Add memreserve to DTC

[Jon Loeliger]

David and Ben,

This patch adds support for memreserve to the DTC's notion
of the "source file".  That is, you can now say this:
        
        
        memreserve =	<
        		 0000 0001  0000 0002
        		 0000 0003  0000 0004
        		>
        
        / {
        	model = "MPC8555CDS";
        	compatible = "MPC85xxCDS";
        	#address-cells = <2>;
        	#size-cells = <2>;
        	linux,phandle = <100>;

There is minor fiddling with the -R flag that needs to be
resolved at this point.

I've included read and writing for the source and blob formats,
but don't have a clue what to do with the /proc/devices format.

I've also tinkered the Makefile to do automatic dependency
file generation and inclusion.  Yep, I got bit by out of
date dependencies during my development here. :-)

Please feel free to adjust my coding approach or argument
passing or whatever as needed.  Hope this helps!

Enjoy,
jdl

PS -- Is this the right forum to continue to post the DTC patches?


Signed-off-by: Jon Loeliger <jdl@freescale.com>



Index: Makefile
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/Makefile  (mode:100644)
+++ uncommitted/Makefile  (mode:100644)
@@ -6,29 +6,35 @@
 OBJS = dtc.o livetree.o flattree.o data.o treesource.o fstree.o \
 	dtc-parser.tab.o lex.yy.o
 
-all: $(TARGETS)
+DEPFILES := $(OBJS:.o=.d)
+
+
+all: $(DEPFILES) $(TARGETS)
 
 dtc:	$(OBJS)
 	$(LINK.c) -o $@ $^
 
-$(OBJS): dtc.h
-
 dtc-parser.tab.c dtc-parser.tab.h dtc-parser.output: dtc-parser.y
 	$(BISON) -d -v $<
 
 lex.yy.c: dtc-lexer.l
 	$(LEX) $<
 
+dtc-lexer.l: dtc-parser.tab.h
 lex.yy.o: lex.yy.c dtc-parser.tab.h
 
-livetree.o:	flat_dt.h
 
 check: all
 	cd tests && $(MAKE) check
 
 clean:
+	rm -f ${DEPFILES}
 	rm -f *~ *.o a.out core $(TARGETS)
 	rm -f *.tab.[ch] lex.yy.c
 	rm -f *.i *.output vgcore.*
 	cd tests && $(MAKE) clean
 
+%.d : %.c
+	$(CC) -MM $< > $@
+
+-include ${DEPFILES}
Index: dtc-lexer.l
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/dtc-lexer.l  (mode:100644)
+++ uncommitted/dtc-lexer.l  (mode:100644)
@@ -87,6 +87,11 @@
 			return ']';
 		}
 
+memreserve	{
+			DPRINT("Keyword: memreserve\n");
+			return DT_MEMRESERVE;
+		}
+
 {PROPCHAR}+	{
 			DPRINT("PropName: %s\n", yytext);
 			yylval.str = strdup(yytext);
Index: dtc-parser.y
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/dtc-parser.y  (mode:100644)
+++ uncommitted/dtc-parser.y  (mode:100644)
@@ -41,6 +41,7 @@
 	int hexlen;
 }
 
+%token DT_MEMRESERVE
 %token <str> DT_PROPNAME
 %token <str> DT_NODENAME
 %token <cval> DT_CELL
@@ -66,6 +67,21 @@
 
 %%
 
+sourcefile:
+	headersection
+	devicetree
+		{
+		}
+	;
+
+headersection:
+		/* empty */
+	|	DT_MEMRESERVE  '=' '<' celllist '>' 
+		{
+			build_mem_reserve($4);
+		}
+	;
+
 devicetree:	{
 			assert(device_tree == NULL);
 		} '/' nodedef { device_tree = name_node($3, "", NULL); }
Index: dtc.c
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/dtc.c  (mode:100644)
+++ uncommitted/dtc.c  (mode:100644)
@@ -103,6 +103,7 @@
 int main(int argc, char *argv[])
 {
 	struct node *dt;
+	struct data mem_reserve_data = empty_data;
 	char *inform = "dts";
 	char *outform = "dts";
 	char *outname = "-";
@@ -151,12 +152,12 @@
 
 	if (streq(inform, "dts")) {
 		inf = dtc_open_file(arg);
-		dt = dt_from_source(inf);
+		dt = dt_from_source(inf, &mem_reserve_data);
 	} else if (streq(inform, "fs")) {
-		dt = dt_from_fs(arg);
+		dt = dt_from_fs(arg, &mem_reserve_data);
 	} else if(streq(inform, "dtb")) {
 		inf = dtc_open_file(arg);
-		dt = dt_from_blob(inf);
+		dt = dt_from_blob(inf, &mem_reserve_data);
 	} else {
 		die("Unknown input format \"%s\"\n", inform);
 	}
@@ -183,11 +184,11 @@
 	}
 
 	if (streq(outform, "dts")) {
-		write_tree_source(outf, dt, 0);
+		write_tree_source(outf, dt, &mem_reserve_data);
 	} else if (streq(outform, "dtb")) {
-		write_dt_blob(outf, dt, outversion, reservenum);
+		write_dt_blob(outf, dt, outversion, &mem_reserve_data);
 	} else if (streq(outform, "asm")) {
-		write_dt_asm(outf, dt, outversion, reservenum);
+		write_dt_asm(outf, dt, outversion, &mem_reserve_data);
 	} else if (streq(outform, "null")) {
 		/* do nothing */
 	} else {
Index: dtc.h
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/dtc.h  (mode:100644)
+++ uncommitted/dtc.h  (mode:100644)
@@ -117,6 +117,9 @@
 
 int data_is_one_string(struct data d);
 
+void build_mem_reserve(struct data d);
+
+
 /* DT constraints */
 
 #define MAX_PROPNAME_LEN	31
@@ -174,20 +177,23 @@
 	FFMT_ASM,
 };
 
-void write_dt_blob(FILE *f, struct node *tree, int version, int reservenum);
-void write_dt_asm(FILE *f, struct node *tree, int version, int reservenum);
+void write_dt_blob(FILE *f, struct node *tree, int version,
+		   struct data *mem_reserve_data);
+void write_dt_asm(FILE *f, struct node *tree, int version,
+		  struct data *mem_reserve_data);
 
-struct node *dt_from_blob(FILE *f);
+struct node *dt_from_blob(FILE *f, struct data *mem_reserve_data);
 
 /* Tree source */
 
-void write_tree_source(FILE *f, struct node *tree, int level);
+void write_tree_source(FILE *f, struct node *tree,
+		       struct data *mem_reserve_data);
 
-struct node *dt_from_source(FILE *f);
+struct node *dt_from_source(FILE *f, struct data *mem_reserve_data);
 
 /* FS trees */
 
-struct node *dt_from_fs(char *dirname);
+struct node *dt_from_fs(char *dirname, struct data *mem_reserve_data);
 
 /* misc */
 
Index: flattree.c
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/flattree.c  (mode:100644)
+++ uncommitted/flattree.c  (mode:100644)
@@ -288,10 +288,12 @@
 
 static void make_bph(struct boot_param_header *bph,
 			     struct version_info *vi,
-			     int reservenum,
+			     struct data *mem_reserve_data,
 			     int dtsize, int strsize)
 {
-	int reservesize = (reservenum+1) * sizeof(struct reserve_entry);
+	int reservenum = mem_reserve_data->len / sizeof(struct reserve_entry);
+	int reservesize = (reservenum + 1) * sizeof(struct reserve_entry);
+	int pad = sizeof(struct boot_param_header) % 8;
 
 	memset(bph, 0xff, sizeof(*bph));
 
@@ -299,11 +301,11 @@
 	bph->version = vi->version;
 	bph->last_comp_version = vi->last_comp_version;
 
-	bph->off_mem_rsvmap = cpu_to_be32(vi->hdr_size);
-	bph->off_dt_struct = cpu_to_be32(vi->hdr_size + reservesize);
-	bph->off_dt_strings = cpu_to_be32(vi->hdr_size + reservesize
+	bph->off_mem_rsvmap = cpu_to_be32(vi->hdr_size + pad);
+	bph->off_dt_struct = cpu_to_be32(vi->hdr_size + pad + reservesize);
+	bph->off_dt_strings = cpu_to_be32(vi->hdr_size + pad + reservesize
 					  + dtsize);
-	bph->totalsize = cpu_to_be32(vi->hdr_size + reservesize
+	bph->totalsize = cpu_to_be32(vi->hdr_size + pad + reservesize
 				     + dtsize + strsize);
 		
 	if (vi->flags & FTF_BOOTCPUID)
@@ -312,7 +314,8 @@
 		bph->size_dt_strings = cpu_to_be32(strsize);
 }
 
-void write_dt_blob(FILE *f, struct node *tree, int version, int reservenum)
+void write_dt_blob(FILE *f, struct node *tree, int version,
+		   struct data *mem_reserve_data)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -320,6 +323,8 @@
 	struct data strbuf = empty_data;
 	struct boot_param_header bph;
 	struct reserve_entry re = {.address = 0, .size = 0};
+	int pad;
+	int pad_buf = 0;
 
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
@@ -334,11 +339,30 @@
 	flatten_tree(tree, &bin_emitter, &dtbuf, &strbuf, vi);
 	bin_emit_cell(&dtbuf, OF_DT_END);
 
-	make_bph(&bph, vi, reservenum, dtbuf.len, strbuf.len);
-
+	/*
+	 * Make header.
+	 */
+	make_bph(&bph, vi, mem_reserve_data, dtbuf.len, strbuf.len);
 	fwrite(&bph, vi->hdr_size, 1, f);
-	for (i = 0; i < reservenum+1; i++)
-		fwrite(&re, sizeof(re), 1, f);
+
+	/*
+	 * Allow possible trailing alignment for mem reserve map.
+	 */
+	if (((pad = sizeof(struct boot_param_header) % 8)) != 0) {
+		fwrite(&pad_buf, 1, pad, f);	
+	}
+
+	/*
+	 * Reserve map entries.
+	 * Since the blob is relocatable, the address of the map is not
+	 * determinable here, so no entry is made for the DT itself.
+	 * Each entry is an (address, size) pair of u64 values.
+	 * Always supply a zero-sized temination entry.
+	 */
+	fwrite(mem_reserve_data->val, mem_reserve_data->len, 1, f);
+	re.address = 0;
+	re.size = 0;
+	fwrite(&re, sizeof(re), 1, f);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
@@ -364,7 +388,8 @@
 	}
 }
 
-void write_dt_asm(FILE *f, struct node *tree, int version, int reservenum)
+void write_dt_asm(FILE *f, struct node *tree, int version,
+		  struct data *mem_reserve_data)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -408,16 +433,27 @@
 		fprintf(f, "\t.long\t_%s_strings_end - _%s_strings_start\t/* size_dt_strings */\n",
 			symprefix, symprefix);
 
+	/*
+	 * Reserve map entries.
+	 * Each entry is an (address, size) pair of u64 values.
+	 * Since the ASM file variant can relocate and compute the address
+	 * and size of the the device tree itself, and an entry for it.
+	 * Always supply a zero-sized temination entry.
+	 */
+	asm_emit_align(f, 8);
 	emit_label(f, symprefix, "reserve_map");
-	/* reserve map entry for the device tree itself */
 	fprintf(f, "\t.long\t0, _%s_blob_start\n", symprefix);
 	fprintf(f, "\t.long\t0, _%s_blob_end - _%s_blob_start\n",
 		symprefix, symprefix);
-	for (i = 0; i < reservenum+1; i++) {
-		fprintf(f, "\t.llong\t0\n");
-		fprintf(f, "\t.llong\t0\n");
+
+	if (mem_reserve_data->len > 0) {
+		fprintf(f, "/* Memory reserve map from source file */\n");
+		asm_emit_data(f, *mem_reserve_data);
 	}
 
+	fprintf(f, "\t.llong\t0\n");
+	fprintf(f, "\t.llong\t0\n");
+
 	emit_label(f, symprefix, "struct_start");
 	flatten_tree(tree, &asm_emitter, f, &strbuf, vi);
 	fprintf(f, "\t.long\tOF_DT_END\n");
@@ -518,7 +554,8 @@
 	p = inb->base + offset;
 	while (1) {
 		if (p >= inb->limit)
-			die("String offset %d overruns string table\n");
+			die("String offset %d overruns string table\n",
+			    offset);
 
 		if (*p == '\0')
 			break;
@@ -549,6 +586,43 @@
 	return build_property(name, val, NULL);
 }
 
+
+static struct data flat_read_mem_reserve(struct inbuf *inb)
+{
+	char *p;
+	int len = 0;
+	int done = 0;
+	cell_t cells[4];
+	struct data d;
+
+	d = empty_data;
+
+	/*
+	 * Each entry is a pair of u64 (addr, size) values for 4 cell_t's.
+	 * List terminates at an entry with size equal to zero.
+	 *
+	 * First pass, count entries.
+	 */
+	p = inb->ptr;
+	do {
+		flat_read_chunk(inb, &cells[0], 4 * sizeof(cell_t));
+		if (cells[2] == 0 && cells[3] == 0) {
+			done = 1;
+		} else {
+			++len;
+		}
+	} while (!done);
+
+	/*
+	 * Back up for pass two, reading the whole data value.
+	 */
+	inb->ptr = p;
+	d = flat_read_data(inb, len * 4 * sizeof(cell_t));
+
+	return d;
+}
+
+
 static char *nodename_from_path(char *ppath, char *cpath)
 {
 	char *lslash;
@@ -656,14 +730,17 @@
 	return node;
 }
 
-struct node *dt_from_blob(FILE *f)
+
+struct node *dt_from_blob(FILE *f, struct data *mem_reserve_data)
 {
-	u32 magic, totalsize, off_dt, off_str, version, size_str;
+	u32 magic, totalsize, version, size_str;
+	u32 off_dt, off_str, off_mem_rsvmap;
 	int rc;
 	char *blob;
 	struct boot_param_header *bph;
 	char *p;
 	struct inbuf dtbuf, strbuf;
+	struct inbuf memresvbuf;
 	int sizeleft;
 	struct node *tree;
 	u32 val;
@@ -723,18 +800,21 @@
 
 	off_dt = be32_to_cpu(bph->off_dt_struct);
 	off_str = be32_to_cpu(bph->off_dt_strings);
+	off_mem_rsvmap = be32_to_cpu(bph->off_mem_rsvmap);
 	version = be32_to_cpu(bph->version);
 
 	fprintf(stderr, "\tmagic:\t\t\t0x%x\n", magic);
 	fprintf(stderr, "\ttotalsize:\t\t%d\n", totalsize);
 	fprintf(stderr, "\toff_dt_struct:\t\t0x%x\n", off_dt);
 	fprintf(stderr, "\toff_dt_strings:\t\t0x%x\n", off_str);
-	fprintf(stderr, "\toff_mem_rsvmap:\t\t0x%x\n",
-		be32_to_cpu(bph->off_mem_rsvmap));
+	fprintf(stderr, "\toff_mem_rsvmap:\t\t0x%x\n", off_mem_rsvmap);
 	fprintf(stderr, "\tversion:\t\t0x%x\n", version );
 	fprintf(stderr, "\tlast_comp_version:\t0x%x\n",
 		be32_to_cpu(bph->last_comp_version));
 
+	if (off_mem_rsvmap >= totalsize)
+		die("Mem Reserve structure offset exceeds total size\n");
+
 	if (off_dt >= totalsize)
 		die("DT structure offset exceeds total size\n");
 
@@ -756,12 +836,16 @@
 		flags |= FTF_FULLPATH | FTF_NAMEPROPS | FTF_VARALIGN;
 	}
 
+	inbuf_init(&memresvbuf,
+		   blob + off_mem_rsvmap, blob + totalsize);
 	inbuf_init(&dtbuf, blob + off_dt, blob + totalsize);
 	inbuf_init(&strbuf, blob + off_str, blob + totalsize);
 
 	if (version >= 3)
 		strbuf.limit = strbuf.base + size_str;
 
+	*mem_reserve_data = flat_read_mem_reserve(&memresvbuf);
+
 	val = flat_read_word(&dtbuf);
 
 	if (val != OF_DT_BEGIN_NODE)
Index: fstree.c
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/fstree.c  (mode:100644)
+++ uncommitted/fstree.c  (mode:100644)
@@ -80,7 +80,7 @@
 	return tree;
 }
 
-struct node *dt_from_fs(char *dirname)
+struct node *dt_from_fs(char *dirname, struct data *mem_reserve_data)
 {
 	struct node *tree;
 
Index: treesource.c
===================================================================
--- abbe76bc79348c0896908c8548d51bdfa5f76500/treesource.c  (mode:100644)
+++ uncommitted/treesource.c  (mode:100644)
@@ -20,13 +20,30 @@
 
 #include "dtc.h"
 
-struct node *device_tree;
-
 extern FILE *yyin;
 extern int yyparse(void);
+extern void yyerror(char const *);
+
+struct node *device_tree;
+struct data *mem_reserve_data_ptr;
 
-struct node *dt_from_source(FILE *f)
+
+void build_mem_reserve(struct data d)
 {
+	/*
+	 * FIXME: Should reconcile the -R parameter here now?
+	 */
+	*mem_reserve_data_ptr = d;
+	if (d.len % 16 != 0) {
+		yyerror("Memory Reserve entries are <u64 addr, u64 size>\n");
+	}
+}
+
+
+struct node *dt_from_source(FILE *f, struct data *mem_reserve_data)
+{
+	mem_reserve_data_ptr = mem_reserve_data;
+
 	yyin = f;
 	if (yyparse() != 0)
 		return NULL;
@@ -75,7 +92,8 @@
 		
 }
 
-void write_tree_source(FILE *f, struct node *tree, int level)
+
+void write_tree_source_node(FILE *f, struct node *tree, int level)
 {
 	struct property *prop;
 	struct node *child;
@@ -133,8 +151,40 @@
 	}
 	for_each_child(tree, child) {
 		fprintf(f, "\n");
-		write_tree_source(f, child, level+1);
+		write_tree_source_node(f, child, level+1);
 	}
 	write_prefix(f, level);
 	fprintf(f, "};\n");
 }
+
+
+void write_tree_source(FILE *f, struct node *tree,
+		       struct data *mem_reserve_data)
+{
+	int i;
+	int ncells;
+	cell_t *cp;
+
+	if (mem_reserve_data->len > 0) {
+		fprintf(f, "memreserve =\t<\n");
+		cp = (cell_t *)mem_reserve_data->val;
+		ncells = mem_reserve_data->len / sizeof(cell_t);
+		for (i = 0; i < ncells; i++) {
+			if (i % 4 == 0) {
+				fprintf(f, "\t\t\t");
+			}
+			fprintf(f, "%x", be32_to_cpu(*cp++));
+			if (i % 4 == 1) {
+				fprintf(f, "  ");
+			} else if (i % 4 == 3) {
+				fprintf(f, "\n");
+			} else {
+				fprintf(f, " ");
+			}
+		}
+		fprintf(f, "\t\t>\n\n");
+	}
+
+	write_tree_source_node(f, tree, 0);
+}
+

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> David and Ben,
> 
> This patch adds support for memreserve to the DTC's notion
> of the "source file".  That is, you can now say this:
>         
>         
>         memreserve =	<
>         		 0000 0001  0000 0002
>         		 0000 0003  0000 0004
>         		>
>         
>         / {
>         	model = "MPC8555CDS";
>         	compatible = "MPC85xxCDS";
>         	#address-cells = <2>;
>         	#size-cells = <2>;
>         	linux,phandle = <100>;

Hrm.. nice idea, but I don't really like the syntax.  It looks like a
property definition, which it's really not, and forcing the user to
split up these 64-bit quantities into cells is kind of silly.  Plus
the fact that "memreserve" is lexed as a reserved word means it can't
be used as a property name.  And it really ought to have a ';' at the
end, for consistency.

Hrm... wonder how to do this, without making the lex and yacc stuff
too unspeakable.

Maybe

/memreserve/ = {  00000001-00000002;
		  00000003-00000004;
	       };

I'm not that fond of the /.../ form, although the '/' is the best way
I can think of to ensure it can't be confused with a property or node
name.  We'lll also need some sort of lexing magic so that it actually
recognizes the things within as numbers, not property names, too.
Hrm... will need to think about that.

> There is minor fiddling with the -R flag that needs to be
> resolved at this point.

I think -R should add the given number of extra empty entries, on top
of the ones given in the source.

> I've included read and writing for the source and blob formats,
> but don't have a clue what to do with the /proc/devices format.

Just ignore it, I think.  If you need memreserves with fs input, you
can just use -R and patch them in later.

> I've also tinkered the Makefile to do automatic dependency
> file generation and inclusion.  Yep, I got bit by out of
> date dependencies during my development here. :-)

Hrm.. seems kinda overkill for a project this small, but ok.  Although
it would be nice to have this as a separate patch.

> Please feel free to adjust my coding approach or argument
> passing or whatever as needed.  Hope this helps!

Yeah, there are some things I'd like to change (in addition to the
input syntax itself), but I'm thinking about just applying it and
fixing up afterwards.

Biggest thing is that rather than passing the tree itself and the
memreserve info about as two parameters all over the place, I'd rather
create a new structure which has both (and later can have anything
else that might be needed).

-- 
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/people/dgibson

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> David and Ben,
> 
> This patch adds support for memreserve to the DTC's notion
> of the "source file".  That is, you can now say this:
>         
>         
>         memreserve =	<
>         		 0000 0001  0000 0002
>         		 0000 0003  0000 0004
>         		>
>         
>         / {
>         	model = "MPC8555CDS";
>         	compatible = "MPC85xxCDS";
>         	#address-cells = <2>;
>         	#size-cells = <2>;
>         	linux,phandle = <100>;
> 
> There is minor fiddling with the -R flag that needs to be
> resolved at this point.
> 
> I've included read and writing for the source and blob formats,
> but don't have a clue what to do with the /proc/devices format.
> 
> I've also tinkered the Makefile to do automatic dependency
> file generation and inclusion.  Yep, I got bit by out of
> date dependencies during my development here. :-)

I applied the dependency portions of the patch to the git tree, with
some tweaks to fix some corner cases.

-- 
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/people/dgibson

Re: PATCH: Add memreserve to DTC

[Jon Loeliger]

On Sun, 2005-07-10 at 23:55, David Gibson wrote:
> On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> > David and Ben,
> > 
> > This patch adds support for memreserve to the DTC's notion
> > of the "source file".  That is, you can now say this:
> >         
> >         
> >         memreserve =	<
> >         		 0000 0001  0000 0002
> >         		 0000 0003  0000 0004
> >         		>

> 
> Hrm.. nice idea, but I don't really like the syntax.  It looks like a
> property definition, which it's really not,

Well, syntax is relatively easy to change.  I picked one 
that was already present in the grammar and lex code.
I wanted to use something more braces oriented, but it
got gross looking once I decided to use the "struct data"
parts for the cell_t values.

>  and forcing the user to
> split up these 64-bit quantities into cells is kind of silly.

Hey, I didn't set that up! :-)  There wasn't an existing
clean way to state 64 bit values, and an arbitrary list of
them.  So I uh, leveraged the existing cell_t support!

>   Plus
> the fact that "memreserve" is lexed as a reserved word means it can't
> be used as a property name.

Yeah, I wasn't happy about that either.  Wasn't sure
what you wanted to do to "fix" that.  Thought it better
to get the code into your hands than try to discuss the
issues via mail.

>   And it really ought to have a ';' at the
> end, for consistency.

Sure.  Easy.

And, I actually anticipated making the "header" parts
of the grammar be more general, of which then the memory
reserve area would be just one part.  For example, suppose
you wanted to specify the "version" too:

    /header/ = {
        version  = 10;
        memreserve = < .... ....  .... .... >;
    };

Or so.


> Hrm... wonder how to do this, without making the lex and yacc stuff
> too unspeakable.
> 
> Maybe
> 
> /memreserve/ = {  00000001-00000002;
> 		  00000003-00000004;
> 	       };
> 
> I'm not that fond of the /.../ form, although the '/' is the best way
> I can think of to ensure it can't be confused with a property or node
> name.  We'lll also need some sort of lexing magic so that it actually
> recognizes the things within as numbers, not property names, too.
> Hrm... will need to think about that.

Which was sort of the problem I faced... :-)

> > There is minor fiddling with the -R flag that needs to be
> > resolved at this point.
> 
> I think -R should add the given number of extra empty entries, on top
> of the ones given in the source.

Except now you have to carry the count along in the header.
How else do you know if the _first_ or _last_ 0-size value
really ends the list?  Well, you could maybe assume ordered
parts an subtract the base offset of the memreserve section
from the following section to get its total size.  Gross, though.


> > Please feel free to adjust my coding approach or argument
> > passing or whatever as needed.  Hope this helps!
> 
> Yeah, there are some things I'd like to change (in addition to the
> input syntax itself), but I'm thinking about just applying it and
> fixing up afterwards.

That sounds good.

> Biggest thing is that rather than passing the tree itself and the
> memreserve info about as two parameters all over the place, I'd rather
> create a new structure which has both (and later can have anything
> else that might be needed).

If you'd like, I'll do this work.

jdl

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Mon, Jul 11, 2005 at 04:22:30PM -0500, Jon Loeliger wrote:
> On Sun, 2005-07-10 at 23:55, David Gibson wrote:
> > On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> > > David and Ben,
> > > 
> > > This patch adds support for memreserve to the DTC's notion
> > > of the "source file".  That is, you can now say this:
> > >         
> > >         
> > >         memreserve =	<
> > >         		 0000 0001  0000 0002
> > >         		 0000 0003  0000 0004
> > >         		>
> 
> > 
> > Hrm.. nice idea, but I don't really like the syntax.  It looks like a
> > property definition, which it's really not,
> 
> Well, syntax is relatively easy to change.  I picked one 
> that was already present in the grammar and lex code.
> I wanted to use something more braces oriented, but it
> got gross looking once I decided to use the "struct data"
> parts for the cell_t values.
> 
> >  and forcing the user to
> > split up these 64-bit quantities into cells is kind of silly.
> 
> Hey, I didn't set that up! :-)  There wasn't an existing
> clean way to state 64 bit values, and an arbitrary list of
> them.  So I uh, leveraged the existing cell_t support!

Cells make sense for the actual OF-like data, becayse they're an OF
concept.  For memreserve, which is purely Linux specific, they don't/

> >   Plus
> > the fact that "memreserve" is lexed as a reserved word means it can't
> > be used as a property name.
> 
> Yeah, I wasn't happy about that either.  Wasn't sure
> what you wanted to do to "fix" that.  Thought it better
> to get the code into your hands than try to discuss the
> issues via mail.

Yeah... I have some ideas, maybe replacing some of the start
conditions with a lexical tie-in from the parser level.  I'll see what
I can come up with.

> >   And it really ought to have a ';' at the
> > end, for consistency.
> 
> Sure.  Easy.
> 
> And, I actually anticipated making the "header" parts
> of the grammar be more general, of which then the memory
> reserve area would be just one part.  For example, suppose
> you wanted to specify the "version" too:
> 
>     /header/ = {
>         version  = 10;
>         memreserve = < .... ....  .... .... >;
>     };

Perhaps, I don't think there's anything else that we really want to
set for now.  I don't think version should be set in the source file.

> Or so.

Actually, I'm having second thoughts on the form I suggested before.
I'm now leaning towards:

	/memreserve/ 00000001-00000002;
	/memreserve/ 00000003-00000004;

	/ {
		...
	};

> > Hrm... wonder how to do this, without making the lex and yacc stuff
> > too unspeakable.
> > 
> > Maybe
> > 
> > /memreserve/ = {  00000001-00000002;
> > 		  00000003-00000004;
> > 	       };
> > 
> > I'm not that fond of the /.../ form, although the '/' is the best way
> > I can think of to ensure it can't be confused with a property or node
> > name.  We'lll also need some sort of lexing magic so that it actually
> > recognizes the things within as numbers, not property names, too.
> > Hrm... will need to think about that.
> 
> Which was sort of the problem I faced... :-)
> 
> > > There is minor fiddling with the -R flag that needs to be
> > > resolved at this point.
> > 
> > I think -R should add the given number of extra empty entries, on top
> > of the ones given in the source.
> 
> Except now you have to carry the count along in the header.
> How else do you know if the _first_ or _last_ 0-size value
> really ends the list?  Well, you could maybe assume ordered
> parts an subtract the base offset of the memreserve section
> from the following section to get its total size.  Gross, though.
> 
> 
> > > Please feel free to adjust my coding approach or argument
> > > passing or whatever as needed.  Hope this helps!
> > 
> > Yeah, there are some things I'd like to change (in addition to the
> > input syntax itself), but I'm thinking about just applying it and
> > fixing up afterwards.
> 
> That sounds good.
> 
> > Biggest thing is that rather than passing the tree itself and the
> > memreserve info about as two parameters all over the place, I'd rather
> > create a new structure which has both (and later can have anything
> > else that might be needed).
> 
> If you'd like, I'll do this work.
> 
> jdl
> 
> 

-- 
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/people/dgibson

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Mon, Jul 11, 2005 at 04:22:30PM -0500, Jon Loeliger wrote:
> On Sun, 2005-07-10 at 23:55, David Gibson wrote:
> > On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
[snip]
> > Biggest thing is that rather than passing the tree itself and the
> > memreserve info about as two parameters all over the place, I'd rather
> > create a new structure which has both (and later can have anything
> > else that might be needed).
> 
> If you'd like, I'll do this work.

That would be helpful.  You'll need to rediff, though, I merged a
couple of bugfixes from your patch that weren't directly related to
the memreserve stuff.

-- 
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/people/dgibson

Re: PATCH: Add memreserve to DTC

[Segher Boessenkool]

>>>  and forcing the user to
>>> split up these 64-bit quantities into cells is kind of silly.
>>
>> Hey, I didn't set that up! :-)  There wasn't an existing
>> clean way to state 64 bit values, and an arbitrary list of
>> them.  So I uh, leveraged the existing cell_t support!
>
> Cells make sense for the actual OF-like data, becayse they're an OF
> concept.  For memreserve, which is purely Linux specific, they don't/

No.  This is _not_ what is called a cell.  "Cell" is a Forth concept.
A cell can be any size.  Open Firmware puts the extra restriction on it
to be _at least_ 32 bits.

The thing you are referring to is what is called in OF

	"32-bit integer property encoding format".

It is defined to always be 32-bit, and not the cell size of the 
firmware,
so that you can use a 64-bit firmware with a 32-bit OS, and vice versa
(of course there could be different reasons why this isn't practical, 
but
that's not the point).

In OF words, this format is normally abbreviated as "int".

Btw -- beware of the fact that such an "int" does _not_ have any
alignment restrictions -- so you better read it byte by byte...


Segher

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Tue, Jul 12, 2005 at 10:02:16AM +0200, Segher Boessenkool wrote:
> >>> and forcing the user to
> >>>split up these 64-bit quantities into cells is kind of silly.
> >>
> >>Hey, I didn't set that up! :-)  There wasn't an existing
> >>clean way to state 64 bit values, and an arbitrary list of
> >>them.  So I uh, leveraged the existing cell_t support!
> >
> >Cells make sense for the actual OF-like data, becayse they're an OF
> >concept.  For memreserve, which is purely Linux specific, they don't/
> 
> No.  This is _not_ what is called a cell.  "Cell" is a Forth concept.
> A cell can be any size.  Open Firmware puts the extra restriction on it
> to be _at least_ 32 bits.
> 
> The thing you are referring to is what is called in OF
> 
> 	"32-bit integer property encoding format".
> 
> It is defined to always be 32-bit, and not the cell size of the 
> firmware,
> so that you can use a 64-bit firmware with a 32-bit OS, and vice versa
> (of course there could be different reasons why this isn't practical, 
> but
> that's not the point).
> 
> In OF words, this format is normally abbreviated as "int".

My mistake, I misunderstood the terminology.  But the basic point is
that lots of things in the kernel already assume a cell is 32-bits, so
it would be silly to try and change that here.  This is not true for
the memreserve values.

> Btw -- beware of the fact that such an "int" does _not_ have any
> alignment restrictions -- so you better read it byte by byte...

Erm.. in what context.  dtc never reads ints from the blob format as
ints - properties are just byte strings to it.  At present you can't
mix cell input format with other sorts, which means the ints must, in
fact, be aligned, since properties are.

-- 
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Re: PATCH: Add memreserve to DTC

[Segher Boessenkool]

> My mistake, I misunderstood the terminology.  But the basic point is
> that lots of things in the kernel already assume a cell is 32-bits, so
> it would be silly to try and change that here.  This is not true for
> the memreserve values.

Of course you should use 32-bit values; but please just don't call it
a cell, there's nothing more confusing than messing up terminology.

>> Btw -- beware of the fact that such an "int" does _not_ have any
>> alignment restrictions -- so you better read it byte by byte...
>
> Erm.. in what context.  dtc never reads ints from the blob format as
> ints - properties are just byte strings to it.  At present you can't
> mix cell input format with other sorts, which means the ints must, in
> fact, be aligned, since properties are.

When reading integers from properties that have been created by an OF
implementation.  Whether this is a problem for DTC or not, I don't know;
I just thought I'd mention the problem to you, people fall in that trap
again and again.


Segher

Re: PATCH: Add memreserve to DTC

[Jon Loeliger]

On Mon, 2005-07-11 at 23:06, David Gibson wrote:
> On Mon, Jul 11, 2005 at 04:22:30PM -0500, Jon Loeliger wrote:
> > On Sun, 2005-07-10 at 23:55, David Gibson wrote:
> > > On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> [snip]
> > > Biggest thing is that rather than passing the tree itself and the
> > > memreserve info about as two parameters all over the place, I'd rather
> > > create a new structure which has both (and later can have anything
> > > else that might be needed).
> > 
> > If you'd like, I'll do this work.
> 
> That would be helpful.  You'll need to rediff, though, I merged a
> couple of bugfixes from your patch that weren't directly related to
> the memreserve stuff.

David,

Here is an updated version of the patch that obsoletes
the previous one I submitted.  I have incorporated all
of your syntactic suggestions except not using the
split-64 values (ie, this still uses 'struct data').
It primarily merges in the changes that you adopted
from earlier and implements a new structure at the
base of the parse tree to hold both the device tree
and the header information.  I called that new stucuture
'struct header_tree'.  Feel free to dream up something
better. :-)

Signed-off-by: Jon Loeliger <jdl@freescale.com>

Thanks,
jdl




diff --git a/dtc-lexer.l b/dtc-lexer.l
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -87,6 +87,11 @@ REFCHAR		({PROPCHAR}|{UNITCHAR}|[/@])
 			return ']';
 		}
 
+"/memreserve/"	{
+			DPRINT("Keyword: /memreserve/\n");
+			return DT_MEMRESERVE;
+		}
+
 {PROPCHAR}+	{
 			DPRINT("PropName: %s\n", yytext);
 			yylval.str = strdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -24,8 +24,6 @@
 int yylex (void);
 void yyerror (char const *);
 
-extern struct node *device_tree;
-
 %}
 
 %union {
@@ -41,6 +39,7 @@ extern struct node *device_tree;
 	int hexlen;
 }
 
+%token DT_MEMRESERVE
 %token <str> DT_PROPNAME
 %token <str> DT_NODENAME
 %token <cval> DT_CELL
@@ -51,11 +50,13 @@ extern struct node *device_tree;
 %token <str> DT_REF
 
 %type <data> propdata
+%type <data> headersection
 %type <data> celllist
 %type <data> bytestring
 %type <prop> propdef
 %type <proplist> proplist
 
+%type <node> devicetree
 %type <node> nodedef
 %type <node> subnode
 %type <nodelist> subnodes
@@ -66,9 +67,29 @@ extern struct node *device_tree;
 
 %%
 
-devicetree:	{
-			assert(device_tree == NULL);
-		} '/' nodedef { device_tree = name_node($3, "", NULL); }
+sourcefile:
+	headersection
+	devicetree
+		{
+			build_header_tree($1, $2);
+		}
+	;
+
+headersection:
+		/* empty */
+		{
+			$$ = empty_data;
+		}
+	|	DT_MEMRESERVE  '=' '<' celllist '>' ';'
+		{
+			$$ = build_mem_reserve($4);
+		}
+	;
+
+devicetree:	'/' nodedef
+		{
+			$$ = name_node($2, "", NULL);
+		}
 	;
 
 nodedef:	'{' proplist subnodes '}' ';' {
diff --git a/dtc.c b/dtc.c
--- a/dtc.c
+++ b/dtc.c
@@ -102,7 +102,7 @@ static void usage(void)
 
 int main(int argc, char *argv[])
 {
-	struct node *dt;
+	struct header_tree *ht;
 	char *inform = "dts";
 	char *outform = "dts";
 	char *outname = "-";
@@ -151,12 +151,12 @@ int main(int argc, char *argv[])
 
 	if (streq(inform, "dts")) {
 		inf = dtc_open_file(arg);
-		dt = dt_from_source(inf);
+		ht = dt_from_source(inf);
 	} else if (streq(inform, "fs")) {
-		dt = dt_from_fs(arg);
+		ht = dt_from_fs(arg);
 	} else if(streq(inform, "dtb")) {
 		inf = dtc_open_file(arg);
-		dt = dt_from_blob(inf);
+		ht = dt_from_blob(inf);
 	} else {
 		die("Unknown input format \"%s\"\n", inform);
 	}
@@ -164,10 +164,10 @@ int main(int argc, char *argv[])
 	if (inf && (inf != stdin))
 		fclose(inf);
 
-	if (! dt)
+	if (! ht || ! ht->root)
 		die("Couldn't read input tree\n");
 
-	if (! check_device_tree(dt)) {
+	if (! check_device_tree(ht->root)) {
 		fprintf(stderr, "Input tree has errors\n");
 		if (! force)
 			exit(1);
@@ -183,11 +183,11 @@ int main(int argc, char *argv[])
 	}
 
 	if (streq(outform, "dts")) {
-		write_tree_source(outf, dt, 0);
+		write_tree_source(outf, ht);
 	} else if (streq(outform, "dtb")) {
-		write_dt_blob(outf, dt, outversion, reservenum);
+		write_dt_blob(outf, ht, outversion);
 	} else if (streq(outform, "asm")) {
-		write_dt_asm(outf, dt, outversion, reservenum);
+		write_dt_asm(outf, ht, outversion);
 	} else if (streq(outform, "null")) {
 		/* do nothing */
 	} else {
diff --git a/dtc.h b/dtc.h
--- a/dtc.h
+++ b/dtc.h
@@ -118,6 +118,24 @@ struct data data_add_fixup(struct data d
 
 int data_is_one_string(struct data d);
 
+struct data build_mem_reserve(struct data d);
+
+
+/*
+ * Combined Header and Tree information.
+ */
+struct header_tree {
+	struct data mem_reserve_data;	/* mem reserve from header */
+	struct node *root;		/* root of the device tree */
+};
+
+struct header_tree *alloc_header_tree(struct data mem_reserve_data,
+				      struct node *tree);
+
+void build_header_tree(struct data mem_reserve_data,
+		       struct node *tree);
+
+
 /* DT constraints */
 
 #define MAX_PROPNAME_LEN	31
@@ -175,20 +193,19 @@ enum flat_dt_format {
 	FFMT_ASM,
 };
 
-void write_dt_blob(FILE *f, struct node *tree, int version, int reservenum);
-void write_dt_asm(FILE *f, struct node *tree, int version, int reservenum);
+void write_dt_blob(FILE *f, struct header_tree *ht, int version);
+void write_dt_asm(FILE *f, struct header_tree *ht, int version);
 
-struct node *dt_from_blob(FILE *f);
+struct header_tree *dt_from_blob(FILE *f);
 
 /* Tree source */
 
-void write_tree_source(FILE *f, struct node *tree, int level);
-
-struct node *dt_from_source(FILE *f);
+void write_tree_source(FILE *f, struct header_tree *ht);
+struct header_tree *dt_from_source(FILE *f);
 
 /* FS trees */
 
-struct node *dt_from_fs(char *dirname);
+struct header_tree *dt_from_fs(char *dirname);
 
 /* misc */
 
diff --git a/flattree.c b/flattree.c
--- a/flattree.c
+++ b/flattree.c
@@ -287,11 +287,12 @@ static void flatten_tree(struct node *tr
 }
 
 static void make_bph(struct boot_param_header *bph,
-			     struct version_info *vi,
-			     int reservenum,
-			     int dtsize, int strsize)
+		     struct version_info *vi,
+		     struct data *mem_reserve_data,
+		     int dtsize, int strsize)
 {
 	int reserve_off;
+	int reservenum = mem_reserve_data->len / sizeof(struct reserve_entry);
 	int reservesize = (reservenum+1) * sizeof(struct reserve_entry);
 
 	memset(bph, 0xff, sizeof(*bph));
@@ -316,7 +317,7 @@ static void make_bph(struct boot_param_h
 		bph->size_dt_strings = cpu_to_be32(strsize);
 }
 
-void write_dt_blob(FILE *f, struct node *tree, int version, int reservenum)
+void write_dt_blob(FILE *f, struct header_tree *ht, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -335,18 +336,31 @@ void write_dt_blob(FILE *f, struct node 
 	dtbuf = empty_data;
 	strbuf = empty_data;
 
-	flatten_tree(tree, &bin_emitter, &dtbuf, &strbuf, vi);
+	flatten_tree(ht->root, &bin_emitter, &dtbuf, &strbuf, vi);
 	bin_emit_cell(&dtbuf, OF_DT_END);
 
-	make_bph(&bph, vi, reservenum, dtbuf.len, strbuf.len);
+	/*
+	 * Make header.
+	 */
+	make_bph(&bph, vi, &ht->mem_reserve_data, dtbuf.len, strbuf.len);
+
+	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);
 
-	fwrite(&bph, vi->hdr_size, 1, f);
-	for (i = 0; i < reservenum+1; i++)
-		fwrite(&re, sizeof(re), 1, f);
+	/*
+	 * Reserve map entries.
+	 * Since the blob is relocatable, the address of the map is not
+	 * determinable here, so no entry is made for the DT itself.
+	 * Each entry is an (address, size) pair of u64 values.
+	 * Always supply a zero-sized temination entry.
+	 */
+	fwrite(ht->mem_reserve_data.val, ht->mem_reserve_data.len, 1, f);
+	re.address = 0;
+	re.size = 0;
+	fwrite(&re, sizeof(re), 1, f);
 
 	fwrite(dtbuf.val, dtbuf.len, 1, f);
 	fwrite(strbuf.val, strbuf.len, 1, f);
@@ -372,7 +386,7 @@ void dump_stringtable_asm(FILE *f, struc
 	}
 }
 
-void write_dt_asm(FILE *f, struct node *tree, int version, int reservenum)
+void write_dt_asm(FILE *f, struct header_tree *ht, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -416,20 +430,30 @@ void write_dt_asm(FILE *f, struct node *
 		fprintf(f, "\t.long\t_%s_strings_end - _%s_strings_start\t/* size_dt_strings */\n",
 			symprefix, symprefix);
 
-	/* align the reserve map to a doubleword boundary */
+	/*
+	 * Reserve map entries.
+	 * Align the reserve map to a doubleword boundary.
+	 * Each entry is an (address, size) pair of u64 values.
+	 * Since the ASM file variant can relocate and compute the address
+	 * and size of the the device tree itself, and an entry for it.
+	 * Always supply a zero-sized temination entry.
+	 */
 	asm_emit_align(f, 8);
 	emit_label(f, symprefix, "reserve_map");
-	/* reserve map entry for the device tree itself */
 	fprintf(f, "\t.long\t0, _%s_blob_start\n", symprefix);
 	fprintf(f, "\t.long\t0, _%s_blob_end - _%s_blob_start\n",
 		symprefix, symprefix);
-	for (i = 0; i < reservenum+1; i++) {
-		fprintf(f, "\t.llong\t0\n");
-		fprintf(f, "\t.llong\t0\n");
+
+	if (ht->mem_reserve_data.len > 0) {
+		fprintf(f, "/* Memory reserve map from source file */\n");
+		asm_emit_data(f, ht->mem_reserve_data);
 	}
 
+	fprintf(f, "\t.llong\t0\n");
+	fprintf(f, "\t.llong\t0\n");
+
 	emit_label(f, symprefix, "struct_start");
-	flatten_tree(tree, &asm_emitter, f, &strbuf, vi);
+	flatten_tree(ht->root, &asm_emitter, f, &strbuf, vi);
 	fprintf(f, "\t.long\tOF_DT_END\n");
 	emit_label(f, symprefix, "struct_end");
 
@@ -560,6 +584,43 @@ struct property *flat_read_property(stru
 	return build_property(name, val, NULL);
 }
 
+
+static struct data flat_read_mem_reserve(struct inbuf *inb)
+{
+	char *p;
+	int len = 0;
+	int done = 0;
+	cell_t cells[4];
+	struct data d;
+
+	d = empty_data;
+
+	/*
+	 * Each entry is a pair of u64 (addr, size) values for 4 cell_t's.
+	 * List terminates at an entry with size equal to zero.
+	 *
+	 * First pass, count entries.
+	 */
+	p = inb->ptr;
+	do {
+		flat_read_chunk(inb, &cells[0], 4 * sizeof(cell_t));
+		if (cells[2] == 0 && cells[3] == 0) {
+			done = 1;
+		} else {
+			++len;
+		}
+	} while (!done);
+
+	/*
+	 * Back up for pass two, reading the whole data value.
+	 */
+	inb->ptr = p;
+	d = flat_read_data(inb, len * 4 * sizeof(cell_t));
+
+	return d;
+}
+
+
 static char *nodename_from_path(char *ppath, char *cpath)
 {
 	char *lslash;
@@ -667,16 +728,21 @@ static struct node *unflatten_tree(struc
 	return node;
 }
 
-struct node *dt_from_blob(FILE *f)
+
+struct header_tree *dt_from_blob(FILE *f)
 {
-	u32 magic, totalsize, off_dt, off_str, version, size_str;
+	u32 magic, totalsize, version, size_str;
+	u32 off_dt, off_str, off_mem_rsvmap;
 	int rc;
 	char *blob;
 	struct boot_param_header *bph;
 	char *p;
 	struct inbuf dtbuf, strbuf;
+	struct inbuf memresvbuf;
 	int sizeleft;
+	struct data mem_reserve_data;
 	struct node *tree;
+	struct header_tree *ht;
 	u32 val;
 	int flags = 0;
 
@@ -734,18 +800,21 @@ struct node *dt_from_blob(FILE *f)
 
 	off_dt = be32_to_cpu(bph->off_dt_struct);
 	off_str = be32_to_cpu(bph->off_dt_strings);
+	off_mem_rsvmap = be32_to_cpu(bph->off_mem_rsvmap);
 	version = be32_to_cpu(bph->version);
 
 	fprintf(stderr, "\tmagic:\t\t\t0x%x\n", magic);
 	fprintf(stderr, "\ttotalsize:\t\t%d\n", totalsize);
 	fprintf(stderr, "\toff_dt_struct:\t\t0x%x\n", off_dt);
 	fprintf(stderr, "\toff_dt_strings:\t\t0x%x\n", off_str);
-	fprintf(stderr, "\toff_mem_rsvmap:\t\t0x%x\n",
-		be32_to_cpu(bph->off_mem_rsvmap));
+	fprintf(stderr, "\toff_mem_rsvmap:\t\t0x%x\n", off_mem_rsvmap);
 	fprintf(stderr, "\tversion:\t\t0x%x\n", version );
 	fprintf(stderr, "\tlast_comp_version:\t0x%x\n",
 		be32_to_cpu(bph->last_comp_version));
 
+	if (off_mem_rsvmap >= totalsize)
+		die("Mem Reserve structure offset exceeds total size\n");
+
 	if (off_dt >= totalsize)
 		die("DT structure offset exceeds total size\n");
 
@@ -767,12 +836,16 @@ struct node *dt_from_blob(FILE *f)
 		flags |= FTF_FULLPATH | FTF_NAMEPROPS | FTF_VARALIGN;
 	}
 
+	inbuf_init(&memresvbuf,
+		   blob + off_mem_rsvmap, blob + totalsize);
 	inbuf_init(&dtbuf, blob + off_dt, blob + totalsize);
 	inbuf_init(&strbuf, blob + off_str, blob + totalsize);
 
 	if (version >= 3)
 		strbuf.limit = strbuf.base + size_str;
 
+	mem_reserve_data = flat_read_mem_reserve(&memresvbuf);
+
 	val = flat_read_word(&dtbuf);
 
 	if (val != OF_DT_BEGIN_NODE)
@@ -786,5 +859,7 @@ struct node *dt_from_blob(FILE *f)
 
 	free(blob);
 
-	return tree;
+	ht = alloc_header_tree(mem_reserve_data, tree);
+
+	return ht;
 }
diff --git a/fstree.c b/fstree.c
--- a/fstree.c
+++ b/fstree.c
@@ -80,8 +80,9 @@ static struct node *read_fstree(char *di
 	return tree;
 }
 
-struct node *dt_from_fs(char *dirname)
+struct header_tree *dt_from_fs(char *dirname)
 {
+	struct header_tree *ht;
 	struct node *tree;
 
 	tree = read_fstree(dirname);
@@ -89,6 +90,7 @@ struct node *dt_from_fs(char *dirname)
 
 	fill_fullpaths(tree, "");
 
-	return tree;
+	ht = alloc_header_tree(empty_data, tree);
+	return ht;
 }
 
diff --git a/treesource.c b/treesource.c
--- a/treesource.c
+++ b/treesource.c
@@ -20,20 +20,60 @@
 
 #include "dtc.h"
 
-struct node *device_tree;
-
 extern FILE *yyin;
 extern int yyparse(void);
+extern void yyerror(char const *);
+
+static struct header_tree *the_header_tree;
+
+
+struct header_tree *alloc_header_tree(struct data mem_reserve_data,
+				      struct node *tree)
+{
+	struct header_tree *ht;
+
+	ht = xmalloc(sizeof(*ht));
+	ht->root = tree;
+	ht->mem_reserve_data = mem_reserve_data;
+
+	return ht;
+}
+
 
-struct node *dt_from_source(FILE *f)
+void build_header_tree(struct data mem_reserve_data,
+		       struct node *tree)
 {
+	struct header_tree *ht;
+
+	ht = alloc_header_tree(mem_reserve_data, tree);
+
+	the_header_tree = ht;
+}
+
+
+struct data build_mem_reserve(struct data d)
+{
+	/*
+	 * FIXME: Should reconcile the -R parameter here now?
+	 */
+	if (d.len % 16 != 0) {
+		yyerror("Memory Reserve entries are <u64 addr, u64 size>\n");
+	}
+	return d;
+}
+
+
+struct header_tree *dt_from_source(FILE *f)
+{
+	the_header_tree = NULL;
+
 	yyin = f;
 	if (yyparse() != 0)
 		return NULL;
 
-	fill_fullpaths(device_tree, "");
+	fill_fullpaths(the_header_tree->root, "");
 
-	return device_tree;
+	return the_header_tree;
 }
 
 static void write_prefix(FILE *f, int level)
@@ -75,7 +115,8 @@ static enum proptype guess_type(struct p
 		
 }
 
-void write_tree_source(FILE *f, struct node *tree, int level)
+
+void write_tree_source_node(FILE *f, struct node *tree, int level)
 {
 	struct property *prop;
 	struct node *child;
@@ -133,8 +174,39 @@ void write_tree_source(FILE *f, struct n
 	}
 	for_each_child(tree, child) {
 		fprintf(f, "\n");
-		write_tree_source(f, child, level+1);
+		write_tree_source_node(f, child, level+1);
 	}
 	write_prefix(f, level);
 	fprintf(f, "};\n");
 }
+
+
+void write_tree_source(FILE *f, struct header_tree *ht)
+{
+	int i;
+	int ncells;
+	cell_t *cp;
+
+	if (ht->mem_reserve_data.len > 0) {
+		fprintf(f, "/memreserve/ =\t<\n");
+		cp = (cell_t *)ht->mem_reserve_data.val;
+		ncells = ht->mem_reserve_data.len / sizeof(cell_t);
+		for (i = 0; i < ncells; i++) {
+			if (i % 4 == 0) {
+				fprintf(f, "\t\t\t");
+			}
+			fprintf(f, "%x", be32_to_cpu(*cp++));
+			if (i % 4 == 1) {
+				fprintf(f, "  ");
+			} else if (i % 4 == 3) {
+				fprintf(f, "\n");
+			} else {
+				fprintf(f, " ");
+			}
+		}
+		fprintf(f, "\t\t>;\n\n");
+	}
+
+	write_tree_source_node(f, ht->root, 0);
+}
+

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Thu, Jul 14, 2005 at 03:03:47PM -0500, Jon Loeliger wrote:
> On Mon, 2005-07-11 at 23:06, David Gibson wrote:
> > On Mon, Jul 11, 2005 at 04:22:30PM -0500, Jon Loeliger wrote:
> > > On Sun, 2005-07-10 at 23:55, David Gibson wrote:
> > > > On Fri, Jul 08, 2005 at 04:44:58PM -0500, Jon Loeliger wrote:
> > [snip]
> > > > Biggest thing is that rather than passing the tree itself and the
> > > > memreserve info about as two parameters all over the place, I'd rather
> > > > create a new structure which has both (and later can have anything
> > > > else that might be needed).
> > > 
> > > If you'd like, I'll do this work.
> > 
> > That would be helpful.  You'll need to rediff, though, I merged a
> > couple of bugfixes from your patch that weren't directly related to
> > the memreserve stuff.
> 
> David,
> 
> Here is an updated version of the patch that obsoletes
> the previous one I submitted.  I have incorporated all
> of your syntactic suggestions except not using the
> split-64 values (ie, this still uses 'struct data').
> It primarily merges in the changes that you adopted
> from earlier and implements a new structure at the
> base of the parse tree to hold both the device tree
> and the header information.  I called that new stucuture
> 'struct header_tree'.  Feel free to dream up something
> better. :-)

Ok, I've merged this, although I've tweaked things substantially in
the process.  I did rename "header_tree" to "boot_info", moved some
things around, and changed the syntax.  Reserve ranges can now be
specified either as an address and length:

	/memreserve/	10000000 00002000;

or as an (inclusive) address range:

	/memreserve/	10000000-10001fff;

I am a bit worried that those two forms may be hard to distinguish at
a glance.  Any sugggestions for changes to the syntax soon please, I'd
really like to keep the source syntax as stable as possible.

-- 
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/people/dgibson

Re: PATCH: Add memreserve to DTC

[Jon Loeliger]

On Fri, 2005-07-15 at 02:19, David Gibson wrote:

> 
> Ok, I've merged this, 

Excellent, thanks!

> although I've tweaked things substantially in the process.

No problem.

>   I did rename "header_tree" to "boot_info", moved some

Oh, good!

> things around, and changed the syntax.  Reserve ranges can now be
> specified either as an address and length:
> 
> 	/memreserve/	10000000 00002000;
> 
> or as an (inclusive) address range:
> 
> 	/memreserve/	10000000-10001fff;
> 
> I am a bit worried that those two forms may be hard to distinguish at
> a glance.  Any sugggestions for changes to the syntax soon please, I'd
> really like to keep the source syntax as stable as possible.

Oh man.  With syntax you can demystify those in any number
of ways.  Just a matter of what you are wanting.  You can
always add sugar:

    /memreserve_block/    10000000 00002000;
    /memreserve_range/    10000000 10001fff;

    /memreserve/    10000000 /for/     2000;        //  or /size/ ?
    /memreserve/    10000000 /through/ 10001fff;

    /memreserve/    10000000 00002000;
    /memreserve/   [10000000, 10001fff];     // or [10000000, 10002000)?

Stuff like that maybe?

jdl

Re: PATCH: Add memreserve to DTC

[David Gibson]

On Fri, Jul 15, 2005 at 09:30:58AM -0500, Jon Loeliger wrote:
> On Fri, 2005-07-15 at 02:19, David Gibson wrote:
> 
> > 
> > Ok, I've merged this, 
> 
> Excellent, thanks!
> 
> > although I've tweaked things substantially in the process.
> 
> No problem.
> 
> >   I did rename "header_tree" to "boot_info", moved some
> 
> Oh, good!
> 
> > things around, and changed the syntax.  Reserve ranges can now be
> > specified either as an address and length:
> > 
> > 	/memreserve/	10000000 00002000;
> > 
> > or as an (inclusive) address range:
> > 
> > 	/memreserve/	10000000-10001fff;
> > 
> > I am a bit worried that those two forms may be hard to distinguish at
> > a glance.  Any sugggestions for changes to the syntax soon please, I'd
> > really like to keep the source syntax as stable as possible.
> 
> Oh man.  With syntax you can demystify those in any number
> of ways.  Just a matter of what you are wanting.  You can
> always add sugar:
> 
>     /memreserve_block/    10000000 00002000;
>     /memreserve_range/    10000000 10001fff;
> 
>     /memreserve/    10000000 /for/     2000;        //  or /size/ ?
>     /memreserve/    10000000 /through/ 10001fff;
> 
>     /memreserve/    10000000 00002000;
>     /memreserve/   [10000000, 10001fff];     // or [10000000, 10002000)?
> 
> Stuff like that maybe?

Hrm.. don't really like any of those better than what I have already,
I'm afraid.  It does occur to me that size > base is going to be a
very rare situation, so the value of the numbers themselves will act
as a reasonable hint as to which form is in use.

-- 
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/people/dgibson