On 19/02/2020 14.27, Wolfgang Denk wrote:
Dear Rasmus,
In message 20200219094726.26798-4-rasmus.villemoes@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