[PATCH 0/5] sandbox: repair compile and run-time for OF_EMBED case
This patch-set repairs ability to use sandbox with CONFIG_OF_EMBED=y.
For now, to use OF_EMBED, the following must be done 1) sandbox64_defconfig should have: ``` -CONFIG_OF_LIVE=y +CONFIG_OF_EMBED=y ```
2) On sandbox when CONFIG_OF_EMBED=y, the u-boot process can't start due to: ``` Bloblist at b000 not found (err=-2) initcall failed at call 000000000011829c (err=-2: No such file or \ directory) ### ERROR ### Please RESET the board ### ``` So, it is natural desire to disable CONFIG_BLOBLIST just to test sandbox with OF_EMBED=y (disable it one way or another): ``` config SANDBOX - select BLOBLIST + select BLOBLIST if SOME_NON_EXISTING_OPTION ```
3) As a result, having such changes (CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n) leads to the link & run-time errors, being fixed by this patch series.
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- Tested: * locally with aforementioned options applied * CI - https://github.com/u-boot/u-boot/pull/704 * No regression
--- Evgeny Bachinin (5): sandbox: fix bloblist_finish() linker error if BLOBLIST=n test: sandbox: fix invalid_use_of_IF_ENABLED_INT if BLOBLIST=n test: sandbox: fix link error with do_ut_bloblist if BLOBLIST=n test: sandbox: fix link error with do_ut_bootm if BLOBLIST=n sandbox: set retval early in board_fdt_blob_setup()
arch/sandbox/cpu/cpu.c | 2 +- include/bloblist.h | 7 +++++++ test/cmd_ut.c | 2 ++ test/lib/kconfig.c | 10 ++++++---- 4 files changed, 16 insertions(+), 5 deletions(-) --- base-commit: 3073246d1be682071d8b3d07d06c2484907aed60 change-id: 20241120-sandbox_repair_of_embed-2188465eb22b
Best regards,
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error: ``` ld: /tmp/ccwtRVty.ltrans0.ltrans.o: in function `state_uninit': arch/sandbox/cpu/state.c:508: undefined reference to `bloblist_finish' ```
Fixes: 1c52fcca72b ("sandbox: Write out bloblist when exiting") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- include/bloblist.h | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/include/bloblist.h b/include/bloblist.h index ff32d3fecfd4e0474aa4047d1ae6421536570ec6..f999391f74bfd21098198bdd24dbb898a17320fd 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -357,6 +357,7 @@ int bloblist_new(ulong addr, uint size, uint flags, uint align_log2); */ int bloblist_check(ulong addr, uint size);
+#if CONFIG_IS_ENABLED(BLOBLIST) /** * bloblist_finish() - Set up the bloblist for the next U-Boot part * @@ -366,6 +367,12 @@ int bloblist_check(ulong addr, uint size); * Return: 0 */ int bloblist_finish(void); +#else +static inline int bloblist_finish(void) +{ + return 0; +} +#endif /* BLOBLIST */
/** * bloblist_get_stats() - Get information about the bloblist
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccwtRVty.ltrans0.ltrans.o: in function `state_uninit': arch/sandbox/cpu/state.c:508: undefined reference to `bloblist_finish'Fixes: 1c52fcca72b ("sandbox: Write out bloblist when exiting") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
include/bloblist.h | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error: ``` ld: /tmp/ccRVty.ltrans40.ltrans.o: in function `lib_test_is_enabled': test/lib/kconfig.c:24: undefined reference to \ `invalid_use_of_IF_ENABLED_INT' ld: test/lib/kconfig.c:26: undefined reference to \ `invalid_use_of_CONFIG_IF_ENABLED_INT' ```
Fixes: 29784d62ede ("test: Add some tests for kconfig.h") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- test/lib/kconfig.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/test/lib/kconfig.c b/test/lib/kconfig.c index 0c463bb794a5e1f9f2db995d544bd723f5e59ca0..a3645abf9464785461c7250260c816e6bca03193 100644 --- a/test/lib/kconfig.c +++ b/test/lib/kconfig.c @@ -21,10 +21,12 @@ static int lib_test_is_enabled(struct unit_test_state *uts) ut_asserteq(0, CONFIG_IS_ENABLED(OF_PLATDATA)); ut_asserteq(0, CONFIG_IS_ENABLED(_UNDEFINED));
- ut_asserteq(0xb000, - IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, CONFIG_BLOBLIST_ADDR)); - ut_asserteq(0xb000, - CONFIG_IF_ENABLED_INT(BLOBLIST_FIXED, BLOBLIST_ADDR)); + if (IS_ENABLED(CONFIG_BLOBLIST)) { + ut_asserteq(0xb000, IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, + CONFIG_BLOBLIST_ADDR)); + ut_asserteq(0xb000, CONFIG_IF_ENABLED_INT(BLOBLIST_FIXED, + BLOBLIST_ADDR)); + }
/* * This fails if CONFIG_TEST_KCONFIG_ENABLE is not enabled, since the
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccRVty.ltrans40.ltrans.o: in function `lib_test_is_enabled': test/lib/kconfig.c:24: undefined reference to \ `invalid_use_of_IF_ENABLED_INT' ld: test/lib/kconfig.c:26: undefined reference to \ `invalid_use_of_CONFIG_IF_ENABLED_INT'Fixes: 29784d62ede ("test: Add some tests for kconfig.h") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
test/lib/kconfig.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
This defeats the test, of course, but I suppose that is fine.
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/lib/kconfig.c b/test/lib/kconfig.c index 0c463bb794a5e1f9f2db995d544bd723f5e59ca0..a3645abf9464785461c7250260c816e6bca03193 100644 --- a/test/lib/kconfig.c +++ b/test/lib/kconfig.c @@ -21,10 +21,12 @@ static int lib_test_is_enabled(struct unit_test_state *uts) ut_asserteq(0, CONFIG_IS_ENABLED(OF_PLATDATA)); ut_asserteq(0, CONFIG_IS_ENABLED(_UNDEFINED));
ut_asserteq(0xb000,IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, CONFIG_BLOBLIST_ADDR));ut_asserteq(0xb000,CONFIG_IF_ENABLED_INT(BLOBLIST_FIXED, BLOBLIST_ADDR));
if (IS_ENABLED(CONFIG_BLOBLIST)) {ut_asserteq(0xb000, IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,CONFIG_BLOBLIST_ADDR));ut_asserteq(0xb000, CONFIG_IF_ENABLED_INT(BLOBLIST_FIXED,BLOBLIST_ADDR));} /* * This fails if CONFIG_TEST_KCONFIG_ENABLE is not enabled, since the-- 2.34.1
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error: ``` ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4b0): undefined \ reference to `do_ut_bloblist' ```
Fixes: 6ea5df39e8d ("test: Only enable bloblist test when supported") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- test/cmd_ut.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 195b7ea50accde470a739771e058c1d664df3f0b..842c2c3e7f65eb9fe017fa1949eb2390be171487 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -104,8 +104,10 @@ static struct cmd_tbl cmd_ut_sub[] = { "", ""), #endif #ifdef CONFIG_SANDBOX +#if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), +#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif #ifdef CONFIG_CMD_ADDRMAP
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4b0): undefined \ reference to `do_ut_bloblist'Fixes: 6ea5df39e8d ("test: Only enable bloblist test when supported") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
test/cmd_ut.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 195b7ea50accde470a739771e058c1d664df3f0b..842c2c3e7f65eb9fe017fa1949eb2390be171487 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -104,8 +104,10 @@ static struct cmd_tbl cmd_ut_sub[] = { "", ""), #endif #ifdef CONFIG_SANDBOX +#if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), +#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif #ifdef CONFIG_CMD_ADDRMAP
-- 2.34.1
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error: ``` ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4e8): undefined \ reference to `do_ut_bootm' ```
Fixes: fe158657a5b ("test: inconsistent bootm tests") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- test/cmd_ut.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 842c2c3e7f65eb9fe017fa1949eb2390be171487..a14dbf4ca5e571a872585ff11bfe7adae3f4edf3 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -107,9 +107,9 @@ static struct cmd_tbl cmd_ut_sub[] = { #if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), -#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif +#endif #ifdef CONFIG_CMD_ADDRMAP U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""), #endif
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4e8): undefined \ reference to `do_ut_bootm'
Are you sure? I don't see that one. The bootm test should not relate to bloblist.
This is what I see:
buildman -a OF_EMBED -a ~BLOBLIST --bo sandbox Building current source for 1 boards (1 thread, 32 jobs per thread) sandbox: + sandbox +===================== WARNING ====================== +CONFIG_OF_EMBED is enabled. This option should only +be used for debugging purposes. Please use +CONFIG_OF_SEPARATE for boards in mainline. +See doc/develop/devicetree/control.rst for more info. +==================================================== + +Some CONFIG adjustments did not take effect. This may be because +the request CONFIGs do not exist or conflict with others. +Failed adjustments: +~BLOBLIST CONFIG_BLOBLIST=y 0 0 1 /1 sandbox Completed: 1 total built, 1 newly), duration 0:00:11, rate 0.09
Fixes: fe158657a5b ("test: inconsistent bootm tests") Signed-off-by: Evgeny Bachinin <EABachinin@salutedevices.com> --- test/cmd_ut.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 842c2c3e7f65eb9fe017fa1949eb2390be171487..a14dbf4ca5e571a872585ff11bfe7adae3f4edf3 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -107,9 +107,9 @@ static struct cmd_tbl cmd_ut_sub[] = { #if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), -#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif +#endif #ifdef CONFIG_CMD_ADDRMAP U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""), #endif -- 2.34.1
Regards, SImon
Hi, Simon. See below, please
On Fri, Dec 06, 2024 at 08:31:22AM -0700, Simon Glass wrote:
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4e8): undefined \ reference to `do_ut_bootm'Are you sure? I don't see that one. The bootm test should not relate to bloblist.
I've double checked - reproduced on v2024.10. There we had in test/Makefile: ``` ifneq ($(CONFIG_$(SPL_)BLOBLIST),) obj-$(CONFIG_$(SPL_)CMDLINE) += bootm.o endif ``` So do_ut_bootm() was not built if BLOBLIST=n
We do not see "undefined reference to `do_ut_bootm'" on v2025.01-rc3 due to this patch: 7f8b8c5abc2 ("bootm: test: Move test into boot") It made bootm.o dependant on CONFIG_SANDBOX and independent from BLOBLIST.
And because aforementioned patch fixes the issue, I'm going to get rid of this current patch.
This is what I see:
buildman -a OF_EMBED -a ~BLOBLIST --bo sandbox Building current source for 1 boards (1 thread, 32 jobs per thread) sandbox: + sandbox +===================== WARNING ====================== +CONFIG_OF_EMBED is enabled. This option should only +be used for debugging purposes. Please use +CONFIG_OF_SEPARATE for boards in mainline. +See doc/develop/devicetree/control.rst for more info. +====================================================
+Some CONFIG adjustments did not take effect. This may be because +the request CONFIGs do not exist or conflict with others. +Failed adjustments: +~BLOBLIST CONFIG_BLOBLIST=y
Just a side note: I am not familiar with buildman yet, but it seems, it can not set up CONFIG_BLOBLIST=n because BLOBLIST is selected by 'select'.
JFYI, I'm going to extend the patch series by new patch to make BLOBLIST selectable from menuconfig for sandbox via 'imply'.
0 0 1 /1 sandboxCompleted: 1 total built, 1 newly), duration 0:00:11, rate 0.09
Fixes: fe158657a5b ("test: inconsistent bootm tests") Signed-off-by: Evgeny Bachinin <EABachinin@salutedevices.com> --- test/cmd_ut.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 842c2c3e7f65eb9fe017fa1949eb2390be171487..a14dbf4ca5e571a872585ff11bfe7adae3f4edf3 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -107,9 +107,9 @@ static struct cmd_tbl cmd_ut_sub[] = { #if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), -#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif +#endif #ifdef CONFIG_CMD_ADDRMAP U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""), #endif -- 2.34.1Regards, SImon
-- Best Regards, Evgeny Bachinin
Hi Evgeny,
On Tue, 17 Dec 2024 at 02:11, Evgeny Bachinin eabachinin@salutedevices.com wrote:
Hi, Simon. See below, please
On Fri, Dec 06, 2024 at 08:31:22AM -0700, Simon Glass wrote:
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Having CONFIG_OF_EMBED=y && CONFIG_BLOBLIST=n leads to the link error:
ld: /tmp/ccwtRVty.ltrans28.ltrans.o:(.data.rel+0x4e8): undefined \ reference to `do_ut_bootm'Are you sure? I don't see that one. The bootm test should not relate to bloblist.
I've double checked - reproduced on v2024.10. There we had in test/Makefile:
ifneq ($(CONFIG_$(SPL_)BLOBLIST),) obj-$(CONFIG_$(SPL_)CMDLINE) += bootm.o endifSo do_ut_bootm() was not built if BLOBLIST=n
We do not see "undefined reference to `do_ut_bootm'" on v2025.01-rc3 due to this patch: 7f8b8c5abc2 ("bootm: test: Move test into boot") It made bootm.o dependant on CONFIG_SANDBOX and independent from BLOBLIST.
And because aforementioned patch fixes the issue, I'm going to get rid of this current patch.
This is what I see:
buildman -a OF_EMBED -a ~BLOBLIST --bo sandbox Building current source for 1 boards (1 thread, 32 jobs per thread) sandbox: + sandbox +===================== WARNING ====================== +CONFIG_OF_EMBED is enabled. This option should only +be used for debugging purposes. Please use +CONFIG_OF_SEPARATE for boards in mainline. +See doc/develop/devicetree/control.rst for more info. +====================================================
+Some CONFIG adjustments did not take effect. This may be because +the request CONFIGs do not exist or conflict with others. +Failed adjustments: +~BLOBLIST CONFIG_BLOBLIST=y
Just a side note: I am not familiar with buildman yet, but it seems, it can not set up CONFIG_BLOBLIST=n because BLOBLIST is selected by 'select'.
That's right.
JFYI, I'm going to extend the patch series by new patch to make BLOBLIST selectable from menuconfig for sandbox via 'imply'.
OK good.
0 0 1 /1 sandboxCompleted: 1 total built, 1 newly), duration 0:00:11, rate 0.09
Fixes: fe158657a5b ("test: inconsistent bootm tests") Signed-off-by: Evgeny Bachinin <EABachinin@salutedevices.com> --- test/cmd_ut.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 842c2c3e7f65eb9fe017fa1949eb2390be171487..a14dbf4ca5e571a872585ff11bfe7adae3f4edf3 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -107,9 +107,9 @@ static struct cmd_tbl cmd_ut_sub[] = { #if CONFIG_IS_ENABLED(BLOBLIST) U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist, "", ""), -#endif U_BOOT_CMD_MKENT(bootm, CONFIG_SYS_MAXARGS, 1, do_ut_bootm, "", ""), #endif +#endif #ifdef CONFIG_CMD_ADDRMAP U_BOOT_CMD_MKENT(addrmap, CONFIG_SYS_MAXARGS, 1, do_ut_addrmap, "", ""), #endif -- 2.34.1Regards, SImon
-- Best Regards, Evgeny Bachinin
Regards, Simon
Bug: nobody sets 'ret' to non-error value inside board_fdt_blob_setup() in case, when gd->fdt_blob has been initialized before calling the board_fdt_blob_setup() (say, due to CONFIG_OF_EMBED=y)
Reproduced with CONFIG_OF_EMBED=y && BLOBLIST=n. ``` $ ./u-boot initcall: 0000000000078e05 (relocated to 000061138d2c7e05) initcall: 0000000000117b12 (relocated to 000061138d366b12) fdtdec_setup(): gd->fdt_blob = dtb_dt_embedded() fdtdec_setup(): gd->fdt_blob = board_fdt_blob_setup(&ret);
board_fdt_blob_setup():380 ret:-2 gd->fdt_blob:0x61138d4db310 if (gd->fdt_blob) return (void *)gd->fdt_blob;
initcall failed at call 0000000000117b12 (err=-2: No such file \ or directory) ### ERROR ### Please RESET the board ### ```
Patch moves code, zeroizing the 'ret', before checking the fdt_blob.
Fixes: d8289e7dfe5 ("sandbox: fdt: Avoid overwriting an existing fdt") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com --- arch/sandbox/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index d1c4dcf0764fb37ec99585a3db16fa34c3a54dac..7e93377e87e75b0fb92a8cf139b4a731a88a6a7b 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -377,10 +377,10 @@ void *board_fdt_blob_setup(int *ret) int err; int fd;
+ *ret = 0; if (gd->fdt_blob) return (void *)gd->fdt_blob; blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0); - *ret = 0; if (!state->fdt_fname) { err = setup_auto_tree(blob); if (!err)
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Bug: nobody sets 'ret' to non-error value inside board_fdt_blob_setup() in case, when gd->fdt_blob has been initialized before calling the board_fdt_blob_setup() (say, due to CONFIG_OF_EMBED=y)
Reproduced with CONFIG_OF_EMBED=y && BLOBLIST=n.
$ ./u-boot initcall: 0000000000078e05 (relocated to 000061138d2c7e05) initcall: 0000000000117b12 (relocated to 000061138d366b12) fdtdec_setup(): gd->fdt_blob = dtb_dt_embedded() fdtdec_setup(): gd->fdt_blob = board_fdt_blob_setup(&ret); board_fdt_blob_setup():380 ret:-2 gd->fdt_blob:0x61138d4db310 if (gd->fdt_blob) return (void *)gd->fdt_blob; initcall failed at call 0000000000117b12 (err=-2: No such file \ or directory) ### ERROR ### Please RESET the board ###Patch moves code, zeroizing the 'ret', before checking the fdt_blob.
Fixes: d8289e7dfe5 ("sandbox: fdt: Avoid overwriting an existing fdt") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
arch/sandbox/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index d1c4dcf0764fb37ec99585a3db16fa34c3a54dac..7e93377e87e75b0fb92a8cf139b4a731a88a6a7b 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -377,10 +377,10 @@ void *board_fdt_blob_setup(int *ret) int err; int fd;
*ret = 0; if (gd->fdt_blob) return (void *)gd->fdt_blob; blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
*ret = 0; if (!state->fdt_fname) { err = setup_auto_tree(blob); if (!err)-- 2.34.1
Reviewed-by: Simon Glass sjg@chromium.org
But note for the future we have:
https://patchwork.ozlabs.org/project/uboot/patch/20241102174944.412088-2-sjg...
Regards, Simon
Hi, Simon, see below
On Fri, Dec 06, 2024 at 08:31:36AM -0700, Simon Glass wrote:
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Bug: nobody sets 'ret' to non-error value inside board_fdt_blob_setup() in case, when gd->fdt_blob has been initialized before calling the board_fdt_blob_setup() (say, due to CONFIG_OF_EMBED=y)
Reproduced with CONFIG_OF_EMBED=y && BLOBLIST=n.
$ ./u-boot initcall: 0000000000078e05 (relocated to 000061138d2c7e05) initcall: 0000000000117b12 (relocated to 000061138d366b12) fdtdec_setup(): gd->fdt_blob = dtb_dt_embedded() fdtdec_setup(): gd->fdt_blob = board_fdt_blob_setup(&ret); board_fdt_blob_setup():380 ret:-2 gd->fdt_blob:0x61138d4db310 if (gd->fdt_blob) return (void *)gd->fdt_blob; initcall failed at call 0000000000117b12 (err=-2: No such file \ or directory) ### ERROR ### Please RESET the board ###Patch moves code, zeroizing the 'ret', before checking the fdt_blob.
Fixes: d8289e7dfe5 ("sandbox: fdt: Avoid overwriting an existing fdt") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
arch/sandbox/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index d1c4dcf0764fb37ec99585a3db16fa34c3a54dac..7e93377e87e75b0fb92a8cf139b4a731a88a6a7b 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -377,10 +377,10 @@ void *board_fdt_blob_setup(int *ret) int err; int fd;
*ret = 0; if (gd->fdt_blob) return (void *)gd->fdt_blob; blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
*ret = 0; if (!state->fdt_fname) { err = setup_auto_tree(blob); if (!err)-- 2.34.1
Reviewed-by: Simon Glass sjg@chromium.org
But note for the future we have:
https://patchwork.ozlabs.org/project/uboot/patch/20241102174944.412088-2-sjg...
Thanks for pointing me!
Could you advise me? Should I abandon this patch from v2 patch-set?
-- Best Regards, Evgeny Bachinin
Hi Evgeny,
On Tue, 17 Dec 2024 at 01:59, Evgeny Bachinin eabachinin@salutedevices.com wrote:
Hi, Simon, see below
On Fri, Dec 06, 2024 at 08:31:36AM -0700, Simon Glass wrote:
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
Bug: nobody sets 'ret' to non-error value inside board_fdt_blob_setup() in case, when gd->fdt_blob has been initialized before calling the board_fdt_blob_setup() (say, due to CONFIG_OF_EMBED=y)
Reproduced with CONFIG_OF_EMBED=y && BLOBLIST=n.
$ ./u-boot initcall: 0000000000078e05 (relocated to 000061138d2c7e05) initcall: 0000000000117b12 (relocated to 000061138d366b12) fdtdec_setup(): gd->fdt_blob = dtb_dt_embedded() fdtdec_setup(): gd->fdt_blob = board_fdt_blob_setup(&ret); board_fdt_blob_setup():380 ret:-2 gd->fdt_blob:0x61138d4db310 if (gd->fdt_blob) return (void *)gd->fdt_blob; initcall failed at call 0000000000117b12 (err=-2: No such file \ or directory) ### ERROR ### Please RESET the board ###Patch moves code, zeroizing the 'ret', before checking the fdt_blob.
Fixes: d8289e7dfe5 ("sandbox: fdt: Avoid overwriting an existing fdt") Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
arch/sandbox/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index d1c4dcf0764fb37ec99585a3db16fa34c3a54dac..7e93377e87e75b0fb92a8cf139b4a731a88a6a7b 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -377,10 +377,10 @@ void *board_fdt_blob_setup(int *ret) int err; int fd;
*ret = 0; if (gd->fdt_blob) return (void *)gd->fdt_blob; blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
*ret = 0; if (!state->fdt_fname) { err = setup_auto_tree(blob); if (!err)-- 2.34.1
Reviewed-by: Simon Glass sjg@chromium.org
But note for the future we have:
https://patchwork.ozlabs.org/project/uboot/patch/20241102174944.412088-2-sjg...
Thanks for pointing me!
Could you advise me? Should I abandon this patch from v2 patch-set?
I believe it should be fixed, but see what you think.
Regards, Simon
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
This patch-set repairs ability to use sandbox with CONFIG_OF_EMBED=y.
For now, to use OF_EMBED, the following must be done
- sandbox64_defconfig should have:
-CONFIG_OF_LIVE=y +CONFIG_OF_EMBED=y
- On sandbox when CONFIG_OF_EMBED=y, the u-boot process can't start
due to:
Bloblist at b000 not found (err=-2) initcall failed at call 000000000011829c (err=-2: No such file or \ directory) ### ERROR ### Please RESET the board ###So, it is natural desire to disable CONFIG_BLOBLIST just to test sandbox with OF_EMBED=y (disable it one way or another):
config SANDBOX - select BLOBLIST + select BLOBLIST if SOME_NON_EXISTING_OPTION
- As a result, having such changes (CONFIG_OF_EMBED=y &&
CONFIG_BLOBLIST=n) leads to the link & run-time errors, being fixed by this patch series.
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
Tested:
- locally with aforementioned options applied
- CI - https://github.com/u-boot/u-boot/pull/704
- No regression
Evgeny Bachinin (5): sandbox: fix bloblist_finish() linker error if BLOBLIST=n test: sandbox: fix invalid_use_of_IF_ENABLED_INT if BLOBLIST=n test: sandbox: fix link error with do_ut_bloblist if BLOBLIST=n test: sandbox: fix link error with do_ut_bootm if BLOBLIST=n sandbox: set retval early in board_fdt_blob_setup()
arch/sandbox/cpu/cpu.c | 2 +- include/bloblist.h | 7 +++++++ test/cmd_ut.c | 2 ++ test/lib/kconfig.c | 10 ++++++---- 4 files changed, 16 insertions(+), 5 deletions(-)
base-commit: 3073246d1be682071d8b3d07d06c2484907aed60 change-id: 20241120-sandbox_repair_of_embed-2188465eb22b
Best regards,
Evgeny Bachinin EABachinin@salutedevices.com
Thanks for looking at this.
You could add a test for ~OF_EMBED in test/py/tests/test_sandbox_opts.py if you want to keep this working in future.
Regards, Simon
Hello, Simon.
On Fri, Dec 06, 2024 at 12:19:42PM -0700, Simon Glass wrote:
Hi Evgeny,
On Mon, 2 Dec 2024 at 06:46, Evgeny Bachinin EABachinin@salutedevices.com wrote:
This patch-set repairs ability to use sandbox with CONFIG_OF_EMBED=y.
For now, to use OF_EMBED, the following must be done
- sandbox64_defconfig should have:
-CONFIG_OF_LIVE=y +CONFIG_OF_EMBED=y
- On sandbox when CONFIG_OF_EMBED=y, the u-boot process can't start
due to:
Bloblist at b000 not found (err=-2) initcall failed at call 000000000011829c (err=-2: No such file or \ directory) ### ERROR ### Please RESET the board ###So, it is natural desire to disable CONFIG_BLOBLIST just to test sandbox with OF_EMBED=y (disable it one way or another):
config SANDBOX - select BLOBLIST + select BLOBLIST if SOME_NON_EXISTING_OPTION
- As a result, having such changes (CONFIG_OF_EMBED=y &&
CONFIG_BLOBLIST=n) leads to the link & run-time errors, being fixed by this patch series.
Signed-off-by: Evgeny Bachinin EABachinin@salutedevices.com
Tested:
- locally with aforementioned options applied
- CI - https://github.com/u-boot/u-boot/pull/704
- No regression
Evgeny Bachinin (5): sandbox: fix bloblist_finish() linker error if BLOBLIST=n test: sandbox: fix invalid_use_of_IF_ENABLED_INT if BLOBLIST=n test: sandbox: fix link error with do_ut_bloblist if BLOBLIST=n test: sandbox: fix link error with do_ut_bootm if BLOBLIST=n sandbox: set retval early in board_fdt_blob_setup()
arch/sandbox/cpu/cpu.c | 2 +- include/bloblist.h | 7 +++++++ test/cmd_ut.c | 2 ++ test/lib/kconfig.c | 10 ++++++---- 4 files changed, 16 insertions(+), 5 deletions(-)
base-commit: 3073246d1be682071d8b3d07d06c2484907aed60 change-id: 20241120-sandbox_repair_of_embed-2188465eb22b
Best regards,
Evgeny Bachinin EABachinin@salutedevices.com
Thanks for looking at this.
You could add a test for ~OF_EMBED in test/py/tests/test_sandbox_opts.py if you want to keep this working in future.
I did not know about this. Thank you, will do.
On Mon, 02 Dec 2024 16:45:21 +0300, Evgeny Bachinin wrote:
This patch-set repairs ability to use sandbox with CONFIG_OF_EMBED=y.
For now, to use OF_EMBED, the following must be done
- sandbox64_defconfig should have:
-CONFIG_OF_LIVE=y +CONFIG_OF_EMBED=y[...]
Applied to u-boot/next, thanks!
participants (4)
-
Evgeny Bachinin -
Evgeny Bachinin
-
Simon Glass -
Tom Rini