| Message ID | 20210819192802.2151226-1-festevam@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [v3,1/2] imx8mm-evk: Generate a single bootable flash.bin again | expand |
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. > > 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 > diff --git a/doc/board/freescale/imx8mm_evk.rst b/doc/board/freescale/imx8mm_evk.rst > index 7fd3d72564..b377c4de27 100644 > --- a/doc/board/freescale/imx8mm_evk.rst > +++ b/doc/board/freescale/imx8mm_evk.rst > @@ -50,7 +50,6 @@ Burn the flash.bin to MicroSD card offset 33KB: > .. code-block:: bash > > $sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc > - $sudo dd if=u-boot.itb of=/dev/sdc bs=1024 seek=384 conv=sync > > Boot > ---- > -- > 2.25.1 > When building from a clean checkout I see the following warnings. It seems that there are some dependency checks that are looking for the files in mkimage config file. AR arch/arm/lib/lib.a AS arch/arm/lib/crt0_aarch64_efi.o CC arch/arm/lib/reloc_aarch64_efi.o WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional AS arch/arm/mach-imx/imx8m/lowlevel_init.o CC arch/arm/mach-imx/imx8m/clock_slice.o CC arch/arm/mach-imx/imx8m/soc.o : : AS spl/arch/arm/lib/crt0_aarch64_efi.o CC spl/arch/arm/lib/reloc_aarch64_efi.o WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional AS spl/arch/arm/mach-imx/imx8m/lowlevel_init.o CC spl/arch/arm/mach-imx/imx8m/clock_slice.o CC spl/arch/arm/mach-imx/imx8m/soc.o : : COPY spl/u-boot-spl.bin SYM spl/u-boot-spl.sym WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional make[1]: Nothing to be done for 'SPL'. OBJCOPY u-boot.srec OBJCOPY u-boot-nodtb.bin
On Fri, 2021-08-20 at 07:41 +0200, Heiko Thiery wrote: > > ... > > When building from a clean checkout I see the following warnings. It > seems that there are some dependency checks that are looking for the > files in mkimage config file. > > AR arch/arm/lib/lib.a > AS arch/arm/lib/crt0_aarch64_efi.o > CC arch/arm/lib/reloc_aarch64_efi.o > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > AS arch/arm/mach-imx/imx8m/lowlevel_init.o > CC arch/arm/mach-imx/imx8m/clock_slice.o > CC arch/arm/mach-imx/imx8m/soc.o > : > : > AS spl/arch/arm/lib/crt0_aarch64_efi.o > CC spl/arch/arm/lib/reloc_aarch64_efi.o > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > AS spl/arch/arm/mach-imx/imx8m/lowlevel_init.o > CC spl/arch/arm/mach-imx/imx8m/clock_slice.o > CC spl/arch/arm/mach-imx/imx8m/soc.o > : > : > COPY spl/u-boot-spl.bin > SYM spl/u-boot-spl.sym > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > make[1]: Nothing to be done for 'SPL'. > OBJCOPY u-boot.srec > OBJCOPY u-boot-nodtb.bin Yes, I believe it really requires explicitly setting the filename as I previously suggested. https://marc.info/?l=u-boot&m=162940109314578
Hi Heiko and Marcel, On Fri, Aug 20, 2021 at 4:31 AM Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote: > > On Fri, 2021-08-20 at 07:41 +0200, Heiko Thiery wrote: > > > > ... > > > > When building from a clean checkout I see the following warnings. It > > seems that there are some dependency checks that are looking for the > > files in mkimage config file. > > > > AR arch/arm/lib/lib.a > > AS arch/arm/lib/crt0_aarch64_efi.o > > CC arch/arm/lib/reloc_aarch64_efi.o > > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > > AS arch/arm/mach-imx/imx8m/lowlevel_init.o > > CC arch/arm/mach-imx/imx8m/clock_slice.o > > CC arch/arm/mach-imx/imx8m/soc.o > > : > > : > > AS spl/arch/arm/lib/crt0_aarch64_efi.o > > CC spl/arch/arm/lib/reloc_aarch64_efi.o > > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > > AS spl/arch/arm/mach-imx/imx8m/lowlevel_init.o > > CC spl/arch/arm/mach-imx/imx8m/clock_slice.o > > CC spl/arch/arm/mach-imx/imx8m/soc.o > > : > > : > > COPY spl/u-boot-spl.bin > > SYM spl/u-boot-spl.sym > > WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional > > make[1]: Nothing to be done for 'SPL'. > > OBJCOPY u-boot.srec > > OBJCOPY u-boot-nodtb.bin > > Yes, I believe it really requires explicitly setting the filename as I previously suggested. > > > https://marc.info/?l=u-boot&m=162940109314578 Peng Fan has sent a patch fixing this: https://www.mail-archive.com/u-boot@lists.denx.de/msg414966.html Thanks
Hi Fabio, > > Yes, I believe it really requires explicitly setting the filename as I previously suggested. > > > > > > https://marc.info/?l=u-boot&m=162940109314578 > > Peng Fan has sent a patch fixing this: > https://www.mail-archive.com/u-boot@lists.denx.de/msg414966.html Ah ok. Thanks
On Fri, 2021-08-20 at 12:47 +0200, Heiko Thiery wrote: > Hi Fabio, > > > > Yes, I believe it really requires explicitly setting the filename as I previously suggested. > > > > > > > > > https://marc.info/?l=u-boot&m=162940109314578 > > > > Peng Fan has sent a patch fixing this: > > https://www.mail-archive.com/u-boot@lists.denx.de/msg414966.html > > Ah ok. Thanks Yes, I can confirm that Peng's patch fixes this. I guess, me explicitly adding the filename did not really change anything but due to me running make again it subsequently picked up the previously generated artifact. Sorry for the noise.
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? I understand that patch 2/2 still needs to be improved, but what about this one? Thanks, Fabio Estevam
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
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?
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.
Hi Heiko, On Mon, Aug 23, 2021 at 8:55 AM Heiko Thiery <heiko.thiery@gmail.com> wrote: > 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? I did as you suggested in v4. Please reply with your Reviewed-by if it looks good to you. Also, I dropped the 2/2 patch (cleaning of the binman files) as it needs improvement. Could you please take care of the binman files cleanup mechanism? Thanks, Fabio Estevam > > -- > Heiko
Hi Fabio, Am Mo., 23. Aug. 2021 um 14:16 Uhr schrieb Fabio Estevam <festevam@gmail.com>: > > Hi Heiko, > > On Mon, Aug 23, 2021 at 8:55 AM Heiko Thiery <heiko.thiery@gmail.com> wrote: > > > 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? > > I did as you suggested in v4. Please reply with your Reviewed-by if it > looks good to you. > > Also, I dropped the 2/2 patch (cleaning of the binman files) as it > needs improvement. > > Could you please take care of the binman files cleanup mechanism? Yes, I will see what I can do. -- Heiko
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 diff --git a/doc/board/freescale/imx8mm_evk.rst b/doc/board/freescale/imx8mm_evk.rst index 7fd3d72564..b377c4de27 100644 --- a/doc/board/freescale/imx8mm_evk.rst +++ b/doc/board/freescale/imx8mm_evk.rst @@ -50,7 +50,6 @@ Burn the flash.bin to MicroSD card offset 33KB: .. code-block:: bash $sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc - $sudo dd if=u-boot.itb of=/dev/sdc bs=1024 seek=384 conv=sync Boot ----
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. 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(-)