[PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Wasim Nazir]

Hello,

This is follow-up attempt for fdtoverlaymerge tool.

Currently all the device-tree (DT) code for a given soc is maintained in a
common kernel repository. For example, this common DT code will have code for
audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
split into a base (soc-common) code and board specific code, with the soc code
being compiled as soc.dtb and board specific code being compiled as respective
overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
of a given SOC while boardX.dtbo represents configuration of a board/platform
designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
target (besides improving the overall size of DT blobs flashed on target, Android
Treble also requires separation of soc and board DT bits). Bootloader will pick
one of the board overlay blobs and merge it with soc.dtb, before booting kernel
which is presented a unified DT blob (soc + board overlay).

For ease of code maintenance and better control over release management, we are
exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
maintain their kernel code (including their DT code) outside a common kernel
repository. In our experience, this simplifies number of branches maintained in
core kernel repo. New/experimental features in fingerprint sensor driver for
example that needs to be on a separate branch will not result in unnecessary
branching in core kenrel repo, affecting all other drivers.

In addition to compiling DT code outside core kernel tree, we also want to merge
the blobs back to respective blobs found in kernel build tree at buildtime
(soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
overlay impacts boot-time.

This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
boardX.dtbo), which currently doesn't seem to be supported and which this patch
series aims to support.

fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
with a base blob. It assumes that all external symbols specified in overlay
blob's __fixups__ section are found in base blob's __symbols__ section and
aborts on the first instance where a symbol could not be found in base blob.
This is mostly fine as the primary use of overlay is on a target for its
bootloader to merge various overlay blobs based on h/w configuration detected.
But when the number of overlays increased then bootloader takes lot of time to
apply the overlays on base DT.

So we need new API/tool to merge all the overlays into single overlay file
at host (build machine) side, so that on target side bootloader needs to only
apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
number file reading/loading & minimizing repeatative overlay apply.
In our test setup we see an improvement of ~60% while applying merged-overlay
at bootloader and the merged-overlay is product of 7 overlays.

To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
which takes input as overlays and gives output to merged-overlay.
The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.

Additional notes:
  If snprintf (in libc) may not available in some environments, then we will need
  to write our own snprintf() in libfdt.

---
Changelog:

v3:
- Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
  - Case1: When target is available and we merge fragments
  - Case2: When target is not available and we add new fragments
- Change the logic to update fixups & local_fixups in case of overlay merge.
- Few patches are squashed, reduced to 4 patches.
- v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/


Srivatsa Vaddagiri (4):
  libfdt: overlay_merge: Introduce fdt_overlay_merge()
  libfdt: overlay_merge: Rename & copy overlay fragments and their
    properties
  libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
  fdtoverlaymerge: A tool that merges overlays

 .gitignore           |   1 +
 Makefile             |   4 +
 Makefile.utils       |   6 +
 fdtoverlaymerge.c    | 223 +++++++++++
 libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
 libfdt/fdt_rw.c      |  14 +-
 libfdt/libfdt.h      |  18 +
 libfdt/version.lds   |   1 +
 meson.build          |   2 +-
 9 files changed, 1146 insertions(+), 24 deletions(-)
 create mode 100644 fdtoverlaymerge.c


base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
--
2.49.0

[PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[Wasim Nazir]

From: Srivatsa Vaddagiri <vatsa@codeaurora.org>

fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
used offline on a build machine to combine two or more overlay blobs into one.

It is intended to help maintain device-tree overlay code in
multiple source repositories, but merge their binary forms (overlay blobs)
into one so that bootloader's task of searching for all relevant overlay blobs
is simplified.

Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
Subsequent patches will introduce required changes to merge overlay blobs.

Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
---
 libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h      | 18 ++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index e6b9eb643958..8690ed55c8f6 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -1098,3 +1098,62 @@ err:

 	return ret;
 }
