On 13/03/19 12:15, Heiko Schocher wrote: > This reverts commit 9a9d66f5eff0f443de4c2c6ca3e27771ed14b1b4. > > because it breaks fw_setenv and U-Boot interworking, if > U-Boot environment is stored in a SPI-NOR. > > Reproduce it with: > boot linux with empty Environment and store a variable > with fw_setenv into it, the Environment is now filled > with 0xff: > > root@ckey5e:10:8e:~# hexdump -C /dev/mtd4 > 00000000 e9 e8 07 fa 01 62 6f 6f 74 63 6d 64 3d 72 75 6e |.....bootcmd=run| > [...] > 00000f30 7d 00 75 62 69 62 6f 6f 74 76 6f 6c 3d 32 00 00 |}.ubibootvol=2..| > 00000f40 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................| > > Boot now U-Boot prints: > > Loading Environment from SPI Flash... SF: Detected s25fl128l with page size > 256 Bytes, erase size 4 KiB, total 16 MiB > *** Warning - bad CRC, using default environment > > Reason is the above commit, as it only reads until \0\0 > is found, and assumes the rest of the Environment > space is filled with 0x00, which is not the case when > saving an Environment under linux with fw_setenv. > > Signed-off-by: Heiko Schocher <h...@denx.de> > ---
Acked-by: Stefano Babic <sba...@denx.de> In fact, the commit breaks the API between U-Boot and Linux and optimizes a specific use case in U-Boot only. The API foresees that there is an environment of a specific size without bothering (in linux) where it is stored. It is strictly required that the environment has the same format for all underlying storage - it must be possible to simply copy the environment. Commit breaks tools like mkenvimage, too. Best regards, Stefano Babic > I already posted a fix: > https://lists.denx.de/pipermail/u-boot/2019-January/355148.html > which I withdrawed: > https://lists.denx.de/pipermail/u-boot/2019-January/355529.html > > because I did not recognised the correlation between fw_setenv > in linux context and saveenv in U-Boot context. > > I think now, best is to revert this commit. > > Comments? > > This patch drops checkpatch warnings: > env/sf.c:120: check: Alignment should match open parenthesis > env/sf.c:224: check: Alignment should match open parenthesis > env/sf.c:281: check: Alignment should match open parenthesis > > but as it is a revert I did not correct them. > > env/sf.c | 56 +++++++++++--------------------------------------------- > 1 file changed, 11 insertions(+), 45 deletions(-) > > diff --git a/env/sf.c b/env/sf.c > index b3dec82c35..23cbad5d88 100644 > --- a/env/sf.c > +++ b/env/sf.c > @@ -81,40 +81,6 @@ static int setup_flash_device(void) > return 0; > } > > -static int is_end(const char *addr, size_t size) > -{ > - /* The end of env variables is marked by '\0\0' */ > - int i = 0; > - > - for (i = 0; i < size - 1; ++i) > - if (addr[i] == 0x0 && addr[i + 1] == 0x0) > - return 1; > - return 0; > -} > - > -static int spi_flash_read_env(struct spi_flash *flash, u32 offset, size_t > len, > - void *buf) > -{ > - u32 addr = 0; > - u32 page_size = flash->page_size; > - > - memset(buf, 0x0, len); > - for (int i = 0; i < len / page_size; ++i) { > - int ret = spi_flash_read(flash, offset, page_size, > - &((char *)buf)[addr]); > - > - if (ret < 0) > - return ret; > - > - if (is_end(&((char *)buf)[addr], page_size)) > - return 0; > - > - addr += page_size; > - offset += page_size; > - } > - return 0; > -} > - > #if defined(CONFIG_ENV_OFFSET_REDUND) > #ifdef CMD_SAVEENV > static int env_sf_save(void) > @@ -150,8 +116,8 @@ static int env_sf_save(void) > ret = -ENOMEM; > goto done; > } > - ret = spi_flash_read_env(env_flash, saved_offset, > - saved_size, saved_buffer); > + ret = spi_flash_read(env_flash, saved_offset, > + saved_size, saved_buffer); > if (ret) > goto done; > } > @@ -217,10 +183,10 @@ static int env_sf_load(void) > if (ret) > goto out; > > - read1_fail = spi_flash_read_env(env_flash, CONFIG_ENV_OFFSET, > - CONFIG_ENV_SIZE, tmp_env1); > - read2_fail = spi_flash_read_env(env_flash, CONFIG_ENV_OFFSET_REDUND, > - CONFIG_ENV_SIZE, tmp_env2); > + read1_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, > + CONFIG_ENV_SIZE, tmp_env1); > + read2_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET_REDUND, > + CONFIG_ENV_SIZE, tmp_env2); > > ret = env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2, > read2_fail); > @@ -254,8 +220,8 @@ static int env_sf_save(void) > if (!saved_buffer) > goto done; > > - ret = spi_flash_read_env(env_flash, saved_offset, > - saved_size, saved_buffer); > + ret = spi_flash_read(env_flash, saved_offset, > + saved_size, saved_buffer); > if (ret) > goto done; > } > @@ -311,10 +277,10 @@ static int env_sf_load(void) > if (ret) > goto out; > > - ret = spi_flash_read_env(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, > - buf); > + ret = spi_flash_read(env_flash, > + CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf); > if (ret) { > - set_default_env("spi_flash_read_env() failed", 0); > + set_default_env("spi_flash_read() failed", 0); > goto err_read; > } > > -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot