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.

Reply via email to