+
+int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
+{
+	uint32_t delta = fdt_get_max_phandle(fdt);
+	int ret;
+
+	FDT_RO_PROBE(fdt);
+	FDT_RO_PROBE(fdto);
+
+	*fdto_nospace = 0;
+
+	ret = overlay_adjust_local_phandles(fdto, delta);
+	if (ret) {
+		if (ret == -FDT_ERR_NOSPACE)
+			*fdto_nospace = 1;
+		goto err;
+	}
+
+	ret = overlay_update_local_references(fdto, delta);
+	if (ret) {
+		if (ret == -FDT_ERR_NOSPACE)
+			*fdto_nospace = 1;
+		goto err;
+	}
+
+	ret = overlay_fixup_phandles(fdt, fdto);
+	if (ret)
+		goto err;
+
+	ret = overlay_merge(fdt, fdto);
+	if (ret)
+		goto err;
+
+	ret = overlay_symbol_update(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.
+	 */
+	if (!*fdto_nospace)
+		fdt_set_magic(fdt, ~0);
+
+	return ret;
+}
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index b5e72001d115..664b141d5334 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
 int fdt_overlay_target_offset(const void *fdt, const void *fdto,
 			      int fragment_offset, char const **pathp);

+/**
+ * fdt_overlay_merge - Merge two overlays into one
+ * @fdt: pointer to the first device tree overlay blob
+ * @fdto: pointer to the second device tree overlay blob
+ * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
+ *
+ * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
+ *
+ * Expect the first 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 first device tree blob
+ *	-FDT_ERR_BADVALUE
+ */
+int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
+
 /**********************************************************************/
 /* Debugging / informational functions                                */
 /**********************************************************************/
--
2.49.0

[PATCH v3 2/4] libfdt: overlay_merge: Rename & copy overlay fragments and their properties

[Wasim Nazir]

From: Srivatsa Vaddagiri <vatsa@codeaurora.org>

When merging two overlay blobs, fragment whose target node can't be found
in base blob would need to be retained as-is (including the fragment names)
in the combined blob. Such unresolved symbols will also need to be listed
in __fixups__ section of combined blob. This could lead to name comflicts
in combined blob (two nodes with same name/path such as /fragment@0).

To avoid such name conflicts in combined blob, rename all fragment@xyz in
overlay blob as fragment@xyz+delta, where delta is the maximum count of
fragment nodes found in base blob.

Another case is where fragment has target node in base blob, then it needs
to be overlaid within the target node.

Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
---
 libfdt/fdt_overlay.c | 270 +++++++++++++++++++++++++++++++++++++++++--
 libfdt/fdt_rw.c      |  14 ++-
 2 files changed, 270 insertions(+), 14 deletions(-)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 8690ed55c8f6..03c88b05be72 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -8,9 +8,12 @@

 #include <fdt.h>
 #include <libfdt.h>
+#include <stdio.h>

 #include "libfdt_internal.h"

+#define MAX_BUF_SIZE	256
+
 /**
  * overlay_get_target_phandle - retrieves the target phandle of a fragment
  * @fdto: pointer to the device tree overlay blob
@@ -811,10 +814,229 @@ static int overlay_apply_node(void *fdt, int target,
 	return 0;
 }

+/**
+ * copy_node - copy a node hierarchically
+ * @fdt - pointer to base device tree
+ * @fdto - pointer to overlay device tree
+ * @fdto_child - offset of node in overlay device tree which needs to be copied
+ * @fdt_parent - offset of parent node in base tree under which @fdto_child
+ *		need to be copied
+ * @name - if not NULL, (new) name of the child in base device tree
+ * @skip_fdto_child - if set, skips creation of @fdto_child under @fdt_parent.
+ *      Instead copies everything under @fdto_child to @fdt_parent.
+ *
+ * This function copies a node in overlay tree along with its child-nodes and
+ * their properties, under a given parent node in base tree.
+ */
+static int copy_node(void *fdt, void *fdto, int fdt_parent,
+		     int fdto_child, const char *name, int skip_fdto_child)
+{
+	int len, prop, parent, child;
+
+	if (!skip_fdto_child) {
+		if (!name) {
+			name = fdt_get_name(fdto, fdto_child, &len);
+			if (!name)
+				return len;
+		}
+
+		parent = fdt_subnode_offset(fdt, fdt_parent, name);
+		if (parent < 0) {
+			parent = fdt_add_subnode(fdt, fdt_parent, name);
+		}
+
+		if (parent < 0)
+			return parent;
+	} else {
+		parent = fdt_parent;
+	}
+
+	fdt_for_each_property_offset(prop, fdto, fdto_child) {
+		int ret;
+		const char *value, *pname;
+		void *p;
+
+		value = fdt_getprop_by_offset(fdto, prop, &pname, &len);
+		if (!value)
+			return len;
+
+		ret = fdt_setprop_placeholder(fdt, parent, pname, len, &p);
+		if (ret)
+			return ret;
+
+		memcpy(p, value, len);
+	}
+
+	fdt_for_each_subnode(child, fdto, fdto_child) {
+		int ret;
+
+		ret = copy_node(fdt, fdto, parent, child, NULL, 0);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int get_fragment_name(void *fdto, int fragment, char *name, int namelen)
+{
+	int len;
+	const char *nname;
+	int size = sizeof("fragment@") - 1;
+
+	nname = fdt_get_name(fdto, fragment, &len);
+	if (!nname)
+		return len;
+
+	if (len < size || len >= namelen || memcmp(nname, "fragment@", size))
+		return -FDT_ERR_BADVALUE;
+
+	memcpy(name, nname, len);
+	name[len] = 0;
+
+	return 0;
+}
+
+static int get_fragment_index(char *name, unsigned long *idxp)
+{
+	char *idx;
+	int size = sizeof("fragment@") - 1;
+	int len = strlen(name);
+	char *stop;
+	unsigned long index;
+
+	if (len < size)
+		return -FDT_ERR_BADVALUE;
+
+	idx = name + size;
+	index = strtoul(idx, &stop, 10);
+	if (*stop != '\0' || stop <= idx)
+		return -FDT_ERR_BADVALUE;
+
+	*idxp = index;
+
+	return 0;
+}
+
+static int set_new_fragment_name(char *name, int namelen,
+				 unsigned long base_fragment_count)
+{
+	unsigned long index;
+	int ret;
+
+	ret = get_fragment_index(name, &index);
+	if (ret)
+		return ret;
+
+	if (ULONG_MAX - base_fragment_count < index)
+		return -FDT_ERR_INTERNAL;
+
+	index += base_fragment_count;
+
+	ret = snprintf(name, namelen, "fragment@%lu", index);
+
+	return ret >= namelen ? -FDT_ERR_INTERNAL : 0;
+}
+
+static int add_phandle(void *fdt, char *node_name, uint32_t phandle)
+{
+	int offset;
+
+	offset = fdt_subnode_offset(fdt, 0, node_name);
+	if (offset < 0)
+		return offset;
+
+	return fdt_setprop_u32(fdt, offset, "phandle", phandle);
+}
+
+static int copy_fragment_to_base(void *fdt, void *fdto,
+				 int fragment, uint32_t *merge_olay_max_phdl,
+				 unsigned long *base_fragment_count)
+{
+	char name[MAX_BUF_SIZE];
+	int ret;
+	uint32_t target_phandle = *merge_olay_max_phdl;
+
+	if (merge_olay_max_phdl == NULL)
+		return -FDT_ERR_BADPHANDLE;
+
+	ret = get_fragment_name(fdto, fragment, name, sizeof(name));
+	if (ret)
+		return ret;
+
+	ret = set_new_fragment_name(name, sizeof(name), *base_fragment_count);
+	if (ret)
+		return ret;
+
+	ret = copy_node(fdt, fdto, 0, fragment, name, 0);
+	if (ret)
+		return ret;
+
+	ret = add_phandle(fdt, name, target_phandle);
+	if (ret)
+		return ret;
+
+	/* Fix target to point to new node in base */
+	ret = fdt_setprop_inplace_u32(fdto, fragment, "target", target_phandle);
+	if (ret)
+		return ret;
+
+	return (++(*merge_olay_max_phdl) == UINT32_MAX ||
+		++(*base_fragment_count) == ULONG_MAX) ?
+	    -FDT_ERR_BADOVERLAY : 0;
+}
+
+/* Return maximum count of overlay fragments */
+static int count_fragments(void *fdt, unsigned long *max_base_fragments)
+{
+	int offset = -1, child_offset, child_len, len, found = 0;
+	const char *name, *child_name, *idx;
+	char *stop;
+	unsigned long index, max = 0;
+
+	offset = fdt_first_subnode(fdt, 0);
+	while (offset >= 0) {
+		name = fdt_get_name(fdt, offset, &len);
+		if (!name)
+			return len;
+
+		if (len < 9 || memcmp(name, "fragment@", 9))
+			goto next_node;
+
+		child_offset = fdt_first_subnode(fdt, offset);
+		if (child_offset < 0)
+			return child_offset;
+
+		child_name = fdt_get_name(fdt, child_offset, &child_len);
+		if (!child_name)
+			return child_len;
+
+		if (child_len < 11 || memcmp(child_name, "__overlay__", 11))
+			goto next_node;
+
+		found = 1;
+		idx = name + 9;
+		index = strtoul(idx, &stop, 10);
+		if (index > max)
+			max = index;
+next_node:
+		offset = fdt_next_subnode(fdt, offset);
+	}
+
+	if (!found)
+		return -FDT_ERR_NOTFOUND;
+
+	*max_base_fragments = max;
+
+	return 0;
+}
+
 /**
  * overlay_merge - Merge an overlay into its base device tree
  * @fdt: Base Device Tree blob
  * @fdto: Device tree overlay blob
+ * @merge_olay_max_phdl: Pointer to max phandle value for merged blobs,
+ *     Both input blobs are overlay blobs that are being merged
  *
  * overlay_merge() merges an overlay into its base device tree.
  *
@@ -826,14 +1048,23 @@ static int overlay_apply_node(void *fdt, int target,
  *      0 on success
  *      Negative error code on failure
  */
-static int overlay_merge(void *fdt, void *fdto)
+static int overlay_merge(void *fdt, void *fdto, uint32_t *merge_olay_max_phdl)
 {
-	int fragment;
+	int fragment, ret;
+	unsigned long base_fragment_count = 0;
+
+	if (merge_olay_max_phdl) {
+		ret = count_fragments(fdt, &base_fragment_count);
+		/* no fragments in base dtb? then nothing to rename */
+		if (ret && ret != -FDT_ERR_NOTFOUND)
+			return ret;
+
+		base_fragment_count++;
+	}

 	fdt_for_each_subnode(fragment, fdto, 0) {
 		int overlay;
 		int target;
-		int ret;

 		/*
 		 * Each fragments will have an __overlay__ node. If
@@ -847,8 +1078,26 @@ static int overlay_merge(void *fdt, void *fdto)
 			return overlay;

 		target = fdt_overlay_target_offset(fdt, fdto, fragment, NULL);
-		if (target < 0)
-			return target;
+		if (target < 0) {
+			if (!merge_olay_max_phdl
+			    || target != -FDT_ERR_BADPHANDLE)
+				return target;
+
+			/*
+			 * No target found which is acceptable situation in case
+			 * of merging two overlay blobs. Copy this fragment to
+			 * base/combined blob, so that it can be considered for
+			 * overlay during a subsequent overlay operation of
+			 * combined blob on another base blob.
+			 */
+			ret = copy_fragment_to_base(fdt, fdto, fragment,
+						    merge_olay_max_phdl,
+						    &base_fragment_count);
+			if (ret)
+				return ret;
+
+			continue;
+		}

 		ret = overlay_apply_node(fdt, target, fdto, overlay);
 		if (ret)
@@ -857,7 +1106,6 @@ static int overlay_merge(void *fdt, void *fdto)

 	return 0;
 }
-
 static int get_path_len(const void *fdt, int nodeoffset)
 {
 	int len = 0, namelen;
@@ -1069,7 +1317,7 @@ int fdt_overlay_apply(void *fdt, void *fdto)
 	if (ret)
 		goto err;

-	ret = overlay_merge(fdt, fdto);
+	ret = overlay_merge(fdt, fdto, NULL);
 	if (ret)
 		goto err;

@@ -1102,6 +1350,8 @@ err:
 int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
 {
 	uint32_t delta = fdt_get_max_phandle(fdt);
+	uint32_t delta0 = fdt_get_max_phandle(fdto);
+	uint32_t max_phandle;
 	int ret;

 	FDT_RO_PROBE(fdt);
@@ -1109,6 +1359,10 @@ int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)

 	*fdto_nospace = 0;

+	if (UINT32_MAX - delta < delta0)
+		return -FDT_ERR_BADOVERLAY;
+	max_phandle = delta + delta0 + 1;
+
 	ret = overlay_adjust_local_phandles(fdto, delta);
 	if (ret) {
 		if (ret == -FDT_ERR_NOSPACE)
@@ -1127,7 +1381,7 @@ int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
 	if (ret)
 		goto err;

-	ret = overlay_merge(fdt, fdto);
+	ret = overlay_merge(fdt, fdto, &max_phandle);
 	if (ret)
 		goto err;

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 7475cafce071..ca7abb296ce2 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -346,6 +346,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	int err;
 	uint32_t tag;
 	fdt32_t *endtag;
+	int depth = 0;

 	FDT_RW_PROBE(fdt);

@@ -355,15 +356,16 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	else if (offset != -FDT_ERR_NOTFOUND)
 		return offset;

-	/* Try to place the new node after the parent's properties */
-	tag = fdt_next_tag(fdt, parentoffset, &nextoffset);
-	/* the fdt_subnode_offset_namelen() should ensure this never hits */
-	if (!can_assume(LIBFDT_FLAWLESS) && (tag != FDT_BEGIN_NODE))
-		return -FDT_ERR_INTERNAL;
+	/* Try to place the new node at the end of the parent */
+	nextoffset = parentoffset;
 	do {
 		offset = nextoffset;
 		tag = fdt_next_tag(fdt, offset, &nextoffset);
-	} while ((tag == FDT_PROP) || (tag == FDT_NOP));
+		if (tag == FDT_BEGIN_NODE)
+			depth++;
+		else if (tag == FDT_END_NODE)
+			depth--;
+	} while (depth > 0);

 	nh = fdt_offset_ptr_w_(fdt, offset);
 	nodelen = sizeof(*nh) + FDT_TAGALIGN(namelen+1) + FDT_TAGSIZE;
--
2.49.0

[PATCH v3 3/4] libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups

[Wasim Nazir]

From: Srivatsa Vaddagiri <vatsa@codeaurora.org>

fdt_overlay_merge(), like fdt_overlay_apply(), has to perform similar
operations on DT blobs.

First operation is going through external symbol references specified
in __fixups__ node of overlay DT blob and resolving them in base DT blob (as
performed by overlay_fixups_update()).

Another operation is going through __local_fixups__ of overlay DT blob
and resolving them in base DT blob (as performed by
overlay_local_fixups_update()).

Unlike overlay case though, in case of merging two overlay blobs, its quite
normal that some of the external references specified in __fixups__ node are
not found in base blob. Modify overlay_fixup_phandles() to understand this
possibility.

Similarly while merging two blobs, the __symbol__ path needs to be
updated based on the possibility that target is available in base DT
or not. Modify overlay_symbol_update() to understand this possibility.

Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
---
 libfdt/fdt_overlay.c | 582 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 568 insertions(+), 14 deletions(-)

diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 03c88b05be72..aa85774b6f54 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -14,6 +14,8 @@

 #define MAX_BUF_SIZE	256

+static bool find_node_str(const char *path_str, const char *node);
+
 /**
  * overlay_get_target_phandle - retrieves the target phandle of a fragment
  * @fdto: pointer to the device tree overlay blob
@@ -396,11 +398,11 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
 	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;
@@ -459,6 +461,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
  *                          device tree
  * @fdt: Base Device Tree blob
  * @fdto: Device tree overlay blob
+ * @merge_olay_max_phdl: Pointer to max phandle value for merged blobs,
+ *     Both input blobs are overlay blobs that are being merged
  *
  * overlay_fixup_phandles() resolves all the overlay phandles pointing
  * to nodes in the base device tree.
@@ -471,7 +475,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
  *      0 on success
  *      Negative error code on failure
  */
-static int overlay_fixup_phandles(void *fdt, void *fdto)
+static int overlay_fixup_phandles(void *fdt, void *fdto,
+				  uint32_t merge_olay_max_phdl)
 {
 	int fixups_off, symbols_off;
 	int property;
@@ -492,7 +497,7 @@ static int overlay_fixup_phandles(void *fdt, void *fdto)
 		int ret;

 		ret = overlay_fixup_phandle(fdt, fdto, symbols_off, property);
-		if (ret)
+		if (ret && (!merge_olay_max_phdl || ret != -FDT_ERR_NOTFOUND))
 			return ret;
 	}

@@ -1106,6 +1111,7 @@ static int overlay_merge(void *fdt, void *fdto, uint32_t *merge_olay_max_phdl)

 	return 0;
 }
+
 static int get_path_len(const void *fdt, int nodeoffset)
 {
 	int len = 0, namelen;
@@ -1138,6 +1144,8 @@ static int get_path_len(const void *fdt, int nodeoffset)
  * overlay_symbol_update - Update the symbols of base tree after a merge
  * @fdt: Base Device Tree blob
  * @fdto: Device tree overlay blob
+ * @merge_olay_max_phdl: Pointer to max phandle value for merged blobs,
+ *         Both input blobs are overlay blobs that are being merged
  *
  * overlay_symbol_update() updates the symbols of the base tree with the
  * symbols of the applied overlay
@@ -1150,15 +1158,16 @@ static int get_path_len(const void *fdt, int nodeoffset)
  *      0 on success
  *      Negative error code on failure
  */
-static int overlay_symbol_update(void *fdt, void *fdto)
+static int overlay_symbol_update(void *fdt, void *fdto,
+				 uint32_t merge_olay_max_phdl)
 {
-	int root_sym, ov_sym, prop, path_len, fragment, target;
-	int len, frag_name_len, ret, rel_path_len;
+	int root_sym, ov_sym, prop, next_prop, path_len, fragment, target;
+	int len, frag_name_len, ret, rel_path_len, rel_path_len_olay = 0;
 	const char *s, *e;
 	const char *path;
 	const char *name;
 	const char *frag_name;
-	const char *rel_path;
+	const char *rel_path, *rel_path_olay = NULL;
 	const char *target_path;
 	char *buf;
 	void *p;
@@ -1180,7 +1189,11 @@ static int overlay_symbol_update(void *fdt, void *fdto)
 		return root_sym;

 	/* iterate over each overlay symbol */
-	fdt_for_each_property_offset(prop, fdto, ov_sym) {
+	/* Safeguard against property being possibly deleted in this loop */
+	prop = fdt_first_property_offset(fdto, ov_sym);
+	while (prop >= 0) {
+		next_prop = fdt_next_property_offset(fdto, prop);
+
 		path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
 		if (!path)
 			return path_len;
@@ -1212,6 +1225,11 @@ static int overlay_symbol_update(void *fdt, void *fdto)
 			/* /<fragment-name>/__overlay__/<relative-subnode-path> */
 			rel_path = s + len;
 			rel_path_len = e - rel_path - 1;
+
+			if (merge_olay_max_phdl != 0) {
+				rel_path_olay = s + 1;
+				rel_path_len_olay = e - rel_path_olay - 1;
+			}
 		} else if ((e - s) == len
 			   && (memcmp(s, "/__overlay__", len - 1) == 0)) {
 			/* /<fragment-name>/__overlay__ */
@@ -1240,8 +1258,24 @@ static int overlay_symbol_update(void *fdt, void *fdto)
 		ret = fdt_overlay_target_offset(fdt, fdto, fragment, &target_path);
 		if (ret < 0)
 			return ret;
+
 		target = ret;

+		/* Before proceeding further, check if you need to update
+		 * rel_path for dtbo-dtbo merging case
+		 */
+		if (rel_path_olay) {
+			uint32_t phandle =
+			    overlay_get_target_phandle(fdto, fragment);
+			int base_symbol_found = (phandle < merge_olay_max_phdl);
+
+			/* For new nodes we need the overlay string in path */
+			if (!base_symbol_found) {
+				rel_path = rel_path_olay;
+				rel_path_len = rel_path_len_olay;
+			}
+		}
+
 		/* if we have a target path use */
 		if (!target_path) {
 			ret = get_path_len(fdt, target);
@@ -1280,6 +1314,7 @@ static int overlay_symbol_update(void *fdt, void *fdto)
 		buf[len] = '/';
 		memcpy(buf + len + 1, rel_path, rel_path_len);
 		buf[len + 1 + rel_path_len] = '\0';
+		prop = next_prop;
 	}

 	return 0;
@@ -1308,7 +1343,7 @@ int fdt_overlay_apply(void *fdt, void *fdto)
 		goto err;

 	/* Update fdto's phandles using symbols from fdt */
-	ret = overlay_fixup_phandles(fdt, fdto);
+	ret = overlay_fixup_phandles(fdt, fdto, 0);
 	if (ret)
 		goto err;

@@ -1321,7 +1356,7 @@ int fdt_overlay_apply(void *fdt, void *fdto)
 	if (ret)
 		goto err;

-	ret = overlay_symbol_update(fdt, fdto);
+	ret = overlay_symbol_update(fdt, fdto, 0);
 	if (ret)
 		goto err;

@@ -1347,6 +1382,514 @@ err:
 	return ret;
 }

+static int find_add_subnode(void *fdt, int parent_off, const char *node_name)
+{
+	int offset;
+
+	offset = fdt_subnode_offset(fdt, parent_off, node_name);
+
+	if (offset < 0)
+		offset = fdt_add_subnode(fdt, parent_off, node_name);
+
+	return offset;
+}
+
+static int prop_exists_in_node(void *fdt, const char *path, const char *prop_name)
+{
+	int offset;
+	const void *val;
+
+	offset = fdt_path_offset(fdt, path);
+	if (offset < 0)
+		return 0;
+
+	val = fdt_getprop(fdt, offset, prop_name, NULL);
+
+	return val != NULL;
+}
+
+static void *get_next_component(const char **p, int *len, char sep)
+{
+	char *q;
+	int consumed;
+
+	q = memchr(*p, sep, *len);
+	if (!q)
+		return NULL;
+
+	/* 1 for ':' */
+	q++;
+
+	consumed = (q - *p);
+	if (*len <= consumed)
+		return NULL;
+
+	*len -= consumed;
+	*p = q;
+
+	return q;
+}
+
+static int lookup_target_path(void *fdt, void *fdto, const char *fragment,
+			      int frag_name_len, char *buf, int buf_len,
+			      int *target_off, int *root_path)
+{
+	int offset, ret, target, len;
+	const char *target_path;
+	static const char fragstr[] = "fragment@";
+	int fragstrlen = sizeof(fragstr) - 1;
+
+	memset(buf, 0, buf_len);
+
+	/* Check fdto-fragment has fragment string */
+	if (frag_name_len < fragstrlen || memcmp(fragment, fragstr, fragstrlen))
+		return -FDT_ERR_BADOVERLAY;
+
+	/* find the fragment index in which the symbol lies */
+	ret = fdt_subnode_offset_namelen(fdto, 0, fragment, frag_name_len);
+	/* not found? */
+	if (ret < 0)
+		return -FDT_ERR_BADOVERLAY;
+
+	offset = ret;
+
+	/* an __overlay__ subnode must exist */
+	ret = fdt_subnode_offset(fdto, offset, "__overlay__");
+	if (ret < 0)
+		return -FDT_ERR_BADOVERLAY;
+
+	/* get the target of the fragment */
+	ret = fdt_overlay_target_offset(fdt, fdto, offset, &target_path);
+	if (ret < 0)
+		return ret;
+
+	target = ret;
+	if (target_off)
+		*target_off = ret;
+
+	/* if we have a target path use */
+	if (!target_path) {
+		ret = get_path_len(fdt, target);
+		if (ret < 0)
+			return ret;
+		len = ret;
+	} else {
+		len = strlen(target_path);
+	}
+	if (len >= buf_len)
+		return -FDT_ERR_INTERNAL;
+
+	if (len > 1) {		/* target is not root */
+		if (!target_path) {
+			ret = fdt_get_path(fdt, target, buf, len + 1);
+			if (ret < 0)
+				return ret;
+		} else
+			memcpy(buf, target_path, len + 1);
+
+	}
+
+	/* Check fdt-path is having fragments or it part of root path */
+	if (root_path && find_node_str(buf, fragstr))
+		*root_path = 0;
+	else
+		*root_path = 1;
+
+	return 0;
+}
+
+static int fixup_snippet_update(void *fdt, void *fdto, const char *snippet,
+				int snippet_len, char *buf, int buflen,
+				int *ignore, int base_symbol_found,
+				uint32_t merge_olay_max_phdl)
+{
+	const char *snippet_o = snippet;
+	const char *path, *fragment, *prop_name, *prop_val, *rel_path;
+	char *sep;
+	int snippet_len_o = snippet_len, fragment_len, rel_path_len;
+	unsigned long int prop_len, path_len;
+	int rem, ret;
+	static const char tprop[] = "target";
+	static const char frag[] = "/fragment";
+	static const char olay[] = "/__overlay__";
+	int root_path = 0;
+
+	/* Validate format:
+	 *      path_to_node : prop_name : prop_offset
+	 */
+	path = snippet;
+	prop_name = get_next_component(&snippet, &snippet_len, ':');
+	if (!prop_name)
+		return -FDT_ERR_BADOVERLAY;
+
+	prop_val = get_next_component(&snippet, &snippet_len, ':');
+	if (!prop_val)
+		return -FDT_ERR_BADOVERLAY;
+
+	path_len = prop_name - path - 1;	/* -1 for ':' */
+	prop_len = prop_val - prop_name - 1;	/* -1 for ':' */
+
+	if (path_len < sizeof(frag) - 1 || memcmp(path, frag, sizeof(frag) - 1))
+		return -FDT_ERR_BADOVERLAY;
+
+	if (base_symbol_found && prop_len == sizeof(tprop) - 1
+	    && !memcmp(prop_name, tprop, sizeof(tprop) - 1)) {
+		*ignore = 1;
+		return 0;
+	}
+
+	fragment = path;
+	/* check if there is a '/' besides the first one in node_path */
+	sep = memchr(fragment + 1, '/', path_len - 1);
+	if (sep) {
+		fragment_len = sep - fragment;
+		path_len -= (sep - fragment);
+		if (path_len < sizeof(olay) - 1
+		    || memcmp(sep, olay, sizeof(olay) - 1))
+			return -FDT_ERR_BADOVERLAY;
+
+		{
+			int frag_offset;
+
+			frag_offset =
+			    fdt_subnode_offset_namelen(fdto, 0, fragment + 1,
+						       fragment_len - 1);
+			if (frag_offset < 0)
+				return -FDT_ERR_BADOVERLAY;
+
+			/* an __overlay__ subnode must exist */
+			ret =
+			    fdt_subnode_offset(fdto, frag_offset,
+					       "__overlay__");
+			if (ret < 0)
+				return -FDT_ERR_BADOVERLAY;
+		}
+		rel_path = sep;
+	} else {
+		rel_path = fragment + path_len;
+		fragment_len = path_len;
+	}
+	rel_path_len = snippet_len_o - (rel_path - snippet_o);
+
+	if (rel_path_len <= 0 || fragment_len >= buflen)
+		return -FDT_ERR_INTERNAL;
+
+	ret =
+	    lookup_target_path(fdt, fdto, fragment + 1, fragment_len - 1, buf,
+			       buflen, NULL, &root_path);
+	if (ret)
+		return ret;
+
+	/* Only single overlay should be present in path */
+	if (find_node_str(buf, "__overlay__")
+	    && find_node_str(rel_path, "__overlay__")) {
+		if (sep)
+			rel_path = sep + sizeof(olay) - 1;
+	}
+
+	rem = buflen - strlen(buf);
+	if (rel_path_len >= rem)
+		return -FDT_ERR_INTERNAL;
+
+	sep = buf + strlen(buf);
+	memcpy(sep, rel_path, rel_path_len);
+
+	return 0;
+}
+
+static const char *next_snippet(const char **prop,
+				int *prop_len, int *snippet_len)
+{
+	const char *next = *prop;
+	const char *tmp;
+	int len;
+
+	if (*prop_len <= 0)
+		return NULL;
+
+	tmp = memchr(next, '\0', *prop_len);
+	if (!tmp)
+		return NULL;
+
+	tmp++;
+
+	len = tmp - next;
+	*snippet_len = len;
+	*prop += len;
+	*prop_len -= len;
+
+	return next;
+}
+
+static int add_to_fixups(void *fdt, char *v, const char *label)
+{
+	const char *val;
+	char *p;
+	int vlen = strlen(v) + 1;	/* +1 for NULL */
+	int len, ret;
+	int root_fixup;
+
+	root_fixup = fdt_subnode_offset(fdt, 0, "__fixups__");
+	if (root_fixup == -FDT_ERR_NOTFOUND)
+		root_fixup = fdt_add_subnode(fdt, 0, "__fixups__");
+
+	if (root_fixup < 0)
+		return root_fixup;
+
+	val = fdt_getprop(fdt, root_fixup, label, &len);
+	if (val)
+		vlen += len;
+
+	ret = fdt_setprop_placeholder(fdt, root_fixup, label,
+				      vlen, (void **)&p);
+	if (ret)
+		return ret;
+
+	if (val) {
+		p += len;
+		vlen -= len;
+	}
+	memcpy(p, v, vlen);
+
+	return 0;
+}
+
+static int fdt_find_add_node(void *fdt, int parent_off, const char *node)
+{
+	int offset;
+
+	offset = fdt_subnode_offset(fdt, parent_off, node);
+	if (offset < 0)
+		offset = fdt_add_subnode(fdt, parent_off, node);
+
+	return offset;
+}
+
+/* path => /abc/def/ghi */
+static const char *next_node(const char **path, int *path_len, int *node_len)
+{
+	const char *sep = *path, *node;
+
+	if (*sep != '/' || *path_len <= 0)
+		return NULL;
+
+	*path = *path + 1;
+	node = *path;
+	*path_len = *path_len - 1;
+
+	sep = memchr(node, '/', *path_len);
+	if (sep)
+		*node_len = sep - node;
+	else
+		*node_len = *path_len;
+
+	*path_len -= *node_len;
+	*path += *node_len;
+
+	return node;
+}
+
+static bool find_node_str(const char *path_str, const char *ip_str)
+{
+	const char *path, *node_str;
+	int path_len = 0, node_len = 0, ip_str_len = 0;
+
+	path = path_str;
+	path_len = strlen(path);
+	ip_str_len = strlen(ip_str);
+
+	while ((node_str = next_node(&path, &path_len, &node_len))) {
+		if (node_len >= ip_str_len
+		    && !memcmp(node_str, ip_str, ip_str_len)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static int convert_to_u32(const char *p, uint32_t *val)
+{
+	char *endptr;
+	unsigned long prop_val;
+
+	prop_val = strtoul(p, &endptr, 10);
+	if ((*endptr != '\0') || (endptr <= p))
+		return -FDT_ERR_BADOVERLAY;
+
+	*val = prop_val;	/* size mis-match? */
+
+	return 0;
+}
+
+static int add_to_local_fixups(void *fdt, const char *snippet)
+{
+	const char *path, *prop_name, *prop_val, *node;
+	unsigned long int prop_len = 0;
+	int path_len, node_len;
+	int parent, ret;
+	int snippet_len = strlen(snippet);
+	uint32_t val = 0;
+	char buf[MAX_BUF_SIZE];
+
+	/* Validate format:
+	 *      path_to_node : prop_name : prop_offset
+	 *      OR
+	 *      path_to_node
+	 */
+	path = snippet;
+	prop_name = get_next_component(&snippet, &snippet_len, ':');
+
+	if (prop_name) {
+		prop_val = get_next_component(&snippet, &snippet_len, ':');
+		if (!prop_val)
+			return -FDT_ERR_BADOVERLAY;
+
+		path_len = prop_name - path - 1;	/* -1 for ':' */
+		prop_len = prop_val - prop_name - 1;	/* -1 for ':' */
+
+		ret = convert_to_u32(prop_val, &val);
+		if (ret)
+			return ret;
+	} else
+		path_len = strlen(snippet);
+
+	parent = fdt_find_add_node(fdt, 0, "__local_fixups__");
+	if (parent < 0)
+		return parent;
+
+	while ((node = next_node(&path, &path_len, &node_len))) {
+		int offset;
+
+		offset =
+		    fdt_subnode_offset_namelen(fdt, parent, node, node_len);
+		if (offset < 0) {
+			offset =
+			    fdt_add_subnode_namelen(fdt, parent, node,
+						    node_len);
+		}
+		if (offset < 0)
+			return offset;
+		parent = offset;
+	}
+
+	if (!prop_name)
+		return parent;
+
+	if (prop_len >= sizeof(buf))
+		return -FDT_ERR_INTERNAL;
+	memcpy(buf, prop_name, prop_len);
+	buf[prop_len] = 0;
+
+	return fdt_appendprop_u32(fdt, parent, buf, val);
+}
+
+static int overlay_fixups_update(void *fdt, void *fdto,
+				 uint32_t merge_olay_max_phdl)
+{
+	int ov_fixup, root_fixup, prop;
+
+	ov_fixup = fdt_subnode_offset(fdto, 0, "__fixups__");
+	if (ov_fixup < 0)
+		return 0;
+
+	root_fixup = find_add_subnode(fdt, 0, "__fixups__");
+	if (root_fixup < 0)
+		return root_fixup;
+
+	fdt_for_each_property_offset(prop, fdto, ov_fixup) {
+		int snippet_len, prop_len, base_symbol_found;
+		const char *label, *snippet, *prop_val;
+
+		prop_val = fdt_getprop_by_offset(fdto, prop, &label, &prop_len);
+		if (prop_val == NULL)
+			return -FDT_ERR_BADOVERLAY;
+
+		base_symbol_found =
+		    prop_exists_in_node(fdt, "/__symbols__", label);
+
+		while ((snippet =
+			next_snippet(&prop_val, &prop_len, &snippet_len))) {
+			char new_val[MAX_BUF_SIZE];
+			int ignore = 0, ret;
+
+			ret =
+			    fixup_snippet_update(fdt, fdto, snippet,
+						 snippet_len, new_val,
+						 sizeof(new_val), &ignore,
+						 base_symbol_found,
+						 merge_olay_max_phdl);
+			if (ret)
+				return ret;
+
+			if (ignore)
+				continue;
+
+			if (!base_symbol_found) {
+				ret = add_to_fixups(fdt, new_val, label);
+			} else {
+				ret = add_to_local_fixups(fdt, new_val);
+			}
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int overlay_local_fixups_update(void *fdt, void *fdto,
+				       uint32_t merge_olay_max_phdl)
+{
+	int ov_lfixups, root_lfixups, node, ret;
+	int root_path = 0;
+
+	ov_lfixups = fdt_subnode_offset(fdto, 0, "__local_fixups__");
+	if (ov_lfixups == -FDT_ERR_NOTFOUND)
+		return 0;
+
+	root_lfixups = fdt_subnode_offset(fdt, 0, "__local_fixups__");
+	if (root_lfixups == -FDT_ERR_NOTFOUND)
+		root_lfixups = fdt_add_subnode(fdt, 0, "__local_fixups__");
+
+	if (root_lfixups < 0)
+		return root_lfixups;
+
+	fdt_for_each_subnode(node, fdto, ov_lfixups) {
+		int len, child_node, parent_node;
+		int skip_fdto_child = 0;
+		const char *name = fdt_get_name(fdto, node, &len);
+		char buf[MAX_BUF_SIZE];
+
+		ret = lookup_target_path(fdt, fdto, name, strlen(name),
+					 buf, sizeof(buf), NULL, &root_path);
+		if (ret)
+			return ret;
+
+		/* We want to skip dtbo overlay child in two cases
+		 * i.e When target path doesn't have fragment@ or __overlay__
+		 * strings
+		 */
+		if (root_path || find_node_str(buf, "__overlay__"))
+			skip_fdto_child = 1;
+
+		parent_node = add_to_local_fixups(fdt, buf);
+		if (parent_node < 0)
+			return parent_node;
+
+		child_node = fdt_subnode_offset(fdto, node, "__overlay__");
+		if (child_node < 0)
+			return -FDT_ERR_BADOVERLAY;
+
+		ret = copy_node(fdt, fdto, parent_node, child_node,
+				NULL, skip_fdto_child);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
 {
 	uint32_t delta = fdt_get_max_phandle(fdt);
@@ -1377,18 +1920,29 @@ int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
 		goto err;
 	}

-	ret = overlay_fixup_phandles(fdt, fdto);
-	if (ret)
+	ret = overlay_fixup_phandles(fdt, fdto, max_phandle);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
 		goto err;

 	ret = overlay_merge(fdt, fdto, &max_phandle);
 	if (ret)
 		goto err;

-	ret = overlay_symbol_update(fdt, fdto);
+	max_phandle = delta + delta0 + 1;
+	ret = overlay_symbol_update(fdt, fdto, max_phandle);
 	if (ret)
 		goto err;

+	/* fixups node is optional */
+	ret = overlay_fixups_update(fdt, fdto, max_phandle);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		goto err;
+
+	/* local_fixups node is optional */
+	ret = overlay_local_fixups_update(fdt, fdto, max_phandle);
+	if (ret && ret != -FDT_ERR_NOTFOUND)
+		goto err;
+
 	/*
 	 * The overlay has been damaged, erase its magic.
 	 */
--
2.49.0

[PATCH v3 4/4] fdtoverlaymerge: A tool that merges overlays

[Wasim Nazir]

From: Srivatsa Vaddagiri <vatsa@codeaurora.org>

Add a new command-line tool, fdtoverlaymerge, that merges two or more
overlay blobs using the fdt_overlay_merge() API. This tool is necessary
to simplify the process of combining multiple overlays into a single blob.

Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
---
 .gitignore         |   1 +
 Makefile           |   4 +
 Makefile.utils     |   6 ++
 fdtoverlaymerge.c  | 223 +++++++++++++++++++++++++++++++++++++++++++++
 libfdt/version.lds |   1 +
 meson.build        |   2 +-
 6 files changed, 236 insertions(+), 1 deletion(-)
 create mode 100644 fdtoverlaymerge.c

diff --git a/.gitignore b/.gitignore
index 8e1e0c05d427..8d032d80a8d0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,6 +17,7 @@ lex.yy.c
 /fdtget
 /fdtput
 /fdtoverlay
+/fdtoverlaymerge
 /patches
 /.pc

diff --git a/Makefile b/Makefile
index a00123d78b1b..d7c1fc4eb5f5 100644
--- a/Makefile
+++ b/Makefile
@@ -149,6 +149,7 @@ BIN += fdtdump
 BIN += fdtget
 BIN += fdtput
 BIN += fdtoverlay
+BIN += fdtoverlaymerge

 SCRIPTS = dtdiff

@@ -185,6 +186,7 @@ ifneq ($(MAKECMDGOALS),libfdt)
 -include $(FDTGET_OBJS:%.o=%.d)
 -include $(FDTPUT_OBJS:%.o=%.d)
 -include $(FDTOVERLAY_OBJS:%.o=%.d)
+-include $(FDTOVERLAYMERGE_OBJS:%.o=%.d)
 endif
 endif

@@ -276,6 +278,8 @@ fdtput:	$(FDTPUT_OBJS) $(LIBFDT_dep)

 fdtoverlay: $(FDTOVERLAY_OBJS) $(LIBFDT_dep)

+fdtoverlaymerge: $(FDTOVERLAYMERGE_OBJS) $(LIBFDT_dep)
+
 dist:
 	git archive --format=tar --prefix=dtc-$(dtc_version)/ HEAD \
 		> ../dtc-$(dtc_version).tar
diff --git a/Makefile.utils b/Makefile.utils
index 9436b34d791f..e24a8abc3b8b 100644
--- a/Makefile.utils
+++ b/Makefile.utils
@@ -29,3 +29,9 @@ FDTOVERLAY_SRCS = \
 	util.c

 FDTOVERLAY_OBJS = $(FDTOVERLAY_SRCS:%.c=%.o)
+
+FDTOVERLAYMERGE_SRCS = \
+	fdtoverlaymerge.c \
+	util.c
+
+FDTOVERLAYMERGE_OBJS = $(FDTOVERLAYMERGE_SRCS:%.c=%.o)
diff --git a/fdtoverlaymerge.c b/fdtoverlaymerge.c
new file mode 100644
index 000000000000..eaf3e97e0983
--- /dev/null
+++ b/fdtoverlaymerge.c
@@ -0,0 +1,223 @@
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for more details.
+ */
+
+#include <assert.h>
+#include <ctype.h>
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <inttypes.h>
+
+#include <libfdt.h>
+
+#include "util.h"
+
+/* Usage related data. */
+static const char usage_synopsis[] =
+	"merge a number of overlays\n"
+	"	fdtoverlaymerge <options> [<overlay.dtbo> [<overlay.dtbo>]]\n"
+	"\n"
+	USAGE_TYPE_MSG;
+static const char usage_short_opts[] = "i:o:v" USAGE_COMMON_SHORT_OPTS;
+static struct option const usage_long_opts[] = {
+	{"input",            required_argument, NULL, 'i'},
+	{"output",	     required_argument, NULL, 'o'},
+	{"verbose",	           no_argument, NULL, 'v'},
+	USAGE_COMMON_LONG_OPTS,
+};
+static const char * const usage_opts_help[] = {
+	"Input base overlay DT blob",
+	"Output DT blob",
+	"Verbose messages",
+	USAGE_COMMON_OPTS_HELP
+};
+
+int verbose = 0;
+
+static void grow_blob(char **blob, off_t extra_len)
+{
+	int blob_len;
+
+	if (!extra_len)
+		return;
+
+	blob_len = fdt_totalsize(*blob) + extra_len;
+	*blob = xrealloc(*blob, blob_len);
+	fdt_open_into(*blob, *blob, blob_len);
+}
+
+static int reload_blob(char *filename, char **blob, off_t extra_len)
+{
+	size_t len;
+
+	free(*blob);
+	*blob = utilfdt_read(filename, &len);
+	if (!*blob) {
+		fprintf(stderr, "Failed to reload blob %s\n", filename);
+		return -1;
+	}
+
+	grow_blob(blob, extra_len);
+
+	return 0;
+}
+
+static int do_fdtoverlay_merge(const char *input_filename,
+			 const char *output_filename,
+			 int argc, char *argv[])
+{
+	char *blob = NULL;
+	char **ovblob = NULL;
+	size_t ov_len, blob_len, total_len, extra_blob_len = 0;
+	off_t *extra_ov_len;
+	int i, ret = -1;
+
+	/* allocate blob pointer array */
+	ovblob = xmalloc(sizeof(*ovblob) * argc);
+	memset(ovblob, 0, sizeof(*ovblob) * argc);
+
+	extra_ov_len = xmalloc(sizeof(*extra_ov_len) * argc);
+	memset(extra_ov_len, 0, sizeof(*extra_ov_len) * argc);
+
+reload_all_blobs:
+	/* Free existing buffer first */
+	free(blob);
+	blob = utilfdt_read(input_filename, &blob_len);
+	if (!blob) {
+		fprintf(stderr, "\nFailed to read base blob %s\n",
+				input_filename);
+		goto out_err;
+	}
+	if (fdt_totalsize(blob) > blob_len) {
+		fprintf(stderr,
+		 "\nBase blob is incomplete (%lu / %" PRIu32 " bytes read)\n",
+			(unsigned long)blob_len, fdt_totalsize(blob));
+		goto out_err;
+	}
+	ret = 0;
+
+	/* read and keep track of the overlay blobs */
+	total_len = extra_blob_len;
+	for (i = 0; i < argc; i++) {
+		/* Free existing buffer first */
+		free(ovblob[i]);
+		ovblob[i] = utilfdt_read(argv[i], &ov_len);
+		if (!ovblob[i]) {
+			fprintf(stderr, "\nFailed to read overlay %s\n",
+					argv[i]);
+			goto out_err;
+		}
+		grow_blob(&ovblob[i], extra_ov_len[i]);
+		total_len += ov_len + extra_ov_len[i];
+	}
+
+	/* grow the blob to worst case */
+	grow_blob(&blob, total_len);
+
+	/* apply the overlays in sequence */
+	for (i = 0; i < argc; i++) {
+		do {
+			int fdto_nospace;
+
+			fprintf(stderr, "Merging overlay blob %s\n", argv[i]);
+			ret = fdt_overlay_merge(blob, ovblob[i], &fdto_nospace);
+			if (ret && ret == -FDT_ERR_NOSPACE) {
+				if (fdto_nospace) {
+					extra_ov_len[i] += 512;
+					fprintf(stderr, "Reloading overlay blob %s\n", argv[i]);
+					ret = reload_blob(argv[i], &ovblob[i], extra_ov_len[i]);
+					if (!ret)
+						continue;
+				} else {
+					extra_blob_len += 512;
+					fprintf(stderr, "Reloading all blobs\n");
+					goto reload_all_blobs;
+				}
+			}
+
+			if (!ret)
+				break;
+
+			if (ret) {
+				fprintf(stderr, "\nFailed to merge %s (%d)\n",
+						argv[i], ret);
+				goto out_err;
+			}
+		} while (1);
+	}
+
+	fdt_pack(blob);
+	ret = utilfdt_write(output_filename, blob);
+	if (ret)
+		fprintf(stderr, "\nFailed to write output blob %s\n",
+				output_filename);
+
+out_err:
+	if (ovblob) {
+		for (i = 0; i < argc; i++) {
+			if (ovblob[i])
+				free(ovblob[i]);
+		}
+		free(ovblob);
+	}
+	free(blob);
+
+	return ret;
+}
+
+int main(int argc, char *argv[])
+{
+	int opt, i;
+	char *input_filename = NULL;
+	char *output_filename = NULL;
+
+	while ((opt = util_getopt_long()) != EOF) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+
+		case 'i':
+			input_filename = optarg;
+			break;
+		case 'o':
+			output_filename = optarg;
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		}
+	}
+
+	if (!input_filename)
+		usage("missing input file");
+
+	if (!output_filename)
+		usage("missing output file");
+
+	argv += optind;
+	argc -= optind;
+
+	if (argc <= 0)
+		usage("missing overlay file(s)");
+
+	if (verbose) {
+		printf("input  = %s\n", input_filename);
+		printf("output = %s\n", output_filename);
+		for (i = 0; i < argc; i++)
+			printf("overlay[%d] = %s\n", i, argv[i]);
+	}
+
+	if (do_fdtoverlay_merge(input_filename, output_filename, argc, argv))
+		return 1;
+
+	return 0;
+}
diff --git a/libfdt/version.lds b/libfdt/version.lds
index cbfef546de6c..942be3f4417c 100644
--- a/libfdt/version.lds
+++ b/libfdt/version.lds
@@ -82,6 +82,7 @@ LIBFDT_1.2 {
 		fdt_overlay_target_offset;
 		fdt_get_symbol;
 		fdt_get_symbol_namelen;
+		fdt_overlay_merge;
 	local:
 		*;
 };
diff --git a/meson.build b/meson.build
index 3026f88d895a..8def38851247 100644
--- a/meson.build
+++ b/meson.build
@@ -106,7 +106,7 @@ if get_option('tools')
     install: true,
   )

-  foreach e: ['fdtdump', 'fdtget', 'fdtput', 'fdtoverlay']
+  foreach e: ['fdtdump', 'fdtget', 'fdtput', 'fdtoverlay', 'fdtoverlaymerge']
     dtc_tools += executable(e, files(e + '.c'), dependencies: util_dep, install: true)
   endforeach

--
2.49.0

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Trilok Soni]

On 5/19/2025 2:10 AM, Wasim Nazir wrote:
> Hello,
> 
> This is follow-up attempt for fdtoverlaymerge tool.

Please add detail of the first attempt, since we are submitting it after a long time. 

> 
> Currently all the device-tree (DT) code for a given soc is maintained in a
> common kernel repository. For example, this common DT code will have code for
> audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> split into a base (soc-common) code and board specific code, with the soc code
> being compiled as soc.dtb and board specific code being compiled as respective
> overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> of a given SOC while boardX.dtbo represents configuration of a board/platform
> designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> target (besides improving the overall size of DT blobs flashed on target, Android
> Treble also requires separation of soc and board DT bits). Bootloader will pick
> one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> which is presented a unified DT blob (soc + board overlay).

Vatsa wrote it as per the Qualcomm source structure in the Android at that time. 
You can see that he is referring "common" kernel repository etc; You need
to rewrite it or explicitly mention it per Qualcomm directory structure
in Android or QCLinux plus the tech-packages / external driver modules we have. 

You need to translate the problem which others outside Qualcomm can understand
and relate in-order to understand these patches. 


> 
> For ease of code maintenance and better control over release management, we are
> exploring allowing some of the tech teams (audio/fingerprint sensor etc) to

"tech teams" is a internal usage within Qualcomm and it will be hard to understand
some times outside QC. Please rewrite. 

> maintain their kernel code (including their DT code) outside a common kernel
> repository. In our experience, this simplifies number of branches maintained in
> core kernel repo. New/experimental features in fingerprint sensor driver for

"kernel repo" is also bit QC specific. I will stop reviewing this patch
since you should consider rewriting this summary. 

> example that needs to be on a separate branch will not result in unnecessary
> branching in core kenrel repo, affecting all other drivers.
> 
> In addition to compiling DT code outside core kernel tree, we also want to merge
> the blobs back to respective blobs found in kernel build tree at buildtime
> (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> overlay impacts boot-time.
> 
> This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> boardX.dtbo), which currently doesn't seem to be supported and which this patch
> series aims to support.
> 
> fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> with a base blob. It assumes that all external symbols specified in overlay
> blob's __fixups__ section are found in base blob's __symbols__ section and
> aborts on the first instance where a symbol could not be found in base blob.
> This is mostly fine as the primary use of overlay is on a target for its
> bootloader to merge various overlay blobs based on h/w configuration detected.
> But when the number of overlays increased then bootloader takes lot of time to
> apply the overlays on base DT.
> 
> So we need new API/tool to merge all the overlays into single overlay file
> at host (build machine) side, so that on target side bootloader needs to only
> apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> number file reading/loading & minimizing repeatative overlay apply.
> In our test setup we see an improvement of ~60% while applying merged-overlay
> at bootloader and the merged-overlay is product of 7 overlays.
> 
> To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> which takes input as overlays and gives output to merged-overlay.
> The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> 
> Additional notes:
>   If snprintf (in libc) may not available in some environments, then we will need
>   to write our own snprintf() in libfdt.
> 
> ---
> Changelog:
> 
> v3:
> - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
>   - Case1: When target is available and we merge fragments
>   - Case2: When target is not available and we add new fragments
> - Change the logic to update fixups & local_fixups in case of overlay merge.
> - Few patches are squashed, reduced to 4 patches.
> - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> 
> 
> Srivatsa Vaddagiri (4):
>   libfdt: overlay_merge: Introduce fdt_overlay_merge()
>   libfdt: overlay_merge: Rename & copy overlay fragments and their
>     properties
>   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
>   fdtoverlaymerge: A tool that merges overlays
> 
>  .gitignore           |   1 +
>  Makefile             |   4 +
>  Makefile.utils       |   6 +
>  fdtoverlaymerge.c    | 223 +++++++++++
>  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
>  libfdt/fdt_rw.c      |  14 +-
>  libfdt/libfdt.h      |  18 +
>  libfdt/version.lds   |   1 +
>  meson.build          |   2 +-
>  9 files changed, 1146 insertions(+), 24 deletions(-)
>  create mode 100644 fdtoverlaymerge.c
> 
> 
> base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> --
> 2.49.0
> 

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[Trilok Soni]

On 5/19/2025 2:10 AM, Wasim Nazir wrote:
> From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> 
> fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> used offline on a build machine to combine two or more overlay blobs into one.
> 
> It is intended to help maintain device-tree overlay code in
> multiple source repositories, but merge their binary forms (overlay blobs)
> into one so that bootloader's task of searching for all relevant overlay blobs
> is simplified.
> 
> Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> Subsequent patches will introduce required changes to merge overlay blobs.
> 
> Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>

It will be nice to switch to oss.qualcomm.com, since we will soon mandating it. 

> ---
>  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  libfdt/libfdt.h      | 18 ++++++++++++++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> index e6b9eb643958..8690ed55c8f6 100644
> --- a/libfdt/fdt_overlay.c
> +++ b/libfdt/fdt_overlay.c
> @@ -1098,3 +1098,62 @@ err:
> 
>  	return ret;
>  }
> +
> +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> +{
> +	uint32_t delta = fdt_get_max_phandle(fdt);
> +	int ret;
> +
> +	FDT_RO_PROBE(fdt);
> +	FDT_RO_PROBE(fdto);
> +
> +	*fdto_nospace = 0;
> +
> +	ret = overlay_adjust_local_phandles(fdto, delta);
> +	if (ret) {
> +		if (ret == -FDT_ERR_NOSPACE)
> +			*fdto_nospace = 1;
> +		goto err;
> +	}
> +
> +	ret = overlay_update_local_references(fdto, delta);
> +	if (ret) {
> +		if (ret == -FDT_ERR_NOSPACE)
> +			*fdto_nospace = 1;
> +		goto err;
> +	}
> +
> +	ret = overlay_fixup_phandles(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_merge(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_symbol_update(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.
> +	 */
> +	if (!*fdto_nospace)
> +		fdt_set_magic(fdt, ~0);
> +
> +	return ret;
> +}
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index b5e72001d115..664b141d5334 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
>  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
>  			      int fragment_offset, char const **pathp);
> 
> +/**
> + * fdt_overlay_merge - Merge two overlays into one
> + * @fdt: pointer to the first device tree overlay blob
> + * @fdto: pointer to the second device tree overlay blob
> + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> + *
> + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> + *
> + * Expect the first 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 first device tree blob
> + *	-FDT_ERR_BADVALUE
> + */
> +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> +
>  /**********************************************************************/
>  /* Debugging / informational functions                                */
>  /**********************************************************************/
> --
> 2.49.0
> 

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[David Gibson]

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

On Mon, May 19, 2025 at 02:40:39PM +0530, Wasim Nazir wrote:
> Hello,
> 
> This is follow-up attempt for fdtoverlaymerge tool.
> 
> Currently all the device-tree (DT) code for a given soc is maintained in a
> common kernel repository. For example, this common DT code will have code for
> audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> split into a base (soc-common) code and board specific code, with the soc code
> being compiled as soc.dtb and board specific code being compiled as respective
> overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> of a given SOC while boardX.dtbo represents configuration of a board/platform
> designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on

So.. *build time* separation of the SoC and board pieces makes sense
to me, which is I think how this convention arose.  *Boot time*
separation of the SoC and board seems kind of pointless.  Almost by
definition of what a "board" is, you must know early in boot which
board it is.  Still, I guess the convention is established, even if
it's stupid.

> target (besides improving the overall size of DT blobs flashed on target, Android
> Treble also requires separation of soc and board DT bits). Bootloader will pick
> one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> which is presented a unified DT blob (soc + board overlay).
> 
> For ease of code maintenance and better control over release management, we are
> exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> maintain their kernel code (including their DT code) outside a common kernel
> repository. In our experience, this simplifies number of branches maintained in
> core kernel repo. New/experimental features in fingerprint sensor driver for
> example that needs to be on a separate branch will not result in unnecessary
> branching in core kenrel repo, affecting all other drivers.
> 
> In addition to compiling DT code outside core kernel tree, we also want to merge
> the blobs back to respective blobs found in kernel build tree at buildtime
> (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> overlay impacts boot-time.

It's again unclear to me why you need a boot time separation of these
devices rather than merely boot time.  What does using separate .dtbo
files give you that just /include/ing multiple pieces into a single
.dtbo at build time would not?

> This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> boardX.dtbo), which currently doesn't seem to be supported and which this patch
> series aims to support.

Merging overlays is a logically sensible operation, but it's not clear
to me why the need for it follows from the premises above.  It's also
unclear why you need to compile to .dtbo *then* merge, rather than
combine .dts files then compile into a single .dtbo.

> fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> with a base blob. It assumes that all external symbols specified in overlay
> blob's __fixups__ section are found in base blob's __symbols__ section and
> aborts on the first instance where a symbol could not be found in base blob.
> This is mostly fine as the primary use of overlay is on a target for its
> bootloader to merge various overlay blobs based on h/w configuration detected.
> But when the number of overlays increased then bootloader takes lot of time to
> apply the overlays on base DT.
> 
> So we need new API/tool to merge all the overlays into single overlay file
> at host (build machine) side,

Merging into a single overlay at build time makes sense to me.  But at
build time you'd expect to have access to the .dts files.  Why do you
need to merge .dtbo rather than merge the .dts before compiling to
.dtbo?  The latter should be possible already by /include/ing each of
the individual overlays in order then compiling with dtc.

> so that on target side bootloader needs to only
> apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> number file reading/loading & minimizing repeatative overlay apply.
> In our test setup we see an improvement of ~60% while applying merged-overlay
> at bootloader and the merged-overlay is product of 7 overlays.
> 
> To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> which takes input as overlays and gives output to merged-overlay.
> The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> 
> Additional notes:
>   If snprintf (in libc) may not available in some environments, then we will need
>   to write our own snprintf() in libfdt.
> 
> ---
> Changelog:
> 
> v3:
> - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
>   - Case1: When target is available and we merge fragments
>   - Case2: When target is not available and we add new fragments
> - Change the logic to update fixups & local_fixups in case of overlay merge.
> - Few patches are squashed, reduced to 4 patches.
> - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> 
> 
> Srivatsa Vaddagiri (4):
>   libfdt: overlay_merge: Introduce fdt_overlay_merge()
>   libfdt: overlay_merge: Rename & copy overlay fragments and their
>     properties
>   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
>   fdtoverlaymerge: A tool that merges overlays
> 
>  .gitignore           |   1 +
>  Makefile             |   4 +
>  Makefile.utils       |   6 +
>  fdtoverlaymerge.c    | 223 +++++++++++
>  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
>  libfdt/fdt_rw.c      |  14 +-
>  libfdt/libfdt.h      |  18 +
>  libfdt/version.lds   |   1 +
>  meson.build          |   2 +-
>  9 files changed, 1146 insertions(+), 24 deletions(-)
>  create mode 100644 fdtoverlaymerge.c
> 
> 
> base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[David Gibson]

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

On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> 
> fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> used offline on a build machine to combine two or more overlay blobs into one.
> 
> It is intended to help maintain device-tree overlay code in
> multiple source repositories, but merge their binary forms (overlay blobs)
> into one so that bootloader's task of searching for all relevant overlay blobs
> is simplified.
> 
> Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> Subsequent patches will introduce required changes to merge overlay blobs.
> 
> Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> ---
>  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  libfdt/libfdt.h      | 18 ++++++++++++++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> index e6b9eb643958..8690ed55c8f6 100644
> --- a/libfdt/fdt_overlay.c
> +++ b/libfdt/fdt_overlay.c
> @@ -1098,3 +1098,62 @@ err:
> 
>  	return ret;
>  }
> +
> +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)

The parameters should be renamed to reflect the new semantics.

The 'fdto_nospace' parameter seems weird.  Why not just return
-FDT_ERR_NOSPACE from the function?

> +{
> +	uint32_t delta = fdt_get_max_phandle(fdt);
> +	int ret;
> +
> +	FDT_RO_PROBE(fdt);
> +	FDT_RO_PROBE(fdto);
> +
> +	*fdto_nospace = 0;
> +
> +	ret = overlay_adjust_local_phandles(fdto, delta);
> +	if (ret) {
> +		if (ret == -FDT_ERR_NOSPACE)
> +			*fdto_nospace = 1;
> +		goto err;
> +	}
> +
> +	ret = overlay_update_local_references(fdto, delta);
> +	if (ret) {
> +		if (ret == -FDT_ERR_NOSPACE)
> +			*fdto_nospace = 1;
> +		goto err;
> +	}
> +
> +	ret = overlay_fixup_phandles(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_merge(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
> +	ret = overlay_symbol_update(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

This comment is no longer accurate in the new context.

> +	 * magic.
> +	 */
> +	if (!*fdto_nospace)
> +		fdt_set_magic(fdt, ~0);
> +
> +	return ret;
> +}
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index b5e72001d115..664b141d5334 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
>  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
>  			      int fragment_offset, char const **pathp);
> 
> +/**
> + * fdt_overlay_merge - Merge two overlays into one
> + * @fdt: pointer to the first device tree overlay blob
> + * @fdto: pointer to the second device tree overlay blob
> + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> + *
> + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> + *
> + * Expect the first 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 first device tree blob
> + *	-FDT_ERR_BADVALUE
> + */
> +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> +
>  /**********************************************************************/
>  /* Debugging / informational functions                                */
>  /**********************************************************************/

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 4/4] fdtoverlaymerge: A tool that merges overlays

[Simon Glass]

Hi Wasim,

On Mon, 19 May 2025 at 11:16, Wasim Nazir <quic_wasimn@quicinc.com> wrote:
>
> From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
>
> Add a new command-line tool, fdtoverlaymerge, that merges two or more
> overlay blobs using the fdt_overlay_merge() API. This tool is necessary
> to simplify the process of combining multiple overlays into a single blob.

Normally this would be done in the bootloader, with the overlays in a
FIT. What environment does this tool run in?

>
> Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> ---
>  .gitignore         |   1 +
>  Makefile           |   4 +
>  Makefile.utils     |   6 ++
>  fdtoverlaymerge.c  | 223 +++++++++++++++++++++++++++++++++++++++++++++
>  libfdt/version.lds |   1 +
>  meson.build        |   2 +-
>  6 files changed, 236 insertions(+), 1 deletion(-)
>  create mode 100644 fdtoverlaymerge.c

How is this tested?

Regards,
Simon

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[Wasim Nazir]

On Wed, May 21, 2025 at 02:23:29PM +1000, David Gibson wrote:
> On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > 
> > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > used offline on a build machine to combine two or more overlay blobs into one.
> > 
> > It is intended to help maintain device-tree overlay code in
> > multiple source repositories, but merge their binary forms (overlay blobs)
> > into one so that bootloader's task of searching for all relevant overlay blobs
> > is simplified.
> > 
> > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > Subsequent patches will introduce required changes to merge overlay blobs.
> > 
> > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > ---
> >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> >  libfdt/libfdt.h      | 18 ++++++++++++++
> >  2 files changed, 77 insertions(+)
> > 
> > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > index e6b9eb643958..8690ed55c8f6 100644
> > --- a/libfdt/fdt_overlay.c
> > +++ b/libfdt/fdt_overlay.c
> > @@ -1098,3 +1098,62 @@ err:
> > 
> >  	return ret;
> >  }
> > +
> > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> 
> The parameters should be renamed to reflect the new semantics.

Sure will change to proper name.

> 
> The 'fdto_nospace' parameter seems weird.  Why not just return
> -FDT_ERR_NOSPACE from the function?

fdto_nospace variable is used to know cases when 2nd fdto (overlaying fdto)
needs more space while -FDT_ERR_NOSPACE is used for cases when
1st fdto (base fdto) needs more space.

I will change the arguments for better clarity.

> 
> > +{
> > +	uint32_t delta = fdt_get_max_phandle(fdt);
> > +	int ret;
> > +
> > +	FDT_RO_PROBE(fdt);
> > +	FDT_RO_PROBE(fdto);
> > +
> > +	*fdto_nospace = 0;
> > +
> > +	ret = overlay_adjust_local_phandles(fdto, delta);
> > +	if (ret) {
> > +		if (ret == -FDT_ERR_NOSPACE)
> > +			*fdto_nospace = 1;
> > +		goto err;
> > +	}
> > +
> > +	ret = overlay_update_local_references(fdto, delta);
> > +	if (ret) {
> > +		if (ret == -FDT_ERR_NOSPACE)
> > +			*fdto_nospace = 1;
> > +		goto err;
> > +	}
> > +
> > +	ret = overlay_fixup_phandles(fdt, fdto);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = overlay_merge(fdt, fdto);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = overlay_symbol_update(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
> 
> This comment is no longer accurate in the new context.

New context here is two overlay files, while earlier it was dts and dtbo?
I'm concerned that I may not have understood correctly, can you provide
more details.

> 
> > +	 * magic.
> > +	 */
> > +	if (!*fdto_nospace)
> > +		fdt_set_magic(fdt, ~0);
> > +
> > +	return ret;
> > +}
> > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > index b5e72001d115..664b141d5334 100644
> > --- a/libfdt/libfdt.h
> > +++ b/libfdt/libfdt.h
> > @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
> >  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
> >  			      int fragment_offset, char const **pathp);
> > 
> > +/**
> > + * fdt_overlay_merge - Merge two overlays into one
> > + * @fdt: pointer to the first device tree overlay blob
> > + * @fdto: pointer to the second device tree overlay blob
> > + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> > + *
> > + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> > + *
> > + * Expect the first 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 first device tree blob
> > + *	-FDT_ERR_BADVALUE
> > + */
> > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> > +
> >  /**********************************************************************/
> >  /* Debugging / informational functions                                */
> >  /**********************************************************************/
> 
> -- 
> David Gibson (he or they)	| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
> 				| around.
> http://www.ozlabs.org/~dgibson

Regards,
Wasim

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Wasim Nazir]

On Wed, May 21, 2025 at 02:20:55PM +1000, David Gibson wrote:
> On Mon, May 19, 2025 at 02:40:39PM +0530, Wasim Nazir wrote:
> > Hello,
> > 
> > This is follow-up attempt for fdtoverlaymerge tool.
> > 
> > Currently all the device-tree (DT) code for a given soc is maintained in a
> > common kernel repository. For example, this common DT code will have code for
> > audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> > split into a base (soc-common) code and board specific code, with the soc code
> > being compiled as soc.dtb and board specific code being compiled as respective
> > overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> > of a given SOC while boardX.dtbo represents configuration of a board/platform
> > designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> 
> So.. *build time* separation of the SoC and board pieces makes sense
> to me, which is I think how this convention arose.  *Boot time*
> separation of the SoC and board seems kind of pointless.  Almost by
> definition of what a "board" is, you must know early in boot which
> board it is.  Still, I guess the convention is established, even if
> it's stupid.

Android Treble requires separation of soc and board DT bits and so we
need to have soc.dtb & board.dtbo in separate images.

In the above example I tried to simplify using 1 soc & multiple
board-variants but we have setup with different combination of socX + boardY,
where X & Y can vary.
So we compile socX & boardY separately and combine socX dtb's in one image
while boardY dtbo's in another image.
Now at run-time based on SKU/HW config, we select particular socX and boardY
to boot the system.

In our workspace each repository i.e kernel & tech-packs (viz. audio, video etc.)
are independent (building in its own workspace) and can create dtb & dtbo.
Kernel can have socX.dtb & boardY.dtbo; Tech-packs can have socX-featureZ.dtbo
and boardY-featureZ.dtbo, Z can vary.

At build-time, we parse sku-id mentioned in all dtb & dtbo and combine matching
files i.e we overlay socX-featureZ.dtbo to socX.dtb & similarly
merge (fdtoverlaymerge) boardY-featureZ.dtbo to boardY.dtbo.

We can create single dtb as socX-boardY.dtb but Android Treble doesn't
allow that. Moreover, this modularity also helps us to reduce dtb + dtbo
image size by choosing N combinations with X+Y files instead of having
X*Y files which can increase image size.
If we don't merge boardY-featureZ.dtbo to boardY.dtbo then we need to
overlay Z number of boardY-featureZ.dtbo at run-time and it increases
boot-time.

> 
> > target (besides improving the overall size of DT blobs flashed on target, Android
> > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > which is presented a unified DT blob (soc + board overlay).
> > 
> > For ease of code maintenance and better control over release management, we are
> > exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> > maintain their kernel code (including their DT code) outside a common kernel
> > repository. In our experience, this simplifies number of branches maintained in
> > core kernel repo. New/experimental features in fingerprint sensor driver for
> > example that needs to be on a separate branch will not result in unnecessary
> > branching in core kenrel repo, affecting all other drivers.
> > 
> > In addition to compiling DT code outside core kernel tree, we also want to merge
> > the blobs back to respective blobs found in kernel build tree at buildtime
> > (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> > overlay impacts boot-time.
> 
> It's again unclear to me why you need a boot time separation of these
> devices rather than merely boot time.  What does using separate .dtbo
> files give you that just /include/ing multiple pieces into a single
> .dtbo at build time would not?
> 

Since our workspace is split into multiple independent repositories we cannot
include the pieces in one place.

> > This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> > boardX.dtbo), which currently doesn't seem to be supported and which this patch
> > series aims to support.
> 
> Merging overlays is a logically sensible operation, but it's not clear
> to me why the need for it follows from the premises above.  It's also
> unclear why you need to compile to .dtbo *then* merge, rather than
> combine .dts files then compile into a single .dtbo.

Due to splitted repository structure we cannot combine all .dts together.
And due to standalone build system for kernel & tech-packs we are
creating dtbo and merging together at end.

> 
> > fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> > with a base blob. It assumes that all external symbols specified in overlay
> > blob's __fixups__ section are found in base blob's __symbols__ section and
> > aborts on the first instance where a symbol could not be found in base blob.
> > This is mostly fine as the primary use of overlay is on a target for its
> > bootloader to merge various overlay blobs based on h/w configuration detected.
> > But when the number of overlays increased then bootloader takes lot of time to
> > apply the overlays on base DT.
> > 
> > So we need new API/tool to merge all the overlays into single overlay file
> > at host (build machine) side,
> 
> Merging into a single overlay at build time makes sense to me.  But at
> build time you'd expect to have access to the .dts files.  Why do you
> need to merge .dtbo rather than merge the .dts before compiling to
> .dtbo?  The latter should be possible already by /include/ing each of
> the individual overlays in order then compiling with dtc.

This is not possible with our current repository structure.
Moreover, this splitting of repository is needed to work independently
without slowing any teck-packs.

> 
> > so that on target side bootloader needs to only
> > apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> > number file reading/loading & minimizing repeatative overlay apply.
> > In our test setup we see an improvement of ~60% while applying merged-overlay
> > at bootloader and the merged-overlay is product of 7 overlays.
> > 
> > To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> > which takes input as overlays and gives output to merged-overlay.
> > The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> > 
> > Additional notes:
> >   If snprintf (in libc) may not available in some environments, then we will need
> >   to write our own snprintf() in libfdt.
> > 
> > ---
> > Changelog:
> > 
> > v3:
> > - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
> >   - Case1: When target is available and we merge fragments
> >   - Case2: When target is not available and we add new fragments
> > - Change the logic to update fixups & local_fixups in case of overlay merge.
> > - Few patches are squashed, reduced to 4 patches.
> > - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> > 
> > 
> > Srivatsa Vaddagiri (4):
> >   libfdt: overlay_merge: Introduce fdt_overlay_merge()
> >   libfdt: overlay_merge: Rename & copy overlay fragments and their
> >     properties
> >   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
> >   fdtoverlaymerge: A tool that merges overlays
> > 
> >  .gitignore           |   1 +
> >  Makefile             |   4 +
> >  Makefile.utils       |   6 +
> >  fdtoverlaymerge.c    | 223 +++++++++++
> >  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
> >  libfdt/fdt_rw.c      |  14 +-
> >  libfdt/libfdt.h      |  18 +
> >  libfdt/version.lds   |   1 +
> >  meson.build          |   2 +-
> >  9 files changed, 1146 insertions(+), 24 deletions(-)
> >  create mode 100644 fdtoverlaymerge.c
> > 
> > 
> > base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> 
> -- 
> David Gibson (he or they)	| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
> 				| around.
> http://www.ozlabs.org/~dgibson

Regards,
Wasim

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[Wasim Nazir]

On Mon, May 19, 2025 at 10:17:41AM -0700, Trilok Soni wrote:
> On 5/19/2025 2:10 AM, Wasim Nazir wrote:
> > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > 
> > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > used offline on a build machine to combine two or more overlay blobs into one.
> > 
> > It is intended to help maintain device-tree overlay code in
> > multiple source repositories, but merge their binary forms (overlay blobs)
> > into one so that bootloader's task of searching for all relevant overlay blobs
> > is simplified.
> > 
> > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > Subsequent patches will introduce required changes to merge overlay blobs.
> > 
> > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> 
> It will be nice to switch to oss.qualcomm.com, since we will soon mandating it. 

Sure, will apply for this.

> 
> > ---
> >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> >  libfdt/libfdt.h      | 18 ++++++++++++++
> >  2 files changed, 77 insertions(+)
> > 
> > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > index e6b9eb643958..8690ed55c8f6 100644
> > --- a/libfdt/fdt_overlay.c
> > +++ b/libfdt/fdt_overlay.c
> > @@ -1098,3 +1098,62 @@ err:
> > 
> >  	return ret;
> >  }
> > +
> > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> > +{
> > +	uint32_t delta = fdt_get_max_phandle(fdt);
> > +	int ret;
> > +
> > +	FDT_RO_PROBE(fdt);
> > +	FDT_RO_PROBE(fdto);
> > +
> > +	*fdto_nospace = 0;
> > +
> > +	ret = overlay_adjust_local_phandles(fdto, delta);
> > +	if (ret) {
> > +		if (ret == -FDT_ERR_NOSPACE)
> > +			*fdto_nospace = 1;
> > +		goto err;
> > +	}
> > +
> > +	ret = overlay_update_local_references(fdto, delta);
> > +	if (ret) {
> > +		if (ret == -FDT_ERR_NOSPACE)
> > +			*fdto_nospace = 1;
> > +		goto err;
> > +	}
> > +
> > +	ret = overlay_fixup_phandles(fdt, fdto);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = overlay_merge(fdt, fdto);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = overlay_symbol_update(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.
> > +	 */
> > +	if (!*fdto_nospace)
> > +		fdt_set_magic(fdt, ~0);
> > +
> > +	return ret;
> > +}
> > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > index b5e72001d115..664b141d5334 100644
> > --- a/libfdt/libfdt.h
> > +++ b/libfdt/libfdt.h
> > @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
> >  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
> >  			      int fragment_offset, char const **pathp);
> > 
> > +/**
> > + * fdt_overlay_merge - Merge two overlays into one
> > + * @fdt: pointer to the first device tree overlay blob
> > + * @fdto: pointer to the second device tree overlay blob
> > + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> > + *
> > + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> > + *
> > + * Expect the first 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 first device tree blob
> > + *	-FDT_ERR_BADVALUE
> > + */
> > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> > +
> >  /**********************************************************************/
> >  /* Debugging / informational functions                                */
> >  /**********************************************************************/
> > --
> > 2.49.0
> > 
> 

Regards,
Wasim

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Wasim Nazir]

On Mon, May 19, 2025 at 10:15:55AM -0700, Trilok Soni wrote:
> On 5/19/2025 2:10 AM, Wasim Nazir wrote:
> > Hello,
> > 
> > This is follow-up attempt for fdtoverlaymerge tool.
> 
> Please add detail of the first attempt, since we are submitting it after a long time. 
> 
> > 
> > Currently all the device-tree (DT) code for a given soc is maintained in a
> > common kernel repository. For example, this common DT code will have code for
> > audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> > split into a base (soc-common) code and board specific code, with the soc code
> > being compiled as soc.dtb and board specific code being compiled as respective
> > overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> > of a given SOC while boardX.dtbo represents configuration of a board/platform
> > designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> > target (besides improving the overall size of DT blobs flashed on target, Android
> > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > which is presented a unified DT blob (soc + board overlay).
> 
> Vatsa wrote it as per the Qualcomm source structure in the Android at that time. 
> You can see that he is referring "common" kernel repository etc; You need
> to rewrite it or explicitly mention it per Qualcomm directory structure
> in Android or QCLinux plus the tech-packages / external driver modules we have. 
> 
> You need to translate the problem which others outside Qualcomm can understand
> and relate in-order to understand these patches. 
> 

Sure, will modify the source-structure add provide more details.

> 
> > 
> > For ease of code maintenance and better control over release management, we are
> > exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> 
> "tech teams" is a internal usage within Qualcomm and it will be hard to understand
> some times outside QC. Please rewrite. 

Sure.

> 
> > maintain their kernel code (including their DT code) outside a common kernel
> > repository. In our experience, this simplifies number of branches maintained in
> > core kernel repo. New/experimental features in fingerprint sensor driver for
> 
> "kernel repo" is also bit QC specific. I will stop reviewing this patch
> since you should consider rewriting this summary. 

Will send updated summary.

> 
> > example that needs to be on a separate branch will not result in unnecessary
> > branching in core kenrel repo, affecting all other drivers.
> > 
> > In addition to compiling DT code outside core kernel tree, we also want to merge
> > the blobs back to respective blobs found in kernel build tree at buildtime
> > (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> > overlay impacts boot-time.
> > 
> > This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> > boardX.dtbo), which currently doesn't seem to be supported and which this patch
> > series aims to support.
> > 
> > fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> > with a base blob. It assumes that all external symbols specified in overlay
> > blob's __fixups__ section are found in base blob's __symbols__ section and
> > aborts on the first instance where a symbol could not be found in base blob.
> > This is mostly fine as the primary use of overlay is on a target for its
> > bootloader to merge various overlay blobs based on h/w configuration detected.
> > But when the number of overlays increased then bootloader takes lot of time to
> > apply the overlays on base DT.
> > 
> > So we need new API/tool to merge all the overlays into single overlay file
> > at host (build machine) side, so that on target side bootloader needs to only
> > apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> > number file reading/loading & minimizing repeatative overlay apply.
> > In our test setup we see an improvement of ~60% while applying merged-overlay
> > at bootloader and the merged-overlay is product of 7 overlays.
> > 
> > To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> > which takes input as overlays and gives output to merged-overlay.
> > The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> > 
> > Additional notes:
> >   If snprintf (in libc) may not available in some environments, then we will need
> >   to write our own snprintf() in libfdt.
> > 
> > ---
> > Changelog:
> > 
> > v3:
> > - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
> >   - Case1: When target is available and we merge fragments
> >   - Case2: When target is not available and we add new fragments
> > - Change the logic to update fixups & local_fixups in case of overlay merge.
> > - Few patches are squashed, reduced to 4 patches.
> > - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> > 
> > 
> > Srivatsa Vaddagiri (4):
> >   libfdt: overlay_merge: Introduce fdt_overlay_merge()
> >   libfdt: overlay_merge: Rename & copy overlay fragments and their
> >     properties
> >   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
> >   fdtoverlaymerge: A tool that merges overlays
> > 
> >  .gitignore           |   1 +
> >  Makefile             |   4 +
> >  Makefile.utils       |   6 +
> >  fdtoverlaymerge.c    | 223 +++++++++++
> >  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
> >  libfdt/fdt_rw.c      |  14 +-
> >  libfdt/libfdt.h      |  18 +
> >  libfdt/version.lds   |   1 +
> >  meson.build          |   2 +-
> >  9 files changed, 1146 insertions(+), 24 deletions(-)
> >  create mode 100644 fdtoverlaymerge.c
> > 
> > 
> > base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> > --
> > 2.49.0
> > 
> 

Regards,
Wasim

Re: [PATCH v3 4/4] fdtoverlaymerge: A tool that merges overlays

[Wasim Nazir]

On Fri, May 23, 2025 at 02:44:15PM +0100, Simon Glass wrote:
> Hi Wasim,
> 
> On Mon, 19 May 2025 at 11:16, Wasim Nazir <quic_wasimn@quicinc.com> wrote:
> >
> > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> >
> > Add a new command-line tool, fdtoverlaymerge, that merges two or more
> > overlay blobs using the fdt_overlay_merge() API. This tool is necessary
> > to simplify the process of combining multiple overlays into a single blob.
> 
> Normally this would be done in the bootloader, with the overlays in a
> FIT. What environment does this tool run in?

This tool is running in Android build system where we need to have
separate images for dtb and dtbo. Moreover, the dtbo is getting
build from multiple independent workspace.
I have provided more details here [1], hope it helps to clarify further
doubts.

[1] https://lore.kernel.org/all/aDnCeF88D6BdCzss@hu-wasimn-hyd.qualcomm.com/

> 
> >
> > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > ---
> >  .gitignore         |   1 +
> >  Makefile           |   4 +
> >  Makefile.utils     |   6 ++
> >  fdtoverlaymerge.c  | 223 +++++++++++++++++++++++++++++++++++++++++++++
> >  libfdt/version.lds |   1 +
> >  meson.build        |   2 +-
> >  6 files changed, 236 insertions(+), 1 deletion(-)
> >  create mode 100644 fdtoverlaymerge.c
> 
> How is this tested?

This is tested with our internal DT framework which comprises of
multiple soc & board level dtb & dtbo.
We comapre the output/final dt blob with & without this patch for
validations.

> 
> Regards,
> Simon

Regards,
Wasim

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[David Gibson]

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

On Fri, May 30, 2025 at 08:06:40PM +0530, Wasim Nazir wrote:
> On Wed, May 21, 2025 at 02:20:55PM +1000, David Gibson wrote:
> > On Mon, May 19, 2025 at 02:40:39PM +0530, Wasim Nazir wrote:
> > > Hello,
> > > 
> > > This is follow-up attempt for fdtoverlaymerge tool.
> > > 
> > > Currently all the device-tree (DT) code for a given soc is maintained in a
> > > common kernel repository. For example, this common DT code will have code for
> > > audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> > > split into a base (soc-common) code and board specific code, with the soc code
> > > being compiled as soc.dtb and board specific code being compiled as respective
> > > overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> > > of a given SOC while boardX.dtbo represents configuration of a board/platform
> > > designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> > 
> > So.. *build time* separation of the SoC and board pieces makes sense
> > to me, which is I think how this convention arose.  *Boot time*
> > separation of the SoC and board seems kind of pointless.  Almost by
> > definition of what a "board" is, you must know early in boot which
> > board it is.  Still, I guess the convention is established, even if
> > it's stupid.
> 
> Android Treble requires separation of soc and board DT bits and so we
> need to have soc.dtb & board.dtbo in separate images.
> 
> In the above example I tried to simplify using 1 soc & multiple
> board-variants but we have setup with different combination of socX + boardY,
> where X & Y can vary.
> So we compile socX & boardY separately and combine socX dtb's in one image
> while boardY dtbo's in another image.
> Now at run-time based on SKU/HW config, we select particular socX and boardY
> to boot the system.

Ok, sure.  Still seems like a silly approach to me, but it sounds like
that's not within your control.

> In our workspace each repository i.e kernel & tech-packs (viz. audio, video etc.)
> are independent (building in its own workspace) and can create dtb & dtbo.
> Kernel can have socX.dtb & boardY.dtbo; Tech-packs can have socX-featureZ.dtbo
> and boardY-featureZ.dtbo, Z can vary.

I mean.. how you organise your repositories should serve the needs of
the problem, not the other way around.  But I guess that's equally
true of dtc and associated tools.

> At build-time, we parse sku-id mentioned in all dtb & dtbo and combine matching
> files i.e we overlay socX-featureZ.dtbo to socX.dtb & similarly
> merge (fdtoverlaymerge) boardY-featureZ.dtbo to boardY.dtbo.
> 
> We can create single dtb as socX-boardY.dtb but Android Treble doesn't
> allow that. Moreover, this modularity also helps us to reduce dtb + dtbo
> image size by choosing N combinations with X+Y files instead of having
> X*Y files which can increase image size.
> If we don't merge boardY-featureZ.dtbo to boardY.dtbo then we need to
> overlay Z number of boardY-featureZ.dtbo at run-time and it increases
> boot-time.

So, you clearly have a late-build stage where you combine the various
dtbos down to just two (soc & board).  Why can't the output from the
earlier (single repo) build stages be a dts instead of a dtbo, then
you use dtc to combine those into the two dtbos you need at the late
build stage?

> > > target (besides improving the overall size of DT blobs flashed on target, Android
> > > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > > which is presented a unified DT blob (soc + board overlay).
> > > 
> > > For ease of code maintenance and better control over release management, we are
> > > exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> > > maintain their kernel code (including their DT code) outside a common kernel
> > > repository. In our experience, this simplifies number of branches maintained in
> > > core kernel repo. New/experimental features in fingerprint sensor driver for
> > > example that needs to be on a separate branch will not result in unnecessary
> > > branching in core kenrel repo, affecting all other drivers.
> > > 
> > > In addition to compiling DT code outside core kernel tree, we also want to merge
> > > the blobs back to respective blobs found in kernel build tree at buildtime
> > > (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> > > overlay impacts boot-time.
> > 
> > It's again unclear to me why you need a boot time separation of these
> > devices rather than merely boot time.  What does using separate .dtbo
> > files give you that just /include/ing multiple pieces into a single
> > .dtbo at build time would not?
> > 
> 
> Since our workspace is split into multiple independent repositories we cannot
> include the pieces in one place.

I still don't see why not.  If you can emit dtbos from the single
repository stages, why can't you emit dts instead?

> > > This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> > > boardX.dtbo), which currently doesn't seem to be supported and which this patch
> > > series aims to support.
> > 
> > Merging overlays is a logically sensible operation, but it's not clear
> > to me why the need for it follows from the premises above.  It's also
> > unclear why you need to compile to .dtbo *then* merge, rather than
> > combine .dts files then compile into a single .dtbo.
> 
> Due to splitted repository structure we cannot combine all .dts together.
> And due to standalone build system for kernel & tech-packs we are
> creating dtbo and merging together at end.
> 
> > > fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> > > with a base blob. It assumes that all external symbols specified in overlay
> > > blob's __fixups__ section are found in base blob's __symbols__ section and
> > > aborts on the first instance where a symbol could not be found in base blob.
> > > This is mostly fine as the primary use of overlay is on a target for its
> > > bootloader to merge various overlay blobs based on h/w configuration detected.
> > > But when the number of overlays increased then bootloader takes lot of time to
> > > apply the overlays on base DT.
> > > 
> > > So we need new API/tool to merge all the overlays into single overlay file
> > > at host (build machine) side,
> > 
> > Merging into a single overlay at build time makes sense to me.  But at
> > build time you'd expect to have access to the .dts files.  Why do you
> > need to merge .dtbo rather than merge the .dts before compiling to
> > .dtbo?  The latter should be possible already by /include/ing each of
> > the individual overlays in order then compiling with dtc.
> 
> This is not possible with our current repository structure.
> Moreover, this splitting of repository is needed to work independently
> without slowing any teck-packs.

I really don't know what you mean by that.

A few other things bother me about the situation, but maybe I'm
misunderstanding.

1) You imply you need many various of the soc.dtb as well as the
board.dtbo.  How does that come to be the case?  Isn't there a fixed
set of SoCs with known features?  Remember that device trees should -
as much as is possible - describe just the hardware, not how it's to
be configured or used.

2) To a certain extent the same concern applies to boards.  What's
controlling when the extra features are needed?  Are extre pieces
physically connected on?  Is it controlled by on-board switches?
Something else?

3) What exactly is costing the additional time when applying may
.dtbos at boot time.  Combining many together at build time will
obviously result in a larger dtbo with more fragments that will itself
take longer to apply.  I can certainly believe it's still faster
overall, but it's not obvious to me why,  Understanding that will
allow us all to reason better about what's a good approach here.

> > > so that on target side bootloader needs to only
> > > apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> > > number file reading/loading & minimizing repeatative overlay apply.
> > > In our test setup we see an improvement of ~60% while applying merged-overlay
> > > at bootloader and the merged-overlay is product of 7 overlays.
> > > 
> > > To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> > > which takes input as overlays and gives output to merged-overlay.
> > > The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> > > 
> > > Additional notes:
> > >   If snprintf (in libc) may not available in some environments, then we will need
> > >   to write our own snprintf() in libfdt.
> > > 
> > > ---
> > > Changelog:
> > > 
> > > v3:
> > > - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
> > >   - Case1: When target is available and we merge fragments
> > >   - Case2: When target is not available and we add new fragments
> > > - Change the logic to update fixups & local_fixups in case of overlay merge.
> > > - Few patches are squashed, reduced to 4 patches.
> > > - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> > > 
> > > 
> > > Srivatsa Vaddagiri (4):
> > >   libfdt: overlay_merge: Introduce fdt_overlay_merge()
> > >   libfdt: overlay_merge: Rename & copy overlay fragments and their
> > >     properties
> > >   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
> > >   fdtoverlaymerge: A tool that merges overlays
> > > 
> > >  .gitignore           |   1 +
> > >  Makefile             |   4 +
> > >  Makefile.utils       |   6 +
> > >  fdtoverlaymerge.c    | 223 +++++++++++
> > >  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
> > >  libfdt/fdt_rw.c      |  14 +-
> > >  libfdt/libfdt.h      |  18 +
> > >  libfdt/version.lds   |   1 +
> > >  meson.build          |   2 +-
> > >  9 files changed, 1146 insertions(+), 24 deletions(-)
> > >  create mode 100644 fdtoverlaymerge.c
> > > 
> > > 
> > > base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> > 
> 
> Regards,
> Wasim
> 

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[David Gibson]

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

On Fri, May 30, 2025 at 05:58:58PM +0530, Wasim Nazir wrote:
> On Wed, May 21, 2025 at 02:23:29PM +1000, David Gibson wrote:
> > On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> > > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > 
> > > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > > used offline on a build machine to combine two or more overlay blobs into one.
> > > 
> > > It is intended to help maintain device-tree overlay code in
> > > multiple source repositories, but merge their binary forms (overlay blobs)
> > > into one so that bootloader's task of searching for all relevant overlay blobs
> > > is simplified.
> > > 
> > > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > > Subsequent patches will introduce required changes to merge overlay blobs.
> > > 
> > > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > > ---
> > >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> > >  libfdt/libfdt.h      | 18 ++++++++++++++
> > >  2 files changed, 77 insertions(+)
> > > 
> > > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > > index e6b9eb643958..8690ed55c8f6 100644
> > > --- a/libfdt/fdt_overlay.c
> > > +++ b/libfdt/fdt_overlay.c
> > > @@ -1098,3 +1098,62 @@ err:
> > > 
> > >  	return ret;
> > >  }
> > > +
> > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> > 
> > The parameters should be renamed to reflect the new semantics.
> 
> Sure will change to proper name.
> 
> > 
> > The 'fdto_nospace' parameter seems weird.  Why not just return
> > -FDT_ERR_NOSPACE from the function?
> 
> fdto_nospace variable is used to know cases when 2nd fdto (overlaying fdto)
> needs more space while -FDT_ERR_NOSPACE is used for cases when
> 1st fdto (base fdto) needs more space.

Ah, I see.  I can see the reason now, but it's still a deeply ugly
interface.

Hmm... why do you need to ever expand the applied size?  It's obvious
you'd need to expand the based dtbo, but not why you'd ever need to
expand the one you're applying on top.

> I will change the arguments for better clarity.
> 
> > 
> > > +{
> > > +	uint32_t delta = fdt_get_max_phandle(fdt);
> > > +	int ret;
> > > +
> > > +	FDT_RO_PROBE(fdt);
> > > +	FDT_RO_PROBE(fdto);
> > > +
> > > +	*fdto_nospace = 0;
> > > +
> > > +	ret = overlay_adjust_local_phandles(fdto, delta);
> > > +	if (ret) {
> > > +		if (ret == -FDT_ERR_NOSPACE)
> > > +			*fdto_nospace = 1;
> > > +		goto err;
> > > +	}
> > > +
> > > +	ret = overlay_update_local_references(fdto, delta);
> > > +	if (ret) {
> > > +		if (ret == -FDT_ERR_NOSPACE)
> > > +			*fdto_nospace = 1;
> > > +		goto err;
> > > +	}
> > > +
> > > +	ret = overlay_fixup_phandles(fdt, fdto);
> > > +	if (ret)
> > > +		goto err;
> > > +
> > > +	ret = overlay_merge(fdt, fdto);
> > > +	if (ret)
> > > +		goto err;
> > > +
> > > +	ret = overlay_symbol_update(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
> > 
> > This comment is no longer accurate in the new context.
> 
> New context here is two overlay files, while earlier it was dts and dtbo?
> I'm concerned that I may not have understood correctly, can you provide
> more details.

No, I think you have it.  The comment references a "base device tree",
i.e. not a dtbo, but that's no longer accurate.

> > > +	 * magic.
> > > +	 */
> > > +	if (!*fdto_nospace)
> > > +		fdt_set_magic(fdt, ~0);
> > > +
> > > +	return ret;
> > > +}
> > > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > > index b5e72001d115..664b141d5334 100644
> > > --- a/libfdt/libfdt.h
> > > +++ b/libfdt/libfdt.h
> > > @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
> > >  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
> > >  			      int fragment_offset, char const **pathp);
> > > 
> > > +/**
> > > + * fdt_overlay_merge - Merge two overlays into one
> > > + * @fdt: pointer to the first device tree overlay blob
> > > + * @fdto: pointer to the second device tree overlay blob
> > > + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> > > + *
> > > + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> > > + *
> > > + * Expect the first 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 first device tree blob
> > > + *	-FDT_ERR_BADVALUE
> > > + */
> > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> > > +
> > >  /**********************************************************************/
> > >  /* Debugging / informational functions                                */
> > >  /**********************************************************************/
> > 
> 
> Regards,
> Wasim
> 

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Wasim Nazir]

On Tue, Jun 03, 2025 at 09:05:19PM +1000, David Gibson wrote:
> On Fri, May 30, 2025 at 08:06:40PM +0530, Wasim Nazir wrote:
> > On Wed, May 21, 2025 at 02:20:55PM +1000, David Gibson wrote:
> > > On Mon, May 19, 2025 at 02:40:39PM +0530, Wasim Nazir wrote:
> > > > Hello,
> > > > 
> > > > This is follow-up attempt for fdtoverlaymerge tool.
> > > > 
> > > > Currently all the device-tree (DT) code for a given soc is maintained in a
> > > > common kernel repository. For example, this common DT code will have code for
> > > > audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> > > > split into a base (soc-common) code and board specific code, with the soc code
> > > > being compiled as soc.dtb and board specific code being compiled as respective
> > > > overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> > > > of a given SOC while boardX.dtbo represents configuration of a board/platform
> > > > designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> > > 
> > > So.. *build time* separation of the SoC and board pieces makes sense
> > > to me, which is I think how this convention arose.  *Boot time*
> > > separation of the SoC and board seems kind of pointless.  Almost by
> > > definition of what a "board" is, you must know early in boot which
> > > board it is.  Still, I guess the convention is established, even if
> > > it's stupid.
> > 
> > Android Treble requires separation of soc and board DT bits and so we
> > need to have soc.dtb & board.dtbo in separate images.
> > 
> > In the above example I tried to simplify using 1 soc & multiple
> > board-variants but we have setup with different combination of socX + boardY,
> > where X & Y can vary.
> > So we compile socX & boardY separately and combine socX dtb's in one image
> > while boardY dtbo's in another image.
> > Now at run-time based on SKU/HW config, we select particular socX and boardY
> > to boot the system.
> 
> Ok, sure.  Still seems like a silly approach to me, but it sounds like
> that's not within your control.
> 
> > In our workspace each repository i.e kernel & tech-packs (viz. audio, video etc.)
> > are independent (building in its own workspace) and can create dtb & dtbo.
> > Kernel can have socX.dtb & boardY.dtbo; Tech-packs can have socX-featureZ.dtbo
> > and boardY-featureZ.dtbo, Z can vary.
> 
> I mean.. how you organise your repositories should serve the needs of
> the problem, not the other way around.  But I guess that's equally
> true of dtc and associated tools.
> 
> > At build-time, we parse sku-id mentioned in all dtb & dtbo and combine matching
> > files i.e we overlay socX-featureZ.dtbo to socX.dtb & similarly
> > merge (fdtoverlaymerge) boardY-featureZ.dtbo to boardY.dtbo.
> > 
> > We can create single dtb as socX-boardY.dtb but Android Treble doesn't
> > allow that. Moreover, this modularity also helps us to reduce dtb + dtbo
> > image size by choosing N combinations with X+Y files instead of having
> > X*Y files which can increase image size.
> > If we don't merge boardY-featureZ.dtbo to boardY.dtbo then we need to
> > overlay Z number of boardY-featureZ.dtbo at run-time and it increases
> > boot-time.
> 
> So, you clearly have a late-build stage where you combine the various
> dtbos down to just two (soc & board).  Why can't the output from the
> earlier (single repo) build stages be a dts instead of a dtbo, then
> you use dtc to combine those into the two dtbos you need at the late
> build stage?

Our repositores are following android structure based on android-treble
where vendor subsystems needs to be modular for ease of OTA upgrade and
development.

As a result of which each feature/techpack (viz. audio, video etc.) have
its own independent repositories where it can build its modules and
overlay DT (soc & board).
Kernel is separate and provides only the core images and base DT (soc &
board).

So, dtbo is the option to build it independently and at last we have
options to either "overlay all dtbo at boot-time" or "merge it at build-time and
overlay final dtbo at boot-time".

This build-time merge is done outside techpack build system.
But if we want to have dts from each techpack, then techpack-build system needs
to communicate somehow to know which dts to include with which base dts.
This is not possible at build-time between techpacks.

> 
> > > > target (besides improving the overall size of DT blobs flashed on target, Android
> > > > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > > > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > > > which is presented a unified DT blob (soc + board overlay).
> > > > 
> > > > For ease of code maintenance and better control over release management, we are
> > > > exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> > > > maintain their kernel code (including their DT code) outside a common kernel
> > > > repository. In our experience, this simplifies number of branches maintained in
> > > > core kernel repo. New/experimental features in fingerprint sensor driver for
> > > > example that needs to be on a separate branch will not result in unnecessary
> > > > branching in core kenrel repo, affecting all other drivers.
> > > > 
> > > > In addition to compiling DT code outside core kernel tree, we also want to merge
> > > > the blobs back to respective blobs found in kernel build tree at buildtime
> > > > (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> > > > overlay impacts boot-time.
> > > 
> > > It's again unclear to me why you need a boot time separation of these
> > > devices rather than merely boot time.  What does using separate .dtbo
> > > files give you that just /include/ing multiple pieces into a single
> > > .dtbo at build time would not?
> > > 
> > 
> > Since our workspace is split into multiple independent repositories we cannot
> > include the pieces in one place.
> 
> I still don't see why not.  If you can emit dtbos from the single
> repository stages, why can't you emit dts instead?
> 
> > > > This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> > > > boardX.dtbo), which currently doesn't seem to be supported and which this patch
> > > > series aims to support.
> > > 
> > > Merging overlays is a logically sensible operation, but it's not clear
> > > to me why the need for it follows from the premises above.  It's also
> > > unclear why you need to compile to .dtbo *then* merge, rather than
> > > combine .dts files then compile into a single .dtbo.
> > 
> > Due to splitted repository structure we cannot combine all .dts together.
> > And due to standalone build system for kernel & tech-packs we are
> > creating dtbo and merging together at end.
> > 
> > > > fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> > > > with a base blob. It assumes that all external symbols specified in overlay
> > > > blob's __fixups__ section are found in base blob's __symbols__ section and
> > > > aborts on the first instance where a symbol could not be found in base blob.
> > > > This is mostly fine as the primary use of overlay is on a target for its
> > > > bootloader to merge various overlay blobs based on h/w configuration detected.
> > > > But when the number of overlays increased then bootloader takes lot of time to
> > > > apply the overlays on base DT.
> > > > 
> > > > So we need new API/tool to merge all the overlays into single overlay file
> > > > at host (build machine) side,
> > > 
> > > Merging into a single overlay at build time makes sense to me.  But at
> > > build time you'd expect to have access to the .dts files.  Why do you
> > > need to merge .dtbo rather than merge the .dts before compiling to
> > > .dtbo?  The latter should be possible already by /include/ing each of
> > > the individual overlays in order then compiling with dtc.
> > 
> > This is not possible with our current repository structure.
> > Moreover, this splitting of repository is needed to work independently
> > without slowing any teck-packs.
> 
> I really don't know what you mean by that.
> 
> A few other things bother me about the situation, but maybe I'm
> misunderstanding.
> 
> 1) You imply you need many various of the soc.dtb as well as the
> board.dtbo.  How does that come to be the case?  Isn't there a fixed
> set of SoCs with known features?  Remember that device trees should -
> as much as is possible - describe just the hardware, not how it's to
> be configured or used.
> 
> 2) To a certain extent the same concern applies to boards.  What's
> controlling when the extra features are needed?  Are extre pieces
> physically connected on?  Is it controlled by on-board switches?
> Something else?
> 

We do have a fixed set of SoCs, but there can be multiple boards using same SoC.
socX-featureZ.dtbo & boardY-featureZ.dtbo is describing one of the
feature (viz. audio, video etc.) for each soc & board respectively.

> 3) What exactly is costing the additional time when applying may
> .dtbos at boot time.  Combining many together at build time will
> obviously result in a larger dtbo with more fragments that will itself
> take longer to apply.  I can certainly believe it's still faster
> overall, but it's not obvious to me why,  Understanding that will
> allow us all to reason better about what's a good approach here.
> 

In our setup, we have 7 overlays (boardY-featureZ.dtbo) for 1 of the board
(soc overlay (socX-featureZ.dtbo) are different so excluding that here).
So, if we do overlay-merge at build-time we are saving ~60% boot-time spent on
overlaying the board dtbo's.

> > > > so that on target side bootloader needs to only
> > > > apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> > > > number file reading/loading & minimizing repeatative overlay apply.
> > > > In our test setup we see an improvement of ~60% while applying merged-overlay
> > > > at bootloader and the merged-overlay is product of 7 overlays.
> > > > 
> > > > To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> > > > which takes input as overlays and gives output to merged-overlay.
> > > > The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> > > > 
> > > > Additional notes:
> > > >   If snprintf (in libc) may not available in some environments, then we will need
> > > >   to write our own snprintf() in libfdt.
> > > > 
> > > > ---
> > > > Changelog:
> > > > 
> > > > v3:
> > > > - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
> > > >   - Case1: When target is available and we merge fragments
> > > >   - Case2: When target is not available and we add new fragments
> > > > - Change the logic to update fixups & local_fixups in case of overlay merge.
> > > > - Few patches are squashed, reduced to 4 patches.
> > > > - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> > > > 
> > > > 
> > > > Srivatsa Vaddagiri (4):
> > > >   libfdt: overlay_merge: Introduce fdt_overlay_merge()
> > > >   libfdt: overlay_merge: Rename & copy overlay fragments and their
> > > >     properties
> > > >   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
> > > >   fdtoverlaymerge: A tool that merges overlays
> > > > 
> > > >  .gitignore           |   1 +
> > > >  Makefile             |   4 +
> > > >  Makefile.utils       |   6 +
> > > >  fdtoverlaymerge.c    | 223 +++++++++++
> > > >  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
> > > >  libfdt/fdt_rw.c      |  14 +-
> > > >  libfdt/libfdt.h      |  18 +
> > > >  libfdt/version.lds   |   1 +
> > > >  meson.build          |   2 +-
> > > >  9 files changed, 1146 insertions(+), 24 deletions(-)
> > > >  create mode 100644 fdtoverlaymerge.c
> > > > 
> > > > 
> > > > base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> > > 
> > 
> > Regards,
> > Wasim
> > 
> 
> -- 
> David Gibson (he or they)	| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
> 				| around.
> http://www.ozlabs.org/~dgibson



-- 
Regards,
Wasim

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[Wasim Nazir]

On Tue, Jun 03, 2025 at 09:09:12PM +1000, David Gibson wrote:
> On Fri, May 30, 2025 at 05:58:58PM +0530, Wasim Nazir wrote:
> > On Wed, May 21, 2025 at 02:23:29PM +1000, David Gibson wrote:
> > > On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> > > > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > 
> > > > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > > > used offline on a build machine to combine two or more overlay blobs into one.
> > > > 
> > > > It is intended to help maintain device-tree overlay code in
> > > > multiple source repositories, but merge their binary forms (overlay blobs)
> > > > into one so that bootloader's task of searching for all relevant overlay blobs
> > > > is simplified.
> > > > 
> > > > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > > > Subsequent patches will introduce required changes to merge overlay blobs.
> > > > 
> > > > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > > > ---
> > > >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  libfdt/libfdt.h      | 18 ++++++++++++++
> > > >  2 files changed, 77 insertions(+)
> > > > 
> > > > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > > > index e6b9eb643958..8690ed55c8f6 100644
> > > > --- a/libfdt/fdt_overlay.c
> > > > +++ b/libfdt/fdt_overlay.c
> > > > @@ -1098,3 +1098,62 @@ err:
> > > > 
> > > >  	return ret;
> > > >  }
> > > > +
> > > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> > > 
> > > The parameters should be renamed to reflect the new semantics.
> > 
> > Sure will change to proper name.
> > 
> > > 
> > > The 'fdto_nospace' parameter seems weird.  Why not just return
> > > -FDT_ERR_NOSPACE from the function?
> > 
> > fdto_nospace variable is used to know cases when 2nd fdto (overlaying fdto)
> > needs more space while -FDT_ERR_NOSPACE is used for cases when
> > 1st fdto (base fdto) needs more space.
> 
> Ah, I see.  I can see the reason now, but it's still a deeply ugly
> interface.
> 
> Hmm... why do you need to ever expand the applied size?  It's obvious
> you'd need to expand the based dtbo, but not why you'd ever need to
> expand the one you're applying on top.

Since, we are updating phandle references of fragments & __local_fixups__ of
2nd dtbo so we need to check that.

> 
> > I will change the arguments for better clarity.
> > 
> > > 
> > > > +{
> > > > +	uint32_t delta = fdt_get_max_phandle(fdt);
> > > > +	int ret;
> > > > +
> > > > +	FDT_RO_PROBE(fdt);
> > > > +	FDT_RO_PROBE(fdto);
> > > > +
> > > > +	*fdto_nospace = 0;
> > > > +
> > > > +	ret = overlay_adjust_local_phandles(fdto, delta);
> > > > +	if (ret) {
> > > > +		if (ret == -FDT_ERR_NOSPACE)
> > > > +			*fdto_nospace = 1;
> > > > +		goto err;
> > > > +	}
> > > > +
> > > > +	ret = overlay_update_local_references(fdto, delta);
> > > > +	if (ret) {
> > > > +		if (ret == -FDT_ERR_NOSPACE)
> > > > +			*fdto_nospace = 1;
> > > > +		goto err;
> > > > +	}
> > > > +
> > > > +	ret = overlay_fixup_phandles(fdt, fdto);
> > > > +	if (ret)
> > > > +		goto err;
> > > > +
> > > > +	ret = overlay_merge(fdt, fdto);
> > > > +	if (ret)
> > > > +		goto err;
> > > > +
> > > > +	ret = overlay_symbol_update(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
> > > 
> > > This comment is no longer accurate in the new context.
> > 
> > New context here is two overlay files, while earlier it was dts and dtbo?
> > I'm concerned that I may not have understood correctly, can you provide
> > more details.
> 
> No, I think you have it.  The comment references a "base device tree",
> i.e. not a dtbo, but that's no longer accurate.

Oh ok, got it.

> 
> > > > +	 * magic.
> > > > +	 */
> > > > +	if (!*fdto_nospace)
> > > > +		fdt_set_magic(fdt, ~0);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > > > index b5e72001d115..664b141d5334 100644
> > > > --- a/libfdt/libfdt.h
> > > > +++ b/libfdt/libfdt.h
> > > > @@ -2313,6 +2313,24 @@ int fdt_overlay_apply(void *fdt, void *fdto);
> > > >  int fdt_overlay_target_offset(const void *fdt, const void *fdto,
> > > >  			      int fragment_offset, char const **pathp);
> > > > 
> > > > +/**
> > > > + * fdt_overlay_merge - Merge two overlays into one
> > > > + * @fdt: pointer to the first device tree overlay blob
> > > > + * @fdto: pointer to the second device tree overlay blob
> > > > + * @fdto_nospace: indicates if FDT_ERR_NOSPACE error code applies to @fdto
> > > > + *
> > > > + * fdt_overlay_merge() will merge second overlay blob into first overlay blob.
> > > > + *
> > > > + * Expect the first 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 first device tree blob
> > > > + *	-FDT_ERR_BADVALUE
> > > > + */
> > > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace);
> > > > +
> > > >  /**********************************************************************/
> > > >  /* Debugging / informational functions                                */
> > > >  /**********************************************************************/
> > > 
> > 
> > Regards,
> > Wasim
> > 
> 
> -- 
> David Gibson (he or they)	| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
> 				| around.
> http://www.ozlabs.org/~dgibson



-- 
Regards,
Wasim

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[David Gibson]

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

On Fri, Jun 27, 2025 at 03:45:36PM +0530, Wasim Nazir wrote:
> On Tue, Jun 03, 2025 at 09:05:19PM +1000, David Gibson wrote:
> > On Fri, May 30, 2025 at 08:06:40PM +0530, Wasim Nazir wrote:
> > > On Wed, May 21, 2025 at 02:20:55PM +1000, David Gibson wrote:
> > > > On Mon, May 19, 2025 at 02:40:39PM +0530, Wasim Nazir wrote:
> > > > > Hello,
> > > > > 
> > > > > This is follow-up attempt for fdtoverlaymerge tool.
> > > > > 
> > > > > Currently all the device-tree (DT) code for a given soc is maintained in a
> > > > > common kernel repository. For example, this common DT code will have code for
> > > > > audio, video, fingerprint, bluetooth etc. Further this, DT code is typically
> > > > > split into a base (soc-common) code and board specific code, with the soc code
> > > > > being compiled as soc.dtb and board specific code being compiled as respective
> > > > > overlay blobs (board1.dtbo, board2.dtbo etc). soc.dtb represents hardware configuration
> > > > > of a given SOC while boardX.dtbo represents configuration of a board/platform
> > > > > designed using that soc.soc.dtb and boardX.dtbo files are flashed separately on
> > > > 
> > > > So.. *build time* separation of the SoC and board pieces makes sense
> > > > to me, which is I think how this convention arose.  *Boot time*
> > > > separation of the SoC and board seems kind of pointless.  Almost by
> > > > definition of what a "board" is, you must know early in boot which
> > > > board it is.  Still, I guess the convention is established, even if
> > > > it's stupid.
> > > 
> > > Android Treble requires separation of soc and board DT bits and so we
> > > need to have soc.dtb & board.dtbo in separate images.
> > > 
> > > In the above example I tried to simplify using 1 soc & multiple
> > > board-variants but we have setup with different combination of socX + boardY,
> > > where X & Y can vary.
> > > So we compile socX & boardY separately and combine socX dtb's in one image
> > > while boardY dtbo's in another image.
> > > Now at run-time based on SKU/HW config, we select particular socX and boardY
> > > to boot the system.
> > 
> > Ok, sure.  Still seems like a silly approach to me, but it sounds like
> > that's not within your control.
> > 
> > > In our workspace each repository i.e kernel & tech-packs (viz. audio, video etc.)
> > > are independent (building in its own workspace) and can create dtb & dtbo.
> > > Kernel can have socX.dtb & boardY.dtbo; Tech-packs can have socX-featureZ.dtbo
> > > and boardY-featureZ.dtbo, Z can vary.
> > 
> > I mean.. how you organise your repositories should serve the needs of
> > the problem, not the other way around.  But I guess that's equally
> > true of dtc and associated tools.
> > 
> > > At build-time, we parse sku-id mentioned in all dtb & dtbo and combine matching
> > > files i.e we overlay socX-featureZ.dtbo to socX.dtb & similarly
> > > merge (fdtoverlaymerge) boardY-featureZ.dtbo to boardY.dtbo.
> > > 
> > > We can create single dtb as socX-boardY.dtb but Android Treble doesn't
> > > allow that. Moreover, this modularity also helps us to reduce dtb + dtbo
> > > image size by choosing N combinations with X+Y files instead of having
> > > X*Y files which can increase image size.
> > > If we don't merge boardY-featureZ.dtbo to boardY.dtbo then we need to
> > > overlay Z number of boardY-featureZ.dtbo at run-time and it increases
> > > boot-time.
> > 
> > So, you clearly have a late-build stage where you combine the various
> > dtbos down to just two (soc & board).  Why can't the output from the
> > earlier (single repo) build stages be a dts instead of a dtbo, then
> > you use dtc to combine those into the two dtbos you need at the late
> > build stage?
> 
> Our repositores are following android structure based on android-treble
> where vendor subsystems needs to be modular for ease of OTA upgrade and
> development.
> 
> As a result of which each feature/techpack (viz. audio, video etc.) have
> its own independent repositories where it can build its modules and
> overlay DT (soc & board).
> Kernel is separate and provides only the core images and base DT (soc &
> board).

The build system should serve the needs of the project, not the other
way around.  If the feature repos can emit a dtbo, why can't they emit
a dts as well?

> So, dtbo is the option to build it independently and at last we have
> options to either "overlay all dtbo at boot-time" or "merge it at build-time and
> overlay final dtbo at boot-time".
> 
> This build-time merge is done outside techpack build system.
> But if we want to have dts from each techpack, then techpack-build system needs
> to communicate somehow to know which dts to include with which base dts.

Why is that a harder problem than knowing which dtbo?  The dtbo must
have been built from some sort of dts, no?

> This is not possible at build-time between techpacks.

The build system is not immutable - you're proposing changes to libfdt
and/or dtc; why not changes to the build system?

> > > > > target (besides improving the overall size of DT blobs flashed on target, Android
> > > > > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > > > > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > > > > which is presented a unified DT blob (soc + board overlay).
> > > > > 
> > > > > For ease of code maintenance and better control over release management, we are
> > > > > exploring allowing some of the tech teams (audio/fingerprint sensor etc) to
> > > > > maintain their kernel code (including their DT code) outside a common kernel
> > > > > repository. In our experience, this simplifies number of branches maintained in
> > > > > core kernel repo. New/experimental features in fingerprint sensor driver for
> > > > > example that needs to be on a separate branch will not result in unnecessary
> > > > > branching in core kenrel repo, affecting all other drivers.
> > > > > 
> > > > > In addition to compiling DT code outside core kernel tree, we also want to merge
> > > > > the blobs back to respective blobs found in kernel build tree at buildtime
> > > > > (soc.dtb or boardX.dtbo), as otherwise relying on bootloader to do all the
> > > > > overlay impacts boot-time.
> > > > 
> > > > It's again unclear to me why you need a boot time separation of these
> > > > devices rather than merely boot time.  What does using separate .dtbo
> > > > files give you that just /include/ing multiple pieces into a single
> > > > .dtbo at build time would not?
> > > > 
> > > 
> > > Since our workspace is split into multiple independent repositories we cannot
> > > include the pieces in one place.
> > 
> > I still don't see why not.  If you can emit dtbos from the single
> > repository stages, why can't you emit dts instead?
> > 
> > > > > This brings up the need to merge two overlay blobs (fingerprint-overlay.dtbo +
> > > > > boardX.dtbo), which currently doesn't seem to be supported and which this patch
> > > > > series aims to support.
> > > > 
> > > > Merging overlays is a logically sensible operation, but it's not clear
> > > > to me why the need for it follows from the premises above.  It's also
> > > > unclear why you need to compile to .dtbo *then* merge, rather than
> > > > combine .dts files then compile into a single .dtbo.
> > > 
> > > Due to splitted repository structure we cannot combine all .dts together.
> > > And due to standalone build system for kernel & tech-packs we are
> > > creating dtbo and merging together at end.
> > > 
> > > > > fdt_overlay_apply() API currently allows for an overlay DT blob to be merged
> > > > > with a base blob. It assumes that all external symbols specified in overlay
> > > > > blob's __fixups__ section are found in base blob's __symbols__ section and
> > > > > aborts on the first instance where a symbol could not be found in base blob.
> > > > > This is mostly fine as the primary use of overlay is on a target for its
> > > > > bootloader to merge various overlay blobs based on h/w configuration detected.
> > > > > But when the number of overlays increased then bootloader takes lot of time to
> > > > > apply the overlays on base DT.
> > > > > 
> > > > > So we need new API/tool to merge all the overlays into single overlay file
> > > > > at host (build machine) side,
> > > > 
> > > > Merging into a single overlay at build time makes sense to me.  But at
> > > > build time you'd expect to have access to the .dts files.  Why do you
> > > > need to merge .dtbo rather than merge the .dts before compiling to
> > > > .dtbo?  The latter should be possible already by /include/ing each of
> > > > the individual overlays in order then compiling with dtc.
> > > 
> > > This is not possible with our current repository structure.
> > > Moreover, this splitting of repository is needed to work independently
> > > without slowing any teck-packs.
> > 
> > I really don't know what you mean by that.
> > 
> > A few other things bother me about the situation, but maybe I'm
> > misunderstanding.
> > 
> > 1) You imply you need many various of the soc.dtb as well as the
> > board.dtbo.  How does that come to be the case?  Isn't there a fixed
> > set of SoCs with known features?  Remember that device trees should -
> > as much as is possible - describe just the hardware, not how it's to
> > be configured or used.
> > 
> > 2) To a certain extent the same concern applies to boards.  What's
> > controlling when the extra features are needed?  Are extre pieces
> > physically connected on?  Is it controlled by on-board switches?
> > Something else?
> > 
> 
> We do have a fixed set of SoCs, but there can be multiple boards using same SoC.
> socX-featureZ.dtbo & boardY-featureZ.dtbo is describing one of the
> feature (viz. audio, video etc.) for each soc & board respectively.

But why does the set of features on a SoC vary?  Generally, by its
nature, a SoC has the components it has, they're not variable.

> > 3) What exactly is costing the additional time when applying may
> > .dtbos at boot time.  Combining many together at build time will
> > obviously result in a larger dtbo with more fragments that will itself
> > take longer to apply.  I can certainly believe it's still faster
> > overall, but it's not obvious to me why,  Understanding that will
> > allow us all to reason better about what's a good approach here.
> 
> In our setup, we have 7 overlays (boardY-featureZ.dtbo) for 1 of the board
> (soc overlay (socX-featureZ.dtbo) are different so excluding that here).
> So, if we do overlay-merge at build-time we are saving ~60% boot-time spent on
> overlaying the board dtbo's.

Well, sure,  but I'm trying to understand *why* doing it all at once
is so much faster.  I know the code for overlay application, and
there's not an obvious large cost per-dtbo - I'd expect the time to be
dominated by things that are per-fragment or (roughly) proportional
to the total size of the applied dtbos.  That leaves two possibilities
that I can see:

1) the later dtbos are frequently overwriting properties in the
   earlier dtbos, resulting in a final dtbo substantially smaller than
   the sum of the original dtbos.  This seems implausible to me given
   the structure you've described - I'd expect each dtbo to be
   altering different parts of the tree.

2) There *is*, contrary to my intuition, a substantial once off cost
   for each fdt_overlay_apply() call, independent of the dtbo's size.
   If that's the case, we should understand why and see if it can be
   mitigated.  That would help all use cases, not just your one.

In short, adding a substantial new feature to dtc/libfdt seems
premature without better understanding the cause of the poor
performance you're seeing.

> > > > > so that on target side bootloader needs to only
> > > > > apply merged-overlay-dt to its base-dt. This saves lot of time due to reduced
> > > > > number file reading/loading & minimizing repeatative overlay apply.
> > > > > In our test setup we see an improvement of ~60% while applying merged-overlay
> > > > > at bootloader and the merged-overlay is product of 7 overlays.
> > > > > 
> > > > > To serve this overlay-merge feature we have introduce fdtoverlaymerge tool
> > > > > which takes input as overlays and gives output to merged-overlay.
> > > > > The tool uses fdt_overlay_merge() API introduced in libfdt to do the actual work.
> > > > > 
> > > > > Additional notes:
> > > > >   If snprintf (in libc) may not available in some environments, then we will need
> > > > >   to write our own snprintf() in libfdt.
> > > > > 
> > > > > ---
> > > > > Changelog:
> > > > > 
> > > > > v3:
> > > > > - Update copy_node & add copy_fragment_to_base to incorporate two cases i.e
> > > > >   - Case1: When target is available and we merge fragments
> > > > >   - Case2: When target is not available and we add new fragments
> > > > > - Change the logic to update fixups & local_fixups in case of overlay merge.
> > > > > - Few patches are squashed, reduced to 4 patches.
> > > > > - v2-link: https://lore.kernel.org/all/1599671882-310027-1-git-send-email-gurbaror@codeaurora.org/
> > > > > 
> > > > > 
> > > > > Srivatsa Vaddagiri (4):
> > > > >   libfdt: overlay_merge: Introduce fdt_overlay_merge()
> > > > >   libfdt: overlay_merge: Rename & copy overlay fragments and their
> > > > >     properties
> > > > >   libfdt: overlay_merge: Update phandles, symbols, fixups & local_fixups
> > > > >   fdtoverlaymerge: A tool that merges overlays
> > > > > 
> > > > >  .gitignore           |   1 +
> > > > >  Makefile             |   4 +
> > > > >  Makefile.utils       |   6 +
> > > > >  fdtoverlaymerge.c    | 223 +++++++++++
> > > > >  libfdt/fdt_overlay.c | 901 ++++++++++++++++++++++++++++++++++++++++++-
> > > > >  libfdt/fdt_rw.c      |  14 +-
> > > > >  libfdt/libfdt.h      |  18 +
> > > > >  libfdt/version.lds   |   1 +
> > > > >  meson.build          |   2 +-
> > > > >  9 files changed, 1146 insertions(+), 24 deletions(-)
> > > > >  create mode 100644 fdtoverlaymerge.c
> > > > > 
> > > > > 
> > > > > base-commit: f4c53f4ebf7809a07666bf728c823005e1f1a612
> > > > 
> > > 
> > > Regards,
> > > Wasim
> > > 
> > 
> 
> 
> 

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 1/4] libfdt: overlay_merge: Introduce fdt_overlay_merge()

