u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH] sandbox: Remove OF_HOSTFILE
@ 2021-09-28  9:05 Ilias Apalodimas
  2021-09-28  9:05 ` Ilias Apalodimas
  0 siblings, 1 reply; 8+ messages in thread
From: Ilias Apalodimas @ 2021-09-28  9:05 UTC (permalink / raw)
  To: trini
  Cc: Ilias Apalodimas, Simon Glass, Pali Rohár, Marek Behún,
	Bin Meng, Chee Hong Ang, Heinrich Schuchardt, Sean Anderson,
	Patrick Delaunay, Stefan Roese, Rasmus Villemoes,
	Nicolas Saenz Julienne, AKASHI Takahiro, Wasim Khan,
	Etienne Carriere, Nandor Han, Alexandru Gagniuc, Steffen Jaeckel,
	Matthias Brugger, Alper Nebi Yasak, Heiko Schocher,
	Asherah Connor, Andre Przywara, u-boot

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@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)
 {
 	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");
 	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
 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)
 build_dtb := y
 endif
 endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-28  9:05 [PATCH] sandbox: Remove OF_HOSTFILE Ilias Apalodimas
@ 2021-09-28  9:05 ` Ilias Apalodimas
  2021-09-28 22:46   ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Ilias Apalodimas @ 2021-09-28  9:05 UTC (permalink / raw)
  To: trini
  Cc: Ilias Apalodimas, Simon Glass, Pali Rohár, Marek Behún,
	Bin Meng, Chee Hong Ang, Heinrich Schuchardt, Sean Anderson,
	Patrick Delaunay, Joe Hershberger, Nicolas Saenz Julienne,
	Wasim Khan, AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc,
	Nandor Han, Steffen Jaeckel, Heiko Schocher, Asherah Connor,
	Andre Przywara, u-boot

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@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)
 {
 	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");
 	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
 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)
 build_dtb := y
 endif
 endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-28  9:05 ` Ilias Apalodimas
@ 2021-09-28 22:46   ` Simon Glass
  2021-09-29  5:39     ` Ilias Apalodimas
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-09-28 22:46 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: Tom Rini, Pali Rohár, Marek Behún, Bin Meng,
	Chee Hong Ang, Heinrich Schuchardt, Sean Anderson,
	Patrick Delaunay, Joe Hershberger, Nicolas Saenz Julienne,
	Wasim Khan, AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc,
	Nandor Han, Steffen Jaeckel, Heiko Schocher, Asherah Connor,
	Andre Przywara, U-Boot Mailing List

Hi Ilias,

On Tue, 28 Sept 2021 at 03:05, Ilias Apalodimas
<ilias.apalodimas@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@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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-28 22:46   ` Simon Glass
@ 2021-09-29  5:39     ` Ilias Apalodimas
  2021-09-29  6:25       ` Ilias Apalodimas
  0 siblings, 1 reply; 8+ messages in thread
From: Ilias Apalodimas @ 2021-09-29  5:39 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, Pali Rohár, Marek Behún, Bin Meng,
	Chee Hong Ang, Heinrich Schuchardt, Sean Anderson,
	Patrick Delaunay, Joe Hershberger, Nicolas Saenz Julienne,
	Wasim Khan, AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc,
	Nandor Han, Steffen Jaeckel, Heiko Schocher, Asherah Connor,
	Andre Przywara, U-Boot Mailing List

Hi Simon, 

[...]
> > -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.

Not without changing board_fdt_blob_setup() itself.  The function returns a
ptr and not an int.  We had a different #ifdef up to now, so the existing
function was returning an int and was setting the gd->fdt_blob internally.
In any case the goto either remains of gets converted to 'return NULL'.

> 
> 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.

So there's two things we could do here.  Either explicitly check for NULL
if sandbox is enabled or change board_fdt_blob_setup and put an error
return code in an argument.  I'll go with the return code in v2

[...]
> > - *
> > - * 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?

Sure

[...]
> > --- 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 ?

I thought we could use either.  I assumed since this is an SPL makefile
CONFIG_SANDBOX_SPL would be defined.  If it's not I can switch to CONFIG_SANDBOX

> 
> >  build_dtb := y
> >  endif
> >  endif
> > --
> > 2.33.0
> >
> 
> Regards,
> Simon

Thanks
/Ilias

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-29  5:39     ` Ilias Apalodimas
@ 2021-09-29  6:25       ` Ilias Apalodimas
  2021-09-29  7:44         ` Heinrich Schuchardt
  0 siblings, 1 reply; 8+ messages in thread
From: Ilias Apalodimas @ 2021-09-29  6:25 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, Pali Rohár, Marek Behún, Bin Meng,
	Chee Hong Ang, Heinrich Schuchardt, Sean Anderson,
	Patrick Delaunay, Joe Hershberger, Nicolas Saenz Julienne,
	Wasim Khan, AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc,
	Nandor Han, Steffen Jaeckel, Heiko Schocher, Asherah Connor,
	Andre Przywara, U-Boot Mailing List

> > > - */
> > > -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?
> 
> Sure

Looking at this again,  is this doable?  I remember 'select' being
available only on bool or tristate options.  Is there another way?

Cheers
/Ilias

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-29  6:25       ` Ilias Apalodimas
@ 2021-09-29  7:44         ` Heinrich Schuchardt
  2021-10-11 19:20           ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2021-09-29  7:44 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: Tom Rini, Pali Rohár, Marek Behún, Bin Meng,
	Chee Hong Ang, Sean Anderson, Patrick Delaunay, Joe Hershberger,
	Nicolas Saenz Julienne, Wasim Khan, AKASHI Takahiro,
	Etienne Carriere, Alexandru Gagniuc, Nandor Han, Steffen Jaeckel,
	Heiko Schocher, Asherah Connor, Andre Przywara,
	U-Boot Mailing List, Simon Glass



On 9/29/21 08:25, Ilias Apalodimas wrote:
>>>> - */
>>>> -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?
>>
>> Sure
>
> Looking at this again,  is this doable?  I remember 'select' being
> available only on bool or tristate options.  Is there another way?
>
> Cheers
> /Ilias
>

This will work:

diff --git a/dts/Kconfig b/dts/Kconfig
index dabe0080c1..6aca8a8738 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -90,6 +90,7 @@ config OF_LIVE
  choice
         prompt "Provider of DTB for DT control"
         depends on OF_CONTROL
+       default OF_BOARD if SANDBOX

  config OF_SEPARATE
         bool "Separate DTB for DT control"

Best regards

Heinrich

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-09-29  7:44         ` Heinrich Schuchardt
@ 2021-10-11 19:20           ` Simon Glass
  2021-10-11 20:16             ` Ilias Apalodimas
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-10-11 19:20 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Tom Rini, Pali Rohár, Marek Behún,
	Bin Meng, Chee Hong Ang, Sean Anderson, Patrick Delaunay,
	Joe Hershberger, Nicolas Saenz Julienne, Wasim Khan,
	AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc, Nandor Han,
	Steffen Jaeckel, Heiko Schocher, Asherah Connor, Andre Przywara,
	U-Boot Mailing List

Hi Ilias,

On Wed, 29 Sept 2021 at 01:52, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 9/29/21 08:25, Ilias Apalodimas wrote:
> >>>> - */
> >>>> -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?
> >>
> >> Sure
> >
> > Looking at this again,  is this doable?  I remember 'select' being
> > available only on bool or tristate options.  Is there another way?
> >
> > Cheers
> > /Ilias
> >
>
> This will work:
>
> diff --git a/dts/Kconfig b/dts/Kconfig
> index dabe0080c1..6aca8a8738 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -90,6 +90,7 @@ config OF_LIVE
>   choice
>          prompt "Provider of DTB for DT control"
>          depends on OF_CONTROL
> +       default OF_BOARD if SANDBOX
>
>   config OF_SEPARATE
>          bool "Separate DTB for DT control"
>

Do you have a new version of this one, please? We should try to get it in soon.

Regards
SImon

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] sandbox: Remove OF_HOSTFILE
  2021-10-11 19:20           ` Simon Glass
@ 2021-10-11 20:16             ` Ilias Apalodimas
  0 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2021-10-11 20:16 UTC (permalink / raw)
  To: Simon Glass
  Cc: Heinrich Schuchardt, Tom Rini, Pali Rohár, Marek Behún,
	Bin Meng, Chee Hong Ang, Sean Anderson, Patrick Delaunay,
	Joe Hershberger, Nicolas Saenz Julienne, Wasim Khan,
	AKASHI Takahiro, Etienne Carriere, Alexandru Gagniuc, Nandor Han,
	Steffen Jaeckel, Heiko Schocher, Asherah Connor, Andre Przywara,
	U-Boot Mailing List

[...]
> >
> > This will work:
> >
> > diff --git a/dts/Kconfig b/dts/Kconfig
> > index dabe0080c1..6aca8a8738 100644
> > --- a/dts/Kconfig
> > +++ b/dts/Kconfig
> > @@ -90,6 +90,7 @@ config OF_LIVE
> >   choice
> >          prompt "Provider of DTB for DT control"
> >          depends on OF_CONTROL
> > +       default OF_BOARD if SANDBOX
> >
> >   config OF_SEPARATE
> >          bool "Separate DTB for DT control"
> >
> 
> Do you have a new version of this one, please? We should try to get it in soon.

Yea it's on my queue.  I was just waiting for Tom to pick up the
OF_PRIOR_STAGE cleanup, since it's based on top of that.
Let me resend that regardless.

Cheers
/Ilias

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-11 20:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28  9:05 [PATCH] sandbox: Remove OF_HOSTFILE Ilias Apalodimas
2021-09-28  9:05 ` Ilias Apalodimas
2021-09-28 22:46   ` Simon Glass
2021-09-29  5:39     ` Ilias Apalodimas
2021-09-29  6:25       ` Ilias Apalodimas
2021-09-29  7:44         ` Heinrich Schuchardt
2021-10-11 19:20           ` Simon Glass
2021-10-11 20:16             ` Ilias Apalodimas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).