[PATCH v11 0/4] dtc: Dynamic DT support

devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v11 0/4] dtc: Dynamic DT support
@ 2016-11-28 16:05 Pantelis Antoniou
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

This patchset adds Dynamic DT support in the DTC compiler
as used in a number of boards like the beaglebone/rpi/chip and others.

The first patch documents the internals of overlay generation, while
the second one adds dynamic object/overlay support proper.

The third patch adds a test method that can is used by the subsequent
patch which adds a few overlay tests verifying operation.

The following 3 patches add support for the syntactic sugar version
of &foo { }; in a similar manner.

This patchset is against DTC mainline and is also available for a pull
request from https://github.com/pantoniou/dtc/tree/overlays

Regards

-- Pantelis

Changes since v10:
* Split out the syntactic sugar version of &foo { }; into a different
patches to make things clearer.
* Reworked a bit the arguments passed to fixup node generation method
making things simpler and utilize common methodology.
* Avoid string parsing the full path using the node walking instead for
local fixup generation.
* Added an option to suppress generation of fixup nodes on base trees.
* Added automatic generation of symbols and fixups when compiling a
plugin.
* Minor rework according to maintainer requests.

Changes since v9:
* Reversed -M switch to by default use new DTBO magic value.
* Removed global versionflags in the parser by using inherited
attributes.
* build_node instead of malloc at add_orphan_node().
* Do not use escape for path copy
* Do not generate /plugin/ when generating a dts file even when
the plugin flag is set..

Changes since v8:
* Removed extra member of boot_info in each node; passing boot_info
parameter to the check methods instead.
* Reworked yacc syntax that supports both old and new plugin syntax
* Added handling for new magic number (enabled by 'M' switch).
* Dropped dtbo/asmo formats.
* Added overlay testsuite.
* Addressed last version maintainer comments.

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

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

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

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

Changes since v3:
* Rebase to latest dtc version.

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

Pantelis Antoniou (7):
  dtc: Document the dynamic plugin internals
  dtc: Plugin and fixup support
  tests: Add check_path test
  tests: Add overlay tests
  overlay: Documentation for the overlay sugar syntax
  overlay: Add syntactic sugar version of overlays
  tests: Add a test for overlays syntactic sugar

 Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
 Documentation/manual.txt             |  29 +++-
 checks.c                             |   8 +-
 dtc-lexer.l                          |   5 +
 dtc-parser.y                         |  49 +++++-
 dtc.c                                |  51 +++++-
 dtc.h                                |  22 ++-
 fdtdump.c                            |   2 +-
 flattree.c                           |  17 +-
 fstree.c                             |   2 +-
 libfdt/fdt.c                         |   2 +-
 libfdt/fdt.h                         |   3 +-
 livetree.c                           | 230 ++++++++++++++++++++++++-
 tests/.gitignore                     |   1 +
 tests/Makefile.tests                 |   3 +-
 tests/check_path.c                   |  82 +++++++++
 tests/mangle-layout.c                |   7 +-
 tests/overlay_overlay_dtc.dts        |  76 +--------
 tests/overlay_overlay_dtc.dtsi       |  83 +++++++++
 tests/overlay_overlay_new_dtc.dts    |  11 ++
 tests/overlay_overlay_simple.dts     |  12 ++
 tests/run_tests.sh                   |  45 +++++
 22 files changed, 952 insertions(+), 106 deletions(-)
 create mode 100644 Documentation/dt-object-internal.txt
 create mode 100644 tests/check_path.c
 create mode 100644 tests/overlay_overlay_dtc.dtsi
 create mode 100644 tests/overlay_overlay_new_dtc.dts
 create mode 100644 tests/overlay_overlay_simple.dts

-- 
2.1.4

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

* [PATCH v11 1/7] dtc: Document the dynamic plugin internals
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-28 16:05   ` Pantelis Antoniou
  2016-11-28 16:05   ` [PATCH v11 2/7] dtc: Plugin and fixup support Pantelis Antoniou
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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 | 302 +++++++++++++++++++++++++++++++++++
 1 file changed, 302 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..026d4ee
--- /dev/null
+++ b/Documentation/dt-object-internal.txt
@@ -0,0 +1,302 @@
+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.
-- 
2.1.4

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

