[U-Boot] [PATCH] jetson-tk1: Set fdtfile environment variable

[Stephen Warren <swarren@wwwdotorg.org>]

On 04/13/2016 11:22 AM, Andreas F?rber wrote:
> Am 13.04.2016 um 17:31 schrieb Stephen Warren:
>> On 04/13/2016 06:55 AM, Andreas F?rber wrote:
>>> Am 13.04.2016 um 14:48 schrieb Andreas F?rber:
>>>> The 4.5.0 kernel cannot cope with U-Boot's internal device tree, and the
>>>> distro boot commands are looking for $fdtfile, so provide it to avoid
>>>> having users supply a dumb boot.scr doing a setenv fdtfile ...; boot,
>>>> defeating the purpose of generic EFI boot.
>>>>
>>>> Cc: Stephen Warren <swarren@nvidia.com>
>>>> Cc: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Andreas F?rber <afaerber@suse.de>
>>>> ---
>>>>    include/configs/jetson-tk1.h | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/include/configs/jetson-tk1.h b/include/configs/jetson-tk1.h
>>>> index 59dbb20..82a4be4 100644
>>>> --- a/include/configs/jetson-tk1.h
>>>> +++ b/include/configs/jetson-tk1.h
>>>> @@ -63,6 +63,10 @@
>>>>    /* General networking support */
>>>>    #define CONFIG_CMD_DHCP
>>>>
>>>> +#define BOARD_EXTRA_ENV_SETTINGS \
>>>> +    "fdtfile=tegra124-jetson-tk1.dtb\0" \
>>>> +    ""
>>>
>>> Is there any more intelligent solution than doing this for each board?
>>
>> Yes, the distro boot scripts shouldn't be using $fdtfile unconditionally
>> since it's not guaranteed to be set. The model is that boot scripts
>> determine the FDT filename, and $fdtfile is an optional override.
>>
>> It looks like the hard-coded use of $fdtfile was added into the EFI
>> path, which I didn't get to review, and which shouldn't be enabled by
>> default but unfortunately is.
>
> As Alex described, you're entirely missing the point here.

Well, that's because the point you're making is re: the benefits of EFI, 
but that's not the point the patch is addressing nor the point I'm 
objecting to. The patch addresses the need for all boards to set 
$fdtfile. That is what I'm arguing about. The benefits of EFI aren't 
relevant to this discussion.

> The EFI bootloader is an alternative to a board-specific script, not an
> addition. The loading logic is all in the U-Boot environment and it
> needs to know what device tree to use without the user telling it:
>
> a) master branch searches for $fdtfile in various prefixes on the
> current boot device partition.
>
> a') We're testing a variation where we load $fdtfile from a different
> partition on the same device (i.e., from ext4/btrfs rather that fat).
>
> b) A pending patch exposes the internal U-Boot device tree.
>
> The former is what we need to boot today. For openSUSE we get the .dtb
> files from rpm packages built from the kernel.
>
> The latter would match the Tianocore/Aptio model where all board info
> comes from the firmware exclusively. As reported elsewhere it does not
> yet work on this board with your DT though; you yourselves discussed
> about differing cell sizes and node names.
>
> Now during my EFI testing I had to repeatedly remember to interrupt
> U-Boot and type:
>
> setenv fdtfile tegra124-jetson-tk1.dtb

You can always run "saveenv" here. Admittedly it's not a nice 
user-experience to have to do that, but it's a workaround that would 
work today.

> boot
>
> until I got so annoyed that I figured out this patch to make it permanent.
>
> The hikey and some other boards got their variable renamed to fit
> standardized $fdtfile, for dragonboard410c I sent a similar patch.
>
> My above question was more precisely: Can we automate supplying the
> $fdtfile at tegra124-common.h or tegra-common.h level instead of as in
> this patch manually at jetson-tk1.h level where I happened to notice?

As I mentioned in my other reply, it would be better if the EFI logic 
handled this, rather than requiring every board to solve the problem 
over and over.

> The Raspberry Pi has been supplying $fdtfile just fine (modulo the rev
> B), so I don't understand why you'd be against it now.

I have no objection to boards setting $fdtfile where they need to. Some 
U-Boot boards support multiple HW, so it's a base requirement that the 
code supply $fdtfile since there's no other way to know what the correct 
value is. Other boards only operate on a single piece of HW, and we 
shouldn't burden every config header (or other board-specific code/...) 
with defining this value since there's a reasonable default that core 
code could use. Rather, let's deal with it in some core code (not 
per-SoC, but U-Boot-wide), for example using the code snippet I posted 
in my other response.

> Thanks,
> Andreas
>
> P.S. Without a standardized $fdtfile you can't have a standard boot.scr
> either, so the generic mechanism becomes moot.

That's not true. the model there is to use ${soc} ${board} and 
${boardver} to construct it. I thought that was documented in 
doc/README.distro, but perhaps I only mentioned it in commit 
descriptions and scripts that build boot.scr, e.g.:

https://github.com/NVIDIA/tegra-uboot-scripts/blob/master/gen-uboot-script.py#L133

:-(