[PATCH v5 0/8] libfdt: Add support for device tree overlays

[PATCH v5 0/8] libfdt: Add support for device tree overlays

[Maxime Ripard]

Hi,

The device tree overlays are a great solution to the issue raised by
the bunch expandable boards we find everywhere these days, like the
Beaglebone, Raspberry Pi or CHIP.

Although for now Linux is the only available tool that can deal with
overlays, some other components like bootloaders or user-space tools
might need to apply these overlays on top of a base device tree.

To address these use cases, we introduce a new function to the libfdt,
fdt_overlay_apply, that does just that.

You can find a test program here: http://code.bulix.org/792zum-99476?raw

This is the last patches sent to U-boot, with the modifications
suggested by David, and the additional test cases.

Let me know what you think!
Maxime

Changes from libfdt v4:
  - Renamed the index parameter in fdt_setprop_inplace_namelen_partial
    to idx
  - Changed the fdt_setprop_inplace_namelen_partial tests to modify
    the existing compatibles instead of relying on new properties


Changes from libfdt v3:
  - Fixed a few error in the fixup parsing code
  - Added test cases with overlays with poorly formatted fixups
  - Fixed a few error, types returned
  - Added a new error for poorly formatted fixups
  - Added new tests for fdt_setprop_inplace_namelen_partial
  - Fixed the documentation of fdt_for_each_property_offset and
    fdt_for_each_subnode

Changes from libfdt v2:
  - Added test cases for the overlays that don't require overlay
    support in dtc
  - Only treat as fragments the top-level subnodes with an __overlay__
    subnode
  - Fixed kerneldoc for subnodes and property iterators
  - Improve kerneldoc for fdt_get_max_phandle
  - Make sure the fixup properties length is a multiple of 4 bytes
  - Improve the error checking on a few places
  - Improve a few variables names and types

Changes from U-Boot v4:
  - Added test cases for the functions
  - Changed the fdt_for_each_subnode argument order
  - Don't error out if -1 phandle is found in fdt_get_max_phandle
  - Used libfdt's fdt_path_offset_namelen

Changes from U-Boot v3:
  - Moved the patch to introduce fdt_getprop_namelen_w earlier to keep
    bisectability
  - Renamed fdt_setprop_inplace_namelen_by_index in
    fdt_setprop_inplace_namelen_partial
  - Reintroduced the check on the property length in fdt_setprop_inplace
  - Made sure the code was taking the non 32bits-aligned phandles
  - Used memchr instead of strchr in the fixup parsing code, and made
    sure the cases where the fixup format was wrong was reported as an
    error.
  - Fixed a bug where a property in a overlay having multiple phandles
    local to that overlay would only resolve the first one. Also added
    a test case for this
  - Added kerneldocs for all the overlay functions
  - Added a patch to fix a typo in separator
  - A few fixes, function renamings, error checking changes here and there

Changes from U-boot v2 / libfdt v1
  - Reworked the code to deal with Pantelis and David numerous
    comments, among which:
    * Remove the need for malloc in the overlay code, and added some
      libfdt functions to do that
    * Remove the DT magic in case of an error to not be able to use it
      anymore
    * Removed the fdt_ and _ function prefix for the static functions
    * Plus the usual bunch of rework, error checking and optimizations.				  

Maxime Ripard (7):
  libfdt: Add iterator over properties
  libfdt: Add max phandle retrieval function
  libfdt: Add fdt_getprop_namelen_w
  libfdt: Add fdt_setprop_inplace_namelen_partial
  libfdt: Introduce FDT_ERR_BADFIXUP
  libfdt: Add overlay application function
  tests: Add tests cases for the overlay code

Thierry Reding (1):
  libfdt: Add a subnodes iterator macro

 libfdt/Makefile.libfdt                      |   2 +-
 libfdt/fdt_overlay.c                        | 641 ++++++++++++++++++++++++++++
 libfdt/fdt_ro.c                             |  26 ++
 libfdt/fdt_strerror.c                       |   1 +
 libfdt/fdt_wip.c                            |  29 +-
 libfdt/libfdt.h                             | 135 +++++-
 libfdt/libfdt_env.h                         |   1 +
 tests/.gitignore                            |   3 +
 tests/Makefile.tests                        |   4 +-
 tests/get_phandle.c                         |   6 +
 tests/overlay.c                             | 232 ++++++++++
 tests/overlay_bad_fixup.c                   |  70 +++
 tests/overlay_bad_fixup_bad_index.dts       |  14 +
 tests/overlay_bad_fixup_base.dtsi           |  18 +
 tests/overlay_bad_fixup_empty.dts           |  14 +
 tests/overlay_bad_fixup_empty_index.dts     |  14 +
 tests/overlay_bad_fixup_index_trailing.dts  |  14 +
 tests/overlay_bad_fixup_path_empty_prop.dts |  14 +
 tests/overlay_bad_fixup_path_only.dts       |  14 +
 tests/overlay_bad_fixup_path_only_sep.dts   |  14 +
 tests/overlay_bad_fixup_path_prop.dts       |  14 +
 tests/overlay_base.dts                      |  21 +
 tests/overlay_overlay_dtc.dts               |  85 ++++
 tests/overlay_overlay_nodtc.dts             |  82 ++++
 tests/property_iterate.c                    |  97 +++++
 tests/property_iterate.dts                  |  24 ++
 tests/run_tests.sh                          |  35 ++
 tests/setprop_inplace.c                     |  10 +
 tests/subnode_iterate.c                     |   8 +-
 tests/testdata.h                            |   3 +
 30 files changed, 1632 insertions(+), 13 deletions(-)
 create mode 100644 libfdt/fdt_overlay.c
 create mode 100644 tests/overlay.c
 create mode 100644 tests/overlay_bad_fixup.c
 create mode 100644 tests/overlay_bad_fixup_bad_index.dts
 create mode 100644 tests/overlay_bad_fixup_base.dtsi
 create mode 100644 tests/overlay_bad_fixup_empty.dts
 create mode 100644 tests/overlay_bad_fixup_empty_index.dts
 create mode 100644 tests/overlay_bad_fixup_index_trailing.dts
 create mode 100644 tests/overlay_bad_fixup_path_empty_prop.dts
 create mode 100644 tests/overlay_bad_fixup_path_only.dts
 create mode 100644 tests/overlay_bad_fixup_path_only_sep.dts
 create mode 100644 tests/overlay_bad_fixup_path_prop.dts
 create mode 100644 tests/overlay_base.dts
 create mode 100644 tests/overlay_overlay_dtc.dts
 create mode 100644 tests/overlay_overlay_nodtc.dts
 create mode 100644 tests/property_iterate.c
 create mode 100644 tests/property_iterate.dts

-- 
2.9.2

[PATCH v5 1/8] libfdt: Add a subnodes iterator macro

[Maxime Ripard]

From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The fdt_for_each_subnode() iterator macro provided by this patch can be
used to iterate over a device tree node's subnodes. At each iteration a
loop variable will be set to the next subnode.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/libfdt.h         | 28 ++++++++++++++++++++++++++++
 tests/subnode_iterate.c |  8 ++------
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 36222fd4a6f4..911e548163d8 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -168,6 +168,34 @@ int fdt_first_subnode(const void *fdt, int offset);
  */
 int fdt_next_subnode(const void *fdt, int offset);
 
+/**
+ * fdt_for_each_subnode - iterate over all subnodes of a parent
+ *
+ * @node:	child node (int, lvalue)
+ * @fdt:	FDT blob (const void *)
+ * @parent:	parent node (int)
+ *
+ * This is actually a wrapper around a for loop and would be used like so:
+ *
+ *	fdt_for_each_subnode(node, fdt, parent) {
+ *		Use node
+ *		...
+ *	}
+ *
+ *	if ((node < 0) && (node != -FDT_ERR_NOT_FOUND)) {
+ *		Error handling
+ *	}
+ *
+ * Note that this is implemented as a macro and @node is used as
+ * iterator in the loop. The parent variable be constant or even a
+ * literal.
+ *
+ */
+#define fdt_for_each_subnode(node, fdt, parent)		\
+	for (node = fdt_first_subnode(fdt, parent);	\
+	     node >= 0;					\
+	     node = fdt_next_subnode(fdt, node))
+
 /**********************************************************************/
 /* General functions                                                  */
 /**********************************************************************/
diff --git a/tests/subnode_iterate.c b/tests/subnode_iterate.c
index b9f379d5963c..0fb5c901ebd7 100644
--- a/tests/subnode_iterate.c
+++ b/tests/subnode_iterate.c
@@ -48,9 +48,7 @@ static void test_node(void *fdt, int parent_offset)
 	subnodes = cpu_to_fdt32(*prop);
 
 	count = 0;
-	for (offset = fdt_first_subnode(fdt, parent_offset);
-	     offset >= 0;
-	     offset = fdt_next_subnode(fdt, offset))
+	fdt_for_each_subnode(offset, fdt, parent_offset)
 		count++;
 
 	if (count != subnodes) {
@@ -65,9 +63,7 @@ static void check_fdt_next_subnode(void *fdt)
 	int offset;
 	int count = 0;
 
-	for (offset = fdt_first_subnode(fdt, 0);
-	     offset >= 0;
-	     offset = fdt_next_subnode(fdt, offset)) {
+	fdt_for_each_subnode(offset, fdt, 0) {
 		test_node(fdt, offset);
 		count++;
 	}
-- 
2.9.2

[PATCH v5 2/8] libfdt: Add iterator over properties

[Maxime Ripard]

Implement a macro based on fdt_first_property_offset and
fdt_next_property_offset that provides a convenience to iterate over all
the properties of a given node.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 libfdt/libfdt.h            | 27 +++++++++++++
 tests/.gitignore           |  1 +
 tests/Makefile.tests       |  1 +
 tests/property_iterate.c   | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/property_iterate.dts | 24 ++++++++++++
 tests/run_tests.sh         |  3 ++
 6 files changed, 153 insertions(+)
 create mode 100644 tests/property_iterate.c
 create mode 100644 tests/property_iterate.dts

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 911e548163d8..be109a8aac84 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -458,6 +458,33 @@ int fdt_first_property_offset(const void *fdt, int nodeoffset);
  */
 int fdt_next_property_offset(const void *fdt, int offset);
 
+/**
+ * fdt_for_each_property_offset - iterate over all properties of a node
+ *
+ * @property_offset:	property offset (int, lvalue)
+ * @fdt:		FDT blob (const void *)
+ * @node:		node offset (int)
+ *
+ * This is actually a wrapper around a for loop and would be used like so:
+ *
+ *	fdt_for_each_property_offset(property, fdt, node) {
+ *		Use property
+ *		...
+ *	}
+ *
+ *	if ((property < 0) && (property != -FDT_ERR_NOT_FOUND)) {
+ *		Error handling
+ *	}
+ *
+ * Note that this is implemented as a macro and property is used as
+ * iterator in the loop. The node variable can be constant or even a
+ * literal.
+ */
+#define fdt_for_each_property_offset(property, fdt, node)	\
+	for (property = fdt_first_property_offset(fdt, node);	\
+	     property >= 0;					\
+	     property = fdt_next_property_offset(fdt, property))
+
 /**
  * fdt_get_property_by_offset - retrieve the property at a given offset
  * @fdt: pointer to the device tree blob
diff --git a/tests/.gitignore b/tests/.gitignore
index e4532da30bf5..fa4616ba28c2 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -40,6 +40,7 @@ tmp.*
 /path_offset
 /path_offset_aliases
 /phandle_format
+/property_iterate
 /propname_escapes
 /references
 /root_node
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index f7c3a4b00ead..196518c83eda 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -23,6 +23,7 @@ LIB_TESTS_L = get_mem_rsv \
 	add_subnode_with_nops path_offset_aliases \
 	utilfdt_test \
 	integer-expressions \
+	property_iterate \
 	subnode_iterate
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
diff --git a/tests/property_iterate.c b/tests/property_iterate.c
new file mode 100644
index 000000000000..0f3959cb8c22
--- /dev/null
+++ b/tests/property_iterate.c
@@ -0,0 +1,97 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Tests that fdt_next_subnode() works as expected
+ *
+ * Copyright (C) 2013 Google, Inc
+ *
+ * Copyright (C) 2007 David Gibson, IBM Corporation.
+ *
+ * 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 <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+static void test_node(void *fdt, int parent_offset)
+{
+	fdt32_t properties;
+	const fdt32_t *prop;
+	int offset, property;
+	int count;
+	int len;
+
+	/*
+	 * This property indicates the number of properties in our
+	 * test node to expect
+	 */
+	prop = fdt_getprop(fdt, parent_offset, "test-properties", &len);
+	if (!prop || len != sizeof(fdt32_t)) {
+		FAIL("Missing/invalid test-properties property at '%s'",
+		     fdt_get_name(fdt, parent_offset, NULL));
+	}
+	properties = cpu_to_fdt32(*prop);
+
+	count = 0;
+	offset = fdt_first_subnode(fdt, parent_offset);
+	if (offset < 0)
+		FAIL("Missing test node\n");
+
+	fdt_for_each_property_offset(property, fdt, offset)
+		count++;
+
+	if (count != properties) {
+		FAIL("Node '%s': Expected %d properties, got %d\n",
+		     fdt_get_name(fdt, parent_offset, NULL), properties,
+		     count);
+	}
+}
+
+static void check_fdt_next_subnode(void *fdt)
+{
+	int offset;
+	int count = 0;
+
+	fdt_for_each_subnode(offset, fdt, 0) {
+		test_node(fdt, offset);
+		count++;
+	}
+
+	if (count != 2)
+		FAIL("Expected %d tests, got %d\n", 2, count);
+}
+
+int main(int argc, char *argv[])
+{
+	void *fdt;
+
+	test_init(argc, argv);
+	if (argc != 2)
+		CONFIG("Usage: %s <dtb file>", argv[0]);
+
+	fdt = load_blob(argv[1]);
+	if (!fdt)
+		FAIL("No device tree available");
+
+	check_fdt_next_subnode(fdt);
+
+	PASS();
+}
diff --git a/tests/property_iterate.dts b/tests/property_iterate.dts
new file mode 100644
index 000000000000..2ed677e7e6ea
--- /dev/null
+++ b/tests/property_iterate.dts
@@ -0,0 +1,24 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	test1 {
+		test-properties = <3>;
+
+		test {
+			linux,phandle = <0x1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	test2 {
+		test-properties = <0>;
+
+		test {
+		};
+	};
+};
+
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 7eb9b3d33108..6a2662b2abaf 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -269,6 +269,9 @@ libfdt_tests () {
     run_dtc_test -I dts -O dtb -o subnode_iterate.dtb subnode_iterate.dts
     run_test subnode_iterate subnode_iterate.dtb
 
+    run_dtc_test -I dts -O dtb -o property_iterate.dtb property_iterate.dts
+    run_test property_iterate property_iterate.dtb
+
     # Tests for behaviour on various sorts of corrupted trees
     run_test truncated_property
 
-- 
2.9.2

[PATCH v5 3/8] libfdt: Add max phandle retrieval function

[Maxime Ripard]

Add a function to retrieve the highest phandle in a given device tree.

Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_ro.c     | 26 ++++++++++++++++++++++++++
 libfdt/libfdt.h     | 15 +++++++++++++++
 tests/get_phandle.c |  6 ++++++
 3 files changed, 47 insertions(+)

diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 50cce864283c..04590984bd51 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -88,6 +88,32 @@ static int _fdt_string_eq(const void *fdt, int stroffset,
 	return (strlen(p) == len) && (memcmp(p, s, len) == 0);
 }
 
+uint32_t fdt_get_max_phandle(const void *fdt)
+{
+	uint32_t max_phandle = 0;
+	int offset;
+
+	for (offset = fdt_next_node(fdt, -1, NULL);;
+	     offset = fdt_next_node(fdt, offset, NULL)) {
+		uint32_t phandle;
+
+		if (offset == -FDT_ERR_NOTFOUND)
+			return max_phandle;
+
+		if (offset < 0)
+			return (uint32_t)-1;
+
+		phandle = fdt_get_phandle(fdt, offset);
+		if (phandle == (uint32_t)-1)
+			continue;
+
+		if (phandle > max_phandle)
+			max_phandle = phandle;
+	}
+
+	return 0;
+}
+
 int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
 {
 	FDT_CHECK_HEADER(fdt);
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index be109a8aac84..06b84cc16bbf 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -286,6 +286,21 @@ int fdt_move(const void *fdt, void *buf, int bufsize);
  */
 const char *fdt_string(const void *fdt, int stroffset);
 
+/**
+ * fdt_get_max_phandle - retrieves the highest phandle in a tree
+ * @fdt: pointer to the device tree blob
+ *
+ * fdt_get_max_phandle retrieves the highest phandle in the given
+ * device tree. This will ignore badly formatted phandles, or phandles
+ * with a value of 0 or -1.
+ *
+ * returns:
+ *      the highest phandle on success
+ *      0, if no phandle was found in the device tree
+ *      -1, if an error occurred
+ */
+uint32_t fdt_get_max_phandle(const void *fdt);
+
 /**
  * fdt_num_mem_rsv - retrieve the number of memory reserve map entries
  * @fdt: pointer to the device tree blob
diff --git a/tests/get_phandle.c b/tests/get_phandle.c
index 2079591d4c49..22bd7b81b3f0 100644
--- a/tests/get_phandle.c
+++ b/tests/get_phandle.c
@@ -44,6 +44,7 @@ static void check_phandle(void *fdt, const char *path, uint32_t checkhandle)
 
 int main(int argc, char *argv[])
 {
+	uint32_t max;
 	void *fdt;
 
 	test_init(argc, argv);
@@ -53,5 +54,10 @@ int main(int argc, char *argv[])
 	check_phandle(fdt, "/subnode@2", PHANDLE_1);
 	check_phandle(fdt, "/subnode@2/subsubnode@0", PHANDLE_2);
 
+	max = fdt_get_max_phandle(fdt);
+	if (max != PHANDLE_2)
+		FAIL("fdt_get_max_phandle returned 0x%x instead of 0x%x\n",
+		     max, PHANDLE_2);
+
 	PASS();
 }
-- 
2.9.2

[PATCH v5 4/8] libfdt: Add fdt_getprop_namelen_w

[Maxime Ripard]

Add a function to retrieve a writeable property only by the first
characters of its name.

Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/libfdt.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 06b84cc16bbf..0189350fb32f 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -629,6 +629,13 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
  */
 const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
 				const char *name, int namelen, int *lenp);
+static inline void *fdt_getprop_namelen_w(void *fdt, int nodeoffset,
+					  const char *name, int namelen,
+					  int *lenp)
+{
+	return (void *)(uintptr_t)fdt_getprop_namelen(fdt, nodeoffset, name,
+						      namelen, lenp);
+}
 
 /**
  * fdt_getprop - retrieve the value of a given property
-- 
2.9.2

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

[PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[Maxime Ripard]

Add a function to modify inplace only a portion of a property..

This is especially useful when the property is an array of values, and you
want to update one of them without changing the DT size.

Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_wip.c        | 29 +++++++++++++++++++++++++----
 libfdt/libfdt.h         | 21 +++++++++++++++++++++
 tests/setprop_inplace.c | 10 ++++++++++
 tests/testdata.h        |  3 +++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
index c5bbb68d3273..a6e158d1889a 100644
--- a/libfdt/fdt_wip.c
+++ b/libfdt/fdt_wip.c
@@ -55,21 +55,42 @@
 
 #include "libfdt_internal.h"
 
+int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
+					const char *name, int namelen,
+					uint32_t idx, const void *val,
+					int len)
+{
+	void *propval;
+	int proplen;
+
+	propval = fdt_getprop_namelen_w(fdt, nodeoffset, name, namelen,
+					&proplen);
+	if (!propval)
+		return proplen;
+
+	if (proplen < (len + idx))
+		return -FDT_ERR_NOSPACE;
+
+	memcpy((unsigned char *)propval + idx, val, len);
+	return 0;
+}
+
 int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
 			const void *val, int len)
 {
-	void *propval;
+	const void *propval;
 	int proplen;
 
-	propval = fdt_getprop_w(fdt, nodeoffset, name, &proplen);
+	propval = fdt_getprop(fdt, nodeoffset, name, &proplen);
 	if (! propval)
 		return proplen;
 
 	if (proplen != len)
 		return -FDT_ERR_NOSPACE;
 
-	memcpy(propval, val, len);
-	return 0;
+	return fdt_setprop_inplace_namelen_partial(fdt, nodeoffset, name,
+						   strlen(name), 0,
+						   val, len);
 }
 
 static void _fdt_nop_region(void *start, int len)
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 0189350fb32f..e534a34355a4 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1079,6 +1079,27 @@ int fdt_size_cells(const void *fdt, int nodeoffset);
 /* Write-in-place functions                                           */
 /**********************************************************************/
 
+/**
+ * fdt_setprop_inplace_namelen_partial - change a property's value,
+ *                                       but not its size
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @namelen: number of characters of name to consider
+ * @index: index of the property to change in the array
+ * @val: pointer to data to replace the property value with
+ * @len: length of the property value
+ *
+ * Identical to fdt_setprop_inplace(), but modifies the given property
+ * starting from the given index, and using only the first characters
+ * of the name. It is useful when you want to manipulate only one value of
+ * an array and you have a string that doesn't end with \0.
+ */
+int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
+					const char *name, int namelen,
+					uint32_t idx, const void *val,
+					int len);
+
 /**
  * fdt_setprop_inplace - change a property's value, but not its size
  * @fdt: pointer to the device tree blob
diff --git a/tests/setprop_inplace.c b/tests/setprop_inplace.c
index daef182d0b28..80447a0b13ab 100644
--- a/tests/setprop_inplace.c
+++ b/tests/setprop_inplace.c
@@ -83,5 +83,15 @@ int main(int argc, char *argv[])
 	strp = check_getprop(fdt, 0, "prop-str", xlen+1, xstr);
 	verbose_printf("New string value is \"%s\"\n", strp);
 
+	err = fdt_setprop_inplace_namelen_partial(fdt, 0, "compatible",
+						  strlen("compatible"), 4,
+						  TEST_STRING_4_PARTIAL,
+						  strlen(TEST_STRING_4_PARTIAL));
+	if (err)
+		FAIL("Failed to set \"compatible\": %s\n", fdt_strerror(err));
+
+	check_getprop(fdt, 0, "compatible", strlen(TEST_STRING_4_RESULT) + 1,
+		      TEST_STRING_4_RESULT);
+
 	PASS();
 }
diff --git a/tests/testdata.h b/tests/testdata.h
index 576974dddee8..3588778ad159 100644
--- a/tests/testdata.h
+++ b/tests/testdata.h
@@ -21,6 +21,9 @@
 #define TEST_STRING_2	"nastystring: \a\b\t\n\v\f\r\\\""
 #define TEST_STRING_3	"\xde\xad\xbe\xef"
 
+#define TEST_STRING_4_PARTIAL	"foobar"
+#define TEST_STRING_4_RESULT	"testfoobar"
+
 #define TEST_CHAR1	'\r'
 #define TEST_CHAR2	'b'
 #define TEST_CHAR3	'\0'
-- 
2.9.2

[PATCH v5 6/8] libfdt: Introduce FDT_ERR_BADFIXUP

[Maxime Ripard]

Add a FDT_ERR_BADFIXUP error used when an overlay fixup property is not
prooperly formatted.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_strerror.c | 1 +
 libfdt/libfdt.h       | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libfdt/fdt_strerror.c b/libfdt/fdt_strerror.c
index e6c3ceee8c58..0fbfb78dcd93 100644
--- a/libfdt/fdt_strerror.c
+++ b/libfdt/fdt_strerror.c
@@ -76,6 +76,7 @@ static struct fdt_errtabent fdt_errtable[] = {
 	FDT_ERRTABENT(FDT_ERR_BADVERSION),
 	FDT_ERRTABENT(FDT_ERR_BADSTRUCTURE),
 	FDT_ERRTABENT(FDT_ERR_BADLAYOUT),
+	FDT_ERRTABENT(FDT_ERR_BADFIXUP),
 };
 #define FDT_ERRTABSIZE	(sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))
 
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index e534a34355a4..f49b45c29006 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -126,7 +126,11 @@
 	 * value. For example: a property expected to contain a string list
 	 * is not NUL-terminated within the length of its value. */
 
-#define FDT_ERR_MAX		15
+#define FDT_ERR_BADFIXUP	16
+	/* FDT_ERR_BADFIXUP: One of the device tree overlay fixup is
+	 * not formatted properly. */
+
+#define FDT_ERR_MAX		16
 
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
-- 
2.9.2

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

[PATCH v5 7/8] libfdt: Add overlay application function

[Maxime Ripard]

The device tree overlays are a good way to deal with user-modifyable
boards or boards with some kind of an expansion mechanism where we can
easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).

Add a new function to merge overlays with a base device tree.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 libfdt/Makefile.libfdt |   2 +-
 libfdt/fdt_overlay.c   | 641 +++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h        |  31 +++
 libfdt/libfdt_env.h    |   1 +
 4 files changed, 674 insertions(+), 1 deletion(-)
 create mode 100644 libfdt/fdt_overlay.c

diff --git a/libfdt/Makefile.libfdt b/libfdt/Makefile.libfdt
index 09c322ed82ba..098b3f36e668 100644
--- a/libfdt/Makefile.libfdt
+++ b/libfdt/Makefile.libfdt
@@ -7,5 +7,5 @@ LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
 LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
 LIBFDT_VERSION = version.lds
 LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c \
-	fdt_addresses.c
+	fdt_addresses.c fdt_overlay.c
 LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
new file mode 100644
index 000000000000..ab48488b4b72
--- /dev/null
+++ b/libfdt/fdt_overlay.c
@@ -0,0 +1,641 @@
+#include "libfdt_env.h"
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "libfdt_internal.h"
+
+/**
+ * overlay_get_target_phandle - retrieves the target phandle of a fragment
+ * @fdto: pointer to the device tree overlay blob
+ * @fragment: node offset of the fragment in the overlay
+ *
+ * overlay_get_target_phandle() retrieves the target phandle of an
+ * overlay fragment when that fragment uses a phandle (target
+ * property) instead of a path (target-path property).
+ *
+ * returns:
+ *      the phandle pointed by the target property
+ *      0, if the phandle was not found
+ *	-1, if the phandle was malformed
+ */
+static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
+{
+	const uint32_t *val;
+	int len;
+
+	val = fdt_getprop(fdto, fragment, "target", &len);
+	if (!val)
+		return 0;
+
+	if ((len != sizeof(*val)) || (*val == (uint32_t)-1))
+		return (uint32_t)-1;
+
+	return fdt32_to_cpu(*val);
+}
+
+/**
+ * overlay_get_target - retrieves the offset of a fragment's target
+ * @fdt: Base device tree blob
+ * @fdto: Device tree overlay blob
+ * @fragment: node offset of the fragment in the overlay
+ *
+ * overlay_get_target() retrieves the target offset in the base
+ * device tree of a fragment, no matter how the actual targetting is
+ * done (through a phandle or a path)
+ *
+ * returns:
+ *      the targetted node offset in the base device tree
+ *      Negative error code on error
+ */
+static int overlay_get_target(const void *fdt, const void *fdto,
+			      int fragment)
+{
+	uint32_t phandle;
+	const char *path;
+
+	/* Try first to do a phandle based lookup */
+	phandle = overlay_get_target_phandle(fdto, fragment);
+	if (phandle == (uint32_t)-1)
+		return -FDT_ERR_BADPHANDLE;
+
+	if (phandle)
+		return fdt_node_offset_by_phandle(fdt, phandle);
+
+	/* And then a path based lookup */
+	path = fdt_getprop(fdto, fragment, "target-path", NULL);
+	if (!path)
+		return -FDT_ERR_NOTFOUND;
+
+	return fdt_path_offset(fdt, path);
+}
+
+/**
+ * overlay_phandle_add_offset - Increases a phandle by an offset
+ * @fdt: Base device tree blob
+ * @node: Device tree overlay blob
+ * @name: Name of the property to modify (phandle or linux,phandle)
+ * @delta: offset to apply
+ *
+ * overlay_phandle_add_offset() increments a node phandle by a given
+ * offset.
+ *
+ * returns:
+ *      0 on success.
+ *      Negative error code on error
+ */
+static int overlay_phandle_add_offset(void *fdt, int node,
+				      const char *name, uint32_t delta)
+{
+	const uint32_t *val;
+	uint32_t adj_val;
+	int len;
+
+	val = fdt_getprop(fdt, node, name, &len);
+	if (!val)
+		return len;
+
+	if (len != sizeof(*val))
+		return -FDT_ERR_BADSTRUCTURE;
+
+	adj_val = fdt32_to_cpu(*val);
+	if ((adj_val + delta) < adj_val)
+		return -FDT_ERR_BADPHANDLE;
+
+	adj_val += delta;
+	if (adj_val == (uint32_t)-1)
+		return -FDT_ERR_BADPHANDLE;
+
+	return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
+}
+
+/**
+ * overlay_adjust_node_phandles - Offsets the phandles of a node
+ * @fdto: Device tree overlay blob
+ * @node: Offset of the node we want to adjust
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_adjust_node_phandles() adds a constant to all the phandles
+ * of a given node. This is mainly use as part of the overlay
+ * application process, when we want to update all the overlay
+ * phandles to not conflict with the overlays of the base device tree.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_adjust_node_phandles(void *fdto, int node,
+					uint32_t delta)
+{
+	int child;
+	int ret;
+
+	ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		return ret;
+
+	ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		return ret;
+
+	fdt_for_each_subnode(child, fdto, node) {
+		ret = overlay_adjust_node_phandles(fdto, child, delta);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_adjust_local_phandles - Adjust the phandles of a whole overlay
+ * @fdto: Device tree overlay blob
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_adjust_local_phandles() adds a constant to all the
+ * phandles of an overlay. This is mainly use as part of the overlay
+ * application process, when we want to update all the overlay
+ * phandles to not conflict with the overlays of the base device tree.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_adjust_local_phandles(void *fdto, uint32_t delta)
+{
+	/*
+	 * Start adjusting the phandles from the overlay root
+	 */
+	return overlay_adjust_node_phandles(fdto, 0, delta);
+}
+
+/**
+ * overlay_update_local_node_references - Adjust the overlay references
+ * @fdto: Device tree overlay blob
+ * @tree_node: Node offset of the node to operate on
+ * @fixup_node: Node offset of the matching local fixups node
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_update_local_nodes_references() update the phandles
+ * pointing to a node within the device tree overlay by adding a
+ * constant delta.
+ *
+ * This is mainly used as part of a device tree application process,
+ * where you want the device tree overlays phandles to not conflict
+ * with the ones from the base device tree before merging them.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_update_local_node_references(void *fdto,
+						int tree_node,
+						int fixup_node,
+						uint32_t delta)
+{
+	int fixup_prop;
+	int fixup_child;
+	int ret;
+
+	fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
+		const uint32_t *fixup_val;
+		const char *tree_val;
+		const char *name;
+		int fixup_len;
+		int tree_len;
+		int i;
+
+		fixup_val = fdt_getprop_by_offset(fdto, fixup_prop,
+						  &name, &fixup_len);
+		if (!fixup_val)
+			return fixup_len;
+
+		if (fixup_len % sizeof(uint32_t))
+			return -FDT_ERR_BADSTRUCTURE;
+
+		tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
+		if (!tree_val)
+			return tree_len;
+
+		for (i = 0; i < (fixup_len / sizeof(uint32_t)); i++) {
+			uint32_t adj_val, index;
+
+			index = fdt32_to_cpu(fixup_val[i]);
+
+			/*
+			 * phandles to fixup can be unaligned.
+			 *
+			 * Use a memcpy for the architectures that do
+			 * not support unaligned accesses.
+			 */
+			memcpy(&adj_val, tree_val + index, sizeof(adj_val));
+
+			adj_val = fdt32_to_cpu(adj_val);
+			adj_val += delta;
+			adj_val = cpu_to_fdt32(adj_val);
+
+			ret = fdt_setprop_inplace_namelen_partial(fdto,
+								  tree_node,
+								  name,
+								  strlen(name),
+								  index,
+								  &adj_val,
+								  sizeof(adj_val));
+			if (ret)
+				return ret;
+		}
+	}
+
+	fdt_for_each_subnode(fixup_child, fdto, fixup_node) {
+		const char *fixup_child_name = fdt_get_name(fdto, fixup_child,
+							    NULL);
+		int tree_child;
+
+		tree_child = fdt_subnode_offset(fdto, tree_node,
+						fixup_child_name);
+		if (tree_child < 0)
+			return tree_child;
+
+		ret = overlay_update_local_node_references(fdto,
+							   tree_child,
+							   fixup_child,
+							   delta);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_update_local_references - Adjust the overlay references
+ * @fdto: Device tree overlay blob
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_update_local_references() update all the phandles pointing
+ * to a node within the device tree overlay by adding a constant
+ * delta to not conflict with the base overlay.
+ *
+ * This is mainly used as part of a device tree application process,
+ * where you want the device tree overlays phandles to not conflict
+ * with the ones from the base device tree before merging them.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_update_local_references(void *fdto, uint32_t delta)
+{
+	int fixups;
+
+	fixups = fdt_path_offset(fdto, "/__local_fixups__");
+	if (fixups < 0) {
+		/* There's no local phandles to adjust, bail out */
+		if (fixups == -FDT_ERR_NOTFOUND)
+			return 0;
+
+		return fixups;
+	}
+
+	/*
+	 * Update our local references from the root of the tree
+	 */
+	return overlay_update_local_node_references(fdto, 0, fixups,
+						    delta);
+}
+
+/**
+ * overlay_fixup_one_phandle - Set an overlay phandle to the base one
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ * @symbols_off: Node offset of the symbols node in the base device tree
+ * @path: Path to a node holding a phandle in the overlay
+ * @path_len: number of path characters to consider
+ * @name: Name of the property holding the phandle reference in the overlay
+ * @name_len: number of name characters to consider
+ * @index: Index in the overlay property where the phandle is stored
+ * @label: Label of the node referenced by the phandle
+ *
+ * overlay_fixup_one_phandle() resolves an overlay phandle pointing to
+ * a node in the base device tree.
+ *
+ * This is part of the device tree overlay application process, when
+ * you want all the phandles in the overlay to point to the actual
+ * base dt nodes.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_fixup_one_phandle(void *fdt, void *fdto,
+				     int symbols_off,
+				     const char *path, uint32_t path_len,
+				     const char *name, uint32_t name_len,
+				     int index, const char *label)
+{
+	const char *symbol_path;
+	uint32_t phandle;
+	int symbol_off, fixup_off;
+	int prop_len;
+
+	symbol_path = fdt_getprop(fdt, symbols_off, label,
+				  &prop_len);
+	if (!symbol_path)
+		return prop_len;
+
+	symbol_off = fdt_path_offset(fdt, symbol_path);
+	if (symbol_off < 0)
+		return symbol_off;
+
+	phandle = fdt_get_phandle(fdt, symbol_off);
+	if (!phandle)
+		return -FDT_ERR_NOTFOUND;
+
+	fixup_off = fdt_path_offset_namelen(fdto, path, path_len);
+	if (fixup_off < 0)
+		return fixup_off;
+
+	phandle = cpu_to_fdt32(phandle);
+	return fdt_setprop_inplace_namelen_partial(fdto, fixup_off,
+						   name, name_len, index,
+						   &phandle, sizeof(phandle));
+};
+
+/**
+ * overlay_fixup_phandle - Set an overlay phandle to the base one
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ * @symbols_off: Node offset of the symbols node in the base device tree
+ * @property: Property offset in the overlay holding the list of fixups
+ *
+ * overlay_fixup_phandle() resolves all the overlay phandles pointed
+ * to in a __fixups__ property, and updates them to match the phandles
+ * in use in the base device tree.
+ *
+ * This is part of the device tree overlay application process, when
+ * you want all the phandles in the overlay to point to the actual
+ * base dt nodes.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
+				 int property)
+{
+	const char *value;
+	const char *label;
+	int len;
+
+	value = fdt_getprop_by_offset(fdto, property,
+				      &label, &len);
+	if (!value) {
+		if (len == -FDT_ERR_NOTFOUND)
+			return -FDT_ERR_INTERNAL;
+
+		return len;
+	}
+
+	do {
+		const char *path, *name, *fixup_end;
+		const char *fixup_str = value;
+		uint32_t path_len, name_len;
+		uint32_t fixup_len;
+		char *sep, *endptr;
+		int index, ret;
+
+		fixup_end = memchr(value, '\0', len);
+		if (!fixup_end)
+			return -FDT_ERR_BADFIXUP;
+		fixup_len = fixup_end - fixup_str;
+
+		path = fixup_str;
+		sep = memchr(fixup_str, ':', fixup_len);
+		if (!sep || *sep != ':')
+			return -FDT_ERR_BADFIXUP;
+
+		path_len = sep - path;
+		if (path_len == (fixup_len - 1))
+			return -FDT_ERR_BADFIXUP;
+
+		fixup_len -= path_len + 1;
+		name = sep + 1;
+		sep = memchr(name, ':', fixup_len);
+		if (!sep || *sep != ':')
+			return -FDT_ERR_BADFIXUP;
+
+		name_len = sep - name;
+		if (!name_len)
+			return -FDT_ERR_BADFIXUP;
+
+		index = strtoul(sep + 1, &endptr, 10);
+		if ((*endptr != '\0') || (endptr <= (sep + 1)))
+			return -FDT_ERR_BADFIXUP;
+
+		len -= fixup_len + 1;
+		value += fixup_len + 1;
+
+		ret = overlay_fixup_one_phandle(fdt, fdto, symbols_off,
+						path, path_len, name, name_len,
+						index, label);
+		if (ret)
+			return ret;
+	} while (len > 0);
+
+	return 0;
+}
+
+/**
+ * overlay_fixup_phandles - Resolve the overlay phandles to the base
+ *                          device tree
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_fixup_phandles() resolves all the overlay phandles pointing
+ * to nodes in the base device tree.
+ *
+ * This is one of the steps of the device tree overlay application
+ * process, when you want all the phandles in the overlay to point to
+ * the actual base dt nodes.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_fixup_phandles(void *fdt, void *fdto)
+{
+	int fixups_off, symbols_off;
+	int property;
+
+	symbols_off = fdt_path_offset(fdt, "/__symbols__");
+	fixups_off = fdt_path_offset(fdto, "/__fixups__");
+
+	fdt_for_each_property_offset(property, fdto, fixups_off) {
+		int ret;
+
+		ret = overlay_fixup_phandle(fdt, fdto, symbols_off, property);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_apply_node - Merges a node into the base device tree
+ * @fdt: Base Device Tree blob
+ * @target: Node offset in the base device tree to apply the fragment to
+ * @fdto: Device tree overlay blob
+ * @node: Node offset in the overlay holding the changes to merge
+ *
+ * overlay_apply_node() merges a node into a target base device tree
+ * node pointed.
+ *
+ * This is part of the final step in the device tree overlay
+ * application process, when all the phandles have been adjusted and
+ * resolved and you just have to merge overlay into the base device
+ * tree.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_apply_node(void *fdt, int target,
+			      void *fdto, int node)
+{
+	int property;
+	int subnode;
+
+	fdt_for_each_property_offset(property, fdto, node) {
+		const char *name;
+		const void *prop;
+		int prop_len;
+		int ret;
+
+		prop = fdt_getprop_by_offset(fdto, property, &name,
+					     &prop_len);
+		if (prop_len == -FDT_ERR_NOTFOUND)
+			return -FDT_ERR_INTERNAL;
+		if (prop_len < 0)
+			return prop_len;
+
+		ret = fdt_setprop(fdt, target, name, prop, prop_len);
+		if (ret)
+			return ret;
+	}
+
+	fdt_for_each_subnode(subnode, fdto, node) {
+		const char *name = fdt_get_name(fdto, subnode, NULL);
+		int nnode;
+		int ret;
+
+		nnode = fdt_add_subnode(fdt, target, name);
+		if (nnode == -FDT_ERR_EXISTS) {
+			nnode = fdt_subnode_offset(fdt, target, name);
+			if (nnode == -FDT_ERR_NOTFOUND)
+				return -FDT_ERR_INTERNAL;
+		}
+
+		if (nnode < 0)
+			return nnode;
+
+		ret = overlay_apply_node(fdt, nnode, fdto, subnode);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * overlay_merge - Merge an overlay into its base device tree
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_merge() merges an overlay into its base device tree.
+ *
+ * This is the final step in the device tree overlay application
+ * process, when all the phandles have been adjusted and resolved and
+ * you just have to merge overlay into the base device tree.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_merge(void *fdt, void *fdto)
+{
+	int fragment;
+
+	fdt_for_each_subnode(fragment, fdto, 0) {
+		int overlay;
+		int target;
+		int ret;
+
+		/*
+		 * Each fragments will have an __overlay__ node. If
+		 * they don't, it's not supposed to be merged
+		 */
+		overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
+		if (overlay == -FDT_ERR_NOTFOUND)
+			continue;
+
+		if (overlay < 0)
+			return overlay;
+
+		target = overlay_get_target(fdt, fdto, fragment);
+		if (target < 0)
+			return target;
+
+		ret = overlay_apply_node(fdt, target, fdto, overlay);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+int fdt_overlay_apply(void *fdt, void *fdto)
+{
+	uint32_t delta = fdt_get_max_phandle(fdt);
+	int ret;
+
+	FDT_CHECK_HEADER(fdt);
+	FDT_CHECK_HEADER(fdto);
+
+	ret = overlay_adjust_local_phandles(fdto, delta);
+	if (ret)
+		goto err;
+
+	ret = overlay_update_local_references(fdto, delta);
+	if (ret)
+		goto err;
+
+	ret = overlay_fixup_phandles(fdt, fdto);
+	if (ret)
+		goto err;
+
+	ret = overlay_merge(fdt, fdto);
+	if (ret)
+		goto err;
+
+	/*
+	 * The overlay has been damaged, erase its magic.
+	 */
+	fdt_set_magic(fdto, ~0);
+
+	return 0;
+
+err:
+	/*
+	 * The overlay might have been damaged, erase its magic.
+	 */
+	fdt_set_magic(fdto, ~0);
+
+	/*
+	 * The base device tree might have been damaged, erase its
+	 * magic.
+	 */
+	fdt_set_magic(fdt, ~0);
+
+	return ret;
+}
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index f49b45c29006..9892d06c06bc 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1756,6 +1756,37 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name);
  */
 int fdt_del_node(void *fdt, int nodeoffset);
 
+/**
+ * fdt_overlay_apply - Applies a DT overlay on a base DT
+ * @fdt: pointer to the base device tree blob
+ * @fdto: pointer to the device tree overlay blob
+ *
+ * fdt_overlay_apply() will apply the given device tree overlay on the
+ * given base device tree.
+ *
+ * Expect the base device tree to be modified, even if the function
+ * returns an error.
+ *
+ * returns:
+ *	0, on success
+ *	-FDT_ERR_NOSPACE, there's not enough space in the base device tree
+ *	-FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
+ *		properties in the base DT
+ *	-FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
+ *		magic
+ *	-FDT_ERR_INTERNAL,
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADOFFSET,
+ *	-FDT_ERR_BADPATH,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADFIXUP,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_overlay_apply(void *fdt, void *fdto);
+
 /**********************************************************************/
 /* Debugging / informational functions                                */
 /**********************************************************************/
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 9dea97dfff81..99f936dacc60 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -54,6 +54,7 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <stdlib.h>
 #include <string.h>
 
 #ifdef __CHECKER__
-- 
2.9.2

[PATCH v5 8/8] tests: Add tests cases for the overlay code

[Maxime Ripard]

Add some test infrastructure to test that the overlay can be merged, but
also that poorly formatted fixups would fail as expected.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 tests/.gitignore                            |   2 +
 tests/Makefile.tests                        |   3 +-
 tests/overlay.c                             | 232 ++++++++++++++++++++++++++++
 tests/overlay_bad_fixup.c                   |  70 +++++++++
 tests/overlay_bad_fixup_bad_index.dts       |  14 ++
 tests/overlay_bad_fixup_base.dtsi           |  18 +++
 tests/overlay_bad_fixup_empty.dts           |  14 ++
 tests/overlay_bad_fixup_empty_index.dts     |  14 ++
 tests/overlay_bad_fixup_index_trailing.dts  |  14 ++
 tests/overlay_bad_fixup_path_empty_prop.dts |  14 ++
 tests/overlay_bad_fixup_path_only.dts       |  14 ++
 tests/overlay_bad_fixup_path_only_sep.dts   |  14 ++
 tests/overlay_bad_fixup_path_prop.dts       |  14 ++
 tests/overlay_base.dts                      |  21 +++
 tests/overlay_overlay_dtc.dts               |  85 ++++++++++
 tests/overlay_overlay_nodtc.dts             |  82 ++++++++++
 tests/run_tests.sh                          |  32 ++++
 17 files changed, 656 insertions(+), 1 deletion(-)
 create mode 100644 tests/overlay.c
 create mode 100644 tests/overlay_bad_fixup.c
 create mode 100644 tests/overlay_bad_fixup_bad_index.dts
 create mode 100644 tests/overlay_bad_fixup_base.dtsi
 create mode 100644 tests/overlay_bad_fixup_empty.dts
 create mode 100644 tests/overlay_bad_fixup_empty_index.dts
 create mode 100644 tests/overlay_bad_fixup_index_trailing.dts
 create mode 100644 tests/overlay_bad_fixup_path_empty_prop.dts
 create mode 100644 tests/overlay_bad_fixup_path_only.dts
 create mode 100644 tests/overlay_bad_fixup_path_only_sep.dts
 create mode 100644 tests/overlay_bad_fixup_path_prop.dts
 create mode 100644 tests/overlay_base.dts
 create mode 100644 tests/overlay_overlay_dtc.dts
 create mode 100644 tests/overlay_overlay_nodtc.dts

diff --git a/tests/.gitignore b/tests/.gitignore
index fa4616ba28c2..354b565aa095 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -35,6 +35,8 @@ tmp.*
 /nopulate
 /notfound
 /open_pack
+/overlay
+/overlay_bad_fixup
 /parent_offset
 /path-references
 /path_offset
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 196518c83eda..eb039c5a40c1 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -24,7 +24,8 @@ LIB_TESTS_L = get_mem_rsv \
 	utilfdt_test \
 	integer-expressions \
 	property_iterate \
-	subnode_iterate
+	subnode_iterate \
+	overlay overlay_bad_fixup
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
 
 LIBTREE_TESTS_L = truncated_property
diff --git a/tests/overlay.c b/tests/overlay.c
new file mode 100644
index 000000000000..e467b037255f
--- /dev/null
+++ b/tests/overlay.c
@@ -0,0 +1,232 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for DT overlays()
+ * Copyright (C) 2016 Free Electrons
+ * Copyright (C) 2016 NextThing Co.
+ *
+ * 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 int fdt_getprop_u32_by_index(void *fdt, const char *path,
+				    const char *name, int index,
+				    unsigned long *out)
+{
+	const fdt32_t *val;
+	int node_off;
+	int len;
+
+	node_off = fdt_path_offset(fdt, path);
+	if (node_off < 0)
+		return node_off;
+
+	val = fdt_getprop(fdt, node_off, name, &len);
+	if (!val || (len < (sizeof(uint32_t) * (index + 1))))
+		return -FDT_ERR_NOTFOUND;
+
+	*out = fdt32_to_cpu(*(val + index));
+
+	return 0;
+}
+
+static int check_getprop_string_by_name(void *fdt, const char *path,
+					const char *name, const char *val)
+{
+	int node_off;
+
+	node_off = fdt_path_offset(fdt, path);
+	if (node_off < 0)
+		return node_off;
+
+	check_getprop_string(fdt, node_off, name, val);
+
+	return 0;
+}
+
+static int check_getprop_u32_by_name(void *fdt, const char *path,
+				     const char *name, uint32_t val)
+{
+	int node_off;
+
+	node_off = fdt_path_offset(fdt, path);
+	CHECK(node_off < 0);
+
+	check_getprop_cell(fdt, node_off, name, val);
+
+	return 0;
+}
+
+static int check_getprop_null_by_name(void *fdt, const char *path,
+				      const char *name)
+{
+	int node_off;
+
+	node_off = fdt_path_offset(fdt, path);
+	CHECK(node_off < 0);
+
+	check_property(fdt, node_off, name, 0, NULL);
+
+	return 0;
+}
+
+static int fdt_overlay_change_int_property(void *fdt)
+{
+	return check_getprop_u32_by_name(fdt, "/test-node", "test-int-property",
+					 43);
+}
+
+static int fdt_overlay_change_str_property(void *fdt)
+{
+	return check_getprop_string_by_name(fdt, "/test-node",
+					    "test-str-property", "foobar");
+}
+
+static int fdt_overlay_add_str_property(void *fdt)
+{
+	return check_getprop_string_by_name(fdt, "/test-node",
+					    "test-str-property-2", "foobar2");
+}
+
+static int fdt_overlay_add_node(void *fdt)
+{
+	return check_getprop_null_by_name(fdt, "/test-node/new-node",
+					  "new-property");
+}
+
+static int fdt_overlay_add_subnode_property(void *fdt)
+{
+	check_getprop_null_by_name(fdt, "/test-node/sub-test-node",
+				   "sub-test-property");
+	check_getprop_null_by_name(fdt, "/test-node/sub-test-node",
+				   "new-sub-test-property");
+
+	return 0;
+}
+
+static int fdt_overlay_local_phandle(void *fdt)
+{
+	uint32_t local_phandle;
+	unsigned long val = 0;
+	int off;
+
+	off = fdt_path_offset(fdt, "/test-node/new-local-node");
+	CHECK(off < 0);
+
+	local_phandle = fdt_get_phandle(fdt, off);
+	CHECK(!local_phandle);
+
+	CHECK(fdt_getprop_u32_by_index(fdt, "/test-node",
+				       "test-several-phandle",
+				       0, &val));
+	CHECK(val != local_phandle);
+
+	CHECK(fdt_getprop_u32_by_index(fdt, "/test-node",
+				       "test-several-phandle",
+				       1, &val));
+	CHECK(val != local_phandle);
+
+	return 0;
+}
+
+static int fdt_overlay_local_phandles(void *fdt)
+{
+	uint32_t local_phandle, test_phandle;
+	unsigned long val = 0;
+	int off;
+
+	off = fdt_path_offset(fdt, "/test-node/new-local-node");
+	CHECK(off < 0);
+
+	local_phandle = fdt_get_phandle(fdt, off);
+	CHECK(!local_phandle);
+
+	off = fdt_path_offset(fdt, "/test-node");
+	CHECK(off < 0);
+
+	test_phandle = fdt_get_phandle(fdt, off);
+	CHECK(!test_phandle);
+
+	CHECK(fdt_getprop_u32_by_index(fdt, "/test-node",
+				       "test-phandle", 0, &val));
+	CHECK(test_phandle != val);
+
+	CHECK(fdt_getprop_u32_by_index(fdt, "/test-node",
+				       "test-phandle", 1, &val));
+	CHECK(local_phandle != val);
+
+	return 0;
+}
+
+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, *fdt_overlay;
+
+	test_init(argc, argv);
+	if (argc != 3)
+		CONFIG("Usage: %s <base dtb> <overlay dtb>", argv[0]);
+
+	fdt_base = open_dt(argv[1]);
+	fdt_overlay = open_dt(argv[2]);
+
+	/* Apply the overlay */
+	CHECK(fdt_overlay_apply(fdt_base, fdt_overlay));
+
+	fdt_overlay_change_int_property(fdt_base);
+	fdt_overlay_change_str_property(fdt_base);
+	fdt_overlay_add_str_property(fdt_base);
+	fdt_overlay_add_node(fdt_base);
+	fdt_overlay_add_subnode_property(fdt_base);
+
+	/*
+	 * If the base tree has a __symbols__ node, do the tests that
+	 * are only successful with a proper phandle support, and thus
+	 * dtc -@
+	 */
+	if (fdt_path_offset(fdt_base, "/__symbols__") >= 0) {
+		fdt_overlay_local_phandle(fdt_base);
+		fdt_overlay_local_phandles(fdt_base);
+	}
+
+	PASS();
+}
diff --git a/tests/overlay_bad_fixup.c b/tests/overlay_bad_fixup.c
new file mode 100644
index 000000000000..c9d81c350cd7
--- /dev/null
+++ b/tests/overlay_bad_fixup.c
@@ -0,0 +1,70 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for DT overlays()
+ * Copyright (C) 2016 Free Electrons
+ * Copyright (C) 2016 NextThing Co.
+ *
+ * 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, expected)					\
+	{							\
+		err = (code);					\
+		if (err != expected)				\
+			FAIL(#code ": %s", fdt_strerror(err));	\
+	}
+
+/* 4k ought to be enough for anybody */
+#define FDT_COPY_SIZE	(4 * 1024)
+
+static void *open_dt(char *path)
+{
+	void *dt, *copy;
+	int err;
+
+	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), 0);
+
+	return copy;
+}
+
+int main(int argc, char *argv[])
+{
+	void *fdt_base, *fdt_overlay;
+	int err;
+
+	test_init(argc, argv);
+	if (argc != 3)
+		CONFIG("Usage: %s <base dtb> <overlay dtb>", argv[0]);
+
+	fdt_base = open_dt(argv[1]);
+	fdt_overlay = open_dt(argv[2]);
+
+	/* Apply the overlay */
+	CHECK(fdt_overlay_apply(fdt_base, fdt_overlay), -FDT_ERR_BADFIXUP);
+
+	PASS();
+}
diff --git a/tests/overlay_bad_fixup_bad_index.dts b/tests/overlay_bad_fixup_bad_index.dts
new file mode 100644
index 000000000000..b5cf13137169
--- /dev/null
+++ b/tests/overlay_bad_fixup_bad_index.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0:target:ab";
+	};
+};
diff --git a/tests/overlay_bad_fixup_base.dtsi b/tests/overlay_bad_fixup_base.dtsi
new file mode 100644
index 000000000000..216bcab52263
--- /dev/null
+++ b/tests/overlay_bad_fixup_base.dtsi
@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+
+/ {
+	fragment@0 {
+		target = <0xffffffff>;
+
+		__overlay__ {
+			test-property;
+		};
+	};
+};
diff --git a/tests/overlay_bad_fixup_empty.dts b/tests/overlay_bad_fixup_empty.dts
new file mode 100644
index 000000000000..e111db4c8527
--- /dev/null
+++ b/tests/overlay_bad_fixup_empty.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "";
+	};
+};
diff --git a/tests/overlay_bad_fixup_empty_index.dts b/tests/overlay_bad_fixup_empty_index.dts
new file mode 100644
index 000000000000..9e12e2177ad5
--- /dev/null
+++ b/tests/overlay_bad_fixup_empty_index.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0:target:";
+	};
+};
diff --git a/tests/overlay_bad_fixup_index_trailing.dts b/tests/overlay_bad_fixup_index_trailing.dts
new file mode 100644
index 000000000000..f586bef4d374
--- /dev/null
+++ b/tests/overlay_bad_fixup_index_trailing.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0:target:0a";
+	};
+};
diff --git a/tests/overlay_bad_fixup_path_empty_prop.dts b/tests/overlay_bad_fixup_path_empty_prop.dts
new file mode 100644
index 000000000000..608b5f9247b5
--- /dev/null
+++ b/tests/overlay_bad_fixup_path_empty_prop.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0::";
+	};
+};
diff --git a/tests/overlay_bad_fixup_path_only.dts b/tests/overlay_bad_fixup_path_only.dts
new file mode 100644
index 000000000000..2485dd965ee5
--- /dev/null
+++ b/tests/overlay_bad_fixup_path_only.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0";
+	};
+};
diff --git a/tests/overlay_bad_fixup_path_only_sep.dts b/tests/overlay_bad_fixup_path_only_sep.dts
new file mode 100644
index 000000000000..3cbf6c40fba5
--- /dev/null
+++ b/tests/overlay_bad_fixup_path_only_sep.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0:";
+	};
+};
diff --git a/tests/overlay_bad_fixup_path_prop.dts b/tests/overlay_bad_fixup_path_prop.dts
new file mode 100644
index 000000000000..ca79b52bcb22
--- /dev/null
+++ b/tests/overlay_bad_fixup_path_prop.dts
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+	__fixups__ {
+		test = "/fragment@0:target";
+	};
+};
diff --git a/tests/overlay_base.dts b/tests/overlay_base.dts
new file mode 100644
index 000000000000..2603adb6821e
--- /dev/null
+++ b/tests/overlay_base.dts
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+
+/ {
+	test: test-node {
+		test-int-property = <42>;
+		test-str-property = "foo";
+
+		subtest: sub-test-node {
+			sub-test-property;
+		};
+	};
+};
+
+
diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
new file mode 100644
index 000000000000..30d2362f746b
--- /dev/null
+++ b/tests/overlay_overlay_dtc.dts
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/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;
+			};
+		};
+	};
+};
diff --git a/tests/overlay_overlay_nodtc.dts b/tests/overlay_overlay_nodtc.dts
new file mode 100644
index 000000000000..e8d0f96d889c
--- /dev/null
+++ b/tests/overlay_overlay_nodtc.dts
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2016 NextThing Co
+ * Copyright (c) 2016 Free Electrons
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+/dts-v1/;
+
+/ {
+	fragment@0 {
+		target-path = "/test-node";
+
+		__overlay__ {
+			test-int-property = <43>;
+		};
+	};
+
+	/* Test that we can replace a string by a longer one */
+	fragment@1 {
+		target-path = "/test-node";
+
+		__overlay__ {
+			test-str-property = "foobar";
+		};
+	};
+
+	/* Test that we add a new property */
+	fragment@2 {
+		target-path = "/test-node";
+
+		__overlay__ {
+			test-str-property-2 = "foobar2";
+		};
+	};
+
+	fragment@3 {
+		target-path = "/test-node";
+
+		__overlay__ {
+			new-node {
+				new-property;
+			};
+		};
+	};
+
+	fragment@4 {
+		target-path = "/";
+
+		__overlay__ {
+			local: new-local-node {
+				new-property;
+			};
+		};
+	};
+
+	fragment@5 {
+		target-path = "/";
+
+		__overlay__ {
+			test-several-phandle = <&local>, <&local>;
+		};
+	};
+
+	fragment@6 {
+		target-path = "/test-node";
+
+		__overlay__ {
+			sub-test-node {
+				new-sub-test-property;
+			};
+		};
+	};
+
+	__local_fixups__ {
+		fragment@5 {
+			__overlay__ {
+				test-several-phandle = <0 4>;
+			};
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 6a2662b2abaf..785a8deed997 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -145,6 +145,37 @@ run_fdtdump_test() {
     base_run_test sh fdtdump-runtest.sh "$file"
 }
 
+BAD_FIXUP_TREES="bad_index \
+		empty \
+		empty_index \
+		index_trailing \
+		path_empty_prop \
+		path_only \
+		path_only_sep \
+		path_prop"
+
+overlay_tests () {
+    # Overlay tests that don't require overlay support in dtc
+    run_dtc_test -I dts -O dtb -o overlay_base.dtb overlay_base.dts
+    run_dtc_test -I dts -O dtb -o overlay_overlay.dtb overlay_overlay_nodtc.dts
+    run_test overlay overlay_base.dtb overlay_overlay.dtb
+
+    # Overlay tests that requires overlay support in dtc
+    echo "/dts-v1/; / {};" | $DTC -@ > /dev/null 2>&1
+    if [ $? -eq 0 ]; then
+        run_dtc_test -@ -I dts -O dtb -o overlay_base.dtb overlay_base.dts
+        run_dtc_test -@ -I dts -O dtb -o overlay_overlay.dtb overlay_overlay_dtc.dts
+        run_test overlay overlay_base.dtb overlay_overlay.dtb
+    fi
+
+    # Bad fixup tests
+    for test in $BAD_FIXUP_TREES; do
+	tree="overlay_bad_fixup_$test"
+	run_dtc_test -I dts -O dtb -o $tree.dtb $tree.dts
+	run_test overlay_bad_fixup overlay_base.dtb $tree.dtb
+    done
+}
+
 tree1_tests () {
     TREE=$1
 
@@ -258,6 +289,7 @@ libfdt_tests () {
     run_test appendprop2 appendprop1.test.dtb
     run_dtc_test -I dts -O dtb -o appendprop.test.dtb appendprop.dts
     run_test dtbs_equal_ordered appendprop2.test.dtb appendprop.test.dtb
+    overlay_tests
 
     for basetree in test_tree1.dtb sw_tree1.test.dtb rw_tree1.test.dtb; do
 	run_test nopulate $basetree
-- 
2.9.2

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[David Gibson]

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

On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> Add a function to modify inplace only a portion of a property..
> 
> This is especially useful when the property is an array of values, and you
> want to update one of them without changing the DT size.
> 
> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Applied, though I made a couple of tiny tweaks, see below.

> ---
>  libfdt/fdt_wip.c        | 29 +++++++++++++++++++++++++----
>  libfdt/libfdt.h         | 21 +++++++++++++++++++++
>  tests/setprop_inplace.c | 10 ++++++++++
>  tests/testdata.h        |  3 +++
>  4 files changed, 59 insertions(+), 4 deletions(-)
> 
> diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
> index c5bbb68d3273..a6e158d1889a 100644
> --- a/libfdt/fdt_wip.c
> +++ b/libfdt/fdt_wip.c
> @@ -55,21 +55,42 @@
>  
>  #include "libfdt_internal.h"
>  
> +int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
> +					const char *name, int namelen,
> +					uint32_t idx, const void *val,
> +					int len)
> +{
> +	void *propval;
> +	int proplen;
> +
> +	propval = fdt_getprop_namelen_w(fdt, nodeoffset, name, namelen,
> +					&proplen);
> +	if (!propval)
> +		return proplen;
> +
> +	if (proplen < (len + idx))
> +		return -FDT_ERR_NOSPACE;
> +
> +	memcpy((unsigned char *)propval + idx, val, len);

The unsigned qualifier is not necessary, so I removed it.

> +	return 0;
> +}
> +
>  int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
>  			const void *val, int len)
>  {
> -	void *propval;
> +	const void *propval;
>  	int proplen;
>  
> -	propval = fdt_getprop_w(fdt, nodeoffset, name, &proplen);
> +	propval = fdt_getprop(fdt, nodeoffset, name, &proplen);
>  	if (! propval)
>  		return proplen;
>  
>  	if (proplen != len)
>  		return -FDT_ERR_NOSPACE;
>  
> -	memcpy(propval, val, len);
> -	return 0;
> +	return fdt_setprop_inplace_namelen_partial(fdt, nodeoffset, name,
> +						   strlen(name), 0,
> +						   val, len);
>  }
>  
>  static void _fdt_nop_region(void *start, int len)
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index 0189350fb32f..e534a34355a4 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1079,6 +1079,27 @@ int fdt_size_cells(const void *fdt, int nodeoffset);
>  /* Write-in-place functions                                           */
>  /**********************************************************************/
>  
> +/**
> + * fdt_setprop_inplace_namelen_partial - change a property's value,
> + *                                       but not its size
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node whose property to change
> + * @name: name of the property to change
> + * @namelen: number of characters of name to consider
> + * @index: index of the property to change in the array

I changed this comment to match the new function signature.

> + * @val: pointer to data to replace the property value with
> + * @len: length of the property value
> + *
> + * Identical to fdt_setprop_inplace(), but modifies the given property
> + * starting from the given index, and using only the first characters
> + * of the name. It is useful when you want to manipulate only one value of
> + * an array and you have a string that doesn't end with \0.
> + */
> +int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
> +					const char *name, int namelen,
> +					uint32_t idx, const void *val,
> +					int len);
> +
>  /**
>   * fdt_setprop_inplace - change a property's value, but not its size
>   * @fdt: pointer to the device tree blob
> diff --git a/tests/setprop_inplace.c b/tests/setprop_inplace.c
> index daef182d0b28..80447a0b13ab 100644
> --- a/tests/setprop_inplace.c
> +++ b/tests/setprop_inplace.c
> @@ -83,5 +83,15 @@ int main(int argc, char *argv[])
>  	strp = check_getprop(fdt, 0, "prop-str", xlen+1, xstr);
>  	verbose_printf("New string value is \"%s\"\n", strp);
>  
> +	err = fdt_setprop_inplace_namelen_partial(fdt, 0, "compatible",
> +						  strlen("compatible"), 4,
> +						  TEST_STRING_4_PARTIAL,
> +						  strlen(TEST_STRING_4_PARTIAL));
> +	if (err)
> +		FAIL("Failed to set \"compatible\": %s\n", fdt_strerror(err));
> +
> +	check_getprop(fdt, 0, "compatible", strlen(TEST_STRING_4_RESULT) + 1,
> +		      TEST_STRING_4_RESULT);
> +
>  	PASS();
>  }
> diff --git a/tests/testdata.h b/tests/testdata.h
> index 576974dddee8..3588778ad159 100644
> --- a/tests/testdata.h
> +++ b/tests/testdata.h
> @@ -21,6 +21,9 @@
>  #define TEST_STRING_2	"nastystring: \a\b\t\n\v\f\r\\\""
>  #define TEST_STRING_3	"\xde\xad\xbe\xef"
>  
> +#define TEST_STRING_4_PARTIAL	"foobar"
> +#define TEST_STRING_4_RESULT	"testfoobar"
> +
>  #define TEST_CHAR1	'\r'
>  #define TEST_CHAR2	'b'
>  #define TEST_CHAR3	'\0'

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

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[Maxime Ripard]

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

Hi David,

On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > Add a function to modify inplace only a portion of a property..
> > 
> > This is especially useful when the property is an array of values, and you
> > want to update one of them without changing the DT size.
> > 
> > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Applied, though I made a couple of tiny tweaks, see below.

Awesome, thanks!

Do you still have comments on the last 3 patches?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[David Gibson]

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

On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> Hi David,
> 
> On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > Add a function to modify inplace only a portion of a property..
> > > 
> > > This is especially useful when the property is an array of values, and you
> > > want to update one of them without changing the DT size.
> > > 
> > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > 
> > Applied, though I made a couple of tiny tweaks, see below.
> 
> Awesome, thanks!
> 
> Do you still have comments on the last 3 patches?

Sorry, I got sidetracked by other things.

The only remaining thing I'd like some changes in is the error
reporting.

The biggset problem is that in some places you are returning
BADSTRUCTURE for badly formatter fixup or other overlay properties.
BADSTRUCTURE should only be used when there is a problem in the
encoding of the tree structure itself, not a problem in the encoding
within a specific property or properties.

I don't think your code should ever generate BADSTRUCTURE directly
(but it could return it when it gets that value returned from a lower
level function).

What I was hoping to have a closer look at was to see if I'd prefer
just a single 'BADOVERLAY' error for any problem in te overlay
encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
like that.  I'll try to get a closer look, but it probably won't
happen this week (I'm at KVM Forum).

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

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[Maxime Ripard]

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

Hi David,

On Mon, Aug 22, 2016 at 09:01:56AM -0400, David Gibson wrote:
> On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> > Hi David,
> > 
> > On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > > Add a function to modify inplace only a portion of a property..
> > > > 
> > > > This is especially useful when the property is an array of values, and you
> > > > want to update one of them without changing the DT size.
> > > > 
> > > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > 
> > > Applied, though I made a couple of tiny tweaks, see below.
> > 
> > Awesome, thanks!
> > 
> > Do you still have comments on the last 3 patches?
> 
> Sorry, I got sidetracked by other things.
> 
> The only remaining thing I'd like some changes in is the error
> reporting.
> 
> The biggset problem is that in some places you are returning
> BADSTRUCTURE for badly formatter fixup or other overlay properties.
> BADSTRUCTURE should only be used when there is a problem in the
> encoding of the tree structure itself, not a problem in the encoding
> within a specific property or properties.
> 
> I don't think your code should ever generate BADSTRUCTURE directly
> (but it could return it when it gets that value returned from a lower
> level function).
> 
> What I was hoping to have a closer look at was to see if I'd prefer
> just a single 'BADOVERLAY' error for any problem in te overlay
> encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
> like that.  I'll try to get a closer look, but it probably won't
> happen this week (I'm at KVM Forum).

Ok, don't worry this was just to get a status.

Enjoy the KVM Forum, I'll wait for your review.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[Maxime Ripard]

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

Hi David,

On Mon, Aug 22, 2016 at 09:01:56AM -0400, David Gibson wrote:
> On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> > Hi David,
> > 
> > On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > > Add a function to modify inplace only a portion of a property..
> > > > 
> > > > This is especially useful when the property is an array of values, and you
> > > > want to update one of them without changing the DT size.
> > > > 
> > > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > 
> > > Applied, though I made a couple of tiny tweaks, see below.
> > 
> > Awesome, thanks!
> > 
> > Do you still have comments on the last 3 patches?
> 
> Sorry, I got sidetracked by other things.
> 
> The only remaining thing I'd like some changes in is the error
> reporting.
> 
> The biggset problem is that in some places you are returning
> BADSTRUCTURE for badly formatter fixup or other overlay properties.
> BADSTRUCTURE should only be used when there is a problem in the
> encoding of the tree structure itself, not a problem in the encoding
> within a specific property or properties.
> 
> I don't think your code should ever generate BADSTRUCTURE directly
> (but it could return it when it gets that value returned from a lower
> level function).
> 
> What I was hoping to have a closer look at was to see if I'd prefer
> just a single 'BADOVERLAY' error for any problem in te overlay
> encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
> like that.  I'll try to get a closer look, but it probably won't
> happen this week (I'm at KVM Forum).

Did you have time to think about this?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v5 7/8] libfdt: Add overlay application function

[David Gibson]

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

On Fri, Jul 29, 2016 at 11:55:50AM +0200, Maxime Ripard wrote:
> The device tree overlays are a good way to deal with user-modifyable
> boards or boards with some kind of an expansion mechanism where we can
> easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).
> 
> Add a new function to merge overlays with a base device tree.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Sorry it's taken me so long to review this properly.

As I mentioned before, the only remaining concern I have is reporting
errors as sensibly as we can.  So here's my thoughts on that.  This is
all negotiable if you have reasons to suggest something else, of
course - especially the names and wordings.

I think you should add these error codes:

FDT_ERR_BADOVERLAY
	This indicates an incorrectly formatted overlay.  That means
	something wrong in the node or property contents which means
	the overlay can't properly be interpreted.  An error in the
	encoding *of* the nodes or properties would (still) be
	BADSTRUCTURE.  This should always be an error that can be
	detected (at least theoretically) in the overlay *alone*
	without reference to the base tree.  i.e. it implies that this
	overlay is bad in a way which means it couldn't apply to *any*
	base tree.

FDT_ERR_NOPHANDLES
	Means we ran out / overran the valid phandles while adjusting
	them during overlay application.

And I suggest an extension to the meaning of one existing code:

FDT_ERR_BADPHANDLE
	Should be expanded to cover encountering a badly formatted
	phandle property (e.g. wrong length) as well as passing a bad
	phandle value (0 or -1).


> ---
>  libfdt/Makefile.libfdt |   2 +-
>  libfdt/fdt_overlay.c   | 641 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libfdt/libfdt.h        |  31 +++
>  libfdt/libfdt_env.h    |   1 +
>  4 files changed, 674 insertions(+), 1 deletion(-)
>  create mode 100644 libfdt/fdt_overlay.c
> 
> diff --git a/libfdt/Makefile.libfdt b/libfdt/Makefile.libfdt
> index 09c322ed82ba..098b3f36e668 100644
> --- a/libfdt/Makefile.libfdt
> +++ b/libfdt/Makefile.libfdt
> @@ -7,5 +7,5 @@ LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
>  LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
>  LIBFDT_VERSION = version.lds
>  LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c \
> -	fdt_addresses.c
> +	fdt_addresses.c fdt_overlay.c
>  LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
> diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> new file mode 100644
> index 000000000000..ab48488b4b72
> --- /dev/null
> +++ b/libfdt/fdt_overlay.c
> @@ -0,0 +1,641 @@
> +#include "libfdt_env.h"
> +
> +#include <fdt.h>
> +#include <libfdt.h>
> +
> +#include "libfdt_internal.h"
> +
> +/**
> + * overlay_get_target_phandle - retrieves the target phandle of a fragment
> + * @fdto: pointer to the device tree overlay blob
> + * @fragment: node offset of the fragment in the overlay
> + *
> + * overlay_get_target_phandle() retrieves the target phandle of an
> + * overlay fragment when that fragment uses a phandle (target
> + * property) instead of a path (target-path property).
> + *
> + * returns:
> + *      the phandle pointed by the target property
> + *      0, if the phandle was not found
> + *	-1, if the phandle was malformed
> + */
> +static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
> +{
> +	const uint32_t *val;
> +	int len;

It might be easier to handle errors if you fold this into its caller.

> +
> +	val = fdt_getprop(fdto, fragment, "target", &len);
> +	if (!val)
> +		return 0;

> +	if ((len != sizeof(*val)) || (*val == (uint32_t)-1))
> +		return (uint32_t)-1;
> +
> +	return fdt32_to_cpu(*val);
> +}
> +
> +/**
> + * overlay_get_target - retrieves the offset of a fragment's target
> + * @fdt: Base device tree blob
> + * @fdto: Device tree overlay blob
> + * @fragment: node offset of the fragment in the overlay
> + *
> + * overlay_get_target() retrieves the target offset in the base
> + * device tree of a fragment, no matter how the actual targetting is
> + * done (through a phandle or a path)
> + *
> + * returns:
> + *      the targetted node offset in the base device tree
> + *      Negative error code on error
> + */
> +static int overlay_get_target(const void *fdt, const void *fdto,
> +			      int fragment)
> +{
> +	uint32_t phandle;
> +	const char *path;
> +
> +	/* Try first to do a phandle based lookup */
> +	phandle = overlay_get_target_phandle(fdto, fragment);
> +	if (phandle == (uint32_t)-1)
> +		return -FDT_ERR_BADPHANDLE;

BADPHANDLE is good here, but see the expansion to its meaning above.

> +
> +	if (phandle)
> +		return fdt_node_offset_by_phandle(fdt, phandle);
> +
> +	/* And then a path based lookup */
> +	path = fdt_getprop(fdto, fragment, "target-path", NULL);
> +	if (!path)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	return fdt_path_offset(fdt, path);
> +}
> +
> +/**
> + * overlay_phandle_add_offset - Increases a phandle by an offset
> + * @fdt: Base device tree blob
> + * @node: Device tree overlay blob
> + * @name: Name of the property to modify (phandle or linux,phandle)
> + * @delta: offset to apply
> + *
> + * overlay_phandle_add_offset() increments a node phandle by a given
> + * offset.
> + *
> + * returns:
> + *      0 on success.
> + *      Negative error code on error
> + */
> +static int overlay_phandle_add_offset(void *fdt, int node,
> +				      const char *name, uint32_t delta)
> +{
> +	const uint32_t *val;
> +	uint32_t adj_val;
> +	int len;
> +
> +	val = fdt_getprop(fdt, node, name, &len);
> +	if (!val)
> +		return len;
> +
> +	if (len != sizeof(*val))
> +		return -FDT_ERR_BADSTRUCTURE;

Should be BADPHANDLE.

> +	adj_val = fdt32_to_cpu(*val);
> +	if ((adj_val + delta) < adj_val)
> +		return -FDT_ERR_BADPHANDLE;

Should be NOPHANDLES.

> +	adj_val += delta;
> +	if (adj_val == (uint32_t)-1)
> +		return -FDT_ERR_BADPHANDLE;

Should be NOPHANDLES.

> +	return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
> +}
> +
> +/**
> + * overlay_adjust_node_phandles - Offsets the phandles of a node
> + * @fdto: Device tree overlay blob
> + * @node: Offset of the node we want to adjust
> + * @delta: Offset to shift the phandles of
> + *
> + * overlay_adjust_node_phandles() adds a constant to all the phandles
> + * of a given node. This is mainly use as part of the overlay
> + * application process, when we want to update all the overlay
> + * phandles to not conflict with the overlays of the base device tree.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_adjust_node_phandles(void *fdto, int node,
> +					uint32_t delta)
> +{
> +	int child;
> +	int ret;
> +
> +	ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
> +	if (ret && ret != -FDT_ERR_NOTFOUND)
> +		return ret;
> +
> +	ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta);
> +	if (ret && ret != -FDT_ERR_NOTFOUND)
> +		return ret;
> +
> +	fdt_for_each_subnode(child, fdto, node) {
> +		ret = overlay_adjust_node_phandles(fdto, child, delta);
> +		if (ret)
> +			return ret;
> +	}

I'm wondering if it might be an idea to add a core set_phandle()
function, which paired with get_phandle() would check that 'phandle'
and 'linux,phandle' are in sync.  But that's not within scope for this
patch.


> +	return 0;
> +}
> +
> +/**
> + * overlay_adjust_local_phandles - Adjust the phandles of a whole overlay
> + * @fdto: Device tree overlay blob
> + * @delta: Offset to shift the phandles of
> + *
> + * overlay_adjust_local_phandles() adds a constant to all the
> + * phandles of an overlay. This is mainly use as part of the overlay
> + * application process, when we want to update all the overlay
> + * phandles to not conflict with the overlays of the base device tree.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_adjust_local_phandles(void *fdto, uint32_t delta)
> +{
> +	/*
> +	 * Start adjusting the phandles from the overlay root
> +	 */
> +	return overlay_adjust_node_phandles(fdto, 0, delta);
> +}
> +
> +/**
> + * overlay_update_local_node_references - Adjust the overlay references
> + * @fdto: Device tree overlay blob
> + * @tree_node: Node offset of the node to operate on
> + * @fixup_node: Node offset of the matching local fixups node
> + * @delta: Offset to shift the phandles of
> + *
> + * overlay_update_local_nodes_references() update the phandles
> + * pointing to a node within the device tree overlay by adding a
> + * constant delta.
> + *
> + * This is mainly used as part of a device tree application process,
> + * where you want the device tree overlays phandles to not conflict
> + * with the ones from the base device tree before merging them.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_update_local_node_references(void *fdto,
> +						int tree_node,
> +						int fixup_node,
> +						uint32_t delta)
> +{
> +	int fixup_prop;
> +	int fixup_child;
> +	int ret;
> +
> +	fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
> +		const uint32_t *fixup_val;
> +		const char *tree_val;
> +		const char *name;
> +		int fixup_len;
> +		int tree_len;
> +		int i;
> +
> +		fixup_val = fdt_getprop_by_offset(fdto, fixup_prop,
> +						  &name, &fixup_len);
> +		if (!fixup_val)
> +			return fixup_len;
> +		if (fixup_len % sizeof(uint32_t))
> +			return -FDT_ERR_BADSTRUCTURE;


Should be BADOVERLAY.


> +
> +		tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
> +		if (!tree_val)
> +			return tree_len;

A NOTFOUND returned from fdt_getprop() should become BADOVERLAY (the
overlay has a local fixup that doesn't resolve).

> +		for (i = 0; i < (fixup_len / sizeof(uint32_t)); i++) {
> +			uint32_t adj_val, index;
> +
> +			index = fdt32_to_cpu(fixup_val[i]);
> +
> +			/*
> +			 * phandles to fixup can be unaligned.
> +			 *
> +			 * Use a memcpy for the architectures that do
> +			 * not support unaligned accesses.
> +			 */
> +			memcpy(&adj_val, tree_val + index, sizeof(adj_val));
> +
> +			adj_val = fdt32_to_cpu(adj_val);
> +			adj_val += delta;

An overflow here (adjval + delta < adjval, or adjval + delta == -1)
should trigger BADOVERLAY - it means we have a local fixup applying to
something that isn't a valid phandle (or we would have hit an error
while adjusting the phandle properties themselves).

> +			adj_val = cpu_to_fdt32(adj_val);
> +
> +			ret = fdt_setprop_inplace_namelen_partial(fdto,
> +								  tree_node,
> +								  name,
> +								  strlen(name),
> +								  index,
> +								  &adj_val,
> +								  sizeof(adj_val));
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	fdt_for_each_subnode(fixup_child, fdto, fixup_node) {
> +		const char *fixup_child_name = fdt_get_name(fdto, fixup_child,
> +							    NULL);
> +		int tree_child;
> +
> +		tree_child = fdt_subnode_offset(fdto, tree_node,
> +						fixup_child_name);
> +		if (tree_child < 0)
> +			return tree_child;

Again a NOTFOUND should become BADOVERLAY, because the overlay is
attempting to fixup something that doesn't exist.

> +
> +		ret = overlay_update_local_node_references(fdto,
> +							   tree_child,
> +							   fixup_child,
> +							   delta);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * overlay_update_local_references - Adjust the overlay references
> + * @fdto: Device tree overlay blob
> + * @delta: Offset to shift the phandles of
> + *
> + * overlay_update_local_references() update all the phandles pointing
> + * to a node within the device tree overlay by adding a constant
> + * delta to not conflict with the base overlay.
> + *
> + * This is mainly used as part of a device tree application process,
> + * where you want the device tree overlays phandles to not conflict
> + * with the ones from the base device tree before merging them.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_update_local_references(void *fdto, uint32_t delta)
> +{
> +	int fixups;
> +
> +	fixups = fdt_path_offset(fdto, "/__local_fixups__");
> +	if (fixups < 0) {
> +		/* There's no local phandles to adjust, bail out */
> +		if (fixups == -FDT_ERR_NOTFOUND)
> +			return 0;
> +
> +		return fixups;
> +	}
> +
> +	/*
> +	 * Update our local references from the root of the tree
> +	 */
> +	return overlay_update_local_node_references(fdto, 0, fixups,
> +						    delta);
> +}
> +
> +/**
> + * overlay_fixup_one_phandle - Set an overlay phandle to the base one
> + * @fdt: Base Device Tree blob
> + * @fdto: Device tree overlay blob
> + * @symbols_off: Node offset of the symbols node in the base device tree
> + * @path: Path to a node holding a phandle in the overlay
> + * @path_len: number of path characters to consider
> + * @name: Name of the property holding the phandle reference in the overlay
> + * @name_len: number of name characters to consider
> + * @index: Index in the overlay property where the phandle is stored
> + * @label: Label of the node referenced by the phandle
> + *
> + * overlay_fixup_one_phandle() resolves an overlay phandle pointing to
> + * a node in the base device tree.
> + *
> + * This is part of the device tree overlay application process, when
> + * you want all the phandles in the overlay to point to the actual
> + * base dt nodes.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_fixup_one_phandle(void *fdt, void *fdto,
> +				     int symbols_off,
> +				     const char *path, uint32_t path_len,
> +				     const char *name, uint32_t name_len,
> +				     int index, const char *label)
> +{
> +	const char *symbol_path;
> +	uint32_t phandle;
> +	int symbol_off, fixup_off;
> +	int prop_len;
> +
> +	symbol_path = fdt_getprop(fdt, symbols_off, label,
> +				  &prop_len);
> +	if (!symbol_path)
> +		return prop_len;

So here, applying an overlay to a base tree it doesn't belong with
will return NOTFOUND.  That seems reasonable to me.

> +	symbol_off = fdt_path_offset(fdt, symbol_path);
> +	if (symbol_off < 0)
> +		return symbol_off;

And here.

> +	phandle = fdt_get_phandle(fdt, symbol_off);
> +	if (!phandle)
> +		return -FDT_ERR_NOTFOUND;

Should return BADPHANDLE if phandle == -1 here.

For phandle == 0, NOTFOUND is probably ok for now.  I wonder if we
should actually attempt to allocate and add a phandle to the base tree
in this case.

> +
> +	fixup_off = fdt_path_offset_namelen(fdto, path, path_len);
> +	if (fixup_off < 0)
> +		return fixup_off;

Here, a NOTFOUND should become BADOVERLAY.

> +
> +	phandle = cpu_to_fdt32(phandle);
> +	return fdt_setprop_inplace_namelen_partial(fdto, fixup_off,
> +						   name, name_len, index,
> +						   &phandle, sizeof(phandle));

And here both NOTFOUND and NOSPACE should become BADOVERLAY.

> +};
> +
> +/**
> + * overlay_fixup_phandle - Set an overlay phandle to the base one
> + * @fdt: Base Device Tree blob
> + * @fdto: Device tree overlay blob
> + * @symbols_off: Node offset of the symbols node in the base device tree
> + * @property: Property offset in the overlay holding the list of fixups
> + *
> + * overlay_fixup_phandle() resolves all the overlay phandles pointed
> + * to in a __fixups__ property, and updates them to match the phandles
> + * in use in the base device tree.
> + *
> + * This is part of the device tree overlay application process, when
> + * you want all the phandles in the overlay to point to the actual
> + * base dt nodes.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
> +				 int property)
> +{
> +	const char *value;
> +	const char *label;
> +	int len;
> +
> +	value = fdt_getprop_by_offset(fdto, property,
> +				      &label, &len);
> +	if (!value) {
> +		if (len == -FDT_ERR_NOTFOUND)
> +			return -FDT_ERR_INTERNAL;
> +		return len;
> +	}
> +
> +	do {
> +		const char *path, *name, *fixup_end;
> +		const char *fixup_str = value;
> +		uint32_t path_len, name_len;
> +		uint32_t fixup_len;
> +		char *sep, *endptr;
> +		int index, ret;
> +
> +		fixup_end = memchr(value, '\0', len);
> +		if (!fixup_end)
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		fixup_len = fixup_end - fixup_str;
> +
> +		path = fixup_str;
> +		sep = memchr(fixup_str, ':', fixup_len);
> +		if (!sep || *sep != ':')
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		path_len = sep - path;
> +		if (path_len == (fixup_len - 1))
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		fixup_len -= path_len + 1;
> +		name = sep + 1;
> +		sep = memchr(name, ':', fixup_len);
> +		if (!sep || *sep != ':')
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		name_len = sep - name;
> +		if (!name_len)
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		index = strtoul(sep + 1, &endptr, 10);
> +		if ((*endptr != '\0') || (endptr <= (sep + 1)))
> +			return -FDT_ERR_BADFIXUP;

BADOVERLAY

> +		len -= fixup_len + 1;
> +		value += fixup_len + 1;
> +
> +		ret = overlay_fixup_one_phandle(fdt, fdto, symbols_off,
> +						path, path_len, name, name_len,
> +						index, label);
> +		if (ret)
> +			return ret;
> +	} while (len > 0);

I can vaguely imagine it might be useful for some overlay generating
tools to be able to insert empty fixup properties so they don't they
don't need a special case for (# places to fix) == 0.  Since
supporting that (as a no-op) is just changing this do while to a
while, we might as well throw it in.

> +	return 0;
> +}
> +
> +/**
> + * overlay_fixup_phandles - Resolve the overlay phandles to the base
> + *                          device tree
> + * @fdt: Base Device Tree blob
> + * @fdto: Device tree overlay blob
> + *
> + * overlay_fixup_phandles() resolves all the overlay phandles pointing
> + * to nodes in the base device tree.
> + *
> + * This is one of the steps of the device tree overlay application
> + * process, when you want all the phandles in the overlay to point to
> + * the actual base dt nodes.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_fixup_phandles(void *fdt, void *fdto)
> +{
> +	int fixups_off, symbols_off;
> +	int property;
> +
> +	symbols_off = fdt_path_offset(fdt, "/__symbols__");
> +	fixups_off = fdt_path_offset(fdto, "/__fixups__");

Need an error check for both symbols_off and fixups_off here.

> +	fdt_for_each_property_offset(property, fdto, fixups_off) {
> +		int ret;
> +
> +		ret = overlay_fixup_phandle(fdt, fdto, symbols_off, property);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * overlay_apply_node - Merges a node into the base device tree
> + * @fdt: Base Device Tree blob
> + * @target: Node offset in the base device tree to apply the fragment to
> + * @fdto: Device tree overlay blob
> + * @node: Node offset in the overlay holding the changes to merge
> + *
> + * overlay_apply_node() merges a node into a target base device tree
> + * node pointed.
> + *
> + * This is part of the final step in the device tree overlay
> + * application process, when all the phandles have been adjusted and
> + * resolved and you just have to merge overlay into the base device
> + * tree.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_apply_node(void *fdt, int target,
> +			      void *fdto, int node)
> +{
> +	int property;
> +	int subnode;
> +
> +	fdt_for_each_property_offset(property, fdto, node) {
> +		const char *name;
> +		const void *prop;
> +		int prop_len;
> +		int ret;
> +
> +		prop = fdt_getprop_by_offset(fdto, property, &name,
> +					     &prop_len);
> +		if (prop_len == -FDT_ERR_NOTFOUND)
> +			return -FDT_ERR_INTERNAL;
> +		if (prop_len < 0)
> +			return prop_len;
> +
> +		ret = fdt_setprop(fdt, target, name, prop, prop_len);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	fdt_for_each_subnode(subnode, fdto, node) {
> +		const char *name = fdt_get_name(fdto, subnode, NULL);
> +		int nnode;
> +		int ret;
> +
> +		nnode = fdt_add_subnode(fdt, target, name);
> +		if (nnode == -FDT_ERR_EXISTS) {
> +			nnode = fdt_subnode_offset(fdt, target, name);
> +			if (nnode == -FDT_ERR_NOTFOUND)
> +				return -FDT_ERR_INTERNAL;
> +		}
> +
> +		if (nnode < 0)
> +			return nnode;
> +
> +		ret = overlay_apply_node(fdt, nnode, fdto, subnode);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * overlay_merge - Merge an overlay into its base device tree
> + * @fdt: Base Device Tree blob
> + * @fdto: Device tree overlay blob
> + *
> + * overlay_merge() merges an overlay into its base device tree.
> + *
> + * This is the final step in the device tree overlay application
> + * process, when all the phandles have been adjusted and resolved and
> + * you just have to merge overlay into the base device tree.
> + *
> + * returns:
> + *      0 on success
> + *      Negative error code on failure
> + */
> +static int overlay_merge(void *fdt, void *fdto)
> +{
> +	int fragment;
> +
> +	fdt_for_each_subnode(fragment, fdto, 0) {
> +		int overlay;
> +		int target;
> +		int ret;
> +
> +		/*
> +		 * Each fragments will have an __overlay__ node. If
> +		 * they don't, it's not supposed to be merged
> +		 */
> +		overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
> +		if (overlay == -FDT_ERR_NOTFOUND)
> +			continue;
> +
> +		if (overlay < 0)
> +			return overlay;
> +
> +		target = overlay_get_target(fdt, fdto, fragment);
> +		if (target < 0)
> +			return target;
> +
> +		ret = overlay_apply_node(fdt, target, fdto, overlay);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int fdt_overlay_apply(void *fdt, void *fdto)
> +{
> +	uint32_t delta = fdt_get_max_phandle(fdt);
> +	int ret;
> +
> +	FDT_CHECK_HEADER(fdt);
> +	FDT_CHECK_HEADER(fdto);
> +
> +	ret = overlay_adjust_local_phandles(fdto, delta);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_update_local_references(fdto, delta);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_fixup_phandles(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_merge(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	/*
> +	 * The overlay has been damaged, erase its magic.
> +	 */
> +	fdt_set_magic(fdto, ~0);
> +
> +	return 0;
> +
> +err:
> +	/*
> +	 * The overlay might have been damaged, erase its magic.
> +	 */
> +	fdt_set_magic(fdto, ~0);
> +
> +	/*
> +	 * The base device tree might have been damaged, erase its
> +	 * magic.
> +	 */
> +	fdt_set_magic(fdt, ~0);
> +
> +	return ret;
> +}
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index f49b45c29006..9892d06c06bc 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1756,6 +1756,37 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name);
>   */
>  int fdt_del_node(void *fdt, int nodeoffset);
>  
> +/**
> + * fdt_overlay_apply - Applies a DT overlay on a base DT
> + * @fdt: pointer to the base device tree blob
> + * @fdto: pointer to the device tree overlay blob
> + *
> + * fdt_overlay_apply() will apply the given device tree overlay on the
> + * given base device tree.
> + *
> + * Expect the base device tree to be modified, even if the function
> + * returns an error.
> + *
> + * returns:
> + *	0, on success
> + *	-FDT_ERR_NOSPACE, there's not enough space in the base device tree
> + *	-FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
> + *		properties in the base DT
> + *	-FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
> + *		magic
> + *	-FDT_ERR_INTERNAL,
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADOFFSET,
> + *	-FDT_ERR_BADPATH,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_BADFIXUP,
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_overlay_apply(void *fdt, void *fdto);
> +
>  /**********************************************************************/
>  /* Debugging / informational functions                                */
>  /**********************************************************************/
> diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
> index 9dea97dfff81..99f936dacc60 100644
> --- a/libfdt/libfdt_env.h
> +++ b/libfdt/libfdt_env.h
> @@ -54,6 +54,7 @@
>  
>  #include <stddef.h>
>  #include <stdint.h>
> +#include <stdlib.h>
>  #include <string.h>
>  
>  #ifdef __CHECKER__

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

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[David Gibson]

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

On Thu, Sep 22, 2016 at 09:39:22AM +0300, Maxime Ripard wrote:
> Hi David,
> 
> On Mon, Aug 22, 2016 at 09:01:56AM -0400, David Gibson wrote:
> > On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> > > Hi David,
> > > 
> > > On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > > > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > > > Add a function to modify inplace only a portion of a property..
> > > > > 
> > > > > This is especially useful when the property is an array of values, and you
> > > > > want to update one of them without changing the DT size.
> > > > > 
> > > > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > > 
> > > > Applied, though I made a couple of tiny tweaks, see below.
> > > 
> > > Awesome, thanks!
> > > 
> > > Do you still have comments on the last 3 patches?
> > 
> > Sorry, I got sidetracked by other things.
> > 
> > The only remaining thing I'd like some changes in is the error
> > reporting.
> > 
> > The biggset problem is that in some places you are returning
> > BADSTRUCTURE for badly formatter fixup or other overlay properties.
> > BADSTRUCTURE should only be used when there is a problem in the
> > encoding of the tree structure itself, not a problem in the encoding
> > within a specific property or properties.
> > 
> > I don't think your code should ever generate BADSTRUCTURE directly
> > (but it could return it when it gets that value returned from a lower
> > level function).
> > 
> > What I was hoping to have a closer look at was to see if I'd prefer
> > just a single 'BADOVERLAY' error for any problem in te overlay
> > encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
> > like that.  I'll try to get a closer look, but it probably won't
> > happen this week (I'm at KVM Forum).
> 
> Did you have time to think about this?

Finally, yes.  Just sent the detailed reply.

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

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

Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[Maxime Ripard]

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

Hi David,

On Fri, Sep 23, 2016 at 03:31:20PM +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 09:39:22AM +0300, Maxime Ripard wrote:
> > Hi David,
> > 
> > On Mon, Aug 22, 2016 at 09:01:56AM -0400, David Gibson wrote:
> > > On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> > > > Hi David,
> > > > 
> > > > On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > > > > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > > > > Add a function to modify inplace only a portion of a property..
> > > > > > 
> > > > > > This is especially useful when the property is an array of values, and you
> > > > > > want to update one of them without changing the DT size.
> > > > > > 
> > > > > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > > > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > > > 
> > > > > Applied, though I made a couple of tiny tweaks, see below.
> > > > 
> > > > Awesome, thanks!
> > > > 
> > > > Do you still have comments on the last 3 patches?
> > > 
> > > Sorry, I got sidetracked by other things.
> > > 
> > > The only remaining thing I'd like some changes in is the error
> > > reporting.
> > > 
> > > The biggset problem is that in some places you are returning
> > > BADSTRUCTURE for badly formatter fixup or other overlay properties.
> > > BADSTRUCTURE should only be used when there is a problem in the
> > > encoding of the tree structure itself, not a problem in the encoding
> > > within a specific property or properties.
> > > 
> > > I don't think your code should ever generate BADSTRUCTURE directly
> > > (but it could return it when it gets that value returned from a lower
> > > level function).
> > > 
> > > What I was hoping to have a closer look at was to see if I'd prefer
> > > just a single 'BADOVERLAY' error for any problem in te overlay
> > > encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
> > > like that.  I'll try to get a closer look, but it probably won't
> > > happen this week (I'm at KVM Forum).
> > 
> > Did you have time to think about this?
> 
> Finally, yes.  Just sent the detailed reply.

I'll have a look, thanks!

Expect a new version some time next week.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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