[David Gibson]

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

On Fri, Jun 27, 2025 at 04:01:02PM +0530, Wasim Nazir wrote:
> On Tue, Jun 03, 2025 at 09:09:12PM +1000, David Gibson wrote:
> > On Fri, May 30, 2025 at 05:58:58PM +0530, Wasim Nazir wrote:
> > > On Wed, May 21, 2025 at 02:23:29PM +1000, David Gibson wrote:
> > > > On Mon, May 19, 2025 at 02:40:40PM +0530, Wasim Nazir wrote:
> > > > > From: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > > 
> > > > > fdt_overlay_merge() merges two overlay blobs. It is largely expected to be
> > > > > used offline on a build machine to combine two or more overlay blobs into one.
> > > > > 
> > > > > It is intended to help maintain device-tree overlay code in
> > > > > multiple source repositories, but merge their binary forms (overlay blobs)
> > > > > into one so that bootloader's task of searching for all relevant overlay blobs
> > > > > is simplified.
> > > > > 
> > > > > Introduce fdt_overlay_merge() which is identical to fdt_overlay_apply().
> > > > > Subsequent patches will introduce required changes to merge overlay blobs.
> > > > > 
> > > > > Signed-off-by: Srivatsa Vaddagiri <vatsa@codeaurora.org>
> > > > > Signed-off-by: Wasim Nazir <quic_wasimn@quicinc.com>
> > > > > ---
> > > > >  libfdt/fdt_overlay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
> > > > >  libfdt/libfdt.h      | 18 ++++++++++++++
> > > > >  2 files changed, 77 insertions(+)
> > > > > 
> > > > > diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> > > > > index e6b9eb643958..8690ed55c8f6 100644
> > > > > --- a/libfdt/fdt_overlay.c
> > > > > +++ b/libfdt/fdt_overlay.c
> > > > > @@ -1098,3 +1098,62 @@ err:
> > > > > 
> > > > >  	return ret;
> > > > >  }
> > > > > +
> > > > > +int fdt_overlay_merge(void *fdt, void *fdto, int *fdto_nospace)
> > > > 
> > > > The parameters should be renamed to reflect the new semantics.
> > > 
> > > Sure will change to proper name.
> > > 
> > > > 
> > > > The 'fdto_nospace' parameter seems weird.  Why not just return
> > > > -FDT_ERR_NOSPACE from the function?
> > > 
> > > fdto_nospace variable is used to know cases when 2nd fdto (overlaying fdto)
> > > needs more space while -FDT_ERR_NOSPACE is used for cases when
> > > 1st fdto (base fdto) needs more space.
> > 
> > Ah, I see.  I can see the reason now, but it's still a deeply ugly
> > interface.
> > 
> > Hmm... why do you need to ever expand the applied size?  It's obvious
> > you'd need to expand the based dtbo, but not why you'd ever need to
> > expand the one you're applying on top.
> 
> Since, we are updating phandle references of fragments & __local_fixups__ of
> 2nd dtbo so we need to check that.

