From patchwork Thu Mar 26 23:01:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 244365 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Fri, 27 Mar 2020 00:01:58 +0100 Subject: [PATCH v2 0/2] allow CONFIG_SPL_SAVEENV to work with ENV_IS_IN_SPI_FLASH In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200326230200.12617-1-rasmus.villemoes@prevas.dk> 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(-) Reviewed-by: Wolfgang Denk