Commit 690af71850149bf242502f688eca80fb302d1f76 changed this condition
from an explicit

IS_ENABLED(CONFIG_SPL_SEPARATE_BSS)

to

CONFIG_IS_ENABLED(SEPARATE_BSS)

The documentation for CONFIG_IS_ENABLED() in include/linux/kconfig.h
implies that we will get the correct behaviour, but the actual behaviour
differs such that this condition is now always false.

This stopped TPL being able to load the device tree blob at least on the
ROCKPro64 board (RK3399 SoC), since the wrong device tree location was
chosen.

The issues causing this behaviour with CONFIG_IS_ENABLED() are:

1. The documentation implies that CONFIG_SPL_BUILD =>
   CONFIG_SPL_<option> is considered before the TPL equivalent.

   Actually, the TPL options have higher priority - see definition of
   _CONFIG_PREFIX.

2. The documentation implies a fallthrough, eg. if CONFIG_SPL_BUILD is
   defined but the CONFIG_SPL_<option> is not, then it will proceed to
   check if CONFIG_TPL_BUILD

   Actually, if CONFIG_TPL_BUILD is defined, then it stops there
   and CONFIG_SPL_BUILD is not considered - see definition of
   _CONFIG_PREFIX.

   During TPL build, at least for the ROCKPro64, both CONFIG_TPL_BUILD
   and CONFIG_SPL_BUILD are defined, but because of the above, only TPL
   options are considered. Since there is no CONFIG_TPL_SEPARATE_BSS,
   this fails.

Fixes: 690af71850 ("fdt: Correct condition for SEPARATE_BSS")
Signed-off-by: Andrew Abbott <and...@mirx.dev>
---

The serial output from a ROCKPro64 booting master (9859465bfe) is:

    U-Boot TPL 2022.04-00787-g9859465bfe-dirty (Jan 01 1980 - 00:00:00)
    Missing DTB
    ### ERROR ### Please RESET the board ###

I'm not 100% sure if this is the correct fix or if my analysis is right. This
issue affects the ROCKPro64 (rockpro64-rk3399_defconfig) but not the ROCK64
(rock64-rk3328_defconfig).

There may be differences in whether CONFIG_TPL_BUILD and
CONFIG_SPL_BUILD are defined for each during SPL or TPL build, or the
boards differ in what device tree operations they do when?

If this isn't the right fix, guidance on the correct approach would be
appreciated!

 lib/fdtdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 0c0ec034ec..e2208cb7d7 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1230,7 +1230,7 @@ static void *fdt_find_separate(void)
 
 #ifdef CONFIG_SPL_BUILD
        /* FDT is at end of BSS unless it is in a different memory region */
-       if (CONFIG_IS_ENABLED(SEPARATE_BSS))
+       if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS))
                fdt_blob = (ulong *)&_image_binary_end;
        else
                fdt_blob = (ulong *)&__bss_end;
-- 
2.35.1

Reply via email to