On Fri, Jan 7, 2022 at 4:42 PM Tom Rini <tr...@konsulko.com> wrote: > On Fri, Jan 07, 2022 at 11:26:49PM +0100, Marcel Ziswiler wrote: > > Hi Tim and Tom > > > > On Fri, 2022-01-07 at 12:37 -0800, Tim Harvey wrote: > > > On Fri, Jan 7, 2022 at 12:27 PM Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > On Fri, Jan 07, 2022 at 12:24:57PM -0800, Tim Harvey wrote: > > > > > On Fri, Jan 7, 2022 at 12:08 PM Adam Ford <aford...@gmail.com> > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Jan 7, 2022 at 12:25 PM Tom Rini <tr...@konsulko.com> > wrote: > > > > > > > > > > > > > > On Fri, Jan 07, 2022 at 12:21:18PM -0600, Adam Ford wrote: > > > > > > > > On Fri, Jan 7, 2022 at 11:38 AM Tom Rini <tr...@konsulko.com> > wrote: > > > > > > > > > > > > > > > > > On Fri, Jan 07, 2022 at 09:27:05AM -0800, Tim Harvey wrote: > > > > > > > > > > On Thu, Jan 6, 2022 at 12:27 PM Tim Harvey < > thar...@gateworks.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Thu, Jan 6, 2022 at 11:18 AM ZHIZHIKIN Andrey > > > > > > > > > > > <andrey.zhizhi...@leica-geosystems.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hello Tom, > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > > From: U-Boot <u-boot-boun...@lists.denx.de> On > Behalf Of Tom Rini > > > > > > > > > > > > > Sent: Thursday, January 6, 2022 7:52 PM > > > > > > > > > > > > > To: u-boot@lists.denx.de > > > > > > > > > > > > > Cc: Tim Harvey <thar...@gateworks.com> > > > > > > > > > > > > > Subject: [PATCH] Revert "tree: imx: remove old fit > generator > > > > > > > > > script" > > > > > > > > > > > > > > > > > > > > > > > > > > This reverts commit > d9a6f0eed66a39206b13513ec914f14084c3bb73. > > > > > > > > > > > > > > > > > > > > > > > > > > For right now, it's too close to the release to > merge the series > > > > > > > > > that > > > > > > > > > > > > > allows for binman to be used to generate the final > images, and > > > > > > > > > also not > > > > > > > > > > > > > break CI, and then also merge all of the series > that convert > > > > > > > > > currently > > > > > > > > > > > > > broken platforms to use binman instead. So, bring > back this > > > > > > > > > script now > > > > > > > > > > > > > and remove it again for real after the release. > > > > > > > > > > > > > > > > > > > > > > > > Please note that this might not work, as the FIT > generator script > > > > > > > > > would > > > > > > > > > > > > generate ITS with '@' symbols which are not > compatible with mkimage > > > > > > > > > due > > > > > > > > > > > > to CVE-2021-27138. This revert should be > complemented with the fix to > > > > > > > > > > > > remove those '@' symbols as well. > > > > > > > > > > > > > > > > > > > > > > Correct, the revert is not enough anymore: > > > > > > > > > > > MKIMAGE u-boot.itb > > > > > > > > > > > u-boot.its:7.11-15.5: Warning (unit_address_vs_reg): > /images/uboot@1: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:16.9-21.5: Warning (unit_address_vs_reg): > /images/fdt@1: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:22.9-27.5: Warning (unit_address_vs_reg): > /images/fdt@2: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:28.9-33.5: Warning (unit_address_vs_reg): > /images/fdt@3: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:34.9-39.5: Warning (unit_address_vs_reg): > /images/fdt@4: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:40.9-45.5: Warning (unit_address_vs_reg): > /images/fdt@5: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:46.9-55.5: Warning (unit_address_vs_reg): > /images/atf@1: > > > > > > > > > > > node has a unit name, but no reg property > > > > > > > > > > > u-boot.its:60.12-65.5: Warning (unit_address_vs_reg): > > > > > > > > > > > /configurations/config@1: node has a unit name, but > no reg property > > > > > > > > > > > u-boot.its:66.12-71.5: Warning (unit_address_vs_reg): > > > > > > > > > > > /configurations/config@2: node has a unit name, but > no reg property > > > > > > > > > > > u-boot.its:72.12-77.5: Warning (unit_address_vs_reg): > > > > > > > > > > > /configurations/config@3: node has a unit name, but > no reg property > > > > > > > > > > > u-boot.its:78.12-83.5: Warning (unit_address_vs_reg): > > > > > > > > > > > /configurations/config@4: node has a unit name, but > no reg property > > > > > > > > > > > u-boot.its:84.12-89.5: Warning (unit_address_vs_reg): > > > > > > > > > > > /configurations/config@5: node has a unit name, but > no reg property > > > > > > > > > > > ./tools/mkimage: verify_header failed for FIT Image > support with exit > > > > > > > > > code 1 > > > > > > > > > > > Makefile:1433: recipe for target 'u-boot.itb' failed > > > > > > > > > > > make: *** [u-boot.itb] Error 1 > > > > > > > > > > > make: *** Deleting file 'u-boot.itb' > > > > > > > > > > > make: *** Waiting for unfinished jobs.... > > > > > > > > > > > > > > > > > > > > > > I don't know what had changed to cause this or when > (again, I stopped > > > > > > > > > > > worrying about it because I thought we were moving to > binman for this > > > > > > > > > > > release). There was a patch that resolved this from > Oliver at > > > > > > > > > > > > https://lists.denx.de/pipermail/u-boot/2021-August/457997.html but I > > > > > > > > > > > don't think that fully solves anything 'at this point' > either. > > > > > > > > > > > > > > > > > > > > > > Even with that applied to current master I then end up > with: > > > > > > > > > > > MKIMAGE flash.bin > > > > > > > > > > > ./tools/mkimage: Can't open spl/u-boot-spl-ddr.bin: No > such file or > > > > > > > > > directory > > > > > > > > > > > arch/arm/mach-imx/Makefile:167: recipe for target > 'flash.bin' failed > > > > > > > > > > > make[1]: *** [flash.bin] Error 1 > > > > > > > > > > > make[1]: *** Deleting file 'flash.bin' > > > > > > > > > > > Makefile:1526: recipe for target 'flash.bin' failed > > > > > > > > > > > > > > > > > > > > > > At some point over the past couple of months that > patch resolved the > > > > > > > > > > > building issue when using the FIT generator but I also > don't know what > > > > > > > > > > > else has changed that now causes that to not work. > > > > > > > > > > > > > > > > > > > > > > As Tom pointed out in another thread these build > failures did not get > > > > > > > > > > > caught by CI apparently because CI does a 'make all' > which did not > > > > > > > > > > > include the FIT images (that was accomplished with the > 'flash.bin' > > > > > > > > > > > target prior to binman conversion). > > > > > > > > > > > > > > > > > > > > > > Is it too late to apply the CI fix and the pending > binman conversions? > > > > > > > > > > > > > > > > > > > > > > I know that my series has been reviewed by Marcel [1] > and as far as I > > > > > > > > > > > know didn't get merged simply because of the CI issue. > It still > > > > > > > > > > > applies and produces a valid flash.bin image. > > > > > > > > > > > I was also able to merge Peng's series [2] which > converts > > > > > > > > > > > imx8mq_evk/imx8mq_phanbell/pico-imx8mq to binman and > was able to build > > > > > > > > > > > flash.bin images for them > > > > > > > > > > > > > > > > > > > > > > I tried to merge Adam's series that moves > imx8mm_beacon to binman [3] > > > > > > > > > > > and imx8mn_beacon to binman [4] but they no longer > apply due to > > > > > > > > > > > defconfig/Kconfig changes > > > > > > > > > > > > > > > > > > > > > > That still leaves the following unbuildable with > > > > > > > > > > > CONFIG_SPL_FIT_GENERATOR = > "arch/arm/mach-imx/mkimage_fit_atf.sh": > > > > > > > > > > > > > > > > > > > > > configs/cgtqmx8_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8mm-icore-mx8mm-ctouch2_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach- > > > > > > > > > imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8mm-icore-mx8mm-edimm2.2_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach- > > > > > > > > > imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8mm_beacon_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8mn_beacon_2g_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach- > > > > > > > > > imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8mn_beacon_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > configs/imx8qm_rom7720_a1_4G_defconfig:CONFIG_SPL_FIT_GENERATOR="arch/arm/mach- > > > > > > > > > imx/mkimage_fit_atf.sh" > > > > > > > > > > > > > > > > > > > > > > Tim > > > > > > > > > > > [1] > https://patchwork.ozlabs.org/project/uboot/list/?series=265765 > > > > > > > > > > > [2] > https://patchwork.ozlabs.org/project/uboot/list/?series=268380 > > > > > > > > > > > [3] > https://patchwork.ozlabs.org/project/uboot/list/?series=261640 > > > > > > > > > > > [4] > https://patchwork.ozlabs.org/project/uboot/list/?series=261822 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Tom, > > > > > > > > > > > > > > > > > > > > I'm not familiar with the U-boot CI tool. > > > > > > > > > > > > > > > > > > It's at > https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html > > > > > > > > > (and in-tree under doc/develop/ci_testing.rst). > > > > > > > > > > > > > > > > > > > Is it a show-stopper that it > > > > > > > > > > does not build for boards using binman for release? From > what you > > > > > > > > > > mentioned in another thread it was never building the > flash.bin target > > > > > > > > > > for the boards using the FIT generator anyway. > > > > > > > > > > > > > > > > > > Yes, breaking CI is a ship-stopper. That all of these > boards were not > > > > > > > > > previously building the final image as part of "all" is a > problem. > > > > > > > > > > > > > > > > > > So, here's what I'm at right now. I've grabbed Heiko's > patch. > > > > > > > > > Everything is currently visible at > > > > > > > > > > > > > > > > > > > https://patchwork.ozlabs.org/bundle/trini/2022-01-07-imx8-and-binman-updates/ > > > > > > > > > and with this, we get some boards building and complaining > as expected: > > > > > > > > > aarch64: w+ imx8mn_evk > > > > > > > > > +(imx8mn_evk) Image 'main-section' is missing external > blobs and is > > > > > > > > > non-functional: blob-ext@1 > > > > > > > > > blob-ext@2 blob-ext@3 blob-ext@4 > > > > > > > > > +(imx8mn_evk) Image 'main-section' is missing external > blobs and is > > > > > > > > > non-functional: blob-ext > > > > > > > > > +(imx8mn_evk) > > > > > > > > > +(imx8mn_evk) Some images are invalid > > > > > > > > > > > > > > > > > > But others are: > > > > > > > > > aarch64: + imx8mn_beacon_2g > > > > > > > > > +(imx8mn_beacon_2g) ===================== WARNING > ====================== > > > > > > > > > +(imx8mn_beacon_2g) This board uses > CONFIG_SPL_FIT_GENERATOR. Please > > > > > > > > > migrate > > > > > > > > > +(imx8mn_beacon_2g) to binman instead, to avoid the > proliferation of > > > > > > > > > +(imx8mn_beacon_2g) arch-specific scripts with no tests. > > > > > > > > > +(imx8mn_beacon_2g) > ==================================================== > > > > > > > > > +(imx8mn_beacon_2g) Image 'main-section' is missing > external blobs and is > > > > > > > > > non-functional: blob- > > > > > > > > > ext@1 blob-ext@2 blob-ext@3 blob-ext@4 > > > > > > > > > +(imx8mn_beacon_2g) binman: Error 1 running 'mkimage -d > > > > > > > > > ./mkimage.spl.mkimage -n spl/u-boot-spl > > > > > > > > > .cfgout -T imx8mimage -e 0x912000 > ./mkimage-out.spl.mkimage': > > > > > > > > > spl/u-boot-spl-ddr.bin: Can't ope > > > > > > > > > n: No such file or directory > > > > > > > > > +(imx8mn_beacon_2g) > > > > > > > > > +(imx8mn_beacon_2g) make[1]: *** [Makefile:1088: all] > Error 1 > > > > > > > > > +(imx8mn_beacon_2g) make: *** [Makefile:177: sub-make] > Error 2 > > > > > > > > > > > > > > > > > > > > > > > > > The beacon boards are mine. I can work on this one today. > Do I just grab > > > > > > > > the binman updates and apply it to master and fix it from > that starting > > > > > > > > point? > > > > > > > > > > > > > > So, yes, grabbing the series from the above link and applying > it on top > > > > > > > of master, and making builds work with fake binaries would be > very > > > > > > > helpful. As best I can tell there's still something missing > with making > > > > > > > fake blobs link and not fail. For example, imx8mm_venice is > part of the > > > > > > > series above but still fails with 'make > BINMAN_FAKE_EXT_BLOBS=1 ...': > > > > > > > Image 'main-section:u-boot-spl-ddr' has faked external blobs > and is non-functional: > > > > > > > lpddr4_pmu_train_1d_imem.bin lpddr4_pmu_train_1d_dmem.bin > lpddr4_pmu_train_2d_imem.bin > > > > > > > lpddr4_pmu_train_2d_dmem.bin > > > > > > > Image 'main-section' is missing external blobs and is > non-functional: atf_blob blob-ext > > > > > > > Wrote map file './imx-boot.map' to show errors > > > > > > > binman: Node '/binman/imx-boot/blob-ext@1': Offset 0x0 (0) > overlaps with previous entry '/binman/imx- > > > > > > > boot/uboot' ending at 0x1aee08 (1764872) > > > > > > > > > > > > > > > > > > > From what I can tell looking at the imx8mm_venice board is that > the imx8mm-u-boot.dtsi has the binman > > > > > > nodes to build the image, but > arch/arm/dts/imx8mm-venice-u-boot.dtsi duplicates that work, so there are > > > > > > two copies trying to occupy the same space. Deleting the > &binman node from the venice-u-boot.dtsi file > > > > > > appears to make the problem go away, and binman is still making > the image. > > > > > > > > > > > > > > > > Adam, > > > > > > > > > > I don't quite follow. I don't see any binman nodes in > > > > > arch/arm/dts/imx8mm-u-boot.dtsi. > > > > > > > > > > I know that Marcel submitted a patch that added them (and I'm not > sure > > > > > if he added them yet in a way that was compatible with multiple > fdt's) > > > > > but this hasn't been merged yet so I don't see any duplicate binman > > > > > nodes for venice. > > > > > > > > Right, so this is with > > > > > https://patchwork.ozlabs.org/bundle/trini/2022-01-07-imx8-and-binman-updates/ > > > > applied and I did have to manually apply Marcel's 4/7 and 7/7, so > maybe > > > > I mismerged something there too? The tree is also at > > > > > https://github.com/trini/u-boot/tree/WIP/2022-01-07-imx8-and-buildman-updates > > > > > > > > > > Tom, > > > > > > Thank you for helping with this. During the merge window we had a lot > > > of collisions because of things moving to defconfig that I think > > > caused a lot of imx patches to have to be rebased. Then as most of us > > > were submitting binman patches this CI issue popped up and as Stefano > > > was unavailable I think it all got left hanging. In the middle of that > > > Marcel's patch moved binman nodes to a common place so some rebasing > > > is needed there as well. Fun times! > > > > > > Yes, I couldn't get all from your patchwork bundle to apply - I didn't > > > try as hard as you to manually merge :) > > > > > > I pulled your tree and see Marcel's patches there now. With the binman > > > node removed from arch/arm/dts/imx8mm-venice-u-boot.dtsi venice works. > > > > I also quickly double checked everything and indeed somehow the removal > of the binman stuff from > > arch/arm/dts/imx8mm-venice-u-boot.dtsi which I had in my 4th patch > "[PATCH v3 4/7] arm64: dts: imx8mm: use > > common binman configuration" got lost. Otherwise it all looks good. > Thanks! > > I must have messed it up, sorry. I'm doing one more CI run right now of > Adam's versions of the patches (just to be extra safe) and then I'll > merge the branch. Thanks all! >
Thanks for your work and helping us get this merged while the imx maintainer is out. adam > > -- > Tom >