Re: [PATCH v10 3/4] dtc: Plugin and fixup support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Nov 28, 2016, at 06:12 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> On Fri, Nov 25, 2016 at 02:32:10PM +0200, Pantelis Antoniou wrote:
>> This patch enable the generation of symbols & local fixup information
>> for trees compiled with the -@ (--symbols) option.
>> 
>> Using this patch labels in the tree and their users emit information
>> in __symbols__ and __local_fixups__ nodes.
>> 
>> The __fixups__ node make possible the dynamic resolution of phandle
>> references which are present in the plugin tree but lie in the
>> tree that are applying the overlay against.
>> 
>> While there is a new magic number for dynamic device tree/overlays blobs
>> it is by default enabled. Remember to use -M to generate compatible
>> blobs.
>> 
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
>> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>> Signed-off-by: Jan Luebbe <jlu@xxxxxxxxxxxxxx>
>> ---
>> Documentation/manual.txt |  25 +++++-
>> checks.c                 |   8 +-
>> dtc-lexer.l              |   5 ++
>> dtc-parser.y             |  50 +++++++++--
>> dtc.c                    |  39 +++++++-
>> dtc.h                    |  20 ++++-
>> fdtdump.c                |   2 +-
>> flattree.c               |  17 ++--
>> fstree.c                 |   2 +-
>> libfdt/fdt.c             |   2 +-
>> libfdt/fdt.h             |   3 +-
>> livetree.c               | 225 ++++++++++++++++++++++++++++++++++++++++++++++-
>> tests/mangle-layout.c    |   7 +-
>> 13 files changed, 375 insertions(+), 30 deletions(-)
>> 
>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>> index 398de32..094893b 100644
>> --- a/Documentation/manual.txt
>> +++ b/Documentation/manual.txt
>> @@ -119,6 +119,24 @@ Options:
>> 	Make space for <number> reserve map entries
>> 	Relevant for dtb and asm output only.
>> 
>> +    -@
>> +	Generates a __symbols__ node at the root node of the resulting blob
>> +	for any node labels used, and for any local references using phandles
>> +	it also generates a __local_fixups__ node that tracks them.
>> +
>> +	When using the /plugin/ tag all unresolved label references to
>> +	be tracked in the __fixups__ node, making dynamic resolution possible.
>> +
>> +    -A
>> +	Generate automatically aliases for all node labels. This is similar to
>> +	the -@ option (the __symbols__ node contain identical information) but
>> +	the semantics are slightly different since no phandles are automatically
>> +	generated for labeled nodes.
>> +
>> +    -M
>> +	Generate blobs with the old FDT magic number for device tree objects.
>> +	By default blobs use the DTBO FDT magic number instead.
>> +
>>     -S <bytes>
>> 	Ensure the blob at least <bytes> long, adding additional
>> 	space if needed.
>> @@ -146,13 +164,18 @@ Additionally, dtc performs various sanity checks on the tree.
>> Here is a very rough overview of the layout of a DTS source file:
>> 
>> 
>> -    sourcefile:   list_of_memreserve devicetree
>> +    sourcefile:   versioninfo plugindecl list_of_memreserve devicetree
>> 
>>     memreserve:   label 'memreserve' ADDR ADDR ';'
>> 		| label 'memreserve' ADDR '-' ADDR ';'
>> 
>>     devicetree:   '/' nodedef
>> 
>> +    versioninfo:  '/' 'dts-v1' '/' ';'
>> +
>> +    plugindecl:   '/' 'plugin' '/' ';'
>> +                | /* empty */
>> +
>>     nodedef:      '{' list_of_property list_of_subnode '}' ';'
>> 
>>     property:     label PROPNAME '=' propdata ';'
>> diff --git a/checks.c b/checks.c
>> index 2bd27a4..4292f4b 100644
>> --- a/checks.c
>> +++ b/checks.c
>> @@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi,
>> 
>> 			refnode = get_node_by_ref(dt, m->ref);
>> 			if (! refnode) {
>> -				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
>> -				     m->ref);
>> +				if (!(bi->versionflags & VF_PLUGIN))
>> +					FAIL(c, "Reference to non-existent node or "
>> +							"label \"%s\"\n", m->ref);
>> +				else /* mark the entry as unresolved */
>> +					*((cell_t *)(prop->val.val + m->offset)) =
>> +						cpu_to_fdt32(0xffffffff);
>> 				continue;
>> 			}
>> 
>> diff --git a/dtc-lexer.l b/dtc-lexer.l
>> index 790fbf6..40bbc87 100644
>> --- a/dtc-lexer.l
>> +++ b/dtc-lexer.l
>> @@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...);
>> 			return DT_V1;
>> 		}
>> 
>> +<*>"/plugin/"	{
>> +			DPRINT("Keyword: /plugin/\n");
>> +			return DT_PLUGIN;
>> +		}
>> +
>> <*>"/memreserve/"	{
>> 			DPRINT("Keyword: /memreserve/\n");
>> 			BEGIN_DEFAULT();
>> diff --git a/dtc-parser.y b/dtc-parser.y
>> index 14aaf2e..1a1f660 100644
>> --- a/dtc-parser.y
>> +++ b/dtc-parser.y
>> @@ -19,6 +19,7 @@
>>  */
>> %{
>> #include <stdio.h>
>> +#include <inttypes.h>
>> 
>> #include "dtc.h"
>> #include "srcpos.h"
>> @@ -33,6 +34,7 @@ extern void yyerror(char const *s);
>> 
>> extern struct boot_info *the_boot_info;
>> extern bool treesource_error;
>> +
> 
> Extraneous whitespace change here
> 

OK.

>> %}
>> 
>> %union {
>> @@ -52,9 +54,11 @@ extern bool treesource_error;
>> 	struct node *nodelist;
>> 	struct reserve_info *re;
>> 	uint64_t integer;
>> +	unsigned int flags;
>> }
>> 
>> %token DT_V1
>> +%token DT_PLUGIN
>> %token DT_MEMRESERVE
>> %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
>> %token DT_BITS
>> @@ -71,6 +75,8 @@ extern bool treesource_error;
>> 
>> %type <data> propdata
>> %type <data> propdataprefix
>> +%type <flags> versioninfo
>> +%type <flags> plugindecl
>> %type <re> memreserve
>> %type <re> memreserves
>> %type <array> arrayprefix
>> @@ -101,16 +107,34 @@ extern bool treesource_error;
>> %%
>> 
>> sourcefile:
>> -	  v1tag memreserves devicetree
>> +	  versioninfo plugindecl memreserves devicetree
>> +		{
>> +			the_boot_info = build_boot_info($1 | $2, $3, $4,
>> +							guess_boot_cpuid($4));
>> +		}
>> +	;
>> +
>> +versioninfo:
>> +	v1tag
>> 		{
>> -			the_boot_info = build_boot_info($2, $3,
>> -							guess_boot_cpuid($3));
>> +			$$ = VF_DT_V1;
>> 		}
>> 	;
>> 
>> v1tag:
>> 	  DT_V1 ';'
>> +	| DT_V1
>> 	| DT_V1 ';' v1tag
>> +
>> +plugindecl:
>> +	DT_PLUGIN ';'
>> +		{
>> +			$$ = VF_PLUGIN;
>> +		}
>> +	| /* empty */
>> +		{
>> +			$$ = 0;
>> +		}
>> 	;
>> 
>> memreserves:
>> @@ -161,10 +185,19 @@ devicetree:
>> 		{
>> 			struct node *target = get_node_by_ref($1, $2);
>> 
>> -			if (target)
>> +			if (target) {
>> 				merge_nodes(target, $3);
>> -			else
>> -				ERROR(&@2, "Label or path %s not found", $2);
>> +			} else {
>> +				/*
>> +				 * We rely on the rule being always:
>> +				 *   versioninfo plugindecl memreserves devicetree
>> +				 * so $-1 is what we want (plugindecl)
>> +				 */
>> +				if ($<flags>-1 & VF_PLUGIN)
> 
> o_O... ok.  I've never seen negative value references before.  Can you
> provide a link to some documentation saying this is actually supported
> usage in bison?  I wasn't able to find it when I looked.
> 

There is a section about inherited attributes in the flex & bison book by O’Reily.

https://books.google.gr/books?id=3Sr1V5J9_qMC&lpg=PP1&dq=flex%20bison&hl=el&pg=PP1#v=onepage&q=flex%20bison&f=false

There’s a direct link to the 2nd Edition of lex & yacc:

https://books.google.gr/books?id=fMPxfWfe67EC&lpg=PA183&ots=RcRSji2NAT&dq=yacc%20inherited%20attributes&hl=el&pg=PA183#v=onepage&q=yacc%20inherited%20attributes&f=false

>> +					add_orphan_node($1, $3, $2);
>> +				else
>> +					ERROR(&@2, "Label or path %s not found", $2);
>> +			}
>> 			$$ = $1;
>> 		}
>> 	| devicetree DT_DEL_NODE DT_REF ';'
>> @@ -179,6 +212,11 @@ devicetree:
>> 
>> 			$$ = $1;
>> 		}
>> +	| /* empty */
>> +		{
>> +			/* build empty node */
>> +			$$ = name_node(build_node(NULL, NULL), "");
>> +		}
>> 	;
>> 
>> nodedef:
>> diff --git a/dtc.c b/dtc.c
>> index 9dcf640..06e91bc 100644
>> --- a/dtc.c
>> +++ b/dtc.c
>> @@ -32,6 +32,9 @@ int minsize;		/* Minimum blob size */
>> int padsize;		/* Additional padding to blob */
>> int alignsize;		/* Additional padding to blob accroding to the alignsize */
>> int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
>> +int symbol_fixup_support;	/* enable symbols & fixup support */
>> +int auto_label_aliases;		/* auto generate labels -> aliases */
>> +int no_dtbo_magic;		/* use old FDT magic values for objects */
>> 
>> static int is_power_of_2(int x)
>> {
>> @@ -59,7 +62,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
>> #define FDT_VERSION(version)	_FDT_VERSION(version)
>> #define _FDT_VERSION(version)	#version
>> static const char usage_synopsis[] = "dtc [options] <input file>";
>> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
>> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMhv";
>> static struct option const usage_long_opts[] = {
>> 	{"quiet",            no_argument, NULL, 'q'},
>> 	{"in-format",         a_argument, NULL, 'I'},
>> @@ -78,6 +81,9 @@ static struct option const usage_long_opts[] = {
>> 	{"phandle",           a_argument, NULL, 'H'},
>> 	{"warning",           a_argument, NULL, 'W'},
>> 	{"error",             a_argument, NULL, 'E'},
>> +	{"symbols",	     no_argument, NULL, '@'},
>> +	{"auto-alias",       no_argument, NULL, 'A'},
>> +	{"no-dtbo-magic",    no_argument, NULL, 'M'},
>> 	{"help",             no_argument, NULL, 'h'},
>> 	{"version",          no_argument, NULL, 'v'},
>> 	{NULL,               no_argument, NULL, 0x0},
>> @@ -109,6 +115,9 @@ static const char * const usage_opts_help[] = {
>> 	 "\t\tboth   - Both \"linux,phandle\" and \"phandle\" properties",
>> 	"\n\tEnable/disable warnings (prefix with \"no-\")",
>> 	"\n\tEnable/disable errors (prefix with \"no-\")",
>> +	"\n\tEnable symbols/fixup support",
>> +	"\n\tEnable auto-alias of labels",
>> +	"\n\tDo not use DTBO magic value for plugin objects",
>> 	"\n\tPrint this help and exit",
>> 	"\n\tPrint version and exit",
>> 	NULL,
>> @@ -153,7 +162,7 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>> 	fclose(f);
>> 
>> 	magic = fdt32_to_cpu(magic);
>> -	if (magic == FDT_MAGIC)
>> +	if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO)
>> 		return "dtb";
>> 
>> 	return guess_type_by_name(fname, fallback);
>> @@ -172,6 +181,7 @@ int main(int argc, char *argv[])
>> 	FILE *outf = NULL;
>> 	int outversion = DEFAULT_FDT_VERSION;
>> 	long long cmdline_boot_cpuid = -1;
>> +	fdt32_t out_magic = FDT_MAGIC;
>> 
>> 	quiet      = 0;
>> 	reservenum = 0;
>> @@ -249,6 +259,16 @@ int main(int argc, char *argv[])
>> 			parse_checks_option(false, true, optarg);
>> 			break;
>> 
>> +		case '@':
>> +			symbol_fixup_support = 1;
>> +			break;
>> +		case 'A':
>> +			auto_label_aliases = 1;
>> +			break;
>> +		case 'M':
>> +			no_dtbo_magic = 1;
>> +			break;
>> +
>> 		case 'h':
>> 			usage(NULL);
>> 		default:
>> @@ -306,6 +326,14 @@ int main(int argc, char *argv[])
>> 	fill_fullpaths(bi->dt, "");
>> 	process_checks(force, bi);
>> 
>> +	if (auto_label_aliases)
>> +		generate_label_tree(bi->dt, "aliases", false);
>> +
>> +	if (symbol_fixup_support) {
>> +		generate_label_tree(bi->dt, "__symbols__", true);
>> +		generate_fixups_tree(bi->dt);
> 
> Hang on.. this doesn't seem right.  I thought -@ controlled the
> __symbols__ side (i.e. the part upon which we overlay) rather than the
> fixups side (the part which overlays).  A dtbo could certainly have
> both, of course, but for base trees, wouldn't you have symbols without
> fixups?  And should it be illegal to try to build a /plugin/ without
> -@?

It does control both for now. For base trees having the fixup nodes
will allow us to do probe order dependency tracking in the future.
For plugins we need the __symbols__ node to support stacked overlays, i.e.
overlays referring label that were introduced by a previous overlay.  

For plugins there is no requirement for now to actually contain references to
be resolved. It can easily be enforced though. 

> 
>> +	}
>> +
>> 	if (sort)
>> 		sort_tree(bi);
>> 
>> @@ -318,12 +346,15 @@ int main(int argc, char *argv[])
>> 			    outname, strerror(errno));
>> 	}
>> 
>> +	if (!no_dtbo_magic && (bi->versionflags & VF_PLUGIN))
>> +		out_magic = FDT_MAGIC_DTBO;
>> +
>> 	if (streq(outform, "dts")) {
>> 		dt_to_source(outf, bi);
>> 	} else if (streq(outform, "dtb")) {
>> -		dt_to_blob(outf, bi, outversion);
>> +		dt_to_blob(outf, bi, out_magic, outversion);
>> 	} else if (streq(outform, "asm")) {
>> -		dt_to_asm(outf, bi, outversion);
>> +		dt_to_asm(outf, bi, out_magic, outversion);
>> 	} else if (streq(outform, "null")) {
>> 		/* do nothing */
>> 	} else {
>> diff --git a/dtc.h b/dtc.h
>> index 32009bc..581b3bf 100644
>> --- a/dtc.h
>> +++ b/dtc.h
>> @@ -55,6 +55,9 @@ extern int minsize;		/* Minimum blob size */
>> extern int padsize;		/* Additional padding to blob */
>> extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
>> extern int phandle_format;	/* Use linux,phandle or phandle properties */
>> +extern int symbol_fixup_support;/* enable symbols & fixup support */
>> +extern int auto_label_aliases;	/* auto generate labels -> aliases */
>> +extern int no_dtbo_magic;	/* use old FDT magic values for objects */
>> 
>> #define PHANDLE_LEGACY	0x1
>> #define PHANDLE_EPAPR	0x2
>> @@ -195,6 +198,7 @@ struct node *build_node_delete(void);
>> struct node *name_node(struct node *node, char *name);
>> struct node *chain_node(struct node *first, struct node *list);
>> struct node *merge_nodes(struct node *old_node, struct node *new_node);
>> +void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
>> 
>> void add_property(struct node *node, struct property *prop);
>> void delete_property_by_name(struct node *node, char *name);
>> @@ -202,6 +206,8 @@ void delete_property(struct property *prop);
>> void add_child(struct node *parent, struct node *child);
>> void delete_node_by_name(struct node *parent, char *name);
>> void delete_node(struct node *node);
>> +void append_to_property(struct node *node,
>> +			char *name, const void *data, int len);
>> 
>> const char *get_unitname(struct node *node);
>> struct property *get_property(struct node *node, const char *propname);
>> @@ -237,14 +243,22 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>> 
>> 
>> struct boot_info {
>> +	unsigned int versionflags;
>> 	struct reserve_info *reservelist;
>> 	struct node *dt;		/* the device tree */
>> 	uint32_t boot_cpuid_phys;
>> };
>> 
>> -struct boot_info *build_boot_info(struct reserve_info *reservelist,
>> +/* version flags definitions */
>> +#define VF_DT_V1	0x0001	/* /dts-v1/ */
>> +#define VF_PLUGIN	0x0002	/* /plugin/ */
>> +
>> +struct boot_info *build_boot_info(unsigned int versionflags,
>> +				  struct reserve_info *reservelist,
>> 				  struct node *tree, uint32_t boot_cpuid_phys);
>> void sort_tree(struct boot_info *bi);
>> +void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph);
>> +void generate_fixups_tree(struct node *dt);
>> 
>> /* Checks */
>> 
>> @@ -253,8 +267,8 @@ void process_checks(bool force, struct boot_info *bi);
>> 
>> /* Flattened trees */
>> 
>> -void dt_to_blob(FILE *f, struct boot_info *bi, int version);
>> -void dt_to_asm(FILE *f, struct boot_info *bi, int version);
>> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
>> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
>> 
>> struct boot_info *dt_from_blob(const char *fname);
>> 
>> diff --git a/fdtdump.c b/fdtdump.c
>> index a9a2484..dd63ac2 100644
>> --- a/fdtdump.c
>> +++ b/fdtdump.c
>> @@ -201,7 +201,7 @@ int main(int argc, char *argv[])
>> 			p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
>> 			if (!p)
>> 				break;
>> -			if (fdt_magic(p) == FDT_MAGIC) {
>> +			if (fdt_magic(p) == FDT_MAGIC || fdt_magic(p) == FDT_MAGIC_DTBO) {
>> 				/* try and validate the main struct */
>> 				off_t this_len = endp - p;
>> 				fdt32_t max_version = 17;
>> diff --git a/flattree.c b/flattree.c
>> index a9d9520..57d76cf 100644
>> --- a/flattree.c
>> +++ b/flattree.c
>> @@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
>> }
>> 
>> static void make_fdt_header(struct fdt_header *fdt,
>> +			    fdt32_t magic,
>> 			    struct version_info *vi,
>> 			    int reservesize, int dtsize, int strsize,
>> 			    int boot_cpuid_phys)
>> @@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>> 
>> 	memset(fdt, 0xff, sizeof(*fdt));
>> 
>> -	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
>> +	fdt->magic = cpu_to_fdt32(magic);
>> 	fdt->version = cpu_to_fdt32(vi->version);
>> 	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
>> 
>> @@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>> 		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
>> }
>> 
>> -void dt_to_blob(FILE *f, struct boot_info *bi, int version)
>> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
>> 	reservebuf = flatten_reserve_list(bi->reservelist, vi);
>> 
>> 	/* Make header */
>> -	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
>> +	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
>> 			bi->boot_cpuid_phys);
>> 
>> 	/*
>> @@ -467,7 +468,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>> 	}
>> }
>> 
>> -void dt_to_asm(FILE *f, struct boot_info *bi, int version)
>> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -830,6 +831,7 @@ struct boot_info *dt_from_blob(const char *fname)
>> 	struct node *tree;
>> 	uint32_t val;
>> 	int flags = 0;
>> +	unsigned int versionflags = VF_DT_V1;
>> 
>> 	f = srcfile_relative_open(fname, NULL);
>> 
>> @@ -845,9 +847,12 @@ struct boot_info *dt_from_blob(const char *fname)
>> 	}
>> 
>> 	magic = fdt32_to_cpu(magic);
>> -	if (magic != FDT_MAGIC)
>> +	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
>> 		die("Blob has incorrect magic number\n");
>> 
>> +	if (magic == FDT_MAGIC_DTBO)
>> +		versionflags |= VF_PLUGIN;
> 
> Not particularly useful yet, but I wonder if we'll want some option to
> force treating dtb input as a plugin, for the case of old plugins
> which don't have the new magic number.
> 

It can easily be added.

>> +
>> 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
>> 	if (ferror(f))
>> 		die("Error reading DT blob size: %s\n", strerror(errno));
>> @@ -942,5 +947,5 @@ struct boot_info *dt_from_blob(const char *fname)
>> 
>> 	fclose(f);
>> 
>> -	return build_boot_info(reservelist, tree, boot_cpuid_phys);
>> +	return build_boot_info(versionflags, reservelist, tree, boot_cpuid_phys);
>> }
>> diff --git a/fstree.c b/fstree.c
>> index 6d1beec..54f520b 100644
>> --- a/fstree.c
>> +++ b/fstree.c
>> @@ -86,6 +86,6 @@ struct boot_info *dt_from_fs(const char *dirname)
>> 	tree = read_fstree(dirname);
>> 	tree = name_node(tree, "");
>> 
>> -	return build_boot_info(NULL, tree, guess_boot_cpuid(tree));
>> +	return build_boot_info(VF_DT_V1, NULL, tree, guess_boot_cpuid(tree));
>> }
>> 
>> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
>> index 22286a1..28d422c 100644
>> --- a/libfdt/fdt.c
>> +++ b/libfdt/fdt.c
>> @@ -57,7 +57,7 @@
>> 
>> int fdt_check_header(const void *fdt)
>> {
>> -	if (fdt_magic(fdt) == FDT_MAGIC) {
>> +	if (fdt_magic(fdt) == FDT_MAGIC || fdt_magic(fdt) == FDT_MAGIC_DTBO) {
>> 		/* Complete tree */
>> 		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
>> 			return -FDT_ERR_BADVERSION;
>> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
>> index 526aedb..493cd55 100644
>> --- a/libfdt/fdt.h
>> +++ b/libfdt/fdt.h
>> @@ -55,7 +55,7 @@
>> #ifndef __ASSEMBLY__
>> 
>> struct fdt_header {
>> -	fdt32_t magic;			 /* magic word FDT_MAGIC */
>> +	fdt32_t magic;			 /* magic word FDT_MAGIC[|_DTBO] */
>> 	fdt32_t totalsize;		 /* total size of DT block */
>> 	fdt32_t off_dt_struct;		 /* offset to structure */
>> 	fdt32_t off_dt_strings;		 /* offset to strings */
>> @@ -93,6 +93,7 @@ struct fdt_property {
>> #endif /* !__ASSEMBLY */
>> 
>> #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
>> +#define FDT_MAGIC_DTBO	0xd00dfdb0	/* DTBO magic */
>> #define FDT_TAGSIZE	sizeof(fdt32_t)
>> 
>> #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
>> diff --git a/livetree.c b/livetree.c
>> index 3dc7559..f2c86bd 100644
>> --- a/livetree.c
>> +++ b/livetree.c
>> @@ -216,6 +216,31 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> 	return old_node;
>> }
>> 
>> +void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
>> +{
>> +	static unsigned int next_orphan_fragment = 0;
>> +	struct node *node = build_node(NULL, NULL);
>> +	struct property *p;
>> +	struct data d = empty_data;
>> +	char *name;
>> +
>> +	memset(node, 0, sizeof(*node));
> 
> You don't need the memset() now that you're using build_node() above.
> 

OK

>> +
>> +	d = data_add_marker(d, REF_PHANDLE, ref);
>> +	d = data_append_integer(d, 0xffffffff, 32);
>> +
>> +	p = build_property("target", d);
>> +	add_property(node, p);
>> +
>> +	xasprintf(&name, "fragment@%u",
>> +			next_orphan_fragment++);
>> +	name_node(node, name);
>> +	name_node(new_node, "__overlay__");
> 
> You can do this more naturally if you do the name_node() here, then
> you can just pass the __overlay__ node into build_node() for the
> fragment@ node instead of having to explicitly add_child.
> 

Hmm, I’ll see if I can rework it like this.

>> +
>> +	add_child(dt, node);
>> +	add_child(node, new_node);
>> +}
>> +
>> struct node *chain_node(struct node *first, struct node *list)
>> {
>> 	assert(first->next_sibling == NULL);
>> @@ -296,6 +321,23 @@ void delete_node(struct node *node)
>> 	delete_labels(&node->labels);
>> }
>> 
>> +void append_to_property(struct node *node,
>> +				    char *name, const void *data, int len)
>> +{
>> +	struct data d;
>> +	struct property *p;
>> +
>> +	p = get_property(node, name);
>> +	if (p) {
>> +		d = data_append_data(p->val, data, len);
>> +		p->val = d;
>> +	} else {
>> +		d = data_append_data(empty_data, data, len);
>> +		p = build_property(name, d);
>> +		add_property(node, p);
>> +	}
>> +}
>> +
>> struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>> {
>> 	struct reserve_info *new = xmalloc(sizeof(*new));
>> @@ -335,12 +377,14 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>> 	return list;
>> }
>> 
>> -struct boot_info *build_boot_info(struct reserve_info *reservelist,
>> +struct boot_info *build_boot_info(unsigned int versionflags,
>> +				  struct reserve_info *reservelist,
>> 				  struct node *tree, uint32_t boot_cpuid_phys)
>> {
>> 	struct boot_info *bi;
>> 
>> 	bi = xmalloc(sizeof(*bi));
>> +	bi->versionflags = versionflags;
>> 	bi->reservelist = reservelist;
>> 	bi->dt = tree;
>> 	bi->boot_cpuid_phys = boot_cpuid_phys;
>> @@ -709,3 +753,182 @@ void sort_tree(struct boot_info *bi)
>> 	sort_reserve_entries(bi);
>> 	sort_node(bi->dt);
>> }
>> +
>> +/* utility helper to avoid code duplication */
>> +static struct node *build_and_name_child_node(struct node *parent, char *name)
>> +{
>> +	struct node *node;
>> +
>> +	node = build_node(NULL, NULL);
>> +	name_node(node, xstrdup(name));
>> +	add_child(parent, node);
>> +
>> +	return node;
>> +}
>> +
>> +static void generate_label_tree_internal(struct node *dt, struct node *node,
>> +					 struct node *an, bool allocph)
>> +{
>> +	struct node *c;
>> +	struct property *p;
>> +	struct label *l;
>> +
>> +	/* if if there are labels */
>> +	if (node->labels) {
>> +		/* now add the label in the node */
>> +		for_each_label(node->labels, l) {
>> +			/* check whether the label already exists */
>> +			p = get_property(an, l->label);
>> +			if (p) {
>> +				fprintf(stderr, "WARNING: label %s already"
>> +					" exists in /%s", l->label,
>> +					an->name);
>> +				continue;
>> +			}
>> +
>> +			/* insert it */
>> +			p = build_property(l->label,
>> +				data_copy_mem(node->fullpath,
>> +						strlen(node->fullpath) + 1));
>> +			add_property(an, p);
>> +		}
>> +
>> +		/* force allocation of a phandle for this node */
>> +		if (allocph)
>> +			(void)get_node_phandle(dt, node);
>> +	}
>> +
>> +	for_each_child(node, c)
>> +		generate_label_tree_internal(dt, c, an, allocph);
>> +}
>> +
>> +void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph)
>> +{
>> +	struct node *an;
>> +
>> +	for_each_child(dt, an)
>> +		if (streq(gen_node_name, an->name))
>> +			break;
>> +
>> +	if (!an)
>> +		an = build_and_name_child_node(dt, gen_node_name);
>> +	if (!an)
>> +		die("Could not build label node /%s\n", gen_node_name);
>> +
>> +	generate_label_tree_internal(dt, dt, an, allocph);
>> +}
>> +
>> +#define FIXUPS	"__fixups__"
>> +#define LOCAL_FIXUPS "__local_fixups__"
>> +
>> +static void add_fixup_entry(struct node *dt, struct node *node,
>> +		struct property *prop, struct marker *m)
>> +{
>> +	struct node *fn;	/* fixup node */
>> +	char *entry;
>> +
>> +	/* m->ref can only be a REF_PHANDLE, but check anyway */
>> +	assert(m->type == REF_PHANDLE);
>> +
>> +	/* fn is the node we're putting entries in */
>> +	fn = get_subnode(dt, FIXUPS);
>> +	assert(fn != NULL);
>> +
>> +	/* there shouldn't be any ':' in the arguments */
>> +	if (strchr(node->fullpath, ':') || strchr(prop->name, ':'))
>> +		die("arguments should not contain ':'\n");
>> +
>> +	xasprintf(&entry, "%s:%s:%u",
>> +			node->fullpath, prop->name, m->offset);
>> +	append_to_property(fn, m->ref, entry, strlen(entry) + 1);
>> +}
>> +
>> +static void add_local_fixup_entry(struct node *dt, struct node *node,
>> +		struct property *prop, struct marker *m,
>> +		struct node *refnode)
>> +{
>> +	struct node *lfn, *wn, *nwn;	/* local fixup node, walk node, new */
>> +	uint32_t value_32;
>> +	char *s, *e, *comp;
>> +	int len;
>> +
>> +	/* fn is the node we're putting entries in */
>> +	lfn = get_subnode(dt, LOCAL_FIXUPS);
>> +	assert(lfn != NULL);
>> +
>> +	/* walk the path components creating nodes if they don't exist */
>> +	comp = xmalloc(strlen(node->fullpath) + 1);
>> +	/* start skipping the first / */
>> +	s = node->fullpath + 1;
>> +	wn = lfn;
>> +	while (*s) {
>> +		/* retrieve path component */
>> +		e = strchr(s, '/');
>> +		if (e == NULL)
>> +			e = s + strlen(s);
>> +		len = e - s;
>> +		memcpy(comp, s, len);
>> +		comp[len] = '\0';
> 
> Parsing the fullpath into components seems an odd way of doing this.
> We have an actual handle on the node, and therefore all it's parents,
> which already have the individual path components split out.
> 

Hmm, I’ll see if it can be done. I don’t remember what was the original
cause for using this form.

>> +
>> +		/* if no node exists, create it */
>> +		nwn = get_subnode(wn, comp);
>> +		if (!nwn)
>> +			nwn = build_and_name_child_node(wn, comp);
>> +		wn = nwn;
>> +
>> +		/* last path component */
>> +		if (!*e)
>> +			break;
>> +
>> +		/* next path component */
>> +		s = e + 1;
>> +	}
>> +	free(comp);
>> +
>> +	value_32 = cpu_to_fdt32(m->offset);
>> +	append_to_property(wn, prop->name, &value_32, sizeof(value_32));
>> +}
>> +
>> +static void generate_fixups_tree_internal(struct node *dt, struct node *node)
>> +{
>> +	struct node *c;
>> +	struct property *prop;
>> +	struct marker *m;
>> +	struct node *refnode;
>> +
>> +	for_each_property(node, prop) {
>> +		m = prop->val.markers;
>> +		for_each_marker_of_type(m, REF_PHANDLE) {
>> +			refnode = get_node_by_ref(dt, m->ref);
>> +			if (!refnode)
>> +				add_fixup_entry(dt, node, prop, m);
>> +			else
>> +				add_local_fixup_entry(dt, node, prop, m,
>> +						refnode);
>> +		}
>> +	}
>> +
>> +	for_each_child(node, c)
>> +		generate_fixups_tree_internal(dt, c);
>> +}
>> +
>> +void generate_fixups_tree(struct node *dt)
>> +{
>> +	struct node *an;
>> +
>> +	for_each_child(dt, an)
>> +		if (streq(FIXUPS, an->name))
>> +			break;
>> +
>> +	if (!an)
>> +		build_and_name_child_node(dt, FIXUPS);
>> +
>> +	for_each_child(dt, an)
>> +		if (streq(LOCAL_FIXUPS, an->name))
>> +			break;
>> +
>> +	if (!an)
>> +		build_and_name_child_node(dt, LOCAL_FIXUPS);
>> +
>> +	generate_fixups_tree_internal(dt, dt);
>> +}
>> diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
>> index a76e51e..d29ebc6 100644
>> --- a/tests/mangle-layout.c
>> +++ b/tests/mangle-layout.c
>> @@ -42,7 +42,8 @@ static void expand_buf(struct bufstate *buf, int newsize)
>> 	buf->size = newsize;
>> }
>> 
>> -static void new_header(struct bufstate *buf, int version, const void *fdt)
>> +static void new_header(struct bufstate *buf, fdt32_t magic, int version,
>> +		       const void *fdt)
>> {
>> 	int hdrsize;
>> 
>> @@ -56,7 +57,7 @@ static void new_header(struct bufstate *buf, int version, const void *fdt)
>> 	expand_buf(buf, hdrsize);
>> 	memset(buf->buf, 0, hdrsize);
>> 
>> -	fdt_set_magic(buf->buf, FDT_MAGIC);
>> +	fdt_set_magic(buf->buf, magic);
>> 	fdt_set_version(buf->buf, version);
>> 	fdt_set_last_comp_version(buf->buf, 16);
>> 	fdt_set_boot_cpuid_phys(buf->buf, fdt_boot_cpuid_phys(fdt));
>> @@ -145,7 +146,7 @@ int main(int argc, char *argv[])
>> 	if (fdt_version(fdt) < 17)
>> 		CONFIG("Input tree must be v17");
>> 
>> -	new_header(&buf, version, fdt);
>> +	new_header(&buf, FDT_MAGIC, version, fdt);
>> 
>> 	while (*blockorder) {
>> 		add_block(&buf, version, *blockorder, fdt);
> 
> -- 
> 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

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux