[PATCH] riscv: Fix arch_fixup_fdt always failing without /chosen

If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together") Signed-off-by: Sean Anderson seanga2@gmail.com --- I have not actually tested this (nor observed the original failure). But this seemed buggy from inspection.
arch/riscv/lib/fdt_fixup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 1f3f23621c..f636b28449 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -151,14 +151,17 @@ int arch_fixup_fdt(void *blob) } chosen_offset = fdt_path_offset(blob, "/chosen"); if (chosen_offset < 0) { - err = fdt_add_subnode(blob, 0, "chosen"); - if (err < 0) { + chosen_offset = fdt_add_subnode(blob, 0, "chosen"); + if (chosen_offset < 0) { log_err("chosen node cannot be added\n"); - return err; + return chosen_offset; } } /* Overwrite the boot-hartid as U-Boot is the last stage BL */ - fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart); + err = fdt_setprop_u32(blob, chosen_offset, "boot-hartid", + gd->arch.boot_hart); + if (err < 0) + return log_msg_ret("could not set boot-hartid", err); #endif
/* Copy the reserved-memory node to the DT used by OS */

On Sun, May 9, 2021 at 12:13 AM Sean Anderson seanga2@gmail.com wrote:
If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together")
I think the correct commit id to fix is: commit 5370478d1c7afb8b2a8a0a00b31ff97c3a2c0451 ("riscv: Add boot hartid to device tree")
Signed-off-by: Sean Anderson seanga2@gmail.com
I have not actually tested this (nor observed the original failure). But this seemed buggy from inspection.
arch/riscv/lib/fdt_fixup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 1f3f23621c..f636b28449 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -151,14 +151,17 @@ int arch_fixup_fdt(void *blob) } chosen_offset = fdt_path_offset(blob, "/chosen"); if (chosen_offset < 0) {
err = fdt_add_subnode(blob, 0, "chosen");
if (err < 0) {
chosen_offset = fdt_add_subnode(blob, 0, "chosen");
if (chosen_offset < 0) { log_err("chosen node cannot be added\n");
return err;
return chosen_offset; } } /* Overwrite the boot-hartid as U-Boot is the last stage BL */
fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
err = fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
gd->arch.boot_hart);
if (err < 0)
return log_msg_ret("could not set boot-hartid", err);
#endif
/* Copy the reserved-memory node to the DT used by OS */
--
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On 5/8/21 12:22 PM, Bin Meng wrote:
On Sun, May 9, 2021 at 12:13 AM Sean Anderson seanga2@gmail.com wrote:
If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together")
I think the correct commit id to fix is: commit 5370478d1c7afb8b2a8a0a00b31ff97c3a2c0451 ("riscv: Add boot hartid to device tree")
Ah, so it is.
--Sean
Signed-off-by: Sean Anderson seanga2@gmail.com
I have not actually tested this (nor observed the original failure). But this seemed buggy from inspection.
arch/riscv/lib/fdt_fixup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 1f3f23621c..f636b28449 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -151,14 +151,17 @@ int arch_fixup_fdt(void *blob) } chosen_offset = fdt_path_offset(blob, "/chosen"); if (chosen_offset < 0) {
err = fdt_add_subnode(blob, 0, "chosen");
if (err < 0) {
chosen_offset = fdt_add_subnode(blob, 0, "chosen");
if (chosen_offset < 0) { log_err("chosen node cannot be added\n");
return err;
return chosen_offset; } } /* Overwrite the boot-hartid as U-Boot is the last stage BL */
fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
err = fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
gd->arch.boot_hart);
if (err < 0)
return log_msg_ret("could not set boot-hartid", err);
#endif
/* Copy the reserved-memory node to the DT used by OS */
--
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Sean,
On Sun, May 9, 2021 at 12:24 AM Sean Anderson seanga2@gmail.com wrote:
On 5/8/21 12:22 PM, Bin Meng wrote:
On Sun, May 9, 2021 at 12:13 AM Sean Anderson seanga2@gmail.com wrote:
If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together")
I think the correct commit id to fix is: commit 5370478d1c7afb8b2a8a0a00b31ff97c3a2c0451 ("riscv: Add boot hartid to device tree")
Ah, so it is.
Would you send v2 with the correct commit id?
Regards, Bin

If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together") Signed-off-by: Sean Anderson seanga2@gmail.com
I have not actually tested this (nor observed the original failure). But this seemed buggy from inspection.
arch/riscv/lib/fdt_fixup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
Good catch !
Reviewed-by: Rick Chen rick@andestech.com

On Sat, May 8, 2021 at 9:13 AM Sean Anderson seanga2@gmail.com wrote:
If /chosen was missing, chosen_offset would never get updated with the new /chosen node. This would cause fdt_setprop_u32 to fail. This patch fixes this by setting chosen_offset. In addition, log any errors from setting boot-hartid as well.
Fixes: 177c53fe6c6 ("riscv: Move all fdt fixups together") Signed-off-by: Sean Anderson seanga2@gmail.com
I have not actually tested this (nor observed the original failure). But this seemed buggy from inspection.
arch/riscv/lib/fdt_fixup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c index 1f3f23621c..f636b28449 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -151,14 +151,17 @@ int arch_fixup_fdt(void *blob) } chosen_offset = fdt_path_offset(blob, "/chosen"); if (chosen_offset < 0) {
err = fdt_add_subnode(blob, 0, "chosen");
if (err < 0) {
chosen_offset = fdt_add_subnode(blob, 0, "chosen");
if (chosen_offset < 0) { log_err("chosen node cannot be added\n");
return err;
return chosen_offset; } } /* Overwrite the boot-hartid as U-Boot is the last stage BL */
fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
err = fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
gd->arch.boot_hart);
if (err < 0)
return log_msg_ret("could not set boot-hartid", err);
#endif
/* Copy the reserved-memory node to the DT used by OS */
-- 2.31.0
Thanks for the fix. Good catch.
With update commit text as suggested by Bin: Reviewed-by: Atish Patra atish.patra@wdc.com
participants (4)
-
Atish Patra
-
Bin Meng
-
Rick Chen
-
Sean Anderson