Updating phandle references doesn't usually require expanding the
tree, since phandles have a fixed size.  Have you actually hit running
out of space for the dtb in practice, or are you just assuming you
need this because you're making modifications to the dtbo.

-- 
David Gibson (he or they)	| 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: 833 bytes --]

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Wasim Nazir]

[...]
> > 
> > Our repositores are following android structure based on android-treble
> > where vendor subsystems needs to be modular for ease of OTA upgrade and
> > development.
> > 
> > As a result of which each feature/techpack (viz. audio, video etc.) have
> > its own independent repositories where it can build its modules and
> > overlay DT (soc & board).
> > Kernel is separate and provides only the core images and base DT (soc &
> > board).
> 
> The build system should serve the needs of the project, not the other
> way around.  If the feature repos can emit a dtbo, why can't they emit
> a dts as well?
> 
> > So, dtbo is the option to build it independently and at last we have
> > options to either "overlay all dtbo at boot-time" or "merge it at build-time and
> > overlay final dtbo at boot-time".
> > 
> > This build-time merge is done outside techpack build system.
> > But if we want to have dts from each techpack, then techpack-build system needs
> > to communicate somehow to know which dts to include with which base dts.
> 
> Why is that a harder problem than knowing which dtbo?  The dtbo must
> have been built from some sort of dts, no?
> 
> > This is not possible at build-time between techpacks.
> 
> The build system is not immutable - you're proposing changes to libfdt
> and/or dtc; why not changes to the build system?
> 

Hi David,

As each techpack operates in a sandboxed environment with its own build
structure, all dependencies are managed within that sandbox, so the
techpack owner only needs to focus on their own module. Since each
techpack produces a compiled DTBO, it is self-contained and can be
merged with other DTBOs without additional coordination.

If we were to use DTS files instead, every techpack owner would need to
be involved in resolving cross-dependencies, which is not scalable in
our modular setup.

> > > > > > target (besides improving the overall size of DT blobs flashed on target, Android
> > > > > > Treble also requires separation of soc and board DT bits). Bootloader will pick
> > > > > > one of the board overlay blobs and merge it with soc.dtb, before booting kernel
> > > > > > which is presented a unified DT blob (soc + board overlay).


[...]
> > > 
> > > I really don't know what you mean by that.
> > > 
> > > A few other things bother me about the situation, but maybe I'm
> > > misunderstanding.
> > > 
> > > 1) You imply you need many various of the soc.dtb as well as the
> > > board.dtbo.  How does that come to be the case?  Isn't there a fixed
> > > set of SoCs with known features?  Remember that device trees should -
> > > as much as is possible - describe just the hardware, not how it's to
> > > be configured or used.
> > > 
> > > 2) To a certain extent the same concern applies to boards.  What's
> > > controlling when the extra features are needed?  Are extre pieces
> > > physically connected on?  Is it controlled by on-board switches?
> > > Something else?
> > > 
> > 
> > We do have a fixed set of SoCs, but there can be multiple boards using same SoC.
> > socX-featureZ.dtbo & boardY-featureZ.dtbo is describing one of the
> > feature (viz. audio, video etc.) for each soc & board respectively.
> 
> But why does the set of features on a SoC vary?  Generally, by its
> nature, a SoC has the components it has, they're not variable.
> 

