[PATCH v2 00/11] riscv: Switch to use binman to generate u-boot.itb

[PATCH v2 00/11] riscv: Switch to use binman to generate u-boot.itb

[Bin Meng]

This series updates binman to handle creation of u-boot.itb image for
RISC-V boards.

Changes in v2:
- drop patch: "binman: test: Correct the name of 170_fit_fdt_missing_prop.dts",
  as it was based on a wrong version
- drop patch: "makefile: Update clean rule to remove files generated by binman",
  as it is better to refactor binman to generate all intermediate files for
  Makefile to clean
- drop "size = <16>" in the binman node
- new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
- new patch: "riscv: dts: Sort build targets in alphabetical order"
- new patch: "riscv: qemu: Switch to use binman to generate u-boot.itb"
- new patch: "riscv: Drop USE_SPL_FIT_GENERATOR"

Bin Meng (11):
  common: kconfig: Correct a typo in SPL_LOAD_FIT
  binman: Correct '-a' description in the doc
  binman: Correct the comment for ATF entry type
  binman: test: Rename 172_fit_fdt.dts to 170_fit_fdt.dts
  binman: Add support for RISC-V OpenSBI fw_dynamic blob
  makefile: Pass OpenSBI blob to binman make rules
  riscv: sifive: unleashed: Switch to use binman to generate u-boot.itb
  binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE
  riscv: dts: Sort build targets in alphabetical order
  riscv: qemu: Switch to use binman to generate u-boot.itb
  riscv: Drop USE_SPL_FIT_GENERATOR

 Makefile                                      |   4 +-
 arch/riscv/cpu/generic/Kconfig                |   1 +
 arch/riscv/dts/Makefile                       |   3 +-
 arch/riscv/dts/binman.dtsi                    |  78 ++++++++++++++
 .../dts/hifive-unleashed-a00-u-boot.dtsi      |   1 +
 arch/riscv/dts/qemu-virt.dts                  |   8 ++
 arch/riscv/lib/mkimage_fit_opensbi.sh         | 100 ------------------
 board/sifive/unleashed/Kconfig                |   1 +
 common/Kconfig.boot                           |   5 +-
 configs/qemu-riscv32_spl_defconfig            |   1 +
 configs/qemu-riscv64_spl_defconfig            |   1 +
 dts/Kconfig                                   |  18 ++++
 lib/Kconfig                                   |   2 +-
 tools/binman/binman.rst                       |   4 +-
 tools/binman/entries.rst                      |  13 +++
 tools/binman/etype/atf_bl31.py                |   2 +-
 tools/binman/etype/opensbi.py                 |  23 ++++
 tools/binman/ftest.py                         |  17 ++-
 .../test/{172_fit_fdt.dts => 170_fit_fdt.dts} |   0
 tools/binman/test/201_opensbi.dts             |  14 +++
 20 files changed, 182 insertions(+), 114 deletions(-)
 create mode 100644 arch/riscv/dts/binman.dtsi
 create mode 100644 arch/riscv/dts/qemu-virt.dts
 delete mode 100755 arch/riscv/lib/mkimage_fit_opensbi.sh
 create mode 100644 tools/binman/etype/opensbi.py
 rename tools/binman/test/{172_fit_fdt.dts => 170_fit_fdt.dts} (100%)
 create mode 100644 tools/binman/test/201_opensbi.dts

-- 
2.25.1

[PATCH v2 01/11] common: kconfig: Correct a typo in SPL_LOAD_FIT

[Bin Meng]

It should be FDT, not FTD.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/Kconfig.boot | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 5a18d62d78..94d82c27dd 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -204,7 +204,7 @@ config SPL_LOAD_FIT
 
 	  This path has the following limitations:
 
-	  1. "loadables" images, other than FTDs, which do not have a "load"
+	  1. "loadables" images, other than FDTs, which do not have a "load"
 	     property will not be loaded. This limitation also applies to FPGA
 	     images with the correct "compatible" string.
 	  2. For FPGA images, only the "compatible" = "u-boot,fpga-legacy"
-- 
2.25.1

[PATCH v2 02/11] binman: Correct '-a' description in the doc

[Bin Meng]

It needs a space around '-a'.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/binman.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 1aa2459d50..b3df3a6428 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -322,9 +322,9 @@ Sometimes it is useful to pass binman the value of an entry property from the
 command line. For example some entries need access to files and it is not
 always convenient to put these filenames in the image definition (device tree).
 
-The-a option supports this::
+The -a option supports this::
 
-    -a<prop>=<value>
+    -a <prop>=<value>
 
 where::
 
-- 
2.25.1

[PATCH v2 03/11] binman: Correct the comment for ATF entry type

[Bin Meng]

This is wrongly referring to Intel ME, which should be ATF.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/etype/atf_bl31.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/etype/atf_bl31.py b/tools/binman/etype/atf_bl31.py
index 163d714184..2041da416c 100644
--- a/tools/binman/etype/atf_bl31.py
+++ b/tools/binman/etype/atf_bl31.py
@@ -2,7 +2,7 @@
 # Copyright 2020 Google LLC
 # Written by Simon Glass <sjg@chromium.org>
 #
-# Entry-type module for Intel Management Engine binary blob
+# Entry-type module for ARM Trusted Firmware binary blob
 #
 
 from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
-- 
2.25.1

[PATCH v2 04/11] binman: test: Rename 172_fit_fdt.dts to 170_fit_fdt.dts

[Bin Meng]

