All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
@ 2021-11-07 22:43 Simon Glass
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2021-11-07 22:43 UTC (permalink / raw)
  To: Devicetree Compiler; +Cc: David Gibson, Simon Glass

Note: This was last sent 6 years ago. It really belongs upstream and I
believe it is useful functionality for libfdt, so I am trying again.
Please take a fresh look at this. It is cut back from the previous series.
If accepted we can expand the feature set piece by piece from here.

This series adds two new functions, fdt_first_region() and
fdt_next_region() which map FDT parts such as nodes and properties to
their regions in the FDT binary. The function is then used to implement
a grep utility for FDTs.

The core code is quite simple and small, but it grew a little due
to the need to make it iterative (first/next/next). Also this series adds
tests and a grep utility, so quite a bit of code is built on it.

The use for this feature is twofold. Firstly it provides a convenient
way of performing a structure-aware grep of the tree. For example it is
possible to grep for a node and get all the properties associated with
that node. Trees can be subsetted easily, by specifying the nodes that
are required, and then writing out the regions returned by this function.
This is useful for small resource-constrained systems, such as boot
loaders, which want to use an FDT but do not need to know about all of
it. The full FDT can be grepped to pull out the few things that are
needed - this can be an automatic part of the build system and does
not require the FDT source code.

This first use makes it easy to implement an FDT grep tool. Options are
provided to search for named nodes and properties. It is possible to
search for non-matches also (useful for excluding a particular property
from the FDT, for example). The output is like fdtdump, but only with
the regions selected by the grep. The fdtgrep utility cannot currently
output valid source in all cases, which can be used by dtc. That will
come later.

Secondly it makes it easy to hash parts of the tree and detect changes.
The intent is to get a list of regions which will be invariant provided
those parts are invariant. For example, if you request a list of regions
for all nodes but exclude the property "data", then you will get the
same region contents regardless of any change to "data" properties.
An assumption is made here that the tree ordering remains the same.

This second use is the mechanism behind U-Boot's verified-boot feature
using Flat Image Tree (FIT) images, introduced 6 years ago. Briefly, this
works by signing configurations (consisting of particular kernel and FDT
combinations) so that the boot loader can verify that these combinations
are valid and permitted. Since a FIT image is in fact an FDT, we need to
be able to hash particular regions of the FDT for the signing and
verification process. This is done by using the region functions to
select the data that needs to be hashed for a particular configuration.

A third use is currently under discussion in the boot-architecture
mailing list, which is to sign vanilla FDT files by adding a signature
node. This will avoid the need for a separate signature for every device
tree file, or inventing yet another file format.

The fdtgrep utility could be used to replace all of the functions of
fdtdump. However fdtdump is intended as a separate, simple way of
dumping the tree (for verifying against dtc output for example). So
fdtdump remains a separate program and this series leaves it alone.

Note: a somewhat unfortunately feature of this implementation is that
a state structure needs to be kept around between calls of
fdt_next_region(). This is declared in libfdt.h but really should be
opaque. Ideas welcome.

Changes in v4:
- Chop back to the minimal useful functionality
- Rebase to master
- Update cover letter for developments over the past 6 years

Changes in v3:
- Add -f option to display offset; make -a display an absolute file address
- Add -s option to include all subnodes
- Add a feature to include all subnodes
- Add documentation on fdtgrep
- Adjust help and command line processing to follow new approach
- Rename -V to -I to avoid using -V for a different purpose to other tools
- Rename -s to -e since it only 'enters' the node and does not include it all

Changes in v2:
- Add local fdt_find_regions() function since libfdt no longer has it
- Add long comment explaining theory of operation
- Add more comments about the -1 return value from h_include
- Add new FDT_ERR_TOODEEP error type and use it
- Add note that changes in node/property order can cause false hash misses
- Change returned error from BADLAYOUT to BADSTRUCTURE
- Drop FDT_IS_COMPAT and pass node offset to h_include function
- Drop stale comment about names / wildcards
- Fix info->count <= info->max_regions in fdt_add_region() merge case
- Move region code to separate fdt_region.c file
- Move to a model with fdt_first_region()/fdt_next_region()
- Return FDT_ERR_BADLAYOUT error if strings block is before structure block

Simon Glass (4):
  README: Explain how to add a new API function
  libfdt: Add functions to find regions in an FDT
  Add fdtgrep to grep and hash FDTs
  Add tests for fdtgrep

 .gitignore               |   1 +
 Documentation/manual.txt |  32 +++
 Makefile                 |   5 +
 Makefile.utils           |   6 +
 README                   |   9 +
 fdtgrep.c                | 554 +++++++++++++++++++++++++++++++++++++++
 libfdt/Makefile.libfdt   |   2 +-
 libfdt/fdt_region.c      | 369 ++++++++++++++++++++++++++
 libfdt/libfdt.h          | 223 +++++++++++++++-
 libfdt/meson.build       |   1 +
 libfdt/version.lds       |   2 +
 tests/grep.dts           |  23 ++
 tests/run_tests.sh       | 111 +++++++-
 tests/testutils.sh       |   1 +
 14 files changed, 1336 insertions(+), 3 deletions(-)
 create mode 100644 fdtgrep.c
 create mode 100644 libfdt/fdt_region.c
 create mode 100644 tests/grep.dts

-- 
2.34.0.rc0.344.g81b53c2807-goog


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

* [PATCH v4 1/4] README: Explain how to add a new API function
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2021-11-07 22:43   ` Simon Glass
       [not found]     ` <20211107224346.3181320-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
  2021-11-07 22:43   ` [PATCH v4 2/4] libfdt: Add functions to find regions in an FDT Simon Glass
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2021-11-07 22:43 UTC (permalink / raw)
  To: Devicetree Compiler; +Cc: David Gibson, Simon Glass

This is not obvious so add a little note about it.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

(no changes since v1)

 README | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/README b/README
index d9bf850..bac1714 100644
--- a/README
+++ b/README
@@ -71,6 +71,15 @@ More work remains to support all of libfdt, including access to numeric
 values.
 
 
+Adding a new function to libfdt.h
+---------------------------------
+
+The shared library uses libfdt/version.lds to list the exported functions, so
+add your new function there. Check that your function works with pylibfdt. If
+it cannot be supported, put the declaration in libfdt.h behind #ifndef SWIG so
+that swig ignores it.
+
+
 Tests
 -----
 
-- 
2.34.0.rc0.344.g81b53c2807-goog


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

* [PATCH v4 2/4] libfdt: Add functions to find regions in an FDT
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
  2021-11-07 22:43   ` [PATCH v4 1/4] README: Explain how to add a new API function Simon Glass
@ 2021-11-07 22:43   ` Simon Glass
  2021-11-07 22:43   ` [PATCH v4 3/4] Add fdtgrep to grep and hash FDTs Simon Glass
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2021-11-07 22:43 UTC (permalink / raw)
  To: Devicetree Compiler; +Cc: David Gibson, Simon Glass

Given a set of nodes and properties, find the regions of the device tree
which describe those parts.

Tests will come as part of fdtgrep.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v4:
- Chop back to the minimal useful functionality

Changes in v3:
- Add a feature to include all subnodes

Changes in v2:
- Add long comment explaining theory of operation
- Add more comments about the -1 return value from h_include
- Add new FDT_ERR_TOODEEP error type and use it
- Add note that changes in node/property order can cause false hash misses
- Change returned error from BADLAYOUT to BADSTRUCTURE
- Drop FDT_IS_COMPAT and pass node offset to h_include function
- Drop stale comment about names / wildcards
- Fix info->count <= info->max_regions in fdt_add_region() merge case
- Move region code to separate fdt_region.c file
- Move to a model with fdt_first_region()/fdt_next_region()
- Return FDT_ERR_BADLAYOUT error if strings block is before structure block

 libfdt/Makefile.libfdt |   2 +-
 libfdt/fdt_region.c    | 369 +++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h        | 223 ++++++++++++++++++++++++-
 libfdt/meson.build     |   1 +
 libfdt/version.lds     |   2 +
 5 files changed, 595 insertions(+), 2 deletions(-)
 create mode 100644 libfdt/fdt_region.c

diff --git a/libfdt/Makefile.libfdt b/libfdt/Makefile.libfdt
index b6d8fc0..45a5bf4 100644
--- a/libfdt/Makefile.libfdt
+++ b/libfdt/Makefile.libfdt
@@ -8,7 +8,7 @@ LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
 LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
 LIBFDT_VERSION = version.lds
 LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c \
-	fdt_addresses.c fdt_overlay.c fdt_check.c
+	fdt_addresses.c fdt_overlay.c fdt_check.c fdt_region.c
 LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
 LIBFDT_LIB = libfdt-$(DTC_VERSION).$(SHAREDLIB_EXT)
 
diff --git a/libfdt/fdt_region.c b/libfdt/fdt_region.c
new file mode 100644
index 0000000..78a006a
--- /dev/null
+++ b/libfdt/fdt_region.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/*
+ * Selection of regions of a devicetree blob
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
+ */
+
+#include "libfdt_env.h"
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "libfdt_internal.h"
+
+/**
+ * fdt_add_region() - Add a new region to our list
+ *
+ * The region is added if there is space, but in any case we increment the
+ * count. If permitted, and the new region overlaps the last one, we merge
+ * them.
+ *
+ * @info: State information
+ * @offset: Start offset of region
+ * @size: Size of region
+ */
+static int fdt_add_region(struct fdt_region_state *info, int offset, int size)
+{
+	struct fdt_region *reg;
+
+	reg = info->region ? &info->region[info->count - 1] : NULL;
+	if (info->can_merge && info->count &&
+			info->count <= info->max_regions &&
+			reg && offset <= reg->offset + reg->size) {
+		reg->size = offset + size - reg->offset;
+	} else if (info->count++ < info->max_regions) {
+		if (reg) {
+			reg++;
+			reg->offset = offset;
+			reg->size = size;
+		}
+	} else {
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * fdt_include_supernodes() - Include supernodes required by this node
+ *
+ * When we decided to include a node or property which is not at the top
+ * level, this function forces the inclusion of higher level nodes. For
+ * example, given this tree:
+ *
+ * / {
+ *     testing {
+ *     }
+ * }
+ *
+ * If we decide to include testing then we need the root node to have a valid
+ * tree. This function adds those regions.
+ *
+ * @info: State information
+ * @depth: Current stack depth
+ */
+static int fdt_include_supernodes(struct fdt_region_state *info, int depth)
+{
+	int base = fdt_off_dt_struct(info->fdt);
+	int start, stop_at;
+	int i;
+
+	/*
+	 * Work down the stack looking for supernodes that we didn't include.
+	 * The algortihm here is actually pretty simple, since we know that
+	 * no previous subnode had to include these nodes, or if it did, we
+	 * marked them as included (on the stack) already.
+	 */
+	for (i = 0; i <= depth; i++) {
+		if (!info->stack[i].included) {
+			start = info->stack[i].offset;
+
+			/* Add the FDT_BEGIN_NODE tag of this supernode */
+			fdt_next_tag(info->fdt, start, &stop_at);
+			if (fdt_add_region(info, base + start,
+					stop_at - start))
+				return -1;
+
+			/* Remember that this supernode is now included */
+			info->stack[i].included = 1;
+			info->can_merge = 1;
+		}
+
+		/* Force (later) generation of the FDT_END_NODE tag */
+		if (!info->stack[i].want)
+			info->stack[i].want = WANT_NODES_ONLY;
+	}
+
+	return 0;
+}
+
+int fdt_first_region(const void *fdt, fdt_region_func h_include, void *priv,
+		     struct fdt_region *region, char *path, int path_len,
+		     int flags, struct fdt_region_state *info)
+{
+	struct fdt_region_ptrs *p = &info->ptrs;
+
+	/* Set up our state */
+	info->fdt = fdt;
+	info->can_merge = 1;
+	info->max_regions = 1;
+	info->start = -1;
+	info->path = path;
+	info->path_len = path_len;
+	info->flags = flags;
+	p->want = WANT_NOTHING;
+	p->end = path;
+	*p->end = '\0';
+	p->nextoffset = 0;
+	p->depth = -1;
+	p->done = FDT_DONE_NOTHING;
+
+	return fdt_next_region(fdt, h_include, priv, region, info);
+}
+
+/*
+ * Theory of operation
+ *
+ * Note: in this description 'included' means that a node (or other part of
+ * the tree) should be included in the region list, i.e. it will have a region
+ * which covers its part of the tree.
+ *
+ * This function maintains some state from the last time it is called. It
+ * checks the next part of the tree that it is supposed to look at
+ * (p.nextoffset) to see if that should be included or not. When it finds
+ * something to include, it sets info->start to its offset. This marks the
+ * start of the region we want to include.
+ *
+ * Once info->start is set to the start (i.e. not -1), we continue scanning
+ * until we find something that we don't want included. This will be the end
+ * of a region. At this point we can close off the region and add it to the
+ * list. So we do so, and reset info->start to -1.
+ *
+ * One complication here is that we want to merge regions. So when we come to
+ * add another region later, we may in fact merge it with the previous one if
+ * one ends where the other starts.
+ *
+ * The function fdt_add_region() will return -1 if it fails to add the region,
+ * because we already have a region ready to be returned, and the new one
+ * cannot be merged in with it. In this case, we must return the region we
+ * found, and wait for another call to this function. When it comes, we will
+ * repeat the processing of the tag and again try to add a region. This time it
+ * will succeed.
+ *
+ * The current state of the pointers (stack, offset, etc.) is maintained in
+ * a ptrs member. At the start of every loop iteration we make a copy of it.
+ * The copy is then updated as the tag is processed. Only if we get to the end
+ * of the loop iteration (and successfully call fdt_add_region() if we need
+ * to) can we commit the changes we have made to these pointers. For example,
+ * if we see an FDT_END_NODE tag we will decrement the depth value. But if we
+ * need to add a region for this tag (let's say because the previous tag is
+ * included and this FDT_END_NODE tag is not included) then we will only commit
+ * the result if we were able to add the region. That allows us to retry again
+ * next time.
+ *
+ * We keep track of a variable called 'want' which tells us what we want to
+ * include when there is no specific information provided by the h_include()
+ * function for a particular property. This basically handles the inclusion of
+ * properties which are pulled in by virtue of the node they are in. So if you
+ * include a node, its properties are also included. In this case 'want' will
+ * be WANT_NODES_AND_PROPS.
+ *
+ * Using 'want' we work out 'include', which tells us whether this current tag
+ * should be included or not. As you can imagine, if the value of 'include'
+ * changes, that means we are on a boundary between nodes to include and nodes
+ * to exclude. At this point we either close off a previous region and add it
+ * to the list, or mark the start of a new region.
+ *
+ * Apart from the nodes, we have the FDT_END tag, mem_rsvmap and the string
+ * list. The last two are not handled at present. The FDT_END tag is dealt with
+ * as a whole (i.e. we create a region for it if is to be included).
+ */
+int fdt_next_region(const void *fdt, fdt_region_func h_include, void *priv,
+		    struct fdt_region *region, struct fdt_region_state *info)
+{
+	int base = fdt_off_dt_struct(fdt);
+	int last_node = 0;
+	const char *str;
+
+	info->region = region;
+	info->count = 0;
+
+	/*
+	 * Work through the tags one by one, deciding whether each needs to
+	 * be included or not. We set the variable 'include' to indicate our
+	 * decision. 'want' is used to track what we want to include absent
+	 * further information from the h_include() function - it allows us to
+	 * pick up all the properties (and/or subnode tags) of a node that we
+	 * have been asked to include.
+	 */
+	while (info->ptrs.done < FDT_DONE_STRUCT) {
+		const struct fdt_property *prop;
+		struct fdt_region_ptrs p;
+		const char *name;
+		int include = 0;
+		int stop_at = 0;
+		uint32_t tag;
+		int offset;
+		int val;
+		int len;
+
+		/*
+		 * Make a copy of our pointers. If we make it to the end of
+		 * this block then we will commit them back to info->ptrs.
+		 * Otherwise we can try again from the same starting state
+		 * next time we are called.
+		 */
+		p = info->ptrs;
+
+		/*
+		 * Find the tag, and the offset of the next one. If we need to
+		 * stop including tags, then by default we stop *after*
+		 * including the current tag
+		 */
+		offset = p.nextoffset;
+		tag = fdt_next_tag(fdt, offset, &p.nextoffset);
+		stop_at = p.nextoffset;
+
+		switch (tag) {
+		case FDT_PROP:
+			stop_at = offset;
+			prop = fdt_get_property_by_offset(fdt, offset, NULL);
+			str = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+			val = h_include(priv, fdt, last_node, FDT_IS_PROP, str,
+					    strlen(str) + 1);
+			if (val == -1) {
+				include = p.want >= WANT_NODES_AND_PROPS;
+			} else {
+				include = val;
+				/*
+				 * Make sure we include the } for this block.
+				 * It might be more correct to have this done
+				 * by the call to fdt_include_supernodes() in
+				 * the case where it adds the node we are
+				 * currently in, but this is equivalent.
+				 */
+				if ((info->flags & FDT_REG_SUPERNODES) && val &&
+						!p.want)
+					p.want = WANT_NODES_ONLY;
+			}
+
+			/* Value grepping is not yet supported */
+			break;
+
+		case FDT_NOP:
+			include = p.want >= WANT_NODES_AND_PROPS;
+			stop_at = offset;
+			break;
+
+		case FDT_BEGIN_NODE:
+			last_node = offset;
+			p.depth++;
+			if (p.depth == FDT_MAX_DEPTH)
+				return -FDT_ERR_TOODEEP;
+			name = fdt_get_name(fdt, offset, &len);
+			if (p.end - info->path + 2 + len >= info->path_len)
+				return -FDT_ERR_NOSPACE;
+
+			/* Build the full path of this node */
+			if (p.end != info->path + 1)
+				*p.end++ = '/';
+			strcpy(p.end, name);
+			p.end += len;
+			info->stack[p.depth].want = p.want;
+			info->stack[p.depth].offset = offset;
+
+			/*
+			 * We are not intending to include this node unless
+			 * it matches, so make sure we stop *before* its tag.
+			 */
+			stop_at = offset;
+			p.want = WANT_NOTHING;
+			val = h_include(priv, fdt, offset, FDT_IS_NODE,
+					info->path, p.end - info->path + 1);
+
+			/* Include this if requested */
+			if (val)
+				p.want = WANT_NODES_AND_PROPS;
+
+			/* If not requested, decay our 'p.want' value */
+			else if (p.want)
+				p.want--;
+
+			/* Not including this tag, so stop now */
+			else
+				stop_at = offset;
+
+			/*
+			 * Decide whether to include this tag, and update our
+			 * stack with the state for this node
+			 */
+			include = p.want;
+			info->stack[p.depth].included = include;
+			break;
+
+		case FDT_END_NODE:
+			include = p.want;
+			if (p.depth < 0)
+				return -FDT_ERR_BADSTRUCTURE;
+
+			/*
+			 * If we don't want this node, stop right away, unless
+			 * we are including subnodes
+			 */
+			if (!p.want)
+				stop_at = offset;
+			p.want = info->stack[p.depth].want;
+			p.depth--;
+			while (p.end > info->path && *--p.end != '/')
+				;
+			*p.end = '\0';
+			break;
+
+		case FDT_END:
+			/* We always include the end tag */
+			include = 1;
+			p.done = FDT_DONE_STRUCT;
+			break;
+		}
+
+		/* If this tag is to be included, mark it as region start */
+		if (include && info->start == -1) {
+			/* Include any supernodes required by this one */
+			if (info->flags & FDT_REG_SUPERNODES) {
+				if (fdt_include_supernodes(info, p.depth))
+					return 0;
+			}
+			info->start = offset;
+		}
+
+		/*
+		 * If this tag is not to be included, finish up the current
+		 * region.
+		 */
+		if (!include && info->start != -1) {
+			if (fdt_add_region(info, base + info->start,
+				       stop_at - info->start))
+				return 0;
+			info->start = -1;
+			info->can_merge = 1;
+		}
+
+		/* If we have made it this far, we can commit our pointers */
+		info->ptrs = p;
+	}
+
+	/* Add a region for the END tag and a separate one for string table */
+	if (info->ptrs.done < FDT_DONE_END) {
+		if (info->ptrs.nextoffset != (int)fdt_size_dt_struct(fdt))
+			return -FDT_ERR_BADSTRUCTURE;
+
+		if (fdt_add_region(info, base + info->start,
+			       info->ptrs.nextoffset - info->start))
+			return 0;
+		info->ptrs.done++;
+	}
+
+	return info->count > 0 ? 0 : -FDT_ERR_NOTFOUND;
+}
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 7f117e8..dcac768 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -106,7 +106,12 @@ extern "C" {
 	/* FDT_ERR_ALIGNMENT: The device tree base address is not 8-byte
 	 * aligned. */
 
-#define FDT_ERR_MAX		19
+#define FDT_ERR_TOODEEP		20
+	/* FDT_ERR_TOODEEP: The depth of a node has exceeded the internal
+	 * libfdt limit. This can happen if you have more than
+	 * FDT_MAX_DEPTH nested nodes. */
+
+#define FDT_ERR_MAX		20
 
 /* constants */
 #define FDT_MAX_PHANDLE 0xfffffffe
@@ -2122,6 +2127,222 @@ int fdt_overlay_apply(void *fdt, void *fdto);
 
 const char *fdt_strerror(int errval);
 
+#ifndef SWIG /* Regions are not yet supported in Python */
+
+/*
+ * The following struction are for for internal libfdt use. Callers may use
+ * struct fdt_region_state for the purpose of allocating a struct to pass to
+ * fdt_first_region()/
+ *
+ * This is to avoid calling malloc(), or requiring libfdt to return the number
+ * of bytes needed for this struct.
+ */
+
+/**
+ * struct fdt_region - A region within the device tree
+ *
+ * @offset: Start offset in bytes from the start of the device tree
+ * @size: Size in bytes
+ */
+struct fdt_region {
+	int offset;
+	int size;
+};
+
+#define FDT_ANY_GLOBAL			(FDT_IS_NODE | FDT_IS_PROP)
+#define FDT_IS_ANY			0x3		/* all the above */
+
+/* We set a reasonable limit on the number of nested nodes */
+#define FDT_MAX_DEPTH			32
+
+/* Decribes what we want to include from the current tag */
+enum want_t {
+	WANT_NOTHING,
+	WANT_NODES_ONLY,		/* No properties */
+	WANT_NODES_AND_PROPS,		/* Everything for one level */
+};
+
+/* Keeps track of the state at parent nodes */
+struct fdt_subnode_stack {
+	int offset;		/* Offset of node */
+	enum want_t want;	/* The 'want' value here */
+	int included;		/* 1 if we included this node, 0 if not */
+};
+
+/* enum fdt_region_done_t - current state of progress through the dtb */
+enum fdt_region_done_t {
+	FDT_DONE_NOTHING,		/* Working on 'struct' region */
+	FDT_DONE_STRUCT,		/* Finished the 'struct' region */
+	FDT_DONE_END,			/* Finished the 'end' tag */
+};
+
+/* struct fdt_region_ptrs - Keeps track of all the position pointers */
+struct fdt_region_ptrs {
+	int depth;			/* Current tree depth */
+	enum fdt_region_done_t done;	/* What we have completed scanning */
+	enum want_t want;		/* What we are currently including */
+	char *end;			/* Pointer to end of full node path */
+	int nextoffset;			/* Next node offset to check */
+};
+
+/* The state of our finding algortihm */
+struct fdt_region_state {
+	struct fdt_subnode_stack stack[FDT_MAX_DEPTH];	/* node stack */
+	struct fdt_region *region;	/* Contains list of regions found */
+	int count;			/* Numnber of regions found */
+	const void *fdt;		/* FDT blob */
+	int max_regions;		/* Maximum regions to find */
+	int can_merge;		/* 1 if we can merge with previous region */
+	int start;			/* Start position of current region */
+	struct fdt_region_ptrs ptrs;	/* Pointers for what we are up to */
+
+	/* These hold information passed in to fdt_first_region(): */
+	int flags;
+	char *path;
+	int path_len;
+};
+
+/*
+ * Flags for region finding
+ *
+ * Add all supernodes of a matching node/property, useful for creating a
+ * valid subset tree
+ */
+#define FDT_REG_SUPERNODES		(1 << 0)
+
+/* Indicates what an fdt part is (node, property) in call to fdt_region_func */
+#define FDT_IS_NODE			(1 << 0)
+#define FDT_IS_PROP			(1 << 1)
+
+/**
+ * fdt_region_func: Determine whether to include a part or not
+ *
+ * This function is provided by the caller to fdt_first/next_region(). It is
+ * called repeatedly for each device tree element to find out whether it should
+ * be included or not. Every node will generate a call with @type == FDT_IS_NODE
+ * and every property will generate a call with @type = FDT_IS_PROP
+ *
+ * @priv: Private pointer as passed to fdt_find_regions()
+ * @fdt: Pointer to FDT blob
+ * @offset: Offset of this node / property
+ * @type: Type of this part, FDT_IS_...
+ * @data: Pointer to data (full path of node / property name)
+ * @size: Size of data (length of node path / property including \0 terminator
+ * @return 0 to exclude, 1 to include, -1 if no information is available
+ */
+typedef int (*fdt_region_func)(void *priv, const void *fdt, int offset,
+			       int type, const char *data, int size);
+
+/**
+ * fdt_first_region() - find regions in device tree
+ *
+ * Given a nodes and properties to include and properties to exclude, find
+ * the regions of the device tree which describe those included parts.
+ *
+ * The use for this function is twofold. Firstly it provides a convenient
+ * way of performing a structure-aware grep of the tree. For example it is
+ * possible to grep for a node and get all the properties associated with
+ * that node. Trees can be subsetted easily, by specifying the nodes that
+ * are required, and then writing out the regions returned by this function.
+ * This is useful for small resource-constrained systems, such as boot
+ * loaders, which want to use an FDT but do not need to know about all of
+ * it.
+ *
+ * Secondly it makes it easy to hash parts of the tree and detect changes.
+ * The intent is to get a list of regions which will be invariant provided
+ * those parts are invariant. For example, if you request a list of regions
+ * for all nodes but exclude the property "data", then you will get the
+ * same region contents regardless of any change to "data" properties.
+ *
+ * This function can be used to produce a byte-stream to send to a hashing
+ * function to verify that critical parts of the FDT have not changed.
+ * Note that semantically null changes in order could still cause false
+ * hash misses. Such reordering might happen if the tree is regenerated
+ * from source, and nodes are reordered (the bytes-stream will be emitted
+ * in a different order and many hash functions will detect this). However
+ * if an existing tree is modified using libfdt functions, such as
+ * fdt_add_subnode() and fdt_setprop(), then this problem is avoided.
+ *
+ * The nodes/properties to include/exclude are defined by a function
+ * provided by the caller. This function is called for each node and
+ * property, and must return:
+ *
+ *    0 - to exclude this part
+ *    1 - to include this part
+ *   -1 - for FDT_IS_PROP only: no information is available, so include
+ *		if its containing node is included
+ *
+ * The last case is only used to deal with properties. Often a property is
+ * included if its containing node is included - this is the case where
+ * -1 is returned.. However if the property is specifically required to be
+ * included/excluded, then 0 or 1 can be returned. Note that including a
+ * property when the FDT_REG_SUPERNODES flag is given will force its
+ * containing node to be included since it is not valid to have a property
+ * that is not in a node.
+ *
+ * Using the information provided, the inclusion of a node can be controlled
+ * either by a node name or its compatible string, or any other property
+ * that the function can determine.
+ *
+ * As an example, including node "/" means to include the root node and all
+ * root properties. A flag provides a way of also including supernodes (of
+ * which there is none for the root node), and another flag includes
+ * immediate subnodes, so in this case we would get the FDT_BEGIN_NODE and
+ * FDT_END_NODE of all subnodes of /.
+ *
+ * The subnode feature helps in a hashing situation since it prevents the
+ * root node from changing at all. Any change to non-excluded properties,
+ * names of subnodes or number of subnodes would be detected.
+ *
+ * When used with Flat Image Trees (FITs) this provides the ability to hash and
+ * sign parts of * the FIT based on different configurations in the FIT. Then it
+ * is * impossible to change anything about that configuration (include images
+ * attached to the configuration), but it may be possible to add new
+ * configurations, new images or new signatures within the existing
+ * framework.
+ *
+ * The device tree header is not included in the region list. Since the
+ * contents of the FDT are changing (shrinking, often), the caller will need
+ * to regenerate the header anyway.
+ *
+ * @fdt:	Device tree to check
+ * @h_include:	Function to call to determine whether to include a part or
+ *		not
+ * @priv:	Private pointer passed to h_include
+ * @region:	Returns first region found
+ * @path:	Pointer to a temporary string for the function to use for
+ *		building path names
+ * @path_len:	Length of path, must be large enough to hold the longest
+ *		path in the tree
+ * @flags:	Various flags that control the region algortihm, see
+ *		FDT_REG_...
+ * @return 0, on success
+ *	-FDT_ERR_BADSTRUCTURE (too deep or more END tags than BEGIN tags
+ *	-FDT_ERR_BADLAYOUT
+ *	-FDT_ERR_NOSPACE (path area is too small)
+ *	-FDT_ERR_TOODEEP if the tree is too deep
+ */
+int fdt_first_region(const void *fdt, fdt_region_func h_include,
+		     void *priv, struct fdt_region *region,
+		     char *path, int path_len, int flags,
+		     struct fdt_region_state *info);
+
+/** fdt_next_region() - find next region
+ *
+ * See fdt_first_region() for full description. This function finds the
+ * next region according to the provided parameters, which must be the same
+ * as passed to fdt_first_region().
+ *
+ * The flags value is used unchanged from the call to fdt_first_region().
+ *
+ * This function can additionally return -FDT_ERR_NOTFOUND when there are no
+ * more regions
+ */
+int fdt_next_region(const void *fdt, fdt_region_func h_include,
+		    void *priv, struct fdt_region *region,
+		    struct fdt_region_state *info);
+#endif /* SWIG */
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libfdt/meson.build b/libfdt/meson.build
index 0307ffb..d0c39dd 100644
--- a/libfdt/meson.build
+++ b/libfdt/meson.build
@@ -9,6 +9,7 @@ sources = files(
   'fdt_check.c',
   'fdt_empty_tree.c',
   'fdt_overlay.c',
+  'fdt_region.c',
   'fdt_ro.c',
   'fdt_rw.c',
   'fdt_strerror.c',
diff --git a/libfdt/version.lds b/libfdt/version.lds
index 7ab85f1..365d560 100644
--- a/libfdt/version.lds
+++ b/libfdt/version.lds
@@ -77,6 +77,8 @@ LIBFDT_1.2 {
 		fdt_appendprop_addrrange;
 		fdt_setprop_inplace_namelen_partial;
 		fdt_create_with_flags;
+                fdt_first_region;
+                fdt_next_region;
 	local:
 		*;
 };
-- 
2.34.0.rc0.344.g81b53c2807-goog


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

* [PATCH v4 3/4] Add fdtgrep to grep and hash FDTs
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
  2021-11-07 22:43   ` [PATCH v4 1/4] README: Explain how to add a new API function Simon Glass
  2021-11-07 22:43   ` [PATCH v4 2/4] libfdt: Add functions to find regions in an FDT Simon Glass
@ 2021-11-07 22:43   ` Simon Glass
  2021-11-07 22:43   ` [PATCH v4 4/4] Add tests for fdtgrep Simon Glass
  2022-02-07  4:09   ` [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs David Gibson
  4 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2021-11-07 22:43 UTC (permalink / raw)
  To: Devicetree Compiler; +Cc: David Gibson, Simon Glass

fdtgrep operates like a grep for device trees, working on binary .dtb
files.

fdtgrep can find nodes by name or compatible string. It can find
properties by name. Using a list of nodes/properties to match, fdtgrep
creates either .dts text output containing just those matches, or binary
output.

The raw binary output, without the normal FDT headers, can be useful for
hashing part of an FDT and making sure that that part does not change,
even if other parts do. This makes it possible to detect an important
change, while ignoring a change that is of no interest.

At its simplest fdtgrep can be used as follows;

   fdtgrep /holiday tests/grep.dtb

and it produces just that node (with a skeleton to hold it):

/ {
    holiday {
        compatible = "ixtapa", "mexico";
        weather = "sunny";
        status = "okay";
    };
};

The binary form of this is:

   fdtgrep -n /holiday -O dtb tests/grep.dtb

which produces a fragment of the dtb with just the above node.

Various options are provided to search for properties and to ensure that
nodes/properties are not present.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v4:
- Chop back to the minimal useful functionality

Changes in v3:
- Add -f option to display offset; make -a display an absolute file address
- Add -s option to include all subnodes
- Add documentation on fdtgrep
- Adjust help and command line processing to follow new approach
- Rename -V to -I to avoid using -V for a different purpose to other tools
- Rename -s to -e since it only 'enters' the node and does not include it all

Changes in v2:
- Add local fdt_find_regions() function since libfdt no longer has it

 .gitignore               |   1 +
 Documentation/manual.txt |  32 +++
 Makefile                 |   5 +
 Makefile.utils           |   6 +
 fdtgrep.c                | 554 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 598 insertions(+)
 create mode 100644 fdtgrep.c

diff --git a/.gitignore b/.gitignore
index 8e332d8..1d514e9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,6 +16,7 @@ lex.yy.c
 /fdtget
 /fdtput
 /fdtoverlay
+/fdtgrep
 /patches
 /.pc
 
diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index 97e53b9..7a19696 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -760,3 +760,35 @@ create/delete nodes and delete properties.
 For the first usage, the command line arguments are joined together into a
 single value which is written to the property. The -t option is required so
 that fdtput knows how to decode its arguments.
+
+
+6) fdtgrep - Grep device tree binaries for nodes or properties
+
+This is useful for finding a node/property in a devicetree and just showing
+that:
+
+    $ fdtgrep -n /chosen file.dtb
+    chosen {
+        stdout-path = "/serial@f00:115200";
+    };
+
+This tool also supports outputting in binary format. This is useful for hashing
+parts of the devicetree, since it outputs just those parts that are selected.
+This binary output can be run through a hashing tool for use in signature
+verification, for example. It is not intended for producing valid dtb files
+(future work will provide this).
+
+The syntax of the fdtgrep is:
+
+    fdtgrep <options> <dt file>|-
+
+Options are:
+
+  -n, --include-node <arg>   Node to include in grep
+  -N, --exclude-node <arg>   Node to exclude in grep
+  -p, --include-prop <arg>   Property to include in grep
+  -P, --exclude-prop <arg>   Property to exclude in grep
+  -o, --out <arg>            -o <output file>
+  -O, --out-format <arg>     -O <output format>
+  -h, --help                 Print this help and exit
+  -V, --version              Print version and exit
diff --git a/Makefile b/Makefile
index ee77115..3707269 100644
--- a/Makefile
+++ b/Makefile
@@ -148,6 +148,7 @@ BIN += fdtdump
 BIN += fdtget
 BIN += fdtput
 BIN += fdtoverlay
+BIN += fdtgrep
 
 SCRIPTS = dtdiff
 
@@ -184,6 +185,7 @@ ifneq ($(MAKECMDGOALS),libfdt)
 -include $(FDTGET_OBJS:%.o=%.d)
 -include $(FDTPUT_OBJS:%.o=%.d)
 -include $(FDTOVERLAY_OBJS:%.o=%.d)
+-include $(FDTGREP_OBJS:%.o=%.d)
 endif
 endif
 
@@ -267,6 +269,8 @@ fdtput:	$(FDTPUT_OBJS) $(LIBFDT_lib)
 
 fdtoverlay: $(FDTOVERLAY_OBJS) $(LIBFDT_lib)
 
+fdtgrep: $(FDTGREP_OBJS) $(LIBFDT_lib)
+
 dist:
 	git archive --format=tar --prefix=dtc-$(dtc_version)/ HEAD \
 		> ../dtc-$(dtc_version).tar
@@ -317,6 +321,7 @@ TESTS_BIN += fdtput
 TESTS_BIN += fdtget
 TESTS_BIN += fdtdump
 TESTS_BIN += fdtoverlay
+TESTS_BIN += fdtgrep
 ifeq ($(NO_PYTHON),0)
 TESTS_PYLIBFDT += maybe_pylibfdt
 endif
diff --git a/Makefile.utils b/Makefile.utils
index 9436b34..430219f 100644
--- a/Makefile.utils
+++ b/Makefile.utils
@@ -29,3 +29,9 @@ FDTOVERLAY_SRCS = \
 	util.c
 
 FDTOVERLAY_OBJS = $(FDTOVERLAY_SRCS:%.c=%.o)
+
+FDTGREP_SRCS = \
+	fdtgrep.c \
+	util.c
+
+FDTGREP_OBJS = $(FDTGREP_SRCS:%.c=%.o)
diff --git a/fdtgrep.c b/fdtgrep.c
new file mode 100644
index 0000000..53aab4d
--- /dev/null
+++ b/fdtgrep.c
@@ -0,0 +1,554 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Tool to selection regions of a devicetree blob
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
+ */
+
+#include <assert.h>
+#include <ctype.h>
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <libfdt.h>
+#include <libfdt_internal.h>
+#include "util.h"
+
+/* Define DEBUG to get some debugging output on stderr */
+#ifdef DEBUG
+#define debug(a, b...) fprintf(stderr, a, ## b)
+#else
+#define debug(a, b...)
+#endif
+
+/* A linked list of values we are grepping for */
+struct value_node {
+	int type;		/* Types this value matches (FDT_IS... mask) */
+	int include;		/* 1 to include matches, 0 to exclude */
+	const char *string;	/* String to match */
+	struct value_node *next;	/* Pointer to next node, or NULL */
+};
+
+/* Output formats we support */
+enum output_t {
+	OUT_DTS,		/* Device tree source */
+	OUT_BIN,		/* Fragment of .dtb, for hashing */
+};
+
+/* Holds information which controls our output and options */
+struct display_info {
+	enum output_t output;	/* Output format */
+	int flags;		/* Flags (FDT_REG_...) */
+	int types_inc;		/* Mask of types that we include (FDT_IS...) */
+	int types_exc;		/* Mask of types that we exclude (FDT_IS...) */
+	struct value_node *value_head;	/* List of values to match */
+	const char *output_fname;	/* Output filename */
+	FILE *fout;		/* File to write dts/dtb output */
+};
+
+static void report_error(const char *where, int err)
+{
+	fprintf(stderr, "Error at '%s': %s\n", where, fdt_strerror(err));
+}
+
+/**
+ * value_add() - Add a new value to our list of things to grep for
+ *
+ * @disp:	Display structure, holding info about our options
+ * @headp:	Pointer to header pointer of list
+ * @type:	Type of this value (FDT_IS_...)
+ * @include:	1 if we want to include matches, 0 to exclude
+ * @str:	String value to match
+ * @return 0 on success, -1 if out of memory
+ */
+static int value_add(struct display_info *disp, struct value_node **headp,
+		     int type, int include, const char *str)
+{
+	struct value_node *node;
+
+	/*
+	 * Keep track of which types we are excluding/including. We don't
+	 * allow both including and excluding things, because it doesn't make
+	 * sense. 'Including' means that everything not mentioned is
+	 * excluded. 'Excluding' means that everything not mentioned is
+	 * included. So using the two together would be meaningless.
+	 */
+	if (include)
+		disp->types_inc |= type;
+	else
+		disp->types_exc |= type;
+	if (disp->types_inc & disp->types_exc & type) {
+		fprintf(stderr, "Cannot use both include and exclude for '%s'\n",
+			str);
+		return -1;
+	}
+
+	str = strdup(str);
+	node = malloc(sizeof(*node));
+	if (!str || !node) {
+		fprintf(stderr, "Out of memory\n");
+		return -1;
+	}
+	node->next = *headp;
+	node->type = type;
+	node->include = include;
+	node->string = str;
+	*headp = node;
+
+	return 0;
+}
+
+/**
+ * display_fdt_by_regions() - Display regions of an FDT source
+ *
+ * This dumps an FDT as source, but only certain regions of it. This is the
+ * final stage of the grep - we have a list of regions we want to display,
+ * and this function displays them.
+ *
+ * @disp:	Display structure, holding info about our options
+ * @blob:	FDT blob to display
+ * @region:	List of regions to display
+ * @count:	Number of regions
+ */
+static void display_fdt_by_regions(struct display_info *disp, const void *blob,
+				   struct fdt_region region[], int count)
+{
+	struct fdt_region *reg = region, *reg_end = region + count;
+	int base = fdt_off_dt_struct(blob);
+	int nextoffset;
+	int tag, depth, shift;
+	FILE *f = disp->fout;
+	int in_region;
+	int file_ofs;
+
+	depth = nextoffset = 0;
+	shift = 4;	/* 4 spaces per indent */
+	do {
+		const struct fdt_property *prop;
+		const char *name;
+		int offset;
+		int show;
+		int len;
+
+		offset = nextoffset;
+
+		/*
+		 * Work out the file offset of this offset, and decide
+		 * whether it is in the region list or not
+		 */
+		file_ofs = base + offset;
+		if (reg < reg_end && file_ofs >= reg->offset + reg->size)
+			reg++;
+		in_region = reg < reg_end && file_ofs >= reg->offset &&
+				file_ofs < reg->offset + reg->size;
+		tag = fdt_next_tag(blob, offset, &nextoffset);
+
+		if (tag == FDT_END)
+			break;
+		show = in_region;
+
+		if (!show) {
+			/* Do this here to avoid 'if (show)' in every 'case' */
+			if (tag == FDT_BEGIN_NODE)
+				depth++;
+			else if (tag == FDT_END_NODE)
+				depth--;
+			continue;
+		}
+
+		switch (tag) {
+		case FDT_PROP:
+			prop = fdt_get_property_by_offset(blob, offset, NULL);
+			name = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
+			fprintf(f, "%*s%s", depth * shift, "", name);
+			utilfdt_print_data(prop->data,
+					   fdt32_to_cpu(prop->len));
+			fprintf(f, ";");
+			break;
+
+		case FDT_NOP:
+			fprintf(f, "%*s// [NOP]", depth * shift, "");
+			break;
+
+		case FDT_BEGIN_NODE:
+			name = fdt_get_name(blob, offset, &len);
+			fprintf(f, "%*s%s {", depth++ * shift, "",
+			       *name ? name : "/");
+			break;
+
+		case FDT_END_NODE:
+			fprintf(f, "%*s};", --depth * shift, "");
+			break;
+		}
+		fprintf(f, "\n");
+	} while (1);
+}
+
+/**
+ * dump_fdt_regions() - Dump regions of an FDT as binary data
+ *
+ * This dumps an FDT as binary, but only certain regions of it. This is the
+ * final stage of the grep - we have a list of regions we want to dump,
+ * and this function dumps them.
+ *
+ * The output of this function may or may not be a valid FDT. To ensure it
+ * is, these disp->flags must be set:
+ *
+ *   FDT_REG_SUPERNODES: ensures that subnodes are preceded by their
+ *		parents. Without this option, fragments of subnode data may be
+ *		output without the supernodes above them, which is useful for
+ *		hashing but cannot produce a valid FDT.
+ *
+ * @disp:	Display structure, holding info about our options
+ * @blob:	FDT blob to display
+ * @region:	List of regions to display
+ * @count:	Number of regions
+ * @out:	Output destination
+ * @return number of bytes dumped
+ */
+static int dump_fdt_regions(struct display_info *disp, const void *blob,
+		struct fdt_region region[], int count, char *out)
+{
+	unsigned int ptr;
+	int i;
+
+	/* Output all the nodes */
+	ptr = 0;
+	for (i = 0; i < count; i++) {
+		struct fdt_region *reg = &region[i];
+
+		memcpy(out + ptr, (const char *)blob + reg->offset, reg->size);
+		ptr += reg->size;
+	}
+
+	return ptr;
+}
+
+/**
+ * check_type_include() - Check whether to include this type
+ *
+ * See fdt_region_func for its purpose
+ *
+ * @disp:	Display structure, holding info about our options
+ * @type: Type of this part, FDT_IS_...
+ * @data: Pointer to data (full path of node / property name)
+ * @size: Size of data (length of node path / property including \0 terminator
+ * @return 0 to exclude, 1 to include, -1 if no information is available
+ */
+static int check_type_include(struct display_info *disp, int type,
+			      const char *data, int size)
+{
+	struct value_node *val;
+	int match, none_match = FDT_IS_ANY;
+
+	/* If none of our conditions mention this type, we know nothing */
+	debug("type=%x, data=%s\n", type, data ? data : "(null)");
+	if (!((disp->types_inc | disp->types_exc) & type)) {
+		debug("   - not in any condition\n");
+		return -1;
+	}
+
+	/*
+	 * Go through the list of conditions. For inclusive conditions, we
+	 * return 1 at the first match. For exclusive conditions, we must
+	 * check that there are no matches.
+	 */
+	for (val = disp->value_head; val; val = val->next) {
+		if (!(type & val->type))
+			continue;
+		match = fdt_stringlist_contains(data, size, val->string);
+		debug("      - val->type=%x, str='%s', match=%d\n",
+		      val->type, val->string, match);
+		if (match && val->include) {
+			debug("   - match inc %s\n", val->string);
+			return 1;
+		}
+		if (match)
+			none_match &= ~val->type;
+	}
+
+	/*
+	 * If this is an exclusive condition, and nothing matches, then we
+	 * should return 1.
+	 */
+	if ((type & disp->types_exc) && (none_match & type)) {
+		debug("   - match exc\n");
+		return 1;
+	}
+
+	debug("   - no match, types_inc=%x, types_exc=%x, none_match=%x\n",
+	      disp->types_inc, disp->types_exc, none_match);
+
+	return 0;
+}
+
+/**
+ * h_include() - Include handler function for fdt_find_regions()
+ *
+ * This function decides whether to include or exclude a node, property or
+ * compatible string. The function is called by fdt_find_regions().
+ *
+ * The algorithm is documented in check_type_include(). See fdt_region_func for
+ * parameters
+ *
+ * @priv: Pointer to our struct display_info
+ */
+static int h_include(void *priv, const void *fdt, int offset, int type,
+		     const char *data, int size)
+{
+	struct display_info *disp = priv;
+	int inc;
+
+	inc = check_type_include(disp, type, data, size);
+
+	debug("   - returning %d\n", inc);
+
+	return inc;
+}
+
+/**
+ * fdt_find_regions() - Find regiions in a devicetree
+ *
+ * Calls fdt_first_region() and then fdt_next_region() repeatedly until all
+ * regions are found.
+ *
+ * This mirrors fdt_first_region() except that @region is an array of regions
+ * of size @max_regions
+ *
+ * @return number of regions found. If this is more than @max_regions then
+ * only @max_regions regions are returned. The @region array should be expanded
+ * and this function called again.
+ */
+static int fdt_find_regions(const void *fdt,
+		int (*include_func)(void *priv, const void *fdt, int offset,
+				 int type, const char *data, int size),
+		struct display_info *disp, struct fdt_region *region,
+		int max_regions, char *path, int path_len, int flags)
+{
+	struct fdt_region_state state;
+	int count;
+	int ret;
+
+	count = 0;
+	ret = fdt_first_region(fdt, include_func, disp, &region[count++], path,
+			       path_len, disp->flags, &state);
+	while (!ret) {
+		ret = fdt_next_region(fdt, include_func, disp,
+				      count < max_regions ? &region[count] :
+				      NULL, &state);
+		if (!ret)
+			count++;
+	}
+
+	/* If there are no more regions, stop */
+	if (ret != -FDT_ERR_NOTFOUND)
+		return ret;
+
+	return count;
+}
+
+/**
+ * Run the main fdtgrep operation, given a filename and valid arguments
+ *
+ * @disp		Display information / options
+ * @filename	Filename of blob file
+ * @return 0 if ok, -ve on error
+ */
+static int do_fdtgrep(struct display_info *disp, const char *filename)
+{
+	struct fdt_region *region;
+	int max_regions;
+	int count = 100;
+	char path[1024];
+	char *blob;
+	int i, ret;
+
+	blob = utilfdt_read(filename, NULL);
+	if (!blob)
+		return -1;
+	ret = fdt_check_header(blob);
+	if (ret) {
+		fprintf(stderr, "Error: %s\n", fdt_strerror(ret));
+		return ret;
+	}
+
+	/* Allow old files, but they are untested */
+	if (fdt_version(blob) < 17 && disp->value_head) {
+		fprintf(stderr, "Warning: fdtgrep does not fully support version %d files\n",
+			fdt_version(blob));
+	}
+
+	/*
+	 * We do two passes, since we don't know how many regions we need.
+	 * The first pass will count the regions, but if it is too many,
+	 * we do another pass to actually record them.
+	 */
+	for (i = 0; i < 2; i++) {
+		region = malloc(count * sizeof(struct fdt_region));
+		if (!region) {
+			fprintf(stderr, "Out of memory for %d regions\n",
+				count);
+			return -1;
+		}
+		max_regions = count;
+		count = fdt_find_regions(blob,
+				h_include, disp,
+				region, max_regions, path, sizeof(path),
+				disp->flags);
+		if (count < 0) {
+			report_error("fdt_find_regions", count);
+			return -1;
+		}
+		if (count <= max_regions)
+			break;
+		free(region);
+	}
+
+	/* Output either source .dts or binary .dtb */
+	if (disp->output == OUT_DTS) {
+		display_fdt_by_regions(disp, blob, region, count);
+	} else {
+		void *fdt;
+		/* Allow reserved memory section to expand slightly */
+		unsigned int size = fdt_totalsize(blob) + 16;
+
+		fdt = malloc(size);
+		if (!fdt) {
+			fprintf(stderr, "Out_of_memory\n");
+			ret = -1;
+			goto err;
+		}
+		size = dump_fdt_regions(disp, blob, region, count, fdt);
+
+		if (size != fwrite(fdt, 1, size, disp->fout)) {
+			fprintf(stderr, "Write failure, %d bytes\n", size);
+			free(fdt);
+			ret = 1;
+			goto err;
+		}
+		free(fdt);
+	}
+err:
+	free(blob);
+	free(region);
+
+	return ret;
+}
+
+static const char usage_synopsis[] =
+	"fdtgrep - extract portions from device tree binary\n"
+	"\n"
+	"Usage:\n"
+	"	fdtgrep <options> <dt file>|-\n\n"
+	"Output formats are:\n"
+	"\tdts - device tree soure text\n"
+	"\tbin - device tree fragment (may not be a valid .dtb)";
+
+static const char usage_short_opts[] =
+		"hn:N:o:O:p:P:"
+		USAGE_COMMON_SHORT_OPTS;
+static const struct option usage_long_opts[] = {
+	{"include-node",	a_argument, NULL, 'n'},
+	{"exclude-node",	a_argument, NULL, 'N'},
+	{"include-prop",	a_argument, NULL, 'p'},
+	{"exclude-prop",	a_argument, NULL, 'P'},
+	{"out",			a_argument, NULL, 'o'},
+	{"out-format",		a_argument, NULL, 'O'},
+	USAGE_COMMON_LONG_OPTS,
+};
+static const char * const usage_opts_help[] = {
+	"Node to include in grep",
+	"Node to exclude in grep",
+	"Property to include in grep",
+	"Property to exclude in grep",
+	"-o <output file>",
+	"-O <output format>",
+	USAGE_COMMON_OPTS_HELP
+};
+
+static void scan_args(struct display_info *disp, int argc, char *argv[])
+{
+	int opt;
+
+	while ((opt = util_getopt_long()) != EOF) {
+		int type = 0;
+		int inc = 1;
+
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+		case 'N':
+			inc = 0;
+			/* no break */
+		case 'n':
+			type = FDT_IS_NODE;
+			break;
+		case 'o':
+			disp->output_fname = optarg;
+			break;
+		case 'O':
+			if (!strcmp(optarg, "dts"))
+				disp->output = OUT_DTS;
+			else if (!strcmp(optarg, "bin"))
+				disp->output = OUT_BIN;
+			else
+				usage("Unknown output format");
+			break;
+		case 'P':
+			inc = 0;
+			/* no break */
+		case 'p':
+			type = FDT_IS_PROP;
+			break;
+		}
+
+		if (type && value_add(disp, &disp->value_head, type, inc,
+					optarg))
+			usage("Cannot add value");
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	char *filename = NULL;
+	struct display_info disp;
+	int ret;
+
+	/* set defaults */
+	memset(&disp, '\0', sizeof(disp));
+	disp.flags = FDT_REG_SUPERNODES;	/* Default flags */
+
+	scan_args(&disp, argc, argv);
+
+	/* Any additional arguments can match anything, just like -g */
+	while (optind < argc - 1) {
+		if (value_add(&disp, &disp.value_head, FDT_IS_ANY, 1,
+				argv[optind++]))
+			usage("Cannot add value");
+	}
+
+	if (optind < argc)
+		filename = argv[optind++];
+	if (!filename)
+		usage("Missing filename");
+
+	if (disp.output_fname) {
+		disp.fout = fopen(disp.output_fname, "w");
+		if (!disp.fout)
+			usage("Cannot open output file");
+	} else {
+		disp.fout = stdout;
+	}
+
+	/* Run the grep and output the results */
+	ret = do_fdtgrep(&disp, filename);
+	if (disp.output_fname)
+		fclose(disp.fout);
+	if (ret)
+		return 1;
+
+	return 0;
+}
-- 
2.34.0.rc0.344.g81b53c2807-goog


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

* [PATCH v4 4/4] Add tests for fdtgrep
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
                     ` (2 preceding siblings ...)
  2021-11-07 22:43   ` [PATCH v4 3/4] Add fdtgrep to grep and hash FDTs Simon Glass
@ 2021-11-07 22:43   ` Simon Glass
  2022-02-07  4:09   ` [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs David Gibson
  4 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2021-11-07 22:43 UTC (permalink / raw)
  To: Devicetree Compiler; +Cc: David Gibson, Simon Glass

Add some tests to cover the current functionality.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v4:
- Chop back to the minimal useful functionality
- Rebase to master
- Update cover letter for developments over the past 6 years

 tests/grep.dts     |  23 ++++++++++
 tests/run_tests.sh | 111 ++++++++++++++++++++++++++++++++++++++++++++-
 tests/testutils.sh |   1 +
 3 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100644 tests/grep.dts

diff --git a/tests/grep.dts b/tests/grep.dts
new file mode 100644
index 0000000..fb4f55e
--- /dev/null
+++ b/tests/grep.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/dts-v1/;
+/ {
+	model = "MyBoardName";
+	compatible = "MyBoardName", "MyBoardFamilyName";
+	#address-cells = <2>;
+	#size-cells = <2>;
+	chosen {
+		bootargs = "root=/dev/sda2";
+		linux,platform = <0x600>;
+	};
+	holiday {
+		compatible = "ixtapa", "mexico";
+		weather = "sunny";
+		status = "okay";
+		flight-1 {
+			airline = "alaska";
+		};
+		flight-2 {
+			airline = "lan";
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index d100d5a..a32f871 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -1032,6 +1032,112 @@ pylibfdt_tests () {
     tot_tests=$((tot_tests + $total_tests))
 }
 
+# Check the number of lines generated matches what we expect
+# Args:
+#   $1: Expected number of lines
+#   $2...: Command line to run to generate output
+check_lines() {
+    local base="$1"
+
+    shift
+    lines=$($@ | wc -l)
+    if [ "$base" != "$lines" ]; then
+	echo "Expected $base lines but got $lines lines"
+	false
+    fi
+}
+
+# Check the number of bytes generated matches what we expect
+# Args:
+#   $1: Expected number of bytes
+#   $2...: Command line to run to generate output
+check_bytes() {
+    local base="$1"
+
+    shift
+    bytes=$($@ | wc -c)
+    if [ "$base" != "$bytes" ]; then
+	echo "Expected $base bytes but got $bytes bytes"
+	false
+    fi
+}
+
+# Check that $2 and $3 are equal. $1 is the test name to display
+equal_test () {
+    echo -n "$1:	"
+    if [ "$2" == "$3" ]; then
+	PASS
+    else
+	FAIL "$2 != $3"
+    fi
+}
+
+fdtgrep_tests () {
+    local all_lines        # Total source lines in .dts file
+    local dt_start
+    local lines
+    local node_lines       # Number of lines of 'struct' output
+    local tmp
+
+    tmp=/tmp/tests.$$
+
+    # Test up the test file
+    dts=grep.dts
+    dtb=grep.dtb
+    run_dtc_test -O dtb -p 0x1000 -o $dtb $dts
+
+    # Count the number of lines in the source file and the number of lines
+    # that don't relate to nodes (i.e. the SPDX and /dts-v1/ lines)
+    all_lines=$(cat $dts | wc -l)
+    node_lines=$(($all_lines - 2))
+
+    # Tests for each argument are roughly in alphabetical order
+
+    # Test -n
+    run_wrap_test check_lines 0 $DTGREP -n // $dtb
+
+    run_wrap_test check_lines 0 $DTGREP -n chosen $dtb
+    run_wrap_test check_lines 0 $DTGREP -n holiday $dtb
+    run_wrap_test check_lines 0 $DTGREP -n \"\" $dtb
+    run_wrap_test check_lines 6 $DTGREP -n /chosen $dtb
+    run_wrap_test check_lines 7 $DTGREP -n /holiday $dtb
+    run_wrap_test check_lines 11 $DTGREP -n /chosen -n /holiday $dtb
+
+    # Using -n and -N together is undefined, so we don't have tests for that
+    # The same applies for -p/-P
+    run_wrap_error_test $DTGREP -n chosen -N holiday $dtb
+    run_wrap_error_test $DTGREP -p chosen -P holiday $dtb
+
+    # Test -o: this should output just the .dts file to a file
+    # Where there is non-dts output it should go to stdout
+    rm -f $tmp
+    run_wrap_test check_lines 0 $DTGREP $dtb -o $tmp
+    run_wrap_test check_lines $node_lines cat $tmp
+
+    # Test -p: we get the nodes containing these properties
+    run_wrap_test check_lines 6 $DTGREP -p compatible -n / $dtb
+    run_wrap_test check_lines 5 $DTGREP -p bootargs -n / $dtb
+
+    # Now similar tests for -P
+    # First get the number of property lines (containing '=')
+    lines=$(grep "=" $dts |wc -l)
+    run_wrap_test check_lines $(($node_lines - 2)) $DTGREP -P compatible \
+	-n // $dtb
+    run_wrap_test check_lines $(($node_lines - 1)) $DTGREP -P bootargs \
+	-n // $dtb
+    run_wrap_test check_lines $(($node_lines - 3)) $DTGREP -P compatible \
+	-P bootargs -n // $dtb
+
+    # Now some dtb tests. The dts tests above have tested the basic grepping
+    # features so we only need to concern ourselves with things that are
+    # different about dtb/bin output.
+
+    # An empty node list should just give us just the FDT_END tag
+    run_wrap_test check_bytes 4 $DTGREP -n // -O bin $dtb
+
+    rm -f $tmp
+}
+
 while getopts "vt:me" ARG ; do
     case $ARG in
 	"v")
@@ -1050,7 +1156,7 @@ while getopts "vt:me" ARG ; do
 done
 
 if [ -z "$TESTSETS" ]; then
-    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay"
+    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay fdtgrep"
 
     # Test pylibfdt if the libfdt Python module is available.
     if ! $no_python; then
@@ -1090,6 +1196,9 @@ for set in $TESTSETS; do
         "fdtoverlay")
 	    fdtoverlay_tests
 	    ;;
+	"fdtgrep")
+	    fdtgrep_tests
+	    ;;
     esac
 done
 
diff --git a/tests/testutils.sh b/tests/testutils.sh
index 6b2f0d1..3a75c48 100644
--- a/tests/testutils.sh
+++ b/tests/testutils.sh
@@ -27,6 +27,7 @@ DTGET=${TEST_BINDIR}/fdtget
 DTPUT=${TEST_BINDIR}/fdtput
 FDTDUMP=${TEST_BINDIR}/fdtdump
 FDTOVERLAY=${TEST_BINDIR}/fdtoverlay
+DTGREP=${TEST_BINDIR}/fdtgrep
 
 verbose_run () {
     if [ -z "$QUIET_TEST" ]; then
-- 
2.34.0.rc0.344.g81b53c2807-goog


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

* Re: [PATCH v4 1/4] README: Explain how to add a new API function
       [not found]     ` <20211107224346.3181320-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2021-12-28  9:14       ` David Gibson
  0 siblings, 0 replies; 20+ messages in thread
From: David Gibson @ 2021-12-28  9:14 UTC (permalink / raw)
  To: Simon Glass; +Cc: Devicetree Compiler

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

On Sun, Nov 07, 2021 at 03:43:43PM -0700, Simon Glass wrote:
> This is not obvious so add a little note about it.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

This is a good change, regardless of anything else; merged.

> ---
> 
> (no changes since v1)
> 
>  README | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/README b/README
> index d9bf850..bac1714 100644
> --- a/README
> +++ b/README
> @@ -71,6 +71,15 @@ More work remains to support all of libfdt, including access to numeric
>  values.
>  
>  
> +Adding a new function to libfdt.h
> +---------------------------------
> +
> +The shared library uses libfdt/version.lds to list the exported functions, so
> +add your new function there. Check that your function works with pylibfdt. If
> +it cannot be supported, put the declaration in libfdt.h behind #ifndef SWIG so
> +that swig ignores it.
> +
> +
>  Tests
>  -----
>  

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

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

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
                     ` (3 preceding siblings ...)
  2021-11-07 22:43   ` [PATCH v4 4/4] Add tests for fdtgrep Simon Glass
@ 2022-02-07  4:09   ` David Gibson
  2022-02-08 21:43     ` Simon Glass
  4 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2022-02-07  4:09 UTC (permalink / raw)
  To: Simon Glass; +Cc: Devicetree Compiler

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

On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> Note: This was last sent 6 years ago. It really belongs upstream and I
> believe it is useful functionality for libfdt, so I am trying again.
> Please take a fresh look at this. It is cut back from the previous series.
> If accepted we can expand the feature set piece by piece from here.

Sorry it's taken me so long to look at this.  Again.  I can't dispute
that it's useful for certain use cases.  But as for belonging
upstream...

This series adds quite a lot of conceptual complexity.  It introduces
a new data structure, new state structures, a entirely new mode of
working with a tree and a bunch of configuration parameter types on
top of the new entry points and new tool.  I still find the semantics
of the different criteria for inclusion/exclusion from a region pretty
bewildering.

That makes me pretty disinclined to add this to the scope of
maintenance for libfdt.  As you've probably noticed, I'm already
struggling to keep on top of maintenance for the existing libfdt
interfaces.  AFAICT everything here can be implemented fairly
naturally in terms of libfdt's existing public interface. so I'm not
really seeing a compelling reason for this to be merged into libfdt,
rather than being its own separate library that depends on libfdt.

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

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

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
  2022-02-07  4:09   ` [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs David Gibson
@ 2022-02-08 21:43     ` Simon Glass
       [not found]       ` <CAPnjgZ2HZCtfhF2BhUYk0a-UOVrMf0i_uQrbGPhWJoaq1ZtW1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-02-08 21:43 UTC (permalink / raw)
  To: David Gibson; +Cc: Devicetree Compiler

Hi David,

On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > Note: This was last sent 6 years ago. It really belongs upstream and I
> > believe it is useful functionality for libfdt, so I am trying again.
> > Please take a fresh look at this. It is cut back from the previous series.
> > If accepted we can expand the feature set piece by piece from here.
>
> Sorry it's taken me so long to look at this.  Again.  I can't dispute
> that it's useful for certain use cases.  But as for belonging
> upstream...
>
> This series adds quite a lot of conceptual complexity.  It introduces
> a new data structure, new state structures, a entirely new mode of
> working with a tree and a bunch of configuration parameter types on
> top of the new entry points and new tool.  I still find the semantics
> of the different criteria for inclusion/exclusion from a region pretty
> bewildering.

It is sufficient to achieve its purpose, but I don't think it is any
more complex than that.

>
> That makes me pretty disinclined to add this to the scope of
> maintenance for libfdt.  As you've probably noticed, I'm already
> struggling to keep on top of maintenance for the existing libfdt
> interfaces.  AFAICT everything here can be implemented fairly
> naturally in terms of libfdt's existing public interface. so I'm not
> really seeing a compelling reason for this to be merged into libfdt,
> rather than being its own separate library that depends on libfdt.

Are you suggesting:

1. that libfdt should move to a new maintainer
2. that you would accept these patches if someone else maintained them
within the libfdt tree
3. that we set up a separate tree to fork libfdt, with these changes in
4. that we put these changes in a separate tree?

Regards,
Simon

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]       ` <CAPnjgZ2HZCtfhF2BhUYk0a-UOVrMf0i_uQrbGPhWJoaq1ZtW1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-02-09  4:05         ` David Gibson
  2022-02-09 16:04           ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: David Gibson @ 2022-02-09  4:05 UTC (permalink / raw)
  To: Simon Glass; +Cc: Devicetree Compiler

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

On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> Hi David,
> 
> On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > believe it is useful functionality for libfdt, so I am trying again.
> > > Please take a fresh look at this. It is cut back from the previous series.
> > > If accepted we can expand the feature set piece by piece from here.
> >
> > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > that it's useful for certain use cases.  But as for belonging
> > upstream...
> >
> > This series adds quite a lot of conceptual complexity.  It introduces
> > a new data structure, new state structures, a entirely new mode of
> > working with a tree and a bunch of configuration parameter types on
> > top of the new entry points and new tool.  I still find the semantics
> > of the different criteria for inclusion/exclusion from a region pretty
> > bewildering.
> 
> It is sufficient to achieve its purpose, but I don't think it is any
> more complex than that.

I don't disagree, but that still ends up being quite complex.

> > That makes me pretty disinclined to add this to the scope of
> > maintenance for libfdt.  As you've probably noticed, I'm already
> > struggling to keep on top of maintenance for the existing libfdt
> > interfaces.  AFAICT everything here can be implemented fairly
> > naturally in terms of libfdt's existing public interface. so I'm not
> > really seeing a compelling reason for this to be merged into libfdt,
> > rather than being its own separate library that depends on libfdt.
> 
> Are you suggesting:
> 
> 1. that libfdt should move to a new maintainer
> 2. that you would accept these patches if someone else maintained them
> within the libfdt tree
> 3. that we set up a separate tree to fork libfdt, with these changes in
> 4. that we put these changes in a separate tree?

Right now (4) is what I'm suggesting.  Or to be more precise, creating
a new repo with "libfdtrange" or whatever, that depends on libfdt.

(1) and/or (2) are potentially worthy of further discussion.  (3) is
just a bad idea, IMO.

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

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

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
  2022-02-09  4:05         ` David Gibson
@ 2022-02-09 16:04           ` Simon Glass
       [not found]             ` <CAPnjgZ2-tgy-L15S=p4dajpF+hSwX-McPSU3Fez2=cXOqXypZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-02-09 16:04 UTC (permalink / raw)
  To: David Gibson, Rob Herring; +Cc: Devicetree Compiler

+Rob Herring

Hi David and Rob,

On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > Hi David,
> >
> > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > >
> > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > If accepted we can expand the feature set piece by piece from here.
> > >
> > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > that it's useful for certain use cases.  But as for belonging
> > > upstream...
> > >
> > > This series adds quite a lot of conceptual complexity.  It introduces
> > > a new data structure, new state structures, a entirely new mode of
> > > working with a tree and a bunch of configuration parameter types on
> > > top of the new entry points and new tool.  I still find the semantics
> > > of the different criteria for inclusion/exclusion from a region pretty
> > > bewildering.
> >
> > It is sufficient to achieve its purpose, but I don't think it is any
> > more complex than that.
>
> I don't disagree, but that still ends up being quite complex.
>
> > > That makes me pretty disinclined to add this to the scope of
> > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > struggling to keep on top of maintenance for the existing libfdt
> > > interfaces.  AFAICT everything here can be implemented fairly
> > > naturally in terms of libfdt's existing public interface. so I'm not
> > > really seeing a compelling reason for this to be merged into libfdt,
> > > rather than being its own separate library that depends on libfdt.
> >
> > Are you suggesting:
> >
> > 1. that libfdt should move to a new maintainer
> > 2. that you would accept these patches if someone else maintained them
> > within the libfdt tree
> > 3. that we set up a separate tree to fork libfdt, with these changes in
> > 4. that we put these changes in a separate tree?
>
> Right now (4) is what I'm suggesting.  Or to be more precise, creating
> a new repo with "libfdtrange" or whatever, that depends on libfdt.
>
> (1) and/or (2) are potentially worthy of further discussion.  (3) is
> just a bad idea, IMO.

Where are things going with device tree validation in terms of
source-code location? Is it likely there might be a separate tree for
that, which could perhaps hold other libfdt dependent things?

Regards,
Simon

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]             ` <CAPnjgZ2-tgy-L15S=p4dajpF+hSwX-McPSU3Fez2=cXOqXypZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-02-10  0:19               ` Rob Herring
       [not found]                 ` <CAL_JsqJkG84jnn3OiC+aYOdqRvYcMWzfEg4r-74bbFsGC-4img-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-02-10  0:19 UTC (permalink / raw)
  To: Simon Glass; +Cc: David Gibson, Devicetree Compiler

On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> +Rob Herring
>
> Hi David and Rob,
>
> On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > Hi David,
> > >
> > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > >
> > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > If accepted we can expand the feature set piece by piece from here.
> > > >
> > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > that it's useful for certain use cases.  But as for belonging
> > > > upstream...
> > > >
> > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > a new data structure, new state structures, a entirely new mode of
> > > > working with a tree and a bunch of configuration parameter types on
> > > > top of the new entry points and new tool.  I still find the semantics
> > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > bewildering.
> > >
> > > It is sufficient to achieve its purpose, but I don't think it is any
> > > more complex than that.
> >
> > I don't disagree, but that still ends up being quite complex.
> >
> > > > That makes me pretty disinclined to add this to the scope of
> > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > struggling to keep on top of maintenance for the existing libfdt
> > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > rather than being its own separate library that depends on libfdt.
> > >
> > > Are you suggesting:
> > >
> > > 1. that libfdt should move to a new maintainer
> > > 2. that you would accept these patches if someone else maintained them
> > > within the libfdt tree
> > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > 4. that we put these changes in a separate tree?
> >
> > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> >
> > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > just a bad idea, IMO.
>
> Where are things going with device tree validation in terms of
> source-code location?

Right now it doesn't use libfdt at all, but that's what I'm working on
if we can get pieces in place to package pylibfdt sorted out. If not,
I may be doing 3 just for packaging.

> Is it likely there might be a separate tree for
> that, which could perhaps hold other libfdt dependent things?

It is a separate tree[1], but it's a pure python project and I don't
think it's the right place for a C library. But we can setup a new
project in the GH devicetree.org group[2] if you want.

Rob

[1] https://github.com/devicetree-org/dt-schema/
[2] https://github.com/devicetree-org/

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                 ` <CAL_JsqJkG84jnn3OiC+aYOdqRvYcMWzfEg4r-74bbFsGC-4img-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-02-17  5:44                   ` David Gibson
  2022-02-17 20:37                   ` Simon Glass
  1 sibling, 0 replies; 20+ messages in thread
From: David Gibson @ 2022-02-17  5:44 UTC (permalink / raw)
  To: Rob Herring; +Cc: Simon Glass, Devicetree Compiler

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

On Wed, Feb 09, 2022 at 06:19:01PM -0600, Rob Herring wrote:
> On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > +Rob Herring
> >
> > Hi David and Rob,
> >
> > On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > >
> > > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > > Hi David,
> > > >
> > > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6ryscVyMRj84@public.gmane.org.au> wrote:
> > > > >
> > > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > > If accepted we can expand the feature set piece by piece from here.
> > > > >
> > > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > > that it's useful for certain use cases.  But as for belonging
> > > > > upstream...
> > > > >
> > > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > > a new data structure, new state structures, a entirely new mode of
> > > > > working with a tree and a bunch of configuration parameter types on
> > > > > top of the new entry points and new tool.  I still find the semantics
> > > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > > bewildering.
> > > >
> > > > It is sufficient to achieve its purpose, but I don't think it is any
> > > > more complex than that.
> > >
> > > I don't disagree, but that still ends up being quite complex.
> > >
> > > > > That makes me pretty disinclined to add this to the scope of
> > > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > > struggling to keep on top of maintenance for the existing libfdt
> > > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > > rather than being its own separate library that depends on libfdt.
> > > >
> > > > Are you suggesting:
> > > >
> > > > 1. that libfdt should move to a new maintainer
> > > > 2. that you would accept these patches if someone else maintained them
> > > > within the libfdt tree
> > > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > > 4. that we put these changes in a separate tree?
> > >
> > > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> > >
> > > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > > just a bad idea, IMO.
> >
> > Where are things going with device tree validation in terms of
> > source-code location?
> 
> Right now it doesn't use libfdt at all, but that's what I'm working on
> if we can get pieces in place to package pylibfdt sorted out. If not,
> I may be doing 3 just for packaging.

It doesn't seem to me that the validation should be using libfdt,
except in a limited capacity for an initial read.  Schema checking is
likely to do lots of random access to various nodes and properties, so
pre-reading the dtb into a "live" tree format seems like it would be a
good idea.

> > Is it likely there might be a separate tree for
> > that, which could perhaps hold other libfdt dependent things?
> 
> It is a separate tree[1], but it's a pure python project and I don't
> think it's the right place for a C library. But we can setup a new
> project in the GH devicetree.org group[2] if you want.
> 
> Rob
> 
> [1] https://github.com/devicetree-org/dt-schema/
> [2] https://github.com/devicetree-org/
> 

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

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

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                 ` <CAL_JsqJkG84jnn3OiC+aYOdqRvYcMWzfEg4r-74bbFsGC-4img-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2022-02-17  5:44                   ` David Gibson
@ 2022-02-17 20:37                   ` Simon Glass
       [not found]                     ` <CAPnjgZ2OjgXxQ0HxC_pBe+yZnmbJL-QYd+jDDQ5eacO=vkerNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-02-17 20:37 UTC (permalink / raw)
  To: Rob Herring; +Cc: David Gibson, Devicetree Compiler

Hi Rob,

On Wed, 9 Feb 2022 at 17:19, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > +Rob Herring
> >
> > Hi David and Rob,
> >
> > On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > >
> > > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > > Hi David,
> > > >
> > > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > >
> > > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > > If accepted we can expand the feature set piece by piece from here.
> > > > >
> > > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > > that it's useful for certain use cases.  But as for belonging
> > > > > upstream...
> > > > >
> > > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > > a new data structure, new state structures, a entirely new mode of
> > > > > working with a tree and a bunch of configuration parameter types on
> > > > > top of the new entry points and new tool.  I still find the semantics
> > > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > > bewildering.
> > > >
> > > > It is sufficient to achieve its purpose, but I don't think it is any
> > > > more complex than that.
> > >
> > > I don't disagree, but that still ends up being quite complex.
> > >
> > > > > That makes me pretty disinclined to add this to the scope of
> > > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > > struggling to keep on top of maintenance for the existing libfdt
> > > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > > rather than being its own separate library that depends on libfdt.
> > > >
> > > > Are you suggesting:
> > > >
> > > > 1. that libfdt should move to a new maintainer
> > > > 2. that you would accept these patches if someone else maintained them
> > > > within the libfdt tree
> > > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > > 4. that we put these changes in a separate tree?
> > >
> > > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> > >
> > > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > > just a bad idea, IMO.
> >
> > Where are things going with device tree validation in terms of
> > source-code location?
>
> Right now it doesn't use libfdt at all, but that's what I'm working on
> if we can get pieces in place to package pylibfdt sorted out. If not,
> I may be doing 3 just for packaging.
>
> > Is it likely there might be a separate tree for
> > that, which could perhaps hold other libfdt dependent things?
>
> It is a separate tree[1], but it's a pure python project and I don't
> think it's the right place for a C library. But we can setup a new
> project in the GH devicetree.org group[2] if you want.

If David has no objection, then I think that would be a good idea,
please. I can add the fdtgrep tool as well as the new fdt_sign stuff
(once we get some thoughts on how exactly that should work).

Regards,
SImon

>
> Rob
>
> [1] https://github.com/devicetree-org/dt-schema/
> [2] https://github.com/devicetree-org/

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                     ` <CAPnjgZ2OjgXxQ0HxC_pBe+yZnmbJL-QYd+jDDQ5eacO=vkerNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-02-17 22:09                       ` Rob Herring
       [not found]                         ` <CAL_JsqLkOMELMjCNJ_xSt7hdVvHx5dMfrifzRWFuZQkGcsXwjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-02-17 22:09 UTC (permalink / raw)
  To: Simon Glass; +Cc: David Gibson, Devicetree Compiler

On Thu, Feb 17, 2022 at 2:37 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> Hi Rob,
>
> On Wed, 9 Feb 2022 at 17:19, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >
> > On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > >
> > > +Rob Herring
> > >
> > > Hi David and Rob,
> > >
> > > On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > >
> > > > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > > > Hi David,
> > > > >
> > > > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > > >
> > > > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > > > If accepted we can expand the feature set piece by piece from here.
> > > > > >
> > > > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > > > that it's useful for certain use cases.  But as for belonging
> > > > > > upstream...
> > > > > >
> > > > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > > > a new data structure, new state structures, a entirely new mode of
> > > > > > working with a tree and a bunch of configuration parameter types on
> > > > > > top of the new entry points and new tool.  I still find the semantics
> > > > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > > > bewildering.
> > > > >
> > > > > It is sufficient to achieve its purpose, but I don't think it is any
> > > > > more complex than that.
> > > >
> > > > I don't disagree, but that still ends up being quite complex.
> > > >
> > > > > > That makes me pretty disinclined to add this to the scope of
> > > > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > > > struggling to keep on top of maintenance for the existing libfdt
> > > > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > > > rather than being its own separate library that depends on libfdt.
> > > > >
> > > > > Are you suggesting:
> > > > >
> > > > > 1. that libfdt should move to a new maintainer
> > > > > 2. that you would accept these patches if someone else maintained them
> > > > > within the libfdt tree
> > > > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > > > 4. that we put these changes in a separate tree?
> > > >
> > > > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > > > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> > > >
> > > > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > > > just a bad idea, IMO.
> > >
> > > Where are things going with device tree validation in terms of
> > > source-code location?
> >
> > Right now it doesn't use libfdt at all, but that's what I'm working on
> > if we can get pieces in place to package pylibfdt sorted out. If not,
> > I may be doing 3 just for packaging.
> >
> > > Is it likely there might be a separate tree for
> > > that, which could perhaps hold other libfdt dependent things?
> >
> > It is a separate tree[1], but it's a pure python project and I don't
> > think it's the right place for a C library. But we can setup a new
> > project in the GH devicetree.org group[2] if you want.
>
> If David has no objection, then I think that would be a good idea,
> please. I can add the fdtgrep tool as well as the new fdt_sign stuff
> (once we get some thoughts on how exactly that should work).

What do you want it called and I can setup an empty repo. Or once you
have a tree in place, I can add it.

Rob

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                         ` <CAL_JsqLkOMELMjCNJ_xSt7hdVvHx5dMfrifzRWFuZQkGcsXwjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-02-17 22:44                           ` Simon Glass
       [not found]                             ` <CAPnjgZ1TM751YD-VOgk-+Kc--shAt4=1FaFdTyGGn5_27P0s0g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-02-17 22:44 UTC (permalink / raw)
  To: Rob Herring; +Cc: David Gibson, Devicetree Compiler

Hi Rob,

I am open to ideas...perhaps dtc-extra?

Yes, empty is fine.

Regards,
Simon

On Thu, 17 Feb 2022 at 15:09, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Thu, Feb 17, 2022 at 2:37 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > Hi Rob,
> >
> > On Wed, 9 Feb 2022 at 17:19, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > >
> > > On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > >
> > > > +Rob Herring
> > > >
> > > > Hi David and Rob,
> > > >
> > > > On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > >
> > > > > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > > > > Hi David,
> > > > > >
> > > > > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > > > >
> > > > > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > > > > If accepted we can expand the feature set piece by piece from here.
> > > > > > >
> > > > > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > > > > that it's useful for certain use cases.  But as for belonging
> > > > > > > upstream...
> > > > > > >
> > > > > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > > > > a new data structure, new state structures, a entirely new mode of
> > > > > > > working with a tree and a bunch of configuration parameter types on
> > > > > > > top of the new entry points and new tool.  I still find the semantics
> > > > > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > > > > bewildering.
> > > > > >
> > > > > > It is sufficient to achieve its purpose, but I don't think it is any
> > > > > > more complex than that.
> > > > >
> > > > > I don't disagree, but that still ends up being quite complex.
> > > > >
> > > > > > > That makes me pretty disinclined to add this to the scope of
> > > > > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > > > > struggling to keep on top of maintenance for the existing libfdt
> > > > > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > > > > rather than being its own separate library that depends on libfdt.
> > > > > >
> > > > > > Are you suggesting:
> > > > > >
> > > > > > 1. that libfdt should move to a new maintainer
> > > > > > 2. that you would accept these patches if someone else maintained them
> > > > > > within the libfdt tree
> > > > > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > > > > 4. that we put these changes in a separate tree?
> > > > >
> > > > > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > > > > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> > > > >
> > > > > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > > > > just a bad idea, IMO.
> > > >
> > > > Where are things going with device tree validation in terms of
> > > > source-code location?
> > >
> > > Right now it doesn't use libfdt at all, but that's what I'm working on
> > > if we can get pieces in place to package pylibfdt sorted out. If not,
> > > I may be doing 3 just for packaging.
> > >
> > > > Is it likely there might be a separate tree for
> > > > that, which could perhaps hold other libfdt dependent things?
> > >
> > > It is a separate tree[1], but it's a pure python project and I don't
> > > think it's the right place for a C library. But we can setup a new
> > > project in the GH devicetree.org group[2] if you want.
> >
> > If David has no objection, then I think that would be a good idea,
> > please. I can add the fdtgrep tool as well as the new fdt_sign stuff
> > (once we get some thoughts on how exactly that should work).
>
> What do you want it called and I can setup an empty repo. Or once you
> have a tree in place, I can add it.
>
> Rob

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                             ` <CAPnjgZ1TM751YD-VOgk-+Kc--shAt4=1FaFdTyGGn5_27P0s0g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-03-12  2:24                               ` Simon Glass
       [not found]                                 ` <CAPnjgZ2HNTfdBz0jp0sqFhFN=sduTjkJR-5uVABvfZ_+O3Qj6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-03-12  2:24 UTC (permalink / raw)
  To: Rob Herring; +Cc: David Gibson, Devicetree Compiler

Hi Rob,

On Thu, 17 Feb 2022 at 15:44, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> Hi Rob,
>
> I am open to ideas...perhaps dtc-extra?
>
> Yes, empty is fine.

Did anything happen here?


Regards,
Simon

>
> On Thu, 17 Feb 2022 at 15:09, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >
> > On Thu, Feb 17, 2022 at 2:37 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Wed, 9 Feb 2022 at 17:19, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > >
> > > > On Wed, Feb 9, 2022 at 10:04 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > > >
> > > > > +Rob Herring
> > > > >
> > > > > Hi David and Rob,
> > > > >
> > > > > On Tue, 8 Feb 2022 at 23:31, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > > >
> > > > > > On Tue, Feb 08, 2022 at 02:43:44PM -0700, Simon Glass wrote:
> > > > > > > Hi David,
> > > > > > >
> > > > > > > On Sun, 6 Feb 2022 at 21:10, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > > > > >
> > > > > > > > On Sun, Nov 07, 2021 at 03:43:42PM -0700, Simon Glass wrote:
> > > > > > > > > Note: This was last sent 6 years ago. It really belongs upstream and I
> > > > > > > > > believe it is useful functionality for libfdt, so I am trying again.
> > > > > > > > > Please take a fresh look at this. It is cut back from the previous series.
> > > > > > > > > If accepted we can expand the feature set piece by piece from here.
> > > > > > > >
> > > > > > > > Sorry it's taken me so long to look at this.  Again.  I can't dispute
> > > > > > > > that it's useful for certain use cases.  But as for belonging
> > > > > > > > upstream...
> > > > > > > >
> > > > > > > > This series adds quite a lot of conceptual complexity.  It introduces
> > > > > > > > a new data structure, new state structures, a entirely new mode of
> > > > > > > > working with a tree and a bunch of configuration parameter types on
> > > > > > > > top of the new entry points and new tool.  I still find the semantics
> > > > > > > > of the different criteria for inclusion/exclusion from a region pretty
> > > > > > > > bewildering.
> > > > > > >
> > > > > > > It is sufficient to achieve its purpose, but I don't think it is any
> > > > > > > more complex than that.
> > > > > >
> > > > > > I don't disagree, but that still ends up being quite complex.
> > > > > >
> > > > > > > > That makes me pretty disinclined to add this to the scope of
> > > > > > > > maintenance for libfdt.  As you've probably noticed, I'm already
> > > > > > > > struggling to keep on top of maintenance for the existing libfdt
> > > > > > > > interfaces.  AFAICT everything here can be implemented fairly
> > > > > > > > naturally in terms of libfdt's existing public interface. so I'm not
> > > > > > > > really seeing a compelling reason for this to be merged into libfdt,
> > > > > > > > rather than being its own separate library that depends on libfdt.
> > > > > > >
> > > > > > > Are you suggesting:
> > > > > > >
> > > > > > > 1. that libfdt should move to a new maintainer
> > > > > > > 2. that you would accept these patches if someone else maintained them
> > > > > > > within the libfdt tree
> > > > > > > 3. that we set up a separate tree to fork libfdt, with these changes in
> > > > > > > 4. that we put these changes in a separate tree?
> > > > > >
> > > > > > Right now (4) is what I'm suggesting.  Or to be more precise, creating
> > > > > > a new repo with "libfdtrange" or whatever, that depends on libfdt.
> > > > > >
> > > > > > (1) and/or (2) are potentially worthy of further discussion.  (3) is
> > > > > > just a bad idea, IMO.
> > > > >
> > > > > Where are things going with device tree validation in terms of
> > > > > source-code location?
> > > >
> > > > Right now it doesn't use libfdt at all, but that's what I'm working on
> > > > if we can get pieces in place to package pylibfdt sorted out. If not,
> > > > I may be doing 3 just for packaging.
> > > >
> > > > > Is it likely there might be a separate tree for
> > > > > that, which could perhaps hold other libfdt dependent things?
> > > >
> > > > It is a separate tree[1], but it's a pure python project and I don't
> > > > think it's the right place for a C library. But we can setup a new
> > > > project in the GH devicetree.org group[2] if you want.
> > >
> > > If David has no objection, then I think that would be a good idea,
> > > please. I can add the fdtgrep tool as well as the new fdt_sign stuff
> > > (once we get some thoughts on how exactly that should work).
> >
> > What do you want it called and I can setup an empty repo. Or once you
> > have a tree in place, I can add it.
> >
> > Rob

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                                 ` <CAPnjgZ2HNTfdBz0jp0sqFhFN=sduTjkJR-5uVABvfZ_+O3Qj6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-03-14 16:22                                   ` Rob Herring
       [not found]                                     ` <CAL_Jsq+KzcUasbCOagXoLopDrNHrQQ5j0nE+CsgK_W1f8y9+0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-03-14 16:22 UTC (permalink / raw)
  To: Simon Glass; +Cc: David Gibson, Devicetree Compiler

On Fri, Mar 11, 2022 at 7:24 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> Hi Rob,
>
> On Thu, 17 Feb 2022 at 15:44, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > Hi Rob,
> >
> > I am open to ideas...perhaps dtc-extra?
> >
> > Yes, empty is fine.
>
> Did anything happen here?

No, sorry. It's not really dtc related, so perhaps 'fdt-tools' or
'fdt-utils' instead?

Rob

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                                     ` <CAL_Jsq+KzcUasbCOagXoLopDrNHrQQ5j0nE+CsgK_W1f8y9+0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-03-14 16:38                                       ` Simon Glass
       [not found]                                         ` <CAPnjgZ0cf=qVS=C-mno_0aB0gGXYu06LBoVp1n8QQT=mFfaj9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2022-03-14 16:38 UTC (permalink / raw)
  To: Rob Herring; +Cc: David Gibson, Devicetree Compiler

Hi Rob,

On Mon, 14 Mar 2022 at 10:22, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Fri, Mar 11, 2022 at 7:24 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > Hi Rob,
> >
> > On Thu, 17 Feb 2022 at 15:44, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > >
> > > Hi Rob,
> > >
> > > I am open to ideas...perhaps dtc-extra?
> > >
> > > Yes, empty is fine.
> >
> > Did anything happen here?
>
> No, sorry. It's not really dtc related, so perhaps 'fdt-tools' or
> 'fdt-utils' instead?

Yes, either of those is fine with me.

Regards,
Simon

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                                         ` <CAPnjgZ0cf=qVS=C-mno_0aB0gGXYu06LBoVp1n8QQT=mFfaj9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-03-14 17:11                                           ` Rob Herring
       [not found]                                             ` <CAL_JsqLMUrEoNXh9y4CpOgbcSsK1SRUPBoeDvd4+Vx9pkyXO2Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2022-03-14 17:11 UTC (permalink / raw)
  To: Simon Glass; +Cc: David Gibson, Devicetree Compiler

On Mon, Mar 14, 2022 at 10:38 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>
> Hi Rob,
>
> On Mon, 14 Mar 2022 at 10:22, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >
> > On Fri, Mar 11, 2022 at 7:24 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Thu, 17 Feb 2022 at 15:44, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > I am open to ideas...perhaps dtc-extra?
> > > >
> > > > Yes, empty is fine.
> > >
> > > Did anything happen here?
> >
> > No, sorry. It's not really dtc related, so perhaps 'fdt-tools' or
> > 'fdt-utils' instead?
>
> Yes, either of those is fine with me.

I created fdt-tools. Just need you to accept DT group membership and I
can give you write permission.

Rob

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

* Re: [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs
       [not found]                                             ` <CAL_JsqLMUrEoNXh9y4CpOgbcSsK1SRUPBoeDvd4+Vx9pkyXO2Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2022-03-14 17:44                                               ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2022-03-14 17:44 UTC (permalink / raw)
  To: Rob Herring; +Cc: David Gibson, Devicetree Compiler

Hi Rob,

On Mon, 14 Mar 2022 at 11:11, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Mon, Mar 14, 2022 at 10:38 AM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > Hi Rob,
> >
> > On Mon, 14 Mar 2022 at 10:22, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > >
> > > On Fri, Mar 11, 2022 at 7:24 PM Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Thu, 17 Feb 2022 at 15:44, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > I am open to ideas...perhaps dtc-extra?
> > > > >
> > > > > Yes, empty is fine.
> > > >
> > > > Did anything happen here?
> > >
> > > No, sorry. It's not really dtc related, so perhaps 'fdt-tools' or
> > > 'fdt-utils' instead?
> >
> > Yes, either of those is fine with me.
>
> I created fdt-tools. Just need you to accept DT group membership and I
> can give you write permission.

OK, done.

Regards,
Simon

> Rob

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

end of thread, other threads:[~2022-03-14 17:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-07 22:43 [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs Simon Glass
     [not found] ` <20211107224346.3181320-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2021-11-07 22:43   ` [PATCH v4 1/4] README: Explain how to add a new API function Simon Glass
     [not found]     ` <20211107224346.3181320-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2021-12-28  9:14       ` David Gibson
2021-11-07 22:43   ` [PATCH v4 2/4] libfdt: Add functions to find regions in an FDT Simon Glass
2021-11-07 22:43   ` [PATCH v4 3/4] Add fdtgrep to grep and hash FDTs Simon Glass
2021-11-07 22:43   ` [PATCH v4 4/4] Add tests for fdtgrep Simon Glass
2022-02-07  4:09   ` [PATCH v4 0/4] Introduce fdtgrep for subsetting and hashing FDTs David Gibson
2022-02-08 21:43     ` Simon Glass
     [not found]       ` <CAPnjgZ2HZCtfhF2BhUYk0a-UOVrMf0i_uQrbGPhWJoaq1ZtW1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-09  4:05         ` David Gibson
2022-02-09 16:04           ` Simon Glass
     [not found]             ` <CAPnjgZ2-tgy-L15S=p4dajpF+hSwX-McPSU3Fez2=cXOqXypZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-10  0:19               ` Rob Herring
     [not found]                 ` <CAL_JsqJkG84jnn3OiC+aYOdqRvYcMWzfEg4r-74bbFsGC-4img-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-17  5:44                   ` David Gibson
2022-02-17 20:37                   ` Simon Glass
     [not found]                     ` <CAPnjgZ2OjgXxQ0HxC_pBe+yZnmbJL-QYd+jDDQ5eacO=vkerNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-17 22:09                       ` Rob Herring
     [not found]                         ` <CAL_JsqLkOMELMjCNJ_xSt7hdVvHx5dMfrifzRWFuZQkGcsXwjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-02-17 22:44                           ` Simon Glass
     [not found]                             ` <CAPnjgZ1TM751YD-VOgk-+Kc--shAt4=1FaFdTyGGn5_27P0s0g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-03-12  2:24                               ` Simon Glass
     [not found]                                 ` <CAPnjgZ2HNTfdBz0jp0sqFhFN=sduTjkJR-5uVABvfZ_+O3Qj6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-03-14 16:22                                   ` Rob Herring
     [not found]                                     ` <CAL_Jsq+KzcUasbCOagXoLopDrNHrQQ5j0nE+CsgK_W1f8y9+0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-03-14 16:38                                       ` Simon Glass
     [not found]                                         ` <CAPnjgZ0cf=qVS=C-mno_0aB0gGXYu06LBoVp1n8QQT=mFfaj9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-03-14 17:11                                           ` Rob Herring
     [not found]                                             ` <CAL_JsqLMUrEoNXh9y4CpOgbcSsK1SRUPBoeDvd4+Vx9pkyXO2Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-03-14 17:44                                               ` Simon Glass

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