SoC features are fixed, but different boards may utilize different
subsets of those features which are also fixed. To support multiple
board combinations efficiently, we select DTs feature-wise, allowing
reuse of SoC/feature overlays across boards.

> > > 3) What exactly is costing the additional time when applying may
> > > .dtbos at boot time.  Combining many together at build time will
> > > obviously result in a larger dtbo with more fragments that will itself
> > > take longer to apply.  I can certainly believe it's still faster
> > > overall, but it's not obvious to me why,  Understanding that will
> > > allow us all to reason better about what's a good approach here.
> > 
> > In our setup, we have 7 overlays (boardY-featureZ.dtbo) for 1 of the board
> > (soc overlay (socX-featureZ.dtbo) are different so excluding that here).
> > So, if we do overlay-merge at build-time we are saving ~60% boot-time spent on
> > overlaying the board dtbo's.
> 

I realized there was an error in my earlier boot-time improvement
estimate due to some unintended factors in measurement. The actual
improvement is around 30%, not 60%. Apologies for the confusion.

> Well, sure,  but I'm trying to understand *why* doing it all at once
> is so much faster.  I know the code for overlay application, and
> there's not an obvious large cost per-dtbo - I'd expect the time to be
> dominated by things that are per-fragment or (roughly) proportional
> to the total size of the applied dtbos.  That leaves two possibilities
> that I can see:
> 
> 1) the later dtbos are frequently overwriting properties in the
>    earlier dtbos, resulting in a final dtbo substantially smaller than
>    the sum of the original dtbos.  This seems implausible to me given
>    the structure you've described - I'd expect each dtbo to be
>    altering different parts of the tree.
> 
> 2) There *is*, contrary to my intuition, a substantial once off cost
>    for each fdt_overlay_apply() call, independent of the dtbo's size.
>    If that's the case, we should understand why and see if it can be
>    mitigated.  That would help all use cases, not just your one.
> 
> In short, adding a substantial new feature to dtc/libfdt seems
> premature without better understanding the cause of the poor
> performance you're seeing.

Thanks, Alex. I appreciate your thoughtful analysis and the effort to
understand the performance aspects of overlay application.

That said, I think we might be diverging from the core intent of this
patch series. The goal here isn’t to optimize or fix the current overlay
mechanism, but to introduce a new capability that’s currently missing:
the ability to merge two DTBOs directly.

Let’s set aside how our build system produces multiple DTBOs. Imagine a
scenario where we have one DTBO from our side and another from an OEM
and we don’t have the source for the OEM overlay. If we want to test
how the OEM overlay behaves in combination with ours, there’s no
straightforward way to do that today. This is where DTBO-to-DTBO merging
becomes useful.

So instead of focusing on optimizing fdt_overlay_apply(), this patch
series introduces support for a currently missing use case: merging
multiple DTBOs into a single overlay at build time.


-- 
Regards,
Wasim

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Konrad Dybcio]

On 9/2/25 12:35 PM, Wasim Nazir wrote:
> [...]
>>>
>>> Our repositores are following android structure based on android-treble
>>> where vendor subsystems needs to be modular for ease of OTA upgrade and
>>> development.
>>>
>>> As a result of which each feature/techpack (viz. audio, video etc.) have
>>> its own independent repositories where it can build its modules and
>>> overlay DT (soc & board).
>>> Kernel is separate and provides only the core images and base DT (soc &
>>> board).
>>
>> The build system should serve the needs of the project, not the other
>> way around.  If the feature repos can emit a dtbo, why can't they emit
>> a dts as well?
>>
>>> So, dtbo is the option to build it independently and at last we have
>>> options to either "overlay all dtbo at boot-time" or "merge it at build-time and
>>> overlay final dtbo at boot-time".
>>>
>>> This build-time merge is done outside techpack build system.
>>> But if we want to have dts from each techpack, then techpack-build system needs
>>> to communicate somehow to know which dts to include with which base dts.
>>
>> Why is that a harder problem than knowing which dtbo?  The dtbo must
>> have been built from some sort of dts, no?
>>
>>> This is not possible at build-time between techpacks.
>>
>> The build system is not immutable - you're proposing changes to libfdt
>> and/or dtc; why not changes to the build system?
>>
> 
> Hi David,
> 
> As each techpack operates in a sandboxed environment with its own build
> structure, all dependencies are managed within that sandbox, so the
> techpack owner only needs to focus on their own module. Since each
> techpack produces a compiled DTBO, it is self-contained and can be
> merged with other DTBOs without additional coordination.
> 
> If we were to use DTS files instead, every techpack owner would need to
> be involved in resolving cross-dependencies, which is not scalable in
> our modular setup.

+CC Couple of probably interested folks

My slightly-related $0.05 are that the whole concept of compartmentalizing
DT development like this makes any sort of uniformity, common style or
overall coherence of platform description impossible, as each team
inadvertently develops tunnel vision of their slice of the SoC/board, with
the only tangible benefit being a smaller LoC-per-reviewer ratio (that is,
unless the same person gets assigned to review multiple compartments..)

This works when you just need DT to convince the OS to bind the resources
to a driver, but actively prevents developers from catching anti-patterns
and/or abuses before they arise

Konrad

Re: [PATCH v3 0/4] Introduce fdt_overlay_merge() to allow merge of overlay blobs

[Konrad Dybcio]

On 9/2/25 2:49 PM, Konrad Dybcio wrote:
> On 9/2/25 12:35 PM, Wasim Nazir wrote:
>> [...]
>>>>
>>>> Our repositores are following android structure based on android-treble
>>>> where vendor subsystems needs to be modular for ease of OTA upgrade and
>>>> development.
>>>>
>>>> As a result of which each feature/techpack (viz. audio, video etc.) have
>>>> its own independent repositories where it can build its modules and
>>>> overlay DT (soc & board).
>>>> Kernel is separate and provides only the core images and base DT (soc &
>>>> board).
>>>
>>> The build system should serve the needs of the project, not the other
>>> way around.  If the feature repos can emit a dtbo, why can't they emit
>>> a dts as well?
>>>
>>>> So, dtbo is the option to build it independently and at last we have
>>>> options to either "overlay all dtbo at boot-time" or "merge it at build-time and
>>>> overlay final dtbo at boot-time".
>>>>
>>>> This build-time merge is done outside techpack build system.
>>>> But if we want to have dts from each techpack, then techpack-build system needs
>>>> to communicate somehow to know which dts to include with which base dts.
>>>
>>> Why is that a harder problem than knowing which dtbo?  The dtbo must
>>> have been built from some sort of dts, no?
>>>
>>>> This is not possible at build-time between techpacks.
>>>
>>> The build system is not immutable - you're proposing changes to libfdt
>>> and/or dtc; why not changes to the build system?
>>>
>>
>> Hi David,
>>
>> As each techpack operates in a sandboxed environment with its own build
>> structure, all dependencies are managed within that sandbox, so the
>> techpack owner only needs to focus on their own module. Since each
>> techpack produces a compiled DTBO, it is self-contained and can be
>> merged with other DTBOs without additional coordination.
>>
>> If we were to use DTS files instead, every techpack owner would need to
>> be involved in resolving cross-dependencies, which is not scalable in
>> our modular setup.
> 
> +CC Couple of probably interested folks
> 
> My slightly-related $0.05 are that the whole concept of compartmentalizing
> DT development like this makes any sort of uniformity, common style or
> overall coherence of platform description impossible, as each team
> inadvertently develops tunnel vision of their slice of the SoC/board, with
> the only tangible benefit being a smaller LoC-per-reviewer ratio (that is,
> unless the same person gets assigned to review multiple compartments..)
> 
> This works when you just need DT to convince the OS to bind the resources
> to a driver, but actively prevents developers from catching anti-patterns
> and/or abuses before they arise

Just to make sure it's clear - this is not any sort of nak to the patchset

Konrad