[PATCH v7 0/5] dtc: Dynamic DT support

[PATCH v7 0/5] dtc: Dynamic DT support

[Pantelis Antoniou]

The first patch introduces a xasprintf utility function.

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

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

The final patch enables backward compatibility with already
present overlays out in the wild.

Only the first two patches are required; the last two are
optional.

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 (5):
  util: Add xasprintf portable asprintf variant
  DTBO magic and dtbo format options
  dtc: Document the dynamic plugin internals
  dtc: Plugin and fixup support
  plugin: Transparently support old style syntax

 Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
 Documentation/manual.txt             |  23 +++
 checks.c                             |   8 +-
 dtc-lexer.l                          |   5 +
 dtc-parser.y                         |  60 ++++++-
 dtc.c                                |  37 +++-
 dtc.h                                |  33 +++-
 fdtdump.c                            |   2 +-
 flattree.c                           |  13 +-
 fstree.c                             |   2 +-
 libfdt/fdt.c                         |   2 +-
 libfdt/fdt.h                         |   3 +-
 livetree.c                           | 209 ++++++++++++++++++++++-
 tests/mangle-layout.c                |   7 +-
 treesource.c                         |   1 +
 util.c                               |  30 ++++
 util.h                               |   1 +
 17 files changed, 724 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/dt-object-internal.txt

-- 
1.7.12

[PATCH v7 1/5] util: Add xasprintf portable asprintf variant

[Pantelis Antoniou]

Include a portable asprintf variant that works on any C99
conforming platform.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 util.c | 30 ++++++++++++++++++++++++++++++
 util.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/util.c b/util.c
index fb124ee..3550f86 100644
--- a/util.c
+++ b/util.c
@@ -46,6 +46,36 @@ char *xstrdup(const char *s)
 	return d;
 }
 
+/* based in part from (3) vsnprintf */
+int xasprintf(char **strp, const char *fmt, ...)
+{
+	int n, size = 128;	/* start with 128 bytes */
+	char *p;
+	va_list ap;
+
+	/* initial pointer is NULL making the fist realloc to be malloc */
+	p = NULL;
+	while (1) {
+		p = xrealloc(p, size);
+
+		/* Try to print in the allocated space. */
+		va_start(ap, fmt);
+		n = vsnprintf(p, size, fmt, ap);
+		va_end(ap);
+
+		/* If that worked, return the string. */
+		if (n > -1 && n < size)
+			break;
+		/* Else try again with more space. */
+		if (n > -1)	/* glibc 2.1 */
+			size = n + 1; /* precisely what is needed */
+		else		/* glibc 2.0 */
+			size *= 2; /* twice the old size */
+	}
+	*strp = p;
+	return strlen(p);
+}
+
 char *join_path(const char *path, const char *name)
 {
 	int lenp = strlen(path);
diff --git a/util.h b/util.h
index f800b60..f5c4f1b 100644
--- a/util.h
+++ b/util.h
@@ -59,6 +59,7 @@ static inline void *xrealloc(void *p, size_t len)
 }
 
 extern char *xstrdup(const char *s);
+extern int xasprintf(char **strp, const char *fmt, ...);
 extern char *join_path(const char *path, const char *name);
 
 /**
-- 
1.7.12

[PATCH v7 2/5] DTBO magic and dtbo format options

[Pantelis Antoniou]

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 |  7 +++++++
 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, 34 insertions(+), 16 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 398de32..f64c4f4 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 objects
+
      - "dts": "source" format
 
      - "asm": assembly language file.  A file that can be sourced
@@ -78,6 +83,8 @@ 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 objects. Identical to "asm"
+
 
 3) Command Line
 
diff --git a/dtc.c b/dtc.c
index 5fa23c4..63c2c9c 100644
--- a/dtc.c
+++ b/dtc.c
@@ -117,6 +117,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;
 }
 
@@ -147,6 +149,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);
 }
@@ -275,7 +279,7 @@ int main(int argc, char *argv[])
 		bi = dt_from_source(arg);
 	else if (streq(inform, "fs"))
 		bi = dt_from_fs(arg);
-	else if(streq(inform, "dtb"))
+	else if(streq(inform, "dtb") || streq(inform, "dtbo"))
 		bi = dt_from_blob(arg);
 	else
 		die("Unknown input format \"%s\"\n", inform);
@@ -306,9 +310,13 @@ int main(int argc, char *argv[])
 	if (streq(outform, "dts")) {
 		dt_to_source(outf, bi);
 	} else if (streq(outform, "dtb")) {
-		dt_to_blob(outf, bi, outversion);
+		dt_to_blob(outf, bi, FDT_MAGIC, outversion);
+	} else if (streq(outform, "dtbo")) {
+		dt_to_blob(outf, bi, FDT_MAGIC_DTBO, outversion);
 	} else if (streq(outform, "asm")) {
-		dt_to_asm(outf, bi, outversion);
+		dt_to_asm(outf, bi, FDT_MAGIC, outversion);
+	} else if (streq(outform, "asmo")) {
+		dt_to_asm(outf, bi, FDT_MAGIC_DTBO, outversion);
 	} else if (streq(outform, "null")) {
 		/* do nothing */
 	} else {
diff --git a/dtc.h b/dtc.h
index 56212c8..9d7f2d6 100644
--- a/dtc.h
+++ b/dtc.h
@@ -252,8 +252,8 @@ void process_checks(bool force, struct boot_info *bi);
 
 /* Flattened trees */
 
-void dt_to_blob(FILE *f, struct boot_info *bi, int version);
-void dt_to_asm(FILE *f, struct boot_info *bi, int version);
+void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
+void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
 
 struct boot_info *dt_from_blob(const char *fname);
 
diff --git a/fdtdump.c b/fdtdump.c
index 9183555..11c2b8d 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -306,7 +306,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 ec14954..64ed375 100644
--- a/flattree.c
+++ b/flattree.c
@@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 }
 
 static void make_fdt_header(struct fdt_header *fdt,
+			    fdt32_t magic,
 			    struct version_info *vi,
 			    int reservesize, int dtsize, int strsize,
 			    int boot_cpuid_phys)
@@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
 
 	memset(fdt, 0xff, sizeof(*fdt));
 
-	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
+	fdt->magic = cpu_to_fdt32(magic);
 	fdt->version = cpu_to_fdt32(vi->version);
 	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
 
@@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
 		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
 }
 
-void dt_to_blob(FILE *f, struct boot_info *bi, int version)
+void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	reservebuf = flatten_reserve_list(bi->reservelist, vi);
 
 	/* Make header */
-	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
+	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
 			bi->boot_cpuid_phys);
 
 	/*
@@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
 	}
 }
 
-void dt_to_asm(FILE *f, struct boot_info *bi, int version)
+void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -832,7 +833,7 @@ struct boot_info *dt_from_blob(const char *fname)
 	}
 
 	magic = fdt32_to_cpu(magic);
-	if (magic != FDT_MAGIC)
+	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
 		die("Blob has incorrect magic number\n");
 
 	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

[PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

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 | 318 +++++++++++++++++++++++++++++++++++
 1 file changed, 318 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..d5b841e
--- /dev/null
+++ b/Documentation/dt-object-internal.txt
@@ -0,0 +1,318 @@
+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/;
+&ocp {
+	/* bar peripheral */
+	bar {
+		compatible = "corp,bar";
+		... /* various properties and child nodes */
+	}
+};
+
+The resulting dtb object is identical.
-- 
1.7.12

[PATCH v7 4/5] dtc: Plugin and fixup support

[Pantelis Antoniou]

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 |  16 ++++
 checks.c                 |   8 +-
 dtc-lexer.l              |   5 ++
 dtc-parser.y             |  45 ++++++++--
 dtc.c                    |  23 +++++-
 dtc.h                    |  29 ++++++-
 flattree.c               |   2 +-
 fstree.c                 |   2 +-
 livetree.c               | 209 ++++++++++++++++++++++++++++++++++++++++++++++-
 treesource.c             |   1 +
 10 files changed, 327 insertions(+), 13 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index f64c4f4..63066ec 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -126,6 +126,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.
@@ -160,6 +174,8 @@ Here is a very rough overview of the layout of a DTS source file:
 
     devicetree:   '/' nodedef
 
+    plugindecl:   '/' 'plugin' '/' ';'
+
     nodedef:      '{' list_of_property list_of_subnode '}' ';'
 
     property:     label PROPNAME '=' propdata ';'
diff --git a/checks.c b/checks.c
index 386f956..3d4c3c6 100644
--- a/checks.c
+++ b/checks.c
@@ -490,8 +490,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 000873f..2890c1c 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -19,6 +19,7 @@
  */
 %{
 #include <stdio.h>
+#include <inttypes.h>
 
 #include "dtc.h"
 #include "srcpos.h"
@@ -33,6 +34,7 @@ extern void yyerror(char const *s);
 
 extern struct boot_info *the_boot_info;
 extern bool treesource_error;
+extern unsigned int the_versionflags;
 %}
 
 %union {
@@ -52,9 +54,11 @@ extern bool treesource_error;
 	struct node *nodelist;
 	struct reserve_info *re;
 	uint64_t integer;
+	unsigned int flags;
 }
 
 %token DT_V1
+%token DT_PLUGIN
 %token DT_MEMRESERVE
 %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
 %token DT_BITS
@@ -71,6 +75,8 @@ extern bool treesource_error;
 
 %type <data> propdata
 %type <data> propdataprefix
+%type <flags> versioninfo
+%type <flags> plugindecl
 %type <re> memreserve
 %type <re> memreserves
 %type <array> arrayprefix
@@ -101,13 +107,33 @@ extern bool treesource_error;
 %%
 
 sourcefile:
-	  DT_V1 ';' memreserves devicetree
+	  versioninfo ';' memreserves devicetree
 		{
-			the_boot_info = build_boot_info($3, $4,
+			the_boot_info = build_boot_info($1, $3, $4,
 							guess_boot_cpuid($4));
 		}
 	;
 
+versioninfo:
+	DT_V1 plugindecl
+		{
+			the_versionflags |= VF_DT_V1 | $2;
+			$$ = the_versionflags;
+		}
+	;
+
+plugindecl:
+	DT_PLUGIN
+		{
+			the_versionflags |= VF_PLUGIN;
+			$$ = VF_PLUGIN;
+		}
+	| /* empty */
+		{
+			$$ = 0;
+		}
+	;
+
 memreserves:
 	  /* empty */
 		{
@@ -156,10 +182,14 @@ devicetree:
 		{
 			struct node *target = get_node_by_ref($1, $2);
 
-			if (target)
+			if (target) {
 				merge_nodes(target, $3);
-			else
-				ERROR(&@2, "Label or path %s not found", $2);
+			} else {
+				if (the_versionflags & VF_PLUGIN)
+					add_orphan_node($1, $3, $2);
+				else
+					ERROR(&@2, "Label or path %s not found", $2);
+			}
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE DT_REF ';'
@@ -174,6 +204,11 @@ devicetree:
 
 			$$ = $1;
 		}
+	| /* empty */
+		{
+			/* build empty node */
+			$$ = name_node(build_node(NULL, NULL), "");
+		}
 	;
 
 nodedef:
diff --git a/dtc.c b/dtc.c
index 63c2c9c..a25f852 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)
 {
@@ -53,7 +55,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 #define FDT_VERSION(version)	_FDT_VERSION(version)
 #define _FDT_VERSION(version)	#version
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p: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'},
@@ -71,6 +73,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},
@@ -101,6 +105,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,
@@ -237,7 +243,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:
@@ -295,6 +306,14 @@ int main(int argc, char *argv[])
 	fill_fullpaths(bi->dt, "");
 	process_checks(force, bi);
 
+	if (auto_label_aliases)
+		generate_label_tree(bi->dt, "aliases", false);
+
+	if (symbol_fixup_support) {
+		generate_label_tree(bi->dt, "__symbols__", true);
+		generate_fixups_tree(bi->dt);
+	}
+
 	if (sort)
 		sort_tree(bi);
 
diff --git a/dtc.h b/dtc.h
index 9d7f2d6..392cde7 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
@@ -158,6 +164,9 @@ struct node {
 	int addr_cells, size_cells;
 
 	struct label *labels;
+
+	/* only for the root (parent == NULL) */
+	struct boot_info *bi;
 };
 
 #define for_each_label_withdel(l0, l) \
@@ -194,6 +203,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);
@@ -201,6 +211,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);
 
 const char *get_unitname(struct node *node);
 struct property *get_property(struct node *node, const char *propname);