* [PATCH v11 2/7] dtc: Plugin and fixup support
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 1/7] dtc: Document the dynamic plugin internals Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
       [not found]     ` <1480349141-14145-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 3/7] tests: Add check_path test Pantelis Antoniou
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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.

While there is a new magic number for dynamic device tree/overlays blobs
it is by default enabled. Remember to use -M to generate compatible
blobs.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-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 |  29 ++++++-
 checks.c                 |   8 +-
 dtc-lexer.l              |   5 ++
 dtc-parser.y             |  29 ++++++-
 dtc.c                    |  51 +++++++++++-
 dtc.h                    |  21 ++++-
 fdtdump.c                |   2 +-
 flattree.c               |  17 ++--
 fstree.c                 |   2 +-
 libfdt/fdt.c             |   2 +-
 libfdt/fdt.h             |   3 +-
 livetree.c               | 208 ++++++++++++++++++++++++++++++++++++++++++++++-
 tests/mangle-layout.c    |   7 +-
 13 files changed, 357 insertions(+), 27 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 398de32..92a4966 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -119,6 +119,28 @@ Options:
 	Make space for <number> reserve map entries
 	Relevant for dtb and asm output only.
 
+    -@
+	Generates a __symbols__ node at the root node of the resulting blob
+	for any node labels used, and for any local references using phandles
+	it also generates a __local_fixups__ node that tracks them.
+
+	When using the /plugin/ tag all unresolved label references to
+	be tracked in the __fixups__ node, making dynamic resolution possible.
+
+    -A
+	Generate automatically aliases for all node labels. This is similar to
+	the -@ option (the __symbols__ node contain identical information) but
+	the semantics are slightly different since no phandles are automatically
+	generated for labeled nodes.
+
+    -M
+	Generate blobs with the old FDT magic number for device tree objects.
+	By default blobs use the DTBO FDT magic number instead.
+
+    -F
+        Suppress generation of fixups when -@ is used. This is useful for generating
+	a base tree with symbols but without fixups that take some amount of space.
+
     -S <bytes>
 	Ensure the blob at least <bytes> long, adding additional
 	space if needed.
@@ -146,13 +168,18 @@ Additionally, dtc performs various sanity checks on the tree.
 Here is a very rough overview of the layout of a DTS source file:
 
 
-    sourcefile:   list_of_memreserve devicetree
+    sourcefile:   versioninfo plugindecl list_of_memreserve devicetree
 
     memreserve:   label 'memreserve' ADDR ADDR ';'
 		| label 'memreserve' ADDR '-' ADDR ';'
 
     devicetree:   '/' nodedef
 
+    versioninfo:  '/' 'dts-v1' '/' ';'
+
+    plugindecl:   '/' 'plugin' '/' ';'
+                | /* empty */
+
     nodedef:      '{' list_of_property list_of_subnode '}' ';'
 
     property:     label PROPNAME '=' propdata ';'
diff --git a/checks.c b/checks.c
index 2bd27a4..4292f4b 100644
--- a/checks.c
+++ b/checks.c
@@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi,
 
 			refnode = get_node_by_ref(dt, m->ref);
 			if (! refnode) {
-				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
-				     m->ref);
+				if (!(bi->versionflags & VF_PLUGIN))
+					FAIL(c, "Reference to non-existent node or "
+							"label \"%s\"\n", m->ref);
+				else /* mark the entry as unresolved */
+					*((cell_t *)(prop->val.val + m->offset)) =
+						cpu_to_fdt32(0xffffffff);
 				continue;
 			}
 
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 790fbf6..40bbc87 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...);
 			return DT_V1;
 		}
 
+<*>"/plugin/"	{
+			DPRINT("Keyword: /plugin/\n");
+			return DT_PLUGIN;
+		}
+
 <*>"/memreserve/"	{
 			DPRINT("Keyword: /memreserve/\n");
 			BEGIN_DEFAULT();
diff --git a/dtc-parser.y b/dtc-parser.y
index 14aaf2e..ad3dbe2 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"
@@ -52,9 +53,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 +74,8 @@ extern bool treesource_error;
 
 %type <data> propdata
 %type <data> propdataprefix
+%type <flags> versioninfo
+%type <flags> plugindecl
 %type <re> memreserve
 %type <re> memreserves
 %type <array> arrayprefix
@@ -101,16 +106,34 @@ extern bool treesource_error;
 %%
 
 sourcefile:
-	  v1tag memreserves devicetree
+	  versioninfo plugindecl memreserves devicetree
 		{
-			the_boot_info = build_boot_info($2, $3,
-							guess_boot_cpuid($3));
+			the_boot_info = build_boot_info($1 | $2, $3, $4,
+							guess_boot_cpuid($4));
+		}
+	;
+
+versioninfo:
+	v1tag
+		{
+			$$ = VF_DT_V1;
 		}
 	;
 
 v1tag:
 	  DT_V1 ';'
+	| DT_V1
 	| DT_V1 ';' v1tag
+
+plugindecl:
+	DT_PLUGIN ';'
+		{
+			$$ = VF_PLUGIN;
+		}
+	| /* empty */
+		{
+			$$ = 0;
+		}
 	;
 
 memreserves:
diff --git a/dtc.c b/dtc.c
index 9dcf640..947d082 100644
--- a/dtc.c
+++ b/dtc.c
@@ -32,6 +32,10 @@ int minsize;		/* Minimum blob size */
 int padsize;		/* Additional padding to blob */
 int alignsize;		/* Additional padding to blob accroding to the alignsize */
 int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
+int symbol_fixup_support;	/* enable symbols & fixup support */
+int auto_label_aliases;		/* auto generate labels -> aliases */
+int no_dtbo_magic;		/* use old FDT magic values for objects */
+int suppress_fixups;		/* suppress generation of fixups on symbol support */
 
 static int is_power_of_2(int x)
 {
@@ -59,7 +63,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 #define FDT_VERSION(version)	_FDT_VERSION(version)
 #define _FDT_VERSION(version)	#version
 static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMFhv";
 static struct option const usage_long_opts[] = {
 	{"quiet",            no_argument, NULL, 'q'},
 	{"in-format",         a_argument, NULL, 'I'},
@@ -78,6 +82,10 @@ static struct option const usage_long_opts[] = {
 	{"phandle",           a_argument, NULL, 'H'},
 	{"warning",           a_argument, NULL, 'W'},
 	{"error",             a_argument, NULL, 'E'},
+	{"symbols",	     no_argument, NULL, '@'},
+	{"auto-alias",       no_argument, NULL, 'A'},
+	{"no-dtbo-magic",    no_argument, NULL, 'M'},
+	{"suppress-fixups",  no_argument, NULL, 'F'},
 	{"help",             no_argument, NULL, 'h'},
 	{"version",          no_argument, NULL, 'v'},
 	{NULL,               no_argument, NULL, 0x0},
@@ -109,6 +117,10 @@ static const char * const usage_opts_help[] = {
 	 "\t\tboth   - Both \"linux,phandle\" and \"phandle\" properties",
 	"\n\tEnable/disable warnings (prefix with \"no-\")",
 	"\n\tEnable/disable errors (prefix with \"no-\")",
+	"\n\tEnable symbols/fixup support",
+	"\n\tEnable auto-alias of labels",
+	"\n\tDo not use DTBO magic value for plugin objects",
+	"\n\tSuppress generation of fixups when symbol support enabled",
 	"\n\tPrint this help and exit",
 	"\n\tPrint version and exit",
 	NULL,
@@ -153,7 +165,7 @@ static const char *guess_input_format(const char *fname, const char *fallback)
 	fclose(f);
 
 	magic = fdt32_to_cpu(magic);
-	if (magic == FDT_MAGIC)
+	if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO)
 		return "dtb";
 
 	return guess_type_by_name(fname, fallback);
@@ -172,6 +184,7 @@ int main(int argc, char *argv[])
 	FILE *outf = NULL;
 	int outversion = DEFAULT_FDT_VERSION;
 	long long cmdline_boot_cpuid = -1;
+	fdt32_t out_magic = FDT_MAGIC;
 
 	quiet      = 0;
 	reservenum = 0;
@@ -249,6 +262,19 @@ int main(int argc, char *argv[])
 			parse_checks_option(false, true, optarg);
 			break;
 
+		case '@':
+			symbol_fixup_support = 1;
+			break;
+		case 'A':
+			auto_label_aliases = 1;
+			break;
+		case 'M':
+			no_dtbo_magic = 1;
+			break;
+		case 'F':
+			suppress_fixups = 1;
+			break;
+
 		case 'h':
 			usage(NULL);
 		default:
@@ -306,6 +332,20 @@ int main(int argc, char *argv[])
 	fill_fullpaths(bi->dt, "");
 	process_checks(force, bi);
 
+	if (auto_label_aliases)
+		generate_label_tree(bi, "aliases", false);
+
+	/* symbol support or plugin is detected */
+	if (symbol_fixup_support || (bi->versionflags & VF_PLUGIN))
+		generate_label_tree(bi, "__symbols__", true);
+
+	/* plugin or symbol support and fixups are not suppressed */
+	if ((bi->versionflags & VF_PLUGIN) ||
+			(symbol_fixup_support && !suppress_fixups)) {
+		generate_fixups_tree(bi, "__fixups__");
+		generate_local_fixups_tree(bi, "__local_fixups__");
+	}
+
 	if (sort)
 		sort_tree(bi);
 
@@ -318,12 +358,15 @@ int main(int argc, char *argv[])
 			    outname, strerror(errno));
 	}
 
+	if (!no_dtbo_magic && (bi->versionflags & VF_PLUGIN))
+		out_magic = FDT_MAGIC_DTBO;
+
 	if (streq(outform, "dts")) {
 		dt_to_source(outf, bi);
 	} else if (streq(outform, "dtb")) {
-		dt_to_blob(outf, bi, outversion);
+		dt_to_blob(outf, bi, out_magic, outversion);
 	} else if (streq(outform, "asm")) {
-		dt_to_asm(outf, bi, outversion);
+		dt_to_asm(outf, bi, out_magic, outversion);
 	} else if (streq(outform, "null")) {
 		/* do nothing */
 	} else {
diff --git a/dtc.h b/dtc.h
index 32009bc..4da5a37 100644
--- a/dtc.h
+++ b/dtc.h
@@ -55,6 +55,10 @@ extern int minsize;		/* Minimum blob size */
 extern int padsize;		/* Additional padding to blob */
 extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
 extern int phandle_format;	/* Use linux,phandle or phandle properties */
+extern int symbol_fixup_support;/* enable symbols & fixup support */
+extern int auto_label_aliases;	/* auto generate labels -> aliases */
+extern int no_dtbo_magic;	/* use old FDT magic values for objects */
+extern int suppress_fixups;	/* suppress generation of fixups on symbol support */
 
 #define PHANDLE_LEGACY	0x1
 #define PHANDLE_EPAPR	0x2
@@ -202,6 +206,8 @@ void delete_property(struct property *prop);
 void add_child(struct node *parent, struct node *child);
 void delete_node_by_name(struct node *parent, char *name);
 void delete_node(struct node *node);
+void append_to_property(struct node *node,
+			char *name, const void *data, int len);
 
 const char *get_unitname(struct node *node);
 struct property *get_property(struct node *node, const char *propname);
@@ -237,14 +243,23 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 
 
 struct boot_info {
+	unsigned int versionflags;
 	struct reserve_info *reservelist;
 	struct node *dt;		/* the device tree */
 	uint32_t boot_cpuid_phys;
 };
 
-struct boot_info *build_boot_info(struct reserve_info *reservelist,
+/* version flags definitions */
+#define VF_DT_V1	0x0001	/* /dts-v1/ */
+#define VF_PLUGIN	0x0002	/* /plugin/ */
+
+struct boot_info *build_boot_info(unsigned int versionflags,
+				  struct reserve_info *reservelist,
 				  struct node *tree, uint32_t boot_cpuid_phys);
 void sort_tree(struct boot_info *bi);
+void generate_label_tree(struct boot_info *bi, char *name, bool allocph);
+void generate_fixups_tree(struct boot_info *bi, char *name);
+void generate_local_fixups_tree(struct boot_info *bi, char *name);
 
 /* Checks */
 
@@ -253,8 +268,8 @@ void process_checks(bool force, struct boot_info *bi);
 
 /* Flattened trees */
 
-void dt_to_blob(FILE *f, struct boot_info *bi, int version);
-void dt_to_asm(FILE *f, struct boot_info *bi, int version);
+void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
+void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
 
 struct boot_info *dt_from_blob(const char *fname);
 
diff --git a/fdtdump.c b/fdtdump.c
index a9a2484..dd63ac2 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -201,7 +201,7 @@ int main(int argc, char *argv[])
 			p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
 			if (!p)
 				break;
-			if (fdt_magic(p) == FDT_MAGIC) {
+			if (fdt_magic(p) == FDT_MAGIC || fdt_magic(p) == FDT_MAGIC_DTBO) {
 				/* try and validate the main struct */
 				off_t this_len = endp - p;
 				fdt32_t max_version = 17;
diff --git a/flattree.c b/flattree.c
index a9d9520..57d76cf 100644
--- a/flattree.c
+++ b/flattree.c
@@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
 }
 
 static void make_fdt_header(struct fdt_header *fdt,
+			    fdt32_t magic,
 			    struct version_info *vi,
 			    int reservesize, int dtsize, int strsize,
 			    int boot_cpuid_phys)
@@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
 
 	memset(fdt, 0xff, sizeof(*fdt));
 
-	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
+	fdt->magic = cpu_to_fdt32(magic);
 	fdt->version = cpu_to_fdt32(vi->version);
 	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
 
@@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
 		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
 }
 
-void dt_to_blob(FILE *f, struct boot_info *bi, int version)
+void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	reservebuf = flatten_reserve_list(bi->reservelist, vi);
 
 	/* Make header */
-	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
+	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
 			bi->boot_cpuid_phys);
 
 	/*
@@ -467,7 +468,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
 	}
 }
 
-void dt_to_asm(FILE *f, struct boot_info *bi, int version)
+void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
 {
 	struct version_info *vi = NULL;
 	int i;
@@ -830,6 +831,7 @@ struct boot_info *dt_from_blob(const char *fname)
 	struct node *tree;
 	uint32_t val;
 	int flags = 0;
+	unsigned int versionflags = VF_DT_V1;
 
 	f = srcfile_relative_open(fname, NULL);
 
@@ -845,9 +847,12 @@ struct boot_info *dt_from_blob(const char *fname)
 	}
 
 	magic = fdt32_to_cpu(magic);
-	if (magic != FDT_MAGIC)
+	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
 		die("Blob has incorrect magic number\n");
 
+	if (magic == FDT_MAGIC_DTBO)
+		versionflags |= VF_PLUGIN;
+
 	rc = fread(&totalsize, sizeof(totalsize), 1, f);
 	if (ferror(f))
 		die("Error reading DT blob size: %s\n", strerror(errno));
@@ -942,5 +947,5 @@ struct boot_info *dt_from_blob(const char *fname)
 
 	fclose(f);
 
-	return build_boot_info(reservelist, tree, boot_cpuid_phys);
+	return build_boot_info(versionflags, reservelist, tree, boot_cpuid_phys);
 }
diff --git a/fstree.c b/fstree.c
index 6d1beec..54f520b 100644
--- a/fstree.c
+++ b/fstree.c
@@ -86,6 +86,6 @@ struct boot_info *dt_from_fs(const char *dirname)
 	tree = read_fstree(dirname);
 	tree = name_node(tree, "");
 
-	return build_boot_info(NULL, tree, guess_boot_cpuid(tree));
+	return build_boot_info(VF_DT_V1, NULL, tree, guess_boot_cpuid(tree));
 }
 
diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index 22286a1..28d422c 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -57,7 +57,7 @@
 
 int fdt_check_header(const void *fdt)
 {
-	if (fdt_magic(fdt) == FDT_MAGIC) {
+	if (fdt_magic(fdt) == FDT_MAGIC || fdt_magic(fdt) == FDT_MAGIC_DTBO) {
 		/* Complete tree */
 		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
 			return -FDT_ERR_BADVERSION;
diff --git a/libfdt/fdt.h b/libfdt/fdt.h
index 526aedb..493cd55 100644
--- a/libfdt/fdt.h
+++ b/libfdt/fdt.h
@@ -55,7 +55,7 @@
 #ifndef __ASSEMBLY__
 
 struct fdt_header {
-	fdt32_t magic;			 /* magic word FDT_MAGIC */
+	fdt32_t magic;			 /* magic word FDT_MAGIC[|_DTBO] */
 	fdt32_t totalsize;		 /* total size of DT block */
 	fdt32_t off_dt_struct;		 /* offset to structure */
 	fdt32_t off_dt_strings;		 /* offset to strings */
@@ -93,6 +93,7 @@ struct fdt_property {
 #endif /* !__ASSEMBLY */
 
 #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
+#define FDT_MAGIC_DTBO	0xd00dfdb0	/* DTBO magic */
 #define FDT_TAGSIZE	sizeof(fdt32_t)
 
 #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
diff --git a/livetree.c b/livetree.c
index 3dc7559..17f8310 100644
--- a/livetree.c
+++ b/livetree.c
@@ -296,6 +296,23 @@ void delete_node(struct node *node)
 	delete_labels(&node->labels);
 }
 
+void append_to_property(struct node *node,
+				    char *name, const void *data, int len)
+{
+	struct data d;
+	struct property *p;
+
+	p = get_property(node, name);
+	if (p) {
+		d = data_append_data(p->val, data, len);
+		p->val = d;
+	} else {
+		d = data_append_data(empty_data, data, len);
+		p = build_property(name, d);
+		add_property(node, p);
+	}
+}
+
 struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
 {
 	struct reserve_info *new = xmalloc(sizeof(*new));
@@ -335,12 +352,14 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 	return list;
 }
 
-struct boot_info *build_boot_info(struct reserve_info *reservelist,
+struct boot_info *build_boot_info(unsigned int versionflags,
+				  struct reserve_info *reservelist,
 				  struct node *tree, uint32_t boot_cpuid_phys)
 {
 	struct boot_info *bi;
 
 	bi = xmalloc(sizeof(*bi));
+	bi->versionflags = versionflags;
 	bi->reservelist = reservelist;
 	bi->dt = tree;
 	bi->boot_cpuid_phys = boot_cpuid_phys;
@@ -709,3 +728,190 @@ 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 boot_info *bi,
+					 struct node *an,
+					 struct node *node,
+					 bool allocph)
+{
+	struct node *dt = bi->dt;
+	struct node *c;
+	struct property *p;
+	struct label *l;
+
+	/* if if there are labels */
+	if (node->labels) {
+		/* now add the label in the node */
+		for_each_label(node->labels, l) {
+			/* check whether the label already exists */
+			p = get_property(an, l->label);
+			if (p) {
+				fprintf(stderr, "WARNING: label %s already"
+					" exists in /%s", l->label,
+					an->name);
+				continue;
+			}
+
+			/* insert it */
+			p = build_property(l->label,
+				data_copy_mem(node->fullpath,
+						strlen(node->fullpath) + 1));
+			add_property(an, p);
+		}
+
+		/* force allocation of a phandle for this node */
+		if (allocph)
+			(void)get_node_phandle(dt, node);
+	}
+
+	for_each_child(node, c)
+		generate_label_tree_internal(bi, an, c, allocph);
+}
+
+static void add_fixup_entry(struct boot_info *bi, struct node *fn,
+			    struct node *node, struct property *prop,
+			    struct marker *m)
+{
+	char *entry;
+
+	/* m->ref can only be a REF_PHANDLE, but check anyway */
+	assert(m->type == REF_PHANDLE);
+
+	/* 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 generate_fixups_tree_internal(struct boot_info *bi,
+					  struct node *fn,
+					  struct node *node)
+{
+	struct node *dt = bi->dt;
+	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(bi, fn, node, prop, m);
+		}
+	}
+
+	for_each_child(node, c)
+		generate_fixups_tree_internal(bi, fn, c);
+}
+
+static void add_local_fixup_entry(struct boot_info *bi,
+		struct node *lfn, struct node *node,
+		struct property *prop, struct marker *m,
+		struct node *refnode)
+{
+	struct node *wn, *nwn;	/* local fixup node, walk node, new */
+	uint32_t value_32;
+	char **compp;
+	int i, depth;
+
+	/* walk back retreiving depth */
+	depth = 0;
+	for (wn = node; wn; wn = wn->parent)
+		depth++;
+
+	/* allocate name array */
+	compp = xmalloc(sizeof(*compp) * depth);
+
+	/* store names in the array */
+	for (wn = node, i = depth - 1; wn; wn = wn->parent, i--)
+		compp[i] = wn->name;
+
+	/* walk the path components creating nodes if they don't exist */
+	for (wn = lfn, i = 1; i < depth; i++, wn = nwn) {
+		/* if no node exists, create it */
+		nwn = get_subnode(wn, compp[i]);
+		if (!nwn)
+			nwn = build_and_name_child_node(wn, compp[i]);
+	}
+
+	free(compp);
+
+	value_32 = cpu_to_fdt32(m->offset);
+	append_to_property(wn, prop->name, &value_32, sizeof(value_32));
+}
+
+static void generate_local_fixups_tree_internal(struct boot_info *bi,
+						struct node *lfn,
+						struct node *node)
+{
+	struct node *dt = bi->dt;
+	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)
+				continue;
+			add_local_fixup_entry(bi, lfn, node, prop, m, refnode);
+		}
+	}
+
+	for_each_child(node, c)
+		generate_local_fixups_tree_internal(bi, lfn, c);
+}
+
+static struct node *build_root_node(struct node *dt, char *name)
+{
+	struct node *an;
+
+	for_each_child(dt, an)
+		if (streq(name, an->name))
+			break;
+
+	if (!an)
+		an = build_and_name_child_node(dt, name);
+
+	if (!an)
+		die("Could not build root node /%s\n", name);
+
+	return an;
+}
+
+void generate_label_tree(struct boot_info *bi, char *name, bool allocph)
+{
+	generate_label_tree_internal(bi, build_root_node(bi->dt, name),
+				     bi->dt, allocph);
+}
+
+void generate_fixups_tree(struct boot_info *bi, char *name)
+{
+	generate_fixups_tree_internal(bi, build_root_node(bi->dt, name),
+				      bi->dt);
+}
+
+void generate_local_fixups_tree(struct boot_info *bi, char *name)
+{
+	generate_local_fixups_tree_internal(bi, build_root_node(bi->dt, name),
+					    bi->dt);
+}
diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
index a76e51e..d29ebc6 100644
--- a/tests/mangle-layout.c
+++ b/tests/mangle-layout.c
@@ -42,7 +42,8 @@ static void expand_buf(struct bufstate *buf, int newsize)
 	buf->size = newsize;
 }
 
-static void new_header(struct bufstate *buf, int version, const void *fdt)
+static void new_header(struct bufstate *buf, fdt32_t magic, int version,
+		       const void *fdt)
 {
 	int hdrsize;
 
@@ -56,7 +57,7 @@ static void new_header(struct bufstate *buf, int version, const void *fdt)
 	expand_buf(buf, hdrsize);
 	memset(buf->buf, 0, hdrsize);
 
-	fdt_set_magic(buf->buf, FDT_MAGIC);
+	fdt_set_magic(buf->buf, magic);
 	fdt_set_version(buf->buf, version);
 	fdt_set_last_comp_version(buf->buf, 16);
 	fdt_set_boot_cpuid_phys(buf->buf, fdt_boot_cpuid_phys(fdt));
@@ -145,7 +146,7 @@ int main(int argc, char *argv[])
 	if (fdt_version(fdt) < 17)
 		CONFIG("Input tree must be v17");
 
-	new_header(&buf, version, fdt);
+	new_header(&buf, FDT_MAGIC, version, fdt);
 
 	while (*blockorder) {
 		add_block(&buf, version, *blockorder, fdt);
-- 
2.1.4

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

* [PATCH v11 3/7] tests: Add check_path test
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 1/7] dtc: Document the dynamic plugin internals Pantelis Antoniou
  2016-11-28 16:05   ` [PATCH v11 2/7] dtc: Plugin and fixup support Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
       [not found]     ` <1480349141-14145-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 4/7] tests: Add overlay tests Pantelis Antoniou
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

Add a test that checks for existence or not of a node.
It is useful for testing the various cases when generating
symbols and fixups for dynamic device tree objects.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 tests/.gitignore     |  1 +
 tests/Makefile.tests |  3 +-
 tests/check_path.c   | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 tests/check_path.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 354b565..9e209d5 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -8,6 +8,7 @@ tmp.*
 /asm_tree_dump
 /boot-cpuid
 /char_literal
+/check_path
 /del_node
 /del_property
 /dtbs_equal_ordered
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index eb039c5..3d7a4f8 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -25,7 +25,8 @@ LIB_TESTS_L = get_mem_rsv \
 	integer-expressions \
 	property_iterate \
 	subnode_iterate \
-	overlay overlay_bad_fixup
+	overlay overlay_bad_fixup \
+	check_path
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
 LIBTREE_TESTS_L = truncated_property
diff --git a/tests/check_path.c b/tests/check_path.c
new file mode 100644
index 0000000..0d6a73b
--- /dev/null
+++ b/tests/check_path.c
@@ -0,0 +1,82 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for node existence
+ * Copyright (C) 2016 Konsulko Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+
+#include <libfdt.h>
+
+#include "tests.h"
+
+#define CHECK(code) \
+	{ \
+		if (code) \
+			FAIL(#code ": %s", fdt_strerror(code)); \
+	}
+
+/* 4k ought to be enough for anybody */
+#define FDT_COPY_SIZE	(4 * 1024)
+
+static void *open_dt(char *path)
+{
+	void *dt, *copy;
+
+	dt = load_blob(path);
+	copy = xmalloc(FDT_COPY_SIZE);
+
+	/*
+	 * Resize our DTs to 4k so that we have room to operate on
+	 */
+	CHECK(fdt_open_into(dt, copy, FDT_COPY_SIZE));
+
+	return copy;
+}
+
+int main(int argc, char *argv[])
+{
+	void *fdt_base;
+	int fail_config, exists, check_exists;
+
+	test_init(argc, argv);
+	fail_config = 0;
+
+	if (argc != 4)
+		fail_config = 1;
+
+	if (!fail_config) {
+		if (!strcmp(argv[2], "exists"))
+			check_exists = 1;
+		else if (!strcmp(argv[2], "not-exists"))
+			check_exists = 0;
+		else
+			fail_config = 1;
+	}
+
+	if (fail_config)
+		CONFIG("Usage: %s <base dtb> <[exists|not-exists]> <node-path>", argv[0]);
+
+	fdt_base = open_dt(argv[1]);
+
+	exists = fdt_path_offset(fdt_base, argv[3]) >= 0;
+
+	if (exists == check_exists)
+		PASS();
+	else
+		FAIL();
+}
-- 
2.1.4

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

* [PATCH v11 4/7] tests: Add overlay tests
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-11-28 16:05   ` [PATCH v11 3/7] tests: Add check_path test Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
       [not found]     ` <1480349141-14145-5-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax Pantelis Antoniou
                     ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

Add a number of tests for dynamic objects/overlays.

Re-use the original test by moving the contents to a .dtsi include

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
 tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
 tests/overlay_overlay_new_dtc.dts | 11 ++++++
 tests/overlay_overlay_simple.dts  | 12 ++++++
 tests/run_tests.sh                | 41 +++++++++++++++++++
 5 files changed, 148 insertions(+), 75 deletions(-)
 create mode 100644 tests/overlay_overlay_dtc.dtsi
 create mode 100644 tests/overlay_overlay_new_dtc.dts
 create mode 100644 tests/overlay_overlay_simple.dts

diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
index 30d2362..ca943ea 100644
--- a/tests/overlay_overlay_dtc.dts
+++ b/tests/overlay_overlay_dtc.dts
@@ -8,78 +8,4 @@
 /dts-v1/;
 /plugin/;
 
-/ {
-	/* Test that we can change an int by another */
-	fragment@0 {
-		target = <&test>;
-
-		__overlay__ {
-			test-int-property = <43>;
-		};
-	};
-
-	/* Test that we can replace a string by a longer one */
-	fragment@1 {
-		target = <&test>;
-
-		__overlay__ {
-			test-str-property = "foobar";
-		};
-	};
-
-	/* Test that we add a new property */
-	fragment@2 {
-		target = <&test>;
-
-		__overlay__ {
-			test-str-property-2 = "foobar2";
-		};
-	};
-
-	/* Test that we add a new node (by phandle) */
-	fragment@3 {
-		target = <&test>;
-
-		__overlay__ {
-			new-node {
-				new-property;
-			};
-		};
-	};
-
-	fragment@5 {
-		target = <&test>;
-
-		__overlay__ {
-			local: new-local-node {
-				new-property;
-			};
-		};
-	};
-
-	fragment@6 {
-		target = <&test>;
-
-		__overlay__ {
-			test-phandle = <&test>, <&local>;
-		};
-	};
-
-	fragment@7 {
-		target = <&test>;
-
-		__overlay__ {
-			test-several-phandle = <&local>, <&local>;
-		};
-	};
-
-	fragment@8 {
-		target = <&test>;
-
-		__overlay__ {
-			sub-test-node {
-				new-sub-test-property;
-			};
-		};
-	};
-};
+/include/ "overlay_overlay_dtc.dtsi"
diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
new file mode 100644
index 0000000..8ea8d5d
--- /dev/null
+++ b/tests/overlay_overlay_dtc.dtsi
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ * Copyright (c) 2016 Konsulko Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/ {
+	/* Test that we can change an int by another */
+	fragment@0 {
+		target = <&test>;
+
+		__overlay__ {
+			test-int-property = <43>;
+		};
+	};
+
+	/* Test that we can replace a string by a longer one */
+	fragment@1 {
+		target = <&test>;
+
+		__overlay__ {
+			test-str-property = "foobar";
+		};
+	};
+
+	/* Test that we add a new property */
+	fragment@2 {
+		target = <&test>;
+
+		__overlay__ {
+			test-str-property-2 = "foobar2";
+		};
+	};
+
+	/* Test that we add a new node (by phandle) */
+	fragment@3 {
+		target = <&test>;
+
+		__overlay__ {
+			new-node {
+				new-property;
+			};
+		};
+	};
+
+	fragment@5 {
+		target = <&test>;
+
+		__overlay__ {
+			local: new-local-node {
+				new-property;
+			};
+		};
+	};
+
+	fragment@6 {
+		target = <&test>;
+
+		__overlay__ {
+			test-phandle = <&test>, <&local>;
+		};
+	};
+
+	fragment@7 {
+		target = <&test>;
+
+		__overlay__ {
+			test-several-phandle = <&local>, <&local>;
+		};
+	};
+
+	fragment@8 {
+		target = <&test>;
+
+		__overlay__ {
+			sub-test-node {
+				new-sub-test-property;
+			};
+		};
+	};
+};
diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
new file mode 100644
index 0000000..14d3f54
--- /dev/null
+++ b/tests/overlay_overlay_new_dtc.dts
@@ -0,0 +1,11 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ * Copyright (c) 2016 Konsulko Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/ /plugin/;
+
+/include/ "overlay_overlay_dtc.dtsi"
diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
new file mode 100644
index 0000000..8657e1e
--- /dev/null
+++ b/tests/overlay_overlay_simple.dts
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) 2016 Konsulko Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+/plugin/;
+
+&test {
+	test-int-property = <43>;
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index e4139dd..74af0ff 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -181,6 +181,47 @@ overlay_tests () {
         run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
         run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
         run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
+
+        # new /plugin/ format
+        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
+	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
+	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
+	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
+
+        # test new magic option
+        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
+	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
+	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
+	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
+
+        # test plugin source to dtb and back
+        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
+        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
+        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
+
+	# test plugin source to dtb and back (with new magic)
+        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
+        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
+        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
+
+        # test plugin auto-generation without using -@
+        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
+	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
+	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
+	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
+
+        # Test suppression of fixups
+        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
+	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
+	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
+	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
+
+        # Test generation of aliases insted of symbols
+        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
+	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
+	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
+	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
+	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
     fi
 
     # Bad fixup tests
-- 
2.1.4

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

* [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2016-11-28 16:05   ` [PATCH v11 4/7] tests: Add overlay tests Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
       [not found]     ` <1480349141-14145-6-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  2016-11-28 16:05   ` [PATCH v11 6/7] overlay: Add syntactic sugar version of overlays Pantelis Antoniou
  2016-11-28 16:05   ` [PATCH v11 7/7] tests: Add a test for overlays syntactic sugar Pantelis Antoniou
  6 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

There exists a syntactic sugar version of overlays which
make them simpler to write for the trivial case of a single target.

Document it in the device tree object internals.

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

diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
index 026d4ee..d5b841e 100644
--- a/Documentation/dt-object-internal.txt
+++ b/Documentation/dt-object-internal.txt
@@ -300,3 +300,19 @@ 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.
-- 
2.1.4

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

* [PATCH v11 6/7] overlay: Add syntactic sugar version of overlays
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
                     ` (4 preceding siblings ...)
  2016-11-28 16:05   ` [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
  2016-11-28 16:05   ` [PATCH v11 7/7] tests: Add a test for overlays syntactic sugar Pantelis Antoniou
  6 siblings, 0 replies; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

For simple overlays that use a single target there exists a
simpler syntax version.

&foo { }; generates an overlay with a single target at foo.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 dtc-parser.y | 20 +++++++++++++++++---
 dtc.h        |  1 +
 livetree.c   | 22 ++++++++++++++++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index ad3dbe2..743233b 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -184,10 +184,19 @@ devicetree:
 		{
 			struct node *target = get_node_by_ref($1, $2);
 
-			if (target)
+			if (target) {
 				merge_nodes(target, $3);
-			else
-				ERROR(&@2, "Label or path %s not found", $2);
+			} else {
+				/*
+				 * We rely on the rule being always:
+				 *   versioninfo plugindecl memreserves devicetree
+				 * so $-1 is what we want (plugindecl)
+				 */
+				if ($<flags>-1 & VF_PLUGIN)
+					add_orphan_node($1, $3, $2);
+				else
+					ERROR(&@2, "Label or path %s not found", $2);
+			}
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE DT_REF ';'
@@ -202,6 +211,11 @@ devicetree:
 
 			$$ = $1;
 		}
+	| /* empty */
+		{
+			/* build empty node */
+			$$ = name_node(build_node(NULL, NULL), "");
+		}
 	;
 
 nodedef:
diff --git a/dtc.h b/dtc.h
index 4da5a37..b6a246f 100644
--- a/dtc.h
+++ b/dtc.h
@@ -199,6 +199,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);
diff --git a/livetree.c b/livetree.c
index 17f8310..94430f2 100644
--- a/livetree.c
+++ b/livetree.c
@@ -216,6 +216,28 @@ 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;
+	struct property *p;
+	struct data d = empty_data;
+	char *name;
+
+	d = data_add_marker(d, REF_PHANDLE, ref);
+	d = data_append_integer(d, 0xffffffff, 32);
+
+	p = build_property("target", d);
+
+	xasprintf(&name, "fragment@%u",
+			next_orphan_fragment++);
+	name_node(new_node, "__overlay__");
+	node = build_node(p, new_node);
+	name_node(node, name);
+
+	add_child(dt, node);
+}
+
 struct node *chain_node(struct node *first, struct node *list)
 {
 	assert(first->next_sibling == NULL);
-- 
2.1.4

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

* [PATCH v11 7/7] tests: Add a test for overlays syntactic sugar
       [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
                     ` (5 preceding siblings ...)
  2016-11-28 16:05   ` [PATCH v11 6/7] overlay: Add syntactic sugar version of overlays Pantelis Antoniou
@ 2016-11-28 16:05   ` Pantelis Antoniou
  6 siblings, 0 replies; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-28 16:05 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Pantelis Antoniou

Add a single test makeing sure the &foo { }; syntax works.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 tests/run_tests.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 74af0ff..e6b687a 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -222,6 +222,10 @@ overlay_tests () {
 	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
 	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
 	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
+
+	# test simplified plugin syntax
+        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_simple.dtb overlay_overlay_simple.dts
+
     fi
 
     # Bad fixup tests
-- 
2.1.4

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

* Re: [PATCH v11 2/7] dtc: Plugin and fixup support
       [not found]     ` <1480349141-14145-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-29  2:38       ` David Gibson
  0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2016-11-29  2:38 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 28, 2016 at 06:05:36PM +0200, Pantelis Antoniou wrote:
> This patch enable the generation of symbols & local fixup information
> for trees compiled with the -@ (--symbols) option.
> 
> Using this patch labels in the tree and their users emit information
> in __symbols__ and __local_fixups__ nodes.
> 
> The __fixups__ node make possible the dynamic resolution of phandle
> references which are present in the plugin tree but lie in the
> tree that are applying the overlay against.
> 
> While there is a new magic number for dynamic device tree/overlays blobs
> it is by default enabled. Remember to use -M to generate compatible
> blobs.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-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 |  29 ++++++-
>  checks.c                 |   8 +-
>  dtc-lexer.l              |   5 ++
>  dtc-parser.y             |  29 ++++++-
>  dtc.c                    |  51 +++++++++++-
>  dtc.h                    |  21 ++++-
>  fdtdump.c                |   2 +-
>  flattree.c               |  17 ++--
>  fstree.c                 |   2 +-
>  libfdt/fdt.c             |   2 +-
>  libfdt/fdt.h             |   3 +-
>  livetree.c               | 208 ++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/mangle-layout.c    |   7 +-
>  13 files changed, 357 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/manual.txt b/Documentation/manual.txt
> index 398de32..92a4966 100644
> --- a/Documentation/manual.txt
> +++ b/Documentation/manual.txt
> @@ -119,6 +119,28 @@ Options:
>  	Make space for <number> reserve map entries
>  	Relevant for dtb and asm output only.
>  
> +    -@
> +	Generates a __symbols__ node at the root node of the resulting blob
> +	for any node labels used, and for any local references using phandles
> +	it also generates a __local_fixups__ node that tracks them.
> +
> +	When using the /plugin/ tag all unresolved label references to
> +	be tracked in the __fixups__ node, making dynamic resolution possible.
> +
> +    -A
> +	Generate automatically aliases for all node labels. This is similar to
> +	the -@ option (the __symbols__ node contain identical information) but
> +	the semantics are slightly different since no phandles are automatically
> +	generated for labeled nodes.
> +
> +    -M
> +	Generate blobs with the old FDT magic number for device tree objects.
> +	By default blobs use the DTBO FDT magic number instead.
> +
> +    -F
> +        Suppress generation of fixups when -@ is used. This is useful for generating
> +	a base tree with symbols but without fixups that take some amount of space.

Urgh, yet another option?

Can you give me more details on how __fixups__ is useful in a base tree?

>      -S <bytes>
>  	Ensure the blob at least <bytes> long, adding additional
>  	space if needed.
> @@ -146,13 +168,18 @@ Additionally, dtc performs various sanity checks on the tree.
>  Here is a very rough overview of the layout of a DTS source file:
>  
>  
> -    sourcefile:   list_of_memreserve devicetree
> +    sourcefile:   versioninfo plugindecl list_of_memreserve devicetree
>  
>      memreserve:   label 'memreserve' ADDR ADDR ';'
>  		| label 'memreserve' ADDR '-' ADDR ';'
>  
>      devicetree:   '/' nodedef
>  
> +    versioninfo:  '/' 'dts-v1' '/' ';'
> +
> +    plugindecl:   '/' 'plugin' '/' ';'
> +                | /* empty */
> +
>      nodedef:      '{' list_of_property list_of_subnode '}' ';'
>  
>      property:     label PROPNAME '=' propdata ';'
> diff --git a/checks.c b/checks.c
> index 2bd27a4..4292f4b 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -487,8 +487,12 @@ static void fixup_phandle_references(struct check *c, struct boot_info *bi,
>  
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (! refnode) {
> -				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
> -				     m->ref);
> +				if (!(bi->versionflags & VF_PLUGIN))
> +					FAIL(c, "Reference to non-existent node or "
> +							"label \"%s\"\n", m->ref);
> +				else /* mark the entry as unresolved */
> +					*((cell_t *)(prop->val.val + m->offset)) =
> +						cpu_to_fdt32(0xffffffff);
>  				continue;
>  			}
>  
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 790fbf6..40bbc87 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -121,6 +121,11 @@ static void lexical_error(const char *fmt, ...);
>  			return DT_V1;
>  		}
>  
> +<*>"/plugin/"	{
> +			DPRINT("Keyword: /plugin/\n");
> +			return DT_PLUGIN;
> +		}
> +
>  <*>"/memreserve/"	{
>  			DPRINT("Keyword: /memreserve/\n");
>  			BEGIN_DEFAULT();
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 14aaf2e..ad3dbe2 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"
> @@ -52,9 +53,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 +74,8 @@ extern bool treesource_error;
>  
>  %type <data> propdata
>  %type <data> propdataprefix
> +%type <flags> versioninfo
> +%type <flags> plugindecl
>  %type <re> memreserve
>  %type <re> memreserves
>  %type <array> arrayprefix
> @@ -101,16 +106,34 @@ extern bool treesource_error;
>  %%
>  
>  sourcefile:
> -	  v1tag memreserves devicetree
> +	  versioninfo plugindecl memreserves devicetree
>  		{
> -			the_boot_info = build_boot_info($2, $3,
> -							guess_boot_cpuid($3));
> +			the_boot_info = build_boot_info($1 | $2, $3, $4,
> +							guess_boot_cpuid($4));
> +		}
> +	;
> +
> +versioninfo:
> +	v1tag
> +		{
> +			$$ = VF_DT_V1;
>  		}
>  	;
>  
>  v1tag:
>  	  DT_V1 ';'
> +	| DT_V1
>  	| DT_V1 ';' v1tag
> +
> +plugindecl:
> +	DT_PLUGIN ';'
> +		{
> +			$$ = VF_PLUGIN;
> +		}
> +	| /* empty */
> +		{
> +			$$ = 0;
> +		}
>  	;
>  
>  memreserves:
> diff --git a/dtc.c b/dtc.c
> index 9dcf640..947d082 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -32,6 +32,10 @@ int minsize;		/* Minimum blob size */
>  int padsize;		/* Additional padding to blob */
>  int alignsize;		/* Additional padding to blob accroding to the alignsize */
>  int phandle_format = PHANDLE_BOTH;	/* Use linux,phandle or phandle properties */
> +int symbol_fixup_support;	/* enable symbols & fixup support */
> +int auto_label_aliases;		/* auto generate labels -> aliases */
> +int no_dtbo_magic;		/* use old FDT magic values for objects */
> +int suppress_fixups;		/* suppress generation of fixups on symbol support */

The symbol_fixup_support and suppress_fixups flags semantics are
starting to confuse me.  I think rework these to 'generate_symbols'
and 'generate_fixups' which should have clearer logic.

Note that - for now - I'm just suggesting a change in the internal
variables, they can still be set appropriately based on the existing
semantics of the command line options.

>  static int is_power_of_2(int x)
>  {
> @@ -59,7 +63,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
>  #define FDT_VERSION(version)	_FDT_VERSION(version)
>  #define _FDT_VERSION(version)	#version
>  static const char usage_synopsis[] = "dtc [options] <input file>";
> -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv";
> +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@AMFhv";
>  static struct option const usage_long_opts[] = {
>  	{"quiet",            no_argument, NULL, 'q'},
>  	{"in-format",         a_argument, NULL, 'I'},
> @@ -78,6 +82,10 @@ static struct option const usage_long_opts[] = {
>  	{"phandle",           a_argument, NULL, 'H'},
>  	{"warning",           a_argument, NULL, 'W'},
>  	{"error",             a_argument, NULL, 'E'},
> +	{"symbols",	     no_argument, NULL, '@'},
> +	{"auto-alias",       no_argument, NULL, 'A'},
> +	{"no-dtbo-magic",    no_argument, NULL, 'M'},
> +	{"suppress-fixups",  no_argument, NULL, 'F'},
>  	{"help",             no_argument, NULL, 'h'},
>  	{"version",          no_argument, NULL, 'v'},
>  	{NULL,               no_argument, NULL, 0x0},
> @@ -109,6 +117,10 @@ static const char * const usage_opts_help[] = {
>  	 "\t\tboth   - Both \"linux,phandle\" and \"phandle\" properties",
>  	"\n\tEnable/disable warnings (prefix with \"no-\")",
>  	"\n\tEnable/disable errors (prefix with \"no-\")",
> +	"\n\tEnable symbols/fixup support",
> +	"\n\tEnable auto-alias of labels",
> +	"\n\tDo not use DTBO magic value for plugin objects",
> +	"\n\tSuppress generation of fixups when symbol support enabled",
>  	"\n\tPrint this help and exit",
>  	"\n\tPrint version and exit",
>  	NULL,
> @@ -153,7 +165,7 @@ static const char *guess_input_format(const char *fname, const char *fallback)
>  	fclose(f);
>  
>  	magic = fdt32_to_cpu(magic);
> -	if (magic == FDT_MAGIC)
> +	if (magic == FDT_MAGIC || magic == FDT_MAGIC_DTBO)
>  		return "dtb";
>  
>  	return guess_type_by_name(fname, fallback);
> @@ -172,6 +184,7 @@ int main(int argc, char *argv[])
>  	FILE *outf = NULL;
>  	int outversion = DEFAULT_FDT_VERSION;
>  	long long cmdline_boot_cpuid = -1;
> +	fdt32_t out_magic = FDT_MAGIC;
>  
>  	quiet      = 0;
>  	reservenum = 0;
> @@ -249,6 +262,19 @@ int main(int argc, char *argv[])
>  			parse_checks_option(false, true, optarg);
>  			break;
>  
> +		case '@':
> +			symbol_fixup_support = 1;
> +			break;
> +		case 'A':
> +			auto_label_aliases = 1;
> +			break;
> +		case 'M':
> +			no_dtbo_magic = 1;
> +			break;
> +		case 'F':
> +			suppress_fixups = 1;
> +			break;
> +
>  		case 'h':
>  			usage(NULL);
>  		default:
> @@ -306,6 +332,20 @@ int main(int argc, char *argv[])
>  	fill_fullpaths(bi->dt, "");
>  	process_checks(force, bi);
>  
> +	if (auto_label_aliases)
> +		generate_label_tree(bi, "aliases", false);
> +
> +	/* symbol support or plugin is detected */
> +	if (symbol_fixup_support || (bi->versionflags & VF_PLUGIN))
> +		generate_label_tree(bi, "__symbols__", true);
> +
> +	/* plugin or symbol support and fixups are not suppressed */
> +	if ((bi->versionflags & VF_PLUGIN) ||
> +			(symbol_fixup_support && !suppress_fixups)) {
> +		generate_fixups_tree(bi, "__fixups__");
> +		generate_local_fixups_tree(bi, "__local_fixups__");
> +	}


Elaborating on the suggestion above, I'd like to see this just be:
	if (generate_symbols)
		generate_label_tree(...);
	if (generate_fixups) {
		generate_fixups_tree(..);
		generate_local_fixups_tree(...);
	}

With the two option flags set earlier according to the command line
options + /plugin/ tag.

>  	if (sort)
>  		sort_tree(bi);
>  
> @@ -318,12 +358,15 @@ int main(int argc, char *argv[])
>  			    outname, strerror(errno));
>  	}
>  
> +	if (!no_dtbo_magic && (bi->versionflags & VF_PLUGIN))
> +		out_magic = FDT_MAGIC_DTBO;
> +
>  	if (streq(outform, "dts")) {
>  		dt_to_source(outf, bi);
>  	} else if (streq(outform, "dtb")) {
> -		dt_to_blob(outf, bi, outversion);
> +		dt_to_blob(outf, bi, out_magic, outversion);
>  	} else if (streq(outform, "asm")) {
> -		dt_to_asm(outf, bi, outversion);
> +		dt_to_asm(outf, bi, out_magic, outversion);
>  	} else if (streq(outform, "null")) {
>  		/* do nothing */
>  	} else {
> diff --git a/dtc.h b/dtc.h
> index 32009bc..4da5a37 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -55,6 +55,10 @@ extern int minsize;		/* Minimum blob size */
>  extern int padsize;		/* Additional padding to blob */
>  extern int alignsize;		/* Additional padding to blob accroding to the alignsize */
>  extern int phandle_format;	/* Use linux,phandle or phandle properties */
> +extern int symbol_fixup_support;/* enable symbols & fixup support */
> +extern int auto_label_aliases;	/* auto generate labels -> aliases */
> +extern int no_dtbo_magic;	/* use old FDT magic values for objects */
> +extern int suppress_fixups;	/* suppress generation of fixups on symbol support */
>  
>  #define PHANDLE_LEGACY	0x1
>  #define PHANDLE_EPAPR	0x2
> @@ -202,6 +206,8 @@ void delete_property(struct property *prop);
>  void add_child(struct node *parent, struct node *child);
>  void delete_node_by_name(struct node *parent, char *name);
>  void delete_node(struct node *node);
> +void append_to_property(struct node *node,
> +			char *name, const void *data, int len);
>  
>  const char *get_unitname(struct node *node);
>  struct property *get_property(struct node *node, const char *propname);
> @@ -237,14 +243,23 @@ 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/ */


Hm, sorry, didn't mention this minor nit before.  Can you rename
versionflags and the associated constants to make it clear these are
about the *dts* version, since we also have entirely different
versioning info for the *dtb* version.

> +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 boot_info *bi, char *name, bool allocph);
> +void generate_fixups_tree(struct boot_info *bi, char *name);
> +void generate_local_fixups_tree(struct boot_info *bi, char *name);
>  
>  /* Checks */
>  
> @@ -253,8 +268,8 @@ void process_checks(bool force, struct boot_info *bi);
>  
>  /* Flattened trees */
>  
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version);
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version);
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version);
>  
>  struct boot_info *dt_from_blob(const char *fname);
>  
> diff --git a/fdtdump.c b/fdtdump.c
> index a9a2484..dd63ac2 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -201,7 +201,7 @@ int main(int argc, char *argv[])
>  			p = memchr(p, smagic[0], endp - p - FDT_MAGIC_SIZE);
>  			if (!p)
>  				break;
> -			if (fdt_magic(p) == FDT_MAGIC) {
> +			if (fdt_magic(p) == FDT_MAGIC || fdt_magic(p) == FDT_MAGIC_DTBO) {
>  				/* try and validate the main struct */
>  				off_t this_len = endp - p;
>  				fdt32_t max_version = 17;
> diff --git a/flattree.c b/flattree.c
> index a9d9520..57d76cf 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -335,6 +335,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist,
>  }
>  
>  static void make_fdt_header(struct fdt_header *fdt,
> +			    fdt32_t magic,
>  			    struct version_info *vi,
>  			    int reservesize, int dtsize, int strsize,
>  			    int boot_cpuid_phys)
> @@ -345,7 +346,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>  
>  	memset(fdt, 0xff, sizeof(*fdt));
>  
> -	fdt->magic = cpu_to_fdt32(FDT_MAGIC);
> +	fdt->magic = cpu_to_fdt32(magic);
>  	fdt->version = cpu_to_fdt32(vi->version);
>  	fdt->last_comp_version = cpu_to_fdt32(vi->last_comp_version);
>  
> @@ -366,7 +367,7 @@ static void make_fdt_header(struct fdt_header *fdt,
>  		fdt->size_dt_struct = cpu_to_fdt32(dtsize);
>  }
>  
> -void dt_to_blob(FILE *f, struct boot_info *bi, int version)
> +void dt_to_blob(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -390,7 +391,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
>  	reservebuf = flatten_reserve_list(bi->reservelist, vi);
>  
>  	/* Make header */
> -	make_fdt_header(&fdt, vi, reservebuf.len, dtbuf.len, strbuf.len,
> +	make_fdt_header(&fdt, magic, vi, reservebuf.len, dtbuf.len, strbuf.len,
>  			bi->boot_cpuid_phys);
>  
>  	/*
> @@ -467,7 +468,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf)
>  	}
>  }
>  
> -void dt_to_asm(FILE *f, struct boot_info *bi, int version)
> +void dt_to_asm(FILE *f, struct boot_info *bi, fdt32_t magic, int version)
>  {
>  	struct version_info *vi = NULL;
>  	int i;
> @@ -830,6 +831,7 @@ struct boot_info *dt_from_blob(const char *fname)
>  	struct node *tree;
>  	uint32_t val;
>  	int flags = 0;
> +	unsigned int versionflags = VF_DT_V1;

Especially here calling this just 'versionflags' is potentially
confusing, since it's not related to the dtb version that we're also
dealing with in this vicinity.

>  	f = srcfile_relative_open(fname, NULL);
>  
> @@ -845,9 +847,12 @@ struct boot_info *dt_from_blob(const char *fname)
>  	}
>  
>  	magic = fdt32_to_cpu(magic);
> -	if (magic != FDT_MAGIC)
> +	if (magic != FDT_MAGIC && magic != FDT_MAGIC_DTBO)
>  		die("Blob has incorrect magic number\n");
>  
> +	if (magic == FDT_MAGIC_DTBO)
> +		versionflags |= VF_PLUGIN;
> +
>  	rc = fread(&totalsize, sizeof(totalsize), 1, f);
>  	if (ferror(f))
>  		die("Error reading DT blob size: %s\n", strerror(errno));
> @@ -942,5 +947,5 @@ struct boot_info *dt_from_blob(const char *fname)
>  
>  	fclose(f);
>  
> -	return build_boot_info(reservelist, tree, boot_cpuid_phys);
> +	return build_boot_info(versionflags, reservelist, tree, boot_cpuid_phys);
>  }
> diff --git a/fstree.c b/fstree.c
> index 6d1beec..54f520b 100644
> --- a/fstree.c
> +++ b/fstree.c
> @@ -86,6 +86,6 @@ struct boot_info *dt_from_fs(const char *dirname)
>  	tree = read_fstree(dirname);
>  	tree = name_node(tree, "");
>  
> -	return build_boot_info(NULL, tree, guess_boot_cpuid(tree));
> +	return build_boot_info(VF_DT_V1, NULL, tree, guess_boot_cpuid(tree));
>  }
>  
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 22286a1..28d422c 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -57,7 +57,7 @@
>  
>  int fdt_check_header(const void *fdt)
>  {
> -	if (fdt_magic(fdt) == FDT_MAGIC) {
> +	if (fdt_magic(fdt) == FDT_MAGIC || fdt_magic(fdt) == FDT_MAGIC_DTBO) {
>  		/* Complete tree */
>  		if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
>  			return -FDT_ERR_BADVERSION;
> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> index 526aedb..493cd55 100644
> --- a/libfdt/fdt.h
> +++ b/libfdt/fdt.h
> @@ -55,7 +55,7 @@
>  #ifndef __ASSEMBLY__
>  
>  struct fdt_header {
> -	fdt32_t magic;			 /* magic word FDT_MAGIC */
> +	fdt32_t magic;			 /* magic word FDT_MAGIC[|_DTBO] */
>  	fdt32_t totalsize;		 /* total size of DT block */
>  	fdt32_t off_dt_struct;		 /* offset to structure */
>  	fdt32_t off_dt_strings;		 /* offset to strings */
> @@ -93,6 +93,7 @@ struct fdt_property {
>  #endif /* !__ASSEMBLY */
>  
>  #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
> +#define FDT_MAGIC_DTBO	0xd00dfdb0	/* DTBO magic */
>  #define FDT_TAGSIZE	sizeof(fdt32_t)
>  
>  #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
> diff --git a/livetree.c b/livetree.c
> index 3dc7559..17f8310 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -296,6 +296,23 @@ void delete_node(struct node *node)
>  	delete_labels(&node->labels);
>  }
>  
> +void append_to_property(struct node *node,
> +				    char *name, const void *data, int len)
> +{
> +	struct data d;
> +	struct property *p;
> +
> +	p = get_property(node, name);
> +	if (p) {
> +		d = data_append_data(p->val, data, len);
> +		p->val = d;
> +	} else {
> +		d = data_append_data(empty_data, data, len);
> +		p = build_property(name, d);
> +		add_property(node, p);
> +	}
> +}
> +
>  struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
>  {
>  	struct reserve_info *new = xmalloc(sizeof(*new));
> @@ -335,12 +352,14 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  	return list;
>  }
>  
> -struct boot_info *build_boot_info(struct reserve_info *reservelist,
> +struct boot_info *build_boot_info(unsigned int versionflags,
> +				  struct reserve_info *reservelist,
>  				  struct node *tree, uint32_t boot_cpuid_phys)
>  {
>  	struct boot_info *bi;
>  
>  	bi = xmalloc(sizeof(*bi));
> +	bi->versionflags = versionflags;
>  	bi->reservelist = reservelist;
>  	bi->dt = tree;
>  	bi->boot_cpuid_phys = boot_cpuid_phys;
> @@ -709,3 +728,190 @@ 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 boot_info *bi,
> +					 struct node *an,
> +					 struct node *node,
> +					 bool allocph)
> +{
> +	struct node *dt = bi->dt;
> +	struct node *c;
> +	struct property *p;
> +	struct label *l;
> +
> +	/* if if there are labels */
> +	if (node->labels) {
> +		/* now add the label in the node */
> +		for_each_label(node->labels, l) {
> +			/* check whether the label already exists */
> +			p = get_property(an, l->label);
> +			if (p) {
> +				fprintf(stderr, "WARNING: label %s already"
> +					" exists in /%s", l->label,
> +					an->name);
> +				continue;
> +			}
> +
> +			/* insert it */
> +			p = build_property(l->label,
> +				data_copy_mem(node->fullpath,
> +						strlen(node->fullpath) + 1));
> +			add_property(an, p);
> +		}
> +
> +		/* force allocation of a phandle for this node */
> +		if (allocph)
> +			(void)get_node_phandle(dt, node);
> +	}
> +
> +	for_each_child(node, c)
> +		generate_label_tree_internal(bi, an, c, allocph);
> +}
> +
> +static void add_fixup_entry(struct boot_info *bi, struct node *fn,
> +			    struct node *node, struct property *prop,
> +			    struct marker *m)
> +{
> +	char *entry;
> +
> +	/* m->ref can only be a REF_PHANDLE, but check anyway */
> +	assert(m->type == REF_PHANDLE);
> +
> +	/* 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 generate_fixups_tree_internal(struct boot_info *bi,
> +					  struct node *fn,
> +					  struct node *node)
> +{
> +	struct node *dt = bi->dt;
> +	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(bi, fn, node, prop, m);
> +		}
> +	}
> +
> +	for_each_child(node, c)
> +		generate_fixups_tree_internal(bi, fn, c);
> +}
> +
> +static void add_local_fixup_entry(struct boot_info *bi,
> +		struct node *lfn, struct node *node,
> +		struct property *prop, struct marker *m,
> +		struct node *refnode)
> +{
> +	struct node *wn, *nwn;	/* local fixup node, walk node, new */
> +	uint32_t value_32;
> +	char **compp;
> +	int i, depth;
> +
> +	/* walk back retreiving depth */
> +	depth = 0;
> +	for (wn = node; wn; wn = wn->parent)
> +		depth++;
> +
> +	/* allocate name array */
> +	compp = xmalloc(sizeof(*compp) * depth);
> +
> +	/* store names in the array */
> +	for (wn = node, i = depth - 1; wn; wn = wn->parent, i--)
> +		compp[i] = wn->name;
> +
> +	/* walk the path components creating nodes if they don't exist */
> +	for (wn = lfn, i = 1; i < depth; i++, wn = nwn) {
> +		/* if no node exists, create it */
> +		nwn = get_subnode(wn, compp[i]);
> +		if (!nwn)
> +			nwn = build_and_name_child_node(wn, compp[i]);
> +	}
> +
> +	free(compp);
> +
> +	value_32 = cpu_to_fdt32(m->offset);
> +	append_to_property(wn, prop->name, &value_32, sizeof(value_32));
> +}
> +
> +static void generate_local_fixups_tree_internal(struct boot_info *bi,
> +						struct node *lfn,
> +						struct node *node)
> +{
> +	struct node *dt = bi->dt;
> +	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)
> +				continue;
> +			add_local_fixup_entry(bi, lfn, node, prop, m, refnode);
> +		}
> +	}
> +
> +	for_each_child(node, c)
> +		generate_local_fixups_tree_internal(bi, lfn, c);
> +}
> +
> +static struct node *build_root_node(struct node *dt, char *name)
> +{
> +	struct node *an;
> +
> +	for_each_child(dt, an)
> +		if (streq(name, an->name))
> +			break;
> +
> +	if (!an)
> +		an = build_and_name_child_node(dt, name);
> +
> +	if (!an)
> +		die("Could not build root node /%s\n", name);
> +
> +	return an;
> +}
> +
> +void generate_label_tree(struct boot_info *bi, char *name, bool allocph)
> +{
> +	generate_label_tree_internal(bi, build_root_node(bi->dt, name),
> +				     bi->dt, allocph);
> +}
> +
> +void generate_fixups_tree(struct boot_info *bi, char *name)
> +{
> +	generate_fixups_tree_internal(bi, build_root_node(bi->dt, name),
> +				      bi->dt);
> +}
> +
> +void generate_local_fixups_tree(struct boot_info *bi, char *name)
> +{
> +	generate_local_fixups_tree_internal(bi, build_root_node(bi->dt, name),
> +					    bi->dt);
> +}
> diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
> index a76e51e..d29ebc6 100644
> --- a/tests/mangle-layout.c
> +++ b/tests/mangle-layout.c
> @@ -42,7 +42,8 @@ static void expand_buf(struct bufstate *buf, int newsize)
>  	buf->size = newsize;
>  }
>  
> -static void new_header(struct bufstate *buf, int version, const void *fdt)
> +static void new_header(struct bufstate *buf, fdt32_t magic, int version,
> +		       const void *fdt)
>  {
>  	int hdrsize;
>  
> @@ -56,7 +57,7 @@ static void new_header(struct bufstate *buf, int version, const void *fdt)
>  	expand_buf(buf, hdrsize);
>  	memset(buf->buf, 0, hdrsize);
>  
> -	fdt_set_magic(buf->buf, FDT_MAGIC);
> +	fdt_set_magic(buf->buf, magic);
>  	fdt_set_version(buf->buf, version);
>  	fdt_set_last_comp_version(buf->buf, 16);
>  	fdt_set_boot_cpuid_phys(buf->buf, fdt_boot_cpuid_phys(fdt));
> @@ -145,7 +146,7 @@ int main(int argc, char *argv[])
>  	if (fdt_version(fdt) < 17)
>  		CONFIG("Input tree must be v17");
>  
> -	new_header(&buf, version, fdt);
> +	new_header(&buf, FDT_MAGIC, version, fdt);
>  
>  	while (*blockorder) {
>  		add_block(&buf, version, *blockorder, fdt);

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

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

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

* Re: [PATCH v11 3/7] tests: Add check_path test
       [not found]     ` <1480349141-14145-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-29  2:40       ` David Gibson
  0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2016-11-29  2:40 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 28, 2016 at 06:05:37PM +0200, Pantelis Antoniou wrote:
> Add a test that checks for existence or not of a node.
> It is useful for testing the various cases when generating
> symbols and fixups for dynamic device tree objects.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

Can you please add a test-for-the-test by putting a couple of
invocations of this on test_tree1 into the runner script (one 'exists'
and one 'not-exists' should suffice).

> ---
>  tests/.gitignore     |  1 +
>  tests/Makefile.tests |  3 +-
>  tests/check_path.c   | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 tests/check_path.c
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 354b565..9e209d5 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -8,6 +8,7 @@ tmp.*
>  /asm_tree_dump
>  /boot-cpuid
>  /char_literal
> +/check_path
>  /del_node
>  /del_property
>  /dtbs_equal_ordered
> diff --git a/tests/Makefile.tests b/tests/Makefile.tests
> index eb039c5..3d7a4f8 100644
> --- a/tests/Makefile.tests
> +++ b/tests/Makefile.tests
> @@ -25,7 +25,8 @@ LIB_TESTS_L = get_mem_rsv \
>  	integer-expressions \
>  	property_iterate \
>  	subnode_iterate \
> -	overlay overlay_bad_fixup
> +	overlay overlay_bad_fixup \
> +	check_path
>  LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
>  
>  LIBTREE_TESTS_L = truncated_property
> diff --git a/tests/check_path.c b/tests/check_path.c
> new file mode 100644
> index 0000000..0d6a73b
> --- /dev/null
> +++ b/tests/check_path.c
> @@ -0,0 +1,82 @@
> +/*
> + * libfdt - Flat Device Tree manipulation
> + *	Testcase for node existence
> + * Copyright (C) 2016 Konsulko Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdio.h>
> +
> +#include <libfdt.h>
> +
> +#include "tests.h"
> +
> +#define CHECK(code) \
> +	{ \
> +		if (code) \
> +			FAIL(#code ": %s", fdt_strerror(code)); \
> +	}
> +
> +/* 4k ought to be enough for anybody */
> +#define FDT_COPY_SIZE	(4 * 1024)
> +
> +static void *open_dt(char *path)
> +{
> +	void *dt, *copy;
> +
> +	dt = load_blob(path);
> +	copy = xmalloc(FDT_COPY_SIZE);
> +
> +	/*
> +	 * Resize our DTs to 4k so that we have room to operate on
> +	 */
> +	CHECK(fdt_open_into(dt, copy, FDT_COPY_SIZE));
> +
> +	return copy;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	void *fdt_base;
> +	int fail_config, exists, check_exists;
> +
> +	test_init(argc, argv);
> +	fail_config = 0;
> +
> +	if (argc != 4)
> +		fail_config = 1;
> +
> +	if (!fail_config) {
> +		if (!strcmp(argv[2], "exists"))
> +			check_exists = 1;
> +		else if (!strcmp(argv[2], "not-exists"))
> +			check_exists = 0;
> +		else
> +			fail_config = 1;
> +	}
> +
> +	if (fail_config)
> +		CONFIG("Usage: %s <base dtb> <[exists|not-exists]> <node-path>", argv[0]);
> +
> +	fdt_base = open_dt(argv[1]);
> +
> +	exists = fdt_path_offset(fdt_base, argv[3]) >= 0;
> +
> +	if (exists == check_exists)
> +		PASS();
> +	else
> +		FAIL();
> +}

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

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

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

* Re: [PATCH v11 4/7] tests: Add overlay tests
       [not found]     ` <1480349141-14145-5-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-29  3:08       ` David Gibson
       [not found]         ` <20161129030807.GH13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2016-11-29  3:08 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> Add a number of tests for dynamic objects/overlays.
> 
> Re-use the original test by moving the contents to a .dtsi include
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
>  tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
>  tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
>  tests/overlay_overlay_new_dtc.dts | 11 ++++++
>  tests/overlay_overlay_simple.dts  | 12 ++++++
>  tests/run_tests.sh                | 41 +++++++++++++++++++
>  5 files changed, 148 insertions(+), 75 deletions(-)
>  create mode 100644 tests/overlay_overlay_dtc.dtsi
>  create mode 100644 tests/overlay_overlay_new_dtc.dts
>  create mode 100644 tests/overlay_overlay_simple.dts
> 
> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> index 30d2362..ca943ea 100644
> --- a/tests/overlay_overlay_dtc.dts
> +++ b/tests/overlay_overlay_dtc.dts
> @@ -8,78 +8,4 @@
>  /dts-v1/;
>  /plugin/;
>  
> -/ {
> -	/* Test that we can change an int by another */
> -	fragment@0 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			test-int-property = <43>;
> -		};
> -	};
> -
> -	/* Test that we can replace a string by a longer one */
> -	fragment@1 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			test-str-property = "foobar";
> -		};
> -	};
> -
> -	/* Test that we add a new property */
> -	fragment@2 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			test-str-property-2 = "foobar2";
> -		};
> -	};
> -
> -	/* Test that we add a new node (by phandle) */
> -	fragment@3 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			new-node {
> -				new-property;
> -			};
> -		};
> -	};
> -
> -	fragment@5 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			local: new-local-node {
> -				new-property;
> -			};
> -		};
> -	};
> -
> -	fragment@6 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			test-phandle = <&test>, <&local>;
> -		};
> -	};
> -
> -	fragment@7 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			test-several-phandle = <&local>, <&local>;
> -		};
> -	};
> -
> -	fragment@8 {
> -		target = <&test>;
> -
> -		__overlay__ {
> -			sub-test-node {
> -				new-sub-test-property;
> -			};
> -		};
> -	};
> -};
> +/include/ "overlay_overlay_dtc.dtsi"

Don't duplicate this, just replace it with the new style.  This only
existed as essentially documentation for the libfdt overlay
application stuff.  Since the new dtc won't support the old tag
format, there's no point having a test for it.

> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> new file mode 100644
> index 0000000..8ea8d5d
> --- /dev/null
> +++ b/tests/overlay_overlay_dtc.dtsi
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2016 NextThing Co
> + * Copyright (c) 2016 Free Electrons
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +/ {
> +	/* Test that we can change an int by another */
> +	fragment@0 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			test-int-property = <43>;
> +		};
> +	};
> +
> +	/* Test that we can replace a string by a longer one */
> +	fragment@1 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			test-str-property = "foobar";
> +		};
> +	};
> +
> +	/* Test that we add a new property */
> +	fragment@2 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			test-str-property-2 = "foobar2";
> +		};
> +	};
> +
> +	/* Test that we add a new node (by phandle) */
> +	fragment@3 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			new-node {
> +				new-property;
> +			};
> +		};
> +	};
> +
> +	fragment@5 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			local: new-local-node {
> +				new-property;
> +			};
> +		};
> +	};
> +
> +	fragment@6 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			test-phandle = <&test>, <&local>;
> +		};
> +	};
> +
> +	fragment@7 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			test-several-phandle = <&local>, <&local>;
> +		};
> +	};
> +
> +	fragment@8 {
> +		target = <&test>;
> +
> +		__overlay__ {
> +			sub-test-node {
> +				new-sub-test-property;
> +			};
> +		};
> +	};
> +};
> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> new file mode 100644
> index 0000000..14d3f54
> --- /dev/null
> +++ b/tests/overlay_overlay_new_dtc.dts
> @@ -0,0 +1,11 @@
> +/*
> + * Copyright (c) 2016 NextThing Co
> + * Copyright (c) 2016 Free Electrons
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +/dts-v1/ /plugin/;
> +
> +/include/ "overlay_overlay_dtc.dtsi"
> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> new file mode 100644
> index 0000000..8657e1e
> --- /dev/null
> +++ b/tests/overlay_overlay_simple.dts
> @@ -0,0 +1,12 @@
> +/*
> + * Copyright (c) 2016 Konsulko Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&test {
> +	test-int-property = <43>;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index e4139dd..74af0ff 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -181,6 +181,47 @@ overlay_tests () {
>          run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
>          run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
>          run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> +
> +        # new /plugin/ format
> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"

Looks like you're mixing tabs and spaces here.  I don't really mind
which, but keep it consistent at least at the same indentation level.

> +        # test new magic option
> +        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> +
> +        # test plugin source to dtb and back
> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> +
> +	# test plugin source to dtb and back (with new magic)
> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> +
> +        # test plugin auto-generation without using -@
> +        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> +
> +        # Test suppression of fixups
> +        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> +
> +        # Test generation of aliases insted of symbols
> +        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
>      fi
>  
>      # Bad fixup tests

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

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

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found]     ` <1480349141-14145-6-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-29  3:10       ` David Gibson
       [not found]         ` <20161129031054.GI13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2016-11-29  3:10 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> There exists a syntactic sugar version of overlays which
> make them simpler to write for the trivial case of a single target.
> 
> Document it in the device tree object internals.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

I'm with Frank that I think this, rather than being regarded mere
syntactic sugar, should be considered the primary way of describing
overlays.

Obviously we need to support the fully written out version as well.

> ---
>  Documentation/dt-object-internal.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> index 026d4ee..d5b841e 100644
> --- a/Documentation/dt-object-internal.txt
> +++ b/Documentation/dt-object-internal.txt
> @@ -300,3 +300,19 @@ 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.

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

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

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found]         ` <20161129031054.GI13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2016-11-29  4:36           ` Frank Rowand
       [not found]             ` <583D05B7.4040109-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Frank Rowand @ 2016-11-29  4:36 UTC (permalink / raw)
  To: David Gibson, Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe, Sascha Hauer,
	Phil Elwell, Simon Glass, Maxime Ripard, Thomas Petazzoni,
	Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/28/16 19:10, David Gibson wrote:
> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
>> There exists a syntactic sugar version of overlays which
>> make them simpler to write for the trivial case of a single target.

It also works for multiple targets.  (See the example I provided in
my comment to v10.)


>>
>> Document it in the device tree object internals.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> 
> I'm with Frank that I think this, rather than being regarded mere
> syntactic sugar, should be considered the primary way of describing
> overlays.
> 
> Obviously we need to support the fully written out version as well.

If we need to support the fully written out version, can we make that
a discouraged, non-preferred method?  Maybe require an option to
enable compiling this style of dts?

I can imagine some reasons to support the fully written out version,
but can we document what those reasons are?

-Frank

> 
>> ---
>>  Documentation/dt-object-internal.txt | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
>> index 026d4ee..d5b841e 100644
>> --- a/Documentation/dt-object-internal.txt
>> +++ b/Documentation/dt-object-internal.txt
>> @@ -300,3 +300,19 @@ 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.
> 

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found]             ` <583D05B7.4040109-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-11-29  5:10               ` David Gibson
       [not found]                 ` <20161129051042.GO13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2016-11-29  5:10 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Rob Herring,
	Jan Luebbe, Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
> On 11/28/16 19:10, David Gibson wrote:
> > On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> >> There exists a syntactic sugar version of overlays which
> >> make them simpler to write for the trivial case of a single target.
> 
> It also works for multiple targets.  (See the example I provided in
> my comment to v10.)
> 
> 
> >>
> >> Document it in the device tree object internals.
> >>
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> > 
> > I'm with Frank that I think this, rather than being regarded mere
> > syntactic sugar, should be considered the primary way of describing
> > overlays.
> > 
> > Obviously we need to support the fully written out version as well.
> 
> If we need to support the fully written out version, can we make that
> a discouraged, non-preferred method?  Maybe require an option to
> enable compiling this style of dts?

Yeah.  To avoid further proliferation of options, I'm thinking a
single "backwards compat" option which would:
	- Use the dtb magic instead of dtb magic
	- Disable checks which would reject explicit creation of
	  __overlay__ / __symbols__ / __fixups__ nodes
	- Anything other special behaviour we need
	
> I can imagine some reasons to support the fully written out version,
> but can we document what those reasons are?

I believe the main one is the dts files in this format out in the
field.  Mind you, I guess we're already requiring them to tweak how
they declare the /plugin/ option.

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

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

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

* Re: [PATCH v11 4/7] tests: Add overlay tests
       [not found]         ` <20161129030807.GH13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2016-11-29 11:11           ` Pantelis Antoniou
       [not found]             ` <17F182F7-CDF7-4052-86C2-B40505706ED4-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-29 11:11 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi David,

> On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
>> Add a number of tests for dynamic objects/overlays.
>> 
>> Re-use the original test by moving the contents to a .dtsi include
>> 
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> ---
>> tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
>> tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
>> tests/overlay_overlay_new_dtc.dts | 11 ++++++
>> tests/overlay_overlay_simple.dts  | 12 ++++++
>> tests/run_tests.sh                | 41 +++++++++++++++++++
>> 5 files changed, 148 insertions(+), 75 deletions(-)
>> create mode 100644 tests/overlay_overlay_dtc.dtsi
>> create mode 100644 tests/overlay_overlay_new_dtc.dts
>> create mode 100644 tests/overlay_overlay_simple.dts
>> 
>> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
>> index 30d2362..ca943ea 100644
>> --- a/tests/overlay_overlay_dtc.dts
>> +++ b/tests/overlay_overlay_dtc.dts
>> @@ -8,78 +8,4 @@
>> /dts-v1/;
>> /plugin/;
>> 
>> -/ {
>> -	/* Test that we can change an int by another */
>> -	fragment@0 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			test-int-property = <43>;
>> -		};
>> -	};
>> -
>> -	/* Test that we can replace a string by a longer one */
>> -	fragment@1 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			test-str-property = "foobar";
>> -		};
>> -	};
>> -
>> -	/* Test that we add a new property */
>> -	fragment@2 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			test-str-property-2 = "foobar2";
>> -		};
>> -	};
>> -
>> -	/* Test that we add a new node (by phandle) */
>> -	fragment@3 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			new-node {
>> -				new-property;
>> -			};
>> -		};
>> -	};
>> -
>> -	fragment@5 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			local: new-local-node {
>> -				new-property;
>> -			};
>> -		};
>> -	};
>> -
>> -	fragment@6 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			test-phandle = <&test>, <&local>;
>> -		};
>> -	};
>> -
>> -	fragment@7 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			test-several-phandle = <&local>, <&local>;
>> -		};
>> -	};
>> -
>> -	fragment@8 {
>> -		target = <&test>;
>> -
>> -		__overlay__ {
>> -			sub-test-node {
>> -				new-sub-test-property;
>> -			};
>> -		};
>> -	};
>> -};
>> +/include/ "overlay_overlay_dtc.dtsi"
> 
> Don't duplicate this, just replace it with the new style.  This only
> existed as essentially documentation for the libfdt overlay
> application stuff.  Since the new dtc won't support the old tag
> format, there's no point having a test for it.
> 

