public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Alex G." <mr.nuke.me@gmail.com>
To: Tom Rini <trini@konsulko.com>, Bin Meng <bmeng.cn@gmail.com>
Cc: Reuben Dowle <reuben.dowle@4rf.com>, Marek Vasut <marex@denx.de>,
	Simon Glass <sjg@chromium.org>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>
Subject: Re: [PATCH] spl: Align device tree blob address at 8-byte boundary
Date: Mon, 12 Jul 2021 11:01:24 -0500	[thread overview]
Message-ID: <300cbb2d-a343-aff8-2c73-00a81ec05af3@gmail.com> (raw)
In-Reply-To: <20210712151510.GT9516@bill-the-cat>

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.dowle@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.

Alex

  parent reply	other threads:[~2021-07-12 16:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12  3:52 [PATCH] spl: Align device tree blob address at 8-byte boundary Bin Meng
2021-07-12  5:21 ` Reuben Dowle
2021-07-12  5:36   ` Bin Meng
2021-07-12 15:15     ` Tom Rini
2021-07-12 15:38       ` Marek Vasut
2021-07-12 15:43         ` Tom Rini
2021-07-12 15:51           ` Marek Vasut
2021-07-12 16:02             ` Tom Rini
2021-07-12 16:09               ` Marek Vasut
2021-07-12 16:01       ` Alex G. [this message]
2021-07-12 19:46         ` Simon Glass
2021-07-13  3:09         ` Bin Meng
2021-07-13 13:47         ` Tom Rini
2021-07-13 14:35           ` Marek Vasut
2021-07-13 14:41             ` Tom Rini
2021-07-13 14:53               ` Marek Vasut
2021-07-13 16:47                 ` Simon Glass
2021-07-13 17:50                   ` Marek Vasut
2021-07-13 18:11                     ` Tom Rini
2021-07-13 20:35                       ` Marek Vasut
2021-07-13 20:46                         ` Alex G
2021-07-13 21:11                         ` Tom Rini
2021-07-26 13:26                           ` Bin Meng
2021-07-26 13:38                             ` Tom Rini
2021-07-13 21:06                       ` Alex G
2021-07-13 17:20         ` Tom Rini
2021-07-13  3:00       ` Bin Meng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=300cbb2d-a343-aff8-2c73-00a81ec05af3@gmail.com \
    --to=mr.nuke.me@gmail.com \
    --cc=bmeng.cn@gmail.com \
    --cc=marex@denx.de \
    --cc=reuben.dowle@4rf.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox