Re: [PATCH RFC v3 03/11] ti: etype: sysfw: Add entry type for sysfw

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 15/06/2022 09:47, Neha Malcom Francis wrote:
> For K3 devices that require a sysfw image, add entry for SYSFW. It can
> contain system firmware image that can be packaged into sysfw.itb by
> binman.
> 
> Signed-off-by: Tarun Sahu <t-sahu@ti.com>
> [n-francis@ti.com: added tests for addition of etype]
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> ---
>  Makefile                           |  1 +
>  tools/binman/entries.rst           | 11 +++++++++++
>  tools/binman/etype/ti_sysfw.py     | 28 ++++++++++++++++++++++++++++
>  tools/binman/ftest.py              |  7 +++++++
>  tools/binman/test/232_ti_sysfw.dts | 13 +++++++++++++
>  5 files changed, 60 insertions(+)
>  create mode 100644 tools/binman/etype/ti_sysfw.py
>  create mode 100644 tools/binman/test/232_ti_sysfw.dts

Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

Minor points below.

> diff --git a/Makefile b/Makefile
> index 61927f8918..d20d264c53 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1345,6 +1345,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 ti-sysfw-path=$(SYSFW) \
>  		-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/tools/binman/entries.rst b/tools/binman/entries.rst
> index ae4305c99e..9fc5c48c35 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -1203,6 +1203,17 @@ This entry holds firmware for an external platform-specific coprocessor.
>  
>  
>  
> +Entry: ti-sysfw: Texas Instruments System Firmware (SYSFW) blob
> +------------------------------------------------------------
> +
> +Properties / Entry arguments:
> +    - ti-sysfw-path: Filename of file to read into the entry, typically sysfw.bin
> +
> +This entry contains system firmware necessary for booting of K3 architecture
> +devices.
> +
> +
> +

Regenerate these with `binman entry-docs >tools/binman/entries.rst` for
all your new entry types.

>  Entry: section: Entry that contains other entries
>  -------------------------------------------------
>  
> diff --git a/tools/binman/etype/ti_sysfw.py b/tools/binman/etype/ti_sysfw.py
> new file mode 100644
> index 0000000000..5b5b307030
> --- /dev/null
> +++ b/tools/binman/etype/ti_sysfw.py
> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> +#
> +# Entry type module for TI SYSFW binary blob
> +#
> +
> +import os
> +import struct
> +import sys
> +import zlib
> +
> +from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
> +from dtoc import fdt_util
> +from patman import tools

Remove unused imports.

> +
> +
> +class Entry_ti_sysfw(Entry_blob_named_by_arg):
> +    """Entry containing Texas Instruments System Firmware (SYSFW) blob
> +
> +    Properties / Entry arguments:
> +        - ti-sysfw-path: Filename of file to read into the entry, typically sysfw.bin
> +
> +    This entry contains system firmware necessary for booting of K3 architecture devices.
> +    """
> +
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node, 'ti-sysfw')
> +        self.external = True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index b5cf549703..671d083c54 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -87,6 +87,7 @@ ATF_BL31_DATA         = b'bl31'
>  TEE_OS_DATA           = b'this is some tee OS data'
>  ATF_BL2U_DATA         = b'bl2u'
>  OPENSBI_DATA          = b'opensbi'
> +TI_SYSFW_DATA         = b'sysfw'
>  SCP_DATA              = b'scp'
>  TEST_FDT1_DATA        = b'fdt1'
>  TEST_FDT2_DATA        = b'test-fdt2'
> @@ -195,6 +196,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('tee-pager.bin', TEE_OS_DATA)
>          TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
> +        TestFunctional._MakeInputFile('sysfw.bin', TI_SYSFW_DATA)
>          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
>  
>          # Add a few .dtb files for testing
> @@ -5529,6 +5531,11 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>          """Test an image with a pre-load header with an invalid key"""
>          with self.assertRaises(ValueError) as e:
>              data = self._DoReadFile('231_pre_load_invalid_key.dts')
> +    
> +    def testPackTiSysfw(self):
> +        """Test that an image with a SYSFW binary can be created"""
> +        data = self._DoReadFile('232_ti_sysfw.dts')
> +        self.assertEqual(TI_SYSFW_DATA, data[:len(TI_SYSFW_DATA)])
>  

Add new tests to the end of the file, renumber the dts files to be unique.

>      def _CheckSafeUniqueNames(self, *images):
>          """Check all entries of given images for unsafe unique names"""
> diff --git a/tools/binman/test/232_ti_sysfw.dts b/tools/binman/test/232_ti_sysfw.dts
> new file mode 100644
> index 0000000000..9e66cbe77b
> --- /dev/null
> +++ b/tools/binman/test/232_ti_sysfw.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	binman {
> +		ti-sysfw {
> +			filename = "sysfw.bin";
> +		};
> +	};
> +};