Hi Tom, On Thu, 24 Aug 2023 at 07:38, Tom Rini <tr...@konsulko.com> wrote: > > On Wed, Aug 23, 2023 at 09:02:53PM -0600, Simon Glass wrote: > > > In this early stage of using binman to produce output files, we are mostly > > seeing people using common extensions such as '.bin' and '.rom' > > > > But unusual extensions appear in some places. > > > > We would like 'buildman -k' to keep the build outputs, but this is hard if > > there is no consistency as to the extension used. > > > > This series adjusts binman to enforce just 4 extensions for output images: > > > > .bin > > .rom > > .itb > > .img > > > > Other extensions will produce an error. With this rule observed, buildman > > can keep the required files. > > > > Some patches are included to fix up some easy problems. But the following > > boards generate 'custMpk.pem' and it is not clear how to fix this, so I am > > asking for help from the maintainers: > > > > am62ax_evm_r5 am62x_evm_r5 am64x_evm_r5 am65x_evm_r5 > > am65x_evm_r5_usbdfu am65x_evm_r5_usbmsc am65x_hs_evm_r5 > > j7200_evm_r5 j721e_evm_r5 j721s2_evm_r5 verdin-am62_r5 > > am62ax_evm_a53 am62x_evm_a53 am64x_evm_a53 am65x_evm_a53 > > am65x_hs_evm_a53 j7200_evm_a72 j721e_evm_a72 j721s2_evm_a72 > > verdin-am62_a53 > > > > It looks like the .pem files are listed as top-level images, e.g.: > > > > &custmpk_pem { > > filename = "../../ti/keys/custMpk.pem"; > > }; > > > > but if the only objective is to pick up an existing file, it is better to > > set BINMAN_INDIRS to include that directory. Also we should only have > > simple leafnames in the 'filename' property, so the '../../ti/keys' is not > > correct. It makes it harder for people to get the files from other places. > > Making this easier is one of binman's goals. > > > > Most likely the custmpk_pem node can be removed and the .pem file can be > > included directly in the place that needs it, e.g. by adjusting the > > ti-secure-rom etype (or ex509_cert) to use tools.get_input_filename() when > > reading the key file. > > > > For example, this: > > > > custMpk { > > filename = "custMpk.pem"; > > custmpk_pem: blob-ext { > > filename = "../keys/custMpk.pem"; > > }; > > }; > > > > is really just copying a file from '../keys/custMpk.pem' to 'custMpk.pem' > > so is equivalent to: > > > > custMpk { > > type = "blob"; > > filename = "custMpk.pem"; > > } > > > > (note that blob-ext implies that the blob may be missing, so blob is a > > better choice, since the key cannot be missing) > > > > The fact that the .pem files are at the top level means that they are > > output images, which surely is not intended. They should be buried in the > > image description, at a lower level, as part of something else. > > > > So please take a loke at the above and see if the binman descriptions can > > be reworked slightly to follow these new rules. > > > > > > Simon Glass (6): > > binman: Require image filenames to have certain extensions > > buildman: Keep all permitted output files > > buildman: Show progress when regenerating the board.cfg file > > buildman: Start the clock when the build starts > > kontron_sl28: Use u-boot-update.bin instead of u-boot.update > > stm32mp15: Use u-boot-spl-stm32.bin instead of u-boot-spl.stm32 > > > > .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 2 +- > > arch/arm/dts/stm32mp15-u-boot.dtsi | 2 +- > > doc/board/kontron/sl28.rst | 4 ++-- > > doc/board/st/stm32mp1.rst | 18 +++++++++--------- > > include/configs/stm32mp15_dh_dhsom.h | 2 +- > > tools/binman/binman.rst | 5 +++++ > > tools/binman/etype/section.py | 3 +-- > > tools/binman/ftest.py | 12 ++++++++++-- > > tools/binman/image.py | 9 +++++++++ > > tools/binman/test/022_image_name.dts | 4 ++-- > > tools/binman/test/311_bad_image_name.dts | 17 +++++++++++++++++ > > tools/buildman/boards.py | 15 ++++++++++++--- > > tools/buildman/builder.py | 3 ++- > > tools/buildman/builderthread.py | 11 +++++++---- > > tools/buildman/control.py | 3 ++- > > 15 files changed, 81 insertions(+), 29 deletions(-) > > create mode 100644 tools/binman/test/311_bad_image_name.dts > > This doesn't seem to address the ones I pointed out on IRC as being the > first order (to me anyhow) problem of needing the _unsigned files on the > platforms you list above for PEM files.
I think it should be xxx-unsigned.bin instead of xxx.bin_unsigned Regards, Simon