[PATCH 0/2] Make finding fixup nodes more robust

[PATCH 0/2] Make finding fixup nodes more robust

[Sascha Hauer]

Recently the node names in i.MX6 device tree files have changed. The
leading zeroes were removed from the addresses. With this a newer
barebox doesn't find the nodes to fixup in an old device tree and
an older barebox doesn't find the nodes in a new device tree. To
fix this we create new names of the nodes based on things that hopefully
don't change: We iterate up to a node which is MMIO space mappable
and get a name that starts with "[adr]". For the remaining parts of
a node we add "<reg>" when the hierarchy level has a "reg" property
and only as last fallback we add "{nodename}" in curly brackets.
This name is safe against node name changes and also against adding
a new hierarchy level in the device tree (when for example a bus level
is added).

This series aims for v2018.01.1

Sascha Hauer (2):
  of: Add function to find the same node in another tree
  of: mtd: partition: Use reproducible node names for fixup

 drivers/of/base.c      | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/partition.c |  5 +++-
 include/of.h           |  5 +++-
 3 files changed, 71 insertions(+), 2 deletions(-)

-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] of: Add function to find the same node in another tree

[Sascha Hauer]

For our device tree fixups we have to find a node corresponding to
another node in another device tree. We used to use the full name
to match the nodes, but this falls apart when nodes get renamed
or for example a new bus hierarchy is introduced. To make this
more robust we create reproducible names from device nodes which
mostly depend on the address in MMIO space, the reg property and
as a last resort the name of the device node.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/base.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/of.h      |  5 ++++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6a582177bf..c2c7afff9b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2173,6 +2173,69 @@ int of_device_disable_path(const char *path)
 	return of_device_disable(node);
 }
 