@@ -236,14 +248,29 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 
 
 struct boot_info {
+	unsigned int versionflags;
 	struct reserve_info *reservelist;
 	struct node *dt;		/* the device tree */
 	uint32_t boot_cpuid_phys;
 };
 
-struct boot_info *build_boot_info(struct reserve_info *reservelist,
+/* version flags definitions */
+#define VF_DT_V1	0x0001	/* /dts-v1/ */
+#define VF_PLUGIN	0x0002	/* /plugin/ */
+
+static inline unsigned int tree_get_versionflags(struct node *dt)
+{
+	if (!dt || !dt->bi)
+		return 0;
+	return dt->bi->versionflags;
+}
+
+struct boot_info *build_boot_info(unsigned int versionflags,
+				  struct reserve_info *reservelist,
 				  struct node *tree, uint32_t boot_cpuid_phys);
 void sort_tree(struct boot_info *bi);
+void generate_label_tree(struct node *dt, char *gen_node_name, bool allocph);
+void generate_fixups_tree(struct node *dt);
 
 /* Checks */
 
diff --git a/flattree.c b/flattree.c
index 64ed375..4fe64d4 100644
--- a/flattree.c
+++ b/flattree.c
@@ -930,5 +930,5 @@ struct boot_info *dt_from_blob(const char *fname)
 
 	fclose(f);
 
-	return build_boot_info(reservelist, tree, boot_cpuid_phys);
+	return build_boot_info(VF_DT_V1, reservelist, tree, boot_cpuid_phys);
 }
diff --git a/fstree.c b/fstree.c
index 6d1beec..54f520b 100644
--- a/fstree.c
+++ b/fstree.c
@@ -86,6 +86,6 @@ struct boot_info *dt_from_fs(const char *dirname)
 	tree = read_fstree(dirname);
 	tree = name_node(tree, "");
 
-	return build_boot_info(NULL, tree, guess_boot_cpuid(tree));
+	return build_boot_info(VF_DT_V1, NULL, tree, guess_boot_cpuid(tree));
 }
 
diff --git a/livetree.c b/livetree.c
index e229b84..3eab9e2 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);
@@ -296,6 +321,24 @@ void delete_node(struct node *node)
 	delete_labels(&node->labels);
 }
 
+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));
@@ -335,15 +378,19 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 	return list;
 }
 
-struct boot_info *build_boot_info(struct reserve_info *reservelist,
+struct boot_info *build_boot_info(unsigned int versionflags,
+				  struct reserve_info *reservelist,
 				  struct node *tree, uint32_t boot_cpuid_phys)
 {
 	struct boot_info *bi;
 
 	bi = xmalloc(sizeof(*bi));
+	bi->versionflags = versionflags;
 	bi->reservelist = reservelist;
 	bi->dt = tree;
 	bi->boot_cpuid_phys = boot_cpuid_phys;
+	/* link back */
+	tree->bi = bi;
 
 	return bi;
 }
@@ -709,3 +756,163 @@ void sort_tree(struct boot_info *bi)
 	sort_reserve_entries(bi);
 	sort_node(bi->dt);
 }
+
+/* utility helper to avoid code duplication */
+static struct node *build_and_name_child_node(struct node *parent, char *name)
+{
+	struct node *node;
+
+	node = build_node(NULL, NULL);
+	name_node(node, xstrdup(name));
+	add_child(parent, node);
+
+	return node;
+}
+
+static void generate_label_tree_internal(struct node *dt, struct node *node,
+					 struct node *an, bool allocph)
+{
+	struct node *c;
+	struct property *p;
+	struct label *l;
+
+	/* if if there are labels */
+	if (node->labels) {
+		/* now add the label in the node */
+		for_each_label(node->labels, l) {
+			/* check whether the label already exists */
+			p = get_property(an, l->label);
+			if (p) {
+				fprintf(stderr, "WARNING: label %s already"
+					" exists in /%s", l->label,
+					an->name);
+				continue;
+			}
+
+			/* insert it */
+			p = build_property(l->label,
+				data_copy_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 a55d1d1..2539a57 100644
--- a/treesource.c
+++ b/treesource.c
@@ -27,6 +27,7 @@ extern YYLTYPE yylloc;
 
 struct boot_info *the_boot_info;
 bool treesource_error;
+unsigned int the_versionflags;
 
 struct boot_info *dt_from_source(const char *fname)
 {
-- 
1.7.12

[PATCH v7 5/5] plugin: Transparently support old style syntax

[Pantelis Antoniou]

The old style syntax for plugins is still out in the wild.
This patch transparently support it.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 dtc-parser.y | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 2890c1c..4a67baf 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -77,6 +77,7 @@ extern unsigned int the_versionflags;
 %type <data> propdataprefix
 %type <flags> versioninfo
 %type <flags> plugindecl
+%type <flags> oldplugindecl
 %type <re> memreserve
 %type <re> memreserves
 %type <array> arrayprefix
@@ -107,10 +108,10 @@ extern unsigned int the_versionflags;
 %%
 
 sourcefile:
-	  versioninfo ';' memreserves devicetree
+	  versioninfo ';' oldplugindecl memreserves devicetree
 		{
-			the_boot_info = build_boot_info($1, $3, $4,
-							guess_boot_cpuid($4));
+			the_boot_info = build_boot_info($1 | $3, $4, $5,
+							guess_boot_cpuid($5));
 		}
 	;
 
@@ -134,6 +135,18 @@ plugindecl:
 		}
 	;
 
+oldplugindecl:
+	DT_PLUGIN ';'
+		{
+			the_versionflags |= VF_PLUGIN;
+			$$ = VF_PLUGIN;
+		}
+	| /* empty */
+		{
+			$$ = 0;
+		}
+	;
+
 memreserves:
 	  /* empty */
 		{
-- 
1.7.12

Re: [PATCH v7 1/5] util: Add xasprintf portable asprintf variant

[David Gibson]

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

On Tue, May 24, 2016 at 08:50:35PM +0300, Pantelis Antoniou wrote:
> Include a portable asprintf variant that works on any C99
> conforming platform.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

Applied, thanks.

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

Re: [PATCH v7 2/5] DTBO magic and dtbo format options

[Frank Rowand]

On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> Introduce a new magic number for dynamic plugin objects,
> which is enabled by selecting dtbo/input output options.

Why is a new magic number needed for .dtbo?  Isn't it the
same format as the existing .dtb, just with some added
nodes?

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand]

On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> 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 | 318 +++++++++++++++++++++++++++++++++++
>  1 file changed, 318 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..d5b841e
> --- /dev/null
> +++ b/Documentation/dt-object-internal.txt
> @@ -0,0 +1,318 @@
> +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 */
> +			}

                        };

> +		};
> +	};
> +};

Other than the fact that the above syntax is already in the Linux
kernel overlay implementation, is there a need for the target
property and the __overlay__ node?  I haven't figured out what
extra value they provide.

Without those added, the overlay dts becomes simpler (though for a
multi-node target path example this would be more complex unless a label
was used for the target node):

+/dts-v1/ /plugin/;	/* allow undefined references and record them */
+/ {
+	....	/* various properties for loader use; i.e. part id etc. */
+	ocp {
+			/* 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/;
> +&ocp {
> +	/* bar peripheral */
> +	bar {
> +		compatible = "corp,bar";
> +		... /* various properties and child nodes */
> +	}
> +};
> +
> +The resulting dtb object is identical.
> 

Re: [PATCH v7 2/5] DTBO magic and dtbo format options

[David Gibson]

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

On Wed, May 25, 2016 at 11:51:26AM -0700, Frank Rowand wrote:
> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> > Introduce a new magic number for dynamic plugin objects,
> > which is enabled by selecting dtbo/input output options.
> 
> Why is a new magic number needed for .dtbo?  Isn't it the
> same format as the existing .dtb, just with some added
> nodes?

I insisted on that.  Yes, the encoding format is the same, but in
order to properly interpret the content, you have to know that it's an
overlay rather than a full tree.  So, it should have a different magic
number.

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

Re: [PATCH v7 2/5] DTBO magic and dtbo format options

[David Gibson]

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

On Tue, May 24, 2016 at 08:50:36PM +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>

Apart from one nit this is ready to go.

> ---
>  Documentation/manual.txt |  7 +++++++
>  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, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 398de32..f64c4f4 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 objects
> +
>       - "dts": "source" format
>  
>       - "asm": assembly language file.  A file that can be sourced
> @@ -78,6 +83,8 @@ 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 objects. Identical to "asm"

Please spell out dynamic device-tree object/overlay here, just
"object" is too vague.

>  
>  3) Command Line
>  
> diff --git a/dtc.c b/dtc.c
> index 5fa23c4..63c2c9c 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -117,6 +117,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;
>  }
>  
> @@ -147,6 +149,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);
>  }
> @@ -275,7 +279,7 @@ int main(int argc, char *argv[])
>  		bi = dt_from_source(arg);
>  	else if (streq(inform, "fs"))
>  		bi = dt_from_fs(arg);
> -	else if(streq(inform, "dtb"))
> +	else if(streq(inform, "dtb") || streq(inform, "dtbo"))
>  		bi = dt_from_blob(arg);
>  	else
>  		die("Unknown input format \"%s\"\n", inform);
> @@ -306,9 +310,13 @@ int main(int argc, char *argv[])
>  	if (streq(outform, "dts")) {
>  		dt_to_source(outf, bi);
>  	} else if (streq(outform, "dtb")) {
> -		dt_to_blob(outf, bi, outversion);
> +		dt_to_blob(outf, bi, FDT_MAGIC, outversion);
> +	} else if (streq(outform, "dtbo")) {
> +		dt_to_blob(outf, bi, FDT_MAGIC_DTBO, outversion);
>  	} else if (streq(outform, "asm")) {
> -		dt_to_asm(outf, bi, outversion);
> +		dt_to_asm(outf, bi, FDT_MAGIC, outversion);
> +	} else if (streq(outform, "asmo")) {
> +		dt_to_asm(outf, bi, FDT_MAGIC_DTBO, outversion);
>  	} else if (streq(outform, "null")) {
>  		/* do nothing */
>  	} else {
> diff --git a/dtc.h b/dtc.h
> index 56212c8..9d7f2d6 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -252,8 +252,8 @@ void process_checks(bool force, struct boot_info *bi);
>  
>  /* Flattened trees */
>  
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version);
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version);
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
>  
>  struct boot_info *dt_from_blob(const char *fname);
>  
> diff --git a/fdtdump.c b/fdtdump.c
> index 9183555..11c2b8d 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -306,7 +306,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 ec14954..64ed375 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
>  }
>  
>  static void make_fdt_header(struct fdt_header *fdt,
> +			    fdt32_t magic,
>  			    struct version_info *vi,
>  			    int reservesize, int dtsize, int strsize,
>  			    int boot_cpuid_phys)
> @@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>  
>  	memset(fdt, 0xff, sizeof(*fdt));
>  
> -	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
> +	fdt->magic = cpu_to_fdt32(magic);
>  	fdt->version = cpu_to_fdt32(vi->version);
>  	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
>  
> @@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>  		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
>  }
>  
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version)
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
>  	reservebuf = flatten_reserve_list(bi->reservelist, vi);
>  
>  	/* Make header */
> -	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
> +	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
>  			bi->boot_cpuid_phys);
>  
>  	/*
> @@ -460,7 +461,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>  	}
>  }
>  
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version)
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -832,7 +833,7 @@ struct boot_info *dt_from_blob(const char *fname)
>  	}
>  
>  	magic = fdt32_to_cpu(magic);
> -	if (magic != FDT_MAGIC)
> +	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
>  		die("Blob has incorrect magic number\n");
>  
>  	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 --]

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Wed, May 25, 2016 at 12:13:35PM -0700, Frank Rowand wrote:
> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> > 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 | 318 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 318 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..d5b841e
> > --- /dev/null
> > +++ b/Documentation/dt-object-internal.txt
> > @@ -0,0 +1,318 @@
> > +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 */
> > +			}
> 
>                         };
> 
> > +		};
> > +	};
> > +};
> 
> Other than the fact that the above syntax is already in the Linux
> kernel overlay implementation, is there a need for the target
> property and the __overlay__ node?  I haven't figured out what
> extra value they provide.

I've been assuming that the fact the syntax is already used in the
kernel was the main reason here.

> Without those added, the overlay dts becomes simpler (though for a
> multi-node target path example this would be more complex unless a label
> was used for the target node):
> 
> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> +/ {
> +	....	/* various properties for loader use; i.e. part id etc. */
> +	ocp {
> +			/* bar peripheral */
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			};
> +	};
> +};

Hmm, that is simpler - and avoids the rather silly fact in the current
version that there's a basically useless phandle value here - it will
always be -1 and have to be resolved using data elsewhere.

That said, I'm not sure the change is enough of a win to recommend
changing it given that the __overlay__ format is in use in the wild.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi Frank,

> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>> 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 | 318 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 318 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..d5b841e
>> --- /dev/null
>> +++ b/Documentation/dt-object-internal.txt
>> @@ -0,0 +1,318 @@
>> +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 */
>> +			}
> 
>                        };
> 
>> +		};
>> +	};
>> +};
> 
> Other than the fact that the above syntax is already in the Linux
> kernel overlay implementation, is there a need for the target
> property and the __overlay__ node?  I haven't figured out what
> extra value they provide.
> 
> Without those added, the overlay dts becomes simpler (though for a
> multi-node target path example this would be more complex unless a label
> was used for the target node):
> 
> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> +/ {
> +	....	/* various properties for loader use; i.e. part id etc. */
> +	ocp {
> +			/* bar peripheral */
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			};
> +	};
> +};
> 

No.

That only works if the overlay is applied in a single platform.

I have working cases where the same overlay is applied on a ppc and a x86
platform.


>> +
>> +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/;
>> +&ocp {
>> +	/* bar peripheral */
>> +	bar {
>> +		compatible = "corp,bar";
>> +		... /* various properties and child nodes */
>> +	}
>> +};
>> +
>> +The resulting dtb object is identical.

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi Frank,

> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>> 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 | 318 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 318 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..d5b841e
>> --- /dev/null
>> +++ b/Documentation/dt-object-internal.txt
>> @@ -0,0 +1,318 @@
>> +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 */
>> +			}
> 
>                        };
> 
>> +		};
>> +	};
>> +};
> 
> Other than the fact that the above syntax is already in the Linux
> kernel overlay implementation, is there a need for the target
> property and the __overlay__ node?  I haven't figured out what
> extra value they provide.
> 
> Without those added, the overlay dts becomes simpler (though for a
> multi-node target path example this would be more complex unless a label
> was used for the target node):
> 
> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> +/ {
> +	....	/* various properties for loader use; i.e. part id etc. */
> +	ocp {
> +			/* bar peripheral */
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			};
> +	};
> +};
> 

No.

That only works if the overlay is applied in a single platform.

I have working cases where the same overlay is applied on a ppc and a x86
platform.


>> +
>> +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/;
>> +&ocp {
>> +	/* bar peripheral */
>> +	bar {
>> +		compatible = "corp,bar";
>> +		... /* various properties and child nodes */
>> +	}
>> +};
>> +
>> +The resulting dtb object is identical.

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi David,

> On May 26, 2016, at 07:58 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Wed, May 25, 2016 at 12:13:35PM -0700, Frank Rowand wrote:
>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>> 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 | 318 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 318 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..d5b841e
>>> --- /dev/null
>>> +++ b/Documentation/dt-object-internal.txt
>>> @@ -0,0 +1,318 @@
>>> +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 */
>>> +			}
>> 
>>                        };
>> 
>>> +		};
>>> +	};
>>> +};
>> 
>> Other than the fact that the above syntax is already in the Linux
>> kernel overlay implementation, is there a need for the target
>> property and the __overlay__ node?  I haven't figured out what
>> extra value they provide.
> 
> I've been assuming that the fact the syntax is already used in the
> kernel was the main reason here.
> 
>> Without those added, the overlay dts becomes simpler (though for a
>> multi-node target path example this would be more complex unless a label
>> was used for the target node):
>> 
>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
>> +/ {
>> +	....	/* various properties for loader use; i.e. part id etc. */
>> +	ocp {
>> +			/* bar peripheral */
>> +			bar {
>> +				compatible = "corp,bar";
>> +				... /* various properties and child nodes */
>> +			};
>> +	};
>> +};
> 
> Hmm, that is simpler - and avoids the rather silly fact in the current
> version that there's a basically useless phandle value here - it will
> always be -1 and have to be resolved using data elsewhere.
> 
> That said, I'm not sure the change is enough of a win to recommend
> changing it given that the __overlay__ format is in use in the wild.
> 

As as replied to Frank, no. This is a simplification that completely
breaks other uses of overlays. This format assumes that the overlay
is only going to be applied on a single platform, where that is not the
case now.

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

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi David,

> On May 26, 2016, at 07:58 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Wed, May 25, 2016 at 12:13:35PM -0700, Frank Rowand wrote:
>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>> 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 | 318 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 318 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..d5b841e
>>> --- /dev/null
>>> +++ b/Documentation/dt-object-internal.txt
>>> @@ -0,0 +1,318 @@
>>> +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 */
>>> +			}
>> 
>>                        };
>> 
>>> +		};
>>> +	};
>>> +};
>> 
>> Other than the fact that the above syntax is already in the Linux
>> kernel overlay implementation, is there a need for the target
>> property and the __overlay__ node?  I haven't figured out what
>> extra value they provide.
> 
> I've been assuming that the fact the syntax is already used in the
> kernel was the main reason here.
> 
>> Without those added, the overlay dts becomes simpler (though for a
>> multi-node target path example this would be more complex unless a label
>> was used for the target node):
>> 
>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
>> +/ {
>> +	....	/* various properties for loader use; i.e. part id etc. */
>> +	ocp {
>> +			/* bar peripheral */
>> +			bar {
>> +				compatible = "corp,bar";
>> +				... /* various properties and child nodes */
>> +			};
>> +	};
>> +};
> 
> Hmm, that is simpler - and avoids the rather silly fact in the current
> version that there's a basically useless phandle value here - it will
> always be -1 and have to be resolved using data elsewhere.
> 
> That said, I'm not sure the change is enough of a win to recommend
> changing it given that the __overlay__ format is in use in the wild.
> 

As as replied to Frank, no. This is a simplification that completely
breaks other uses of overlays. This format assumes that the overlay
is only going to be applied on a single platform, where that is not the
case now.

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

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
> Hi Frank,
> 
> > On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> >> 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 | 318 +++++++++++++++++++++++++++++++++++
> >> 1 file changed, 318 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..d5b841e
> >> --- /dev/null
> >> +++ b/Documentation/dt-object-internal.txt
> >> @@ -0,0 +1,318 @@
> >> +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 */
> >> +			}
> > 
> >                        };
> > 
> >> +		};
> >> +	};
> >> +};
> > 
> > Other than the fact that the above syntax is already in the Linux
> > kernel overlay implementation, is there a need for the target
> > property and the __overlay__ node?  I haven't figured out what
> > extra value they provide.
> > 
> > Without those added, the overlay dts becomes simpler (though for a
> > multi-node target path example this would be more complex unless a label
> > was used for the target node):
> > 
> > +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> > +/ {
> > +	....	/* various properties for loader use; i.e. part id etc. */
> > +	ocp {
> > +			/* bar peripheral */
> > +			bar {
> > +				compatible = "corp,bar";
> > +				... /* various properties and child nodes */
> > +			};
> > +	};
> > +};
> > 
> 
> No.
> 
> That only works if the overlay is applied in a single platform.
> 
> I have working cases where the same overlay is applied on a ppc and a x86
> platform.

Huh?  How so..

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi David,

> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>> Hi Frank,
>> 
>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 
>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>> 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 | 318 +++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 318 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..d5b841e
>>>> --- /dev/null
>>>> +++ b/Documentation/dt-object-internal.txt
>>>> @@ -0,0 +1,318 @@
>>>> +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 */
>>>> +			}
>>> 
>>>                       };
>>> 
>>>> +		};
>>>> +	};
>>>> +};
>>> 
>>> Other than the fact that the above syntax is already in the Linux
>>> kernel overlay implementation, is there a need for the target
>>> property and the __overlay__ node?  I haven't figured out what
>>> extra value they provide.
>>> 
>>> Without those added, the overlay dts becomes simpler (though for a
>>> multi-node target path example this would be more complex unless a label
>>> was used for the target node):
>>> 
>>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
>>> +/ {
>>> +	....	/* various properties for loader use; i.e. part id etc. */
>>> +	ocp {
>>> +			/* bar peripheral */
>>> +			bar {
>>> +				compatible = "corp,bar";
>>> +				... /* various properties and child nodes */
>>> +			};
>>> +	};
>>> +};
>>> 
>> 
>> No.
>> 
>> That only works if the overlay is applied in a single platform.
>> 
>> I have working cases where the same overlay is applied on a ppc and a x86
>> platform.
> 
> Huh?  How so..
> 

Yes, it does work. Yes it’s being used right now. It is a very valid use case.

Think carrier boards on enterprise routers, plugging to a main board
that’s either ppc or x86 (or anything else for that matter).

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

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
> Hi David,
> 
> > On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
> >> Hi Frank,
> >> 
> >>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>> 
> >>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> >>>> 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 | 318 +++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 318 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..d5b841e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/dt-object-internal.txt
> >>>> @@ -0,0 +1,318 @@
> >>>> +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 */
> >>>> +			}
> >>> 
> >>>                       };
> >>> 
> >>>> +		};
> >>>> +	};
> >>>> +};
> >>> 
> >>> Other than the fact that the above syntax is already in the Linux
> >>> kernel overlay implementation, is there a need for the target
> >>> property and the __overlay__ node?  I haven't figured out what
> >>> extra value they provide.
> >>> 
> >>> Without those added, the overlay dts becomes simpler (though for a
> >>> multi-node target path example this would be more complex unless a label
> >>> was used for the target node):
> >>> 
> >>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> >>> +/ {
> >>> +	....	/* various properties for loader use; i.e. part id etc. */
> >>> +	ocp {
> >>> +			/* bar peripheral */
> >>> +			bar {
> >>> +				compatible = "corp,bar";
> >>> +				... /* various properties and child nodes */
> >>> +			};
> >>> +	};
> >>> +};
> >>> 
> >> 
> >> No.
> >> 
> >> That only works if the overlay is applied in a single platform.
> >> 
> >> I have working cases where the same overlay is applied on a ppc and a x86
> >> platform.
> > 
> > Huh?  How so..
> > 
> 
> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
> 
> Think carrier boards on enterprise routers, plugging to a main board
> that’s either ppc or x86 (or anything else for that matter).

Sorry, I wasn't clear.  I have no problem believing overlays can be
applied on multiple platforms.

What I can't see is how Frank's format breaks that.  AFAICT it
contains exactly the same information in a simpler encoding.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi David,

> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>> Hi David,
>> 
>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6rL0yjEaa7Ru@public.gmane.orgau> wrote:
>>> 
>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>> Hi Frank,
>>>> 
>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> 
>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>> 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 | 318 +++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 318 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..d5b841e
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/dt-object-internal.txt
>>>>>> @@ -0,0 +1,318 @@
>>>>>> +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 */
>>>>>> +			}
>>>>> 
>>>>>                      };
>>>>> 
>>>>>> +		};
>>>>>> +	};
>>>>>> +};
>>>>> 
>>>>> Other than the fact that the above syntax is already in the Linux
>>>>> kernel overlay implementation, is there a need for the target
>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>> extra value they provide.
>>>>> 
>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>> multi-node target path example this would be more complex unless a label
>>>>> was used for the target node):
>>>>> 
>>>>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
>>>>> +/ {
>>>>> +	....	/* various properties for loader use; i.e. part id etc. */
>>>>> +	ocp {
>>>>> +			/* bar peripheral */
>>>>> +			bar {
>>>>> +				compatible = "corp,bar";
>>>>> +				... /* various properties and child nodes */
>>>>> +			};
>>>>> +	};
>>>>> +};
>>>>> 
>>>> 
>>>> No.
>>>> 
>>>> That only works if the overlay is applied in a single platform.
>>>> 
>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>> platform.
>>> 
>>> Huh?  How so..
>>> 
>> 
>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>> 
>> Think carrier boards on enterprise routers, plugging to a main board
>> that’s either ppc or x86 (or anything else for that matter).
> 
> Sorry, I wasn't clear.  I have no problem believing overlays can be
> applied on multiple platforms.
> 
> What I can't see is how Frank's format breaks that.  AFAICT it
> contains exactly the same information in a simpler encoding.
> 

It breaks it because it’s missing the target property.

The layout of the base tree is not going to be the same in different
platforms, so in the above example ‘ocp’ would not exist in x86 for
instance.


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

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
> Hi David,
> 
> > On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
> >> Hi David,
> >> 
> >>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >>> 
> >>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
> >>>> Hi Frank,
> >>>> 
> >>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>> 
> >>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> >>>>>> 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 | 318 +++++++++++++++++++++++++++++++++++
> >>>>>> 1 file changed, 318 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..d5b841e
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/dt-object-internal.txt
> >>>>>> @@ -0,0 +1,318 @@
> >>>>>> +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 */
> >>>>>> +			}
> >>>>> 
> >>>>>                      };
> >>>>> 
> >>>>>> +		};
> >>>>>> +	};
> >>>>>> +};
> >>>>> 
> >>>>> Other than the fact that the above syntax is already in the Linux
> >>>>> kernel overlay implementation, is there a need for the target
> >>>>> property and the __overlay__ node?  I haven't figured out what
> >>>>> extra value they provide.
> >>>>> 
> >>>>> Without those added, the overlay dts becomes simpler (though for a
> >>>>> multi-node target path example this would be more complex unless a label
> >>>>> was used for the target node):
> >>>>> 
> >>>>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> >>>>> +/ {
> >>>>> +	....	/* various properties for loader use; i.e. part id etc. */
> >>>>> +	ocp {
> >>>>> +			/* bar peripheral */
> >>>>> +			bar {
> >>>>> +				compatible = "corp,bar";
> >>>>> +				... /* various properties and child nodes */
> >>>>> +			};
> >>>>> +	};
> >>>>> +};
> >>>>> 
> >>>> 
> >>>> No.
> >>>> 
> >>>> That only works if the overlay is applied in a single platform.
> >>>> 
> >>>> I have working cases where the same overlay is applied on a ppc and a x86
> >>>> platform.
> >>> 
> >>> Huh?  How so..
> >>> 
> >> 
> >> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
> >> 
> >> Think carrier boards on enterprise routers, plugging to a main board
> >> that’s either ppc or x86 (or anything else for that matter).
> > 
> > Sorry, I wasn't clear.  I have no problem believing overlays can be
> > applied on multiple platforms.
> > 
> > What I can't see is how Frank's format breaks that.  AFAICT it
> > contains exactly the same information in a simpler encoding.
> > 
> 
> It breaks it because it’s missing the target property.
> 
> The layout of the base tree is not going to be the same in different
> platforms, so in the above example ‘ocp’ would not exist in x86 for
> instance.

I think you're misinterpreting Frank's suggestion.  As I understand it
the node names of the top level nodes in his format aren't treated as
literal node names, but instead treated as label names which are
resolved similarly to the phandle external fixups.

Actually.. that is one serious problem with Frank's format, it doesn't
(easily) allow multiple fragments to be applied to the same target.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi David,

> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>> Hi David,
>> 
>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6rL0yjEaa7Ru@public.gmane.orgau> wrote:
>>> 
>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>> Hi David,
>>>> 
>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6vUpdFzICT1y@public.gmane.orgd.au> wrote:
>>>>> 
>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>> Hi Frank,
>>>>>> 
>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list-Re5JQEeQqe8@public.gmane.orgm> wrote:
>>>>>>> 
>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>> plugins and options.
>>>>>>>> 
>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>> ---
>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>>>>>>>> 1 file changed, 318 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..d5b841e
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/dt-object-internal.txt
>>>>>>>> @@ -0,0 +1,318 @@
>>>>>>>> +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 */
>>>>>>>> +			}
>>>>>>> 
>>>>>>>                     };
>>>>>>> 
>>>>>>>> +		};
>>>>>>>> +	};
>>>>>>>> +};
>>>>>>> 
>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>> extra value they provide.
>>>>>>> 
>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>> was used for the target node):
>>>>>>> 
>>>>>>> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
>>>>>>> +/ {
>>>>>>> +	....	/* various properties for loader use; i.e. part id etc. */
>>>>>>> +	ocp {
>>>>>>> +			/* bar peripheral */
>>>>>>> +			bar {
>>>>>>> +				compatible = "corp,bar";
>>>>>>> +				... /* various properties and child nodes */
>>>>>>> +			};
>>>>>>> +	};
>>>>>>> +};
>>>>>>> 
>>>>>> 
>>>>>> No.
>>>>>> 
>>>>>> That only works if the overlay is applied in a single platform.
>>>>>> 
>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>> platform.
>>>>> 
>>>>> Huh?  How so..
>>>>> 
>>>> 
>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>> 
>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>> that’s either ppc or x86 (or anything else for that matter).
>>> 
>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>> applied on multiple platforms.
>>> 
>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>> contains exactly the same information in a simpler encoding.
>>> 
>> 
>> It breaks it because it’s missing the target property.
>> 
>> The layout of the base tree is not going to be the same in different
>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>> instance.
> 
> I think you're misinterpreting Frank's suggestion.  As I understand it
> the node names of the top level nodes in his format aren't treated as
> literal node names, but instead treated as label names which are
> resolved similarly to the phandle external fixups.
> 
> Actually.. that is one serious problem with Frank's format, it doesn't
> (easily) allow multiple fragments to be applied to the same target.
> 

Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
I queued with multiple target support.

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

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Rob Herring]

On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Hi David,
>
>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6iEOx6SR5NKn@public.gmane.orgu> wrote:
>>
>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>> Hi David,
>>>
>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6ryscVyMRj84@public.gmane.org.au> wrote:
>>>>
>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>> Hi David,
>>>>>
>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6g@public.gmane.orgid.au> wrote:
>>>>>>
>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>> Hi Frank,
>>>>>>>
>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>
>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>> plugins and options.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>> ---
>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>>>>>>>>> 1 file changed, 318 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..d5b841e
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/dt-object-internal.txt
>>>>>>>>> @@ -0,0 +1,318 @@
>>>>>>>>> +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 */
>>>>>>>>> +                      }
>>>>>>>>
>>>>>>>>                     };
>>>>>>>>
>>>>>>>>> +              };
>>>>>>>>> +      };
>>>>>>>>> +};
>>>>>>>>
>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>> extra value they provide.
>>>>>>>>
>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>> was used for the target node):
>>>>>>>>
>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>> +/ {
>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>> +       ocp {
>>>>>>>> +                       /* bar peripheral */
>>>>>>>> +                       bar {
>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>> +                       };
>>>>>>>> +       };
>>>>>>>> +};
>>>>>>>>
>>>>>>>
>>>>>>> No.
>>>>>>>
>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>
>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>> platform.
>>>>>>
>>>>>> Huh?  How so..
>>>>>>
>>>>>
>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>
>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>
>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>> applied on multiple platforms.
>>>>
>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>> contains exactly the same information in a simpler encoding.
>>>>
>>>
>>> It breaks it because it’s missing the target property.
>>>
>>> The layout of the base tree is not going to be the same in different
>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>> instance.
>>
>> I think you're misinterpreting Frank's suggestion.  As I understand it
>> the node names of the top level nodes in his format aren't treated as
>> literal node names, but instead treated as label names which are
>> resolved similarly to the phandle external fixups.
>>
>> Actually.. that is one serious problem with Frank's format, it doesn't
>> (easily) allow multiple fragments to be applied to the same target.
>>
>
> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
> I queued with multiple target support.

Queued implies accepted which they are not. The multiple ways of
expressing targets bothers me. Upstream still has no external
interface to overlays, so I think there is still room to change things
if we decide it is worthwhile. Better now than stuck with something
forever.

I too was wondering about the current syntax before this thread
started. We have 2 levels of nodes before we get to any useful
information with the current syntax.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand]

Hi Pantelis,

On 5/26/2016 6:49 AM, Rob Herring wrote:
> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> Hi David,
>>
>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6rL0yjEaa7Ru@public.gmane.orgau> wrote:
>>>
>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>>> Hi David,
>>>>
>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6vUpdFzICT1y@public.gmane.orgd.au> wrote:
>>>>>
>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>>> Hi David,
>>>>>>
>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnQMXKaNCjofrA@public.gmane.org.id.au> wrote:
>>>>>>>
>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>>> Hi Frank,
>>>>>>>>
>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>>> plugins and options.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>>> ---
>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++

< snip >

>>>>>>>>>> +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 */
>>>>>>>>>> +                      }
>>>>>>>>>
>>>>>>>>>                     };
>>>>>>>>>
>>>>>>>>>> +              };
>>>>>>>>>> +      };
>>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>>> extra value they provide.
>>>>>>>>>
>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>>> was used for the target node):
>>>>>>>>>
>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>>> +/ {
>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>> +       ocp {
>>>>>>>>> +                       /* bar peripheral */
>>>>>>>>> +                       bar {
>>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>>> +                       };
>>>>>>>>> +       };
>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>
>>>>>>>> No.
>>>>>>>>
>>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>>
>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>>> platform.
>>>>>>>
>>>>>>> Huh?  How so..
>>>>>>>
>>>>>>
>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>>
>>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>>
>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>>> applied on multiple platforms.
>>>>>
>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>>> contains exactly the same information in a simpler encoding.
>>>>>
>>>>
>>>> It breaks it because it’s missing the target property.
>>>>
>>>> The layout of the base tree is not going to be the same in different
>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>>> instance.
>>>
>>> I think you're misinterpreting Frank's suggestion.  As I understand it
>>> the node names of the top level nodes in his format aren't treated as
>>> literal node names, but instead treated as label names which are
>>> resolved similarly to the phandle external fixups.
>>>
>>> Actually.. that is one serious problem with Frank's format, it doesn't
>>> (easily) allow multiple fragments to be applied to the same target.
>>>
>>
>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
>> I queued with multiple target support.

OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
email from 4/27 (just to provide an easy link for everyone).  I'm still
trying to figure that out.

So other than that, am I missing something else about what extra
functionality the extra layers of nodes provides?


> Queued implies accepted which they are not. The multiple ways of
> expressing targets bothers me. Upstream still has no external
> interface to overlays, so I think there is still room to change things
> if we decide it is worthwhile. Better now than stuck with something
> forever.
> 
> I too was wondering about the current syntax before this thread
> started. We have 2 levels of nodes before we get to any useful
> information with the current syntax.
> 
> Rob
> 

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi Frank,

> On May 26, 2016, at 19:55 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> Hi Pantelis,
> 
> On 5/26/2016 6:49 AM, Rob Herring wrote:
>> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>> Hi David,
>>> 
>>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6ryscVyMRj84@public.gmane.org.au> wrote:
>>>> 
>>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>>>> Hi David,
>>>>> 
>>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6g@public.gmane.orgid.au> wrote:
>>>>>> 
>>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>>>> Hi David,
>>>>>>> 
>>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnSZP5U4TJ/6Hg@public.gmane.orgr.id.au> wrote:
>>>>>>>> 
>>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>>>> Hi Frank,
>>>>>>>>> 
>>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>>>> plugins and options.
>>>>>>>>>>> 
>>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>>>> ---
>>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
> 
> < snip >
> 
>>>>>>>>>>> +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 */
>>>>>>>>>>> +                      }
>>>>>>>>>> 
>>>>>>>>>>                    };
>>>>>>>>>> 
>>>>>>>>>>> +              };
>>>>>>>>>>> +      };
>>>>>>>>>>> +};
>>>>>>>>>> 
>>>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>>>> extra value they provide.
>>>>>>>>>> 
>>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>>>> was used for the target node):
>>>>>>>>>> 
>>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>>>> +/ {
>>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>>> +       ocp {
>>>>>>>>>> +                       /* bar peripheral */
>>>>>>>>>> +                       bar {
>>>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>>>> +                       };
>>>>>>>>>> +       };
>>>>>>>>>> +};
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> No.
>>>>>>>>> 
>>>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>>> 
>>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>>>> platform.
>>>>>>>> 
>>>>>>>> Huh?  How so..
>>>>>>>> 
>>>>>>> 
>>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>>> 
>>>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>>> 
>>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>>>> applied on multiple platforms.
>>>>>> 
>>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>>>> contains exactly the same information in a simpler encoding.
>>>>>> 
>>>>> 
>>>>> It breaks it because it’s missing the target property.
>>>>> 
>>>>> The layout of the base tree is not going to be the same in different
>>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>>>> instance.
>>>> 
>>>> I think you're misinterpreting Frank's suggestion.  As I understand it
>>>> the node names of the top level nodes in his format aren't treated as
>>>> literal node names, but instead treated as label names which are
>>>> resolved similarly to the phandle external fixups.
>>>> 
>>>> Actually.. that is one serious problem with Frank's format, it doesn't
>>>> (easily) allow multiple fragments to be applied to the same target.
>>>> 
>>> 
>>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
>>> I queued with multiple target support.
> 
> OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
> email from 4/27 (just to provide an easy link for everyone).  I'm still
> trying to figure that out.
> 
> So other than that, am I missing something else about what extra
> functionality the extra layers of nodes provides?
> 

No, I’m talking about the new target options patchset.

"of: overlays: New target methods” & in particular

"of: overlay: Implement target index support"

> 
>> Queued implies accepted which they are not. The multiple ways of
>> expressing targets bothers me. Upstream still has no external
>> interface to overlays, so I think there is still room to change things
>> if we decide it is worthwhile. Better now than stuck with something
>> forever.
>> 
>> I too was wondering about the current syntax before this thread
>> started. We have 2 levels of nodes before we get to any useful
>> information with the current syntax.
>> 

That’s on purpose. The first level is to contain load manager specific details,
i.e. part-numbers and other platform specific properties.

The second level contains the per fragment properties (at the moment targets).

>> Rob
>> 

Regards

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand]

On 5/26/2016 10:09 AM, Pantelis Antoniou wrote:
> Hi Frank,
> 
>> On May 26, 2016, at 19:55 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Hi Pantelis,
>>
>> On 5/26/2016 6:49 AM, Rob Herring wrote:
>>> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
>>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>>> Hi David,
>>>>
>>>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6vUpdFzICT1y@public.gmane.orgd.au> wrote:
>>>>>
>>>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>>>>> Hi David,
>>>>>>
>>>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnQMXKaNCjofrA@public.gmane.org.id.au> wrote:
>>>>>>>
>>>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnSXP4QZYUMu1g@public.gmane.orgar.id.au> wrote:
>>>>>>>>>
>>>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>>>>> Hi Frank,
>>>>>>>>>>
>>>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>>>>> plugins and options.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>>
>> < snip >
>>
>>>>>>>>>>>> +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 */
>>>>>>>>>>>> +                      }
>>>>>>>>>>>
>>>>>>>>>>>                    };
>>>>>>>>>>>
>>>>>>>>>>>> +              };
>>>>>>>>>>>> +      };
>>>>>>>>>>>> +};
>>>>>>>>>>>
>>>>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>>>>> extra value they provide.
>>>>>>>>>>>
>>>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>>>>> was used for the target node):
>>>>>>>>>>>
>>>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>>>>> +/ {
>>>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>>>> +       ocp {
>>>>>>>>>>> +                       /* bar peripheral */
>>>>>>>>>>> +                       bar {
>>>>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>>>>> +                       };
>>>>>>>>>>> +       };
>>>>>>>>>>> +};
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> No.
>>>>>>>>>>
>>>>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>>>>
>>>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>>>>> platform.
>>>>>>>>>
>>>>>>>>> Huh?  How so..
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>>>>
>>>>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>>>>
>>>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>>>>> applied on multiple platforms.
>>>>>>>
>>>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>>>>> contains exactly the same information in a simpler encoding.
>>>>>>>
>>>>>>
>>>>>> It breaks it because it’s missing the target property.
>>>>>>
>>>>>> The layout of the base tree is not going to be the same in different
>>>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>>>>> instance.
>>>>>
>>>>> I think you're misinterpreting Frank's suggestion.  As I understand it
>>>>> the node names of the top level nodes in his format aren't treated as
>>>>> literal node names, but instead treated as label names which are
>>>>> resolved similarly to the phandle external fixups.
>>>>>
>>>>> Actually.. that is one serious problem with Frank's format, it doesn't
>>>>> (easily) allow multiple fragments to be applied to the same target.
>>>>>
>>>>
>>>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
>>>> I queued with multiple target support.
>>
>> OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
>> email from 4/27 (just to provide an easy link for everyone).  I'm still
>> trying to figure that out.
>>
>> So other than that, am I missing something else about what extra
>> functionality the extra layers of nodes provides?
>>
> 
> No, I’m talking about the new target options patchset.
> 
> "of: overlays: New target methods” & in particular
> 
> "of: overlay: Implement target index support"

Thanks for the pointer.  I don't think the target options approach
is the way to handle the issue (see my reply a couple of minutes
ago in that thread).

> 
>>
>>> Queued implies accepted which they are not. The multiple ways of
>>> expressing targets bothers me. Upstream still has no external
>>> interface to overlays, so I think there is still room to change things
>>> if we decide it is worthwhile. Better now than stuck with something
>>> forever.
>>>
>>> I too was wondering about the current syntax before this thread
>>> started. We have 2 levels of nodes before we get to any useful
>>> information with the current syntax.
>>>
> 
> That’s on purpose. The first level is to contain load manager specific details,
> i.e. part-numbers and other platform specific properties.
> 
> The second level contains the per fragment properties (at the moment targets).

My suggestion removed the target property.

What other properties are you envisioning?  (Looking for the architectural
vision that you have.)

If load manager specific details are appropriate in the devicetree (a whole
different discussion) then maybe a /chosen/load-manager node could exist to
hold them instead of putting them in /, where the patch currently locates
"/* various properties for loader use; i.e. part id etc. */".

Re: [PATCH v7 4/5] dtc: Plugin and fixup support

[David Gibson]

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

On Tue, May 24, 2016 at 08:50:38PM +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>

So, I think I've identified the underlying thing which was bothering
me about these patches.

With the new dynamic patching stuff, "overlays" (for want of a better
term) now have a real existence both in the dts source format, and in
the dtb object format.  However, these patches don't give them a
concrete, explicit representation within dtc itself - instead we just
kind of mangle one representation to the other as we're parsing.  I
think this is a mistaken approach.

I'm toying with some patches to give overlays a full representation in
dtc which I think will handle these cases better - and allow for
easier experimentation with different possible ways of encoding the
overlays.

