[PATCH 1/5] fdtput: Fix nit in help message

[PATCH 1/5] fdtput: Fix nit in help message

[Simon Glass]

There was an extra < in the help message, so fix it.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 fdtput.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index f6ebd24..244d1f1 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -162,7 +162,7 @@ static const char *usage_msg =
 	"The command line arguments are joined together into a single value.\n"
 	"\n"
 	"Usage:\n"
-	"	fdtput <options> <dt file> <<node> <property> [<value>...]\n"
+	"	fdtput <options> <dt file> <node> <property> [<value>...]\n"
 	"Options:\n"
 	"\t-t <type>\tType of data\n"
 	"\t-v\t\tVerbose: display each value decoded from command line\n"
-- 
1.7.7.3

[PATCH 2/5] fdtput: Prepare to support multiple operations

[Simon Glass]

We want to add new options to this tool. In preparation for this, add
the concept of a current operation.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 fdtput.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index 244d1f1..eceb7f2 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -28,7 +28,13 @@
 
 #include "util.h"
 
+/* These are the operations we support */
+enum operation_t {
+	OPER_WRITE_PROP,		/* Write a property in a node */
+};
+
 struct display_info {
+	enum operation_t oper;	/* operation to perform */
 	int type;		/* data type (s/i/u/x or 0 for default) */
 	int size;		/* data size (1/2/4) */
 	int verbose;		/* verbose output */
@@ -143,13 +149,19 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 	if (!blob)
 		return -1;
 
-	/* convert the arguments into a single binary value, then store */
-	assert(arg_count >= 2);
-	if (encode_value(disp, arg + 2, arg_count - 2, &value, &len) ||
-		store_key_value(blob, *arg, arg[1], value, len))
-		ret = -1;
-
-	if (!ret)
+	switch (disp->oper) {
+	case OPER_WRITE_PROP:
+		/*
+		 * Convert the arguments into a single binary value, then
+		 * store them into the property.
+		 */
+		assert(arg_count >= 2);
+		if (encode_value(disp, arg + 2, arg_count - 2, &value, &len) ||
+			store_key_value(blob, *arg, arg[1], value, len))
+			ret = -1;
+		break;
+	}
+	if (ret >= 0)
 		ret = utilfdt_write(filename, blob);
 
 	free(blob);
@@ -185,6 +197,7 @@ int main(int argc, char *argv[])
 
 	memset(&disp, '\0', sizeof(disp));
 	disp.size = -1;
+	disp.oper = OPER_WRITE_PROP;
 	for (;;) {
 		int c = getopt(argc, argv, "ht:v");
 		if (c == -1)
@@ -224,10 +237,12 @@ int main(int argc, char *argv[])
 	argv += optind;
 	argc -= optind;
 
-	if (argc < 1)
-		usage("Missing node");
-	if (argc < 2)
-		usage("Missing property");
+	if (disp.oper == OPER_WRITE_PROP) {
+		if (argc < 1)
+			usage("Missing node");
+		if (argc < 2)
+			usage("Missing property");
+	}
 
 	if (do_fdtput(&disp, filename, argv, argc))
 		return 1;
-- 
1.7.7.3

[PATCH 3/5] fdtput: Add -c option to create nodes

[Simon Glass]

This option allows the creation of new nodes in a dtb file. The syntax
is:

   fdtput -c <dtb_file> <node_path>

The node_path contains the path of the node to be created. All path
components up to the final one must exist already. The final one must
not exist already.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 fdtput.c           |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/run_tests.sh |   13 +++++++++++++
 2 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index eceb7f2..fc6b85e 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -31,6 +31,7 @@
 /* These are the operations we support */
 enum operation_t {
 	OPER_WRITE_PROP,		/* Write a property in a node */
+	OPER_CREATE_NODE,		/* Create a new node */
 };
 
 struct display_info {
@@ -138,6 +139,46 @@ static int store_key_value(void *blob, const char *node_name,
 	return 0;
 }
 
+/**
+ * Create a new node in the fdt.
+ *
+ * This will overwrite the node_name string. Any error is reported.
+ *
+ * TODO: Perhaps create fdt_path_offset_namelen() so we don't need to do this.
+ *
+ * @param blob		FDT blob to write into
+ * @param node_name	Name of node to create
+ * @return new node offset if found, or -1 on failure
+ */
+static int create_node(void *blob, const char *node_name)
+{
+	int node = 0;
+	char *p;
+
+	p = strrchr(node_name, '/');
+	if (!p) {
+		report_error(node_name, -FDT_ERR_BADPATH);
+		return -1;
+	}
+	*p = '\0';
+
+	if (p > node_name) {
+		node = fdt_path_offset(blob, node_name);
+		if (node < 0) {
+			report_error(node_name, node);
+			return -1;
+		}
+	}
+
+	node = fdt_add_subnode(blob, node, p + 1);
+	if (node < 0) {
+		report_error(p + 1, node);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int do_fdtput(struct display_info *disp, const char *filename,
 		    char **arg, int arg_count)
 {
@@ -160,6 +201,10 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 			store_key_value(blob, *arg, arg[1], value, len))
 			ret = -1;
 		break;
+	case OPER_CREATE_NODE:
+		for (; ret >= 0 && arg_count--; arg++)
+			ret = create_node(blob, *arg);
+		break;
 	}
 	if (ret >= 0)
 		ret = utilfdt_write(filename, blob);
@@ -175,7 +220,9 @@ static const char *usage_msg =
 	"\n"
 	"Usage:\n"
 	"	fdtput <options> <dt file> <node> <property> [<value>...]\n"
+	"	fdtput -c <dt file> [<node>...]\n"
 	"Options:\n"
+	"\t-c\tCreate nodes if they don't already exist\n"
 	"\t-t <type>\tType of data\n"
 	"\t-v\t\tVerbose: display each value decoded from command line\n"
 	"\t-h\t\tPrint this help\n\n"
@@ -199,7 +246,7 @@ int main(int argc, char *argv[])
 	disp.size = -1;
 	disp.oper = OPER_WRITE_PROP;
 	for (;;) {
-		int c = getopt(argc, argv, "ht:v");
+		int c = getopt(argc, argv, "cht:v");
 		if (c == -1)
 			break;
 
@@ -213,6 +260,9 @@ int main(int argc, char *argv[])
 		 * - expand fdt if value doesn't fit
 		 */
 		switch (c) {
+		case 'c':
+			disp.oper = OPER_CREATE_NODE;
+			break;
 		case 'h':
 		case '?':
 			usage(NULL);
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 073a833..acec2a5 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -539,6 +539,19 @@ fdtput_tests () {
     # This should be larger than available space in the fdt
     run_wrap_error_test $DTPUT $dtb /randomnode blob -ts "$(cat $text $text)"
 
+    # Start again with a fresh dtb
+    run_dtc_test -O dtb -p $(stat -c %s $text) -o $dtb $dts
+
+    # Node creation
+    run_wrap_error_test $DTPUT $dtb -c /baldrick sod
+    run_wrap_test $DTPUT $dtb -c /chosen/son /chosen/daughter
+    run_fdtput_test "eva" $dtb /chosen/daughter name "" -ts "eva"
+    run_fdtput_test "adam" $dtb /chosen/son name "" -ts "adam"
+
+    # Not allowed to create an existing node
+    run_wrap_error_test $DTPUT $dtb -c /chosen
+    run_wrap_error_test $DTPUT $dtb -c /chosen/son
+
     # TODO: Add tests for verbose mode?
 }
 
-- 
1.7.7.3

[PATCH 4/5] fdtput: Adjust report_error() to use name, namelen params

[Simon Glass]

As with many fdt functions, report_error() should permit a namelen to
be specified, thus obviating the need for nul termination in strings
passed to it.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 fdtput.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index fc6b85e..f217f1d 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -41,9 +41,20 @@ struct display_info {
 	int verbose;		/* verbose output */
 };
 
-static void report_error(const char *where, int err)
+
+/**
+ * Report an error with a particular node.
+ *
+ * @param name		Node name to report error on
+ * @param namelen	Length of node name, or -1 to use entire string
+ * @param err		Error number to report (-FDT_ERR_...)
+ */
+static void report_error(const char *name, int namelen, int err)
 {
-	fprintf(stderr, "Error at '%s': %s\n", where, fdt_strerror(err));
+	if (namelen == -1)
+		namelen = strlen(name);
+	fprintf(stderr, "Error at '%1.*s': %s\n", namelen, name,
+		fdt_strerror(err));
 }
 
 /**
@@ -127,13 +138,13 @@ static int store_key_value(void *blob, const char *node_name,
 
 	node = fdt_path_offset(blob, node_name);
 	if (node < 0) {
-		report_error(node_name, node);
+		report_error(node_name, -1, node);
 		return -1;
 	}
 
 	err = fdt_setprop(blob, node, property, buf, len);
 	if (err) {
-		report_error(property, err);
+		report_error(property, -1, err);
 		return -1;
 	}
 	return 0;
@@ -157,7 +168,7 @@ static int create_node(void *blob, const char *node_name)
 
 	p = strrchr(node_name, '/');
 	if (!p) {
-		report_error(node_name, -FDT_ERR_BADPATH);
+		report_error(node_name, -1, -FDT_ERR_BADPATH);
 		return -1;
 	}
 	*p = '\0';
@@ -165,14 +176,14 @@ static int create_node(void *blob, const char *node_name)
 	if (p > node_name) {
 		node = fdt_path_offset(blob, node_name);
 		if (node < 0) {
-			report_error(node_name, node);
+			report_error(node_name, -1, node);
 			return -1;
 		}
 	}
 
 	node = fdt_add_subnode(blob, node, p + 1);
 	if (node < 0) {
-		report_error(p + 1, node);
+		report_error(p + 1, -1, node);
 		return -1;
 	}
 
-- 
1.7.7.3

[PATCH 5/5] fdtput: Add -p option to create subnodes along entire path

[Simon Glass]

This option mimics mkdir's -p option. It automatically creates nodes
as needed along the path provided. If the node already exists, no
error is given.

Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 fdtput.c           |   59 ++++++++++++++++++++++++++++++++++++++++++++++++---
 tests/run_tests.sh |   13 +++++++++++
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index f217f1d..637d9fa 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -39,6 +39,7 @@ struct display_info {
 	int type;		/* data type (s/i/u/x or 0 for default) */
 	int size;		/* data size (1/2/4) */
 	int verbose;		/* verbose output */
+	int auto_path;		/* automatically create all path components */
 };
 
 
@@ -151,6 +152,47 @@ static int store_key_value(void *blob, const char *node_name,
 }
 
 /**
+ * Create paths as needed for all components of a path
+ *
+ * Any components of the path that do not exist are created. Errors are
+ * reported.
+ *
+ * @param blob		FDT blob to write into
+ * @param in_path	Path to process
+ * @return 0 if ok, -1 on error
+ */
+static int create_paths(void *blob, const char *in_path)
+{
+	const char *path = in_path;
+	const char *sep;
+	int node, offset = 0;
+
+	/* skip leading '/' */
+	while (*path == '/')
+		path++;
+
+	for (sep = path; *sep; path = sep + 1, offset = node) {
+		/* equivalent to strchrnul(), but it requires _GNU_SOURCE */
+		sep = strchr(path, '/');
+		if (!sep)
+			sep = path + strlen(path);
+
+		node = fdt_subnode_offset_namelen(blob, offset, path,
+				sep - path);
+		if (node == -FDT_ERR_NOTFOUND) {
+			node = fdt_add_subnode_namelen(blob, offset, path,
+						       sep - path);
+		}
+		if (node < 0) {
+			report_error(path, sep - path, node);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+/**
  * Create a new node in the fdt.
  *
  * This will overwrite the node_name string. Any error is reported.
@@ -208,13 +250,19 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 		 * store them into the property.
 		 */
 		assert(arg_count >= 2);
+		if (disp->auto_path && create_paths(blob, *arg))
+			return -1;
 		if (encode_value(disp, arg + 2, arg_count - 2, &value, &len) ||
 			store_key_value(blob, *arg, arg[1], value, len))
 			ret = -1;
 		break;
 	case OPER_CREATE_NODE:
-		for (; ret >= 0 && arg_count--; arg++)
-			ret = create_node(blob, *arg);
+		for (; ret >= 0 && arg_count--; arg++) {
+			if (disp->auto_path)
+				ret = create_paths(blob, *arg);
+			else
+				ret = create_node(blob, *arg);
+		}
 		break;
 	}
 	if (ret >= 0)
@@ -234,6 +282,7 @@ static const char *usage_msg =
 	"	fdtput -c <dt file> [<node>...]\n"
 	"Options:\n"
 	"\t-c\tCreate nodes if they don't already exist\n"
+	"\t-p\tAutomatically create nodes as needed through the whole path\n"
 	"\t-t <type>\tType of data\n"
 	"\t-v\t\tVerbose: display each value decoded from command line\n"
 	"\t-h\t\tPrint this help\n\n"
@@ -257,7 +306,7 @@ int main(int argc, char *argv[])
 	disp.size = -1;
 	disp.oper = OPER_WRITE_PROP;
 	for (;;) {
-		int c = getopt(argc, argv, "cht:v");
+		int c = getopt(argc, argv, "chpt:v");
 		if (c == -1)
 			break;
 
@@ -277,7 +326,9 @@ int main(int argc, char *argv[])
 		case 'h':
 		case '?':
 			usage(NULL);
-
+		case 'p':
+			disp.auto_path = 1;
+			break;
 		case 't':
 			if (utilfdt_decode_type(optarg, &disp.type,
 					&disp.size))
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index acec2a5..b825b81 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -552,6 +552,19 @@ fdtput_tests () {
     run_wrap_error_test $DTPUT $dtb -c /chosen
     run_wrap_error_test $DTPUT $dtb -c /chosen/son
 
+    # Automatic node creation
+    run_wrap_test $DTPUT $dtb -cp /blackadder/the-second/turnip \
+	/blackadder/the-second/potato
+    run_fdtput_test 1000 $dtb /blackadder/the-second/turnip cost "" 1000
+    run_fdtput_test "fine wine" $dtb /blackadder/the-second/potato drink \
+	"-ts" "fine wine"
+    run_wrap_test $DTPUT $dtb -p /you/are/drunk/sir/winston slurp -ts twice
+    run_wrap_error_test $DTPUT $dtb -cp "$(cat $text $text)/longish"
+
+    # Allowed to create an existing node with -p
+    run_wrap_test $DTPUT $dtb -cp /chosen
+    run_wrap_test $DTPUT $dtb -cp /chosen/son
+
     # TODO: Add tests for verbose mode?
 }
 
-- 
1.7.7.3

Re: [PATCH 1/5] fdtput: Fix nit in help message

[David Gibson]

On Tue, Jul 10, 2012 at 05:56:44AM -0700, Simon Glass wrote:
> There was an extra < in the help message, so fix it.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.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

Re: [PATCH 1/5] fdtput: Fix nit in help message

[Jon Loeliger]

> On Tue, Jul 10, 2012 at 05:56:44AM -0700, Simon Glass wrote:
> > There was an extra < in the help message, so fix it.
> > 
> > Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Applied.

jdl

Re: [PATCH 2/5] fdtput: Prepare to support multiple operations

[David Gibson]

On Tue, Jul 10, 2012 at 05:56:45AM -0700, Simon Glass wrote:
> We want to add new options to this tool. In preparation for this, add
> the concept of a current operation.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  fdtput.c |   37 ++++++++++++++++++++++++++-----------
>  1 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/fdtput.c b/fdtput.c
> index 244d1f1..eceb7f2 100644
> --- a/fdtput.c
> +++ b/fdtput.c
> @@ -28,7 +28,13 @@
>  
>  #include "util.h"
>  
> +/* These are the operations we support */
> +enum operation_t {

This isn't quite usual convention xxx_t usually indicates a typedef

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

Re: [PATCH 3/5] fdtput: Add -c option to create nodes

[David Gibson]

On Tue, Jul 10, 2012 at 05:56:46AM -0700, Simon Glass wrote:
> This option allows the creation of new nodes in a dtb file. The syntax
> is:
> 
>    fdtput -c <dtb_file> <node_path>
> 
> The node_path contains the path of the node to be created. All path
> components up to the final one must exist already. The final one must
> not exist already.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Looks reasonable.

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.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

Re: [PATCH 4/5] fdtput: Adjust report_error() to use name, namelen params

[David Gibson]

On Tue, Jul 10, 2012 at 05:56:47AM -0700, Simon Glass wrote:
> As with many fdt functions, report_error() should permit a namelen to
> be specified, thus obviating the need for nul termination in strings
> passed to it.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Fair enough.

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.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

Re: [PATCH 2/5] fdtput: Prepare to support multiple operations

[Simon Glass]

Hi David,

On Thu, Jul 12, 2012 at 2:37 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Jul 10, 2012 at 05:56:45AM -0700, Simon Glass wrote:
>> We want to add new options to this tool. In preparation for this, add
>> the concept of a current operation.
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>>  fdtput.c |   37 ++++++++++++++++++++++++++-----------
>>  1 files changed, 26 insertions(+), 11 deletions(-)
>>
>> diff --git a/fdtput.c b/fdtput.c
>> index 244d1f1..eceb7f2 100644
>> --- a/fdtput.c
>> +++ b/fdtput.c
>> @@ -28,7 +28,13 @@
>>
>>  #include "util.h"
>>
>> +/* These are the operations we support */
>> +enum operation_t {
>
> This isn't quite usual convention xxx_t usually indicates a typedef

OK I will fix it, and also update the help messages a little in two of
the patches (something I just noticed).

Regards,
Simon

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