[U-Boot] [PATCH 0/2] fw_env: fix bugs in fw_setenv when erase-block-size < env-size

Hi All,
I'm having a problem where fw_setenv fails writing the environment to a SPI NOR device with a 4K erase block size. This led me to two discoveries and two patches:
1) The "default" number of environment sectors is assumed to be one, even if the environment is larger than a sector.
2) fw_setenv has a bug calculating bytes written for NOR devices.
Patches to follow:
[PATCH 1/2] fw_env: calculate default number of env sectors [PATCH 2/2] fw_env: correct writes to devices with small erase blocks
Thanks,
--Dustin

The assumed number of environment sectors (always 1) leads to an incorrect top_of_range calculation in fw.env.c when a flash device has an erase block size smaller than the environment data size (number of environment sectors > 1).
This change updates the default number of environment sectors to at least cover the size of the environment.
Also corrected a false statement about the number of sectors column in fw_env.config.
Signed-off-by: Dustin Byford dustin@cumulusnetworks.com --- tools/env/fw_env.c | 14 ++++++++------ tools/env/fw_env.config | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 577ce2d..649db04 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1246,9 +1246,10 @@ static int parse_config () strcpy (DEVNAME (0), DEVICE1_NAME); DEVOFFSET (0) = DEVICE1_OFFSET; ENVSIZE (0) = ENV1_SIZE; - /* Default values are: erase-size=env-size, #sectors=1 */ + /* Default values are: erase-size=env-size */ DEVESIZE (0) = ENVSIZE (0); - ENVSECTORS (0) = 1; + /* #sectors=env-size/erase-size (rounded up) */ + ENVSECTORS (0) = (ENVSIZE(0) + DEVESIZE(0) - 1) / DEVESIZE(0); #ifdef DEVICE1_ESIZE DEVESIZE (0) = DEVICE1_ESIZE; #endif @@ -1260,9 +1261,10 @@ static int parse_config () strcpy (DEVNAME (1), DEVICE2_NAME); DEVOFFSET (1) = DEVICE2_OFFSET; ENVSIZE (1) = ENV2_SIZE; - /* Default values are: erase-size=env-size, #sectors=1 */ + /* Default values are: erase-size=env-size */ DEVESIZE (1) = ENVSIZE (1); - ENVSECTORS (1) = 1; + /* #sectors=env-size/erase-size (rounded up) */ + ENVSECTORS (1) = (ENVSIZE(1) + DEVESIZE(1) - 1) / DEVESIZE(1); #ifdef DEVICE2_ESIZE DEVESIZE (1) = DEVICE2_ESIZE; #endif @@ -1320,8 +1322,8 @@ static int get_config (char *fname) DEVESIZE(i) = ENVSIZE(i);
if (rc < 5) - /* Default - 1 sector */ - ENVSECTORS (i) = 1; + /* Assume enough env sectors to cover the environment */ + ENVSECTORS (i) = (ENVSIZE(i) + DEVESIZE(i) - 1) / DEVESIZE(i);
i++; } diff --git a/tools/env/fw_env.config b/tools/env/fw_env.config index 90e499d..c9b9f6a 100644 --- a/tools/env/fw_env.config +++ b/tools/env/fw_env.config @@ -1,7 +1,7 @@ # Configuration file for fw_(printenv/setenv) utility. # Up to two entries are valid, in this case the redundant # environment sector is assumed present. -# Notice, that the "Number of sectors" is ignored on NOR and SPI-dataflash. +# Notice, that the "Number of sectors" is not required on NOR and SPI-dataflash. # Futhermore, if the Flash sector size is ommitted, this value is assumed to # be the same as the Environment size, which is valid for NOR and SPI-dataflash

On Thu, Mar 06, 2014 at 08:48:22PM -0800, Dustin Byford wrote:
The assumed number of environment sectors (always 1) leads to an incorrect top_of_range calculation in fw.env.c when a flash device has an erase block size smaller than the environment data size (number of environment sectors > 1).
This change updates the default number of environment sectors to at least cover the size of the environment.
Also corrected a false statement about the number of sectors column in fw_env.config.
Signed-off-by: Dustin Byford dustin@cumulusnetworks.com
Applied to u-boot/master, thanks!

Some NOR flash devices have a small erase block size. For example, the Micron N25Q512 can erase in 4K blocks. These devices expose a bug in fw_env.c where flash_write_buf() incorrectly calculates bytes written and attempts to write past the environment sectors. Luckily, a range check prevents any real damage, but this does cause fw_setenv to fail with an error.
This change corrects the write length calculation.
The bug was introduced with commit 56086921 from 2008 and only affects configurations where the erase block size is smaller than the total environment data size.
Signed-off-by: Dustin Byford dustin@cumulusnetworks.com --- tools/env/fw_env.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 649db04..d228cc3 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -863,9 +863,9 @@ static int flash_write_buf (int dev, int fd, void *buf, size_t count, if (mtd_type != MTD_ABSENT) ioctl(fd, MEMLOCK, &erase);
- processed += blocklen; + processed += erasesize; block_seek = 0; - blockstart += blocklen; + blockstart += erasesize; }
if (write_total > count)

On Thu, Mar 06, 2014 at 08:48:23PM -0800, Dustin Byford wrote:
Some NOR flash devices have a small erase block size. For example, the Micron N25Q512 can erase in 4K blocks. These devices expose a bug in fw_env.c where flash_write_buf() incorrectly calculates bytes written and attempts to write past the environment sectors. Luckily, a range check prevents any real damage, but this does cause fw_setenv to fail with an error.
This change corrects the write length calculation.
The bug was introduced with commit 56086921 from 2008 and only affects configurations where the erase block size is smaller than the total environment data size.
Signed-off-by: Dustin Byford dustin@cumulusnetworks.com
Applied to u-boot/master, thanks!
participants (2)
-
Dustin Byford
-
Tom Rini