The parser now handles both tag formats just fine. I could remove support
for it if you’re willing to tackle the flak. 

>> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
>> new file mode 100644
>> index 0000000..8ea8d5d
>> --- /dev/null
>> +++ b/tests/overlay_overlay_dtc.dtsi
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2016 NextThing Co
>> + * Copyright (c) 2016 Free Electrons
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +/ {
>> +	/* Test that we can change an int by another */
>> +	fragment@0 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			test-int-property = <43>;
>> +		};
>> +	};
>> +
>> +	/* Test that we can replace a string by a longer one */
>> +	fragment@1 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			test-str-property = "foobar";
>> +		};
>> +	};
>> +
>> +	/* Test that we add a new property */
>> +	fragment@2 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			test-str-property-2 = "foobar2";
>> +		};
>> +	};
>> +
>> +	/* Test that we add a new node (by phandle) */
>> +	fragment@3 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			new-node {
>> +				new-property;
>> +			};
>> +		};
>> +	};
>> +
>> +	fragment@5 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			local: new-local-node {
>> +				new-property;
>> +			};
>> +		};
>> +	};
>> +
>> +	fragment@6 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			test-phandle = <&test>, <&local>;
>> +		};
>> +	};
>> +
>> +	fragment@7 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			test-several-phandle = <&local>, <&local>;
>> +		};
>> +	};
>> +
>> +	fragment@8 {
>> +		target = <&test>;
>> +
>> +		__overlay__ {
>> +			sub-test-node {
>> +				new-sub-test-property;
>> +			};
>> +		};
>> +	};
>> +};
>> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
>> new file mode 100644
>> index 0000000..14d3f54
>> --- /dev/null
>> +++ b/tests/overlay_overlay_new_dtc.dts
>> @@ -0,0 +1,11 @@
>> +/*
>> + * Copyright (c) 2016 NextThing Co
>> + * Copyright (c) 2016 Free Electrons
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +/dts-v1/ /plugin/;
>> +
>> +/include/ "overlay_overlay_dtc.dtsi"
>> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
>> new file mode 100644
>> index 0000000..8657e1e
>> --- /dev/null
>> +++ b/tests/overlay_overlay_simple.dts
>> @@ -0,0 +1,12 @@
>> +/*
>> + * Copyright (c) 2016 Konsulko Inc.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +&test {
>> +	test-int-property = <43>;
>> +};
>> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
>> index e4139dd..74af0ff 100755
>> --- a/tests/run_tests.sh
>> +++ b/tests/run_tests.sh
>> @@ -181,6 +181,47 @@ overlay_tests () {
>>         run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
>>         run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
>>         run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
>> +
>> +        # new /plugin/ format
>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
> 
> Looks like you're mixing tabs and spaces here.  I don't really mind
> which, but keep it consistent at least at the same indentation level.
> 

Oh, sorry, I use tabs but this sections has spaces… Will fix.

>> +        # test new magic option
>> +        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
>> +
>> +        # test plugin source to dtb and back
>> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
>> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
>> +
>> +	# test plugin source to dtb and back (with new magic)
>> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
>> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
>> +
>> +        # test plugin auto-generation without using -@
>> +        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
>> +
>> +        # Test suppression of fixups
>> +        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
>> +
>> +        # Test generation of aliases insted of symbols
>> +        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
>>     fi
>> 
>>     # Bad fixup tests
> 
> -- 
> 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

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found]                 ` <20161129051042.GO13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2016-11-29 16:45                   ` Frank Rowand
       [not found]                     ` <583DB09C.7060105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Frank Rowand @ 2016-11-29 16:45 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Rob Herring,
	Jan Luebbe, Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/28/16 21:10, David Gibson wrote:
> On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
>> On 11/28/16 19:10, David Gibson wrote:
>>> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
>>>> There exists a syntactic sugar version of overlays which
>>>> make them simpler to write for the trivial case of a single target.
>>
>> It also works for multiple targets.  (See the example I provided in
>> my comment to v10.)
>>
>>
>>>>
>>>> Document it in the device tree object internals.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>
>>> I'm with Frank that I think this, rather than being regarded mere
>>> syntactic sugar, should be considered the primary way of describing
>>> overlays.
>>>
>>> Obviously we need to support the fully written out version as well.
>>
>> If we need to support the fully written out version, can we make that
>> a discouraged, non-preferred method?  Maybe require an option to
>> enable compiling this style of dts?
> 
> Yeah.  To avoid further proliferation of options, I'm thinking a
> single "backwards compat" option which would:
> 	- Use the dtb magic instead of dtb magic
> 	- Disable checks which would reject explicit creation of
> 	  __overlay__ / __symbols__ / __fixups__ nodes
> 	- Anything other special behaviour we need
> 	
>> I can imagine some reasons to support the fully written out version,
>> but can we document what those reasons are?
> 
> I believe the main one is the dts files in this format out in the
> field.  Mind you, I guess we're already requiring them to tweak how
> they declare the /plugin/ option.

It might be easy to write a program that transforms the expanded
format to the simple format.  I'll try to make some time to see
how difficult it is.  The transformation is relatively easy to
do manually, but I don't know how many dts files would need to
be converted.

-Frank

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

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

* Re: [PATCH v11 4/7] tests: Add overlay tests
       [not found]             ` <17F182F7-CDF7-4052-86C2-B40505706ED4-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-11-30  0:35               ` David Gibson
  2016-11-30  9:04                 ` Pantelis Antoniou
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2016-11-30  0:35 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Nov 29, 2016 at 01:11:43PM +0200, Pantelis Antoniou wrote:
> Hi David,
> 
> > On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> >> Add a number of tests for dynamic objects/overlays.
> >> 
> >> Re-use the original test by moving the contents to a .dtsi include
> >> 
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> ---
> >> tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
> >> tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
> >> tests/overlay_overlay_new_dtc.dts | 11 ++++++
> >> tests/overlay_overlay_simple.dts  | 12 ++++++
> >> tests/run_tests.sh                | 41 +++++++++++++++++++
> >> 5 files changed, 148 insertions(+), 75 deletions(-)
> >> create mode 100644 tests/overlay_overlay_dtc.dtsi
> >> create mode 100644 tests/overlay_overlay_new_dtc.dts
> >> create mode 100644 tests/overlay_overlay_simple.dts
> >> 
> >> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> >> index 30d2362..ca943ea 100644
> >> --- a/tests/overlay_overlay_dtc.dts
> >> +++ b/tests/overlay_overlay_dtc.dts
> >> @@ -8,78 +8,4 @@
> >> /dts-v1/;
> >> /plugin/;
> >> 
> >> -/ {
> >> -	/* Test that we can change an int by another */
> >> -	fragment@0 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-int-property = <43>;
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we can replace a string by a longer one */
> >> -	fragment@1 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-str-property = "foobar";
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we add a new property */
> >> -	fragment@2 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-str-property-2 = "foobar2";
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we add a new node (by phandle) */
> >> -	fragment@3 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			new-node {
> >> -				new-property;
> >> -			};
> >> -		};
> >> -	};
> >> -
> >> -	fragment@5 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			local: new-local-node {
> >> -				new-property;
> >> -			};
> >> -		};
> >> -	};
> >> -
> >> -	fragment@6 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-phandle = <&test>, <&local>;
> >> -		};
> >> -	};
> >> -
> >> -	fragment@7 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-several-phandle = <&local>, <&local>;
> >> -		};
> >> -	};
> >> -
> >> -	fragment@8 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			sub-test-node {
> >> -				new-sub-test-property;
> >> -			};
> >> -		};
> >> -	};
> >> -};
> >> +/include/ "overlay_overlay_dtc.dtsi"
> > 
> > Don't duplicate this, just replace it with the new style.  This only
> > existed as essentially documentation for the libfdt overlay
> > application stuff.  Since the new dtc won't support the old tag
> > format, there's no point having a test for it.
> 
> The parser now handles both tag formats just fine. I could remove support
> for it if you’re willing to tackle the flak.

Oh, sorry, I missed that.

I'd suggest having that controlled by the same "backwards compat"
option to control magic number and other things.  Although changing
parser behaviour based on flags can get fiddly.

> 
> >> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> >> new file mode 100644
> >> index 0000000..8ea8d5d
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_dtc.dtsi
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/ {
> >> +	/* Test that we can change an int by another */
> >> +	fragment@0 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-int-property = <43>;
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we can replace a string by a longer one */
> >> +	fragment@1 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-str-property = "foobar";
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we add a new property */
> >> +	fragment@2 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-str-property-2 = "foobar2";
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we add a new node (by phandle) */
> >> +	fragment@3 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			new-node {
> >> +				new-property;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	fragment@5 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			local: new-local-node {
> >> +				new-property;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	fragment@6 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-phandle = <&test>, <&local>;
> >> +		};
> >> +	};
> >> +
> >> +	fragment@7 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-several-phandle = <&local>, <&local>;
> >> +		};
> >> +	};
> >> +
> >> +	fragment@8 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			sub-test-node {
> >> +				new-sub-test-property;
> >> +			};
> >> +		};
> >> +	};
> >> +};
> >> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> >> new file mode 100644
> >> index 0000000..14d3f54
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_new_dtc.dts
> >> @@ -0,0 +1,11 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/ /plugin/;
> >> +
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> >> new file mode 100644
> >> index 0000000..8657e1e
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_simple.dts
> >> @@ -0,0 +1,12 @@
> >> +/*
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/plugin/;
> >> +
> >> +&test {
> >> +	test-int-property = <43>;
> >> +};
> >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> >> index e4139dd..74af0ff 100755
> >> --- a/tests/run_tests.sh
> >> +++ b/tests/run_tests.sh
> >> @@ -181,6 +181,47 @@ overlay_tests () {
> >>         run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
> >>         run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
> >>         run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> >> +
> >> +        # new /plugin/ format
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
> > 
> > Looks like you're mixing tabs and spaces here.  I don't really mind
> > which, but keep it consistent at least at the same indentation level.
> > 
> 
> Oh, sorry, I use tabs but this sections has spaces… Will fix.

Well, it's more tha the first line in the block seems to be using
spaces, then the rest using tabs.

> 
> >> +        # test new magic option
> >> +        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> >> +
> >> +        # test plugin source to dtb and back
> >> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> >> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> >> +
> >> +	# test plugin source to dtb and back (with new magic)
> >> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> >> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> >> +
> >> +        # test plugin auto-generation without using -@
> >> +        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> >> +
> >> +        # Test suppression of fixups
> >> +        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> >> +
> >> +        # Test generation of aliases insted of symbols
> >> +        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
> >>     fi
> >> 
> >>     # Bad fixup tests
> > 
> 
> Regards
> 
> — Pantelis
> 

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

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

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
       [not found]                     ` <583DB09C.7060105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-11-30  0:39                       ` David Gibson
  2016-11-30  9:07                         ` Pantelis Antoniou
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2016-11-30  0:39 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Rob Herring,
	Jan Luebbe, Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Nov 29, 2016 at 08:45:16AM -0800, Frank Rowand wrote:
> On 11/28/16 21:10, David Gibson wrote:
> > On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
> >> On 11/28/16 19:10, David Gibson wrote:
> >>> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> >>>> There exists a syntactic sugar version of overlays which
> >>>> make them simpler to write for the trivial case of a single target.
> >>
> >> It also works for multiple targets.  (See the example I provided in
> >> my comment to v10.)
> >>
> >>
> >>>>
> >>>> Document it in the device tree object internals.
> >>>>
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >>>
> >>> I'm with Frank that I think this, rather than being regarded mere
> >>> syntactic sugar, should be considered the primary way of describing
> >>> overlays.
> >>>
> >>> Obviously we need to support the fully written out version as well.
> >>
> >> If we need to support the fully written out version, can we make that
> >> a discouraged, non-preferred method?  Maybe require an option to
> >> enable compiling this style of dts?
> > 
> > Yeah.  To avoid further proliferation of options, I'm thinking a
> > single "backwards compat" option which would:
> > 	- Use the dtb magic instead of dtb magic
> > 	- Disable checks which would reject explicit creation of
> > 	  __overlay__ / __symbols__ / __fixups__ nodes
> > 	- Anything other special behaviour we need
> > 	
> >> I can imagine some reasons to support the fully written out version,
> >> but can we document what those reasons are?
> > 
> > I believe the main one is the dts files in this format out in the
> > field.  Mind you, I guess we're already requiring them to tweak how
> > they declare the /plugin/ option.
> 
> It might be easy to write a program that transforms the expanded
> format to the simple format.  I'll try to make some time to see
> how difficult it is.  The transformation is relatively easy to
> do manually, but I don't know how many dts files would need to
> be converted.

It's not totally trivial, because such a program would basically need
a full dts parser.  But.. if we change the dtc internals to work with
a list of overlays rather than a single tree, there's a relatively
obvious path to it: we can implement parsing a dtbo input into a set
of fragments rather than an assembled overlay and dts output in
fragment form.  Then converting from old-dts to dtb and back to dts
would pretty much do what's needed.  At the cost of losing comments,
though :/.

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

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

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

* Re: [PATCH v11 4/7] tests: Add overlay tests
  2016-11-30  0:35               ` David Gibson
@ 2016-11-30  9:04                 ` Pantelis Antoniou
  0 siblings, 0 replies; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-30  9:04 UTC (permalink / raw)
  To: David Gibson
  Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi David,

> On Nov 30, 2016, at 02:35 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Tue, Nov 29, 2016 at 01:11:43PM +0200, Pantelis Antoniou wrote:
>> Hi David,
>> 
>>> On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>>> 
>>> On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
>>>> Add a number of tests for dynamic objects/overlays.
>>>> 
>>>> Re-use the original test by moving the contents to a .dtsi include
>>>> 
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
>>>> tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
>>>> tests/overlay_overlay_new_dtc.dts | 11 ++++++
>>>> tests/overlay_overlay_simple.dts  | 12 ++++++
>>>> tests/run_tests.sh                | 41 +++++++++++++++++++
>>>> 5 files changed, 148 insertions(+), 75 deletions(-)
>>>> create mode 100644 tests/overlay_overlay_dtc.dtsi
>>>> create mode 100644 tests/overlay_overlay_new_dtc.dts
>>>> create mode 100644 tests/overlay_overlay_simple.dts
>>>> 
>>>> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
>>>> index 30d2362..ca943ea 100644
>>>> --- a/tests/overlay_overlay_dtc.dts
>>>> +++ b/tests/overlay_overlay_dtc.dts
>>>> @@ -8,78 +8,4 @@
>>>> /dts-v1/;
>>>> /plugin/;
>>>> 
>>>> -/ {
>>>> -	/* Test that we can change an int by another */
>>>> -	fragment@0 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			test-int-property = <43>;
>>>> -		};
>>>> -	};
>>>> -
>>>> -	/* Test that we can replace a string by a longer one */
>>>> -	fragment@1 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			test-str-property = "foobar";
>>>> -		};
>>>> -	};
>>>> -
>>>> -	/* Test that we add a new property */
>>>> -	fragment@2 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			test-str-property-2 = "foobar2";
>>>> -		};
>>>> -	};
>>>> -
>>>> -	/* Test that we add a new node (by phandle) */
>>>> -	fragment@3 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			new-node {
>>>> -				new-property;
>>>> -			};
>>>> -		};
>>>> -	};
>>>> -
>>>> -	fragment@5 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			local: new-local-node {
>>>> -				new-property;
>>>> -			};
>>>> -		};
>>>> -	};
>>>> -
>>>> -	fragment@6 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			test-phandle = <&test>, <&local>;
>>>> -		};
>>>> -	};
>>>> -
>>>> -	fragment@7 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			test-several-phandle = <&local>, <&local>;
>>>> -		};
>>>> -	};
>>>> -
>>>> -	fragment@8 {
>>>> -		target = <&test>;
>>>> -
>>>> -		__overlay__ {
>>>> -			sub-test-node {
>>>> -				new-sub-test-property;
>>>> -			};
>>>> -		};
>>>> -	};
>>>> -};
>>>> +/include/ "overlay_overlay_dtc.dtsi"
>>> 
>>> Don't duplicate this, just replace it with the new style.  This only
>>> existed as essentially documentation for the libfdt overlay
>>> application stuff.  Since the new dtc won't support the old tag
>>> format, there's no point having a test for it.
>> 
>> The parser now handles both tag formats just fine. I could remove support
>> for it if you’re willing to tackle the flak.
> 
> Oh, sorry, I missed that.
> 
> I'd suggest having that controlled by the same "backwards compat"
> option to control magic number and other things.  Although changing
> parser behaviour based on flags can get fiddly.
> 

