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

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.

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__;!!OOPJ... .

• 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

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

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__;!!OOPJ... .

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

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

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

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";
  

• };
  

• };

+};

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

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

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

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 }, };

/**

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 }, };

  /**

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')

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%3E%3E%3E 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

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

Regards, Jonas

Regards, Simon [..]