diff mbox

[U-Boot,v2,2/9] fdt_support: refactor with fdt_find_or_add_subnode helper func

Message ID 1397810465-10006-3-git-send-email-yamada.m@jp.panasonic.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada April 18, 2014, 8:40 a.m. UTC 
Some functions in fdt_support.c do the same routine:
search a node with a given name ("chosen", "memory", etc.)
or newly create it if it does not exist.

So this commit makes that routine to a helper function.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

Changes in v2: None

 common/fdt_support.c | 71 ++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

Comments

Simon Glass May 24, 2014, 1:18 a.m. UTC | #1 
Hi Masahiro,

On 17 April 2014 22:40, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Some functions in fdt_support.c do the same routine:
> search a node with a given name ("chosen", "memory", etc.)
> or newly create it if it does not exist.
>
> So this commit makes that routine to a helper function.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> ---
>
> Changes in v2: None
>
>  common/fdt_support.c | 71 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 2464847..849bdc8 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
[snip]
> @@ -160,14 +185,10 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
>         const char *path;
>         uint64_t addr, size;
>
> -       /* Find the "chosen" node.  */
> -       nodeoffset = fdt_path_offset (fdt, "/chosen");
> -
> -       /* If there is no "chosen" node in the blob return */
> -       if (nodeoffset < 0) {
> -               printf("fdt_initrd: %s\n", fdt_strerror(nodeoffset));
> +       /* find or create "/chosen" node. */
> +       nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
> +       if (nodeoffset < 0)

It looks like you are changing the behaviour here, but I think that is
fine. We should create it if it is missing.

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
Tom Rini June 19, 2014, 3:20 p.m. UTC | #2 
On Fri, Apr 18, 2014 at 05:40:58PM +0900, Masahiro Yamada wrote:

> Some functions in fdt_support.c do the same routine:
> search a node with a given name ("chosen", "memory", etc.)
> or newly create it if it does not exist.
> 
> So this commit makes that routine to a helper function.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 2464847..849bdc8 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -98,6 +98,31 @@  int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 	return fdt_setprop(fdt, nodeoff, prop, val, len);
 }
 
+/**
+ * fdt_find_or_add_subnode - find or possibly add a subnode of a given node
+ * @fdt: pointer to the device tree blob
+ * @parentoffset: structure block offset of a node
+ * @name: name of the subnode to locate
+ *
+ * fdt_subnode_offset() finds a subnode of the node with a given name.
+ * If the subnode does not exist, it will be created.
+ */
+static int fdt_find_or_add_subnode(void *fdt, int parentoffset,
+				   const char *name)
+{
+	int offset;
+
+	offset = fdt_subnode_offset(fdt, parentoffset, name);
+
+	if (offset == -FDT_ERR_NOTFOUND)
+		offset = fdt_add_subnode(fdt, parentoffset, name);
+
+	if (offset < 0)
+		printf("%s: %s: %s\n", __func__, name, fdt_strerror(offset));
+
+	return offset;
+}
+
 #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
 
 #ifdef CONFIG_CONS_INDEX
@@ -160,14 +185,10 @@  int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 	const char *path;
 	uint64_t addr, size;
 
-	/* Find the "chosen" node.  */
-	nodeoffset = fdt_path_offset (fdt, "/chosen");
-
-	/* If there is no "chosen" node in the blob return */
-	if (nodeoffset < 0) {
-		printf("fdt_initrd: %s\n", fdt_strerror(nodeoffset));
+	/* find or create "/chosen" node. */
+	nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
+	if (nodeoffset < 0)
 		return nodeoffset;
-	}
 
 	/* just return if initrd_start/end aren't valid */
 	if ((initrd_start == 0) || (initrd_end == 0))
@@ -233,25 +254,10 @@  int fdt_chosen(void *fdt, int force)
 		return err;
 	}
 
-	/*
-	 * Find the "chosen" node.
-	 */
-	nodeoffset = fdt_path_offset (fdt, "/chosen");
-
-	/*
-	 * If there is no "chosen" node in the blob, create it.
-	 */
-	if (nodeoffset < 0) {
-		/*
-		 * Create a new node "/chosen" (offset 0 is root level)
-		 */
-		nodeoffset = fdt_add_subnode(fdt, 0, "chosen");
-		if (nodeoffset < 0) {
-			printf("WARNING: could not create /chosen %s.\n",
-				fdt_strerror(nodeoffset));
-			return nodeoffset;
-		}
-	}
+	/* find or create "/chosen" node. */
+	nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
+	if (nodeoffset < 0)
+		return nodeoffset;
 
 	/*
 	 * Create /chosen properites that don't exist in the fdt.
@@ -393,16 +399,11 @@  int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 		return err;
 	}
 
-	/* update, or add and update /memory node */
-	nodeoffset = fdt_path_offset(blob, "/memory");
-	if (nodeoffset < 0) {
-		nodeoffset = fdt_add_subnode(blob, 0, "memory");
-		if (nodeoffset < 0) {
-			printf("WARNING: could not create /memory: %s.\n",
-					fdt_strerror(nodeoffset));
+	/* find or create "/memory" node. */
+	nodeoffset = fdt_find_or_add_subnode(blob, 0, "memory");
+	if (nodeoffset < 0)
 			return nodeoffset;
-		}
-	}
+
 	err = fdt_setprop(blob, nodeoffset, "device_type", "memory",
 			sizeof("memory"));
 	if (err < 0) {