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

[Alexander Graf]

> Am 13.04.2016 um 19:31 schrieb Stephen Warren <swarren at wwwdotorg.org>:
> 
>> On 04/13/2016 11:21 AM, Alexander Graf wrote:
>> 
>> 
>>>> Am 13.04.2016 um 19:00 schrieb Stephen Warren <swarren at wwwdotorg.org>:
>>>> 
>>>>> On 04/13/2016 09:51 AM, Alexander Graf wrote:
>>>>>> On 04/13/2016 05:31 PM, Stephen Warren wrote:
>>>>>>> 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.
>>>> 
>>>> The point of all of the efi magic is that we can completely get rid of
>>>> boot scripts. Boards use the distro scripts, everything else gets
>>>> implicitly detected and executed. The way other boards deal with common
>>>> code mapping into separate boards is to either implement a "findfdt"
>>>> scriptlet or directly write the fdtfile variable (e.g. beaglebone) in
>>>> board init (e.g. rpi).
>>>> 
>>>>> 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.
>>>> 
>>>> s/un// :)
>>>> 
>>>> Just imagine a world where people don't have to worry about bootloaders
>>>> anymore. Things would "just work". You plug in a usb stick, it comes up,
>>>> boots Linux, everthing goes without anyone touching boot scripts,
>>>> downloading board specific files, etc. You could get a random
>>>> distribution from a common download page from somewhere and just run it.
>>>> 
>>>> Well, you can also just look at any random x86 system. They get at least
>>>> that part pretty right these days.
>>> 
>>> Well, you can also get the same benefit using extlinux.conf, and without relying on EFI:-P
>>> 
>>> Anyway, nothing in your benefits-of-EFI statement implies that relying on $fdtfile being set is correct. That's a new requirement that didn't exist before. Either the requirement needs to be removed (e.g. using a default FDT filename such as "${soc}-${board}${boardver}.dts") or only enabling this functionality on boards that do set $fdtfile, since it relies on that.
>> 
>> On boards that fon't set fdtfile we just don't load it, because we can't find the file. So you're getting a working grub2 payload, but Linux gets an empty device tree unless you pass it in using the grub2 "devicetree" command.
>> 
>> It's really just a convenience helper. And a nice piece to the puzzle that by convention allows users to think less about u-boot internals. The efi code works fine without.
> 
> Except for the fact that it doesn't just work, since if it did there wouldn't be a need for the patch in this email thread.
> 
> I think the correct fix is to abandon this patch, and have the EFI code (i.e. the scripts in the distro boot commands most likely) calculate a default value for $fdtfile if it isn't already set. Something like:
> 
> if test -n "${fdtfile}"; then
>    set _fdt ${fdtfile};
> else
>    set _fdt ${soc}-${board}${boardver}.dtb;
> 
> ... and use ${_fdt} instead of relying on ${fdtfile}.
> 
> If you still want to pursue this patch, it should be enhanced to cover all boards where the EFI code is enabled and $fdtfile isn't already set somehow, which I expect is almost all U-Boot boards since the Kconfig option is "default y".

Only the ones that do distro bootcmd. I'll send a patch later. I really like the idea :).

Thanks!

Alex