Hi Jonas, On Fri, 20 Jan 2023 at 01:26, Jonas Karlman <jo...@kwiboo.se> wrote: > > In some cases it is desired for SPL to start TF-A instead of U-Boot > proper. Add support to prepend a list of strings to the loadables list > generated by the split-elf generator. > > Signed-off-by: Jonas Karlman <jo...@kwiboo.se> > --- > v2: > - New patch > > tools/binman/entries.rst | 5 +- > tools/binman/etype/fit.py | 13 +++- > tools/binman/ftest.py | 37 +++++++++++ > tools/binman/test/276_fit_loadables.dts | 87 +++++++++++++++++++++++++ > 4 files changed, 137 insertions(+), 5 deletions(-) > create mode 100644 tools/binman/test/276_fit_loadables.dts > > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst > index 78f95dae1a..0ffffd60f2 100644 > --- a/tools/binman/entries.rst > +++ b/tools/binman/entries.rst > @@ -724,6 +724,7 @@ split-elf > fit,loadables > Generates a `loadable = <...>` property with a list of the generated > nodes (including all nodes if this operation is used multiple times) > + Optional property value is prepended to the generated list value > > > Here is an example showing ATF, TEE and a device tree all combined:: > @@ -791,8 +792,8 @@ Here is an example showing ATF, TEE and a device tree all > combined:: > @config-SEQ { > description = "conf-NAME.dtb"; > fdt = "fdt-SEQ"; > - firmware = "u-boot"; > - fit,loadables; > + firmware = "atf-1"; > + fit,loadables = "u-boot"; > }; > }; > }; > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py > index bcb606f3f9..3c90b50b7e 100644 > --- a/tools/binman/etype/fit.py > +++ b/tools/binman/etype/fit.py > @@ -190,6 +190,7 @@ class Entry_fit(Entry_section): > fit,loadables > Generates a `loadable = <...>` property with a list of the > generated > nodes (including all nodes if this operation is used multiple > times) > + Optional property value is prepended to the generated list value > > > Here is an example showing ATF, TEE and a device tree all combined:: > @@ -257,8 +258,8 @@ class Entry_fit(Entry_section): > @config-SEQ { > description = "conf-NAME.dtb"; > fdt = "fdt-SEQ"; > - firmware = "u-boot"; > - fit,loadables; > + firmware = "atf-1"; > + fit,loadables = "u-boot"; > }; > }; > }; > @@ -533,7 +534,13 @@ class Entry_fit(Entry_section): > with fsw.add_node(node_name): > for pname, prop in node.props.items(): > if pname == 'fit,loadables': > - val = '\0'.join(self._loadables) + '\0' > + if type(prop.value) is str: > + val = [prop.value] > + elif type(prop.value) is list: > + val = prop.value > + else: > + val = [] > + val = '\0'.join(val + self._loadables) + '\0' > fsw.property('loadables', > val.encode('utf-8')) > elif pname == 'fit,operation': > pass > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > index cd27572571..053ae99bee 100644 > --- a/tools/binman/ftest.py > +++ b/tools/binman/ftest.py > @@ -6337,6 +6337,43 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > } > self.assertEqual(expected, props) > > + def testFitLoadables(self): > + """Test an image with an FIT with prepended loadables""" > + if not elf.ELF_TOOLS: > + self.skipTest('Python elftools not available') > + entry_args = { > + 'of-list': 'test-fdt1', > + 'default-dt': 'test-fdt1', > + 'atf-bl31-path': 'bl31.elf', > + 'tee-os-path': 'tee.elf', > + } > + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) > + data = self._DoReadFileDtb( > + '276_fit_loadables.dts', > + entry_args=entry_args, > + extra_indirs=[test_subdir])[0] > + > + dtb = fdt.Fdt.FromData(data) > + dtb.Scan() > + > + node = dtb.GetNode('/configurations/conf-uboot-1') > + self.assertEqual('u-boot', node.props['firmware'].value) > + self.assertEqual( > + ['atf-1', 'atf-2'], > + fdt_util.GetStringList(node, 'loadables')) > + > + node = dtb.GetNode('/configurations/conf-atf-1') > + self.assertEqual('atf-1', node.props['firmware'].value) > + self.assertEqual( > + ['u-boot', 'atf-1', 'atf-2'], > + fdt_util.GetStringList(node, 'loadables')) > + > + node = dtb.GetNode('/configurations/conf-tee-1') > + self.assertEqual('atf-1', node.props['firmware'].value) > + self.assertEqual( > + ['u-boot', 'tee', 'atf-1', 'atf-2'], > + fdt_util.GetStringList(node, 'loadables')) > + > > if __name__ == "__main__": > unittest.main() > diff --git a/tools/binman/test/276_fit_loadables.dts > b/tools/binman/test/276_fit_loadables.dts > new file mode 100644 > index 0000000000..66dbc1fdf6 > --- /dev/null > +++ b/tools/binman/test/276_fit_loadables.dts > @@ -0,0 +1,87 @@ > +// SPDX-License-Identifier: GPL-2.0+ > + > +/dts-v1/; > + > +/ { > + #address-cells = <1>; > + #size-cells = <1>; > + > + binman { > + fit { > + description = "test desc"; > + #address-cells = <1>; > + fit,fdt-list = "of-list"; > + > + images { > + u-boot { > + description = "test u-boot"; > + type = "standalone"; > + arch = "arm64"; > + os = "u-boot"; > + compression = "none"; > + load = <0x00000000>; > + entry = <0x00000000>; > + > + u-boot-nodtb { > + }; > + }; > + > + tee { > + description = "TEE"; > + type = "tee"; > + arch = "arm64"; > + os = "tee"; > + compression = "none"; > + load = <0x00200000>; > + > + tee-os { > + optional; > + }; > + }; > + > + @atf-SEQ { > + fit,operation = "split-elf"; > + description = "ARM Trusted Firmware"; > + type = "firmware"; > + arch = "arm64"; > + os = "arm-trusted-firmware"; > + compression = "none"; > + fit,load; > + fit,entry; > + fit,data; > + > + atf-bl31 { > + }; > + }; > + > + @fdt-SEQ { > + description = "test fdt"; > + type = "flat_dt"; > + compression = "none"; > + }; > + }; > + > + configurations { > + default = "@conf-uboot-DEFAULT-SEQ"; > + @conf-uboot-SEQ { > + description = "test config"; > + fdt = "fdt-SEQ"; > + firmware = "u-boot"; > + fit,loadables; > + }; > + @conf-atf-SEQ { > + description = "test config"; > + fdt = "fdt-SEQ"; > + firmware = "atf-1"; > + fit,loadables = "u-boot"; > + }; > + @conf-tee-SEQ { > + description = "test config"; > + fdt = "fdt-SEQ"; > + firmware = "atf-1"; > + fit,loadables = "u-boot", "tee"; > + }; > + }; > + }; > + }; > +}; > -- > 2.39.1 >
The problem with this is that aft-1 can be missing, in which case it is still referenced in the 'firmware' property. I suspect we need a way to say that the firmware should be something other than U-Boot, if it exists. How about: atf-SEQ { fit,operation = "split-elf"; fit,firmware-next; /* bumps 'u-boot' out of the 'firmware' spot if atf is present */ description = "ARM Trusted Firmware"; type = "firmware"; } fit,firmware = "u-boot"; /* default value for 'firmware' if no atf */ fit,loadables; /* ends up holding 'u-boot' too, if it is spilled by atf */ I also think the 'atf-1' should not appear in 'loadables' if it is in 'firmware'. Regards, Simon