Hi Marek, On Tue, 13 Jul 2021 at 08:53, Marek Vasut <ma...@denx.de> wrote: > > On 7/13/21 4:41 PM, Tom Rini wrote: > > On Tue, Jul 13, 2021 at 04:35:38PM +0200, Marek Vasut wrote: > >> On 7/13/21 3:47 PM, Tom Rini wrote: > >>> On Mon, Jul 12, 2021 at 11:01:24AM -0500, Alex G. wrote: > >>>> On 7/12/21 10:15 AM, Tom Rini wrote: > >>>>> On Mon, Jul 12, 2021 at 01:36:14PM +0800, Bin Meng wrote: > >>>>>> On Mon, Jul 12, 2021 at 1:21 PM Reuben Dowle <reuben.do...@4rf.com> > >>>>>> wrote: > >>>>>>> > >>>>>>> I submitted an almost identical patch. See > >>>>>>> https://github.com/u-boot/u-boot/commit/eb39d8ba5f0d1468b01b89a2a464d18612d3ea76 > >>>>>>> > >>>>>>> This patch eventually had to be reverted > >>>>>>> (https://github.com/u-boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114), > >>>>>>> because it was causing issues on some platforms that had FIT on 32 > >>>>>>> bit boundary. However I continue to use it in production code, as > >>>>>>> without it the boot on my platform aborts. > >>>>>>> > >>>>>>> I don't have time to investigate why this was happening, but you need > >>>>>>> to check this code won't just cause exactly the same faults. > >>>>>> > >>>>>> Thanks for your information. > >>>>>> > >>>>>> +Marek who did the revert > >>>>>> > >>>>>> The revert commit message says: > >>>>>> > >>>>>> "The commit breaks booting of fitImage by SPL, the system simply > >>>>>> hangs. This is because on arm32, the fitImage and all of its content > >>>>>> can be aligned to 4 bytes and U-Boot expects just that." > >>>>>> > >>>>>> I don't understand this. If an address is aligned to 8, it is already > >>>>>> aligned to 4, so how did this commit make the system hang on arm32? > >>>>> > >>>>> I think this had something to do with embedding contents somewhere in > >>>>> the image? There is a thread on the ML from then but I don't know how > >>>>> informative it will end up being. > >>>> > >>>> It's true that the flat devicetree spec requires an 8-byte alignment, > >>>> even > >>>> on 32-bit. The issues here are specific to u-boot. > >>>> > >>>> SPL and u-boot have to agree where u-boot's FDT is located. We'll look at > >>>> two cases: > >>>> 1) u-boot as a FIT (binary and FDT separately loaded) > >>>> 2) u-boot with embedded FDT > >>>> > >>>> In case (1) SPL must place the FDT at a location where u-boot will find > >>>> it. > >>>> The current logic is > >>>> SPL: fdt = ALIGN_4(u_boot + u_boot_size) > >>>> u-boot: fdt = ALIGN_4(u_boot + u_boot_size) > >>>> > >>>> In case (2), SPL's view of the FDT is not relevant, but instead the build > >>>> system must place the FDT correctly: > >>>> build: fdt >> u-boot.bin > >>>> u-boot: fdt = ALIGN_4(u_boot + u_boot_size) > >>>> > >>>> We have 3 places that must agree. A correct and complete patch could > >>>> change > >>>> all three, but one has to consider compatibility issues when crossing > >>>> u-boot > >>>> and SPL versions. > >>>> > >>>> I had proposed in the revert discussion that SPL use r2 or similar > >>>> mechanism > >>>> to pass the location of the FDT to u-boot. > >>> > >>> I'm not sure that we need to worry too much about mix-and-match > >>> SPL/U-Boot, but documenting what to go change if you must do it > >>> somewhere under doc/ would be good. I think we can just switch to > >>> ALIGN(8) not ALIGN(4) and be done with it? > >> > >> Remember, there is also falcon boot. And we definitely have to be able to > >> have old u-boot (SPL) boot new fitImage and vice versa. > > > > I don't follow you, sorry. But since you seem to have the best > > understanding of where all of the cases something could go wrong here, > > can you perhaps post an RFC patch? That is likely to be clearer than > > another long thread here. > > I don't follow you, sorry. I believe the revert did the right thing and > new systems should use mkimage -E when generating fitImages, to avoid > the string alignment problem. That is all.
Using -E should be optional and things really should work without it. Regards, Simon
