diff mbox

flashcp: improve speed & some clean ups

Message ID 1302188641-30514-1-git-send-email-fransmeulenbroeks@gmail.com
State Changes Requested, archived
Headers show

Commit Message

Frans Meulenbroeks April 7, 2011, 3:04 p.m. UTC
the speed of the program is improved by:
- checking if a block needs to be erased before actually erasing it
- only writing blocks that are not modified

In order to implement this, the main loop of the program has
been reworked. I have tried to stay in the style of the
original author

Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
---
 flashcp.c |  266 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 133 insertions(+), 133 deletions(-)

Comments

Leon Woestenberg April 7, 2011, 3:38 p.m. UTC | #1
Hello Frans,

On Thu, Apr 7, 2011 at 5:04 PM, Frans Meulenbroeks
<fransmeulenbroeks@gmail.com> wrote:
> +static int compare(unsigned char *src, unsigned char *dst, ssize_t len)
> +{
> +       int rv = 0;
> +       unsigned long long *sp = (unsigned long long *)src;
> +       unsigned long long *dp = (unsigned long long *)dst;
> +

Dangerous on platforms that do not support non-natural alignments (ARM is one).

Unless the alignment rules are enforced elsewhere?

Regards,

Leon.
Mike Frysinger April 7, 2011, 3:43 p.m. UTC | #2
On Thu, Apr 7, 2011 at 11:04, Frans Meulenbroeks wrote:
> the speed of the program is improved by:
> - checking if a block needs to be erased before actually erasing it

doesnt this make an assumption about the values of an unwritten flash
?  iirc, this came up in the past and we opted to not blindly make
that assumption.  probably best to put it behind a new command line
option that the user has to opt into.

> -static void showusage(bool error)
> +static void showusage (const char *progname,bool error)

your style throughout this patch is wrong.  this is lkml style, not
GNU.  for example:
 - no space between func and open paren
 - space after commas
 - braces after if/while/for/etc... need to be cuddled

> @@ -94,15 +87,15 @@ static void showusage(bool error)
>                        "\n"
>                        "Flash Copy - Written by Abraham van der Merwe <abraham@2d3d.co.za>\n"
>                        "\n"
> -                       "usage: %1$s [ -v | --verbose ] <filename> <device>\n"
> -                       "       %1$s -h | --help\n"
> +                       "usage: %s [ -v | --verbose ] <filename> <device>\n"
> +                       "       %s -h | --help\n"
>                        "\n"
>                        "   -h | --help      Show this help message\n"
>                        "   -v | --verbose   Show progress reports\n"
>                        "   <filename>       File which you want to copy to flash\n"
>                        "   <device>         Flash device to write to (e.g. /dev/mtd0, /dev/mtd1, etc.)\n"
>                        "\n",
> -                       PROGRAM_NAME);
> +                       progname,progname);

ignoring the style issue you introduced, i dont see how your change
from %1$s to %s is correct.  all you had to do was change PROGRAM_NAME
to progname.