One side point - writing plugins in dts format leads to an irritating
little ambiguity in the grammar.  Well, not an ambiguity technically,
but a place where we need more lookahead than normal, meaning we get
shift/reduce conflicts.  It arises because both memreserves and
overlays can have a label in front of them.  So, if we see a label as
our next token after the version tag, we don't know if a memreserve or
overlay is coming next, so the parser doesn't know which path to go
down (with a single token lookahead).  We could handle it with
%glr-parser, of course, but I have been trying to avoid that.  I think
this will apply both with your patches and with the approach I'm
working on - not sure what to do about it yet.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Pantelis Antoniou]

Hi Frank,

> On May 27, 2016, at 00:31 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On 5/26/2016 10:09 AM, Pantelis Antoniou wrote:
>> Hi Frank,
>> 
>>> On May 26, 2016, at 19:55 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 
>>> Hi Pantelis,
>>> 
>>> On 5/26/2016 6:49 AM, Rob Herring wrote:
>>>> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
>>>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>>>> Hi David,
>>>>> 
>>>>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6g@public.gmane.orgid.au> wrote:
>>>>>> 
>>>>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
>>>>>>> Hi David,
>>>>>>> 
>>>>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnSZP5U4TJ/6Hg@public.gmane.orgr.id.au> wrote:
>>>>>>>> 
>>>>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
>>>>>>>>> Hi David,
>>>>>>>>> 
>>>>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnSoZLXPBrMMyA@public.gmane.orgear.id.au> wrote:
>>>>>>>>>> 
>>>>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
>>>>>>>>>>> Hi Frank,
>>>>>>>>>>> 
>>>>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
>>>>>>>>>>>>> Provides the document explaining the internal mechanics of
>>>>>>>>>>>>> plugins and options.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>>> 
>>> < snip >
>>> 
>>>>>>>>>>>>> +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 */
>>>>>>>>>>>>> +                      }
>>>>>>>>>>>> 
>>>>>>>>>>>>                   };
>>>>>>>>>>>> 
>>>>>>>>>>>>> +              };
>>>>>>>>>>>>> +      };
>>>>>>>>>>>>> +};
>>>>>>>>>>>> 
>>>>>>>>>>>> Other than the fact that the above syntax is already in the Linux
>>>>>>>>>>>> kernel overlay implementation, is there a need for the target
>>>>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
>>>>>>>>>>>> extra value they provide.
>>>>>>>>>>>> 
>>>>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
>>>>>>>>>>>> multi-node target path example this would be more complex unless a label
>>>>>>>>>>>> was used for the target node):
>>>>>>>>>>>> 
>>>>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
>>>>>>>>>>>> +/ {
>>>>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
>>>>>>>>>>>> +       ocp {
>>>>>>>>>>>> +                       /* bar peripheral */
>>>>>>>>>>>> +                       bar {
>>>>>>>>>>>> +                               compatible = "corp,bar";
>>>>>>>>>>>> +                               ... /* various properties and child nodes */
>>>>>>>>>>>> +                       };
>>>>>>>>>>>> +       };
>>>>>>>>>>>> +};
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> No.
>>>>>>>>>>> 
>>>>>>>>>>> That only works if the overlay is applied in a single platform.
>>>>>>>>>>> 
>>>>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
>>>>>>>>>>> platform.
>>>>>>>>>> 
>>>>>>>>>> Huh?  How so..
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
>>>>>>>>> 
>>>>>>>>> Think carrier boards on enterprise routers, plugging to a main board
>>>>>>>>> that’s either ppc or x86 (or anything else for that matter).
>>>>>>>> 
>>>>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
>>>>>>>> applied on multiple platforms.
>>>>>>>> 
>>>>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
>>>>>>>> contains exactly the same information in a simpler encoding.
>>>>>>>> 
>>>>>>> 
>>>>>>> It breaks it because it’s missing the target property.
>>>>>>> 
>>>>>>> The layout of the base tree is not going to be the same in different
>>>>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
>>>>>>> instance.
>>>>>> 
>>>>>> I think you're misinterpreting Frank's suggestion.  As I understand it
>>>>>> the node names of the top level nodes in his format aren't treated as
>>>>>> literal node names, but instead treated as label names which are
>>>>>> resolved similarly to the phandle external fixups.
>>>>>> 
>>>>>> Actually.. that is one serious problem with Frank's format, it doesn't
>>>>>> (easily) allow multiple fragments to be applied to the same target.
>>>>>> 
>>>>> 
>>>>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
>>>>> I queued with multiple target support.
>>> 
>>> OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
>>> email from 4/27 (just to provide an easy link for everyone).  I'm still
>>> trying to figure that out.
>>> 
>>> So other than that, am I missing something else about what extra
>>> functionality the extra layers of nodes provides?
>>> 
>> 
>> No, I’m talking about the new target options patchset.
>> 
>> "of: overlays: New target methods” & in particular
>> 
>> "of: overlay: Implement target index support"
> 
> Thanks for the pointer.  I don't think the target options approach
> is the way to handle the issue (see my reply a couple of minutes
> ago in that thread).
> 
>> 
>>> 
>>>> Queued implies accepted which they are not. The multiple ways of
>>>> expressing targets bothers me. Upstream still has no external
>>>> interface to overlays, so I think there is still room to change things
>>>> if we decide it is worthwhile. Better now than stuck with something
>>>> forever.
>>>> 
>>>> I too was wondering about the current syntax before this thread
>>>> started. We have 2 levels of nodes before we get to any useful
>>>> information with the current syntax.
>>>> 
>> 
>> That’s on purpose. The first level is to contain load manager specific details,
>> i.e. part-numbers and other platform specific properties.
>> 
>> The second level contains the per fragment properties (at the moment targets).
> 
> My suggestion removed the target property.
> 

Removing the target property requires using a loader. That may be fine
in general and in fact that’s what the target root methods do (setting
the target root to ‘/‘ makes them equivalent to your version).

> What other properties are you envisioning?  (Looking for the architectural
> vision that you have.)
> 

Oh, there are a lot of properties that can be provided.

For instance you can declare manufacturing info (like part numbers, version numbers,
serial numbers that can be used for quirking). You can declare things like load order
when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
should be disabled when an add on cape with display capability is attached).
You can declare resources (i.e. pins or power draw figures) to make a decision
whether enabling an expansion board is safe.

I’m sure more ideas will come when we put it into wide-spread use.  

> If load manager specific details are appropriate in the devicetree (a whole
> different discussion) then maybe a /chosen/load-manager node could exist to
> hold them instead of putting them in /, where the patch currently locates
> "/* various properties for loader use; i.e. part id etc. */".

Oh yes. But that’s something for us to figure out.

Regards

— Pantelis

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Fri, May 27, 2016 at 05:52:41PM +0300, Pantelis Antoniou wrote:
> Hi Frank,
> 
> > On May 27, 2016, at 00:31 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > On 5/26/2016 10:09 AM, Pantelis Antoniou wrote:
> >> Hi Frank,
> >> 
> >>> On May 26, 2016, at 19:55 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>> 
> >>> Hi Pantelis,
> >>> 
> >>> On 5/26/2016 6:49 AM, Rob Herring wrote:
> >>>> On Thu, May 26, 2016 at 2:16 AM, Pantelis Antoniou
> >>>> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> >>>>> Hi David,
> >>>>> 
> >>>>>> On May 26, 2016, at 10:12 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6ryscVyMRj84@public.gmane.org.au> wrote:
> >>>>>> 
> >>>>>> On Thu, May 26, 2016 at 09:36:02AM +0300, Pantelis Antoniou wrote:
> >>>>>>> Hi David,
> >>>>>>> 
> >>>>>>>> On May 26, 2016, at 09:33 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6g@public.gmane.orgid.au> wrote:
> >>>>>>>> 
> >>>>>>>> On Thu, May 26, 2016 at 09:31:20AM +0300, Pantelis Antoniou wrote:
> >>>>>>>>> Hi David,
> >>>>>>>>> 
> >>>>>>>>>> On May 26, 2016, at 09:28 , David Gibson <david-xT8FGy+AXnSZP5U4TJ/6Hg@public.gmane.orgr.id.au> wrote:
> >>>>>>>>>> 
> >>>>>>>>>> On Thu, May 26, 2016 at 09:14:49AM +0300, Pantelis Antoniou wrote:
> >>>>>>>>>>> Hi Frank,
> >>>>>>>>>>> 
> >>>>>>>>>>>> On May 25, 2016, at 22:13 , Frank Rowand <frowand.list@gmail.com> wrote:
> >>>>>>>>>>>> 
> >>>>>>>>>>>> On 5/24/2016 10:50 AM, Pantelis Antoniou wrote:
> >>>>>>>>>>>>> Provides the document explaining the internal mechanics of
> >>>>>>>>>>>>> plugins and options.
> >>>>>>>>>>>>> 
> >>>>>>>>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> >>>>>>>>>>>>> ---
> >>>>>>>>>>>>> Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
> >>> 
> >>> < snip >
> >>> 
> >>>>>>>>>>>>> +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 */
> >>>>>>>>>>>>> +                      }
> >>>>>>>>>>>> 
> >>>>>>>>>>>>                   };
> >>>>>>>>>>>> 
> >>>>>>>>>>>>> +              };
> >>>>>>>>>>>>> +      };
> >>>>>>>>>>>>> +};
> >>>>>>>>>>>> 
> >>>>>>>>>>>> Other than the fact that the above syntax is already in the Linux
> >>>>>>>>>>>> kernel overlay implementation, is there a need for the target
> >>>>>>>>>>>> property and the __overlay__ node?  I haven't figured out what
> >>>>>>>>>>>> extra value they provide.
> >>>>>>>>>>>> 
> >>>>>>>>>>>> Without those added, the overlay dts becomes simpler (though for a
> >>>>>>>>>>>> multi-node target path example this would be more complex unless a label
> >>>>>>>>>>>> was used for the target node):
> >>>>>>>>>>>> 
> >>>>>>>>>>>> +/dts-v1/ /plugin/;     /* allow undefined references and record them */
> >>>>>>>>>>>> +/ {
> >>>>>>>>>>>> +       ....    /* various properties for loader use; i.e. part id etc. */
> >>>>>>>>>>>> +       ocp {
> >>>>>>>>>>>> +                       /* bar peripheral */
> >>>>>>>>>>>> +                       bar {
> >>>>>>>>>>>> +                               compatible = "corp,bar";
> >>>>>>>>>>>> +                               ... /* various properties and child nodes */
> >>>>>>>>>>>> +                       };
> >>>>>>>>>>>> +       };
> >>>>>>>>>>>> +};
> >>>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>>>> No.
> >>>>>>>>>>> 
> >>>>>>>>>>> That only works if the overlay is applied in a single platform.
> >>>>>>>>>>> 
> >>>>>>>>>>> I have working cases where the same overlay is applied on a ppc and a x86
> >>>>>>>>>>> platform.
> >>>>>>>>>> 
> >>>>>>>>>> Huh?  How so..
> >>>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> Yes, it does work. Yes it’s being used right now. It is a very valid use case.
> >>>>>>>>> 
> >>>>>>>>> Think carrier boards on enterprise routers, plugging to a main board
> >>>>>>>>> that’s either ppc or x86 (or anything else for that matter).
> >>>>>>>> 
> >>>>>>>> Sorry, I wasn't clear.  I have no problem believing overlays can be
> >>>>>>>> applied on multiple platforms.
> >>>>>>>> 
> >>>>>>>> What I can't see is how Frank's format breaks that.  AFAICT it
> >>>>>>>> contains exactly the same information in a simpler encoding.
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> It breaks it because it’s missing the target property.
> >>>>>>> 
> >>>>>>> The layout of the base tree is not going to be the same in different
> >>>>>>> platforms, so in the above example ‘ocp’ would not exist in x86 for
> >>>>>>> instance.
> >>>>>> 
> >>>>>> I think you're misinterpreting Frank's suggestion.  As I understand it
> >>>>>> the node names of the top level nodes in his format aren't treated as
> >>>>>> literal node names, but instead treated as label names which are
> >>>>>> resolved similarly to the phandle external fixups.
> >>>>>> 
> >>>>>> Actually.. that is one serious problem with Frank's format, it doesn't
> >>>>>> (easily) allow multiple fragments to be applied to the same target.
> >>>>>> 
> >>>>> 
> >>>>> Ugh, yeah I misinterpreted that. Still, it is not going to work with the patches
> >>>>> I queued with multiple target support.
> >>> 
> >>> OK, so you are talking about the "[RFC] of: Portable Device Tree connector"
> >>> email from 4/27 (just to provide an easy link for everyone).  I'm still
> >>> trying to figure that out.
> >>> 
> >>> So other than that, am I missing something else about what extra
> >>> functionality the extra layers of nodes provides?
> >>> 
> >> 
> >> No, I’m talking about the new target options patchset.
> >> 
> >> "of: overlays: New target methods” & in particular
> >> 
> >> "of: overlay: Implement target index support"
> > 
> > Thanks for the pointer.  I don't think the target options approach
> > is the way to handle the issue (see my reply a couple of minutes
> > ago in that thread).
> > 
> >> 
> >>> 
> >>>> Queued implies accepted which they are not. The multiple ways of
> >>>> expressing targets bothers me. Upstream still has no external
> >>>> interface to overlays, so I think there is still room to change things
> >>>> if we decide it is worthwhile. Better now than stuck with something
> >>>> forever.
> >>>> 
> >>>> I too was wondering about the current syntax before this thread
> >>>> started. We have 2 levels of nodes before we get to any useful
> >>>> information with the current syntax.
> >>>> 
> >> 
> >> That’s on purpose. The first level is to contain load manager specific details,
> >> i.e. part-numbers and other platform specific properties.
> >> 
> >> The second level contains the per fragment properties (at the moment targets).
> > 
> > My suggestion removed the target property.
> 
> Removing the target property requires using a loader. That may be fine
> in general and in fact that’s what the target root methods do (setting
> the target root to ‘/‘ makes them equivalent to your version).

I can't quite make sense of that comment.  No matter the encoding,
you're always going to need some sort of a loader.

> > What other properties are you envisioning?  (Looking for the architectural
> > vision that you have.)
> > 
> 
> Oh, there are a lot of properties that can be provided.
> 
> For instance you can declare manufacturing info (like part numbers, version numbers,
> serial numbers that can be used for quirking). You can declare things like load order
> when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
> should be disabled when an add on cape with display capability is attached).
> You can declare resources (i.e. pins or power draw figures) to make a decision
> whether enabling an expansion board is safe.
> 
> I’m sure more ideas will come when we put it into wide-spread use.  

Yeah.  I'm not entirely sure I'm convinced by the specific examples
given so far.  However, in general I can see the value in providing a
way we can extend to add more metadata.  The two level structure with
__overlay__ gives us that, whereas the one level approach doesn't.

> > If load manager specific details are appropriate in the devicetree (a whole
> > different discussion) then maybe a /chosen/load-manager node could exist to
> > hold them instead of putting them in /, where the patch currently locates
> > "/* various properties for loader use; i.e. part id etc. */".
> 
> Oh yes. But that’s something for us to figure out.
> 
> 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 --]

Re: [PATCH v7 4/5] dtc: Plugin and fixup support

[David Gibson]

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

On Fri, May 27, 2016 at 05:33:06PM +1000, David Gibson wrote:
> On Tue, May 24, 2016 at 08:50:38PM +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>
> 
> So, I think I've identified the underlying thing which was bothering
> me about these patches.
> 
> With the new dynamic patching stuff, "overlays" (for want of a better
> term) now have a real existence both in the dts source format, and in
> the dtb object format.  However, these patches don't give them a
> concrete, explicit representation within dtc itself - instead we just
> kind of mangle one representation to the other as we're parsing.  I
> think this is a mistaken approach.
> 
> I'm toying with some patches to give overlays a full representation in
> dtc which I think will handle these cases better - and allow for
> easier experimentation with different possible ways of encoding the
> overlays.
> 
> One side point - writing plugins in dts format leads to an irritating
> little ambiguity in the grammar.  Well, not an ambiguity technically,
> but a place where we need more lookahead than normal, meaning we get
> shift/reduce conflicts.  It arises because both memreserves and
> overlays can have a label in front of them.  So, if we see a label as
> our next token after the version tag, we don't know if a memreserve or
> overlay is coming next, so the parser doesn't know which path to go
> down (with a single token lookahead).  We could handle it with
> %glr-parser, of course, but I have been trying to avoid that.  I think
> this will apply both with your patches and with the approach I'm
> working on - not sure what to do about it yet.

I now have a first cut at said experiments, see:
    https://github.com/dgibson/dtc/tree/overlay

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

Re: [PATCH v7 4/5] dtc: Plugin and fixup support

[Pantelis Antoniou]

Hi David,

> On May 31, 2016, at 08:06 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Fri, May 27, 2016 at 05:33:06PM +1000, David Gibson wrote:
>> On Tue, May 24, 2016 at 08:50:38PM +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>
>> 
>> So, I think I've identified the underlying thing which was bothering
>> me about these patches.
>> 
>> With the new dynamic patching stuff, "overlays" (for want of a better
>> term) now have a real existence both in the dts source format, and in
>> the dtb object format.  However, these patches don't give them a
>> concrete, explicit representation within dtc itself - instead we just
>> kind of mangle one representation to the other as we're parsing.  I
>> think this is a mistaken approach.
>> 
>> I'm toying with some patches to give overlays a full representation in
>> dtc which I think will handle these cases better - and allow for
>> easier experimentation with different possible ways of encoding the
>> overlays.
>> 
>> One side point - writing plugins in dts format leads to an irritating
>> little ambiguity in the grammar.  Well, not an ambiguity technically,
>> but a place where we need more lookahead than normal, meaning we get
>> shift/reduce conflicts.  It arises because both memreserves and
>> overlays can have a label in front of them.  So, if we see a label as
>> our next token after the version tag, we don't know if a memreserve or
>> overlay is coming next, so the parser doesn't know which path to go
>> down (with a single token lookahead).  We could handle it with
>> %glr-parser, of course, but I have been trying to avoid that.  I think
>> this will apply both with your patches and with the approach I'm
>> working on - not sure what to do about it yet.
> 
> I now have a first cut at said experiments, see:
>    https://github.com/dgibson/dtc/tree/overlay
> 

Rebased my work and will submit again a couple of minutes.

Everything works besides a small niggle with the overlay syntax.

You cannot get rid of the basetree token. So…

&foo { }; 

Does not work for an overlay, you need a dummy basetree 

/ { }; &foo { };

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

Re: [PATCH v7 4/5] dtc: Plugin and fixup support

[David Gibson]

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

On Thu, Jun 02, 2016 at 08:13:51PM +0300, Pantelis Antoniou wrote:
> Hi David,
> 
> > On May 31, 2016, at 08:06 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Fri, May 27, 2016 at 05:33:06PM +1000, David Gibson wrote:
> >> On Tue, May 24, 2016 at 08:50:38PM +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>
> >> 
> >> So, I think I've identified the underlying thing which was bothering
> >> me about these patches.
> >> 
> >> With the new dynamic patching stuff, "overlays" (for want of a better
> >> term) now have a real existence both in the dts source format, and in
> >> the dtb object format.  However, these patches don't give them a
> >> concrete, explicit representation within dtc itself - instead we just
> >> kind of mangle one representation to the other as we're parsing.  I
> >> think this is a mistaken approach.
> >> 
> >> I'm toying with some patches to give overlays a full representation in
> >> dtc which I think will handle these cases better - and allow for
> >> easier experimentation with different possible ways of encoding the
> >> overlays.
> >> 
> >> One side point - writing plugins in dts format leads to an irritating
> >> little ambiguity in the grammar.  Well, not an ambiguity technically,
> >> but a place where we need more lookahead than normal, meaning we get
> >> shift/reduce conflicts.  It arises because both memreserves and
> >> overlays can have a label in front of them.  So, if we see a label as
> >> our next token after the version tag, we don't know if a memreserve or
> >> overlay is coming next, so the parser doesn't know which path to go
> >> down (with a single token lookahead).  We could handle it with
> >> %glr-parser, of course, but I have been trying to avoid that.  I think
> >> this will apply both with your patches and with the approach I'm
> >> working on - not sure what to do about it yet.
> > 
> > I now have a first cut at said experiments, see:
> >    https://github.com/dgibson/dtc/tree/overlay
> > 
> 
> Rebased my work and will submit again a couple of minutes.
> 
> Everything works besides a small niggle with the overlay syntax.
> 
> You cannot get rid of the basetree token. So…
> 
> &foo { }; 
> 
> Does not work for an overlay, you need a dummy basetree 
> 
> / { }; &foo { };


Right, it was my intention to remove that restriction but it hits the
grammar complication discussed above.  I might have to bite the bullet
and turn on %glr-parser.

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

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand]

Hi Pantelis,

On 05/29/16 21:22, David Gibson wrote:

< massive snip >

>>> What other properties are you envisioning?  (Looking for the architectural
>>> vision that you have.)
>>>
>>
>> Oh, there are a lot of properties that can be provided.
>>
>> For instance you can declare manufacturing info (like part numbers, version numbers,
>> serial numbers that can be used for quirking). You can declare things like load order
>> when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
>> should be disabled when an add on cape with display capability is attached).
>> You can declare resources (i.e. pins or power draw figures) to make a decision
>> whether enabling an expansion board is safe.
>>
>> I’m sure more ideas will come when we put it into wide-spread use.  
> 
> Yeah.  I'm not entirely sure I'm convinced by the specific examples
> given so far.  However, in general I can see the value in providing a
> way we can extend to add more metadata.  The two level structure with
> __overlay__ gives us that, whereas the one level approach doesn't.

I'm not convinced about putting additional properties (beyond "target") in the
fragment nodes.  And I still find the two extra levels of "fragment" nodes and
"__overlay__" nodes extra complexity, and that the complexity is especially
unwelcome if the overlay dts is hand written (as the beagle cape overlays
that I have seen appear to be).  HOWEVER, I learned something new today that
makes me more comfortable with the two extra node levels.  Here is an example:

$ cat ex_1_overlay.dts 

/dts-v1/;

/plugin/;

&tree_1 {
	remote_prop = <0xfeedfeed &foo>;
};

$ dtc -@ -O dts ex_1_overlay.dts 
Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
/dts-v1/;

