Hi Mark,
On Sat, Sep 25, 2021 at 07:01:07PM +0200, Mark Kettenis wrote:
From: Ilias Apalodimas ilias.apalodimas@linaro.org Date: Fri, 24 Sep 2021 16:12:51 +0300
Forgot to include Mark, which showed some interest for MBPs
Well, I am currently using OF_BOARD, so this doesn't affect me. I was merely pointing out that having OF_PRIOR_STAGE makes some sense as it clearly indicates that the DT is coming from an earlier firmware stage. OF_BOARD is more flexible and could be used to read the DT from an onboard ROM or something like that.
Sure, the naming is something we'd loose. But it doesn't truly offer us any advantage does it? Unless we know of a board that can read the DT from an onboard ROM *or* a register. That would make some sense in having 2 config options, so that a user could specify which DT he wants. But even in that case, I'd prefer the config to be easier and hide the details under the hood.
E.g have a callback that reads the register and if you don't find any valid DTB go read a ROM (which is close too what I suggested on a following mail). The obvious disadvantage of this approach would be not allowing someone to explicitly request which DT to read, but I can live with that.
Regards /Ilias
On Fri, 24 Sept 2021 at 16:10, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
At some point back in 2018 prior_stage_fdt_address and OF_PRIOR_STAGE got introduced, in order to support a DTB handed over by an earlier stage boot loader. However we have another option in the Kconfig (OF_BOARD) which has identical semantics.
A good example of this is RISC-V boards which during their startup, pick up the DTB from a1 and copy it in their private gd_t. Apart from that they also copy it to prior_stage_fdt_address, if the Kconfig option is selected, which seems unnecessary(??).
This is mostly an RFC, trying to figure out if I am missing some subtle functionality, which would justify having 2 Kconfig options doing similar things present.
- Should we do this?
- Doesn't OF_BOARD and OF_PRIOR_STAGE practically mean "Someone else is going to pass me my DTB". Why should we care if that someone is a prior bootloader or runtime memory generated on the fly by U-Boot? It all boils down to having a *board* specific callback for that.
- RISC-V binman should get rid of the option as well if we decide to go though with this (but I have no idea what RISC-V expects there).
- Can we apply similar logic to OF_HOSTFILE? It seems like we could just have a board_fdt_blob_setup() for the sandbox that reads the file we want and get rid of another Kconfig option.
Note that the original board which introduced CONFIG_OF_PRIOR_STAGE is still there. If someone cares enough I guess he could fix that as well, but I don't have the board around, so I prefer keeping it as is and mark the option as deprecated. For that board we could also keep the prior_stage_fdt_address without the Kconfig option and simply copy the location there, but the board must define it's own board_fdt_blob_setup(). That would get rid of the Kconfig option entirely instead of limiting it to that board only.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
arch/riscv/cpu/cpu.c | 3 --- arch/riscv/cpu/start.S | 5 ----- board/emulation/qemu-riscv/qemu-riscv.c | 8 ++++++++ board/sifive/unleashed/unleashed.c | 10 ++++------ board/sifive/unmatched/unmatched.c | 10 ++++------ configs/ae350_rv32_defconfig | 2 +- configs/ae350_rv32_spl_defconfig | 2 +- configs/ae350_rv64_defconfig | 2 +- configs/ae350_rv64_spl_defconfig | 2 +- configs/bcm7260_defconfig | 2 +- configs/qemu-riscv32_defconfig | 2 +- configs/qemu-riscv32_smode_defconfig | 2 +- configs/qemu-riscv32_spl_defconfig | 2 +- configs/qemu-riscv64_defconfig | 2 +- configs/qemu-riscv64_smode_defconfig | 2 +- configs/qemu-riscv64_spl_defconfig | 2 +- dts/Kconfig | 3 ++- lib/fdtdec.c | 4 ++++ 18 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b536..e16f1df30254 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -16,9 +16,6 @@
- The variables here must be stored in the data section since they are used
- before the bss section is available.
*/ -#ifdef CONFIG_OF_PRIOR_STAGE -phys_addr_t prior_stage_fdt_address __section(".data"); -#endif #ifndef CONFIG_XIP u32 hart_lottery __section(".data") = 0;
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 308b0a97a58f..76850ec9be2c 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -142,11 +142,6 @@ call_harts_early_init: bnez tp, secondary_hart_loop #endif
-#ifdef CONFIG_OF_PRIOR_STAGE
la t0, prior_stage_fdt_addressSREG s1, 0(t0)-#endif
jal board_init_f_init_reserve SREG s1, GD_FIRMWARE_FDT_ADDR(gp)diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c index dcfd3f20bee6..7dfe471dee15 100644 --- a/board/emulation/qemu-riscv/qemu-riscv.c +++ b/board/emulation/qemu-riscv/qemu-riscv.c @@ -14,6 +14,8 @@ #include <virtio_types.h> #include <virtio.h>
+DECLARE_GLOBAL_DATA_PTR;
int board_init(void) { /* @@ -69,3 +71,9 @@ int board_fit_config_name_match(const char *name) return 0; } #endif +void *board_fdt_blob_setup(void) +{
/* Stored the DTB address there during our init */return (void *)gd->arch.firmware_fdt_addr;+}
diff --git a/board/sifive/unleashed/unleashed.c b/board/sifive/unleashed/unleashed.c index 8cd514df3005..9aa2b3e6f43a 100644 --- a/board/sifive/unleashed/unleashed.c +++ b/board/sifive/unleashed/unleashed.c @@ -116,12 +116,10 @@ int misc_init_r(void)
void *board_fdt_blob_setup(void) {
if (IS_ENABLED(CONFIG_OF_SEPARATE)) {if (gd->arch.firmware_fdt_addr)return (ulong *)gd->arch.firmware_fdt_addr;elsereturn (ulong *)&_end;}
if (gd->arch.firmware_fdt_addr)return (ulong *)gd->arch.firmware_fdt_addr;elsereturn (ulong *)&_end;}
int board_init(void) diff --git a/board/sifive/unmatched/unmatched.c b/board/sifive/unmatched/unmatched.c index d90b252baef7..b703f9c3efc3 100644 --- a/board/sifive/unmatched/unmatched.c +++ b/board/sifive/unmatched/unmatched.c @@ -13,12 +13,10 @@
void *board_fdt_blob_setup(void) {
if (IS_ENABLED(CONFIG_OF_SEPARATE)) {if (gd->arch.firmware_fdt_addr)return (ulong *)gd->arch.firmware_fdt_addr;elsereturn (ulong *)&_end;}
if (gd->arch.firmware_fdt_addr)return (ulong *)gd->arch.firmware_fdt_addr;elsereturn (ulong *)&_end;}
int board_init(void) diff --git a/configs/ae350_rv32_defconfig b/configs/ae350_rv32_defconfig index 4e7a1686a64d..8b6c0b8a4a0a 100644 --- a/configs/ae350_rv32_defconfig +++ b/configs/ae350_rv32_defconfig @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y CONFIG_CMD_CACHE=y -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y diff --git a/configs/ae350_rv32_spl_defconfig b/configs/ae350_rv32_spl_defconfig index 34c6af6e7e17..a0fe9b9a71df 100644 --- a/configs/ae350_rv32_spl_defconfig +++ b/configs/ae350_rv32_spl_defconfig @@ -19,7 +19,7 @@ CONFIG_CMD_SF_TEST=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y CONFIG_CMD_CACHE=y -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_BOOTP_SEND_HOSTNAME=y diff --git a/configs/ae350_rv64_defconfig b/configs/ae350_rv64_defconfig index 05eee371ac2f..b12a8810a221 100644 --- a/configs/ae350_rv64_defconfig +++ b/configs/ae350_rv64_defconfig @@ -16,7 +16,7 @@ CONFIG_CMD_SF_TEST=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y CONFIG_CMD_CACHE=y -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_board=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y diff --git a/configs/ae350_rv64_spl_defconfig b/configs/ae350_rv64_spl_defconfig index 9cd7848c92eb..9ad312505db3 100644 --- a/configs/ae350_rv64_spl_defconfig +++ b/configs/ae350_rv64_spl_defconfig @@ -20,7 +20,7 @@ CONFIG_CMD_SF_TEST=y # CONFIG_CMD_SETEXPR is not set CONFIG_BOOTP_PREFER_SERVERIP=y CONFIG_CMD_CACHE=y -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_BOOTP_SEND_HOSTNAME=y diff --git a/configs/bcm7260_defconfig b/configs/bcm7260_defconfig index a42a6acb06d5..be0c945dc811 100644 --- a/configs/bcm7260_defconfig +++ b/configs/bcm7260_defconfig @@ -22,7 +22,7 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_EXT2=y CONFIG_CMD_EXT4=y CONFIG_CMD_FS_GENERIC=y -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_ENV_OVERWRITE=y CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig index 8ac16cf4186e..6fe133c268d7 100644 --- a/configs/qemu-riscv32_defconfig +++ b/configs/qemu-riscv32_defconfig @@ -9,6 +9,6 @@ CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_CMD_NVEDIT_EFI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/configs/qemu-riscv32_smode_defconfig b/configs/qemu-riscv32_smode_defconfig index 05eda439618f..c67e8206d1ab 100644 --- a/configs/qemu-riscv32_smode_defconfig +++ b/configs/qemu-riscv32_smode_defconfig @@ -10,6 +10,6 @@ CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_CMD_NVEDIT_EFI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/configs/qemu-riscv32_spl_defconfig b/configs/qemu-riscv32_spl_defconfig index ee81e552724d..77e81fac3af7 100644 --- a/configs/qemu-riscv32_spl_defconfig +++ b/configs/qemu-riscv32_spl_defconfig @@ -12,6 +12,6 @@ CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_SBI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index daf5d655d01f..90e87672aab0 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -10,6 +10,6 @@ CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_CMD_NVEDIT_EFI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/configs/qemu-riscv64_smode_defconfig b/configs/qemu-riscv64_smode_defconfig index 4a6416e2540b..0a8393903368 100644 --- a/configs/qemu-riscv64_smode_defconfig +++ b/configs/qemu-riscv64_smode_defconfig @@ -13,6 +13,6 @@ CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_CMD_NVEDIT_EFI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig index 429d4d814e65..a15e82dd3ee1 100644 --- a/configs/qemu-riscv64_spl_defconfig +++ b/configs/qemu-riscv64_spl_defconfig @@ -13,6 +13,6 @@ CONFIG_DISPLAY_CPUINFO=y CONFIG_DISPLAY_BOARDINFO=y CONFIG_CMD_SBI=y # CONFIG_CMD_MII is not set -CONFIG_OF_PRIOR_STAGE=y +CONFIG_OF_BOARD=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1ef..2c5b2ec2d1f8 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -107,7 +107,7 @@ config OF_EMBED Boards in the mainline U-Boot tree should not use it.
config OF_BOARD
bool "Provided by the board at runtime"
bool "Provided by the board (e.g a previous loader) at runtime" depends on !SANDBOX help If this option is enabled, the device tree will be provided by@@ -124,6 +124,7 @@ config OF_HOSTFILE
config OF_PRIOR_STAGE bool "Prior stage bootloader DTB for DT control"
depends on ARCH_BCMSTB help If this option is enabled, the device tree used for DT control will be read from a device tree binary, at a memorydiff --git a/lib/fdtdec.c b/lib/fdtdec.c index 7358cb6dd168..8d0db5ac6173 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1581,6 +1581,10 @@ int fdtdec_setup(void) return -1; } # elif defined(CONFIG_OF_PRIOR_STAGE)
/** obsolete don't use this on newer boards. Prefer CONFIG_OF_BOARD* instead*/ gd->fdt_blob = (void *)(uintptr_t)prior_stage_fdt_address;# endif
# ifndef CONFIG_SPL_BUILD
2.33.0