i'll hold off on reviewing more until these fundamental issues are fixed.
-mike
Mike Frysinger April 7, 2011, 3:45 p.m. UTC | #3
On Thu, Apr 7, 2011 at 11:38, Leon Woestenberg wrote:
> On Thu, Apr 7, 2011 at 5:04 PM, Frans Meulenbroeks wrote:
>> +static int compare(unsigned char *src, unsigned char *dst, ssize_t len)
>> +{
>> +       int rv = 0;
>> +       unsigned long long *sp = (unsigned long long *)src;
>> +       unsigned long long *dp = (unsigned long long *)dst;
>> +
>
> Dangerous on platforms that do not support non-natural alignments (ARM is one).
>
> Unless the alignment rules are enforced elsewhere?

i dont think they are.  looking at this code though, seems like it
could be solved without needing to use 64/128 bit types and ugly
casts.  i feel like it could be done with a clever memcmp or memchr or
memmem.
-mike
Russ Dill April 7, 2011, 6:45 p.m. UTC | #4
On Thu, Apr 7, 2011 at 8:04 AM, Frans Meulenbroeks
<fransmeulenbroeks@gmail.com> wrote:
> the speed of the program is improved by:
> - checking if a block needs to be erased before actually erasing it
> - only writing blocks that are not modified
>

I would caution against treating all 0xff blocks as being erased. A
partially erased block will often return mostly 0xff, but some of the
bits are random. If they return 1, you'll think all is well when it is
not. This is why jffs2 has clean-markers. Secondarily, a partially
erased block that randomly returns a few bits that match you data to
be written would also be bad. In fact, no matter what stage power was
lost during an erase cycle, no matter how marginal any percentage of
the bits, your tool will smile and nod if it is writing all zeros.
Frans Meulenbroeks April 7, 2011, 7:56 p.m. UTC | #5
Dear Leon, Mike, Russ,

Thanks for your feedback.

The program was modified initially because it erased the whole
partition even if the image only used part of it.
While working on it I noticed some additional improvements and
implemented them. I've been using the program
without any problem for about a year, and figured I should give my
changed back to the community.
As the code has been modified quite a while ago, I do not recall the
rationale behind all of the changes.

Wrt the comments:

> Dangerous on platforms that do not support non-natural alignments (ARM is one).
>
> Unless the alignment rules are enforced elsewhere?

Good point. I haven't really considered this. It is probably not too
difficult to enforce these constraints.

> doesnt this make an assumption about the values of an unwritten flash
> ?  iirc, this came up in the past and we opted to not blindly make
> that assumption.  probably best to put it behind a new command line
> option that the user has to opt into.

Yes, it assumes unwritten flash is 1 and writing turns it from 1 to 0.
I can easily make an option to opt into erase-only-if-needed

> your style throughout this patch is wrong.  this is lkml style, not
> GNU.  for example:
>  - no space between func and open paren
>  - space after commas
>  - braces after if/while/for/etc... need to be cuddled

Hm, yes, I noticed that I have been sloppy here with spaces and
commas. Peeking back at the original source that also had those
problems at some places.
What style did you exactly want? Do you have a pointer? Are the above
examples of what is wrong, or rules how it should be.
I'm aware of lkml style

Wrt cuddled: English is not my native language. Do you mean you want
the opening brace on the same line as the if ?

> ignoring the style issue you introduced, i dont see how your change
> from %1$s to %s is correct.  all you had to do was change PROGRAM_NAME
> to progname.

The change is tested and correct (I used progname twice).
As it stands this change was already pending on my list to submit for
quite a while. I guess the %1$s was replaced by %s because the libc I
tested this with did not support the 1$ position specifier.
Btw I am not too sure about the status of the positional modifiers and
how standard it is. It definitely was not in c89/c90.
Anyway, I can retest and change back if desired.

> i dont think they are.  looking at this code though, seems like it
> could be solved without needing to use 64/128 bit types and ugly
> casts.  i feel like it could be done with a clever memcmp or memchr or
> memmem.

The rationale for rolling my own test is because I wanted to test two
things in one loop:
- whether the new data is different from the existing data (if not no
action is needed)
- if the new data can be written without an erase cycle (so overwrite
the data, again this is under the assumption that erased bits are 1
and writing will turn 1 to 0 as needed).
Also memcmp and friends do not allow me to make this second test (at
least not easily), it would be something like (!src | dst) == 0
Programming the loop as I did was benched to be the fastest solution.

Btw I feel the program (also in the current form is perhaps not too
suitable for NAND flash as it does not contain any bad block detection
or handling (unless ofc there is a translation layer on top of mtd).
(please enlighten me if I am missing something here).

> I would caution against treating all 0xff blocks as being erased. A
> partially erased block will often return mostly 0xff, but some of the
> bits are random. If they return 1, you'll think all is well when it is
> not. This is why jffs2 has clean-markers. Secondarily, a partially
> erased block that randomly returns a few bits that match you data to
> be written would also be bad. In fact, no matter what stage power was
> lost during an erase cycle, no matter how marginal any percentage of
> the bits, your tool will smile and nod if it is writing all zeros.

Hm yes.
I did not consider power loss in a previous erase cycle.
Actually I assumed that 1's and 0's would be pretty solid and not
dangling in mid-air.
Taking that into account it is definitely a must to put it behind an option.

Taking that into account is it still worthwhile/interesting to have this patch?
Or should I see if I can modify the program in such a way that it only
erases what is needed
(the change was triggered by having to copy a kernel image and/or
initrd and/or rootfs to a partition that was quite overdimensioned)

Thanks all for your feedback!
Frans
Mike Frysinger April 7, 2011, 8:06 p.m. UTC | #6
On Thu, Apr 7, 2011 at 15:56, Frans Meulenbroeks wrote:
>> your style throughout this patch is wrong.  this is lkml style, not
>> GNU.  for example:
>>  - no space between func and open paren
>>  - space after commas
>>  - braces after if/while/for/etc... need to be cuddled
>
> Hm, yes, I noticed that I have been sloppy here with spaces and
> commas. Peeking back at the original source that also had those
> problems at some places.
> What style did you exactly want? Do you have a pointer? Are the above
> examples of what is wrong, or rules how it should be.
> I'm aware of lkml style

yes, the mtd-utils code base can be a bit messy itself.  but when
adding new code, we should aspire for correctness.  which means please
use lkml style.

> Wrt cuddled: English is not my native language. Do you mean you want
> the opening brace on the same line as the if ?

correct

>> ignoring the style issue you introduced, i dont see how your change
>> from %1$s to %s is correct.  all you had to do was change PROGRAM_NAME
>> to progname.
>
> The change is tested and correct (I used progname twice).

sorry, when i said "correct", i did not mean that it would not work.
i'm sure your new version does.  i meant that the original version was
doing what was intended and it was not correct to change it.
certainly not without a pervasive argument.

> As it stands this change was already pending on my list to submit for
> quite a while. I guess the %1$s was replaced by %s because the libc I
> tested this with did not support the 1$ position specifier.
> Btw I am not too sure about the status of the positional modifiers and
> how standard it is. It definitely was not in c89/c90.

well, the C standard doesnt matter too much.  the POSIX standard however does.

if i look at the recent specs:
2004: http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
2008: http://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html

they are marked as XSI or CX, but in either case, in order to be
"POSIX compliant", your libc needs to support it.

i tend to leans towards "if it's in POSIX, it's fair game".  so you'll
need something more than "my libc doesnt like it" to sway me ;).

> Btw I feel the program (also in the current form is perhaps not too
> suitable for NAND flash as it does not contain any bad block detection
> or handling (unless ofc there is a translation layer on top of mtd).
> (please enlighten me if I am missing something here).

i think you're right.  this program is not geared towards flashes that
do bad block handling.  but i dont think that is a bad thing, nor
should you worry about it.
-mike
Artem Bityutskiy April 8, 2011, 1:30 p.m. UTC | #7
On Thu, 2011-04-07 at 17:04 +0200, Frans Meulenbroeks wrote:
> the speed of the program is improved by:
> - checking if a block needs to be erased before actually erasing it
> - only writing blocks that are not modified
> 
> In order to implement this, the main loop of the program has
> been reworked. I have tried to stay in the style of the
> original author
> 
> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>

Frans,

any work to improve mtd-utils is appreciated. But the changes should be
sent by small reviewable pieces. I know the feeling when you can't
resist the temptation to re-write whole piece of crap, but we should not
do this.

Please, send separate small patches which do one thing at a time. Do not
hesitate to do even small or massive clean-ups, as long as they are done
in one-type-per-patch. Feel free to change functionality, as long as you
split your work on several nice patches, which can be easily reviewed.
Normal way is to do preparation patches, and then functionality changes.

Thanks!
Russ Dill April 9, 2011, 1:22 a.m. UTC | #8
>> I would caution against treating all 0xff blocks as being erased. A
>> partially erased block will often return mostly 0xff, but some of the
>> bits are random. If they return 1, you'll think all is well when it is
>> not. This is why jffs2 has clean-markers. Secondarily, a partially
>> erased block that randomly returns a few bits that match you data to
>> be written would also be bad. In fact, no matter what stage power was
>> lost during an erase cycle, no matter how marginal any percentage of
>> the bits, your tool will smile and nod if it is writing all zeros.
>
> Hm yes.
> I did not consider power loss in a previous erase cycle.
> Actually I assumed that 1's and 0's would be pretty solid and not
> dangling in mid-air.
> Taking that into account it is definitely a must to put it behind an option.
>
> Taking that into account is it still worthwhile/interesting to have this patch?
> Or should I see if I can modify the program in such a way that it only
> erases what is needed
> (the change was triggered by having to copy a kernel image and/or
> initrd and/or rootfs to a partition that was quite overdimensioned)

I think if the data in flash matches the data you are about to write,
its ok to leave it, otherwise I'd erase and rewrite. There are a few
people on this list who have extensive experience with power failure
modes of NOR flash. Hopefully one of them can chime in. From what I
understand about the design of JFFS2 though, the only thing to watch
out for is an eraseblock that looks erased, but in reality isn't fully
erased. I think leaving in the part about checking for changes only
from 1->0 is a bit dangerous, if you leave it in, I'd put a big
warning on it.
Frans Meulenbroeks April 10, 2011, 11:05 a.m. UTC | #9
@Artem,

Thanks, I will try to do this in smaller chunks. Probably I'll come
with a patch to let the code comply with the coding guidelines first,
then step by step introduce the changes

@Russ,

Thanks for the info. I'll add a warning and make it opt-in.
That way if you know there was no power fail during the erase and you
know what you are doing you can safe some time.

Note that a new patch might take a little while as I am fairly busy
with severa things at the moment.

Best regards, Frans
diff mbox

Patch

diff --git a/flashcp.c b/flashcp.c
index d58c81b..d3bec5e 100644
--- a/flashcp.c
+++ b/flashcp.c
@@ -1,6 +1,7 @@ 
 /*
  * Copyright (c) 2d3D, Inc.
  * Written by Abraham vd Merwe <abraham@2d3d.co.za>
+ * Almost full rewrte of the main loop: Frans Meulenbroeks
  * All rights reserved.
  *
  * Renamed to flashcp.c to avoid conflicts with fcp from fsh package
@@ -47,9 +48,6 @@  typedef int bool;
 #define true 1
 #define false 0
 
-#define EXIT_FAILURE 1
-#define EXIT_SUCCESS 0
-
 /* for debugging purposes only */
 #ifdef DEBUG
 #undef DEBUG
@@ -62,15 +60,10 @@  typedef int bool;
 #define KB(x) ((x) / 1024)
 #define PERCENTAGE(x,total) (((x) * 100) / (total))
 
-/* size of read/write buffer */
-#define BUFSIZE (10 * 1024)
-
 /* cmd-line flags */
 #define FLAG_NONE		0x00
 #define FLAG_VERBOSE	0x01
 #define FLAG_HELP		0x02
-#define FLAG_FILENAME	0x04
-#define FLAG_DEVICE		0x08
 
 /* error levels */
 #define LOG_NORMAL	1
@@ -86,7 +79,7 @@  static void log_printf (int level,const char *fmt, ...)
 	fflush (fp);
 }
 
-static void showusage(bool error)
+static void showusage (const char *progname,bool error)
 {
 	int level = error ? LOG_ERROR : LOG_NORMAL;
 
@@ -94,15 +87,15 @@  static void showusage(bool error)
 			"\n"
 			"Flash Copy - Written by Abraham van der Merwe <abraham@2d3d.co.za>\n"
 			"\n"
-			"usage: %1$s [ -v | --verbose ] <filename> <device>\n"
-			"       %1$s -h | --help\n"
+			"usage: %s [ -v | --verbose ] <filename> <device>\n"
+			"       %s -h | --help\n"
 			"\n"
 			"   -h | --help      Show this help message\n"
 			"   -v | --verbose   Show progress reports\n"
 			"   <filename>       File which you want to copy to flash\n"
 			"   <device>         Flash device to write to (e.g. /dev/mtd0, /dev/mtd1, etc.)\n"
 			"\n",
-			PROGRAM_NAME);
+			progname,progname);
 
 	exit (error ? EXIT_FAILURE : EXIT_SUCCESS);
 }
@@ -146,11 +139,11 @@  static void safe_read (int fd,const char *filename,void *buf,size_t count,bool v
 	}
 }
 
-static void safe_rewind (int fd,const char *filename)
+static void safe_seek (int fd,off_t offset,int whence,const char *filename)
 {
-	if (lseek (fd,0L,SEEK_SET) < 0)
+	if (lseek (fd,offset,whence) < 0)
 	{
-		log_printf (LOG_ERROR,"While seeking to start of %s: %m\n",filename);
+		log_printf (LOG_ERROR,"While seeking with offset %ld (whence = %d) in %s: %m\n", (long)offset,whence,filename);
 		exit (EXIT_FAILURE);
 	}
 }
@@ -165,16 +158,62 @@  static void cleanup (void)
 	if (fil_fd > 0) close (fil_fd);
 }
 
+/*
+  The compare function below checks if we need to erase the block.
+  if writing the block only will turn bits from 1 to 0 we do not
+  need to erase.
+  Most likely this is because the block is already erased, but it
+  could be that we are e.g. just appending.
+  This is tested by doing a src AND dest and verifying that
+  this is indeed src.
+  And if we are writing data that is already present in that
+  form we do not need to write at all
+*/
+#define CMP_ERASE 1
+#define CMP_WRITE 2
+
+static int compare(unsigned char *src, unsigned char *dst, ssize_t len)
+{
+	int rv = 0;
+	unsigned long long *sp = (unsigned long long *)src;
+	unsigned long long *dp = (unsigned long long *)dst;
+	unsigned long long *ep; /* end ptr */
+	
+	len = len / sizeof(unsigned long long);
+	ep = sp + len;
+
+	while (sp < ep)
+	{
+		/* if src AND dst == src we do not need to erase */
+		if ((*sp & *dp) != *sp)
+		{
+			rv = CMP_ERASE | CMP_WRITE;
+			return rv;
+		}
+		if (*sp != *dp)
+		{
+			rv = CMP_WRITE;
+		}
+		sp++;
+		dp++;
+	}
+	return rv;
+}
+
 int main (int argc,char *argv[])
 {
-	const char *filename = NULL,*device = NULL;
+	const char *progname,*filename = NULL,*device = NULL;
 	int i,flags = FLAG_NONE;
 	ssize_t result;
-	size_t size,written;
+	size_t size,written,todo;
 	struct mtd_info_user mtd;
 	struct erase_info_user erase;
 	struct stat filestat;
-	unsigned char src[BUFSIZE],dest[BUFSIZE];
+	unsigned char *src, *dest;
+	int blocks;
+	int cmp;
+
+	(progname = strrchr (argv[0],'/')) ? progname++ : (progname = argv[0]);
 
 	/*********************
 	 * parse cmd-line
@@ -206,26 +245,28 @@  int main (int argc,char *argv[])
 				break;
 			default:
 				DEBUG("Unknown parameter: %s\n",argv[option_index]);
-				showusage(true);
+				showusage (progname,true);
 		}
 	}
+
+	if (flags & FLAG_HELP)
+		showusage (progname, false);
+
 	if (optind+2 == argc) {
-		flags |= FLAG_FILENAME;
 		filename = argv[optind];
 		DEBUG("Got filename: %s\n",filename);
 
-		flags |= FLAG_DEVICE;
 		device = argv[optind+1];
 		DEBUG("Got device: %s\n",device);
 	}
 
-	if (flags & FLAG_HELP || device == NULL)
-		showusage(flags != FLAG_HELP);
+	if (progname == NULL || device == NULL)
+		showusage (progname, true);
 
 	atexit (cleanup);
 
+	dev_fd = safe_open (device, O_SYNC | O_RDWR);
 	/* get some info about the flash device */
-	dev_fd = safe_open (device,O_SYNC | O_RDWR);
 	if (ioctl (dev_fd,MEMGETINFO,&mtd) < 0)
 	{
 		DEBUG("ioctl(): %m\n");
@@ -249,142 +290,101 @@  int main (int argc,char *argv[])
 	}
 
 	/*****************************************************
-	 * erase enough blocks so that we can write the file *
+	 * allocate buffers				     *
 	 *****************************************************/
 
+	src = malloc(2 * mtd.erasesize + sizeof(unsigned long long));
+	if (!src)
+	{
+		log_printf (LOG_ERROR,"not enough memory for two %d byte buffers!\n", mtd.erasesize);
+		exit (EXIT_FAILURE);
+	}
+	/* align on unsigned long long boundary */
+	src = (unsigned long long *)(((unsigned long)src + sizeof(unsigned long long)) & (~(sizeof(unsigned long long) - 1))); 
+	dest = src + mtd.erasesize;
+	size = filestat.st_size;
+
 #warning "Check for smaller erase regions"
 
 	erase.start = 0;
-	erase.length = (filestat.st_size + mtd.erasesize - 1) / mtd.erasesize;
-	erase.length *= mtd.erasesize;
+        erase.length = filestat.st_size & ~(mtd.erasesize - 1);
+        if (filestat.st_size % mtd.erasesize) erase.length += mtd.erasesize;
 
-	if (flags & FLAG_VERBOSE)
+	blocks = erase.length / mtd.erasesize;
+	erase.length = mtd.erasesize;
+
+	for (i = 1; i <= blocks; i++)
 	{
-		/* if the user wants verbose output, erase 1 block at a time and show him/her what's going on */
-		int blocks = erase.length / mtd.erasesize;
-		erase.length = mtd.erasesize;
-		log_printf (LOG_NORMAL,"Erasing blocks: 0/%d (0%%)",blocks);
-		for (i = 1; i <= blocks; i++)
+		if (size < mtd.erasesize)
+		{
+			todo = size;
+		}
+		else
+		{
+			todo = mtd.erasesize;
+		}
+		if (flags & FLAG_VERBOSE)
+		{
+			log_printf (LOG_NORMAL,"\rProcessing block: %d/%d (%d%%)",i,blocks,PERCENTAGE (i,blocks));
+		}
+		/* read from filename */
+		safe_read (fil_fd,filename,src,todo,flags & FLAG_VERBOSE);
+
+		/* read from device */
+		safe_read (dev_fd,device,dest,todo,flags & FLAG_VERBOSE);
+
+		cmp = compare(src,dest,todo);
+		if (cmp & CMP_ERASE)
 		{
-			log_printf (LOG_NORMAL,"\rErasing blocks: %d/%d (%d%%)",i,blocks,PERCENTAGE (i,blocks));
 			if (ioctl (dev_fd,MEMERASE,&erase) < 0)
 			{
-				log_printf (LOG_NORMAL,"\n");
+				if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"\n");
 				log_printf (LOG_ERROR,
 						"While erasing blocks 0x%.8x-0x%.8x on %s: %m\n",
 						(unsigned int) erase.start,(unsigned int) (erase.start + erase.length),device);
 				exit (EXIT_FAILURE);
 			}
-			erase.start += mtd.erasesize;
 		}
-		log_printf (LOG_NORMAL,"\rErasing blocks: %d/%d (100%%)\n",blocks,blocks);
-	}
-	else
-	{
-		/* if not, erase the whole chunk in one shot */
-		if (ioctl (dev_fd,MEMERASE,&erase) < 0)
+		if (cmp & CMP_WRITE)
 		{
-			log_printf (LOG_ERROR,
-					"While erasing blocks from 0x%.8x-0x%.8x on %s: %m\n",
-					(unsigned int) erase.start,(unsigned int) (erase.start + erase.length),device);
-			exit (EXIT_FAILURE);
-		}
-	}
-	DEBUG("Erased %u / %luk bytes\n",erase.length,filestat.st_size);
-
-	/**********************************
-	 * write the entire file to flash *
-	 **********************************/
-
-	if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"Writing data: 0k/%luk (0%%)",KB (filestat.st_size));
-	size = filestat.st_size;
-	i = BUFSIZE;
-	written = 0;
-	while (size)
-	{
-		if (size < BUFSIZE) i = size;
-		if (flags & FLAG_VERBOSE)
-			log_printf (LOG_NORMAL,"\rWriting data: %dk/%luk (%lu%%)",
-					KB (written + i),
-					KB (filestat.st_size),
-					PERCENTAGE (written + i,filestat.st_size));
-
-		/* read from filename */
-		safe_read (fil_fd,filename,src,i,flags & FLAG_VERBOSE);
+			safe_seek (dev_fd,(off_t)erase.start,SEEK_SET,device);
+		    	result = write (dev_fd,src,todo);
+			if (todo != result)
+			{
+				if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"\n");
+				if (result < 0)
+				{
+					log_printf (LOG_ERROR,
+							"While writing data to 0x%.8x-0x%.8x on %s: %m\n",
+							written,written + i,device);
+					exit (EXIT_FAILURE);
+				}
+				log_printf (LOG_ERROR,
+						"Short write count returned while writing to x%.8x-0x%.8x on %s: %d/%lu bytes written to flash\n",
+						erase.start,erase.start + todo,device,erase.start + result,filestat.st_size);
+				exit (EXIT_FAILURE);
+			}
+			/* verify */
+			safe_seek (dev_fd,(off_t)erase.start,SEEK_SET,device);
+			safe_read (dev_fd,device,dest,todo,flags & FLAG_VERBOSE);
 
-		/* write to device */
-		result = write (dev_fd,src,i);
-		if (i != result)
-		{
-			if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"\n");
-			if (result < 0)
+			/* compare buffers */
+			if (memcmp (src,dest,todo))
 			{
 				log_printf (LOG_ERROR,
-						"While writing data to 0x%.8x-0x%.8x on %s: %m\n",
-						written,written + i,device);
+						"File does not seem to match flash data. First mismatch at 0x%.8x-0x%.8x\n",
+						written,written + i);
 				exit (EXIT_FAILURE);
 			}
-			log_printf (LOG_ERROR,
-					"Short write count returned while writing to x%.8x-0x%.8x on %s: %d/%lu bytes written to flash\n",
-					written,written + i,device,written + result,filestat.st_size);
-			exit (EXIT_FAILURE);
 		}
-
-		written += i;
-		size -= i;
+		erase.start += mtd.erasesize;
+		size -= todo;
 	}
+
 	if (flags & FLAG_VERBOSE)
-		log_printf (LOG_NORMAL,
-				"\rWriting data: %luk/%luk (100%%)\n",
-				KB (filestat.st_size),
-				KB (filestat.st_size));
-	DEBUG("Wrote %d / %luk bytes\n",written,filestat.st_size);
-
-	/**********************************
-	 * verify that flash == file data *
-	 **********************************/
-
-	safe_rewind (fil_fd,filename);
-	safe_rewind (dev_fd,device);
-	size = filestat.st_size;
-	i = BUFSIZE;
-	written = 0;
-	if (flags & FLAG_VERBOSE) log_printf (LOG_NORMAL,"Verifying data: 0k/%luk (0%%)",KB (filestat.st_size));
-	while (size)
 	{
-		if (size < BUFSIZE) i = size;
-		if (flags & FLAG_VERBOSE)
-			log_printf (LOG_NORMAL,
-					"\rVerifying data: %dk/%luk (%lu%%)",
-					KB (written + i),
-					KB (filestat.st_size),
-					PERCENTAGE (written + i,filestat.st_size));
-
-		/* read from filename */
-		safe_read (fil_fd,filename,src,i,flags & FLAG_VERBOSE);
-
-		/* read from device */
-		safe_read (dev_fd,device,dest,i,flags & FLAG_VERBOSE);
-
-		/* compare buffers */
-		if (memcmp (src,dest,i))
-		{
-			log_printf (LOG_ERROR,
-					"File does not seem to match flash data. First mismatch at 0x%.8x-0x%.8x\n",
-					written,written + i);
-			exit (EXIT_FAILURE);
-		}
-
-		written += i;
-		size -= i;
+		log_printf (LOG_NORMAL,"\n");
 	}
-	if (flags & FLAG_VERBOSE)
-		log_printf (LOG_NORMAL,
-				"\rVerifying data: %luk/%luk (100%%)\n",
-				KB (filestat.st_size),
-				KB (filestat.st_size));
-	DEBUG("Verified %d / %luk bytes\n",written,filestat.st_size);
 
 	exit (EXIT_SUCCESS);
 }
-