All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/3] dtc: Dynamic DT support
@ 2016-06-02 17:47 Pantelis Antoniou
       [not found] ` <1464889642-28080-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-02 17:47 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The first patch introduces a new magic number and new output/input
format options marking dynamic objects.

The next two patches enable generation of symbol & fixup
information for use with dynamic DT loading, along with the
documentation about the internal operation.

This patchset is generated against the tree and branch at
https://github.com/dgibson/dtc/tree/overlay

It is also available for a pull request at
https://github.com/pantoniou/dtc/tree/dgibson-overlay-panto

Changes since v7:
* Dropped xasprintf & backward compatibility patch
* Rebased against dgibson's overlay branch
* Minor doc wording fixes.

Changes since v6:
* Introduced xasprintf
* Added append_to_property and used it
* Changed some die()'s to assert
* Reordered node generation to respect sort
* Addressed remaining maintainer changes from v6

Changes since v5:
* Rebase to latest dtc version.
* Addressed all the maintainer requested changes from v5
* Added new magic value for dynamic objects and new format

Changes since v4:
* Rebase to latest dtc version.
* Completely redesigned the generation of resolution data.
Now instead of being generated as part of blob generation
they are created in the live tree.
* Consequently the patchset is much smaller.
* Added -A auto-label alias generation option.
* Addressed maintainer comments.
* Added syntactic sugar for overlays in the form of .dtsi
* Added /dts-v1/ /plugin/ preferred plugin form and deprecate
the previous form (although still works for backward compatibility)

Changes since v3:
* Rebase to latest dtc version.

Changes since v2:
* Split single patch to a patchset.
* Updated to dtc mainline.
* Changed __local_fixups__ format
* Clean up for better legibility.

Pantelis Antoniou (3):
  DTBO magic and dtbo format options
  dtc: Document the dynamic plugin internals
  dtc: Plugin and fixup support

 Documentation/dt-object-internal.txt | 321 +++++++++++++++++++++++++++++++++++
 Documentation/manual.txt             |  32 +++-
 checks.c                             |   8 +-
 dtc-lexer.l                          |   5 +
 dtc-parser.y                         |  33 +++-
 dtc.c                                |  37 +++-
 dtc.h                                |  35 +++-
 fdtdump.c                            |   2 +-
 flattree.c                           |  13 +-
 fstree.c                             |   2 +-
 libfdt/fdt.c                         |   2 +-
 libfdt/fdt.h                         |   3 +-
 livetree.c                           | 219 +++++++++++++++++++++++-
 tests/mangle-layout.c                |   7 +-
 treesource.c                         |   1 +
 15 files changed, 688 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/dt-object-internal.txt

-- 
1.7.12

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v8 1/3] DTBO magic and dtbo format options
       [not found] ` <1464889642-28080-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-06-02 17:47   ` Pantelis Antoniou
       [not found]     ` <1464889642-28080-2-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-06-02 17:47   ` [PATCH v8 2/3] dtc: Document the dynamic plugin internals Pantelis Antoniou
  2016-06-02 17:47   ` [PATCH v8 3/3] dtc: Plugin and fixup support Pantelis Antoniou
  2 siblings, 1 reply; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-02 17:47 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Introduce a new magic number for dynamic plugin objects,
which is enabled by selecting dtbo/input output options.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 Documentation/manual.txt |  8 ++++++++
 dtc.c                    | 14 +++++++++++---
 dtc.h                    |  4 ++--
 fdtdump.c                |  2 +-
 flattree.c               | 11 ++++++-----
 libfdt/fdt.c             |  2 +-
 libfdt/fdt.h             |  3 ++-
 tests/mangle-layout.c    |  7 ++++---
 8 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 398de32..6d2811b 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -60,6 +60,9 @@ The currently supported Input Formats are:
     - "dtb": "blob" format.  A flattened device-tree block with
         header in one binary blob.
 
+    - "dtbo" : "blob" format. Identical with "dtb" but meant
+        for use with dynamic-device tree objects.
+
     - "dts": "source" format.  A text file containing a "source"
         for a device-tree.
 
@@ -71,6 +74,8 @@ The currently supported Output Formats are:
 
      - "dtb": "blob" format
 
+     - "dtbo": "blob" format - for dynamic device tree objects/overlays
+
      - "dts": "source" format
 
      - "asm": assembly language file.  A file that can be sourced
@@ -78,6 +83,9 @@ The currently supported Output Formats are:
         then simply be added to your Makefile.  Additionally, the
         assembly file exports some symbols that can be used.
 
+     - "asmo": assembly language file for dynamic device tree objects/overlays.
+        Identical to "asm" for most purposes.
+
 
 3) Command Line
 
diff --git a/dtc.c b/dtc.c
index 1c1f88f..43ba19d 100644
--- a/dtc.c
+++ b/dtc.c
@@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
 		return "dts";
 	if (!strcasecmp(s, ".dtb"))
 		return "dtb";
+	if (!strcasecmp(s, ".dtbo"))
+		return "dtbo";
 	return fallback;
 }
 
@@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
 	magic = fdt32_to_cpu(magic);
 	if (magic == FDT_MAGIC)
 		return "dtb";
+	if (magic == FDT_MAGIC_DTBO)
+		return "dtbo";
 
 	return guess_type_by_name(fname, fallback);
 }
@@ -285,7 +289,7 @@ int main(int argc, char *argv[])
 		dti = dt_from_source(arg);
 	else if (streq(inform, "fs"))
 		dti = dt_from_fs(arg);
-	else if(streq(inform, "dtb"))
+	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
 		dti = dt_from_blob(arg);
 	else
 		die("Unknown input format \"%s\"\n", inform);
@@ -319,9 +323,13 @@ int main(int argc, char *argv[])
 	if (streq(outform, "dts")) {
 		dt_to_source(outf, dti);
 	} else if (streq(outform, "dtb")) {
-		dt_to_blob(outf, dti, outversion);
+		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
+	} else if (streq(outform, "dtbo")) {
+		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
 	} else if (streq(outform, "asm")) {
-		dt_to_asm(outf, dti, outversion);
+		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
+	} else if (streq(outform, "asmo")) {
+		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
 	} else if (streq(outform, "null")) {
 		/* do nothing */
 	} else {
diff --git a/dtc.h b/dtc.h
index 040f8d4..fa66dca 100644
--- a/dtc.h
+++ b/dtc.h
@@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
 
 /* Flattened trees */
 
-void dt_to_blob(FILE *f, struct dt_info *dti, int version);
-void dt_to_asm(FILE *f, struct dt_info *dti, int version);
+void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
+void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
 
 struct dt_info *dt_from_blob(const char *fname);
 
diff --git a/fdtdump.c b/fdtdump.c
index 95a6a20..fa5a050 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -199,7 +199,7 @@ int main(int argc, char *argv[])
 			p = memchr(p, smagic[0], endp - p - 4);
 			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 45fcb04..59bdc7d 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 dt_info *dti, int version)
