[PATCH v2 0/2] allow CONFIG_SPL_SAVEENV to work with ENV_IS_IN_SPI_FLASH

[Rasmus Villemoes]

Currently, CONFIG_SPL_SAVEENV is not very well supported by the
various storage backends, as many of them contain variants of some
logic that end up not compiling the .save method when
CONFIG_SPL_BUILD.

As I need environment save support in SPL for a target that uses
ENV_IS_IN_SPI_FLASH, these patches fix env/sf.c to honour
CONFIG_SPL_SAVEENV (and fixes a build warning in the rare case where
one sets CONFIG_CMD_SAVEENV=n). In order to fix this properly and not
add to the existing maze of preprocessor directives, the first patch
adds a convenience config symbol so the existing CONFIG_IS_ENABLED()
helper can be used - which then, as a bonus, ends up reducing said maze.

Should others need to enable CONFIG_SPL_SAVEENV with one of the
remaining backends that currently ignore it, they can most likely use
a similar approach as done for sf.c here.

Difference from v1 is that patches for ext4.c and fat.c have been
dropped, as well as a patch that introduced a ENV_SAVE_PTR() macro -
and the sf.c patch consequently doesn't use that macro but just uses
'CONFIG_IS_ENABLED(SAVEENV) ? (x) : NULL' directly. Also, the series
is no longer branded as a "cleanup" - it intentionally changes the
generated code for certain configurations, but it does so in way that
happens to reduce ifdeffery.

=== testing ===

This has been run-time tested on a mpc8309-derived board to verify
that saving the environment does indeed work in SPL with these patches
applied.

As far as I can tell, the only in-tree defconfig that sets both
SPL_ENV_IS_IN_SPI_FLASH and SPL_SAVEENV is display5_defconfig, which
also happens to be the only one setting SPL_SAVEENV at all. Let's see
how these patches affect that:

# avoid differences due to different git commit or wallclock time
$ export SOURCE_DATE_EPOCH=1585252702
$ echo 'test' > .scmversion
$ export ARCH=arm
$ export CROSS_COMPILE=arm-linux-gnueabi-
$ git checkout master ; make display5_defconfig ; make -j8
$ cp u-boot u-boot.1 ; cp spl/u-boot-spl u-boot-spl.1
$ git checkout sf-spl-saveenv ; make display5_defconfig ; make -j8
$ cp u-boot u-boot.2 ; cp spl/u-boot-spl u-boot-spl.2
$ size u-boot{,-spl}.{1,2}
   text    data     bss     dec     hex filename
 377468   24620   66164  468252   7251c u-boot.1
 377468   24620   66164  468252   7251c u-boot.2
  58411    2020     116   60547    ec83 u-boot-spl.1
  59976    2028     116   62120    f2a8 u-boot-spl.2

So U-Boot proper is not affected (the files even yield identical
objdump -d output), while the SPL grows by the ~1.5K necessary to
implement saving the environment. Borrowing the bloat-o-meter script
from linux, we can also see the functions/data items that are now
included:

../linux/scripts/bloat-o-meter u-boot-spl.1 u-boot-spl.2
add/remove: 11/0 grow/shrink: 0/1 up/down: 1340/-24 (1316)
Function                                     old     new   delta
hexport_r                                      -     408    +408
env_sf_save                                    -     332    +332
qsort                                          -     144    +144
match_entry                                    -     124    +124
env_export                                     -     100    +100
match_string                                   -      92     +92
strstr                                         -      64     +64
setup_flash_device                             -      56     +56
cmpkey                                         -      12     +12
env_offset                                     -       4      +4
env_new_offset                                 -       4      +4
env_sf_load                                  184     160     -24
Total: Before=52986, After=54302, chg +2.48%

[The difference between 1316 and 62120-60547=1573 is most likely due
to string literals that are referenced from the above functions].

Now, to check that other storage backends are not affected, and also
that nothing (neither U-Boot or SPL) changes for ENV_IS_IN_SPI_FLASH
when CONFIG_SPL_SAVEENV=n, I have repeated the above with
am335x_shc_netboot_defconfig (MMC), pengwyn_defconfig (NAND),
mccmon6_sd_defconfig (FLASH),
ls1046ardb_qspi_spl_defconfig (SPI_FLASH):

$ for c in am335x_shc_netboot_defconfig pengwyn_defconfig mccmon6_sd_defconfig ls1046ardb_qspi_spl_defconfig ; do
  [ $c = "ls1046ardb_qspi_spl_defconfig" ] && CROSS_COMPILE=aarch64-linux-gnu- || CROSS_COMPILE=arm-linux-gnueabi-
  for b in master sf-spl-saveenv ; do
    git checkout $b ;
    make $c && make -j8 ;
    cp u-boot u-boot.$b && cp spl/u-boot-spl u-boot-spl.$b ;
  done ;
  for x in u-boot u-boot-spl ; do
    diff -u <(objdump -d $x.master | sed -e '/file format/d') <(objdump -d $x.sf-spl-saveenv | sed -e '/file format/d') > $c.$x.diff ;
  done ;
done
$ ls -l *.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 am335x_shc_netboot_defconfig.u-boot.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 am335x_shc_netboot_defconfig.u-boot-spl.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 ls1046ardb_qspi_spl_defconfig.u-boot.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 ls1046ardb_qspi_spl_defconfig.u-boot-spl.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 mccmon6_sd_defconfig.u-boot.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 mccmon6_sd_defconfig.u-boot-spl.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 pengwyn_defconfig.u-boot.diff
-rw-rw-r-- 1 ravi ravi 0 Mar 26 23:20 pengwyn_defconfig.u-boot-spl.diff

[the sed -e '/file format/d' is of course just to avoid a false
positive from the

u-boot.master:     file format elf32-littlearm

header line]

Rasmus Villemoes (2):
  env: add SAVEENV as an alias of the CMD_SAVEENV symbol
  env/sf.c: honour CONFIG_SPL_SAVEENV

 env/Kconfig |  3 +++
 env/sf.c    | 12 +-----------
 2 files changed, 4 insertions(+), 11 deletions(-)

-- 
2.23.0