[U-Boot] [PATCH v2 1/2] libfdt: Correct fdt handling of overlays without fixups and base trees without symbols

From: David Gibson david@gibson.dropbear.id.au
The fdt_overlay_apply() function purports to support the edge cases where an overlay has no fixups to be applied, or a base tree which has no symbols (the latter can only work if the former is also true). However it gets it wrong in a couple of small ways:
* In the no fixups case, it doesn't fail immediately, but will attempt fdt_for_each_property_offset() giving -FDT_ERR_NOTFOUND as the node offset, which will fail. Instead it should succeed immediately, since there's nothing to do. * In the case of no symbols, it again doesn't fail immediately. However if there is an actual fixup it will fail with an unexpected error, because -FDT_ERR_NOTFOUND is passed to fdt_getprop() when attempting to look up the symbols. We should instead return -FDT_ERR_NOTFOUND directly.
Both of these errors lead to the code returning misleading error codes in failing cases.
[ DTC commit: 7d8ef6e1db9794f72805a0855f4f7f12fadd03d3 ]
Signed-off-by: David Gibson david@gibson.dropbear.id.au Signed-off-by: Stefan Agner stefan.agner@toradex.com ---
Changes in v2: - Backported patch from libfdt
lib/libfdt/fdt_overlay.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c index bb41404..56cb70e 100644 --- a/lib/libfdt/fdt_overlay.c +++ b/lib/libfdt/fdt_overlay.c @@ -359,6 +359,9 @@ static int overlay_fixup_one_phandle(void *fdt, void *fdto, int symbol_off, fixup_off; int prop_len;
+ if (symbols_off < 0) + return symbols_off; + symbol_path = fdt_getprop(fdt, symbols_off, label, &prop_len); if (!symbol_path) @@ -492,7 +495,9 @@ static int overlay_fixup_phandles(void *fdt, void *fdto)
/* We can have overlays without any fixups */ fixups_off = fdt_path_offset(fdto, "/__fixups__"); - if ((fixups_off < 0 && (fixups_off != -FDT_ERR_NOTFOUND))) + if (fixups_off == -FDT_ERR_NOTFOUND) + return 0; /* nothing to do */ + if (fixups_off < 0) return fixups_off;
/* And base DTs without symbols */

From: Stefan Agner stefan.agner@toradex.com
There are lots of reason why a FDT application might fail, the error code might give an indication. Let the error code translate in a error string so users can try to understand what went wrong.
Signed-off-by: Stefan Agner stefan.agner@toradex.com Acked-by: Maxime Ripard maxime.ripard@free-electrons.com Acked-by: Simon Glass sjg@chromium.org --- Currently I get this when I apply a device tree overlay to a device tree without symbols:
Colibri iMX7 # fdt apply ${loadaddr} Colibri iMX7 # fdt print libfdt fdt_path_offset() returned FDT_ERR_BADMAGIC Colibri iMX7 #
With this change, the situation is a bit more indicative:
Colibri iMX7 # fdt apply ${loadaddr} fdt_overlay_apply(): FDT_ERR_NOTFOUND Colibri iMX7 # fdt print libfdt fdt_path_offset() returned FDT_ERR_BADMAGIC Colibri iMX7 #
Changes in v2: None
cmd/fdt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 8bd345a..6883e75 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -642,6 +642,7 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) else if (strncmp(argv[1], "ap", 2) == 0) { unsigned long addr; struct fdt_header *blob; + int ret;
if (argc != 3) return CMD_RET_USAGE; @@ -654,8 +655,11 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!fdt_valid(&blob)) return CMD_RET_FAILURE;
- if (fdt_overlay_apply(working_fdt, blob)) + ret = fdt_overlay_apply(working_fdt, blob); + if (ret) { + printf("fdt_overlay_apply(): %s\n", fdt_strerror(ret)); return CMD_RET_FAILURE; + } } #endif /* resize the fdt */

On 21 December 2016 at 03:58, Stefan Agner stefan@agner.ch wrote:
From: David Gibson david@gibson.dropbear.id.au
The fdt_overlay_apply() function purports to support the edge cases where an overlay has no fixups to be applied, or a base tree which has no symbols (the latter can only work if the former is also true). However it gets it wrong in a couple of small ways:
- In the no fixups case, it doesn't fail immediately, but will attempt fdt_for_each_property_offset() giving -FDT_ERR_NOTFOUND as the node offset, which will fail. Instead it should succeed immediately, since there's nothing to do.
- In the case of no symbols, it again doesn't fail immediately. However if there is an actual fixup it will fail with an unexpected error, because -FDT_ERR_NOTFOUND is passed to fdt_getprop() when attempting to look up the symbols. We should instead return -FDT_ERR_NOTFOUND directly.
Both of these errors lead to the code returning misleading error codes in failing cases.
[ DTC commit: 7d8ef6e1db9794f72805a0855f4f7f12fadd03d3 ]
Signed-off-by: David Gibson david@gibson.dropbear.id.au Signed-off-by: Stefan Agner stefan.agner@toradex.com
Changes in v2:
- Backported patch from libfdt
lib/libfdt/fdt_overlay.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org

On 25 December 2016 at 22:24, Simon Glass sjg@chromium.org wrote:
On 21 December 2016 at 03:58, Stefan Agner stefan@agner.ch wrote:
From: David Gibson david@gibson.dropbear.id.au
The fdt_overlay_apply() function purports to support the edge cases where an overlay has no fixups to be applied, or a base tree which has no symbols (the latter can only work if the former is also true). However it gets it wrong in a couple of small ways:
- In the no fixups case, it doesn't fail immediately, but will attempt fdt_for_each_property_offset() giving -FDT_ERR_NOTFOUND as the node offset, which will fail. Instead it should succeed immediately, since there's nothing to do.
- In the case of no symbols, it again doesn't fail immediately. However if there is an actual fixup it will fail with an unexpected error, because -FDT_ERR_NOTFOUND is passed to fdt_getprop() when attempting to look up the symbols. We should instead return -FDT_ERR_NOTFOUND directly.
Both of these errors lead to the code returning misleading error codes in failing cases.
[ DTC commit: 7d8ef6e1db9794f72805a0855f4f7f12fadd03d3 ]
Signed-off-by: David Gibson david@gibson.dropbear.id.au Signed-off-by: Stefan Agner stefan.agner@toradex.com
Changes in v2:
- Backported patch from libfdt
lib/libfdt/fdt_overlay.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-fdt, thanks!
participants (2)
-
Simon Glass
-
Stefan Agner