[PATCH 3/5] env/fat.c: remove private CMD_SAVEENV logic

[Rasmus Villemoes]

On 19/02/2020 14.27, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20200219094726.26798-4-rasmus.villemoes at prevas.dk> you wrote:
>> Always compile the env_fat_save() function, and let
>> CONFIG_IS_ENABLED(SAVEENV) (via the ENV_SAVE_PTR macro) decide whether
>> it actually ends up being compiled in.
> 
> Have you tested that this works?  How do the sizes of the
> images differe before and after applying your changes?

With or without these patches, I get

$ size u-boot spl/u-boot-spl
   text    data     bss     dec     hex filename
 407173   45308   98352  550833   867b1 u-boot
  52403    3360     276   56039    dae7 spl/u-boot-spl
$ nm spl/u-boot-spl | grep env_fat
0090c5e8 t env_fat_load
$ nm u-boot | grep env_fat
17826cb4 t env_fat_load
17826c10 t env_fat_save

for a wandboard_defconfig modified by

-CONFIG_SPL_FS_EXT4=y
+CONFIG_SPL_FS_FAT=y
+CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_ENV_IS_IN_FAT=y

So in the "read-only environment access in SPL" case, everything is the
same before and after.

Now also enable CONFIG_SPL_SAVEENV and SPL_FAT_WRITE, then with my
patches we get

$ size u-boot spl/u-boot-spl
   text    data     bss     dec     hex filename
 407173   45308   98352  550833   867b1 u-boot
  58298    3360   65860  127518   1f21e spl/u-boot-spl
$ nm spl/u-boot-spl | grep env_fat
0090c6e0 t env_fat_load
0090c63c t env_fat_save

but without,

$ size u-boot spl/u-boot-spl
   text    data     bss     dec     hex filename
 407173   45308   98352  550833   867b1 u-boot
  52659    3360     280   56299    dbeb spl/u-boot-spl
$ nm spl/u-boot-spl | grep env_fat
0090c5e8 t env_fat_load

So without the fat.c patch, CONFIG_SPL_SAVEENV is effectively ignored.

> [Same question for ext4]

Actually, the situation for ext4 is even worse than indicated.

Just from reading code in ext4.c, env_ext4_save gets built into the SPL
if CONFIG_CMD_SAVEENV, whether or not CONFIG_SPL_SAVEENV is set. So I
expected my patch to simply reduce the spl image size in the
CONFIG_SPL_SAVEENV=n case. But one cannot compare - currently building with

CONFIG_CMD_SAVEENV=y
CONFIG_SPL_ENV_IS_IN_EXT4=y
CONFIG_SPL_SAVEENV=n

simply fails the SPL build because env_ext4_save refers to hexport_r,
which is only compiled if

!(defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SAVEENV))

- which took me a while to read, it's a little easier if spelled

!defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_SAVEENV)

Anyway, a side-effect of my ext4 patch is that the above combination
actually builds, because env_ext4_save is not linked in, so hexport_r
isn't needed. And turning on SPL_SAVEENV also works as expected.

Rasmus