From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Thu, 14 Apr 2016 09:27:31 -0600
Subject: [U-Boot] [PATCH 1/2] efi_loader: Pass fdt address directly to
	bootefi cmd
In-Reply-To: <570F9FA4.8080704@suse.de>
References: <1460582546-128312-1-git-send-email-agraf@suse.de>
 <570ED425.40309@wwwdotorg.org> <570F9FA4.8080704@suse.de>
Message-ID: <570FB6E3.8040400@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 04/14/2016 07:48 AM, Alexander Graf wrote:
> On 04/14/2016 01:20 AM, Stephen Warren wrote:
>> On 04/13/2016 03:22 PM, Alexander Graf wrote:
>>> The bootefi cmd today fetches its device tree pointer from either the
>>> location appointed by "fdt addr" with a fallback to the U-Boot control
>>> fdt.
>>>
>>> This integration is unusual for U-Boot and diverges from the way we
>>> usually handle parameters to boot commands. So let's pass the fdt
>>> directly into the bootefi command instead and move the control fdt
>>> logic into the distro boot script.
>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>
>>> @@ -155,13 +154,7 @@ static unsigned long do_bootefi_exec(void *efi)
>>>        */
>>>       efi_save_gd();
>>>
>>> -    /* Update system table to point to our currently loaded FDT */
>>> -
>>> -    /* Fall back to included fdt if none was manually loaded */
>>> -    if (!fdt && gd->fdt_blob)
>>> -        fdt = (void *)gd->fdt_blob;
>>> -
>>> -    if (fdt) {
>>> +    if (fdt && !fdt_check_header(fdt)) {
>>>           /* Prepare fdt for payload */
>>>           fdt = copy_fdt(fdt);
>>>
>>> @@ -185,7 +178,7 @@ static unsigned long do_bootefi_exec(void *efi)
>>>           efi_add_memory_map(fdt_start, fdt_pages,
>>>                      EFI_BOOT_SERVICES_DATA, true);
>>>       } else {
>>> -        printf("WARNING: No device tree loaded, expect boot to
>>> fail\n");
>>> +        printf("WARNING: Invalid device tree, expect boot to fail\n");
>>
>> I'm not familiar with this code, so I'm not really sure what the
>> implication of this if/else is.
>>
>> Still, this looks like it's a move in the right direction. I do agree
>> the FDT address parameter likely should be optional, and if not
>> specified by the user default to whatever other bootz/booti default
>> too (I suspect $fdt_addr_r).
>
> So what bootz/booti do is to either take the 3rd parameter as fdt, have
> the fdt included in the image you boot (FIT or legacy combined images)
> and when it's loaded, they call
>
>    set_working_fdt_addr((ulong)images.ft_addr);
>
> which basically does the same as my previous "fdt addr". But I see no
> fallback to $fdt_addr_r. It just doesn't use an fdt if no fdt parameter
> / fit image was provided.

Ah yes, I guess that's true. No DTB parameter to the command means ATAGS 
boot for bootz at least, right? I'd expect booti to be different, but I 
haven't looked at that.