[PATCH] sandbox: Remove OF_HOSTFILE
Simon Glass
sjg at chromium.org
Wed Sep 29 00:46:23 CEST 2021
Hi Ilias,
On Tue, 28 Sept 2021 at 03:05, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> OF_HOSTFILE is used on sandbox configs only. Although it's pretty
> unique not a source of any confusions, we are better of having simple
> config options for the DTB.
> So let's replace that with the existing OF_BOARD. This will make U-Boot
> have only three different config options for the DTB origin.
> - OF_SEPARATE, build separately from U-Boot
> - OF_BOARD, board specific way of providing the DTB
> - OF_EMBED embedded in the u-boot binary, but discouraged from being used
> in production
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> Makefile | 6 +++---
> arch/sandbox/cpu/cpu.c | 21 ++++++++++++---------
> arch/sandbox/include/asm/u-boot-sandbox.h | 8 --------
> configs/sandbox64_defconfig | 2 +-
> configs/sandbox_defconfig | 2 +-
> configs/sandbox_flattree_defconfig | 2 +-
> configs/sandbox_noinst_defconfig | 2 +-
> configs/sandbox_spl_defconfig | 2 +-
> configs/tools-only_defconfig | 2 +-
> doc/develop/devicetree/control.rst | 7 +++----
> dts/Kconfig | 9 ---------
> lib/fdtdec.c | 5 -----
> scripts/Makefile.spl | 4 ++--
> 13 files changed, 26 insertions(+), 46 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3014788e14e8..cf3d98d00a62 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -957,7 +957,7 @@ INPUTS-$(CONFIG_BINMAN_STANDALONE_FDT) += u-boot.dtb
> ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> endif
> -INPUTS-$(CONFIG_OF_HOSTFILE) += u-boot.dtb
> +INPUTS-$(CONFIG_SANDBOX) += u-boot.dtb
> ifneq ($(CONFIG_SPL_TARGET),)
> INPUTS-$(CONFIG_SPL) += $(CONFIG_SPL_TARGET:"%"=%)
> endif
> @@ -1423,7 +1423,7 @@ u-boot-lzma.img: u-boot.bin.lzma FORCE
>
> u-boot-dtb.img u-boot.img u-boot.kwb u-boot.pbl u-boot-ivt.img: \
> $(if $(CONFIG_SPL_LOAD_FIT),u-boot-nodtb.bin \
> - $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE)$(CONFIG_BINMAN_STANDALONE_FDT),dts/dt.dtb) \
> + $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SANDBOX)$(CONFIG_BINMAN_STANDALONE_FDT),dts/dt.dtb) \
> ,$(UBOOT_BIN)) FORCE
> $(call if_changed,mkimage)
> $(BOARD_SIZE_CHECK)
> @@ -1437,7 +1437,7 @@ MKIMAGEFLAGS_u-boot.itb += -B 0x8
>
> ifdef U_BOOT_ITS
> u-boot.itb: u-boot-nodtb.bin \
> - $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),dts/dt.dtb) \
> + $(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SANDBOX),dts/dt.dtb) \
> $(U_BOOT_ITS) FORCE
> $(call if_changed,mkfitimage)
> $(BOARD_SIZE_CHECK)
> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> index 48636ab63919..bc67a5a5a10b 100644
> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
> @@ -291,44 +291,47 @@ void invalidate_dcache_range(unsigned long start, unsigned long stop)
> {
> }
>
> -int sandbox_read_fdt_from_file(void)
> +void *board_fdt_blob_setup(void)
Can you instead keep the function and call it from your new
board_fdt_blob_setup() function? That way the error path goes through
one place and we don't need to add goto.
The other problem is that we need to know whether a DT is required
(i.e. -d, -D or -T). So it must fail (and exit) if it is required but
cannot be loaded.
> {
> struct sandbox_state *state = state_get_current();
> const char *fname = state->fdt_fname;
> - void *blob;
> + void *blob = NULL;
> loff_t size;
> int err;
> int fd;
>
> + printf("SETUP BLOB\n");
drop debugging
> blob = map_sysmem(CONFIG_SYS_FDT_LOAD_ADDR, 0);
> if (!state->fdt_fname) {
> err = fdt_create_empty_tree(blob, 256);
> if (!err)
> goto done;
> printf("Unable to create empty FDT: %s\n", fdt_strerror(err));
> - return -EINVAL;
> + goto fail;
> }
>
> err = os_get_filesize(fname, &size);
> if (err < 0) {
> printf("Failed to file FDT file '%s'\n", fname);
> - return err;
> + goto fail;
> }
> fd = os_open(fname, OS_O_RDONLY);
> if (fd < 0) {
> printf("Failed to open FDT file '%s'\n", fname);
> - return -EACCES;
> + goto fail;
> }
> +
> if (os_read(fd, blob, size) != size) {
> os_close(fd);
> - return -EIO;
> + printf("Failed to read file '%s'\n", fname);
> + goto fail;
> }
> os_close(fd);
>
> done:
> - gd->fdt_blob = blob;
> -
> - return 0;
> + return blob;
> +fail:
> + return NULL;
> }
>
> ulong timer_get_boot_us(void)
> diff --git a/arch/sandbox/include/asm/u-boot-sandbox.h b/arch/sandbox/include/asm/u-boot-sandbox.h
> index 73b1897191d5..56dc13c3eb11 100644
> --- a/arch/sandbox/include/asm/u-boot-sandbox.h
> +++ b/arch/sandbox/include/asm/u-boot-sandbox.h
> @@ -76,14 +76,6 @@ int pci_unmap_physmem(const void *addr, unsigned long len,
> */
> void sandbox_set_enable_pci_map(int enable);
>
> -/**
> - * sandbox_read_fdt_from_file() - Read a device tree from a file
> - *
> - * Read a device tree file from a host file and set it up for use as the
> - * control FDT.
> - */
> -int sandbox_read_fdt_from_file(void);
> -
> /**
> * sandbox_reset() - reset sandbox
> *
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index f7098b496983..358a6c168259 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -86,7 +86,7 @@ CONFIG_MAC_PARTITION=y
> CONFIG_AMIGA_PARTITION=y
> CONFIG_OF_CONTROL=y
> CONFIG_OF_LIVE=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
Can we put this in Kconfig instead, so it is enabled for all sandbox boards?
> CONFIG_ENV_IS_NOWHERE=y
> CONFIG_ENV_IS_IN_EXT4=y
> CONFIG_ENV_EXT4_INTERFACE="host"
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index ea08a9e5bd18..b8f8d8cf4454 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -110,7 +110,7 @@ CONFIG_MAC_PARTITION=y
> CONFIG_AMIGA_PARTITION=y
> CONFIG_OF_CONTROL=y
> CONFIG_OF_LIVE=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
> CONFIG_ENV_IS_NOWHERE=y
> CONFIG_ENV_IS_IN_EXT4=y
> CONFIG_ENV_EXT4_INTERFACE="host"
> diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
> index a6e2544dc138..7aceaf5ad05b 100644
> --- a/configs/sandbox_flattree_defconfig
> +++ b/configs/sandbox_flattree_defconfig
> @@ -67,7 +67,7 @@ CONFIG_CMD_EXT4_WRITE=y
> CONFIG_MAC_PARTITION=y
> CONFIG_AMIGA_PARTITION=y
> CONFIG_OF_CONTROL=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
> CONFIG_ENV_IS_NOWHERE=y
> CONFIG_ENV_IS_IN_EXT4=y
> CONFIG_ENV_EXT4_INTERFACE="host"
> diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
> index 88443f5ab274..8aec5df6f11b 100644
> --- a/configs/sandbox_noinst_defconfig
> +++ b/configs/sandbox_noinst_defconfig
> @@ -85,7 +85,7 @@ CONFIG_MAC_PARTITION=y
> CONFIG_AMIGA_PARTITION=y
> CONFIG_OF_CONTROL=y
> CONFIG_SPL_OF_CONTROL=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
> CONFIG_SPL_OF_PLATDATA=y
> CONFIG_ENV_IS_NOWHERE=y
> CONFIG_ENV_IS_IN_EXT4=y
> diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
> index 77dd83cf6fdd..c889049c9132 100644
> --- a/configs/sandbox_spl_defconfig
> +++ b/configs/sandbox_spl_defconfig
> @@ -86,7 +86,7 @@ CONFIG_MAC_PARTITION=y
> CONFIG_AMIGA_PARTITION=y
> CONFIG_OF_CONTROL=y
> CONFIG_SPL_OF_CONTROL=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
> CONFIG_SPL_OF_PLATDATA=y
> CONFIG_SPL_OF_PLATDATA_INST=y
> CONFIG_ENV_IS_NOWHERE=y
> diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig
> index f54bc1802ca7..ea38ef15532b 100644
> --- a/configs/tools-only_defconfig
> +++ b/configs/tools-only_defconfig
> @@ -12,7 +12,7 @@ CONFIG_MISC_INIT_F=y
> CONFIG_BOOTP_DNS2=y
> # CONFIG_CMD_DATE is not set
> CONFIG_OF_CONTROL=y
> -CONFIG_OF_HOSTFILE=y
> +CONFIG_OF_BOARD=y
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> CONFIG_BOOTP_SEND_HOSTNAME=y
> CONFIG_IP_DEFRAG=y
> diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst
> index e84dfb6677a6..0e6f85d5af11 100644
> --- a/doc/develop/devicetree/control.rst
> +++ b/doc/develop/devicetree/control.rst
> @@ -108,10 +108,9 @@ If CONFIG_OF_BOARD is defined, a board-specific routine will provide the
> devicetree at runtime, for example if an earlier bootloader stage creates
> it and passes it to U-Boot.
>
> -If CONFIG_OF_HOSTFILE is defined, then it will be read from a file on
> -startup. This is only useful for sandbox. Use the -d flag to U-Boot to
> -specify the file to read, -D for the default and -T for the test devicetree,
> -used to run sandbox unit tests.
> +If CONFIG_SANDBOX is defined, then it will be read from a file on
> +startup. Use the -d flag to U-Boot to specify the file to read, -D for the
> +default and -T for the test devicetree, used to run sandbox unit tests.
>
> You cannot use more than one of these options at the same time.
>
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 100769017e12..60d56cf6a907 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -108,20 +108,11 @@ config OF_EMBED
>
> config OF_BOARD
> 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
> the board at runtime if the board supports it, instead of being
> bundled with the image.
>
> -config OF_HOSTFILE
> - bool "Host filed DTB for DT control"
> - depends on SANDBOX
> - help
> - If this option is enabled, DTB will be read from a file on startup.
> - This is only useful for Sandbox. Use the -d flag to U-Boot to
> - specify the file to read.
> -
> endchoice
>
> config DEFAULT_DEVICE_TREE
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 7b379564600d..e84472dc2e59 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1575,11 +1575,6 @@ int fdtdec_setup(void)
> # elif defined(CONFIG_OF_BOARD) || defined(CONFIG_OF_SEPARATE)
> /* Allow the board to override the fdt address. */
> gd->fdt_blob = board_fdt_blob_setup();
> -# elif defined(CONFIG_OF_HOSTFILE)
> - if (sandbox_read_fdt_from_file()) {
> - puts("Failed to read control FDT\n");
> - return -1;
> - }
> # endif
> # ifndef CONFIG_SPL_BUILD
> /* Allow the early environment to override the fdt address */
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 25a3e7fa52e9..fa362495e1b4 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -298,11 +298,11 @@ endif
> # Build the .dtb file if:
> # - we are not using OF_PLATDATA
> # - we are using OF_CONTROL
> -# - we have either OF_SEPARATE or OF_HOSTFILE
> +# - we have either OF_SEPARATE or we are compiling for sandbox
> build_dtb :=
> ifeq ($(CONFIG_$(SPL_TPL_)OF_PLATDATA),)
> ifneq ($(CONFIG_$(SPL_TPL_)OF_CONTROL),)
> -ifeq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_HOSTFILE),y)
> +ifeq ($(CONFIG_OF_SEPARATE)$(CONFIG_SANDBOX_SPL),y)
Do you mean CONFIG_SANDBOX ?
> build_dtb := y
> endif
> endif
> --
> 2.33.0
>
Regards,
Simon
More information about the U-Boot
mailing list