[PATCH 0/2] Support skipping a certain number of good blocks

[PATCH 0/2] Support skipping a certain number of good blocks

[Mike Crowe]

I found myself needing to write part of a partition that will be read
by the bootloader by skipping blocks from the start of the partition.
These new command-line options let me write and verify.

Mike Crowe (2):
  nandwrite: Add --output-skip
  nanddump: Add --input-skip

 nand-utils/nanddump.c  | 30 +++++++++++++++++++++++++++---
 nand-utils/nandwrite.c | 20 ++++++++++++++++++--
 2 files changed, 45 insertions(+), 5 deletions(-)

-- 
2.1.4

[PATCH 1/2] nandwrite: Add --output-skip

[Mike Crowe]

Skip the specified number of bytes (which must be page aligned) before
writing. This differs from --start when there are bad blocks: --start
always seeks to the specified offset in the flash, --output-skip works its
way through the flash a page at a time from the specified start, skipping
bad blocks provided --noskipbad was not also passed.

This can be useful when writing part way through a partition that will be
read using a simple bad-block-skipping algorithm.
---
 nand-utils/nandwrite.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
index b376869..b4a6d8c 100644
--- a/nand-utils/nandwrite.c
+++ b/nand-utils/nandwrite.c
@@ -60,6 +60,7 @@ static void display_help(int status)
 "  -b, --blockalign=1|2|4  Set multiple of eraseblocks to align to\n"
 "      --input-skip=length Skip |length| bytes of the input file\n"
 "      --input-size=length Only read |length| bytes of the input file\n"
+"      --output-skip=length Skip |length| bytes before writing\n"
 "  -q, --quiet             Don't display progress messages\n"
 "  -h, --help              Display this help and exit\n"
 "  -V, --version           Output version information and exit\n"
@@ -87,6 +88,7 @@ static const char	*mtd_device, *img;
 static long long	mtdoffset = 0;
 static long long	inputskip = 0;
 static long long	inputsize = 0;
+static long long	outputskip = 0;
 static bool		quiet = false;
 static bool		writeoob = false;
 static bool		onlyoob = false;
@@ -110,6 +112,7 @@ static void process_options(int argc, char * const argv[])
 			{"version", no_argument, 0, 'V'},
 			{"input-skip", required_argument, 0, 0},
 			{"input-size", required_argument, 0, 0},
+			{"output-skip", required_argument, 0, 0},
 			{"help", no_argument, 0, 'h'},
 			{"blockalign", required_argument, 0, 'b'},
 			{"markbad", no_argument, 0, 'm'},
@@ -139,6 +142,9 @@ static void process_options(int argc, char * const argv[])
 			case 2: /* --input-size */
 				inputsize = simple_strtoll(optarg, &error);
 				break;
+			case 3: /* --output-skip */
+				outputskip = simple_strtoll(optarg, &error);
+				break;
 			}
 			break;
 		case 'V':
@@ -286,6 +292,11 @@ int main(int argc, char * const argv[])
 			   "The pagesize of this NAND Flash is 0x%x.\n",
 			   mtd.min_io_size);
 
+	if (outputskip % ebsize_aligned)
+		errmsg_die("The output skip length is not page-aligned !\n"
+			   "The pagesize of this NAND flash is 0x%x.\n",
+			   mtd.min_io_size);
+
 	/* Select OOB write mode */
 	if (noecc)
 		write_mode = MTD_OPS_RAW;
@@ -350,7 +361,7 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Check, if length fits into device */
-	if ((imglen / pagelen) * mtd.min_io_size > mtd.size - mtdoffset) {
+	if ((imglen / pagelen) * mtd.min_io_size > mtd.size - mtdoffset - outputskip) {
 		fprintf(stderr, "Image %lld bytes, NAND page %d bytes, OOB area %d"
 				" bytes, device size %lld bytes\n",
 				imglen, pagelen, mtd.oob_size, mtd.size);
@@ -400,7 +411,7 @@ int main(int argc, char * const argv[])
 			}
 
 			baderaseblock = false;
-			if (!quiet)
+			if (!quiet && (outputskip == 0))
 				fprintf(stdout, "Writing data to block %lld at offset 0x%llx\n",
 						 blockstart / ebsize_aligned, blockstart);
 
@@ -432,7 +443,12 @@ int main(int argc, char * const argv[])
 
 				offs +=  ebsize_aligned / blockalign;
 			} while (offs < blockstart + ebsize_aligned);
+		}
 
+		if (outputskip > 0) {
+			mtdoffset += mtd.min_io_size;
+			outputskip -= mtd.min_io_size;
+			continue;
 		}
 
 		/* Read more data from the input if there isn't enough in the buffer */
-- 
2.1.4

[PATCH 2/2] nanddump: Add --input-skip

[Mike Crowe]

Skip the specified number of bytes (which must be page aligned) before
dumping. This differs from --startaddress when there are bad blocks:
--startaddress always seeks to the specified offset in the partition,
--output-skip works its way through the partition a page at a time from the
specified start, skipping bad blocks as required by --bb.

This can be useful if other readers of the partition will be reading using
a simple bad-block-skipping algorithm.
---
 nand-utils/nanddump.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/nand-utils/nanddump.c b/nand-utils/nanddump.c
index a8ad334..bd0d420 100644
--- a/nand-utils/nanddump.c
+++ b/nand-utils/nanddump.c
@@ -46,6 +46,7 @@ static void display_help(int status)
 "-c         --canonicalprint     Print canonical Hex+ASCII dump\n"
 "-f file    --file=file          Dump to file\n"
 "-l length  --length=length      Length\n"
+"           --input-skip=length  Skip |length| bytes before dumping\n"
 "-n         --noecc              Read without error correction\n"
 "           --omitoob            Omit OOB data (default)\n"
 "-o         --oob                Dump OOB data\n"
@@ -81,6 +82,7 @@ static bool			noecc = false;		// don't error correct
 static bool			omitoob = true;		// omit oob data
 static long long		start_addr;		// start address
 static long long		length;			// dump length
+static long long		inputskip;              // skip bytes before dump
 static const char		*mtddev;		// mtd device name
 static const char		*dumpfile;		// dump file name
 static bool			quiet = false;		// suppress diagnostic output
@@ -105,6 +107,7 @@ static void process_options(int argc, char * const argv[])
 			{"version", no_argument, 0, 'V'},
 			{"bb", required_argument, 0, 0},
 			{"omitoob", no_argument, 0, 0},
+			{"input-skip", required_argument, 0, 0},
 			{"help", no_argument, 0, 'h'},
 			{"forcebinary", no_argument, 0, 'a'},
 			{"canonicalprint", no_argument, 0, 'c'},
@@ -146,6 +149,9 @@ static void process_options(int argc, char * const argv[])
 							errmsg_die("--oob and --oomitoob are mutually exclusive");
 						}
 						break;
+					case 3: /* --input-skip */
+						inputskip = simple_strtoll(optarg, &error);
+						break;
 				}
 				break;
 			case 'V':
@@ -404,13 +410,21 @@ int main(int argc, char * const argv[])
 
 	bs = mtd.min_io_size;
 
+	if (inputskip & (mtd.min_io_size - 1)) {
+		fprintf(stderr, "the input-skip parameter is not page-aligned!\n"
+			        "The pagesize of this NAND flash is 0x%x.\n",
+			        mtd.min_io_size);
+		goto closeall;
+	}
+
 	/* Print informative message */
 	if (!quiet) {
 		fprintf(stderr, "Block size %d, page size %d, OOB size %d\n",
 				mtd.eb_size, mtd.min_io_size, mtd.oob_size);
-		fprintf(stderr,
-				"Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
-				start_addr, end_addr);
+		if (!inputskip)
+			fprintf(stderr,
+				        "Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
+				        start_addr, end_addr);
 	}
 
 	/* Dump the flash contents */
@@ -463,6 +477,16 @@ int main(int argc, char * const argv[])
 			stat1 = stat2;
 		}
 
