All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/4] libfdt: Add support for device tree overlays
@ 2016-09-30 13:57 Maxime Ripard
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2016-09-30 13:57 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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 v6:
  - Removed FDT_ERR_BADFIXUP
  - Changed a few errors returned according to David suggestions

Changes from v5:
  - Added the extra error codes suggested by David
  - Changed slightly the meaning of BADPHANDLE
  - Added an extra error check to deal with overlay without fixups
  - Fixed a test failure that went unnoticed so far (Thanks Tim!)
  - Rebased on top of 1.4.2

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 (4):
  libfdt: Add new errors for the overlay code
  libfdt: Extend the reach of FDT_ERR_BADPHANDLE
  libfdt: Add overlay application function
  tests: Add tests cases for the overlay code

 libfdt/Makefile.libfdt                      |   2 +-
 libfdt/fdt_overlay.c                        | 670 ++++++++++++++++++++++++++++
 libfdt/fdt_strerror.c                       |   3 +
 libfdt/libfdt.h                             |  48 +-
 libfdt/libfdt_env.h                         |   1 +
 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 ++
 22 files changed, 1376 insertions(+), 5 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

-- 
2.9.3

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

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

* [PATCH v7 1/4] libfdt: Add new errors for the overlay code
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-09-30 13:57   ` Maxime Ripard
  2016-09-30 13:57   ` [PATCH v7 2/4] libfdt: Extend the reach of FDT_ERR_BADPHANDLE Maxime Ripard
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2016-09-30 13:57 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Maxime Ripard

Add a few new error codes to report the failure conditions we might
encounter in the overlay application code:
   - FDT_ERR_BADOVERLAY, when an overlay cannot be parsed, even though its
     structure is correct
   - FDT_ERR_NOPHANDLES, when we ran out of available phandles and we
     cannot use a new phandle without either using an invalid one (-1 or
     0), or one already used.

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

diff --git a/libfdt/fdt_strerror.c b/libfdt/fdt_strerror.c
index e6c3ceee8c58..3cb357f60454 100644
--- a/libfdt/fdt_strerror.c
+++ b/libfdt/fdt_strerror.c
@@ -76,6 +76,9 @@ 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_BADOVERLAY),
+	FDT_ERRTABENT(FDT_ERR_BADOVERLAY),
+	FDT_ERRTABENT(FDT_ERR_NOPHANDLES),
 };
 #define FDT_ERRTABSIZE	(sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))
 
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index ecb11fc9e985..361cc6c7198c 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -126,7 +126,16 @@
 	 * 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_BADOVERLAY	16
+	/* FDT_ERR_BADOVERLAY: The device tree overlay, while
+	 * correctly structured, cannot be applied due to some
+	 * unexpected or missing value, property or node. */
+
+#define FDT_ERR_NOPHANDLES	17
+	/* FDT_ERR_NOPHANDLES: The device tree doesn't have any
+	 * phandle available anymore without causing an overflow */
+
+#define FDT_ERR_MAX		17
 
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
-- 
2.9.3

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

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

* [PATCH v7 2/4] libfdt: Extend the reach of FDT_ERR_BADPHANDLE
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-09-30 13:57   ` [PATCH v7 1/4] libfdt: Add new errors for the overlay code Maxime Ripard
@ 2016-09-30 13:57   ` Maxime Ripard
  2016-09-30 13:57   ` [PATCH v7 3/4] libfdt: Add overlay application function Maxime Ripard
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2016-09-30 13:57 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Maxime Ripard

So far, the BADPHANDLE error was only used for incorrect phandle values.
Extend that meaning to an improperly formatted phandle property.

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

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 361cc6c7198c..d2e5e031fb35 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -79,8 +79,10 @@
 	 * (e.g. missing a leading / for a function which requires an
 	 * absolute path) */
 #define FDT_ERR_BADPHANDLE	6
-	/* FDT_ERR_BADPHANDLE: Function was passed an invalid phandle
-	 * value.  phandle values of 0 and -1 are not permitted. */
+	/* FDT_ERR_BADPHANDLE: Function was passed an invalid phandle.
+	 * This can be caused either by an invalid phandle property
+	 * length, or the phandle value was either 0 or -1, which are
+	 * not permitted. */
 #define FDT_ERR_BADSTATE	7
 	/* FDT_ERR_BADSTATE: Function was passed an incomplete device
 	 * tree created by the sequential-write functions, which is
-- 
2.9.3

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

* [PATCH v7 3/4] libfdt: Add overlay application function
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-09-30 13:57   ` [PATCH v7 1/4] libfdt: Add new errors for the overlay code Maxime Ripard
  2016-09-30 13:57   ` [PATCH v7 2/4] libfdt: Extend the reach of FDT_ERR_BADPHANDLE Maxime Ripard
@ 2016-09-30 13:57   ` Maxime Ripard
       [not found]     ` <20160930135717.16511-4-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-09-30 13:57   ` [PATCH v7 4/4] tests: Add tests cases for the overlay code Maxime Ripard
  2016-10-06  9:47   ` [PATCH v7 0/4] libfdt: Add support for device tree overlays David Gibson
  4 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2016-09-30 13:57 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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   | 670 +++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h        |  31 +++
 libfdt/libfdt_env.h    |   1 +
 4 files changed, 703 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..c7e5508abf30
--- /dev/null
+++ b/libfdt/fdt_overlay.c
@@ -0,0 +1,670 @@
+#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;
+	int path_len;
+
+	/* 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", &path_len);
+	if (!path) {
+		/*
+		 * If we haven't found either a target or a
+		 * target-path property in a node that contains a
+		 * __overlay__ subnode (we wouldn't be called
+		 * otherwise), consider it a improperly written
+		 * overlay
+		 */
+		if (path_len == -FDT_ERR_NOTFOUND)
+			return -FDT_ERR_BADOVERLAY;
+
+		return path_len;
+	}
+
+	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_BADPHANDLE;
+
+	adj_val = fdt32_to_cpu(*val);
+	if ((adj_val + delta) < adj_val)
+		return -FDT_ERR_NOPHANDLES;
+
+	adj_val += delta;
+	if (adj_val == (uint32_t)-1)
+		return -FDT_ERR_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;
+	}
+
+	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_BADOVERLAY;
+
+		tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
+		if (!tree_val) {
+			if (tree_len == -FDT_ERR_NOTFOUND)
+				return -FDT_ERR_BADOVERLAY;
+
+			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 == -FDT_ERR_NOSPACE)
+				return -FDT_ERR_BADOVERLAY;
+
+			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 == -FDT_ERR_NOTFOUND)
+		return -FDT_ERR_BADOVERLAY;
+	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_BADOVERLAY;
+		fixup_len = fixup_end - fixup_str;
+
+		len -= fixup_len + 1;
+		value += fixup_len + 1;
+
+		path = fixup_str;
+		sep = memchr(fixup_str, ':', fixup_len);
+		if (!sep || *sep != ':')
+			return -FDT_ERR_BADOVERLAY;
+
+		path_len = sep - path;
+		if (path_len == (fixup_len - 1))
+			return -FDT_ERR_BADOVERLAY;
+
+		fixup_len -= path_len + 1;
+		name = sep + 1;
+		sep = memchr(name, ':', fixup_len);
+		if (!sep || *sep != ':')
+			return -FDT_ERR_BADOVERLAY;
+
+		name_len = sep - name;
+		if (!name_len)
+			return -FDT_ERR_BADOVERLAY;
+
+		index = strtoul(sep + 1, &endptr, 10);
+		if ((*endptr != '\0') || (endptr <= (sep + 1)))
+			return -FDT_ERR_BADOVERLAY;
+
+		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;
+
+	/* We can have overlays without any fixups */
+	fixups_off = fdt_path_offset(fdto, "/__fixups__");
+	if (fixups_off == -FDT_ERR_NOTFOUND)
+		return 0;
+	if (fixups_off < 0)
+		return fixups_off;
+
+	symbols_off = fdt_path_offset(fdt, "/__symbols__");
+	if (symbols_off < 0)
+		return symbols_off;
+
+	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 d2e5e031fb35..c69e9188996f 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1763,6 +1763,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,
+ *	-FDT_ERR_BADOVERLAY,
+ *	-FDT_ERR_NOPHANDLES,
+ *	-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_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.3

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

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

* [PATCH v7 4/4] tests: Add tests cases for the overlay code
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
                     ` (2 preceding siblings ...)
  2016-09-30 13:57   ` [PATCH v7 3/4] libfdt: Add overlay application function Maxime Ripard
@ 2016-09-30 13:57   ` Maxime Ripard
       [not found]     ` <20160930135717.16511-5-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
  2016-10-06  9:47   ` [PATCH v7 0/4] libfdt: Add support for device tree overlays David Gibson
  4 siblings, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2016-09-30 13:57 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA,
	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..5014f5ec0868
--- /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_BADOVERLAY);
+
+	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 f4b32e4296d0..dc19f80b2fb6 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -160,6 +160,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
 
@@ -273,6 +304,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.3

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

* Re: [PATCH v7 3/4] libfdt: Add overlay application function
       [not found]     ` <20160930135717.16511-4-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-10-06  6:46       ` David Gibson
  0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2016-10-06  6:46 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Fri, Sep 30, 2016 at 03:57:16PM +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>

One tiny nit to think about fixing in a follow up patch, otherwise
looks good.

> ---
>  libfdt/Makefile.libfdt |   2 +-
>  libfdt/fdt_overlay.c   | 670 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libfdt/libfdt.h        |  31 +++
>  libfdt/libfdt_env.h    |   1 +
>  4 files changed, 703 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..c7e5508abf30
> --- /dev/null
> +++ b/libfdt/fdt_overlay.c
> @@ -0,0 +1,670 @@
> +#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;
> +	int path_len;
> +
> +	/* 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", &path_len);
> +	if (!path) {
> +		/*
> +		 * If we haven't found either a target or a
> +		 * target-path property in a node that contains a
> +		 * __overlay__ subnode (we wouldn't be called
> +		 * otherwise), consider it a improperly written
> +		 * overlay
> +		 */
> +		if (path_len == -FDT_ERR_NOTFOUND)
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		return path_len;
> +	}
> +
> +	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_BADPHANDLE;
> +
> +	adj_val = fdt32_to_cpu(*val);
> +	if ((adj_val + delta) < adj_val)
> +		return -FDT_ERR_NOPHANDLES;
> +
> +	adj_val += delta;
> +	if (adj_val == (uint32_t)-1)
> +		return -FDT_ERR_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;
> +	}
> +
> +	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_BADOVERLAY;
> +
> +		tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
> +		if (!tree_val) {
> +			if (tree_len == -FDT_ERR_NOTFOUND)
> +				return -FDT_ERR_BADOVERLAY;
> +
> +			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 == -FDT_ERR_NOSPACE)
> +				return -FDT_ERR_BADOVERLAY;
> +
> +			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;

Technically a NOTFOUND here should also be translated into a
BADOVERLAY, but I'm not going to hold the patch up just for that.

> +
> +		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 == -FDT_ERR_NOTFOUND)
> +		return -FDT_ERR_BADOVERLAY;
> +	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_BADOVERLAY;
> +		fixup_len = fixup_end - fixup_str;
> +
> +		len -= fixup_len + 1;
> +		value += fixup_len + 1;
> +
> +		path = fixup_str;
> +		sep = memchr(fixup_str, ':', fixup_len);
> +		if (!sep || *sep != ':')
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		path_len = sep - path;
> +		if (path_len == (fixup_len - 1))
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		fixup_len -= path_len + 1;
> +		name = sep + 1;
> +		sep = memchr(name, ':', fixup_len);
> +		if (!sep || *sep != ':')
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		name_len = sep - name;
> +		if (!name_len)
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		index = strtoul(sep + 1, &endptr, 10);
> +		if ((*endptr != '\0') || (endptr <= (sep + 1)))
> +			return -FDT_ERR_BADOVERLAY;
> +
> +		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;
> +
> +	/* We can have overlays without any fixups */
> +	fixups_off = fdt_path_offset(fdto, "/__fixups__");
> +	if (fixups_off == -FDT_ERR_NOTFOUND)
> +		return 0;
> +	if (fixups_off < 0)
> +		return fixups_off;
> +
> +	symbols_off = fdt_path_offset(fdt, "/__symbols__");
> +	if (symbols_off < 0)
> +		return symbols_off;
> +
> +	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 d2e5e031fb35..c69e9188996f 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1763,6 +1763,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,
> + *	-FDT_ERR_BADOVERLAY,
> + *	-FDT_ERR_NOPHANDLES,
> + *	-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_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 --]

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

* Re: [PATCH v7 4/4] tests: Add tests cases for the overlay code
       [not found]     ` <20160930135717.16511-5-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2016-10-06  8:33       ` David Gibson
       [not found]         ` <20161006083348.GN18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2016-10-06  8:33 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Fri, Sep 30, 2016 at 03:57:17PM +0200, Maxime Ripard wrote:
> 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>

Hrm.  All the overlay_bad_fixup testcases fail for me.

> ---
>  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..5014f5ec0868
> --- /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_BADOVERLAY);
> +
> +	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 f4b32e4296d0..dc19f80b2fb6 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -160,6 +160,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
>  
> @@ -273,6 +304,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

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

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

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

* Re: [PATCH v7 4/4] tests: Add tests cases for the overlay code
       [not found]         ` <20161006083348.GN18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2016-10-06  9:28           ` David Gibson
  0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2016-10-06  9:28 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Thu, Oct 06, 2016 at 07:33:48PM +1100, David Gibson wrote:
> On Fri, Sep 30, 2016 at 03:57:17PM +0200, Maxime Ripard wrote:
> > 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>
> 
> Hrm.  All the overlay_bad_fixup testcases fail for me.

Ah, that's because they should be under the the conditional for
overlay-aware dtc.  I've adjusted that and applied the series to
master.

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

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

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
       [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
                     ` (3 preceding siblings ...)
  2016-09-30 13:57   ` [PATCH v7 4/4] tests: Add tests cases for the overlay code Maxime Ripard
@ 2016-10-06  9:47   ` David Gibson
       [not found]     ` <20161006094756.GR18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  4 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2016-10-06  9:47 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
> 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

Looking pretty good.  There's a minor error in 3/4 which I've pointed
out, it can be fixed in a followup.

An error in the conditionals means it's trying to run some of the
tests which require an overlay-aware dtc on even when that's not
available - I adjusted 4/4 to correct that.

It also hit build failures on Travis, because the compiler version
there complains about local variables named 'index' shadowing the
standard library function.  I've corrected that with a followup patch.

With that done, the whole lot has been committed to master.

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

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

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
       [not found]     ` <20161006094756.GR18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2016-10-06 11:24       ` Maxime Ripard
  2016-10-06 15:11         ` Phil Elwell
  2016-10-06 21:16       ` Phil Elwell
  1 sibling, 1 reply; 14+ messages in thread
From: Maxime Ripard @ 2016-10-06 11:24 UTC (permalink / raw)
  To: David Gibson
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

Hi David,

On Thu, Oct 06, 2016 at 08:47:56PM +1100, David Gibson wrote:
> On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
> > 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
> 
> Looking pretty good.  There's a minor error in 3/4 which I've pointed
> out, it can be fixed in a followup.
> 
> An error in the conditionals means it's trying to run some of the
> tests which require an overlay-aware dtc on even when that's not
> available - I adjusted 4/4 to correct that.

While chasing that, I found that it was actually a bug in a condition
that was stricter than it should, I'll send follow-up with that and
the fix you asked.

> It also hit build failures on Travis, because the compiler version
> there complains about local variables named 'index' shadowing the
> standard library function.  I've corrected that with a followup patch.
> 
> With that done, the whole lot has been committed to master.

Thanks a lot,
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 --]

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
  2016-10-06 11:24       ` Maxime Ripard
@ 2016-10-06 15:11         ` Phil Elwell
  0 siblings, 0 replies; 14+ messages in thread
From: Phil Elwell @ 2016-10-06 15:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: David Gibson, Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, Oct 6, 2016 at 12:24 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi David,
>
> On Thu, Oct 06, 2016 at 08:47:56PM +1100, David Gibson wrote:
>> On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
>> > 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
>>
>> Looking pretty good.  There's a minor error in 3/4 which I've pointed
>> out, it can be fixed in a followup.
>>
>> An error in the conditionals means it's trying to run some of the
>> tests which require an overlay-aware dtc on even when that's not
>> available - I adjusted 4/4 to correct that.
>
> While chasing that, I found that it was actually a bug in a condition
> that was stricter than it should, I'll send follow-up with that and
> the fix you asked.
>
>> It also hit build failures on Travis, because the compiler version
>> there complains about local variables named 'index' shadowing the
>> standard library function.  I've corrected that with a followup patch.
>>
>> With that done, the whole lot has been committed to master.
>
> Thanks a lot,
> Maxime

Now that upstream libfdt can resolve and apply overlays we need a way
of generating them. I have a set of three patches from Pantelis that
add "plugin" support to dtc, together with two downstream Raspberry Pi
patches that fix an uninitialised memory reference and restrict
__local_fixups__  generation to overlays.

Is there already a plan to get these upstreamed? If so, can we get the
downstream fixes included?

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

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
       [not found]     ` <20161006094756.GR18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
  2016-10-06 11:24       ` Maxime Ripard
@ 2016-10-06 21:16       ` Phil Elwell
       [not found]         ` <6338fcb9-ffda-a04b-6c6e-2fd254b58dda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 14+ messages in thread
From: Phil Elwell @ 2016-10-06 21:16 UTC (permalink / raw)
  To: David Gibson, Maxime Ripard
  Cc: Pantelis Antoniou, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

On 06/10/2016 10:47, David Gibson wrote:
> On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
>> 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
> Looking pretty good.  There's a minor error in 3/4 which I've pointed
> out, it can be fixed in a followup.
>
> An error in the conditionals means it's trying to run some of the
> tests which require an overlay-aware dtc on even when that's not
> available - I adjusted 4/4 to correct that.
>
> It also hit build failures on Travis, because the compiler version
> there complains about local variables named 'index' shadowing the
> standard library function.  I've corrected that with a followup patch.
>
> With that done, the whole lot has been committed to master.
>
Now that upstream libfdt can resolve and apply overlays we need a way
of generating them. I have a set of three patches from Pantelis that
add "plugin" support to dtc, together with two downstream (Raspberry Pi)
patches that fix an uninitialised memory reference and restrict
__local_fixups__  generation to overlays.

Is there already a plan to get the enhanced dtc upstreamed? If so, can I
get the downstream fixes included?

Phil Elwell

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

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
       [not found]         ` <6338fcb9-ffda-a04b-6c6e-2fd254b58dda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2016-10-12 15:06           ` Pantelis Antoniou
       [not found]             ` <965C6014-692F-4021-9E3D-945ED32307B7-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Pantelis Antoniou @ 2016-10-12 15:06 UTC (permalink / raw)
  To: Phil Elwell
  Cc: David Gibson, Maxime Ripard, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Phil,

> On Oct 6, 2016, at 23:16 , Phil Elwell <philip.j.elwell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> On 06/10/2016 10:47, David Gibson wrote:
>> On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
>>> 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
>> Looking pretty good.  There's a minor error in 3/4 which I've pointed
>> out, it can be fixed in a followup.
>> 
>> An error in the conditionals means it's trying to run some of the
>> tests which require an overlay-aware dtc on even when that's not
>> available - I adjusted 4/4 to correct that.
>> 
>> It also hit build failures on Travis, because the compiler version
>> there complains about local variables named 'index' shadowing the
>> standard library function.  I've corrected that with a followup patch.
>> 
>> With that done, the whole lot has been committed to master.
>> 
> Now that upstream libfdt can resolve and apply overlays we need a way
> of generating them. I have a set of three patches from Pantelis that
> add "plugin" support to dtc, together with two downstream (Raspberry Pi)
> patches that fix an uninitialised memory reference and restrict
> __local_fixups__  generation to overlays.
> 
> Is there already a plan to get the enhanced dtc upstreamed? If so, can I
> get the downstream fixes included?
> 

