[PATCH 0/3] rockchip: Use external TPL binary to create a working firmware image

[PATCH 0/3] rockchip: Use external TPL binary to create a working firmware image

[Jonas Karlman]

Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
back to boot-rom to load the next stage of the boot flow, U-Boot SPL.

For RK356x there is currently no support to initialize DRAM using U-Boot
TPL and instead an external TPL binary must be used to generate a
working u-boot-rockchip.bin image.

This adds a new generic external-tpl entry to binman and make use of
this new entry in rockchip-u-boot.dtsi.

Please note that the allow-missing flag and the added missing-msg entry
does not work as expected becuase the wrapping mkimage entry used
requires that the files to all child entries exists.
Instead without a provided EXTERNAL_TPL the build fails with:

  ValueError: Filename 'ddr.bin' not found in input path (...)

originating from

  Entry_mkimage.ObtainContents:
    fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))

Not sure how to properly add support for allow-missing flag to mkimage
entry, possible something for another series?

Regards,
Jonas

Jonas Karlman (3):
  binman: Add support for an external-tpl entry
  rockchip: Require an external TPL binary when TPL is missing
  Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"

 Makefile                               |  1 +
 arch/arm/dts/rockchip-u-boot.dtsi      | 16 ++++++++++++----
 configs/evb-rk3568_defconfig           |  1 -
 tools/binman/entries.rst               | 12 ++++++++++++
 tools/binman/etype/external_tpl.py     | 18 ++++++++++++++++++
 tools/binman/ftest.py                  |  7 +++++++
 tools/binman/missing-blob-help         |  5 +++++
 tools/binman/test/277_external_tpl.dts | 16 ++++++++++++++++
 8 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 tools/binman/etype/external_tpl.py
 create mode 100644 tools/binman/test/277_external_tpl.dts

-- 
2.39.1

[PATCH 1/3] binman: Add support for an external-tpl entry

[Jonas Karlman]

The external-tpl entry can be used when an external TPL binary must be
used instead of normal U-Boot TPL.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 tools/binman/entries.rst               | 12 ++++++++++++
 tools/binman/etype/external_tpl.py     | 18 ++++++++++++++++++
 tools/binman/ftest.py                  |  7 +++++++
 tools/binman/test/277_external_tpl.dts | 16 ++++++++++++++++
 4 files changed, 53 insertions(+)
 create mode 100644 tools/binman/etype/external_tpl.py
 create mode 100644 tools/binman/test/277_external_tpl.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 7a04a613992d..95317271bb74 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -468,6 +468,18 @@ updating the EC on startup via software sync.
 
 
 
+.. _etype_external_tpl:
+
+Entry: external-tpl: External TPL binary
+----------------------------------------
+
+Properties / Entry arguments:
+    - external-tpl-path: Filename of file to read into the entry.
+
+This entry holds an external TPL binary.
+
+
+
 .. _etype_fdtmap:
 
 Entry: fdtmap: An entry which contains an FDT map
diff --git a/tools/binman/etype/external_tpl.py b/tools/binman/etype/external_tpl.py
new file mode 100644
index 000000000000..50562914711e
--- /dev/null
+++ b/tools/binman/etype/external_tpl.py
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Entry-type module for external TPL binary
+#
+
+from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
+
+class Entry_external_tpl(Entry_blob_named_by_arg):
+    """External TPL binary
+
+    Properties / Entry arguments:
+        - external-tpl-path: Filename of file to read into the entry.
+
+    This entry holds an external TPL binary.
+    """
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node, 'external-tpl')
+        self.external = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6b203dfb644f..2446122eea29 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
 ATF_BL2U_DATA         = b'bl2u'
 OPENSBI_DATA          = b'opensbi'
 SCP_DATA              = b'scp'
+EXTERNAL_TPL_DATA     = b'external-tpl'
 TEST_FDT1_DATA        = b'fdt1'
 TEST_FDT2_DATA        = b'test-fdt2'
 ENV_DATA              = b'var1=1\nvar2="2"'
@@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
         TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
         TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
+        TestFunctional._MakeInputFile('external-tpl.bin', EXTERNAL_TPL_DATA)
 
         # Add a few .dtb files for testing
         TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
@@ -4103,6 +4105,11 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('172_scp.dts')
         self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
 
+    def testPackExternalTpl(self):
+        """Test that an image with an external TPL binary can be created"""
+        data = self._DoReadFile('277_external_tpl.dts')
+        self.assertEqual(EXTERNAL_TPL_DATA, data[:len(EXTERNAL_TPL_DATA)])
+
     def testFitFdt(self):
         """Test an image with an FIT with multiple FDT images"""
         def _CheckFdt(seq, expected_data):
diff --git a/tools/binman/test/277_external_tpl.dts b/tools/binman/test/277_external_tpl.dts
new file mode 100644
index 000000000000..99899771ea7c
--- /dev/null
+++ b/tools/binman/test/277_external_tpl.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <16>;
+
+		external-tpl {
+			filename = "external-tpl.bin";
+		};
+	};
+};
-- 
2.39.1

[PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Jonas Karlman]

Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
back to boot-rom to load the next stage of the boot flow, U-Boot SPL.

For RK356x there is currently no support to initialize DRAM using U-Boot
TPL and instead an external TPL binary must be used to generate a
working u-boot-rockchip.bin image.

Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
external TPL binary must be provided to generate a working firmware.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 Makefile                          |  1 +
 arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
 tools/binman/missing-blob-help    |  5 +++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 7eaf45496c1c..7e9272be937f 100644
--- a/Makefile
+++ b/Makefile
@@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 		-a opensbi-path=${OPENSBI} \
 		-a default-dt=$(default_dt) \
 		-a scp-path=$(SCP) \
+		-a external-tpl-path=$(EXTERNAL_TPL) \
 		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
 		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
 		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
index 6c662a72d4f9..bc3bc9bc3e37 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -20,12 +20,16 @@
 		mkimage {
 			filename = "idbloader.img";
 			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
-#ifdef CONFIG_TPL
 			multiple-data-files;
 
+#ifdef CONFIG_TPL
 			u-boot-tpl {
-			};
+#else
+			external-tpl {
+				filename = "ddr.bin";
+				missing-msg = "external-tpl-rockchip";
 #endif
+			};
 			u-boot-spl {
 			};
 		};
@@ -134,12 +138,16 @@
 		mkimage {
 			filename = "idbloader-spi.img";
 			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
-#ifdef CONFIG_TPL
 			multiple-data-files;
 
+#ifdef CONFIG_TPL
 			u-boot-tpl {
-			};
+#else
+			external-tpl {
+				filename = "ddr.bin";
+				missing-msg = "external-tpl-rockchip";
 #endif
+			};
 			u-boot-spl {
 			};
 		};
diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
index c61ca02a35ee..e850824032dd 100644
--- a/tools/binman/missing-blob-help
+++ b/tools/binman/missing-blob-help
@@ -14,6 +14,11 @@ atf-bl31-sunxi:
 Please read the section on ARM Trusted Firmware (ATF) in
 board/sunxi/README.sunxi64
 
+external-tpl-rockchip:
+External TPL is required to initialize DRAM. Get external TPL binary and
+build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
+external TPL binary is https://github.com/rockchip-linux/rkbin.
+
 scp-sunxi:
 SCP firmware is required for system suspend, but is otherwise optional.
 Please read the section on SCP firmware in board/sunxi/README.sunxi64
-- 
2.39.1

[PATCH 3/3] Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"

[Jonas Karlman]

This reverts commit 31500e7bcfaca08ab7c2879f502a6cf852410244.

[why]
External TPL binary is now expected to be provided using EXTERNAL_TPL
when building RK356x targets.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 configs/evb-rk3568_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
index a76d924d3872..7374ee42fb19 100644
--- a/configs/evb-rk3568_defconfig
+++ b/configs/evb-rk3568_defconfig
@@ -65,5 +65,4 @@ CONFIG_BAUDRATE=1500000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550_MEM32=y
 CONFIG_SYSRESET=y
-# CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
-- 
2.39.1

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Jagan Teki]

On Mon, Feb 6, 2023 at 1:52 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>
> For RK356x there is currently no support to initialize DRAM using U-Boot
> TPL and instead an external TPL binary must be used to generate a
> working u-boot-rockchip.bin image.
>
> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
> external TPL binary must be provided to generate a working firmware.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  Makefile                          |  1 +
>  arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>  tools/binman/missing-blob-help    |  5 +++++
>  3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 7eaf45496c1c..7e9272be937f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                 -a opensbi-path=${OPENSBI} \
>                 -a default-dt=$(default_dt) \
>                 -a scp-path=$(SCP) \
> +               -a external-tpl-path=$(EXTERNAL_TPL) \
>                 -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 6c662a72d4f9..bc3bc9bc3e37 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -20,12 +20,16 @@
>                 mkimage {
>                         filename = "idbloader.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_TPL
>                         u-boot-tpl {
> -                       };
> +#else
> +                       external-tpl {
> +                               filename = "ddr.bin";
> +                               missing-msg = "external-tpl-rockchip";
>  #endif
> +                       };
>                         u-boot-spl {
>                         };
>                 };
> @@ -134,12 +138,16 @@
>                 mkimage {
>                         filename = "idbloader-spi.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_TPL
>                         u-boot-tpl {
> -                       };
> +#else
> +                       external-tpl {
> +                               filename = "ddr.bin";
> +                               missing-msg = "external-tpl-rockchip";
>  #endif
> +                       };
>                         u-boot-spl {
>                         };
>                 };
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> index c61ca02a35ee..e850824032dd 100644
> --- a/tools/binman/missing-blob-help
> +++ b/tools/binman/missing-blob-help
> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>  Please read the section on ARM Trusted Firmware (ATF) in
>  board/sunxi/README.sunxi64
>
> +external-tpl-rockchip:
> +External TPL is required to initialize DRAM. Get external TPL binary and
> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for

Look like this requires DDR bin renaming every time, Is there any
possibility to use a direct ddr bin name instead of ddr.bin as we did
for BL31 elf?
$ export EXTERNAL_TPL=rk3588_ddr_lp4_2112MHz_lp5_2736MHz_v1.08.bin

Jagan.

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Jonas Karlman]

Hi Jagan,

On 2023-02-05 21:28, Jagan Teki wrote:
> On Mon, Feb 6, 2023 at 1:52 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>
>> For RK356x there is currently no support to initialize DRAM using U-Boot
>> TPL and instead an external TPL binary must be used to generate a
>> working u-boot-rockchip.bin image.
>>
>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>> external TPL binary must be provided to generate a working firmware.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>  Makefile                          |  1 +
>>  arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>  tools/binman/missing-blob-help    |  5 +++++
>>  3 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 7eaf45496c1c..7e9272be937f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>>                 -a opensbi-path=${OPENSBI} \
>>                 -a default-dt=$(default_dt) \
>>                 -a scp-path=$(SCP) \
>> +               -a external-tpl-path=$(EXTERNAL_TPL) \
>>                 -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>> @@ -20,12 +20,16 @@
>>                 mkimage {
>>                         filename = "idbloader.img";
>>                         args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>> -#ifdef CONFIG_TPL
>>                         multiple-data-files;
>>
>> +#ifdef CONFIG_TPL
>>                         u-boot-tpl {
>> -                       };
>> +#else
>> +                       external-tpl {
>> +                               filename = "ddr.bin";
>> +                               missing-msg = "external-tpl-rockchip";
>>  #endif
>> +                       };
>>                         u-boot-spl {
>>                         };
>>                 };
>> @@ -134,12 +138,16 @@
>>                 mkimage {
>>                         filename = "idbloader-spi.img";
>>                         args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
>> -#ifdef CONFIG_TPL
>>                         multiple-data-files;
>>
>> +#ifdef CONFIG_TPL
>>                         u-boot-tpl {
>> -                       };
>> +#else
>> +                       external-tpl {
>> +                               filename = "ddr.bin";
>> +                               missing-msg = "external-tpl-rockchip";
>>  #endif
>> +                       };
>>                         u-boot-spl {
>>                         };
>>                 };
>> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
>> index c61ca02a35ee..e850824032dd 100644
>> --- a/tools/binman/missing-blob-help
>> +++ b/tools/binman/missing-blob-help
>> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>>  Please read the section on ARM Trusted Firmware (ATF) in
>>  board/sunxi/README.sunxi64
>>
>> +external-tpl-rockchip:
>> +External TPL is required to initialize DRAM. Get external TPL binary and
>> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
> 
> Look like this requires DDR bin renaming every time, Is there any
> possibility to use a direct ddr bin name instead of ddr.bin as we did
> for BL31 elf?
> $ export EXTERNAL_TPL=rk3588_ddr_lp4_2112MHz_lp5_2736MHz_v1.08.bin

By default it will look for a ddr.bin in the binman include folders, or
it should be possible to use export EXTERNAL_TPL= in the same way you
can use export BL31=.

Regards,
Jonas

> 
> Jagan.

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Quentin Schulz]

Hi Jonas,

On 2/5/23 21:21, Jonas Karlman wrote:
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
> 
> For RK356x there is currently no support to initialize DRAM using U-Boot
> TPL and instead an external TPL binary must be used to generate a
> working u-boot-rockchip.bin image.
> 
> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
> external TPL binary must be provided to generate a working firmware.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   Makefile                          |  1 +
>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>   tools/binman/missing-blob-help    |  5 +++++
>   3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 7eaf45496c1c..7e9272be937f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>   		-a opensbi-path=${OPENSBI} \
>   		-a default-dt=$(default_dt) \
>   		-a scp-path=$(SCP) \
> +		-a external-tpl-path=$(EXTERNAL_TPL) \
>   		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>   		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>   		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 6c662a72d4f9..bc3bc9bc3e37 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -20,12 +20,16 @@
>   		mkimage {
>   			filename = "idbloader.img";
>   			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
>   			multiple-data-files;
>   
> +#ifdef CONFIG_TPL
>   			u-boot-tpl {
> -			};
> +#else
> +			external-tpl {
> +				filename = "ddr.bin";
> +				missing-msg = "external-tpl-rockchip";
>   #endif
> +			};

NACK. This forces the use of a TPL (either built by U-Boot or external) 
which is not always the case. There are still boards without a TPL which 
work perfectly fine (at least I would hope so :) ).

Basically there are three possible cases:
SPL
TPL+SPL
TPL(external blob)+SPL

Here you remove the ability to have SPL only.

Hence why I suggested we add a new Kconfig option for EXTERNAL_TPL, not 
for the path but to explicit this third possibility.

Cheers,
Quentin

>   			u-boot-spl {
>   			};
>   		};
> @@ -134,12 +138,16 @@
>   		mkimage {
>   			filename = "idbloader-spi.img";
>   			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
>   			multiple-data-files;
>   
> +#ifdef CONFIG_TPL
>   			u-boot-tpl {
> -			};
> +#else
> +			external-tpl {
> +				filename = "ddr.bin";
> +				missing-msg = "external-tpl-rockchip";
>   #endif
> +			};
>   			u-boot-spl {
>   			};
>   		};
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> index c61ca02a35ee..e850824032dd 100644
> --- a/tools/binman/missing-blob-help
> +++ b/tools/binman/missing-blob-help
> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>   Please read the section on ARM Trusted Firmware (ATF) in
>   board/sunxi/README.sunxi64
>   
> +external-tpl-rockchip:
> +External TPL is required to initialize DRAM. Get external TPL binary and
> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
> +external TPL binary is https://urldefense.com/v3/__https://github.com/rockchip-linux/rkbin__;!!OOPJP91ZZw!jRwonQRHKoxzegx3S3cRYfalkhW1ESLyBCTmVc2c6fnmPaQBOZyxG2I7phwM3pEZxR2QIHQG8Hw3JStyx4tDcMsalwYDCg$ .
> +
>   scp-sunxi:
>   SCP firmware is required for system suspend, but is otherwise optional.
>   Please read the section on SCP firmware in board/sunxi/README.sunxi64

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Jonas Karlman]

Hi Quentin,
On 2023-02-06 12:26, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 2/5/23 21:21, Jonas Karlman wrote:
>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>
>> For RK356x there is currently no support to initialize DRAM using U-Boot
>> TPL and instead an external TPL binary must be used to generate a
>> working u-boot-rockchip.bin image.
>>
>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>> external TPL binary must be provided to generate a working firmware.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>   Makefile                          |  1 +
>>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>   tools/binman/missing-blob-help    |  5 +++++
>>   3 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 7eaf45496c1c..7e9272be937f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>>   		-a opensbi-path=${OPENSBI} \
>>   		-a default-dt=$(default_dt) \
>>   		-a scp-path=$(SCP) \
>> +		-a external-tpl-path=$(EXTERNAL_TPL) \
>>   		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>   		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>   		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>> @@ -20,12 +20,16 @@
>>   		mkimage {
>>   			filename = "idbloader.img";
>>   			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>> -#ifdef CONFIG_TPL
>>   			multiple-data-files;
>>   
>> +#ifdef CONFIG_TPL
>>   			u-boot-tpl {
>> -			};
>> +#else
>> +			external-tpl {
>> +				filename = "ddr.bin";
>> +				missing-msg = "external-tpl-rockchip";
>>   #endif
>> +			};
> 
> NACK. This forces the use of a TPL (either built by U-Boot or external) 
> which is not always the case. There are still boards without a TPL which 
> work perfectly fine (at least I would hope so :) ).
> 
> Basically there are three possible cases:
> SPL
> TPL+SPL
> TPL(external blob)+SPL
> 
> Here you remove the ability to have SPL only.
> 
> Hence why I suggested we add a new Kconfig option for EXTERNAL_TPL, not 
> for the path but to explicit this third possibility.

Thanks for the feedback, I will add a Kconfig option to make this explicit.
We could also add the optional prop in the external-tpl node to only include
the external TPL when it is provided. This will require also fixing the
missing/optional handling for the mkimage entry.

Regards,
Jonas

> 
> Cheers,
> Quentin
> 
>>   			u-boot-spl {
>>   			};
>>   		};
>> @@ -134,12 +138,16 @@
>>   		mkimage {
>>   			filename = "idbloader-spi.img";
>>   			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
>> -#ifdef CONFIG_TPL
>>   			multiple-data-files;
>>   
>> +#ifdef CONFIG_TPL
>>   			u-boot-tpl {
>> -			};
>> +#else
>> +			external-tpl {
>> +				filename = "ddr.bin";
>> +				missing-msg = "external-tpl-rockchip";
>>   #endif
>> +			};
>>   			u-boot-spl {
>>   			};
>>   		};
>> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
>> index c61ca02a35ee..e850824032dd 100644
>> --- a/tools/binman/missing-blob-help
>> +++ b/tools/binman/missing-blob-help
>> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>>   Please read the section on ARM Trusted Firmware (ATF) in
>>   board/sunxi/README.sunxi64
>>   
>> +external-tpl-rockchip:
>> +External TPL is required to initialize DRAM. Get external TPL binary and
>> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
>> +external TPL binary is https://urldefense.com/v3/__https://github.com/rockchip-linux/rkbin__;!!OOPJP91ZZw!jRwonQRHKoxzegx3S3cRYfalkhW1ESLyBCTmVc2c6fnmPaQBOZyxG2I7phwM3pEZxR2QIHQG8Hw3JStyx4tDcMsalwYDCg$ .
>> +
>>   scp-sunxi:
>>   SCP firmware is required for system suspend, but is otherwise optional.
>>   Please read the section on SCP firmware in board/sunxi/README.sunxi64

Re: [PATCH 0/3] rockchip: Use external TPL binary to create a working firmware image

[Simon Glass]

Hi Jonas,

On Sun, 5 Feb 2023 at 13:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>
> For RK356x there is currently no support to initialize DRAM using U-Boot
> TPL and instead an external TPL binary must be used to generate a
> working u-boot-rockchip.bin image.

Who is working on this suppose? Having a binary blob is a pain.

>
> This adds a new generic external-tpl entry to binman and make use of
> this new entry in rockchip-u-boot.dtsi.
>
> Please note that the allow-missing flag and the added missing-msg entry
> does not work as expected becuase the wrapping mkimage entry used
> requires that the files to all child entries exists.
> Instead without a provided EXTERNAL_TPL the build fails with:
>
>   ValueError: Filename 'ddr.bin' not found in input path (...)
>
> originating from
>
>   Entry_mkimage.ObtainContents:
>     fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
>
> Not sure how to properly add support for allow-missing flag to mkimage
> entry, possible something for another series?

If it's an input file, then Bincan supports creating a fake external
blob, which should already be handled in mkimage.py

But if I misunderstand, or there is a bug, let me know.

Eegards,
SImon



>
> Regards,
> Jonas
>
> Jonas Karlman (3):
>   binman: Add support for an external-tpl entry
>   rockchip: Require an external TPL binary when TPL is missing
>   Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"
>
>  Makefile                               |  1 +
>  arch/arm/dts/rockchip-u-boot.dtsi      | 16 ++++++++++++----
>  configs/evb-rk3568_defconfig           |  1 -
>  tools/binman/entries.rst               | 12 ++++++++++++
>  tools/binman/etype/external_tpl.py     | 18 ++++++++++++++++++
>  tools/binman/ftest.py                  |  7 +++++++
>  tools/binman/missing-blob-help         |  5 +++++
>  tools/binman/test/277_external_tpl.dts | 16 ++++++++++++++++
>  8 files changed, 71 insertions(+), 5 deletions(-)
>  create mode 100644 tools/binman/etype/external_tpl.py
>  create mode 100644 tools/binman/test/277_external_tpl.dts
>
> --
> 2.39.1
>

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Simon Glass]

Hi Jonas,

On Sun, 5 Feb 2023 at 13:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>
> For RK356x there is currently no support to initialize DRAM using U-Boot
> TPL and instead an external TPL binary must be used to generate a
> working u-boot-rockchip.bin image.
>
> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
> external TPL binary must be provided to generate a working firmware.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  Makefile                          |  1 +
>  arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>  tools/binman/missing-blob-help    |  5 +++++
>  3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 7eaf45496c1c..7e9272be937f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                 -a opensbi-path=${OPENSBI} \
>                 -a default-dt=$(default_dt) \
>                 -a scp-path=$(SCP) \
> +               -a external-tpl-path=$(EXTERNAL_TPL) \

rockchip-tpl-path

>                 -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 6c662a72d4f9..bc3bc9bc3e37 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -20,12 +20,16 @@
>                 mkimage {
>                         filename = "idbloader.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_TPL

As you said, this needs an explicit config

>                         u-boot-tpl {
> -                       };
> +#else
> +                       external-tpl {
> +                               filename = "ddr.bin";
> +                               missing-msg = "external-tpl-rockchip";
>  #endif
> +                       };
>                         u-boot-spl {
>                         };
>                 };
> @@ -134,12 +138,16 @@
>                 mkimage {
>                         filename = "idbloader-spi.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_TPL
>                         u-boot-tpl {
> -                       };
> +#else
> +                       external-tpl {
> +                               filename = "ddr.bin";
> +                               missing-msg = "external-tpl-rockchip";
>  #endif
> +                       };
>                         u-boot-spl {
>                         };
>                 };
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> index c61ca02a35ee..e850824032dd 100644
> --- a/tools/binman/missing-blob-help
> +++ b/tools/binman/missing-blob-help
> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>  Please read the section on ARM Trusted Firmware (ATF) in
>  board/sunxi/README.sunxi64
>
> +external-tpl-rockchip:
> +External TPL is required to initialize DRAM. Get external TPL binary and
> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
> +external TPL binary is https://github.com/rockchip-linux/rkbin.
> +
>  scp-sunxi:
>  SCP firmware is required for system suspend, but is otherwise optional.
>  Please read the section on SCP firmware in board/sunxi/README.sunxi64
> --
> 2.39.1
>

Regards,
Simon

Re: [PATCH 1/3] binman: Add support for an external-tpl entry

[Simon Glass]

Hi Jonas,

On Sun, 5 Feb 2023 at 13:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> The external-tpl entry can be used when an external TPL binary must be
> used instead of normal U-Boot TPL.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  tools/binman/entries.rst               | 12 ++++++++++++
>  tools/binman/etype/external_tpl.py     | 18 ++++++++++++++++++
>  tools/binman/ftest.py                  |  7 +++++++
>  tools/binman/test/277_external_tpl.dts | 16 ++++++++++++++++
>  4 files changed, 53 insertions(+)
>  create mode 100644 tools/binman/etype/external_tpl.py
>  create mode 100644 tools/binman/test/277_external_tpl.dts
>
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 7a04a613992d..95317271bb74 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -468,6 +468,18 @@ updating the EC on startup via software sync.
>
>
>
> +.. _etype_external_tpl:
> +
> +Entry: external-tpl: External TPL binary
> +----------------------------------------
> +
> +Properties / Entry arguments:
> +    - external-tpl-path: Filename of file to read into the entry.
> +
> +This entry holds an external TPL binary.

Could you make this specific to Rockchip? I think something like
rockchip_tpl would work. We try to keep things specific to a vendor.

> +
> +
> +
>  .. _etype_fdtmap:
>
>  Entry: fdtmap: An entry which contains an FDT map
> diff --git a/tools/binman/etype/external_tpl.py b/tools/binman/etype/external_tpl.py
> new file mode 100644
> index 000000000000..50562914711e
> --- /dev/null
> +++ b/tools/binman/etype/external_tpl.py
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Entry-type module for external TPL binary
> +#
> +
> +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
> +
> +class Entry_external_tpl(Entry_blob_named_by_arg):
> +    """External TPL binary
> +
> +    Properties / Entry arguments:
> +        - external-tpl-path: Filename of file to read into the entry.
> +
> +    This entry holds an external TPL binary.
> +    """
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node, 'external-tpl')
> +        self.external = True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 6b203dfb644f..2446122eea29 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
>  ATF_BL2U_DATA         = b'bl2u'
>  OPENSBI_DATA          = b'opensbi'
>  SCP_DATA              = b'scp'
> +EXTERNAL_TPL_DATA     = b'external-tpl'
>  TEST_FDT1_DATA        = b'fdt1'
>  TEST_FDT2_DATA        = b'test-fdt2'
>  ENV_DATA              = b'var1=1\nvar2="2"'
> @@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
>          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
> +        TestFunctional._MakeInputFile('external-tpl.bin', EXTERNAL_TPL_DATA)

rockchip-tpl

>
>          # Add a few .dtb files for testing
>          TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
> @@ -4103,6 +4105,11 @@ class TestFunctional(unittest.TestCase):
>          data = self._DoReadFile('172_scp.dts')
>          self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
>
> +    def testPackExternalTpl(self):
> +        """Test that an image with an external TPL binary can be created"""
> +        data = self._DoReadFile('277_external_tpl.dts')
> +        self.assertEqual(EXTERNAL_TPL_DATA, data[:len(EXTERNAL_TPL_DATA)])
> +
>      def testFitFdt(self):
>          """Test an image with an FIT with multiple FDT images"""
>          def _CheckFdt(seq, expected_data):
> diff --git a/tools/binman/test/277_external_tpl.dts b/tools/binman/test/277_external_tpl.dts
> new file mode 100644
> index 000000000000..99899771ea7c
> --- /dev/null
> +++ b/tools/binman/test/277_external_tpl.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               size = <16>;
> +
> +               external-tpl {
> +                       filename = "external-tpl.bin";
> +               };
> +       };
> +};
> --
> 2.39.1

Regards,
SImon

Re: [PATCH 3/3] Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"

[Simon Glass]

On Sun, 5 Feb 2023 at 13:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> This reverts commit 31500e7bcfaca08ab7c2879f502a6cf852410244.
>
> [why]
> External TPL binary is now expected to be provided using EXTERNAL_TPL
> when building RK356x targets.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  configs/evb-rk3568_defconfig | 1 -
>  1 file changed, 1 deletion(-)

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

Re: [PATCH 0/3] rockchip: Use external TPL binary to create a working firmware image

[Jonas Karlman]

Hi Simon,

On 2023-02-07 05:02, Simon Glass wrote:
> Hi Jonas,
> 
> On Sun, 5 Feb 2023 at 13:21, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>
>> For RK356x there is currently no support to initialize DRAM using U-Boot
>> TPL and instead an external TPL binary must be used to generate a
>> working u-boot-rockchip.bin image.
> 
> Who is working on this suppose? Having a binary blob is a pain.

Not sure if anyone is working on this, but I am hoping that someday
someone will work on this :-)

> 
>>
>> This adds a new generic external-tpl entry to binman and make use of
>> this new entry in rockchip-u-boot.dtsi.
>>
>> Please note that the allow-missing flag and the added missing-msg entry
>> does not work as expected becuase the wrapping mkimage entry used
>> requires that the files to all child entries exists.
>> Instead without a provided EXTERNAL_TPL the build fails with:
>>
>>   ValueError: Filename 'ddr.bin' not found in input path (...)
>>
>> originating from
>>
>>   Entry_mkimage.ObtainContents:
>>     fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
>>
>> Not sure how to properly add support for allow-missing flag to mkimage
>> entry, possible something for another series?
> 
> If it's an input file, then Bincan supports creating a fake external
> blob, which should already be handled in mkimage.py
> 
> But if I misunderstand, or there is a bug, let me know.

I think there may be a bug, mkimage with multiple-data-files does not
handle missing/optional external blobs in a way that I was expecting.
Will take a closer look for v2, or at least create a testcase to
reproduce such issue.

Will also rename to rockchip-tpl and address rest of your comments in v2.

Regards,
Jonas

> 
> Eegards,
> SImon
> 
> 
> 
>>
>> Regards,
>> Jonas
>>
>> Jonas Karlman (3):
>>   binman: Add support for an external-tpl entry
>>   rockchip: Require an external TPL binary when TPL is missing
>>   Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"
>>
>>  Makefile                               |  1 +
>>  arch/arm/dts/rockchip-u-boot.dtsi      | 16 ++++++++++++----
>>  configs/evb-rk3568_defconfig           |  1 -
>>  tools/binman/entries.rst               | 12 ++++++++++++
>>  tools/binman/etype/external_tpl.py     | 18 ++++++++++++++++++
>>  tools/binman/ftest.py                  |  7 +++++++
>>  tools/binman/missing-blob-help         |  5 +++++
>>  tools/binman/test/277_external_tpl.dts | 16 ++++++++++++++++
>>  8 files changed, 71 insertions(+), 5 deletions(-)
>>  create mode 100644 tools/binman/etype/external_tpl.py
>>  create mode 100644 tools/binman/test/277_external_tpl.dts
>>
>> --
>> 2.39.1
>>

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Kever Yang]

Hi Quentin,

On 2023/2/6 19:26, Quentin Schulz wrote:
> Hi Jonas,
>
> On 2/5/23 21:21, Jonas Karlman wrote:
>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>
>> For RK356x there is currently no support to initialize DRAM using U-Boot
>> TPL and instead an external TPL binary must be used to generate a
>> working u-boot-rockchip.bin image.
>>
>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>> external TPL binary must be provided to generate a working firmware.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>   Makefile                          |  1 +
>>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>   tools/binman/missing-blob-help    |  5 +++++
>>   3 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 7eaf45496c1c..7e9272be937f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman 
>> $(if $(BINMAN_DEBUG),-D) \
>>           -a opensbi-path=${OPENSBI} \
>>           -a default-dt=$(default_dt) \
>>           -a scp-path=$(SCP) \
>> +        -a external-tpl-path=$(EXTERNAL_TPL) \
>>           -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>           -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>           -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi 
>> b/arch/arm/dts/rockchip-u-boot.dtsi
>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>> @@ -20,12 +20,16 @@
>>           mkimage {
>>               filename = "idbloader.img";
>>               args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>> -#ifdef CONFIG_TPL
>>               multiple-data-files;
>>   +#ifdef CONFIG_TPL
>>               u-boot-tpl {
>> -            };
>> +#else
>> +            external-tpl {
>> +                filename = "ddr.bin";
>> +                missing-msg = "external-tpl-rockchip";
>>   #endif
>> +            };
>
> NACK. This forces the use of a TPL (either built by U-Boot or 
> external) which is not always the case. There are still boards without 
> a TPL which work perfectly fine (at least I would hope so :) ).
>
> Basically there are three possible cases:
> SPL
> TPL+SPL
> TPL(external blob)+SPL

I'm afraid all the boards support by mainline U-Boot is using 
"TPL(U-Boot or external)+SPL+U-Boot" mode, and this mode also always 
used by rockchip vendor branch.

For "SPL+U-Boot" use case, it only happen many years ago in some of 
rk3288 board and rk3399 board,

the SPL will need to support both sdram driver and storage(SPI, SDCard, 
eMMC) driver and without back to BootRom,

and other function may add to SPL, but the sram size limit is always 
there, so all the rk3288 and rk3399 board have migrate to use 
"TPL+SPL+U-Boot.itb" mode.

The "TPL+SPL+U-Boot.itb" is much clear and easy to maintine for all the 
boards and will be the only accept mode for new board support in the 
future on rockchip platform:

- TPL for dram init;

- SPL for storage init and load next stage firmware, decode FIT image 
with ATF/OPTEE support, secure boot support and etc;

- U-Boot.itb including ATF and U-Boot proper, maybe also OPTEE.

This model can very easy to do the debug with replace the binary from 
rockchip  vendor tree during board bringup.

The SPL+U-Boot mode can only happen for legacy board with only need 
U-Boot raw image instead of itb which including trust support,

this kind of board can get the image with only one mkimage command, I 
don't think it will need binman support because it only have one image.


Thanks,

- Kever

>
> Here you remove the ability to have SPL only.
>
> Hence why I suggested we add a new Kconfig option for EXTERNAL_TPL, 
> not for the path but to explicit this third possibility.
>
> Cheers,
> Quentin
>
>>               u-boot-spl {
>>               };
>>           };
>> @@ -134,12 +138,16 @@
>>           mkimage {
>>               filename = "idbloader-spi.img";
>>               args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
>> -#ifdef CONFIG_TPL
>>               multiple-data-files;
>>   +#ifdef CONFIG_TPL
>>               u-boot-tpl {
>> -            };
>> +#else
>> +            external-tpl {
>> +                filename = "ddr.bin";
>> +                missing-msg = "external-tpl-rockchip";
>>   #endif
>> +            };
>>               u-boot-spl {
>>               };
>>           };
>> diff --git a/tools/binman/missing-blob-help 
>> b/tools/binman/missing-blob-help
>> index c61ca02a35ee..e850824032dd 100644
>> --- a/tools/binman/missing-blob-help
>> +++ b/tools/binman/missing-blob-help
>> @@ -14,6 +14,11 @@ atf-bl31-sunxi:
>>   Please read the section on ARM Trusted Firmware (ATF) in
>>   board/sunxi/README.sunxi64
>>   +external-tpl-rockchip:
>> +External TPL is required to initialize DRAM. Get external TPL binary 
>> and
>> +build with EXTERNAL_TPL=/path/to/ddr.bin. One possible source for
>> +external TPL binary is 
>> https://urldefense.com/v3/__https://github.com/rockchip-linux/rkbin__;!!OOPJP91ZZw!jRwonQRHKoxzegx3S3cRYfalkhW1ESLyBCTmVc2c6fnmPaQBOZyxG2I7phwM3pEZxR2QIHQG8Hw3JStyx4tDcMsalwYDCg$ 
>> .
>> +
>>   scp-sunxi:
>>   SCP firmware is required for system suspend, but is otherwise 
>> optional.
>>   Please read the section on SCP firmware in board/sunxi/README.sunxi64

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Quentin Schulz]

Hi Kever,

On 2/8/23 16:41, Kever Yang wrote:
> Hi Quentin,
> 
> On 2023/2/6 19:26, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 2/5/23 21:21, Jonas Karlman wrote:
>>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>>
>>> For RK356x there is currently no support to initialize DRAM using U-Boot
>>> TPL and instead an external TPL binary must be used to generate a
>>> working u-boot-rockchip.bin image.
>>>
>>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>>> external TPL binary must be provided to generate a working firmware.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>>   Makefile                          |  1 +
>>>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>>   tools/binman/missing-blob-help    |  5 +++++
>>>   3 files changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 7eaf45496c1c..7e9272be937f 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman 
>>> $(if $(BINMAN_DEBUG),-D) \
>>>           -a opensbi-path=${OPENSBI} \
>>>           -a default-dt=$(default_dt) \
>>>           -a scp-path=$(SCP) \
>>> +        -a external-tpl-path=$(EXTERNAL_TPL) \
>>>           -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>>           -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>>           -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi 
>>> b/arch/arm/dts/rockchip-u-boot.dtsi
>>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>> @@ -20,12 +20,16 @@
>>>           mkimage {
>>>               filename = "idbloader.img";
>>>               args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>>> -#ifdef CONFIG_TPL
>>>               multiple-data-files;
>>>   +#ifdef CONFIG_TPL
>>>               u-boot-tpl {
>>> -            };
>>> +#else
>>> +            external-tpl {
>>> +                filename = "ddr.bin";
>>> +                missing-msg = "external-tpl-rockchip";
>>>   #endif
>>> +            };
>>
>> NACK. This forces the use of a TPL (either built by U-Boot or 
>> external) which is not always the case. There are still boards without 
>> a TPL which work perfectly fine (at least I would hope so :) ).
>>
>> Basically there are three possible cases:
>> SPL
>> TPL+SPL
>> TPL(external blob)+SPL
> 
> I'm afraid all the boards support by mainline U-Boot is using 
> "TPL(U-Boot or external)+SPL+U-Boot" mode, and this mode also always 
> used by rockchip vendor branch.
> 

That seems to be incorrect.

for conf in $(git grep -l ARCH_ROCKCHIP configs); do
     make $(basename $conf) > /dev/null 2>&1
     if ! grep -q CONFIG_TPL=y .config; then
         echo $conf does not enable TPL;
     fi
done

returns:
configs/chromebit_mickey_defconfig does not enable TPL
configs/chromebook_bob_defconfig does not enable TPL
configs/chromebook_jerry_defconfig does not enable TPL
configs/chromebook_kevin_defconfig does not enable TPL
configs/chromebook_minnie_defconfig does not enable TPL
configs/chromebook_speedy_defconfig does not enable TPL
configs/elgin-rv1108_defconfig does not enable TPL
configs/evb-rk3036_defconfig does not enable TPL
configs/evb-rk3128_defconfig does not enable TPL
configs/evb-rk3308_defconfig does not enable TPL
configs/evb-rk3568_defconfig does not enable TPL
configs/evb-rv1108_defconfig does not enable TPL
configs/ficus-rk3399_defconfig does not enable TPL
configs/geekbox_defconfig does not enable TPL
configs/kylin-rk3036_defconfig does not enable TPL
configs/miqi-rk3288_defconfig does not enable TPL
configs/phycore-rk3288_defconfig does not enable TPL
configs/popmetal-rk3288_defconfig does not enable TPL
configs/roc-cc-rk3308_defconfig does not enable TPL
configs/rock2_defconfig does not enable TPL
configs/rock_defconfig does not enable TPL
configs/sheep-rk3368_defconfig does not enable TPL

is there something I'm missing?

> For "SPL+U-Boot" use case, it only happen many years ago in some of 
> rk3288 board and rk3399 board,
> 
> the SPL will need to support both sdram driver and storage(SPI, SDCard, 
> eMMC) driver and without back to BootRom,
> 
> and other function may add to SPL, but the sram size limit is always 
> there, so all the rk3288 and rk3399 board have migrate to use 
> "TPL+SPL+U-Boot.itb" mode.
> 

It seems not all have migrated.

> The "TPL+SPL+U-Boot.itb" is much clear and easy to maintine for all the 
> boards and will be the only accept mode for new board support in the 
> future on rockchip platform:
> 
> - TPL for dram init;
> 
> - SPL for storage init and load next stage firmware, decode FIT image 
> with ATF/OPTEE support, secure boot support and etc;
> 
> - U-Boot.itb including ATF and U-Boot proper, maybe also OPTEE.
> 
> This model can very easy to do the debug with replace the binary from 
> rockchip  vendor tree during board bringup.
> 

That's fine by me, but we currently have Rockchip boards in mainline 
which do NOT have TPL enabled, so we need to handle those properly, in 
binman since it's now the only way to generate the images.

> The SPL+U-Boot mode can only happen for legacy board with only need 
> U-Boot raw image instead of itb which including trust support,
> 

What makes SPL+U-Boot not capable of using U-Boot.itb or support 
secure-boot/trust?

> this kind of board can get the image with only one mkimage command, I 
> don't think it will need binman support because it only have one image.
> 

I suggest we leave it as it is today, handling TPL+SPL+U-Boot AND 
SPL+U-Boot, or we "upgrade" current boards which do not have TPL support 
enabled yet so we only need to have TPL+SPL+U-Boot to support and 
maintenance is easier in the long term (for the latter case, you can 
then enforce it by having ARCH_ROCKCHIP imply TPL for example).

Cheers,
Quentin

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Kever Yang]

On 2023/2/9 00:06, Quentin Schulz wrote:
> Hi Kever,
>
> On 2/8/23 16:41, Kever Yang wrote:
>> Hi Quentin,
>>
>> On 2023/2/6 19:26, Quentin Schulz wrote:
>>> Hi Jonas,
>>>
>>> On 2/5/23 21:21, Jonas Karlman wrote:
>>>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>>>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>>>
>>>> For RK356x there is currently no support to initialize DRAM using 
>>>> U-Boot
>>>> TPL and instead an external TPL binary must be used to generate a
>>>> working u-boot-rockchip.bin image.
>>>>
>>>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>>>> external TPL binary must be provided to generate a working firmware.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>>   Makefile                          |  1 +
>>>>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>>>   tools/binman/missing-blob-help    |  5 +++++
>>>>   3 files changed, 18 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 7eaf45496c1c..7e9272be937f 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman 
>>>> $(if $(BINMAN_DEBUG),-D) \
>>>>           -a opensbi-path=${OPENSBI} \
>>>>           -a default-dt=$(default_dt) \
>>>>           -a scp-path=$(SCP) \
>>>> +        -a external-tpl-path=$(EXTERNAL_TPL) \
>>>>           -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>>>           -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>>>           -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi 
>>>> b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> @@ -20,12 +20,16 @@
>>>>           mkimage {
>>>>               filename = "idbloader.img";
>>>>               args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>>>> -#ifdef CONFIG_TPL
>>>>               multiple-data-files;
>>>>   +#ifdef CONFIG_TPL
>>>>               u-boot-tpl {
>>>> -            };
>>>> +#else
>>>> +            external-tpl {
>>>> +                filename = "ddr.bin";
>>>> +                missing-msg = "external-tpl-rockchip";
>>>>   #endif
>>>> +            };
>>>
>>> NACK. This forces the use of a TPL (either built by U-Boot or 
>>> external) which is not always the case. There are still boards 
>>> without a TPL which work perfectly fine (at least I would hope so :) ).
>>>
>>> Basically there are three possible cases:
>>> SPL
>>> TPL+SPL
>>> TPL(external blob)+SPL
>>
>> I'm afraid all the boards support by mainline U-Boot is using 
>> "TPL(U-Boot or external)+SPL+U-Boot" mode, and this mode also always 
>> used by rockchip vendor branch.
>>
>
> That seems to be incorrect.
>
> for conf in $(git grep -l ARCH_ROCKCHIP configs); do
>     make $(basename $conf) > /dev/null 2>&1
>     if ! grep -q CONFIG_TPL=y .config; then
>         echo $conf does not enable TPL;
>     fi
> done
>
> returns:
> configs/chromebit_mickey_defconfig does not enable TPL
> configs/chromebook_bob_defconfig does not enable TPL
> configs/chromebook_jerry_defconfig does not enable TPL
> configs/chromebook_kevin_defconfig does not enable TPL
> configs/chromebook_minnie_defconfig does not enable TPL
> configs/chromebook_speedy_defconfig does not enable TPL
> configs/elgin-rv1108_defconfig does not enable TPL
> configs/evb-rk3036_defconfig does not enable TPL
> configs/evb-rk3128_defconfig does not enable TPL
> configs/evb-rk3308_defconfig does not enable TPL
> configs/evb-rk3568_defconfig does not enable TPL
> configs/evb-rv1108_defconfig does not enable TPL
> configs/ficus-rk3399_defconfig does not enable TPL
> configs/geekbox_defconfig does not enable TPL
> configs/kylin-rk3036_defconfig does not enable TPL
> configs/miqi-rk3288_defconfig does not enable TPL
> configs/phycore-rk3288_defconfig does not enable TPL
> configs/popmetal-rk3288_defconfig does not enable TPL
> configs/roc-cc-rk3308_defconfig does not enable TPL
> configs/rock2_defconfig does not enable TPL
> configs/rock_defconfig does not enable TPL
> configs/sheep-rk3368_defconfig does not enable TPL
>
> is there something I'm missing?

Most of them use simple SPL+U-Boot(for 32bit processer) or have external 
TPL.

We can keep an option to support SPL only case for now.

>
>> For "SPL+U-Boot" use case, it only happen many years ago in some of 
>> rk3288 board and rk3399 board,
>>
>> the SPL will need to support both sdram driver and storage(SPI, 
>> SDCard, eMMC) driver and without back to BootRom,
>>
>> and other function may add to SPL, but the sram size limit is always 
>> there, so all the rk3288 and rk3399 board have migrate to use 
>> "TPL+SPL+U-Boot.itb" mode.
>>
>
> It seems not all have migrated.
>
>> The "TPL+SPL+U-Boot.itb" is much clear and easy to maintine for all 
>> the boards and will be the only accept mode for new board support in 
>> the future on rockchip platform:
>>
>> - TPL for dram init;
>>
>> - SPL for storage init and load next stage firmware, decode FIT image 
>> with ATF/OPTEE support, secure boot support and etc;
>>
>> - U-Boot.itb including ATF and U-Boot proper, maybe also OPTEE.
>>
>> This model can very easy to do the debug with replace the binary from 
>> rockchip  vendor tree during board bringup.
>>
>
> That's fine by me, but we currently have Rockchip boards in mainline 
> which do NOT have TPL enabled, so we need to handle those properly, in 
> binman since it's now the only way to generate the images.
>
>> The SPL+U-Boot mode can only happen for legacy board with only need 
>> U-Boot raw image instead of itb which including trust support,
>>
>
> What makes SPL+U-Boot not capable of using U-Boot.itb or support 
> secure-boot/trust?

It's possible to use SPL+U-Boot.itb, but it's not easy to maintain, 
because this model need to make

all the driver available(FDT support, SDMMC/EMMC driver, FIT decode and 
etc)in the SPL, the SPL

running in SRAM which has limit size, often cause size overflow issue.

Since  BACK_TO_BOOTROM feature is available for all the rockchip SoCs, 
we can us TPL+SPL instead

for much flexible solution.


Thanks,
- Kever
>
>> this kind of board can get the image with only one mkimage command, I 
>> don't think it will need binman support because it only have one image.
>>
>
> I suggest we leave it as it is today, handling TPL+SPL+U-Boot AND 
> SPL+U-Boot, or we "upgrade" current boards which do not have TPL 
> support enabled yet so we only need to have TPL+SPL+U-Boot to support 
> and maintenance is easier in the long term (for the latter case, you 
> can then enforce it by having ARCH_ROCKCHIP imply TPL for example).
>
> Cheers,
> Quentin

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Kever Yang]

Hi Jonas,

On 2023/2/6 20:51, Jonas Karlman wrote:
> Hi Quentin,
> On 2023-02-06 12:26, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 2/5/23 21:21, Jonas Karlman wrote:
>>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>>
>>> For RK356x there is currently no support to initialize DRAM using U-Boot
>>> TPL and instead an external TPL binary must be used to generate a
>>> working u-boot-rockchip.bin image.
>>>
>>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>>> external TPL binary must be provided to generate a working firmware.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>>    Makefile                          |  1 +
>>>    arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>>    tools/binman/missing-blob-help    |  5 +++++
>>>    3 files changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 7eaf45496c1c..7e9272be937f 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>>>    		-a opensbi-path=${OPENSBI} \
>>>    		-a default-dt=$(default_dt) \
>>>    		-a scp-path=$(SCP) \
>>> +		-a external-tpl-path=$(EXTERNAL_TPL) \
>>>    		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>>    		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>>    		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>> @@ -20,12 +20,16 @@
>>>    		mkimage {
>>>    			filename = "idbloader.img";
>>>    			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>>> -#ifdef CONFIG_TPL
>>>    			multiple-data-files;
>>>    
>>> +#ifdef CONFIG_TPL
>>>    			u-boot-tpl {
>>> -			};
>>> +#else
>>> +			external-tpl {
>>> +				filename = "ddr.bin";
>>> +				missing-msg = "external-tpl-rockchip";
>>>    #endif
>>> +			};
>> NACK. This forces the use of a TPL (either built by U-Boot or external)
>> which is not always the case. There are still boards without a TPL which
>> work perfectly fine (at least I would hope so :) ).
>>
>> Basically there are three possible cases:
>> SPL
>> TPL+SPL
>> TPL(external blob)+SPL
>>
>> Here you remove the ability to have SPL only.
>>
>> Hence why I suggested we add a new Kconfig option for EXTERNAL_TPL, not
>> for the path but to explicit this third possibility.
> Thanks for the feedback, I will add a Kconfig option to make this explicit.
> We could also add the optional prop in the external-tpl node to only include
> the external TPL when it is provided. This will require also fixing the
> missing/optional handling for the mkimage entry.

Could you help to send the new patchset with this update?


Thanks,

- Kever

[PATCH v2 0/5] rockchip: Use external TPL binary to create a working firmware image

[Jonas Karlman]

Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
jumps to U-Boot SPL to continue the boot flow.

For RK356x there is no support to initialize DRAM using U-Boot
TPL and instead an external TPL binary must be used to generate a
bootable u-boot-rockchip.bin image.

This adds a new rockchip-tpl entry to binman and make use of this new
entry in rockchip-u-boot.dtsi.

Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
bootable u-boot-rockchip.bin image for RK356x.

I have included 3 new patches in v2:

First one to sync init size limit in mkimage with vendor u-boot and
SRAM size defined in TRM. Fixes running mkimage with latest vendor TPL
for RK3328.

Second one updates defconfig for evb-rk3568 with new options.

Last patch is an RFC patch to help mitigate an unexpected behavior with
using BINMAN_ALLOW_MISSING=1 and not supplying a value to the
rockchip-tpl-path option. See patch for more details.

Changes in v2:
- Renamed external-tpl to rockchip-tpl
- Renamed EXTERNAL_TPL to ROCKCHIP_TPL
- Add CONFIG_ROCKCHIP_EXTERNAL_TPL Kconfig option
- New patch to sync init size limit in mkimage
- New RFC patch to improve allow-missing/fake-ext-blobs handling for
  binman mkimage entry

Jonas Karlman (6):
  binman: Add support for a rockchip-tpl entry
  rockchip: Use an external TPL binary on RK3568
  Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"
  rockchip: mkimage: Update init size limit
  rockchip: evb-rk3568: Update defconfig
  RFC: binman: Improve allow missing for mkimage entry

 Makefile                               |  1 +
 arch/arm/dts/rockchip-u-boot.dtsi      | 10 +++++--
 arch/arm/mach-rockchip/Kconfig         |  4 +++
 configs/evb-rk3568_defconfig           | 14 +++++-----
 tools/binman/entries.rst               | 14 ++++++++++
 tools/binman/etype/mkimage.py          | 37 +++++++++++++++++++++++---
 tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++
 tools/binman/ftest.py                  |  7 +++++
 tools/binman/missing-blob-help         |  5 ++++
 tools/binman/test/277_rockchip_tpl.dts | 16 +++++++++++
 tools/rkcommon.c                       | 16 +++++------
 11 files changed, 123 insertions(+), 21 deletions(-)
 create mode 100644 tools/binman/etype/rockchip_tpl.py
 create mode 100644 tools/binman/test/277_rockchip_tpl.dts

-- 
2.39.1

[PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Jonas Karlman]

The rockchip-tpl entry can be used when an external TPL binary should be
used instead of the normal U-Boot TPL.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- rename external-tpl to rockchip-tpl
- missing message moved to this patch

 tools/binman/entries.rst               | 14 ++++++++++++++
 tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
 tools/binman/ftest.py                  |  7 +++++++
 tools/binman/missing-blob-help         |  5 +++++
 tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
 5 files changed, 62 insertions(+)
 create mode 100644 tools/binman/etype/rockchip_tpl.py
 create mode 100644 tools/binman/test/277_rockchip_tpl.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 7a04a613992d..e177860a6a82 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1386,6 +1386,20 @@ For example, this creates an image with a pre-load header and a binary::
 
 
 
+.. _etype_rockchip_tpl:
+
+Entry: rockchip-tpl: Rockchip TPL binary
+----------------------------------------
+
+Properties / Entry arguments:
+    - rockchip-tpl-path: Filename of file to read into the entry,
+                         typically <soc>_ddr_<version>.bin
+
+This entry holds an external TPL binary used by some Rockchip SoCs
+instead of normal U-Boot TPL, typically to initialize DRAM.
+
+
+
 .. _etype_scp:
 
 Entry: scp: System Control Processor (SCP) firmware blob
diff --git a/tools/binman/etype/rockchip_tpl.py b/tools/binman/etype/rockchip_tpl.py
new file mode 100644
index 000000000000..74f58ba8570c
--- /dev/null
+++ b/tools/binman/etype/rockchip_tpl.py
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Entry-type module for Rockchip TPL binary
+#
+
+from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
+
+class Entry_rockchip_tpl(Entry_blob_named_by_arg):
+    """Rockchip TPL binary
+
+    Properties / Entry arguments:
+        - rockchip-tpl-path: Filename of file to read into the entry,
+                             typically <soc>_ddr_<version>.bin
+
+    This entry holds an external TPL binary used by some Rockchip SoCs
+    instead of normal U-Boot TPL, typically to initialize DRAM.
+    """
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node, 'rockchip-tpl')
+        self.external = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 062f54adb0ed..ed4b5c987557 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
 ATF_BL2U_DATA         = b'bl2u'
 OPENSBI_DATA          = b'opensbi'
 SCP_DATA              = b'scp'
+ROCKCHIP_TPL_DATA     = b'rockchip-tpl'
 TEST_FDT1_DATA        = b'fdt1'
 TEST_FDT2_DATA        = b'test-fdt2'
 ENV_DATA              = b'var1=1\nvar2="2"'
@@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
         TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
         TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
+        TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA)
 
         # Add a few .dtb files for testing
         TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
@@ -4097,6 +4099,11 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('172_scp.dts')
         self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
 
+    def testPackRockchipTpl(self):
+        """Test that an image with a Rockchip TPL binary can be created"""
+        data = self._DoReadFile('277_rockchip_tpl.dts')
+        self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
+
     def testFitFdt(self):
         """Test an image with an FIT with multiple FDT images"""
         def _CheckFdt(seq, expected_data):
diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
index c61ca02a35ee..e8c991206fe5 100644
--- a/tools/binman/missing-blob-help
+++ b/tools/binman/missing-blob-help
@@ -34,6 +34,11 @@ If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for
 the R5F core(s) to trigger the system reset. One possible source is
 https://github.com/siemens/k3-rti-wdt.
 
+rockchip-tpl:
+An external TPL is required to initialize DRAM. Get the external TPL
+binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
+for the external TPL binary is https://github.com/rockchip-linux/rkbin.
+
 tee-os:
 See the documentation for your board. You may need to build Open Portable
 Trusted Execution Environment (OP-TEE) with TEE=/path/to/tee.bin
diff --git a/tools/binman/test/277_rockchip_tpl.dts b/tools/binman/test/277_rockchip_tpl.dts
new file mode 100644
index 000000000000..269f56e2545c
--- /dev/null
+++ b/tools/binman/test/277_rockchip_tpl.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <16>;
+
+		rockchip-tpl {
+			filename = "rockchip-tpl.bin";
+		};
+	};
+};
-- 
2.39.1

[PATCH v2 2/6] rockchip: Use an external TPL binary on RK3568

[Jonas Karlman]

Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
jumps to U-Boot SPL to continue the normal boot flow.

However, there is no support to initialize DRAM on RK35xx SoCs using
U-Boot TPL and instead an external TPL binary must be used to generate a
bootable u-boot-rockchip.bin image.

Add CONFIG_ROCKCHIP_EXTERNAL_TPL to indicate that an external TPL should
be used. Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
bootable u-boot-rockchip.bin image for RK3568.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- rename external-tpl-path to rockchip-tpl-path
- rename EXTERNAL_TPL to ROCKCHIP_TPL
- add CONFIG_ROCKCHIP_EXTERNAL_TPL option

 Makefile                          |  1 +
 arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++--
 arch/arm/mach-rockchip/Kconfig    |  4 ++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 54f894dab841..58f8c7a35335 100644
--- a/Makefile
+++ b/Makefile
@@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 		-a opensbi-path=${OPENSBI} \
 		-a default-dt=$(default_dt) \
 		-a scp-path=$(SCP) \
+		-a rockchip-tpl-path=$(ROCKCHIP_TPL) \
 		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
 		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
 		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
index 6c662a72d4f9..2878b80926c4 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -20,9 +20,12 @@
 		mkimage {
 			filename = "idbloader.img";
 			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
-#ifdef CONFIG_TPL
 			multiple-data-files;
 
+#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
+			rockchip-tpl {
+			};
+#elif defined(CONFIG_TPL)
 			u-boot-tpl {
 			};
 #endif
@@ -134,9 +137,12 @@
 		mkimage {
 			filename = "idbloader-spi.img";
 			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
-#ifdef CONFIG_TPL
 			multiple-data-files;
 
+#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
+			rockchip-tpl {
+			};
+#elif defined(CONFIG_TPL)
 			u-boot-tpl {
 			};
 #endif
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index b678ec41318e..4a5415403446 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -377,6 +377,10 @@ config TPL_ROCKCHIP_BACK_TO_BROM
           SPL will return to the boot rom, which will then load the U-Boot
           binary to keep going on.
 
+config ROCKCHIP_EXTERNAL_TPL
+	bool "Use external TPL binary"
+	default y if ROCKCHIP_RK3568
+
 config ROCKCHIP_COMMON_BOARD
 	bool "Rockchip common board file"
 	help
-- 
2.39.1

[PATCH v2 3/6] Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"

[Jonas Karlman]

An external TPL binary is now expected to be provided using ROCKCHIP_TPL
when building RK3568 targets.

This reverts commit 31500e7bcfaca08ab7c2879f502a6cf852410244.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v2:
- Collect r-b tag

 configs/evb-rk3568_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
index a76d924d3872..7374ee42fb19 100644
--- a/configs/evb-rk3568_defconfig
+++ b/configs/evb-rk3568_defconfig
@@ -65,5 +65,4 @@ CONFIG_BAUDRATE=1500000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550_MEM32=y
 CONFIG_SYSRESET=y
-# CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
-- 
2.39.1

[PATCH v2 4/6] rockchip: mkimage: Update init size limit

[Jonas Karlman]

Sync init size limit from vendor u-boot and the SRAM size specified in a
SoCs TRM. Size is typically defined using the following expression:
<SRAM size> - <BootRom stack size>

This makes it possible to use latest vendor TPL with RK3328 without
getting a size limit error running the mkimage command.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- new patch

 tools/rkcommon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/rkcommon.c b/tools/rkcommon.c
index 1f1eaa16752b..7ad4780daf31 100644
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -121,20 +121,20 @@ struct spl_info {
 };
 
 static struct spl_info spl_infos[] = {
-	{ "px30", "RK33", 0x2800, false, RK_HEADER_V1 },
-	{ "rk3036", "RK30", 0x1000, false, RK_HEADER_V1 },
+	{ "px30", "RK33", 0x4000 - 0x1000, false, RK_HEADER_V1 },
+	{ "rk3036", "RK30", 0x2000 - 0x1000, false, RK_HEADER_V1 },
 	{ "rk3066", "RK30", 0x8000 - 0x800, true, RK_HEADER_V1 },
-	{ "rk3128", "RK31", 0x1800, false, RK_HEADER_V1 },
+	{ "rk3128", "RK31", 0x2000 - 0x800, false, RK_HEADER_V1 },
 	{ "rk3188", "RK31", 0x8000 - 0x800, true, RK_HEADER_V1 },
 	{ "rk322x", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
-	{ "rk3288", "RK32", 0x8000, false, RK_HEADER_V1 },
+	{ "rk3288", "RK32", 0x18000 - 0x1000, false, RK_HEADER_V1 },
 	{ "rk3308", "RK33", 0x40000 - 0x1000, false, RK_HEADER_V1 },
-	{ "rk3328", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
-	{ "rk3368", "RK33", 0x8000 - 0x1000, false, RK_HEADER_V1 },
+	{ "rk3328", "RK32", 0x8000 - 0x800, false, RK_HEADER_V1 },
+	{ "rk3368", "RK33", 0x10000 - 0x1000, false, RK_HEADER_V1 },
 	{ "rk3399", "RK33", 0x30000 - 0x2000, false, RK_HEADER_V1 },
-	{ "rv1108", "RK11", 0x1800, false, RK_HEADER_V1 },
+	{ "rv1108", "RK11", 0x2000 - 0x800, false, RK_HEADER_V1 },
 	{ "rv1126", "110B", 0x10000 - 0x1000, false, RK_HEADER_V1 },
-	{ "rk3568", "RK35", 0x14000 - 0x1000, false, RK_HEADER_V2 },
+	{ "rk3568", "RK35", 0x10000 - 0x1000, false, RK_HEADER_V2 },
 };
 
 /**
-- 
2.39.1

[PATCH v2 5/6] rockchip: evb-rk3568: Update defconfig

[Jonas Karlman]

Update defconfig for evb-rk3568 with new defaults.

Remove the SPL_ROCKCHIP_BACK_TO_BROM=y option, SPL is expected to load
next stage from a FIT image and then jump to next stage not back to
BootRom.

Extend SPL_MAX_SIZE to 0x40000, SPL is loaded to 0x0 and TF-A is loaded
to 0x40000, use the space in between as SPL_MAX_SIZE.

Add CONFIG_SPL_FIT_SIGNATURE=y to let SPL verify an auto generated hash
of FIT images. This help indicate if there is an issue loading any of
the images to memory.

Add CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y to prevent vendor TF-A from
crashing in some cases when fdt_addr is provided as platform param.

Filter out assigned-clock props with CONFIG_OF_SPL_REMOVE_PROPS,
U-Boot proper will read and configure assigned-clock props.

Resync with savedefconfig.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- new patch

 configs/evb-rk3568_defconfig | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
index 7374ee42fb19..e5b69b50b7ab 100644
--- a/configs/evb-rk3568_defconfig
+++ b/configs/evb-rk3568_defconfig
@@ -6,42 +6,43 @@ CONFIG_TEXT_BASE=0x00a00000
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
 CONFIG_DEFAULT_DEVICE_TREE="rk3568-evb"
 CONFIG_ROCKCHIP_RK3568=y
-CONFIG_SPL_ROCKCHIP_BACK_TO_BROM=y
 CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
-CONFIG_SPL_MMC=y
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_STACK_R_ADDR=0x600000
 CONFIG_TARGET_EVB_RK3568=y
+CONFIG_SPL_STACK=0x400000
 CONFIG_DEBUG_UART_BASE=0xFE660000
 CONFIG_DEBUG_UART_CLOCK=24000000
 CONFIG_SYS_LOAD_ADDR=0xc00800
 CONFIG_DEBUG_UART=y
-CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
+CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-evb.dtb"
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_SPL_MAX_SIZE=0x20000
+CONFIG_SPL_MAX_SIZE=0x40000
 CONFIG_SPL_PAD_TO=0x7f8000
 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
 CONFIG_SPL_BSS_START_ADDR=0x4000000
 CONFIG_SPL_BSS_MAX_SIZE=0x4000
 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
-CONFIG_SPL_STACK=0x400000
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_ATF=y
+CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_MMC=y
 # CONFIG_CMD_SETEXPR is not set
 # CONFIG_SPL_DOS_PARTITION is not set
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_OF_LIVE=y
+CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPL_REGMAP=y
 CONFIG_SPL_SYSCON=y
-- 
2.39.1

[PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

[Jonas Karlman]

Implement CheckMissing and CheckOptional methods that is adapted to
Entry_mkimage in order to improve support for allow missing flag.

Use collect_contents_to_file in multiple-data-files handling to improve
support for allow missing and fake blobs handling.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
Building for RK3568 without ROCKCHIP_TPL will result in the following
error message, regardless if BINMAN_ALLOW_MISSING is used or not.

  binman: Filename 'rockchip-tpl' not found in input path (...)

With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob
with no content is created and then passed to mkimage, resulting in an
error from the mkimage command.

  binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument

If the --fake-ext-blobs argument is also used I get the message I was
expecting to see from the beginning.

  Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl

  /binman/simple-bin/mkimage/rockchip-tpl:
     An external TPL is required to initialize DRAM. Get the external TPL
     binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
     for the external TPL binary is https://github.com/rockchip-linux/rkbin.
  Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl

  Some images are invalid

Not sure how this should work, but I was expecting to see the message
about the missing rockchip-tpl from the beginning instead of the generic
"not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.

 tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb264c3cad0b..44510a8c40ba 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -153,10 +153,12 @@ class Entry_mkimage(Entry):
         if self._multiple_data_files:
             fnames = []
             uniq = self.GetUniqueName()
-            for entry in self._mkimage_entries.values():
-                if not entry.ObtainContents(fake_size=fake_size):
+            for idx, entry in enumerate(self._mkimage_entries.values()):
+                entry_data, entry_fname, _ = self.collect_contents_to_file(
+                    [entry], 'mkimage-%s' % idx, fake_size)
+                if entry_data is None:
                     return False
-                fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
+                fnames.append(entry_fname)
             input_fname = ":".join(fnames)
         else:
             data, input_fname, uniq = self.collect_contents_to_file(
@@ -165,7 +167,7 @@ class Entry_mkimage(Entry):
                 return False
         if self._imagename:
             image_data, imagename_fname, _ = self.collect_contents_to_file(
-                [self._imagename], 'mkimage-n', 1024)
+                [self._imagename], 'mkimage-n', fake_size)
             if image_data is None:
                 return False
         outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
@@ -216,6 +218,20 @@ class Entry_mkimage(Entry):
         if self._imagename:
             self._imagename.SetAllowFakeBlob(allow_fake)
 
+    def CheckMissing(self, missing_list):
+        """Check if any entries in this section have missing external blobs
+
+        If there are missing (non-optional) blobs, the entries are added to the
+        list
+
+        Args:
+            missing_list: List of Entry objects to be added to
+        """
+        for entry in self._mkimage_entries.values():
+            entry.CheckMissing(missing_list)
+        if self._imagename:
+            self._imagename.CheckMissing(missing_list)
+
     def CheckFakedBlobs(self, faked_blobs_list):
         """Check if any entries in this section have faked external blobs
 
@@ -229,6 +245,19 @@ class Entry_mkimage(Entry):
         if self._imagename:
             self._imagename.CheckFakedBlobs(faked_blobs_list)
 
+    def CheckOptional(self, optional_list):
+        """Check the section for missing but optional external blobs
+
+        If there are missing (optional) blobs, the entries are added to the list
+
+        Args:
+            optional_list (list): List of Entry objects to be added to
+        """
+        for entry in self._mkimage_entries.values():
+            entry.CheckOptional(optional_list)
+        if self._imagename:
+            self._imagename.CheckOptional(optional_list)
+
     def AddBintools(self, btools):
         super().AddBintools(btools)
         self.mkimage = self.AddBintool(btools, 'mkimage')
-- 
2.39.1

Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing

[Jonas Karlman]

Hi Kever,
On 2023-02-14 04:45, Kever Yang wrote:
> Hi Jonas,
> 
> On 2023/2/6 20:51, Jonas Karlman wrote:
>> Hi Quentin,
>> On 2023-02-06 12:26, Quentin Schulz wrote:
>>> Hi Jonas,
>>>
>>> On 2/5/23 21:21, Jonas Karlman wrote:
>>>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>>>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL.
>>>>
>>>> For RK356x there is currently no support to initialize DRAM using U-Boot
>>>> TPL and instead an external TPL binary must be used to generate a
>>>> working u-boot-rockchip.bin image.
>>>>
>>>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an
>>>> external TPL binary must be provided to generate a working firmware.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>>    Makefile                          |  1 +
>>>>    arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++----
>>>>    tools/binman/missing-blob-help    |  5 +++++
>>>>    3 files changed, 18 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 7eaf45496c1c..7e9272be937f 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>>>>    		-a opensbi-path=${OPENSBI} \
>>>>    		-a default-dt=$(default_dt) \
>>>>    		-a scp-path=$(SCP) \
>>>> +		-a external-tpl-path=$(EXTERNAL_TPL) \
>>>>    		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>>>    		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>>>    		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> index 6c662a72d4f9..bc3bc9bc3e37 100644
>>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> @@ -20,12 +20,16 @@
>>>>    		mkimage {
>>>>    			filename = "idbloader.img";
>>>>    			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>>>> -#ifdef CONFIG_TPL
>>>>    			multiple-data-files;
>>>>    
>>>> +#ifdef CONFIG_TPL
>>>>    			u-boot-tpl {
>>>> -			};
>>>> +#else
>>>> +			external-tpl {
>>>> +				filename = "ddr.bin";
>>>> +				missing-msg = "external-tpl-rockchip";
>>>>    #endif
>>>> +			};
>>> NACK. This forces the use of a TPL (either built by U-Boot or external)
>>> which is not always the case. There are still boards without a TPL which
>>> work perfectly fine (at least I would hope so :) ).
>>>
>>> Basically there are three possible cases:
>>> SPL
>>> TPL+SPL
>>> TPL(external blob)+SPL
>>>
>>> Here you remove the ability to have SPL only.
>>>
>>> Hence why I suggested we add a new Kconfig option for EXTERNAL_TPL, not
>>> for the path but to explicit this third possibility.
>> Thanks for the feedback, I will add a Kconfig option to make this explicit.
>> We could also add the optional prop in the external-tpl node to only include
>> the external TPL when it is provided. This will require also fixing the
>> missing/optional handling for the mkimage entry.
> 
> Could you help to send the new patchset with this update?
> 

I have now send out a v2 of this, see [1].

[1] https://patchwork.ozlabs.org/project/uboot/cover/20230214103300.690542-1-jonas@kwiboo.se/

Regards,
Jonas

> 
> Thanks,
> 
> - Kever
> 

Re: [PATCH v2 2/6] rockchip: Use an external TPL binary on RK3568

[Simon Glass]

On Tue, 14 Feb 2023 at 03:33, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
> jumps to U-Boot SPL to continue the normal boot flow.
>
> However, there is no support to initialize DRAM on RK35xx SoCs using
> U-Boot TPL and instead an external TPL binary must be used to generate a
> bootable u-boot-rockchip.bin image.
>
> Add CONFIG_ROCKCHIP_EXTERNAL_TPL to indicate that an external TPL should
> be used. Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
> bootable u-boot-rockchip.bin image for RK3568.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - rename external-tpl-path to rockchip-tpl-path
> - rename EXTERNAL_TPL to ROCKCHIP_TPL
> - add CONFIG_ROCKCHIP_EXTERNAL_TPL option
>
>  Makefile                          |  1 +
>  arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++--
>  arch/arm/mach-rockchip/Kconfig    |  4 ++++
>  3 files changed, 13 insertions(+), 2 deletions(-)

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

Please add help for Kconfig


>
> diff --git a/Makefile b/Makefile
> index 54f894dab841..58f8c7a35335 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                 -a opensbi-path=${OPENSBI} \
>                 -a default-dt=$(default_dt) \
>                 -a scp-path=$(SCP) \
> +               -a rockchip-tpl-path=$(ROCKCHIP_TPL) \
>                 -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 6c662a72d4f9..2878b80926c4 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -20,9 +20,12 @@
>                 mkimage {
>                         filename = "idbloader.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> +                       rockchip-tpl {
> +                       };
> +#elif defined(CONFIG_TPL)
>                         u-boot-tpl {
>                         };
>  #endif
> @@ -134,9 +137,12 @@
>                 mkimage {
>                         filename = "idbloader-spi.img";
>                         args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
>                         multiple-data-files;
>
> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> +                       rockchip-tpl {
> +                       };
> +#elif defined(CONFIG_TPL)
>                         u-boot-tpl {
>                         };
>  #endif
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index b678ec41318e..4a5415403446 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -377,6 +377,10 @@ config TPL_ROCKCHIP_BACK_TO_BROM
>            SPL will return to the boot rom, which will then load the U-Boot
>            binary to keep going on.
>
> +config ROCKCHIP_EXTERNAL_TPL
> +       bool "Use external TPL binary"
> +       default y if ROCKCHIP_RK3568
> +
>  config ROCKCHIP_COMMON_BOARD
>         bool "Rockchip common board file"
>         help
> --
> 2.39.1
>

Re: [PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Simon Glass]

Hi Jonas,

On Tue, 14 Feb 2023 at 03:33, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> The rockchip-tpl entry can be used when an external TPL binary should be
> used instead of the normal U-Boot TPL.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - rename external-tpl to rockchip-tpl
> - missing message moved to this patch
>
>  tools/binman/entries.rst               | 14 ++++++++++++++
>  tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
>  tools/binman/ftest.py                  |  7 +++++++
>  tools/binman/missing-blob-help         |  5 +++++
>  tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
>  5 files changed, 62 insertions(+)
>  create mode 100644 tools/binman/etype/rockchip_tpl.py
>  create mode 100644 tools/binman/test/277_rockchip_tpl.dts

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

>
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 7a04a613992d..e177860a6a82 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -1386,6 +1386,20 @@ For example, this creates an image with a pre-load header and a binary::
>
>
>
> +.. _etype_rockchip_tpl:
> +
> +Entry: rockchip-tpl: Rockchip TPL binary
> +----------------------------------------
> +
> +Properties / Entry arguments:
> +    - rockchip-tpl-path: Filename of file to read into the entry,
> +                         typically <soc>_ddr_<version>.bin
> +
> +This entry holds an external TPL binary used by some Rockchip SoCs
> +instead of normal U-Boot TPL, typically to initialize DRAM.
> +
> +
> +
>  .. _etype_scp:
>
>  Entry: scp: System Control Processor (SCP) firmware blob
> diff --git a/tools/binman/etype/rockchip_tpl.py b/tools/binman/etype/rockchip_tpl.py
> new file mode 100644
> index 000000000000..74f58ba8570c
> --- /dev/null
> +++ b/tools/binman/etype/rockchip_tpl.py
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Entry-type module for Rockchip TPL binary
> +#
> +
> +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
> +
> +class Entry_rockchip_tpl(Entry_blob_named_by_arg):
> +    """Rockchip TPL binary
> +
> +    Properties / Entry arguments:
> +        - rockchip-tpl-path: Filename of file to read into the entry,
> +                             typically <soc>_ddr_<version>.bin
> +
> +    This entry holds an external TPL binary used by some Rockchip SoCs
> +    instead of normal U-Boot TPL, typically to initialize DRAM.
> +    """
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node, 'rockchip-tpl')
> +        self.external = True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 062f54adb0ed..ed4b5c987557 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
>  ATF_BL2U_DATA         = b'bl2u'
>  OPENSBI_DATA          = b'opensbi'
>  SCP_DATA              = b'scp'
> +ROCKCHIP_TPL_DATA     = b'rockchip-tpl'
>  TEST_FDT1_DATA        = b'fdt1'
>  TEST_FDT2_DATA        = b'test-fdt2'
>  ENV_DATA              = b'var1=1\nvar2="2"'
> @@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
>          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
> +        TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA)
>
>          # Add a few .dtb files for testing
>          TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
> @@ -4097,6 +4099,11 @@ class TestFunctional(unittest.TestCase):
>          data = self._DoReadFile('172_scp.dts')
>          self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
>
> +    def testPackRockchipTpl(self):
> +        """Test that an image with a Rockchip TPL binary can be created"""
> +        data = self._DoReadFile('277_rockchip_tpl.dts')
> +        self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
> +

nit: please put new tests at the end of the file

>      def testFitFdt(self):
>          """Test an image with an FIT with multiple FDT images"""
>          def _CheckFdt(seq, expected_data):
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> index c61ca02a35ee..e8c991206fe5 100644
> --- a/tools/binman/missing-blob-help
> +++ b/tools/binman/missing-blob-help
> @@ -34,6 +34,11 @@ If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for
>  the R5F core(s) to trigger the system reset. One possible source is
>  https://github.com/siemens/k3-rti-wdt.
>
> +rockchip-tpl:
> +An external TPL is required to initialize DRAM. Get the external TPL
> +binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
> +for the external TPL binary is https://github.com/rockchip-linux/rkbin.
> +
>  tee-os:
>  See the documentation for your board. You may need to build Open Portable
>  Trusted Execution Environment (OP-TEE) with TEE=/path/to/tee.bin
> diff --git a/tools/binman/test/277_rockchip_tpl.dts b/tools/binman/test/277_rockchip_tpl.dts
> new file mode 100644
> index 000000000000..269f56e2545c
> --- /dev/null
> +++ b/tools/binman/test/277_rockchip_tpl.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               size = <16>;
> +
> +               rockchip-tpl {
> +                       filename = "rockchip-tpl.bin";
> +               };
> +       };
> +};
> --
> 2.39.1
>

Regards,
Simon

Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

[Simon Glass]

Hi Jonas,

On Tue, 14 Feb 2023 at 03:34, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Implement CheckMissing and CheckOptional methods that is adapted to
> Entry_mkimage in order to improve support for allow missing flag.
>
> Use collect_contents_to_file in multiple-data-files handling to improve
> support for allow missing and fake blobs handling.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> Building for RK3568 without ROCKCHIP_TPL will result in the following
> error message, regardless if BINMAN_ALLOW_MISSING is used or not.
>
>   binman: Filename 'rockchip-tpl' not found in input path (...)
>
> With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob
> with no content is created and then passed to mkimage, resulting in an
> error from the mkimage command.
>
>   binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument
>
> If the --fake-ext-blobs argument is also used I get the message I was
> expecting to see from the beginning.
>
>   Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl
>
>   /binman/simple-bin/mkimage/rockchip-tpl:
>      An external TPL is required to initialize DRAM. Get the external TPL
>      binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
>      for the external TPL binary is https://github.com/rockchip-linux/rkbin.
>   Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl
>
>   Some images are invalid
>
> Not sure how this should work, but I was expecting to see the message
> about the missing rockchip-tpl from the beginning instead of the generic
> "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.
>
>  tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> index cb264c3cad0b..44510a8c40ba 100644
> --- a/tools/binman/etype/mkimage.py
> +++ b/tools/binman/etype/mkimage.py
> @@ -153,10 +153,12 @@ class Entry_mkimage(Entry):
>          if self._multiple_data_files:
>              fnames = []
>              uniq = self.GetUniqueName()
> -            for entry in self._mkimage_entries.values():
> -                if not entry.ObtainContents(fake_size=fake_size):
> +            for idx, entry in enumerate(self._mkimage_entries.values()):
> +                entry_data, entry_fname, _ = self.collect_contents_to_file(
> +                    [entry], 'mkimage-%s' % idx, fake_size)
> +                if entry_data is None:

This is OK, I suppose, but I'm not quite sure what actually changes
here, other than writing each entry to a file?

Also, if you do this, please add / extend a test that checks that the
output files are written, since there is otherwise no coverage here.

What test uses these optional mkimage pieces?

>                      return False
> -                fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
> +                fnames.append(entry_fname)
>              input_fname = ":".join(fnames)
>          else:
>              data, input_fname, uniq = self.collect_contents_to_file(
> @@ -165,7 +167,7 @@ class Entry_mkimage(Entry):
>                  return False
>          if self._imagename:
>              image_data, imagename_fname, _ = self.collect_contents_to_file(
> -                [self._imagename], 'mkimage-n', 1024)
> +                [self._imagename], 'mkimage-n', fake_size)
>              if image_data is None:
>                  return False
>          outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
> @@ -216,6 +218,20 @@ class Entry_mkimage(Entry):
>          if self._imagename:
>              self._imagename.SetAllowFakeBlob(allow_fake)
>
> +    def CheckMissing(self, missing_list):
> +        """Check if any entries in this section have missing external blobs
> +
> +        If there are missing (non-optional) blobs, the entries are added to the
> +        list
> +
> +        Args:
> +            missing_list: List of Entry objects to be added to
> +        """
> +        for entry in self._mkimage_entries.values():
> +            entry.CheckMissing(missing_list)
> +        if self._imagename:
> +            self._imagename.CheckMissing(missing_list)
> +
>      def CheckFakedBlobs(self, faked_blobs_list):
>          """Check if any entries in this section have faked external blobs
>
> @@ -229,6 +245,19 @@ class Entry_mkimage(Entry):
>          if self._imagename:
>              self._imagename.CheckFakedBlobs(faked_blobs_list)
>
> +    def CheckOptional(self, optional_list):
> +        """Check the section for missing but optional external blobs
> +
> +        If there are missing (optional) blobs, the entries are added to the list
> +
> +        Args:
> +            optional_list (list): List of Entry objects to be added to
> +        """
> +        for entry in self._mkimage_entries.values():
> +            entry.CheckOptional(optional_list)
> +        if self._imagename:
> +            self._imagename.CheckOptional(optional_list)
> +
>      def AddBintools(self, btools):
>          super().AddBintools(btools)
>          self.mkimage = self.AddBintool(btools, 'mkimage')
> --
> 2.39.1
>

Regards,
Simon

Re: [PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Jonas Karlman]

Hi Simon,
On 2023-02-14 20:48, Simon Glass wrote:
> Hi Jonas,
> 
> On Tue, 14 Feb 2023 at 03:33, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> The rockchip-tpl entry can be used when an external TPL binary should be
>> used instead of the normal U-Boot TPL.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - rename external-tpl to rockchip-tpl
>> - missing message moved to this patch
>>
>>  tools/binman/entries.rst               | 14 ++++++++++++++
>>  tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
>>  tools/binman/ftest.py                  |  7 +++++++
>>  tools/binman/missing-blob-help         |  5 +++++
>>  tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
>>  5 files changed, 62 insertions(+)
>>  create mode 100644 tools/binman/etype/rockchip_tpl.py
>>  create mode 100644 tools/binman/test/277_rockchip_tpl.dts
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> nit below
> 
>>
>> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
>> index 7a04a613992d..e177860a6a82 100644
>> --- a/tools/binman/entries.rst
>> +++ b/tools/binman/entries.rst
>> @@ -1386,6 +1386,20 @@ For example, this creates an image with a pre-load header and a binary::
>>
>>
>>
>> +.. _etype_rockchip_tpl:
>> +
>> +Entry: rockchip-tpl: Rockchip TPL binary
>> +----------------------------------------
>> +
>> +Properties / Entry arguments:
>> +    - rockchip-tpl-path: Filename of file to read into the entry,
>> +                         typically <soc>_ddr_<version>.bin
>> +
>> +This entry holds an external TPL binary used by some Rockchip SoCs
>> +instead of normal U-Boot TPL, typically to initialize DRAM.
>> +
>> +
>> +
>>  .. _etype_scp:
>>
>>  Entry: scp: System Control Processor (SCP) firmware blob
>> diff --git a/tools/binman/etype/rockchip_tpl.py b/tools/binman/etype/rockchip_tpl.py
>> new file mode 100644
>> index 000000000000..74f58ba8570c
>> --- /dev/null
>> +++ b/tools/binman/etype/rockchip_tpl.py
>> @@ -0,0 +1,20 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Entry-type module for Rockchip TPL binary
>> +#
>> +
>> +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
>> +
>> +class Entry_rockchip_tpl(Entry_blob_named_by_arg):
>> +    """Rockchip TPL binary
>> +
>> +    Properties / Entry arguments:
>> +        - rockchip-tpl-path: Filename of file to read into the entry,
>> +                             typically <soc>_ddr_<version>.bin
>> +
>> +    This entry holds an external TPL binary used by some Rockchip SoCs
>> +    instead of normal U-Boot TPL, typically to initialize DRAM.
>> +    """
>> +    def __init__(self, section, etype, node):
>> +        super().__init__(section, etype, node, 'rockchip-tpl')
>> +        self.external = True
>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>> index 062f54adb0ed..ed4b5c987557 100644
>> --- a/tools/binman/ftest.py
>> +++ b/tools/binman/ftest.py
>> @@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
>>  ATF_BL2U_DATA         = b'bl2u'
>>  OPENSBI_DATA          = b'opensbi'
>>  SCP_DATA              = b'scp'
>> +ROCKCHIP_TPL_DATA     = b'rockchip-tpl'
>>  TEST_FDT1_DATA        = b'fdt1'
>>  TEST_FDT2_DATA        = b'test-fdt2'
>>  ENV_DATA              = b'var1=1\nvar2="2"'
>> @@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
>>          TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>>          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
>>          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
>> +        TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA)
>>
>>          # Add a few .dtb files for testing
>>          TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
>> @@ -4097,6 +4099,11 @@ class TestFunctional(unittest.TestCase):
>>          data = self._DoReadFile('172_scp.dts')
>>          self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
>>
>> +    def testPackRockchipTpl(self):
>> +        """Test that an image with a Rockchip TPL binary can be created"""
>> +        data = self._DoReadFile('277_rockchip_tpl.dts')
>> +        self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
>> +
> 
> nit: please put new tests at the end of the file

Thanks, I will move the test to the end of the file.

Regards,
Jonas

> 
>>      def testFitFdt(self):
>>          """Test an image with an FIT with multiple FDT images"""
>>          def _CheckFdt(seq, expected_data):
>> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
>> index c61ca02a35ee..e8c991206fe5 100644
>> --- a/tools/binman/missing-blob-help
>> +++ b/tools/binman/missing-blob-help
>> @@ -34,6 +34,11 @@ If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for
>>  the R5F core(s) to trigger the system reset. One possible source is
>>  https://github.com/siemens/k3-rti-wdt>>>
>> +rockchip-tpl:
>> +An external TPL is required to initialize DRAM. Get the external TPL
>> +binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
>> +for the external TPL binary is https://github.com/rockchip-linux/rkbin>>> +
>>  tee-os:
>>  See the documentation for your board. You may need to build Open Portable
>>  Trusted Execution Environment (OP-TEE) with TEE=/path/to/tee.bin
>> diff --git a/tools/binman/test/277_rockchip_tpl.dts b/tools/binman/test/277_rockchip_tpl.dts
>> new file mode 100644
>> index 000000000000..269f56e2545c
>> --- /dev/null
>> +++ b/tools/binman/test/277_rockchip_tpl.dts
>> @@ -0,0 +1,16 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +/dts-v1/;
>> +
>> +/ {
>> +       #address-cells = <1>;
>> +       #size-cells = <1>;
>> +
>> +       binman {
>> +               size = <16>;
>> +
>> +               rockchip-tpl {
>> +                       filename = "rockchip-tpl.bin";
>> +               };
>> +       };
>> +};
>> --
>> 2.39.1
>>
> 
> Regards,
> Simon

Re: [PATCH v2 2/6] rockchip: Use an external TPL binary on RK3568

[Jonas Karlman]

On 2023-02-14 20:48, Simon Glass wrote:
> On Tue, 14 Feb 2023 at 03:33, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
>> back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
>> jumps to U-Boot SPL to continue the normal boot flow.
>>
>> However, there is no support to initialize DRAM on RK35xx SoCs using
>> U-Boot TPL and instead an external TPL binary must be used to generate a
>> bootable u-boot-rockchip.bin image.
>>
>> Add CONFIG_ROCKCHIP_EXTERNAL_TPL to indicate that an external TPL should
>> be used. Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
>> bootable u-boot-rockchip.bin image for RK3568.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - rename external-tpl-path to rockchip-tpl-path
>> - rename EXTERNAL_TPL to ROCKCHIP_TPL
>> - add CONFIG_ROCKCHIP_EXTERNAL_TPL option
>>
>>  Makefile                          |  1 +
>>  arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++--
>>  arch/arm/mach-rockchip/Kconfig    |  4 ++++
>>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Please add help for Kconfig

Thanks, will add help text.

Regards,
Jonas

> 
> 
>>
>> diff --git a/Makefile b/Makefile
>> index 54f894dab841..58f8c7a35335 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>>                 -a opensbi-path=${OPENSBI} \
>>                 -a default-dt=$(default_dt) \
>>                 -a scp-path=$(SCP) \
>> +               -a rockchip-tpl-path=$(ROCKCHIP_TPL) \
>>                 -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>> index 6c662a72d4f9..2878b80926c4 100644
>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>> @@ -20,9 +20,12 @@
>>                 mkimage {
>>                         filename = "idbloader.img";
>>                         args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
>> -#ifdef CONFIG_TPL
>>                         multiple-data-files;
>>
>> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
>> +                       rockchip-tpl {
>> +                       };
>> +#elif defined(CONFIG_TPL)
>>                         u-boot-tpl {
>>                         };
>>  #endif
>> @@ -134,9 +137,12 @@
>>                 mkimage {
>>                         filename = "idbloader-spi.img";
>>                         args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
>> -#ifdef CONFIG_TPL
>>                         multiple-data-files;
>>
>> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
>> +                       rockchip-tpl {
>> +                       };
>> +#elif defined(CONFIG_TPL)
>>                         u-boot-tpl {
>>                         };
>>  #endif
>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>> index b678ec41318e..4a5415403446 100644
>> --- a/arch/arm/mach-rockchip/Kconfig
>> +++ b/arch/arm/mach-rockchip/Kconfig
>> @@ -377,6 +377,10 @@ config TPL_ROCKCHIP_BACK_TO_BROM
>>            SPL will return to the boot rom, which will then load the U-Boot
>>            binary to keep going on.
>>
>> +config ROCKCHIP_EXTERNAL_TPL
>> +       bool "Use external TPL binary"
>> +       default y if ROCKCHIP_RK3568
>> +
>>  config ROCKCHIP_COMMON_BOARD
>>         bool "Rockchip common board file"
>>         help
>> --
>> 2.39.1
>>

Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

[Jonas Karlman]

Hi Simon,

On 2023-02-14 20:48, Simon Glass wrote:
> Hi Jonas,
> 
> On Tue, 14 Feb 2023 at 03:34, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Implement CheckMissing and CheckOptional methods that is adapted to
>> Entry_mkimage in order to improve support for allow missing flag.
>>
>> Use collect_contents_to_file in multiple-data-files handling to improve
>> support for allow missing and fake blobs handling.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> Building for RK3568 without ROCKCHIP_TPL will result in the following
>> error message, regardless if BINMAN_ALLOW_MISSING is used or not.
>>
>>   binman: Filename 'rockchip-tpl' not found in input path (...)
>>
>> With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob
>> with no content is created and then passed to mkimage, resulting in an
>> error from the mkimage command.
>>
>>   binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument
>>
>> If the --fake-ext-blobs argument is also used I get the message I was
>> expecting to see from the beginning.
>>
>>   Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl
>>
>>   /binman/simple-bin/mkimage/rockchip-tpl:
>>      An external TPL is required to initialize DRAM. Get the external TPL
>>      binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
>>      for the external TPL binary is https://github.com/rockchip-linux/rkbin>>>   Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl
>>
>>   Some images are invalid
>>
>> Not sure how this should work, but I was expecting to see the message
>> about the missing rockchip-tpl from the beginning instead of the generic
>> "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.
>>
>>  tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++----
>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
>> index cb264c3cad0b..44510a8c40ba 100644
>> --- a/tools/binman/etype/mkimage.py
>> +++ b/tools/binman/etype/mkimage.py
>> @@ -153,10 +153,12 @@ class Entry_mkimage(Entry):
>>          if self._multiple_data_files:
>>              fnames = []
>>              uniq = self.GetUniqueName()
>> -            for entry in self._mkimage_entries.values():
>> -                if not entry.ObtainContents(fake_size=fake_size):
>> +            for idx, entry in enumerate(self._mkimage_entries.values()):
>> +                entry_data, entry_fname, _ = self.collect_contents_to_file(
>> +                    [entry], 'mkimage-%s' % idx, fake_size)
>> +                if entry_data is None:
> 
> This is OK, I suppose, but I'm not quite sure what actually changes
> here, other than writing each entry to a file?

The collect_contents_to_file function seemed to handle the
external/missing/optional/faked entry flags. So I changed to use that
function in order to see if that would change anything, see below.

Use of this function does make it possible to use entry type other
then external blobs, not sure if that would ever be needed/useful.

> 
> Also, if you do this, please add / extend a test that checks that the
> output files are written, since there is otherwise no coverage here.
> 
> What test uses these optional mkimage pieces?

Sorry, I was not clear enough about the reason for these changes in my
message above.

Building with --rockchip-tpl-path=/path/to/existing/tpl works as
expected and generates a working image.

I was expecting that the missing-blob-help message added in patch 1
would be shown by binman when rockchip-tpl-path was empty/not-existing,
or at least together with the allow-missing flag.

However, whatever I tested, empty/none-existing rockchip-tpl-path,
allow-missing, fake-ext-blobs, I was not able to see this message.
Instead, all I could get from binman was this "Filename 'rockchip-tpl'
not found in input path" message.

Maybe my assumptions about when these missing messages should be shown
is wrong?

Trying binman with the following two dts and --allow-missing gives
different results. First one shows the missing message, second one show
filename not found.

binman {
	rockchip-tpl {
	};
};

binman {
	mkimage {
		args = "-n rk3568 -T rksd";
		multiple-data-files;

		rockchip-tpl {
		};
	};
};

With the changes in this patch I instead get the missing message when I
also add the --fake-ext-blobs flag, so it clearly needs more work or
a completely different approach if we want to be able to see the missing
message added in patch 1.

Adding a message that never will be displayed annoys me :-)
Maybe I should just drop this rfc/patch for a v3 of this series?

Regards,
Jonas

> 
>>                      return False
>> -                fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
>> +                fnames.append(entry_fname)
>>              input_fname = ":".join(fnames)
>>          else:
>>              data, input_fname, uniq = self.collect_contents_to_file(
>> @@ -165,7 +167,7 @@ class Entry_mkimage(Entry):
>>                  return False
>>          if self._imagename:
>>              image_data, imagename_fname, _ = self.collect_contents_to_file(
>> -                [self._imagename], 'mkimage-n', 1024)
>> +                [self._imagename], 'mkimage-n', fake_size)
>>              if image_data is None:
>>                  return False
>>          outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
>> @@ -216,6 +218,20 @@ class Entry_mkimage(Entry):
>>          if self._imagename:
>>              self._imagename.SetAllowFakeBlob(allow_fake)
>>
>> +    def CheckMissing(self, missing_list):
>> +        """Check if any entries in this section have missing external blobs
>> +
>> +        If there are missing (non-optional) blobs, the entries are added to the
>> +        list
>> +
>> +        Args:
>> +            missing_list: List of Entry objects to be added to
>> +        """
>> +        for entry in self._mkimage_entries.values():
>> +            entry.CheckMissing(missing_list)
>> +        if self._imagename:
>> +            self._imagename.CheckMissing(missing_list)
>> +
>>      def CheckFakedBlobs(self, faked_blobs_list):
>>          """Check if any entries in this section have faked external blobs
>>
>> @@ -229,6 +245,19 @@ class Entry_mkimage(Entry):
>>          if self._imagename:
>>              self._imagename.CheckFakedBlobs(faked_blobs_list)
>>
>> +    def CheckOptional(self, optional_list):
>> +        """Check the section for missing but optional external blobs
>> +
>> +        If there are missing (optional) blobs, the entries are added to the list
>> +
>> +        Args:
>> +            optional_list (list): List of Entry objects to be added to
>> +        """
>> +        for entry in self._mkimage_entries.values():
>> +            entry.CheckOptional(optional_list)
>> +        if self._imagename:
>> +            self._imagename.CheckOptional(optional_list)
>> +
>>      def AddBintools(self, btools):
>>          super().AddBintools(btools)
>>          self.mkimage = self.AddBintool(btools, 'mkimage')
>> --
>> 2.39.1
>>
> 
> Regards,
> Simon

Re: [PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Kever Yang]

On 2023/2/14 18:33, Jonas Karlman wrote:
> The rockchip-tpl entry can be used when an external TPL binary should be
> used instead of the normal U-Boot TPL.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - rename external-tpl to rockchip-tpl
> - missing message moved to this patch
>
>   tools/binman/entries.rst               | 14 ++++++++++++++
>   tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
>   tools/binman/ftest.py                  |  7 +++++++
>   tools/binman/missing-blob-help         |  5 +++++
>   tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
>   5 files changed, 62 insertions(+)
>   create mode 100644 tools/binman/etype/rockchip_tpl.py
>   create mode 100644 tools/binman/test/277_rockchip_tpl.dts
>
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 7a04a613992d..e177860a6a82 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -1386,6 +1386,20 @@ For example, this creates an image with a pre-load header and a binary::
>   
>   
>   
> +.. _etype_rockchip_tpl:
> +
> +Entry: rockchip-tpl: Rockchip TPL binary
> +----------------------------------------
> +
> +Properties / Entry arguments:
> +    - rockchip-tpl-path: Filename of file to read into the entry,
> +                         typically <soc>_ddr_<version>.bin
> +
> +This entry holds an external TPL binary used by some Rockchip SoCs
> +instead of normal U-Boot TPL, typically to initialize DRAM.
> +
> +
> +
>   .. _etype_scp:
>   
>   Entry: scp: System Control Processor (SCP) firmware blob
> diff --git a/tools/binman/etype/rockchip_tpl.py b/tools/binman/etype/rockchip_tpl.py
> new file mode 100644
> index 000000000000..74f58ba8570c
> --- /dev/null
> +++ b/tools/binman/etype/rockchip_tpl.py
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Entry-type module for Rockchip TPL binary
> +#
> +
> +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
> +
> +class Entry_rockchip_tpl(Entry_blob_named_by_arg):
> +    """Rockchip TPL binary
> +
> +    Properties / Entry arguments:
> +        - rockchip-tpl-path: Filename of file to read into the entry,
> +                             typically <soc>_ddr_<version>.bin
> +
> +    This entry holds an external TPL binary used by some Rockchip SoCs
> +    instead of normal U-Boot TPL, typically to initialize DRAM.
> +    """
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node, 'rockchip-tpl')
> +        self.external = True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 062f54adb0ed..ed4b5c987557 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -90,6 +90,7 @@ TEE_OS_DATA           = b'this is some tee OS data'
>   ATF_BL2U_DATA         = b'bl2u'
>   OPENSBI_DATA          = b'opensbi'
>   SCP_DATA              = b'scp'
> +ROCKCHIP_TPL_DATA     = b'rockchip-tpl'
>   TEST_FDT1_DATA        = b'fdt1'
>   TEST_FDT2_DATA        = b'test-fdt2'
>   ENV_DATA              = b'var1=1\nvar2="2"'
> @@ -205,6 +206,7 @@ class TestFunctional(unittest.TestCase):
>           TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>           TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
>           TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
> +        TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA)
>   
>           # Add a few .dtb files for testing
>           TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
> @@ -4097,6 +4099,11 @@ class TestFunctional(unittest.TestCase):
>           data = self._DoReadFile('172_scp.dts')
>           self.assertEqual(SCP_DATA, data[:len(SCP_DATA)])
>   
> +    def testPackRockchipTpl(self):
> +        """Test that an image with a Rockchip TPL binary can be created"""
> +        data = self._DoReadFile('277_rockchip_tpl.dts')
> +        self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
> +
>       def testFitFdt(self):
>           """Test an image with an FIT with multiple FDT images"""
>           def _CheckFdt(seq, expected_data):
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> index c61ca02a35ee..e8c991206fe5 100644
> --- a/tools/binman/missing-blob-help
> +++ b/tools/binman/missing-blob-help
> @@ -34,6 +34,11 @@ If CONFIG_WDT_K3_RTI_LOAD_FW is enabled, a firmware image is needed for
>   the R5F core(s) to trigger the system reset. One possible source is
>   https://github.com/siemens/k3-rti-wdt.
>   
> +rockchip-tpl:
> +An external TPL is required to initialize DRAM. Get the external TPL
> +binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
> +for the external TPL binary is https://github.com/rockchip-linux/rkbin.
> +
>   tee-os:
>   See the documentation for your board. You may need to build Open Portable
>   Trusted Execution Environment (OP-TEE) with TEE=/path/to/tee.bin
> diff --git a/tools/binman/test/277_rockchip_tpl.dts b/tools/binman/test/277_rockchip_tpl.dts
> new file mode 100644
> index 000000000000..269f56e2545c
> --- /dev/null
> +++ b/tools/binman/test/277_rockchip_tpl.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	binman {
> +		size = <16>;
> +
> +		rockchip-tpl {
> +			filename = "rockchip-tpl.bin";
> +		};
> +	};
> +};

Re: [PATCH v2 2/6] rockchip: Use an external TPL binary on RK3568

[Kever Yang]

On 2023/2/14 18:33, Jonas Karlman wrote:
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
> jumps to U-Boot SPL to continue the normal boot flow.
>
> However, there is no support to initialize DRAM on RK35xx SoCs using
> U-Boot TPL and instead an external TPL binary must be used to generate a
> bootable u-boot-rockchip.bin image.
>
> Add CONFIG_ROCKCHIP_EXTERNAL_TPL to indicate that an external TPL should
> be used. Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
> bootable u-boot-rockchip.bin image for RK3568.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - rename external-tpl-path to rockchip-tpl-path
> - rename EXTERNAL_TPL to ROCKCHIP_TPL
> - add CONFIG_ROCKCHIP_EXTERNAL_TPL option
>
>   Makefile                          |  1 +
>   arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++--
>   arch/arm/mach-rockchip/Kconfig    |  4 ++++
>   3 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 54f894dab841..58f8c7a35335 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>   		-a opensbi-path=${OPENSBI} \
>   		-a default-dt=$(default_dt) \
>   		-a scp-path=$(SCP) \
> +		-a rockchip-tpl-path=$(ROCKCHIP_TPL) \
>   		-a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \
>   		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>   		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 6c662a72d4f9..2878b80926c4 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -20,9 +20,12 @@
>   		mkimage {
>   			filename = "idbloader.img";
>   			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
>   			multiple-data-files;
>   
> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> +			rockchip-tpl {
> +			};
> +#elif defined(CONFIG_TPL)
>   			u-boot-tpl {
>   			};
>   #endif
> @@ -134,9 +137,12 @@
>   		mkimage {
>   			filename = "idbloader-spi.img";
>   			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
>   			multiple-data-files;
>   
> +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> +			rockchip-tpl {
> +			};
> +#elif defined(CONFIG_TPL)
>   			u-boot-tpl {
>   			};
>   #endif
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index b678ec41318e..4a5415403446 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -377,6 +377,10 @@ config TPL_ROCKCHIP_BACK_TO_BROM
>             SPL will return to the boot rom, which will then load the U-Boot
>             binary to keep going on.
>   
> +config ROCKCHIP_EXTERNAL_TPL
> +	bool "Use external TPL binary"
> +	default y if ROCKCHIP_RK3568
> +
>   config ROCKCHIP_COMMON_BOARD
>   	bool "Rockchip common board file"
>   	help

Re: [PATCH v2 3/6] Revert "board: rockchip: Fix binman_init failure on EVB-RK3568"

[Kever Yang]

On 2023/2/14 18:33, Jonas Karlman wrote:
> An external TPL binary is now expected to be provided using ROCKCHIP_TPL
> when building RK3568 targets.
>
> This reverts commit 31500e7bcfaca08ab7c2879f502a6cf852410244.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v2:
> - Collect r-b tag
>
>   configs/evb-rk3568_defconfig | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
> index a76d924d3872..7374ee42fb19 100644
> --- a/configs/evb-rk3568_defconfig
> +++ b/configs/evb-rk3568_defconfig
> @@ -65,5 +65,4 @@ CONFIG_BAUDRATE=1500000
>   CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_SYS_NS16550_MEM32=y
>   CONFIG_SYSRESET=y
> -# CONFIG_BINMAN_FDT is not set
>   CONFIG_ERRNO_STR=y

Re: [PATCH v2 4/6] rockchip: mkimage: Update init size limit

[Kever Yang]

Hi Jonas,

On 2023/2/14 18:33, Jonas Karlman wrote:
> Sync init size limit from vendor u-boot and the SRAM size specified in a
> SoCs TRM. Size is typically defined using the following expression:
> <SRAM size> - <BootRom stack size>

Although most of SoC follow this rule, but not for all.

So please just do the sync from vendor u-boot, but not extra modify 
under this rule.

Thanks,

- Kever

>
> This makes it possible to use latest vendor TPL with RK3328 without
> getting a size limit error running the mkimage command.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - new patch
>
>   tools/rkcommon.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
> index 1f1eaa16752b..7ad4780daf31 100644
> --- a/tools/rkcommon.c
> +++ b/tools/rkcommon.c
> @@ -121,20 +121,20 @@ struct spl_info {
>   };
>   
>   static struct spl_info spl_infos[] = {
> -	{ "px30", "RK33", 0x2800, false, RK_HEADER_V1 },
> -	{ "rk3036", "RK30", 0x1000, false, RK_HEADER_V1 },
> +	{ "px30", "RK33", 0x4000 - 0x1000, false, RK_HEADER_V1 },
> +	{ "rk3036", "RK30", 0x2000 - 0x1000, false, RK_HEADER_V1 },
>   	{ "rk3066", "RK30", 0x8000 - 0x800, true, RK_HEADER_V1 },
> -	{ "rk3128", "RK31", 0x1800, false, RK_HEADER_V1 },
> +	{ "rk3128", "RK31", 0x2000 - 0x800, false, RK_HEADER_V1 },
>   	{ "rk3188", "RK31", 0x8000 - 0x800, true, RK_HEADER_V1 },
>   	{ "rk322x", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
> -	{ "rk3288", "RK32", 0x8000, false, RK_HEADER_V1 },
> +	{ "rk3288", "RK32", 0x18000 - 0x1000, false, RK_HEADER_V1 },
>   	{ "rk3308", "RK33", 0x40000 - 0x1000, false, RK_HEADER_V1 },
> -	{ "rk3328", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
> -	{ "rk3368", "RK33", 0x8000 - 0x1000, false, RK_HEADER_V1 },
> +	{ "rk3328", "RK32", 0x8000 - 0x800, false, RK_HEADER_V1 },
> +	{ "rk3368", "RK33", 0x10000 - 0x1000, false, RK_HEADER_V1 },
>   	{ "rk3399", "RK33", 0x30000 - 0x2000, false, RK_HEADER_V1 },
> -	{ "rv1108", "RK11", 0x1800, false, RK_HEADER_V1 },
> +	{ "rv1108", "RK11", 0x2000 - 0x800, false, RK_HEADER_V1 },
>   	{ "rv1126", "110B", 0x10000 - 0x1000, false, RK_HEADER_V1 },
> -	{ "rk3568", "RK35", 0x14000 - 0x1000, false, RK_HEADER_V2 },
> +	{ "rk3568", "RK35", 0x10000 - 0x1000, false, RK_HEADER_V2 },
>   };
>   
>   /**

Re: [PATCH v2 2/6] rockchip: Use an external TPL binary on RK3568

[Jagan Teki]

On Tue, Feb 14, 2023 at 4:03 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps
> back to BootRom to load next stage, U-Boot SPL, into DRAM. BootRom then
> jumps to U-Boot SPL to continue the normal boot flow.
>
> However, there is no support to initialize DRAM on RK35xx SoCs using
> U-Boot TPL and instead an external TPL binary must be used to generate a
> bootable u-boot-rockchip.bin image.
>
> Add CONFIG_ROCKCHIP_EXTERNAL_TPL to indicate that an external TPL should
> be used. Build U-Boot with ROCKCHIP_TPL=/path/to/ddr.bin to generate a
> bootable u-boot-rockchip.bin image for RK3568.

Please add documentation in doc/board/rockchip/rockchip.rst and take
an example of a real DDR bin from rkbin.

Jagan.

Re: [PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Eugen Hristev]

On 2/14/23 12:33, Jonas Karlman wrote:
> The rockchip-tpl entry can be used when an external TPL binary should be
> used instead of the normal U-Boot TPL.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - rename external-tpl to rockchip-tpl
> - missing message moved to this patch
> 
>   tools/binman/entries.rst               | 14 ++++++++++++++
>   tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
>   tools/binman/ftest.py                  |  7 +++++++
>   tools/binman/missing-blob-help         |  5 +++++
>   tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
>   5 files changed, 62 insertions(+)
>   create mode 100644 tools/binman/etype/rockchip_tpl.py
>   create mode 100644 tools/binman/test/277_rockchip_tpl.dts

Hi Jonas,

Is it possible to add the filename 'rockchip-tpl' to gitignore, such 
that it won't show up all the time when you do git status ?
(in the case where you place the rockchip-tpl in the same dir )

Otherwise, I tested your series:

Tested-by: Eugen Hristev <eugen.hristev@collabora.com>

Eugen

Re: [PATCH v2 1/6] binman: Add support for a rockchip-tpl entry

[Jonas Karlman]

Hi Eugen,

On 2023-02-16 12:26, Eugen Hristev wrote:
> On 2/14/23 12:33, Jonas Karlman wrote:
>> The rockchip-tpl entry can be used when an external TPL binary should be
>> used instead of the normal U-Boot TPL.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - rename external-tpl to rockchip-tpl
>> - missing message moved to this patch
>>
>>   tools/binman/entries.rst               | 14 ++++++++++++++
>>   tools/binman/etype/rockchip_tpl.py     | 20 ++++++++++++++++++++
>>   tools/binman/ftest.py                  |  7 +++++++
>>   tools/binman/missing-blob-help         |  5 +++++
>>   tools/binman/test/277_rockchip_tpl.dts | 16 ++++++++++++++++
>>   5 files changed, 62 insertions(+)
>>   create mode 100644 tools/binman/etype/rockchip_tpl.py
>>   create mode 100644 tools/binman/test/277_rockchip_tpl.dts
> 
> Hi Jonas,
> 
> Is it possible to add the filename 'rockchip-tpl' to gitignore, such 
> that it won't show up all the time when you do git status ?
> (in the case where you place the rockchip-tpl in the same dir )

You should not need to name the file 'rockchip-tpl', instead of placing a
file in your u-boot folder, use the ROCKCHIP_TPL env variable like:

export BL31=../rkbin/bin/rk35/rk3588_bl31_v1.36.elf
export ROCKCHIP_TPL=../rkbin/bin/rk35/rk3588_ddr_lp4_2112MHz_lp5_2736MHz_v1.09.bin
make

or possible a make argument:

make ROCKCHIP_TPL=../rkbin/bin/rk35/rk3588_ddr_lp4_2112MHz_lp5_2736MHz_v1.09.bin

Regards,
Jonas

> 
> Otherwise, I tested your series:
> 
> Tested-by: Eugen Hristev <eugen.hristev@collabora.com>
> 
> Eugen

Re: [PATCH v2 4/6] rockchip: mkimage: Update init size limit

[Jonas Karlman]

Hi Kever,

On 2023-02-16 08:59, Kever Yang wrote:
> Hi Jonas,
> 
> On 2023/2/14 18:33, Jonas Karlman wrote:
>> Sync init size limit from vendor u-boot and the SRAM size specified in a
>> SoCs TRM. Size is typically defined using the following expression:
>> <SRAM size> - <BootRom stack size>
> 
> Although most of SoC follow this rule, but not for all.
> 
> So please just do the sync from vendor u-boot, but not extra modify 
> under this rule.

Sure, I will only sync from vendor u-boot.

After such sync init size limit changes for:
px30: +2KiB -> 12KiB
rk3066: +2KiB -> 32KiB
rk3328: +2KiB -> 30KiB
rk3568: -16KiB -> 60KiB

Regards,
Jonas

> 
> Thanks,
> 
> - Kever
> 
>>
>> This makes it possible to use latest vendor TPL with RK3328 without
>> getting a size limit error running the mkimage command.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - new patch
>>
>>   tools/rkcommon.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
>> index 1f1eaa16752b..7ad4780daf31 100644
>> --- a/tools/rkcommon.c
>> +++ b/tools/rkcommon.c
>> @@ -121,20 +121,20 @@ struct spl_info {
>>   };
>>   
>>   static struct spl_info spl_infos[] = {
>> -	{ "px30", "RK33", 0x2800, false, RK_HEADER_V1 },
>> -	{ "rk3036", "RK30", 0x1000, false, RK_HEADER_V1 },
>> +	{ "px30", "RK33", 0x4000 - 0x1000, false, RK_HEADER_V1 },
>> +	{ "rk3036", "RK30", 0x2000 - 0x1000, false, RK_HEADER_V1 },
>>   	{ "rk3066", "RK30", 0x8000 - 0x800, true, RK_HEADER_V1 },
>> -	{ "rk3128", "RK31", 0x1800, false, RK_HEADER_V1 },
>> +	{ "rk3128", "RK31", 0x2000 - 0x800, false, RK_HEADER_V1 },
>>   	{ "rk3188", "RK31", 0x8000 - 0x800, true, RK_HEADER_V1 },
>>   	{ "rk322x", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>> -	{ "rk3288", "RK32", 0x8000, false, RK_HEADER_V1 },
>> +	{ "rk3288", "RK32", 0x18000 - 0x1000, false, RK_HEADER_V1 },
>>   	{ "rk3308", "RK33", 0x40000 - 0x1000, false, RK_HEADER_V1 },
>> -	{ "rk3328", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>> -	{ "rk3368", "RK33", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>> +	{ "rk3328", "RK32", 0x8000 - 0x800, false, RK_HEADER_V1 },
>> +	{ "rk3368", "RK33", 0x10000 - 0x1000, false, RK_HEADER_V1 },
>>   	{ "rk3399", "RK33", 0x30000 - 0x2000, false, RK_HEADER_V1 },
>> -	{ "rv1108", "RK11", 0x1800, false, RK_HEADER_V1 },
>> +	{ "rv1108", "RK11", 0x2000 - 0x800, false, RK_HEADER_V1 },
>>   	{ "rv1126", "110B", 0x10000 - 0x1000, false, RK_HEADER_V1 },
>> -	{ "rk3568", "RK35", 0x14000 - 0x1000, false, RK_HEADER_V2 },
>> +	{ "rk3568", "RK35", 0x10000 - 0x1000, false, RK_HEADER_V2 },
>>   };
>>   
>>   /**

Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

[Simon Glass]

Hi Jonas,

On Wed, 15 Feb 2023 at 11:25, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2023-02-14 20:48, Simon Glass wrote:
> > Hi Jonas,
> >
> > On Tue, 14 Feb 2023 at 03:34, Jonas Karlman <jonas@kwiboo.se> wrote:
> >>
> >> Implement CheckMissing and CheckOptional methods that is adapted to
> >> Entry_mkimage in order to improve support for allow missing flag.
> >>
> >> Use collect_contents_to_file in multiple-data-files handling to improve
> >> support for allow missing and fake blobs handling.
> >>
> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >> ---
> >> Building for RK3568 without ROCKCHIP_TPL will result in the following
> >> error message, regardless if BINMAN_ALLOW_MISSING is used or not.
> >>
> >>   binman: Filename 'rockchip-tpl' not found in input path (...)
> >>
> >> With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob
> >> with no content is created and then passed to mkimage, resulting in an
> >> error from the mkimage command.
> >>
> >>   binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument
> >>
> >> If the --fake-ext-blobs argument is also used I get the message I was
> >> expecting to see from the beginning.
> >>
> >>   Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl
> >>
> >>   /binman/simple-bin/mkimage/rockchip-tpl:
> >>      An external TPL is required to initialize DRAM. Get the external TPL
> >>      binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
> >>      for the external TPL binary is https://github.com/rockchip-linux/rkbin>>>   Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl
> >>
> >>   Some images are invalid
> >>
> >> Not sure how this should work, but I was expecting to see the message
> >> about the missing rockchip-tpl from the beginning instead of the generic
> >> "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.
> >>
> >>  tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++----
> >>  1 file changed, 33 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> >> index cb264c3cad0b..44510a8c40ba 100644
> >> --- a/tools/binman/etype/mkimage.py
> >> +++ b/tools/binman/etype/mkimage.py
> >> @@ -153,10 +153,12 @@ class Entry_mkimage(Entry):
> >>          if self._multiple_data_files:
> >>              fnames = []
> >>              uniq = self.GetUniqueName()
> >> -            for entry in self._mkimage_entries.values():
> >> -                if not entry.ObtainContents(fake_size=fake_size):
> >> +            for idx, entry in enumerate(self._mkimage_entries.values()):
> >> +                entry_data, entry_fname, _ = self.collect_contents_to_file(
> >> +                    [entry], 'mkimage-%s' % idx, fake_size)
> >> +                if entry_data is None:
> >
> > This is OK, I suppose, but I'm not quite sure what actually changes
> > here, other than writing each entry to a file?
>
> The collect_contents_to_file function seemed to handle the
> external/missing/optional/faked entry flags. So I changed to use that
> function in order to see if that would change anything, see below.
>
> Use of this function does make it possible to use entry type other
> then external blobs, not sure if that would ever be needed/useful.
>
> >
> > Also, if you do this, please add / extend a test that checks that the
> > output files are written, since there is otherwise no coverage here.
> >
> > What test uses these optional mkimage pieces?
>
> Sorry, I was not clear enough about the reason for these changes in my
> message above.
>
> Building with --rockchip-tpl-path=/path/to/existing/tpl works as
> expected and generates a working image.
>
> I was expecting that the missing-blob-help message added in patch 1
> would be shown by binman when rockchip-tpl-path was empty/not-existing,
> or at least together with the allow-missing flag.
>
> However, whatever I tested, empty/none-existing rockchip-tpl-path,
> allow-missing, fake-ext-blobs, I was not able to see this message.
> Instead, all I could get from binman was this "Filename 'rockchip-tpl'
> not found in input path" message.
>
> Maybe my assumptions about when these missing messages should be shown
> is wrong?
>
> Trying binman with the following two dts and --allow-missing gives
> different results. First one shows the missing message, second one show
> filename not found.
>
> binman {
>         rockchip-tpl {
>         };
> };
>
> binman {
>         mkimage {
>                 args = "-n rk3568 -T rksd";
>                 multiple-data-files;
>
>                 rockchip-tpl {
>                 };
>         };
> };
>
> With the changes in this patch I instead get the missing message when I
> also add the --fake-ext-blobs flag, so it clearly needs more work or
> a completely different approach if we want to be able to see the missing
> message added in patch 1.
>
> Adding a message that never will be displayed annoys me :-)
> Maybe I should just drop this rfc/patch for a v3 of this series?
>

Does the mkimage etype need a CheckMissing() function? That is how
binman knows whether something is missing.

Regards,
Simon
[..]

Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

[Jonas Karlman]

Hi Simon,
On 2023-02-17 03:55, Simon Glass wrote:
> Hi Jonas,
> 
> On Wed, 15 Feb 2023 at 11:25, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Hi Simon,
>>
>> On 2023-02-14 20:48, Simon Glass wrote:
>>> Hi Jonas,
>>>
>>> On Tue, 14 Feb 2023 at 03:34, Jonas Karlman <jonas@kwiboo.se> wrote:
>>>>
>>>> Implement CheckMissing and CheckOptional methods that is adapted to
>>>> Entry_mkimage in order to improve support for allow missing flag.
>>>>
>>>> Use collect_contents_to_file in multiple-data-files handling to improve
>>>> support for allow missing and fake blobs handling.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>> Building for RK3568 without ROCKCHIP_TPL will result in the following
>>>> error message, regardless if BINMAN_ALLOW_MISSING is used or not.
>>>>
>>>>    binman: Filename 'rockchip-tpl' not found in input path (...)
>>>>
>>>> With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob
>>>> with no content is created and then passed to mkimage, resulting in an
>>>> error from the mkimage command.
>>>>
>>>>    binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument
>>>>
>>>> If the --fake-ext-blobs argument is also used I get the message I was
>>>> expecting to see from the beginning.
>>>>
>>>>    Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl
>>>>
>>>>    /binman/simple-bin/mkimage/rockchip-tpl:
>>>>       An external TPL is required to initialize DRAM. Get the external TPL
>>>>       binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source
>>>>       for the external TPL binary is https://github.com/rockchip-linux/rkbin   Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl
>>>>
>>>>    Some images are invalid
>>>>
>>>> Not sure how this should work, but I was expecting to see the message
>>>> about the missing rockchip-tpl from the beginning instead of the generic
>>>> "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.
>>>>
>>>>   tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++----
>>>>   1 file changed, 33 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
>>>> index cb264c3cad0b..44510a8c40ba 100644
>>>> --- a/tools/binman/etype/mkimage.py
>>>> +++ b/tools/binman/etype/mkimage.py
>>>> @@ -153,10 +153,12 @@ class Entry_mkimage(Entry):
>>>>           if self._multiple_data_files:
>>>>               fnames = []
>>>>               uniq = self.GetUniqueName()
>>>> -            for entry in self._mkimage_entries.values():
>>>> -                if not entry.ObtainContents(fake_size=fake_size):
>>>> +            for idx, entry in enumerate(self._mkimage_entries.values()):
>>>> +                entry_data, entry_fname, _ = self.collect_contents_to_file(
>>>> +                    [entry], 'mkimage-%s' % idx, fake_size)
>>>> +                if entry_data is None:
>>>
>>> This is OK, I suppose, but I'm not quite sure what actually changes
>>> here, other than writing each entry to a file?
>>
>> The collect_contents_to_file function seemed to handle the
>> external/missing/optional/faked entry flags. So I changed to use that
>> function in order to see if that would change anything, see below.
>>
>> Use of this function does make it possible to use entry type other
>> then external blobs, not sure if that would ever be needed/useful.
>>
>>>
>>> Also, if you do this, please add / extend a test that checks that the
>>> output files are written, since there is otherwise no coverage here.
>>>
>>> What test uses these optional mkimage pieces?
>>
>> Sorry, I was not clear enough about the reason for these changes in my
>> message above.
>>
>> Building with --rockchip-tpl-path=/path/to/existing/tpl works as
>> expected and generates a working image.
>>
>> I was expecting that the missing-blob-help message added in patch 1
>> would be shown by binman when rockchip-tpl-path was empty/not-existing,
>> or at least together with the allow-missing flag.
>>
>> However, whatever I tested, empty/none-existing rockchip-tpl-path,
>> allow-missing, fake-ext-blobs, I was not able to see this message.
>> Instead, all I could get from binman was this "Filename 'rockchip-tpl'
>> not found in input path" message.
>>
>> Maybe my assumptions about when these missing messages should be shown
>> is wrong?
>>
>> Trying binman with the following two dts and --allow-missing gives
>> different results. First one shows the missing message, second one show
>> filename not found.
>>
>> binman {
>>          rockchip-tpl {
>>          };
>> };
>>
>> binman {
>>          mkimage {
>>                  args = "-n rk3568 -T rksd";
>>                  multiple-data-files;
>>
>>                  rockchip-tpl {
>>                  };
>>          };
>> };
>>
>> With the changes in this patch I instead get the missing message when I
>> also add the --fake-ext-blobs flag, so it clearly needs more work or
>> a completely different approach if we want to be able to see the missing
>> message added in patch 1.
>>
>> Adding a message that never will be displayed annoys me :-)
>> Maybe I should just drop this rfc/patch for a v3 of this series?
>>
> 
> Does the mkimage etype need a CheckMissing() function? That is how
> binman knows whether something is missing.

I think I have found something that can work, will send a v3 of this series
without this rfc/path and instead send a separate series together with a
follow-up patch mentioned at [1].

[1] https://patchwork.ozlabs.org/project/uboot/patch/20230121190136.456341-6-jonas@kwiboo.se/#3044726

Regards,
Jonas

> 
> Regards,
> Simon
> [..]