[U-Boot] [PATCH 0/9] Refactoring and Endian bug fixes of fdt_support

[U-Boot] [PATCH 0/9] Refactoring and Endian bug fixes of fdt_support

[Masahiro Yamada]

Masahiro Yamada (9):
  fdt_support: delete unnecessary DECLARE_GLOBAL_DATA_PTR
  fdt_support: refactor with fdt_find_or_add_subnode helper func
  fdt_support: delete force argument of fdt_initrd()
  fdt_support: delete force argument of fdt_chosen()
  fdt_support: refactor fdt_fixup_stdout() function
  fdt_support: add 'const' qualifier for unchanged argument.
  fdt_support: fix an endian bug of fdt_fixup_memory_banks
  fdt_support: fix an endian bug of fdt_initrd()
  fdt_support: correct the return condition of fdt_initrd()

 arch/microblaze/lib/bootm.c |   2 +-
 common/cmd_fdt.c            |   4 +-
 common/fdt_support.c        | 295 +++++++++++++++++++++-----------------------
 common/image-fdt.c          |   4 +-
 include/fdt_support.h       |   4 +-
 5 files changed, 151 insertions(+), 158 deletions(-)

-- 
1.8.3.2

[U-Boot] [PATCH 1/9] fdt_support: delete unnecessary DECLARE_GLOBAL_DATA_PTR

[Masahiro Yamada]

gd->bd is not used in fdt_support.c.

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

 common/fdt_support.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index f9f358e..2464847 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -17,11 +17,6 @@
 #include <exports.h>
 
 /*
- * Global data (for the gd->bd)
- */
-DECLARE_GLOBAL_DATA_PTR;
-
-/*
  * Get cells len in bytes
  *     if #NNNN-cells property is 2 then len is 8
  *     otherwise len is 4
-- 
1.8.3.2

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

[Masahiro Yamada]

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>
---

 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
@@ -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) {
-- 
1.8.3.2

[U-Boot] [PATCH 3/9] fdt_support: delete force argument of fdt_initrd()

[Masahiro Yamada]

After all, we have realized "force" argument is completely
useless. fdt_initrd() was always called with force = 1.

We should always want to do the same thing
(set appropriate value to the property)
even if the property already exists.

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

 arch/microblaze/lib/bootm.c |  2 +-
 common/cmd_fdt.c            |  2 +-
 common/fdt_support.c        | 35 +++++++++++++++--------------------
 common/image-fdt.c          |  2 +-
 include/fdt_support.h       |  2 +-
 5 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/arch/microblaze/lib/bootm.c b/arch/microblaze/lib/bootm.c
index d60b307..6977dd6 100644
--- a/arch/microblaze/lib/bootm.c
+++ b/arch/microblaze/lib/bootm.c
@@ -58,7 +58,7 @@ int do_bootm_linux(int flag, int argc, char * const argv[],
 	/* fixup the initrd now that we know where it should be */
 	if (images->rd_start && images->rd_end && of_flat_tree)
 		ret = fdt_initrd(of_flat_tree, images->rd_start,
-				 images->rd_end, 1);
+				 images->rd_end);
 		if (ret)
 			return 1;
 
diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
index 3a9edd6..71ea367 100644
--- a/common/cmd_fdt.c
+++ b/common/cmd_fdt.c
@@ -582,7 +582,7 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		}
 
 		fdt_chosen(working_fdt, 1);
-		fdt_initrd(working_fdt, initrd_start, initrd_end, 1);
+		fdt_initrd(working_fdt, initrd_start, initrd_end);
 	}
 	/* resize the fdt */
 	else if (strncmp(argv[1], "re", 2) == 0) {
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 849bdc8..3e16e8a 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -177,12 +177,11 @@ static int fdt_fixup_stdout(void *fdt, int chosenoff)
 }
 #endif
 
