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

[Roger Quadros <rogerq@kernel.org>]

On 06/05/2022 07:37, Neha Malcom Francis wrote:
> For K3 devices that require a sysfw image, add entry for SYSFW. It can

'can' or 'should'?

For binman, 'sysfw' and 'dm' (added in patch 4) are just binary blobs. correct?
Why can't you just use blob entry type?

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

cheers,
-roger

> ---
>  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
> 
> diff --git a/Makefile b/Makefile
> index 4b347d3603..581fbba4c3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1338,6 +1338,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..6c0f03b34f 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: 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.
> +
> +
> +
>  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
> +
> +
> +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 4ce181a066..ec408de334 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
> @@ -5522,6 +5524,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)])
>  
>  if __name__ == "__main__":
>      unittest.main()
> 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";
> +		};
> +	};
> +};