All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] env: Allow environment in text files
@ 2021-10-02  0:38 Simon Glass
  2021-10-02  0:38 ` [PATCH v5 1/5] sandbox: Drop distro_boot Simon Glass
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass, Joe Hershberger

One barrier to completing the 7-year-long Kconfig migration is that
the default environment is implemented using ad-hoc CONFIG options.
At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS.

It is not really feasible to move the environment to Kconfig as it is
hundreds of lines of text in some cases.

Even considering the current situation, it is painful to add large
amounts of text to the config-header file and dealing with quoting and
newlines is harder than it should be. It would be better if we could just
type the script into a text file and have it included by U-Boot.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file in a board/<vendor>/env
directory called <board>.env (or common.env if you want the same
environment for all boards).

The environment variables should be of the form "var=value". Values can
extend to multiple lines. This series converts the existing environment
documentation to rST and updates it to explain how to use this.

Note: this series was originally sent eight years ago:

https://patchwork.ozlabs.org/project/uboot/patch/1382763695-2849-4-git-send-email-sjg@chromium.org/

It has been updated to work with Kconfig, etc. Some review comments in
that patch were infeasible so I have not addressed them. I would like
this series to be considered independently, on its merits.

Rather than deal with the complexity of rewriting the distro-boot
script, this is disabled for sandbox. The forthcoming bootmethod approach
should provide the same functionality without needing the complex
scripting in the environment.

Migration needs more thought, although it can be done later. It may be
possible to do migrate automatically, using buildman to extract the
built-in environmnent from the ELF file.

This would produce a pretty ugly conversion though, since it would drop
all the intermediate variables used to create the environment.

Better would be to parse the config.h file, figure out the components of
CONFIG_EXTRA_ENV_SETTINGS then output these as separate pieces in the
file. It is not clear how easy that would be, nor whether the result would
be very pretty. Also the __stringify() macro needs to be handled somehow.

This series is available at u-boot-dm/env-working

Comments welcome.

Changes in v5:
- Minor updates as suggested by Wolfgang
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer

Changes in v4:
- Add new patch to move environment documentation to rST
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang
- Add documentation in rST format instead of README
- Move use of += to this patch
- Explain that environment variables may not end in +

Changes in v3:
- Adjust Makefile to generate the .inc and .h files in separate fules
- Add more detail in the README about the format of .env files
- Improve the comment about " in the awk script
- Correctly terminate environment files with \n
- Define __UBOOT_CONFIG__ when collecting environment files
- Add new patch to use a text-based environment for sandbox

Changes in v2:
- Move .env file from include/configs to board/
- Use awk script to process environment since it is much easier on the brain
- Add information and updated example script to README
- Add dependency rule so that the environment is rebuilt when it changes
- Add separate patch to enable C preprocessor for environment files
- Enable var+=value form to simplify composing variables in multiple steps

Simon Glass (5):
  sandbox: Drop distro_boot
  doc: Move environment documentation to rST
  env: Allow U-Boot scripts to be placed in a .env file
  env: Allow environment files to use the C preprocessor
  sandbox: Use a text-based environment

 MAINTAINERS                   |   7 +
 Makefile                      |  39 ++-
 README                        | 328 -------------------------
 board/sandbox/env/sandbox.env |  25 ++
 config.mk                     |   2 +
 doc/usage/environment.rst     | 444 ++++++++++++++++++++++++++++++++++
 doc/usage/index.rst           |   1 +
 env/embedded.c                |   1 +
 include/configs/sandbox.h     |  40 ---
 include/env_default.h         |   8 +
 scripts/env2string.awk        |  56 +++++
 11 files changed, 582 insertions(+), 369 deletions(-)
 create mode 100644 board/sandbox/env/sandbox.env
 create mode 100644 doc/usage/environment.rst
 create mode 100644 scripts/env2string.awk

-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 1/5] sandbox: Drop distro_boot
  2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
@ 2021-10-02  0:38 ` Simon Glass
  2021-10-02  0:38 ` [PATCH v5 2/5] doc: Move environment documentation to rST Simon Glass
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass

This is a complicated set of #defines and it is painful to convert to a
text file. We can (once pending patches are applied) provide the same
functionality with bootmethod. Drop this for sandbox to allow conversion
to a text-file environment.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/configs/sandbox.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 24c9a84fa35..c19232f202f 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -49,16 +49,6 @@
 #define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, 57600,\
 					115200}
 
-#define BOOT_TARGET_DEVICES(func) \
-	func(HOST, host, 1) \
-	func(HOST, host, 0)
-
-#ifdef __ASSEMBLY__
-#define BOOTENV
-#else
-#include <config_distro_bootcmd.h>
-#endif
-
 #define CONFIG_KEEP_SERVERADDR
 #define CONFIG_UDP_CHECKSUM
 #define CONFIG_TIMESTAMP
@@ -103,7 +93,6 @@
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	SANDBOX_SERIAL_SETTINGS \
 	SANDBOX_ETH_SETTINGS \
-	BOOTENV \
 	MEM_LAYOUT_ENV_SETTINGS
 
 #ifndef CONFIG_SPL_BUILD
-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/5] doc: Move environment documentation to rST
  2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
  2021-10-02  0:38 ` [PATCH v5 1/5] sandbox: Drop distro_boot Simon Glass
@ 2021-10-02  0:38 ` Simon Glass
  2021-10-04 12:05   ` Wolfgang Denk
  2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass

Move this from the README to rST format.

Drop i2cfast since it is obviously obsolete and breaks the formatting.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Minor updates as suggested by Wolfgang

Changes in v4:
- Add new patch to move environment documentation to rST

 README                    | 328 --------------------------------
 doc/usage/environment.rst | 390 ++++++++++++++++++++++++++++++++++++++
 doc/usage/index.rst       |   1 +
 3 files changed, 391 insertions(+), 328 deletions(-)
 create mode 100644 doc/usage/environment.rst

diff --git a/README b/README
index 28eb9203595..1fbc457462b 100644
--- a/README
+++ b/README
@@ -3007,334 +3007,6 @@ TODO.
 For now: just type "help <command>".
 
 
-Environment Variables:
-======================
-
-U-Boot supports user configuration using Environment Variables which
-can be made persistent by saving to Flash memory.
-
-Environment Variables are set using "setenv", printed using
-"printenv", and saved to Flash using "saveenv". Using "setenv"
-without a value can be used to delete a variable from the
-environment. As long as you don't save the environment you are
-working with an in-memory copy. In case the Flash area containing the
-environment is erased by accident, a default environment is provided.
-
-Some configuration options can be set using Environment Variables.
-
-List of environment variables (most likely not complete):
-
-  baudrate	- see CONFIG_BAUDRATE
-
-  bootdelay	- see CONFIG_BOOTDELAY
-
-  bootcmd	- see CONFIG_BOOTCOMMAND
-
-  bootargs	- Boot arguments when booting an RTOS image
-
-  bootfile	- Name of the image to load with TFTP
-
-  bootm_low	- Memory range available for image processing in the bootm
-		  command can be restricted. This variable is given as
-		  a hexadecimal number and defines lowest address allowed
-		  for use by the bootm command. See also "bootm_size"
-		  environment variable. Address defined by "bootm_low" is
-		  also the base of the initial memory mapping for the Linux
-		  kernel -- see the description of CONFIG_SYS_BOOTMAPSZ and
-		  bootm_mapsize.
-
-  bootm_mapsize - Size of the initial memory mapping for the Linux kernel.
-		  This variable is given as a hexadecimal number and it
-		  defines the size of the memory region starting at base
-		  address bootm_low that is accessible by the Linux kernel
-		  during early boot.  If unset, CONFIG_SYS_BOOTMAPSZ is used
-		  as the default value if it is defined, and bootm_size is
-		  used otherwise.
-
-  bootm_size	- Memory range available for image processing in the bootm
-		  command can be restricted. This variable is given as
-		  a hexadecimal number and defines the size of the region
-		  allowed for use by the bootm command. See also "bootm_low"
-		  environment variable.
-
-  bootstopkeysha256, bootdelaykey, bootstopkey	- See README.autoboot
-
-  updatefile	- Location of the software update file on a TFTP server, used
-		  by the automatic software update feature. Please refer to
-		  documentation in doc/README.update for more details.
-
-  autoload	- if set to "no" (any string beginning with 'n'),
-		  "bootp" will just load perform a lookup of the
-		  configuration from the BOOTP server, but not try to
-		  load any image using TFTP
-
-  autostart	- if set to "yes", an image loaded using the "bootp",
-		  "rarpboot", "tftpboot" or "diskboot" commands will
-		  be automatically started (by internally calling
-		  "bootm")
-
-		  If set to "no", a standalone image passed to the
-		  "bootm" command will be copied to the load address
-		  (and eventually uncompressed), but NOT be started.
-		  This can be used to load and uncompress arbitrary
-		  data.
-
-  fdt_high	- if set this restricts the maximum address that the
-		  flattened device tree will be copied into upon boot.
-		  For example, if you have a system with 1 GB memory
-		  at physical address 0x10000000, while Linux kernel
-		  only recognizes the first 704 MB as low memory, you
-		  may need to set fdt_high as 0x3C000000 to have the
-		  device tree blob be copied to the maximum address
-		  of the 704 MB low memory, so that Linux kernel can
-		  access it during the boot procedure.
-
-		  If this is set to the special value 0xFFFFFFFF then
-		  the fdt will not be copied at all on boot.  For this
-		  to work it must reside in writable memory, have
-		  sufficient padding on the end of it for u-boot to
-		  add the information it needs into it, and the memory
-		  must be accessible by the kernel.
-
-  fdtcontroladdr- if set this is the address of the control flattened
-		  device tree used by U-Boot when CONFIG_OF_CONTROL is
-		  defined.
-
-  i2cfast	- (PPC405GP|PPC405EP only)
-		  if set to 'y' configures Linux I2C driver for fast
-		  mode (400kHZ). This environment variable is used in
-		  initialization code. So, for changes to be effective
-		  it must be saved and board must be reset.
-
-  initrd_high	- restrict positioning of initrd images:
-		  If this variable is not set, initrd images will be
-		  copied to the highest possible address in RAM; this
-		  is usually what you want since it allows for
-		  maximum initrd size. If for some reason you want to
-		  make sure that the initrd image is loaded below the
-		  CONFIG_SYS_BOOTMAPSZ limit, you can set this environment
-		  variable to a value of "no" or "off" or "0".
-		  Alternatively, you can set it to a maximum upper
-		  address to use (U-Boot will still check that it
-		  does not overwrite the U-Boot stack and data).
-
-		  For instance, when you have a system with 16 MB
-		  RAM, and want to reserve 4 MB from use by Linux,
-		  you can do this by adding "mem=12M" to the value of
-		  the "bootargs" variable. However, now you must make
-		  sure that the initrd image is placed in the first
-		  12 MB as well - this can be done with
-
-		  setenv initrd_high 00c00000
-
-		  If you set initrd_high to 0xFFFFFFFF, this is an
-		  indication to U-Boot that all addresses are legal
-		  for the Linux kernel, including addresses in flash
-		  memory. In this case U-Boot will NOT COPY the
-		  ramdisk at all. This may be useful to reduce the
-		  boot time on your system, but requires that this
-		  feature is supported by your Linux kernel.
-
-  ipaddr	- IP address; needed for tftpboot command
-
-  loadaddr	- Default load address for commands like "bootp",
-		  "rarpboot", "tftpboot", "loadb" or "diskboot"
-
-  loads_echo	- see CONFIG_LOADS_ECHO
-
-  serverip	- TFTP server IP address; needed for tftpboot command
-
-  bootretry	- see CONFIG_BOOT_RETRY_TIME
-
-  bootdelaykey	- see CONFIG_AUTOBOOT_DELAY_STR
-
-  bootstopkey	- see CONFIG_AUTOBOOT_STOP_STR
-
-  ethprime	- controls which interface is used first.
-
-  ethact	- controls which interface is currently active.
-		  For example you can do the following
-
-		  => setenv ethact FEC
-		  => ping 192.168.0.1 # traffic sent on FEC
-		  => setenv ethact SCC
-		  => ping 10.0.0.1 # traffic sent on SCC
-
-  ethrotate	- When set to "no" U-Boot does not go through all
-		  available network interfaces.
-		  It just stays at the currently selected interface.
-
-  netretry	- When set to "no" each network operation will
-		  either succeed or fail without retrying.
-		  When set to "once" the network operation will
-		  fail when all the available network interfaces
-		  are tried once without success.
-		  Useful on scripts which control the retry operation
-		  themselves.
-
-  npe_ucode	- set load address for the NPE microcode
-
-  silent_linux  - If set then Linux will be told to boot silently, by
-		  changing the console to be empty. If "yes" it will be
-		  made silent. If "no" it will not be made silent. If
-		  unset, then it will be made silent if the U-Boot console
-		  is silent.
-
-  tftpsrcp	- If this is set, the value is used for TFTP's
-		  UDP source port.
-
-  tftpdstp	- If this is set, the value is used for TFTP's UDP
-		  destination port instead of the Well Know Port 69.
-
-  tftpblocksize - Block size to use for TFTP transfers; if not set,
-		  we use the TFTP server's default block size
-
-  tftptimeout	- Retransmission timeout for TFTP packets (in milli-
-		  seconds, minimum value is 1000 = 1 second). Defines
-		  when a packet is considered to be lost so it has to
-		  be retransmitted. The default is 5000 = 5 seconds.
-		  Lowering this value may make downloads succeed
-		  faster in networks with high packet loss rates or
-		  with unreliable TFTP servers.
-
-  tftptimeoutcountmax	- maximum count of TFTP timeouts (no
-		  unit, minimum value = 0). Defines how many timeouts
-		  can happen during a single file transfer before that
-		  transfer is aborted. The default is 10, and 0 means
-		  'no timeouts allowed'. Increasing this value may help
-		  downloads succeed with high packet loss rates, or with
-		  unreliable TFTP servers or client hardware.
-
-  tftpwindowsize	- if this is set, the value is used for TFTP's
-		  window size as described by RFC 7440.
-		  This means the count of blocks we can receive before
-		  sending ack to server.
-
-  vlan		- When set to a value < 4095 the traffic over
-		  Ethernet is encapsulated/received over 802.1q
-		  VLAN tagged frames.
-
-  bootpretryperiod	- Period during which BOOTP/DHCP sends retries.
-		  Unsigned value, in milliseconds. If not set, the period will
-		  be either the default (28000), or a value based on
-		  CONFIG_NET_RETRY_COUNT, if defined. This value has
-		  precedence over the valu based on CONFIG_NET_RETRY_COUNT.
-
-  memmatches	- Number of matches found by the last 'ms' command, in hex
-
-  memaddr	- Address of the last match found by the 'ms' command, in hex,
-		  or 0 if none
-
-  mempos	- Index position of the last match found by the 'ms' command,
-		  in units of the size (.b, .w, .l) of the search
-
-  zbootbase	- (x86 only) Base address of the bzImage 'setup' block
-
-  zbootaddr	- (x86 only) Address of the loaded bzImage, typically
-		  BZIMAGE_LOAD_ADDR which is 0x100000
-
-The following image location variables contain the location of images
-used in booting. The "Image" column gives the role of the image and is
-not an environment variable name. The other columns are environment
-variable names. "File Name" gives the name of the file on a TFTP
-server, "RAM Address" gives the location in RAM the image will be
-loaded to, and "Flash Location" gives the image's address in NOR
-flash or offset in NAND flash.
-
-*Note* - these variables don't have to be defined for all boards, some
-boards currently use other variables for these purposes, and some
-boards use these variables for other purposes.
-
-Image		    File Name	     RAM Address       Flash Location
------		    ---------	     -----------       --------------
-u-boot		    u-boot	     u-boot_addr_r     u-boot_addr
-Linux kernel	    bootfile	     kernel_addr_r     kernel_addr
-device tree blob    fdtfile	     fdt_addr_r	       fdt_addr
-ramdisk		    ramdiskfile	     ramdisk_addr_r    ramdisk_addr
-
-The following environment variables may be used and automatically
-updated by the network boot commands ("bootp" and "rarpboot"),
-depending the information provided by your boot server:
-
-  bootfile	- see above
-  dnsip		- IP address of your Domain Name Server
-  dnsip2	- IP address of your secondary Domain Name Server
-  gatewayip	- IP address of the Gateway (Router) to use
-  hostname	- Target hostname
-  ipaddr	- see above
-  netmask	- Subnet Mask
-  rootpath	- Pathname of the root filesystem on the NFS server
-  serverip	- see above
-
-
-There are two special Environment Variables:
-
-  serial#	- contains hardware identification information such
-		  as type string and/or serial number
-  ethaddr	- Ethernet address
-
-These variables can be set only once (usually during manufacturing of
-the board). U-Boot refuses to delete or overwrite these variables
-once they have been set once.
-
-
-Further special Environment Variables:
-
-  ver		- Contains the U-Boot version string as printed
-		  with the "version" command. This variable is
-		  readonly (see CONFIG_VERSION_VARIABLE).
-
-
-Please note that changes to some configuration parameters may take
-only effect after the next boot (yes, that's just like Windoze :-).
-
-
-Callback functions for environment variables:
----------------------------------------------
-
-For some environment variables, the behavior of u-boot needs to change
-when their values are changed.  This functionality allows functions to
-be associated with arbitrary variables.  On creation, overwrite, or
-deletion, the callback will provide the opportunity for some side
-effect to happen or for the change to be rejected.
-
-The callbacks are named and associated with a function using the
-U_BOOT_ENV_CALLBACK macro in your board or driver code.
-
-These callbacks are associated with variables in one of two ways.  The
-static list can be added to by defining CONFIG_ENV_CALLBACK_LIST_STATIC
-in the board configuration to a string that defines a list of
-associations.  The list must be in the following format:
-
-	entry = variable_name[:callback_name]
-	list = entry[,list]
-
-If the callback name is not specified, then the callback is deleted.
-Spaces are also allowed anywhere in the list.
-
-Callbacks can also be associated by defining the ".callbacks" variable
-with the same list format above.  Any association in ".callbacks" will
-override any association in the static list. You can define
-CONFIG_ENV_CALLBACK_LIST_DEFAULT to a list (string) to define the
-".callbacks" environment variable in the default or embedded environment.
-
-If CONFIG_REGEX is defined, the variable_name above is evaluated as a
-regular expression. This allows multiple variables to be connected to
-the same callback without explicitly listing them all out.
-
-The signature of the callback functions is:
-
-    int callback(const char *name, const char *value, enum env_op op, int flags)
-
-* name - changed environment variable
-* value - new value of the environment variable
-* op - operation (create, overwrite, or delete)
-* flags - attributes of the environment variable change, see flags H_* in
-  include/search.h
-
-The return value is 0 if the variable change is accepted and 1 otherwise.
-
-
 Note for Redundant Ethernet Interfaces:
 =======================================
 
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
new file mode 100644
index 00000000000..6b88b60f433
--- /dev/null
+++ b/doc/usage/environment.rst
@@ -0,0 +1,390 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Environment Variables
+=====================
+
+
+U-Boot supports user configuration using Environment Variables which
+can be made persistent by saving to persistent storage, for example flash
+memory.
+
+Environment Variables are set using "env set" (alias "setenv"), printed using
+"env print" (alias "printenv"), and saved to persistent storage using
+"env save" (alias "saveenv"). Using "env set"
+without a value can be used to delete a variable from the
+environment. As long as you don't save the environment you are
+working with an in-memory copy. In case the Flash area containing the
+environment is erased by accident, a default environment is provided.
+
+Some configuration is controlled by Environment Variables, so that setting the
+variable can adjust the behaviour of U-Boot (e.g. autoboot delay, autoloading
+from tftp).
+
+List of environment variables (most likely not complete):
+
+baudrate
+    see CONFIG_BAUDRATE
+
+bootdelay
+    see CONFIG_BOOTDELAY
+
+bootcmd
+    see CONFIG_BOOTCOMMAND
+
+bootargs
+    Boot arguments when booting an RTOS image
+
+bootfile
+    Name of the image to load with TFTP
+
+bootm_low
+    Memory range available for image processing in the bootm
+    command can be restricted. This variable is given as
+    a hexadecimal number and defines lowest address allowed
+    for use by the bootm command. See also "bootm_size"
+    environment variable. Address defined by "bootm_low" is
+    also the base of the initial memory mapping for the Linux
+    kernel -- see the description of CONFIG_SYS_BOOTMAPSZ and
+    bootm_mapsize.
+
+bootm_mapsize
+    Size of the initial memory mapping for the Linux kernel.
+    This variable is given as a hexadecimal number and it
+    defines the size of the memory region starting at base
+    address bootm_low that is accessible by the Linux kernel
+    during early boot.  If unset, CONFIG_SYS_BOOTMAPSZ is used
+    as the default value if it is defined, and bootm_size is
+    used otherwise.
+
+bootm_size
+    Memory range available for image processing in the bootm
+    command can be restricted. This variable is given as
+    a hexadecimal number and defines the size of the region
+    allowed for use by the bootm command. See also "bootm_low"
+    environment variable.
+
+bootstopkeysha256, bootdelaykey, bootstopkey
+    See README.autoboot
+
+updatefile
+    Location of the software update file on a TFTP server, used
+    by the automatic software update feature. Please refer to
+    documentation in doc/README.update for more details.
+
+autoload
+    if set to "no" (any string beginning with 'n'),
+    "bootp" will just load perform a lookup of the
+    configuration from the BOOTP server, but not try to
+    load any image using TFTP
+
+autostart
+    if set to "yes", an image loaded using the "bootp",
+    "rarpboot", "tftpboot" or "diskboot" commands will
+    be automatically started (by internally calling
+    "bootm")
+
+    If set to "no", a standalone image passed to the
+    "bootm" command will be copied to the load address
+    (and eventually uncompressed), but NOT be started.
+    This can be used to load and uncompress arbitrary
+    data.
+
+fdt_high
+    if set this restricts the maximum address that the
+    flattened device tree will be copied into upon boot.
+    For example, if you have a system with 1 GB memory
+    at physical address 0x10000000, while Linux kernel
+    only recognizes the first 704 MB as low memory, you
+    may need to set fdt_high as 0x3C000000 to have the
+    device tree blob be copied to the maximum address
+    of the 704 MB low memory, so that Linux kernel can
+    access it during the boot procedure.
+
+    If this is set to the special value 0xFFFFFFFF then
+    the fdt will not be copied at all on boot.  For this
+    to work it must reside in writable memory, have
+    sufficient padding on the end of it for u-boot to
+    add the information it needs into it, and the memory
+    must be accessible by the kernel.
+
+fdtcontroladdr
+    if set this is the address of the control flattened
+    device tree used by U-Boot when CONFIG_OF_CONTROL is
+    defined.
+
+initrd_high
+    restrict positioning of initrd images:
+    If this variable is not set, initrd images will be
+    copied to the highest possible address in RAM; this
+    is usually what you want since it allows for
+    maximum initrd size. If for some reason you want to
+    make sure that the initrd image is loaded below the
+    CONFIG_SYS_BOOTMAPSZ limit, you can set this environment
+    variable to a value of "no" or "off" or "0".
+    Alternatively, you can set it to a maximum upper
+    address to use (U-Boot will still check that it
+    does not overwrite the U-Boot stack and data).
+
+    For instance, when you have a system with 16 MB
+    RAM, and want to reserve 4 MB from use by Linux,
+    you can do this by adding "mem=12M" to the value of
+    the "bootargs" variable. However, now you must make
+    sure that the initrd image is placed in the first
+    12 MB as well - this can be done with::
+
+        setenv initrd_high 00c00000
+
+    If you set initrd_high to 0xFFFFFFFF, this is an
+    indication to U-Boot that all addresses are legal
+    for the Linux kernel, including addresses in flash
+    memory. In this case U-Boot will NOT COPY the
+    ramdisk at all. This may be useful to reduce the
+    boot time on your system, but requires that this
+    feature is supported by your Linux kernel.
+
+ipaddr
+    IP address; needed for tftpboot command
+
+loadaddr
+    Default load address for commands like "bootp",
+    "rarpboot", "tftpboot", "loadb" or "diskboot"
+
+loads_echo
+    see CONFIG_LOADS_ECHO
+
+serverip
+    TFTP server IP address; needed for tftpboot command
+
+bootretry
+    see CONFIG_BOOT_RETRY_TIME
+
+bootdelaykey
+    see CONFIG_AUTOBOOT_DELAY_STR
+
+bootstopkey
+    see CONFIG_AUTOBOOT_STOP_STR
+
+ethprime
+    controls which interface is used first.
+
+ethact
+    controls which interface is currently active.
+    For example you can do the following::
+
+    => setenv ethact FEC
+    => ping 192.168.0.1 # traffic sent on FEC
+    => setenv ethact SCC
+    => ping 10.0.0.1 # traffic sent on SCC
+
+ethrotate
+    When set to "no" U-Boot does not go through all
+    available network interfaces.
+    It just stays at the currently selected interface.
+
+netretry
+    When set to "no" each network operation will
+    either succeed or fail without retrying.
+    When set to "once" the network operation will
+    fail when all the available network interfaces
+    are tried once without success.
+    Useful on scripts which control the retry operation
+    themselves.
+
+npe_ucode
+    set load address for the NPE microcode
+
+silent_linux
+    If set then Linux will be told to boot silently, by
+    changing the console to be empty. If "yes" it will be
+    made silent. If "no" it will not be made silent. If
+    unset, then it will be made silent if the U-Boot console
+    is silent.
+
+tftpsrcp
+    If this is set, the value is used for TFTP's
+    UDP source port.
+
+tftpdstp
+    If this is set, the value is used for TFTP's UDP
+    destination port instead of the Well Know Port 69.
+
+tftpblocksize
+    Block size to use for TFTP transfers; if not set,
+    we use the TFTP server's default block size
+
+tftptimeout
+    Retransmission timeout for TFTP packets (in milli-
+    seconds, minimum value is 1000 = 1 second). Defines
+    when a packet is considered to be lost so it has to
+    be retransmitted. The default is 5000 = 5 seconds.
+    Lowering this value may make downloads succeed
+    faster in networks with high packet loss rates or
+    with unreliable TFTP servers.
+
+tftptimeoutcountmax
+    maximum count of TFTP timeouts (no
+    unit, minimum value = 0). Defines how many timeouts
+    can happen during a single file transfer before that
+    transfer is aborted. The default is 10, and 0 means
+    'no timeouts allowed'. Increasing this value may help
+    downloads succeed with high packet loss rates, or with
+    unreliable TFTP servers or client hardware.
+
+tftpwindowsize
+    if this is set, the value is used for TFTP's
+    window size as described by RFC 7440.
+    This means the count of blocks we can receive before
+    sending ack to server.
+
+vlan
+    When set to a value < 4095 the traffic over
+    Ethernet is encapsulated/received over 802.1q
+    VLAN tagged frames.
+
+bootpretryperiod
+    Period during which BOOTP/DHCP sends retries.
+    Unsigned value, in milliseconds. If not set, the period will
+    be either the default (28000), or a value based on
+    CONFIG_NET_RETRY_COUNT, if defined. This value has
+    precedence over the valu based on CONFIG_NET_RETRY_COUNT.
+
+memmatches
+    Number of matches found by the last 'ms' command, in hex
+
+memaddr
+    Address of the last match found by the 'ms' command, in hex,
+    or 0 if none
+
+mempos
+    Index position of the last match found by the 'ms' command,
+    in units of the size (.b, .w, .l) of the search
+
+zbootbase
+    (x86 only) Base address of the bzImage 'setup' block
+
+zbootaddr
+    (x86 only) Address of the loaded bzImage, typically
+    BZIMAGE_LOAD_ADDR which is 0x100000
+
+
+Image locations
+---------------
+
+The following image location variables contain the location of images
+used in booting. The "Image" column gives the role of the image and is
+not an environment variable name. The other columns are environment
+variable names. "File Name" gives the name of the file on a TFTP
+server, "RAM Address" gives the location in RAM the image will be
+loaded to, and "Flash Location" gives the image's address in NOR
+flash or offset in NAND flash.
+
+*Note* - these variables don't have to be defined for all boards, some
+boards currently use other variables for these purposes, and some
+boards use these variables for other purposes.
+
+Also note that most of these variables are just a commonly used set of variable
+names, used in some other variable definitions, but are not hard-coded anywhere
+in U-Boot code.
+
+================= ============== ================ ==============
+Image             File Name      RAM Address      Flash Location
+================= ============== ================ ==============
+u-boot            u-boot         u-boot_addr_r    u-boot_addr
+Linux kernel      bootfile       kernel_addr_r    kernel_addr
+device tree blob  fdtfile        fdt_addr_r       fdt_addr
+ramdisk           ramdiskfile    ramdisk_addr_r   ramdisk_addr
+================= ============== ================ ==============
+
+
+Automatically updated variables
+-------------------------------
+
+The following environment variables may be used and automatically
+updated by the network boot commands ("bootp" and "rarpboot"),
+depending the information provided by your boot server:
+
+=========  ===================================================
+Variable   Notes
+=========  ===================================================
+bootfile   see above
+dnsip      IP address of your Domain Name Server
+dnsip2     IP address of your secondary Domain Name Server
+gatewayip  IP address of the Gateway (Router) to use
+hostname   Target hostname
+ipaddr     See above
+netmask    Subnet Mask
+rootpath   Pathname of the root filesystem on the NFS server
+serverip   see above
+=========  ===================================================
+
+
+Special environment variables
+-----------------------------
+
+There are two special Environment Variables:
+
+serial#
+    contains hardware identification information such as type string and/or
+    serial number
+ethaddr
+    Ethernet address
+
+These variables can be set only once (usually during manufacturing of
+the board). U-Boot refuses to delete or overwrite these variables
+once they have been set once.
+
+Also:
+
+ver
+    Contains the U-Boot version string as printed
+    with the "version" command. This variable is
+    readonly (see CONFIG_VERSION_VARIABLE).
+
+Please note that changes to some configuration parameters may take
+only effect after the next boot (yes, that's just like Windoze :-).
+
+
+Callback functions for environment variables
+--------------------------------------------
+
+For some environment variables, the behavior of u-boot needs to change
+when their values are changed.  This functionality allows functions to
+be associated with arbitrary variables.  On creation, overwrite, or
+deletion, the callback will provide the opportunity for some side
+effect to happen or for the change to be rejected.
+
+The callbacks are named and associated with a function using the
+U_BOOT_ENV_CALLBACK macro in your board or driver code.
+
+These callbacks are associated with variables in one of two ways.  The
+static list can be added to by defining CONFIG_ENV_CALLBACK_LIST_STATIC
+in the board configuration to a string that defines a list of
+associations.  The list must be in the following format::
+
+    entry = variable_name[:callback_name]
+    list = entry[,list]
+
+If the callback name is not specified, then the callback is deleted.
+Spaces are also allowed anywhere in the list.
+
+Callbacks can also be associated by defining the ".callbacks" variable
+with the same list format above.  Any association in ".callbacks" will
+override any association in the static list. You can define
+CONFIG_ENV_CALLBACK_LIST_DEFAULT to a list (string) to define the
+".callbacks" environment variable in the default or embedded environment.
+
+If CONFIG_REGEX is defined, the variable_name above is evaluated as a
+regular expression. This allows multiple variables to be connected to
+the same callback without explicitly listing them all out.
+
+The signature of the callback functions is::
+
+    int callback(const char *name, const char *value, enum env_op op, int flags)
+
+* name - changed environment variable
+* value - new value of the environment variable
+* op - operation (create, overwrite, or delete)
+* flags - attributes of the environment variable change, see flags H_* in
+  include/search.h
+
+The return value is 0 if the variable change is accepted and 1 otherwise.
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 356f2a56181..1a79d1c03eb 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -5,6 +5,7 @@ Use U-Boot
    :maxdepth: 1
 
    dfu
+   environment
    fdt_overlays
    fit
    netconsole
-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
  2021-10-02  0:38 ` [PATCH v5 1/5] sandbox: Drop distro_boot Simon Glass
  2021-10-02  0:38 ` [PATCH v5 2/5] doc: Move environment documentation to rST Simon Glass
@ 2021-10-02  0:38 ` Simon Glass
  2021-10-04  7:28   ` Rasmus Villemoes
  2021-10-04 12:08   ` Wolfgang Denk
  2021-10-02  0:38 ` [PATCH v5 4/5] env: Allow environment files to use the C preprocessor Simon Glass
  2021-10-02  0:38 ` [PATCH v5 5/5] sandbox: Use a text-based environment Simon Glass
  4 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass, Joe Hershberger

At present U-Boot environment variables, and thus scripts, are defined
by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
to this file and dealing with quoting and newlines is harder than it
should be. It would be better if we could just type the script into a
text file and have it included by U-Boot.

Add a feature that brings in a .env file associated with the board
config, if present. To use it, create a file in a board/<vendor>/env
directory called <board>.env (or common.env if you want the same
environment for all boards).

The environment variables should be of the form "var=value". Values can
extend to multiple lines. See the README under 'Environment Variables:'
for more information and an example.

Comments are not permitted in the environment with this commit.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v5:
- Explain how to include the common.env file
- Explain why variables starting with _ , and / are not supported
- Expand the definition of how to declare an environment variable
- Explain what happens to empty variables
- Update maintainer

Changes in v4:
- Move this from being part of configuring U-Boot to part of building it
- Don't put the environment in autoconf.mk as it is not needed
- Add documentation in rST format instead of README
- Drop mention of import/export
- Update awk script to ignore blank lines, as generated by clang

Changes in v3:
- Adjust Makefile to generate the .inc and .h files in separate fules
- Add more detail in the README about the format of .env files
- Improve the comment about " in the awk script
- Correctly terminate environment files with \n

Changes in v2:
- Move .env file from include/configs to board/
- Use awk script to process environment since it is much easier on the brain
- Add information and updated example script to README
- Add dependency rule so that the environment is rebuilt when it changes

 MAINTAINERS               |  7 ++++++
 Makefile                  | 34 +++++++++++++++++++++++++-
 config.mk                 |  2 ++
 doc/usage/environment.rst | 42 ++++++++++++++++++++++++++++++++
 env/embedded.c            |  1 +
 include/env_default.h     |  8 +++++++
 scripts/env2string.awk    | 50 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 scripts/env2string.awk

diff --git a/MAINTAINERS b/MAINTAINERS
index 31b49c0a95f..d79dac06cb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -738,6 +738,13 @@ F:	test/env/
 F:	tools/env*
 F:	tools/mkenvimage.c
 
+ENVIRONMENT AS TEXT
+M:	Simon Glass <sjg@chromium.org>
+R:	Wolfgang Denk <wd@denx.de>
+S:	Maintained
+F:	doc/usage/environment.rst
+F:	scripts/env2string.awk
+
 FPGA
 M:	Michal Simek <michal.simek@xilinx.com>
 S:	Maintained
diff --git a/Makefile b/Makefile
index b23c2cea912..5c3ba37398f 100644
--- a/Makefile
+++ b/Makefile
@@ -513,6 +513,7 @@ version_h := include/generated/version_autogenerated.h
 timestamp_h := include/generated/timestamp_autogenerated.h
 defaultenv_h := include/generated/defaultenv_autogenerated.h
 dt_h := include/generated/dt.h
+env_h := include/generated/environment.h
 
 no-dot-config-targets := clean clobber mrproper distclean \
 			 help %docs check% coccicheck \
@@ -1785,6 +1786,37 @@ quiet_cmd_sym ?= SYM     $@
 u-boot.sym: u-boot FORCE
 	$(call if_changed,sym)
 
+# We expect '<board>.env' but failing that will use 'common.env'
+ENV_DIR := $(if $(VENDOR),$(VENDOR)/env,$(BOARD)/env)
+ENV_FILE_BOARD := $(srctree)/board/${ENV_DIR}/$(BOARD).env
+ENV_FILE_COMMON := $(srctree)/board/${ENV_DIR}/common.env
+ENV_FILE := $(if $(wildcard $(ENV_FILE_BOARD)),$(ENV_FILE_BOARD),$(ENV_FILE_COMMON))
+
+# Run the environment text file through the preprocessor
+quiet_cmd_gen_envp = ENVP    $@
+      cmd_gen_envp = \
+	if [ -f "$(ENV_FILE)" ]; then \
+		cat $(ENV_FILE) >$@; \
+	else \
+		echo -n >$@ ; \
+	fi
+
+# Regenerate the environment if it changes
+# We use 'wildcard' since the file is not required to exist (at present), in
+# which case we don't want this dependency, but instead should create an empty
+# file
+include/generated/environment.in: \
+		$(if $(wildcard $(ENV_FILE)),$(wildcard $(ENV_FILE)),FORCE)
+	$(call cmd,gen_envp)
+
+quiet_cmd_gen_envt = ENVT    $@
+      cmd_gen_envt = \
+	echo -n "\#define CONFIG_EXTRA_ENV_TEXT " >$@; \
+	awk -f $(srctree)/scripts/env2string.awk $< >>$@
+
+$(env_h): include/generated/environment.in
+	$(call cmd,gen_envt)
+
 # The actual objects are generated when descending,
 # make sure no implicit rule kicks in
 $(sort $(u-boot-init) $(u-boot-main)): $(u-boot-dirs) ;
@@ -1840,7 +1872,7 @@ endif
 # prepare2 creates a makefile if using a separate output directory
 prepare2: prepare3 outputmakefile cfg
 
-prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) \
+prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) $(env_h) \
                    include/config/auto.conf
 ifeq ($(wildcard $(LDSCRIPT)),)
 	@echo >&2 "  Could not find linker script."
diff --git a/config.mk b/config.mk
index 7bb1fd4ed1b..2595aed218b 100644
--- a/config.mk
+++ b/config.mk
@@ -50,8 +50,10 @@ endif
 ifneq ($(BOARD),)
 ifdef	VENDOR
 BOARDDIR = $(VENDOR)/$(BOARD)
+ENVDIR=${vendor}/env
 else
 BOARDDIR = $(BOARD)
+ENVDIR=${board}/env
 endif
 endif
 ifdef	BOARD
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index 6b88b60f433..4e0770523eb 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -20,6 +20,48 @@ Some configuration is controlled by Environment Variables, so that setting the
 variable can adjust the behaviour of U-Boot (e.g. autoboot delay, autoloading
 from tftp).
 
+The default environment is created in `include/env_default.h`, and can be
+augmented by various `CONFIG` defines. See that file for details. In
+particular you can define `CONFIG_EXTRA_ENV_SETTINGS` in your board file
+to add environment variables (see `CONFIG_EXTRA_ENV_SETTINGS` for details).
+
+It is also possible to create an environment file with the name
+`board/<vendor>/env/<board>.env` for your board. If that file is not present
+then U-Boot will look for `board/<vendor>/env/common.env` so that you can
+have a common environment for all vendor boards.
+
+To include the common.env file in your board.env file, use `#include`.
+
+This is a plain text file where you can type your environment variables in
+the form `var=value`. Blank lines and multi-line variables are supported.
+The conversion script looks for a line that starts in column 1 with a string
+and has an equals sign immediately afterwards. Spaces before the = are not
+permitted. It is a good idea to indent your scripts so that only the 'var='
+appears at the start of a line.
+
+For example, for snapper9260 you would create a text file called
+`board/bluewater/env/snapper9260.env` containing the environment text.
+
+Example::
+
+    stdout=serial
+    bootcmd=
+        /* U-Boot script for booting */
+
+        if [ -z ${tftpserverip} ]; then
+            echo "Use 'setenv tftpserverip a.b.c.d' to set IP address."
+        fi
+
+        usb start; setenv autoload n; bootp;
+        tftpboot ${tftpserverip}:
+        bootm
+    failed=
+        /* Print a message when boot fails */
+        echo CONFIG_SYS_BOARD boot failed - please check your image
+        echo Load address is CONFIG_SYS_LOAD_ADDR
+
+Some configuration options can be set using Environment Variables.
+
 List of environment variables (most likely not complete):
 
 baudrate
diff --git a/env/embedded.c b/env/embedded.c
index 208553e6af1..9f26e6cad9c 100644
--- a/env/embedded.c
+++ b/env/embedded.c
@@ -66,6 +66,7 @@
 #endif
 
 #define DEFAULT_ENV_INSTANCE_EMBEDDED
+#include <config.h>
 #include <env_default.h>
 
 #ifdef CONFIG_ENV_ADDR_REDUND
diff --git a/include/env_default.h b/include/env_default.h
index 66e203eb6e4..48190a7c1af 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -10,6 +10,10 @@
 #include <env_callback.h>
 #include <linux/stringify.h>
 
+#ifndef USE_HOSTCC
+#include <generated/environment.h>
+#endif
+
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {
 	ENV_CRC,	/* CRC Sum */
@@ -112,6 +116,10 @@ const uchar default_environment[] = {
 #endif
 #ifdef	CONFIG_EXTRA_ENV_SETTINGS
 	CONFIG_EXTRA_ENV_SETTINGS
+#endif
+#ifdef CONFIG_EXTRA_ENV_TEXT
+	/* This is created in the Makefile */
+	CONFIG_EXTRA_ENV_TEXT
 #endif
 	"\0"
 #else /* CONFIG_USE_DEFAULT_ENV_FILE */
diff --git a/scripts/env2string.awk b/scripts/env2string.awk
new file mode 100644
index 00000000000..661defdc350
--- /dev/null
+++ b/scripts/env2string.awk
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2021 Google, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+# Awk script to parse a text file containing an environment and convert it
+# to a C string which can be compiled into U-Boot.
+
+# We output a double quote before starting, and again when we finish so that
+# all output is quoted.
+BEGIN {
+	# env holds the env variable we are currently processing
+	env = "";
+	ORS = ""
+	print "\""
+}
+
+# Skip empty lines, as these are generated by the clang preprocessor
+NF {
+	# Quote quotes
+	gsub("\"", "\\\"")
+
+	# Is this the start of a new environment variable?
+	if (match($0, "^([^ =][^ =]*)=(.*)", arr)) {
+		if (length(env) != 0) {
+			# Record the value of the variable now completed
+			vars[var] = env
+		}
+		var = arr[1]
+		env = arr[2]
+	} else {
+		# Change newline to \n
+		env = env "\\n" $0;
+	}
+}
+
+END {
+	# Record the value of the variable now completed. If the variable is
+	# empty it is not set.
+	if (length(env) != 0) {
+		vars[var] = env
+	}
+
+	# Print out all the variables
+	for (var in vars) {
+		print var "=" vars[var] "\\0";
+	}
+	print "\"\n"
+}
-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 4/5] env: Allow environment files to use the C preprocessor
  2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
                   ` (2 preceding siblings ...)
  2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
@ 2021-10-02  0:38 ` Simon Glass
  2021-10-04 12:12   ` Wolfgang Denk
  2021-10-02  0:38 ` [PATCH v5 5/5] sandbox: Use a text-based environment Simon Glass
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass, Joe Hershberger

In many cases environment variables need access to the U-Boot CONFIG
variables to select different options. Enable this so that the environment
scripts can be as useful as the ones currently in the board config files.

Also support += to allow variables to be appended to. This is needed when
using the preprocessor.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- Add documentation in rST format instead of README
- Move use of += to this patch
- Explain that environment variables may not end in +

Changes in v3:
- Define __UBOOT_CONFIG__ when collecting environment files

Changes in v2:
- Add separate patch to enable C preprocessor for environment files
- Enable var+=value form to simplify composing variables in multiple steps

 Makefile                  |  7 ++++++-
 doc/usage/environment.rst | 12 ++++++++++++
 scripts/env2string.awk    |  6 ++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 5c3ba37398f..289a2188cec 100644
--- a/Makefile
+++ b/Makefile
@@ -1796,7 +1796,12 @@ ENV_FILE := $(if $(wildcard $(ENV_FILE_BOARD)),$(ENV_FILE_BOARD),$(ENV_FILE_COMM
 quiet_cmd_gen_envp = ENVP    $@
       cmd_gen_envp = \
 	if [ -f "$(ENV_FILE)" ]; then \
-		cat $(ENV_FILE) >$@; \
+		$(CPP) -P $(CFLAGS) -x assembler-with-cpp -D__ASSEMBLY__ \
+			-D__UBOOT_CONFIG__ \
+			-I . -I include \
+			-I $(srctree)/include -include include/config.h \
+			-I$(srctree)/arch/$(ARCH)/include \
+			$(ENV_FILE) -o $@; \
 	else \
 		echo -n >$@ ; \
 	fi
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index 4e0770523eb..426d8f65329 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -39,12 +39,24 @@ and has an equals sign immediately afterwards. Spaces before the = are not
 permitted. It is a good idea to indent your scripts so that only the 'var='
 appears at the start of a line.
 
+To add additional text to a variable you can use var+=value. This text is
+merged into the variable during the make process and made available as a
+single value to U-Boot. To support this, environment variables may not end
+in `+`.
+
+This file can include C-style comments. Blank lines and multi-line
+variables are supported, and you can use normal C preprocessor directives
+and CONFIG defines from your board config also.
+
 For example, for snapper9260 you would create a text file called
 `board/bluewater/env/snapper9260.env` containing the environment text.
 
 Example::
 
     stdout=serial
+    #ifdef CONFIG_LCD
+    stdout+=,lcd
+    #endif
     bootcmd=
         /* U-Boot script for booting */
 
diff --git a/scripts/env2string.awk b/scripts/env2string.awk
index 661defdc350..6de3e3dd87e 100644
--- a/scripts/env2string.awk
+++ b/scripts/env2string.awk
@@ -29,6 +29,12 @@ NF {
 		}
 		var = arr[1]
 		env = arr[2]
+
+		# Deal with +=
+		if (match(var, "(.*)[+]$", var_arr)) {
+			var = var_arr[1]
+			env = vars[var] env
+		}
 	} else {
 		# Change newline to \n
 		env = env "\\n" $0;
-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 5/5] sandbox: Use a text-based environment
  2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
                   ` (3 preceding siblings ...)
  2021-10-02  0:38 ` [PATCH v5 4/5] env: Allow environment files to use the C preprocessor Simon Glass
@ 2021-10-02  0:38 ` Simon Glass
  4 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-02  0:38 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Simon Glass

Use a text file for the environment instead of the #define settings.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v3)

Changes in v3:
- Add new patch to use a text-based environment for sandbox

 board/sandbox/env/sandbox.env | 25 +++++++++++++++++++++++++
 include/configs/sandbox.h     | 29 -----------------------------
 2 files changed, 25 insertions(+), 29 deletions(-)
 create mode 100644 board/sandbox/env/sandbox.env

diff --git a/board/sandbox/env/sandbox.env b/board/sandbox/env/sandbox.env
new file mode 100644
index 00000000000..0f8d95b8db0
--- /dev/null
+++ b/board/sandbox/env/sandbox.env
@@ -0,0 +1,25 @@
+stdin=serial
+#ifdef CONFIG_SANDBOX_SDL
+stdin+=,cros-ec-keyb,usbkbd
+#endif
+stdout=serial,vidconsole
+stderr=serial,vidconsole
+
+ethaddr=00:00:11:22:33:44
+eth2addr=00:00:11:22:33:48
+eth3addr=00:00:11:22:33:45
+eth4addr=00:00:11:22:33:48
+eth5addr=00:00:11:22:33:46
+eth6addr=00:00:11:22:33:47
+ipaddr=1.2.3.4
+
+/*
+ * These are used for distro boot which is not supported. But once bootmethod
+ * is provided these will be used again.
+ */
+bootm_size=0x10000000
+kernel_addr_r=0x1000000
+fdt_addr_r=0xc00000
+ramdisk_addr_r=0x2000000
+scriptaddr=0x1000
+pxefile_addr_r=0x2000
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index c19232f202f..c703a1330c0 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -64,37 +64,8 @@
 #define CONFIG_LCD_BMP_RLE8
 
 #define CONFIG_KEYBOARD
-
-#define SANDBOX_SERIAL_SETTINGS		"stdin=serial,cros-ec-keyb,usbkbd\0" \
-					"stdout=serial,vidconsole\0" \
-					"stderr=serial,vidconsole\0"
-#else
-#define SANDBOX_SERIAL_SETTINGS		"stdin=serial\0" \
-					"stdout=serial,vidconsole\0" \
-					"stderr=serial,vidconsole\0"
 #endif
 
-#define SANDBOX_ETH_SETTINGS		"ethaddr=00:00:11:22:33:44\0" \
-					"eth2addr=00:00:11:22:33:48\0" \
-					"eth3addr=00:00:11:22:33:45\0" \
-					"eth4addr=00:00:11:22:33:48\0" \
-					"eth5addr=00:00:11:22:33:46\0" \
-					"eth6addr=00:00:11:22:33:47\0" \
-					"ipaddr=1.2.3.4\0"
-
-#define MEM_LAYOUT_ENV_SETTINGS \
-	"bootm_size=0x10000000\0" \
-	"kernel_addr_r=0x1000000\0" \
-	"fdt_addr_r=0xc00000\0" \
-	"ramdisk_addr_r=0x2000000\0" \
-	"scriptaddr=0x1000\0" \
-	"pxefile_addr_r=0x2000\0"
-
-#define CONFIG_EXTRA_ENV_SETTINGS \
-	SANDBOX_SERIAL_SETTINGS \
-	SANDBOX_ETH_SETTINGS \
-	MEM_LAYOUT_ENV_SETTINGS
-
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_IDE_MAXBUS		1
 #define CONFIG_SYS_ATA_IDE0_OFFSET	0
-- 
2.33.0.800.g4c38ced690-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
@ 2021-10-04  7:28   ` Rasmus Villemoes
  2021-10-04 15:38     ` Tom Rini
  2021-10-05 14:42     ` Simon Glass
  2021-10-04 12:08   ` Wolfgang Denk
  1 sibling, 2 replies; 18+ messages in thread
From: Rasmus Villemoes @ 2021-10-04  7:28 UTC (permalink / raw)
  To: Simon Glass, U-Boot Mailing List; +Cc: Wolfgang Denk, Tom Rini, Joe Hershberger

On 02/10/2021 02.38, Simon Glass wrote:
> At present U-Boot environment variables, and thus scripts, are defined
> by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> to this file and dealing with quoting and newlines is harder than it
> should be. It would be better if we could just type the script into a
> text file and have it included by U-Boot.

Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation
for introducing CONFIG_USE_DEFAULT_ENV_FILE.

> Add a feature that brings in a .env file associated with the board
> config, if present. To use it, create a file in a board/<vendor>/env
> directory called <board>.env (or common.env if you want the same
> environment for all boards).
> 
> The environment variables should be of the form "var=value". Values can
> extend to multiple lines. See the README under 'Environment Variables:'
> for more information and an example.
> 
> Comments are not permitted in the environment with this commit.

Perhaps some remarks on how this compares/relates to
CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order?
In particular, the latter seems like it could already do the "amend the
environent per vendor/board" with appropriate settings in the
-u-boot.dtsi files?

I don't think either of those currently support using CONFIG_ variables
in the definitions, but perhaps that could be fixed.

I don't have anything against these patches as such, I'd just like to
understand precisely what they bring that cannot already be done with
existing mechanisms.

Rasmus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/5] doc: Move environment documentation to rST
  2021-10-02  0:38 ` [PATCH v5 2/5] doc: Move environment documentation to rST Simon Glass
@ 2021-10-04 12:05   ` Wolfgang Denk
  0 siblings, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2021-10-04 12:05 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini

Dear Simon,

In message <20211002003848.1803446-3-sjg@chromium.org> you wrote:
>
> +baudrate
> +    see CONFIG_BAUDRATE
> +
> +bootdelay
> +    see CONFIG_BOOTDELAY
> +
> +bootcmd
> +    see CONFIG_BOOTCOMMAND

I know you only copied this, but the text is actually not helpful,
as it does not even mention where to look for documentation for
these CONFIG_* options

> +autoload
> +    if set to "no" (any string beginning with 'n'),
> +    "bootp" will just load perform a lookup of the
> +    configuration from the BOOTP server, but not try to
> +    load any image using TFTP

We should add "dhcp" here, too.

> +autostart
> +    if set to "yes", an image loaded using the "bootp",
> +    "rarpboot", "tftpboot" or "diskboot" commands will
> +    be automatically started (by internally calling
> +    "bootm")
> +
> +    If set to "no", a standalone image passed to the
> +    "bootm" command will be copied to the load address
> +    (and eventually uncompressed), but NOT be started.
> +    This can be used to load and uncompress arbitrary
> +    data.

There is a consistency problem here.

In "bootm" (cmd/bootm.c), the "yes" part applies - any other value
or the variable not being defined omitting the call of do_bootm().

This also applies to nandboot, btw.

In "bootelf" (cmd/elf.c), the "no" part applies - any other value or
the variable not being defined will cause do_bootelf_exec() to be
called.

This should be both documented and fixed.

> +ipaddr
> +    IP address; needed for tftpboot command

...and for ping etc.

> +vlan
> +    When set to a value < 4095 the traffic over
> +    Ethernet is encapsulated/received over 802.1q
> +    VLAN tagged frames.

Really?  I can't see where "vlan" is actually used in the code.
[I looked for it because I could not remember if the values is
interpreted as decimal or hex number...]


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@denx.de
Systems programmers are the high priests of a low cult.
                                                       -- R.S. Barton

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
  2021-10-04  7:28   ` Rasmus Villemoes
@ 2021-10-04 12:08   ` Wolfgang Denk
  2021-10-05 14:42     ` Simon Glass
  1 sibling, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2021-10-04 12:08 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Dear Simon,

In message <20211001183842.v5.3.If789ba3e2667c46c03eda3386ca84a863baeda55@changeid> you wrote:
>
> Add a feature that brings in a .env file associated with the board
> config, if present. To use it, create a file in a board/<vendor>/env
> directory called <board>.env (or common.env if you want the same
> environment for all boards).

This should be no exclusive "or" here. If a common.env exists, it
should be used for all boards, and if additionally one ore more
<board>.env files exist, these should ALSO be applied to the
respective boards.

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@denx.de
Save yourself!  Reboot in 5 seconds!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/5] env: Allow environment files to use the C preprocessor
  2021-10-02  0:38 ` [PATCH v5 4/5] env: Allow environment files to use the C preprocessor Simon Glass
@ 2021-10-04 12:12   ` Wolfgang Denk
  0 siblings, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2021-10-04 12:12 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Dear Simon Glass,

In message <20211001183842.v5.4.Ie78bfbfca0d01d9cba501e127f446ec48e1f7afe@changeid> you wrote:
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.
>
> Also support += to allow variables to be appended to. This is needed when
> using the preprocessor.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - Add documentation in rST format instead of README
> - Move use of += to this patch
> - Explain that environment variables may not end in +

Sorry, I disagree here.  There was intentionally only very little
restrictions on what a environment variable name should look like -
the only exceptions were the '=' and the NUL characters.

Adding artificial restrictions now just to enable your custom
notation for appending seems not acceptable to me.  You might want
to chose a different notation or implement a proper parser instead.

Thanks.

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@denx.de
Never call a man a fool.  Borrow from him.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-04  7:28   ` Rasmus Villemoes
@ 2021-10-04 15:38     ` Tom Rini
  2021-10-05 14:42     ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2021-10-04 15:38 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Simon Glass, U-Boot Mailing List, Wolfgang Denk, Joe Hershberger

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Mon, Oct 04, 2021 at 09:28:43AM +0200, Rasmus Villemoes wrote:

> On 02/10/2021 02.38, Simon Glass wrote:
> > At present U-Boot environment variables, and thus scripts, are defined
> > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> > to this file and dealing with quoting and newlines is harder than it
> > should be. It would be better if we could just type the script into a
> > text file and have it included by U-Boot.
> 
> Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation
> for introducing CONFIG_USE_DEFAULT_ENV_FILE.
> 
> > Add a feature that brings in a .env file associated with the board
> > config, if present. To use it, create a file in a board/<vendor>/env
> > directory called <board>.env (or common.env if you want the same
> > environment for all boards).
> > 
> > The environment variables should be of the form "var=value". Values can
> > extend to multiple lines. See the README under 'Environment Variables:'
> > for more information and an example.
> > 
> > Comments are not permitted in the environment with this commit.
> 
> Perhaps some remarks on how this compares/relates to
> CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order?
> In particular, the latter seems like it could already do the "amend the
> environent per vendor/board" with appropriate settings in the
> -u-boot.dtsi files?
> 
> I don't think either of those currently support using CONFIG_ variables
> in the definitions, but perhaps that could be fixed.
> 
> I don't have anything against these patches as such, I'd just like to
> understand precisely what they bring that cannot already be done with
> existing mechanisms.

So, the high level requirement is to move the environment out of
the board.h file (or the nest of #includes that uses to cobble it
together).  That does mean that CONFIG_USE_DEFAULT_ENV_FILE at least
would likely need some tweaking, but may also be just not as useful, if
it's no longer such a pain to modify the default environment.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-04 12:08   ` Wolfgang Denk
@ 2021-10-05 14:42     ` Simon Glass
  2021-10-05 14:55       ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-10-05 14:42 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Hi Wolfgang,

On Mon, 4 Oct 2021 at 06:08, Wolfgang Denk <wd@denx.de> wrote:
>
> Dear Simon,
>
> In message <20211001183842.v5.3.If789ba3e2667c46c03eda3386ca84a863baeda55@changeid> you wrote:
> >
> > Add a feature that brings in a .env file associated with the board
> > config, if present. To use it, create a file in a board/<vendor>/env
> > directory called <board>.env (or common.env if you want the same
> > environment for all boards).
>
> This should be no exclusive "or" here. If a common.env exists, it
> should be used for all boards, and if additionally one ore more
> <board>.env files exist, these should ALSO be applied to the
> respective boards.

Is it not enough to use #include in the main file? We have a similar
feature with the u-boot.dtsi files and in that case we only choose the
most specific.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-04  7:28   ` Rasmus Villemoes
  2021-10-04 15:38     ` Tom Rini
@ 2021-10-05 14:42     ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-05 14:42 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: U-Boot Mailing List, Wolfgang Denk, Tom Rini, Joe Hershberger

Hi Rasmus,

On Mon, 4 Oct 2021 at 01:28, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 02/10/2021 02.38, Simon Glass wrote:
> > At present U-Boot environment variables, and thus scripts, are defined
> > by CONFIG_EXTRA_ENV_SETTINGS. It is painful to add large amounts of text
> > to this file and dealing with quoting and newlines is harder than it
> > should be. It would be better if we could just type the script into a
> > text file and have it included by U-Boot.
>
> Indeed, the pain of CONFIG_EXTRA_ENV_SETTINGS was part of the motivation
> for introducing CONFIG_USE_DEFAULT_ENV_FILE.
>
> > Add a feature that brings in a .env file associated with the board
> > config, if present. To use it, create a file in a board/<vendor>/env
> > directory called <board>.env (or common.env if you want the same
> > environment for all boards).
> >
> > The environment variables should be of the form "var=value". Values can
> > extend to multiple lines. See the README under 'Environment Variables:'
> > for more information and an example.
> >
> > Comments are not permitted in the environment with this commit.
>
> Perhaps some remarks on how this compares/relates to
> CONFIG_USE_DEFAULT_ENV_FILE and CONFIG_ENV_IMPORT_FDT would be in order?
> In particular, the latter seems like it could already do the "amend the
> environent per vendor/board" with appropriate settings in the
> -u-boot.dtsi files?
>
> I don't think either of those currently support using CONFIG_ variables
> in the definitions, but perhaps that could be fixed.
>
> I don't have anything against these patches as such, I'd just like to
> understand precisely what they bring that cannot already be done with
> existing mechanisms.

Yes I forgot about that. I will take a look.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-05 14:42     ` Simon Glass
@ 2021-10-05 14:55       ` Wolfgang Denk
  2021-10-05 15:33         ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2021-10-05 14:55 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Dear Simon,

In message <CAPnjgZ1tRi5SsqU0K8HXgj-4xCs7i9TLX4Mj0_D=Cpj8BAn32w@mail.gmail.com> you wrote:
>
> > > Add a feature that brings in a .env file associated with the board
> > > config, if present. To use it, create a file in a board/<vendor>/env
> > > directory called <board>.env (or common.env if you want the same
> > > environment for all boards).
> >
> > This should be no exclusive "or" here. If a common.env exists, it
> > should be used for all boards, and if additionally one ore more
> > <board>.env files exist, these should ALSO be applied to the
> > respective boards.
>
> Is it not enough to use #include in the main file? We have a similar
> feature with the u-boot.dtsi files and in that case we only choose the
> most specific.

1) This requires that the .env files are run through CPP, which is
   only added in a later patch.
 
2) Even if I add an "#include board/<vendor>/env/common.env" in my
   <board>.env files, your logic would trigger on the existence of
   the common.env file and ignore the <board>.env files.

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@denx.de
Work 8 hours, sleep 8 hours; but not the same 8 hours.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-05 14:55       ` Wolfgang Denk
@ 2021-10-05 15:33         ` Simon Glass
  2021-10-05 15:52           ` Tom Rini
  2021-10-06  7:08           ` Wolfgang Denk
  0 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-05 15:33 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Hi Wolfgang,

On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk <wd@denx.de> wrote:
>
> Dear Simon,
>
> In message <CAPnjgZ1tRi5SsqU0K8HXgj-4xCs7i9TLX4Mj0_D=Cpj8BAn32w@mail.gmail.com> you wrote:
> >
> > > > Add a feature that brings in a .env file associated with the board
> > > > config, if present. To use it, create a file in a board/<vendor>/env
> > > > directory called <board>.env (or common.env if you want the same
> > > > environment for all boards).
> > >
> > > This should be no exclusive "or" here. If a common.env exists, it
> > > should be used for all boards, and if additionally one ore more
> > > <board>.env files exist, these should ALSO be applied to the
> > > respective boards.
> >
> > Is it not enough to use #include in the main file? We have a similar
> > feature with the u-boot.dtsi files and in that case we only choose the
> > most specific.
>
> 1) This requires that the .env files are run through CPP, which is
>    only added in a later patch.

OK perhaps I should just merge the patches. It is a bit artificial
having two and it seems that people agree we need the += syntax.
>
> 2) Even if I add an "#include board/<vendor>/env/common.env" in my
>    <board>.env files, your logic would trigger on the existence of
>    the common.env file and ignore the <board>.env files.

OK, so I if reverse that, are you happy? What do you think about my
explanation above?

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-05 15:33         ` Simon Glass
@ 2021-10-05 15:52           ` Tom Rini
  2021-10-05 17:27             ` Simon Glass
  2021-10-06  7:08           ` Wolfgang Denk
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Rini @ 2021-10-05 15:52 UTC (permalink / raw)
  To: Simon Glass; +Cc: Wolfgang Denk, U-Boot Mailing List, Joe Hershberger

[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]

On Tue, Oct 05, 2021 at 09:33:18AM -0600, Simon Glass wrote:
> Hi Wolfgang,
> 
> On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk <wd@denx.de> wrote:
> >
> > Dear Simon,
> >
> > In message <CAPnjgZ1tRi5SsqU0K8HXgj-4xCs7i9TLX4Mj0_D=Cpj8BAn32w@mail.gmail.com> you wrote:
> > >
> > > > > Add a feature that brings in a .env file associated with the board
> > > > > config, if present. To use it, create a file in a board/<vendor>/env
> > > > > directory called <board>.env (or common.env if you want the same
> > > > > environment for all boards).
> > > >
> > > > This should be no exclusive "or" here. If a common.env exists, it
> > > > should be used for all boards, and if additionally one ore more
> > > > <board>.env files exist, these should ALSO be applied to the
> > > > respective boards.
> > >
> > > Is it not enough to use #include in the main file? We have a similar
> > > feature with the u-boot.dtsi files and in that case we only choose the
> > > most specific.
> >
> > 1) This requires that the .env files are run through CPP, which is
> >    only added in a later patch.
> 
> OK perhaps I should just merge the patches. It is a bit artificial
> having two and it seems that people agree we need the += syntax.

It's important to maintain bisectability, yes.  But functionality
should be evaluated at the end of the series, not intermediate steps.  I
don't have a strong opinion either way on if these two patches are
merged, or not.  So on a similar note, all of the feedback about the
current env documentation is good and helpful, but I think a txt -> rST
then enhance the rST makes the most sense so that we don't "hide"
improvements within the migration.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-05 15:52           ` Tom Rini
@ 2021-10-05 17:27             ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-10-05 17:27 UTC (permalink / raw)
  To: Tom Rini; +Cc: Wolfgang Denk, U-Boot Mailing List, Joe Hershberger

Hi Tom,

On Tue, 5 Oct 2021 at 09:52, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Oct 05, 2021 at 09:33:18AM -0600, Simon Glass wrote:
> > Hi Wolfgang,
> >
> > On Tue, 5 Oct 2021 at 08:56, Wolfgang Denk <wd@denx.de> wrote:
> > >
> > > Dear Simon,
> > >
> > > In message <CAPnjgZ1tRi5SsqU0K8HXgj-4xCs7i9TLX4Mj0_D=Cpj8BAn32w@mail.gmail.com> you wrote:
> > > >
> > > > > > Add a feature that brings in a .env file associated with the board
> > > > > > config, if present. To use it, create a file in a board/<vendor>/env
> > > > > > directory called <board>.env (or common.env if you want the same
> > > > > > environment for all boards).
> > > > >
> > > > > This should be no exclusive "or" here. If a common.env exists, it
> > > > > should be used for all boards, and if additionally one ore more
> > > > > <board>.env files exist, these should ALSO be applied to the
> > > > > respective boards.
> > > >
> > > > Is it not enough to use #include in the main file? We have a similar
> > > > feature with the u-boot.dtsi files and in that case we only choose the
> > > > most specific.
> > >
> > > 1) This requires that the .env files are run through CPP, which is
> > >    only added in a later patch.
> >
> > OK perhaps I should just merge the patches. It is a bit artificial
> > having two and it seems that people agree we need the += syntax.
>
> It's important to maintain bisectability, yes.  But functionality
> should be evaluated at the end of the series, not intermediate steps.  I
> don't have a strong opinion either way on if these two patches are
> merged, or not.  So on a similar note, all of the feedback about the
> current env documentation is good and helpful, but I think a txt -> rST
> then enhance the rST makes the most sense so that we don't "hide"
> improvements within the migration.

OK. I added Wolfgang's comments in the patch where he made them but
will split them out.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
  2021-10-05 15:33         ` Simon Glass
  2021-10-05 15:52           ` Tom Rini
@ 2021-10-06  7:08           ` Wolfgang Denk
  1 sibling, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2021-10-06  7:08 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Joe Hershberger

Dear Simon,

In message <CAPnjgZ3WVvso8uSeKyiOg6oR_R=nkguGZYXGb9Ws-s5dmO5GmQ@mail.gmail.com> you wrote:
>
> > 1) This requires that the .env files are run through CPP, which is
> >    only added in a later patch.
>
> OK perhaps I should just merge the patches. It is a bit artificial
> having two and it seems that people agree we need the += syntax.

Yes, some way to append to the environment is useful, and using '+='
is an intuitive syntax for it.  I'm just not happy about sneeking in
a new, undocumented restriction on U-Boot variable names.  Yes,
there are probably not many systems around (if any) where a variable
name ends in a plus sign, but we should be consistent.

I have to admit that I don't have any nice alternative suggestion.
If we stick with the rule that only NUL and '=' cannot be used in a
variable name, we could write

	variable=+value

instead.  But this does not look as nice as '+=', and we need an
escape mechanism for the case where we want a simple assignment of a
value that starts with a '+'.


BTW: If we would add such a feature to U-Boot (which seems to be no
bad idea to me) we would probably implement an "env append" command?


> > 2) Even if I add an "#include board/<vendor>/env/common.env" in my
> >    <board>.env files, your logic would trigger on the existence of
> >    the common.env file and ignore the <board>.env files.
>
> OK, so I if reverse that, are you happy? What do you think about my
> explanation above?

The longer I think about it the more I wonder if any hard coded file
names are really a good way to handle this.  For example there might
be the case where we have 4 boards A, B, C and D, and boards A and B
would use one env file, and C and D would use another.  this does
not match the "<board>.env" scheme, and "common.env" does not fit
either, as we have two "common" files.

I wonder if there should rather be a Kconfig option so each board
can select it's env file name; default would be "<board>.env".
what do you think?

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@denx.de
Life would be so much easier if we could  just  look  at  the  source
code.                                                   -- Dave Olson

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-06  7:08 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
2021-10-02  0:38 ` [PATCH v5 1/5] sandbox: Drop distro_boot Simon Glass
2021-10-02  0:38 ` [PATCH v5 2/5] doc: Move environment documentation to rST Simon Glass
2021-10-04 12:05   ` Wolfgang Denk
2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
2021-10-04  7:28   ` Rasmus Villemoes
2021-10-04 15:38     ` Tom Rini
2021-10-05 14:42     ` Simon Glass
2021-10-04 12:08   ` Wolfgang Denk
2021-10-05 14:42     ` Simon Glass
2021-10-05 14:55       ` Wolfgang Denk
2021-10-05 15:33         ` Simon Glass
2021-10-05 15:52           ` Tom Rini
2021-10-05 17:27             ` Simon Glass
2021-10-06  7:08           ` Wolfgang Denk
2021-10-02  0:38 ` [PATCH v5 4/5] env: Allow environment files to use the C preprocessor Simon Glass
2021-10-04 12:12   ` Wolfgang Denk
2021-10-02  0:38 ` [PATCH v5 5/5] sandbox: Use a text-based environment Simon Glass

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.