From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexander Graf <agraf@suse.de>
Date: Tue, 16 Oct 2018 15:04:26 +0200
Subject: [U-Boot] [PATCH 2/2] ARM: qemu-arm: define fdt_addr_r
In-Reply-To: <20181015051417.GB32578@linaro.org>
References: <20181012050757.6925-1-takahiro.akashi@linaro.org>
 <20181012050757.6925-2-takahiro.akashi@linaro.org>
 <20181015040106.78c6f7a2@thinkpad> <20181015051417.GB32578@linaro.org>
Message-ID: <cf1fbe26-f998-a8d9-c21f-2ea9c490b92f@suse.de>
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 15.10.18 07:14, AKASHI Takahiro wrote:
> On Mon, Oct 15, 2018 at 04:01:06AM +0300, Tuomas Tynkkynen wrote:
>> Hi Takahiro,
>>
>> On Fri, 12 Oct 2018 14:07:57 +0900
>> AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>>
>>> This variable, fdt_addr_t, is missing in the current qemu-arm.h while
>>> it seems to be mandatory, at least, to run distro_bootcmd as expected.
>>> So just add its definition. A size of 1MB would be enough.
>>>
>>
>> In what way is this required for distro_bootcmd to work? At least in the
>> past I've tested qemu_arm64_defconfig and EFI boot with the Fedora
>> netinst image and it has worked fine in stock U-Boot.
>>
>> Note that these '-machine virt' based targets are slightly different
>> from real hardware in the sense that instead of loading a .dtb file
>> provided by the OS, the device tree is provided by QEMU. In the hunk
>> below you can see "fdt_addr=0x40000000\0" providing the address of
>> the QEMU-provided device tree which the distro scripts should be
>> using.
> 
> Yep, I know that.
> I was not clear; what distro bootcmd, or more specifically scan_dev_for_efi,
> tries to do regarding fdt handling is that, if "fdtfile" variable is defined,
> it supersedes qemu's own dtb (that is what "fdt_addr" points to), but
> this trick doesn't work expectedly if "fdt_addr_r" variable is not defined.
> 
>> I guess in principle having ${fdt_addr_r} set as well shouldn't hurt and
>> might be used for testing/unusual purposes.
> 
> I don't know whether the case is "unsual" or not.
> But doc/README.distro cleary says that fdt_addr_r is *mandatory*.
> If u-boot works without it, it's a bug, otherwise we must correct
> the doc (or scan_dev_for_efi in the first place?).

I agree. Boards that support distro boot *must* provide fdt_addr_r.
Otherwise distro scripts may fail unexpectedly because they assume the
variable is present.

> 
> Thanks,
> -Takahiro Akashi
> 
>> Glancing at cmd/pxe.c,
>> there is a problem though, in that if ${fdt_addr_r} were defined, a PXE
>> file using the FDTDIR directive would attempt loading a dtb file named
>> "<NULL>-qemu-arm.dtb" instead of falling back to using ${fdt_addr}.
>> That bug would need to be fixed first before applying this patch.

Well, and that load will fail and everyone's happy, no? IMHO we should
fall back to $fdtcontroladdr always, but the pxe boot path is not
something I would endorse onto anyone (what you want as PXE really is
called 'dhcp' in the efi_loader distro boot world)


Alex