public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [PATCH 4/5] env: allow default environment to be amended from control dtb
Date: Fri, 20 Nov 2020 11:13:10 +0100	[thread overview]
Message-ID: <13170.1605867190@gemini.denx.de> (raw)
In-Reply-To: <76615995-6700-1b3e-b598-4913e9882c26@prevas.dk>

Dear Rasmus,

In message <76615995-6700-1b3e-b598-4913e9882c26@prevas.dk> you wrote:
>
> >> The default environment is something which is NOT INTENDED for
> >> regular use.  it is what you will fall back to in case (and ONLY in
> >> that case) when your regular persistent environment cannot be used,
> >> for example because it is not readable (I/O errors or such) or not
> >> properly initialized or corrupted (CRC checksum error).
>
> You seem to be ignoring the many cases where one chooses, for
> simplicity, robustness and/or security, not to have a writable
> environment at all.

You seem to ignore what I write.

Yes, there are many use cases where no writable environment is
needed / wanted, but this is not what the default environment was
intended for.

For such cases some null driver similar to /dev/null should be used.

> I'm not really doing anything other than allowing a
> CONFIG_ENV_EXTRA_SETTINGS to live in .dtb rather than cooking all of it
> into the U-Boot binary itself. That's where board-specific additions are
> supposed to live nowadays I'd assume.

Your statement is incorrect.  CONFIG_ENV_EXTRA_SETTINGS is something
that is processed at compile time, while your code attempts to
modify the default environment in run time.  these are totally
different things.

> >> And please, for the sake of avoiding further confusiion, please do
> >> not name this "default-environment".
>
> Sure. So would you be ok with some /config/extra-environment node which
> gets appended after the normal environment has been loaded? Then if I

In principle yes, I see that this is a nice and useful feature.

Just the notation of "append" seems wrong to me.  We already have
"env import" which can import additional / new environmane settings
in different formats.  What I think should be done is extending this
by support for a new format (you may call it driver if you want)
to import environment settings from a DTB.

> set CONFIG_ENV_IS_NOWHERE, I'd get what I have now. The only problem

Can't you see that this is not logical?  If the environment is
nowhere, then how can you add something to it?

> with that is that it might be a little weird to have static content of
> the chosen control dtb override something that might have been loaded
> from a writable storage location. But that's not really an intended
> combination anyway, and I can probably add a knob that allows one to
> choose whether the .dtb settings should be applied or not.

This is not weird in any way - this is what the "env import" command
does by definition: it imports environment settigns from some other
storage.

> > I wonder if we should have a way to load the (whole) environment from DT?
>
> That will be my second choice, i.e. adding a "CONFIG_ENV_IS_IN_DTB"
> which obviously doesn't support saving, but has the advantages I'm after
> here of using one U-Boot binary with slightly different environments.

I see no difference here.  "env import" into an empty environment
does just that.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offense.                   - E. W. Dijkstra

  reply	other threads:[~2020-11-20 10:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 20:25 [PATCH 0/5] allow default environment to be amended from dtb Rasmus Villemoes
2020-11-10 20:25 ` [PATCH 1/5] fdtdec: make fdtdec_get_config_string() return const char* Rasmus Villemoes
2020-11-16 23:52   ` Simon Glass
2020-11-10 20:26 ` [PATCH 2/5] fdtdec: introduce fdtdec_get_config_property Rasmus Villemoes
2020-11-16 23:52   ` Simon Glass
2020-11-10 20:26 ` [PATCH 3/5] env: make env_set_default_vars() return void Rasmus Villemoes
2020-11-12 19:51   ` Wolfgang Denk
2020-11-10 20:26 ` [PATCH 4/5] env: allow default environment to be amended from control dtb Rasmus Villemoes
2020-11-12 19:58   ` Wolfgang Denk
2020-11-16 23:52     ` Simon Glass
2020-11-17  9:49       ` Rasmus Villemoes
2020-11-20 10:13         ` Wolfgang Denk [this message]
2020-11-20 10:33           ` Rasmus Villemoes
2020-11-20 10:53             ` Wolfgang Denk
2020-11-17 11:31       ` Wolfgang Denk
2020-11-17 18:23         ` Tom Rini
2020-11-18 14:37           ` Simon Glass
2020-11-20 10:28             ` Wolfgang Denk
2020-11-21 23:07               ` Simon Glass
2020-11-22 16:12                 ` Wolfgang Denk
2020-11-20 10:16           ` Wolfgang Denk
2020-11-17 10:01   ` Stefano Babic
2020-11-10 20:26 ` [PATCH 5/5] test: add tests for default environment Rasmus Villemoes

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=13170.1605867190@gemini.denx.de \
    --to=wd@denx.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox