diff mbox

[U-Boot] tools/env: check flash length before probing

Message ID 1333603758-26498-1-git-send-email-vapier@gentoo.org
State Deferred
Delegated to: Joe Hershberger
Headers show

Commit Message

Mike Frysinger April 5, 2012, 5:29 a.m. UTC
If we attempt to probe beyond the end of flash, MEMGETBADBLOCK will fail
(as well it should), but we end up erroring out with the distracting:
	Cannot read bad block mark: Invalid argument

Instead of the correct error:
	Too few good blocks within range

Re-order the tests so we check for the end of the flash before probing
so we don't probe blocks that don't exist.

Reported-by: Mark Bishop <Mark.Bishop@cooperindustries.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/env/fw_env.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Comments

Bishop, Mark April 5, 2012, 1:15 p.m. UTC | #1
> -----Original Message-----
> From: u-boot-bounces@lists.denx.de [mailto:u-boot-
> bounces@lists.denx.de] On Behalf Of Mike Frysinger
> Sent: Thursday, April 05, 2012 1:29 AM
> To: u-boot@lists.denx.de
> Subject: [U-Boot] [PATCH] tools/env: check flash length before probing
> 
> If we attempt to probe beyond the end of flash, MEMGETBADBLOCK will
> fail
> (as well it should), but we end up erroring out with the distracting:
> 	Cannot read bad block mark: Invalid argument
> 
> Instead of the correct error:
> 	Too few good blocks within range
> 
> Re-order the tests so we check for the end of the flash before probing
> so we don't probe blocks that don't exist.
> 

Applied the patch:

root:/bin> fw_printenv
Too few good blocks within range

root:/bin> strace fw_printenv
ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS,
{B57600 opost isig icanon echo ...}) = 0
ioctl(1, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS,
{B57600 opost isig icanon echo ...}) = 0
open("/etc/fw_env.config", O_RDONLY)    = 3
ioctl(3, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS,
0x2d9ccb4) = -1 ENOTTY (Inappropriate ioctl for device)
mmap2(NULL, 4096, PROT_READ|PROT_WRITE,
MAP_SHARED|MAP_ANONYMOUS|0x4000000, 0, 0) = 0x2d55000
read(3, "# Configuration file for fw_(pri"..., 256) = 256
read(3, "Flash sector size\tNumber of sect"..., 256) = 163
read(3, "", 256)                        = 0
close(3)                                = 0
stat("/dev/mtd0", {st_mode=S_IFCHR|0660, st_rdev=makedev(90, 0), ...}) =
0
mmap2(NULL, 135168, PROT_READ|PROT_WRITE,
MAP_SHARED|MAP_ANONYMOUS|0x4000000, 0, 0) = 0x2e00000
open("/dev/mtd0", O_RDONLY)             = 3
ioctl(3, MEMGETINFO or MFB_SET_CHROMA_KEY, {type=MTD_NANDFLASH,
flags=MTD_WRITEABLE, size=0x80000, erasesize=0x20000, writesize=0x800,
oobsize=0x40, padding=0xffffffff}) = 0
ioctl(3, MEMGETBADBLOCK, [393216])      = 1
write(2, "Too few good blocks within range"..., 33Too few good blocks
within range
) = 33
close(3)                                = 0
_exit(1)                                = ?
root:/bin>
Mike Frysinger April 5, 2012, 5:44 p.m. UTC | #2
On Thursday 05 April 2012 09:15:12 Bishop, Mark wrote:
> From: Mike Frysinger
> > If we attempt to probe beyond the end of flash, MEMGETBADBLOCK will
> > fail
> > 
> > (as well it should), but we end up erroring out with the distracting:
> > 	Cannot read bad block mark: Invalid argument
> > 
> > Instead of the correct error:
> > 	Too few good blocks within range
> > 
> > Re-order the tests so we check for the end of the flash before probing
> > so we don't probe blocks that don't exist.
> 
> Applied the patch:
> 
> root:/bin> fw_printenv
> Too few good blocks within range

good -- that's what should be displayed.  i had to hack up my local printenv 
for testing against /dev/zero and had to guess at the results :).

> ioctl(3, MEMGETBADBLOCK, [393216])      = 1

this is what we need to root cause.  but let's do it in the existing thread.
-mike
Mike Frysinger July 21, 2012, 5:57 p.m. UTC | #3
On Thursday 05 April 2012 01:29:18 Mike Frysinger wrote:
> If we attempt to probe beyond the end of flash, MEMGETBADBLOCK will fail
> (as well it should), but we end up erroring out with the distracting:
> 	Cannot read bad block mark: Invalid argument
> 
> Instead of the correct error:
> 	Too few good blocks within range
> 
> Re-order the tests so we check for the end of the flash before probing
> so we don't probe blocks that don't exist.

this should be an easy one to pick up :)
-mike
diff mbox

Patch

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index e292d2b..d0fbbb0 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -712,10 +712,6 @@  static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 
 	/* This only runs once on NOR flash */
 	while (processed < count) {
-		rc = flash_bad_block (fd, mtd_type, &blockstart);
-		if (rc < 0)		/* block test failed */
-			return -1;
-
 		if (blockstart + block_seek + readlen > top_of_range) {
 			/* End of range is reached */
 			fprintf (stderr,
@@ -723,6 +719,10 @@  static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 			return -1;
 		}
 
+		rc = flash_bad_block (fd, mtd_type, &blockstart);
+		if (rc < 0)		/* block test failed */
+			return -1;
+
 		if (rc) {		/* block is bad */
 			blockstart += blocklen;
 			continue;
@@ -845,15 +845,15 @@  static int flash_write_buf (int dev, int fd, void *buf, size_t count,
 
 	/* This only runs once on NOR flash and SPI-dataflash */
 	while (processed < write_total) {
-		rc = flash_bad_block (fd, mtd_type, &blockstart);
-		if (rc < 0)		/* block test failed */
-			return rc;
-
 		if (blockstart + erasesize > top_of_range) {
 			fprintf (stderr, "End of range reached, aborting\n");
 			return -1;
 		}
 
+		rc = flash_bad_block (fd, mtd_type, &blockstart);
+		if (rc < 0)		/* block test failed */
+			return rc;
+
 		if (rc) {		/* block is bad */
 			blockstart += blocklen;
 			continue;