| Message ID | 20230120082611.4131362-6-jonas@kwiboo.se |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: Align FIT images to SD/MMC block length | expand |
Hi Jonas, On Fri, 20 Jan 2023 at 01:26, Jonas Karlman <jonas@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 <jonas@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
Hi Simon, On 2023-01-20 20:19, Simon Glass wrote: > Hi Jonas, > > On Fri, 20 Jan 2023 at 01:26, Jonas Karlman <jonas@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 <jonas@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 */ This looks reasonable, I do however wonder if it will be more flexible if we can skip the fit,firmware-next prop altogether and just handle it with a fit,firmware prop alone, if we treat it like a string list. fit,firmware: List of possible entries, the first existing entry is used for the 'firmware' property. fit,loadables: Adds 'loadables' property with a list of all remaining existing entries in 'fit,firmware' and remaining generated loadables. That way we do not create a dependency between the images and configurations and make it possible to generate configs with different 'firmware' like in the test dts. Example for known entries 'atf-1', 'atf-2' and 'u-boot': fit,firmware = "u-boot"; firmware = "u-boot"; fit,loadables; loadables = "atf-1", "atf-2"; fit,firmware = "atf-1", "u-boot"; firmware = "atf-1"; fit,loadables; loadables = "u-boot", "atf-2"; fit,firmware = "fw-1", "u-boot"; firmware = "u-boot"; fit,loadables; loadables = "atf-1", "atf-2"; Regards, Jonas > > I also think the 'atf-1' should not appear in 'loadables' if it is in > 'firmware'. > > Regards, > Simon
Hi Jonas, On Fri, 20 Jan 2023 at 13:47, Jonas Karlman <jonas@kwiboo.se> wrote: > > Hi Simon, > > On 2023-01-20 20:19, Simon Glass wrote: > > Hi Jonas, > > > > On Fri, 20 Jan 2023 at 01:26, Jonas Karlman <jonas@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 <jonas@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 */ > > This looks reasonable, I do however wonder if it will be more flexible > if we can skip the fit,firmware-next prop altogether and just handle it > with a fit,firmware prop alone, if we treat it like a string list. > > fit,firmware: List of possible entries, the first existing entry is used > for the 'firmware' property. > > fit,loadables: Adds 'loadables' property with a list of all remaining existing > entries in 'fit,firmware' and remaining generated loadables. > > That way we do not create a dependency between the images and configurations > and make it possible to generate configs with different 'firmware' like in > the test dts. > > Example for known entries 'atf-1', 'atf-2' and 'u-boot': > > fit,firmware = "u-boot"; firmware = "u-boot"; > fit,loadables; loadables = "atf-1", "atf-2"; > > fit,firmware = "atf-1", "u-boot"; firmware = "atf-1"; > fit,loadables; loadables = "u-boot", "atf-2"; > > fit,firmware = "fw-1", "u-boot"; firmware = "u-boot"; > fit,loadables; loadables = "atf-1", "atf-2"; Yes that seems better, thanks. So things in fit,firmware are omitted if they are 'missing'. Regards, Simon > > Regards, > Jonas > > > > > I also think the 'atf-1' should not appear in 'loadables' if it is in > > 'firmware'. > > > > Regards, > > Simon >
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"; + }; + }; + }; + }; +};
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 <jonas@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