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

[Andreas Färber]

Am 13.04.2016 um 19:40 schrieb Stephen Warren:
> 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 at nvidia.com>
>>>>> Cc: Alexander Graf <agraf at suse.de>
>>>>> Signed-off-by: Andreas Färber <afaerber at 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.

Alex was arguing about benefits, not me. I am specifically objecting to
you telling me that the solution for a generic boot mechanism were for
the user to supply custom board information. That has nothing to do with
EFI as I underlined by referring to boot.scr as well.

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

That does not help me as developer since the environment is getting
updated with several of the EFI patches I'm testing. How would I know
when to do that? And as soon as I do reset the environment to default
it'll be gone again.

Not all boards actually have saveenv anyway.

>> 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
> 
> 
> :-(

See my other reply with another suggestion to be used at distro header
level.

But my point was that U-Boot != Linux filename in some cases. For
example, the dragonboard410c is using dragonboard410c.dts but in Linux
it's qcom/apq8016-sbc.dts. To make things worse, for arm64 they're in
vendor subdirectories, for arm they're flat. It's not as easy as you
make it sound.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)