[coreboot-gerrit] Change in ...coreboot[master]: device_tree: Have absolute paths start with '/'

[Julius Werner (Code Review)]

Hello Patrick Rudolph,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/32865

to review the following change.

Change subject: device_tree: Have absolute paths start with '/'
......................................................................

device_tree: Have absolute paths start with '/'

Currently DT paths are *not* expected to start with '/'. This is not
what the spec says (see Devicetree Specification v0.2, 2.2.3 Path Names)
and also not what is done by Linux.

Change dt_find_node_by_path() to expect paths to start with '/' and add
a leading '/' to all DT path strings. Besides the compatibility with the
spec this change is also needed to support aliases in the future.

This patch was adapted from depthcharge's http://crosreview.com/1252770

Change-Id: Ibdf59ccbb4ead38c6193b630642fd1f1e847dd89
Signed-off-by: Julius Werner 
---
M src/lib/device_tree.c
M src/soc/cavium/cn81xx/soc.c
2 files changed, 9 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/32865/1

diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c
index 3e1dd35..e26021e 100644
--- a/src/lib/device_tree.c
+++ b/src/lib/device_tree.c
@@ -556,7 +556,7 @@
  *
  * @param tree		The device tree to search.
  * @param path          A string representing a path in the device tree, with
- *			nodes separated by '/'. Example: "soc/firmware/coreboot"
+ *			nodes separated by '/'. Example: "/firmware/coreboot"
  * @param addrcp	Pointer that will be updated with any #address-cells
  *			value found in the path. May be NULL to ignore.
  * @param sizecp	Pointer that will be updated with any #size-cells
@@ -565,13 +565,13 @@
  * @return		The found/created node, or NULL.
  *
  * It is the caller responsibility to provide the correct path string, namely
- * not starting or ending with a '/', and not having "//" anywhere in it.
+ * starting with a '/', not ending in a '/' and not having "//" anywhere in it.
  */
 struct device_tree_node *dt_find_node_by_path(struct device_tree *tree,
    				      const char *path, u32 *addrcp,
    				      u32 *sizecp, int create)
 {
-	char *dup_path = strdup(path);
+	char *dup_path = strdup(&path[1]);	/* remove leading '/' */
    /* Hopefully enough depth for any node. */
    const char *path_array[15];
    int i;
@@ -995,7 +995,7 @@
    struct device_tree_node *reserved;
    u32 addr = 0, size = 0;

-	reserved = dt_find_node_by_path(tree, "reserved-memory", &addr,
+	reserved = dt_find_node_by_path(tree, "/reserved-memory", &addr,
    				&size, 1);
    if (!reserved)
    	return NULL;
diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c
index 20542e5..efe7d6d 100644
--- a/src/soc/cavium/cn81xx/soc.c
+++ b/src/soc/cavium/cn81xx/soc.c
@@ -185,7 +185,7 @@
    size_t i;

/* Set the sclk clock rate. */
-	dt_node = dt_find_node_by_path(tree, "soc@0/sclk", NULL, NULL, 0);
+	dt_node = dt_find_node_by_path(tree, "/soc@0/sclk", NULL, NULL, 0);
    if (dt_node) {
    	const u32 freq = thunderx_get_io_clock();
    	printk(BIOS_INFO, "%s: Set SCLK to %u Hz\n", __func__, freq);
@@ -195,7 +195,7 @@
    	       __func__);

/* Set refclkuaa clock rate. */
-	dt_node = dt_find_node_by_path(tree, "soc@0/refclkuaa", NULL,
+	dt_node = dt_find_node_by_path(tree, "/soc@0/refclkuaa", NULL,
    			       NULL, 0);
    if (dt_node) {
    	const u32 freq = uart_platform_refclk();
@@ -211,7 +211,7 @@
    	char path[32];
    	const uint64_t addr = UAAx_PF_BAR0(i);
    	/* Remove the node */
-		snprintf(path, sizeof(path), "soc@0/serial@%llx", addr);
+		snprintf(path, sizeof(path), "/soc@0/serial@%llx", addr);
    	dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0);
    	if (!dt_node || uart_is_enabled(i)) {
    		printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path);
@@ -227,7 +227,7 @@
    	u32 phandle = 0;
    	const uint64_t addr = PEM_PEMX_PF_BAR0(i);
    	/* Remove the node */
-		snprintf(path, sizeof(path), "soc@0/pci@%llx", addr);
+		snprintf(path, sizeof(path), "/soc@0/pci@%llx", addr);
    	dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0);
    	if (!dt_node || bdk_pcie_is_running(0, i)) {
    		printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path);
@@ -239,7 +239,7 @@
    	list_remove(&dt_node->list_node);

/* Remove phandle to non existing nodes */
-		snprintf(path, sizeof(path), "soc@0/smmu0@%llx", SMMU_PF_BAR0);
+		snprintf(path, sizeof(path), "/soc@0/smmu0@%llx", SMMU_PF_BAR0);
    	dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0);
    	if (!dt_node) {
    		printk(BIOS_ERR, "%s: SMMU entry not found\n",