Ugh, I’d rather not. It’s not going to look good. I’d rather leave it like
this, make a note that it’s deprecated and remove it completely a year down
the line letting enough time for users to get up to date.

>> 
>>>> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
>>>> new file mode 100644
>>>> index 0000000..8ea8d5d
>>>> --- /dev/null
>>>> +++ b/tests/overlay_overlay_dtc.dtsi
>>>> @@ -0,0 +1,83 @@
>>>> +/*
>>>> + * Copyright (c) 2016 NextThing Co
>>>> + * Copyright (c) 2016 Free Electrons
>>>> + * Copyright (c) 2016 Konsulko Inc.
>>>> + *
>>>> + * SPDX-License-Identifier:	GPL-2.0+
>>>> + */
>>>> +
>>>> +/ {
>>>> +	/* Test that we can change an int by another */
>>>> +	fragment@0 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			test-int-property = <43>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	/* Test that we can replace a string by a longer one */
>>>> +	fragment@1 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			test-str-property = "foobar";
>>>> +		};
>>>> +	};
>>>> +
>>>> +	/* Test that we add a new property */
>>>> +	fragment@2 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			test-str-property-2 = "foobar2";
>>>> +		};
>>>> +	};
>>>> +
>>>> +	/* Test that we add a new node (by phandle) */
>>>> +	fragment@3 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			new-node {
>>>> +				new-property;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +
>>>> +	fragment@5 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			local: new-local-node {
>>>> +				new-property;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +
>>>> +	fragment@6 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			test-phandle = <&test>, <&local>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	fragment@7 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			test-several-phandle = <&local>, <&local>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	fragment@8 {
>>>> +		target = <&test>;
>>>> +
>>>> +		__overlay__ {
>>>> +			sub-test-node {
>>>> +				new-sub-test-property;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +};
>>>> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
>>>> new file mode 100644
>>>> index 0000000..14d3f54
>>>> --- /dev/null
>>>> +++ b/tests/overlay_overlay_new_dtc.dts
>>>> @@ -0,0 +1,11 @@
>>>> +/*
>>>> + * Copyright (c) 2016 NextThing Co
>>>> + * Copyright (c) 2016 Free Electrons
>>>> + * Copyright (c) 2016 Konsulko Inc.
>>>> + *
>>>> + * SPDX-License-Identifier:	GPL-2.0+
>>>> + */
>>>> +
>>>> +/dts-v1/ /plugin/;
>>>> +
>>>> +/include/ "overlay_overlay_dtc.dtsi"
>>>> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
>>>> new file mode 100644
>>>> index 0000000..8657e1e
>>>> --- /dev/null
>>>> +++ b/tests/overlay_overlay_simple.dts
>>>> @@ -0,0 +1,12 @@
>>>> +/*
>>>> + * Copyright (c) 2016 Konsulko Inc.
>>>> + *
>>>> + * SPDX-License-Identifier:	GPL-2.0+
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +
>>>> +&test {
>>>> +	test-int-property = <43>;
>>>> +};
>>>> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
>>>> index e4139dd..74af0ff 100755
>>>> --- a/tests/run_tests.sh
>>>> +++ b/tests/run_tests.sh
>>>> @@ -181,6 +181,47 @@ overlay_tests () {
>>>>        run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
>>>>        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
>>>>        run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
>>>> +
>>>> +        # new /plugin/ format
>>>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
>>>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
>>>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
>>>> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
>>> 
>>> Looks like you're mixing tabs and spaces here.  I don't really mind
>>> which, but keep it consistent at least at the same indentation level.
>>> 
>> 
>> Oh, sorry, I use tabs but this sections has spaces… Will fix.
> 
> Well, it's more tha the first line in the block seems to be using
> spaces, then the rest using tabs.
> 
>> 
>>>> +        # test new magic option
>>>> +        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
>>>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
>>>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
>>>> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
>>>> +
>>>> +        # test plugin source to dtb and back
>>>> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
>>>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
>>>> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
>>>> +
>>>> +	# test plugin source to dtb and back (with new magic)
>>>> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
>>>> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
>>>> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
>>>> +
>>>> +        # test plugin auto-generation without using -@
>>>> +        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
>>>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
>>>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
>>>> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
>>>> +
>>>> +        # Test suppression of fixups
>>>> +        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
>>>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
>>>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
>>>> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
>>>> +
>>>> +        # Test generation of aliases insted of symbols
>>>> +        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
>>>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
>>>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
>>>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
>>>> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
>>>>    fi
>>>> 
>>>>    # Bad fixup tests
>>> 
>> 
>> Regards
>> 
>> — Pantelis
>> 
> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson

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

* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
  2016-11-30  0:39                       ` David Gibson
@ 2016-11-30  9:07                         ` Pantelis Antoniou
  0 siblings, 0 replies; 20+ messages in thread
From: Pantelis Antoniou @ 2016-11-30  9:07 UTC (permalink / raw)
  To: David Gibson
  Cc: Frank Rowand, Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe,
	Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
	Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi David,

> On Nov 30, 2016, at 02:39 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> On Tue, Nov 29, 2016 at 08:45:16AM -0800, Frank Rowand wrote:
>> On 11/28/16 21:10, David Gibson wrote:
>>> On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
>>>> On 11/28/16 19:10, David Gibson wrote:
>>>>> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
>>>>>> There exists a syntactic sugar version of overlays which
>>>>>> make them simpler to write for the trivial case of a single target.
>>>> 
>>>> It also works for multiple targets.  (See the example I provided in
>>>> my comment to v10.)
>>>> 
>>>> 
>>>>>> 
>>>>>> Document it in the device tree object internals.
>>>>>> 
>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>> 
>>>>> I'm with Frank that I think this, rather than being regarded mere
>>>>> syntactic sugar, should be considered the primary way of describing
>>>>> overlays.
>>>>> 
>>>>> Obviously we need to support the fully written out version as well.
>>>> 
>>>> If we need to support the fully written out version, can we make that
>>>> a discouraged, non-preferred method?  Maybe require an option to
>>>> enable compiling this style of dts?
>>> 
>>> Yeah.  To avoid further proliferation of options, I'm thinking a
>>> single "backwards compat" option which would:
>>> 	- Use the dtb magic instead of dtb magic
>>> 	- Disable checks which would reject explicit creation of
>>> 	  __overlay__ / __symbols__ / __fixups__ nodes
>>> 	- Anything other special behaviour we need
>>> 	
>>>> I can imagine some reasons to support the fully written out version,
>>>> but can we document what those reasons are?
>>> 
>>> I believe the main one is the dts files in this format out in the
>>> field.  Mind you, I guess we're already requiring them to tweak how
>>> they declare the /plugin/ option.
>> 
>> It might be easy to write a program that transforms the expanded
>> format to the simple format.  I'll try to make some time to see
>> how difficult it is.  The transformation is relatively easy to
>> do manually, but I don't know how many dts files would need to
>> be converted.
> 
> It's not totally trivial, because such a program would basically need
> a full dts parser.  But.. if we change the dtc internals to work with
> a list of overlays rather than a single tree, there's a relatively
> obvious path to it: we can implement parsing a dtbo input into a set
> of fragments rather than an assembled overlay and dts output in
> fragment form.  Then converting from old-dts to dtb and back to dts
> would pretty much do what's needed.  At the cost of losing comments,
> though :/.
> 

It’s not just the comments. New style dts’es in the kernel now use CPP
pre-processed input which make the dts more readable.

Converting back to dts is not even close to the original source.

You might as well use fdtdump instead.

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

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

end of thread, other threads:[~2016-11-30  9:07 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-28 16:05 [PATCH v11 0/4] dtc: Dynamic DT support Pantelis Antoniou
     [not found] ` <1480349141-14145-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-28 16:05   ` [PATCH v11 1/7] dtc: Document the dynamic plugin internals Pantelis Antoniou
2016-11-28 16:05   ` [PATCH v11 2/7] dtc: Plugin and fixup support Pantelis Antoniou
     [not found]     ` <1480349141-14145-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-29  2:38       ` David Gibson
2016-11-28 16:05   ` [PATCH v11 3/7] tests: Add check_path test Pantelis Antoniou
     [not found]     ` <1480349141-14145-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-29  2:40       ` David Gibson
2016-11-28 16:05   ` [PATCH v11 4/7] tests: Add overlay tests Pantelis Antoniou
     [not found]     ` <1480349141-14145-5-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-29  3:08       ` David Gibson
     [not found]         ` <20161129030807.GH13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2016-11-29 11:11           ` Pantelis Antoniou
     [not found]             ` <17F182F7-CDF7-4052-86C2-B40505706ED4-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-30  0:35               ` David Gibson
2016-11-30  9:04                 ` Pantelis Antoniou
2016-11-28 16:05   ` [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax Pantelis Antoniou
     [not found]     ` <1480349141-14145-6-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-11-29  3:10       ` David Gibson
     [not found]         ` <20161129031054.GI13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2016-11-29  4:36           ` Frank Rowand
     [not found]             ` <583D05B7.4040109-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-11-29  5:10               ` David Gibson
     [not found]                 ` <20161129051042.GO13307-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2016-11-29 16:45                   ` Frank Rowand
     [not found]                     ` <583DB09C.7060105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-11-30  0:39                       ` David Gibson
2016-11-30  9:07                         ` Pantelis Antoniou
2016-11-28 16:05   ` [PATCH v11 6/7] overlay: Add syntactic sugar version of overlays Pantelis Antoniou
2016-11-28 16:05   ` [PATCH v11 7/7] tests: Add a test for overlays syntactic sugar Pantelis Antoniou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).