-int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
+int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {
 	int   nodeoffset, addr_cell_len;
 	int   err, j, total;
 	fdt64_t  tmp;
-	const char *path;
 	uint64_t addr, size;
 
 	/* find or create "/chosen" node. */
@@ -216,26 +215,22 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 
 	addr_cell_len = get_cells_len(fdt, "#address-cells");
 
-	path = fdt_getprop(fdt, nodeoffset, "linux,initrd-start", NULL);
-	if ((path == NULL) || force) {
-		write_cell((u8 *)&tmp, initrd_start, addr_cell_len);
-		err = fdt_setprop(fdt, nodeoffset,
-			"linux,initrd-start", &tmp, addr_cell_len);
-		if (err < 0) {
-			printf("WARNING: "
-				"could not set linux,initrd-start %s.\n",
-				fdt_strerror(err));
-			return err;
-		}
-		write_cell((u8 *)&tmp, initrd_end, addr_cell_len);
-		err = fdt_setprop(fdt, nodeoffset,
+	write_cell((u8 *)&tmp, initrd_start, addr_cell_len);
+	err = fdt_setprop(fdt, nodeoffset,
+			  "linux,initrd-start", &tmp, addr_cell_len);
+	if (err < 0) {
+		printf("WARNING: could not set linux,initrd-start %s.\n",
+		       fdt_strerror(err));
+		return err;
+	}
+	write_cell((u8 *)&tmp, initrd_end, addr_cell_len);
+	err = fdt_setprop(fdt, nodeoffset,
 			"linux,initrd-end", &tmp, addr_cell_len);
-		if (err < 0) {
-			printf("WARNING: could not set linux,initrd-end %s.\n",
-				fdt_strerror(err));
+	if (err < 0) {
+		printf("WARNING: could not set linux,initrd-end %s.\n",
+		       fdt_strerror(err));
 
-			return err;
-		}
+		return err;
 	}
 
 	return 0;
diff --git a/common/image-fdt.c b/common/image-fdt.c
index a54a919..a632c84 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -483,7 +483,7 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
 	/* Create a new LMB reservation */
 	lmb_reserve(lmb, (ulong)blob, of_size);
 
-	fdt_initrd(blob, *initrd_start, *initrd_end, 1);
+	fdt_initrd(blob, *initrd_start, *initrd_end);
 	if (!ft_verify_fdt(blob))
 		return -1;
 
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 9871e2f..786e797 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -15,7 +15,7 @@
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
 				const char *prop, const u32 dflt);
 int fdt_chosen(void *fdt, int force);
-int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force);
+int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
 void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop,
-- 
1.8.3.2

[U-Boot] [PATCH 4/9] fdt_support: delete force argument of fdt_chosen()

[Masahiro Yamada]

After all, we have realized "force" argument is completely
useless. fdt_chosen() was always called with force = 1.

We should always want to do the same thing
(set appropriate value to the property)
even if the property already exists.

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

 common/cmd_fdt.c      |  2 +-
 common/fdt_support.c  | 38 ++++++++++++--------------------------
 common/image-fdt.c    |  2 +-
 include/fdt_support.h |  2 +-
 4 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
index 71ea367..fbe688f 100644
--- a/common/cmd_fdt.c
+++ b/common/cmd_fdt.c
@@ -581,7 +581,7 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			initrd_end = simple_strtoul(argv[3], NULL, 16);
 		}
 
-		fdt_chosen(working_fdt, 1);
+		fdt_chosen(working_fdt);
 		fdt_initrd(working_fdt, initrd_start, initrd_end);
 	}
 	/* resize the fdt */
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 3e16e8a..c714ffa 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -236,12 +236,11 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 	return 0;
 }
 
-int fdt_chosen(void *fdt, int force)
+int fdt_chosen(void *fdt)
 {
 	int   nodeoffset;
 	int   err;
 	char  *str;		/* used to set string properties */
-	const char *path;
 
 	err = fdt_check_header(fdt);
 	if (err < 0) {
@@ -254,38 +253,25 @@ int fdt_chosen(void *fdt, int force)
 	if (nodeoffset < 0)
 		return nodeoffset;
 
-	/*
-	 * Create /chosen properites that don't exist in the fdt.
-	 * If the property exists, update it only if the "force" parameter
-	 * is true.
-	 */
 	str = getenv("bootargs");
 	if (str != NULL) {
-		path = fdt_getprop(fdt, nodeoffset, "bootargs", NULL);
-		if ((path == NULL) || force) {
-			err = fdt_setprop(fdt, nodeoffset,
-				"bootargs", str, strlen(str)+1);
-			if (err < 0)
-				printf("WARNING: could not set bootargs %s.\n",
-					fdt_strerror(err));
-		}
+		err = fdt_setprop(fdt, nodeoffset,
+				  "bootargs", str, strlen(str)+1);
+		if (err < 0)
+			printf("WARNING: could not set bootargs %s.\n",
+			       fdt_strerror(err));
 	}
 
 #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
-	path = fdt_getprop(fdt, nodeoffset, "linux,stdout-path", NULL);
-	if ((path == NULL) || force)
-		err = fdt_fixup_stdout(fdt, nodeoffset);
+	err = fdt_fixup_stdout(fdt, nodeoffset);
 #endif
 
 #ifdef OF_STDOUT_PATH
-	path = fdt_getprop(fdt, nodeoffset, "linux,stdout-path", NULL);
-	if ((path == NULL) || force) {
-		err = fdt_setprop(fdt, nodeoffset,
-			"linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
-		if (err < 0)
-			printf("WARNING: could not set linux,stdout-path %s.\n",
-				fdt_strerror(err));
-	}
+	err = fdt_setprop(fdt, nodeoffset, "linux,stdout-path",
+			  OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
+	if (err < 0)
+		printf("WARNING: could not set linux,stdout-path %s.\n",
+		       fdt_strerror(err));
 #endif
 
 	return err;
diff --git a/common/image-fdt.c b/common/image-fdt.c
index a632c84..781ab8e 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -457,7 +457,7 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
 	ulong *initrd_end = &images->initrd_end;
 	int ret;
 
-	if (fdt_chosen(blob, 1) < 0) {
+	if (fdt_chosen(blob) < 0) {
 		puts("ERROR: /chosen node create failed");
 		puts(" - must RESET the board to recover.\n");
 		return -1;
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 786e797..fc50484 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -14,7 +14,7 @@
 
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
 				const char *prop, const u32 dflt);
-int fdt_chosen(void *fdt, int force);
+int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
-- 
1.8.3.2

[U-Boot] [PATCH 5/9] fdt_support: refactor fdt_fixup_stdout() function

[Masahiro Yamada]

 - Do not use a deep indentation. We have only 80-character
   on each line and 1 indentation consumes 8 spaces. Before the
   code moves far to the right, you should consider to
   fix your code. See Linux Documentation/CodingStyle.

 - Add CONFIG_OF_STDOUT_VIA_ALIAS and OF_STDOUT_PATH macros
   only to their definition. Do not add them to both
   callee and caller. This is a tip to avoid using #ifdef
   everywhere.

 - OF_STDOUT_PATH and CONFIG_OF_STDOUT_VIA_ALIAS are exclusive.
   If both are defined, the former takes precedence.
   Do not try to fix-up "linux,stdout-path" property twice.

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

 common/fdt_support.c | 85 ++++++++++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 43 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index c714ffa..f641e68 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -123,9 +123,14 @@ static int fdt_find_or_add_subnode(void *fdt, int parentoffset,
 	return offset;
 }
 
-#ifdef CONFIG_OF_STDOUT_VIA_ALIAS
-
-#ifdef CONFIG_CONS_INDEX
+/* rename to CONFIG_OF_STDOUT_PATH ? */
+#if defined(OF_STDOUT_PATH)
+static int fdt_fixup_stdout(void *fdt, int chosenoff)
+{
+	return fdt_setprop(fdt, chosenoff, "linux,stdout-path",
+			      OF_STDOUT_PATH, strlen(OF_STDOUT_PATH) + 1);
+}
+#elif defined(CONFIG_OF_STDOUT_VIA_ALIAS) && defined(CONFIG_CONS_INDEX)
 static void fdt_fill_multisername(char *sername, size_t maxlen)
 {
 	const char *outname = stdio_devices[stdout]->name;
@@ -137,44 +142,48 @@ static void fdt_fill_multisername(char *sername, size_t maxlen)
 	if (strcmp(outname + 1, "serial") > 0)
 		strncpy(sername, outname + 1, maxlen);
 }
-#endif
 
 static int fdt_fixup_stdout(void *fdt, int chosenoff)
 {
-	int err = 0;
-#ifdef CONFIG_CONS_INDEX
-	int node;
+	int err;
+	int aliasoff;
 	char sername[9] = { 0 };
-	const char *path;
+	const void *path;
+	int len;
+	char tmp[256]; /* long enough */
 
 	fdt_fill_multisername(sername, sizeof(sername) - 1);
 	if (!sername[0])
 		sprintf(sername, "serial%d", CONFIG_CONS_INDEX - 1);
 
-	err = node = fdt_path_offset(fdt, "/aliases");
-	if (node >= 0) {
-		int len;
-		path = fdt_getprop(fdt, node, sername, &len);
-		if (path) {
-			char *p = malloc(len);
-			err = -FDT_ERR_NOSPACE;
-			if (p) {
-				memcpy(p, path, len);
-				err = fdt_setprop(fdt, chosenoff,
-					"linux,stdout-path", p, len);
-				free(p);
-			}
-		} else {
-			err = len;
-		}
+	aliasoff = fdt_path_offset(fdt, "/aliases");
+	if (aliasoff < 0) {
+		err = aliasoff;
+		goto error;
 	}
-#endif
+
+	path = fdt_getprop(fdt, aliasoff, sername, &len);
+	if (!path) {
+		err = len;
+		goto error;
+	}
+
+	/* fdt_setprop may break "path" so we copy it to tmp buffer */
+	memcpy(tmp, path, len);
+
+	err = fdt_setprop(fdt, chosenoff, "linux,stdout-path", tmp, len);
+error:
 	if (err < 0)
 		printf("WARNING: could not set linux,stdout-path %s.\n",
-				fdt_strerror(err));
+		       fdt_strerror(err));
 
 	return err;
 }
+#else
+static int fdt_fixup_stdout(void *fdt, int chosenoff)
+{
+	return 0;
+}
 #endif
 
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
@@ -254,27 +263,17 @@ int fdt_chosen(void *fdt)
 		return nodeoffset;
 
 	str = getenv("bootargs");
-	if (str != NULL) {
-		err = fdt_setprop(fdt, nodeoffset,
-				  "bootargs", str, strlen(str)+1);
-		if (err < 0)
+	if (str) {
+		err = fdt_setprop(fdt, nodeoffset, "bootargs", str,
+				  strlen(str) + 1);
+		if (err < 0) {
 			printf("WARNING: could not set bootargs %s.\n",
 			       fdt_strerror(err));
+			return err;
+		}
 	}
 
-#ifdef CONFIG_OF_STDOUT_VIA_ALIAS
-	err = fdt_fixup_stdout(fdt, nodeoffset);
-#endif
-
-#ifdef OF_STDOUT_PATH
-	err = fdt_setprop(fdt, nodeoffset, "linux,stdout-path",
-			  OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
-	if (err < 0)
-		printf("WARNING: could not set linux,stdout-path %s.\n",
-		       fdt_strerror(err));
-#endif
-
-	return err;
+	return fdt_fixup_stdout(fdt, nodeoffset);
 }
 
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
-- 
1.8.3.2

[U-Boot] [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.

[Masahiro Yamada]

In the next commit, I will add a new function, fdt_pack_reg()
which uses get_cells_len().

Beforehand, this commit adds 'const' qualifier to get_cells_len().
Otherwise, a warning message will appear:
 warning: passing argument 1 of 'get_cells_len' discards 'const'
 qualifier from pointer target type [enabled by default]

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

 common/fdt_support.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index f641e68..bdc5ce1 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -21,11 +21,11 @@
  *     if #NNNN-cells property is 2 then len is 8
  *     otherwise len is 4
  */
-static int get_cells_len(void *blob, char *nr_cells_name)
+static int get_cells_len(const void *fdt, const char *nr_cells_name)
 {
 	const fdt32_t *cell;
 
-	cell = fdt_getprop(blob, 0, nr_cells_name, NULL);
+	cell = fdt_getprop(fdt, 0, nr_cells_name, NULL);
 	if (cell && fdt32_to_cpu(*cell) == 2)
 		return 8;
 
-- 
1.8.3.2

[U-Boot] [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.

[Masahiro Yamada]

Hi.


> [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.

This might be really trivial, but I notice I had added a period at the
end of the subject by mistake.

Is it possible to remove it when this patch is applied?
Or need to post v2?

Best Regards
Masahiro Yamada

[U-Boot] [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.

[Tom Rini]

On Thu, Feb 20, 2014 at 05:55:47PM +0900, Masahiro Yamada wrote:
> Hi.
> 
> 
> > [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.
> 
> This might be really trivial, but I notice I had added a period at the
> end of the subject by mistake.
> 
> Is it possible to remove it when this patch is applied?
> Or need to post v2?

I'll do my best to remember to fix it up.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140220/7516ef17/attachment.pgp>

[U-Boot] [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.

[Masahiro Yamada]

Hi Tom,

On Thu, 20 Feb 2014 08:31:56 -0500
Tom Rini <trini@ti.com> wrote:

> On Thu, Feb 20, 2014 at 05:55:47PM +0900, Masahiro Yamada wrote:
> > Hi.
> > 
> > 
> > > [PATCH 6/9] fdt_support: add 'const' qualifier for unchanged argument.
> > 
> > This might be really trivial, but I notice I had added a period at the
> > end of the subject by mistake.
> > 
> > Is it possible to remove it when this patch is applied?
> > Or need to post v2?
> 
> I'll do my best to remember to fix it up.
> 
> -- 
> Tom



When and by whom will my fdt_support series be reviewed?

This series includes endian bug fixes (and various refactoring),
so I believe it is worth the review.


Best Regards
Masahiro Yamada

[U-Boot] [PATCH 7/9] fdt_support: fix an endian bug of fdt_fixup_memory_banks

[Masahiro Yamada]

Data written to DTB must be converted to big endian order.
It is usually done by using cpu_to_fdt32(), cpu_to_fdt64(), etc.

fdt_fixup_memory_banks() invoked write_cell(), which always
swaps byte order.
It means the function only worked on little endian architectures.

This commit adds and uses a new helper function, fdt_pack_reg(),
which works on both big endian and little endian architrectures.

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

 common/fdt_support.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index bdc5ce1..58d1ef7 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -354,6 +354,34 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
 	do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create);
 }
 
+/*
+ * fdt_pack_reg - pack address and size array into the "reg"-suitable stream
+ */
+static int fdt_pack_reg(const void *fdt, void *buf, uint64_t *address,
+			uint64_t *size, int n)
+{
+	int i;
+	int address_len = get_cells_len(fdt, "#address-cells");
+	int size_len = get_cells_len(fdt, "#size-cells");
+	char *p = buf;
+
+	for (i = 0; i < n; i++) {
+		if (address_len == 8)
+			*(fdt64_t *)p = cpu_to_fdt64(address[i]);
+		else
+			*(fdt32_t *)p = cpu_to_fdt32(address[i]);
+		p += address_len;
+
+		if (size_len == 8)
+			*(fdt64_t *)p = cpu_to_fdt64(size[i]);
+		else
+			*(fdt32_t *)p = cpu_to_fdt32(size[i]);
+		p += size_len;
+	}
+
+	return p - (char *)buf;
+}
+
 #ifdef CONFIG_NR_DRAM_BANKS
 #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
 #else
@@ -362,9 +390,8 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
 int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 {
 	int err, nodeoffset;
-	int addr_cell_len, size_cell_len, len;
+	int len;
 	u8 tmp[MEMORY_BANKS_MAX * 16]; /* Up to 64-bit address + 64-bit size */
-	int bank;
 
 	if (banks > MEMORY_BANKS_MAX) {
 		printf("%s: num banks %d exceeds hardcoded limit %d."
@@ -392,16 +419,7 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 		return err;
 	}
 
-	addr_cell_len = get_cells_len(blob, "#address-cells");
-	size_cell_len = get_cells_len(blob, "#size-cells");
-
-	for (bank = 0, len = 0; bank < banks; bank++) {
-		write_cell(tmp + len, start[bank], addr_cell_len);
-		len += addr_cell_len;
-
-		write_cell(tmp + len, size[bank], size_cell_len);
-		len += size_cell_len;
-	}
+	len = fdt_pack_reg(blob, tmp, start, size, banks);
 
 	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
 	if (err < 0) {
-- 
1.8.3.2

[U-Boot] [PATCH 8/9] fdt_support: fix an endian bug of fdt_initrd()

[Masahiro Yamada]

Data written to DTB must be converted to big endian order.
It is usually done by using cpu_to_fdt32(), cpu_to_fdt64(), etc.

fdt_initrd() invoked write_cell(), which always swaps byte order.
It means the function only worked on little endian architectures.
(On big endian architectures, the byte order should be kept as it is)

This commit uses cpu_to_fdt32() and cpu_to_fdt64()
and deletes write_cell().

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

 common/fdt_support.c | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 58d1ef7..5631f16 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -32,18 +32,6 @@ static int get_cells_len(const void *fdt, const char *nr_cells_name)
 	return 4;
 }
 
-/*
- * Write a 4 or 8 byte big endian cell
- */
-static void write_cell(u8 *addr, u64 val, int size)
-{
-	int shift = (size - 1) * 8;
-	while (size-- > 0) {
-		*addr++ = (val >> shift) & 0xff;
-		shift -= 8;
-	}
-}
-
 /**
  * fdt_getprop_u32_default - Find a node and return it's property or a default
  *
@@ -186,11 +174,21 @@ static int fdt_fixup_stdout(void *fdt, int chosenoff)
 }
 #endif
 
+static inline int fdt_setprop_uxx(void *fdt, int nodeoffset, const char *name,
+				  uint64_t val, int is_u64)
+{
+	if (is_u64)
+		return fdt_setprop_u64(fdt, nodeoffset, name, val);
+	else
+		return fdt_setprop_u32(fdt, nodeoffset, name, (uint32_t)val);
+}
+
+
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {
-	int   nodeoffset, addr_cell_len;
+	int   nodeoffset;
 	int   err, j, total;
-	fdt64_t  tmp;
+	int is_u64;
 	uint64_t addr, size;
 
 	/* find or create "/chosen" node. */
@@ -222,19 +220,20 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 		return err;
 	}
 
-	addr_cell_len = get_cells_len(fdt, "#address-cells");
+	is_u64 = (get_cells_len(fdt, "#address-cells") == 8);
+
+	err = fdt_setprop_uxx(fdt, nodeoffset, "linux,initrd-start",
+			      (uint64_t)initrd_start, is_u64);
 
-	write_cell((u8 *)&tmp, initrd_start, addr_cell_len);
-	err = fdt_setprop(fdt, nodeoffset,
-			  "linux,initrd-start", &tmp, addr_cell_len);
 	if (err < 0) {
 		printf("WARNING: could not set linux,initrd-start %s.\n",
 		       fdt_strerror(err));
 		return err;
 	}
-	write_cell((u8 *)&tmp, initrd_end, addr_cell_len);
-	err = fdt_setprop(fdt, nodeoffset,
-			"linux,initrd-end", &tmp, addr_cell_len);
+
+	err = fdt_setprop_uxx(fdt, nodeoffset, "linux,initrd-end",
+			      (uint64_t)initrd_end, is_u64);
+
 	if (err < 0) {
 		printf("WARNING: could not set linux,initrd-end %s.\n",
 		       fdt_strerror(err));
-- 
1.8.3.2

[U-Boot] [PATCH 9/9] fdt_support: correct the return condition of fdt_initrd()

[Masahiro Yamada]

Before this commit, fdt_initrd() just returned if initrd
start address is zero.
But it is possible if the RAM is located at address 0.

This commit makes the return condition more reasonable:
Just return if the size of initrd is zero.

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

 common/fdt_support.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5631f16..89119be 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -191,15 +191,15 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 	int is_u64;
 	uint64_t addr, size;
 
+	/* just return if the size of initrd is zero */
+	if (initrd_start == initrd_end)
+		return 0;
+
 	/* 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))
-		return 0;
-
 	total = fdt_num_mem_rsv(fdt);
 
 	/*
-- 
1.8.3.2