Currently there are 2 binman test cases using the same 172 number.
It seems that 172_fit_fdt.dts was originally named as 170_, but
commit c0f1ebe9c1b9 ("binman: Allow selecting default FIT configuration")
changed its name to 172_ for no reason. Let's change it back.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/ftest.py                                  | 10 +++++-----
 tools/binman/test/{172_fit_fdt.dts => 170_fit_fdt.dts} |  0
 2 files changed, 5 insertions(+), 5 deletions(-)
 rename tools/binman/test/{172_fit_fdt.dts => 170_fit_fdt.dts} (100%)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f36823f51b..b0daccbc3b 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3826,7 +3826,7 @@ class TestFunctional(unittest.TestCase):
             'default-dt': 'test-fdt2',
         }
         data = self._DoReadFileDtb(
-            '172_fit_fdt.dts',
+            '170_fit_fdt.dts',
             entry_args=entry_args,
             extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
         self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
@@ -3848,7 +3848,7 @@ class TestFunctional(unittest.TestCase):
     def testFitFdtMissingList(self):
         """Test handling of a missing 'of-list' entry arg"""
         with self.assertRaises(ValueError) as e:
-            self._DoReadFile('172_fit_fdt.dts')
+            self._DoReadFile('170_fit_fdt.dts')
         self.assertIn("Generator node requires 'of-list' entry argument",
                       str(e.exception))
 
@@ -3871,7 +3871,7 @@ class TestFunctional(unittest.TestCase):
         entry_args = {
             'of-list': '',
         }
-        data = self._DoReadFileDtb('172_fit_fdt.dts', entry_args=entry_args)[0]
+        data = self._DoReadFileDtb('170_fit_fdt.dts', entry_args=entry_args)[0]
 
     def testFitFdtMissing(self):
         """Test handling of a missing 'default-dt' entry arg"""
@@ -3880,7 +3880,7 @@ class TestFunctional(unittest.TestCase):
         }
         with self.assertRaises(ValueError) as e:
             self._DoReadFileDtb(
-                '172_fit_fdt.dts',
+                '170_fit_fdt.dts',
                 entry_args=entry_args,
                 extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
         self.assertIn("Generated 'default' node requires default-dt entry argument",
@@ -3894,7 +3894,7 @@ class TestFunctional(unittest.TestCase):
         }
         with self.assertRaises(ValueError) as e:
             self._DoReadFileDtb(
-                '172_fit_fdt.dts',
+                '170_fit_fdt.dts',
                 entry_args=entry_args,
                 extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
         self.assertIn("default-dt entry argument 'test-fdt3' not found in fdt list: test-fdt1, test-fdt2",
diff --git a/tools/binman/test/172_fit_fdt.dts b/tools/binman/test/170_fit_fdt.dts
similarity index 100%
rename from tools/binman/test/172_fit_fdt.dts
rename to tools/binman/test/170_fit_fdt.dts
-- 
2.25.1

[PATCH v2 05/11] binman: Add support for RISC-V OpenSBI fw_dynamic blob

[Bin Meng]

Add an entry for RISC-V OpenSBI's 'fw_dynamic' firmware payload.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changes in v2:
- drop "size = <16>" in the binman node

 tools/binman/entries.rst          | 13 +++++++++++++
 tools/binman/etype/opensbi.py     | 23 +++++++++++++++++++++++
 tools/binman/ftest.py             |  7 +++++++
 tools/binman/test/201_opensbi.dts | 14 ++++++++++++++
 4 files changed, 57 insertions(+)
 create mode 100644 tools/binman/etype/opensbi.py
 create mode 100644 tools/binman/test/201_opensbi.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index f1c3b7de7a..dcac700c46 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -761,6 +761,19 @@ binman.
 
 
 
+Entry: opensbi: RISC-V OpenSBI fw_dynamic blob
+----------------------------------------------
+
+Properties / Entry arguments:
+    - opensbi-path: Filename of file to read into entry. This is typically
+        called fw_dynamic.bin
+
+This entry holds the run-time firmware, typically started by U-Boot SPL.
+See the U-Boot README for your architecture or board for how to use it. See
+https://github.com/riscv/opensbi for more information about OpenSBI.
+
+
+
 Entry: powerpc-mpc85xx-bootpg-resetvec: PowerPC mpc85xx bootpg + resetvec code for U-Boot
 -----------------------------------------------------------------------------------------
 
diff --git a/tools/binman/etype/opensbi.py b/tools/binman/etype/opensbi.py
new file mode 100644
index 0000000000..74d473d535
--- /dev/null
+++ b/tools/binman/etype/opensbi.py
@@ -0,0 +1,23 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2021, Bin Meng <bmeng.cn@gmail.com>
+#
+# Entry-type module for RISC-V OpenSBI binary blob
+#
+
+from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
+
+class Entry_opensbi(Entry_blob_named_by_arg):
+    """RISC-V OpenSBI fw_dynamic blob
+
+    Properties / Entry arguments:
+        - opensbi-path: Filename of file to read into entry. This is typically
+            called fw_dynamic.bin
+
+    This entry holds the run-time firmware, typically started by U-Boot SPL.
+    See the U-Boot README for your architecture or board for how to use it. See
+    https://github.com/riscv/opensbi for more information about OpenSBI.
+    """
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node, 'opensbi')
+        self.external = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b0daccbc3b..5383eec489 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -76,6 +76,7 @@ FSP_M_DATA            = b'fsp_m'
 FSP_S_DATA            = b'fsp_s'
 FSP_T_DATA            = b'fsp_t'
 ATF_BL31_DATA         = b'bl31'
+OPENSBI_DATA          = b'opensbi'
 SCP_DATA              = b'scp'
 TEST_FDT1_DATA        = b'fdt1'
 TEST_FDT2_DATA        = b'test-fdt2'
@@ -178,6 +179,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('compress', COMPRESS_DATA)
         TestFunctional._MakeInputFile('compress_big', COMPRESS_DATA_BIG)
         TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA)
+        TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
         TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
 
         # Add a few .dtb files for testing
@@ -4535,5 +4537,10 @@ class TestFunctional(unittest.TestCase):
         expected += tools.GetBytes(0, 88 - len(expected)) + U_BOOT_NODTB_DATA
         self.assertEqual(expected, data)
 
+    def testPackOpenSBI(self):
+        """Test that an image with an OpenSBI binary can be created"""
+        data = self._DoReadFile('201_opensbi.dts')
+        self.assertEqual(OPENSBI_DATA, data[:len(OPENSBI_DATA)])
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/201_opensbi.dts b/tools/binman/test/201_opensbi.dts
new file mode 100644
index 0000000000..942183f990
--- /dev/null
+++ b/tools/binman/test/201_opensbi.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		opensbi {
+			filename = "fw_dynamic.bin";
+		};
+	};
+};
-- 
2.25.1

[PATCH v2 06/11] makefile: Pass OpenSBI blob to binman make rules

[Bin Meng]

This updates the make rules to pass OpenSBI blob to binman.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 404977efa5..3d88559b67 100644
--- a/Makefile
+++ b/Makefile
@@ -1287,6 +1287,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 		-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
 		-I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
 		-a atf-bl31-path=${BL31} \
+		-a opensbi-path=${OPENSBI} \
 		-a default-dt=$(default_dt) \
 		-a scp-path=$(SCP) \
 		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
-- 
2.25.1

[PATCH v2 07/11] riscv: sifive: unleashed: Switch to use binman to generate u-boot.itb

[Bin Meng]

At present SiFive Unleashed board uses the Makefile to create the FIT,
using USE_SPL_FIT_GENERATOR, which is deprecated as per the Makefile
warning. Update to use binman instead.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 arch/riscv/dts/binman.dtsi                    | 70 +++++++++++++++++++
 .../dts/hifive-unleashed-a00-u-boot.dtsi      |  1 +
 board/sifive/unleashed/Kconfig                |  1 +
 configs/sifive_unleashed_defconfig            |  1 +
 4 files changed, 73 insertions(+)
 create mode 100644 arch/riscv/dts/binman.dtsi

diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
new file mode 100644
index 0000000000..f2f8647b24
--- /dev/null
+++ b/arch/riscv/dts/binman.dtsi
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021, Bin Meng <bmeng.cn@gmail.com>
+ */
+
+#include <config.h>
+
+/ {
+	binman: binman {
+		multiple-images;
+	};
+};
+
+&binman {
+	itb {
+		filename = "u-boot.itb";
+
+		fit {
+			description = "Configuration to load OpenSBI before U-Boot";
+			#address-cells = <1>;
+			fit,fdt-list = "of-list";
+
+			images {
+				uboot {
+					description = "U-Boot";
+					type = "standalone";
+					os = "U-Boot";
+					arch = "riscv";
+					compression = "none";
+					load = <CONFIG_SYS_TEXT_BASE>;
+
+					uboot_blob: blob-ext {
+						filename = "u-boot-nodtb.bin";
+					};
+				};
+
+				opensbi {
+					description = "OpenSBI fw_dynamic Firmware";
+					type = "firmware";
+					os = "opensbi";
+					arch = "riscv";
+					compression = "none";
+					load = <CONFIG_SPL_OPENSBI_LOAD_ADDR>;
+					entry = <CONFIG_SPL_OPENSBI_LOAD_ADDR>;
+
+					opensbi_blob: blob-ext {
+						filename = "fw_dynamic.bin";
+					};
+				};
+
+				@fdt-SEQ {
+					description = "NAME";
+					type = "flat_dt";
+					compression = "none";
+				};
+			};
+
+			configurations {
+				default = "conf-1";
+
+				@conf-SEQ {
+					description = "NAME";
+					firmware = "opensbi";
+					loadables = "uboot";
+					fdt = "fdt-SEQ";
+				};
+			};
+		};
+	};
+};
diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
index 1996149c95..51b566116d 100644
--- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
+++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
@@ -3,6 +3,7 @@
  * Copyright (C) 2019 Jagan Teki <jagan@amarulasolutions.com>
  */
 
+#include "binman.dtsi"
 #include "fu540-c000-u-boot.dtsi"
 #include "fu540-hifive-unleashed-a00-ddr.dtsi"
 
diff --git a/board/sifive/unleashed/Kconfig b/board/sifive/unleashed/Kconfig
index dbffd59c98..502916e86a 100644
--- a/board/sifive/unleashed/Kconfig
+++ b/board/sifive/unleashed/Kconfig
@@ -27,6 +27,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	def_bool y
 	select SIFIVE_FU540
 	select ENV_IS_IN_SPI_FLASH
+	select BINMAN
 	imply CMD_DHCP
 	imply CMD_EXT2
 	imply CMD_EXT4
diff --git a/configs/sifive_unleashed_defconfig b/configs/sifive_unleashed_defconfig
index 62416a7c1d..dc9313e572 100644
--- a/configs/sifive_unleashed_defconfig
+++ b/configs/sifive_unleashed_defconfig
@@ -14,6 +14,7 @@ CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x84000000
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 CONFIG_MISC_INIT_R=y
-- 
2.25.1

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Bin Meng]

For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
in the U-Boot build phase hence the binman node information is not
available. In order to support such use case, a new Kconfig option
BINMAN_DTB is introduced, to tell U-Boot build system that a device
tree blob is explicitly required when using binman to package U-Boot.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"

 Makefile    |  3 ++-
 dts/Kconfig | 18 ++++++++++++++++++
 lib/Kconfig |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 3d88559b67..9043828675 100644
--- a/Makefile
+++ b/Makefile
@@ -918,6 +918,7 @@ endif
 endif
 INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
 INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
+INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
 ifeq ($(CONFIG_SPL_FRAMEWORK),y)
 INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
 endif
@@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
 u-boot.bin: u-boot-dtb.bin FORCE
 	$(call if_changed,copy)
 else
-u-boot.bin: u-boot-nodtb.bin FORCE
+u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
 	$(call if_changed,copy)
 endif
 
diff --git a/dts/Kconfig b/dts/Kconfig
index 99ce75e1a2..84d43fc6a4 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -19,6 +19,24 @@ config BINMAN
 	bool
 	select DTOC
 
+config BINMAN_DTB
+	bool
+	depends on BINMAN
+	default y if OF_PRIOR_STAGE
+	help
+	  This option tells U-Boot build system that a device tree blob is
+	  explicitly required when using binman to package U-Boot.
+
+	  This is not necessary in a common scenario where a device tree blob
+	  is provided in the arch/<arch>/dts directory for a specific board,
+	  where the binman node has been put in the board's device tree, aka
+	  an implicit way. However for a scenario like the board's device tree
+	  is not provided in the U-Boot source tree, but given to U-Boot in the
+	  runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
+	  is passed by a prior stage bootloader. For such scenario, we will
+	  need to provide a device tree blob containing binman node to describe
+	  how to package U-Boot.
+
 menu "Device Tree Control"
 	depends on SUPPORT_OF_CONTROL
 
diff --git a/lib/Kconfig b/lib/Kconfig
index 6d2d41de30..7326d95357 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -25,7 +25,7 @@ config BCH
 config BINMAN_FDT
 	bool "Allow access to binman information in the device tree"
 	depends on BINMAN && DM && OF_CONTROL
-	default y
+	default y if !BINMAN_DTB
 	help
 	  This enables U-Boot to access information about binman entries,
 	  stored in the device tree in a binman node. Typical uses are to
-- 
2.25.1

[PATCH v2 09/11] riscv: dts: Sort build targets in alphabetical order

[Bin Meng]

Sort the RISC-V dts build targets in alphabetical order.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: "riscv: dts: Sort build targets in alphabetical order"

 arch/riscv/dts/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
index 8138d89d84..3780334875 100644
--- a/arch/riscv/dts/Makefile
+++ b/arch/riscv/dts/Makefile
@@ -1,9 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
+dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
 dtb-$(CONFIG_TARGET_SIFIVE_UNLEASHED) += hifive-unleashed-a00.dtb
 dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
-dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
 
 targets += $(dtb-y)
 
-- 
2.25.1

[PATCH v2 10/11] riscv: qemu: Switch to use binman to generate u-boot.itb

[Bin Meng]

By utilizing the newly introduced BINMAN_DTB option, along with a
new dedicated device tree source file for the QEMU virt target used
for binman only, we can now use binman to generate u-boot.itb.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: "riscv: qemu: Switch to use binman to generate u-boot.itb"

 arch/riscv/cpu/generic/Kconfig     | 1 +
 arch/riscv/dts/Makefile            | 1 +
 arch/riscv/dts/binman.dtsi         | 8 ++++++++
 arch/riscv/dts/qemu-virt.dts       | 8 ++++++++
 configs/qemu-riscv32_spl_defconfig | 2 ++
 configs/qemu-riscv64_spl_defconfig | 2 ++
 6 files changed, 22 insertions(+)
 create mode 100644 arch/riscv/dts/qemu-virt.dts

diff --git a/arch/riscv/cpu/generic/Kconfig b/arch/riscv/cpu/generic/Kconfig
index 198e36e969..a4934bb957 100644
--- a/arch/riscv/cpu/generic/Kconfig
+++ b/arch/riscv/cpu/generic/Kconfig
@@ -4,6 +4,7 @@
 
 config GENERIC_RISCV
 	bool
+	select BINMAN if SPL
 	select ARCH_EARLY_INIT_R
 	imply CPU
 	imply CPU_RISCV
diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
index 3780334875..26ef853282 100644
--- a/arch/riscv/dts/Makefile
+++ b/arch/riscv/dts/Makefile
@@ -2,6 +2,7 @@
 
 dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
 dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
+dtb-$(CONFIG_TARGET_QEMU_VIRT) += qemu-virt.dtb
 dtb-$(CONFIG_TARGET_SIFIVE_UNLEASHED) += hifive-unleashed-a00.dtb
 dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
 
diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
index f2f8647b24..40f9644b68 100644
--- a/arch/riscv/dts/binman.dtsi
+++ b/arch/riscv/dts/binman.dtsi
@@ -48,21 +48,29 @@
 					};
 				};
 
+#ifndef CONFIG_OF_PRIOR_STAGE
 				@fdt-SEQ {
 					description = "NAME";
 					type = "flat_dt";
 					compression = "none";
 				};
+#endif
 			};
 
 			configurations {
 				default = "conf-1";
 
+#ifndef CONFIG_OF_PRIOR_STAGE
 				@conf-SEQ {
+#else
+				conf-1 {
+#endif
 					description = "NAME";
 					firmware = "opensbi";
 					loadables = "uboot";
+#ifndef CONFIG_OF_PRIOR_STAGE
 					fdt = "fdt-SEQ";
+#endif
 				};
 			};
 		};
diff --git a/arch/riscv/dts/qemu-virt.dts b/arch/riscv/dts/qemu-virt.dts
new file mode 100644
index 0000000000..fecff542b9
--- /dev/null
+++ b/arch/riscv/dts/qemu-virt.dts
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021, Bin Meng <bmeng.cn@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "binman.dtsi"
diff --git a/configs/qemu-riscv32_spl_defconfig b/configs/qemu-riscv32_spl_defconfig
index 18dfe33ca8..a4c156612a 100644
--- a/configs/qemu-riscv32_spl_defconfig
+++ b/configs/qemu-riscv32_spl_defconfig
@@ -2,11 +2,13 @@ CONFIG_RISCV=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x20000
 CONFIG_SPL=y
+CONFIG_DEFAULT_DEVICE_TREE="qemu-virt"
 CONFIG_TARGET_QEMU_VIRT=y
 CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x80200000
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 # CONFIG_CMD_MII is not set
diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig
index 897adf6a29..6c680483ce 100644
--- a/configs/qemu-riscv64_spl_defconfig
+++ b/configs/qemu-riscv64_spl_defconfig
@@ -2,12 +2,14 @@ CONFIG_RISCV=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x20000
 CONFIG_SPL=y
+CONFIG_DEFAULT_DEVICE_TREE="qemu-virt"
 CONFIG_TARGET_QEMU_VIRT=y
 CONFIG_ARCH_RV64I=y
 CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x80200000
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 # CONFIG_CMD_MII is not set
-- 
2.25.1

[PATCH v2 11/11] riscv: Drop USE_SPL_FIT_GENERATOR

[Bin Meng]

Now that we have switched binman to generate u-boot.itb on both
SiFive Unleashed and QEMU virt, USE_SPL_FIT_GENERATOR is no longer
needed and can be dropped.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: "riscv: Drop USE_SPL_FIT_GENERATOR"

 arch/riscv/lib/mkimage_fit_opensbi.sh | 100 --------------------------
 common/Kconfig.boot                   |   3 +-
 configs/qemu-riscv32_spl_defconfig    |   1 -
 configs/qemu-riscv64_spl_defconfig    |   1 -
 configs/sifive_unleashed_defconfig    |   1 -
 5 files changed, 1 insertion(+), 105 deletions(-)
 delete mode 100755 arch/riscv/lib/mkimage_fit_opensbi.sh

diff --git a/arch/riscv/lib/mkimage_fit_opensbi.sh b/arch/riscv/lib/mkimage_fit_opensbi.sh
deleted file mode 100755
index d6f95e5bfd..0000000000
--- a/arch/riscv/lib/mkimage_fit_opensbi.sh
+++ /dev/null
@@ -1,100 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0+
-#
-# script to generate FIT image source for RISC-V boards with OpenSBI
-# and, optionally, multiple device trees (given on the command line).
-#
-# usage: $0 [<dt_name> [<dt_name] ...]
-
-[ -z "$OPENSBI" ] && OPENSBI="fw_dynamic.bin"
-
-if [ -z "$UBOOT_LOAD_ADDR" ]; then
-	UBOOT_LOAD_ADDR="$(grep "^CONFIG_SYS_TEXT_BASE=" .config | awk 'BEGIN{FS="="} {print $2}')"
-fi
-
-if [ -z "$OPENSBI_LOAD_ADDR" ]; then
-	OPENSBI_LOAD_ADDR="$(grep "^CONFIG_SPL_OPENSBI_LOAD_ADDR=" .config | awk 'BEGIN{FS="="} {print $2}')"
-fi
-
-if [ ! -f $OPENSBI ]; then
-	echo "WARNING: OpenSBI binary \"$OPENSBI\" not found, resulting binary is not functional." >&2
-	OPENSBI=/dev/null
-fi
-
-cat << __HEADER_EOF
-/dts-v1/;
-
-/ {
-	description = "Configuration to load OpenSBI before U-Boot";
-
-	images {
-		uboot {
-			description = "U-Boot";
-			data = /incbin/("u-boot-nodtb.bin");
-			type = "standalone";
-			os = "U-Boot";
-			arch = "riscv";
-			compression = "none";
-			load = <$UBOOT_LOAD_ADDR>;
-		};
-		opensbi {
-			description = "RISC-V OpenSBI";
-			data = /incbin/("$OPENSBI");
-			type = "firmware";
-			os = "opensbi";
-			arch = "riscv";
-			compression = "none";
-			load = <$OPENSBI_LOAD_ADDR>;
-			entry = <$OPENSBI_LOAD_ADDR>;
-		};
-__HEADER_EOF
-
-cnt=1
-for dtname in $*
-do
-	cat << __FDT_IMAGE_EOF
-		fdt_$cnt {
-			description = "$(basename $dtname .dtb)";
-			data = /incbin/("$dtname");
-			type = "flat_dt";
-			compression = "none";
-		};
-__FDT_IMAGE_EOF
-cnt=$((cnt+1))
-done
-
-cat << __CONF_HEADER_EOF
-	};
-	configurations {
-		default = "config_1";
-
-__CONF_HEADER_EOF
-
-if [ $# -eq 0 ]; then
-cat << __CONF_SECTION_EOF
-		config_1 {
-			description = "U-Boot FIT";
-			firmware = "opensbi";
-			loadables = "uboot";
-		};
-__CONF_SECTION_EOF
-else
-cnt=1
-for dtname in $*
-do
-cat << __CONF_SECTION_EOF
-		config_$cnt {
-			description = "$(basename $dtname .dtb)";
-			firmware = "opensbi";
-			loadables = "uboot";
-			fdt = "fdt_$cnt";
-		};
-__CONF_SECTION_EOF
-cnt=$((cnt+1))
-done
-fi
-
-cat << __ITS_EOF
-	};
-};
-__ITS_EOF
diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 94d82c27dd..89a3161f1f 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -273,14 +273,13 @@ config SPL_FIT_SOURCE
 
 config USE_SPL_FIT_GENERATOR
 	bool "Use a script to generate the .its script"
-	default y if SPL_FIT && !ARCH_SUNXI
+	default y if SPL_FIT && (!ARCH_SUNXI && !RISCV)
 
 config SPL_FIT_GENERATOR
 	string ".its file generator script for U-Boot FIT image"
 	depends on USE_SPL_FIT_GENERATOR
 	default "arch/arm/mach-rockchip/make_fit_atf.py" if SPL_LOAD_FIT && ARCH_ROCKCHIP
 	default "arch/arm/mach-zynqmp/mkimage_fit_atf.sh" if SPL_LOAD_FIT && ARCH_ZYNQMP
-	default "arch/riscv/lib/mkimage_fit_opensbi.sh" if SPL_LOAD_FIT && RISCV
 	help
 	  Specifies a (platform specific) script file to generate the FIT
 	  source file used to build the U-Boot FIT image file. This gets
diff --git a/configs/qemu-riscv32_spl_defconfig b/configs/qemu-riscv32_spl_defconfig
index a4c156612a..f30bd5f0a0 100644
--- a/configs/qemu-riscv32_spl_defconfig
+++ b/configs/qemu-riscv32_spl_defconfig
@@ -8,7 +8,6 @@ CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x80200000
-# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 # CONFIG_CMD_MII is not set
diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig
index 6c680483ce..ee91ece0ec 100644
--- a/configs/qemu-riscv64_spl_defconfig
+++ b/configs/qemu-riscv64_spl_defconfig
@@ -9,7 +9,6 @@ CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x80200000
-# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 # CONFIG_CMD_MII is not set
diff --git a/configs/sifive_unleashed_defconfig b/configs/sifive_unleashed_defconfig
index dc9313e572..62416a7c1d 100644
--- a/configs/sifive_unleashed_defconfig
+++ b/configs/sifive_unleashed_defconfig
@@ -14,7 +14,6 @@ CONFIG_RISCV_SMODE=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x84000000
-# CONFIG_USE_SPL_FIT_GENERATOR is not set
 CONFIG_DISPLAY_CPUINFO=y
 CONFIG_DISPLAY_BOARDINFO=y
 CONFIG_MISC_INIT_R=y
-- 
2.25.1

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> in the U-Boot build phase hence the binman node information is not
> available. In order to support such use case, a new Kconfig option
> BINMAN_DTB is introduced, to tell U-Boot build system that a device
> tree blob is explicitly required when using binman to package U-Boot.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
>
>  Makefile    |  3 ++-
>  dts/Kconfig | 18 ++++++++++++++++++
>  lib/Kconfig |  2 +-
>  3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3d88559b67..9043828675 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -918,6 +918,7 @@ endif
>  endif
>  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
>  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
>  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
>  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
>  endif
> @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
>  u-boot.bin: u-boot-dtb.bin FORCE
>         $(call if_changed,copy)
>  else
> -u-boot.bin: u-boot-nodtb.bin FORCE
> +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
>         $(call if_changed,copy)
>  endif
>
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 99ce75e1a2..84d43fc6a4 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -19,6 +19,24 @@ config BINMAN
>         bool
>         select DTOC
>
> +config BINMAN_DTB
> +       bool
> +       depends on BINMAN
> +       default y if OF_PRIOR_STAGE
> +       help
> +         This option tells U-Boot build system that a device tree blob is
> +         explicitly required when using binman to package U-Boot.
> +
> +         This is not necessary in a common scenario where a device tree blob
> +         is provided in the arch/<arch>/dts directory for a specific board,
> +         where the binman node has been put in the board's device tree, aka
> +         an implicit way. However for a scenario like the board's device tree
> +         is not provided in the U-Boot source tree, but given to U-Boot in the
> +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> +         is passed by a prior stage bootloader. For such scenario, we will
> +         need to provide a device tree blob containing binman node to describe
> +         how to package U-Boot.

I don't fully understand the effect of this. Is binman (at build time)
using a binary DT provided by another build system? Or is it not
available at build time and binman cannot run?

The name is too vanilla I think. It should indicate that this is an
unusual situation.

> +
>  menu "Device Tree Control"
>         depends on SUPPORT_OF_CONTROL
>
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 6d2d41de30..7326d95357 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -25,7 +25,7 @@ config BCH
>  config BINMAN_FDT
>         bool "Allow access to binman information in the device tree"
>         depends on BINMAN && DM && OF_CONTROL
> -       default y
> +       default y if !BINMAN_DTB
>         help
>           This enables U-Boot to access information about binman entries,
>           stored in the device tree in a binman node. Typical uses are to
> --
> 2.25.1
>

Regards,
Simon

[PATCH v2 10/11] riscv: qemu: Switch to use binman to generate u-boot.itb

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> By utilizing the newly introduced BINMAN_DTB option, along with a
> new dedicated device tree source file for the QEMU virt target used
> for binman only, we can now use binman to generate u-boot.itb.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - new patch: "riscv: qemu: Switch to use binman to generate u-boot.itb"
>
>  arch/riscv/cpu/generic/Kconfig     | 1 +
>  arch/riscv/dts/Makefile            | 1 +
>  arch/riscv/dts/binman.dtsi         | 8 ++++++++
>  arch/riscv/dts/qemu-virt.dts       | 8 ++++++++
>  configs/qemu-riscv32_spl_defconfig | 2 ++
>  configs/qemu-riscv64_spl_defconfig | 2 ++
>  6 files changed, 22 insertions(+)
>  create mode 100644 arch/riscv/dts/qemu-virt.dts

Reviewed-by: Simon Glass <sjg@chromium.org>

Do you think it would be better to have qemu use a completely
different DT? Do 'normal' boards define OF_PRIOR_STAGE?

Regards,
Simon

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Bin Meng]

Hi Simon,

On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > in the U-Boot build phase hence the binman node information is not
> > available. In order to support such use case, a new Kconfig option
> > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > tree blob is explicitly required when using binman to package U-Boot.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > ---
> >
> > Changes in v2:
> > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> >
> >  Makefile    |  3 ++-
> >  dts/Kconfig | 18 ++++++++++++++++++
> >  lib/Kconfig |  2 +-
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 3d88559b67..9043828675 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -918,6 +918,7 @@ endif
> >  endif
> >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> >  endif
> > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> >  u-boot.bin: u-boot-dtb.bin FORCE
> >         $(call if_changed,copy)
> >  else
> > -u-boot.bin: u-boot-nodtb.bin FORCE
> > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
> >         $(call if_changed,copy)
> >  endif
> >
> > diff --git a/dts/Kconfig b/dts/Kconfig
> > index 99ce75e1a2..84d43fc6a4 100644
> > --- a/dts/Kconfig
> > +++ b/dts/Kconfig
> > @@ -19,6 +19,24 @@ config BINMAN
> >         bool
> >         select DTOC
> >
> > +config BINMAN_DTB
> > +       bool
> > +       depends on BINMAN
> > +       default y if OF_PRIOR_STAGE
> > +       help
> > +         This option tells U-Boot build system that a device tree blob is
> > +         explicitly required when using binman to package U-Boot.
> > +
> > +         This is not necessary in a common scenario where a device tree blob
> > +         is provided in the arch/<arch>/dts directory for a specific board,
> > +         where the binman node has been put in the board's device tree, aka
> > +         an implicit way. However for a scenario like the board's device tree
> > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > +         is passed by a prior stage bootloader. For such scenario, we will
> > +         need to provide a device tree blob containing binman node to describe
> > +         how to package U-Boot.
>
> I don't fully understand the effect of this. Is binman (at build time)
> using a binary DT provided by another build system? Or is it not
> available at build time and binman cannot run?

binman uses a DT provided by U-Boot, but that DT is only for binman if
OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
there.

You can check patch 10 for how it works.

>
> The name is too vanilla I think. It should indicate that this is an
> unusual situation.

I cannot think of a good name :(

Regards,
Bin

[PATCH v2 10/11] riscv: qemu: Switch to use binman to generate u-boot.itb

[Bin Meng]

Hi Simon,

On Sat, May 8, 2021 at 11:49 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > By utilizing the newly introduced BINMAN_DTB option, along with a
> > new dedicated device tree source file for the QEMU virt target used
> > for binman only, we can now use binman to generate u-boot.itb.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > ---
> >
> > Changes in v2:
> > - new patch: "riscv: qemu: Switch to use binman to generate u-boot.itb"
> >
> >  arch/riscv/cpu/generic/Kconfig     | 1 +
> >  arch/riscv/dts/Makefile            | 1 +
> >  arch/riscv/dts/binman.dtsi         | 8 ++++++++
> >  arch/riscv/dts/qemu-virt.dts       | 8 ++++++++
> >  configs/qemu-riscv32_spl_defconfig | 2 ++
> >  configs/qemu-riscv64_spl_defconfig | 2 ++
> >  6 files changed, 22 insertions(+)
> >  create mode 100644 arch/riscv/dts/qemu-virt.dts
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Do you think it would be better to have qemu use a completely
> different DT? Do 'normal' boards define OF_PRIOR_STAGE?

I am not sure what do you mean by using a completely different DT for
QEMU. The device nodes are provided by QEMU and passed to U-Boot, via
OF_PRIOR_STAGE.

I would not expect "normal" boards to define OF_PRIOR_STAGE, but looks
there are still some. A grep in the U-Boot source shows the following
boards support OF_PRIOR_STAGE except QEMU riscv:

./configs/bcm7260_defconfig:25:CONFIG_OF_PRIOR_STAGE=y
./configs/bcm7445_defconfig:26:CONFIG_OF_PRIOR_STAGE=y
./configs/ae350_rv64_spl_defconfig:21:CONFIG_OF_PRIOR_STAGE=y
./configs/ae350_rv32_spl_defconfig:20:CONFIG_OF_PRIOR_STAGE=y
./configs/ae350_rv64_defconfig:19:CONFIG_OF_PRIOR_STAGE=y
./configs/ae350_rv32_defconfig:18:CONFIG_OF_PRIOR_STAGE=y

config ARCH_BCMSTB
        bool "Broadcom BCM7XXX family"
        select CPU_V7A
        select DM
        select OF_CONTROL
        select OF_PRIOR_STAGE
        imply CMD_DM

Regards,
Bin

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > in the U-Boot build phase hence the binman node information is not
> > > available. In order to support such use case, a new Kconfig option
> > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > tree blob is explicitly required when using binman to package U-Boot.
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > >
> > >  Makefile    |  3 ++-
> > >  dts/Kconfig | 18 ++++++++++++++++++
> > >  lib/Kconfig |  2 +-
> > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 3d88559b67..9043828675 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -918,6 +918,7 @@ endif
> > >  endif
> > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > >  endif
> > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > >  u-boot.bin: u-boot-dtb.bin FORCE
> > >         $(call if_changed,copy)
> > >  else
> > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE

What is this change for, specifically? Can you add a comment?

It seems to be overriding the qemu thing of not actually building a DT?

> > >         $(call if_changed,copy)
> > >  endif
> > >
> > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > index 99ce75e1a2..84d43fc6a4 100644
> > > --- a/dts/Kconfig
> > > +++ b/dts/Kconfig
> > > @@ -19,6 +19,24 @@ config BINMAN
> > >         bool
> > >         select DTOC
> > >
> > > +config BINMAN_DTB
> > > +       bool
> > > +       depends on BINMAN
> > > +       default y if OF_PRIOR_STAGE
> > > +       help
> > > +         This option tells U-Boot build system that a device tree blob is
> > > +         explicitly required when using binman to package U-Boot.
> > > +
> > > +         This is not necessary in a common scenario where a device tree blob
> > > +         is provided in the arch/<arch>/dts directory for a specific board,
> > > +         where the binman node has been put in the board's device tree, aka
> > > +         an implicit way. However for a scenario like the board's device tree
> > > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > > +         is passed by a prior stage bootloader. For such scenario, we will
> > > +         need to provide a device tree blob containing binman node to describe
> > > +         how to package U-Boot.
> >
> > I don't fully understand the effect of this. Is binman (at build time)
> > using a binary DT provided by another build system? Or is it not
> > available at build time and binman cannot run?
>
> binman uses a DT provided by U-Boot, but that DT is only for binman if
> OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
> there.
>
> You can check patch 10 for how it works.

OK I think I get it.

>
> >
> > The name is too vanilla I think. It should indicate that this is an
> > unusual situation.
>
> I cannot think of a good name :(

How about BINMAN_FAKE_FDT ?

Why do you need to turn off BINMAN_FDT?

Regards,
Simon

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Bin Meng]

Hi Simon,

On Sun, May 9, 2021 at 12:48 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > > in the U-Boot build phase hence the binman node information is not
> > > > available. In order to support such use case, a new Kconfig option
> > > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > > tree blob is explicitly required when using binman to package U-Boot.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > > >
> > > >  Makefile    |  3 ++-
> > > >  dts/Kconfig | 18 ++++++++++++++++++
> > > >  lib/Kconfig |  2 +-
> > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 3d88559b67..9043828675 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -918,6 +918,7 @@ endif
> > > >  endif
> > > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > > >  endif
> > > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > > >  u-boot.bin: u-boot-dtb.bin FORCE
> > > >         $(call if_changed,copy)
> > > >  else
> > > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
>
> What is this change for, specifically? Can you add a comment?
>
> It seems to be overriding the qemu thing of not actually building a DT?
>
> > > >         $(call if_changed,copy)
> > > >  endif
> > > >
> > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > index 99ce75e1a2..84d43fc6a4 100644
> > > > --- a/dts/Kconfig
> > > > +++ b/dts/Kconfig
> > > > @@ -19,6 +19,24 @@ config BINMAN
> > > >         bool
> > > >         select DTOC
> > > >
> > > > +config BINMAN_DTB
> > > > +       bool
> > > > +       depends on BINMAN
> > > > +       default y if OF_PRIOR_STAGE
> > > > +       help
> > > > +         This option tells U-Boot build system that a device tree blob is
> > > > +         explicitly required when using binman to package U-Boot.
> > > > +
> > > > +         This is not necessary in a common scenario where a device tree blob
> > > > +         is provided in the arch/<arch>/dts directory for a specific board,
> > > > +         where the binman node has been put in the board's device tree, aka
> > > > +         an implicit way. However for a scenario like the board's device tree
> > > > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > > > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > > > +         is passed by a prior stage bootloader. For such scenario, we will
> > > > +         need to provide a device tree blob containing binman node to describe
> > > > +         how to package U-Boot.
> > >
> > > I don't fully understand the effect of this. Is binman (at build time)
> > > using a binary DT provided by another build system? Or is it not
> > > available at build time and binman cannot run?
> >
> > binman uses a DT provided by U-Boot, but that DT is only for binman if
> > OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
> > there.
> >
> > You can check patch 10 for how it works.
>
> OK I think I get it.
>
> >
> > >
> > > The name is too vanilla I think. It should indicate that this is an
> > > unusual situation.
> >
> > I cannot think of a good name :(
>
> How about BINMAN_FAKE_FDT ?

Better than mine :)

If FAKE confuses people, maybe BINMAN_STANDALONE_FDT, or BINMAN_PACKAGE_FDT ?

>
> Why do you need to turn off BINMAN_FDT?

Because binman_init() fails on locating the image node in the device
tree, where the binman node does not exist at all. The DT used only
exists in the U-Boot build phase.

So here is what we have for different OF_CONTROL methods:

OF_SEPARATE / OF_EMBEDED: binman node is available in the DT (exists
along with other device nodes)
OF_PRIOR_STAGE: binman node is not available in the DT (DT only
contains the device nodes)

Regards,
Bin

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Bin Meng]

On Sun, May 9, 2021 at 12:48 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > > in the U-Boot build phase hence the binman node information is not
> > > > available. In order to support such use case, a new Kconfig option
> > > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > > tree blob is explicitly required when using binman to package U-Boot.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > > >
> > > >  Makefile    |  3 ++-
> > > >  dts/Kconfig | 18 ++++++++++++++++++
> > > >  lib/Kconfig |  2 +-
> > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 3d88559b67..9043828675 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -918,6 +918,7 @@ endif
> > > >  endif
> > > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > > >  endif
> > > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > > >  u-boot.bin: u-boot-dtb.bin FORCE
> > > >         $(call if_changed,copy)
> > > >  else
> > > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
>
> What is this change for, specifically? Can you add a comment?
>

Oops, I missed this comment. Sure.

> It seems to be overriding the qemu thing of not actually building a DT?

This is to make sure the dependency is met (ie: the *faked* DT
containing binman node gets built first) otherwise it may fail in a
parallel build.

Regards,
Bin

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 10:07, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, May 9, 2021 at 12:48 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > > > in the U-Boot build phase hence the binman node information is not
> > > > > available. In order to support such use case, a new Kconfig option
> > > > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > > > tree blob is explicitly required when using binman to package U-Boot.
> > > > >
> > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > > > >
> > > > >  Makefile    |  3 ++-
> > > > >  dts/Kconfig | 18 ++++++++++++++++++
> > > > >  lib/Kconfig |  2 +-
> > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 3d88559b67..9043828675 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -918,6 +918,7 @@ endif
> > > > >  endif
> > > > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > > > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > > > >  endif
> > > > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > > > >  u-boot.bin: u-boot-dtb.bin FORCE
> > > > >         $(call if_changed,copy)
> > > > >  else
> > > > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
> >
> > What is this change for, specifically? Can you add a comment?
> >
> > It seems to be overriding the qemu thing of not actually building a DT?
> >
> > > > >         $(call if_changed,copy)
> > > > >  endif
> > > > >
> > > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > > index 99ce75e1a2..84d43fc6a4 100644
> > > > > --- a/dts/Kconfig
> > > > > +++ b/dts/Kconfig
> > > > > @@ -19,6 +19,24 @@ config BINMAN
> > > > >         bool
> > > > >         select DTOC
> > > > >
> > > > > +config BINMAN_DTB
> > > > > +       bool
> > > > > +       depends on BINMAN
> > > > > +       default y if OF_PRIOR_STAGE
> > > > > +       help
> > > > > +         This option tells U-Boot build system that a device tree blob is
> > > > > +         explicitly required when using binman to package U-Boot.
> > > > > +
> > > > > +         This is not necessary in a common scenario where a device tree blob
> > > > > +         is provided in the arch/<arch>/dts directory for a specific board,
> > > > > +         where the binman node has been put in the board's device tree, aka
> > > > > +         an implicit way. However for a scenario like the board's device tree
> > > > > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > > > > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > > > > +         is passed by a prior stage bootloader. For such scenario, we will
> > > > > +         need to provide a device tree blob containing binman node to describe
> > > > > +         how to package U-Boot.
> > > >
> > > > I don't fully understand the effect of this. Is binman (at build time)
> > > > using a binary DT provided by another build system? Or is it not
> > > > available at build time and binman cannot run?
> > >
> > > binman uses a DT provided by U-Boot, but that DT is only for binman if
> > > OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
> > > there.
> > >
> > > You can check patch 10 for how it works.
> >
> > OK I think I get it.
> >
> > >
> > > >
> > > > The name is too vanilla I think. It should indicate that this is an
> > > > unusual situation.
> > >
> > > I cannot think of a good name :(
> >
> > How about BINMAN_FAKE_FDT ?
>
> Better than mine :)
>
> If FAKE confuses people, maybe BINMAN_STANDALONE_FDT, or BINMAN_PACKAGE_FDT ?

BINMAN_STANDALONE_FDT seems better actually

>
> >
> > Why do you need to turn off BINMAN_FDT?
>
> Because binman_init() fails on locating the image node in the device
> tree, where the binman node does not exist at all. The DT used only
> exists in the U-Boot build phase.

Oh yes I see. So confusing. Can you add some docs to binman about this?

>
> So here is what we have for different OF_CONTROL methods:
>
> OF_SEPARATE / OF_EMBEDED: binman node is available in the DT (exists

OF_EMBED

> along with other device nodes)
> OF_PRIOR_STAGE: binman node is not available in the DT (DT only
> contains the device nodes)

OK yes.

Regards,
Simon

[PATCH v2 08/11] binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE

[Bin Meng]

Hi Simon,

On Sun, May 9, 2021 at 1:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Sat, 8 May 2021 at 10:07, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sun, May 9, 2021 at 12:48 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Sat, 8 May 2021 at 08:55, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sat, May 8, 2021 at 11:47 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > For scenarios like OF_PRIOR_STAGE, no device tree blob is provided
> > > > > > in the U-Boot build phase hence the binman node information is not
> > > > > > available. In order to support such use case, a new Kconfig option
> > > > > > BINMAN_DTB is introduced, to tell U-Boot build system that a device
> > > > > > tree blob is explicitly required when using binman to package U-Boot.
> > > > > >
> > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Changes in v2:
> > > > > > - new patch: "binman: Support packaging U-Boot for scenarios like OF_PRIOR_STAGE"
> > > > > >
> > > > > >  Makefile    |  3 ++-
> > > > > >  dts/Kconfig | 18 ++++++++++++++++++
> > > > > >  lib/Kconfig |  2 +-
> > > > > >  3 files changed, 21 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index 3d88559b67..9043828675 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -918,6 +918,7 @@ endif
> > > > > >  endif
> > > > > >  INPUTS-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
> > > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot.dtb
> > > > > > +INPUTS-$(CONFIG_BINMAN_DTB) += u-boot.dtb
> > > > > >  ifeq ($(CONFIG_SPL_FRAMEWORK),y)
> > > > > >  INPUTS-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img
> > > > > >  endif
> > > > > > @@ -1161,7 +1162,7 @@ u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE
> > > > > >  u-boot.bin: u-boot-dtb.bin FORCE
> > > > > >         $(call if_changed,copy)
> > > > > >  else
> > > > > > -u-boot.bin: u-boot-nodtb.bin FORCE
> > > > > > +u-boot.bin: u-boot-nodtb.bin $(if $(CONFIG_BINMAN_DTB),dts/dt.dtb) FORCE
> > >
> > > What is this change for, specifically? Can you add a comment?
> > >
> > > It seems to be overriding the qemu thing of not actually building a DT?
> > >
> > > > > >         $(call if_changed,copy)
> > > > > >  endif
> > > > > >
> > > > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > > > index 99ce75e1a2..84d43fc6a4 100644
> > > > > > --- a/dts/Kconfig
> > > > > > +++ b/dts/Kconfig
> > > > > > @@ -19,6 +19,24 @@ config BINMAN
> > > > > >         bool
> > > > > >         select DTOC
> > > > > >
> > > > > > +config BINMAN_DTB
> > > > > > +       bool
> > > > > > +       depends on BINMAN
> > > > > > +       default y if OF_PRIOR_STAGE
> > > > > > +       help
> > > > > > +         This option tells U-Boot build system that a device tree blob is
> > > > > > +         explicitly required when using binman to package U-Boot.
> > > > > > +
> > > > > > +         This is not necessary in a common scenario where a device tree blob
> > > > > > +         is provided in the arch/<arch>/dts directory for a specific board,
> > > > > > +         where the binman node has been put in the board's device tree, aka
> > > > > > +         an implicit way. However for a scenario like the board's device tree
> > > > > > +         is not provided in the U-Boot source tree, but given to U-Boot in the
> > > > > > +         runtime, e.g.: in the OF_PRIOR_STAGE case that the device tree blob
> > > > > > +         is passed by a prior stage bootloader. For such scenario, we will
> > > > > > +         need to provide a device tree blob containing binman node to describe
> > > > > > +         how to package U-Boot.
> > > > >
> > > > > I don't fully understand the effect of this. Is binman (at build time)
> > > > > using a binary DT provided by another build system? Or is it not
> > > > > available at build time and binman cannot run?
> > > >
> > > > binman uses a DT provided by U-Boot, but that DT is only for binman if
> > > > OF_PRIOR_STAGE. For OF_SEPARATE / EMBEDDED, we can include binman node
> > > > there.
> > > >
> > > > You can check patch 10 for how it works.
> > >
> > > OK I think I get it.
> > >
> > > >
> > > > >
> > > > > The name is too vanilla I think. It should indicate that this is an
> > > > > unusual situation.
> > > >
> > > > I cannot think of a good name :(
> > >
> > > How about BINMAN_FAKE_FDT ?
> >
> > Better than mine :)
> >
> > If FAKE confuses people, maybe BINMAN_STANDALONE_FDT, or BINMAN_PACKAGE_FDT ?
>
> BINMAN_STANDALONE_FDT seems better actually

Great :)

>
> >
> > >
> > > Why do you need to turn off BINMAN_FDT?
> >
> > Because binman_init() fails on locating the image node in the device
> > tree, where the binman node does not exist at all. The DT used only
> > exists in the U-Boot build phase.
>
> Oh yes I see. So confusing. Can you add some docs to binman about this?

Sure will update the doc on this.

>
> >
> > So here is what we have for different OF_CONTROL methods:
> >
> > OF_SEPARATE / OF_EMBEDED: binman node is available in the DT (exists
>
> OF_EMBED
>
> > along with other device nodes)
> > OF_PRIOR_STAGE: binman node is not available in the DT (DT only
> > contains the device nodes)
>
> OK yes.

Regards,
Bin

[PATCH v2 10/11] riscv: qemu: Switch to use binman to generate u-boot.itb

[Simon Glass]

Hi Bin,

On Sat, 8 May 2021 at 09:00, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sat, May 8, 2021 at 11:49 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Sat, 8 May 2021 at 08:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > By utilizing the newly introduced BINMAN_DTB option, along with a
> > > new dedicated device tree source file for the QEMU virt target used
> > > for binman only, we can now use binman to generate u-boot.itb.
> > >
> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - new patch: "riscv: qemu: Switch to use binman to generate u-boot.itb"
> > >
> > >  arch/riscv/cpu/generic/Kconfig     | 1 +
> > >  arch/riscv/dts/Makefile            | 1 +
> > >  arch/riscv/dts/binman.dtsi         | 8 ++++++++
> > >  arch/riscv/dts/qemu-virt.dts       | 8 ++++++++
> > >  configs/qemu-riscv32_spl_defconfig | 2 ++
> > >  configs/qemu-riscv64_spl_defconfig | 2 ++
> > >  6 files changed, 22 insertions(+)
> > >  create mode 100644 arch/riscv/dts/qemu-virt.dts
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Do you think it would be better to have qemu use a completely
> > different DT? Do 'normal' boards define OF_PRIOR_STAGE?
>
> I am not sure what do you mean by using a completely different DT for
> QEMU. The device nodes are provided by QEMU and passed to U-Boot, via
> OF_PRIOR_STAGE.
>
> I would not expect "normal" boards to define OF_PRIOR_STAGE, but looks
> there are still some. A grep in the U-Boot source shows the following
> boards support OF_PRIOR_STAGE except QEMU riscv:
>
> ./configs/bcm7260_defconfig:25:CONFIG_OF_PRIOR_STAGE=y
> ./configs/bcm7445_defconfig:26:CONFIG_OF_PRIOR_STAGE=y
> ./configs/ae350_rv64_spl_defconfig:21:CONFIG_OF_PRIOR_STAGE=y
> ./configs/ae350_rv32_spl_defconfig:20:CONFIG_OF_PRIOR_STAGE=y
> ./configs/ae350_rv64_defconfig:19:CONFIG_OF_PRIOR_STAGE=y
> ./configs/ae350_rv32_defconfig:18:CONFIG_OF_PRIOR_STAGE=y
>
> config ARCH_BCMSTB
>         bool "Broadcom BCM7XXX family"
>         select CPU_V7A
>         select DM
>         select OF_CONTROL
>         select OF_PRIOR_STAGE
>         imply CMD_DM

OK I see. It is very strange, but it seems that U-Boot is somewhere in
the middle of the boot, not at the start, on these devices.

Regards,
Simon