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

[Alexander Graf]

On 14.04.16 00:29, Tom Rini wrote:
> On Wed, Apr 13, 2016 at 07:42:11PM +0200, Andreas Färber wrote:
> [snip]
>> $fdtfile needs to be the Linux filename. It does not always follow the
>> same pattern as the U-Boot variables you suggest here.
>> CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
>> question to you.
>>
>> It's part of the generic mechanism, so not just select boards. Yet I was
>> told that all boards are expected to set their cacheline size (although
>> that is not a board but CPU property), so similarly we can (yes, newly)
>> desire all boards to provide DT related settings as well.
>>
>> If you would supply a feature-complete DT in the first place, we
>> wouldn't need $fdtfile here, but it seemed that that was not realistic
>> to expect for the upcoming U-Boot release.
> 
> So here's the thing.  Figuring out what the device tree to load is, and
> where it's going to reside is a sucky problem.  For most of the complex
> cases we do this today with "run findfdt".  Why?  Well, check out the
> implementations in "git grep -l findfdt=" right now.  It sounds like we
> need to figure out how to get EFI in line with everything else that
> U-Boot does/supports rather than to re-invent the wheel here.

Sure, I fully agree. Where exactly do you see the EFI bits
reimplementing the wheel here? We use the same logic as the rest of
U-Boot for this. Findfdt gets called before the distro boot command, so
we use that. If a board sets fdtfile, we use that. If it doesn't, with
v2 we fall back to the same logic as pxe boot for fdtfile naming fallbacks.

So in the end, I'd say it's pretty much in line :).


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stephen Warren]

On 04/13/2016 02:27 PM, Andreas Färber wrote:

Am 13.04.2016 um 20:51 schrieb Stephen Warren:

On 04/13/2016 12:17 PM, Andreas Färber wrote:

Am 13.04.2016 um 19:58 schrieb Stephen Warren:

On 04/13/2016 11:42 AM, Andreas Färber wrote:

Am 13.04.2016 um 19:00 schrieb Stephen Warren:

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.


$fdtfile needs to be the Linux filename. It does not always follow the
same pattern as the U-Boot variables you suggest here.
CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
question to you.


That pattern is a good default that at least historically applied to all
the systems where the distro bootcmds were enabled. Perhaps the set of
systems using the distro bootcmds has increased now so the default isn't
always applicable. Boards can set $fdtfile /if/ needed because of that,
but I don't think should be forced to in all cases where the default
makes sense.


I really don't care whether we set fdtfile and use $fdtfile or whether
we insert the filename string directly into the appropriate command
variable... My point is U-Boot via its jetson-tk1_defconfig / .config
knows this (or should know) better than any user.


Yes, U-Boot is a good place to put this information. I disagree that it
/has/ to come from the defconfig/.config file.


You're twisting my words around!


That wasn't intentional. You explicitly mentioned those two files, so I 
responded to that aspect of your statement.



I'm saying when I build U-Boot with
jetson-tk1_defconfig then I'm intentionally building it for the Jetson
TK1 board and not some random other board. In this case that uniquely
identifies the .dtb file via an entry in .config. It may or may not
choose to read EEPROMs or whatever heuristics it knows, but it will know
that it's not a BeagleBone Black and it doesn't need a user i.e.
boot.scr to be told that it's a Jetson TK1.


And it seemed to me
that variables were not exactly used sparingly in the distro mechanism
so far, so I don't see why not to populate that variable _if_ we know
what its value needs to be. Do you have any real reason for being
against populating fdtfile at whatever level turns out to be suitable?


Yes, $fdtfile} can be auto-populated /at whatver level turns out to be
suitable/. I think the only question is what "level" /is/ suitable.


I believe there is no argument that this patch will not be applied.


There's a double negative there so I may not be interpreting you correctly.


There's nothing to misinterpret here: this patch will not be applied.


(Note: This part of the response is intended to explain the 
communication issue; I'm not trying to be a nit-picking asshole, even 
though it might look like it)


Unfortunately, "I believe there is no argument that" can mean either 
"the following is true" or "the following is false". "No argument" 
simply means there's no argument; it doesn't imply anything much about 
the Boolean value of the rest of the sentence.



I believe this patch should not be applied.


Agreed.


My objections are:

a) This only solves the problem for a single board, yet the issue it
solves occurs on many/all boards. We need a solution that solves them all.


Partially agree. I doubt we can solve everything at once, as indicated.