+/**
+ * of_get_reproducible_name() - get a reproducible name of a node
+ * @node: The node to get a name from
+ *
+ * This function constructs a reproducible name for a node. This name can be
+ * used to find the same node in another device tree. The name is constructed
+ * from different patterns which are appended to each other.
+ * - If a node has no "reg" property, the name of the node is used in angle
+ *   brackets, prepended with the result of the parent node
+ * - If the parent node has a "ranges" property then the address in MMIO space
+ *   is used in square brackets
+ * - If a node has a "reg" property, but is not translatable in MMIO space then
+ *   the start address is used in curly brackets, prepended with the result of
+ *   the parent node.
+ *
+ * Returns a dynamically allocated string containing the name
+ */
+char *of_get_reproducible_name(struct device_node *node)
+{
+	const __be32 *reg;
+	u64 addr;
+	u64 offset;
+	int na;
+	char *str, *res;
+
+	if (!node)
+		return 0;
+
+	reg = of_get_property(node, "reg", NULL);
+        if (!reg) {
+		str = of_get_reproducible_name(node->parent);
+		res = basprintf("%s<%s>", str, node->name);
+		free(str);
+		return res;
+	}
+
+	if (node->parent && of_get_property(node->parent, "ranges", NULL)) {
+		addr = of_translate_address(node, reg);
+		return basprintf("[0x%llx]", addr);
+	}
+
+	na = of_n_addr_cells(node);
+
+	offset = of_read_number(reg, na);
+
+	str = of_get_reproducible_name(node->parent);
+	res = basprintf("%s{%llx}", str, offset);
+	free(str);
+
+	return res;
+}
+
+struct device_node *of_find_node_by_reproducible_name(struct device_node *from,
+						      const char *name)
+{
+	struct device_node *np;
+
+	of_tree_for_each_node_from(np, from)
+		if (!of_node_cmp(of_get_reproducible_name(np), name))
+			return np;
+	return NULL;
+}
+
 /**
  * of_graph_parse_endpoint() - parse common endpoint node properties
  * @node: pointer to endpoint device_node
diff --git a/include/of.h b/include/of.h
index 1b9719d603..4564f48514 100644
--- a/include/of.h
+++ b/include/of.h
@@ -164,7 +164,10 @@ extern int of_get_child_count(const struct device_node *parent);
 extern int of_get_available_child_count(const struct device_node *parent);
 extern struct device_node *of_get_child_by_name(const struct device_node *node,
 					const char *name);
-
+extern char *of_get_reproducible_name(struct device_node *node);
+extern struct device_node *of_find_node_by_reproducible_name(struct device_node
+							     *from,
+							     const char *name);
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] of: mtd: partition: Use reproducible node names for fixup

[Sascha Hauer]

The full names of device nodes are not as stable as we like them
to be. Lately the leading zeroes in the i.MX6 device trees were
removed which led to the result that we can no longer find the
partition nodes in the to be fixed tree. Use reproducible names
to overcome this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/of/partition.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index ac20490231..6f9651a9e4 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -130,6 +130,7 @@ static int of_partition_fixup(struct device_node *root, void *ctx)
 {
 	struct cdev *cdev = ctx, *partcdev;
 	struct device_node *np, *part, *partnode;
+	char *name;
 	int ret;
 	int n_cells, n_parts = 0;
 
@@ -153,7 +154,9 @@ static int of_partition_fixup(struct device_node *root, void *ctx)
 	else
 		n_cells = 1;
 
-	np = of_find_node_by_path_from(root, cdev->device_node->full_name);
+	name = of_get_reproducible_name(cdev->device_node);
+	np = of_find_node_by_reproducible_name(root, name);
+	free(name);
 	if (!np) {
 		dev_err(cdev->dev, "Cannot find nodepath %s, cannot fixup\n",
 				cdev->device_node->full_name);
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] Make finding fixup nodes more robust

[Sam Ravnborg]

Hi Sasha.

On Fri, Jan 26, 2018 at 08:26:42PM +0100, Sascha Hauer wrote:
> Recently the node names in i.MX6 device tree files have changed. The
> leading zeroes were removed from the addresses. With this a newer
> barebox doesn't find the nodes to fixup in an old device tree and
> an older barebox doesn't find the nodes in a new device tree. To
> fix this we create new names of the nodes based on things that hopefully
> don't change: We iterate up to a node which is MMIO space mappable
> and get a name that starts with "[adr]". For the remaining parts of
> a node we add "<reg>" when the hierarchy level has a "reg" property
> and only as last fallback we add "{nodename}" in curly brackets.
> This name is safe against node name changes and also against adding
> a new hierarchy level in the device tree (when for example a bus level
> is added).
I think I was bitten by something similar while toying with display
timings.

So I think commands/of_display_timings.c would benefit
from a conversion to this new scheme too - as well as the mtd stuff
in of/partition.c

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] Make finding fixup nodes more robust

[Sascha Hauer]

Hi Sam,

On Fri, Jan 26, 2018 at 09:46:32PM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> On Fri, Jan 26, 2018 at 08:26:42PM +0100, Sascha Hauer wrote:
> > Recently the node names in i.MX6 device tree files have changed. The
> > leading zeroes were removed from the addresses. With this a newer
> > barebox doesn't find the nodes to fixup in an old device tree and
> > an older barebox doesn't find the nodes in a new device tree. To
> > fix this we create new names of the nodes based on things that hopefully
> > don't change: We iterate up to a node which is MMIO space mappable
> > and get a name that starts with "[adr]". For the remaining parts of
> > a node we add "<reg>" when the hierarchy level has a "reg" property
> > and only as last fallback we add "{nodename}" in curly brackets.
> > This name is safe against node name changes and also against adding
> > a new hierarchy level in the device tree (when for example a bus level
> > is added).
> I think I was bitten by something similar while toying with display
> timings.
> 
> So I think commands/of_display_timings.c would benefit
> from a conversion to this new scheme too - as well as the mtd stuff
> in of/partition.c

of/partition.c is fixed in this series. I haven't looked around what
else could benefit from this. Could you try it for the display timing?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox