* [PATCH v2 0/4] fdtput: Make it clearer how to create nodes
@ 2026-10-28 7:34 Simon Glass
2026-10-28 7:34 ` [PATCH v2 1/4] fdtput: Require a leading '/' with -p Simon Glass
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Simon Glass @ 2026-10-28 7:34 UTC (permalink / raw)
To: Devicetree Compiler; +Cc: David Gibson, Simon Glass
fdtput can create nodes with -c, and -p creates any missing nodes along
the way, but this is easy to miss. A user has asked for a way to add
nodes, not realising that one exists.
The first patch makes -p reject a path without a leading '/', rather
than creating it from the root, as fdtput already does without -p. The
second allows such a path to start with an alias, creating nodes below
the node which the alias points to. The third suggests -p when a node
is missing, naming the parent which is missing. The fourth names the
property, as well as the node, when it cannot be deleted.
Link: https://github.com/dgibson/dtc/issues/49
Changes in v2:
- Add new patch to reject a path without a leading '/'
- Rework on top of the new patch which requires a leading '/'
- Drop the Fixes tag, since the new patch fixes the bug
- Check that the error suggests -p, using a new helper
- Drop David's Reviewed-by, since the tests have changed
- Report the node as well as the property
- Check the error message in the test
- Drop 2 patches previously applied
Simon Glass (4):
fdtput: Require a leading '/' with -p
fdtput: Follow an alias when creating nodes with -p
fdtput: Suggest -p when a node is missing
fdtput: Name the property when it cannot be deleted
fdtput.c | 41 ++++++++++++++++++++++++++++++-----
tests/run_tests.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 88 insertions(+), 7 deletions(-)
---
base-commit: 1d50f3f8fa8bf9429eebb0769dafa5df37381ae1
branch: fdtput-nodes2
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/4] fdtput: Require a leading '/' with -p
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
@ 2026-10-28 7:34 ` Simon Glass
2026-10-28 20:14 ` Herve Codina
2026-10-28 7:34 ` [PATCH v2 2/4] fdtput: Follow an alias when creating nodes " Simon Glass
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2026-10-28 7:34 UTC (permalink / raw)
To: Devicetree Compiler; +Cc: David Gibson, Simon Glass
With -p, a path which does not start with '/' is created from the root,
so a mistyped path silently adds nodes at the top level:
$ fdtput -cp test.dtb foo/bar
This creates /foo/bar. The same happens for a path which starts with an
alias, such as serial0/sub, which creates /serial0/sub rather than
adding to the node which the alias points to.
Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
path without a leading '/' as starting with an alias:
$ fdtput -c test.dtb foo/bar
Error at 'foo': FDT_ERR_BADPATH
Report an error with -p too. The next patch allows such a path to start
with an alias.
This changes the behaviour of -p, so a script which relies on creating
nodes this way must add a leading '/'. Do this for the existing test
which relies on it, and add a test for the error.
Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add new patch to reject a path without a leading '/'
fdtput.c | 9 +++++++--
tests/run_tests.sh | 6 +++++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/fdtput.c b/fdtput.c
index c139de1..d0f8420 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -201,8 +201,8 @@ static int store_key_value(char **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.
+ * Any components of the path that do not exist are created. The path
+ * must start with '/'. Errors are reported.
*
* @param blob FDT blob to write into
* @param in_path Path to process
@@ -214,6 +214,11 @@ static int create_paths(char **blob, const char *in_path)
const char *sep;
int node, offset = 0;
+ if (*path != '/') {
+ report_error(path, -1, -FDT_ERR_BADPATH);
+ return -1;
+ }
+
/* skip leading '/' */
while (*path == '/')
path++;
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 842b543..644c405 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -970,8 +970,12 @@ fdtput_tests () {
"-ts" "fine wine"
run_wrap_test $DTPUT $dtb -p /you/are/drunk/sir/winston slurp -ts twice
+ # A path must start with '/'
+ run_wrap_error_test $DTPUT $dtb -cp baldrick/turnip
+ run_wrap_error_test $DTGET $dtb -l /baldrick
+
# Test expansion of the blob when insufficient room for a new node
- run_wrap_test $DTPUT $dtb -cp "$(cat $text $text)/longish"
+ run_wrap_test $DTPUT $dtb -cp "/$(cat $text $text)/longish"
# Allowed to create an existing node with -p
run_wrap_test $DTPUT $dtb -cp /chosen
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] fdtput: Follow an alias when creating nodes with -p
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
2026-10-28 7:34 ` [PATCH v2 1/4] fdtput: Require a leading '/' with -p Simon Glass
@ 2026-10-28 7:34 ` Simon Glass
2026-10-28 20:34 ` Herve Codina
2026-10-28 7:34 ` [PATCH v2 3/4] fdtput: Suggest -p when a node is missing Simon Glass
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2026-10-28 7:34 UTC (permalink / raw)
To: Devicetree Compiler; +Cc: David Gibson, Simon Glass
A node path can start with an alias, such as serial0/sub. fdtput allows
this when writing to an existing node, but -p requires the path to
start with '/', so it cannot create nodes below an alias:
$ fdtput -cp test.dtb serial0/sub
Error at 'serial0/sub': FDT_ERR_BADPATH
When the path does not start with '/', look up its first component as an
alias and create any missing nodes below the node which the alias points
to. A path which starts with neither '/' nor an alias is still
rejected.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Rework on top of the new patch which requires a leading '/'
- Drop the Fixes tag, since the new patch fixes the bug
fdtput.c | 17 +++++++++++++----
tests/run_tests.sh | 9 ++++++++-
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/fdtput.c b/fdtput.c
index d0f8420..3c6cc98 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -201,8 +201,9 @@ static int store_key_value(char **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. The path
- * must start with '/'. Errors are reported.
+ * Any components of the path that do not exist are created. If the path
+ * starts with an alias, they are created below the node which the alias
+ * points to. Otherwise the path must start with '/'. Errors are reported.
*
* @param blob FDT blob to write into
* @param in_path Path to process
@@ -214,9 +215,17 @@ static int create_paths(char **blob, const char *in_path)
const char *sep;
int node, offset = 0;
+ /* a path which does not start with '/' must start with an alias */
if (*path != '/') {
- report_error(path, -1, -FDT_ERR_BADPATH);
- return -1;
+ sep = strchr(path, '/');
+ if (!sep)
+ sep = path + strlen(path);
+ offset = fdt_path_offset_namelen(*blob, path, sep - path);
+ if (offset < 0) {
+ report_error(path, -1, offset);
+ return -1;
+ }
+ path = sep;
}
/* skip leading '/' */
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 644c405..b3d3f3a 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -970,7 +970,14 @@ fdtput_tests () {
"-ts" "fine wine"
run_wrap_test $DTPUT $dtb -p /you/are/drunk/sir/winston slurp -ts twice
- # A path must start with '/'
+ # Automatic node creation below an alias
+ run_wrap_test $DTPUT $dtb -p /aliases second -ts /blackadder/the-second
+ run_wrap_test $DTPUT $dtb -cp second/carrot
+ run_fdtput_test "orange" $dtb second/carrot colour -ts "orange"
+ run_fdtget_test "orange" $dtb /blackadder/the-second/carrot colour
+ run_wrap_error_test $DTGET $dtb -l /second
+
+ # A path must start with '/' or an alias
run_wrap_error_test $DTPUT $dtb -cp baldrick/turnip
run_wrap_error_test $DTGET $dtb -l /baldrick
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] fdtput: Suggest -p when a node is missing
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
2026-10-28 7:34 ` [PATCH v2 1/4] fdtput: Require a leading '/' with -p Simon Glass
2026-10-28 7:34 ` [PATCH v2 2/4] fdtput: Follow an alias when creating nodes " Simon Glass
@ 2026-10-28 7:34 ` Simon Glass
2026-10-28 21:08 ` Herve Codina
2026-10-28 7:34 ` [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted Simon Glass
2026-10-29 2:06 ` [PATCH v2 0/4] fdtput: Make it clearer how to create nodes David Gibson
4 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2026-10-28 7:34 UTC (permalink / raw)
To: Devicetree Compiler; +Cc: David Gibson, Simon Glass
When the node being written to does not exist, fdtput just prints
FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
it is easy to conclude that fdtput cannot add nodes at all.
When creating a node with -c, the error also names the full path, even
though it is the parent which is missing.
Add a hint to use -p when a node is missing, both when writing a
property and when creating a node. Report the parent path when it is
the parent which is missing.
Add tests that writing a property or creating a node fails when a parent
node is missing and -p is not given, and that the error suggests -p.
Add a run_wrap_error_msg_test() helper for this, which checks the
output as well as the return code.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Check that the error suggests -p, using a new helper
- Drop David's Reviewed-by, since the tests have changed
fdtput.c | 20 ++++++++++++++++++--
tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/fdtput.c b/fdtput.c
index 3c6cc98..b8d5081 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -47,6 +47,22 @@ static void report_error(const char *name, int namelen, int err)
fdt_strerror(err));
}
+/**
+ * Report an error looking up a node which must exist before writing to it.
+ *
+ * If the node is missing, suggest -p, since it creates missing nodes.
+ *
+ * @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_node_error(const char *name, int namelen, int err)
+{
+ report_error(name, namelen, err);
+ if (err == -FDT_ERR_NOTFOUND)
+ fprintf(stderr, "Use -p to create missing nodes\n");
+}
+
/**
* Encode a series of arguments in a property value.
*
@@ -182,7 +198,7 @@ static int store_key_value(char **blob, const char *node_name,
node = fdt_path_offset(*blob, node_name);
if (node < 0) {
- report_error(node_name, -1, node);
+ report_node_error(node_name, -1, node);
return -1;
}
@@ -284,7 +300,7 @@ static int create_node(char **blob, const char *node_name)
node = fdt_path_offset(*blob, path);
free(path);
if (node < 0) {
- report_error(node_name, -1, node);
+ report_node_error(node_name, p - node_name, node);
return -1;
}
}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index b3d3f3a..cf978ee 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -181,6 +181,37 @@ run_wrap_error_test () {
base_run_test wrap_error "$@"
}
+# Run a command, checking that it fails and that its output has $1 in it
+wrap_error_msg () {
+ (
+ msg="$1"
+ shift
+ log=tmp.log.$$
+ trap "rm -f $log" 0
+ verbose_run_log "$log" "$@"
+ ret="$?"
+ FAIL_IF_SIGNAL $ret
+ if [ "$ret" -eq 0 ]; then
+ FAIL "Expected non-zero return code"
+ fi
+ if ! grep -qF -- "$msg" "$log"; then
+ FAIL "Expected message '$msg'"
+ fi
+ PASS
+ )
+}
+
+# Check that a command fails and prints a message
+# $1: message to look for in the output
+# $2...: command to run
+run_wrap_error_msg_test () {
+ msg="$1"
+ shift
+ shorten_echo "$@"
+ printf " {!= 0, \"%s\"}: " "$msg"
+ base_run_test wrap_error_msg "$msg" "$@"
+}
+
# $1: dtb file
# $2: align base
check_align () {
@@ -962,6 +993,12 @@ fdtput_tests () {
run_wrap_error_test $DTPUT $dtb -c /chosen
run_wrap_error_test $DTPUT $dtb -c /chosen/son
+ # Writing below a missing node fails without -p, suggesting -p
+ run_wrap_error_msg_test "Use -p to create missing nodes" \
+ $DTPUT $dtb -c /blackadder/the-second
+ run_wrap_error_msg_test "Use -p to create missing nodes" \
+ $DTPUT $dtb /blackadder/the-second cost -ts high
+
# Automatic node creation
run_wrap_test $DTPUT $dtb -cp /blackadder/the-second/turnip \
/blackadder/the-second/potato
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
` (2 preceding siblings ...)
2026-10-28 7:34 ` [PATCH v2 3/4] fdtput: Suggest -p when a node is missing Simon Glass
@ 2026-10-28 7:34 ` Simon Glass
2026-10-28 21:18 ` Herve Codina
2026-10-29 2:06 ` [PATCH v2 0/4] fdtput: Make it clearer how to create nodes David Gibson
4 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2026-10-28 7:34 UTC (permalink / raw)
To: Devicetree Compiler; +Cc: David Gibson, Simon Glass
When 'fdtput -d' fails to delete a property, for example because it does
not exist, the error names the node rather than the property:
$ fdtput -d test.dtb /chosen missing
Error at '/chosen': FDT_ERR_NOTFOUND
This suggests that the node is missing, when it is the property that
is.
Report the property name as well as the node:
$ fdtput -d test.dtb /chosen missing
Error at '/chosen', property 'missing': FDT_ERR_NOTFOUND
Check the message in the existing test for this.
Fixes: aa719618a8b7 ("fdtput: add delete node and property function")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Report the node as well as the property
- Check the error message in the test
fdtput.c | 3 ++-
tests/run_tests.sh | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/fdtput.c b/fdtput.c
index b8d5081..610cad4 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -334,7 +334,8 @@ static int delete_prop(char *blob, const char *node_name, const char *prop_name)
node = fdt_delprop(blob, node, prop_name);
if (node < 0) {
- report_error(node_name, -1, node);
+ fprintf(stderr, "Error at '%s', property '%s': %s\n",
+ node_name, prop_name, fdt_strerror(node));
return -1;
}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index cf978ee..5bc7817 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -1045,7 +1045,9 @@ fdtput_tests () {
run_fdtget_test "bootargs\nlinux,platform" $dtb -p /chosen
# Delete the non-existent property
- run_wrap_error_test $DTPUT $dtb -d /chosen non-existent-prop
+ run_wrap_error_msg_test \
+ "Error at '/chosen', property 'non-existent-prop'" \
+ $DTPUT $dtb -d /chosen non-existent-prop
# TODO: Add tests for verbose mode?
}
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p
2026-10-28 7:34 ` [PATCH v2 1/4] fdtput: Require a leading '/' with -p Simon Glass
@ 2026-10-28 20:14 ` Herve Codina
2026-10-29 2:07 ` David Gibson
0 siblings, 1 reply; 13+ messages in thread
From: Herve Codina @ 2026-10-28 20:14 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, David Gibson
On Wed, 28 Oct 2026 01:34:17 -0600
Simon Glass <sjg@chromium.org> wrote:
> With -p, a path which does not start with '/' is created from the root,
> so a mistyped path silently adds nodes at the top level:
>
> $ fdtput -cp test.dtb foo/bar
>
> This creates /foo/bar. The same happens for a path which starts with an
> alias, such as serial0/sub, which creates /serial0/sub rather than
> adding to the node which the alias points to.
>
> Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> path without a leading '/' as starting with an alias:
>
> $ fdtput -c test.dtb foo/bar
> Error at 'foo': FDT_ERR_BADPATH
>
> Report an error with -p too. The next patch allows such a path to start
> with an alias.
I would reword "The next patch allows" using "A future patch could allow" or
I would simply remove the whole sentence ("The next patch ...");
Indeed, this current patch can be applied without the next one.
Other than that:
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
>
> This changes the behaviour of -p, so a script which relies on creating
> nodes this way must add a leading '/'. Do this for the existing test
> which relies on it, and add a test for the error.
>
> Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] fdtput: Follow an alias when creating nodes with -p
2026-10-28 7:34 ` [PATCH v2 2/4] fdtput: Follow an alias when creating nodes " Simon Glass
@ 2026-10-28 20:34 ` Herve Codina
0 siblings, 0 replies; 13+ messages in thread
From: Herve Codina @ 2026-10-28 20:34 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, David Gibson
Hi Simon,
On Wed, 28 Oct 2026 01:34:18 -0600
Simon Glass <sjg@chromium.org> wrote:
> A node path can start with an alias, such as serial0/sub. fdtput allows
> this when writing to an existing node, but -p requires the path to
> start with '/', so it cannot create nodes below an alias:
>
> $ fdtput -cp test.dtb serial0/sub
> Error at 'serial0/sub': FDT_ERR_BADPATH
>
> When the path does not start with '/', look up its first component as an
> alias and create any missing nodes below the node which the alias points
> to. A path which starts with neither '/' nor an alias is still
> rejected.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
LGTM.
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] fdtput: Suggest -p when a node is missing
2026-10-28 7:34 ` [PATCH v2 3/4] fdtput: Suggest -p when a node is missing Simon Glass
@ 2026-10-28 21:08 ` Herve Codina
2026-10-29 2:02 ` David Gibson
0 siblings, 1 reply; 13+ messages in thread
From: Herve Codina @ 2026-10-28 21:08 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, David Gibson
Hi Simon,
On Wed, 28 Oct 2026 01:34:19 -0600
Simon Glass <sjg@chromium.org> wrote:
> When the node being written to does not exist, fdtput just prints
> FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
> it is easy to conclude that fdtput cannot add nodes at all.
>
> When creating a node with -c, the error also names the full path, even
> though it is the parent which is missing.
>
> Add a hint to use -p when a node is missing, both when writing a
> property and when creating a node. Report the parent path when it is
> the parent which is missing.
>
> Add tests that writing a property or creating a node fails when a parent
> node is missing and -p is not given, and that the error suggests -p.
> Add a run_wrap_error_msg_test() helper for this, which checks the
> output as well as the return code.
Nice idea!
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Check that the error suggests -p, using a new helper
> - Drop David's Reviewed-by, since the tests have changed
>
> fdtput.c | 20 ++++++++++++++++++--
> tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/fdtput.c b/fdtput.c
> index 3c6cc98..b8d5081 100644
> --- a/fdtput.c
> +++ b/fdtput.c
...
I am ok with the implementation in fdtput.c.
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index b3d3f3a..cf978ee 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -181,6 +181,37 @@ run_wrap_error_test () {
> base_run_test wrap_error "$@"
> }
>
> +# Run a command, checking that it fails and that its output has $1 in it
> +wrap_error_msg () {
> + (
> + msg="$1"
> + shift
> + log=tmp.log.$$
> + trap "rm -f $log" 0
> + verbose_run_log "$log" "$@"
> + ret="$?"
> + FAIL_IF_SIGNAL $ret
> + if [ "$ret" -eq 0 ]; then
> + FAIL "Expected non-zero return code"
> + fi
> + if ! grep -qF -- "$msg" "$log"; then
> + FAIL "Expected message '$msg'"
> + fi
> + PASS
> + )
> +}
> +
> +# Check that a command fails and prints a message
> +# $1: message to look for in the output
> +# $2...: command to run
> +run_wrap_error_msg_test () {
> + msg="$1"
> + shift
> + shorten_echo "$@"
> + printf " {!= 0, \"%s\"}: " "$msg"
> + base_run_test wrap_error_msg "$msg" "$@"
> +}
> +
Right, you introduce this wrapper and I think they can be useful for other
cases.
You also sent the series "dtc: Fix deletes within a section and before a
redefinition" [1].
The patch "tests: Allow dtc-fatal.sh to check the error message" [2]
available in the series plays also with error messages, adding a "-m" option
to tests/dtc-fatal.sh.
At some point, could all tests use the same uniform method (probably the
wrappers added here)?
Could dtc-fatal.sh script be replaced by an existing wrapper?
Again, I think wrappers you added here are nice. I would like to have your
opinion related to dtc-fatal.sh and a way to avoid the '-m' option in order
to have uniform features.
[1]: https://lore.kernel.org/devicetree-compiler/20261028093620.3705074-1-sjg@chromium.org/
[2]: https://lore.kernel.org/devicetree-compiler/20261028093620.3705074-5-sjg@chromium.org/
> # $1: dtb file
> # $2: align base
> check_align () {
> @@ -962,6 +993,12 @@ fdtput_tests () {
> run_wrap_error_test $DTPUT $dtb -c /chosen
> run_wrap_error_test $DTPUT $dtb -c /chosen/son
>
> + # Writing below a missing node fails without -p, suggesting -p
> + run_wrap_error_msg_test "Use -p to create missing nodes" \
> + $DTPUT $dtb -c /blackadder/the-second
> + run_wrap_error_msg_test "Use -p to create missing nodes" \
> + $DTPUT $dtb /blackadder/the-second cost -ts high
> +
> # Automatic node creation
> run_wrap_test $DTPUT $dtb -cp /blackadder/the-second/turnip \
> /blackadder/the-second/potato
Best regards,
Hervé
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted
2026-10-28 7:34 ` [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted Simon Glass
@ 2026-10-28 21:18 ` Herve Codina
0 siblings, 0 replies; 13+ messages in thread
From: Herve Codina @ 2026-10-28 21:18 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, David Gibson
Hi Simon,
On Wed, 28 Oct 2026 01:34:20 -0600
Simon Glass <sjg@chromium.org> wrote:
> When 'fdtput -d' fails to delete a property, for example because it does
> not exist, the error names the node rather than the property:
>
> $ fdtput -d test.dtb /chosen missing
> Error at '/chosen': FDT_ERR_NOTFOUND
>
> This suggests that the node is missing, when it is the property that
> is.
>
> Report the property name as well as the node:
>
> $ fdtput -d test.dtb /chosen missing
> Error at '/chosen', property 'missing': FDT_ERR_NOTFOUND
>
> Check the message in the existing test for this.
>
> Fixes: aa719618a8b7 ("fdtput: add delete node and property function")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
This patch will depends on discussion on previous one.
If test wrappers introduced previously are accepted (IHMO again, they are
good), for this current patch:
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] fdtput: Suggest -p when a node is missing
2026-10-28 21:08 ` Herve Codina
@ 2026-10-29 2:02 ` David Gibson
0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2026-10-29 2:02 UTC (permalink / raw)
To: Herve Codina; +Cc: Simon Glass, Devicetree Compiler
[-- Attachment #1: Type: text/plain, Size: 3844 bytes --]
On Wed, Oct 28, 2026 at 10:08:54PM +0100, Herve Codina wrote:
> Hi Simon,
>
> On Wed, 28 Oct 2026 01:34:19 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > When the node being written to does not exist, fdtput just prints
> > FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
> > it is easy to conclude that fdtput cannot add nodes at all.
> >
> > When creating a node with -c, the error also names the full path, even
> > though it is the parent which is missing.
> >
> > Add a hint to use -p when a node is missing, both when writing a
> > property and when creating a node. Report the parent path when it is
> > the parent which is missing.
> >
> > Add tests that writing a property or creating a node fails when a parent
> > node is missing and -p is not given, and that the error suggests -p.
> > Add a run_wrap_error_msg_test() helper for this, which checks the
> > output as well as the return code.
>
> Nice idea!
>
> >
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Check that the error suggests -p, using a new helper
> > - Drop David's Reviewed-by, since the tests have changed
> >
> > fdtput.c | 20 ++++++++++++++++++--
> > tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 55 insertions(+), 2 deletions(-)
> >
> > diff --git a/fdtput.c b/fdtput.c
> > index 3c6cc98..b8d5081 100644
> > --- a/fdtput.c
> > +++ b/fdtput.c
>
> ...
>
> I am ok with the implementation in fdtput.c.
>
>
> > diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> > index b3d3f3a..cf978ee 100755
> > --- a/tests/run_tests.sh
> > +++ b/tests/run_tests.sh
> > @@ -181,6 +181,37 @@ run_wrap_error_test () {
> > base_run_test wrap_error "$@"
> > }
> >
> > +# Run a command, checking that it fails and that its output has $1 in it
> > +wrap_error_msg () {
> > + (
> > + msg="$1"
> > + shift
> > + log=tmp.log.$$
> > + trap "rm -f $log" 0
> > + verbose_run_log "$log" "$@"
> > + ret="$?"
> > + FAIL_IF_SIGNAL $ret
> > + if [ "$ret" -eq 0 ]; then
> > + FAIL "Expected non-zero return code"
> > + fi
> > + if ! grep -qF -- "$msg" "$log"; then
> > + FAIL "Expected message '$msg'"
> > + fi
> > + PASS
> > + )
> > +}
> > +
> > +# Check that a command fails and prints a message
> > +# $1: message to look for in the output
> > +# $2...: command to run
> > +run_wrap_error_msg_test () {
> > + msg="$1"
> > + shift
> > + shorten_echo "$@"
> > + printf " {!= 0, \"%s\"}: " "$msg"
> > + base_run_test wrap_error_msg "$msg" "$@"
> > +}
> > +
>
> Right, you introduce this wrapper and I think they can be useful for other
> cases.
>
> You also sent the series "dtc: Fix deletes within a section and before a
> redefinition" [1].
>
> The patch "tests: Allow dtc-fatal.sh to check the error message" [2]
> available in the series plays also with error messages, adding a "-m" option
> to tests/dtc-fatal.sh.
>
> At some point, could all tests use the same uniform method (probably the
> wrappers added here)?
>
> Could dtc-fatal.sh script be replaced by an existing wrapper?
>
> Again, I think wrappers you added here are nice. I would like to have your
> opinion related to dtc-fatal.sh and a way to avoid the '-m' option in order
> to have uniform features.
Yes, it would be good to consolidate. For now, this series is ready
and the other still needs review, so I think we make it adjust to the
changes from this one, rather than the other way around.
--
David Gibson (he or they) | 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/4] fdtput: Make it clearer how to create nodes
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
` (3 preceding siblings ...)
2026-10-28 7:34 ` [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted Simon Glass
@ 2026-10-29 2:06 ` David Gibson
4 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2026-10-29 2:06 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler
[-- Attachment #1: Type: text/plain, Size: 951 bytes --]
On Wed, Oct 28, 2026 at 01:34:16AM -0600, Simon Glass wrote:
> fdtput can create nodes with -c, and -p creates any missing nodes along
> the way, but this is easy to miss. A user has asked for a way to add
> nodes, not realising that one exists.
>
> The first patch makes -p reject a path without a leading '/', rather
> than creating it from the root, as fdtput already does without -p. The
> second allows such a path to start with an alias, creating nodes below
> the node which the alias points to. The third suggests -p when a node
> is missing, naming the parent which is missing. The fourth names the
> property, as well as the node, when it cannot be deleted.
>
> Link: https://github.com/dgibson/dtc/issues/49
Merged, thanks.
--
David Gibson (he or they) | 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p
2026-10-28 20:14 ` Herve Codina
@ 2026-10-29 2:07 ` David Gibson
2026-10-29 6:44 ` Simon Glass
0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2026-10-29 2:07 UTC (permalink / raw)
To: Herve Codina; +Cc: Simon Glass, Devicetree Compiler
[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]
On Wed, Oct 28, 2026 at 09:14:49PM +0100, Herve Codina wrote:
> On Wed, 28 Oct 2026 01:34:17 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > With -p, a path which does not start with '/' is created from the root,
> > so a mistyped path silently adds nodes at the top level:
> >
> > $ fdtput -cp test.dtb foo/bar
> >
> > This creates /foo/bar. The same happens for a path which starts with an
> > alias, such as serial0/sub, which creates /serial0/sub rather than
> > adding to the node which the alias points to.
> >
> > Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> > path without a leading '/' as starting with an alias:
> >
> > $ fdtput -c test.dtb foo/bar
> > Error at 'foo': FDT_ERR_BADPATH
> >
> > Report an error with -p too. The next patch allows such a path to start
> > with an alias.
>
> I would reword "The next patch allows" using "A future patch could allow" or
> I would simply remove the whole sentence ("The next patch ...");
As a rule, that is a better way of phrasing a commit message, so that
it doesn't become inaccurate if this are reordered. In this case it's
not worth a respin, though.
>
> Indeed, this current patch can be applied without the next one.
>
> Other than that:
> Reviewed-by: Herve Codina <herve.codina@bootlin.com>
>
> Best regards,
> Hervé
>
> >
> > This changes the behaviour of -p, so a script which relies on creating
> > nodes this way must add a leading '/'. Do this for the existing test
> > which relies on it, and add a test for the error.
> >
> > Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
>
--
David Gibson (he or they) | 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p
2026-10-29 2:07 ` David Gibson
@ 2026-10-29 6:44 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2026-10-29 6:44 UTC (permalink / raw)
To: David Gibson; +Cc: Herve Codina, Devicetree Compiler
Hi David, Herve,
On Thu, 29 Oct 2026 at 02:07, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Wed, Oct 28, 2026 at 09:14:49PM +0100, Herve Codina wrote:
> > On Wed, 28 Oct 2026 01:34:17 -0600
> > Simon Glass <sjg@chromium.org> wrote:
> >
> > > With -p, a path which does not start with '/' is created from the root,
> > > so a mistyped path silently adds nodes at the top level:
> > >
> > > $ fdtput -cp test.dtb foo/bar
> > >
> > > This creates /foo/bar. The same happens for a path which starts with an
> > > alias, such as serial0/sub, which creates /serial0/sub rather than
> > > adding to the node which the alias points to.
> > >
> > > Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> > > path without a leading '/' as starting with an alias:
> > >
> > > $ fdtput -c test.dtb foo/bar
> > > Error at 'foo': FDT_ERR_BADPATH
> > >
> > > Report an error with -p too. The next patch allows such a path to start
> > > with an alias.
> >
> > I would reword "The next patch allows" using "A future patch could allow" or
> > I would simply remove the whole sentence ("The next patch ...");
>
> As a rule, that is a better way of phrasing a commit message, so that
> it doesn't become inaccurate if this are reordered. In this case it's
> not worth a respin, though.
>
> >
> > Indeed, this current patch can be applied without the next one.
> >
> > Other than that:
> > Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Thanks both, yes that was better left out.
> >
> > Best regards,
> > Hervé
> >
> > >
> > > This changes the behaviour of -p, so a script which relies on creating
> > > nodes this way must add a leading '/'. Do this for the existing test
> > > which relies on it, and add a test for the error.
> > >
> > > Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> > > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> >
[..]
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-29 6:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 7:34 [PATCH v2 0/4] fdtput: Make it clearer how to create nodes Simon Glass
2026-10-28 7:34 ` [PATCH v2 1/4] fdtput: Require a leading '/' with -p Simon Glass
2026-10-28 20:14 ` Herve Codina
2026-10-29 2:07 ` David Gibson
2026-10-29 6:44 ` Simon Glass
2026-10-28 7:34 ` [PATCH v2 2/4] fdtput: Follow an alias when creating nodes " Simon Glass
2026-10-28 20:34 ` Herve Codina
2026-10-28 7:34 ` [PATCH v2 3/4] fdtput: Suggest -p when a node is missing Simon Glass
2026-10-28 21:08 ` Herve Codina
2026-10-29 2:02 ` David Gibson
2026-10-28 7:34 ` [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted Simon Glass
2026-10-28 21:18 ` Herve Codina
2026-10-29 2:06 ` [PATCH v2 0/4] fdtput: Make it clearer how to create nodes David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.