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