David, my patchset is kept updated against mainline dtc.
What are the remaining stumbling blocks in getting in accepted?

> Phil Elwell

Regards

— Pantelis

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

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

* Re: [PATCH v7 0/4] libfdt: Add support for device tree overlays
       [not found]             ` <965C6014-692F-4021-9E3D-945ED32307B7-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
@ 2016-10-12 21:41               ` David Gibson
  0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2016-10-12 21:41 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Phil Elwell, Maxime Ripard, Simon Glass, Boris Brezillon,
	devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
	Antoine Ténart, devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Wed, Oct 12, 2016 at 05:06:07PM +0200, Pantelis Antoniou wrote:
> Hi Phil,
> 
> > On Oct 6, 2016, at 23:16 , Phil Elwell <philip.j.elwell-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > On 06/10/2016 10:47, David Gibson wrote:
> >> On Fri, Sep 30, 2016 at 03:57:13PM +0200, Maxime Ripard wrote:
> >>> 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
> >> Looking pretty good.  There's a minor error in 3/4 which I've pointed
> >> out, it can be fixed in a followup.
> >> 
> >> An error in the conditionals means it's trying to run some of the
> >> tests which require an overlay-aware dtc on even when that's not
> >> available - I adjusted 4/4 to correct that.
> >> 
> >> It also hit build failures on Travis, because the compiler version
> >> there complains about local variables named 'index' shadowing the
> >> standard library function.  I've corrected that with a followup patch.
> >> 
> >> With that done, the whole lot has been committed to master.
> >> 
> > Now that upstream libfdt can resolve and apply overlays we need a way
> > of generating them. I have a set of three patches from Pantelis that
> > add "plugin" support to dtc, together with two downstream (Raspberry Pi)
> > patches that fix an uninitialised memory reference and restrict
> > __local_fixups__  generation to overlays.
> > 
> > Is there already a plan to get the enhanced dtc upstreamed? If so, can I
> > get the downstream fixes included?
> > 
> 
> David, my patchset is kept updated against mainline dtc.
> What are the remaining stumbling blocks in getting in accepted?

Well, repost it and we'll see.

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

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

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

end of thread, other threads:[~2016-10-12 21:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 13:57 [PATCH v7 0/4] libfdt: Add support for device tree overlays Maxime Ripard
     [not found] ` <20160930135717.16511-1-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-09-30 13:57   ` [PATCH v7 1/4] libfdt: Add new errors for the overlay code Maxime Ripard
2016-09-30 13:57   ` [PATCH v7 2/4] libfdt: Extend the reach of FDT_ERR_BADPHANDLE Maxime Ripard
2016-09-30 13:57   ` [PATCH v7 3/4] libfdt: Add overlay application function Maxime Ripard
     [not found]     ` <20160930135717.16511-4-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-10-06  6:46       ` David Gibson
2016-09-30 13:57   ` [PATCH v7 4/4] tests: Add tests cases for the overlay code Maxime Ripard
     [not found]     ` <20160930135717.16511-5-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2016-10-06  8:33       ` David Gibson
     [not found]         ` <20161006083348.GN18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2016-10-06  9:28           ` David Gibson
2016-10-06  9:47   ` [PATCH v7 0/4] libfdt: Add support for device tree overlays David Gibson
     [not found]     ` <20161006094756.GR18733-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2016-10-06 11:24       ` Maxime Ripard
2016-10-06 15:11         ` Phil Elwell
2016-10-06 21:16       ` Phil Elwell
     [not found]         ` <6338fcb9-ffda-a04b-6c6e-2fd254b58dda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-10-12 15:06           ` Pantelis Antoniou
     [not found]             ` <965C6014-692F-4021-9E3D-945ED32307B7-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-10-12 21:41               ` David Gibson

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