+void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
 	reservebuf = flatten_reserve_list(dti->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,
 			dti->boot_cpuid_phys);
 
 	/*
@@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
 	}
 }
 
-void dt_to_asm(FILE *f, struct dt_info *dti, int version)
+void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -832,7 +833,7 @@ struct dt_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");
 
 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
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/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);
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v8 2/3] dtc: Document the dynamic plugin internals
       [not found] ` <1464889642-28080-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-06-02 17:47   ` [PATCH v8 1/3] DTBO magic and dtbo format options Pantelis Antoniou
@ 2016-06-02 17:47   ` Pantelis Antoniou
  2016-06-02 17:47   ` [PATCH v8 3/3] dtc: Plugin and fixup support Pantelis Antoniou
  2 siblings, 0 replies; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-02 17:47 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Provides the document explaining the internal mechanics of
plugins and options.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 Documentation/dt-object-internal.txt | 321 +++++++++++++++++++++++++++++++++++
 1 file changed, 321 insertions(+)
 create mode 100644 Documentation/dt-object-internal.txt

diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
new file mode 100644
index 0000000..3c22fae
--- /dev/null
+++ b/Documentation/dt-object-internal.txt
@@ -0,0 +1,321 @@
+Device Tree Dynamic Object format internals
+-------------------------------------------
+
+The Device Tree for most platforms is a static representation of
+the hardware capabilities. This is insufficient for many platforms
+that need to dynamically insert device tree fragments to the
+running kernel's live tree.
+
+This document explains the the device tree object format and the
+modifications made to the device tree compiler, which make it possible.
+
+1. Simplified Problem Definition
+--------------------------------
+
+Assume we have a platform which boots using following simplified device tree.
+
+---- foo.dts -----------------------------------------------------------------
+	/* FOO platform */
+	/ {
+		compatible = "corp,foo";
+
+		/* shared resources */
+		res: res {
+		};
+
+		/* On chip peripherals */
+		ocp: ocp {
+			/* peripherals that are always instantiated */
+			peripheral1 { ... };
+		};
+	};
+---- foo.dts -----------------------------------------------------------------
+
+We have a number of peripherals that after probing (using some undefined method)
+should result in different device tree configuration.
+
+We cannot boot with this static tree because due to the configuration of the
+foo platform there exist multiple conficting peripherals DT fragments.
+
+So for the bar peripheral we would have this:
+
+---- foo+bar.dts -------------------------------------------------------------
+	/* FOO platform + bar peripheral */
+	/ {
+		compatible = "corp,foo";
+
+		/* shared resources */
+		res: res {
+		};
+
+		/* On chip peripherals */
+		ocp: ocp {
+			/* peripherals that are always instantiated */
+			peripheral1 { ... };
+
+			/* bar peripheral */
+			bar {
+				compatible = "corp,bar";
+				... /* various properties and child nodes */
+			};
+		};
+	};
+---- foo+bar.dts -------------------------------------------------------------
+
+While for the baz peripheral we would have this:
+
+---- foo+baz.dts -------------------------------------------------------------
+	/* FOO platform + baz peripheral */
+	/ {
+		compatible = "corp,foo";
+
+		/* shared resources */
+		res: res {
+			/* baz resources */
+			baz_res: res_baz { ... };
+		};
+
+		/* On chip peripherals */
+		ocp: ocp {
+			/* peripherals that are always instantiated */
+			peripheral1 { ... };
+
+			/* baz peripheral */
+			baz {
+				compatible = "corp,baz";
+				/* reference to another point in the tree */
+				ref-to-res = <&baz_res>;
+				... /* various properties and child nodes */
+			};
+		};
+	};
+---- foo+baz.dts -------------------------------------------------------------
+
+We note that the baz case is more complicated, since the baz peripheral needs to
+reference another node in the DT tree.
+
+2. Device Tree Object Format Requirements
+-----------------------------------------
+
+Since the device tree is used for booting a number of very different hardware
+platforms it is imperative that we tread very carefully.
+
+2.a) No changes to the Device Tree binary format for the base tree. We cannot
+modify the tree format at all and all the information we require should be
+encoded using device tree itself. We can add nodes that can be safely ignored
+by both bootloaders and the kernel. The plugin dtb's are optionally tagged
+with a different magic number in the header but otherwise they too are simple
+blobs.
+
+2.b) Changes to the DTS source format should be absolutely minimal, and should
+only be needed for the DT fragment definitions, and not the base boot DT.
+
+2.c) An explicit option should be used to instruct DTC to generate the required
+information needed for object resolution. Platforms that don't use the
+dynamic object format can safely ignore it.
+
+2.d) Finally, DT syntax changes should be kept to a minimum. It should be
+possible to express everything using the existing DT syntax.
+
+3. Implementation
+-----------------
+
+The basic unit of addressing in Device Tree is the phandle. Turns out it's
+relatively simple to extend the way phandles are generated and referenced
+so that it's possible to dynamically convert symbolic references (labels)
+to phandle values. This is a valid assumption as long as the author uses
+reference syntax and does not assign phandle values manually (which might
+be a problem with decompiled source files).
+
+We can roughly divide the operation into two steps.
+
+3.a) Compilation of the base board DTS file using the '-@' option
+generates a valid DT blob with an added __symbols__ node at the root node,
+containing a list of all nodes that are marked with a label.
+
+Using the foo.dts file above the following node will be generated;
+
+$ dtc -@ -O dtb -o foo.dtb -b 0 foo.dts
+$ fdtdump foo.dtb
+...
+/ {
+	...
+	res {
+		...
+		phandle = <0x00000001>;
+		...
+	};
+	ocp {
+		...
+		phandle = <0x00000002>;
+		...
+	};
+	__symbols__ {
+		res="/res";
+		ocp="/ocp";
+	};
+};
+
+Notice that all the nodes that had a label have been recorded, and that
+phandles have been generated for them.
+
+This blob can be used to boot the board normally, the __symbols__ node will
+be safely ignored both by the bootloader and the kernel (the only loss will
+be a few bytes of memory and disk space).
+
+3.b) The Device Tree fragments must be compiled with the same option but they
+must also have a tag (/plugin/) that allows undefined references to nodes
+that are not present at compilation time to be recorded so that the runtime
+loader can fix them.
+
+So the bar peripheral's DTS format would be of the form:
+
+/dts-v1/ /plugin/;	/* allow undefined references and record them */
+/ {
+	....	/* various properties for loader use; i.e. part id etc. */
+	fragment@0 {
+		target = <&ocp>;
+		__overlay__ {
+			/* bar peripheral */
+			bar {
+				compatible = "corp,bar";
+				... /* various properties and child nodes */
+			}
+		};
+	};
+};
+
+Note that there's a target property that specifies the location where the
+contents of the overlay node will be placed, and it references the node
+in the foo.dts file.
+
+$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
+$ fdtdump bar.dtbo
+...
+/ {
+	... /* properties */
+	fragment@0 {
+		target = <0xffffffff>;
+		__overlay__ {
+			bar {
+				compatible = "corp,bar";
+				... /* various properties and child nodes */
+			}
+		};
+	};
+	__fixups__ {
+	    ocp = "/fragment@0:target:0";
+	};
+};
+
+No __symbols__ has been generated (no label in bar.dts).
+Note that the target's ocp label is undefined, so the phandle handle
+value is filled with the illegal value '0xffffffff', while a __fixups__
+node has been generated, which marks the location in the tree where
+the label lookup should store the runtime phandle value of the ocp node.
+
+The format of the __fixups__ node entry is
+
+	<label> = "<local-full-path>:<property-name>:<offset>";
+
+<label> 		Is the label we're referring
+<local-full-path>	Is the full path of the node the reference is
+<property-name>		Is the name of the property containing the
+			reference
+<offset>		The offset (in bytes) of where the property's
+			phandle value is located.
+
+Doing the same with the baz peripheral's DTS format is a little bit more
+involved, since baz contains references to local labels which require
+local fixups.
+
+/dts-v1/ /plugin/;	/* allow undefined label references and record them */
+/ {
+	....	/* various properties for loader use; i.e. part id etc. */
+	fragment@0 {
+		target = <&res>;
+		__overlay__ {
+			/* baz resources */
+			baz_res: res_baz { ... };
+		};
+	};
+	fragment@1 {
+		target = <&ocp>;
+		__overlay__ {
+			/* baz peripheral */
+			baz {
+				compatible = "corp,baz";
+				/* reference to another point in the tree */
+				ref-to-res = <&baz_res>;
+				... /* various properties and child nodes */
+			}
+		};
+	};
+};
+
+Note that &bar_res reference.
+
+$ dtc -@ -O dtb -o baz.dtbo -b 0 baz.dts
+$ fdtdump baz.dtbo
+...
+/ {
+	... /* properties */
+	fragment@0 {
+		target = <0xffffffff>;
+		__overlay__ {
+			res_baz {
+				....
+				phandle = <0x00000001>;
+			};
+		};
+	};
+	fragment@1 {
+		target = <0xffffffff>;
+		__overlay__ {
+			baz {
+				compatible = "corp,baz";
+				... /* various properties and child nodes */
+				ref-to-res = <0x00000001>;
+			}
+		};
+	};
+	__fixups__ {
+		res = "/fragment@0:target:0";
+		ocp = "/fragment@1:target:0";
+	};
+	__local_fixups__ {
+		fragment@1 {
+			__overlay__ {
+				baz {
+					ref-to-res = <0>;
+				};
+			};
+		};
+	};
+};
+
+This is similar to the bar case, but the reference of a local label by the
+baz node generates a __local_fixups__ entry that records the place that the
+local reference is being made. No matter how phandles are allocated from dtc
+the run time loader must apply an offset to each phandle in every dynamic
+DT object loaded. The __local_fixups__ node records the place of every
+local reference so that the loader can apply the offset.
+
+There is an alternative syntax to the expanded form for overlays with phandle
+targets which makes the format similar to the one using in .dtsi include files.
+
+So for the &ocp target example above one can simply write:
+
+/dts-v1/ /plugin/;
+
+/ { };	/* required for parsing */
+
+&ocp {
+	/* bar peripheral */
+	bar {
+		compatible = "corp,bar";
+		... /* various properties and child nodes */
+	}
+};
+
+The resulting dtb object is identical.
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v8 3/3] dtc: Plugin and fixup support
       [not found] ` <1464889642-28080-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-06-02 17:47   ` [PATCH v8 1/3] DTBO magic and dtbo format options Pantelis Antoniou
  2016-06-02 17:47   ` [PATCH v8 2/3] dtc: Document the dynamic plugin internals Pantelis Antoniou
@ 2016-06-02 17:47   ` Pantelis Antoniou
       [not found]     ` <1464889642-28080-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2 siblings, 1 reply; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-02 17:47 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/manual.txt |  24 +++++-
 checks.c                 |   8 +-
 dtc-lexer.l              |   5 ++
 dtc-parser.y             |  33 ++++++-
 dtc.c                    |  23 ++++-
 dtc.h                    |  31 ++++++-
 flattree.c               |   2 +-
 fstree.c                 |   2 +-
 livetree.c               | 219 ++++++++++++++++++++++++++++++++++++++++++++++-
 treesource.c             |   1 +
 10 files changed, 332 insertions(+), 16 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 6d2811b..deffcdb 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -127,6 +127,20 @@ 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.
+
     -S <bytes>
 	Ensure the blob at least <bytes> long, adding additional
 	space if needed.
@@ -154,12 +168,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 basetree overlays
+
+    versioninfo:  "/dts-v1/"
+
+    plugindecl:   "/plugin/"
 
     memreserve:   label 'memreserve' ADDR ADDR ';'
 		| label 'memreserve' ADDR '-' ADDR ';'
 
-    devicetree:   '/' nodedef
+    basetree:   '/' nodedef
+
+    overlays:   list_of_overlay_definitions (at compile time)
 
     nodedef:      '{' list_of_property list_of_subnode '}' ';'
 
diff --git a/checks.c b/checks.c
index 1829f0e..355bef9 100644
--- a/checks.c
+++ b/checks.c
@@ -482,8 +482,12 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
 
 			refnode = get_node_by_ref(dt, m->ref);
 			if (! refnode) {
-				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
-				     m->ref);
+				if (!(tree_get_versionflags(dt) & 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 e0d7212..bb89631 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 dt_info *parser_output;
 extern bool treesource_error;
+extern unsigned int the_versionflags;
 %}
 
 %union {
@@ -54,9 +56,11 @@ extern bool treesource_error;
 	struct overlay *overlaylist;
 	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
@@ -73,6 +77,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
@@ -106,16 +112,37 @@ extern bool treesource_error;
 %%
 
 sourcefile:
-	  v1tag memreserves basetree overlays
+	  versioninfo plugindecl memreserves basetree overlays
 		{
-			parser_output = build_dt_info($2, $3, $4,
-						      guess_boot_cpuid($3));
+			assert(($1 | $2) == the_versionflags);
+			parser_output = build_dt_info($1 | $2, $3, $4, $5,
+							guess_boot_cpuid($4));
+		}
+	;
+
+versioninfo:
+	v1tag
+		{
+			the_versionflags |= VF_DT_V1;
+			$$ = VF_DT_V1;
 		}
 	;
 
 v1tag:
 	  DT_V1 ';'
 	| DT_V1 ';' v1tag
+	| DT_V1
+
+plugindecl:
+	DT_PLUGIN ';'
+		{
+			the_versionflags |= VF_PLUGIN;
+			$$ = the_versionflags;
+		}
+	| /* empty */
+		{
+			$$ = 0;
+		}
 	;
 
 memreserves:
diff --git a/dtc.c b/dtc.c
index 43ba19d..8b716ad 100644
--- a/dtc.c
+++ b/dtc.c
@@ -31,6 +31,8 @@ int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
+int symbol_fixup_support;
+int auto_label_aliases;
 
 static void fill_fullpaths(struct node *tree, const char *prefix)
 {
@@ -63,7 +65,7 @@ static void resolve_overlays(struct dt_info *dti)
 #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:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:@Ahv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},
@@ -81,6 +83,8 @@ 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'},
 	{"help",             no_argument, NULL, 'h'},
 	{"version",          no_argument, NULL, 'v'},
 	{NULL,               no_argument, NULL, 0x0},
@@ -111,6 +115,8 @@ 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\tPrint this help and exit",
 	"\n\tPrint version and exit",
 	NULL,
@@ -247,7 +253,12 @@ int main(int argc, char *argv[])
 		case 'E':
 			parse_checks_option(false, true, optarg);
 			break;
-
+		case '@':
+			symbol_fixup_support = 1;
+			break;
+		case 'A':
+			auto_label_aliases = 1;
+			break;
 		case 'h':
 			usage(NULL);
 		default:
@@ -308,6 +319,14 @@ int main(int argc, char *argv[])
 
 	process_checks(force, dti);
 
+	if (auto_label_aliases)
+		generate_label_tree(dti->dt, "aliases", false);
+
+	if (symbol_fixup_support) {
+		generate_label_tree(dti->dt, "__symbols__", true);
+		generate_fixups_tree(dti->dt);
+	}
+
 	if (sort)
 		sort_tree(dti);
 
diff --git a/dtc.h b/dtc.h
index fa66dca..a9fac44 100644
--- a/dtc.h
+++ b/dtc.h
@@ -54,6 +54,12 @@ extern int reservenum;		/* Number of memory reservation slots */
 extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
 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 */
+
+/*
+ * Tree source globals
+ */
 
 #define PHANDLE_LEGACY	0x1
 #define PHANDLE_EPAPR	0x2
@@ -142,6 +148,8 @@ struct property {
 	struct label *labels;
 };
 
+struct dt_info;
+
 struct node {
 	bool deleted;
 	char *name;
@@ -158,6 +166,9 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	/* only for the root (parent == NULL) */
+	struct dt_info *dti;
 };
 
 struct overlay {
@@ -200,6 +211,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);
@@ -207,6 +219,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);
+struct property *append_to_property(struct node *node,
+				    char *name, const void *data, int len);
 
 struct overlay *build_overlay(char *target, struct node *dt);
 struct overlay *chain_overlay(struct overlay *first, struct overlay *list);
@@ -246,6 +260,7 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 
 
 struct dt_info {
+	unsigned int versionflags;
 	struct reserve_info *reservelist;
 	uint32_t boot_cpuid_phys;
 
@@ -253,11 +268,25 @@ struct dt_info {
 	struct overlay *overlays;
 };
 
-struct dt_info *build_dt_info(struct reserve_info *reservelist,
+/* version flags definitions */
+#define VF_DT_V1	0x0001	/* /dts-v1/ */
+#define VF_PLUGIN	0x0002	/* /plugin/ */
+
+static inline unsigned int tree_get_versionflags(struct node *dt)
+{
+	if (!dt || !dt->dti)
+		return 0;
+	return dt->dti->versionflags;
+}
+
+struct dt_info *build_dt_info(unsigned int versionflags,
+			      struct reserve_info *reservelist,
 			      struct node *basetree,
 			      struct overlay *overlays,
 			      uint32_t boot_cpuid_phys);
 void sort_tree(struct dt_info *dti);
+void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph);
+void generate_fixups_tree(struct node *dt);
 
 /* Checks */
 
diff --git a/flattree.c b/flattree.c
index 59bdc7d..b674cbf 100644
--- a/flattree.c
+++ b/flattree.c
@@ -930,5 +930,5 @@ struct dt_info *dt_from_blob(const char *fname)
 
 	fclose(f);
 
-	return build_dt_info(reservelist, tree, NULL, boot_cpuid_phys);
+	return build_dt_info(VF_DT_V1, reservelist, tree, NULL, boot_cpuid_phys);
 }
diff --git a/fstree.c b/fstree.c
index 0ef96a8..4f1a69d 100644
--- a/fstree.c
+++ b/fstree.c
@@ -86,5 +86,5 @@ struct dt_info *dt_from_fs(const char *dirname)
 	tree = read_fstree(dirname);
 	tree = name_node(tree, "");
 
-	return build_dt_info(NULL, tree, NULL, guess_boot_cpuid(tree));
+	return build_dt_info(VF_DT_V1, NULL, tree, NULL, guess_boot_cpuid(tree));
 }
diff --git a/livetree.c b/livetree.c
index 9a44214..10a81eb 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 = xmalloc(sizeof(*node));
+	struct property *p;
+	struct data d = empty_data;
+	char *name;
+
+	memset(node, 0, sizeof(*node));
+
+	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__");
+
+	add_child(dt, node);
+	add_child(node, new_node);
+}
+
 struct node *chain_node(struct node *first, struct node *list)
 {
 	assert(first->next_sibling == NULL);
@@ -318,9 +343,14 @@ void apply_overlay(struct node *base, struct overlay *overlay)
 {
 	struct node *target = get_node_by_ref(base, overlay->target);
 
-	if (!target)
-		die("Couldn't find label or path %s for overlay\n",
-		    overlay->target);
+	if (!target) {
+		if (base->dti->versionflags & VF_PLUGIN)
+			add_orphan_node(base, overlay->dt, overlay->target);
+		else
+			die("Couldn't find label or path %s for overlay\n",
+				overlay->target);
+		return;
+	}
 
 	if (!overlay->dt)
 		/* Deletion */
@@ -329,6 +359,24 @@ void apply_overlay(struct node *base, struct overlay *overlay)
 		merge_nodes(target, overlay->dt);
 }
 
+struct property *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);
+	}
+	return p;
+}
+
 struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
 {
 	struct reserve_info *new = xmalloc(sizeof(*new));
@@ -368,7 +416,8 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 	return list;
 }
 
-struct dt_info *build_dt_info(struct reserve_info *reservelist,
+struct dt_info *build_dt_info(unsigned int versionflags,
+			      struct reserve_info *reservelist,
 			      struct node *basetree,
 			      struct overlay *overlays,
 			      uint32_t boot_cpuid_phys)
@@ -376,10 +425,12 @@ struct dt_info *build_dt_info(struct reserve_info *reservelist,
 	struct dt_info *dti;
 
 	dti = xmalloc(sizeof(*dti));
+	dti->versionflags = versionflags;
 	dti->reservelist = reservelist;
 	dti->boot_cpuid_phys = boot_cpuid_phys;
 	dti->dt = basetree;
 	dti->overlays = overlays;
+	basetree->dti = dti;
 
 	return dti;
 }
@@ -745,3 +796,163 @@ void sort_tree(struct dt_info *dti)
 	sort_reserve_entries(dti);
 	sort_node(dti->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_escape_string(node->fullpath,
+						strlen(node->fullpath)));
+			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;
+
+	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);
+}
+
+static char *fixups_name = "__fixups__";
+static char *local_fixups_name = "__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_name);
+	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_name);
+	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';
+
+		/* 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)
+{
+	build_and_name_child_node(dt, fixups_name);
+	build_and_name_child_node(dt, local_fixups_name);
+	generate_fixups_tree_internal(dt, dt);
+}
diff --git a/treesource.c b/treesource.c
index c9d8967..0c84f02 100644
--- a/treesource.c
+++ b/treesource.c
@@ -27,6 +27,7 @@ extern YYLTYPE yylloc;
 
 struct dt_info *parser_output;
 bool treesource_error;
+unsigned int the_versionflags;
 
 struct dt_info *dt_from_source(const char *fname)
 {
-- 
1.7.12

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 1/3] DTBO magic and dtbo format options
       [not found]     ` <1464889642-28080-2-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-06-08  6:51       ` David Gibson
       [not found]         ` <20160608065100.GA9226-RXTfZT5YzpxwFLYp8hBm2A@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: David Gibson @ 2016-06-08  6:51 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 9542 bytes --]

On Thu, Jun 02, 2016 at 08:47:20PM +0300, Pantelis Antoniou wrote:
> Introduce a new magic number for dynamic plugin objects,
> which is enabled by selecting dtbo/input output options.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

So, I don't think we should need an explicit -I dtbo or -O dtbo.
Instead we should be able to autoselect the right magic number based
on the presence of the /plugin/ flag.  In the other direction we
should be able to set the equivalent of the /plugin/ flag based on the
magic number.

> ---
>  Documentation/manual.txt |  8 ++++++++
>  dtc.c                    | 14 +++++++++++---
>  dtc.h                    |  4 ++--
>  fdtdump.c                |  2 +-
>  flattree.c               | 11 ++++++-----
>  libfdt/fdt.c             |  2 +-
>  libfdt/fdt.h             |  3 ++-
>  tests/mangle-layout.c    |  7 ++++---
>  8 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 398de32..6d2811b 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -60,6 +60,9 @@ The currently supported Input Formats are:
>      - "dtb": "blob" format.  A flattened device-tree block with
>          header in one binary blob.
>  
> +    - "dtbo" : "blob" format. Identical with "dtb" but meant
> +        for use with dynamic-device tree objects.
> +
>      - "dts": "source" format.  A text file containing a "source"
>          for a device-tree.
>  
> @@ -71,6 +74,8 @@ The currently supported Output Formats are:
>  
>       - "dtb": "blob" format
>  
> +     - "dtbo": "blob" format - for dynamic device tree objects/overlays
> +
>       - "dts": "source" format
>  
>       - "asm": assembly language file.  A file that can be sourced
> @@ -78,6 +83,9 @@ The currently supported Output Formats are:
>          then simply be added to your Makefile.  Additionally, the
>          assembly file exports some symbols that can be used.
>  
> +     - "asmo": assembly language file for dynamic device tree objects/overlays.
> +        Identical to "asm" for most purposes.
> +
>  
>  3) Command Line
>  
> diff --git a/dtc.c b/dtc.c
> index 1c1f88f..43ba19d 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
>  		return "dts";
>  	if (!strcasecmp(s, ".dtb"))
>  		return "dtb";
> +	if (!strcasecmp(s, ".dtbo"))
> +		return "dtbo";
>  	return fallback;
>  }
>  
> @@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>  	magic = fdt32_to_cpu(magic);
>  	if (magic == FDT_MAGIC)
>  		return "dtb";
> +	if (magic == FDT_MAGIC_DTBO)
> +		return "dtbo";
>  
>  	return guess_type_by_name(fname, fallback);
>  }
> @@ -285,7 +289,7 @@ int main(int argc, char *argv[])
>  		dti = dt_from_source(arg);
>  	else if (streq(inform, "fs"))
>  		dti = dt_from_fs(arg);
> -	else if(streq(inform, "dtb"))
> +	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
>  		dti = dt_from_blob(arg);
>  	else
>  		die("Unknown input format \"%s\"\n", inform);
> @@ -319,9 +323,13 @@ int main(int argc, char *argv[])
>  	if (streq(outform, "dts")) {
>  		dt_to_source(outf, dti);
>  	} else if (streq(outform, "dtb")) {
> -		dt_to_blob(outf, dti, outversion);
> +		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
> +	} else if (streq(outform, "dtbo")) {
> +		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
>  	} else if (streq(outform, "asm")) {
> -		dt_to_asm(outf, dti, outversion);
> +		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
> +	} else if (streq(outform, "asmo")) {
> +		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
>  	} else if (streq(outform, "null")) {
>  		/* do nothing */
>  	} else {
> diff --git a/dtc.h b/dtc.h
> index 040f8d4..fa66dca 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
>  
>  /* Flattened trees */
>  
> -void dt_to_blob(FILE *f, struct dt_info *dti, int version);
> -void dt_to_asm(FILE *f, struct dt_info *dti, int version);
> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
>  
>  struct dt_info *dt_from_blob(const char *fname);
>  
> diff --git a/fdtdump.c b/fdtdump.c
> index 95a6a20..fa5a050 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
>  			p = memchr(p, smagic[0], endp - p - 4);
>  			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 45fcb04..59bdc7d 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 dt_info *dti, int version)
> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
>  	reservebuf = flatten_reserve_list(dti->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,
>  			dti->boot_cpuid_phys);
>  
>  	/*
> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>  	}
>  }
>  
> -void dt_to_asm(FILE *f, struct dt_info *dti, int version)
> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -832,7 +833,7 @@ struct dt_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");
>  
>  	rc = fread(&totalsize, sizeof(totalsize), 1, f);
> 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/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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 1/3] DTBO magic and dtbo format options
       [not found]         ` <20160608065100.GA9226-RXTfZT5YzpxwFLYp8hBm2A@public.gmane.org>
@ 2016-06-08  8:30             ` Pantelis Antoniou
  0 siblings, 0 replies; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-08  8:30 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA


> On Jun 8, 2016, at 09:51 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Thu, Jun 02, 2016 at 08:47:20PM +0300, Pantelis Antoniou wrote:
>> Introduce a new magic number for dynamic plugin objects,
>> which is enabled by selecting dtbo/input output options.
>> 
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> 
> So, I don't think we should need an explicit -I dtbo or -O dtbo.
> Instead we should be able to autoselect the right magic number based
> on the presence of the /plugin/ flag.  In the other direction we
> should be able to set the equivalent of the /plugin/ flag based on the
> magic number.
> 

Sure but this will break everything since nothing can grok the different
magic number. Not the kernel unflattener nor any bootloader.

We need an option to keep the old magic number for compatibility.

>> ---
>> Documentation/manual.txt |  8 ++++++++
>> dtc.c                    | 14 +++++++++++---
>> dtc.h                    |  4 ++--
>> fdtdump.c                |  2 +-
>> flattree.c               | 11 ++++++-----
>> libfdt/fdt.c             |  2 +-
>> libfdt/fdt.h             |  3 ++-
>> tests/mangle-layout.c    |  7 ++++---
>> 8 files changed, 35 insertions(+), 16 deletions(-)
>> 
>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>> index 398de32..6d2811b 100644
>> --- a/Documentation/manual.txt
>> +++ b/Documentation/manual.txt
>> @@ -60,6 +60,9 @@ The currently supported Input Formats are:
>>     - "dtb": "blob" format.  A flattened device-tree block with
>>         header in one binary blob.
>> 
>> +    - "dtbo" : "blob" format. Identical with "dtb" but meant
>> +        for use with dynamic-device tree objects.
>> +
>>     - "dts": "source" format.  A text file containing a "source"
>>         for a device-tree.
>> 
>> @@ -71,6 +74,8 @@ The currently supported Output Formats are:
>> 
>>      - "dtb": "blob" format
>> 
>> +     - "dtbo": "blob" format - for dynamic device tree objects/overlays
>> +
>>      - "dts": "source" format
>> 
>>      - "asm": assembly language file.  A file that can be sourced
>> @@ -78,6 +83,9 @@ The currently supported Output Formats are:
>>         then simply be added to your Makefile.  Additionally, the
>>         assembly file exports some symbols that can be used.
>> 
>> +     - "asmo": assembly language file for dynamic device tree objects/overlays.
>> +        Identical to "asm" for most purposes.
>> +
>> 
>> 3) Command Line
>> 
>> diff --git a/dtc.c b/dtc.c
>> index 1c1f88f..43ba19d 100644
>> --- a/dtc.c
>> +++ b/dtc.c
>> @@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
>> 		return "dts";
>> 	if (!strcasecmp(s, ".dtb"))
>> 		return "dtb";
>> +	if (!strcasecmp(s, ".dtbo"))
>> +		return "dtbo";
>> 	return fallback;
>> }
>> 
>> @@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>> 	magic = fdt32_to_cpu(magic);
>> 	if (magic == FDT_MAGIC)
>> 		return "dtb";
>> +	if (magic == FDT_MAGIC_DTBO)
>> +		return "dtbo";
>> 
>> 	return guess_type_by_name(fname, fallback);
>> }
>> @@ -285,7 +289,7 @@ int main(int argc, char *argv[])
>> 		dti = dt_from_source(arg);
>> 	else if (streq(inform, "fs"))
>> 		dti = dt_from_fs(arg);
>> -	else if(streq(inform, "dtb"))
>> +	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
>> 		dti = dt_from_blob(arg);
>> 	else
>> 		die("Unknown input format \"%s\"\n", inform);
>> @@ -319,9 +323,13 @@ int main(int argc, char *argv[])
>> 	if (streq(outform, "dts")) {
>> 		dt_to_source(outf, dti);
>> 	} else if (streq(outform, "dtb")) {
>> -		dt_to_blob(outf, dti, outversion);
>> +		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
>> +	} else if (streq(outform, "dtbo")) {
>> +		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
>> 	} else if (streq(outform, "asm")) {
>> -		dt_to_asm(outf, dti, outversion);
>> +		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
>> +	} else if (streq(outform, "asmo")) {
>> +		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
>> 	} else if (streq(outform, "null")) {
>> 		/* do nothing */
>> 	} else {
>> diff --git a/dtc.h b/dtc.h
>> index 040f8d4..fa66dca 100644
>> --- a/dtc.h
>> +++ b/dtc.h
>> @@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
>> 
>> /* Flattened trees */
>> 
>> -void dt_to_blob(FILE *f, struct dt_info *dti, int version);
>> -void dt_to_asm(FILE *f, struct dt_info *dti, int version);
>> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
>> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
>> 
>> struct dt_info *dt_from_blob(const char *fname);
>> 
>> diff --git a/fdtdump.c b/fdtdump.c
>> index 95a6a20..fa5a050 100644
>> --- a/fdtdump.c
>> +++ b/fdtdump.c
>> @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
>> 			p = memchr(p, smagic[0], endp - p - 4);
>> 			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 45fcb04..59bdc7d 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 dt_info *dti, int version)
>> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
>> 	reservebuf = flatten_reserve_list(dti->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,
>> 			dti->boot_cpuid_phys);
>> 
>> 	/*
>> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>> 	}
>> }
>> 
>> -void dt_to_asm(FILE *f, struct dt_info *dti, int version)
>> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -832,7 +833,7 @@ struct dt_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");
>> 
>> 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
>> 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/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);
> 

Regards

— Pantelis

> -- 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 1/3] DTBO magic and dtbo format options
@ 2016-06-08  8:30             ` Pantelis Antoniou
  0 siblings, 0 replies; 9+ messages in thread
From: Pantelis Antoniou @ 2016-06-08  8:30 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA


> On Jun 8, 2016, at 09:51 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Thu, Jun 02, 2016 at 08:47:20PM +0300, Pantelis Antoniou wrote:
>> Introduce a new magic number for dynamic plugin objects,
>> which is enabled by selecting dtbo/input output options.
>> 
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> 
> So, I don't think we should need an explicit -I dtbo or -O dtbo.
> Instead we should be able to autoselect the right magic number based
> on the presence of the /plugin/ flag.  In the other direction we
> should be able to set the equivalent of the /plugin/ flag based on the
> magic number.
> 

Sure but this will break everything since nothing can grok the different
magic number. Not the kernel unflattener nor any bootloader.

We need an option to keep the old magic number for compatibility.

>> ---
>> Documentation/manual.txt |  8 ++++++++
>> dtc.c                    | 14 +++++++++++---
>> dtc.h                    |  4 ++--
>> fdtdump.c                |  2 +-
>> flattree.c               | 11 ++++++-----
>> libfdt/fdt.c             |  2 +-
>> libfdt/fdt.h             |  3 ++-
>> tests/mangle-layout.c    |  7 ++++---
>> 8 files changed, 35 insertions(+), 16 deletions(-)
>> 
>> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
>> index 398de32..6d2811b 100644
>> --- a/Documentation/manual.txt
>> +++ b/Documentation/manual.txt
>> @@ -60,6 +60,9 @@ The currently supported Input Formats are:
>>     - "dtb": "blob" format.  A flattened device-tree block with
>>         header in one binary blob.
>> 
>> +    - "dtbo" : "blob" format. Identical with "dtb" but meant
>> +        for use with dynamic-device tree objects.
>> +
>>     - "dts": "source" format.  A text file containing a "source"
>>         for a device-tree.
>> 
>> @@ -71,6 +74,8 @@ The currently supported Output Formats are:
>> 
>>      - "dtb": "blob" format
>> 
>> +     - "dtbo": "blob" format - for dynamic device tree objects/overlays
>> +
>>      - "dts": "source" format
>> 
>>      - "asm": assembly language file.  A file that can be sourced
>> @@ -78,6 +83,9 @@ The currently supported Output Formats are:
>>         then simply be added to your Makefile.  Additionally, the
>>         assembly file exports some symbols that can be used.
>> 
>> +     - "asmo": assembly language file for dynamic device tree objects/overlays.
>> +        Identical to "asm" for most purposes.
>> +
>> 
>> 3) Command Line
>> 
>> diff --git a/dtc.c b/dtc.c
>> index 1c1f88f..43ba19d 100644
>> --- a/dtc.c
>> +++ b/dtc.c
>> @@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
>> 		return "dts";
>> 	if (!strcasecmp(s, ".dtb"))
>> 		return "dtb";
>> +	if (!strcasecmp(s, ".dtbo"))
>> +		return "dtbo";
>> 	return fallback;
>> }
>> 
>> @@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>> 	magic = fdt32_to_cpu(magic);
>> 	if (magic == FDT_MAGIC)
>> 		return "dtb";
>> +	if (magic == FDT_MAGIC_DTBO)
>> +		return "dtbo";
>> 
>> 	return guess_type_by_name(fname, fallback);
>> }
>> @@ -285,7 +289,7 @@ int main(int argc, char *argv[])
>> 		dti = dt_from_source(arg);
>> 	else if (streq(inform, "fs"))
>> 		dti = dt_from_fs(arg);
>> -	else if(streq(inform, "dtb"))
>> +	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
>> 		dti = dt_from_blob(arg);
>> 	else
>> 		die("Unknown input format \"%s\"\n", inform);
>> @@ -319,9 +323,13 @@ int main(int argc, char *argv[])
>> 	if (streq(outform, "dts")) {
>> 		dt_to_source(outf, dti);
>> 	} else if (streq(outform, "dtb")) {
>> -		dt_to_blob(outf, dti, outversion);
>> +		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
>> +	} else if (streq(outform, "dtbo")) {
>> +		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
>> 	} else if (streq(outform, "asm")) {
>> -		dt_to_asm(outf, dti, outversion);
>> +		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
>> +	} else if (streq(outform, "asmo")) {
>> +		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
>> 	} else if (streq(outform, "null")) {
>> 		/* do nothing */
>> 	} else {
>> diff --git a/dtc.h b/dtc.h
>> index 040f8d4..fa66dca 100644
>> --- a/dtc.h
>> +++ b/dtc.h
>> @@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
>> 
>> /* Flattened trees */
>> 
>> -void dt_to_blob(FILE *f, struct dt_info *dti, int version);
>> -void dt_to_asm(FILE *f, struct dt_info *dti, int version);
>> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
>> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
>> 
>> struct dt_info *dt_from_blob(const char *fname);
>> 
>> diff --git a/fdtdump.c b/fdtdump.c
>> index 95a6a20..fa5a050 100644
>> --- a/fdtdump.c
>> +++ b/fdtdump.c
>> @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
>> 			p = memchr(p, smagic[0], endp - p - 4);
>> 			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 45fcb04..59bdc7d 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 dt_info *dti, int version)
>> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
>> 	reservebuf = flatten_reserve_list(dti->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,
>> 			dti->boot_cpuid_phys);
>> 
>> 	/*
>> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>> 	}
>> }
>> 
>> -void dt_to_asm(FILE *f, struct dt_info *dti, int version)
>> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
>> {
>> 	struct version_info *vi = NULL;
>> 	int i;
>> @@ -832,7 +833,7 @@ struct dt_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");
>> 
>> 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
>> 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/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);
> 

Regards

— Pantelis

> -- 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 1/3] DTBO magic and dtbo format options
       [not found]             ` <91D401B5-5C8F-4AEB-918C-1BF0BCCCC9D8-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-06-09  4:19               ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2016-06-09  4:19 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 10786 bytes --]

On Wed, Jun 08, 2016 at 11:30:43AM +0300, Pantelis Antoniou wrote:
> 
> > On Jun 8, 2016, at 09:51 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Thu, Jun 02, 2016 at 08:47:20PM +0300, Pantelis Antoniou wrote:
> >> Introduce a new magic number for dynamic plugin objects,
> >> which is enabled by selecting dtbo/input output options.
> >> 
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > 
> > So, I don't think we should need an explicit -I dtbo or -O dtbo.
> > Instead we should be able to autoselect the right magic number based
> > on the presence of the /plugin/ flag.  In the other direction we
> > should be able to set the equivalent of the /plugin/ flag based on the
> > magic number.
> 
> Sure but this will break everything since nothing can grok the different
> magic number. Not the kernel unflattener nor any bootloader.

Yes, but I'd prefer to implement the preferred way of doing things
first - with the new magic number, then add the backward compatibility
shim afterwards.

> We need an option to keep the old magic number for compatibility.
> 
> >> ---
> >> Documentation/manual.txt |  8 ++++++++
> >> dtc.c                    | 14 +++++++++++---
> >> dtc.h                    |  4 ++--
> >> fdtdump.c                |  2 +-
> >> flattree.c               | 11 ++++++-----
> >> libfdt/fdt.c             |  2 +-
> >> libfdt/fdt.h             |  3 ++-
> >> tests/mangle-layout.c    |  7 ++++---
> >> 8 files changed, 35 insertions(+), 16 deletions(-)
> >> 
> >> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> >> index 398de32..6d2811b 100644
> >> --- a/Documentation/manual.txt
> >> +++ b/Documentation/manual.txt
> >> @@ -60,6 +60,9 @@ The currently supported Input Formats are:
> >>     - "dtb": "blob" format.  A flattened device-tree block with
> >>         header in one binary blob.
> >> 
> >> +    - "dtbo" : "blob" format. Identical with "dtb" but meant
> >> +        for use with dynamic-device tree objects.
> >> +
> >>     - "dts": "source" format.  A text file containing a "source"
> >>         for a device-tree.
> >> 
> >> @@ -71,6 +74,8 @@ The currently supported Output Formats are:
> >> 
> >>      - "dtb": "blob" format
> >> 
> >> +     - "dtbo": "blob" format - for dynamic device tree objects/overlays
> >> +
> >>      - "dts": "source" format
> >> 
> >>      - "asm": assembly language file.  A file that can be sourced
> >> @@ -78,6 +83,9 @@ The currently supported Output Formats are:
> >>         then simply be added to your Makefile.  Additionally, the
> >>         assembly file exports some symbols that can be used.
> >> 
> >> +     - "asmo": assembly language file for dynamic device tree objects/overlays.
> >> +        Identical to "asm" for most purposes.
> >> +
> >> 
> >> 3) Command Line
> >> 
> >> diff --git a/dtc.c b/dtc.c
> >> index 1c1f88f..43ba19d 100644
> >> --- a/dtc.c
> >> +++ b/dtc.c
> >> @@ -127,6 +127,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback)
> >> 		return "dts";
> >> 	if (!strcasecmp(s, ".dtb"))
> >> 		return "dtb";
> >> +	if (!strcasecmp(s, ".dtbo"))
> >> +		return "dtbo";
> >> 	return fallback;
> >> }
> >> 
> >> @@ -157,6 +159,8 @@ static const char *guess_input_format(const char *fname, const char *fallback)
> >> 	magic = fdt32_to_cpu(magic);
> >> 	if (magic == FDT_MAGIC)
> >> 		return "dtb";
> >> +	if (magic == FDT_MAGIC_DTBO)
> >> +		return "dtbo";
> >> 
> >> 	return guess_type_by_name(fname, fallback);
> >> }
> >> @@ -285,7 +289,7 @@ int main(int argc, char *argv[])
> >> 		dti = dt_from_source(arg);
> >> 	else if (streq(inform, "fs"))
> >> 		dti = dt_from_fs(arg);
> >> -	else if(streq(inform, "dtb"))
> >> +	else if (streq(inform, "dtb") || streq(inform, "dtbo"))
> >> 		dti = dt_from_blob(arg);
> >> 	else
> >> 		die("Unknown input format \"%s\"\n", inform);
> >> @@ -319,9 +323,13 @@ int main(int argc, char *argv[])
> >> 	if (streq(outform, "dts")) {
> >> 		dt_to_source(outf, dti);
> >> 	} else if (streq(outform, "dtb")) {
> >> -		dt_to_blob(outf, dti, outversion);
> >> +		dt_to_blob(outf, dti, FDT_MAGIC, outversion);
> >> +	} else if (streq(outform, "dtbo")) {
> >> +		dt_to_blob(outf, dti, FDT_MAGIC_DTBO, outversion);
> >> 	} else if (streq(outform, "asm")) {
> >> -		dt_to_asm(outf, dti, outversion);
> >> +		dt_to_asm(outf, dti, FDT_MAGIC, outversion);
> >> +	} else if (streq(outform, "asmo")) {
> >> +		dt_to_asm(outf, dti, FDT_MAGIC_DTBO, outversion);
> >> 	} else if (streq(outform, "null")) {
> >> 		/* do nothing */
> >> 	} else {
> >> diff --git a/dtc.h b/dtc.h
> >> index 040f8d4..fa66dca 100644
> >> --- a/dtc.h
> >> +++ b/dtc.h
> >> @@ -266,8 +266,8 @@ void process_checks(bool force, struct dt_info *dti);
> >> 
> >> /* Flattened trees */
> >> 
> >> -void dt_to_blob(FILE *f, struct dt_info *dti, int version);
> >> -void dt_to_asm(FILE *f, struct dt_info *dti, int version);
> >> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
> >> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version);
> >> 
> >> struct dt_info *dt_from_blob(const char *fname);
> >> 
> >> diff --git a/fdtdump.c b/fdtdump.c
> >> index 95a6a20..fa5a050 100644
> >> --- a/fdtdump.c
> >> +++ b/fdtdump.c
> >> @@ -199,7 +199,7 @@ int main(int argc, char *argv[])
> >> 			p = memchr(p, smagic[0], endp - p - 4);
> >> 			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 45fcb04..59bdc7d 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 dt_info *dti, int version)
> >> +void dt_to_blob(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
> >> {
> >> 	struct version_info *vi = NULL;
> >> 	int i;
> >> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct dt_info *dti, int version)
> >> 	reservebuf = flatten_reserve_list(dti->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,
> >> 			dti->boot_cpuid_phys);
> >> 
> >> 	/*
> >> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
> >> 	}
> >> }
> >> 
> >> -void dt_to_asm(FILE *f, struct dt_info *dti, int version)
> >> +void dt_to_asm(FILE *f, struct dt_info *dti, fdt32_t magic, int version)
> >> {
> >> 	struct version_info *vi = NULL;
> >> 	int i;
> >> @@ -832,7 +833,7 @@ struct dt_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");
> >> 
> >> 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
> >> 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/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);
> > 
> 
> Regards
> 
> — Pantelis
> 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 3/3] dtc: Plugin and fixup support
       [not found]     ` <1464889642-28080-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-06-09  4:53       ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2016-06-09  4:53 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Frank Rowand,
	Mark Rutland, Jan Luebbe, Sascha Hauer, Matt Porter,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 22149 bytes --]

On Thu, Jun 02, 2016 at 08:47:22PM +0300, 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.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Jan Luebbe <jlu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  Documentation/manual.txt |  24 +++++-
>  checks.c                 |   8 +-
>  dtc-lexer.l              |   5 ++
>  dtc-parser.y             |  33 ++++++-
>  dtc.c                    |  23 ++++-
>  dtc.h                    |  31 ++++++-
>  flattree.c               |   2 +-
>  fstree.c                 |   2 +-
>  livetree.c               | 219 ++++++++++++++++++++++++++++++++++++++++++++++-
>  treesource.c             |   1 +
>  10 files changed, 332 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 6d2811b..deffcdb 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -127,6 +127,20 @@ 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.
> +
>      -S <bytes>
>  	Ensure the blob at least <bytes> long, adding additional
>  	space if needed.
> @@ -154,12 +168,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 basetree overlays
> +
> +    versioninfo:  "/dts-v1/"
> +
> +    plugindecl:   "/plugin/"

This still isn't quite right, because it doesn't have the ';'.

>  
>      memreserve:   label 'memreserve' ADDR ADDR ';'
>  		| label 'memreserve' ADDR '-' ADDR ';'
>  
> -    devicetree:   '/' nodedef
> +    basetree:   '/' nodedef
> +
> +    overlays:   list_of_overlay_definitions (at compile time)

format of an overlay isn't defined, and this doesn't really look like BNF.

>      nodedef:      '{' list_of_property list_of_subnode '}' ';'
>  
> diff --git a/checks.c b/checks.c
> index 1829f0e..355bef9 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -482,8 +482,12 @@ static void fixup_phandle_references(struct check *c, struct node *dt,
>  
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (! refnode) {
> -				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
> -				     m->ref);
> +				if (!(tree_get_versionflags(dt) & 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 e0d7212..bb89631 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 dt_info *parser_output;
>  extern bool treesource_error;
> +extern unsigned int the_versionflags;

You don't need this additional global, you're already putting it into
struct dt_info.

>  %}
>  
>  %union {
> @@ -54,9 +56,11 @@ extern bool treesource_error;
>  	struct overlay *overlaylist;
>  	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
> @@ -73,6 +77,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
> @@ -106,16 +112,37 @@ extern bool treesource_error;
>  %%
>  
>  sourcefile:
> -	  v1tag memreserves basetree overlays
> +	  versioninfo plugindecl memreserves basetree overlays
>  		{
> -			parser_output = build_dt_info($2, $3, $4,
> -						      guess_boot_cpuid($3));
> +			assert(($1 | $2) == the_versionflags);
> +			parser_output = build_dt_info($1 | $2, $3, $4, $5,
> +							guess_boot_cpuid($4));
> +		}
> +	;
> +
> +versioninfo:
> +	v1tag
> +		{
> +			the_versionflags |= VF_DT_V1;
> +			$$ = VF_DT_V1;
>  		}
>  	;
>  
>  v1tag:
>  	  DT_V1 ';'
>  	| DT_V1 ';' v1tag
> +	| DT_V1
> +
> +plugindecl:
> +	DT_PLUGIN ';'
> +		{
> +			the_versionflags |= VF_PLUGIN;
> +			$$ = the_versionflags;
> +		}
> +	| /* empty */
> +		{
> +			$$ = 0;
> +		}
>  	;
>  
>  memreserves:
> diff --git a/dtc.c b/dtc.c
> index 43ba19d..8b716ad 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -31,6 +31,8 @@ int reservenum;		/* Number of memory reservation slots */
>  int minsize;		/* Minimum blob size */
>  int padsize;		/* Additional padding to blob */
>  int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
> +int symbol_fixup_support;
> +int auto_label_aliases;
>  
>  static void fill_fullpaths(struct node *tree, const char *prefix)
>  {
> @@ -63,7 +65,7 @@ static void resolve_overlays(struct dt_info *dti)
>  #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:fb:i:H:sW:E:hv";
> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:@Ahv";
>  static struct option const usage_long_opts[] = {
>  	{"quiet",            no_argument, NULL, 'q'},
>  	{"in-format",         a_argument, NULL, 'I'},
> @@ -81,6 +83,8 @@ 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'},
>  	{"help",             no_argument, NULL, 'h'},
>  	{"version",          no_argument, NULL, 'v'},
>  	{NULL,               no_argument, NULL, 0x0},
> @@ -111,6 +115,8 @@ 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\tPrint this help and exit",
>  	"\n\tPrint version and exit",
>  	NULL,
> @@ -247,7 +253,12 @@ int main(int argc, char *argv[])
>  		case 'E':
>  			parse_checks_option(false, true, optarg);
>  			break;
> -
> +		case '@':
> +			symbol_fixup_support = 1;
> +			break;
> +		case 'A':
> +			auto_label_aliases = 1;
> +			break;
>  		case 'h':
>  			usage(NULL);
>  		default:
> @@ -308,6 +319,14 @@ int main(int argc, char *argv[])
>  
>  	process_checks(force, dti);
>  
> +	if (auto_label_aliases)
> +		generate_label_tree(dti->dt, "aliases", false);
> +
> +	if (symbol_fixup_support) {
> +		generate_label_tree(dti->dt, "__symbols__", true);
> +		generate_fixups_tree(dti->dt);
> +	}
> +
>  	if (sort)
>  		sort_tree(dti);
>  
> diff --git a/dtc.h b/dtc.h
> index fa66dca..a9fac44 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -54,6 +54,12 @@ extern int reservenum;		/* Number of memory reservation slots */
>  extern int minsize;		/* Minimum blob size */
>  extern int padsize;		/* Additional padding to blob */
>  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 */
> +
> +/*
> + * Tree source globals
> + */
>  
>  #define PHANDLE_LEGACY	0x1
>  #define PHANDLE_EPAPR	0x2
> @@ -142,6 +148,8 @@ struct property {
>  	struct label *labels;
>  };
>  
> +struct dt_info;
> +
>  struct node {
>  	bool deleted;
>  	char *name;
> @@ -158,6 +166,9 @@ struct node {
>  	int addr_cells, size_cells;
>  
>  	struct label *labels;
> +
> +	/* only for the root (parent == NULL) */
> +	struct dt_info *dti;

I still don't think you should need this - things which need the
version info should be taking struct dt_info instead of struct node.

>  };
>  
>  struct overlay {
> @@ -200,6 +211,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);
> @@ -207,6 +219,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);
> +struct property *append_to_property(struct node *node,
> +				    char *name, const void *data, int len);
>  
>  struct overlay *build_overlay(char *target, struct node *dt);
>  struct overlay *chain_overlay(struct overlay *first, struct overlay *list);
> @@ -246,6 +260,7 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  
>  
>  struct dt_info {
> +	unsigned int versionflags;
>  	struct reserve_info *reservelist;
>  	uint32_t boot_cpuid_phys;
>  
> @@ -253,11 +268,25 @@ struct dt_info {
>  	struct overlay *overlays;
>  };
>  
> -struct dt_info *build_dt_info(struct reserve_info *reservelist,
> +/* version flags definitions */
> +#define VF_DT_V1	0x0001	/* /dts-v1/ */
> +#define VF_PLUGIN	0x0002	/* /plugin/ */
> +
> +static inline unsigned int tree_get_versionflags(struct node *dt)
> +{
> +	if (!dt || !dt->dti)
> +		return 0;
> +	return dt->dti->versionflags;
> +}
> +
> +struct dt_info *build_dt_info(unsigned int versionflags,
> +			      struct reserve_info *reservelist,
>  			      struct node *basetree,
>  			      struct overlay *overlays,
>  			      uint32_t boot_cpuid_phys);
>  void sort_tree(struct dt_info *dti);
> +void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph);
> +void generate_fixups_tree(struct node *dt);
>  
>  /* Checks */
>  
> diff --git a/flattree.c b/flattree.c
> index 59bdc7d..b674cbf 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -930,5 +930,5 @@ struct dt_info *dt_from_blob(const char *fname)
>  
>  	fclose(f);
>  
> -	return build_dt_info(reservelist, tree, NULL, boot_cpuid_phys);
> +	return build_dt_info(VF_DT_V1, reservelist, tree, NULL, boot_cpuid_phys);

Actually, I think this should set the plugin flag in the dt_info based
on the input's magic number.  That doesn't need to be in the first
pass, though.

>  }
> diff --git a/fstree.c b/fstree.c
> index 0ef96a8..4f1a69d 100644
> --- a/fstree.c
> +++ b/fstree.c
> @@ -86,5 +86,5 @@ struct dt_info *dt_from_fs(const char *dirname)
>  	tree = read_fstree(dirname);
>  	tree = name_node(tree, "");
>  
> -	return build_dt_info(NULL, tree, NULL, guess_boot_cpuid(tree));
> +	return build_dt_info(VF_DT_V1, NULL, tree, NULL, guess_boot_cpuid(tree));
>  }
> diff --git a/livetree.c b/livetree.c
> index 9a44214..10a81eb 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 = xmalloc(sizeof(*node));
> +	struct property *p;
> +	struct data d = empty_data;
> +	char *name;
> +
> +	memset(node, 0, sizeof(*node));
> +
> +	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__");
> +
> +	add_child(dt, node);
> +	add_child(node, new_node);
> +}
> +
>  struct node *chain_node(struct node *first, struct node *list)
>  {
>  	assert(first->next_sibling == NULL);
> @@ -318,9 +343,14 @@ void apply_overlay(struct node *base, struct overlay *overlay)
>  {
>  	struct node *target = get_node_by_ref(base, overlay->target);
>  
> -	if (!target)
> -		die("Couldn't find label or path %s for overlay\n",
> -		    overlay->target);
> +	if (!target) {
> +		if (base->dti->versionflags & VF_PLUGIN)
> +			add_orphan_node(base, overlay->dt, overlay->target);
> +		else
> +			die("Couldn't find label or path %s for overlay\n",
> +				overlay->target);
> +		return;
> +	}

Hrm.. so rather than making apply_overlay() handle both cases, I was
thinking instead that we have two different paths in main()

    - In the non overlay output case we call resolve_overlays() which
      applies the overlays immediately and fails if it can't
    - In the overlay output case, we call a different function which
      instead coverts each source overlay into a fragment@ node.

The second function would also construct the fixups node - AFAICT that
should always exist in a plugin, even if you don't have a __symbols__ node.

The other thing I have in mind to think about is that I'm looking into
having (some of) the checks run on each overlay individually as well
as on the final combined tree.  That shouldn't directly impact what
you're doing, but might inform some details.

>  
>  	if (!overlay->dt)
>  		/* Deletion */
> @@ -329,6 +359,24 @@ void apply_overlay(struct node *base, struct overlay *overlay)
>  		merge_nodes(target, overlay->dt);
>  }
>  
> +struct property *append_to_property(struct node *node,
> +				    char *name, const void *data, int len)

AFAICT none of your callers use the return value.

> +{
> +	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);
> +	}
> +	return p;
> +}
> +
>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>  {
>  	struct reserve_info *new = xmalloc(sizeof(*new));
> @@ -368,7 +416,8 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  	return list;
>  }
>  
> -struct dt_info *build_dt_info(struct reserve_info *reservelist,
> +struct dt_info *build_dt_info(unsigned int versionflags,
> +			      struct reserve_info *reservelist,
>  			      struct node *basetree,
>  			      struct overlay *overlays,
>  			      uint32_t boot_cpuid_phys)
> @@ -376,10 +425,12 @@ struct dt_info *build_dt_info(struct reserve_info *reservelist,
>  	struct dt_info *dti;
>  
>  	dti = xmalloc(sizeof(*dti));
> +	dti->versionflags = versionflags;
>  	dti->reservelist = reservelist;
>  	dti->boot_cpuid_phys = boot_cpuid_phys;
>  	dti->dt = basetree;
>  	dti->overlays = overlays;
> +	basetree->dti = dti;
>  
>  	return dti;
>  }
> @@ -745,3 +796,163 @@ void sort_tree(struct dt_info *dti)
>  	sort_reserve_entries(dti);
>  	sort_node(dti->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_escape_string(node->fullpath,
> +						strlen(node->fullpath)));
> +			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;
> +
> +	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);
> +}
> +
> +static char *fixups_name = "__fixups__";
> +static char *local_fixups_name = "__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_name);
> +	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_name);
> +	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';
> +
> +		/* 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)
> +{
> +	build_and_name_child_node(dt, fixups_name);
> +	build_and_name_child_node(dt, local_fixups_name);
> +	generate_fixups_tree_internal(dt, dt);
> +}
> diff --git a/treesource.c b/treesource.c
> index c9d8967..0c84f02 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -27,6 +27,7 @@ extern YYLTYPE yylloc;
>  
>  struct dt_info *parser_output;
>  bool treesource_error;
> +unsigned int the_versionflags;
>  
>  struct dt_info *dt_from_source(const char *fname)
>  {

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-09  4:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-02 17:47 [PATCH v8 0/3] dtc: Dynamic DT support Pantelis Antoniou
     [not found] ` <1464889642-28080-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-06-02 17:47   ` [PATCH v8 1/3] DTBO magic and dtbo format options Pantelis Antoniou
     [not found]     ` <1464889642-28080-2-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-06-08  6:51       ` David Gibson
     [not found]         ` <20160608065100.GA9226-RXTfZT5YzpxwFLYp8hBm2A@public.gmane.org>
2016-06-08  8:30           ` Pantelis Antoniou
2016-06-08  8:30             ` Pantelis Antoniou
     [not found]             ` <91D401B5-5C8F-4AEB-918C-1BF0BCCCC9D8-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-06-09  4:19               ` David Gibson
2016-06-02 17:47   ` [PATCH v8 2/3] dtc: Document the dynamic plugin internals Pantelis Antoniou
2016-06-02 17:47   ` [PATCH v8 3/3] dtc: Plugin and fixup support Pantelis Antoniou
     [not found]     ` <1464889642-28080-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-06-09  4:53       ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.