+		if (inputskip) {
+			inputskip -= bs;
+			end_addr += bs;
+			if (!inputskip && !quiet)
+				fprintf(stderr,
+					"Dumping data starting at 0x%08llx and ending at 0x%08llx...\n",
+					ofs + bs, end_addr);
+			continue;
+		}
+
 		/* Write out page data */
 		if (pretty_print) {
 			for (i = 0; i < bs; i += PRETTY_ROW_SIZE) {
-- 
2.1.4

Re: [PATCH 0/2] Support skipping a certain number of good blocks

[Mike Crowe]

On Tuesday 06 December 2016 at 18:04:15 +0000, Mike Crowe wrote:
> I found myself needing to write part of a partition that will be read
> by the bootloader by skipping blocks from the start of the partition.
> These new command-line options let me write and verify.

I now realise that I forgot to add Signed-off-by headers to these patches.
If they are found to be acceptable without further change then I will
resend them with the headers.

Mike.

Re: [PATCH 0/2] Support skipping a certain number of good blocks

[David Oberhollenzer]

Since both tools already have a --start[address] option to specify an offset,
wouldn't it make more sense for your case to add a flag that this offset should
not include bad blocks? IMO this would remove the ambiguity of having two
different offset options.

For both tools, instead of skipping bad blocks on the fly, it would IMO be cleaner
if you could add a simple check that iterates over the erase blocks ahead of time
and adjusts the offset if a block is bad. Then you wouldn't have to change the
later checks or add additional clutches to the already cluttered read/write loop.


David

Re: [PATCH 0/2] Support skipping a certain number of good blocks

[Mike Crowe]

On Wednesday 07 December 2016 at 09:53:49 +0100, David Oberhollenzer wrote:
> Since both tools already have a --start[address] option to specify an offset,
> wouldn't it make more sense for your case to add a flag that this offset should
> not include bad blocks? IMO this would remove the ambiguity of having two
> different offset options.

I agree that having two options isn't exactly clear. However, being able to
specify both a start address and a skip offset together might be useful if
the partition table is incorrect (this may occur during upgrades to a
bootloader version that uses a different partition table.)

But, if you'd prefer a separate option I can have a go at implementing
that.

> For both tools, instead of skipping bad blocks on the fly, it would IMO be cleaner
> if you could add a simple check that iterates over the erase blocks ahead of time
> and adjusts the offset if a block is bad. Then you wouldn't have to change the
> later checks or add additional clutches to the already cluttered read/write loop.

I tried to make my changes as minimal as possible, but you are correct that
it just makes the complex main loop even worse. I did initially try to scan
first but decided against that idea for reasons I've now forgotten. I'll
have another go at it.

Thanks for the review.

Mike.

Re: [PATCH 0/2] Support skipping a certain number of good blocks

[Mike Crowe]

On Wednesday 07 December 2016 at 09:56:50 +0000, Mike Crowe wrote:
> On Wednesday 07 December 2016 at 09:53:49 +0100, David Oberhollenzer wrote:
> > Since both tools already have a --start[address] option to specify an offset,
> > wouldn't it make more sense for your case to add a flag that this offset should
> > not include bad blocks? IMO this would remove the ambiguity of having two
> > different offset options.
> 
> I agree that having two options isn't exactly clear. However, being able to
> specify both a start address and a skip offset together might be useful if
> the partition table is incorrect (this may occur during upgrades to a
> bootloader version that uses a different partition table.)

I've implemented this for nandwrite and it seemed to be straightforward.
Unfortunately getopt_long requires my flag to be an int - I can change it
to bool and add code to set it explicitly if required. I was also unsure
which style to use for my variable names since the file uses a mixture of
runningwordstogether and underscore_separators.

If this solution is acceptable then I can add support to nanddump too.

Thanks.

Mike.

--8<--

Subject: [PATCH] nandwrite: Add --skip-bad-blocks-to-start option

The --skip-bad-blocks-to-start option will increase the seek offset by the
size of each bad block encountered between the start of the partition and
the specified start address.

This can be useful when writing part way through a partition that will be
read using a simple bad-block-skipping algorithm.
---
 nand-utils/nandwrite.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/nand-utils/nandwrite.c b/nand-utils/nandwrite.c
index 9602a6e..c8c7463 100644
--- a/nand-utils/nandwrite.c
+++ b/nand-utils/nandwrite.c
@@ -56,6 +56,8 @@ static void display_help(int status)
 "  -o, --oob               Input contains oob data\n"
 "  -O, --onlyoob           Input contains oob data and only write the oob part\n"
 "  -s addr, --start=addr   Set output start address (default is 0)\n"
+"  --skip-bad-blocks-to-start"
+"                          Skip bad blocks when seeking to the start address\n"
 "  -p, --pad               Pad writes to page size\n"
 "  -b, --blockalign=1|2|4  Set multiple of eraseblocks to align to\n"
 "      --input-skip=length Skip |length| bytes of the input file\n"
@@ -96,6 +98,7 @@ static bool		autoplace = false;
 static bool		skipallffs = false;
 static bool		noskipbad = false;
 static bool		pad = false;
+static int		skip_bad_blocks_to_start = false;
 static int		blockalign = 1; /* default to using actual block size */
 
 static void process_options(int argc, char * const argv[])
@@ -110,6 +113,7 @@ static void process_options(int argc, char * const argv[])
 			{"version", no_argument, 0, 'V'},
 			{"input-skip", required_argument, 0, 0},
 			{"input-size", required_argument, 0, 0},
+			{"skip-bad-blocks-to-start", no_argument, &skip_bad_blocks_to_start, 1},
 			{"help", no_argument, 0, 'h'},
 			{"blockalign", required_argument, 0, 'b'},
 			{"markbad", no_argument, 0, 'm'},
@@ -349,6 +353,25 @@ int main(int argc, char * const argv[])
 		goto closeall;
 	}
 
+	/* Skip bad blocks on the way to the start address if necessary */
+	if (skip_bad_blocks_to_start) {
+		unsigned long long bbs_offset = 0;
+		while (bbs_offset < mtdoffset) {
+			ret = mtd_is_bad(&mtd, fd, bbs_offset / ebsize_aligned);
+			if (ret < 0) {
+				sys_errmsg("%s: MTD get bad block failed", mtd_device);
+				goto closeall;
+			} else if (ret == 1) {
+				if (!quiet)
+					fprintf(stderr, "Bad block at %llx, %u block(s) "
+						"from %llx will be skipped\n",
+						bbs_offset, blockalign, bbs_offset);
+				mtdoffset += ebsize_aligned;
+			}
+			bbs_offset += ebsize_aligned;
+		}
+	}
+
 	/* Check, if length fits into device */
 	if ((imglen / pagelen) * mtd.min_io_size > mtd.size - mtdoffset) {
 		fprintf(stderr, "Image %lld bytes, NAND page %d bytes, OOB area %d"