/ {

	fragment@0 {
		target = <0xffffffff>;

		__overlay__ {
			remote_prop = <0xfeedfeed 0xffffffff>;
		};
	};

	__symbols__ {
	};

	__fixups__ {
		tree_1 = "/fragment@0:target:0";
		foo = "/fragment@0/__overlay__:remote_prop:4";
	};

	__local_fixups__ {
	};
};

That example is using dtc from:
  url = https://github.com/pantoniou/dtc
  branch: dt-overlays8
  as of commit: 6f4db2fc2354

I do not know if that is current or not.

So the nice thing about that is that the overlay source file does not have to
provide the fragment and __overlay__ nodes.  They just magically appear in the
compiled blob.  Is that behavior that I can count on continuing to exist in dtc?

Note the warning about no reg property in /fragment@0.

The other issue with the device tree source in my example is that I don't think
there is a way to add extra properties to node "fragment@0" in the general case
where there are multiple fragments.  It _is_ possible to add a property as I
show in the next example, but it seems fragile to be able to count on the order
and names of fragments auto generated by dtc.  (And again, I don't really want
those extra properties anyway.)

Example of adding a property to fragment@0:

$ cat ex_1b_overlay.dts

/dts-v1/;

/plugin/;

&tree_1 {
	remote_prop = <0xfeedfeed &foo>;
};

/ {
	fragment@0 {
		another_fragment_prop = "frag property";
	};
};

$ dtc -@ -O dts ex_1b_overlay.dts 
Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
/dts-v1/;

/ {

	fragment@0 {
		target = <0xffffffff>;
		another_fragment_prop = "frag property";

		__overlay__ {
			remote_prop = <0xfeedfeed 0xffffffff>;
		};
	};

	__symbols__ {
	};

	__fixups__ {
		tree_1 = "/fragment@0:target:0";
		foo = "/fragment@0/__overlay__:remote_prop:4";
	};

	__local_fixups__ {
	};
};


>>> If load manager specific details are appropriate in the devicetree (a whole
>>> different discussion) then maybe a /chosen/load-manager node could exist to
>>> hold them instead of putting them in /, where the patch currently locates
>>> "/* various properties for loader use; i.e. part id etc. */".

-Frank

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[Frank Rowand]

Hi Pantelis,

On 05/29/16 21:22, David Gibson wrote:

< massive snip >

>>> What other properties are you envisioning?  (Looking for the architectural
>>> vision that you have.)
>>>
>>
>> Oh, there are a lot of properties that can be provided.
>>
>> For instance you can declare manufacturing info (like part numbers, version numbers,
>> serial numbers that can be used for quirking). You can declare things like load order
>> when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
>> should be disabled when an add on cape with display capability is attached).
>> You can declare resources (i.e. pins or power draw figures) to make a decision
>> whether enabling an expansion board is safe.
>>
>> I’m sure more ideas will come when we put it into wide-spread use.  
> 
> Yeah.  I'm not entirely sure I'm convinced by the specific examples
> given so far.  However, in general I can see the value in providing a
> way we can extend to add more metadata.  The two level structure with
> __overlay__ gives us that, whereas the one level approach doesn't.

I'm not convinced about putting additional properties (beyond "target") in the
fragment nodes.  And I still find the two extra levels of "fragment" nodes and
"__overlay__" nodes extra complexity, and that the complexity is especially
unwelcome if the overlay dts is hand written (as the beagle cape overlays
that I have seen appear to be).  HOWEVER, I learned something new today that
makes me more comfortable with the two extra node levels.  Here is an example:

$ cat ex_1_overlay.dts 

/dts-v1/;

/plugin/;

&tree_1 {
	remote_prop = <0xfeedfeed &foo>;
};

$ dtc -@ -O dts ex_1_overlay.dts 
Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
/dts-v1/;

/ {

	fragment@0 {
		target = <0xffffffff>;

		__overlay__ {
			remote_prop = <0xfeedfeed 0xffffffff>;
		};
	};

	__symbols__ {
	};

	__fixups__ {
		tree_1 = "/fragment@0:target:0";
		foo = "/fragment@0/__overlay__:remote_prop:4";
	};

	__local_fixups__ {
	};
};

That example is using dtc from:
  url = https://github.com/pantoniou/dtc
  branch: dt-overlays8
  as of commit: 6f4db2fc2354

I do not know if that is current or not.

So the nice thing about that is that the overlay source file does not have to
provide the fragment and __overlay__ nodes.  They just magically appear in the
compiled blob.  Is that behavior that I can count on continuing to exist in dtc?

Note the warning about no reg property in /fragment@0.

The other issue with the device tree source in my example is that I don't think
there is a way to add extra properties to node "fragment@0" in the general case
where there are multiple fragments.  It _is_ possible to add a property as I
show in the next example, but it seems fragile to be able to count on the order
and names of fragments auto generated by dtc.  (And again, I don't really want
those extra properties anyway.)

Example of adding a property to fragment@0:

$ cat ex_1b_overlay.dts

/dts-v1/;

/plugin/;

&tree_1 {
	remote_prop = <0xfeedfeed &foo>;
};

/ {
	fragment@0 {
		another_fragment_prop = "frag property";
	};
};

$ dtc -@ -O dts ex_1b_overlay.dts 
Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
/dts-v1/;

/ {

	fragment@0 {
		target = <0xffffffff>;
		another_fragment_prop = "frag property";

		__overlay__ {
			remote_prop = <0xfeedfeed 0xffffffff>;
		};
	};

	__symbols__ {
	};

	__fixups__ {
		tree_1 = "/fragment@0:target:0";
		foo = "/fragment@0/__overlay__:remote_prop:4";
	};

	__local_fixups__ {
	};
};


>>> If load manager specific details are appropriate in the devicetree (a whole
>>> different discussion) then maybe a /chosen/load-manager node could exist to
>>> hold them instead of putting them in /, where the patch currently locates
>>> "/* various properties for loader use; i.e. part id etc. */".

-Frank

Re: [PATCH v7 3/5] dtc: Document the dynamic plugin internals

[David Gibson]

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

On Wed, Jun 29, 2016 at 07:59:13PM -0700, Frank Rowand wrote:
> Hi Pantelis,
> 
> On 05/29/16 21:22, David Gibson wrote:
> 
> < massive snip >
> 
> >>> What other properties are you envisioning?  (Looking for the architectural
> >>> vision that you have.)
> >>>
> >>
> >> Oh, there are a lot of properties that can be provided.
> >>
> >> For instance you can declare manufacturing info (like part numbers, version numbers,
> >> serial numbers that can be used for quirking). You can declare things like load order
> >> when you need precedence of overlays (i.e. on the bone the soldered on hdmi output
> >> should be disabled when an add on cape with display capability is attached).
> >> You can declare resources (i.e. pins or power draw figures) to make a decision
> >> whether enabling an expansion board is safe.
> >>
> >> I’m sure more ideas will come when we put it into wide-spread use.  
> > 
> > Yeah.  I'm not entirely sure I'm convinced by the specific examples
> > given so far.  However, in general I can see the value in providing a
> > way we can extend to add more metadata.  The two level structure with
> > __overlay__ gives us that, whereas the one level approach doesn't.
> 
> I'm not convinced about putting additional properties (beyond "target") in the
> fragment nodes.  And I still find the two extra levels of "fragment" nodes and
> "__overlay__" nodes extra complexity, and that the complexity is especially
> unwelcome if the overlay dts is hand written (as the beagle cape overlays
> that I have seen appear to be).  HOWEVER, I learned something new today that
> makes me more comfortable with the two extra node levels.  Here is an example:
> 
> $ cat ex_1_overlay.dts 
> 
> /dts-v1/;
> 
> /plugin/;
> 
> &tree_1 {
> 	remote_prop = <0xfeedfeed &foo>;
> };
> 
> $ dtc -@ -O dts ex_1_overlay.dts 
> Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
> /dts-v1/;
> 
> / {
> 
> 	fragment@0 {
> 		target = <0xffffffff>;
> 
> 		__overlay__ {
> 			remote_prop = <0xfeedfeed 0xffffffff>;
> 		};
> 	};
> 
> 	__symbols__ {
> 	};
> 
> 	__fixups__ {
> 		tree_1 = "/fragment@0:target:0";
> 		foo = "/fragment@0/__overlay__:remote_prop:4";
> 	};
> 
> 	__local_fixups__ {
> 	};
> };
> 
> That example is using dtc from:
>   url = https://github.com/pantoniou/dtc
>   branch: dt-overlays8
>   as of commit: 6f4db2fc2354
> 
> I do not know if that is current or not.
> 
> So the nice thing about that is that the overlay source file does not have to
> provide the fragment and __overlay__ nodes.  They just magically appear in the
> compiled blob.  Is that behavior that I can count on continuing to exist in dtc?

Yes.  There are still some unclear points about how we want to
integrate this into upstream dtc.  However, automatically translating
the existing dts overlay syntax into the dtb encoding, including
constructing the fragment@ and other special nodes is absolutely the
intention.

> Note the warning about no reg property in /fragment@0.

Yes - pretty much the next thing on my list when I get another chance
to look at this is how to properly apply checks in the preence of
overlays.  My intention is that some will be flagged as being run on
each overlay fragment individually, others only on the fully
constructed tree (and so not at all when in plugin mode).

This should address a number of problems with checks and overlays,
including this one.

> The other issue with the device tree source in my example is that I don't think
> there is a way to add extra properties to node "fragment@0" in the general case
> where there are multiple fragments.  It _is_ possible to add a property as I
> show in the next example, but it seems fragile to be able to count on the order
> and names of fragments auto generated by dtc.  (And again, I don't really want
> those extra properties anyway.)

Right, the below is indeed fragile.  I expect it will break once we've
fully implemented the upstream approach to overlays I'm intending -
that will delay resolution of the overlays until after the input is
fully parsed.

My expectation was that any extra properties in the fragment@ nodes
would be metadata and would therefore either be auto-generated by dtc,
or we'd create special dtc syntax to populate it if necessary.

> Example of adding a property to fragment@0:
> 
> $ cat ex_1b_overlay.dts
> 
> /dts-v1/;
> 
> /plugin/;
> 
> &tree_1 {
> 	remote_prop = <0xfeedfeed &foo>;
> };
> 
> / {
> 	fragment@0 {
> 		another_fragment_prop = "frag property";
> 	};
> };
> 
> $ dtc -@ -O dts ex_1b_overlay.dts 
> Warning (unit_address_vs_reg): Node /fragment@0 has a unit name, but no reg property
> /dts-v1/;
> 
> / {
> 
> 	fragment@0 {
> 		target = <0xffffffff>;
> 		another_fragment_prop = "frag property";
> 
> 		__overlay__ {
> 			remote_prop = <0xfeedfeed 0xffffffff>;
> 		};
> 	};
> 
> 	__symbols__ {
> 	};
> 
> 	__fixups__ {
> 		tree_1 = "/fragment@0:target:0";
> 		foo = "/fragment@0/__overlay__:remote_prop:4";
> 	};
> 
> 	__local_fixups__ {
> 	};
> };
> 
> 
> >>> If load manager specific details are appropriate in the devicetree (a whole
> >>> different discussion) then maybe a /chosen/load-manager node could exist to
> >>> hold them instead of putting them in /, where the patch currently locates
> >>> "/* various properties for loader use; i.e. part id etc. */".
> 
> -Frank
> 

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