[PATCH] distro_bootcmd: Add support for loading user environment

[David Abdurachmanov]

On Fri, Jan 31, 2020 at 3:42 PM Jon Hunter <jonathanh at nvidia.com> wrote:
>
>
> On 31/01/2020 14:05, Tom Rini wrote:
> > On Fri, Jan 31, 2020 at 01:24:21PM +0000, Jon Hunter wrote:
> >>
> >> On 31/01/2020 12:42, Soeren Moch wrote:
> >>> On 31.01.20 11:59, Jon Hunter wrote:
> >>>> U-Boot supports loading a user environment from a file in the
> >>>> file-system. Therefore to make it easier for users to override the
> >>>> default environment, add support to the 'distro_bootcmd' to look for
> >>>> and load a user environment in a file called 'uEnv.txt' in the same
> >>>> locations where an extlinux.conf or boot script might be found.
> >>>
> >>> We already have boot script support, which can easily be used to modify
> >>> the environment. Do we really need to bloat the distro_boot machinery
> >>> further with environment import, that is quite limited in contrast to
> >>> boot scripts?
> >>
> >> If you are booting with an extlinux.conf file, as we do by default, then
> >> if this file is found, this is always booted from before you have the
> >> opportunity to run the boot.scr script.
> >>
> >> Furthermore if you did switch the order to boot from a boot.scr script
> >> before the extlinux.conf, but you just wanted to do some simple
> >> modifications of the environment before booting (ie. so the boot.scr
> >> does not actually boot the system), then as the code is today you get a
> >> 'echo SCRIPT FAILED: continuing...' message.
> >>
> >> Yes this could be changed, but just seems cleaner and simpler to add
> >> support to make changes to the environment before the extlinux.conf is
> >> loaded.
> >>
> >> Yes this does add more to the environment, but it is hardly significant
> >> bloat, but if that is a concern then we could always disable this by
> >> default and allow users to enable it.
> >
> > This is something I think we had talked about and rejected doing
> > initially.  Can you please expand on the use-case here, and why it's
> > perhaps not better handled via PREBOOT on some platforms?  Thanks!
>
> We have one specific use-case in a downstream version of u-boot where we
> store some parameters in the u-boot environment for tweaking the DT
> firmware on boot [0]. Now there are cases where we do not wish to modify
> DT at all and so want to clear these variables and I would like a way to
> automate the re-configuration of the u-boot environment without the user
> having to manually set these just by the presence of a uEnv.txt file on
> the disk.
>
> Admittedly, this particular use-case/scenario is not directly applicable
> to the mainline u-boot branch as there is no support for these
> environment variables for Tegra (yet). However, it seemed that this
> change would have some value in upstream u-boot as well to allow the
> user to change any default environment variables without having to
> re-compile and hence, I decided submit this change here.

>From Fedora/RISCV maintainer this sounds quite interesting. While
working on boot flow I do re-building U-Boot multiple times tweaking
the default environment (e.g. using preboot), but that's so annoying.
uEnv.txt would be a lot easier and probably easier to explain to users
who want to make quick modifications (as quick as modifying
extlinux.conf).

Also every time I re-build U-Boot I have to rebuild OpenSBI (which has
U-Boot as a payload right now).

>
> For example, if we wanted to tweak any of the default environment
> variables that are currently in 'include/configs/tegra210-common.h' we
> could. Now you may say if we are not tweaking these very often, then why
> not recompile, and we could, but given that we have the ability to load
> environment parameters from a file on disk, it seems as shame that in
> the current boot flow there is no way to make use of this extremely
> useful feature.
>
> Cheers!
> Jon
>
> [0]
> https://nv-tegra.nvidia.com/gitweb/?p=3rdparty/u-boot.git;a=blob;f=include/configs/tegra210-common.h;h=81a3a2a447efacf34227847997fcc50639f54999;hb=refs/heads/l4t/l4t-r32.3.1-v2016.07#l59
>
> --
> nvpublic