If we did solve this problem by editing config.h files to just set 
fdtfile in the default environment, it should be pretty easy to find all 
the config.h files that enable the distro boot commands, work out the 
value fdtfile should have, and add those all in one patch or a series of 
patches. Or, disable EFI+distro_bootcmd on those patches until someone 
more familiar with those platforms can send a patch to set fdtfile 
correctly there.



b) Boards should not need to define this value given that it can be
calculated automatically.


Agree on the board part. However it seemed that you were generally
against defining fdtfile at whatever level. Misunderstanding?

I feel it is much easier if U-Boot does your

fdtfile=${soc}-${board}${boardrev}.dtb

than expecting a) boot.scr scripts from users and b) bootefi internal
implementation for some $_fdt to both recreate the same thing.


I'd prefer any new code to follow the same algorithms as existing code 
for obtaining $fdtfile values. pxe.c already has code that generates a 
default if fdtfile is missing. That logic could be re-used elsewhere.



What I am unclear about (not being too familiar with U-Boot's codebase)
is how the generic level would know that it should not define fdtfile
according to the generic scheme because it's already getting defined in
some other way. Because in the other cases fdtfile= was just a line
inmidst some 

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

[Andreas Färber]

Am 14.04.2016 um 00:05 schrieb Tom Rini:
> On Wed, Apr 13, 2016 at 08:02:24PM +0200, Andreas Färber wrote:
>> 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.
> 
> Ug, really?  Did someone mention that on the devicetree list or did it
> just sneak in?

Don't have the discussions handy, but I recall it was a conscious
decision to refactor arm64:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=ca5b34100c571658e605c5554aac374649593327

AMD Seattle was the first new one:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=419043609689d0aba9f727a6faf1ff406e269ecf

>  I know you're just the messenger here but having arm64
> be different from ARM and PowerPC (and MIPS?  Or is MIPS doing the
> vendor subdirectory too) seems like a bad idea...
[snip]

Quick arch/ survey:

Vendor subdirs:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/mips/boot/dts

Flat:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arc/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/c6x/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/cris/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/h8300/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/metag/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/microblaze/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/nios2/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/openrisc/boot/dts
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/boot/dts

Regards,
Andreas

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



signature.asc
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Tom Rini]

On Thu, Apr 14, 2016 at 12:38:22AM +0200, Alexander Graf wrote:
> 
> 
> On 14.04.16 00:17, Tom Rini wrote:
> > On Wed, Apr 13, 2016 at 07:21:27PM +0200, Alexander Graf wrote:
> >>
> >>
> >>> Am 13.04.2016 um 19:00 schrieb Stephen Warren :
> >>>
>  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 
> >>> Cc: Alexander Graf 
> >>> Signed-off-by: Andreas Färber 
> >>> ---
> >>>  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.
> > 
> > Well, hold up.  We are _not_ at the point right now where the device
> > tree we use is the kernel device tree.  We're trying to make sure there
> 
> If I understand correctly that depends on the board. On some systems
> like the pine64 the device tree is just a copy of the Linux device tree.
> I don't see why we couldn't aim for that model going forward.
> 
> And with that I'm not saying "this is how it works today". If you want a
> reasonable working system today you probably need to provide your own
> device tree at a well-known location. But thinking mid-term I don't see
> why a board author couldn't just keep the U-Boot device tree and the
> Linux device tree in sync.
> 
> Again, none of this is mandatory, required, or even recommended today. I
> just want to make sure that people see the potential and make the (in my
> opinion) best path be the easiest one to take :).

Yes, the long term goal is that everyone be able to share the same
canonical device tree, bootloader and OS.  And that's making progress
all around and I think devicetree.org will help there too.  But the
problem I want to make sure you see is that you can't just use the
kernel device tree on 

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

[Alexander Graf]

On 14.04.16 00:17, Tom Rini wrote:
> On Wed, Apr 13, 2016 at 07:21:27PM +0200, Alexander Graf wrote:
>>
>>
>>> Am 13.04.2016 um 19:00 schrieb Stephen Warren :
>>>
 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 
>>> Cc: Alexander Graf 
>>> Signed-off-by: Andreas Färber 
>>> ---
>>>  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.
> 
> Well, hold up.  We are _not_ at the point right now where the device
> tree we use is the kernel device tree.  We're trying to make sure there

If I understand correctly that depends on the board. On some systems
like the pine64 the device tree is just a copy of the Linux device tree.
I don't see why we couldn't aim for that model going forward.

And with that I'm not saying "this is how it works today". If you want a
reasonable working system today you probably need to provide your own
device tree at a well-known location. But thinking mid-term I don't see
why a board author couldn't just keep the U-Boot device tree and the
Linux device tree in sync.

Again, none of this is mandatory, required, or even recommended today. I
just want to make sure that people see the potential and make the (in my
opinion) best path be the easiest one to take :).

> are no cases of incompatible bindings, and we're trying to make sure we
> play nicely and get the canonical device tree to have everything we
> need.  But it seems risky at best to assume the loaded tree is the right
> tree for the kernel.

So would you prefer if the board manually specifies it as the right tree
for the kernel? I'm not sure that buys us anything really. If it's the
wrong one, booting will fail.


Alex

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

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

[Tom Rini]

On Wed, Apr 13, 2016 at 07:42:11PM +0200, Andreas Färber wrote:
[snip]
> $fdtfile needs to be the Linux filename. It does not always follow the
> same pattern as the U-Boot variables you suggest here.
> CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
> question to you.
> 
> It's part of the generic mechanism, so not just select boards. Yet I was
> told that all boards are expected to set their cacheline size (although
> that is not a board but CPU property), so similarly we can (yes, newly)
> desire all boards to provide DT related settings as well.
> 
> If you would supply a feature-complete DT in the first place, we
> wouldn't need $fdtfile here, but it seemed that that was not realistic
> to expect for the upcoming U-Boot release.

So here's the thing.  Figuring out what the device tree to load is, and
where it's going to reside is a sucky problem.  For most of the complex
cases we do this today with "run findfdt".  Why?  Well, check out the
implementations in "git grep -l findfdt=" right now.  It sounds like we
need to figure out how to get EFI in line with everything else that
U-Boot does/supports rather than to re-invent the wheel here.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Tom Rini]

On Wed, Apr 13, 2016 at 07:21:27PM +0200, Alexander Graf wrote:
> 
> 
> > Am 13.04.2016 um 19:00 schrieb Stephen Warren :
> > 
> >> 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 
> > Cc: Alexander Graf 
> > Signed-off-by: Andreas Färber 
> > ---
> >  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.

Well, hold up.  We are _not_ at the point right now where the device
tree we use is the kernel device tree.  We're trying to make sure there
are no cases of incompatible bindings, and we're trying to make sure we
play nicely and get the canonical device tree to have everything we
need.  But it seems risky at best to assume the loaded tree is the right
tree for the kernel.

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

Yes, but it's also something that requires some external logic.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Tom Rini]

On Wed, Apr 13, 2016 at 08:02:24PM +0200, Andreas Färber wrote:
[snip]
> 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.

Ug, really?  Did someone mention that on the devicetree list or did it
just sneak in?  I know you're just the messenger here but having arm64
be different from ARM and PowerPC (and MIPS?  Or is MIPS doing the
vendor subdirectory too) seems like a bad idea...

That said, in U-Boot it should be apq8016-sbc.dts at least.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Tom Rini]

On Wed, Apr 13, 2016 at 09:31:43AM -0600, 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 
> >>Cc: Alexander Graf 
> >>Signed-off-by: Andreas Färber 
> >>---
> >>  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.

Bah.  But the good news is we haven't done a release with EFI stuff yet,
so we can fix it.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Andreas Färber]

Am 13.04.2016 um 20:51 schrieb Stephen Warren:
> On 04/13/2016 12:17 PM, Andreas Färber wrote:
>> Am 13.04.2016 um 19:58 schrieb Stephen Warren:
>>> On 04/13/2016 11:42 AM, Andreas Färber wrote:
 Am 13.04.2016 um 19:00 schrieb Stephen Warren:
> 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.

 $fdtfile needs to be the Linux filename. It does not always follow the
 same pattern as the U-Boot variables you suggest here.
 CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
 question to you.
>>>
>>> That pattern is a good default that at least historically applied to all
>>> the systems where the distro bootcmds were enabled. Perhaps the set of
>>> systems using the distro bootcmds has increased now so the default isn't
>>> always applicable. Boards can set $fdtfile /if/ needed because of that,
>>> but I don't think should be forced to in all cases where the default
>>> makes sense.
>>
>> I really don't care whether we set fdtfile and use $fdtfile or whether
>> we insert the filename string directly into the appropriate command
>> variable... My point is U-Boot via its jetson-tk1_defconfig / .config
>> knows this (or should know) better than any user.
> 
> Yes, U-Boot is a good place to put this information. I disagree that it
> /has/ to come from the defconfig/.config file.

You're twisting my words around! I'm saying when I build U-Boot with
jetson-tk1_defconfig then I'm intentionally building it for the Jetson
TK1 board and not some random other board. In this case that uniquely
identifies the .dtb file via an entry in .config. It may or may not
choose to read EEPROMs or whatever heuristics it knows, but it will know
that it's not a BeagleBone Black and it doesn't need a user i.e.
boot.scr to be told that it's a Jetson TK1.

>> And it seemed to me
>> that variables were not exactly used sparingly in the distro mechanism
>> so far, so I don't see why not to populate that variable _if_ we know
>> what its value needs to be. Do you have any real reason for being
>> against populating fdtfile at whatever level turns out to be suitable?
> 
> Yes, $fdtfile} can be auto-populated /at whatver level turns out to be
> suitable/. I think the only question is what "level" /is/ suitable.
> 
>> I believe there is no argument that this patch will not be applied.
> 
> There's a double negative there so I may not be interpreting you correctly.

There's nothing to misinterpret here: this patch will not be applied.

> I believe this patch should not be applied.

Agreed.

> My objections are:
> 
> a) This only solves the problem for a single board, yet the issue it
> solves occurs on many/all boards. We need a solution that solves them all.

Partially agree. I doubt we can solve everything at once, as indicated.

> b) Boards should not need to define this value given that it can be
> calculated automatically.

Agree on the board part. However it seemed that you were generally
against defining fdtfile at whatever level. Misunderstanding?

I feel it is much easier if U-Boot does your

fdtfile=${soc}-${board}${boardrev}.dtb

than expecting a) boot.scr scripts from users and b) bootefi internal
implementation for some $_fdt to both recreate the same thing.

What I am unclear about (not being too familiar with U-Boot's codebase)
is how the generic level would know that it should not define fdtfile
according to the generic scheme because it's already getting defined in
some other way. Because in the other cases fdtfile= was just a line
inmidst some EXTRA_ENV_VARS or so define, not a define that I could
#ifndef on.

>> However I am strongly rejecting your attitude that everything is there
>> already with variables and that nothing new is needed. Something needs
>> to be done somewhere - and we need to figure out what exactly and where
>> for minimum impact to the release.
> 
> Everything is there.
> 
> $soc/$board are set by include/env_default.h, with the option for
> per-board configuration files to override those values if the default
> CONFIG_SYS_* are incorrect for some reason. Differences between
> CONFIG_SYS_* and DTB filenames should not be an argument against the
> default I proposed. So, those values are always available (well, the
> board must enable CONFIG_ENV_VARS_UBOOT_CONFIG to get them; part of
> using distro_bootcmd is either enabling that config option or setting
> $fdtfile some other way).
> 
> I put effort into ensuring that "${soc}-${board}${boardver}.dtb" was the
> correct filename for Tegra boards, and I believe this holds true for a
> variety of non-Tegra boards as well.

So the counter-example I already mentioned is 

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

[Stephen Warren]

On 04/13/2016 12:17 PM, Andreas Färber wrote:

Am 13.04.2016 um 19:58 schrieb Stephen Warren:

On 04/13/2016 11:42 AM, Andreas Färber wrote:

Am 13.04.2016 um 19:00 schrieb Stephen Warren:

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.


$fdtfile needs to be the Linux filename. It does not always follow the
same pattern as the U-Boot variables you suggest here.
CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
question to you.


That pattern is a good default that at least historically applied to all
the systems where the distro bootcmds were enabled. Perhaps the set of
systems using the distro bootcmds has increased now so the default isn't
always applicable. Boards can set $fdtfile /if/ needed because of that,
but I don't think should be forced to in all cases where the default
makes sense.


I really don't care whether we set fdtfile and use $fdtfile or whether
we insert the filename string directly into the appropriate command
variable... My point is U-Boot via its jetson-tk1_defconfig / .config
knows this (or should know) better than any user.


Yes, U-Boot is a good place to put this information. I disagree that it 
/has/ to come from the defconfig/.config file.


> And it seemed to me

that variables were not exactly used sparingly in the distro mechanism
so far, so I don't see why not to populate that variable _if_ we know
what its value needs to be. Do you have any real reason for being
against populating fdtfile at whatever level turns out to be suitable?


Yes, $fdtfile} can be auto-populated /at whatver level turns out to be 
suitable/. I think the only question is what "level" /is/ suitable.



I believe there is no argument that this patch will not be applied.


There's a double negative there so I may not be interpreting you correctly.

I believe this patch should not be applied.

My objections are:

a) This only solves the problem for a single board, yet the issue it 
solves occurs on many/all boards. We need a solution that solves them all.


b) Boards should not need to define this value given that it can be 
calculated automatically.



However I am strongly rejecting your attitude that everything is there
already with variables and that nothing new is needed. Something needs
to be done somewhere - and we need to figure out what exactly and where
for minimum impact to the release.


Everything is there.

$soc/$board are set by include/env_default.h, with the option for 
per-board configuration files to override those values if the default 
CONFIG_SYS_* are incorrect for some reason. Differences between 
CONFIG_SYS_* and DTB filenames should not be an argument against the 
default I proposed. So, those values are always available (well, the 
board must enable CONFIG_ENV_VARS_UBOOT_CONFIG to get them; part of 
using distro_bootcmd is either enabling that config option or setting 
$fdtfile some other way).


I put effort into ensuring that "${soc}-${board}${boardver}.dtb" was the 
correct filename for Tegra boards, and I believe this holds true for a 
variety of non-Tegra boards as well.


The proposed default $fdtfile value I mentioned above should be the 
default, since everything required to construct it is already there. 
Boards that use distro_bootcmd should ensure that they set those 
variables correctly to allow that default to work; that's the whole 
point of those variables. Anyone enabling distro_bootcmd on other 
platforms hopefully ensured those variables were set correctly. If not, 
we can surely go back and fix those platforms.


Do you have a handle on exactly how many boards don't set those 
variables in a way that default won't work for them? Can't we fix their 
$soc/$board values so that the default does work?


I believe the calculation of the default/fallback value for $fdtfile 
does need to happen at runtime. ${boardver} is by definition a runtime 
variable since it reflects the HW version. As such, we can't simply put 
the following into e.g. tegra-common.h:


#define fdtfile "${soc}-${board}${boardver}.dtb"

(or any equivalent of that is actually valid syntax, i.e. using C 
pre-processor macros)


(Right now, I believe we don't actually set $boardver at runtime since 
for the board where it matters we only support one HW revision at all. 
However, I don't want to break that runtime capability. If we do, we 
should just override $board for that board, and delete all references to 
$boardver. I vaguely recall that some non-Tegra boards might use 
$boardver though?)

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Andreas Färber]

Am 13.04.2016 um 19:58 schrieb Stephen Warren:
> On 04/13/2016 11:42 AM, Andreas Färber wrote:
>> Am 13.04.2016 um 19:00 schrieb Stephen Warren:
>>> 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.
>>
>> $fdtfile needs to be the Linux filename. It does not always follow the
>> same pattern as the U-Boot variables you suggest here.
>> CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
>> question to you.
> 
> That pattern is a good default that at least historically applied to all
> the systems where the distro bootcmds were enabled. Perhaps the set of
> systems using the distro bootcmds has increased now so the default isn't
> always applicable. Boards can set $fdtfile /if/ needed because of that,
> but I don't think should be forced to in all cases where the default
> makes sense.

I really don't care whether we set fdtfile and use $fdtfile or whether
we insert the filename string directly into the appropriate command
variable... My point is U-Boot via its jetson-tk1_defconfig / .config
knows this (or should know) better than any user. And it seemed to me
that variables were not exactly used sparingly in the distro mechanism
so far, so I don't see why not to populate that variable _if_ we know
what its value needs to be. Do you have any real reason for being
against populating fdtfile at whatever level turns out to be suitable?

I believe there is no argument that this patch will not be applied.
However I am strongly rejecting your attitude that everything is there
already with variables and that nothing new is needed. Something needs
to be done somewhere - and we need to figure out what exactly and where
for minimum impact to the release.

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 
> Cc: Alexander Graf 
> Signed-off-by: Andreas Färber 
> ---
>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 

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

[Stephen Warren]

On 04/13/2016 11:50 AM, Alexander Graf wrote:




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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
   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


So do all systems follow this scheme? We need to make sure that fdtfile ends up 
with the same value as what arch/arm{,64}/boot/dtb 

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

[Stephen Warren]

On 04/13/2016 11:42 AM, Andreas Färber wrote:

