Hi Fabio,

Am Do., 19. Aug. 2021 um 21:28 Uhr schrieb Fabio Estevam festevam@denx.de:

Clean the binaries generated by binman on imx8mm-evk: spl.* mkimage*.mkimage imx-boot.*

Reported-by: Frieder Schrempf frieder.schrempf@kontron.de Signed-off-by: Fabio Estevam festevam@denx.de

---

Changes since v2:

• None. Newly introducedin this series.

Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile index 3c8437d21a..7096fdf895 100644 --- a/Makefile +++ b/Makefile @@ -2095,7 +2095,8 @@ CLEAN_FILES += include/bmp_logo.h include/bmp_logo_data.h tools/version.h \ boot* u-boot* MLO* SPL System.map fit-dtb.blob* \ u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \ lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \

•          idbloader.img flash.bin flash.log defconfig keep-syms-lto.c
  

•          idbloader.img flash.bin flash.log defconfig keep-syms-lto.c \
  

•          spl.* mkimage*.mkimage imx-boot.*
  

it might be useful to use one variable for all BINMAN clean files. Otherwise it is difficult to understand by whom the files were created.

Something like that:

--- a/Makefile +++ b/Makefile @@ -2091,12 +2091,14 @@ CLEAN_DIRS += $(MODVERDIR) \ $(foreach d, spl tpl, $(patsubst %,$d/%, \ $(filter-out include, $(shell ls -1 $d 2>/dev/null))))

+BINMAN_CLEAN_FILES = spl.* mkimage*.mkimage + CLEAN_FILES += include/bmp_logo.h include/bmp_logo_data.h tools/version.h \ boot* u-boot* MLO* SPL System.map fit-dtb.blob* \ u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \ lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \ idbloader.img flash.bin flash.log defconfig keep-syms-lto.c \ - spl.* mkimage*.mkimage imx-boot.* + imx-boot.* $(BINMAN_CLEAN_FILES) +

I had the idea, if binman could create a list with the created files. This could then be used to set the files to be deleted. I think of other users where the output files have a different name. Then the files could be deleted "automatically" with clean.

Hi Fabio

On Mon, 2021-08-23 at 08:27 -0300, Fabio Estevam wrote:

Hi Heiko Thiery and Marcel,

On 19/08/2021 16:28, Fabio Estevam wrote:

...

diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi b/arch/arm/dts/imx8mm-evk-u-boot.dtsi index f200afac9f..75cd59e545 100644 --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi @@ -150,7 +150,7 @@         };

-       flash { +       spl {                 mkimage {                         args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x7e1000";

@@ -217,4 +217,19 @@                         };                 };         };

+       imx-boot { +               filename = "flash.bin"; +               pad-byte = <0x00>;

+               spl: blob-ext@1 { +                       offset = <0x0>; +                       filename = "spl.bin"; +               };

+               uboot: blob-ext@2 { +                       offset = <0x57c00>; +                       filename = "u-boot.itb"; +               }; +       };  };

Do we have a consensus on the binman format to generate a monolithic flash.bin?

Yes, I would say so. Remember, I did include this already in our latest verdin-imx8mm specific patch set together with its migration to using binman [1].

I understand that patch 2/2 still needs to be improved, but what about this one?

Yes, I agree.

Reviewed-by: Marcel Ziswiler marcel.ziswiler@toradex.com

[1] https://marc.info/?l=u-boot&m=162949278629882

Thanks,

Fabio Estevam

Cheers

Marcel

On Mon, 2021-08-23 at 13:55 +0200, Heiko Thiery wrote:

Hi Fabio,

Am Do., 19. Aug. 2021 um 21:28 Uhr schrieb Fabio Estevam festevam@denx.de:

...

After the conversion to binman in commit 8996e6b7c6a1 ("imx8mm_evk: switch to use binman to pack images"), it is necessary to flash both flash.bin and u-boot.itb to get a bootable system. Prior to this commit, only flash.bin was needed. and if it is not Such new requirement breaks existing distro mechanisms to generate the final binary because the extra u-boot.itb is now required.

Generate a final flash.bin that can be used again as a single bootable binary to keep the original behavior.

After this change the SPL binary is called spl.bin, which is a more descriptive name for its purpose, and can still be used standalone (for example, for secure boot purposes).

Also update imx8mm_evk.rst to remove the u-boot.itb copy step.

Signed-off-by: Fabio Estevam festevam@denx.de Reviewed-by: Frieder Schrempf frieder.schrempf@kontron.de Reviewed-by: Heiko Schocher hs@denx.de

Signed-off-by: Fabio Estevam festevam@denx.de

Changes since v2:

• Change the LOADER to mkimage.spl.mkimage (Frieder)

arch/arm/dts/imx8mm-evk-u-boot.dtsi             | 17 ++++++++++++++++-  .../imx8mm_evk/imximage-8mm-lpddr4.cfg          |  2 +-  doc/board/freescale/imx8mm_evk.rst              |  1 -  3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi b/arch/arm/dts/imx8mm-evk-u-boot.dtsi index f200afac9f..75cd59e545 100644 --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi @@ -150,7 +150,7 @@         };

-       flash { +       spl {                 mkimage {                         args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x7e1000";

@@ -217,4 +217,19 @@                         };                 };         };

+       imx-boot { +               filename = "flash.bin"; +               pad-byte = <0x00>;

+               spl: blob-ext@1 { +                       offset = <0x0>; +                       filename = "spl.bin"; +               };

+               uboot: blob-ext@2 { +                       offset = <0x57c00>; +                       filename = "u-boot.itb"; +               }; +       };  }; diff --git a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg b/board/freescale/imx8mm_evk/imximage-8mm- lpddr4.cfg index b89092a559..2c15dbc413 100644 --- a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg +++ b/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg @@ -6,4 +6,4 @@  #define __ASSEMBLY__

BOOT_FROM      sd -LOADER         mkimage.flash.mkimage   0x7E1000 +LOADER         mkimage.spl.mkimage     0x7E1000

I think the "mkimage.spl.mkimage" is a temporarily created file from binman. Isn't it correct to use the output file of the binman image u-boot-spl-ddr ("u-boot-spl-ddr.bin") here?

Yes, looking at it again, that really seems the proper one and I can confirm that this works fine.