All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] jetson-tk1: Set fdtfile environment variable
Date: Wed, 13 Apr 2016 20:02:24 +0200	[thread overview]
Message-ID: <570E89B0.6070309@suse.de> (raw)
In-Reply-To: <570E848E.2060408@wwwdotorg.org>

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

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

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

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

Not all boards actually have saveenv anyway.

>> boot
>>
>> until I got so annoyed that I figured out this patch to make it
>> permanent.
>>
>> The hikey and some other boards got their variable renamed to fit
>> standardized $fdtfile, for dragonboard410c I sent a similar patch.
>>
>> My above question was more precisely: Can we automate supplying the
>> $fdtfile at tegra124-common.h or tegra-common.h level instead of as in
>> this patch manually at jetson-tk1.h level where I happened to notice?
> 
> As I mentioned in my other reply, it would be better if the EFI logic
> handled this, rather than requiring every board to solve the problem
> over and over.
> 
>> The Raspberry Pi has been supplying $fdtfile just fine (modulo the rev
>> B), so I don't understand why you'd be against it now.
> 
> I have no objection to boards setting $fdtfile where they need to. Some
> U-Boot boards support multiple HW, so it's a base requirement that the
> code supply $fdtfile since there's no other way to know what the correct
> value is. Other boards only operate on a single piece of HW, and we
> shouldn't burden every config header (or other board-specific code/...)
> with defining this value since there's a reasonable default that core
> code could use. Rather, let's deal with it in some core code (not
> per-SoC, but U-Boot-wide), for example using the code snippet I posted
> in my other response.
> 
>> Thanks,
>> Andreas
>>
>> P.S. Without a standardized $fdtfile you can't have a standard boot.scr
>> either, so the generic mechanism becomes moot.
> 
> That's not true. the model there is to use ${soc} ${board} and
> ${boardver} to construct it. I thought that was documented in
> doc/README.distro, but perhaps I only mentioned it in commit
> descriptions and scripts that build boot.scr, e.g.:
> 
> https://github.com/NVIDIA/tegra-uboot-scripts/blob/master/gen-uboot-script.py#L133
> 
> 
> :-(

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

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

Regards,
Andreas

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

  parent reply	other threads:[~2016-04-13 18:02 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-13 12:48 [U-Boot] [PATCH] jetson-tk1: Set fdtfile environment variable Andreas Färber
2016-04-13 12:55 ` Andreas Färber
2016-04-13 15:31   ` Stephen Warren
2016-04-13 15:51     ` Alexander Graf
2016-04-13 17:00       ` Stephen Warren
2016-04-13 17:21         ` Alexander Graf
2016-04-13 17:31           ` Stephen Warren
2016-04-13 17:40             ` Alexander Graf
2016-04-13 22:17           ` Tom Rini
2016-04-13 22:38             ` Alexander Graf
2016-04-13 22:49               ` Tom Rini
2016-04-13 17:42         ` Andreas Färber
2016-04-13 17:58           ` Stephen Warren
2016-04-13 18:17             ` Andreas Färber
2016-04-13 18:51               ` Stephen Warren
2016-04-13 20:27                 ` Andreas Färber
2016-04-14  4:43                   ` Stephen Warren
2016-04-13 22:29           ` Tom Rini
2016-04-15 21:15             ` Alexander Graf
2016-04-13 17:22     ` Andreas Färber
2016-04-13 17:40       ` Stephen Warren
2016-04-13 17:50         ` Alexander Graf
2016-04-13 18:01           ` Stephen Warren
2016-04-13 18:02         ` Andreas Färber [this message]
2016-04-13 22:05           ` Tom Rini
2016-04-13 23:14             ` Andreas Färber
2016-04-13 21:59     ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=570E89B0.6070309@suse.de \
    --to=afaerber@suse.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.