Am 13.04.2016 um 19:00 schrieb Stephen Warren:

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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
   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


You're late for that discussion, we had that months ago on this mailing
list. We already concluded that SUSE does not and will not generate
extlinux.conf; EFI is a boot mechanism that we already support from x86
and aarch64 and that there are tools for (e.g., grub-mkconfig), unlike
extlinux.conf. There was also a FOSDEM talk on extlinux.conf that can be
summarized as some people like it and there's nothing wrong with it but
it's not a one-size-fits-all solution for everyone, including non-Linux
OSes such as Haiku.


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.


$fdtfile needs to be the Linux filename. It does not always follow the
same pattern as the U-Boot variables you suggest here.
CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
question to you.


That pattern is a good default that at least historically applied to all 
the systems where the distro bootcmds were enabled. Perhaps the set of 
systems using the distro bootcmds has increased now so the default isn't 
always applicable. Boards can set $fdtfile /if/ needed because of that, 
but I don't think should be forced to in all cases where the default 
makes sense.



It's part of the generic mechanism, so not just select boards. Yet I was
told that all boards are expected to set their cacheline size (although
that is not a board but CPU property), so similarly we can (yes, newly)
desire all boards to provide DT related settings as well.


OK, but enabling the feature on boards where we know the requirements 
aren't met doesn't seem useful, since it won't work well, as evidenced 
by this patch.



If you would supply a feature-complete DT in the first place, we
wouldn't need $fdtfile here, but it seemed that that was not realistic
to expect for the upcoming U-Boot release.


Given the current primary DT source location, I don't think the issue is 
complete-vs-incomplete DTs at all. However, that's straying quite off-topic.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

> 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 
> Cc: Alexander Graf 
> Signed-off-by: Andreas Färber 
> ---
>   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, 

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

[Andreas Färber]

Am 13.04.2016 um 19:00 schrieb Stephen Warren:
> 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 
> Cc: Alexander Graf 
> Signed-off-by: Andreas Färber 
> ---
>   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

You're late for that discussion, we had that months ago on this mailing
list. We already concluded that SUSE does not and will not generate
extlinux.conf; EFI is a boot mechanism that we already support from x86
and aarch64 and that there are tools for (e.g., grub-mkconfig), unlike
extlinux.conf. There was also a FOSDEM talk on extlinux.conf that can be
summarized as some people like it and there's nothing wrong with it but
it's not a one-size-fits-all solution for everyone, including non-Linux
OSes such as Haiku.

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

$fdtfile needs to be the Linux filename. It does not always follow the
same pattern as the U-Boot variables you suggest here.
CONFIG_DEFAULT_DEVICE_TREE ".dtb" might work better, and that was my
question to you.

It's part of the generic mechanism, so not just select boards. Yet I was
told that all boards are expected to set their cacheline size (although
that is not a board but CPU property), so similarly we can (yes, newly)
desire all boards to provide DT related settings as well.

If you would supply a feature-complete DT in the first place, we
wouldn't need $fdtfile here, but it seemed that that was not realistic
to expect for the upcoming U-Boot release.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
   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

:-(
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

> Am 13.04.2016 um 19:31 schrieb Stephen Warren :
> 
>> On 04/13/2016 11:21 AM, Alexander Graf wrote:
>> 
>> 
 Am 13.04.2016 um 19:00 schrieb Stephen Warren :
 
> 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 
>>> Cc: Alexander Graf 
>>> Signed-off-by: Andreas Färber 
>>> ---
>>>  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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stephen Warren]

On 04/13/2016 11:21 AM, Alexander Graf wrote:




Am 13.04.2016 um 19:00 schrieb Stephen Warren :


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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
  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".

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Andreas Färber]

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 
>>> Cc: Alexander Graf 
>>> Signed-off-by: Andreas Färber 
>>> ---
>>>   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.

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

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.

Thanks,
Andreas

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

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

> Am 13.04.2016 um 19:00 schrieb Stephen Warren :
> 
>> 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 
> Cc: Alexander Graf 
> Signed-off-by: Andreas Färber 
> ---
>  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.


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stephen Warren]

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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
  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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
  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.



Alex

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[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 
Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
---
  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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Andreas Färber]

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 
> Cc: Alexander Graf 
> Signed-off-by: Andreas Färber 
> ---
>  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?

Andreas

> +
>  #include "tegra-common-usb-gadget.h"
>  #include "tegra-common-post.h"
>  
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot